Re: [Qemu-devel] [PATCH 03/28] ide-test: add test for werror=stop

[Markus Armbruster]

John Snow  writes:

> On 07/31/2014 06:58 AM, Stefan Hajnoczi wrote:
>> On Mon, Jul 07, 2014 at 02:17:44PM -0400, John Snow wrote:
>>> +static void test_retry_flush(void)
>>> +{
>>> +uint8_t data;
>>> +const char *s;
>>> +
>>> +prepare_blkdebug_script(debug_path, "flush_to_disk");
>>> +
>>> +ide_test_start(
>>> +"-vnc none "
>>> + "-drive
>>> file=blkdebug:%s:%s,if=ide,cache=writeback,rerror=stop,werror=stop",
>>> +debug_path, tmp_path);
>>> +
>>> +/* FLUSH CACHE command on device 0*/
>>> +outb(IDE_BASE + reg_device, 0);
>>> +outb(IDE_BASE + reg_command, CMD_FLUSH_CACHE);
>>> +
>>> +/* Check status while request is in flight*/
>>> +data = inb(IDE_BASE + reg_status);
>>> +assert_bit_set(data, BSY | DRDY);
>>> +assert_bit_clear(data, DF | ERR | DRQ);
>>> +
>>> +sleep(1);/* HACK: wait for event */
>>> +
>>> +/* Complete the command */
>>> +s = "{'execute':'cont' }";
>>> +while (!qmp(s)) {
>>> +s = "";
>>> +sleep(1);
>>> +}
>> I guess we're supposed to wait for the block I/O error event when the
>> machine stops.  Please implement that and replace this polling loop.
>>
>> See the STOP event in docs/qmp/qmp-events.txt.
>>
> Paolo: I edited in a bit to check for the STOP event instead of
> sleeping in V2, but what's the point of setting s = "" and sleeping
> and resending a blank string? Can't we just g_assert(qmp(s)) the first
> go around, provided the STOP event has already occurred?
>
> It works in practice, but I am curious.

I don't understand the need for sleep.  qmp() & friends wait for a
reply.

Sending "", i.e. nothing, is a crude way to just receive a reply.

Example: boot-order-test.c waits for an event without examining it:

qmp_discard_response("");   /* HACK: wait for event */

Your loop seems to try to consume replies (events?) until you get NULL,
which I guess happens when the other end closes the connection.

By the way, your loop leaks the responses.  Pretty harmless, but you may
want to QDECREF() just to be orderly.

Proper support for events in libqtest would be nice.

Re: [Qemu-devel] [PATCH 15/28] ide: stop PIO transfer on errors

[Paolo Bonzini]

Il 31/07/2014 14:23, Stefan Hajnoczi ha scritto:
> On Mon, Jul 07, 2014 at 02:17:56PM -0400, John Snow wrote:
>> From: Paolo Bonzini 
>>
>> This will provide a hook for sending the result of the command via the
>> FIS receive area.
>>
>> Signed-off-by: Paolo Bonzini 
>> Signed-off-by: John Snow 
>> ---
>>  hw/ide/core.c | 7 ---
>>  1 file changed, 4 insertions(+), 3 deletions(-)
>>
>> diff --git a/hw/ide/core.c b/hw/ide/core.c
>> index bd3bd34..d900ba0 100644
>> --- a/hw/ide/core.c
>> +++ b/hw/ide/core.c
>> @@ -422,6 +422,9 @@ static inline void ide_abort_command(IDEState *s)
>>  {
>>  s->status = READY_STAT | ERR_STAT;
>>  s->error = ABRT_ERR;
>> +if (s->end_transfer_func != ide_transfer_stop) {
>> +ide_transfer_stop(s);
>> +}
>>  }
> 
> I don't understand this.
> 
> ide_transfer_stop() sets s->send_transfer_func = ide_transfer_stop.
> This has the side-effect of making ide_is_pio_out() return true (not
> sure if that poses a problem).

ide_is_pio_out is only called if DRQ_STAT is set, and it is never set
after ide_transfer_stop returns, so that is not a problem.

> Why can't we call ide_transfer_stop() when s->end_transfer_func ==
> ide_transfer_stop?

The idea was to only call it for PIO commands (if s->end_transfer_func
== ide_transfer_stop, the PIO command cannot fail anymore), but we might
as well call it unconditionally like ide_dma_error does, i.e.

 {
+ide_transfer_stop(s);
 s->status = READY_STAT | ERR_STAT;
 s->error = ABRT_ERR;
 }

It's simpler indeed.

Paolo

Re: [Qemu-devel] [PATCH 14/15] hw/block/virtio-blk: create num_queues vqs if dataplane is enabled

[Ming Lei]

On Fri, Aug 1, 2014 at 2:10 PM, Paolo Bonzini  wrote:
> Il 01/08/2014 05:09, Ming Lei ha scritto:
 >> Per virtio spec, only the feature is set, the VM can be allowed to
 >> access the 'num_queues' field, and I didn't see any problem from
 >> VM's view point.
 >>
 >> So could you explain why both dataplane and non-dataplane have
 >> to support the feature.
>>> >
>>> > Because otherwise you change the guest ABI.
>> Sorry, I don't understand why the guest ABI is changed since
>> VM only parses 'num_queues' iff the feature is set, and the DATAPLANE
>> macro is determined during compiling.
>
> Even recompiling the same version of QEMU should not change the guest
> ABI.  Recompilation may affect which devices are present, but then the
> migration destination will not even start if something is broken.
>
> And as you pointed out, migration from dataplane to non-dataplane will
> break because you didn't convert callers of virtqueue_pop/push.

OK, I will convert non-dataplane to support multi virtqueues in V1,
and the conversion is not difficult and straightforward.

BTW, docs/migration.txt mentions that "QEMU has to be launched
with the same arguments the two times", so can I understand that
migration from dataplane to non-dataplane shouldn't have been
allowed?

Thanks,

Re: [Qemu-devel] [PATCH 0/7] icount migration and clock.

[Paolo Bonzini]

Il 01/08/2014 01:37, fred.kon...@greensocs.com ha scritto:
> From: KONRAD Frederic 
> 
> Those are some icount patches required for reverse execution.
> 
> It introduces an icount clock which is only growing with icount.
> It allows QEMU to migrate icount so virtual clock is kept if the VM is 
> migrated
> in icount mode (which is mandatory for migration based snapshot).
> 
> They are rebased on uq/master of
> git://git.kernel.org/pub/scm/virt/kvm/qemu-kvm.git
> 
> And can be cloned here:
> git://git.greensocs.com/qemu_cexe.git:cexe_1_3_v6_rebased
> 
> Thanks,
> Fred
> 
> KONRAD Frederic (7):
>   icount: put icount variables into TimerState.
>   migration: migrate icount fields.
>   migration: make qemu_savevm_state public.
>   icount: introduce icount timer.
>   icount: check for icount clock deadline when cpu loop exits.
>   icount: make icount extra computed on icount clock as well.
>   timer: add cpu_icount_to_ns function.
> 
>  cpus.c  | 95 
> ++---
>  include/qemu/timer.h| 10 +-
>  include/sysemu/sysemu.h |  1 
>  qemu-timer.c|  8 -
>  savevm.c|  2 +-
>  stubs/cpu-get-icount.c  |  2 +-
>  6 files changed, 94 insertions(+), 24 deletions(-)
> 

Thanks, I applied all patches and pushed everything to icount on
git://github.com/bonzini/qemu.git.  I've also dropped Sebastian's
"icount: Make icount_time_shift available everywhere" and adjusted his
stuff to use cpu_icount_to_ns instead.

I'm planning to send out patches 1/2/7 in my first 2.2 pull request,
together with Sebastian's host clock alignment feature.  The others will
have to wait for the reverse execution patches which actually use it.

Paolo

Re: [Qemu-devel] [PATCH 07/15] dataplane: use object pool to speed up allocation for virtio blk request

[Ming Lei]

On Thu, Jul 31, 2014 at 5:18 PM, Paolo Bonzini  wrote:
> Il 31/07/2014 05:22, Ming Lei ha scritto:
>>> >
>>> > The problem is that g_slice here is not using the slab-style allocator
>>> > because the object is larger than roughly 500 bytes.  One solution would
>>> > be to make virtqueue_pop/vring_pop allocate a VirtQueueElement of the
>>> > right size (and virtqueue_push/vring_push free it), as mentioned in the
>>> > review of patch 8.
>> Unluckily both iovec and addr array can't be fitted into 500 bytes, :-(
>> Not mention all users of VirtQueueElement need to be changed too,
>> I hate to make that work involved in this patchset, :-)
>
> Well, the point of dataplane was not just to get maximum iops.  It was
> also to provide guidance in the work necessary to improve the code and
> get maximum iops without special-casing everything.  This can be a lot
> of work indeed.
>
>>> >
>>> > However, I now remembered that VirtQueueElement is a mess because it's
>>> > serialized directly into the migration state. :(  So you basically
>>> > cannot change it without mucking with migration.  Please leave out patch
>>> > 8 for now.
>> save_device code serializes elem in this way:
>>
>>  qemu_put_buffer(f, (unsigned char *)&req->elem,
>> sizeof(VirtQueueElement));
>>
>> so I am wondering why this patch may break migration.
>
> Because you change the on-wire format and break migration from 2.1 to
> 2.2.  Sorry, I wasn't clear enough.

That is really a mess, but in future we still may convert VirtQueueElement
into a smart one, and keep the original structure only for save/load, but
a conversion between the two structures is required in save/load.


Thanks,

Re: [Qemu-devel] [PATCH 14/15] hw/block/virtio-blk: create num_queues vqs if dataplane is enabled

[Paolo Bonzini]

Il 01/08/2014 09:35, Ming Lei ha scritto:
> OK, I will convert non-dataplane to support multi virtqueues in V1,
> and the conversion is not difficult and straightforward.
> 
> BTW, docs/migration.txt mentions that "QEMU has to be launched
> with the same arguments the two times", so can I understand that
> migration from dataplane to non-dataplane shouldn't have been
> allowed?

You're right, that text gives a rule of thumb but it is not absolutely true.

It is for example obviously okay to have different paths for drives in
the two invocations.  It is also okay to use different formats (raw vs.
qcow2) if, for example, you are migrating disk contents (either with
"migrate -b" or with the embedded NBD server).

Dataplane is another such case where an option does not affect how the
device behaves.  It is a bit more clearer if you think of it as just
"use an iothread different from the default one", which is what we're
aiming at.  Everything else is working around limitations in QEMU, and
will disappear once these limitations are lifted.

Paolo

[Qemu-devel] [PATCH v3 7/8] vl: don't use 'Yoda conditions'

[arei.gonglei]

From: Gonglei 

imitate nearby code about using '!value' or 'value == NULL'

Signed-off-by: Gonglei 
---
 vl.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/vl.c b/vl.c
index fe451aa..04c5abd 100644
--- a/vl.c
+++ b/vl.c
@@ -1136,7 +1136,7 @@ static int drive_init_func(QemuOpts *opts, void *opaque)
 
 static int drive_enable_snapshot(QemuOpts *opts, void *opaque)
 {
-if (NULL == qemu_opt_get(opts, "snapshot")) {
+if (qemu_opt_get(opts, "snapshot") == NULL) {
 qemu_opt_set(opts, "snapshot", "on");
 }
 return 0;
@@ -2488,8 +2488,9 @@ static int foreach_device_config(int type, int 
(*func)(const char *cmdline))
 loc_push_restore(&conf->loc);
 rc = func(conf->cmdline);
 loc_pop(&conf->loc);
-if (0 != rc)
+if (rc) {
 return rc;
+}
 }
 return 0;
 }
-- 
1.7.12.4

[Qemu-devel] [PATCH v3 3/8] audio: don't use 'Yoda conditions'

[arei.gonglei]

From: Gonglei 

imitate nearby code about using '!value' or 'value == NULL'

Signed-off-by: Gonglei 
---
 hw/audio/gus.c   | 2 +-
 hw/audio/hda-codec.c | 3 ++-
 hw/audio/sb16.c  | 6 +++---
 3 files changed, 6 insertions(+), 5 deletions(-)

diff --git a/hw/audio/gus.c b/hw/audio/gus.c
index bba6840..4a43ce7 100644
--- a/hw/audio/gus.c
+++ b/hw/audio/gus.c
@@ -212,7 +212,7 @@ static int GUS_read_DMA (void *opaque, int nchan, int 
dma_pos, int dma_len)
 pos += copied;
 }
 
-if (0 == ((mode >> 4) & 1)) {
+if (((mode >> 4) & 1) == 0) {
 DMA_release_DREQ (s->emu.gusdma);
 }
 return dma_len;
diff --git a/hw/audio/hda-codec.c b/hw/audio/hda-codec.c
index cbcf521..3c03ff5 100644
--- a/hw/audio/hda-codec.c
+++ b/hw/audio/hda-codec.c
@@ -489,8 +489,9 @@ static int hda_audio_init(HDACodecDevice *hda, const struct 
desc_codec *desc)
 for (i = 0; i < a->desc->nnodes; i++) {
 node = a->desc->nodes + i;
 param = hda_codec_find_param(node, AC_PAR_AUDIO_WIDGET_CAP);
-if (NULL == param)
+if (param == NULL) {
 continue;
+}
 type = (param->val & AC_WCAP_TYPE) >> AC_WCAP_TYPE_SHIFT;
 switch (type) {
 case AC_WID_AUD_OUT:
diff --git a/hw/audio/sb16.c b/hw/audio/sb16.c
index 60c4b3b..bda26d0 100644
--- a/hw/audio/sb16.c
+++ b/hw/audio/sb16.c
@@ -928,7 +928,7 @@ static IO_WRITE_PROTO (dsp_write)
 /* if (s->highspeed) */
 /* break; */
 
-if (0 == s->needed_bytes) {
+if (s->needed_bytes == 0) {
 command (s, val);
 #if 0
 if (0 == s->needed_bytes) {
@@ -1212,7 +1212,7 @@ static int SB_read_DMA (void *opaque, int nchan, int 
dma_pos, int dma_len)
 #endif
 
 if (till <= copy) {
-if (0 == s->dma_auto) {
+if (s->dma_auto == 0) {
 copy = till;
 }
 }
@@ -1224,7 +1224,7 @@ static int SB_read_DMA (void *opaque, int nchan, int 
dma_pos, int dma_len)
 if (s->left_till_irq <= 0) {
 s->mixer_regs[0x82] |= (nchan & 4) ? 2 : 1;
 qemu_irq_raise (s->pic);
-if (0 == s->dma_auto) {
+if (s->dma_auto == 0) {
 control (s, 0);
 speaker (s, 0);
 }
-- 
1.7.12.4

[Qemu-devel] [PATCH v3 for-2.2 0/8] don't use Yoda conditions

[arei.gonglei]

From: Gonglei 

$WHATEVER: don't use 'Yoda conditions'

'Yoda conditions' are not part of idiomatic QEMU coding
style, so rewrite them in the more usual order.

v3:
 - rewrite CODINT_STYLE file suggested by Eric, thanks.
 - rename the patch serials.
 - imitate nearby code about using '!value' or 'value == NULL' at
   every patch suggested by Markus.

v2:
 - add more specific commit messages suggested by PMM, thanks.
 - introduce section of conditional statement to CODING_STYLE.


Gonglei (8):
  CODING_STYLE: Section about conditional statement
  usb: don't use 'Yoda conditions'
  audio: don't use 'Yoda conditions'
  isa-bus: don't use 'Yoda conditions'
  don't use 'Yoda conditions'
  spice: don't use 'Yoda conditions'
  vl: don't use 'Yoda conditions'
  vmxnet3: don't use 'Yoda conditions'

 CODING_STYLE | 14 ++
 hw/audio/gus.c   |  2 +-
 hw/audio/hda-codec.c |  3 ++-
 hw/audio/sb16.c  |  6 +++---
 hw/isa/isa-bus.c |  2 +-
 hw/net/vmxnet3.c | 16 
 hw/usb/dev-audio.c   |  2 +-
 hw/usb/dev-mtp.c |  4 ++--
 hw/usb/hcd-ehci.c|  2 +-
 qdev-monitor.c   |  2 +-
 qemu-char.c  |  2 +-
 ui/spice-core.c  |  4 ++--
 util/qemu-sockets.c  |  2 +-
 vl.c |  5 +++--
 14 files changed, 41 insertions(+), 25 deletions(-)

-- 
1.7.12.4

[Qemu-devel] [PATCH v3 6/8] spice: don't use 'Yoda conditions'

[arei.gonglei]

From: Gonglei 

imitate nearby code about using '!value' or 'value == NULL'

Signed-off-by: Gonglei 
---
 ui/spice-core.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/ui/spice-core.c b/ui/spice-core.c
index 7bb91e6..1a2fb4b 100644
--- a/ui/spice-core.c
+++ b/ui/spice-core.c
@@ -677,7 +677,7 @@ void qemu_spice_init(void)
 
 if (tls_port) {
 x509_dir = qemu_opt_get(opts, "x509-dir");
-if (NULL == x509_dir) {
+if (!x509_dir) {
 x509_dir = ".";
 }
 
@@ -803,7 +803,7 @@ void qemu_spice_init(void)
 
 seamless_migration = qemu_opt_get_bool(opts, "seamless-migration", 0);
 spice_server_set_seamless_migration(spice_server, seamless_migration);
-if (0 != spice_server_init(spice_server, &core_interface)) {
+if (spice_server_init(spice_server, &core_interface) != 0) {
 error_report("failed to initialize spice server");
 exit(1);
 };
-- 
1.7.12.4

[Qemu-devel] [PATCH v3 5/8] don't use 'Yoda conditions'

[arei.gonglei]

From: Gonglei 

imitate nearby code about using '!value' or 'value == NULL'

Signed-off-by: Gonglei 
---
 qdev-monitor.c  | 2 +-
 qemu-char.c | 2 +-
 util/qemu-sockets.c | 2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/qdev-monitor.c b/qdev-monitor.c
index f87f3d8..81a4e9b 100644
--- a/qdev-monitor.c
+++ b/qdev-monitor.c
@@ -694,7 +694,7 @@ void qmp_device_del(const char *id, Error **errp)
 DeviceState *dev;
 
 dev = qdev_find_recursive(sysbus_get_default(), id);
-if (NULL == dev) {
+if (!dev) {
 error_set(errp, QERR_DEVICE_NOT_FOUND, id);
 return;
 }
diff --git a/qemu-char.c b/qemu-char.c
index 956be49..70d5a64 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -4117,7 +4117,7 @@ void qmp_chardev_remove(const char *id, Error **errp)
 CharDriverState *chr;
 
 chr = qemu_chr_find(id);
-if (NULL == chr) {
+if (chr == NULL) {
 error_setg(errp, "Chardev '%s' not found", id);
 return;
 }
diff --git a/util/qemu-sockets.c b/util/qemu-sockets.c
index 74cf078..5d38395 100644
--- a/util/qemu-sockets.c
+++ b/util/qemu-sockets.c
@@ -732,7 +732,7 @@ int unix_connect_opts(QemuOpts *opts, Error **errp,
 ConnectState *connect_state = NULL;
 int sock, rc;
 
-if (NULL == path) {
+if (path == NULL) {
 error_setg(errp, "unix connect: no path specified");
 return -1;
 }
-- 
1.7.12.4

[Qemu-devel] [PATCH v3 2/8] usb: don't use 'Yoda conditions'

[arei.gonglei]

From: Gonglei 

imitate nearby code about using '!value' or 'value == NULL'

Signed-off-by: Gonglei 
---
 hw/usb/dev-audio.c | 2 +-
 hw/usb/dev-mtp.c   | 4 ++--
 hw/usb/hcd-ehci.c  | 2 +-
 3 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/hw/usb/dev-audio.c b/hw/usb/dev-audio.c
index bfebfe9..7b9957b 100644
--- a/hw/usb/dev-audio.c
+++ b/hw/usb/dev-audio.c
@@ -371,7 +371,7 @@ static void output_callback(void *opaque, int avail)
 return;
 }
 data = streambuf_get(&s->out.buf);
-if (NULL == data) {
+if (!data) {
 return;
 }
 AUD_write(s->out.voice, data, USBAUDIO_PACKET_SIZE);
diff --git a/hw/usb/dev-mtp.c b/hw/usb/dev-mtp.c
index 384d4a5..0820046 100644
--- a/hw/usb/dev-mtp.c
+++ b/hw/usb/dev-mtp.c
@@ -832,7 +832,7 @@ static void usb_mtp_command(MTPState *s, MTPControl *c)
 return;
 }
 data_in = usb_mtp_get_object(s, c, o);
-if (NULL == data_in) {
+if (data_in == NULL) {
 usb_mtp_queue_result(s, RES_GENERAL_ERROR,
  c->trans, 0, 0, 0);
 return;
@@ -851,7 +851,7 @@ static void usb_mtp_command(MTPState *s, MTPControl *c)
 return;
 }
 data_in = usb_mtp_get_partial_object(s, c, o);
-if (NULL == data_in) {
+if (data_in == NULL) {
 usb_mtp_queue_result(s, RES_GENERAL_ERROR,
  c->trans, 0, 0, 0);
 return;
diff --git a/hw/usb/hcd-ehci.c b/hw/usb/hcd-ehci.c
index a00a93c..448e007 100644
--- a/hw/usb/hcd-ehci.c
+++ b/hw/usb/hcd-ehci.c
@@ -1596,7 +1596,7 @@ static EHCIQueue *ehci_state_fetchqh(EHCIState *ehci, int 
async)
 
 entry = ehci_get_fetch_addr(ehci, async);
 q = ehci_find_queue_by_qh(ehci, entry, async);
-if (NULL == q) {
+if (q == NULL) {
 q = ehci_alloc_queue(ehci, entry, async);
 }
 
-- 
1.7.12.4

[Qemu-devel] [PATCH v3 8/8] vmxnet3: don't use 'Yoda conditions'

[arei.gonglei]

From: Gonglei 

imitate nearby code about using '!value' or 'value == NULL'

Signed-off-by: Gonglei 
---
 hw/net/vmxnet3.c | 16 
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/hw/net/vmxnet3.c b/hw/net/vmxnet3.c
index 77bea6f..588149d 100644
--- a/hw/net/vmxnet3.c
+++ b/hw/net/vmxnet3.c
@@ -1009,7 +1009,7 @@ vmxnet3_indicate_packet(VMXNET3State *s)
 
 vmxnet3_dump_rx_descr(&rxd);
 
-if (0 != ready_rxcd_pa) {
+if (ready_rxcd_pa != 0) {
 cpu_physical_memory_write(ready_rxcd_pa, &rxcd, sizeof(rxcd));
 }
 
@@ -1020,7 +1020,7 @@ vmxnet3_indicate_packet(VMXNET3State *s)
 rxcd.gen = new_rxcd_gen;
 rxcd.rqID = RXQ_IDX + rx_ridx * s->rxq_num;
 
-if (0 == bytes_left) {
+if (bytes_left == 0) {
 vmxnet3_rx_update_descr(s->rx_pkt, &rxcd);
 }
 
@@ -1038,16 +1038,16 @@ vmxnet3_indicate_packet(VMXNET3State *s)
 num_frags++;
 }
 
-if (0 != ready_rxcd_pa) {
+if (ready_rxcd_pa != 0) {
 rxcd.eop = 1;
-rxcd.err = (0 != bytes_left);
+rxcd.err = (bytes_left != 0);
 cpu_physical_memory_write(ready_rxcd_pa, &rxcd, sizeof(rxcd));
 
 /* Flush RX descriptor changes */
 smp_wmb();
 }
 
-if (0 != new_rxcd_pa) {
+if (new_rxcd_pa != 0) {
 vmxnet3_revert_rxc_descr(s, RXQ_IDX);
 }
 
@@ -1190,8 +1190,8 @@ static void vmxnet3_update_mcast_filters(VMXNET3State *s)
 s->mcast_list_len = list_bytes / sizeof(s->mcast_list[0]);
 
 s->mcast_list = g_realloc(s->mcast_list, list_bytes);
-if (NULL == s->mcast_list) {
-if (0 == s->mcast_list_len) {
+if (!s->mcast_list) {
+if (s->mcast_list_len == 0) {
 VMW_CFPRN("Current multicast list is empty");
 } else {
 VMW_ERPRN("Failed to allocate multicast list of %d elements",
@@ -1667,7 +1667,7 @@ vmxnet3_io_bar1_write(void *opaque,
  * memory address. We save it to temp variable and set the
  * shared address only after we get the high part
  */
-if (0 == val) {
+if (val == 0) {
 s->device_active = false;
 }
 s->temp_shared_guest_driver_memory = val;
-- 
1.7.12.4

[Qemu-devel] [PATCH v3 4/8] isa-bus: don't use 'Yoda conditions'

[arei.gonglei]

From: Gonglei 

imitate nearby code about using '!value' or 'value == NULL'

Signed-off-by: Gonglei 
---
 hw/isa/isa-bus.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/isa/isa-bus.c b/hw/isa/isa-bus.c
index b28981b..cc85e53 100644
--- a/hw/isa/isa-bus.c
+++ b/hw/isa/isa-bus.c
@@ -50,7 +50,7 @@ ISABus *isa_bus_new(DeviceState *dev, MemoryRegion 
*address_space_io)
 fprintf(stderr, "Can't create a second ISA bus\n");
 return NULL;
 }
-if (NULL == dev) {
+if (!dev) {
 dev = qdev_create(NULL, "isabus-bridge");
 qdev_init_nofail(dev);
 }
-- 
1.7.12.4

[Qemu-devel] [PATCH v3 1/8] CODING_STYLE: Section about conditional statement

[arei.gonglei]

From: Gonglei 

Yoda conditions lack readability, and QEMU has a
strict compiler configuration for checking a common
mistake like "if (dev = NULL)". Make it a written rule.

Signed-off-by: Gonglei 
---
 CODING_STYLE | 14 ++
 1 file changed, 14 insertions(+)

diff --git a/CODING_STYLE b/CODING_STYLE
index 4280945..b08bfb4 100644
--- a/CODING_STYLE
+++ b/CODING_STYLE
@@ -91,3 +91,17 @@ Mixed declarations (interleaving statements and declarations 
within blocks)
 are not allowed; declarations should be at the beginning of blocks.  In other
 words, the code should not generate warnings if using GCC's
 -Wdeclaration-after-statement option.
+
+6. Conditional statements
+
+When comparing a variable for (in)equality with a constant, list the
+constant on the right, as in:
+
+if (a == 0) {
+/* Reads like: "If a is equal to 0" */
+do_something();
+}
+
+Rationale: Yoda conditions (as in 'if (0 == a)') are awkward to read.
+Besides, good compilers already warn users when '==' is mis-typed as '=',
+even when the constant is on the right.
-- 
1.7.12.4

Re: [Qemu-devel] [PATCH v2 1/3] trace: teach lttng backend to use format strings

[Alex Bennée]

Alex Bennée writes:

> This makes the UST backend pay attention to the format string arguments
> that are defined when defining payload data. With this you can now
> ensure integers are reported in hex mode if you want.
>


Ping Stefan, can this one at least be slurped up into your tracing tree?

-- 
Alex Bennée

[Qemu-devel] [PATCH v3 0/9] Virtio PCI libqos driver

[Marc Marí]

Add functions for virtio PCI libqos driver. Add more debugging tools. Solve
bugs found while generating tests.

v3: Solved problems, added indirect descriptor support and test for
configuration changes

Marc Marí (9):
  tests: Functions bus_foreach and device_find from libqos virtio API
  tests: Add virtio device initialization
  libqtest: add QTEST_LOG for debugging qtest testcases
  libqos: Correct mask to align size to PAGE_SIZE in malloc-pc
  libqos: Change free function called in malloc
  virtio-blk: Correct bug in support for flexible descriptor layout
  libqos: Added basic virtqueue support to virtio implementation
  libqos: Added indirect descriptor support to virtio implementation
  libqos: Added test case for configuration changes in virtio-blk test

 hw/block/virtio-blk.c |   14 +-
 tests/Makefile|3 +-
 tests/libqos/malloc-pc.c  |2 +-
 tests/libqos/malloc.h |2 +-
 tests/libqos/virtio-pci.c |  219 
 tests/libqos/virtio-pci.h |   49 +++
 tests/libqos/virtio.c |  206 ++
 tests/libqos/virtio.h |  152 +++
 tests/libqtest.c  |7 +-
 tests/virtio-blk-test.c   |  355 -
 10 files changed, 990 insertions(+), 19 deletions(-)
 create mode 100644 tests/libqos/virtio-pci.c
 create mode 100644 tests/libqos/virtio-pci.h
 create mode 100644 tests/libqos/virtio.c
 create mode 100644 tests/libqos/virtio.h

-- 
1.7.10.4

[Qemu-devel] [PATCH v3 5/9] libqos: Change free function called in malloc

[Marc Marí]

Reviewed-by: John Snow 
Reviewed-by: Stefan Hajnoczi 
Signed-off-by: Marc Marí 
---
 tests/libqos/malloc.h |2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/libqos/malloc.h b/tests/libqos/malloc.h
index 46f6000..5565381 100644
--- a/tests/libqos/malloc.h
+++ b/tests/libqos/malloc.h
@@ -32,7 +32,7 @@ static inline uint64_t guest_alloc(QGuestAllocator 
*allocator, size_t size)
 
 static inline void guest_free(QGuestAllocator *allocator, uint64_t addr)
 {
-allocator->alloc(allocator, addr);
+allocator->free(allocator, addr);
 }
 
 #endif
-- 
1.7.10.4

[Qemu-devel] [PATCH v3 1/9] tests: Functions bus_foreach and device_find from libqos virtio API

[Marc Marí]

Virtio header has been changed to compile and work with a real device.
Functions bus_foreach and device_find have been implemented for PCI.
Virtio-blk test case now opens a fake device.

Signed-off-by: Marc Marí 
---
 tests/Makefile|3 +-
 tests/libqos/virtio-pci.c |   75 +
 tests/libqos/virtio-pci.h |   24 +++
 tests/libqos/virtio.h |   23 ++
 tests/virtio-blk-test.c   |   61 +++-
 5 files changed, 177 insertions(+), 9 deletions(-)
 create mode 100644 tests/libqos/virtio-pci.c
 create mode 100644 tests/libqos/virtio-pci.h
 create mode 100644 tests/libqos/virtio.h

diff --git a/tests/Makefile b/tests/Makefile
index 4b2e1bb..7c0f670 100644
--- a/tests/Makefile
+++ b/tests/Makefile
@@ -294,6 +294,7 @@ libqos-obj-y += tests/libqos/i2c.o
 libqos-pc-obj-y = $(libqos-obj-y) tests/libqos/pci-pc.o
 libqos-pc-obj-y += tests/libqos/malloc-pc.o
 libqos-omap-obj-y = $(libqos-obj-y) tests/libqos/i2c-omap.o
+libqos-virtio-obj-y = $(libqos-obj-y) $(libqos-pc-obj-y) 
tests/libqos/virtio-pci.o
 
 tests/rtc-test$(EXESUF): tests/rtc-test.o
 tests/m48t59-test$(EXESUF): tests/m48t59-test.o
@@ -315,7 +316,7 @@ tests/vmxnet3-test$(EXESUF): tests/vmxnet3-test.o
 tests/ne2000-test$(EXESUF): tests/ne2000-test.o
 tests/wdt_ib700-test$(EXESUF): tests/wdt_ib700-test.o
 tests/virtio-balloon-test$(EXESUF): tests/virtio-balloon-test.o
-tests/virtio-blk-test$(EXESUF): tests/virtio-blk-test.o
+tests/virtio-blk-test$(EXESUF): tests/virtio-blk-test.o $(libqos-virtio-obj-y)
 tests/virtio-net-test$(EXESUF): tests/virtio-net-test.o
 tests/virtio-rng-test$(EXESUF): tests/virtio-rng-test.o
 tests/virtio-scsi-test$(EXESUF): tests/virtio-scsi-test.o
diff --git a/tests/libqos/virtio-pci.c b/tests/libqos/virtio-pci.c
new file mode 100644
index 000..fde1b1f
--- /dev/null
+++ b/tests/libqos/virtio-pci.c
@@ -0,0 +1,75 @@
+/*
+ * libqos virtio PCI driver
+ *
+ * Copyright (c) 2014 Marc Marí
+ *
+ * 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 
+#include "libqtest.h"
+#include "libqos/virtio.h"
+#include "libqos/virtio-pci.h"
+#include "libqos/pci.h"
+#include "libqos/pci-pc.h"
+
+#include "hw/pci/pci_regs.h"
+
+typedef struct QVirtioPCIForeachData {
+void (*func)(QVirtioDevice *d, void *data);
+uint16_t device_type;
+void *user_data;
+} QVirtioPCIForeachData;
+
+static QVirtioPCIDevice *qpcidevice_to_qvirtiodevice(QPCIDevice *pdev)
+{
+QVirtioPCIDevice *vpcidev;
+vpcidev = g_malloc0(sizeof(*vpcidev));
+
+if (pdev) {
+vpcidev->pdev = pdev;
+vpcidev->vdev.device_type =
+qpci_config_readw(vpcidev->pdev, PCI_SUBSYSTEM_ID);
+}
+
+return vpcidev;
+}
+
+static void qvirtio_pci_foreach_callback(
+QPCIDevice *dev, int devfn, void *data)
+{
+QVirtioPCIForeachData *d = data;
+QVirtioPCIDevice *vpcidev = qpcidevice_to_qvirtiodevice(dev);
+
+if (vpcidev->vdev.device_type == d->device_type) {
+d->func(&vpcidev->vdev, d->user_data);
+} else {
+g_free(vpcidev);
+}
+}
+
+static void qvirtio_pci_assign_device(QVirtioDevice *d, void *data)
+{
+QVirtioPCIDevice **vpcidev = data;
+*vpcidev = (QVirtioPCIDevice *)d;
+}
+
+void qvirtio_pci_foreach(QPCIBus *bus, uint16_t device_type,
+void (*func)(QVirtioDevice *d, void *data), void *data)
+{
+QVirtioPCIForeachData d = { .func = func,
+.device_type = device_type,
+.user_data = data };
+
+qpci_device_foreach(bus, QVIRTIO_VENDOR_ID, -1,
+qvirtio_pci_foreach_callback, &d);
+}
+
+QVirtioPCIDevice *qvirtio_pci_device_find(QPCIBus *bus, uint16_t device_type)
+{
+QVirtioPCIDevice *dev = NULL;
+qvirtio_pci_foreach(bus, device_type, qvirtio_pci_assign_device, &dev);
+
+return dev;
+}
diff --git a/tests/libqos/virtio-pci.h b/tests/libqos/virtio-pci.h
new file mode 100644
index 000..5101abb
--- /dev/null
+++ b/tests/libqos/virtio-pci.h
@@ -0,0 +1,24 @@
+/*
+ * libqos virtio PCI definitions
+ *
+ * Copyright (c) 2014 Marc Marí
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ */
+
+#ifndef LIBQOS_VIRTIO_PCI_H
+#define LIBQOS_VIRTIO_PCI_H
+
+#include "libqos/virtio.h"
+#include "libqos/pci.h"
+
+typedef struct QVirtioPCIDevice {
+QVirtioDevice vdev;
+QPCIDevice *pdev;
+} QVirtioPCIDevice;
+
+void qvirtio_pci_foreach(QPCIBus *bus, uint16_t device_type,
+void (*func)(QVirtioDevice *d, void *data), void *data);
+QVirtioPCIDevice *qvirtio_pci_device_find(QPCIBus *bus, uint16_t device_type);
+#endif
diff --git a/tests/libqos/virtio.h b/tests/libqos/virtio.h
new file mode 100644
index 000..2a05798
--- /dev/null
+++ b/tests/libqos/virtio.h
@@ 

[Qemu-devel] [PATCH v3 6/9] virtio-blk: Correct bug in support for flexible descriptor layout

[Marc Marí]

Without this correction, only a three descriptor layout is accepted, and
requests with just two descriptors are not completed and no error message is
displayed.

Signed-off-by: Stefan Hajnoczi 
Signed-off-by: Marc Marí 
---
 hw/block/virtio-blk.c |   14 +++---
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c
index c241c50..302c39e 100644
--- a/hw/block/virtio-blk.c
+++ b/hw/block/virtio-blk.c
@@ -404,19 +404,19 @@ void virtio_blk_handle_request(VirtIOBlockReq *req, 
MultiReqBuffer *mrb)
  * NB: per existing s/n string convention the string is
  * terminated by '\0' only when shorter than buffer.
  */
-strncpy(req->elem.in_sg[0].iov_base,
-s->blk.serial ? s->blk.serial : "",
-MIN(req->elem.in_sg[0].iov_len, VIRTIO_BLK_ID_BYTES));
+const char *serial = s->blk.serial ? s->blk.serial : "";
+size_t size = MIN(strlen(serial) + 1,
+  MIN(iov_size(in_iov, in_num),
+  VIRTIO_BLK_ID_BYTES));
+iov_from_buf(in_iov, in_num, 0, serial, size);
 virtio_blk_req_complete(req, VIRTIO_BLK_S_OK);
 virtio_blk_free_request(req);
 } else if (type & VIRTIO_BLK_T_OUT) {
-qemu_iovec_init_external(&req->qiov, &req->elem.out_sg[1],
- req->elem.out_num - 1);
+qemu_iovec_init_external(&req->qiov, iov, out_num);
 virtio_blk_handle_write(req, mrb);
 } else if (type == VIRTIO_BLK_T_IN || type == VIRTIO_BLK_T_BARRIER) {
 /* VIRTIO_BLK_T_IN is 0, so we can't just & it. */
-qemu_iovec_init_external(&req->qiov, &req->elem.in_sg[0],
- req->elem.in_num - 1);
+qemu_iovec_init_external(&req->qiov, in_iov, in_num);
 virtio_blk_handle_read(req);
 } else {
 virtio_blk_req_complete(req, VIRTIO_BLK_S_UNSUPP);
-- 
1.7.10.4

[Qemu-devel] [PATCH v3 4/9] libqos: Correct mask to align size to PAGE_SIZE in malloc-pc

[Marc Marí]

Reviewed-by: John Snow 
Reviewed-by: Stefan Hajnoczi 
Signed-off-by: Paolo Bonzini 
Signed-off-by: Marc Marí 
---
 tests/libqos/malloc-pc.c |2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/libqos/malloc-pc.c b/tests/libqos/malloc-pc.c
index db1496c..2efd095 100644
--- a/tests/libqos/malloc-pc.c
+++ b/tests/libqos/malloc-pc.c
@@ -36,7 +36,7 @@ static uint64_t pc_alloc(QGuestAllocator *allocator, size_t 
size)
 
 
 size += (PAGE_SIZE - 1);
-size &= PAGE_SIZE;
+size &= -PAGE_SIZE;
 
 g_assert_cmpint((s->start + size), <=, s->end);
 
-- 
1.7.10.4

[Qemu-devel] [PATCH v3 2/9] tests: Add virtio device initialization

[Marc Marí]

Add functions to read and write virtio header fields.
Add status bit setting in virtio-blk-device.

Signed-off-by: Marc Marí 
---
 tests/Makefile|2 +-
 tests/libqos/virtio-pci.c |   57 +
 tests/libqos/virtio-pci.h |   18 ++
 tests/libqos/virtio.c |   55 +++
 tests/libqos/virtio.h |   30 
 tests/virtio-blk-test.c   |   14 +++
 6 files changed, 175 insertions(+), 1 deletion(-)
 create mode 100644 tests/libqos/virtio.c

diff --git a/tests/Makefile b/tests/Makefile
index 7c0f670..e0e203f 100644
--- a/tests/Makefile
+++ b/tests/Makefile
@@ -294,7 +294,7 @@ libqos-obj-y += tests/libqos/i2c.o
 libqos-pc-obj-y = $(libqos-obj-y) tests/libqos/pci-pc.o
 libqos-pc-obj-y += tests/libqos/malloc-pc.o
 libqos-omap-obj-y = $(libqos-obj-y) tests/libqos/i2c-omap.o
-libqos-virtio-obj-y = $(libqos-obj-y) $(libqos-pc-obj-y) 
tests/libqos/virtio-pci.o
+libqos-virtio-obj-y = $(libqos-obj-y) $(libqos-pc-obj-y) tests/libqos/virtio.o 
tests/libqos/virtio-pci.o
 
 tests/rtc-test$(EXESUF): tests/rtc-test.o
 tests/m48t59-test$(EXESUF): tests/m48t59-test.o
diff --git a/tests/libqos/virtio-pci.c b/tests/libqos/virtio-pci.c
index fde1b1f..b707c8d 100644
--- a/tests/libqos/virtio-pci.c
+++ b/tests/libqos/virtio-pci.c
@@ -55,6 +55,51 @@ static void qvirtio_pci_assign_device(QVirtioDevice *d, void 
*data)
 *vpcidev = (QVirtioPCIDevice *)d;
 }
 
+static uint8_t qvirtio_pci_config_readb(QVirtioDevice *d, void *addr)
+{
+QVirtioPCIDevice *dev = (QVirtioPCIDevice *)d;
+return qpci_io_readb(dev->pdev, addr);
+}
+
+static uint16_t qvirtio_pci_config_readw(QVirtioDevice *d, void *addr)
+{
+QVirtioPCIDevice *dev = (QVirtioPCIDevice *)d;
+return qpci_io_readw(dev->pdev, addr);
+}
+
+static uint32_t qvirtio_pci_config_readl(QVirtioDevice *d, void *addr)
+{
+QVirtioPCIDevice *dev = (QVirtioPCIDevice *)d;
+return qpci_io_readl(dev->pdev, addr);
+}
+
+static uint64_t qvirtio_pci_config_readq(QVirtioDevice *d, void *addr)
+{
+QVirtioPCIDevice *dev = (QVirtioPCIDevice *)d;
+return qpci_io_readl(dev->pdev, addr) | qpci_io_readl(dev->pdev, addr+4);
+}
+
+static uint8_t qvirtio_pci_get_status(QVirtioDevice *d)
+{
+QVirtioPCIDevice *dev = (QVirtioPCIDevice *)d;
+return qpci_io_readb(dev->pdev, dev->addr + QVIRTIO_DEVICE_STATUS);
+}
+
+static void qvirtio_pci_set_status(QVirtioDevice *d, uint8_t val)
+{
+QVirtioPCIDevice *dev = (QVirtioPCIDevice *)d;
+qpci_io_writeb(dev->pdev, dev->addr + QVIRTIO_DEVICE_STATUS, val);
+}
+
+const QVirtioBus qvirtio_pci = {
+.config_readb = qvirtio_pci_config_readb,
+.config_readw = qvirtio_pci_config_readw,
+.config_readl = qvirtio_pci_config_readl,
+.config_readq = qvirtio_pci_config_readq,
+.get_status = qvirtio_pci_get_status,
+.set_status = qvirtio_pci_set_status,
+};
+
 void qvirtio_pci_foreach(QPCIBus *bus, uint16_t device_type,
 void (*func)(QVirtioDevice *d, void *data), void *data)
 {
@@ -73,3 +118,15 @@ QVirtioPCIDevice *qvirtio_pci_device_find(QPCIBus *bus, 
uint16_t device_type)
 
 return dev;
 }
+
+void qvirtio_pci_enable_device(QVirtioPCIDevice *d)
+{
+qpci_device_enable(d->pdev);
+d->addr = qpci_iomap(d->pdev, 0);
+g_assert(d->addr != NULL);
+}
+
+void qvirtio_pci_disable_device(QVirtioPCIDevice *d)
+{
+qpci_iounmap(d->pdev, d->addr);
+}
diff --git a/tests/libqos/virtio-pci.h b/tests/libqos/virtio-pci.h
index 5101abb..3060238 100644
--- a/tests/libqos/virtio-pci.h
+++ b/tests/libqos/virtio-pci.h
@@ -13,12 +13,30 @@
 #include "libqos/virtio.h"
 #include "libqos/pci.h"
 
+#define QVIRTIO_DEVICE_FEATURES 0x00
+#define QVIRTIO_GUEST_FEATURES  0x04
+#define QVIRTIO_QUEUE_ADDRESS   0x08
+#define QVIRTIO_QUEUE_SIZE  0x0C
+#define QVIRTIO_QUEUE_SELECT0x0E
+#define QVIRTIO_QUEUE_NOTIFY0x10
+#define QVIRTIO_DEVICE_STATUS   0x12
+#define QVIRTIO_ISR_STATUS  0x13
+#define QVIRTIO_MSIX_CONF_VECTOR0x14
+#define QVIRTIO_MSIX_QUEUE_VECTOR   0x16
+#define QVIRTIO_DEVICE_SPECIFIC_MSIX0x18
+#define QVIRTIO_DEVICE_SPECIFIC_NO_MSIX 0x14
+
 typedef struct QVirtioPCIDevice {
 QVirtioDevice vdev;
 QPCIDevice *pdev;
+void *addr;
 } QVirtioPCIDevice;
 
+extern const QVirtioBus qvirtio_pci;
+
 void qvirtio_pci_foreach(QPCIBus *bus, uint16_t device_type,
 void (*func)(QVirtioDevice *d, void *data), void *data);
 QVirtioPCIDevice *qvirtio_pci_device_find(QPCIBus *bus, uint16_t device_type);
+void qvirtio_pci_enable_device(QVirtioPCIDevice *d);
+void qvirtio_pci_disable_device(QVirtioPCIDevice *d);
 #endif
diff --git a/tests/libqos/virtio.c b/tests/libqos/virtio.c
new file mode 100644
index 000..577d679
--- /dev/null
+++ b/tests/libqos/virtio.c
@@ -0,0 +1,55 @@
+/*
+ * libqos virtio driver
+ *
+ * Copyright (c) 2014 Marc Marí
+ *
+ * This work is licensed und

[Qemu-devel] [PATCH v3 8/9] libqos: Added indirect descriptor support to virtio implementation

[Marc Marí]

Add functions necessary for working with indirect descriptors.
Add test using new functions.

Signed-off-by: Marc Marí 
---
 tests/libqos/virtio.c   |   62 
 tests/libqos/virtio.h   |   11 +
 tests/virtio-blk-test.c |  120 +++
 3 files changed, 193 insertions(+)

diff --git a/tests/libqos/virtio.c b/tests/libqos/virtio.c
index fe45836..9657b4c 100644
--- a/tests/libqos/virtio.c
+++ b/tests/libqos/virtio.c
@@ -105,6 +105,51 @@ void qvring_init(const QGuestAllocator *alloc, QVirtQueue 
*vq, uint64_t addr)
 writew(vq->used, 0);
 }
 
+QVRingIndirectDesc *qvring_indirect_desc_setup(QVirtioDevice *d,
+QGuestAllocator *alloc, uint16_t elem)
+{
+int i;
+QVRingIndirectDesc *indirect = g_malloc(sizeof(*indirect));
+
+indirect->index = 0;
+indirect->elem = elem;
+indirect->desc = guest_alloc(alloc, sizeof(QVRingDesc)*elem);
+
+for (i = 0; i < elem-1; ++i) {
+/* indirect->desc[i].addr */
+writeq(indirect->desc+(16*i), 0);
+/* indirect->desc[i].flags */
+writew(indirect->desc+(16*i)+12, QVRING_DESC_F_NEXT);
+/* indirect->desc[i].next */
+writew(indirect->desc+(16*i)+14, i+1);
+}
+
+return indirect;
+}
+
+void qvring_indirect_desc_add(QVRingIndirectDesc *indirect, uint64_t data,
+uint32_t len, int write)
+{
+uint16_t flags;
+
+g_assert_cmpint(indirect->index, <, indirect->elem);
+
+flags = readw(indirect->desc+(16*indirect->index)+12);
+
+if (write) {
+flags |= QVRING_DESC_F_WRITE;
+}
+
+/* indirect->desc[indirect->index].addr */
+writeq(indirect->desc+(16*indirect->index), data);
+/* indirect->desc[indirect->index].len */
+writel(indirect->desc+(16*indirect->index)+8, len);
+/* indirect->desc[indirect->index].flags */
+writew(indirect->desc+(16*indirect->index)+12, flags);
+
+indirect->index++;
+}
+
 uint32_t qvirtqueue_add(QVirtQueue *vq, uint64_t data, uint32_t len, int write,
 int next)
 {
@@ -129,6 +174,23 @@ uint32_t qvirtqueue_add(QVirtQueue *vq, uint64_t data, 
uint32_t len, int write,
 return vq->free_head++; /* Return and increase, in this order */
 }
 
+uint32_t qvirtqueue_add_indirect(QVirtQueue *vq, QVRingIndirectDesc *indirect)
+{
+g_assert_cmpint(vq->size, >=, indirect->elem);
+g_assert_cmpint(indirect->index, ==, indirect->elem);
+
+vq->num_free--;
+
+/* vq->desc[vq->free_head].addr */
+writeq(vq->desc+(16*vq->free_head), indirect->desc);
+/* vq->desc[vq->free_head].len */
+writel(vq->desc+(16*vq->free_head)+8, sizeof(QVRingDesc)*indirect->elem);
+/* vq->desc[vq->free_head].flags */
+writew(vq->desc+(16*vq->free_head)+12, QVRING_DESC_F_INDIRECT);
+
+return vq->free_head++; /* Return and increase, in this order */
+}
+
 void qvirtqueue_kick(const QVirtioBus *bus, QVirtioDevice *d, QVirtQueue *vq,
 uint32_t free_head)
 {
diff --git a/tests/libqos/virtio.h b/tests/libqos/virtio.h
index 1ac1fea..f5b8c84 100644
--- a/tests/libqos/virtio.h
+++ b/tests/libqos/virtio.h
@@ -70,6 +70,12 @@ typedef struct QVirtQueue {
 uint32_t align;
 } QVirtQueue;
 
+typedef struct QVRingIndirectDesc {
+uint64_t desc; /* This points to an array fo QVRingDesc */
+uint16_t index;
+uint16_t elem;
+} QVRingIndirectDesc;
+
 typedef struct QVirtioBus {
 uint8_t (*config_readb)(QVirtioDevice *d, void *addr);
 uint16_t (*config_readw)(QVirtioDevice *d, void *addr);
@@ -133,8 +139,13 @@ int qvirtio_wait_isr(const QVirtioBus *bus, QVirtioDevice 
*d, uint8_t mask,
 uint64_t timeout);
 
 void qvring_init(const QGuestAllocator *alloc, QVirtQueue *vq, uint64_t addr);
+QVRingIndirectDesc *qvring_indirect_desc_setup(QVirtioDevice *d,
+QGuestAllocator *alloc, uint16_t elem);
+void qvring_indirect_desc_add(QVRingIndirectDesc *indirect, uint64_t data,
+uint32_t len, int write);
 uint32_t qvirtqueue_add(QVirtQueue *vq, uint64_t data, uint32_t len, int write,
 int next);
+uint32_t qvirtqueue_add_indirect(QVirtQueue *vq, QVRingIndirectDesc *indirect);
 void qvirtqueue_kick(const QVirtioBus *bus, QVirtioDevice *d, QVirtQueue *vq,
 uint32_t 
free_head);
 
diff --git a/tests/virtio-blk-test.c b/tests/virtio-blk-test.c
index 4f53bcb..54326d3 100644
--- a/tests/virtio-blk-test.c
+++ b/tests/virtio-blk-test.c
@@ -229,6 +229,125 @@ static void pci_basic(void)
 test_end();
 }
 
+static void pci_indirect(void)
+{
+QVirtioPCIDevice *dev;
+QPCIBus *bus;
+QVirtQueue *vq;
+QGuest

[Qemu-devel] [PATCH v3 3/9] libqtest: add QTEST_LOG for debugging qtest testcases

[Marc Marí]

Signed-off-by: Paolo Bonzini 
Signed-off-by: Marc Marí 
---
 tests/libqtest.c |7 ++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/tests/libqtest.c b/tests/libqtest.c
index 98e8f4b..fbd600d 100644
--- a/tests/libqtest.c
+++ b/tests/libqtest.c
@@ -167,11 +167,12 @@ QTestState *qtest_init(const char *extra_args)
 if (s->qemu_pid == 0) {
 command = g_strdup_printf("exec %s "
   "-qtest unix:%s,nowait "
-  "-qtest-log /dev/null "
+  "-qtest-log %s "
   "-qmp unix:%s,nowait "
   "-machine accel=qtest "
   "-display none "
   "%s", qemu_binary, socket_path,
+  getenv("QTEST_LOG") ? "/dev/fd/2" : 
"/dev/null",
   qmp_socket_path,
   extra_args ?: "");
 execlp("/bin/sh", "sh", "-c", command, NULL);
@@ -397,10 +398,14 @@ QDict *qtest_qmpv(QTestState *s, const char *fmt, va_list 
ap)
 
 /* No need to send anything for an empty QObject.  */
 if (qobj) {
+int log = getenv("QTEST_LOG") != NULL;
 QString *qstr = qobject_to_json(qobj);
 const char *str = qstring_get_str(qstr);
 size_t size = qstring_get_length(qstr);
 
+if (log) {
+fprintf(stderr, "%s", str);
+}
 /* Send QMP request */
 socket_send(s->qmp_fd, str, size);
 
-- 
1.7.10.4

[Qemu-devel] [PATCH v3 7/9] libqos: Added basic virtqueue support to virtio implementation

[Marc Marí]

Add status changing and feature negotiation.
Add basic virtqueue support for adding and sending virtqueue requests.
Add ISR checking.

Signed-off-by: Marc Marí 
---
 tests/libqos/virtio-pci.c |   91 +-
 tests/libqos/virtio-pci.h |7 ++
 tests/libqos/virtio.c |   89 +
 tests/libqos/virtio.h |   90 +-
 tests/virtio-blk-test.c   |  158 +++--
 5 files changed, 428 insertions(+), 7 deletions(-)

diff --git a/tests/libqos/virtio-pci.c b/tests/libqos/virtio-pci.c
index b707c8d..5d00826 100644
--- a/tests/libqos/virtio-pci.c
+++ b/tests/libqos/virtio-pci.c
@@ -13,6 +13,8 @@
 #include "libqos/virtio-pci.h"
 #include "libqos/pci.h"
 #include "libqos/pci-pc.h"
+#include "libqos/malloc.h"
+#include "libqos/malloc-pc.h"
 
 #include "hw/pci/pci_regs.h"
 
@@ -79,16 +81,93 @@ static uint64_t qvirtio_pci_config_readq(QVirtioDevice *d, 
void *addr)
 return qpci_io_readl(dev->pdev, addr) | qpci_io_readl(dev->pdev, addr+4);
 }
 
+static uint32_t qvirtio_pci_get_features(QVirtioDevice *d)
+{
+QVirtioPCIDevice *dev = (QVirtioPCIDevice *)d;
+return qpci_io_readl(dev->pdev, dev->addr + QVIRTIO_DEVICE_FEATURES);
+}
+
+static void qvirtio_pci_set_features(QVirtioDevice *d, uint32_t features)
+{
+QVirtioPCIDevice *dev = (QVirtioPCIDevice *)d;
+qpci_io_writel(dev->pdev, dev->addr + QVIRTIO_GUEST_FEATURES, features);
+}
+
 static uint8_t qvirtio_pci_get_status(QVirtioDevice *d)
 {
 QVirtioPCIDevice *dev = (QVirtioPCIDevice *)d;
 return qpci_io_readb(dev->pdev, dev->addr + QVIRTIO_DEVICE_STATUS);
 }
 
-static void qvirtio_pci_set_status(QVirtioDevice *d, uint8_t val)
+static void qvirtio_pci_set_status(QVirtioDevice *d, uint8_t status)
+{
+QVirtioPCIDevice *dev = (QVirtioPCIDevice *)d;
+qpci_io_writeb(dev->pdev, dev->addr + QVIRTIO_DEVICE_STATUS, status);
+}
+
+static uint8_t qvirtio_pci_get_isr_status(QVirtioDevice *d)
+{
+QVirtioPCIDevice *dev = (QVirtioPCIDevice *)d;
+return qpci_io_readb(dev->pdev, dev->addr + QVIRTIO_ISR_STATUS);
+}
+
+static void qvirtio_pci_queue_select(QVirtioDevice *d, uint16_t index)
+{
+QVirtioPCIDevice *dev = (QVirtioPCIDevice *)d;
+qpci_io_writeb(dev->pdev, dev->addr + QVIRTIO_QUEUE_SELECT, index);
+}
+
+static uint16_t qvirtio_pci_get_queue_size(QVirtioDevice *d)
+{
+QVirtioPCIDevice *dev = (QVirtioPCIDevice *)d;
+return qpci_io_readw(dev->pdev, dev->addr + QVIRTIO_QUEUE_SIZE);
+}
+
+static void qvirtio_pci_set_queue_address(QVirtioDevice *d, uint32_t pfn)
+{
+QVirtioPCIDevice *dev = (QVirtioPCIDevice *)d;
+qpci_io_writel(dev->pdev, dev->addr + QVIRTIO_QUEUE_ADDRESS, pfn);
+}
+
+static QVirtQueue *qvirtio_pci_virtqueue_setup(QVirtioDevice *d,
+QGuestAllocator *alloc, uint16_t index)
+{
+uint16_t aux;
+uint64_t addr;
+QVirtQueue *vq;
+
+vq = g_malloc0(sizeof(*vq));
+
+qvirtio_pci_queue_select(d, index);
+vq->index = index;
+vq->size = qvirtio_pci_get_queue_size(d);
+vq->free_head = 0;
+vq->num_free = vq->size;
+vq->align = QVIRTIO_PCI_ALIGN;
+
+/* Check different than 0 */
+g_assert_cmpint(vq->size, !=, 0);
+
+/* Check power of 2 */
+aux = vq->size;
+while ((aux & 1) != 0) {
+aux = aux >> 1;
+}
+g_assert_cmpint(aux, !=, 1);
+
+addr = guest_alloc(alloc, qvring_size(vq->size, QVIRTIO_PCI_ALIGN));
+qvring_init(alloc, vq, addr);
+qvirtio_pci_set_queue_address(d, vq->desc/4096);
+
+/* TODO: MSI-X configuration */
+
+return vq;
+}
+
+static void qvirtio_pci_virtqueue_kick(QVirtioDevice *d, QVirtQueue *vq)
 {
 QVirtioPCIDevice *dev = (QVirtioPCIDevice *)d;
-qpci_io_writeb(dev->pdev, dev->addr + QVIRTIO_DEVICE_STATUS, val);
+qpci_io_writew(dev->pdev, dev->addr + QVIRTIO_QUEUE_NOTIFY, vq->index);
 }
 
 const QVirtioBus qvirtio_pci = {
@@ -96,8 +175,16 @@ const QVirtioBus qvirtio_pci = {
 .config_readw = qvirtio_pci_config_readw,
 .config_readl = qvirtio_pci_config_readl,
 .config_readq = qvirtio_pci_config_readq,
+.get_features = qvirtio_pci_get_features,
+.set_features = qvirtio_pci_set_features,
 .get_status = qvirtio_pci_get_status,
 .set_status = qvirtio_pci_set_status,
+.get_isr_status = qvirtio_pci_get_isr_status,
+.queue_select = qvirtio_pci_queue_select,
+.get_queue_size = qvirtio_pci_get_queue_size,
+.set_queue_address = qvirtio_pci_set_queue_address,
+.virtqueue_setup = qvirtio_pci_virtqueue_setup,
+.virtqueue_kick = qvirtio_pci_virtqueue_kick,
 };
 
 void qvirtio_pci_foreach(QPCIBus *bus, uint16_t device_type,
diff --git a/tests/libqos/virtio-pci.h b/tests/libqos/virtio-pci.h
index 3060238..1e1154a 100644
--- a/tests/libqos/virtio-pci.h
+++ b/tests/libqos/virtio-pci.h
@@ -26,6 +26,13 @@
 #define QVIRTIO_DEVICE_SPECIFIC_MSIX0x18
 #define QVIRTIO_DEVICE_SPECIFIC_NO_MSIX 0x14
 
+#define QVIRTIO_F_NOTIFY_ON_EMPTY   0

[Qemu-devel] [PATCH v3 9/9] libqos: Added test case for configuration changes in virtio-blk test

[Marc Marí]

Signed-off-by: Marc Marí 
---
 tests/virtio-blk-test.c |   14 --
 1 file changed, 12 insertions(+), 2 deletions(-)

diff --git a/tests/virtio-blk-test.c b/tests/virtio-blk-test.c
index 54326d3..c7b3bf4 100644
--- a/tests/virtio-blk-test.c
+++ b/tests/virtio-blk-test.c
@@ -229,7 +229,7 @@ static void pci_basic(void)
 test_end();
 }
 
-static void pci_indirect(void)
+static void pci_indirect_config(void)
 {
 QVirtioPCIDevice *dev;
 QPCIBus *bus;
@@ -237,6 +237,7 @@ static void pci_indirect(void)
 QGuestAllocator *alloc;
 QVRingIndirectDesc *indirect;
 int isr_result;
+int n_size = TEST_IMAGE_SIZE/2;
 void *addr;
 uint64_t w_req;
 uint64_t r_req;
@@ -274,6 +275,15 @@ static void pci_indirect(void)
 
 qvirtio_set_driver_ok(&qvirtio_pci, &dev->vdev);
 
+qmp("{ 'execute': 'block_resize', 'arguments': { 'device': 'drive0', "
+" 'size': %d } }", n_size);
+
+isr_result = qvirtio_wait_isr(&qvirtio_pci, &dev->vdev, 0x40,
+QVIRTIO_BLK_TIMEOUT);
+
+capacity = qvirtio_config_readq(&qvirtio_pci, &dev->vdev, addr);
+g_assert_cmpint(capacity, ==, n_size/512);
+
 /* Write and read with 2 descriptor layout */
 data = g_malloc0(512);
 strcpy(data, "TEST");
@@ -355,7 +365,7 @@ int main(int argc, char **argv)
 g_test_init(&argc, &argv, NULL);
 
 g_test_add_func("/virtio/blk/pci/basic", pci_basic);
-g_test_add_func("/virtio/blk/pci/indirect", pci_indirect);
+g_test_add_func("/virtio/blk/pci/indirect_config", pci_indirect_config);
 
 ret = g_test_run();
 
-- 
1.7.10.4

Re: [Qemu-devel] [PATCH 2/2] qemu-iotests: add multiwrite test cases

[Kevin Wolf]

Am 31.07.2014 um 20:19 hat Slava Pestov geschrieben:
> Why are you guys merging requests in qemu at all? Just submit them to the
> kernel and let the kernel do it.

Because the kernel generally isn't the next one seeing the requests. You
might be right for the special case of using only raw images with
cache=none,aio=native, where we would theoretically have a chance to
submit all requests in a batch with a single syscall and then let the
kernel merge them. It just isn't what everyone is running.

An important case is requests going to non-raw image format drivers,
where many small writes on sparse images are inefficient because they
cause a lot of unnecessary COW activity. Another case are backends that
don't even send the requests to the kernel, but use e.g. a network
protocol as their backend.

Kevin

[Qemu-devel] [PATCH 1/3] query-memdev: fix potential memory leaks

[Chen Fan]

Signed-off-by: Chen Fan 
---
 numa.c | 6 +-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/numa.c b/numa.c
index 7bf7834..a2b4bca 100644
--- a/numa.c
+++ b/numa.c
@@ -318,10 +318,11 @@ void memory_region_allocate_system_memory(MemoryRegion 
*mr, Object *owner,
 static int query_memdev(Object *obj, void *opaque)
 {
 MemdevList **list = opaque;
+MemdevList *m = NULL;
 Error *err = NULL;
 
 if (object_dynamic_cast(obj, TYPE_MEMORY_BACKEND)) {
-MemdevList *m = g_malloc0(sizeof(*m));
+m = g_malloc0(sizeof(*m));
 
 m->value = g_malloc0(sizeof(*m->value));
 
@@ -369,6 +370,9 @@ static int query_memdev(Object *obj, void *opaque)
 
 return 0;
 error:
+g_free(m->value);
+g_free(m);
+
 return -1;
 }
 
-- 
1.9.3

[Qemu-devel] [PATCH 0/3] Fix some memory leaks about query memdev

[Chen Fan]

when using valgrind to test the command "query memdev", I had
found some memory leaks. the test result:

==13802== 4 bytes in 1 blocks are definitely lost in loss record 125 of 8,508
==13802==at 0x4A08934: malloc (vg_replace_malloc.c:291)
==13802==by 0x4A08AA8: realloc (vg_replace_malloc.c:687)
==13802==by 0x64C5736: g_realloc (in /usr/lib64/libglib-2.0.so.0.3400.2)
==13802==by 0x64DE226: ??? (in /usr/lib64/libglib-2.0.so.0.3400.2)
==13802==by 0x64DE279: g_string_sized_new (in 
/usr/lib64/libglib-2.0.so.0.3400.2)
==13802==by 0x47CFBB: string_output_visitor_new 
(string-output-visitor.c:341)
==13802==by 0x3F456F: object_property_get_uint16List (object.c:970)
==13802==by 0x1E8764: query_memdev (numa.c:361)
==13802==by 0x3F3248: object_child_foreach (object.c:686)
==13802==by 0x1E9141: qmp_query_memdev (numa.c:389)
==13802==by 0x2D79A0: qmp_marshal_input_query_memdev (qmp-marshal.c:5057)
==13802==by 0x1DD7D7: handle_qmp_command (monitor.c:5038)

==15046== 48 (16 direct, 32 indirect) bytes in 1 blocks are definitely lost in 
loss record 4,722 of 8,549
==15046==at 0x4A08934: malloc (vg_replace_malloc.c:291)
==15046==by 0x64C541D: ??? (in /usr/lib64/libglib-2.0.so.0.3400.2)
==15046==by 0x64C56E6: g_malloc0 (in /usr/lib64/libglib-2.0.so.0.3400.2)
==15046==by 0x1E868C: query_memdev (numa.c:325)
==15046==by 0x3F3258: object_child_foreach (object.c:686)
==15046==by 0x1E9141: qmp_query_memdev (numa.c:389)
==15046==by 0x2DDFF3: hmp_info_memdev (hmp.c:1687)
==15046==by 0x1E4B08: handle_user_command (monitor.c:4119)
==15046==by 0x1E4E7A: monitor_command_cb (monitor.c:5156)
==15046==by 0x496056: readline_handle_byte (readline.c:391)
==15046==by 0x1E4BCE: monitor_read (monitor.c:5139)
==15046==by 0x2BCDEF: fd_chr_read (qemu-char.c:213)

Chen Fan (3):
  query-memdev: fix potential memory leaks
  qom/object.c: fix string_output_get_string() memory leak
  hmp: fix MemdevList memory leak

 hmp.c| 15 ++-
 numa.c   |  6 +-
 qom/object.c | 11 ---
 3 files changed, 23 insertions(+), 9 deletions(-)

-- 
1.9.3

[Qemu-devel] [PATCH 2/3] qom/object.c: fix string_output_get_string() memory leak

[Chen Fan]

string_output_get_string() always return the data the sov->string
point. and never free.

Signed-off-by: Chen Fan 
---
 hmp.c|  6 --
 qom/object.c | 11 ---
 2 files changed, 12 insertions(+), 5 deletions(-)

diff --git a/hmp.c b/hmp.c
index 4d1838e..2414cc7 100644
--- a/hmp.c
+++ b/hmp.c
@@ -1687,6 +1687,7 @@ void hmp_info_memdev(Monitor *mon, const QDict *qdict)
 MemdevList *memdev_list = qmp_query_memdev(&err);
 MemdevList *m = memdev_list;
 StringOutputVisitor *ov;
+char *str;
 int i = 0;
 
 
@@ -1704,10 +1705,11 @@ void hmp_info_memdev(Monitor *mon, const QDict *qdict)
m->value->prealloc ? "true" : "false");
 monitor_printf(mon, "  policy: %s\n",
HostMemPolicy_lookup[m->value->policy]);
-monitor_printf(mon, "  host nodes: %s\n",
-   string_output_get_string(ov));
+str = string_output_get_string(ov);
+monitor_printf(mon, "  host nodes: %s\n", str);
 
 string_output_visitor_cleanup(ov);
+g_free(str);
 m = m->next;
 i++;
 }
diff --git a/qom/object.c b/qom/object.c
index 0e8267b..7ae4f33 100644
--- a/qom/object.c
+++ b/qom/object.c
@@ -948,16 +948,18 @@ int object_property_get_enum(Object *obj, const char 
*name,
 {
 StringOutputVisitor *sov;
 StringInputVisitor *siv;
+char *str;
 int ret;
 
 sov = string_output_visitor_new(false);
 object_property_get(obj, string_output_get_visitor(sov), name, errp);
-siv = string_input_visitor_new(string_output_get_string(sov));
+str = string_output_get_string(sov);
+siv = string_input_visitor_new(str);
 string_output_visitor_cleanup(sov);
 visit_type_enum(string_input_get_visitor(siv),
 &ret, strings, NULL, name, errp);
 string_input_visitor_cleanup(siv);
-
+g_free(str);
 return ret;
 }
 
@@ -966,15 +968,18 @@ void object_property_get_uint16List(Object *obj, const 
char *name,
 {
 StringOutputVisitor *ov;
 StringInputVisitor *iv;
+char *str;
 
 ov = string_output_visitor_new(false);
 object_property_get(obj, string_output_get_visitor(ov),
 name, errp);
-iv = string_input_visitor_new(string_output_get_string(ov));
+str = string_output_get_string(ov);
+iv = string_input_visitor_new(str);
 visit_type_uint16List(string_input_get_visitor(iv),
   list, NULL, errp);
 string_output_visitor_cleanup(ov);
 string_input_visitor_cleanup(iv);
+g_free(str);
 }
 
 void object_property_parse(Object *obj, const char *string,
-- 
1.9.3

[Qemu-devel] [PATCH 3/3] hmp: fix MemdevList memory leak

[Chen Fan]

Signed-off-by: Chen Fan 
---
 hmp.c | 9 ++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/hmp.c b/hmp.c
index 2414cc7..0b1c830 100644
--- a/hmp.c
+++ b/hmp.c
@@ -1685,13 +1685,14 @@ void hmp_info_memdev(Monitor *mon, const QDict *qdict)
 {
 Error *err = NULL;
 MemdevList *memdev_list = qmp_query_memdev(&err);
-MemdevList *m = memdev_list;
+MemdevList *m;
 StringOutputVisitor *ov;
 char *str;
 int i = 0;
 
 
-while (m) {
+while (memdev_list) {
+m = memdev_list;
 ov = string_output_visitor_new(false);
 visit_type_uint16List(string_output_get_visitor(ov),
   &m->value->host_nodes, NULL, NULL);
@@ -1710,7 +1711,9 @@ void hmp_info_memdev(Monitor *mon, const QDict *qdict)
 
 string_output_visitor_cleanup(ov);
 g_free(str);
-m = m->next;
+memdev_list = memdev_list->next;
+g_free(m->value);
+g_free(m);
 i++;
 }
 
-- 
1.9.3

Re: [Qemu-devel] [PATCH v2] target-mips: fix broken MIPS16 and microMIPS

[Yongbok Kim]

Ping!
Patch for v2.1.0

-Original Message-
From: Yongbok Kim 
Sent: 01 July 2014 17:43
To: qemu-devel@nongnu.org
Cc: aurel...@aurel32.net; Leon Alrae; Cristian Cuna; Yongbok Kim
Subject: [PATCH v2] target-mips: fix broken MIPS16 and microMIPS

Commit 240ce26a broke MIPS16 and microMIPS support as it didn't
care those branches and jumps don't have delay slot in
MIPS16 and microMIPS.

This patch introduces a new argument delayslot_size to the
gen_compute_branch() indicating size of delay slot {0, 2, 4}.
And the information is used to call handle_delay_slot() forcingly
when no delay slot is required.

There are some microMIPS branch and jump instructions that requires
exact size of instruction in the delay slot. For indicating
these instructions, MIPS_HFLAG_BDS_STRICT flag is introduced.

Those fictional branch opcodes defined to support MIPS16 and
microMIPS are no longer needed.

Signed-off-by: Yongbok Kim 
---
v2:
* correct MIPS_HFLAG_TMASK
---
 target-mips/cpu.h   |   13 +-
 target-mips/translate.c |  284 ++-
 2 files changed, 117 insertions(+), 180 deletions(-)

diff --git a/target-mips/cpu.h b/target-mips/cpu.h
index 8b9a92e..c81dfac 100644
--- a/target-mips/cpu.h
+++ b/target-mips/cpu.h
@@ -431,7 +431,7 @@ struct CPUMIPSState {
 int error_code;
 uint32_t hflags;/* CPU State */
 /* TMASK defines different execution modes */
-#define MIPS_HFLAG_TMASK  0xC07FF
+#define MIPS_HFLAG_TMASK  0x1807FF
 #define MIPS_HFLAG_MODE   0x7 /* execution modes*/
 /* The KSU flags must be the lowest bits in hflags. The flag order
must be the same as defined for CP0 Status. This allows to use
@@ -463,17 +463,18 @@ struct CPUMIPSState {
 #define MIPS_HFLAG_BL 0x01800 /* Likely branch  */
 #define MIPS_HFLAG_BR 0x02000 /* branch to register (can't link TB) */
 /* Extra flags about the current pending branch.  */
-#define MIPS_HFLAG_BMASK_EXT 0x3C000
+#define MIPS_HFLAG_BMASK_EXT 0x7C000
 #define MIPS_HFLAG_B160x04000 /* branch instruction was 16 bits */
 #define MIPS_HFLAG_BDS16  0x08000 /* branch requires 16-bit delay slot  */
 #define MIPS_HFLAG_BDS32  0x1 /* branch requires 32-bit delay slot  */
-#define MIPS_HFLAG_BX 0x2 /* branch exchanges execution mode*/
+#define MIPS_HFLAG_BDS_STRICT  0x2 /* Strict delay slot size */
+#define MIPS_HFLAG_BX 0x4 /* branch exchanges execution mode*/
 #define MIPS_HFLAG_BMASK  (MIPS_HFLAG_BMASK_BASE | MIPS_HFLAG_BMASK_EXT)
 /* MIPS DSP resources access. */
-#define MIPS_HFLAG_DSP   0x4  /* Enable access to MIPS DSP resources. */
-#define MIPS_HFLAG_DSPR2 0x8  /* Enable access to MIPS DSPR2 resources. */
+#define MIPS_HFLAG_DSP   0x08  /* Enable access to MIPS DSP resources. */
+#define MIPS_HFLAG_DSPR2 0x10  /* Enable access to MIPS DSPR2 resources. */
 /* Extra flag about HWREna register. */
-#define MIPS_HFLAG_HWRENA_ULR 0x10 /* ULR bit from HWREna is set. */
+#define MIPS_HFLAG_HWRENA_ULR 0x20 /* ULR bit from HWREna is set. */
 target_ulong btarget;/* Jump / branch target   */
 target_ulong bcond;  /* Branch condition (if needed)   */
 
diff --git a/target-mips/translate.c b/target-mips/translate.c
index 2f91959..a654ae8 100644
--- a/target-mips/translate.c
+++ b/target-mips/translate.c
@@ -61,7 +61,6 @@ enum {
 /* Jump and branches */
 OPC_J= (0x02 << 26),
 OPC_JAL  = (0x03 << 26),
-OPC_JALS = OPC_JAL | 0x5,
 OPC_BEQ  = (0x04 << 26),  /* Unconditional if rs = rt = 0 (B) */
 OPC_BEQL = (0x14 << 26),
 OPC_BNE  = (0x05 << 26),
@@ -70,8 +69,7 @@ enum {
 OPC_BLEZL= (0x16 << 26),
 OPC_BGTZ = (0x07 << 26),
 OPC_BGTZL= (0x17 << 26),
-OPC_JALX = (0x1D << 26),  /* MIPS 16 only */
-OPC_JALXS= OPC_JALX | 0x5,
+OPC_JALX = (0x1D << 26),
 /* Load and stores */
 OPC_LDL  = (0x1A << 26),
 OPC_LDR  = (0x1B << 26),
@@ -171,8 +169,6 @@ enum {
 /* Jumps */
 OPC_JR   = 0x08 | OPC_SPECIAL, /* Also JR.HB */
 OPC_JALR = 0x09 | OPC_SPECIAL, /* Also JALR.HB */
-OPC_JALRC= OPC_JALR | (0x5 << 6),
-OPC_JALRS= 0x10 | OPC_SPECIAL | (0x5 << 6),
 /* Traps */
 OPC_TGE  = 0x30 | OPC_SPECIAL,
 OPC_TGEU = 0x31 | OPC_SPECIAL,
@@ -236,10 +232,8 @@ enum {
 OPC_BGEZ = (0x01 << 16) | OPC_REGIMM,
 OPC_BGEZL= (0x03 << 16) | OPC_REGIMM,
 OPC_BLTZAL   = (0x10 << 16) | OPC_REGIMM,
-OPC_BLTZALS  = OPC_BLTZAL | 0x5, /* microMIPS */
 OPC_BLTZALL  = (0x12 << 16) | OPC_REGIMM,
 OPC_BGEZAL   = (0x11 << 16) | OPC_REGIMM,
-OPC_BGEZALS  = OPC_BGEZAL | 0x5, /* microMIPS */
 OPC_BGEZALL  = (0x13 << 16) | OPC_REGIMM,
 OPC_TGEI = (0x08 << 16) | OPC_REGIMM,
 OPC_TGEIU= (0x09 << 16) | OPC_REGIMM,
@@ -3597,7 +3591,8 @@ static inline void gen_goto_tb(DisasContext *ctx, 

Re: [Qemu-devel] [PATCH] hw/arm/virt: formatting: memory map

[Peter Maydell]

On 29 July 2014 17:46, Andrew Jones  wrote:
> Add some spacing and zeros to make it easier to read and
> modify the map. This patch has no functional changes. The
> review looks ugly, but it's actually pretty easy to confirm
> all the addresses are as they should be - thanks to the new
> formatting ;-)
>
> Applies on top of 'v2.1.0-rc5'.
>
> Signed-off-by: Andrew Jones 
> ---
>  hw/arm/virt.c | 16 
>  1 file changed, 8 insertions(+), 8 deletions(-)

Thanks, applied to target-arm.next.

-- PMM

Re: [Qemu-devel] [PATCH 24/28] ahci: Add test_pci_spec to ahci-test.

[Stefan Hajnoczi]

On Thu, Jul 31, 2014 at 01:42:02PM -0400, John Snow wrote:
> 
> On 07/31/2014 09:19 AM, Stefan Hajnoczi wrote:
> >On Mon, Jul 07, 2014 at 02:18:05PM -0400, John Snow wrote:
> >>+static void ahci_test_pci_caps(QPCIDevice *ahci, uint16_t header,
> >>+   uint8_t offset)
> >>+{
> >>+uint8_t cid = header & 0xFF;
> >>+uint8_t next = header >> 8;
> >>+
> >>+g_test_message("CID: %02x; next: %02x", cid, next);
> >Debugging output that should be removed?
> The output here is only visible via --verbose. If even this is undesirable,
> I can remove it completely ... but I figured it would be nice to leave in
> some really basic debugging messages.

When I come across verbose output during review I'm not sure whether you
have it there because you weren't sure about something that still needs
to be discussed before merging the patch.  I'm trying to identify loose
ends by asking these questions.

> >>+
> >>+switch (cid) {
> >>+case PCI_CAP_ID_PM:
> >>+ahci_test_pmcap(ahci, offset);
> >>+break;
> >>+case PCI_CAP_ID_MSI:
> >>+ahci_test_msicap(ahci, offset);
> >>+break;
> >>+case PCI_CAP_ID_SATA:
> >>+ahci_test_satacap(ahci, offset);
> >>+break;
> >>+
> >>+default:
> >>+g_test_message("Unknown CAP 0x%02x", cid);
> >This should be a failure.  If a new capability is added, the test should
> >be extended for that capability.
> The specification does allow for any number of arbitrary capabilities, in
> which the specification has no say about order or compliance. Any further
> investigation really delves into the PCI specification, which was not my aim
> here. I think mentioning the inability to verify a capability is fine via
> --verbose for the purposes of basic AHCI sanity tests. At any rate, I don't
> think this is a failure -- at least not in THIS test, which I tried to keep
> as a "spec-adherent, QEMU indifferent" test. If we want to test
> QEMU-specifics, I would like to add those into a separate test case -- at
> which point failures for unrecognized capabilities would be appropriate.
> 
> In the future, we might even abstract out these pieces that apply to PCI
> devices as a whole to be generic PCI spec compliance tests, with an AHCI and
> then a QEMU layer on top, in order of increasing specificity.

Okay, just keep in mind that only someone actively developing the test
case will notice verbose output.  By skipping unknown capabilities we
won't have a reminder that this test case needs to be updated when a new
one is added.

Stefan


pgpWEsnCXvSdT.pgp
Description: PGP signature

Re: [Qemu-devel] [PATCH target-arm v1 1/1] sd: sdhci: Fix ADMA dma_memory_read access

[Peter Maydell]

On 31 July 2014 04:11, Peter Crosthwaite  wrote:
> This dma_memory_read was giving too big a size when begin was non-zero.
> This could cause segfaults in some circumstances. Fix.
>
> Signed-off-by: Peter Crosthwaite 

Applied to target-arm.next.

thanks
-- PMM

Re: [Qemu-devel] [PATCH] arm64: 64K pages and > 1024MB guest

[Peter Maydell]

On 23 July 2014 21:09, Joel Schopp  wrote:
> kvm_set_phys_mem doesn't work on arm64 with memory > 1GB.  It exits with:
> kvm_set_phys_mem: error registering slot: Invalid argument
>
> An example of the failing address and size are start_addr == 0x90011000
> and size=0xaffef000.  As you can see both of these are 4K aligned, not
> 64K aligned.
>
> At 1024MB or smaller qemu only makes one call to kvm_set_user_memory_region,
> so the start_addr and size are aligned by accident and the bug doesn't happen.
>
> The following patch makes things work for me on an arm64 SOC.  I also smoke
> tested the patch on an x86-64 box and qemu seemed to still run fine there
> with the patch applied.
>
> Cc: Peter Maydell 
> Signed-off-by: Joel Schopp 
> ---
>  kvm-all.c |6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/kvm-all.c b/kvm-all.c
> index 1402f4f..1975862 100644
> --- a/kvm-all.c
> +++ b/kvm-all.c
> @@ -618,14 +618,14 @@ static void kvm_set_phys_mem(MemoryRegionSection 
> *section, bool add)
>
>  /* kvm works in page size chunks, but the function may be called
> with sub-page size and unaligned start address. */
> -delta = TARGET_PAGE_ALIGN(size) - size;
> +delta = HOST_PAGE_ALIGN(start_addr) - start_addr;
>  if (delta > size) {
>  return;
>  }
>  start_addr += delta;
>  size -= delta;
> -size &= TARGET_PAGE_MASK;
> -if (!size || (start_addr & ~TARGET_PAGE_MASK)) {
> +size &= qemu_host_page_mask;
> +if (!size || (start_addr & ~qemu_host_page_mask)) {
>  return;
>  }
>
>

Paolo: can you review this? Do we also need to do something
about the use of TARGET_PAGE_BITS in
kvm_physical_sync_dirty_bitmap? Is it really OK to define
PAGE_SIZE to TARGET_PAGE_SIZE (it's certainly really
misleading and suggests the kernel headers could be more
helpful).

Basically I think kvm-all.c should almost certainly not be
using any of the TARGET_PAGE_* constants anywhere.

thanks
-- PMM

Re: [Qemu-devel] [PATCH] block: add watermark event

[Stefan Hajnoczi]

On Tue, Jul 08, 2014 at 04:49:24PM +0200, Francesco Romani wrote:
> @@ -5813,3 +5815,57 @@ void bdrv_flush_io_queue(BlockDriverState *bs)
>  bdrv_flush_io_queue(bs->file);
>  }
>  }
> +
> +static bool watermark_exceeded(BlockDriverState *bs,
> +   int64_t sector_num,
> +   int nb_sectors)
> +{
> +
> +if (bs->wr_watermark_perc > 0) {
> +int64_t watermark = (bs->total_sectors) / 100 * 
> bs->wr_watermark_perc;

bs->total_sectors should not be used directly.

Have you considered making the watermark parameter take sector units
instead of a percentage?

I'm not sure whether a precentage makes sense because 25% of a 10GB
image is 2.5 GB so a 75% watermark might be reasonable.  25% of a 1 TB
image is 250 GB and that's probably not a reasonable watermark.

So let the block-set-watermark caller pass an absolute sector number
instead.  It keeps things simple for both QEMU and thin provisioning
manager.

> +if (sector_num >= watermark) {
> +return true;
> +}
> +}
> +return false;
> +}
> +
> +static int coroutine_fn watermark_before_write_notify(NotifierWithReturn 
> *notifier,
> +  void *opaque)
> +{
> +BdrvTrackedRequest *req = opaque;
> +int64_t sector_num = req->offset >> BDRV_SECTOR_BITS;
> +int nb_sectors = req->bytes >> BDRV_SECTOR_BITS;
> +
> +/*  FIXME: needed? */
> +assert((req->offset & (BDRV_SECTOR_SIZE - 1)) == 0);
> +assert((req->bytes & (BDRV_SECTOR_SIZE - 1)) == 0);

Not really needed here.  Emulated storage controllers either get
requests in block units (i.e. they are automatically aligned) or check
them (like virtio-blk).

I guess there's no harm in checking, but I would drop it.

> +
> +if (watermark_exceeded(req->bs, sector_num, nb_sectors)) {
> +BlockDriverState *bs = req->bs;
> +qapi_event_send_block_watermark(
> +bdrv_get_device_name(bs),
> +sector_num,
> +bs->wr_highest_sector,
> +&error_abort);

How do you prevent flooding events if every write request exceeds the
watermark?

Perhaps the watermark should be disabled until block-set-watermark is
called again.

> +}
> +
> +return 0; /* should always let other notifiers run */
> +}
> +
> +void bdrv_set_watermark_perc(BlockDriverState *bs,
> + int watermark_perc)
> +{
> +NotifierWithReturn before_write = {
> +.notify = watermark_before_write_notify,
> +};
> +
> +if (watermark_perc <= 0) {
> +return;
> +}
> +
> +if (bs->wr_watermark_perc == 0) {
> +bdrv_add_before_write_notifier(bs, &before_write);

before_write must be a BlockDriverState field so it has the correct
lifetime.  In this patch before_write is allocated on the stack and will
cause invalid memory accesses once we leave this function.


pgpZNt375E3Bs.pgp
Description: PGP signature

[Qemu-devel] "cpu-del" support over and above the RFC patches for cpu-hotunplug

[Anshul Makkar]

Hi,

Attached is the patch for addition of  "cpu-del" command over and
above the below patches for hot removing the cpu.

[RFC PATCH 0/7] i386: add cpu hot remove support
[RFC PATCH 1/7] x86: add x86_cpu_unrealizefn() for cpu apic remove
[RFC PATCH 2/7] i386: add cpu device_del support
[RFC PATCH 3/7] qom cpu: rename variable 'cpu_added_notifier' to
'cpu_hotplug_notifier'
[RFC PATCH 4/7] qom cpu: add UNPLUG cpu notify support
[RFC PATCH 5/7] i386: implement pc interface cpu_common_unrealizefn()
in qom/cpu.c
[RFC PATCH 6/7] cpu hotplug: implement function cpu_status_write() for
vcpu ejection
[RFC PATCH 7/7] cpus: reclaim allocated vCPU objects

Useful, just in case if anyone wants to continue with the old /
compatible "cpu-del" command.

Patch for addition of "cpu-del" command:
+++ b/include/hw/boards.h
@@ -25,6 +25,8 @@ typedef void QEMUMachineResetFunc(void);

 typedef void QEMUMachineHotAddCPUFunc(const int64_t id, Error **errp);

+typedef void QEMUMachineHotDelCPUFunc(const int64_t id, Error **errp);
+
 typedef int QEMUMachineGetKvmtypeFunc(const char *arg);

 struct QEMUMachine {
@@ -34,6 +36,7 @@ struct QEMUMachine {
 QEMUMachineInitFunc *init;
 QEMUMachineResetFunc *reset;
 QEMUMachineHotAddCPUFunc *hot_add_cpu;
+QEMUMachineHotDelCPUFunc *hot_del_cpu;
 QEMUMachineGetKvmtypeFunc *kvm_type;
 BlockInterfaceType block_default_type;
 int max_cpus;
diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
index f2d39d2..33350fc 100644
--- a/include/hw/i386/pc.h
+++ b/include/hw/i386/pc.h
@@ -163,6 +163,7 @@ void pc_acpi_smi_interrupt(void *opaque, int irq,
int level);

 void pc_cpus_init(const char *cpu_model, DeviceState *icc_bridge);
 void pc_hot_add_cpu(const int64_t id, Error **errp);
+void pc_hot_del_cpu(const int64_t id, Error **errp);
 void pc_acpi_init(const char *default_dsdt);

 PcGuestInfo *pc_guest_info_init(ram_addr_t below_4g_mem_size,
@@ -472,6 +473,7 @@ int e820_add_entry(uint64_t, uint64_t, uint32_t);
 #define PC_DEFAULT_MACHINE_OPTIONS \
 PC_COMMON_MACHINE_OPTIONS, \
 .hot_add_cpu = pc_hot_add_cpu, \
+.hot_del_cpu = pc_hot_del_cpu, \
 .max_cpus = 255

 #endif
diff --git a/qmp.c b/qmp.c
index 95369f9..fc494da 100644
--- a/qmp.c
+++ b/qmp.c
@@ -126,6 +126,18 @@ void qmp_cpu_add(int64_t id, Error **errp)
 }
 }

+void qmp_cpu_del(int64_t id, Error **errp)
+{
+MachineClass *mc;
+mc = MACHINE_GET_CLASS(current_machine);
+if (mc->qemu_machine->hot_del_cpu) {
+mc->qemu_machine->hot_del_cpu(id, errp);
+}
+else {
+error_setg(errp, "Not supported");
+}
+}
+
 #ifndef CONFIG_VNC
 /* If VNC support is enabled, the "true" query-vnc command is
defined in the VNC subsystem */

hw/i386/pc.c
+void pc_hot_del_cpu(const int64_t id, Error **errp)
+{
+   int64_t apic_id = x86_cpu_apic_id_from_index(id);
+fprintf(stderr, "pc.c: pc_hot_del_cpu for apic_id = %d\n", apic_id);
+
+if (id < 0) {
+error_setg(errp, "Invalid CPU id: %" PRIi64, id);
+return;
+}
+
+if (!cpu_exists(apic_id)) {
+error_setg(errp, "Unable to remove CPU: %" PRIi64
+   ", it does not exists", id);
+return;
+   }
+
+if (id >= max_cpus) {
+error_setg(errp, "Unable to remove CPU: %" PRIi64
+   ", max allowed: %d", id, max_cpus - 1);
+return;
+}
+
+CPUState *cpu = first_cpu;
+X86CPUClass *xcc = NULL;

+while (cpu = CPU_NEXT(cpu)) {
+fprintf(stderr, "cpu threa_id = %d, cpu_index = %d\n",
cpu->thread_id, cpu->cpu_index);
+if ((cpu->cpu_index + 1) == apic_id)
+break;
+}

if (cpu == first_cpu) {
fprintf(stderr, "Unable to delete the last one cpu.\n");
return;
}
xcc = X86_CPU_GET_CLASS(DEVICE(cpu));
xcc->parent_unrealize(DEVICE(cpu), errp);
}

Anshul Makkar
wwwdotjustkerneldotcom

Re: [Qemu-devel] [PATCH] arm64: 64K pages and > 1024MB guest

[Paolo Bonzini]

Il 01/08/2014 13:28, Peter Maydell ha scritto:
> Paolo: can you review this? Do we also need to do something
> about the use of TARGET_PAGE_BITS in
> kvm_physical_sync_dirty_bitmap?

No, it should be the host page size, matching
cpu_physical_memory_set_dirty_lebitmap.

> Is it really OK to define
> PAGE_SIZE to TARGET_PAGE_SIZE (it's certainly really
> misleading and suggests the kernel headers could be more
> helpful).

No, it's wrong.

> Basically I think kvm-all.c should almost certainly not be
> using any of the TARGET_PAGE_* constants anywhere.

I agree.

I think the patch is right but, besides these considerations, does this
bug still manifest itself after Andrew fixed the start address of the
device at 0x9001 (IIRC it was the pl031)?

Paolo

Re: [Qemu-devel] [PATCH v2 1/3] trace: teach lttng backend to use format strings

[Stefan Hajnoczi]

On Fri, Aug 01, 2014 at 10:05:30AM +0100, Alex Bennée wrote:
> 
> Alex Bennée writes:
> 
> > This makes the UST backend pay attention to the format string arguments
> > that are defined when defining payload data. With this you can now
> > ensure integers are reported in hex mode if you want.
> >
> 
> 
> Ping Stefan, can this one at least be slurped up into your tracing tree?

Yes, but please rebase onto the tracing-next branch (for QEMU 2.2):

https://github.com/stefanha/qemu/tree/tracing-next

Stefan


pgpsqhCnoU7ww.pgp
Description: PGP signature

Re: [Qemu-devel] [PATCH 1/3] query-memdev: fix potential memory leaks

[Peter Crosthwaite]

On Fri, Aug 1, 2014 at 8:22 PM, Chen Fan  wrote:
> Signed-off-by: Chen Fan 

Reviewed-by: Peter Crosthwaite 

> ---
>  numa.c | 6 +-
>  1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/numa.c b/numa.c
> index 7bf7834..a2b4bca 100644
> --- a/numa.c
> +++ b/numa.c
> @@ -318,10 +318,11 @@ void memory_region_allocate_system_memory(MemoryRegion 
> *mr, Object *owner,
>  static int query_memdev(Object *obj, void *opaque)
>  {
>  MemdevList **list = opaque;
> +MemdevList *m = NULL;
>  Error *err = NULL;
>
>  if (object_dynamic_cast(obj, TYPE_MEMORY_BACKEND)) {
> -MemdevList *m = g_malloc0(sizeof(*m));
> +m = g_malloc0(sizeof(*m));
>
>  m->value = g_malloc0(sizeof(*m->value));
>
> @@ -369,6 +370,9 @@ static int query_memdev(Object *obj, void *opaque)
>
>  return 0;
>  error:
> +g_free(m->value);
> +g_free(m);
> +
>  return -1;
>  }
>
> --
> 1.9.3
>
>

Re: [Qemu-devel] [PATCH 01/15] qemu coroutine: support bypass mode

[Stefan Hajnoczi]

On Fri, Aug 01, 2014 at 10:54:02AM +0800, Ming Lei wrote:
> On Fri, Aug 1, 2014 at 12:30 AM, Paolo Bonzini  wrote:
> > Il 31/07/2014 18:13, Ming Lei ha scritto:
> >> Follows 'perf report' result on cycles event for with/without bypass
> >> coroutine:
> >>
> >> http://pastebin.com/ae0vnQ6V
> >>
> >> From the profiling result, looks bdrv_co_do_preadv() is a bit slow
> >> without bypass coroutine.
> >
> > Yeah, I can count at least 3.3% time spent here:
> >
> > 0.87%  bdrv_co_do_preadv
> > 0.79%  bdrv_aligned_preadv
> > 0.71%  qemu_coroutine_switch
> > 0.52%  tracked_request_begin
> > 0.45%  coroutine_swap
> >
> > Another ~3% wasted in malloc, etc.
> 
> That should be related with coroutine and the BH in bdrv_co_do_rw().
> In this post I didn't apply Stephan's coroutine resize patch which might
> decrease usage of malloc() for coroutine.

Please rerun with "[PATCH v3 0/2] coroutine: dynamically scale pool
size".

> At least, coroutine isn't cheap from the profile result.

Instead of bypassing coroutines we should first understand the overhead
that they impose.  Is it due to the coroutine implementation (switching
stacks) or due to the bdrv_co_*() code that happens to use coroutines
but slow for other reasons.


pgpLT0E6T5BUR.pgp
Description: PGP signature

Re: [Qemu-devel] [PATCH v3 0/2] coroutine: dynamically scale pool size

[Stefan Hajnoczi]

On Mon, Jul 07, 2014 at 03:15:51PM +0200, Stefan Hajnoczi wrote:
> v3:
>  * Use COROUTINE_POOL_RESERVATION constant in block.c [Lluis]
> 
> v2:
>  * Assert that callers never reduce pool below default size [eblake]
> 
> The coroutine pool reuses exited coroutines to make qemu_coroutine_create()
> cheap.  The size of the pool is capped to prevent it from hogging memory after
> a period of high coroutine activity.  Previously the max size was hardcoded to
> 64 but this doesn't scale with guest size.
> 
> A guest with lots of disks can do more parallel I/O and therefore requires a
> larger coroutine pool size.  This series tries to solve the problem by scaling
> pool size according to the number of drives.
> 
> Ming has confirmed that this patch series, together with his block plug/unplug
> series, solves the dataplane performance regression in QEMU 2.1.
> 
> Stefan Hajnoczi (2):
>   coroutine: make pool size dynamic
>   block: bump coroutine pool size for drives
> 
>  block.c   |  6 ++
>  include/block/coroutine.h | 11 +++
>  qemu-coroutine.c  | 26 +++---
>  3 files changed, 40 insertions(+), 3 deletions(-)

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

Stefan


pgpTgsmHbRjd0.pgp
Description: PGP signature

Re: [Qemu-devel] [PATCH 2/3] qom/object.c: fix string_output_get_string() memory leak

[Peter Crosthwaite]

On Fri, Aug 1, 2014 at 8:22 PM, Chen Fan  wrote:
> string_output_get_string() always return the data the sov->string

"returns the data sov->string points to and never frees it"

Although "sov" is a little out of context however of this change, and
you need to go diving into qapi code to get context on what you mean
by that. Perhaps just say it uses g_string_free which requires callers
to free the returned data?

> point. and never free.
>
> Signed-off-by: Chen Fan 
> ---
>  hmp.c|  6 --
>  qom/object.c | 11 ---
>  2 files changed, 12 insertions(+), 5 deletions(-)
>
> diff --git a/hmp.c b/hmp.c
> index 4d1838e..2414cc7 100644
> --- a/hmp.c
> +++ b/hmp.c
> @@ -1687,6 +1687,7 @@ void hmp_info_memdev(Monitor *mon, const QDict *qdict)
>  MemdevList *memdev_list = qmp_query_memdev(&err);
>  MemdevList *m = memdev_list;
>  StringOutputVisitor *ov;
> +char *str;
>  int i = 0;
>
>
> @@ -1704,10 +1705,11 @@ void hmp_info_memdev(Monitor *mon, const QDict *qdict)
> m->value->prealloc ? "true" : "false");
>  monitor_printf(mon, "  policy: %s\n",
> HostMemPolicy_lookup[m->value->policy]);
> -monitor_printf(mon, "  host nodes: %s\n",
> -   string_output_get_string(ov));
> +str = string_output_get_string(ov);
> +monitor_printf(mon, "  host nodes: %s\n", str);
>
>  string_output_visitor_cleanup(ov);
> +g_free(str);
>  m = m->next;
>  i++;
>  }
> diff --git a/qom/object.c b/qom/object.c
> index 0e8267b..7ae4f33 100644
> --- a/qom/object.c
> +++ b/qom/object.c
> @@ -948,16 +948,18 @@ int object_property_get_enum(Object *obj, const char 
> *name,
>  {
>  StringOutputVisitor *sov;
>  StringInputVisitor *siv;
> +char *str;
>  int ret;
>
>  sov = string_output_visitor_new(false);
>  object_property_get(obj, string_output_get_visitor(sov), name, errp);
> -siv = string_input_visitor_new(string_output_get_string(sov));
> +str = string_output_get_string(sov);
> +siv = string_input_visitor_new(str);

Free here (ASAP) instead of below.

>  string_output_visitor_cleanup(sov);
>  visit_type_enum(string_input_get_visitor(siv),
>  &ret, strings, NULL, name, errp);
>  string_input_visitor_cleanup(siv);
> -

Don't delete existing blank lines that separate sections like this.

> +g_free(str);
>  return ret;
>  }
>
> @@ -966,15 +968,18 @@ void object_property_get_uint16List(Object *obj, const 
> char *name,
>  {
>  StringOutputVisitor *ov;
>  StringInputVisitor *iv;
> +char *str;
>
>  ov = string_output_visitor_new(false);
>  object_property_get(obj, string_output_get_visitor(ov),
>  name, errp);
> -iv = string_input_visitor_new(string_output_get_string(ov));
> +str = string_output_get_string(ov);
> +iv = string_input_visitor_new(str);

Ditto on the ASAP free

Regards,
Peter

>  visit_type_uint16List(string_input_get_visitor(iv),
>list, NULL, errp);
>  string_output_visitor_cleanup(ov);
>  string_input_visitor_cleanup(iv);
> +g_free(str);
>  }
>
>  void object_property_parse(Object *obj, const char *string,
> --
> 1.9.3
>
>

Re: [Qemu-devel] [PATCH v3 07/16] target-arm: Add HCR_EL2

[Peter Maydell]

On 17 June 2014 09:45, Edgar E. Iglesias  wrote:
> From: "Edgar E. Iglesias" 
>
> Signed-off-by: Edgar E. Iglesias 

> diff --git a/target-arm/helper.c b/target-arm/helper.c
> index 7170086..b04fb5d 100644
> --- a/target-arm/helper.c
> +++ b/target-arm/helper.c
> @@ -2107,10 +2107,36 @@ static const ARMCPRegInfo v8_el3_no_el2_cp_reginfo[] 
> = {
>.opc0 = 3, .opc1 = 4, .crn = 12, .crm = 0, .opc2 = 0,
>.access = PL2_RW,
>.readfn = arm_cp_read_zero, .writefn = arm_cp_write_ignore },
> +{ .name = "HCR_EL2", .state = ARM_CP_STATE_AA64,
> +  .type = ARM_CP_NO_MIGRATE,
> +  .opc0 = 3, .opc1 = 4, .crn = 1, .crm = 1, .opc2 = 0,
> +  .readfn = arm_cp_read_zero, .writefn = arm_cp_write_ignore },

Isn't this missing the .access specifier ?

>  REGINFO_SENTINEL
>  };
>
> +static void hcr_write(CPUARMState *env, const ARMCPRegInfo *ri, uint64_t 
> value)
> +{
> +ARMCPU *cpu = arm_env_get_cpu(env);
> +uint64_t valid_mask = HCR_MASK;
> +
> +if (!arm_feature(env, ARM_FEATURE_EL3)) {
> +valid_mask &= ~HCR_HCD;
> +}

This is inconsistent. HCD isn't the only bit that's "RES0 if
EL3 unimplemented"; TSC is as well, for instance.
(In fact the RES0 definition means you don't actually have
to mask this out unless it's more convenient to do so.)

> +
> +/* Clear RES0 bits.  */
> +value &= valid_mask;
> +
> +if ((raw_read(env, ri) ^ value) & (HCR_VM | HCR_RW | HCR_PTW | HCR_DC)) {
> +tlb_flush(CPU(cpu), 1);

Could maybe use a comment about why we need a TLB flush.

> +}
> +raw_write(env, ri, value);
> +}
> +
>  static const ARMCPRegInfo v8_el2_cp_reginfo[] = {
> +{ .name = "HCR_EL2", .state = ARM_CP_STATE_AA64,
> +  .opc0 = 3, .opc1 = 4, .crn = 1, .crm = 1, .opc2 = 0,
> +  .access = PL2_RW, .fieldoffset = offsetof(CPUARMState, cp15.hcr_el2),
> +  .writefn = hcr_write },
>  { .name = "ELR_EL2", .state = ARM_CP_STATE_AA64,
>.type = ARM_CP_NO_MIGRATE,
>.opc0 = 3, .opc1 = 4, .crn = 4, .crm = 0, .opc2 = 1,
> --

thanks
-- PMM

Re: [Qemu-devel] [PATCH v3 08/16] target-arm: Add SCR_EL3

[Peter Maydell]

On 17 June 2014 09:45, Edgar E. Iglesias  wrote:
> From: "Edgar E. Iglesias" 
>
> Signed-off-by: Edgar E. Iglesias 
> ---
>  target-arm/cpu.h| 16 +++-
>  target-arm/helper.c | 31 ++-
>  2 files changed, 45 insertions(+), 2 deletions(-)
>
> diff --git a/target-arm/cpu.h b/target-arm/cpu.h
> index fd57fb5..fa8dee0 100644
> --- a/target-arm/cpu.h
> +++ b/target-arm/cpu.h
> @@ -172,7 +172,6 @@ typedef struct CPUARMState {
>  uint64_t c1_sys; /* System control register.  */
>  uint64_t c1_coproc; /* Coprocessor access register.  */
>  uint32_t c1_xscaleauxcr; /* XScale auxiliary control register.  */
> -uint32_t c1_scr; /* secure config register.  */
>  uint64_t ttbr0_el1; /* MMU translation table base 0. */
>  uint64_t ttbr1_el1; /* MMU translation table base 1. */
>  uint64_t c2_control; /* MMU translation table base control.  */
> @@ -185,6 +184,7 @@ typedef struct CPUARMState {
>  uint32_t pmsav5_data_ap; /* PMSAv5 MPU data access permissions */
>  uint32_t pmsav5_insn_ap; /* PMSAv5 MPU insn access permissions */
>  uint64_t hcr_el2; /* Hypervisor configuration register */
> +uint32_t scr_el3; /* Secure configuration register.  */
>  uint32_t ifsr_el2; /* Fault status registers.  */
>  uint64_t esr_el[4];
>  uint32_t c6_region[8]; /* MPU base/size registers.  */
> @@ -562,6 +562,20 @@ static inline void xpsr_write(CPUARMState *env, uint32_t 
> val, uint32_t mask)
>  #define HCR_ID(1ULL << 33)
>  #define HCR_MASK  ((1ULL << 34) - 1)
>
> +#define SCR_NS(1U << 0)
> +#define SCR_IRQ   (1U << 1)
> +#define SCR_FIQ   (1U << 2)
> +#define SCR_EA(1U << 3)
> +#define SCR_SMD   (1U << 7)
> +#define SCR_HCE   (1U << 8)
> +#define SCR_SIF   (1U << 9)
> +#define SCR_RW(1U << 10)
> +#define SCR_ST(1U << 11)
> +#define SCR_TWI   (1U << 12)
> +#define SCR_TWE   (1U << 13)
> +#define SCR_RES1_MASK (3U << 4)
> +#define SCR_MASK  (0x3fff & ~SCR_RES1_MASK)
> +
>  /* Return the current FPSCR value.  */
>  uint32_t vfp_get_fpscr(CPUARMState *env);
>  void vfp_set_fpscr(CPUARMState *env, uint32_t val);
> diff --git a/target-arm/helper.c b/target-arm/helper.c
> index b04fb5d..6bacc24 100644
> --- a/target-arm/helper.c
> +++ b/target-arm/helper.c
> @@ -793,7 +793,7 @@ static const ARMCPRegInfo v7_cp_reginfo[] = {
>.fieldoffset = offsetof(CPUARMState, cp15.vbar_el[1]),
>.resetvalue = 0 },
>  { .name = "SCR", .cp = 15, .crn = 1, .crm = 1, .opc1 = 0, .opc2 = 0,
> -  .access = PL1_RW, .fieldoffset = offsetof(CPUARMState, cp15.c1_scr),
> +  .access = PL1_RW, .fieldoffset = offsetof(CPUARMState, cp15.scr_el3),
>.resetvalue = 0, },

It's awkward that this is now separate from the AArch64 reginfo
below, because it makes it non-obvious that they're both the
same underlying state. In particular that probably means this
one now needs a NO_MIGRATE marker?

>  { .name = "CCSIDR", .state = ARM_CP_STATE_BOTH,
>.opc0 = 3, .crn = 0, .crm = 0, .opc1 = 1, .opc2 = 0,
> @@ -2161,6 +2161,31 @@ static const ARMCPRegInfo v8_el2_cp_reginfo[] = {
>  REGINFO_SENTINEL
>  };
>
> +static void scr_write(CPUARMState *env, const ARMCPRegInfo *ri, uint64_t 
> value)
> +{
> +uint32_t valid_mask = SCR_MASK;
> +
> +if (!arm_feature(env, ARM_FEATURE_EL2)) {
> +valid_mask &= ~SCR_HCE;
> +
> +/* On ARMv7, SMD (or SCD as it is called in v7) is only
> + * supported if EL2 exists. The bit is UNK/SBZP when
> + * EL2 is unavailable. In QEMU ARMv7, we force it to always zero
> + * when EL2 is unavailable.
> + */
> +if (arm_feature(env, ARM_FEATURE_V7)) {
> +valid_mask &= ~SCR_SMD;
> +}
> +}
> +
> +/* Set RES1 bits.  */
> +value |= SCR_RES1_MASK;
> +
> +/* Clear RES0 bits.  */
> +value &= valid_mask;
> +raw_write(env, ri, value);
> +}
> +
>  static const ARMCPRegInfo v8_el3_cp_reginfo[] = {
>  { .name = "ELR_EL3", .state = ARM_CP_STATE_AA64,
>.type = ARM_CP_NO_MIGRATE,
> @@ -2183,6 +2208,10 @@ static const ARMCPRegInfo v8_el3_cp_reginfo[] = {
>.access = PL3_RW, .writefn = vbar_write,
>.fieldoffset = offsetof(CPUARMState, cp15.vbar_el[3]),
>.resetvalue = 0 },
> +{ .name = "SCR_EL3", .state = ARM_CP_STATE_AA64,
> +  .opc0 = 3, .opc1 = 6, .crn = 1, .crm = 1, .opc2 = 0,
> +  .access = PL3_RW, .fieldoffset = offsetof(CPUARMState, cp15.scr_el3),
> +  .writefn = scr_write },
>  REGINFO_SENTINEL
>  };

thanks
-- PMM

Re: [Qemu-devel] [PATCH v4 1/8] bootindex: add modify_boot_device_path function

[Eduardo Habkost]

On Thu, Jul 31, 2014 at 05:47:26PM +0800, arei.gong...@huawei.com wrote:
[...]
> +void modify_boot_device_path(int32_t bootindex, DeviceState *dev,
> + const char *suffix)
> +{
> +FWBootEntry *i, *old_entry = NULL;
> +
> +assert(dev != NULL || suffix != NULL);
> +
> +if (bootindex >= 0) {
> +QTAILQ_FOREACH(i, &fw_boot_order, link) {
> +if (i->bootindex == bootindex) {
> +qerror_report(ERROR_CLASS_GENERIC_ERROR,
> +  "The bootindex %d has already been used",
> +  bootindex);

Isn't an Error** parameter preferable here, instead of using qerror_report()?

> +return;
> +}
> +}
> +}
[...]

-- 
Eduardo

Re: [Qemu-devel] [PATCH 01/15] qemu coroutine: support bypass mode

[Ming Lei]

On Thu, Jul 31, 2014 at 6:45 PM, Paolo Bonzini  wrote:
> Il 31/07/2014 11:47, Ming Lei ha scritto:
>>> Block mirroring of a device for example is done using coroutines.
>>> As block mirroring can be done on a raw device you need coroutines.
>>
>> If block layer knows the mirroring is in-progress, it still can enable
>> coroutine by ignoring bypass coroutine, or just let device disable
>> bypass coroutine in case of mirroring, and the current APIs are very
>> flexible.
>
> What matters is not whether you're mirroring.  What matters is whether
> you're calling bdrv_aio_readv/writev or bdrv_co_readv/writev.  Under
> some limitation, if you are calling bdrv_aio_readv/writev you can bypass
> coroutines.  (In fact drive mirroring uses those APIs too so it doesn't
> need coroutines and can benefit from the speedup too. :)  But
> drive-backup does need them).
>
> The limitations are essentially "would bdrv_co_do_preadv or
> bdrv_co_do_pwritev do anything special?"  This means for example for
> bdrv_co_do_pwritev:
>
> - bs->drv must be non-NULL
>
> - bs->read_only must be false
>
> - bdrv_check_byte_request(bs, offset, bytes) must return false
>
> - bs->io_limits_enabled must be false
>
> - the request must be aligned
>
> - (in bdrv_aligned_pwritev) the before_write_notifiers must be empty
>
> - (in bdrv_aligned_pwritev) bs->detect_zeroes must be off
>
> - (in bdrv_aligned_pwritev) the BDRV_REQ_ZERO_WRITE flag must be off
>
> - (in bdrv_aligned_pwritev) bs->enable_write_cache must be false
>
> and the hard part is organizing the code so that the code duplication is
> as limited as possible.

In this patchset, only implementation of bypassing coroutine is
introduced, and simply apply it on raw image for dataplane, and it
won't break drive mirror & backup, if I understand correctly. So I
suggest we focus on the implementation of bypassing coroutine.

All the above is mostly about if the bypass can be applied in
other cases, so this patchset shouldn't have caused code duplication.

I appreciate very much the implementation of bypassing coroutine
can be reviewed in the discussion.

Thanks,

Re: [Qemu-devel] [PATCH 3/3] hmp: fix MemdevList memory leak

[Peter Crosthwaite]

On Fri, Aug 1, 2014 at 8:22 PM, Chen Fan  wrote:
> Signed-off-by: Chen Fan 
> ---
>  hmp.c | 9 ++---
>  1 file changed, 6 insertions(+), 3 deletions(-)
>
> diff --git a/hmp.c b/hmp.c
> index 2414cc7..0b1c830 100644
> --- a/hmp.c
> +++ b/hmp.c
> @@ -1685,13 +1685,14 @@ void hmp_info_memdev(Monitor *mon, const QDict *qdict)
>  {
>  Error *err = NULL;
>  MemdevList *memdev_list = qmp_query_memdev(&err);
> -MemdevList *m = memdev_list;
> +MemdevList *m;
>  StringOutputVisitor *ov;
>  char *str;
>  int i = 0;
>
>
> -while (m) {
> +while (memdev_list) {
> +m = memdev_list;
>  ov = string_output_visitor_new(false);
>  visit_type_uint16List(string_output_get_visitor(ov),
>&m->value->host_nodes, NULL, NULL);
> @@ -1710,7 +1711,9 @@ void hmp_info_memdev(Monitor *mon, const QDict *qdict)
>
>  string_output_visitor_cleanup(ov);
>  g_free(str);
> -m = m->next;
> +memdev_list = memdev_list->next;
> +g_free(m->value);
> +g_free(m);

This doesnt feel quite right. You are piecewise freeing an entire data
structure as allocated by a single function (qmp_query_memdev). I
think you should create a function that cleans up MemdevList (just
loops and frees) so that any and all callers of qmp_query_memdev can
cleanup in one single action.

Note that qmp_query_memdev already has the code you need in it's error path:

while (list) {
m = list;
list = list->next;
g_free(m->value);
g_free(m);
}

So you can split this out into it's own fn and call it both here and
in that error path.

Regards,
Peter

>  i++;
>  }
>
> --
> 1.9.3
>
>

[Qemu-devel] [PATCH 0/3] vpc: support probing of fixed size images

[Levente Kurusa]

Fixed size VPC images do not have a footer, hence the current probe
function will fail and QEMU will fall back to the raw_bsd driver, which is
not the correct behaviour. The specification of the format says that fixed
size images have a footer as the last 512 bytes of the file. The footer is
exactly the same as the header would be in the case of dynamically growing
images.

For this, we need to read the last 512 bytes of the image, however the
current mechanics predominantly read the first 2048 bytes and pass that
as a buffer to the probe functions. Solve this by passing the
BlockDriverState to the probe functions, hence giving them a chance to read
the extra bytes they might need.

Levente Kurusa (3):
  block: format: pass down the current state to the format's probe
function
  block: vpc: introduce vpc_check_signature function
  block: vpc: handle fixed size images in probe function

 block.c   |  2 +-
 block/bochs.c |  3 ++-
 block/cloop.c |  3 ++-
 block/cow.c   |  3 ++-
 block/dmg.c   |  3 ++-
 block/parallels.c |  3 ++-
 block/qcow.c  |  3 ++-
 block/qcow2.c |  3 ++-
 block/qed.c   |  4 ++--
 block/raw_bsd.c   |  3 ++-
 block/vdi.c   |  3 ++-
 block/vmdk.c  |  3 ++-
 block/vpc.c   | 34 +-
 include/block/block_int.h |  3 ++-
 14 files changed, 54 insertions(+), 19 deletions(-)

-- 
1.9.3

[Qemu-devel] [PATCH 2/3] block: vpc: introduce vpc_check_signature function

[Levente Kurusa]

Check the signature in a helper function instead of in plain
code in order to avoid code duplication

Reviewed-by: Andrew Jones 
Signed-off-by: Levente Kurusa 
---
 block/vpc.c | 10 +++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/block/vpc.c b/block/vpc.c
index 2ba8fc2..a6a7213 100644
--- a/block/vpc.c
+++ b/block/vpc.c
@@ -156,11 +156,15 @@ static uint32_t vpc_checksum(uint8_t* buf, size_t size)
 return ~res;
 }
 
+static int vpc_check_signature(const void *buf)
+{
+return !strncmp((char *)buf, "conectix", 8);
+}
 
 static int vpc_probe(BlockDriverState *bs, const uint8_t *buf, int buf_size,
  const char *filename)
 {
-if (buf_size >= 8 && !strncmp((char *)buf, "conectix", 8))
+if (buf_size >= 8 && vpc_check_signature(buf))
return 100;
 return 0;
 }
@@ -184,7 +188,7 @@ static int vpc_open(BlockDriverState *bs, QDict *options, 
int flags,
 }
 
 footer = (VHDFooter *) s->footer_buf;
-if (strncmp(footer->creator, "conectix", 8)) {
+if (!vpc_check_signature(footer->creator)) {
 int64_t offset = bdrv_getlength(bs->file);
 if (offset < 0) {
 ret = offset;
@@ -200,7 +204,7 @@ static int vpc_open(BlockDriverState *bs, QDict *options, 
int flags,
 if (ret < 0) {
 goto fail;
 }
-if (strncmp(footer->creator, "conectix", 8)) {
+if (!vpc_check_signature(footer->creator)) {
 error_setg(errp, "invalid VPC image");
 ret = -EINVAL;
 goto fail;
-- 
1.9.3

Re: [Qemu-devel] [PATCH v4 1/8] bootindex: add modify_boot_device_path function

[Luiz Capitulino]

On Fri, 1 Aug 2014 10:36:18 -0300
Eduardo Habkost  wrote:

> On Thu, Jul 31, 2014 at 05:47:26PM +0800, arei.gong...@huawei.com wrote:
> [...]
> > +void modify_boot_device_path(int32_t bootindex, DeviceState *dev,
> > + const char *suffix)
> > +{
> > +FWBootEntry *i, *old_entry = NULL;
> > +
> > +assert(dev != NULL || suffix != NULL);
> > +
> > +if (bootindex >= 0) {
> > +QTAILQ_FOREACH(i, &fw_boot_order, link) {
> > +if (i->bootindex == bootindex) {
> > +qerror_report(ERROR_CLASS_GENERIC_ERROR,
> > +  "The bootindex %d has already been used",
> > +  bootindex);
> 
> Isn't an Error** parameter preferable here, instead of using qerror_report()?

Good catch. I'm not following this series, but using qerror_report() is
almost always wrong these days.

[Qemu-devel] [PATCH 1/3] block: format: pass down the current state to the format's probe function

[Levente Kurusa]

While most ->probe functions are content with reading the first 2048
bytes of their disk, there are some (namely VPC) which might need to
know more about the disk. Pass down the BlockDriverState to the probe
functions, so that the drivers can read more data should they need it.

Reviewed-by: Andrew Jones 
Signed-off-by: Levente Kurusa 
---
 block.c   | 2 +-
 block/bochs.c | 3 ++-
 block/cloop.c | 3 ++-
 block/cow.c   | 3 ++-
 block/dmg.c   | 3 ++-
 block/parallels.c | 3 ++-
 block/qcow.c  | 3 ++-
 block/qcow2.c | 3 ++-
 block/qed.c   | 4 ++--
 block/raw_bsd.c   | 3 ++-
 block/vdi.c   | 3 ++-
 block/vmdk.c  | 3 ++-
 block/vpc.c   | 3 ++-
 include/block/block_int.h | 3 ++-
 14 files changed, 27 insertions(+), 15 deletions(-)

diff --git a/block.c b/block.c
index 8cf519b..90dd8f0 100644
--- a/block.c
+++ b/block.c
@@ -683,7 +683,7 @@ static int find_image_format(BlockDriverState *bs, const 
char *filename,
 drv = NULL;
 QLIST_FOREACH(drv1, &bdrv_drivers, list) {
 if (drv1->bdrv_probe) {
-score = drv1->bdrv_probe(buf, ret, filename);
+score = drv1->bdrv_probe(bs, buf, ret, filename);
 if (score > score_max) {
 score_max = score;
 drv = drv1;
diff --git a/block/bochs.c b/block/bochs.c
index eba23df..9e445f4 100644
--- a/block/bochs.c
+++ b/block/bochs.c
@@ -76,7 +76,8 @@ typedef struct BDRVBochsState {
 uint32_t extent_size;
 } BDRVBochsState;
 
-static int bochs_probe(const uint8_t *buf, int buf_size, const char *filename)
+static int bochs_probe(BlockDriverState *bs, const uint8_t *buf, int buf_size,
+   const char *filename)
 {
 const struct bochs_header *bochs = (const void *)buf;
 
diff --git a/block/cloop.c b/block/cloop.c
index 8457737..f707699 100644
--- a/block/cloop.c
+++ b/block/cloop.c
@@ -41,7 +41,8 @@ typedef struct BDRVCloopState {
 z_stream zstream;
 } BDRVCloopState;
 
-static int cloop_probe(const uint8_t *buf, int buf_size, const char *filename)
+static int cloop_probe(BlockDriverState *bs, const uint8_t *buf, int buf_size,
+   const char *filename)
 {
 const char *magic_version_2_0 = "#!/bin/sh\n"
 "#V2.0 Format\n"
diff --git a/block/cow.c b/block/cow.c
index 6ee4833..6002c62 100644
--- a/block/cow.c
+++ b/block/cow.c
@@ -46,7 +46,8 @@ typedef struct BDRVCowState {
 int64_t cow_sectors_offset;
 } BDRVCowState;
 
-static int cow_probe(const uint8_t *buf, int buf_size, const char *filename)
+static int cow_probe(BlockDriverState *bs, const uint8_t *buf, int buf_size,
+ const char *filename)
 {
 const struct cow_header_v2 *cow_header = (const void *)buf;
 
diff --git a/block/dmg.c b/block/dmg.c
index 1e153cd..962c4fc 100644
--- a/block/dmg.c
+++ b/block/dmg.c
@@ -57,7 +57,8 @@ typedef struct BDRVDMGState {
 z_stream zstream;
 } BDRVDMGState;
 
-static int dmg_probe(const uint8_t *buf, int buf_size, const char *filename)
+static int dmg_probe(BlockDriverState *bs, const uint8_t *buf, int buf_size,
+ const char *filename)
 {
 int len;
 
diff --git a/block/parallels.c b/block/parallels.c
index 1a5bd35..157d0a0 100644
--- a/block/parallels.c
+++ b/block/parallels.c
@@ -54,7 +54,8 @@ typedef struct BDRVParallelsState {
 unsigned int tracks;
 } BDRVParallelsState;
 
-static int parallels_probe(const uint8_t *buf, int buf_size, const char 
*filename)
+static int parallels_probe(BlockDriverState *bs, const uint8_t *buf,
+   int buf_size, const char *filename)
 {
 const struct parallels_header *ph = (const void *)buf;
 
diff --git a/block/qcow.c b/block/qcow.c
index a874056..acd126a 100644
--- a/block/qcow.c
+++ b/block/qcow.c
@@ -81,7 +81,8 @@ typedef struct BDRVQcowState {
 
 static int decompress_cluster(BlockDriverState *bs, uint64_t cluster_offset);
 
-static int qcow_probe(const uint8_t *buf, int buf_size, const char *filename)
+static int qcow_probe(BlockDriverState *bs, const uint8_t *buf, int buf_size,
+  const char *filename)
 {
 const QCowHeader *cow_header = (const void *)buf;
 
diff --git a/block/qcow2.c b/block/qcow2.c
index 1e3ab6b..14593db 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -59,7 +59,8 @@ typedef struct {
 #define  QCOW2_EXT_MAGIC_BACKING_FORMAT 0xE2792ACA
 #define  QCOW2_EXT_MAGIC_FEATURE_TABLE 0x6803f857
 
-static int qcow2_probe(const uint8_t *buf, int buf_size, const char *filename)
+static int qcow2_probe(BlockDriverState *bs, const uint8_t *buf, int buf_size,
+   const char *filename)
 {
 const QCowHeader *cow_header = (const void *)buf;
 
diff --git a/block/qed.c b/block/qed.c
index 7944832..7b492e9 100644
--- a/block/qed.c
+++ b/block/qed.c
@@ -36,8 +36,8 @@ static const AIOCBInfo qed_aiocb_info = {
 .cancel = qed_aio_can

Re: [Qemu-devel] [PATCH v2 0/2] thread-pool: avoid fd usage and fix nested aio_poll() deadlock

[Stefan Hajnoczi]

On Tue, Jul 15, 2014 at 04:44:24PM +0200, Stefan Hajnoczi wrote:
> v2:
>  * Leave BH scheduled so that the code can be simplified [Paolo]
> 
> These patches convert thread-pool.c from EventNotifier to QEMUBH.  They then
> solve the deadlock when nested aio_poll() calls are made.
> 
> Please speak out whether you want this in QEMU 2.1 or not.  I'm not aware of
> the nested aio_poll() deadlock ever having been reported, so maybe we can 
> defer
> to QEMU 2.2.
> 
> Stefan Hajnoczi (2):
>   thread-pool: avoid per-thread-pool EventNotifier
>   thread-pool: avoid deadlock in nested aio_poll() calls
> 
>  thread-pool.c | 27 ++-
>  1 file changed, 14 insertions(+), 13 deletions(-)

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

Stefan


pgpLNakuwXeT9.pgp
Description: PGP signature

[Qemu-devel] [PATCH 3/3] block: vpc: handle fixed size images in probe function

[Levente Kurusa]

Fixed size images do not have a header, only dynamic images have that.
This type uses a footer, which is the same structure in the last 512
bytes of the image. We need to parse that too to be able to recognize
fixed length images, so check there as well.

Reviewed-by: Andrew Jones 
Signed-off-by: Levente Kurusa 
---
 block/vpc.c | 23 +--
 1 file changed, 21 insertions(+), 2 deletions(-)

diff --git a/block/vpc.c b/block/vpc.c
index a6a7213..b12354a 100644
--- a/block/vpc.c
+++ b/block/vpc.c
@@ -164,8 +164,27 @@ static int vpc_check_signature(const void *buf)
 static int vpc_probe(BlockDriverState *bs, const uint8_t *buf, int buf_size,
  const char *filename)
 {
-if (buf_size >= 8 && vpc_check_signature(buf))
-   return 100;
+char sig[8];
+
+if (buf_size < 8) {
+return 0;
+}
+
+if (vpc_check_signature(buf)) {
+return 100;
+}
+
+/*
+ * Don't give up just yet, since the spec say that only dynamic
+ * images have a header (which in fact is a copy of the footer).
+ * Check the signature in the footer as well in order to handle
+ * fixed size images.
+ */
+buf_size = bdrv_pread(bs, bdrv_getlength(bs) - HEADER_SIZE, sig, 8);
+if (buf_size >= 8 && vpc_check_signature(sig)) {
+return 100;
+}
+
 return 0;
 }
 
-- 
1.9.3

Re: [Qemu-devel] [PATCH 01/15] qemu coroutine: support bypass mode

[Ming Lei]

On Fri, Aug 1, 2014 at 9:13 PM, Stefan Hajnoczi  wrote:
> On Fri, Aug 01, 2014 at 10:54:02AM +0800, Ming Lei wrote:
>> On Fri, Aug 1, 2014 at 12:30 AM, Paolo Bonzini  wrote:
>> > Il 31/07/2014 18:13, Ming Lei ha scritto:
>> >> Follows 'perf report' result on cycles event for with/without bypass
>> >> coroutine:
>> >>
>> >> http://pastebin.com/ae0vnQ6V
>> >>
>> >> From the profiling result, looks bdrv_co_do_preadv() is a bit slow
>> >> without bypass coroutine.
>> >
>> > Yeah, I can count at least 3.3% time spent here:
>> >
>> > 0.87%  bdrv_co_do_preadv
>> > 0.79%  bdrv_aligned_preadv
>> > 0.71%  qemu_coroutine_switch
>> > 0.52%  tracked_request_begin
>> > 0.45%  coroutine_swap
>> >
>> > Another ~3% wasted in malloc, etc.
>>
>> That should be related with coroutine and the BH in bdrv_co_do_rw().
>> In this post I didn't apply Stephan's coroutine resize patch which might
>> decrease usage of malloc() for coroutine.
>
> Please rerun with "[PATCH v3 0/2] coroutine: dynamically scale pool
> size".

No problem, will do that. Actually in my last post with rfc, this patchset
was against your coroutine resize patches.

I will provide the profile data tomorrow.

>
>> At least, coroutine isn't cheap from the profile result.
>
> Instead of bypassing coroutines we should first understand the overhead
> that they impose.  Is it due to the coroutine implementation (switching
> stacks) or due to the bdrv_co_*() code that happens to use coroutines
> but slow for other reasons.

>From the 3th patch(block: support to bypass qemu coroutinue)
and the 5th patch(dataplane: enable selective bypassing coroutine),
the change is to bypass coroutine and BH, and the other bdrv code
path is same, so it is due to the coroutine implementation, IMO.

Thanks,

Re: [Qemu-devel] [PATCH v3 10/16] target-arm: Break out exception masking to a separate func

[Peter Maydell]

On 17 June 2014 09:45, Edgar E. Iglesias  wrote:
>
> +static inline bool arm_excp_unmasked(CPUState *cs, unsigned int excp_idx)
> +{
> +CPUARMState *env = cs->env_ptr;
> +
> +switch (excp_idx) {
> +case EXCP_FIQ:
> +return !(env->daif & PSTATE_F);
> +case EXCP_IRQ:
> +return ((IS_M(env) && env->regs[15] < 0xfff0)
> +|| !(env->daif & PSTATE_I));
> +default:
> +g_assert_not_reached();
> +break;

You don't need a break, we've just asserted that this isn't
reachable. (Conversely if it was possible to get past the
assert we'd be falling out of the function without returning
a value, so break is wrong either way. But "just don't
put in the break" is what we do elsewhere in target-arm.)

> +}
> +}

thanks
-- PMM

[Qemu-devel] MSI-X interrupt emulation

[Danzer, Uwe]

Hi there,

I'm implementing an emulated PCIe Memory class device, but can't get MSI-X 
interrupt emulation working.

So far, the card appears in the guest system and the driver for the card 
recognises it and the emulation of 1MB of accessible r/w registers works as 
desired.

As the real card is connected to the outside world, it can signal events from 
there to the software via 5 MSI-X interrupts. Though I do not manage do get 
MSI-X emulation working in my implementation.

The guest OS is QNX and works just fine on the real hardware. Running QNX as 
guest inside QEMU, the command pci -vvv (QNX equivalent of lspci on Linux) 
shows my card and that it says it's able to do the desired 5 MSI-X interrupts, 
but the QNX driver doesn't activate MSI-X for the card.

In my init function of the PCIe card, I try to make MSI-X available with this 
code:

ret = msix_init_exclusive_bar (dev, 5, 1);
if (ret) {
printf("msix_init() failed\n");
} else {
int i;

for (i = 0; i < 5; i++) {
msix_vector_use (dev, i);
}

msix = 1;
}

Can someone tell me, what is wrong or missing in my code or does somebody have 
a minimal example for a (pseudo)device with MSI-X?

Many thanks in advance!

Best regards
Uwe Danzer

Re: [Qemu-devel] [PATCH v3 12/16] target-arm: A64: Correct updates to FAR and ESR on exceptions

[Peter Maydell]

On 17 June 2014 09:45, Edgar E. Iglesias  wrote:
> From: "Edgar E. Iglesias" 
>
> Not all exception types update both FAR and ESR.
>
> Reviewed-by: Alex Bennée 
> Reviewed-by: Greg Bellows 
> Signed-off-by: Edgar E. Iglesias 
> ---
>  target-arm/helper-a64.c | 6 ++
>  1 file changed, 2 insertions(+), 4 deletions(-)
>
> diff --git a/target-arm/helper-a64.c b/target-arm/helper-a64.c
> index 4be0784..cf8ce1e 100644
> --- a/target-arm/helper-a64.c
> +++ b/target-arm/helper-a64.c
> @@ -466,18 +466,16 @@ void aarch64_cpu_do_interrupt(CPUState *cs)
>env->exception.syndrome);
>  }
>
> -env->cp15.esr_el[new_el] = env->exception.syndrome;
> -env->cp15.far_el[new_el] = env->exception.vaddress;
> -
>  switch (cs->exception_index) {
>  case EXCP_PREFETCH_ABORT:
>  case EXCP_DATA_ABORT:
> +env->cp15.far_el[new_el] = env->exception.vaddress;
>  qemu_log_mask(CPU_LOG_INT, "...with FAR 0x%" PRIx64 "\n",
>env->cp15.far_el[new_el]);
> -break;

If you want this to fall through, you need a /* fall through */ comment.

>  case EXCP_BKPT:
>  case EXCP_UDEF:
>  case EXCP_SWI:
> +env->cp15.esr_el[new_el] = env->exception.syndrome;
>  break;
>  case EXCP_IRQ:
>  addr += 0x80;
> --

thanks
-- PMM

Re: [Qemu-devel] [PATCH] monitor: Remove hardcoded watchdog event names

[Luiz Capitulino]

On Tue, 29 Jul 2014 23:22:40 +0100
Hani Benhabiles  wrote:

> Signed-off-by: Hani Benhabiles 

Applied to the qmp-next branch, thanks.

> ---
>  monitor.c | 11 +--
>  1 file changed, 5 insertions(+), 6 deletions(-)
> 
> diff --git a/monitor.c b/monitor.c
> index 5bc70a6..7465775 100644
> --- a/monitor.c
> +++ b/monitor.c
> @@ -4520,16 +4520,15 @@ void netdev_del_completion(ReadLineState *rs, int 
> nb_args, const char *str)
>  
>  void watchdog_action_completion(ReadLineState *rs, int nb_args, const char 
> *str)
>  {
> +int i;
> +
>  if (nb_args != 2) {
>  return;
>  }
>  readline_set_completion_index(rs, strlen(str));
> -add_completion_option(rs, str, "reset");
> -add_completion_option(rs, str, "shutdown");
> -add_completion_option(rs, str, "poweroff");
> -add_completion_option(rs, str, "pause");
> -add_completion_option(rs, str, "debug");
> -add_completion_option(rs, str, "none");
> +for (i = 0; WatchdogExpirationAction_lookup[i]; i++) {
> +add_completion_option(rs, str, WatchdogExpirationAction_lookup[i]);
> +}
>  }
>  
>  void migrate_set_capability_completion(ReadLineState *rs, int nb_args,

[Qemu-devel] [Bug 1351271] [NEW] qemu deletes the backing file in snapshot mode

[Giancarlo Formicuccia]

Public bug reported:

The file removal occurs during the "change" operation. It happens only
in snapshot mode.

Minimal steps to reproduce the problem:

$ dd if=/dev/zero of=file.img bs=1M count=1
1+0 records in
1+0 records out
1048576 bytes (1.0 MB) copied, 0.00288727 s, 363 MB/s
$ ls file.img
file.img
$ qemu-system-x86_64 -monitor stdio -snapshot
QEMU 2.0.0 monitor - type 'help' for more information   


(qemu) change ide1-cd0 file.img
(qemu) change ide1-cd0 file.img
(qemu) change ide1-cd0 file.img
Could not open 'file.img': No such file or directory


(qemu) q
$ ls file.img
ls: cannot access file.img: No such file or directory

qemu version is 2.0.0 from arch linux, which is the vanilla version plus
one patch unrelated to this problem.

info block in snapshot mode reports:
ide1-cd0: /var/tmp/vl.G5WENk (qcow2)
Removable device: not locked, tray closed
Backing file: file.img (chain depth: 1)

whereas info block NOT in snapshot mode correctly reports:
ide1-cd0: file.img (raw, read-only)
Removable device: not locked, tray closed

** Affects: qemu
 Importance: Undecided
 Status: New

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

Title:
  qemu deletes the backing file in snapshot mode

Status in QEMU:
  New

Bug description:
  The file removal occurs during the "change" operation. It happens only
  in snapshot mode.

  Minimal steps to reproduce the problem:

  $ dd if=/dev/zero of=file.img bs=1M count=1
  1+0 records in
  1+0 records out
  1048576 bytes (1.0 MB) copied, 0.00288727 s, 363 MB/s
  $ ls file.img
  file.img
  $ qemu-system-x86_64 -monitor stdio -snapshot
  QEMU 2.0.0 monitor - type 'help' for more information 

  
  (qemu) change ide1-cd0 file.img
  (qemu) change ide1-cd0 file.img
  (qemu) change ide1-cd0 file.img
  Could not open 'file.img': No such file or directory  

  
  (qemu) q
  $ ls file.img
  ls: cannot access file.img: No such file or directory

  qemu version is 2.0.0 from arch linux, which is the vanilla version
  plus one patch unrelated to this problem.

  info block in snapshot mode reports:
  ide1-cd0: /var/tmp/vl.G5WENk (qcow2)
  Removable device: not locked, tray closed
  Backing file: file.img (chain depth: 1)

  whereas info block NOT in snapshot mode correctly reports:
  ide1-cd0: file.img (raw, read-only)
  Removable device: not locked, tray closed

To manage notifications about this bug go to:
https://bugs.launchpad.net/qemu/+bug/1351271/+subscriptions

Re: [Qemu-devel] [PATCH v3 2/7] bootindex: add del_boot_device_path function

[Eduardo Habkost]

On Sat, Jul 26, 2014 at 12:45:28PM +0800, arei.gong...@huawei.com wrote:
> From: Gonglei 
> 
> Introduce a del_boot_device_path() cleanup fw_cfg content
> when hot-unplugging devcie refer to bootindex.
> 
> Signed-off-by: Gonglei 
> Signed-off-by: Chenliang 
> ---
>  include/sysemu/sysemu.h |  1 +
>  vl.c| 17 +
>  2 files changed, 18 insertions(+)
> 
> diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h
> index e1b0659..7a79ff4 100644
> --- a/include/sysemu/sysemu.h
> +++ b/include/sysemu/sysemu.h
> @@ -209,6 +209,7 @@ void usb_info(Monitor *mon, const QDict *qdict);
>  
>  void add_boot_device_path(int32_t bootindex, DeviceState *dev,
>const char *suffix);
> +void del_boot_device_path(DeviceState *dev);
>  void modify_boot_device_path(int32_t bootindex, DeviceState *dev,
>   const char *suffix);
>  char *get_boot_devices_list(size_t *size, bool ignore_suffixes);
> diff --git a/vl.c b/vl.c
> index 3e42eaa..0cdadb4 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -1248,6 +1248,23 @@ void add_boot_device_path(int32_t bootindex, 
> DeviceState *dev,
>  QTAILQ_INSERT_TAIL(&fw_boot_order, node, link);
>  }
>  
> +void del_boot_device_path(DeviceState *dev)
> +{
> +FWBootEntry *i;
> +
> +assert(dev != NULL);
> +
> +QTAILQ_FOREACH(i, &fw_boot_order, link) {
> +if (i->dev->id && dev->id && !strcmp(i->dev->id, dev->id)) {

What if the device doesn't have any ID set? I don't see anything on
add_boot_device_path() ensuring that dev->id is always set.

Why you don't just check if i->dev == dev?


> +/* remove all entries of the assigend dev */
> +QTAILQ_REMOVE(&fw_boot_order, i, link);
> +g_free(i->suffix);
> +g_free(i);
> +break;
> +}
> +}
> +}
> +
>  void modify_boot_device_path(int32_t bootindex, DeviceState *dev,
>const char *suffix)
>  {
> -- 
> 1.7.12.4
> 
> 
> 

-- 
Eduardo

Re: [Qemu-devel] [PATCH 01/15] qemu coroutine: support bypass mode

[Paolo Bonzini]

Il 01/08/2014 15:48, Ming Lei ha scritto:
> On Fri, Aug 1, 2014 at 9:13 PM, Stefan Hajnoczi  wrote:
>> On Fri, Aug 01, 2014 at 10:54:02AM +0800, Ming Lei wrote:
>>> On Fri, Aug 1, 2014 at 12:30 AM, Paolo Bonzini  wrote:
 Il 31/07/2014 18:13, Ming Lei ha scritto:
> Follows 'perf report' result on cycles event for with/without bypass
> coroutine:
>
> http://pastebin.com/ae0vnQ6V
>
> From the profiling result, looks bdrv_co_do_preadv() is a bit slow
> without bypass coroutine.

 Yeah, I can count at least 3.3% time spent here:

 0.87%  bdrv_co_do_preadv
 0.79%  bdrv_aligned_preadv
 0.71%  qemu_coroutine_switch
 0.52%  tracked_request_begin
 0.45%  coroutine_swap

 Another ~3% wasted in malloc, etc.
>>>
>>> That should be related with coroutine and the BH in bdrv_co_do_rw().
>>> In this post I didn't apply Stephan's coroutine resize patch which might
>>> decrease usage of malloc() for coroutine.
>>
>> Please rerun with "[PATCH v3 0/2] coroutine: dynamically scale pool
>> size".
> 
> No problem, will do that. Actually in my last post with rfc, this patchset
> was against your coroutine resize patches.
> 
> I will provide the profile data tomorrow.
> 
>>
>>> At least, coroutine isn't cheap from the profile result.
>>
>> Instead of bypassing coroutines we should first understand the overhead
>> that they impose.  Is it due to the coroutine implementation (switching
>> stacks) or due to the bdrv_co_*() code that happens to use coroutines
>> but slow for other reasons.
> 
> From the 3th patch(block: support to bypass qemu coroutinue)
> and the 5th patch(dataplane: enable selective bypassing coroutine),
> the change is to bypass coroutine and BH, and the other bdrv code
> path is same, so it is due to the coroutine implementation, IMO.

But your code breaks all sort of invariants.  For example, the aiocb
must be valid when bdrv_aio_readv/writev return.  virtio-blk does not
use it, but virtio-scsi does.  If we apply your patches now, we will
have to redo it soon.

Basically we should be rewriting parts of block.c so that
bdrv_co_readv/writev calls bdrv_aio_readv/writev instead of vice versa.
 Coroutine creation should be pushed down to the
bdrv_aligned_preadv/bdrv_aligned_pwritev and, in the fast path, you can
simply call the driver's bdrv_aio_readv/writev.

Paolo

Re: [Qemu-devel] [PATCH] arm64: 64K pages and > 1024MB guest

[Paolo Bonzini]

Il 01/08/2014 16:02, Joel Schopp ha scritto:
>> >
>> > I think the patch is right but, besides these considerations, does this
>> > bug still manifest itself after Andrew fixed the start address of the
>> > device at 0x9001 (IIRC it was the pl031)?
> The device I see with that address is:
> hw/arm/virt.c:[VIRT_RTC] = { 0x9001, 0x1000 },
> 
> The bug still manifests itself with that in the tree (without my patch
> applied).

In 2.1-rc5 it is

[VIRT_RTC] = { 0x901, 0x1000 },

with one zero less:

commit 1373e140f0b0554a8b3aba9761cd96df49520f97
Author: Andrew Jones 
Date:   Tue Jul 29 18:32:01 2014 +0200

hw/arm/virt: fix pl031 addr typo

pl031's base address should be 0x901, not 0x9001, otherwise
it sits in ram when configuring a guest with greater than 1G.

Signed-off-by: Andrew Jones 
Signed-off-by: Peter Maydell 

diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index 405c61d..89532bd 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -104,7 +104,7 @@ static const MemMapEntry a15memmap[] = {
 [VIRT_GIC_DIST] = { 0x800, 0x1 },
 [VIRT_GIC_CPU] = { 0x801, 0x1 },
 [VIRT_UART] = { 0x900, 0x1000 },
-[VIRT_RTC] = { 0x9001, 0x1000 },
+[VIRT_RTC] = { 0x901, 0x1000 },
 [VIRT_MMIO] = { 0xa00, 0x200 },
 /* ...repeating for a total of NUM_VIRTIO_TRANSPORTS, each of that size */
 /* 0x1000 .. 0x4000 reserved for PCI */

Paolo

[Qemu-devel] [PATCH v3 1/2] trace: teach lttng backend to use format strings

[Alex Bennée]

This makes the UST backend pay attention to the format string arguments
that are defined when defining payload data. With this you can now
ensure integers are reported in hex mode if you want.

Signed-off-by: Alex Bennée 

---

v2
  - remove silly debug statements
v3
  - fix spelling
  - rebase to latest tracetool
v4:
  - rebase onto tracing-next

diff --git a/scripts/tracetool/__init__.py b/scripts/tracetool/__init__.py
index a51e0f2..36c789d 100644
--- a/scripts/tracetool/__init__.py
+++ b/scripts/tracetool/__init__.py
@@ -136,6 +136,8 @@ class Event(object):
 Properties of the event.
 args : Arguments
 The event arguments.
+arg_fmts : str
+The format strings for each argument.
 """
 
 _CRE = re.compile("((?P.*)\s+)?"
@@ -144,10 +146,11 @@ class Event(object):
   "\s*"
   "(?:(?:(?P\".+),)?\s*(?P\".+))?"
   "\s*")
+_FMT = re.compile("(%\w+|%.*PRI\S+)")
 
 _VALID_PROPS = set(["disable", "tcg", "tcg-trans", "tcg-exec"])
 
-def __init__(self, name, props, fmt, args, orig=None):
+def __init__(self, name, props, fmt, args, arg_fmts, orig=None):
 """
 Parameters
 --
@@ -159,13 +162,17 @@ class Event(object):
 Event printing format (or formats).
 args : Arguments
 Event arguments.
+arg_fmts : list of str
+Format strings for each argument.
 orig : Event or None
 Original Event before transformation.
+
 """
 self.name = name
 self.properties = props
 self.fmt = fmt
 self.args = args
+self.arg_fmts = arg_fmts
 
 if orig is None:
 self.original = weakref.ref(self)
@@ -203,6 +210,7 @@ class Event(object):
 if len(fmt_trans) > 0:
 fmt = [fmt_trans, fmt]
 args = Arguments.build(groups["args"])
+arg_fmts = Event._FMT.findall(fmt)
 
 if "tcg-trans" in props:
 raise ValueError("Invalid property 'tcg-trans'")
@@ -213,7 +221,7 @@ class Event(object):
 if "tcg" in props and isinstance(fmt, str):
 raise ValueError("Events with 'tcg' property must have two 
formats")
 
-return Event(name, props, fmt, args)
+return Event(name, props, fmt, args, arg_fmts)
 
 def __repr__(self):
 """Evaluable string representation for this object."""
diff --git a/scripts/tracetool/format/ust_events_h.py 
b/scripts/tracetool/format/ust_events_h.py
index 5102565..d189899 100644
--- a/scripts/tracetool/format/ust_events_h.py
+++ b/scripts/tracetool/format/ust_events_h.py
@@ -63,13 +63,20 @@ def generate(events, backend):
 name=e.name,
 args=", ".join(", ".join(i) for i in e.args))
 
-for t, n in e.args:
-if ('int' in t) or ('long' in t) or ('unsigned' in t) or 
('size_t' in t):
+types = e.args.types()
+names = e.args.names()
+fmts = e.arg_fmts
+for t,n,f in zip(types, names, fmts):
+if ('char *' in t) or ('char*' in t):
+out('   ctf_string(' + n + ', ' + n + ')')
+elif ("%p" in f) or ("x" in f) or ("PRIx" in f):
+out('   ctf_integer_hex('+ t + ', ' + n + ', ' + n + 
')')
+elif ("ptr" in t) or ("*" in t):
+out('   ctf_integer_hex('+ t + ', ' + n + ', ' + n + 
')')
+elif ('int' in t) or ('long' in t) or ('unsigned' in t) or 
('size_t' in t):
 out('   ctf_integer(' + t + ', ' + n + ', ' + n + ')')
 elif ('double' in t) or ('float' in t):
 out('   ctf_float(' + t + ', ' + n + ', ' + n + ')')
-elif ('char *' in t) or ('char*' in t):
-out('   ctf_string(' + n + ', ' + n + ')')
 elif ('void *' in t) or ('void*' in t):
 out('   ctf_integer_hex(unsigned long, ' + n + ', ' + 
n + ')')
 
-- 
2.0.3

[Qemu-devel] [PATCH v3 2/2] trace: add some tcg tracing support

[Alex Bennée]

This adds a couple of tcg specific trace-events which are useful for
tracing execution though tcg generated blocks. It's been tested with
lttng user space tracing but is generic enough for all systems. The tcg
events are:

  * translate_block - when a subject block is translated
  * exec_tb - when a translated block is entered
  * exec_tb_exit - when we exit the translated code
  * exec_tb_nocache - special case translations

Of course we can only trace the entrance to the first block of a chain
as each block will jump directly to the next when it can. See the -d
nochain patch to allow more complete tracing at the expense of
performance.

Signed-off-by: Alex Bennée 

---
v2
  - rebase

v3:
  - checkpatch clean-ups
  - add sign-off
  - disable exec_tb by default

diff --git a/cpu-exec.c b/cpu-exec.c
index 38e5f02..d209568 100644
--- a/cpu-exec.c
+++ b/cpu-exec.c
@@ -18,6 +18,7 @@
  */
 #include "config.h"
 #include "cpu.h"
+#include "trace.h"
 #include "disas/disas.h"
 #include "tcg.h"
 #include "qemu/atomic.h"
@@ -65,6 +66,9 @@ static inline tcg_target_ulong cpu_tb_exec(CPUState *cpu, 
uint8_t *tb_ptr)
 #endif /* DEBUG_DISAS */
 
 next_tb = tcg_qemu_tb_exec(env, tb_ptr);
+trace_exec_tb_exit((void *) (next_tb & ~TB_EXIT_MASK),
+   next_tb & TB_EXIT_MASK);
+
 if ((next_tb & TB_EXIT_MASK) > TB_EXIT_IDX1) {
 /* We didn't start executing this TB (eg because the instruction
  * counter hit zero); we must restore the guest PC to the address
@@ -105,6 +109,7 @@ static void cpu_exec_nocache(CPUArchState *env, int 
max_cycles,
  max_cycles);
 cpu->current_tb = tb;
 /* execute the generated code */
+trace_exec_tb_nocache(tb, tb->pc);
 cpu_tb_exec(cpu, tb->tc_ptr);
 cpu->current_tb = NULL;
 tb_phys_invalidate(tb, -1);
@@ -637,6 +642,7 @@ int cpu_exec(CPUArchState *env)
 cpu->current_tb = tb;
 barrier();
 if (likely(!cpu->exit_request)) {
+trace_exec_tb(tb, tb->pc);
 tc_ptr = tb->tc_ptr;
 /* execute the generated code */
 next_tb = cpu_tb_exec(cpu, tc_ptr);
diff --git a/trace-events b/trace-events
index 11a17a8..8aaa613 100644
--- a/trace-events
+++ b/trace-events
@@ -1265,6 +1265,15 @@ kvm_failed_spr_get(int str, const char *msg) "Warning: 
Unable to retrieve SPR %d
 kvm_failed_reg_get(uint64_t id, const char *msg) "Warning: Unable to retrieve 
ONEREG %" PRIu64 " from KVM: %s"
 kvm_failed_reg_set(uint64_t id, const char *msg) "Warning: Unable to set 
ONEREG %" PRIu64 " to KVM: %s"
 
+# TCG related tracing (mostly disabled by default)
+# cpu-exec.c
+disable exec_tb(void *tb, uintptr_t pc) "tb:%p pc=0x%x"
+disable exec_tb_nocache(void *tb, uintptr_t pc) "tb:%p pc=0x%x"
+disable exec_tb_exit(void *next_tb, unsigned int flags) "tb:%p flags=%x"
+
+# translate-all.c
+translate_block(void *tb, uintptr_t pc, uint8_t *tb_code) "tb:%p, pc:0x%x, 
tb_code:%p"
+
 # memory.c
 memory_region_ops_read(void *mr, uint64_t addr, uint64_t value, unsigned size) 
"mr %p addr %#"PRIx64" value %#"PRIx64" size %u"
 memory_region_ops_write(void *mr, uint64_t addr, uint64_t value, unsigned 
size) "mr %p addr %#"PRIx64" value %#"PRIx64" size %u"
diff --git a/translate-all.c b/translate-all.c
index 8f7e11b..2e0265a 100644
--- a/translate-all.c
+++ b/translate-all.c
@@ -33,6 +33,7 @@
 #include "qemu-common.h"
 #define NO_CPU_IO_DEFS
 #include "cpu.h"
+#include "trace.h"
 #include "disas/disas.h"
 #include "tcg.h"
 #if defined(CONFIG_USER_ONLY)
@@ -158,6 +159,8 @@ int cpu_gen_code(CPUArchState *env, TranslationBlock *tb, 
int *gen_code_size_ptr
 
 gen_intermediate_code(env, tb);
 
+trace_translate_block(tb, tb->pc, tb->tc_ptr);
+
 /* generate machine code */
 gen_code_buf = tb->tc_ptr;
 tb->tb_next_offset[0] = 0x;
-- 
2.0.3

Re: [Qemu-devel] [PATCH v3 13/16] target-arm: A64: Emulate the HVC insn

[Peter Maydell]

On 17 June 2014 09:45, Edgar E. Iglesias  wrote:
> From: "Edgar E. Iglesias" 
>
> Signed-off-by: Edgar E. Iglesias 
> ---
>  target-arm/cpu.h   |  1 +
>  target-arm/helper-a64.c|  1 +
>  target-arm/helper.c| 28 +++-
>  target-arm/helper.h|  1 +
>  target-arm/internals.h |  6 ++
>  target-arm/op_helper.c | 33 +
>  target-arm/translate-a64.c | 21 -
>  7 files changed, 85 insertions(+), 6 deletions(-)
>
> diff --git a/target-arm/cpu.h b/target-arm/cpu.h
> index 4f273ac..258bc09 100644
> --- a/target-arm/cpu.h
> +++ b/target-arm/cpu.h
> @@ -51,6 +51,7 @@
>  #define EXCP_EXCEPTION_EXIT  8   /* Return from v7M exception.  */
>  #define EXCP_KERNEL_TRAP 9   /* Jumped to kernel code page.  */
>  #define EXCP_STREX  10
> +#define EXCP_HVC11   /* HyperVisor Call */
>
>  #define ARMV7M_EXCP_RESET   1
>  #define ARMV7M_EXCP_NMI 2
> diff --git a/target-arm/helper-a64.c b/target-arm/helper-a64.c
> index cf8ce1e..7382d0a 100644
> --- a/target-arm/helper-a64.c
> +++ b/target-arm/helper-a64.c
> @@ -475,6 +475,7 @@ void aarch64_cpu_do_interrupt(CPUState *cs)
>  case EXCP_BKPT:
>  case EXCP_UDEF:
>  case EXCP_SWI:
> +case EXCP_HVC:
>  env->cp15.esr_el[new_el] = env->exception.syndrome;
>  break;
>  case EXCP_IRQ:
> diff --git a/target-arm/helper.c b/target-arm/helper.c
> index 972e91c..44e8d47 100644
> --- a/target-arm/helper.c
> +++ b/target-arm/helper.c
> @@ -3281,7 +3281,33 @@ void switch_mode(CPUARMState *env, int mode)
>   */
>  unsigned int arm_excp_target_el(CPUState *cs, unsigned int excp_idx)
>  {
> -return 1;
> +CPUARMState *env = cs->env_ptr;
> +unsigned int cur_el = arm_current_pl(env);
> +unsigned int target_el = 1;
> +bool route_to_el2 = false;
> +/* FIXME: Use actual secure state.  */
> +bool secure = false;
> +
> +if (!env->aarch64) {
> +/* TODO: Add EL2 and 3 exception handling for AArch32.  */
> +return 1;
> +}
> +
> +if (!secure
> +&& arm_feature(env, ARM_FEATURE_EL2)
> +&& cur_el < 2
> +&& (env->cp15.hcr_el2 & HCR_TGE)) {
> +route_to_el2 = true;
> +}
> +
> +target_el = MAX(cur_el, route_to_el2 ? 2 : 1);
> +
> +switch (excp_idx) {
> +case EXCP_HVC:
> +target_el = MAX(target_el, 2);
> +break;
> +}
> +return target_el;

I was wondering if this was going to get a bit heavyweight to
call twice on each trip round the cpu-exec.c main loop, but
we'll only do that in the case that an interrupt is pending
but currently masked I guess so it should be OK.

>  }
>
>  static void v7m_push(CPUARMState *env, uint32_t val)
> diff --git a/target-arm/helper.h b/target-arm/helper.h
> index facfcd2..70cfd28 100644
> --- a/target-arm/helper.h
> +++ b/target-arm/helper.h
> @@ -50,6 +50,7 @@ DEF_HELPER_2(exception_internal, void, env, i32)
>  DEF_HELPER_3(exception_with_syndrome, void, env, i32, i32)
>  DEF_HELPER_1(wfi, void, env)
>  DEF_HELPER_1(wfe, void, env)
> +DEF_HELPER_2(hvc, void, env, i32)
>
>  DEF_HELPER_3(cpsr_write, void, env, i32, i32)
>  DEF_HELPER_1(cpsr_read, i32, env)
> diff --git a/target-arm/internals.h b/target-arm/internals.h
> index 08fa697..b68e6f9 100644
> --- a/target-arm/internals.h
> +++ b/target-arm/internals.h
> @@ -53,6 +53,7 @@ static const char * const excnames[] = {
>  [EXCP_EXCEPTION_EXIT] = "QEMU v7M exception exit",
>  [EXCP_KERNEL_TRAP] = "QEMU intercept of kernel commpage",
>  [EXCP_STREX] = "QEMU intercept of STREX",
> +[EXCP_HVC] = "Hypervisor Call",
>  };
>
>  static inline void arm_log_exception(int idx)
> @@ -204,6 +205,11 @@ static inline uint32_t syn_aa64_svc(uint32_t imm16)
>  return (EC_AA64_SVC << ARM_EL_EC_SHIFT) | ARM_EL_IL | (imm16 & 0x);
>  }
>
> +static inline uint32_t syn_aa64_hvc(uint16_t imm16)

This isn't consistent with the other syn_ functions which take a
uint32_t imm16. (Didn't we do this before?)

> +{
> +return (EC_AA64_HVC << ARM_EL_EC_SHIFT) | ARM_EL_IL | imm16;
> +}
> +
>  static inline uint32_t syn_aa32_svc(uint32_t imm16, bool is_thumb)
>  {
>  return (EC_AA32_SVC << ARM_EL_EC_SHIFT) | (imm16 & 0x)
> diff --git a/target-arm/op_helper.c b/target-arm/op_helper.c
> index 25ad902..c1fa797 100644
> --- a/target-arm/op_helper.c
> +++ b/target-arm/op_helper.c
> @@ -369,6 +369,39 @@ void HELPER(msr_i_pstate)(CPUARMState *env, uint32_t op, 
> uint32_t imm)
>  }
>  }
>
> +void HELPER(hvc)(CPUARMState *env, uint32_t syndrome)
> +{
> +int cur_el = arm_current_pl(env);
> +/* FIXME: Use actual secure state.  */
> +bool secure = false;
> +bool udef;
> +
> +/* We've already checked that EL2 exists at translation time.
> + * EL3.HCE has priority over EL2.HCD.
> + */
> +if (arm_feature(env, ARM_FEATURE_EL3)) {
> +udef = !(env->cp15.scr_el3 & SCR_HCE);
> +} else {
> +udef = env->cp15.hcr

[Qemu-devel] [PATCH v3 0/2] a few simple trace fixes

[Alex Bennée]

Hi Stefan,

I've re-based these two on tracing-next. Otherwise now changes from
the last post.

Alex Bennée (2):
  trace: teach lttng backend to use format strings
  trace: add some tcg tracing support

 cpu-exec.c   |  6 ++
 scripts/tracetool/__init__.py| 12 ++--
 scripts/tracetool/format/ust_events_h.py | 15 +++
 trace-events |  9 +
 translate-all.c  |  3 +++
 5 files changed, 39 insertions(+), 6 deletions(-)

-- 
2.0.3

Re: [Qemu-devel] [PATCH v3 14/16] target-arm: A64: Emulate the SMC insn

[Peter Maydell]

On 17 June 2014 09:45, Edgar E. Iglesias  wrote:
> From: "Edgar E. Iglesias" 
>
> Signed-off-by: Edgar E. Iglesias 
> ---
>  target-arm/cpu.h   |  1 +
>  target-arm/helper-a64.c|  1 +
>  target-arm/helper.c|  6 ++
>  target-arm/helper.h|  1 +
>  target-arm/internals.h |  6 ++
>  target-arm/op_helper.c | 27 +++
>  target-arm/translate-a64.c | 10 ++
>  7 files changed, 52 insertions(+)
>
> diff --git a/target-arm/cpu.h b/target-arm/cpu.h
> index 258bc09..42e0ed3 100644
> --- a/target-arm/cpu.h
> +++ b/target-arm/cpu.h
> @@ -52,6 +52,7 @@
>  #define EXCP_KERNEL_TRAP 9   /* Jumped to kernel code page.  */
>  #define EXCP_STREX  10
>  #define EXCP_HVC11   /* HyperVisor Call */
> +#define EXCP_SMC12   /* Secure Monitor Call */
>
>  #define ARMV7M_EXCP_RESET   1
>  #define ARMV7M_EXCP_NMI 2
> diff --git a/target-arm/helper-a64.c b/target-arm/helper-a64.c
> index 7382d0a..f0f33af 100644
> --- a/target-arm/helper-a64.c
> +++ b/target-arm/helper-a64.c
> @@ -476,6 +476,7 @@ void aarch64_cpu_do_interrupt(CPUState *cs)
>  case EXCP_UDEF:
>  case EXCP_SWI:
>  case EXCP_HVC:
> +case EXCP_SMC:
>  env->cp15.esr_el[new_el] = env->exception.syndrome;
>  break;
>  case EXCP_IRQ:
> diff --git a/target-arm/helper.c b/target-arm/helper.c
> index 44e8d47..4945f67 100644
> --- a/target-arm/helper.c
> +++ b/target-arm/helper.c
> @@ -3306,6 +3306,12 @@ unsigned int arm_excp_target_el(CPUState *cs, unsigned 
> int excp_idx)
>  case EXCP_HVC:
>  target_el = MAX(target_el, 2);
>  break;
> +case EXCP_SMC:
> +target_el = 3;
> +if (!secure && cur_el == 1 && (env->cp15.hcr_el2 & HCR_TSC)) {
> +target_el = 2;
> +}
> +break;
>  }
>  return target_el;
>  }
> diff --git a/target-arm/helper.h b/target-arm/helper.h
> index 70cfd28..4293453 100644
> --- a/target-arm/helper.h
> +++ b/target-arm/helper.h
> @@ -51,6 +51,7 @@ DEF_HELPER_3(exception_with_syndrome, void, env, i32, i32)
>  DEF_HELPER_1(wfi, void, env)
>  DEF_HELPER_1(wfe, void, env)
>  DEF_HELPER_2(hvc, void, env, i32)
> +DEF_HELPER_2(smc, void, env, i32)
>
>  DEF_HELPER_3(cpsr_write, void, env, i32, i32)
>  DEF_HELPER_1(cpsr_read, i32, env)
> diff --git a/target-arm/internals.h b/target-arm/internals.h
> index b68e6f9..f6b7156 100644
> --- a/target-arm/internals.h
> +++ b/target-arm/internals.h
> @@ -54,6 +54,7 @@ static const char * const excnames[] = {
>  [EXCP_KERNEL_TRAP] = "QEMU intercept of kernel commpage",
>  [EXCP_STREX] = "QEMU intercept of STREX",
>  [EXCP_HVC] = "Hypervisor Call",
> +[EXCP_SMC] = "Secure Monitor Call",
>  };
>
>  static inline void arm_log_exception(int idx)
> @@ -210,6 +211,11 @@ static inline uint32_t syn_aa64_hvc(uint16_t imm16)
>  return (EC_AA64_HVC << ARM_EL_EC_SHIFT) | ARM_EL_IL | imm16;
>  }
>
> +static inline uint32_t syn_aa64_smc(uint16_t imm16)

uint16_t again.

> +{
> +return (EC_AA64_SMC << ARM_EL_EC_SHIFT) | ARM_EL_IL | imm16;
> +}
> +
>  static inline uint32_t syn_aa32_svc(uint32_t imm16, bool is_thumb)
>  {
>  return (EC_AA32_SVC << ARM_EL_EC_SHIFT) | (imm16 & 0x)
> diff --git a/target-arm/op_helper.c b/target-arm/op_helper.c
> index c1fa797..11636e3 100644
> --- a/target-arm/op_helper.c
> +++ b/target-arm/op_helper.c
> @@ -402,6 +402,33 @@ void HELPER(hvc)(CPUARMState *env, uint32_t syndrome)
>  raise_exception(env, EXCP_HVC);
>  }
>
> +void HELPER(smc)(CPUARMState *env, uint32_t syndrome)
> +{
> +int cur_el = arm_current_pl(env);
> +/* FIXME: Use real secure state.  */
> +bool secure = false;
> +bool smd = env->cp15.scr_el3 & SCR_SMD;
> +/* On ARMv8 AArch32, SMD only applies to NS mode.
> + * On ARMv7 SMD only applies to NS mode and only if EL2 is available.
> + * For ARMv7 non EL2, we force SMD to zero so we don't need to re-check
> + * the EL2 condition here.
> + */
> +bool udef = is_a64(env) ? smd : !secure && smd;
> +
> +/* In NS EL1, HCR controlled routing to EL2 has priority over SMD.  */
> +if (!secure && cur_el == 1 && (env->cp15.hcr_el2 & HCR_TSC)) {
> +udef = false;
> +}
> +
> +/* We've already checked that EL3 exists at translation time.  */
> +if (udef) {
> +env->exception.syndrome = syn_uncategorized();
> +raise_exception(env, EXCP_UDEF);
> +}
> +env->exception.syndrome = syndrome;
> +raise_exception(env, EXCP_SMC);
> +}
> +
>  void HELPER(exception_return)(CPUARMState *env)
>  {
>  int cur_el = arm_current_pl(env);
> diff --git a/target-arm/translate-a64.c b/target-arm/translate-a64.c
> index 5692dff..dca98168 100644
> --- a/target-arm/translate-a64.c
> +++ b/target-arm/translate-a64.c
> @@ -1454,6 +1454,16 @@ static void disas_exc(DisasContext *s, uint32_t insn)
>  gen_helper_hvc(cpu_env, tmp);
>  tcg_temp_free_i32(tmp);
>  break;
>

[Qemu-devel] QEMU Developer Collaboration Patterns

[codeface]

Dear QEMU Developers,

The University of Passau is currently studying the mechanisms that  
contribute to effective collaboration in open-source projects so that  
appropriate tools and techniques are created to support the needs of  
open-source developers. To achieve this goal, we are evaluating the  
usefulness of software archives (e.g., mailing lists, version-control  
system, and bug tracker) to quantitatively model social relationships  
and collaborative patterns in open-source projects. The culmination of  
this effort is an open-source project called Codeface, which is a  
framework and web front-end for analyzing social and technical aspects  
of software development. To learn more about Codeface, please visit  
http://siemens.github.io/codeface.


We are now recruiting open-source developers to participate in a short  
survey. The survey is composed of 4 questions and takes about *5  
minutes* to complete. We ensure that all of your information will be  
kept confidential and will only be used for scientific research  
purposes. There is no commercial interest in the results. We are  
merely interested in learning about your collaborative experiences as  
an open-source developer.


To access the survey, please click on the link below.
http://rfhinf067.hs-regensburg.de:8080/login.html

We highly appreciate your efforts, and we sincerely hope that you will  
take the time to participate. Upon completion of the survey, you may  
include your e-mail address so that we can send you the anonymized  
survey results.


We would also like to express our gratitude for support from Siemens  
Corporate Research and University of Applied Sciences Regensburg.



Sincerely,

Mitchell Joblin

PhD Student
Department of Informatics and Mathematics
University of Passau

Re: [Qemu-devel] [PATCH v3 15/16] target-arm: Add IRQ and FIQ routing to EL2 and 3

[Peter Maydell]

On 17 June 2014 09:45, Edgar E. Iglesias  wrote:
> From: "Edgar E. Iglesias" 

> --- a/target-arm/helper.c
> +++ b/target-arm/helper.c
> @@ -3312,6 +3312,19 @@ unsigned int arm_excp_target_el(CPUState *cs, unsigned 
> int excp_idx)
>  target_el = 2;
>  }
>  break;
> +case EXCP_FIQ:
> +case EXCP_IRQ: {

A trivial style nit, but I prefer the { to go on its own line when
opening a new scope for a case statement like this.

> +const uint64_t hcr_mask = excp_idx == EXCP_FIQ ? HCR_FMO : 
> HCR_IMO;
> +const uint32_t scr_mask = excp_idx == EXCP_FIQ ? SCR_FIQ : 
> SCR_IRQ;
> +
> +if (!secure && (env->cp15.hcr_el2 & hcr_mask)) {
> +target_el = 2;
> +}
> +if (env->cp15.scr_el3 & scr_mask) {
> +target_el = 3;
> +}
> +break;
> +}
>  }
>  return target_el;
>  }

thanks
-- PMM

Re: [Qemu-devel] [PATCH v3 16/16] target-arm: Add support for VIRQ and VFIQ

[Peter Maydell]

On 17 June 2014 09:45, Edgar E. Iglesias  wrote:
> From: "Edgar E. Iglesias" 
>
> Acked-by: Greg Bellows 
> Signed-off-by: Edgar E. Iglesias 
> ---
>  cpu-exec.c  | 12 
>  target-arm/cpu.c| 20 ++--
>  target-arm/cpu.h| 24 ++--
>  target-arm/helper-a64.c |  2 ++
>  target-arm/helper.c |  4 
>  target-arm/internals.h  |  2 ++
>  6 files changed, 60 insertions(+), 4 deletions(-)
>
> diff --git a/cpu-exec.c b/cpu-exec.c
> index a579ffc..baf5643 100644
> --- a/cpu-exec.c
> +++ b/cpu-exec.c
> @@ -498,6 +498,18 @@ int cpu_exec(CPUArchState *env)
>  cc->do_interrupt(cpu);
>  next_tb = 0;
>  }
> +if (interrupt_request & CPU_INTERRUPT_VIRQ
> +&& arm_excp_unmasked(cpu, EXCP_VIRQ)) {
> +cpu->exception_index = EXCP_VIRQ;
> +cc->do_interrupt(cpu);
> +next_tb = 0;
> +}
> +if (interrupt_request & CPU_INTERRUPT_VFIQ
> +&& arm_excp_unmasked(cpu, EXCP_VFIQ)) {
> +cpu->exception_index = EXCP_VFIQ;
> +cc->do_interrupt(cpu);
> +next_tb = 0;
> +}
>  #elif defined(TARGET_UNICORE32)
>  if (interrupt_request & CPU_INTERRUPT_HARD
>  && !(env->uncached_asr & ASR_I)) {
> diff --git a/target-arm/cpu.c b/target-arm/cpu.c
> index 8e64c5a..cd7a5df 100644
> --- a/target-arm/cpu.c
> +++ b/target-arm/cpu.c
> @@ -195,6 +195,20 @@ static void arm_cpu_set_irq(void *opaque, int irq, int 
> level)
>  cpu_reset_interrupt(cs, CPU_INTERRUPT_FIQ);
>  }
>  break;
> +case ARM_CPU_VIRQ:
> +if (level) {
> +cpu_interrupt(cs, CPU_INTERRUPT_VIRQ);
> +} else {
> +cpu_reset_interrupt(cs, CPU_INTERRUPT_VIRQ);
> +}
> +break;
> +case ARM_CPU_VFIQ:
> +if (level) {
> +cpu_interrupt(cs, CPU_INTERRUPT_VFIQ);
> +} else {
> +cpu_reset_interrupt(cs, CPU_INTERRUPT_VFIQ);
> +}
> +break;

With four cases this kind of wants a lookup of irq to CPU_INTERRUPT_
value I think, rather than the code duplication.

>  default:
>  hw_error("arm_cpu_set_irq: Bad interrupt line %d\n", irq);
>  }
> @@ -242,9 +256,11 @@ static void arm_cpu_initfn(Object *obj)
>  #ifndef CONFIG_USER_ONLY
>  /* Our inbound IRQ and FIQ lines */
>  if (kvm_enabled()) {
> -qdev_init_gpio_in(DEVICE(cpu), arm_cpu_kvm_set_irq, 2);
> +/* VIRQ and VFIQ are unused with KVM but we add them to maintain
> +   the same interface as non-KVM CPUs.  */

/* Multiline comments
 * should look like this.
 */

> +qdev_init_gpio_in(DEVICE(cpu), arm_cpu_kvm_set_irq, 4);
>  } else {
> -qdev_init_gpio_in(DEVICE(cpu), arm_cpu_set_irq, 2);
> +qdev_init_gpio_in(DEVICE(cpu), arm_cpu_set_irq, 4);
>  }
>
>  cpu->gt_timer[GTIMER_PHYS] = timer_new(QEMU_CLOCK_VIRTUAL, GTIMER_SCALE,
> diff --git a/target-arm/cpu.h b/target-arm/cpu.h
> index bb123bd..df61bbd 100644
> --- a/target-arm/cpu.h
> +++ b/target-arm/cpu.h
> @@ -53,6 +53,8 @@
>  #define EXCP_STREX  10
>  #define EXCP_HVC11   /* HyperVisor Call */
>  #define EXCP_SMC12   /* Secure Monitor Call */
> +#define EXCP_VIRQ   13
> +#define EXCP_VFIQ   14
>
>  #define ARMV7M_EXCP_RESET   1
>  #define ARMV7M_EXCP_NMI 2
> @@ -67,6 +69,8 @@
>
>  /* ARM-specific interrupt pending bits.  */
>  #define CPU_INTERRUPT_FIQ   CPU_INTERRUPT_TGT_EXT_1
> +#define CPU_INTERRUPT_VIRQ  CPU_INTERRUPT_TGT_EXT_2
> +#define CPU_INTERRUPT_VFIQ  CPU_INTERRUPT_TGT_EXT_3
>
>  /* The usual mapping for an AArch64 system register to its AArch32
>   * counterpart is for the 32 bit world to have access to the lower
> @@ -85,6 +89,9 @@
>  /* Meanings of the ARMCPU object's two inbound GPIO lines */

You forgot to update this comment ^^^

>  #define ARM_CPU_IRQ 0
>  #define ARM_CPU_FIQ 1
> +#define ARM_CPU_VIRQ 2
> +#define ARM_CPU_VFIQ 3

>
>  typedef void ARMWriteCPFunc(void *opaque, int cp_info,
>  int srcreg, int operand, uint32_t value);
> @@ -1142,6 +1149,8 @@ static inline bool arm_excp_unmasked(CPUState *cs, 
> unsigned int excp_idx)
>   * EL2 if we are in NS EL0/1.
>   */
>  bool irq_can_hyp = !secure && cur_el < 2 && target_el == 2;
> +bool irq_unmasked = ((IS_M(env) && env->regs[15] < 0xfff0)
> +  || !(env->daif & PSTATE_I));

The M profile stuff is starting to look increasingly weird here.

>
>  /* Don't take exceptions if they target a lower EL.  */
>  if (cur_el > target_el) {
> @@ -1158,8 +1167,19 @@ static inline bool arm_excp_unmasked(CPUState *cs, 
> unsigned int excp_idx)
>  

Re: [Qemu-devel] [PATCH v3 11/16] target-arm: Don't take interrupts targeting lower ELs

[Peter Maydell]

On 17 June 2014 09:45, Edgar E. Iglesias  wrote:
> From: "Edgar E. Iglesias" 
>
> Reviewed-by: Alex Bennée 
> Reviewed-by: Greg Bellows 
> Signed-off-by: Edgar E. Iglesias 
> ---
>  target-arm/cpu.h | 7 +++
>  1 file changed, 7 insertions(+)
>
> diff --git a/target-arm/cpu.h b/target-arm/cpu.h
> index 9d21361..4f273ac 100644
> --- a/target-arm/cpu.h
> +++ b/target-arm/cpu.h
> @@ -1132,6 +1132,13 @@ bool write_cpustate_to_list(ARMCPU *cpu);
>  static inline bool arm_excp_unmasked(CPUState *cs, unsigned int excp_idx)
>  {
>  CPUARMState *env = cs->env_ptr;
> +unsigned int cur_el = arm_current_pl(env);
> +unsigned int target_el = arm_excp_target_el(cs, excp_idx);
> +
> +/* Don't take exceptions if they target a lower EL.  */
> +if (cur_el > target_el) {
> +return false;
> +}
>
>  switch (excp_idx) {
>  case EXCP_FIQ:

Reviewed-by: Peter Maydell 

thanks
-- PMM

Re: [Qemu-devel] [PATCH v2 4/7] bootindex: delete bootindex when device is removed

[Eduardo Habkost]

On Wed, Jul 30, 2014 at 07:29:56AM +, Gonglei (Arei) wrote:
> Hi,
> 
> > -Original Message-
> > From: Gerd Hoffmann [mailto:kra...@redhat.com]
> > Sent: Friday, July 25, 2014 5:52 PM
> > 
> > > +del_boot_device_path(dev);
> > 
> > You can call this from device_finalize() instead of placing it into each
> > individual device.
> > 
> Maybe put this call in device_finalize is not a good idea. 
> I have three reasons:
> 
> 1. the device's some memory have been freed before call device_finalize,
>  such as device->id. It is too later.

I don't think you even need id. See my reply to v4 2/8.

But you have a point about being too late: some devices call
add_boot_device_path() on realize, so those would need to revert the
operation on unrealize; others do it on init, so they need to do it on
finalize.

On either case, I believe an extra check inside device_finalize()
wouldn't hurt, even if it becomes redundant on some devices.


> 2. not every kinds of device can configure bootindex property, such as usb
>  host adapters. It is a waste and useless for those devices. This is the
>  main reason.

I would prefer to waste a few cycles scanning the boot index list every
time a device is removed, than risking crashing QEMU in case somebody
forget to add a del_boot_device_path() call.


> 3. virtio-net device's parent is virtio-pci device, which configured id 
> property,
>  But the device saved in global fw_boot_order list is virtio-net device have 
> not
>  id property. If we put call del_boot_device_path(dev) in 
> virtio_net_device_unrealize
>  we can delete it from fw_boot_order directly.

Sorry, I don't understand what you mean here. If virtio-net doesn't have
an id property, would the current version of del_boot_device_path() even
work?

-- 
Eduardo

Re: [Qemu-devel] [PATCH v3 09/16] target-arm: A64: Refactor aarch64_cpu_do_interrupt

[Peter Maydell]

On 17 June 2014 09:45, Edgar E. Iglesias  wrote:
> From: "Edgar E. Iglesias" 
>
> Introduce new_el and new_mode in preparation for future patches
> that add support for taking exceptions to and from EL2 and 3.
> No functional change.
>
> Signed-off-by: Edgar E. Iglesias 
> ---
>  target-arm/cpu.h|  7 +++
>  target-arm/helper-a64.c | 24 +---
>  target-arm/helper.c | 13 +
>  3 files changed, 33 insertions(+), 11 deletions(-)

Reviewed-by: Peter Maydell 

thanks
-- PMM

Re: [Qemu-devel] [PATCH v1 00/16] target-arm: Parts of the AArch64 EL2/3 exception model

[Peter Maydell]

On 30 May 2014 08:28, Edgar E. Iglesias  wrote:
> This is a second round of AArch64 EL2/3 patches working on the exception
> model. Among other things adding HVC/SMC, interrupt routing to EL2/3 and
> Virtual IRQs/FIQs. The VIRQ/VFIQ support only adds the external signal
> delivery method.
>
> Patch 3 is a bug fix.
> Patch 14 fails checkpatch, seems like a bug in checkpatch, CC:d Blue.
>
> This conflicts slightly with the PSCI emulation patches that Rob posted.
> A rebase should be trivial, hooking in the PSCI emulation calls in the
> HVC/SMC helpers.

Sorry for letting this patchset sit in my to-review queue for so long.
Patches 1-6 are good and I'm going to put them in target-arm.next.
Patches 9 and 11 I've added my R-by to. Patches 7, 8, 10, 12..16
I've replied to with review comments.

thanks
-- PMM

Re: [Qemu-devel] [PATCH 01/10] AioContext: take bottom halves into account when computing aio_poll timeout

[Stefan Hajnoczi]

On Wed, Jul 09, 2014 at 11:53:01AM +0200, Paolo Bonzini wrote:
> diff --git a/async.c b/async.c
> index 34af0b2..ac40eab 100644
> --- a/async.c
> +++ b/async.c
> @@ -152,39 +152,43 @@ void qemu_bh_delete(QEMUBH *bh)
>  bh->deleted = 1;
>  }
>  
> -static gboolean
> -aio_ctx_prepare(GSource *source, gint*timeout)
> +int
> +aio_compute_timeout(AioContext *ctx)

The return value is now nanoseconds so a 32-bit int doesn't offer much
range (only 2 seconds for a signed int).

Any reason to use int instead of int64_t as used by the timer API?


pgpSE4x3Dr7qL.pgp
Description: PGP signature

Re: [Qemu-devel] [PATCH] arm64: 64K pages and > 1024MB guest

[Joel Schopp]

> I agree.
>
> I think the patch is right but, besides these considerations, does this
> bug still manifest itself after Andrew fixed the start address of the
> device at 0x9001 (IIRC it was the pl031)?
The device I see with that address is:
hw/arm/virt.c:[VIRT_RTC] = { 0x9001, 0x1000 },

The bug still manifests itself with that in the tree (without my patch
applied).

Re: [Qemu-devel] [PATCH v1 00/16] target-arm: Parts of the AArch64 EL2/3 exception model

[Peter Maydell]

On 1 August 2014 15:35, Peter Maydell  wrote:
> On 30 May 2014 08:28, Edgar E. Iglesias  wrote:
>> This is a second round of AArch64 EL2/3 patches working on the exception
>> model. Among other things adding HVC/SMC, interrupt routing to EL2/3 and
>> Virtual IRQs/FIQs. The VIRQ/VFIQ support only adds the external signal
>> delivery method.
>>
>> Patch 3 is a bug fix.
>> Patch 14 fails checkpatch, seems like a bug in checkpatch, CC:d Blue.
>>
>> This conflicts slightly with the PSCI emulation patches that Rob posted.
>> A rebase should be trivial, hooking in the PSCI emulation calls in the
>> HVC/SMC helpers.
>
> Sorry for letting this patchset sit in my to-review queue for so long.
> Patches 1-6 are good and I'm going to put them in target-arm.next.
> Patches 9 and 11 I've added my R-by to. Patches 7, 8, 10, 12..16
> I've replied to with review comments.

...this reply was to the v1 cover letter, but I did review v3 :-)

thanks
-- PMM

Re: [Qemu-devel] [RFC PATCH 04/17] COLO info: use colo info to tell migration target colo is enabled

[Dr. David Alan Gilbert]

* Yang Hongyang (yan...@cn.fujitsu.com) wrote:
> migrate colo info to migration target to tell the target colo is
> enabled.

If I understand this correctly this means that you send a 'colo info' device
information for migrations that don't have COLO enabled; that's bad because
it breaks migration unless the destination has it; I guess it's OK if you
were to guard it with a thing so it didn't do it for old machine-types.

You could use the QEMU_VM_COMMAND sections I've created for postcopy;
( http://lists.gnu.org/archive/html/qemu-devel/2014-07/msg00889.html ) and 
add a QEMU_VM_CMD_COLO to indicate you want the destination to become an SVM,
  then check the capability near the start of migration and send the command.

Or perhaps there's a way to add the colo-info device on the command line so it's
not always there.

Dave

> Signed-off-by: Yang Hongyang 
> ---
>  Makefile.objs  |  1 +
>  include/migration/migration-colo.h |  3 ++
>  migration-colo-comm.c  | 68 
> ++
>  vl.c   |  4 +++
>  4 files changed, 76 insertions(+)
>  create mode 100644 migration-colo-comm.c
> 
> diff --git a/Makefile.objs b/Makefile.objs
> index cab5824..1836a68 100644
> --- a/Makefile.objs
> +++ b/Makefile.objs
> @@ -50,6 +50,7 @@ common-obj-$(CONFIG_POSIX) += os-posix.o
>  common-obj-$(CONFIG_LINUX) += fsdev/
>  
>  common-obj-y += migration.o migration-tcp.o
> +common-obj-y += migration-colo-comm.o
>  common-obj-$(CONFIG_COLO) += migration-colo.o
>  common-obj-y += vmstate.o
>  common-obj-y += qemu-file.o
> diff --git a/include/migration/migration-colo.h 
> b/include/migration/migration-colo.h
> index 35b384c..e3735d8 100644
> --- a/include/migration/migration-colo.h
> +++ b/include/migration/migration-colo.h
> @@ -12,6 +12,9 @@
>  #define QEMU_MIGRATION_COLO_H
>  
>  #include "qemu-common.h"
> +#include "migration/migration.h"
> +
> +void colo_info_mig_init(void);
>  
>  bool colo_supported(void);
>  
> diff --git a/migration-colo-comm.c b/migration-colo-comm.c
> new file mode 100644
> index 000..ccbc246
> --- /dev/null
> +++ b/migration-colo-comm.c
> @@ -0,0 +1,68 @@
> +/*
> + *  COarse-grain LOck-stepping Virtual Machines for Non-stop Service (COLO)
> + *  (a.k.a. Fault Tolerance or Continuous Replication)
> + *
> + *  Copyright (C) 2014 FUJITSU LIMITED
> + *
> + * 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 
> +
> +#define DEBUG_COLO
> +
> +#ifdef DEBUG_COLO
> +#define DPRINTF(fmt, ...) \
> +do { fprintf(stdout, "COLO: " fmt, ## __VA_ARGS__); } while (0)
> +#else
> +#define DPRINTF(fmt, ...) \
> +do { } while (0)
> +#endif
> +
> +static bool colo_requested;
> +
> +/* save */
> +
> +static bool migrate_use_colo(void)
> +{
> +MigrationState *s = migrate_get_current();
> +return s->enabled_capabilities[MIGRATION_CAPABILITY_COLO];
> +}
> +
> +static void colo_info_save(QEMUFile *f, void *opaque)
> +{
> +qemu_put_byte(f, migrate_use_colo());
> +}
> +
> +/* restore */
> +
> +static int colo_info_load(QEMUFile *f, void *opaque, int version_id)
> +{
> +int value = qemu_get_byte(f);
> +
> +if (value && !colo_supported()) {
> +fprintf(stderr, "COLO is not supported\n");
> +return -EINVAL;
> +}
> +
> +if (value && !colo_requested) {
> +DPRINTF("COLO requested!\n");
> +}
> +
> +colo_requested = value;
> +
> +return 0;
> +}
> +
> +static SaveVMHandlers savevm_colo_info_handlers = {
> +.save_state = colo_info_save,
> +.load_state = colo_info_load,
> +};
> +
> +void colo_info_mig_init(void)
> +{
> +register_savevm_live(NULL, "colo info", -1, 1,
> + &savevm_colo_info_handlers, NULL);
> +}
> diff --git a/vl.c b/vl.c
> index fe451aa..1a282d8 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -89,6 +89,7 @@ int main(int argc, char **argv)
>  #include "sysemu/dma.h"
>  #include "audio/audio.h"
>  #include "migration/migration.h"
> +#include "migration/migration-colo.h"
>  #include "sysemu/kvm.h"
>  #include "qapi/qmp/qjson.h"
>  #include "qemu/option.h"
> @@ -4339,6 +4340,9 @@ int main(int argc, char **argv, char **envp)
>  
>  blk_mig_init();
>  ram_mig_init();
> +if (colo_supported()) {
> +colo_info_mig_init();
> +}
>  
>  /* open the virtual block devices */
>  if (snapshot)
> -- 
> 1.9.1
> 
--
Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK

Re: [Qemu-devel] [PATCH v4 4/8] bootindex: delete bootindex when device is removed

[Eduardo Habkost]

On Thu, Jul 31, 2014 at 05:47:29PM +0800, arei.gong...@huawei.com wrote:
> From: Gonglei 
> 
> Device should be removed from global boot list when
> it is hot-unplugged.
> 
> Signed-off-by: Chenliang 
> Signed-off-by: Gonglei 
> ---
>  hw/block/virtio-blk.c| 1 +
>  hw/i386/kvm/pci-assign.c | 1 +
>  hw/misc/vfio.c   | 1 +
>  hw/net/e1000.c   | 1 +
>  hw/net/eepro100.c| 1 +
>  hw/net/ne2000.c  | 1 +
>  hw/net/rtl8139.c | 1 +
>  hw/net/virtio-net.c  | 1 +
>  hw/net/vmxnet3.c | 1 +
>  hw/scsi/scsi-generic.c   | 1 +
>  hw/usb/dev-network.c | 1 +
>  hw/usb/host-libusb.c | 1 +
>  hw/usb/redirect.c| 1 +
>  13 files changed, 13 insertions(+)

Grepping for add_boot_device_path, I don't see corresponding
del_boot_device_path() calls in this patch for the following:

  hw/ide/qdev.c:add_boot_device_path(dev->conf.bootindex, &dev->qdev,
  hw/block/fdc.c:add_boot_device_path(isa->bootindexA, dev, "/floppy@0");
  hw/block/fdc.c:add_boot_device_path(isa->bootindexB, dev, "/floppy@1");
  hw/net/pcnet.c:add_boot_device_path(s->conf.bootindex, dev, 
"/ethernet-phy@0");
  hw/net/spapr_llan.c:add_boot_device_path(dev->nicconf.bootindex, 
DEVICE(dev), "");
  hw/scsi/scsi-disk.c:add_boot_device_path(s->qdev.conf.bootindex, 
&dev->qdev, NULL);

Why we don't need del_boot_device_path() calls for those?

These seem to be OK, and are handled by this patch:

  hw/i386/kvm/pci-assign.c:add_boot_device_path(dev->bootindex, 
&pci_dev->qdev, NULL);
  hw/block/virtio-blk.c:add_boot_device_path(s->conf->bootindex, dev, 
"/disk@0,0");
  hw/misc/vfio.c:add_boot_device_path(vdev->bootindex, &pdev->qdev, NULL);
  hw/net/rtl8139.c:add_boot_device_path(s->conf.bootindex, d, 
"/ethernet-phy@0");
  hw/net/e1000.c:add_boot_device_path(d->conf.bootindex, dev, 
"/ethernet-phy@0");
  hw/net/eepro100.c:add_boot_device_path(s->conf.bootindex, &pci_dev->qdev, 
"/ethernet-phy@0");
  hw/net/ne2000.c:add_boot_device_path(s->c.bootindex, &pci_dev->qdev, 
"/ethernet-phy@0");
  hw/net/virtio-net.c:add_boot_device_path(n->nic_conf.bootindex, dev, 
"/ethernet-phy@0");
  hw/net/vmxnet3.c:add_boot_device_path(s->conf.bootindex, dev, 
"/ethernet-phy@0");
  hw/scsi/scsi-generic.c:add_boot_device_path(s->conf.bootindex, 
&s->qdev, NULL);
  hw/usb/dev-network.c:add_boot_device_path(s->conf.bootindex, &dev->qdev, 
"/ethernet@0");
  hw/usb/host-libusb.c:add_boot_device_path(s->bootindex, &udev->qdev, 
NULL);
  hw/usb/redirect.c:add_boot_device_path(dev->bootindex, &udev->qdev, NULL);

This one has dev==NULL, so it looks OK:

  hw/core/loader.c:add_boot_device_path(bootindex, NULL, devpath);

This is modify_boot_device_path(), so it's OK:

  vl.c:add_boot_device_path(bootindex, dev, old_entry->suffix);

-- 
Eduardo

Re: [Qemu-devel] [RFC PATCH 05/17] COLO save: integrate COLO checkpointed save into qemu migration

[Dr. David Alan Gilbert]

* Yang Hongyang (yan...@cn.fujitsu.com) wrote:
>   Integrate COLO checkpointed save flow into qemu migration.
>   Add a migrate state: MIG_STATE_COLO, enter this migrate state
> after the first live migration successfully finished.
>   Create a colo thread to do the checkpointed save.

In postcopy I added a 'migration_already_active' function
to merge all the different places that check for ACTIVE/SETUP etc.
( http://lists.gnu.org/archive/html/qemu-devel/2014-07/msg00850.html )

> +/*TODO: COLO checkpointed save loop*/
> +
> +if (s->state != MIG_STATE_ERROR) {
> +migrate_set_state(s, MIG_STATE_COLO, MIG_STATE_COMPLETED);
> +}

I thought migrate_set_state only changed the state if the old state
matched the 1st value - i.e. I think it'll only change to COMPLETED
if the state is COLO; so I don't think you need the if.

Dave
--
Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK

Re: [Qemu-devel] [PATCH for-2.2 00/10] AioContext cleanups and Win32 socket support

[Stefan Hajnoczi]

On Wed, Jul 09, 2014 at 11:53:00AM +0200, Paolo Bonzini wrote:
> This series simplifies heavily aio_poll by splitting it into three
> phases: prepare (aio_compute_timeout), poll, dispatch.  The resulting
> code shares more logic between aio_poll and the GSource wrappers,
> and makes it easier to add Win32 support for sockets.
> 
> Win32 support for sockets is a prerequisite for moving the NBD server
> into the BlockDriverState's attached AioContext.  It is done in the
> final patch, based on earlier work from Or Goshen (from Intel).
> I had to more or less rewrite it to fit the new framework, but you
> can see parts of Or's work, as well as traces of aio-posix.c and
> main-loop.c logic.
> 
> Tested with NBD boot under Wine.
> 
> Paolo
> 
> Paolo Bonzini (10):
>   AioContext: take bottom halves into account when computing aio_poll
> timeout
>   aio-win32: Evaluate timers after handles
>   aio-win32: Factor out duplicate code into aio_dispatch_handlers
>   AioContext: run bottom halves after polling
>   AioContext: export and use aio_dispatch
>   test-aio: test timers on Windows too
>   aio-win32: add aio_set_dispatching optimization
>   AioContext: introduce aio_prepare
>   qemu-coroutine-io: fix for Win32
>   aio-win32: add support for sockets
> 
>  aio-posix.c |  58 
>  aio-win32.c | 262 
> +++-
>  async.c |  39 +---
>  block/Makefile.objs |   2 -
>  include/block/aio.h |  25 -
>  nbd.c   |   2 +-
>  qemu-coroutine-io.c |   4 +-
>  tests/test-aio.c|  48 +++---
>  8 files changed, 277 insertions(+), 163 deletions(-)

I'm happy with this series except for my question about int vs int64_t
types for nanosecond time values.


pgp_u71eoXlgk.pgp
Description: PGP signature

Re: [Qemu-devel] [RFC PATCH 07/17] COLO buffer: implement colo buffer as well as QEMUFileOps based on it

[Dr. David Alan Gilbert]

* Yang Hongyang (yan...@cn.fujitsu.com) wrote:
> We need a buffer to store migration data.
> 
> On save side:
>   all saved data was write into colo buffer first, so that we can know
> the total size of the migration data. this can also separate the data
> transmission from colo control data, we use colo control data over
> socket fd to synchronous both side's stat.
> 
> On restore side:
>   all migration data was read into colo buffer first, then load data
> from the buffer: If network error happens while data transmission,
> the slaver can still functinal because the migration data are not yet
> loaded.

This is very similar to the QEMUSizedBuffer based QEMUFile's that Stefan Berger
wrote and that I use in both my postcopy and BER patchsets:

 http://lists.gnu.org/archive/html/qemu-devel/2014-07/msg00846.html

 (and to the similar code from Isaku Yamahata).

I think we should be able to use a shared version even if we need some changes.

> 
> Signed-off-by: Yang Hongyang 
> ---
>  migration-colo.c | 112 
> +++
>  1 file changed, 112 insertions(+)
> 
> diff --git a/migration-colo.c b/migration-colo.c
> index d566b9d..b90d9b6 100644
> --- a/migration-colo.c
> +++ b/migration-colo.c
> @@ -11,6 +11,7 @@
>  #include "qemu/main-loop.h"
>  #include "qemu/thread.h"
>  #include "block/coroutine.h"
> +#include "qemu/error-report.h"
>  #include "migration/migration-colo.h"
>  
>  static QEMUBH *colo_bh;
> @@ -20,14 +21,122 @@ bool colo_supported(void)
>  return true;
>  }
>  
> +/* colo buffer */
> +
> +#define COLO_BUFFER_BASE_SIZE (1000*1000*4ULL)
> +#define COLO_BUFFER_MAX_SIZE (1000*1000*1000*10ULL)

Powers of 2 are nicer!

Dave
--
Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK

Re: [Qemu-devel] [PATCH 01/15] qemu coroutine: support bypass mode

[Ming Lei]

On Fri, Aug 1, 2014 at 9:48 PM, Ming Lei  wrote:
> On Fri, Aug 1, 2014 at 9:13 PM, Stefan Hajnoczi  wrote:
>> On Fri, Aug 01, 2014 at 10:54:02AM +0800, Ming Lei wrote:
>>> On Fri, Aug 1, 2014 at 12:30 AM, Paolo Bonzini  wrote:
>>> > Il 31/07/2014 18:13, Ming Lei ha scritto:
>>> >> Follows 'perf report' result on cycles event for with/without bypass
>>> >> coroutine:
>>> >>
>>> >> http://pastebin.com/ae0vnQ6V
>>> >>
>>> >> From the profiling result, looks bdrv_co_do_preadv() is a bit slow
>>> >> without bypass coroutine.
>>> >
>>> > Yeah, I can count at least 3.3% time spent here:
>>> >
>>> > 0.87%  bdrv_co_do_preadv
>>> > 0.79%  bdrv_aligned_preadv
>>> > 0.71%  qemu_coroutine_switch
>>> > 0.52%  tracked_request_begin
>>> > 0.45%  coroutine_swap
>>> >
>>> > Another ~3% wasted in malloc, etc.
>>>
>>> That should be related with coroutine and the BH in bdrv_co_do_rw().
>>> In this post I didn't apply Stephan's coroutine resize patch which might
>>> decrease usage of malloc() for coroutine.
>>
>> Please rerun with "[PATCH v3 0/2] coroutine: dynamically scale pool
>> size".
>
> No problem, will do that. Actually in my last post with rfc, this patchset
> was against your coroutine resize patches.
>
> I will provide the profile data tomorrow.

Please see below link for without bypass coroutine, and with
your coroutine resize patches(V3):

   http://pastebin.com/10y00sir

BTW, if anyone likes to play this stuff, these patches can be
pulled from below tree easily:

git://kernel.ubuntu.com/ming/qemu.git

branch:
 - v2.1.0-mq-co-resize:
  these 15 patches against 2.1.0-rc5 plus Stefan's coroutine
  resize patches(V3)
 - v2.1.0-mq:
  only these 15 patches against 2.1.0-rc5

Thanks,

Re: [Qemu-devel] [RFC PATCH 10/17] COLO ctl: introduce is_slave() and is_master()

[Dr. David Alan Gilbert]

* Yang Hongyang (yan...@cn.fujitsu.com) wrote:
> is_slaver is to determine whether the QEMU instance is a
> slaver(migration target) at runtime.
> is_master is to determine whether the QEMU instance is a
> master(migration starter) at runtime.
> This 2 APIs will be used later.

Since the names are made global in patch 15, I think it's best to
do it here, but also use a more specific name for them, like
colo_is_master.

Dave

> Signed-off-by: Yang Hongyang 
> ---
>  migration-colo.c | 11 +++
>  1 file changed, 11 insertions(+)
> 
> diff --git a/migration-colo.c b/migration-colo.c
> index 802f8b0..2699e77 100644
> --- a/migration-colo.c
> +++ b/migration-colo.c
> @@ -187,6 +187,12 @@ static const QEMUFileOps colo_read_ops = {
>  
>  /* save */
>  
> +static __attribute__((unused)) bool is_master(void)
> +{
> +MigrationState *s = migrate_get_current();
> +return (s->state == MIG_STATE_COLO);
> +}
> +
>  static void *colo_thread(void *opaque)
>  {
>  MigrationState *s = opaque;
> @@ -275,6 +281,11 @@ void colo_init_checkpointer(MigrationState *s)
>  
>  static Coroutine *colo;
>  
> +static __attribute__((unused)) bool is_slave(void)
> +{
> +return colo != NULL;
> +}
> +
>  /*
>   * return:
>   * 0: start a checkpoint
> -- 
> 1.9.1
> 
--
Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK

Re: [Qemu-devel] [RFC PATCH 11/17] COLO ctl: implement colo checkpoint protocol

[Dr. David Alan Gilbert]

* Yang Hongyang (yan...@cn.fujitsu.com) wrote:
> implement colo checkpoint protocol.
> 
> Checkpoint synchronzing points.
> 
>   Primary Secondary
>   NEW @
>   Suspend
>   SUSPENDED   @
>   Suspend&Save state
>   SEND@
>   Send state  Receive state
>   RECEIVED@
>   Flush network   Load state
>   LOADED  @
>   Resume  Resume
> 
>   Start Comparing
> NOTE:
>  1) '@' who sends the message
>  2) Every sync-point is synchronized by two sides with only
> one handshake(single direction) for low-latency.
> If more strict synchronization is required, a opposite direction
> sync-point should be added.
>  3) Since sync-points are single direction, the remote side may
> go forward a lot when this side just receives the sync-point.
> 
> Signed-off-by: Yang Hongyang 
> ---
>  migration-colo.c | 268 
> +--
>  1 file changed, 262 insertions(+), 6 deletions(-)
> 
> diff --git a/migration-colo.c b/migration-colo.c
> index 2699e77..a708872 100644
> --- a/migration-colo.c
> +++ b/migration-colo.c
> @@ -24,6 +24,41 @@
>   */
>  #define CHKPOINT_TIMER 1
>  
> +enum {
> +COLO_READY = 0x46,
> +
> +/*
> + * Checkpoint synchronzing points.
> + *
> + *  Primary Secondary
> + *  NEW @
> + *  Suspend
> + *  SUSPENDED   @
> + *  Suspend&Save state
> + *  SEND@
> + *  Send state  Receive state
> + *  RECEIVED@
> + *  Flush network   Load state
> + *  LOADED  @
> + *  Resume  Resume
> + *
> + *  Start Comparing
> + * NOTE:
> + * 1) '@' who sends the message
> + * 2) Every sync-point is synchronized by two sides with only
> + *one handshake(single direction) for low-latency.
> + *If more strict synchronization is required, a opposite direction
> + *sync-point should be added.
> + * 3) Since sync-points are single direction, the remote side may
> + *go forward a lot when this side just receives the sync-point.
> + */
> +COLO_CHECKPOINT_NEW,
> +COLO_CHECKPOINT_SUSPENDED,
> +COLO_CHECKPOINT_SEND,
> +COLO_CHECKPOINT_RECEIVED,
> +COLO_CHECKPOINT_LOADED,
> +};
> +
>  static QEMUBH *colo_bh;
>  
>  bool colo_supported(void)
> @@ -185,30 +220,161 @@ static const QEMUFileOps colo_read_ops = {
>  .close = colo_close,
>  };
>  
> +/* colo checkpoint control helper */
> +static bool is_master(void);
> +static bool is_slave(void);
> +
> +static void ctl_error_handler(void *opaque, int err)
> +{
> +if (is_slave()) {
> +/* TODO: determine whether we need to failover */
> +/* FIXME: we will not failover currently, just kill slave */
> +error_report("error: colo transmission failed!\n");
> +exit(1);
> +} else if (is_master()) {
> +/* Master still alive, do not failover */
> +error_report("error: colo transmission failed!\n");
> +return;
> +} else {
> +error_report("COLO: Unexpected error happend!\n");
> +exit(EXIT_FAILURE);
> +}
> +}
> +
> +static int colo_ctl_put(QEMUFile *f, uint64_t request)
> +{
> +int ret = 0;
> +
> +qemu_put_be64(f, request);
> +qemu_fflush(f);
> +
> +ret = qemu_file_get_error(f);
> +if (ret < 0) {
> +ctl_error_handler(f, ret);
> +return 1;
> +}
> +
> +return ret;
> +}
> +
> +static int colo_ctl_get_value(QEMUFile *f, uint64_t *value)
> +{
> +int ret = 0;
> +uint64_t temp;
> +
> +temp = qemu_get_be64(f);
> +
> +ret = qemu_file_get_error(f);
> +if (ret < 0) {
> +ctl_error_handler(f, ret);
> +return 1;
> +}
> +
> +*value = temp;
> +return 0;
> +}
> +
> +static int colo_ctl_get(QEMUFile *f, uint64_t require)
> +{
> +int ret;
> +uint64_t value;
> +
> +ret = colo_ctl_get_value(f, &value);
> +if (ret) {
> +return ret;
> +}
> +
> +if (value != require) {
> +error_report("unexpected state received!\n");

I find it useful to print the expected/received state to
be able to figure out what went wrong.

> +exit(1);
> +}
> +
> +return ret;
> +}
> +
>  /* save */
>  
> -static __attribute__((unused)) bool is_master(void)
> +static bool is_master(void)
>  {
>  MigrationState *s = migrate_get_current();
>  return (s->state == MIG_STATE_COLO);
>  }
>  
> +static int do_colo_transaction(MigrationState

Re: [Qemu-devel] [PATCH v2 5/7] qmp: add set-bootindex command

[Eduardo Habkost]

On Fri, Jul 25, 2014 at 02:52:50PM +0800, arei.gong...@huawei.com wrote:
> From: Gonglei 
> 
> Adds "set-bootindex id=xx,bootindex=xx,suffix=xx" QMP command.
> 
> Example QMP command:
> -> { "execute": "set-bootindex", "arguments": { "id": "ide0-0-1", 
> "bootindex": 1, "suffix": "/disk@0"}}
> <- { "return": {} }
> 
> Signed-off-by: Gonglei 
> Signed-off-by: Chenliang 
> ---
>  qapi-schema.json | 16 
>  qmp-commands.hx  | 24 
>  qmp.c| 17 +
>  3 files changed, 57 insertions(+)
> 
> diff --git a/qapi-schema.json b/qapi-schema.json
> index b11aad2..a9ef0be 100644
> --- a/qapi-schema.json
> +++ b/qapi-schema.json
> @@ -1704,6 +1704,22 @@
>  { 'command': 'device_del', 'data': {'id': 'str'} }
>  
>  ##
> +# @set-bootindex:
> +#
> +# set bootindex of a devcie
> +#
> +# @id: the name of the device
> +# @bootindex: the bootindex of the device
> +# @suffix: #optional a suffix of the device
> +#
> +# Returns: Nothing on success
> +#  If @id is not a valid device, DeviceNotFound
> +#
> +# Since: 2.2
> +##
> +{ 'command': 'set-bootindex', 'data': {'id': 'str', 'bootindex': 'int', 
> '*suffix': 'str'} }
> +
> +##

I wonder if we could simply use qom-set for that. How many devices
actually support having multiple bootindex entries with different
suffixes?

-- 
Eduardo

Re: [Qemu-devel] [PATCH for-2.2 00/10] AioContext cleanups and Win32 socket support

[Paolo Bonzini]

Il 01/08/2014 16:52, Stefan Hajnoczi ha scritto:
> I'm happy with this series except for my question about int vs int64_t
> types for nanosecond time values.

That was just an oversight, thanks for the review.  I'll take a look
next Monday.

Paolo

Re: [Qemu-devel] [PATCH 3/4] libqos: add a simple first-fit memory allocator

[Stefan Hajnoczi]

On Thu, Jul 31, 2014 at 05:14:12PM -0400, John Snow wrote:
> 
> On 07/31/2014 06:13 AM, Stefan Hajnoczi wrote:
> >On Wed, Jul 30, 2014 at 06:28:28PM -0400, John Snow wrote:
> >>-static uint64_t pc_alloc(QGuestAllocator *allocator, size_t size)
> >>+static inline void mlist_insert(MemList *head, MemBlock *insr)
> >>  {
> >>-PCAlloc *s = container_of(allocator, PCAlloc, alloc);
> >>-uint64_t addr;
> >>+QTAILQ_INSERT_HEAD(head, insr, MLIST_ENTNAME);
> >>+}
> >>+
> >>+static inline void mlist_append(MemList *head, MemBlock *node)
> >>+{
> >>+QTAILQ_INSERT_TAIL(head, node, MLIST_ENTNAME);
> >>+}
> >>+
> >>+static inline void mlist_unlink(MemList *head, MemBlock *rem)
> >>+{
> >>+QTAILQ_REMOVE(head, rem, MLIST_ENTNAME);
> >>+}
> >For the same reasons as my comments about the macros, these trivial functions
> >are boilerplate.  Why not use the QTAILQ macros directly?
> For /at least/ the append and insert cases, I desired call-by-value
> semantics.
> As a matter of taste, I find macros annoying for the reason that you cannot
> inline things such as:
> mlist_insert(list, mlist_new(...));
> 
> but unlink is certainly superfluous, and just something I did for some
> consistency.
> 
> If there is a matter of style where in-line function call is to be avoided
> entirely, I'll just nix all of these trivial inlines.

As a reviewer I prefer to see familiar APIs rather than a convenience
layer because it's extra stuff I need to keep in mind.  Sticking to the
APIs makes it quicker for other QEMU developers to parse the code.

That said, feel free to keep the functions if you want.

Stefan


pgpAiqsJSLYvi.pgp
Description: PGP signature

Re: [Qemu-devel] [RFC PATCH 15/17] COLO save: reuse migration bitmap under colo checkpoint

[Dr. David Alan Gilbert]

* Yang Hongyang (yan...@cn.fujitsu.com) wrote:
> reuse migration bitmap under colo checkpoint, only send dirty pages
> per-checkpoint.
> 
> Signed-off-by: Yang Hongyang 
> ---
>  arch_init.c| 20 +++-
>  include/migration/migration-colo.h |  2 ++
>  migration-colo.c   |  6 ++
>  stubs/migration-colo.c | 10 ++
>  4 files changed, 33 insertions(+), 5 deletions(-)
> 
> diff --git a/arch_init.c b/arch_init.c
> index 8ddaf35..c84e6c8 100644
> --- a/arch_init.c
> +++ b/arch_init.c
> @@ -52,6 +52,7 @@
>  #include "exec/ram_addr.h"
>  #include "hw/acpi/acpi.h"
>  #include "qemu/host-utils.h"
> +#include "migration/migration-colo.h"
>  
>  #ifdef DEBUG_ARCH_INIT
>  #define DPRINTF(fmt, ...) \
> @@ -769,6 +770,15 @@ static int ram_save_setup(QEMUFile *f, void *opaque)
>  RAMBlock *block;
>  int64_t ram_bitmap_pages; /* Size of bitmap in pages, including gaps */
>  
> +/*
> + * migration has already setup the bitmap, reuse it.
> + */
> +if (is_master()) {
> +qemu_mutex_lock_ramlist();
> +reset_ram_globals();
> +goto out_setup;
> +}
> +
>  mig_throttle_on = false;
>  dirty_rate_high_cnt = 0;
>  bitmap_sync_count = 0;
> @@ -828,6 +838,7 @@ static int ram_save_setup(QEMUFile *f, void *opaque)
>  migration_bitmap_sync();
>  qemu_mutex_unlock_iothread();
>  
> +out_setup:
>  qemu_put_be64(f, ram_bytes_total() | RAM_SAVE_FLAG_MEM_SIZE);
>  
>  QTAILQ_FOREACH(block, &ram_list.blocks, next) {

Is it necessary to send the block list for each of your snapshots?

Dave

> @@ -937,7 +948,14 @@ static int ram_save_complete(QEMUFile *f, void *opaque)
>  }
>  
>  ram_control_after_iterate(f, RAM_CONTROL_FINISH);
> -migration_end();
> +
> +/*
> + * Since we need to reuse dirty bitmap in colo,
> + * don't cleanup the bitmap.
> + */
> +if (!migrate_use_colo() || migration_has_failed(migrate_get_current())) {
> +migration_end();
> +}
>  
>  qemu_mutex_unlock_ramlist();
>  qemu_put_be64(f, RAM_SAVE_FLAG_EOS);
> diff --git a/include/migration/migration-colo.h 
> b/include/migration/migration-colo.h
> index 861fa27..c286a60 100644
> --- a/include/migration/migration-colo.h
> +++ b/include/migration/migration-colo.h
> @@ -21,10 +21,12 @@ bool colo_supported(void);
>  /* save */
>  bool migrate_use_colo(void);
>  void colo_init_checkpointer(MigrationState *s);
> +bool is_master(void);
>  
>  /* restore */
>  bool restore_use_colo(void);
>  void restore_exit_colo(void);
> +bool is_slave(void);
>  
>  void colo_process_incoming_checkpoints(QEMUFile *f);
>  
> diff --git a/migration-colo.c b/migration-colo.c
> index 8596845..13a6a57 100644
> --- a/migration-colo.c
> +++ b/migration-colo.c
> @@ -222,8 +222,6 @@ static const QEMUFileOps colo_read_ops = {
>  };
>  
>  /* colo checkpoint control helper */
> -static bool is_master(void);
> -static bool is_slave(void);
>  
>  static void ctl_error_handler(void *opaque, int err)
>  {
> @@ -295,7 +293,7 @@ static int colo_ctl_get(QEMUFile *f, uint64_t require)
>  
>  /* save */
>  
> -static bool is_master(void)
> +bool is_master(void)
>  {
>  MigrationState *s = migrate_get_current();
>  return (s->state == MIG_STATE_COLO);
> @@ -499,7 +497,7 @@ void colo_init_checkpointer(MigrationState *s)
>  
>  static Coroutine *colo;
>  
> -static bool is_slave(void)
> +bool is_slave(void)
>  {
>  return colo != NULL;
>  }
> diff --git a/stubs/migration-colo.c b/stubs/migration-colo.c
> index 55f0d37..ef65be6 100644
> --- a/stubs/migration-colo.c
> +++ b/stubs/migration-colo.c
> @@ -22,3 +22,13 @@ void colo_init_checkpointer(MigrationState *s)
>  void colo_process_incoming_checkpoints(QEMUFile *f)
>  {
>  }
> +
> +bool is_master(void)
> +{
> +return false;
> +}
> +
> +bool is_slave(void)
> +{
> +return false;
> +}
> -- 
> 1.9.1
> 
--
Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK

Re: [Qemu-devel] [RFC PATCH 13/17] COLO ctl: implement colo save

[Dr. David Alan Gilbert]

* Yang Hongyang (yan...@cn.fujitsu.com) wrote:
> implement colo save

My postcopy 'QEMU_VM_CMD_PACKAGED' does something similar to
parts of this with the QEMUSizedBuffer, we might be able to share some more:
https://lists.nongnu.org/archive/html/qemu-devel/2014-07/msg00886.html

> +/* we send the total size of the vmstate first */
> +ret = colo_ctl_put(s->file, colo_buffer.used);
> +if (ret) {
> +goto out;
> +}
> +
> +qemu_put_buffer_async(s->file, colo_buffer.data, colo_buffer.used);
> +ret = qemu_file_get_error(s->file);
> +if (ret < 0) {
> +goto out;
> +}
> +qemu_fflush(s->file);

Is there a reason to use _async here?  I thought the only gain is
if you were going to do other writes in the shadow of the async, with the fflush
immediately after I'm not sure it helps.

Dave

>  
>  ret = colo_ctl_get(control, COLO_CHECKPOINT_RECEIVED);
>  if (ret) {
>  goto out;
>  }
>  
> -/* TODO: Flush network etc. */
> +/* Flush network etc. */
> +colo_compare_flush();
>  
>  ret = colo_ctl_get(control, COLO_CHECKPOINT_LOADED);
>  if (ret) {
>  goto out;
>  }
>  
> -/* TODO: resume master */
> +colo_compare_resume();
> +ret = 0;
>  
>  out:
> +/* resume master */
> +qemu_mutex_lock_iothread();
> +vm_start();
> +qemu_mutex_unlock_iothread();
> +
>  return ret;
>  }
>  
> -- 
> 1.9.1
> 
--
Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK

Re: [Qemu-devel] [RFC PATCH 16/17] COLO ram cache: implement colo ram cache on slaver

[Dr. David Alan Gilbert]

* Yang Hongyang (yan...@cn.fujitsu.com) wrote:
> The ram cache was initially the same as PVM's memory. At
> checkpoint, we cache the dirty memory of PVM into ram cache
> (so that ram cache always the same as PVM's memory at every
> checkpoint), flush cached memory to SVM after we received
> all PVM dirty memory(only needed to flush memory that was
> both dirty on PVM and SVM since last checkpoint).

(Typo: 'r' on the end of the title)

I think I understand the need for the cache, to be able to restore pages
that the SVM has modified that the PVM hadn't; however, if I understand
the change here, (to host_from_stream_offset) the SVM will load the
snapshot into the ram_cache rather than directly into host memory - why
is this necessary?  If the SVMs CPU is stopped at this point couldn't
it load snapshot pages directly into host memory, clearing pages in the SVMs
bitmap, so that the only pages that then get copied in flush_cache are
the pages that the SVM modified but the PVM *didn't* include in the snapshot?
I can see that you would need to do it the way you've done it if the
snapshot-load could fail (at the sametime the PVM failed) and thus the old SVM
state would be the surviving state, but how could it fail at this point
given the whole stream is in the colo-buffer?


> +static void ram_flush_cache(void);
>  static int ram_load(QEMUFile *f, void *opaque, int version_id)
>  {
>  ram_addr_t addr;
>  int flags, ret = 0;
>  static uint64_t seq_iter;
> +bool need_flush = false;

Probably better as 'ram_cache_needs_flush'

Dave
--
Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK

Re: [Qemu-devel] vfio in the guest: no available reset mechanism

[Alex Williamson]

On Fri, 2014-08-01 at 09:35 +0800, Le Tan wrote:
> Hi Alex,
> 
> 2014-07-30 22:46 GMT+08:00 Alex Williamson :
> > On Wed, 2014-07-30 at 22:16 +0800, Le Tan wrote:
> >> Hi Michael,
> >>
> >> 2014-07-30 21:16 GMT+08:00 Michael S. Tsirkin :
> >> > On Wed, Jul 30, 2014 at 08:24:04PM +0800, Le Tan wrote:
> >> >> Hi,
> >> >> I am testing vfio in L1 with my VT-d emulation project. I assigned one
> >> >> of the two AHCI controllers in L1 to L2 via vfio. After I ran the QEMU
> >> >> in L1, it complains that:
> >> >> qemu-system-x86_64: vfio: Cannot reset device :00:03.0, no
> >> >> available reset mechanism.
> >> >> qemu-system-x86_64: vfio: Cannot reset device :00:03.0, no
> >> >> available reset mechanism.
> >> >>
> >> >> Then L2 paused when the SeaBIOS executed in ahci_controller_setup(). I
> >> >> look into this and found that:
> >> >> val = ahci_ctrl_readl(ctrl, HOST_CTL);
> >> >> ahci_ctrl_writel(ctrl, HOST_CTL, val | HOST_CTL_AHCI_EN);
> >> >> When the BIOS tried to read the HOST_CTL, it returns 0x8002, which
> >> >> bit 2 (Interrupt Enable) is 1. The AHCI manual says that this bit
> >> >> should be cleared by default. So maybe L1 didn't reset the device
> >> >> before assigning it to L2?
> >> >> Then the BIOS tried to write back to HOST_CTL and it was stuck here. :(
> >> >>
> >> >> So can anyone give me some advice? About the state of PCI device or
> >> >> bus-level reset?
> >> >>
> >> >> Here is the detail of the environment and the way I did the vfio.
> >> >> 1. lspci in L1 said:
> >> >> 00:03.0 SATA controller [0106]: Intel Corporation 82801IR/IO/IH
> >> >> (ICH9R/DO/DH) 6 port SATA Controller [AHCI mode] [8086:2922] (rev 02)
> >> >> 00:1f.0 ISA bridge [0601]: Intel Corporation 82801IB (ICH9) LPC
> >> >> Interface Controller [8086:2918] (rev 02)
> >> >> 00:1f.2 SATA controller [0106]: Intel Corporation 82801IR/IO/IH
> >> >> (ICH9R/DO/DH) 6 port SATA Controller [AHCI mode] [8086:2922] (rev 02)
> >> >> 00:1f.3 SMBus [0c05]: Intel Corporation 82801I (ICH9 Family) SMBus
> >> >> Controller [8086:2930] (rev 02)
> >> >> 2. Unbind 00:03.0 and do vfio:
> >> >> modprobe -r vfio_iommu_type1
> >> >> modprobe vfio_iommu_type1 allow_unsafe_interrupts=1
> >> >> modprobe vfio-pci
> >> >> echo :00:03.0 > /sys/bus/pci/devices/\:00\:03.0/driver/unbind
> >> >> echo "8086 2922" > /sys/bus/pci/drivers/vfio-pci/new_id
> >> >> 3. run L2 with "-device vfio-pci,host=00:03.0"
> >> >>
> >> >> Any help is appreciated! Thanks very much!
> >> >>
> >> >> Regards,
> >> >> Le
> >> >
> >> > Clearly, bus level reset can't work for the root bus :)
> >>
> >> Thanks very much!
> >> I test the vfio with a second-bus ahci controller and it didn't
> >> complain about the lack of reset mechanism. :) And the return val of
> >> HOST_CTL is normal now (the same as emulated ahci controller).
> >> However, it still paused when the BIOS tried to write to the HOST_CTL.
> >> Do you have any idea?
> >> And we should just test vfio and legacy pci-assignment with second bus
> >> devices, not considering the root-bus devices?
> >
> > AHCI seems like a poor choice of device for this work, they typically
> > don't support any kind of reset and they can be troublesome even for the
> > L1 assignment.  You really want something with FLR support so that both
> > the host and L1 guest can potentially reset the device.  That said, you
> > may still run into bugs with a L1 guest directed FLR.  Thanks,
> >
> > Alex
> >
> 
> So what device do you think is suitable for the pci-assign test? e1000?
> I just tested it with sound card ac97 and USB controller. But I don't
> know how to attach them to a pcie-to-pci bridge, so maybe they weren't
> reset before being assigned to L2. But it seems that they can work.
> 1. With the sound card, I assigned it to L2 via both vfio and legacy
> pci-assign and I could hear the music played in L2 from host's
> speakers. Of course, the vfio also complained about the lack of reset
> mechanism.

First off, maybe I'm a little confused, are you trying to assigned
emulated devices for the L1 guest to the L2 guest or are these assigned
devices to the L1 guest that you want to re-assign to L2?  AFAIK, we
don't have any emulated devices that support reset, but it wouldn't be
that hard to add a PM capability to one that advertises a soft reset on
D3hot->D0 transition and calls the QEMU driver reset function when that
occurs.  This would provide the most flexibility.

The other choice is to create a topology where an individual device can
be reset via a bus reset, so putting a single device behind a bridge,
root port, or downstream switch port.

Otherwise, the emulated e1000 is probably a reasonable choice, network
drivers generally seem to be pretty good about accepting a device in a
running state since it might have been used as a boot device.

If you're looking at re-assigning an L1 assigned device to an L2 guest,
I'm not sure how that's supposed to work.  The VT-d emulation would need
to somehow push L2 physical mappings to 

Re: [Qemu-devel] [PATCH 01/15] qemu coroutine: support bypass mode

[Ming Lei]

On Fri, Aug 1, 2014 at 10:17 PM, Paolo Bonzini  wrote:
> Il 01/08/2014 15:48, Ming Lei ha scritto:
>> On Fri, Aug 1, 2014 at 9:13 PM, Stefan Hajnoczi  wrote:
>>> On Fri, Aug 01, 2014 at 10:54:02AM +0800, Ming Lei wrote:
 On Fri, Aug 1, 2014 at 12:30 AM, Paolo Bonzini  wrote:
> Il 31/07/2014 18:13, Ming Lei ha scritto:
>> Follows 'perf report' result on cycles event for with/without bypass
>> coroutine:
>>
>> http://pastebin.com/ae0vnQ6V
>>
>> From the profiling result, looks bdrv_co_do_preadv() is a bit slow
>> without bypass coroutine.
>
> Yeah, I can count at least 3.3% time spent here:
>
> 0.87%  bdrv_co_do_preadv
> 0.79%  bdrv_aligned_preadv
> 0.71%  qemu_coroutine_switch
> 0.52%  tracked_request_begin
> 0.45%  coroutine_swap
>
> Another ~3% wasted in malloc, etc.

 That should be related with coroutine and the BH in bdrv_co_do_rw().
 In this post I didn't apply Stephan's coroutine resize patch which might
 decrease usage of malloc() for coroutine.
>>>
>>> Please rerun with "[PATCH v3 0/2] coroutine: dynamically scale pool
>>> size".
>>
>> No problem, will do that. Actually in my last post with rfc, this patchset
>> was against your coroutine resize patches.
>>
>> I will provide the profile data tomorrow.
>>
>>>
 At least, coroutine isn't cheap from the profile result.
>>>
>>> Instead of bypassing coroutines we should first understand the overhead
>>> that they impose.  Is it due to the coroutine implementation (switching
>>> stacks) or due to the bdrv_co_*() code that happens to use coroutines
>>> but slow for other reasons.
>>
>> From the 3th patch(block: support to bypass qemu coroutinue)
>> and the 5th patch(dataplane: enable selective bypassing coroutine),
>> the change is to bypass coroutine and BH, and the other bdrv code
>> path is same, so it is due to the coroutine implementation, IMO.
>
> But your code breaks all sort of invariants.  For example, the aiocb
> must be valid when bdrv_aio_readv/writev return.  virtio-blk does not
> use it, but virtio-scsi does.  If we apply your patches now, we will
> have to redo it soon.

It can be supported by scheduling BH in bdrv_co_io_em_complete() too.

>
> Basically we should be rewriting parts of block.c so that
> bdrv_co_readv/writev calls bdrv_aio_readv/writev instead of vice versa.
>  Coroutine creation should be pushed down to the
> bdrv_aligned_preadv/bdrv_aligned_pwritev and, in the fast path, you can
> simply call the driver's bdrv_aio_readv/writev.

This idea is very constructive, and I will investigate further to see
if it is easy to start.


Thanks,

Re: [Qemu-devel] [PATCH v2 2/7] target-arm: Implement PMCCNTR_EL0 and related registers

[Peter Maydell]

On 26 June 2014 06:02, Alistair Francis  wrote:
> This patch adds support for the ARMv8 version of the PMCCNTR and
> related registers. It also starts to implement the PMCCFILTR_EL0
> register.
>
> Signed-off-by: Peter Crosthwaite 
> Signed-off-by: Alistair Francis 
> ---
>
>  target-arm/cpu.h|1 +
>  target-arm/helper.c |   39 +++
>  2 files changed, 40 insertions(+), 0 deletions(-)
>
> diff --git a/target-arm/cpu.h b/target-arm/cpu.h
> index cd1c7b6..6a2efd8 100644
> --- a/target-arm/cpu.h
> +++ b/target-arm/cpu.h
> @@ -224,6 +224,7 @@ typedef struct CPUARMState {
>   * was reset. Otherwise it stores the counter value
>   */
>  uint64_t c15_ccnt;
> +uint64_t pmccfiltr_el0; /* Performance Monitor Filter Register */
>  } cp15;
>
>  struct {
> diff --git a/target-arm/helper.c b/target-arm/helper.c
> index ac10564..ce986ee 100644
> --- a/target-arm/helper.c
> +++ b/target-arm/helper.c
> @@ -738,7 +738,22 @@ static const ARMCPRegInfo v7_cp_reginfo[] = {
>.writefn = pmcntenset_write,
>.accessfn = pmreg_access,
>.raw_writefn = raw_write },
> +{ .name = "PMCNTENSET_EL0", .crn = 9, .crm = 12, .opc0 = 3, .opc1 = 3,
> +  .opc2 = 1, .access = PL0_RW, .resetvalue = 0,
> +  .state = ARM_CP_STATE_AA64,
> +  .fieldoffset = offsetof(CPUARMState, cp15.c9_pmcnten),

fieldoffset for an AArch64 register has to point at a uint64_t;
this is going to read the state next to cp15.c9_pmcnten as
well, resulting in garbage in half the register. You need to widen
that field to uint64_t and change the AArch32 accessor to
use offsetoflow32().

> +  .writefn = pmcntenset_write,
> +  .accessfn = pmreg_access,
> +  .raw_writefn = raw_write },

This is a pretty random order for the fields in a reginfo struct
(though existing code is not great here either).
Preferred is to put the .name first, then .state, then the
encoding in the same order as the v8 ARM ARM:
.cp [if needed], .opc0, .opc1, .crn, .crm, .opc2
then .access and .accessfn
then .fieldoffset and .resetvalue, then read and writefns.

>  { .name = "PMCNTENCLR", .cp = 15, .crn = 9, .crm = 12, .opc1 = 0, .opc2 
> = 2,
> +  .access = PL0_RW,
> +  .fieldoffset = offsetof(CPUARMState, cp15.c9_pmcnten),
> +  .accessfn = pmreg_access,
> +  .writefn = pmcntenclr_write,
> +  .type = ARM_CP_NO_MIGRATE },
> +{ .name = "PMCNTENCLR_EL0", .crn = 9, .crm = 12, .opc0 = 3, .opc1 = 3,
> +  .opc2 = 2,
> +  .state = ARM_CP_STATE_AA64,
>.access = PL0_RW, .fieldoffset = offsetof(CPUARMState, 
> cp15.c9_pmcnten),
>.accessfn = pmreg_access,
>.writefn = pmcntenclr_write,
> @@ -762,11 +777,25 @@ static const ARMCPRegInfo v7_cp_reginfo[] = {
>.access = PL0_RW, .resetvalue = 0, .type = ARM_CP_IO,
>.readfn = pmccntr_read, .writefn = pmccntr_write,
>.accessfn = pmreg_access },
> +{ .name = "PMCCNTR_EL0", .cp = 15, .crn = 9, .crm = 13, .opc1 = 3,
> +  .opc2 = 0,
> +  .access = PL0_RW, .resetvalue = 0, .type = ARM_CP_IO,
> +  .state = ARM_CP_STATE_AA64,
> +  .readfn = pmccntr_read, .writefn = pmccntr_write,
> +  .accessfn = pmreg_access },

The AArch32 reginfo now needs a NO_MIGRATE marker, or we'll
migrate the underlying state twice.

You don't need to specify a .cp for a STATE_AA64 only reginfo.

The ARM ARM says the semantics of a 32 bit write to PMCCNTR
are to write the lower 32 bits and not touch the high bits. I suspect
your code will zero them instead. (Maybe this should have been a
comment on patch 1...)

>  #endif
> +{ .name = "PMCCFILTR_EL0", .state = ARM_CP_STATE_AA64,
> +  .opc0 = 3, .opc1 = 3, .crn = 14, .crm = 15, .opc2 = 7,
> +  .access = PL0_RW, .accessfn = pmreg_access,
> +  .state = ARM_CP_STATE_AA64,
> +  .resetvalue = 0,
> +  .type = ARM_CP_IO,
> +  .fieldoffset = offsetof(CPUARMState, cp15.pmccfiltr_el0), },
>  { .name = "PMXEVTYPER", .cp = 15, .crn = 9, .crm = 13, .opc1 = 0, .opc2 
> = 1,
>.access = PL0_RW,
>.fieldoffset = offsetof(CPUARMState, cp15.c9_pmxevtyper),
>.accessfn = pmreg_access, .writefn = pmxevtyper_write,
> +  .resetvalue = 0,

This seems like a stray unnecessary change.

>.raw_writefn = raw_write },
>  /* Unimplemented, RAZ/WI. */
>  { .name = "PMXEVCNTR", .cp = 15, .crn = 9, .crm = 13, .opc1 = 0, .opc2 = 
> 2,
> @@ -2324,6 +2353,16 @@ void register_cp_regs_for_features(ARMCPU *cpu)
>  .raw_writefn = raw_write,
>  };
>  define_one_arm_cp_reg(cpu, &pmcr);
> +ARMCPRegInfo pmcr64 = {
> +.name = "PMCR_EL0", .crn = 9, .crm = 12, .opc0 = 3, .opc1 = 3,
> +.opc2 = 0,
> +.state = ARM_CP_STATE_AA64,
> +.access = PL0_RW, .resetvalue = cpu->midr & 0xff00,
> +.fieldoffset = offsetof(CPUARMState, cp15.c9_pmcr),
> +.accessfn = pmreg_access, .writefn = pmcr_write,

Re: [Qemu-devel] [RFC PATCH 7/7] cpus: reclaim allocated vCPU objects

[Anshul Makkar]

Hi Gu,

Thanks for clarifying.

Ah I missed that bit of the patch. Sorry about that and for making noise.

Yes, now cpu-hotplug and unplug works fine. Next week I plan to run a
series of automated and stress test. Will keep the group posted about
the results.

Thanks
Anshul Makkar

On Fri, Aug 1, 2014 at 6:42 AM, Gu Zheng  wrote:
> Hi Anshul,
> Thanks for your test.
> On 07/30/2014 10:31 PM, Anshul Makkar wrote:
>
>> Hi,
>>
>> I am testing the cpu-hotunplug  patches. I observed that after the
>> deletion of the cpu with id = x, if I cpu-add the same cpu again id =
>> x, then qemu exits with the error that file descriptor already exists.
>
> Could you please offer the whole reproduce routine? In my test box, we
> can add a removed cpu with the id.
>
>>
>> On debugging I found that if I give cpu-add , then
>> qemu_kvm_cpu_thread_fn->kvm_init_vcpu is called which sends an IOCTL
>> (KVM_CREATE_VCPU) to kvm to create a new fd. As the fd already exists
>> in KVM as we never delete the fd from the kernel and just park it in
>> separate list, it returns false and QEMU exits. In the above code
>> flow, no where its being checked if we have the cpu with cpuid = x
>> available in the parked list and we can reuse it.
>>
>> Am I missing something or this bit is yet to be implmented.
>
> Yes, it is implemented, in the same way as you mention above, please refer
> to function kvm_get_vcpu().
>
> Thanks,
> Gu
>
>>
>> Thanks
>> Anshul Makkar
>>
>> On Fri, Jul 18, 2014 at 4:09 AM, Gu Zheng  wrote:
>>> Hi Anshul,
>>> On 07/18/2014 12:24 AM, Anshul Makkar wrote:
>>>
 Are we not going to introduce new command cpu_del for deleting the cpu ?

 I couldn't find any patch for addition of cpu_del command. Is this
 intentional and we intend to use device_del (and similarly device_add)
 for cpu hot(un)plug or just skipped to be added later. I have the
 patch for the same which I can release, if the intent is to add this
 command.
>>>
>>> The "device_add/device_del" interface is the approved way to support 
>>> add/del cpu,
>>> which is also more common and elegant than "cpu_add/del".
>>> 
>>> so we intend to use device_del rather than the cpu_del.
>>> And IMO, the cpu_add will be replaced by "device_add" sooner or later.
>>>
>>> Thanks,
>>> Gu
>>>

 Thanks
 Anshul Makkar

 On Fri, Jul 11, 2014 at 11:59 AM, Gu Zheng  wrote:
> After ACPI get a signal to eject a vCPU, the vCPU must be
> removed from CPU list，before the vCPU really removed,  then
> release the all related vCPU objects.
> But we do not close KVM vcpu fd, just record it into a list, in
> order to reuse it.
>
> Signed-off-by: Chen Fan 
> Signed-off-by: Gu Zheng 
> ---
>  cpus.c   |   37 
>  include/sysemu/kvm.h |1 +
>  kvm-all.c|   57 
> +-
>  3 files changed, 94 insertions(+), 1 deletions(-)
>
> diff --git a/cpus.c b/cpus.c
> index 4dfb889..9a73407 100644
> --- a/cpus.c
> +++ b/cpus.c
> @@ -786,6 +786,24 @@ void async_run_on_cpu(CPUState *cpu, void 
> (*func)(void *data), void *data)
>  qemu_cpu_kick(cpu);
>  }
>
> +static void qemu_kvm_destroy_vcpu(CPUState *cpu)
> +{
> +CPU_REMOVE(cpu);
> +
> +if (kvm_destroy_vcpu(cpu) < 0) {
> +fprintf(stderr, "kvm_destroy_vcpu failed.\n");
> +exit(1);
> +}
> +
> +object_unparent(OBJECT(cpu));
> +}
> +
> +static void qemu_tcg_destroy_vcpu(CPUState *cpu)
> +{
> +CPU_REMOVE(cpu);
> +object_unparent(OBJECT(cpu));
> +}
> +
>  static void flush_queued_work(CPUState *cpu)
>  {
>  struct qemu_work_item *wi;
> @@ -877,6 +895,11 @@ static void *qemu_kvm_cpu_thread_fn(void *arg)
>  }
>  }
>  qemu_kvm_wait_io_event(cpu);
> +if (cpu->exit && !cpu_can_run(cpu)) {
> +qemu_kvm_destroy_vcpu(cpu);
> +qemu_mutex_unlock(&qemu_global_mutex);
> +return NULL;
> +}
>  }
>
>  return NULL;
> @@ -929,6 +952,7 @@ static void tcg_exec_all(void);
>  static void *qemu_tcg_cpu_thread_fn(void *arg)
>  {
>  CPUState *cpu = arg;
> +CPUState *remove_cpu = NULL;
>
>  qemu_tcg_init_cpu_signals();
>  qemu_thread_get_self(cpu->thread);
> @@ -961,6 +985,16 @@ static void *qemu_tcg_cpu_thread_fn(void *arg)
>  }
>  }
>  qemu_tcg_wait_io_event();
> +CPU_FOREACH(cpu) {
> +if (cpu->exit && !cpu_can_run(cpu)) {
> +remove_cpu = cpu;
> +break;
> +}
> +}
> +if (remove_cpu) {
> +qemu_tcg_destroy_vcpu(remove_cpu);
> +   

Re: [Qemu-devel] [PATCH v2 7/7] target-arm: Call the pmccntr_sync function when swapping ELs

[Peter Maydell]

On 26 June 2014 06:02, Alistair Francis  wrote:
> Call the new pmccntr_sync() function when there is a possibility
> of swapping ELs (I.E. when there is an exception)
>
> Signed-off-by: Alistair Francis 
> ---
>
>  target-arm/helper-a64.c |5 +
>  target-arm/helper.c |7 +++
>  target-arm/op_helper.c  |6 ++
>  3 files changed, 18 insertions(+), 0 deletions(-)
>
> diff --git a/target-arm/helper-a64.c b/target-arm/helper-a64.c
> index 2b4ce6a..b61174f 100644
> --- a/target-arm/helper-a64.c
> +++ b/target-arm/helper-a64.c
> @@ -446,6 +446,8 @@ void aarch64_cpu_do_interrupt(CPUState *cs)
>  target_ulong addr = env->cp15.vbar_el[1];
>  int i;
>
> +pmccntr_sync(env);
> +
>  if (arm_current_pl(env) == 0) {
>  if (env->aarch64) {
>  addr += 0x400;
> @@ -484,6 +486,7 @@ void aarch64_cpu_do_interrupt(CPUState *cs)
>  addr += 0x100;
>  break;
>  default:
> +pmccntr_sync(env);
>  cpu_abort(cs, "Unhandled exception 0x%x\n", cs->exception_index);
>  }
>
> @@ -511,4 +514,6 @@ void aarch64_cpu_do_interrupt(CPUState *cs)
>
>  env->pc = addr;
>  cs->interrupt_request |= CPU_INTERRUPT_EXITTB;
> +
> +pmccntr_sync(env);
>  }
> diff --git a/target-arm/helper.c b/target-arm/helper.c
> index e78c5a7..f05d912 100644
> --- a/target-arm/helper.c
> +++ b/target-arm/helper.c
> @@ -3457,6 +3457,8 @@ void arm_cpu_do_interrupt(CPUState *cs)
>
>  assert(!IS_M(env));
>
> +pmccntr_sync(env);
> +
>  arm_log_exception(cs->exception_index);
>
>  /* TODO: Vectored interrupt controller.  */
> @@ -3487,6 +3489,7 @@ void arm_cpu_do_interrupt(CPUState *cs)
>&& (env->uncached_cpsr & CPSR_M) != ARM_CPU_MODE_USR) {
>  env->regs[0] = do_arm_semihosting(env);
>  qemu_log_mask(CPU_LOG_INT, "...handled as semihosting 
> call\n");
> +pmccntr_sync(env);
>  return;
>  }
>  }
> @@ -3505,6 +3508,7 @@ void arm_cpu_do_interrupt(CPUState *cs)
>  env->regs[15] += 2;
>  env->regs[0] = do_arm_semihosting(env);
>  qemu_log_mask(CPU_LOG_INT, "...handled as semihosting 
> call\n");
> +pmccntr_sync(env);
>  return;
>  }
>  }
> @@ -3548,6 +3552,7 @@ void arm_cpu_do_interrupt(CPUState *cs)
>  offset = 4;
>  break;
>  default:
> +pmccntr_sync(env);
>  cpu_abort(cs, "Unhandled exception 0x%x\n", cs->exception_index);
>  return; /* Never happens.  Keep compiler happy.  */
>  }
> @@ -3580,6 +3585,8 @@ void arm_cpu_do_interrupt(CPUState *cs)
>  env->regs[14] = env->regs[15] + offset;
>  env->regs[15] = addr;
>  cs->interrupt_request |= CPU_INTERRUPT_EXITTB;
> +
> +pmccntr_sync(env);
>  }
>
>  /* Check section/page access permissions.
> diff --git a/target-arm/op_helper.c b/target-arm/op_helper.c
> index 9c1ef52..07ab30b 100644
> --- a/target-arm/op_helper.c
> +++ b/target-arm/op_helper.c
> @@ -376,6 +376,8 @@ void HELPER(exception_return)(CPUARMState *env)
>  uint32_t spsr = env->banked_spsr[spsr_idx];
>  int new_el, i;
>
> +pmccntr_sync(env);
> +
>  if (env->pstate & PSTATE_SP) {
>  env->sp_el[cur_el] = env->xregs[31];
>  } else {
> @@ -418,6 +420,8 @@ void HELPER(exception_return)(CPUARMState *env)
>  env->pc = env->elr_el[cur_el];
>  }
>
> +pmccntr_sync(env);
> +
>  return;
>
>  illegal_return:
> @@ -433,6 +437,8 @@ illegal_return:
>  spsr &= PSTATE_NZCV | PSTATE_DAIF;
>  spsr |= pstate_read(env) & ~(PSTATE_NZCV | PSTATE_DAIF);
>  pstate_write(env, spsr);
> +
> +pmccntr_sync(env);
>  }

This feels way too fragile to me to be scattering these sync
calls all over the place like this. We need to find a better
approach than this.

thanks
-- PMM

[Qemu-devel] [PATCH v3 1/4] libqos: Correct mask to align size to PAGE_SIZE in malloc-pc

[John Snow]

From: Marc Marí 

Signed-off-by: Paolo Bonzini 
Signed-off-by: Marc Marí 
Signed-off-by: John Snow 
---
 tests/libqos/malloc-pc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/libqos/malloc-pc.c b/tests/libqos/malloc-pc.c
index db1496c..2efd095 100644
--- a/tests/libqos/malloc-pc.c
+++ b/tests/libqos/malloc-pc.c
@@ -36,7 +36,7 @@ static uint64_t pc_alloc(QGuestAllocator *allocator, size_t 
size)
 
 
 size += (PAGE_SIZE - 1);
-size &= PAGE_SIZE;
+size &= -PAGE_SIZE;
 
 g_assert_cmpint((s->start + size), <=, s->end);
 
-- 
1.9.3

[Qemu-devel] [PATCH v3 0/4] libqos: add a simple first-fit memory allocator

[John Snow]

This set collects two patches by Marc Marí already on the mailing list,
but goes further by adding a simple memory allocator that allows us to
track and debug freed memory, and optionally keep track of any leaks.

For convenience: https://github.com/jnsnow/qemu/tree/libqos-alloc

v2: use QTAILQ as a basis for the linked list implementation instead.
Correct an error in the size of the initial node.
v3: remove mlist wrappers around QTAILQ interface for clarity.
adjust the options controlling when to do allocation list debugging.

John Snow (2):
  libqos: add a simple first-fit memory allocator
  qtest/ide: Uninitialize PC allocator

Marc Marí (2):
  libqos: Correct mask to align size to PAGE_SIZE in malloc-pc
  libqos: Change free function called in malloc

 tests/ide-test.c |   2 +
 tests/libqos/malloc-pc.c | 280 +--
 tests/libqos/malloc-pc.h |   9 ++
 tests/libqos/malloc.h|   2 +-
 4 files changed, 282 insertions(+), 11 deletions(-)

-- 
1.9.3