Re: [Qemu-devel] qemu drive-mirror to rbd storage : no sparse rbd image

2014-10-13 Thread Alexandre DERUMIER
Lack of bdrv_co_write_zeroes is why detect-zeroes does not work.

Lack of bdrv_get_block_status is why sparse-sparse does not work
without detect-zeroes.

Ok, thanks Paolo !

Both are missing in rbd block driver. @ceph-devel . Could it be possible to 
implement them ?



Also, about drive-mirror, I had tried with detect-zeroes with simple qcow2 file,
and It don't seem to help.
I'm not sure that detect-zeroes is implement in drive-mirror.

also, the target mirrored volume don't seem to have the detect-zeroes option


# info block
drive-virtio1: /source.qcow2 (qcow2)
Detect zeroes:on

#du -sh source.qcow2 : 2M

drive-mirror  source.qcow2 - target.qcow2

# info block
drive-virtio1: /target.qcow2 (qcow2)

#du -sh target.qcow2 : 11G




- Mail original -

De: Paolo Bonzini pbonz...@redhat.com
À: Alexandre DERUMIER aderum...@odiso.com, qemu-devel 
qemu-devel@nongnu.org
Cc: Ceph Devel ceph-de...@vger.kernel.org
Envoyé: Dimanche 12 Octobre 2014 15:02:12
Objet: Re: qemu drive-mirror to rbd storage : no sparse rbd image

Il 08/10/2014 13:15, Alexandre DERUMIER ha scritto:
 Hi,

 I'm currently planning to migrate our storage to ceph/rbd through qemu 
 drive-mirror

 and It seem that drive-mirror with rbd block driver, don't create a sparse 
 image. (all zeros are copied to the target rbd).

 Also note, that it's working fine with qemu-img convert , the rbd volume is 
 sparse after conversion.


 Could it be related to the bdrv_co_write_zeroes missing features in 
 block/rbd.c ?

 (It's available in other block drivers (scsi,gluster,raw-aio) , and I don't 
 have this problem with theses block drivers).

Lack of bdrv_co_write_zeroes is why detect-zeroes does not work.

Lack of bdrv_get_block_status is why sparse-sparse does not work
without detect-zeroes.

Paolo



Re: [Qemu-devel] [question] is it possible that big-endian l1 tableoffset referenced by other I/O while updating l1 table offset in qcow2_update_snapshot_refcount?

2014-10-13 Thread Max Reitz

Am 13.10.2014 um 05:17 schrieb Zhang Haoyu:

Hi,
I encounter a problem that after deleting snapshot, the qcow2 image size is 
very larger than that it should be displayed by ls command,
but the virtual disk size is okay via qemu-img info.
I suspect that during updating l1 table offset, other I/O job reference the 
big-endian l1 table offset (very large value),
so the file is truncated to very large.

Not quite.  Rather, all the data that the snapshot used to occupy is
still consuming holes in the file; the maximum offset of the file is
still unchanged, even if the file is no longer using as many referenced
clusters.  Recent changes have gone in to sparsify the file when
possible (punching holes if your kernel and file system is new enough to
support that), so that it is not consuming the amount of disk space that
a mere ls reports.  But if what you are asking for is a way to compact
the file back down, then you'll need to submit a patch.  The idea of
having an online defragmenter for qcow2 files has been kicked around
before, but it is complex enough that no one has attempted a patch yet.

Sorry, I didn't clarify the problem clearly.
In qcow2_update_snapshot_refcount(), below code,
  /* Update L1 only if it isn't deleted anyway (addend = -1) */
  if (ret == 0  addend = 0  l1_modified) {
  for (i = 0; i  l1_size; i++) {
  cpu_to_be64s(l1_table[i]);
  }

  ret = bdrv_pwrite_sync(bs-file, l1_table_offset, l1_table, l1_size2);

  for (i = 0; i  l1_size; i++) {
  be64_to_cpus(l1_table[i]);
  }
  }
between cpu_to_be64s(l1_table[i]); and be64_to_cpus(l1_table[i]);,
is it possible that there is other I/O reference this interim l1 table whose 
entries contain the be64 l2 table offset?
The be64 l2 table offset maybe a very large value, hundreds of TB is possible,
then the qcow2 file will be truncated to far larger than normal size.
So we'll see the huge size of the qcow2 file by ls -hl, but the size is still 
normal displayed by qemu-img info.

If the possibility mentioned above exists, below raw code may fix it,
   if (ret == 0  addend = 0  l1_modified) {
  tmp_l1_table = g_malloc0(l1_size * sizeof(uint64_t))
  memcpy(tmp_l1_table, l1_table, l1_size * sizeof(uint64_t));
  for (i = 0; i  l1_size; i++) {
  cpu_to_be64s(tmp_l1_table[i]);
  }
  ret = bdrv_pwrite_sync(bs-file, l1_table_offset, tmp_l1_table, 
l1_size2);

  free(tmp_l1_table);
  }

l1_table is already a local variable (local to
qcow2_update_snapshot_refcount()), so I can't really imagine how
introducing another local buffer should mitigate the problem, if there
is any.


l1_table is not necessarily a local variable to qcow2_update_snapshot_refcount,
which depends on condition of if (l1_table_offset != s-l1_table_offset),
if the condition not true, l1_table = s-l1_table.


Oh, yes, you're right. Okay, so in theory nothing should happen anyway, 
because qcow2 does not have to be reentrant (so s-l1_table will not be 
accessed while it's big endian and therefore possibly not in CPU order). 
But I find it rather ugly to convert the cached L1 table to big endian, 
so I'd be fine with the patch you proposed.


Max



Re: [Qemu-devel] [PATCH RFC 03/11] virtio: support more feature bits

2014-10-13 Thread Rusty Russell
Cornelia Huck cornelia.h...@de.ibm.com writes:
 With virtio-1, we support more than 32 feature bits. Let's make
 vdev-guest_features depend on the number of supported feature bits,
 allowing us to grow the feature bits automatically.

It's a judgement call, but I would say that simply using uint64_t
will be sufficient for quite a while.

Cheers,
Rusty.



Re: [Qemu-devel] [PATCH RFC 08/11] virtio_blk: use virtio v1.0 endian

2014-10-13 Thread Rusty Russell
Cornelia Huck cornelia.h...@de.ibm.com writes:
 Note that we care only about the fields still in use for virtio v1.0.

 Reviewed-by: Thomas Huth th...@linux.vnet.ibm.com
 Reviewed-by: David Hildenbrand d...@linux.vnet.ibm.com
 Signed-off-by: Cornelia Huck cornelia.h...@de.ibm.com

Hi Cornelia,

These patches all look good; I'm a bit nervous about our testing
missing some conversion, so we'll need qemu patches for PCI so we can
test on other platforms too.

Thanks,
Rusty.



Re: [Qemu-devel] qemu drive-mirror to rbd storage : no sparse rbd image

2014-10-13 Thread Paolo Bonzini
Il 13/10/2014 08:06, Alexandre DERUMIER ha scritto:
 
 Also, about drive-mirror, I had tried with detect-zeroes with simple qcow2 
 file, 
 and It don't seem to help.
 I'm not sure that detect-zeroes is implement in drive-mirror.
 
 also, the target mirrored volume don't seem to have the detect-zeroes option
 
 
 # info block
 drive-virtio1: /source.qcow2 (qcow2)
 Detect zeroes:on
 
 #du -sh source.qcow2 : 2M
 
 drive-mirror  source.qcow2 - target.qcow2
 
 # info block
 drive-virtio1: /target.qcow2 (qcow2)
 
 #du -sh target.qcow2 : 11G
 

Ah, you're right.  We need to add an options field, or use a new
blockdev-mirror command.

Paolo



Re: [Qemu-devel] [question] is it possible that big-endian l1 tableoffsetreferenced by other I/O while updating l1 table offset in qcow2_update_snapshot_refcount?

2014-10-13 Thread Zhang Haoyu
 Hi,
 I encounter a problem that after deleting snapshot, the qcow2 image size 
 is very larger than that it should be displayed by ls command,
 but the virtual disk size is okay via qemu-img info.
 I suspect that during updating l1 table offset, other I/O job reference 
 the big-endian l1 table offset (very large value),
 so the file is truncated to very large.
 Not quite.  Rather, all the data that the snapshot used to occupy is
 still consuming holes in the file; the maximum offset of the file is
 still unchanged, even if the file is no longer using as many referenced
 clusters.  Recent changes have gone in to sparsify the file when
 possible (punching holes if your kernel and file system is new enough to
 support that), so that it is not consuming the amount of disk space that
 a mere ls reports.  But if what you are asking for is a way to compact
 the file back down, then you'll need to submit a patch.  The idea of
 having an online defragmenter for qcow2 files has been kicked around
 before, but it is complex enough that no one has attempted a patch yet.
 Sorry, I didn't clarify the problem clearly.
 In qcow2_update_snapshot_refcount(), below code,
   /* Update L1 only if it isn't deleted anyway (addend = -1) */
   if (ret == 0  addend = 0  l1_modified) {
   for (i = 0; i  l1_size; i++) {
   cpu_to_be64s(l1_table[i]);
   }

   ret = bdrv_pwrite_sync(bs-file, l1_table_offset, l1_table, 
 l1_size2);

   for (i = 0; i  l1_size; i++) {
   be64_to_cpus(l1_table[i]);
   }
   }
 between cpu_to_be64s(l1_table[i]); and be64_to_cpus(l1_table[i]);,
 is it possible that there is other I/O reference this interim l1 table 
 whose entries contain the be64 l2 table offset?
 The be64 l2 table offset maybe a very large value, hundreds of TB is 
 possible,
 then the qcow2 file will be truncated to far larger than normal size.
 So we'll see the huge size of the qcow2 file by ls -hl, but the size is 
 still normal displayed by qemu-img info.

 If the possibility mentioned above exists, below raw code may fix it,
if (ret == 0  addend = 0  l1_modified) {
   tmp_l1_table = g_malloc0(l1_size * sizeof(uint64_t))
   memcpy(tmp_l1_table, l1_table, l1_size * sizeof(uint64_t));
   for (i = 0; i  l1_size; i++) {
   cpu_to_be64s(tmp_l1_table[i]);
   }
   ret = bdrv_pwrite_sync(bs-file, l1_table_offset, tmp_l1_table, 
 l1_size2);

   free(tmp_l1_table);
   }
 l1_table is already a local variable (local to
 qcow2_update_snapshot_refcount()), so I can't really imagine how
 introducing another local buffer should mitigate the problem, if there
 is any.

 l1_table is not necessarily a local variable to 
 qcow2_update_snapshot_refcount,
 which depends on condition of if (l1_table_offset != s-l1_table_offset),
 if the condition not true, l1_table = s-l1_table.

Oh, yes, you're right. Okay, so in theory nothing should happen anyway, 
because qcow2 does not have to be reentrant (so s-l1_table will not be 
accessed while it's big endian and therefore possibly not in CPU order). 
Could you detail how qcow2 does not have to be reentrant?
In below stack,
qcow2_update_snapshot_refcount
|- cpu_to_be64s(l1_table[i])
|- bdrv_pwrite_sync
|-- bdrv_pwrite
|--- bdrv_pwritev
| bdrv_prwv_co
|- aio_poll(aio_context) == this aio_context is qemu_aio_context
|-- aio_dispatch
|--- bdrv_co_io_em_complete
| qemu_coroutine_enter(co-coroutine, NULL); == coroutine entry is 
bdrv_co_do_rw
bdrv_co_do_rw will access l1_table to perform I/O operation.

Thanks,
Zhang Haoyu
But I find it rather ugly to convert the cached L1 table to big endian, 
so I'd be fine with the patch you proposed.

Max




Re: [Qemu-devel] [question] is it possible that big-endian l1 tableoffsetreferenced by other I/O while updating l1 table offset in qcow2_update_snapshot_refcount?

2014-10-13 Thread Max Reitz

Am 13.10.2014 um 09:13 schrieb Zhang Haoyu:

Hi,
I encounter a problem that after deleting snapshot, the qcow2 image size is 
very larger than that it should be displayed by ls command,
but the virtual disk size is okay via qemu-img info.
I suspect that during updating l1 table offset, other I/O job reference the 
big-endian l1 table offset (very large value),
so the file is truncated to very large.

Not quite.  Rather, all the data that the snapshot used to occupy is
still consuming holes in the file; the maximum offset of the file is
still unchanged, even if the file is no longer using as many referenced
clusters.  Recent changes have gone in to sparsify the file when
possible (punching holes if your kernel and file system is new enough to
support that), so that it is not consuming the amount of disk space that
a mere ls reports.  But if what you are asking for is a way to compact
the file back down, then you'll need to submit a patch.  The idea of
having an online defragmenter for qcow2 files has been kicked around
before, but it is complex enough that no one has attempted a patch yet.

Sorry, I didn't clarify the problem clearly.
In qcow2_update_snapshot_refcount(), below code,
   /* Update L1 only if it isn't deleted anyway (addend = -1) */
   if (ret == 0  addend = 0  l1_modified) {
   for (i = 0; i  l1_size; i++) {
   cpu_to_be64s(l1_table[i]);
   }

   ret = bdrv_pwrite_sync(bs-file, l1_table_offset, l1_table, 
l1_size2);

   for (i = 0; i  l1_size; i++) {
   be64_to_cpus(l1_table[i]);
   }
   }
between cpu_to_be64s(l1_table[i]); and be64_to_cpus(l1_table[i]);,
is it possible that there is other I/O reference this interim l1 table whose 
entries contain the be64 l2 table offset?
The be64 l2 table offset maybe a very large value, hundreds of TB is possible,
then the qcow2 file will be truncated to far larger than normal size.
So we'll see the huge size of the qcow2 file by ls -hl, but the size is still 
normal displayed by qemu-img info.

If the possibility mentioned above exists, below raw code may fix it,
if (ret == 0  addend = 0  l1_modified) {
   tmp_l1_table = g_malloc0(l1_size * sizeof(uint64_t))
   memcpy(tmp_l1_table, l1_table, l1_size * sizeof(uint64_t));
   for (i = 0; i  l1_size; i++) {
   cpu_to_be64s(tmp_l1_table[i]);
   }
   ret = bdrv_pwrite_sync(bs-file, l1_table_offset, tmp_l1_table, 
l1_size2);

   free(tmp_l1_table);
   }

l1_table is already a local variable (local to
qcow2_update_snapshot_refcount()), so I can't really imagine how
introducing another local buffer should mitigate the problem, if there
is any.


l1_table is not necessarily a local variable to qcow2_update_snapshot_refcount,
which depends on condition of if (l1_table_offset != s-l1_table_offset),
if the condition not true, l1_table = s-l1_table.

Oh, yes, you're right. Okay, so in theory nothing should happen anyway,
because qcow2 does not have to be reentrant (so s-l1_table will not be
accessed while it's big endian and therefore possibly not in CPU order).

Could you detail how qcow2 does not have to be reentrant?
In below stack,
qcow2_update_snapshot_refcount
|- cpu_to_be64s(l1_table[i])
|- bdrv_pwrite_sync


This is executed on bs-file, not the qcow2 BDS.

Max


|-- bdrv_pwrite
|--- bdrv_pwritev
| bdrv_prwv_co
|- aio_poll(aio_context) == this aio_context is qemu_aio_context
|-- aio_dispatch
|--- bdrv_co_io_em_complete
| qemu_coroutine_enter(co-coroutine, NULL); == coroutine entry is 
bdrv_co_do_rw
bdrv_co_do_rw will access l1_table to perform I/O operation.

Thanks,
Zhang Haoyu

But I find it rather ugly to convert the cached L1 table to big endian,
so I'd be fine with the patch you proposed.

Max





Re: [Qemu-devel] qemu drive-mirror to rbd storage : no sparse rbd image

2014-10-13 Thread Alexandre DERUMIER
Ah, you're right.  We need to add an options field, or use a new
blockdev-mirror command.

Ok, thanks. Can't help to implement this, but I'll glad to help for testing.


- Mail original -

De: Paolo Bonzini pbonz...@redhat.com
À: Alexandre DERUMIER aderum...@odiso.com
Cc: Ceph Devel ceph-de...@vger.kernel.org, qemu-devel 
qemu-devel@nongnu.org
Envoyé: Lundi 13 Octobre 2014 09:06:01
Objet: Re: qemu drive-mirror to rbd storage : no sparse rbd image

Il 13/10/2014 08:06, Alexandre DERUMIER ha scritto:

 Also, about drive-mirror, I had tried with detect-zeroes with simple qcow2 
 file,
 and It don't seem to help.
 I'm not sure that detect-zeroes is implement in drive-mirror.

 also, the target mirrored volume don't seem to have the detect-zeroes option


 # info block
 drive-virtio1: /source.qcow2 (qcow2)
 Detect zeroes: on

 #du -sh source.qcow2 : 2M

 drive-mirror source.qcow2 - target.qcow2

 # info block
 drive-virtio1: /target.qcow2 (qcow2)

 #du -sh target.qcow2 : 11G


Ah, you're right. We need to add an options field, or use a new
blockdev-mirror command.

Paolo



Re: [Qemu-devel] [question] is it possible that big-endian l1 tableoffsetreferencedby other I/O while updating l1 table offset in qcow2_update_snapshot_refcount?

2014-10-13 Thread Zhang Haoyu
 Hi,
 I encounter a problem that after deleting snapshot, the qcow2 image 
 size is very larger than that it should be displayed by ls command,
 but the virtual disk size is okay via qemu-img info.
 I suspect that during updating l1 table offset, other I/O job 
 reference the big-endian l1 table offset (very large value),
 so the file is truncated to very large.
 Not quite.  Rather, all the data that the snapshot used to occupy is
 still consuming holes in the file; the maximum offset of the file is
 still unchanged, even if the file is no longer using as many referenced
 clusters.  Recent changes have gone in to sparsify the file when
 possible (punching holes if your kernel and file system is new enough to
 support that), so that it is not consuming the amount of disk space that
 a mere ls reports.  But if what you are asking for is a way to compact
 the file back down, then you'll need to submit a patch.  The idea of
 having an online defragmenter for qcow2 files has been kicked around
 before, but it is complex enough that no one has attempted a patch yet.
 Sorry, I didn't clarify the problem clearly.
 In qcow2_update_snapshot_refcount(), below code,
/* Update L1 only if it isn't deleted anyway (addend = -1) */
if (ret == 0  addend = 0  l1_modified) {
for (i = 0; i  l1_size; i++) {
cpu_to_be64s(l1_table[i]);
}

ret = bdrv_pwrite_sync(bs-file, l1_table_offset, l1_table, 
 l1_size2);

for (i = 0; i  l1_size; i++) {
be64_to_cpus(l1_table[i]);
}
}
 between cpu_to_be64s(l1_table[i]); and be64_to_cpus(l1_table[i]);,
 is it possible that there is other I/O reference this interim l1 table 
 whose entries contain the be64 l2 table offset?
 The be64 l2 table offset maybe a very large value, hundreds of TB is 
 possible,
 then the qcow2 file will be truncated to far larger than normal size.
 So we'll see the huge size of the qcow2 file by ls -hl, but the size is 
 still normal displayed by qemu-img info.

 If the possibility mentioned above exists, below raw code may fix it,
 if (ret == 0  addend = 0  l1_modified) {
tmp_l1_table = g_malloc0(l1_size * sizeof(uint64_t))
memcpy(tmp_l1_table, l1_table, l1_size * sizeof(uint64_t));
for (i = 0; i  l1_size; i++) {
cpu_to_be64s(tmp_l1_table[i]);
}
ret = bdrv_pwrite_sync(bs-file, l1_table_offset, 
 tmp_l1_table, l1_size2);

free(tmp_l1_table);
}
 l1_table is already a local variable (local to
 qcow2_update_snapshot_refcount()), so I can't really imagine how
 introducing another local buffer should mitigate the problem, if there
 is any.

 l1_table is not necessarily a local variable to 
 qcow2_update_snapshot_refcount,
 which depends on condition of if (l1_table_offset != s-l1_table_offset),
 if the condition not true, l1_table = s-l1_table.
 Oh, yes, you're right. Okay, so in theory nothing should happen anyway,
 because qcow2 does not have to be reentrant (so s-l1_table will not be
 accessed while it's big endian and therefore possibly not in CPU order).
 Could you detail how qcow2 does not have to be reentrant?
 In below stack,
 qcow2_update_snapshot_refcount
 |- cpu_to_be64s(l1_table[i])
 |- bdrv_pwrite_sync

This is executed on bs-file, not the qcow2 BDS.

Yes, bs-file is passed to bdrv_pwrite_sync here, 
but aio_poll(aio_context) will poll all BDS's aio, not only that of bs-file, 
doesn't it?
Is it possible that there are pending aio which belong to this qcow2 BDS still 
exist?

Thanks,
Zhang Haoyu
Max

 |-- bdrv_pwrite
 |--- bdrv_pwritev
 | bdrv_prwv_co
 |- aio_poll(aio_context) == this aio_context is qemu_aio_context
 |-- aio_dispatch
 |--- bdrv_co_io_em_complete
 | qemu_coroutine_enter(co-coroutine, NULL); == coroutine entry is 
 bdrv_co_do_rw
 bdrv_co_do_rw will access l1_table to perform I/O operation.

 Thanks,
 Zhang Haoyu
 But I find it rather ugly to convert the cached L1 table to big endian,
 so I'd be fine with the patch you proposed.

 Max




Re: [Qemu-devel] [PATCH 1/1] pci-host: add educational driver

2014-10-13 Thread Jiri Slaby
On 10/10/2014, 04:54 PM, Claudio Fontana wrote:
 Hello,
 
 On 10.10.2014 14:09, Jiri Slaby wrote:
 I am using qemu for teaching the Linux kernel at our university. I
 wrote a simple PCI device that can answer to writes/reads, generate
 interrupts and perform DMA. As I am dragging it locally over 2 years,
 I am sending it to you now.

 Signed-off-by: Jiri Slaby jsl...@suse.cz
 
 is this supposed to be architecture independent, or is it X86-specific?

Hi,

I did not plan it to be only x86 specific. If you see any problems, I
will fix them.

 Also at first glance I see multiple 32bit variables used to hold addresses,
 is this 32bit-only?

No, the DMA addresses are on purpose 32-bit: to teach the people always
set the dma mask properly in the driver. This driver copies COMBO6x
devices (liberouter.org) behaviour which I used until the cards got
obsoleted (hard to find PCI-X slots nowadays).

I can make this configurable if you wish.

 I wonder if this work could be merged / integrated with the Generic PCI host 
 patches that are flying around since some time...

Could you point me to some?

thanks,
-- 
js
suse labs



Re: [Qemu-devel] [PATCH] virtio-pci: fix migration for pci bus master

2014-10-13 Thread Greg Kurz
On Mon, 6 Oct 2014 20:25:04 +0300
Michael S. Tsirkin m...@redhat.com wrote:
 [...]
 
 BTW I reverted that patch, and to fix migration, I'm thinking
 about applying the following patch on top of master.
 

Michael,

I could force the migration issue with a rhel65 guest thanks to the
following patch, applied to hw/virtio/virtio-pci.c in QEMU v2.1.

@@ -271,6 +272,16 @@ static void virtio_ioport_write(void *opaque, uint32_t 
addr, uint32_t val)
 VirtIODevice *vdev = virtio_bus_get_device(proxy-bus);
 hwaddr pa;
 
+if ((vdev-status == (VIRTIO_CONFIG_S_ACKNOWLEDGE | 
VIRTIO_CONFIG_S_DRIVER))
+ (!(proxy-pci_dev.config[PCI_COMMAND]  PCI_COMMAND_MASTER))
+ getenv(MIG_BUG))
+{
+fprintf(stderr, \n\n\n\tMIGRATE !\n\n\n);
+qmp_migrate(getenv(MIG_BUG), false, false, false, false, false,
+false, NULL);
+unsetenv(MIG_BUG);
+}
+
 switch (addr) {
 case VIRTIO_PCI_GUEST_FEATURES:
 /* Guest does not negotiate properly?  We have to assume nothing. */


Indeed, the destination QEMU master hangs because bus master isn't set.

 Would appreciate testing cross-version migration (2.1 to master)
 with this patch applied.
 

I had first to to enable the property for pseries of course. I could then
migrate QEMU v2.1 to QEMU master and back to QEMU v2.1, in the window
where we have DRIVER enabled and MASTER disabled, without experiencing
the hang.

Your fix works as expected (just a remark below).

 
 
 diff --git a/hw/virtio/virtio-pci.h b/hw/virtio/virtio-pci.h
 index 1cea157..8873b6d 100644
 --- a/hw/virtio/virtio-pci.h
 +++ b/hw/virtio/virtio-pci.h
 @@ -53,6 +53,11 @@ typedef struct VirtioBusClass VirtioPCIBusClass;
  #define VIRTIO_PCI_BUS_CLASS(klass) \
  OBJECT_CLASS_CHECK(VirtioPCIBusClass, klass, TYPE_VIRTIO_PCI_BUS)
 
 +/* Need to activate work-arounds for buggy guests at vmstate load. */
 +#define VIRTIO_PCI_FLAG_BUS_MASTER_BUG_MIGRATION_BIT  0
 +#define VIRTIO_PCI_FLAG_BUS_MASTER_BUG_MIGRATION \
 +(1  VIRTIO_PCI_FLAG_BUS_MASTER_BUG_MIGRATION_BIT)
 +
  /* Performance improves when virtqueue kick processing is decoupled from the
   * vcpu thread using ioeventfd for some devices. */
  #define VIRTIO_PCI_FLAG_USE_IOEVENTFD_BIT 1
 diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
 index bae023a..e07b6c4 100644
 --- a/include/hw/i386/pc.h
 +++ b/include/hw/i386/pc.h
 @@ -312,6 +312,11 @@ bool e820_get_entry(int, uint32_t, uint64_t *, uint64_t 
 *);
  .driver   = intel-hda,\
  .property = old_msi_addr,\
  .value= on,\
 +},\
 +{\
 +.driver   = virtio-pci,\
 +.property = virtio-pci-bus-master-bug-migration,\
 +.value= on,\
  }
 
  #define PC_COMPAT_2_0 \

FWIW, the issue does not occur with intel targets, at least not
in my test case (booting rhel6 on a virtio-blk disk). I see bus
master is set early (bios ?) and never unset...

If you decide to apply for intel anyway, shouldn't the enablement be
in a separate patch ?

Will you resend or would you like me to do it, along with the pseries
enablement part ? In this case, I would need your SoB for the present
patch.

Thanks.

--
Greg

 diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c
 index a827cd4..a499a3c 100644
 --- a/hw/virtio/virtio-pci.c
 +++ b/hw/virtio/virtio-pci.c
 @@ -86,9 +86,6 @@
   * 12 is historical, and due to x86 page size. */
  #define VIRTIO_PCI_QUEUE_ADDR_SHIFT12
 
 -/* Flags track per-device state like workarounds for quirks in older guests. 
 */
 -#define VIRTIO_PCI_FLAG_BUS_MASTER_BUG  (1  0)
 -
  static void virtio_pci_bus_new(VirtioBusState *bus, size_t bus_size,
 VirtIOPCIProxy *dev);
 
 @@ -323,14 +320,6 @@ static void virtio_ioport_write(void *opaque, uint32_t 
 addr, uint32_t val)
   proxy-pci_dev.config[PCI_COMMAND] |
   PCI_COMMAND_MASTER, 1);
  }
 -
 -/* Linux before 2.6.34 sets the device as OK without enabling
 -   the PCI device bus master bit. In this case we need to disable
 -   some safety checks. */
 -if ((val  VIRTIO_CONFIG_S_DRIVER_OK) 
 -!(proxy-pci_dev.config[PCI_COMMAND]  PCI_COMMAND_MASTER)) {
 -proxy-flags |= VIRTIO_PCI_FLAG_BUS_MASTER_BUG;
 -}
  break;
  case VIRTIO_MSI_CONFIG_VECTOR:
  msix_vector_unuse(proxy-pci_dev, vdev-config_vector);
 @@ -483,8 +472,7 @@ static void virtio_write_config(PCIDevice *pci_dev, 
 uint32_t address,
  pci_default_write_config(pci_dev, address, val, len);
 
  if (range_covers_byte(address, len, PCI_COMMAND) 
 -!(pci_dev-config[PCI_COMMAND]  PCI_COMMAND_MASTER) 
 -!(proxy-flags  VIRTIO_PCI_FLAG_BUS_MASTER_BUG)) {
 +!(pci_dev-config[PCI_COMMAND]  PCI_COMMAND_MASTER)) {
  virtio_pci_stop_ioeventfd(proxy);
  virtio_set_status(vdev, 

Re: [Qemu-devel] [PATCH] virtio-pci: fix migration for pci bus master

2014-10-13 Thread Michael S. Tsirkin
On Mon, Oct 13, 2014 at 10:49:41AM +0200, Greg Kurz wrote:
 On Mon, 6 Oct 2014 20:25:04 +0300
 Michael S. Tsirkin m...@redhat.com wrote:
  [...]
  
  BTW I reverted that patch, and to fix migration, I'm thinking
  about applying the following patch on top of master.
  
 
 Michael,
 
 I could force the migration issue with a rhel65 guest thanks to the
 following patch, applied to hw/virtio/virtio-pci.c in QEMU v2.1.
 
 @@ -271,6 +272,16 @@ static void virtio_ioport_write(void *opaque, uint32_t 
 addr, uint32_t val)
  VirtIODevice *vdev = virtio_bus_get_device(proxy-bus);
  hwaddr pa;
  
 +if ((vdev-status == (VIRTIO_CONFIG_S_ACKNOWLEDGE | 
 VIRTIO_CONFIG_S_DRIVER))
 + (!(proxy-pci_dev.config[PCI_COMMAND]  PCI_COMMAND_MASTER))
 + getenv(MIG_BUG))
 +{
 +fprintf(stderr, \n\n\n\tMIGRATE !\n\n\n);
 +qmp_migrate(getenv(MIG_BUG), false, false, false, false, false,
 +false, NULL);
 +unsetenv(MIG_BUG);
 +}
 +
  switch (addr) {
  case VIRTIO_PCI_GUEST_FEATURES:
  /* Guest does not negotiate properly?  We have to assume nothing. */
 
 
 Indeed, the destination QEMU master hangs because bus master isn't set.
 
  Would appreciate testing cross-version migration (2.1 to master)
  with this patch applied.
  
 
 I had first to to enable the property for pseries of course. I could then
 migrate QEMU v2.1 to QEMU master and back to QEMU v2.1, in the window
 where we have DRIVER enabled and MASTER disabled, without experiencing
 the hang.
 
 Your fix works as expected (just a remark below).
 
  
  
  diff --git a/hw/virtio/virtio-pci.h b/hw/virtio/virtio-pci.h
  index 1cea157..8873b6d 100644
  --- a/hw/virtio/virtio-pci.h
  +++ b/hw/virtio/virtio-pci.h
  @@ -53,6 +53,11 @@ typedef struct VirtioBusClass VirtioPCIBusClass;
   #define VIRTIO_PCI_BUS_CLASS(klass) \
   OBJECT_CLASS_CHECK(VirtioPCIBusClass, klass, TYPE_VIRTIO_PCI_BUS)
  
  +/* Need to activate work-arounds for buggy guests at vmstate load. */
  +#define VIRTIO_PCI_FLAG_BUS_MASTER_BUG_MIGRATION_BIT  0
  +#define VIRTIO_PCI_FLAG_BUS_MASTER_BUG_MIGRATION \
  +(1  VIRTIO_PCI_FLAG_BUS_MASTER_BUG_MIGRATION_BIT)
  +
   /* Performance improves when virtqueue kick processing is decoupled from 
  the
* vcpu thread using ioeventfd for some devices. */
   #define VIRTIO_PCI_FLAG_USE_IOEVENTFD_BIT 1
  diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
  index bae023a..e07b6c4 100644
  --- a/include/hw/i386/pc.h
  +++ b/include/hw/i386/pc.h
  @@ -312,6 +312,11 @@ bool e820_get_entry(int, uint32_t, uint64_t *, 
  uint64_t *);
   .driver   = intel-hda,\
   .property = old_msi_addr,\
   .value= on,\
  +},\
  +{\
  +.driver   = virtio-pci,\
  +.property = virtio-pci-bus-master-bug-migration,\
  +.value= on,\
   }
  
   #define PC_COMPAT_2_0 \
 
 FWIW, the issue does not occur with intel targets, at least not
 in my test case (booting rhel6 on a virtio-blk disk). I see bus
 master is set early (bios ?) and never unset...
 
 If you decide to apply for intel anyway, shouldn't the enablement be
 in a separate patch ?
 
 Will you resend or would you like me to do it, along with the pseries
 enablement part ? In this case, I would need your SoB for the present
 patch.
 
 Thanks.
 
 --
 Greg

AFAIK pseries doesn't support cross-version migration, does it?

  diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c
  index a827cd4..a499a3c 100644
  --- a/hw/virtio/virtio-pci.c
  +++ b/hw/virtio/virtio-pci.c
  @@ -86,9 +86,6 @@
* 12 is historical, and due to x86 page size. */
   #define VIRTIO_PCI_QUEUE_ADDR_SHIFT12
  
  -/* Flags track per-device state like workarounds for quirks in older 
  guests. */
  -#define VIRTIO_PCI_FLAG_BUS_MASTER_BUG  (1  0)
  -
   static void virtio_pci_bus_new(VirtioBusState *bus, size_t bus_size,
  VirtIOPCIProxy *dev);
  
  @@ -323,14 +320,6 @@ static void virtio_ioport_write(void *opaque, uint32_t 
  addr, uint32_t val)
proxy-pci_dev.config[PCI_COMMAND] |
PCI_COMMAND_MASTER, 1);
   }
  -
  -/* Linux before 2.6.34 sets the device as OK without enabling
  -   the PCI device bus master bit. In this case we need to disable
  -   some safety checks. */
  -if ((val  VIRTIO_CONFIG_S_DRIVER_OK) 
  -!(proxy-pci_dev.config[PCI_COMMAND]  PCI_COMMAND_MASTER)) {
  -proxy-flags |= VIRTIO_PCI_FLAG_BUS_MASTER_BUG;
  -}
   break;
   case VIRTIO_MSI_CONFIG_VECTOR:
   msix_vector_unuse(proxy-pci_dev, vdev-config_vector);
  @@ -483,8 +472,7 @@ static void virtio_write_config(PCIDevice *pci_dev, 
  uint32_t address,
   pci_default_write_config(pci_dev, address, val, len);
  
   if (range_covers_byte(address, len, PCI_COMMAND) 
  -

Re: [Qemu-devel] [question] is it possible that big-endian l1 tableoffsetreferencedby other I/O while updating l1 table offset in qcow2_update_snapshot_refcount?

2014-10-13 Thread Max Reitz

Am 13.10.2014 um 10:19 schrieb Zhang Haoyu:

Hi,
I encounter a problem that after deleting snapshot, the qcow2 image size is 
very larger than that it should be displayed by ls command,
but the virtual disk size is okay via qemu-img info.
I suspect that during updating l1 table offset, other I/O job reference the 
big-endian l1 table offset (very large value),
so the file is truncated to very large.

Not quite.  Rather, all the data that the snapshot used to occupy is
still consuming holes in the file; the maximum offset of the file is
still unchanged, even if the file is no longer using as many referenced
clusters.  Recent changes have gone in to sparsify the file when
possible (punching holes if your kernel and file system is new enough to
support that), so that it is not consuming the amount of disk space that
a mere ls reports.  But if what you are asking for is a way to compact
the file back down, then you'll need to submit a patch.  The idea of
having an online defragmenter for qcow2 files has been kicked around
before, but it is complex enough that no one has attempted a patch yet.

Sorry, I didn't clarify the problem clearly.
In qcow2_update_snapshot_refcount(), below code,
/* Update L1 only if it isn't deleted anyway (addend = -1) */
if (ret == 0  addend = 0  l1_modified) {
for (i = 0; i  l1_size; i++) {
cpu_to_be64s(l1_table[i]);
}

ret = bdrv_pwrite_sync(bs-file, l1_table_offset, l1_table, 
l1_size2);

for (i = 0; i  l1_size; i++) {
be64_to_cpus(l1_table[i]);
}
}
between cpu_to_be64s(l1_table[i]); and be64_to_cpus(l1_table[i]);,
is it possible that there is other I/O reference this interim l1 table whose 
entries contain the be64 l2 table offset?
The be64 l2 table offset maybe a very large value, hundreds of TB is possible,
then the qcow2 file will be truncated to far larger than normal size.
So we'll see the huge size of the qcow2 file by ls -hl, but the size is still 
normal displayed by qemu-img info.

If the possibility mentioned above exists, below raw code may fix it,
 if (ret == 0  addend = 0  l1_modified) {
tmp_l1_table = g_malloc0(l1_size * sizeof(uint64_t))
memcpy(tmp_l1_table, l1_table, l1_size * sizeof(uint64_t));
for (i = 0; i  l1_size; i++) {
cpu_to_be64s(tmp_l1_table[i]);
}
ret = bdrv_pwrite_sync(bs-file, l1_table_offset, tmp_l1_table, 
l1_size2);

free(tmp_l1_table);
}

l1_table is already a local variable (local to
qcow2_update_snapshot_refcount()), so I can't really imagine how
introducing another local buffer should mitigate the problem, if there
is any.


l1_table is not necessarily a local variable to qcow2_update_snapshot_refcount,
which depends on condition of if (l1_table_offset != s-l1_table_offset),
if the condition not true, l1_table = s-l1_table.

Oh, yes, you're right. Okay, so in theory nothing should happen anyway,
because qcow2 does not have to be reentrant (so s-l1_table will not be
accessed while it's big endian and therefore possibly not in CPU order).

Could you detail how qcow2 does not have to be reentrant?
In below stack,
qcow2_update_snapshot_refcount
|- cpu_to_be64s(l1_table[i])
|- bdrv_pwrite_sync

This is executed on bs-file, not the qcow2 BDS.


Yes, bs-file is passed to bdrv_pwrite_sync here,
but aio_poll(aio_context) will poll all BDS's aio, not only that of bs-file, 
doesn't it?
Is it possible that there are pending aio which belong to this qcow2 BDS still 
exist?


qcow2 is generally not reentrant, this is secured by locking 
(BDRVQcowState.lock). As long as one request for a BDS is still running, 
it will not be interrupted.


Max


Thanks,
Zhang Haoyu

Max


|-- bdrv_pwrite
|--- bdrv_pwritev
| bdrv_prwv_co
|- aio_poll(aio_context) == this aio_context is qemu_aio_context
|-- aio_dispatch
|--- bdrv_co_io_em_complete
| qemu_coroutine_enter(co-coroutine, NULL); == coroutine entry is 
bdrv_co_do_rw
bdrv_co_do_rw will access l1_table to perform I/O operation.

Thanks,
Zhang Haoyu

But I find it rather ugly to convert the cached L1 table to big endian,
so I'd be fine with the patch you proposed.

Max





[Qemu-devel] vhost-user:add VHOST_USER_CLEAR_MEM_TABLE

2014-10-13 Thread Linhaifeng
I want to add this message for vhost-user backend's memory changed.Any 
suggestion?

 * VHOST_USER_CLEAR_MEM_TABLE
  Id: 15
  Equivalent ioctl: VHOST_USER_CLEAR_MEM_TABLE
  Master payload: u64
  Clear the memory regions on the slave when the memory of the forward 
freed.e.g.when unplug the vhost-user device or reload the virio-net driver.
  Bits (0-7) of the payload contain nothing.




Re: [Qemu-devel] [PATCH] virtio-pci: fix migration for pci bus master

2014-10-13 Thread Greg Kurz
On Mon, 13 Oct 2014 12:01:07 +0300
Michael S. Tsirkin m...@redhat.com wrote:

 On Mon, Oct 13, 2014 at 10:49:41AM +0200, Greg Kurz wrote:
  On Mon, 6 Oct 2014 20:25:04 +0300
  Michael S. Tsirkin m...@redhat.com wrote:
   [...]
   
   BTW I reverted that patch, and to fix migration, I'm thinking
   about applying the following patch on top of master.
   
  
  Michael,
  
  I could force the migration issue with a rhel65 guest thanks to the
  following patch, applied to hw/virtio/virtio-pci.c in QEMU v2.1.
  
  @@ -271,6 +272,16 @@ static void virtio_ioport_write(void *opaque, uint32_t 
  addr, uint32_t val)
   VirtIODevice *vdev = virtio_bus_get_device(proxy-bus);
   hwaddr pa;
   
  +if ((vdev-status == (VIRTIO_CONFIG_S_ACKNOWLEDGE | 
  VIRTIO_CONFIG_S_DRIVER))
  + (!(proxy-pci_dev.config[PCI_COMMAND]  PCI_COMMAND_MASTER))
  + getenv(MIG_BUG))
  +{
  +fprintf(stderr, \n\n\n\tMIGRATE !\n\n\n);
  +qmp_migrate(getenv(MIG_BUG), false, false, false, false, false,
  +false, NULL);
  +unsetenv(MIG_BUG);
  +}
  +
   switch (addr) {
   case VIRTIO_PCI_GUEST_FEATURES:
   /* Guest does not negotiate properly?  We have to assume nothing. 
  */
  
  
  Indeed, the destination QEMU master hangs because bus master isn't set.
  
   Would appreciate testing cross-version migration (2.1 to master)
   with this patch applied.
   
  
  I had first to to enable the property for pseries of course. I could then
  migrate QEMU v2.1 to QEMU master and back to QEMU v2.1, in the window
  where we have DRIVER enabled and MASTER disabled, without experiencing
  the hang.
  
  Your fix works as expected (just a remark below).
  
   
   
   diff --git a/hw/virtio/virtio-pci.h b/hw/virtio/virtio-pci.h
   index 1cea157..8873b6d 100644
   --- a/hw/virtio/virtio-pci.h
   +++ b/hw/virtio/virtio-pci.h
   @@ -53,6 +53,11 @@ typedef struct VirtioBusClass VirtioPCIBusClass;
#define VIRTIO_PCI_BUS_CLASS(klass) \
OBJECT_CLASS_CHECK(VirtioPCIBusClass, klass, TYPE_VIRTIO_PCI_BUS)
   
   +/* Need to activate work-arounds for buggy guests at vmstate load. */
   +#define VIRTIO_PCI_FLAG_BUS_MASTER_BUG_MIGRATION_BIT  0
   +#define VIRTIO_PCI_FLAG_BUS_MASTER_BUG_MIGRATION \
   +(1  VIRTIO_PCI_FLAG_BUS_MASTER_BUG_MIGRATION_BIT)
   +
/* Performance improves when virtqueue kick processing is decoupled from 
   the
 * vcpu thread using ioeventfd for some devices. */
#define VIRTIO_PCI_FLAG_USE_IOEVENTFD_BIT 1
   diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
   index bae023a..e07b6c4 100644
   --- a/include/hw/i386/pc.h
   +++ b/include/hw/i386/pc.h
   @@ -312,6 +312,11 @@ bool e820_get_entry(int, uint32_t, uint64_t *, 
   uint64_t *);
.driver   = intel-hda,\
.property = old_msi_addr,\
.value= on,\
   +},\
   +{\
   +.driver   = virtio-pci,\
   +.property = virtio-pci-bus-master-bug-migration,\
   +.value= on,\
}
   
#define PC_COMPAT_2_0 \
  
  FWIW, the issue does not occur with intel targets, at least not
  in my test case (booting rhel6 on a virtio-blk disk). I see bus
  master is set early (bios ?) and never unset...
  
  If you decide to apply for intel anyway, shouldn't the enablement be
  in a separate patch ?
  
  Will you resend or would you like me to do it, along with the pseries
  enablement part ? In this case, I would need your SoB for the present
  patch.
  
  Thanks.
  
  --
  Greg
 
 AFAIK pseries doesn't support cross-version migration, does it?
 

(Cc'ing Alexander and Alexey)

It is still at an early stage, but we have that:

commit 6026db4501f773caaa2895cde7f93022960c7169
Author: Alexey Kardashevskiy a...@ozlabs.ru
Date:   Wed Jun 25 14:08:45 2014 +1000

spapr: Define a 2.1 pseries machine

   diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c
   index a827cd4..a499a3c 100644
   --- a/hw/virtio/virtio-pci.c
   +++ b/hw/virtio/virtio-pci.c
   @@ -86,9 +86,6 @@
 * 12 is historical, and due to x86 page size. */
#define VIRTIO_PCI_QUEUE_ADDR_SHIFT12
   
   -/* Flags track per-device state like workarounds for quirks in older 
   guests. */
   -#define VIRTIO_PCI_FLAG_BUS_MASTER_BUG  (1  0)
   -
static void virtio_pci_bus_new(VirtioBusState *bus, size_t bus_size,
   VirtIOPCIProxy *dev);
   
   @@ -323,14 +320,6 @@ static void virtio_ioport_write(void *opaque, 
   uint32_t addr, uint32_t val)
 proxy-pci_dev.config[PCI_COMMAND] |
 PCI_COMMAND_MASTER, 1);
}
   -
   -/* Linux before 2.6.34 sets the device as OK without enabling
   -   the PCI device bus master bit. In this case we need to disable
   -   some safety checks. */
   -if ((val  VIRTIO_CONFIG_S_DRIVER_OK) 
   -

Re: [Qemu-devel] [PATCH RFC 08/11] virtio_blk: use virtio v1.0 endian

2014-10-13 Thread Cornelia Huck
On Mon, 13 Oct 2014 16:28:32 +1030
Rusty Russell ru...@rustcorp.com.au wrote:

 Cornelia Huck cornelia.h...@de.ibm.com writes:
  Note that we care only about the fields still in use for virtio v1.0.
 
  Reviewed-by: Thomas Huth th...@linux.vnet.ibm.com
  Reviewed-by: David Hildenbrand d...@linux.vnet.ibm.com
  Signed-off-by: Cornelia Huck cornelia.h...@de.ibm.com
 
 Hi Cornelia,
 
 These patches all look good; 

Cool, thanks.

 I'm a bit nervous about our testing
 missing some conversion, so we'll need qemu patches for PCI so we can
 test on other platforms too.

The devices I looked at (blk and net) are probably OK as ccw needs to
convert always. I agree that we want to test on other platforms as
well, but unfortunately I not familiar enough with other platforms to
feel confident enough to convert virtio-pci and test it :(




Re: [Qemu-devel] [PATCH RFC 03/11] virtio: support more feature bits

2014-10-13 Thread Cornelia Huck
On Mon, 13 Oct 2014 16:23:58 +1030
Rusty Russell ru...@rustcorp.com.au wrote:

 Cornelia Huck cornelia.h...@de.ibm.com writes:
  With virtio-1, we support more than 32 feature bits. Let's make
  vdev-guest_features depend on the number of supported feature bits,
  allowing us to grow the feature bits automatically.
 
 It's a judgement call, but I would say that simply using uint64_t
 will be sufficient for quite a while.

I prefer this as an array for two reasons:

- It matches what ccw does anyway (we read/write features in chunks of
  32 bit), so this makes the backend handling for this transport very
  straightforward.
- While I don't see us running out of 64 feature bits for a while, we'd
  have to touch every device and transport again when we do. I'd prefer
  to do this once, as we need to change code anyway.




Re: [Qemu-devel] [PATCH v5 0/5] add description field in ObjectProperty and PropertyInfo struct

2014-10-13 Thread Gonglei
On 2014/10/9 19:51, Gonglei wrote:

 Andreas, ping?
 


Ping..., again.

 Best regards,
 -Gonglei
 
 -Original Message-
 From: qemu-devel-bounces+arei.gonglei=hotmail@nongnu.org
 [mailto:qemu-devel-bounces+arei.gonglei=hotmail@nongnu.org] On
 Behalf Of Gonglei
 Sent: Wednesday, October 08, 2014 6:46 PM
 To: Paolo Bonzini
 Cc: Huangweidong (C); m...@redhat.com; Luonengjun; arm...@redhat.com;
 qemu-devel@nongnu.org; Huangpeng (Peter); lcapitul...@redhat.com;
 afaer...@suse.de
 Subject: Re: [Qemu-devel] [PATCH v5 0/5] add description field in
 ObjectProperty and PropertyInfo struct

 On 2014/10/8 6:22, Paolo Bonzini wrote:

 Il 07/10/2014 08:33, arei.gong...@huawei.com ha scritto:
 From: Gonglei arei.gong...@huawei.com

 v5 - v4:
  1. add some improvements by Michael's suggtion, Thanks. (Michael)
  2. add 'Reviewed-by' tag (Paolo, Michael, Eric)

 Andreas, this series depends on patches in qom-next so you'll have to
 take it.


 Yes, please. Thanks!

 Best regards,
 -Gonglei

 Thanks,

 Paolo

 v4 - v3:
  1. rebase on qom-next tree (Andreas)
  2. fix memory leak in PATCH 2, move object_property_set_description
 calling
 in object_property_add_alias() from PATCH 3 to PATCH 2. (Paolo)
  3. drop ?: in PATCH 2, call g_strdup() directly
  4. rework PATCH 4, change description as optional field,
 drop ?: conditional express (Eric)

 v3 - v2:
  1. add a new description field to DevicePropertyInfo, and format
 it in qdev_device_help() in PATCH 6 (Paolo)

 v2 - v1:
  1. rename fail label to out in PATCH 1 (Andreas)
  2. improve descriptions in PATCH 3 (Paolo, adding Signed-off-by Paolo in
 this patch)
  3. rework PATCH 5, set description at qdev_property_add_static(),
 then copy the description of target_obj.property. (Paolo)
  4. free description filed of ObjectProperty avoid memory leak in PATCH 4.

 This patch series based on qom-next tree:
  https://github.com/afaerber/qemu-cpu/commits/qom-next

 Add a description field in both ObjectProperty and PropertyInfo struct.
 The descriptions can serve as documentation in the code,
 and they can be used to provide better help. For example:

 Before this patch series:

 $./qemu-system-x86_64 -device virtio-blk-pci,?

 virtio-blk-pci.iothread=linkiothread
 virtio-blk-pci.x-data-plane=bool
 virtio-blk-pci.scsi=bool
 virtio-blk-pci.config-wce=bool
 virtio-blk-pci.serial=str
 virtio-blk-pci.secs=uint32
 virtio-blk-pci.heads=uint32
 virtio-blk-pci.cyls=uint32
 virtio-blk-pci.discard_granularity=uint32
 virtio-blk-pci.bootindex=int32
 virtio-blk-pci.opt_io_size=uint32
 virtio-blk-pci.min_io_size=uint16
 virtio-blk-pci.physical_block_size=uint16
 virtio-blk-pci.logical_block_size=uint16
 virtio-blk-pci.drive=str
 virtio-blk-pci.virtio-backend=childvirtio-blk-device
 virtio-blk-pci.command_serr_enable=on/off
 virtio-blk-pci.multifunction=on/off
 virtio-blk-pci.rombar=uint32
 virtio-blk-pci.romfile=str
 virtio-blk-pci.addr=pci-devfn
 virtio-blk-pci.event_idx=on/off
 virtio-blk-pci.indirect_desc=on/off
 virtio-blk-pci.vectors=uint32
 virtio-blk-pci.ioeventfd=on/off
 virtio-blk-pci.class=uint32

 After:

 $./qemu-system-x86_64 -device virtio-blk-pci,?

 virtio-blk-pci.iothread=linkiothread
 virtio-blk-pci.x-data-plane=bool (on/off)
 virtio-blk-pci.scsi=bool (on/off)
 virtio-blk-pci.config-wce=bool (on/off)
 virtio-blk-pci.serial=str
 virtio-blk-pci.secs=uint32
 virtio-blk-pci.heads=uint32
 virtio-blk-pci.cyls=uint32
 virtio-blk-pci.discard_granularity=uint32
 virtio-blk-pci.bootindex=int32
 virtio-blk-pci.opt_io_size=uint32
 virtio-blk-pci.min_io_size=uint16
 virtio-blk-pci.physical_block_size=uint16 (A power of two between 512 and
 32768)
 virtio-blk-pci.logical_block_size=uint16 (A power of two between 512 and
 32768)
 virtio-blk-pci.drive=str (ID of a drive to use as a backend)
 virtio-blk-pci.virtio-backend=childvirtio-blk-device
 virtio-blk-pci.command_serr_enable=bool (on/off)
 virtio-blk-pci.multifunction=bool (on/off)
 virtio-blk-pci.rombar=uint32
 virtio-blk-pci.romfile=str
 virtio-blk-pci.addr=int32 (Slot and optional function number, example: 06.0
 or 06)
 virtio-blk-pci.event_idx=bool (on/off)
 virtio-blk-pci.indirect_desc=bool (on/off)
 virtio-blk-pci.vectors=uint32
 virtio-blk-pci.ioeventfd=bool (on/off)
 virtio-blk-pci.class=uint32


 Gonglei (5):
   qdev: add description field in PropertyInfo struct
   qom: add description field in ObjectProperty struct
   qdev: set the object property's description to the qdev property's.
   qmp: print descriptions of object properties
   qdev: drop legacy_name from qdev properties

  hw/core/qdev-properties-system.c |  8 
  hw/core/qdev-properties.c| 14 --
  hw/core/qdev.c   |  5 +
  include/hw/qdev-core.h   |  2 +-
  include/qom/object.h | 14 ++
  qapi-schema.json |  4 +++-
  qdev-monitor.c   |  7 ++-
  qmp.c| 13 ++---
  qom/object.c  

[Qemu-devel] [Bug 921208] Re: win7/x64 installer hangs on startup with 0x0000005d.

2014-10-13 Thread Alessandro Pilotti
Hi guys, is there any update on this issue?

Tx

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

Title:
  win7/x64 installer hangs on startup with 0x005d.

Status in QEMU:
  Confirmed
Status in “qemu” package in Ubuntu:
  Triaged

Bug description:
  hi,

  during booting win7/x64 installer i'm observing a bsod with 0x005d
  ( msdn: unsupported_processor ).

  used command line: qemu-system-x86_64 -m 2048 -hda w7-system.img
  -cdrom win7_x64.iso -boot d

  adding '-machine accel=kvm' instead of default tcg accel helps to
  boot.

  
  installed software:

  qemu-1.0
  linux-3.2.1
  glibc-2.14.1
  gcc-4.6.2

  hw cpu:

  processor   : 0..7
  vendor_id   : GenuineIntel
  cpu family  : 6
  model   : 42
  model name  : Intel(R) Core(TM) i7-2630QM CPU @ 2.00GHz
  stepping: 7
  microcode   : 0x14
  cpu MHz : 1995.739
  cache size  : 6144 KB
  physical id : 0
  siblings: 8
  core id : 3
  cpu cores   : 4
  apicid  : 7
  initial apicid  : 7
  fpu : yes
  fpu_exception   : yes
  cpuid level : 13
  wp  : yes
  flags   : fpu vme de pse tsc msr pae mce cx8 apic sep mtrr pge mca 
cmov pat pse36 clflush dts acpi mmx fxsr sse sse2 ss ht tm pbe syscall nx 
rdtscp lm constant_tsc arch_perfmon pebs bts rep_good nopl xtopology 
nonstop_tsc aperfmperf pni pclmulqdq dtes64 monitor ds_cpl vmx est tm2 ssse3 
cx16 xtpr pdcm pcid sse4_1 sse4_2 x2apic popcnt tsc_deadline_timer xsave avx 
lahf_lm ida arat epb xsaveopt pln pts dts tpr_shadow vnmi flexpriority ept vpid
  bogomips: 3992.23
  clflush size: 64
  cache_alignment : 64
  address sizes   : 36 bits physical, 48 bits virtual

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



Re: [Qemu-devel] [PATCH] virtio-pci: fix migration for pci bus master

2014-10-13 Thread Michael S. Tsirkin
On Mon, Oct 13, 2014 at 10:49:41AM +0200, Greg Kurz wrote:
 On Mon, 6 Oct 2014 20:25:04 +0300
 Michael S. Tsirkin m...@redhat.com wrote:
  [...]
  
  BTW I reverted that patch, and to fix migration, I'm thinking
  about applying the following patch on top of master.
  
 
 Michael,
 
 I could force the migration issue with a rhel65 guest thanks to the
 following patch, applied to hw/virtio/virtio-pci.c in QEMU v2.1.
 
 @@ -271,6 +272,16 @@ static void virtio_ioport_write(void *opaque, uint32_t 
 addr, uint32_t val)
  VirtIODevice *vdev = virtio_bus_get_device(proxy-bus);
  hwaddr pa;
  
 +if ((vdev-status == (VIRTIO_CONFIG_S_ACKNOWLEDGE | 
 VIRTIO_CONFIG_S_DRIVER))
 + (!(proxy-pci_dev.config[PCI_COMMAND]  PCI_COMMAND_MASTER))
 + getenv(MIG_BUG))
 +{
 +fprintf(stderr, \n\n\n\tMIGRATE !\n\n\n);
 +qmp_migrate(getenv(MIG_BUG), false, false, false, false, false,
 +false, NULL);
 +unsetenv(MIG_BUG);
 +}
 +
  switch (addr) {
  case VIRTIO_PCI_GUEST_FEATURES:
  /* Guest does not negotiate properly?  We have to assume nothing. */
 
 
 Indeed, the destination QEMU master hangs because bus master isn't set.
 
  Would appreciate testing cross-version migration (2.1 to master)
  with this patch applied.
  
 
 I had first to to enable the property for pseries of course. I could then
 migrate QEMU v2.1 to QEMU master and back to QEMU v2.1, in the window
 where we have DRIVER enabled and MASTER disabled, without experiencing
 the hang.
 
 Your fix works as expected (just a remark below).
 
  
  
  diff --git a/hw/virtio/virtio-pci.h b/hw/virtio/virtio-pci.h
  index 1cea157..8873b6d 100644
  --- a/hw/virtio/virtio-pci.h
  +++ b/hw/virtio/virtio-pci.h
  @@ -53,6 +53,11 @@ typedef struct VirtioBusClass VirtioPCIBusClass;
   #define VIRTIO_PCI_BUS_CLASS(klass) \
   OBJECT_CLASS_CHECK(VirtioPCIBusClass, klass, TYPE_VIRTIO_PCI_BUS)
  
  +/* Need to activate work-arounds for buggy guests at vmstate load. */
  +#define VIRTIO_PCI_FLAG_BUS_MASTER_BUG_MIGRATION_BIT  0
  +#define VIRTIO_PCI_FLAG_BUS_MASTER_BUG_MIGRATION \
  +(1  VIRTIO_PCI_FLAG_BUS_MASTER_BUG_MIGRATION_BIT)
  +
   /* Performance improves when virtqueue kick processing is decoupled from 
  the
* vcpu thread using ioeventfd for some devices. */
   #define VIRTIO_PCI_FLAG_USE_IOEVENTFD_BIT 1
  diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
  index bae023a..e07b6c4 100644
  --- a/include/hw/i386/pc.h
  +++ b/include/hw/i386/pc.h
  @@ -312,6 +312,11 @@ bool e820_get_entry(int, uint32_t, uint64_t *, 
  uint64_t *);
   .driver   = intel-hda,\
   .property = old_msi_addr,\
   .value= on,\
  +},\
  +{\
  +.driver   = virtio-pci,\
  +.property = virtio-pci-bus-master-bug-migration,\
  +.value= on,\
   }
  
   #define PC_COMPAT_2_0 \
 
 FWIW, the issue does not occur with intel targets, at least not
 in my test case (booting rhel6 on a virtio-blk disk).

This will reproduce with rhel5 though.

 I see bus
 master is set early (bios ?) and never unset...

IIUC if you don't boot from device, it won't be set,
and will not be set with older guests.

 If you decide to apply for intel anyway, shouldn't the enablement be
 in a separate patch ?
 
 Will you resend or would you like me to do it, along with the pseries
 enablement part ? In this case, I would need your SoB for the present
 patch.
 
 Thanks.
 
 --
 Greg
 
  diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c
  index a827cd4..a499a3c 100644
  --- a/hw/virtio/virtio-pci.c
  +++ b/hw/virtio/virtio-pci.c
  @@ -86,9 +86,6 @@
* 12 is historical, and due to x86 page size. */
   #define VIRTIO_PCI_QUEUE_ADDR_SHIFT12
  
  -/* Flags track per-device state like workarounds for quirks in older 
  guests. */
  -#define VIRTIO_PCI_FLAG_BUS_MASTER_BUG  (1  0)
  -
   static void virtio_pci_bus_new(VirtioBusState *bus, size_t bus_size,
  VirtIOPCIProxy *dev);
  
  @@ -323,14 +320,6 @@ static void virtio_ioport_write(void *opaque, uint32_t 
  addr, uint32_t val)
proxy-pci_dev.config[PCI_COMMAND] |
PCI_COMMAND_MASTER, 1);
   }
  -
  -/* Linux before 2.6.34 sets the device as OK without enabling
  -   the PCI device bus master bit. In this case we need to disable
  -   some safety checks. */
  -if ((val  VIRTIO_CONFIG_S_DRIVER_OK) 
  -!(proxy-pci_dev.config[PCI_COMMAND]  PCI_COMMAND_MASTER)) {
  -proxy-flags |= VIRTIO_PCI_FLAG_BUS_MASTER_BUG;
  -}
   break;
   case VIRTIO_MSI_CONFIG_VECTOR:
   msix_vector_unuse(proxy-pci_dev, vdev-config_vector);
  @@ -483,8 +472,7 @@ static void virtio_write_config(PCIDevice *pci_dev, 
  uint32_t address,
   pci_default_write_config(pci_dev, address, val, len);
 

Re: [Qemu-devel] [PATCH v3 0/2] monitor: add peripheral device del completion support

2014-10-13 Thread Zhu Guihua
ping...

On Mon, 2014-10-06 at 19:38 +0800, Zhu Guihua wrote:
 After inputting device_del command in monitor, we expect to list all
 hotpluggable devices automatically by pressing tab key. This patchset provides
 the function to list all peripheral devices such as memory devices.
 
 v3:
 - commit message changes (Igor)
 - rename function in patch 1 (Igor)
 - use 'hotpluggable' property to discard non-hotpluggable devices (Igor)
 
 v2:
 - use object_child_foreach() to simplify the implementation (Andreas)
 
 
 Zhu Guihua (2):
   qdev: add qdev_build_hotpluggable_device_list helper
   monitor: add del completion for peripheral device
 
  hw/core/qdev.c | 14 ++
  include/hw/qdev-core.h |  2 ++
  monitor.c  | 24 
  3 files changed, 40 insertions(+)
 





Re: [Qemu-devel] [PATCH v3 3/3] util: Infrastructure for computing recent averages

2014-10-13 Thread Paolo Bonzini
Il 24/09/2014 17:21, Benoît Canet ha scritto:
 The module takes care of computing minimal and maximal
 values over the time slice duration.

The code looks good, just two comments:

 +/* Get the average value
 + *
 + * @ta:  the timed average structure used
 + * @ret: the average value
 + */
 +uint64_t timed_average_avg(TimedAverage *ta)
 +{
 +Window *w;
 +check_expirations(ta);
 +
 +w = current_window(ta);
 +
 +if (w-count) {
 +return w-sum / w-count;

First, do we want this to return double?

Second, this will return the min/max/avg in an unknown amount of time
between period/2 and period---on average period*3/4, e.g. 0.75 seconds
for a period equal to one second.

Would it make sense to tweak the TimedAverage period to be higher, e.g.
1.33 seconds/80 seconds/80 minutes, so that the _average_ period is 1
second/1 minute/1 hour?

This only applies to how the code is used, not to TimedAverage itself;
hence, feel free to post the patch with Reviewed-by once
timed_average_avg's return type is changed to a double.

Paolo

 +}
 +
 +return 0;
 +}
 +
 +/* Get the maximum value
 + *
 + * @ta:  the timed average structure used
 + * @ret: the maximal value
 + */
 +uint64_t timed_average_max(TimedAverage *ta)
 +{
 +check_expirations(ta);
 +return current_window(ta)-max;
 +}
 




Re: [Qemu-devel] [PATCH] virtio-pci: fix migration for pci bus master

2014-10-13 Thread Alexander Graf


On 13.10.14 12:42, Greg Kurz wrote:
 On Mon, 13 Oct 2014 12:01:07 +0300
 Michael S. Tsirkin m...@redhat.com wrote:
 
 On Mon, Oct 13, 2014 at 10:49:41AM +0200, Greg Kurz wrote:
 On Mon, 6 Oct 2014 20:25:04 +0300
 Michael S. Tsirkin m...@redhat.com wrote:
 [...]

 BTW I reverted that patch, and to fix migration, I'm thinking
 about applying the following patch on top of master.


 Michael,

 I could force the migration issue with a rhel65 guest thanks to the
 following patch, applied to hw/virtio/virtio-pci.c in QEMU v2.1.

 @@ -271,6 +272,16 @@ static void virtio_ioport_write(void *opaque, uint32_t 
 addr, uint32_t val)
  VirtIODevice *vdev = virtio_bus_get_device(proxy-bus);
  hwaddr pa;
  
 +if ((vdev-status == (VIRTIO_CONFIG_S_ACKNOWLEDGE | 
 VIRTIO_CONFIG_S_DRIVER))
 + (!(proxy-pci_dev.config[PCI_COMMAND]  PCI_COMMAND_MASTER))
 + getenv(MIG_BUG))
 +{
 +fprintf(stderr, \n\n\n\tMIGRATE !\n\n\n);
 +qmp_migrate(getenv(MIG_BUG), false, false, false, false, false,
 +false, NULL);
 +unsetenv(MIG_BUG);
 +}
 +
  switch (addr) {
  case VIRTIO_PCI_GUEST_FEATURES:
  /* Guest does not negotiate properly?  We have to assume nothing. 
 */


 Indeed, the destination QEMU master hangs because bus master isn't set.

 Would appreciate testing cross-version migration (2.1 to master)
 with this patch applied.


 I had first to to enable the property for pseries of course. I could then
 migrate QEMU v2.1 to QEMU master and back to QEMU v2.1, in the window
 where we have DRIVER enabled and MASTER disabled, without experiencing
 the hang.

 Your fix works as expected (just a remark below).



 diff --git a/hw/virtio/virtio-pci.h b/hw/virtio/virtio-pci.h
 index 1cea157..8873b6d 100644
 --- a/hw/virtio/virtio-pci.h
 +++ b/hw/virtio/virtio-pci.h
 @@ -53,6 +53,11 @@ typedef struct VirtioBusClass VirtioPCIBusClass;
  #define VIRTIO_PCI_BUS_CLASS(klass) \
  OBJECT_CLASS_CHECK(VirtioPCIBusClass, klass, TYPE_VIRTIO_PCI_BUS)

 +/* Need to activate work-arounds for buggy guests at vmstate load. */
 +#define VIRTIO_PCI_FLAG_BUS_MASTER_BUG_MIGRATION_BIT  0
 +#define VIRTIO_PCI_FLAG_BUS_MASTER_BUG_MIGRATION \
 +(1  VIRTIO_PCI_FLAG_BUS_MASTER_BUG_MIGRATION_BIT)
 +
  /* Performance improves when virtqueue kick processing is decoupled from 
 the
   * vcpu thread using ioeventfd for some devices. */
  #define VIRTIO_PCI_FLAG_USE_IOEVENTFD_BIT 1
 diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
 index bae023a..e07b6c4 100644
 --- a/include/hw/i386/pc.h
 +++ b/include/hw/i386/pc.h
 @@ -312,6 +312,11 @@ bool e820_get_entry(int, uint32_t, uint64_t *, 
 uint64_t *);
  .driver   = intel-hda,\
  .property = old_msi_addr,\
  .value= on,\
 +},\
 +{\
 +.driver   = virtio-pci,\
 +.property = virtio-pci-bus-master-bug-migration,\
 +.value= on,\
  }

  #define PC_COMPAT_2_0 \

 FWIW, the issue does not occur with intel targets, at least not
 in my test case (booting rhel6 on a virtio-blk disk). I see bus
 master is set early (bios ?) and never unset...

 If you decide to apply for intel anyway, shouldn't the enablement be
 in a separate patch ?

 Will you resend or would you like me to do it, along with the pseries
 enablement part ? In this case, I would need your SoB for the present
 patch.

 Thanks.

 --
 Greg

 AFAIK pseries doesn't support cross-version migration, does it?

We're trying to make sure we don't break cross-version migration for the
pseries machines. If nothing else, at least as a learning exercise.


Alex



Re: [Qemu-devel] [PATCH v6 01/32] target-arm: increase arrays of registers R13 R14

2014-10-13 Thread Peter Maydell
On 10 October 2014 18:03, Greg Bellows greg.bell...@linaro.org wrote:
 From: Fabian Aggeler aggel...@ethz.ch

 Increasing banked_r13 and banked_r14 to store LR_mon and SP_mon (bank
 index 7).

 Signed-off-by: Fabian Aggeler aggel...@ethz.ch
 Signed-off-by: Greg Bellows greg.bell...@linaro.org

Reviewed-by: Peter Maydell peter.mayd...@linaro.org

thanks
-- PMM



Re: [Qemu-devel] [PATCH] virtio-pci: fix migration for pci bus master

2014-10-13 Thread Michael S. Tsirkin
On Mon, Oct 13, 2014 at 02:29:20PM +0200, Alexander Graf wrote:
 
 
 On 13.10.14 12:42, Greg Kurz wrote:
  On Mon, 13 Oct 2014 12:01:07 +0300
  Michael S. Tsirkin m...@redhat.com wrote:
  
  On Mon, Oct 13, 2014 at 10:49:41AM +0200, Greg Kurz wrote:
  On Mon, 6 Oct 2014 20:25:04 +0300
  Michael S. Tsirkin m...@redhat.com wrote:
  [...]
 
  BTW I reverted that patch, and to fix migration, I'm thinking
  about applying the following patch on top of master.
 
 
  Michael,
 
  I could force the migration issue with a rhel65 guest thanks to the
  following patch, applied to hw/virtio/virtio-pci.c in QEMU v2.1.
 
  @@ -271,6 +272,16 @@ static void virtio_ioport_write(void *opaque, 
  uint32_t addr, uint32_t val)
   VirtIODevice *vdev = virtio_bus_get_device(proxy-bus);
   hwaddr pa;
   
  +if ((vdev-status == (VIRTIO_CONFIG_S_ACKNOWLEDGE | 
  VIRTIO_CONFIG_S_DRIVER))
  + (!(proxy-pci_dev.config[PCI_COMMAND]  PCI_COMMAND_MASTER))
  + getenv(MIG_BUG))
  +{
  +fprintf(stderr, \n\n\n\tMIGRATE !\n\n\n);
  +qmp_migrate(getenv(MIG_BUG), false, false, false, false, false,
  +false, NULL);
  +unsetenv(MIG_BUG);
  +}
  +
   switch (addr) {
   case VIRTIO_PCI_GUEST_FEATURES:
   /* Guest does not negotiate properly?  We have to assume 
  nothing. */
 
 
  Indeed, the destination QEMU master hangs because bus master isn't set.
 
  Would appreciate testing cross-version migration (2.1 to master)
  with this patch applied.
 
 
  I had first to to enable the property for pseries of course. I could then
  migrate QEMU v2.1 to QEMU master and back to QEMU v2.1, in the window
  where we have DRIVER enabled and MASTER disabled, without experiencing
  the hang.
 
  Your fix works as expected (just a remark below).
 
 
 
  diff --git a/hw/virtio/virtio-pci.h b/hw/virtio/virtio-pci.h
  index 1cea157..8873b6d 100644
  --- a/hw/virtio/virtio-pci.h
  +++ b/hw/virtio/virtio-pci.h
  @@ -53,6 +53,11 @@ typedef struct VirtioBusClass VirtioPCIBusClass;
   #define VIRTIO_PCI_BUS_CLASS(klass) \
   OBJECT_CLASS_CHECK(VirtioPCIBusClass, klass, 
  TYPE_VIRTIO_PCI_BUS)
 
  +/* Need to activate work-arounds for buggy guests at vmstate load. */
  +#define VIRTIO_PCI_FLAG_BUS_MASTER_BUG_MIGRATION_BIT  0
  +#define VIRTIO_PCI_FLAG_BUS_MASTER_BUG_MIGRATION \
  +(1  VIRTIO_PCI_FLAG_BUS_MASTER_BUG_MIGRATION_BIT)
  +
   /* Performance improves when virtqueue kick processing is decoupled 
  from the
* vcpu thread using ioeventfd for some devices. */
   #define VIRTIO_PCI_FLAG_USE_IOEVENTFD_BIT 1
  diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
  index bae023a..e07b6c4 100644
  --- a/include/hw/i386/pc.h
  +++ b/include/hw/i386/pc.h
  @@ -312,6 +312,11 @@ bool e820_get_entry(int, uint32_t, uint64_t *, 
  uint64_t *);
   .driver   = intel-hda,\
   .property = old_msi_addr,\
   .value= on,\
  +},\
  +{\
  +.driver   = virtio-pci,\
  +.property = virtio-pci-bus-master-bug-migration,\
  +.value= on,\
   }
 
   #define PC_COMPAT_2_0 \
 
  FWIW, the issue does not occur with intel targets, at least not
  in my test case (booting rhel6 on a virtio-blk disk). I see bus
  master is set early (bios ?) and never unset...
 
  If you decide to apply for intel anyway, shouldn't the enablement be
  in a separate patch ?
 
  Will you resend or would you like me to do it, along with the pseries
  enablement part ? In this case, I would need your SoB for the present
  patch.
 
  Thanks.
 
  --
  Greg
 
  AFAIK pseries doesn't support cross-version migration, does it?
 
 We're trying to make sure we don't break cross-version migration for the
 pseries machines. If nothing else, at least as a learning exercise.
 
 
 Alex

I see.
In that case, I think we need a common header, where
we can add device compatibility defines.
With PC and PSERIES would then pull it in.

Let's not duplicate code.

I'll try to come up with a patch.

-- 
MST



Re: [Qemu-devel] [PATCH v6 02/32] target-arm: add arm_is_secure() function

2014-10-13 Thread Peter Maydell
On 10 October 2014 18:03, Greg Bellows greg.bell...@linaro.org wrote:
 From: Fabian Aggeler aggel...@ethz.ch

 arm_is_secure() function allows to determine CPU security state
 if the CPU implements Security Extensions/EL3.
 arm_is_secure_below_el3() returns true if CPU is in secure state
 below EL3.

 Signed-off-by: Sergey Fedorov s.fedo...@samsung.com
 Signed-off-by: Fabian Aggeler aggel...@ethz.ch
 Signed-off-by: Greg Bellows greg.bell...@linaro.org

 ==

 v5 - v6
 - Broaden CONFIG_USER conditional
 - Merge resulting false returns with common comment
 - Globally change Aarch# to AArch#
 - Replace direct access of env-aarch64 with is_a64()
 ---
  target-arm/cpu.h | 42 ++
  1 file changed, 42 insertions(+)

 diff --git a/target-arm/cpu.h b/target-arm/cpu.h
 index 81fffd2..4f6db0f 100644
 --- a/target-arm/cpu.h
 +++ b/target-arm/cpu.h
 @@ -753,6 +753,48 @@ static inline int arm_feature(CPUARMState *env, int 
 feature)
  return (env-features  (1ULL  feature)) != 0;
  }

 +#if !defined(CONFIG_USER_ONLY)
 +/* Return true if exception level below EL3 is in secure state */

This is still missing the clarifying comment I was hoping for.

Make this:

/* Return true if exception levels below EL3 are in secure state,
 * or would be following an exception return to that level.
 * Unlike arm_is_secure() (which is alvays a question about the
 * _current_ state of the CPU) this doesn't care about the current
 * EL or mode.
 */

and then you can add my reviewed-by tag.

thanks
-- PMM



[Qemu-devel] [PATCH v2 2/2] Xen: Use the ioreq-server API when available

2014-10-13 Thread Paul Durrant
The ioreq-server API added to Xen 4.5 offers better security than
the existing Xen/QEMU interface because the shared pages that are
used to pass emulation request/results back and forth are removed
from the guest's memory space before any requests are serviced.
This prevents the guest from mapping these pages (they are in a
well known location) and attempting to attack QEMU by synthesizing
its own request structures. Hence, this patch modifies configure
to detect whether the API is available, and adds the necessary
code to use the API if it is.

Signed-off-by: Paul Durrant paul.durr...@citrix.com
Cc: Stefano Stabellini stefano.stabell...@eu.citrix.com
Cc: Peter Maydell peter.mayd...@linaro.org
Cc: Paolo Bonzini pbonz...@redhat.com
Cc: Michael Tokarev m...@tls.msk.ru
Cc: Stefan Hajnoczi stefa...@redhat.com
Cc: Stefan Weil s...@weilnetz.de
Cc: Olaf Hering o...@aepfle.de
Cc: Gerd Hoffmann kra...@redhat.com
Cc: Alexey Kardashevskiy a...@ozlabs.ru
Cc: Alexander Graf ag...@suse.de
---
 configure   |   29 ++
 include/hw/xen/xen_common.h |  222 +++
 trace-events|8 ++
 xen-hvm.c   |  163 +++
 4 files changed, 403 insertions(+), 19 deletions(-)

diff --git a/configure b/configure
index 9ac2600..c2db574 100755
--- a/configure
+++ b/configure
@@ -1876,6 +1876,32 @@ int main(void) {
   xc_gnttab_open(NULL, 0);
   xc_domain_add_to_physmap(0, 0, XENMAPSPACE_gmfn, 0, 0);
   xc_hvm_inject_msi(xc, 0, 0xf000, 0x);
+  xc_hvm_create_ioreq_server(xc, 0, 0, NULL);
+  return 0;
+}
+EOF
+  compile_prog  $xen_libs
+then
+xen_ctrl_version=450
+xen=yes
+
+  elif
+  cat  $TMPC EOF 
+#include xenctrl.h
+#include xenstore.h
+#include stdint.h
+#include xen/hvm/hvm_info_table.h
+#if !defined(HVM_MAX_VCPUS)
+# error HVM_MAX_VCPUS not defined
+#endif
+int main(void) {
+  xc_interface *xc;
+  xs_daemon_open();
+  xc = xc_interface_open(0, 0, 0);
+  xc_hvm_set_mem_type(0, 0, HVMMEM_ram_ro, 0, 0);
+  xc_gnttab_open(NULL, 0);
+  xc_domain_add_to_physmap(0, 0, XENMAPSPACE_gmfn, 0, 0);
+  xc_hvm_inject_msi(xc, 0, 0xf000, 0x);
   return 0;
 }
 EOF
@@ -4282,6 +4308,9 @@ if test -n $sparc_cpu; then
 echo Target Sparc Arch $sparc_cpu
 fi
 echo xen support   $xen
+if test $xen = yes ; then
+  echo xen ctrl version  $xen_ctrl_version
+fi
 echo brlapi support$brlapi
 echo bluez  support$bluez
 echo Documentation $docs
diff --git a/include/hw/xen/xen_common.h b/include/hw/xen/xen_common.h
index 07731b9..7040506 100644
--- a/include/hw/xen/xen_common.h
+++ b/include/hw/xen/xen_common.h
@@ -16,7 +16,9 @@
 
 #include hw/hw.h
 #include hw/xen/xen.h
+#include hw/pci/pci.h
 #include qemu/queue.h
+#include trace.h
 
 /*
  * We don't support Xen prior to 3.3.0.
@@ -164,4 +166,224 @@ void destroy_hvm_domain(bool reboot);
 /* shutdown/destroy current domain because of an error */
 void xen_shutdown_fatal_error(const char *fmt, ...) GCC_FMT_ATTR(1, 2);
 
+/* Xen before 4.5 */
+#if CONFIG_XEN_CTRL_INTERFACE_VERSION  450
+
+#ifndef HVM_PARAM_BUFIOREQ_EVTCHN
+#define HVM_PARAM_BUFIOREQ_EVTCHN 26
+#endif
+
+#define IOREQ_TYPE_PCI_CONFIG 2
+
+typedef uint32_t ioservid_t;
+
+static inline void xen_map_memory_section(XenXC xc, domid_t dom,
+  ioservid_t ioservid,
+  MemoryRegionSection *section)
+{
+}
+
+static inline void xen_unmap_memory_section(XenXC xc, domid_t dom,
+ioservid_t ioservid,
+MemoryRegionSection *section)
+{
+}
+
+static inline void xen_map_io_section(XenXC xc, domid_t dom,
+  ioservid_t ioservid,
+  MemoryRegionSection *section)
+{
+}
+
+static inline void xen_unmap_io_section(XenXC xc, domid_t dom,
+ioservid_t ioservid,
+MemoryRegionSection *section)
+{
+}
+
+static inline void xen_map_pcidev(XenXC xc, domid_t dom,
+  ioservid_t ioservid,
+  PCIDevice *pci_dev)
+{
+}
+
+static inline void xen_unmap_pcidev(XenXC xc, domid_t dom,
+ioservid_t ioservid,
+PCIDevice *pci_dev)
+{
+}
+
+static inline int xen_create_ioreq_server(XenXC xc, domid_t dom,
+  ioservid_t *ioservid)
+{
+return 0;
+}
+
+static inline void xen_destroy_ioreq_server(XenXC xc, domid_t dom,
+ioservid_t ioservid)
+{
+}
+
+static inline int xen_get_ioreq_server_info(XenXC xc, domid_t dom,
+ioservid_t ioservid,
+xen_pfn_t *ioreq_pfn,
+xen_pfn_t 

[Qemu-devel] [PATCH v2 0/2] Xen: Use ioreq-server API

2014-10-13 Thread Paul Durrant
This patch series is v2 of what was the single patch Xen: Use
the ioreq-server API when available. The code that adds the
PCI bus listener is now in patch #1 of this series and the
remainder of the changes, in patch #2, have been re-worked to
constrain the #ifdefing to xen_common.h, as requested by
Stefano.




[Qemu-devel] [PATCH v2 1/2] Add PCI bus listener interface

2014-10-13 Thread Paul Durrant
The Xen ioreq-server API, introduced in Xen 4.5, requires that PCI device
models explicitly register with Xen for config space accesses. This patch
adds a PCI bus listener interface which can be used by the Xen interface
code to monitor PCI buses for arrival and departure of devices.

Signed-off-by: Paul Durrant paul.durr...@citrix.com
Cc: Michael S. Tsirkin m...@redhat.com
Cc: Paolo Bonzini pbonz...@redhat.com
Cc: Andreas Faerber afaer...@suse.de
Cc: Thomas Huth th...@linux.vnet.ibm.com
Cc: Peter Crosthwaite peter.crosthwa...@xilinx.com
Cc: Christian Borntraeger borntrae...@de.ibm.com
---
 hw/pci/pci.c|   65 +++
 include/hw/pci/pci.h|9 +++
 include/qemu/typedefs.h |1 +
 3 files changed, 75 insertions(+)

diff --git a/hw/pci/pci.c b/hw/pci/pci.c
index 6ce75aa..53c955d 100644
--- a/hw/pci/pci.c
+++ b/hw/pci/pci.c
@@ -122,6 +122,66 @@ static uint16_t pci_default_sub_device_id = 
PCI_SUBDEVICE_ID_QEMU;
 
 static QLIST_HEAD(, PCIHostState) pci_host_bridges;
 
+static QTAILQ_HEAD(pci_listeners, PCIListener) pci_listeners
+= QTAILQ_HEAD_INITIALIZER(pci_listeners);
+
+enum ListenerDirection { Forward, Reverse };
+
+#define PCI_LISTENER_CALL(_callback, _direction, _args...)  \
+do {\
+PCIListener *_listener; \
+\
+switch (_direction) {   \
+case Forward:   \
+QTAILQ_FOREACH(_listener, pci_listeners, link) {   \
+if (_listener-_callback) { \
+_listener-_callback(_listener, ##_args);   \
+}   \
+}   \
+break;  \
+case Reverse:   \
+QTAILQ_FOREACH_REVERSE(_listener, pci_listeners,   \
+   pci_listeners, link) {   \
+if (_listener-_callback) { \
+_listener-_callback(_listener, ##_args);   \
+}   \
+}   \
+break;  \
+default:\
+abort();\
+}   \
+} while (0)
+
+static int pci_listener_add(DeviceState *dev, void *opaque)
+{
+if (object_dynamic_cast(OBJECT(dev), TYPE_PCI_DEVICE)) {
+PCIDevice *pci_dev = PCI_DEVICE(dev);
+
+PCI_LISTENER_CALL(device_add, Forward, pci_dev);
+}
+
+return 0;
+}
+
+void pci_listener_register(PCIListener *listener)
+{
+PCIHostState *host;
+
+QTAILQ_INSERT_TAIL(pci_listeners, listener, link);
+
+QLIST_FOREACH(host, pci_host_bridges, next) {
+PCIBus *bus = host-bus;
+
+qbus_walk_children(bus-qbus, NULL, NULL, pci_listener_add,
+   NULL, NULL);
+}
+}
+
+void pci_listener_unregister(PCIListener *listener)
+{
+QTAILQ_REMOVE(pci_listeners, listener, link);
+}
+
 static int pci_bar(PCIDevice *d, int reg)
 {
 uint8_t type;
@@ -795,6 +855,8 @@ static void pci_config_free(PCIDevice *pci_dev)
 
 static void do_pci_unregister_device(PCIDevice *pci_dev)
 {
+PCI_LISTENER_CALL(device_del, Reverse, pci_dev);
+
 pci_dev-bus-devices[pci_dev-devfn] = NULL;
 pci_config_free(pci_dev);
 
@@ -878,6 +940,9 @@ static PCIDevice *do_pci_register_device(PCIDevice 
*pci_dev, PCIBus *bus,
 pci_dev-config_write = config_write;
 bus-devices[devfn] = pci_dev;
 pci_dev-version_id = 2; /* Current pci device vmstate version */
+
+PCI_LISTENER_CALL(device_add, Forward, pci_dev);
+
 return pci_dev;
 }
 
diff --git a/include/hw/pci/pci.h b/include/hw/pci/pci.h
index c352c7b..6c21b37 100644
--- a/include/hw/pci/pci.h
+++ b/include/hw/pci/pci.h
@@ -303,6 +303,15 @@ struct PCIDevice {
 MSIVectorPollNotifier msix_vector_poll_notifier;
 };
 
+struct PCIListener {
+void (*device_add)(PCIListener *listener, PCIDevice *pci_dev);
+void (*device_del)(PCIListener *listener, PCIDevice *pci_dev);
+QTAILQ_ENTRY(PCIListener) link;
+};
+
+void pci_listener_register(PCIListener *listener);
+void pci_listener_unregister(PCIListener *listener);
+
 void pci_register_bar(PCIDevice *pci_dev, int region_num,
   uint8_t attr, MemoryRegion *memory);
 void pci_register_vga(PCIDevice *pci_dev, MemoryRegion *mem,
diff --git a/include/qemu/typedefs.h b/include/qemu/typedefs.h
index 04df51b..2b974c6 100644
--- a/include/qemu/typedefs.h
+++ 

Re: [Qemu-devel] [PATCH v6 03/32] target-arm: reject switching to monitor mode

2014-10-13 Thread Peter Maydell
On 10 October 2014 18:03, Greg Bellows greg.bell...@linaro.org wrote:
 From: Sergey Fedorov s.fedo...@samsung.com

 Reject switching to monitor mode from non-secure state.

 Signed-off-by: Sergey Fedorov s.fedo...@samsung.com
 Signed-off-by: Fabian Aggeler aggel...@ethz.ch
 Reviewed-by: Edgar E. Iglesias edgar.igles...@xilinx.com
 Signed-off-by: Greg Bellows greg.bell...@linaro.org
 ---
  target-arm/helper.c | 2 ++
  1 file changed, 2 insertions(+)

Reviewed-by: Peter Maydell peter.mayd...@linaro.org

thanks
-- PMM



Re: [Qemu-devel] [PATCH 1/1] pci-host: add educational driver

2014-10-13 Thread Paolo Bonzini
Il 10/10/2014 14:09, Jiri Slaby ha scritto:
 I am using qemu for teaching the Linux kernel at our university. I
 wrote a simple PCI device that can answer to writes/reads, generate
 interrupts and perform DMA. As I am dragging it locally over 2 years,
 I am sending it to you now.
 
 Signed-off-by: Jiri Slaby jsl...@suse.cz
 ---
  MAINTAINERS   |   5 +
  hw/pci-host/Makefile.objs |   1 +
  hw/pci-host/edu.c | 336 
 ++

hw/pci-host is for PCI host bridges.  You can put it in hw/misc.

  3 files changed, 342 insertions(+)
  create mode 100644 hw/pci-host/edu.c
 
 diff --git a/MAINTAINERS b/MAINTAINERS
 index 206bf7ea4582..7f4e8591b74b 100644
 --- a/MAINTAINERS
 +++ b/MAINTAINERS
 @@ -567,6 +567,11 @@ F: hw/xtensa/xtensa_lx60.c
  
  Devices
  ---
 +EDU
 +M: Jiri Slaby jsl...@suse.cz
 +S: Maintained
 +F: hw/pci-host/edu.c
 +
  IDE
  M: Kevin Wolf kw...@redhat.com
  M: Stefan Hajnoczi stefa...@redhat.com
 diff --git a/hw/pci-host/Makefile.objs b/hw/pci-host/Makefile.objs
 index bb65f9c4d2d0..b01f614ed248 100644
 --- a/hw/pci-host/Makefile.objs
 +++ b/hw/pci-host/Makefile.objs
 @@ -13,5 +13,6 @@ common-obj-$(CONFIG_VERSATILE_PCI) += versatile.o
  
  common-obj-$(CONFIG_PCI_APB) += apb.o
  common-obj-$(CONFIG_FULONG) += bonito.o
 +common-obj-$(CONFIG_PCI) += edu.o

Please add CONFIG_EDU=y to default-configs/pci.mak, and use CONFIG_EDU here.

  common-obj-$(CONFIG_PCI_PIIX) += piix.o
  common-obj-$(CONFIG_PCI_Q35) += q35.o
 diff --git a/hw/pci-host/edu.c b/hw/pci-host/edu.c
 new file mode 100644
 index ..72e09dff6f5d
 --- /dev/null
 +++ b/hw/pci-host/edu.c
 @@ -0,0 +1,336 @@
 +/*
 + * QEMU education PCI device
 + *
 + * Copyright (c) 2012 Jiri Slaby
 + *
 + * Permission is hereby granted, free of charge, to any person obtaining a
 + * copy of this software and associated documentation files (the Software),
 + * to deal in the Software without restriction, including without limitation
 + * the rights to use, copy, modify, merge, publish, distribute, sublicense,
 + * and/or sell copies of the Software, and to permit persons to whom the
 + * Software is furnished to do so, subject to the following conditions:
 + *
 + * The above copyright notice and this permission notice shall be included in
 + * all copies or substantial portions of the Software.
 + *
 + * THE SOFTWARE IS PROVIDED AS IS, WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
 + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
 + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL 
 THE
 + * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
 + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
 + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
 + * DEALINGS IN THE SOFTWARE.
 + */
 +
 +#include hw/pci/pci.h
 +#include qemu/timer.h
 +
 +#define DMA_START0x4
 +#define DMA_SIZE 4096
 +#define DMA_IRQ  0x0100
 +
 +typedef struct {
 +PCIDevice pdev;
 +MemoryRegion mmio;
 +
 +QemuThread thread;
 +QemuMutex thr_mutex;
 +QemuCond thr_cond;
 +bool stopping;
 +
 +uint32_t addr4;
 +uint32_t fact;
 +#define EDU_STATUS_COMPUTING 0x1
 +uint32_t status;
 +
 +uint32_t irq_status;
 +QemuMutex irq_mutex;
 +
 +#define EDU_DMA_RUN  0x1
 +#define EDU_DMA_DIR(cmd) (((cmd)  0x2)  1)
 +# define EDU_DMA_FROM_PCI0
 +# define EDU_DMA_TO_PCI  1
 +#define EDU_DMA_IRQ  0x4
 +struct dma_state {
 + uint32_t src;
 + uint32_t dst;
 + uint32_t cnt;
 + uint32_t cmd;
 +} dma;
 +QEMUTimer *dma_timer;
 +QemuMutex dma_mutex;
 +char dma_buf[DMA_SIZE];
 +} EduState;
 +
 +static bool within(uint32_t addr, uint32_t start, uint32_t end)
 +{
 + return start = addr  addr  end;
 +}
 +
 +static void check_range(uint32_t addr, uint32_t size1, uint32_t start,
 + uint32_t size2)
 +{
 + uint32_t end1 = addr + size1;
 + uint32_t end2 = start + size2;
 +
 + if (within(addr, start, end2) 
 + end1  addr  within(end1, start, end2))
 + return;
 +
 + hw_error(EDU: DMA range 0x%.8x-0x%.8x out of bounds (0x%.8x-0x%.8x)!,
 + addr, end1 - 1, start, end2 - 1);
 +}
 +
 +static void edu_dma_timer(void *opaque)
 +{
 + EduState *edu = opaque;
 + bool raise_irq = false;
 +
 + qemu_mutex_lock(edu-dma_mutex);

dma_mutex and mutex and irq_mutex are not necessary.  All I/O happens
under the big QEMU lock (qemu_lock/unlock_iothread).  I can certainly
imagine that edu.c would be one of the first devices we make
thread-safe, but... not yet. :)

 + if (!(edu-dma.cmd  EDU_DMA_RUN))
 + goto end;
 +
 + if (EDU_DMA_DIR(edu-dma.cmd) == EDU_DMA_FROM_PCI) {
 + uint32_t dst = edu-dma.dst;
 + check_range(dst, edu-dma.cnt, DMA_START, DMA_SIZE);
 + dst -= 

Re: [Qemu-devel] [PATCH v6 04/32] target-arm: rename arm_current_pl to arm_current_el

2014-10-13 Thread Peter Maydell
On 10 October 2014 18:03, Greg Bellows greg.bell...@linaro.org wrote:
 Renamed the arm_current_pl CPU function to more accurately represent that it
 returns the ARMv8 EL rather than ARMv7 PL.

 Signed-off-by: Greg Bellows greg.bell...@linaro.org

 ==

 v5 - v6
 - Renamed DisasContext current_pl field to current_el
 - Added comment to arm_current_el on handling v7 PL
 - Fixed comments referencing PL
 ---
  target-arm/cpu.h   | 27 +++
  target-arm/helper-a64.c|  6 +++---
  target-arm/helper.c| 22 +++---
  target-arm/internals.h |  2 +-
  target-arm/op_helper.c | 16 
  target-arm/translate-a64.c | 16 
  target-arm/translate.c |  4 ++--
  target-arm/translate.h |  4 ++--
  8 files changed, 50 insertions(+), 47 deletions(-)

 diff --git a/target-arm/cpu.h b/target-arm/cpu.h
 index 4f6db0f..149f258 100644
 --- a/target-arm/cpu.h
 +++ b/target-arm/cpu.h
 @@ -986,7 +986,10 @@ static inline bool cptype_valid(int cptype)
  #define PL1_RW (PL1_R | PL1_W)
  #define PL0_RW (PL0_R | PL0_W)

 -static inline int arm_current_pl(CPUARMState *env)
 +/* Return the current Exception Level (as per ARMv8 note that this differs

Needs a semicolon between ARMv8 and note.

 + * from the ARMv7 Privilege Level).
 + */
 +static inline int arm_current_el(CPUARMState *env)

Otherwise
Reviewed-by: Peter Maydell peter.mayd...@linaro.org

thanks
-- PMM



Re: [Qemu-devel] [PATCH 1/1] pci-host: add educational driver

2014-10-13 Thread Paolo Bonzini
Il 13/10/2014 10:32, Jiri Slaby ha scritto:
 No, the DMA addresses are on purpose 32-bit: to teach the people always
 set the dma mask properly in the driver. This driver copies COMBO6x
 devices (liberouter.org) behaviour which I used until the cards got
 obsoleted (hard to find PCI-X slots nowadays).
 
 I can make this configurable if you wish.

Yeah, that would help (to avoid setting a bad example).  You could have
extra commands to switch between 32- and 64-bit DMA masks.

Paolo



Re: [Qemu-devel] [PATCH v6 06/32] target-arm: A32: Emulate the SMC instruction

2014-10-13 Thread Peter Maydell
On 10 October 2014 18:03, Greg Bellows greg.bell...@linaro.org wrote:
 From: Fabian Aggeler aggel...@ethz.ch

 Implements SMC instruction in AArch32 using the A32 syndrome. When executing
 SMC instruction from monitor CPU mode SCR.NS bit is reset.

 Signed-off-by: Sergey Fedorov s.fedo...@samsung.com
 Signed-off-by: Fabian Aggeler aggel...@ethz.ch
 Signed-off-by: Greg Bellows greg.bell...@linaro.org

 ==

 v5 - v6
 - Fixed PC offsetting for presmc
 - Removed extraneous semi-colon
 - Fixed merge issue

 v4 - v5
 - Merge pre_smc upstream changes and incorporated ss_advance
 ---
  target-arm/helper.c| 11 +++
  target-arm/internals.h |  5 +
  target-arm/op_helper.c |  3 +--
  target-arm/translate.c | 39 +--
  4 files changed, 46 insertions(+), 12 deletions(-)

 diff --git a/target-arm/helper.c b/target-arm/helper.c
 index 2381e6f..7f3f049 100644
 --- a/target-arm/helper.c
 +++ b/target-arm/helper.c
 @@ -4090,6 +4090,12 @@ void arm_cpu_do_interrupt(CPUState *cs)
  mask = CPSR_A | CPSR_I | CPSR_F;
  offset = 4;
  break;
 +case EXCP_SMC:
 +new_mode = ARM_CPU_MODE_MON;
 +addr = 0x08;
 +mask = CPSR_A | CPSR_I | CPSR_F;
 +offset = 0;
 +break;
  default:
  cpu_abort(cs, Unhandled exception 0x%x\n, cs-exception_index);
  return; /* Never happens.  Keep compiler happy.  */
 @@ -4108,6 +4114,11 @@ void arm_cpu_do_interrupt(CPUState *cs)
   */
  addr += env-cp15.vbar_el[1];
  }
 +
 +if ((env-uncached_cpsr  CPSR_M) == ARM_CPU_MODE_MON) {
 +env-cp15.scr_el3 = ~SCR_NS;
 +}
 +
  switch_mode (env, new_mode);
  /* For exceptions taken to AArch32 we must clear the SS bit in both
   * PSTATE and in the old-state value we save to SPSR_mode, so zero it 
 now.
 diff --git a/target-arm/internals.h b/target-arm/internals.h
 index fd69a83..544fb42 100644
 --- a/target-arm/internals.h
 +++ b/target-arm/internals.h
 @@ -236,6 +236,11 @@ static inline uint32_t syn_aa32_svc(uint32_t imm16, bool 
 is_thumb)
  | (is_thumb ? 0 : ARM_EL_IL);
  }

 +static inline uint32_t syn_aa32_smc(void)
 +{
 +return (EC_AA32_SMC  ARM_EL_EC_SHIFT) | ARM_EL_IL;
 +}
 +
  static inline uint32_t syn_aa64_bkpt(uint32_t imm16)
  {
  return (EC_AA64_BKPT  ARM_EL_EC_SHIFT) | ARM_EL_IL | (imm16  0x);
 diff --git a/target-arm/op_helper.c b/target-arm/op_helper.c
 index 0809d63..9e38f26 100644
 --- a/target-arm/op_helper.c
 +++ b/target-arm/op_helper.c
 @@ -419,8 +419,7 @@ void HELPER(pre_hvc)(CPUARMState *env)
  void HELPER(pre_smc)(CPUARMState *env, uint32_t syndrome)
  {
  int cur_el = arm_current_el(env);
 -/* FIXME: Use real secure state.  */
 -bool secure = false;
 +bool secure = arm_is_secure(env);
  bool smd = env-cp15.scr_el3  SCR_SMD;
  /* On ARMv8 AArch32, SMD only applies to NS state.
   * On ARMv7 SMD only applies to NS state and only if EL2 is available.
 diff --git a/target-arm/translate.c b/target-arm/translate.c
 index 617e6a9..60655e1 100644
 --- a/target-arm/translate.c
 +++ b/target-arm/translate.c
 @@ -7872,15 +7872,27 @@ static void disas_arm_insn(CPUARMState * env, 
 DisasContext *s)
  case 7:
  {
  int imm16 = extract32(insn, 0, 4) | (extract32(insn, 8, 12)  
 4);
 -/* SMC instruction (op1 == 3)
 -   and undefined instructions (op1 == 0 || op1 == 2)
 -   will trap */
 -if (op1 != 1) {
 +if (op1 == 1) {
 +/* bkpt */
 +ARCH(5);
 +gen_exception_insn(s, 4, EXCP_BKPT,
 +syn_aa32_bkpt(imm16, false));
 +} else if (op1 == 3) {
 +/* smi/smc */
 +if (!arm_dc_feature(s, ARM_FEATURE_EL3) ||
 +s-current_el == 0) {
 +goto illegal_op;
 +}
 +gen_set_pc_im(s, s-pc - 4);
 +tmp = tcg_const_i32(syn_aa32_smc());
 +gen_helper_pre_smc(cpu_env, tmp);
 +tcg_temp_free_i32(tmp);
 +gen_ss_advance(s);
 +gen_exception_insn(s, 0, EXCP_SMC, syn_aa32_smc());
 +break;

This is wrong; you should be basing this series on top of my PSCI
series which will get you a correct implementation of the translate.c
changes for SMC for A32/T32. (You'll still need the do_interrupt
changes.)

thanks
-- PMM



Re: [Qemu-devel] [PATCH v6 05/32] target-arm: make arm_current_el() return EL3

2014-10-13 Thread Peter Maydell
On 10 October 2014 18:03, Greg Bellows greg.bell...@linaro.org wrote:
 From: Fabian Aggeler aggel...@ethz.ch

 Make arm_current_el() return EL3 for secure PL1 and monitor mode.
 Increase MMU modes since mmu_index is directly infered from arm_

inferred

 current_el(). Changes assertion in arm_el_is_aa64() to allow EL3.

Change


 Signed-off-by: Fabian Aggeler aggel...@ethz.ch
 Signed-off-by: Greg Bellows greg.bell...@linaro.org

 ==

 v5 - v6
 - Rework arm_current_el() logic to properly return EL3 for secure PL1 when EL3
   is 32-bit.
 - Replace direct access of env-aarch64 with is_a64()
 ---
  target-arm/cpu.h | 29 -
  1 file changed, 20 insertions(+), 9 deletions(-)

 diff --git a/target-arm/cpu.h b/target-arm/cpu.h
 index 149f258..ed32b97 100644
 --- a/target-arm/cpu.h
 +++ b/target-arm/cpu.h
 @@ -100,7 +100,7 @@ typedef uint32_t ARMReadCPFunc(void *opaque, int cp_info,

  struct arm_boot_info;

 -#define NB_MMU_MODES 2
 +#define NB_MMU_MODES 4

  /* We currently assume float and double are IEEE single and double
 precision respectively.
 @@ -798,11 +798,12 @@ static inline bool arm_is_secure(CPUARMState *env)
  /* Return true if the specified exception level is running in AArch64 state. 
 */
  static inline bool arm_el_is_aa64(CPUARMState *env, int el)
  {
 -/* We don't currently support EL2 or EL3, and this isn't valid for EL0
 +/* We don't currently support EL2, and this isn't valid for EL0
   * (if we're in EL0, is_a64() is what you want, and if we're not in EL0
   * then the state of EL0 isn't well defined.)
   */
 -assert(el == 1);
 +assert(el == 1 || el == 3);
 +
  /* AArch64-capable CPUs always run with EL1 in AArch64 mode. This
   * is a QEMU-imposed simplification which we may wish to change later.
   * If we in future support EL2 and/or EL3, then the state of lower
 @@ -991,17 +992,27 @@ static inline bool cptype_valid(int cptype)
   */
  static inline int arm_current_el(CPUARMState *env)
  {
 -if (env-aarch64) {
 +if (is_a64(env)) {
  return extract32(env-pstate, 2, 2);
  }

 -if ((env-uncached_cpsr  0x1f) == ARM_CPU_MODE_USR) {
 +switch (env-uncached_cpsr  0x1f) {

Use CPSR_M, not a raw 0x1f, please.

 +case ARM_CPU_MODE_USR:
  return 0;
 +case ARM_CPU_MODE_HYP:
 +return 2;
 +case ARM_CPU_MODE_MON:
 +return 3;
 +default:
 +if (arm_is_secure(env)  !arm_el_is_aa64(env, 3)) {
 +/* If EL3 is 32-bit then all secure privileged modes run in
 + * EL3
 + */
 +return 3;
 +}
 +
 +return 1;

Otherwise
Reviewed-by: Peter Maydell peter.mayd...@linaro.org

thanks
-- PMM



Re: [Qemu-devel] [PATCH v6 06/32] target-arm: A32: Emulate the SMC instruction

2014-10-13 Thread Greg Bellows
I realize that, but I don't believe your changes were available yet and
still sounded to be a bit in flux, so I was waiting to merge.

As I mentioned previously, I had already merged on top of your initial
changes.

I'll recommit with your changes.

Greg

On 13 October 2014 08:06, Peter Maydell peter.mayd...@linaro.org wrote:

 On 10 October 2014 18:03, Greg Bellows greg.bell...@linaro.org wrote:
  From: Fabian Aggeler aggel...@ethz.ch
 
  Implements SMC instruction in AArch32 using the A32 syndrome. When
 executing
  SMC instruction from monitor CPU mode SCR.NS bit is reset.
 
  Signed-off-by: Sergey Fedorov s.fedo...@samsung.com
  Signed-off-by: Fabian Aggeler aggel...@ethz.ch
  Signed-off-by: Greg Bellows greg.bell...@linaro.org
 
  ==
 
  v5 - v6
  - Fixed PC offsetting for presmc
  - Removed extraneous semi-colon
  - Fixed merge issue
 
  v4 - v5
  - Merge pre_smc upstream changes and incorporated ss_advance
  ---
   target-arm/helper.c| 11 +++
   target-arm/internals.h |  5 +
   target-arm/op_helper.c |  3 +--
   target-arm/translate.c | 39 +--
   4 files changed, 46 insertions(+), 12 deletions(-)
 
  diff --git a/target-arm/helper.c b/target-arm/helper.c
  index 2381e6f..7f3f049 100644
  --- a/target-arm/helper.c
  +++ b/target-arm/helper.c
  @@ -4090,6 +4090,12 @@ void arm_cpu_do_interrupt(CPUState *cs)
   mask = CPSR_A | CPSR_I | CPSR_F;
   offset = 4;
   break;
  +case EXCP_SMC:
  +new_mode = ARM_CPU_MODE_MON;
  +addr = 0x08;
  +mask = CPSR_A | CPSR_I | CPSR_F;
  +offset = 0;
  +break;
   default:
   cpu_abort(cs, Unhandled exception 0x%x\n,
 cs-exception_index);
   return; /* Never happens.  Keep compiler happy.  */
  @@ -4108,6 +4114,11 @@ void arm_cpu_do_interrupt(CPUState *cs)
*/
   addr += env-cp15.vbar_el[1];
   }
  +
  +if ((env-uncached_cpsr  CPSR_M) == ARM_CPU_MODE_MON) {
  +env-cp15.scr_el3 = ~SCR_NS;
  +}
  +
   switch_mode (env, new_mode);
   /* For exceptions taken to AArch32 we must clear the SS bit in both
* PSTATE and in the old-state value we save to SPSR_mode, so
 zero it now.
  diff --git a/target-arm/internals.h b/target-arm/internals.h
  index fd69a83..544fb42 100644
  --- a/target-arm/internals.h
  +++ b/target-arm/internals.h
  @@ -236,6 +236,11 @@ static inline uint32_t syn_aa32_svc(uint32_t imm16,
 bool is_thumb)
   | (is_thumb ? 0 : ARM_EL_IL);
   }
 
  +static inline uint32_t syn_aa32_smc(void)
  +{
  +return (EC_AA32_SMC  ARM_EL_EC_SHIFT) | ARM_EL_IL;
  +}
  +
   static inline uint32_t syn_aa64_bkpt(uint32_t imm16)
   {
   return (EC_AA64_BKPT  ARM_EL_EC_SHIFT) | ARM_EL_IL | (imm16 
 0x);
  diff --git a/target-arm/op_helper.c b/target-arm/op_helper.c
  index 0809d63..9e38f26 100644
  --- a/target-arm/op_helper.c
  +++ b/target-arm/op_helper.c
  @@ -419,8 +419,7 @@ void HELPER(pre_hvc)(CPUARMState *env)
   void HELPER(pre_smc)(CPUARMState *env, uint32_t syndrome)
   {
   int cur_el = arm_current_el(env);
  -/* FIXME: Use real secure state.  */
  -bool secure = false;
  +bool secure = arm_is_secure(env);
   bool smd = env-cp15.scr_el3  SCR_SMD;
   /* On ARMv8 AArch32, SMD only applies to NS state.
* On ARMv7 SMD only applies to NS state and only if EL2 is
 available.
  diff --git a/target-arm/translate.c b/target-arm/translate.c
  index 617e6a9..60655e1 100644
  --- a/target-arm/translate.c
  +++ b/target-arm/translate.c
  @@ -7872,15 +7872,27 @@ static void disas_arm_insn(CPUARMState * env,
 DisasContext *s)
   case 7:
   {
   int imm16 = extract32(insn, 0, 4) | (extract32(insn, 8, 12)
  4);
  -/* SMC instruction (op1 == 3)
  -   and undefined instructions (op1 == 0 || op1 == 2)
  -   will trap */
  -if (op1 != 1) {
  +if (op1 == 1) {
  +/* bkpt */
  +ARCH(5);
  +gen_exception_insn(s, 4, EXCP_BKPT,
  +syn_aa32_bkpt(imm16, false));
  +} else if (op1 == 3) {
  +/* smi/smc */
  +if (!arm_dc_feature(s, ARM_FEATURE_EL3) ||
  +s-current_el == 0) {
  +goto illegal_op;
  +}
  +gen_set_pc_im(s, s-pc - 4);
  +tmp = tcg_const_i32(syn_aa32_smc());
  +gen_helper_pre_smc(cpu_env, tmp);
  +tcg_temp_free_i32(tmp);
  +gen_ss_advance(s);
  +gen_exception_insn(s, 0, EXCP_SMC, syn_aa32_smc());
  +break;

 This is wrong; you should be basing this series on top of my PSCI
 series which will get you a correct implementation of the translate.c
 changes for SMC for A32/T32. (You'll still need the do_interrupt
 changes.)

 thanks
 -- PMM



Re: [Qemu-devel] [PATCH v6 06/32] target-arm: A32: Emulate the SMC instruction

2014-10-13 Thread Peter Maydell
On 13 October 2014 15:13, Greg Bellows greg.bell...@linaro.org wrote:
 I realize that, but I don't believe your changes were available yet and
 still sounded to be a bit in flux, so I was waiting to merge.

 As I mentioned previously, I had already merged on top of your initial
 changes.

Ah. I thought when you said you'd merged your series on top of mine
that you'd merged it on top of the relevant patches...


 I'll recommit with your changes.

Thanks.

I'm going to finish reviewing the rest of this series, since
the later patches are basically independent of this. I may
be a day or two though as the next few require deeper thought
than the minor nits in the first half dozen patches. So you
don't need to push out a v7 just yet.

-- PMM



Re: [Qemu-devel] [PATCH v4 14/21] target-mips: add AUI, LSA and PCREL instruction families

2014-10-13 Thread Yongbok Kim

I have just few minor comments.

Reviewed-by: Yongbok Kim yongbok@imgtec.com

regards,
Yongbok

On 08/10/2014 11:55, Leon Alrae wrote:

Signed-off-by: Leon Alrae leon.al...@imgtec.com
---
v3:
* use sextract32 instead of open coding the bit field extraction
* replace _i64 with _tl in DAHI, DATI and DAUI
* fix misleading LDPC comment
---
  disas/mips.c|  42 ++-
  target-mips/translate.c | 197 +---
  2 files changed, 225 insertions(+), 14 deletions(-)

diff --git a/disas/mips.c b/disas/mips.c
index 8234369..091f4e2 100644
--- a/disas/mips.c
+++ b/disas/mips.c
@@ -407,6 +407,12 @@ struct mips_opcode
 +3 UDI immediate bits 6-20
 +4 UDI immediate bits 6-25
  
+   R6 immediates/displacements :

+   (adding suffix to 'o' to avoid adding new characters)
+   +o  9 bits immediate/displacement (shift = 7)
+   +o1 18 bits immediate/displacement (shift = 0)
+   +o2 19 bits immediate/displacement (shift = 0)
+
 Other:
 () parens surrounding optional value
 ,  separates operands
@@ -1217,6 +1223,17 @@ const struct mips_opcode mips_builtin_opcodes[] =
 them first.  The assemblers uses a hash table based on the
 instruction name anyhow.  */
  /* name,args, match,  mask,   pinfo,  
membership */
+{lwpc,s,+o2,0xec08, 0xfc18, WR_d, 0, 
I32R6},
+{lwupc,   s,+o2,0xec10, 0xfc18, WR_d, 0, 
I32R6},


I64R6


+{ldpc,s,+o1,0xec18, 0xfc1c, WR_d, 0, 
I64R6},
+{addiupc, s,+o2,0xec00, 0xfc18, WR_d, 0, 
I32R6},
+{auipc,   s,u,  0xec1e, 0xfc1f, WR_d, 0, 
I32R6},
+{aluipc,  s,u,  0xec1f, 0xfc1f, WR_d, 0, 
I32R6},
+{daui,s,t,u,0x7400, 0xfc00, RD_s|WR_t,0, 
I64R6},
+{dahi,s,u,  0x0406, 0xfc1f, RD_s, 0, 
I64R6},
+{dati,s,u,  0x041e, 0xfc1f, RD_s, 0, 
I64R6},
+{lsa, d,s,t,0x0005, 0xfc00073f, WR_d|RD_s|RD_t,   0, 
I32R6},
+{dlsa,d,s,t,0x0015, 0xfc00073f, WR_d|RD_s|RD_t,   0, 
I64R6},
  {clz, U,s,  0x0050, 0xfc1f07ff, WR_d|RD_s,0, 
I32R6},
  {clo, U,s,  0x0051, 0xfc1f07ff, WR_d|RD_s,0, 
I32R6},
  {dclz,U,s,  0x0052, 0xfc1f07ff, WR_d|RD_s,0, 
I64R6},
@@ -1822,6 +1839,7 @@ const struct mips_opcode mips_builtin_opcodes[] =
  {lld, t,o(b), 0xd000, 0xfc00, LDD|RD_b|WR_t,  0,  
I3  },
  {lld, t,A(b), 0,(int) M_LLD_AB,   INSN_MACRO, 0,  
I3  },
  {lui, t,u,0x3c00, 0xffe0, WR_t,   0,  
I1  },
+{aui, s,t,u,0x3c00, 0xfc00, RD_s|WR_t,0, 
I32R6},
  {luxc1,   D,t(b), 0x4c05, 0xfc00f83f, LDD|WR_D|RD_t|RD_b|FP_D, 0, 
I5|I33|N55},
  {lw,  t,o(b), 0x8c00, 0xfc00, LDD|RD_b|WR_t,  0,  
I1  },
  {lw,  t,A(b), 0,(int) M_LW_AB,INSN_MACRO, 0,  
I1  },
@@ -3645,10 +3663,28 @@ print_insn_args (const char *d,
  break;
  
  case 'o':

-delta = (l  OP_SH_DELTA_R6)  OP_MASK_DELTA_R6;
-if (delta  0x8000) {
-delta |= ~0x;
+switch (*(d+1)) {
+case '1':
+d++;
+delta = l  ((1  18) - 1);
+if (delta  0x2) {
+delta |= ~0x1;
+}
+break;
+case '2':
+d++;
+delta = l  ((1  19) - 1);
+if (delta  0x4) {
+delta |= ~0x3;
+}
+break;
+default:
+delta = (l  OP_SH_DELTA_R6)  OP_MASK_DELTA_R6;
+if (delta  0x8000) {
+delta |= ~0x;
+}
  }
+
  (*info-fprintf_func) (info-stream, %d, delta);
  break;
  
diff --git a/target-mips/translate.c b/target-mips/translate.c

index 06ececb..6f64c47 100644
--- a/target-mips/translate.c
+++ b/target-mips/translate.c
@@ -75,6 +75,7 @@ enum {
  OPC_BGTZ = (0x07  26),
  OPC_BGTZL= (0x17  26),
  OPC_JALX = (0x1D  26),  /* MIPS 16 only */
+OPC_DAUI = (0x1D  26),
  OPC_JALXS= OPC_JALX | 0x5,
  /* Load and stores */
  OPC_LDL  = (0x1A  26),
@@ -141,8 +142,25 @@ enum {
  /* Cache and prefetch */
  OPC_CACHE= (0x2F  26),
  OPC_PREF = (0x33  26),
-/* Reserved major opcode */
-OPC_MAJOR3B_RESERVED = (0x3B  26),
+/* PC-relative address computation / loads */
+OPC_PCREL= (0x3B  26),
+};
+
+/* 

Re: [Qemu-devel] [PATCH v5 0/5] add description field in ObjectProperty and PropertyInfo struct

2014-10-13 Thread Andreas Färber
Am 13.10.2014 um 12:55 schrieb Gonglei:
 On 2014/10/9 19:51, Gonglei wrote:
 
 Andreas, ping?

 
 
 Ping..., again.

Already queued, not yet pushed. On my way to KVM Forum...

Regards,
Andreas

 
 Best regards,
 -Gonglei

 -Original Message-
 From: qemu-devel-bounces+arei.gonglei=hotmail@nongnu.org
 [mailto:qemu-devel-bounces+arei.gonglei=hotmail@nongnu.org] On
 Behalf Of Gonglei
 Sent: Wednesday, October 08, 2014 6:46 PM
 To: Paolo Bonzini
 Cc: Huangweidong (C); m...@redhat.com; Luonengjun; arm...@redhat.com;
 qemu-devel@nongnu.org; Huangpeng (Peter); lcapitul...@redhat.com;
 afaer...@suse.de
 Subject: Re: [Qemu-devel] [PATCH v5 0/5] add description field in
 ObjectProperty and PropertyInfo struct

 On 2014/10/8 6:22, Paolo Bonzini wrote:

 Il 07/10/2014 08:33, arei.gong...@huawei.com ha scritto:
 From: Gonglei arei.gong...@huawei.com

 v5 - v4:
  1. add some improvements by Michael's suggtion, Thanks. (Michael)
  2. add 'Reviewed-by' tag (Paolo, Michael, Eric)

 Andreas, this series depends on patches in qom-next so you'll have to
 take it.


 Yes, please. Thanks!

 Best regards,
 -Gonglei

 Thanks,

 Paolo

 v4 - v3:
  1. rebase on qom-next tree (Andreas)
  2. fix memory leak in PATCH 2, move object_property_set_description
 calling
 in object_property_add_alias() from PATCH 3 to PATCH 2. (Paolo)
  3. drop ?: in PATCH 2, call g_strdup() directly
  4. rework PATCH 4, change description as optional field,
 drop ?: conditional express (Eric)

 v3 - v2:
  1. add a new description field to DevicePropertyInfo, and format
 it in qdev_device_help() in PATCH 6 (Paolo)

 v2 - v1:
  1. rename fail label to out in PATCH 1 (Andreas)
  2. improve descriptions in PATCH 3 (Paolo, adding Signed-off-by Paolo in
 this patch)
  3. rework PATCH 5, set description at qdev_property_add_static(),
 then copy the description of target_obj.property. (Paolo)
  4. free description filed of ObjectProperty avoid memory leak in PATCH 4.

 This patch series based on qom-next tree:
  https://github.com/afaerber/qemu-cpu/commits/qom-next

 Add a description field in both ObjectProperty and PropertyInfo struct.
 The descriptions can serve as documentation in the code,
 and they can be used to provide better help. For example:

 Before this patch series:

 $./qemu-system-x86_64 -device virtio-blk-pci,?

 virtio-blk-pci.iothread=linkiothread
 virtio-blk-pci.x-data-plane=bool
 virtio-blk-pci.scsi=bool
 virtio-blk-pci.config-wce=bool
 virtio-blk-pci.serial=str
 virtio-blk-pci.secs=uint32
 virtio-blk-pci.heads=uint32
 virtio-blk-pci.cyls=uint32
 virtio-blk-pci.discard_granularity=uint32
 virtio-blk-pci.bootindex=int32
 virtio-blk-pci.opt_io_size=uint32
 virtio-blk-pci.min_io_size=uint16
 virtio-blk-pci.physical_block_size=uint16
 virtio-blk-pci.logical_block_size=uint16
 virtio-blk-pci.drive=str
 virtio-blk-pci.virtio-backend=childvirtio-blk-device
 virtio-blk-pci.command_serr_enable=on/off
 virtio-blk-pci.multifunction=on/off
 virtio-blk-pci.rombar=uint32
 virtio-blk-pci.romfile=str
 virtio-blk-pci.addr=pci-devfn
 virtio-blk-pci.event_idx=on/off
 virtio-blk-pci.indirect_desc=on/off
 virtio-blk-pci.vectors=uint32
 virtio-blk-pci.ioeventfd=on/off
 virtio-blk-pci.class=uint32

 After:

 $./qemu-system-x86_64 -device virtio-blk-pci,?

 virtio-blk-pci.iothread=linkiothread
 virtio-blk-pci.x-data-plane=bool (on/off)
 virtio-blk-pci.scsi=bool (on/off)
 virtio-blk-pci.config-wce=bool (on/off)
 virtio-blk-pci.serial=str
 virtio-blk-pci.secs=uint32
 virtio-blk-pci.heads=uint32
 virtio-blk-pci.cyls=uint32
 virtio-blk-pci.discard_granularity=uint32
 virtio-blk-pci.bootindex=int32
 virtio-blk-pci.opt_io_size=uint32
 virtio-blk-pci.min_io_size=uint16
 virtio-blk-pci.physical_block_size=uint16 (A power of two between 512 and
 32768)
 virtio-blk-pci.logical_block_size=uint16 (A power of two between 512 and
 32768)
 virtio-blk-pci.drive=str (ID of a drive to use as a backend)
 virtio-blk-pci.virtio-backend=childvirtio-blk-device
 virtio-blk-pci.command_serr_enable=bool (on/off)
 virtio-blk-pci.multifunction=bool (on/off)
 virtio-blk-pci.rombar=uint32
 virtio-blk-pci.romfile=str
 virtio-blk-pci.addr=int32 (Slot and optional function number, example: 
 06.0
 or 06)
 virtio-blk-pci.event_idx=bool (on/off)
 virtio-blk-pci.indirect_desc=bool (on/off)
 virtio-blk-pci.vectors=uint32
 virtio-blk-pci.ioeventfd=bool (on/off)
 virtio-blk-pci.class=uint32


 Gonglei (5):
   qdev: add description field in PropertyInfo struct
   qom: add description field in ObjectProperty struct
   qdev: set the object property's description to the qdev property's.
   qmp: print descriptions of object properties
   qdev: drop legacy_name from qdev properties

  hw/core/qdev-properties-system.c |  8 
  hw/core/qdev-properties.c| 14 --
  hw/core/qdev.c   |  5 +
  include/hw/qdev-core.h   |  2 +-
  include/qom/object.h | 14 ++
  qapi-schema.json |  4 +++-
  

Re: [Qemu-devel] [PATCH v6 06/32] target-arm: A32: Emulate the SMC instruction

2014-10-13 Thread Greg Bellows
I reapplied my changes on top of your v5 with the latest backing.  It
basically scraps most of my changes on this patch for yours, except for
some slight updates here and there.

I'll continue to make any v7 updates on your v5 set.

Greg

On 13 October 2014 08:36, Peter Maydell peter.mayd...@linaro.org wrote:

 On 13 October 2014 15:13, Greg Bellows greg.bell...@linaro.org wrote:
  I realize that, but I don't believe your changes were available yet and
  still sounded to be a bit in flux, so I was waiting to merge.
 
  As I mentioned previously, I had already merged on top of your initial
  changes.

 Ah. I thought when you said you'd merged your series on top of mine
 that you'd merged it on top of the relevant patches...


  I'll recommit with your changes.

 Thanks.

 I'm going to finish reviewing the rest of this series, since
 the later patches are basically independent of this. I may
 be a day or two though as the next few require deeper thought
 than the minor nits in the first half dozen patches. So you
 don't need to push out a v7 just yet.

 -- PMM



[Qemu-devel] [PATCH] target-ppc: kvm: Fix memory overflow issue about strncat()

2014-10-13 Thread Chen Gang
strncat() will append additional '\0' to destination buffer, so need
additional 1 byte for it, or may cause memory overflow, just like other
area within QEMU have done.

Signed-off-by: Chen Gang gang.chen.5...@gmail.com
---
 target-ppc/kvm.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/target-ppc/kvm.c b/target-ppc/kvm.c
index 9c23c6b..66e7ce5 100644
--- a/target-ppc/kvm.c
+++ b/target-ppc/kvm.c
@@ -1794,8 +1794,8 @@ static uint64_t kvmppc_read_int_cpu_dt(const char 
*propname)
 return -1;
 }
 
-strncat(buf, /, sizeof(buf) - strlen(buf));
-strncat(buf, propname, sizeof(buf) - strlen(buf));
+strncat(buf, /, sizeof(buf) - strlen(buf) - 1);
+strncat(buf, propname, sizeof(buf) - strlen(buf) - 1);
 
 f = fopen(buf, rb);
 if (!f) {
-- 
1.9.3



Re: [Qemu-devel] [PATCH] target-ppc: kvm: Fix memory overflow issue about strncat()

2014-10-13 Thread Alexander Graf


On 13.10.14 16:36, Chen Gang wrote:
 strncat() will append additional '\0' to destination buffer, so need
 additional 1 byte for it, or may cause memory overflow, just like other
 area within QEMU have done.
 
 Signed-off-by: Chen Gang gang.chen.5...@gmail.com

I agree with this patch. However, the code is pretty ugly - I'm sure it
must've been me who wrote it :).

Could you please instead rewrite it to use g_strdup_printf() rather than
strncat()s? That way we resolve all string pitfalls automatically - and
this code is not the fast path, so doing an extra memory allocation is ok.


Alex

 ---
  target-ppc/kvm.c | 4 ++--
  1 file changed, 2 insertions(+), 2 deletions(-)
 
 diff --git a/target-ppc/kvm.c b/target-ppc/kvm.c
 index 9c23c6b..66e7ce5 100644
 --- a/target-ppc/kvm.c
 +++ b/target-ppc/kvm.c
 @@ -1794,8 +1794,8 @@ static uint64_t kvmppc_read_int_cpu_dt(const char 
 *propname)
  return -1;
  }
  
 -strncat(buf, /, sizeof(buf) - strlen(buf));
 -strncat(buf, propname, sizeof(buf) - strlen(buf));
 +strncat(buf, /, sizeof(buf) - strlen(buf) - 1);
 +strncat(buf, propname, sizeof(buf) - strlen(buf) - 1);
  
  f = fopen(buf, rb);
  if (!f) {
 



Re: [Qemu-devel] [PATCH v2] qemu-log: add log category for MMU info

2014-10-13 Thread Alexander Graf


On 10.10.14 16:47, Peter Maydell wrote:
 On 10 October 2014 11:59, Antony Pavlov antonynpav...@gmail.com wrote:
 Running barebox on qemu-system-mips* with '-d unimp' overloads
 stderr by very very many mips_cpu_handle_mmu_fault() messages:

   mips_cpu_handle_mmu_fault address=b80003fd ret 0 physical 180003fd 
 prot 3
   mips_cpu_handle_mmu_fault address=a0800884 ret 0 physical 00800884 
 prot 3
   mips_cpu_handle_mmu_fault pc a080cd80 ad b80003fd rw 0 mmu_idx 0

 So it's very difficult to find LOG_UNIMP message.

 The mips_cpu_handle_mmu_fault() messages appears on enabling ANY
 logging! It's not very handy.

 Adding separate log category for *_cpu_handle_mmu_fault()
 logging fixes the problem.

 Signed-off-by: Antony Pavlov antonynpav...@gmail.com
 
 This mostly looks good, thanks!
 
 I should also note that I'm happy for us to just implement
 the common-code (ie the log flag) and fix those targets
 which are particularly obnoxious about logging and/or
 easy to fix. We can always leave the other targets to
 update their code later (or you could update other targets
 in separate patches once the main one has gone in).
 
 A minor tweak:
 
 --- a/target-cris/helper.c
 +++ b/target-cris/helper.c
 @@ -30,9 +30,11 @@
  #ifdef CRIS_HELPER_DEBUG
  #define D(x) x
  #define D_LOG(...) qemu_log(__VA_ARGS__)
 +#define LOG_MMU(...) qemu_log_mask(CPU_LOG_MMU, __VA_ARGS__)
  #else
  #define D(x)
  #define D_LOG(...) do { } while (0)
 +#define LOG_MMU(...) do { } while (0)
  #endif
 
 Now this logging is configurably enablable at runtime,
 we should just call qemu_log_mask() directly, rather
 than wrapping it in a LOG_MMU macro which might be compiled
 out.

The MMU lookups are in a pretty hot path. Are you sure we always want to
have the log enabled checks and register pollution in there?


Alex



Re: [Qemu-devel] [PATCH v2] libvixl: a64: Skip -Wunused-variable for gcc 5.0.0 or higher

2014-10-13 Thread Eric Blake
On 10/12/2014 01:50 AM, Peter Maydell wrote:
 On 12 October 2014 01:32, Chen Gang gang.chen.5...@gmail.com wrote:
 On 10/12/14 5:25, Peter Maydell wrote:
 Some other approaches to this that would confine the
 fix to the makefiles rather than requiring us to modify
 the vixl source itself:
  a) add a -Wno- option for the affected .o files

 It is one way, but may have effect with gcc 4 version, and also it is
 effect with the whole file which is wider than current way.

  b) use -isystem rather than -I to include the libvixl
 directory on the include path


 It sounds good to me, although for me, it is not related with current
 issue.
 
 -isystem disables a bunch of gcc warnings automatically,
 which is why I suggested it. I'm not overall sure it's
 a great idea though.

-isystem is a heavy hammer, affecting the entire compilation.  Better
might be just marking the ONE header as being a system header (silence
various warnings caused by just that header, while still letting the
rest of the compilation warn).  If the header comes from third-party
sources, this is probably the best approach.  It is done by adding:

#if __GNUC__ = 3
#pragma GCC system_header
#endif

to the header that would otherwise trigger warnings.

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



signature.asc
Description: OpenPGP digital signature


Re: [Qemu-devel] [PATCH v3 3/3] util: Infrastructure for computing recent averages

2014-10-13 Thread Benoît Canet
On Mon, Oct 13, 2014 at 02:05:34PM +0200, Paolo Bonzini wrote:
 Il 24/09/2014 17:21, Benoît Canet ha scritto:
  The module takes care of computing minimal and maximal
  values over the time slice duration.
 
 The code looks good, just two comments:
 
  +/* Get the average value
  + *
  + * @ta:  the timed average structure used
  + * @ret: the average value
  + */
  +uint64_t timed_average_avg(TimedAverage *ta)
  +{
  +Window *w;
  +check_expirations(ta);
  +
  +w = current_window(ta);
  +
  +if (w-count) {
  +return w-sum / w-count;
 
 First, do we want this to return double?
 
 Second, this will return the min/max/avg in an unknown amount of time
 between period/2 and period---on average period*3/4, e.g. 0.75 seconds
 for a period equal to one second.
 
 Would it make sense to tweak the TimedAverage period to be higher, e.g.
 1.33 seconds/80 seconds/80 minutes, so that the _average_ period is 1
 second/1 minute/1 hour?
 
 This only applies to how the code is used, not to TimedAverage itself;
 hence, feel free to post the patch with Reviewed-by once
 timed_average_avg's return type is changed to a double.

This would require either changing _init period units or making it a float
or baking the 1.33 ratio in timed average.

Which one would you prefer ?

Best regards

Benoît

Thanks for reviewing.

 
 Paolo
 
  +}
  +
  +return 0;
  +}
  +
  +/* Get the maximum value
  + *
  + * @ta:  the timed average structure used
  + * @ret: the maximal value
  + */
  +uint64_t timed_average_max(TimedAverage *ta)
  +{
  +check_expirations(ta);
  +return current_window(ta)-max;
  +}
  
 



Re: [Qemu-devel] [PATCH v2] libvixl: a64: Skip -Wunused-variable for gcc 5.0.0 or higher

2014-10-13 Thread Peter Maydell
On 13 October 2014 16:59, Eric Blake ebl...@redhat.com wrote:
 -isystem is a heavy hammer, affecting the entire compilation.  Better
 might be just marking the ONE header as being a system header (silence
 various warnings caused by just that header, while still letting the
 rest of the compilation warn).  If the header comes from third-party
 sources, this is probably the best approach.  It is done by adding:

 #if __GNUC__ = 3
 #pragma GCC system_header
 #endif

 to the header that would otherwise trigger warnings.

If we're going to modify the header then we might as well just
disable the specific warning that's being triggered. The only
benefit of -isystem is you can do it in the makefile and leave
the source file untouched.

-- PMM



[Qemu-devel] [PATCH v3 1/5] target-tricore: Cleanup and Bugfixes

2014-10-13 Thread Bastian Koppelmann
Move FCX loading of save_context_ to caller functions, for STLCX, STUCX insn to 
use those functions.
Move FCX storing of restore_context_ to caller functions, for LDLCX, LDUCX insn 
to use those functions.
Remove do_raise_exception function, which caused clang to emit a warning.
Fix: save_context_lower now saves a[11] instead of PSW.
Fix: MASK_OP_ABSB_BPOS starting at wrong offset.

Signed-off-by: Bastian Koppelmann kbast...@mail.uni-paderborn.de
Reviewed-by: Richard Henderson r...@twiddle.net
---
 target-tricore/op_helper.c   | 47 ++--
 target-tricore/tricore-opcodes.h |  2 +-
 2 files changed, 22 insertions(+), 27 deletions(-)

diff --git a/target-tricore/op_helper.c b/target-tricore/op_helper.c
index 6376f07..f2a5cbc 100644
--- a/target-tricore/op_helper.c
+++ b/target-tricore/op_helper.c
@@ -114,10 +114,8 @@ static bool cdc_zero(target_ulong *psw)
 return count == 0;
 }
 
-static void save_context_upper(CPUTriCoreState *env, int ea,
-   target_ulong *new_FCX)
+static void save_context_upper(CPUTriCoreState *env, int ea)
 {
-*new_FCX = cpu_ldl_data(env, ea);
 cpu_stl_data(env, ea, env-PCXI);
 cpu_stl_data(env, ea+4, env-PSW);
 cpu_stl_data(env, ea+8, env-gpr_a[10]);
@@ -134,15 +132,12 @@ static void save_context_upper(CPUTriCoreState *env, int 
ea,
 cpu_stl_data(env, ea+52, env-gpr_d[13]);
 cpu_stl_data(env, ea+56, env-gpr_d[14]);
 cpu_stl_data(env, ea+60, env-gpr_d[15]);
-
 }
 
-static void save_context_lower(CPUTriCoreState *env, int ea,
-   target_ulong *new_FCX)
+static void save_context_lower(CPUTriCoreState *env, int ea)
 {
-*new_FCX = cpu_ldl_data(env, ea);
 cpu_stl_data(env, ea, env-PCXI);
-cpu_stl_data(env, ea+4, env-PSW);
+cpu_stl_data(env, ea+4, env-gpr_a[11]);
 cpu_stl_data(env, ea+8, env-gpr_a[2]);
 cpu_stl_data(env, ea+12, env-gpr_a[3]);
 cpu_stl_data(env, ea+16, env-gpr_d[0]);
@@ -178,7 +173,6 @@ static void restore_context_upper(CPUTriCoreState *env, int 
ea,
 env-gpr_d[13] = cpu_ldl_data(env, ea+52);
 env-gpr_d[14] = cpu_ldl_data(env, ea+56);
 env-gpr_d[15] = cpu_ldl_data(env, ea+60);
-cpu_stl_data(env, ea, env-FCX);
 }
 
 void helper_call(CPUTriCoreState *env, uint32_t next_pc)
@@ -206,11 +200,12 @@ void helper_call(CPUTriCoreState *env, uint32_t next_pc)
 /* EA = {FCX.FCXS, 6'b0, FCX.FCXO, 6'b0}; */
 ea = ((env-FCX  MASK_FCX_FCXS)  12) +
  ((env-FCX  MASK_FCX_FCXO)  6);
-/* new_FCX = M(EA, word);
-   M(EA, 16 * word) = {PCXI, PSW, A[10], A[11], D[8], D[9], D[10], D[11],
-  A[12], A[13], A[14], A[15], D[12], D[13], D[14],
-  D[15]}; */
-save_context_upper(env, ea, new_FCX);
+/* new_FCX = M(EA, word); */
+new_FCX = cpu_ldl_data(env, ea);
+/* M(EA, 16 * word) = {PCXI, PSW, A[10], A[11], D[8], D[9], D[10], D[11],
+   A[12], A[13], A[14], A[15], D[12], D[13], D[14],
+   D[15]}; */
+save_context_upper(env, ea);
 
 /* PCXI.PCPN = ICR.CCPN; */
 env-PCXI = (env-PCXI  0xff) +
@@ -263,9 +258,10 @@ void helper_ret(CPUTriCoreState *env)
 ea = ((env-PCXI  MASK_PCXI_PCXS)  12) +
  ((env-PCXI  MASK_PCXI_PCXO)  6);
 /* {new_PCXI, new_PSW, A[10], A[11], D[8], D[9], D[10], D[11], A[12],
-A[13], A[14], A[15], D[12], D[13], D[14], D[15]} = M(EA, 16 * word);
-M(EA, word) = FCX; */
+A[13], A[14], A[15], D[12], D[13], D[14], D[15]} = M(EA, 16 * word); */
 restore_context_upper(env, ea, new_PCXI, new_PSW);
+/* M(EA, word) = FCX; */
+cpu_stl_data(env, ea, env-FCX);
 /* FCX[19: 0] = PCXI[19: 0]; */
 env-FCX = (env-FCX  0xfff0) + (env-PCXI  0x000f);
 /* PCXI = new_PCXI; */
@@ -293,7 +289,12 @@ void helper_bisr(CPUTriCoreState *env, uint32_t const9)
 tmp_FCX = env-FCX;
 ea = ((env-FCX  0xf)  12) + ((env-FCX  0x)  6);
 
-save_context_lower(env, ea, new_FCX);
+/* new_FCX = M(EA, word); */
+new_FCX = cpu_ldl_data(env, ea);
+/* M(EA, 16 * word) = {PCXI, A[11], A[2], A[3], D[0], D[1], D[2], D[3], 
A[4]
+   , A[5], A[6], A[7], D[4], D[5], D[6], D[7]}; */
+save_context_lower(env, ea);
+
 
 /* PCXI.PCPN = ICR.CCPN */
 env-PCXI = (env-PCXI  0xff) +
@@ -343,9 +344,10 @@ void helper_rfe(CPUTriCoreState *env)
 ea = ((env-PCXI  MASK_PCXI_PCXS)  12) +
  ((env-PCXI  MASK_PCXI_PCXO)  6);
 /*{new_PCXI, PSW, A[10], A[11], D[8], D[9], D[10], D[11], A[12],
-  A[13], A[14], A[15], D[12], D[13], D[14], D[15]} = M(EA, 16 * word);
-  M(EA, word) = FCX;*/
+  A[13], A[14], A[15], D[12], D[13], D[14], D[15]} = M(EA, 16 * word); */
 restore_context_upper(env, ea, new_PCXI, new_PSW);
+/* M(EA, word) = FCX;*/
+cpu_stl_data(env, ea, env-FCX);
 /* FCX[19: 0] = PCXI[19: 0]; */
 env-FCX = (env-FCX  0xfff0) + (env-PCXI  0x000f);
   

[Qemu-devel] [PATCH v3 2/5] target-tricore: Add instructions of ABS, ABSB opcode format

2014-10-13 Thread Bastian Koppelmann
Add instructions of ABS, ABSB opcode format.
Add microcode generator functions for ld/st of two 32bit reg as one 64bit value.
Add microcode generator functions for ldmst and swap.
Add helper ldlcx, lducx, stlcx and stucx.

Signed-off-by: Bastian Koppelmann kbast...@mail.uni-paderborn.de
Reviewed-by: Richard Henderson r...@twiddle.net
---
 target-tricore/helper.h|   4 +
 target-tricore/op_helper.c |  45 +++
 target-tricore/translate.c | 303 +
 3 files changed, 352 insertions(+)

diff --git a/target-tricore/helper.h b/target-tricore/helper.h
index 7b7d74b..fbabbd5 100644
--- a/target-tricore/helper.h
+++ b/target-tricore/helper.h
@@ -23,3 +23,7 @@ DEF_HELPER_2(call, void, env, i32)
 DEF_HELPER_1(ret, void, env)
 DEF_HELPER_2(bisr, void, env, i32)
 DEF_HELPER_1(rfe, void, env)
+DEF_HELPER_2(ldlcx, void, env, i32)
+DEF_HELPER_2(lducx, void, env, i32)
+DEF_HELPER_2(stlcx, void, env, i32)
+DEF_HELPER_2(stucx, void, env, i32)
diff --git a/target-tricore/op_helper.c b/target-tricore/op_helper.c
index f2a5cbc..94b5d8e 100644
--- a/target-tricore/op_helper.c
+++ b/target-tricore/op_helper.c
@@ -175,6 +175,27 @@ static void restore_context_upper(CPUTriCoreState *env, 
int ea,
 env-gpr_d[15] = cpu_ldl_data(env, ea+60);
 }
 
+static void restore_context_lower(CPUTriCoreState *env, int ea,
+  target_ulong *ra, target_ulong *pcxi)
+{
+*pcxi = cpu_ldl_data(env, ea);
+*ra = cpu_ldl_data(env, ea+4);
+env-gpr_a[2] = cpu_ldl_data(env, ea+8);
+env-gpr_a[3] = cpu_ldl_data(env, ea+12);
+env-gpr_d[0] = cpu_ldl_data(env, ea+16);
+env-gpr_d[1] = cpu_ldl_data(env, ea+20);
+env-gpr_d[2] = cpu_ldl_data(env, ea+24);
+env-gpr_d[3] = cpu_ldl_data(env, ea+28);
+env-gpr_a[4] = cpu_ldl_data(env, ea+32);
+env-gpr_a[5] = cpu_ldl_data(env, ea+36);
+env-gpr_a[6] = cpu_ldl_data(env, ea+40);
+env-gpr_a[7] = cpu_ldl_data(env, ea+44);
+env-gpr_d[4] = cpu_ldl_data(env, ea+48);
+env-gpr_d[5] = cpu_ldl_data(env, ea+52);
+env-gpr_d[6] = cpu_ldl_data(env, ea+56);
+env-gpr_d[7] = cpu_ldl_data(env, ea+60);
+}
+
 void helper_call(CPUTriCoreState *env, uint32_t next_pc)
 {
 target_ulong tmp_FCX;
@@ -356,6 +377,30 @@ void helper_rfe(CPUTriCoreState *env)
 psw_write(env, new_PSW);
 }
 
+void helper_ldlcx(CPUTriCoreState *env, uint32_t ea)
+{
+uint32_t dummy;
+/* insn doesn't load PCXI and RA */
+restore_context_lower(env, ea, dummy, dummy);
+}
+
+void helper_lducx(CPUTriCoreState *env, uint32_t ea)
+{
+uint32_t dummy;
+/* insn doesn't load PCXI and PSW */
+restore_context_upper(env, ea, dummy, dummy);
+}
+
+void helper_stlcx(CPUTriCoreState *env, uint32_t ea)
+{
+save_context_lower(env, ea);
+}
+
+void helper_stucx(CPUTriCoreState *env, uint32_t ea)
+{
+save_context_upper(env, ea);
+}
+
 static inline void QEMU_NORETURN do_raise_exception_err(CPUTriCoreState *env,
 uint32_t exception,
 int error_code,
diff --git a/target-tricore/translate.c b/target-tricore/translate.c
index 4f654de..fc89a43 100644
--- a/target-tricore/translate.c
+++ b/target-tricore/translate.c
@@ -115,6 +115,8 @@ void tricore_cpu_dump_state(CPUState *cs, FILE *f,
 tcg_temp_free_i32(helper_tmp);\
 } while (0)
 
+#define EA_ABS_FORMAT(con) (((con  0x3C000)  14) + (con  0x3FFF))
+
 /* Functions for load/save to/from memory */
 
 static inline void gen_offset_ld(DisasContext *ctx, TCGv r1, TCGv r2,
@@ -135,6 +137,62 @@ static inline void gen_offset_st(DisasContext *ctx, TCGv 
r1, TCGv r2,
 tcg_temp_free(temp);
 }
 
+static void gen_st_2regs_64(TCGv rh, TCGv rl, TCGv address, DisasContext *ctx)
+{
+TCGv_i64 temp = tcg_temp_new_i64();
+
+tcg_gen_concat_i32_i64(temp, rl, rh);
+tcg_gen_qemu_st_i64(temp, address, ctx-mem_idx, MO_LEQ);
+
+tcg_temp_free_i64(temp);
+}
+
+static void gen_ld_2regs_64(TCGv rh, TCGv rl, TCGv address, DisasContext *ctx)
+{
+TCGv_i64 temp = tcg_temp_new_i64();
+
+tcg_gen_qemu_ld_i64(temp, address, ctx-mem_idx, MO_LEQ);
+/* write back to two 32 bit regs */
+tcg_gen_extr_i64_i32(rl, rh, temp);
+
+tcg_temp_free_i64(temp);
+}
+
+/* M(EA, word) = (M(EA, word)  ~E[a][63:32]) | (E[a][31:0]  E[a][63:32]); */
+static void gen_ldmst(DisasContext *ctx, int ereg, TCGv ea)
+{
+TCGv temp = tcg_temp_new();
+TCGv temp2 = tcg_temp_new();
+
+/* temp = (M(EA, word) */
+tcg_gen_qemu_ld_tl(temp, ea, ctx-mem_idx, MO_LEUL);
+/* temp = temp  ~E[a][63:32]) */
+tcg_gen_andc_tl(temp, temp, cpu_gpr_d[ereg+1]);
+/* temp2 = (E[a][31:0]  E[a][63:32]); */
+tcg_gen_and_tl(temp2, cpu_gpr_d[ereg], cpu_gpr_d[ereg+1]);
+/* temp = temp | temp2; */
+tcg_gen_or_tl(temp, temp, temp2);
+/* M(EA, word) = temp; */
+tcg_gen_qemu_st_tl(temp, ea, ctx-mem_idx, MO_LEUL);
+
+

[Qemu-devel] [PATCH v3 0/5] Add TriCore ABS, ABSB, B, BIT, BO instructions

2014-10-13 Thread Bastian Koppelmann
Hi guys,

here is the next round of TriCore patches. The first patch addresses a clang 
issue mentioned by Peter Maydell and
some bugfixes. And the other four add instructions of the ABS, ABSB, B, BIT and 
BO opcode format.

Thanks,

Bastian

v2 - v3:
- OR_NOR_T, AND_NOR_T: Now uses normal conditionals instead of preprocessor.
- Add microcode generator functions gen_st/ld_preincr, which write back the
  address after the memory access.
- ST/LD_PREINC insn now use gen_st/ld_preincr or write back the address 
after
  after the memory access.

Bastian Koppelmann (5):
  target-tricore: Cleanup and Bugfixes
  target-tricore: Add instructions of ABS, ABSB opcode format
  target-tricore: Add instructions of B opcode format
  target-tricore: Add instructions of BIT opcode format
  target-tricore: Add instructions of BO opcode format

 target-tricore/helper.h  |7 +
 target-tricore/op_helper.c   |  128 +++-
 target-tricore/translate.c   | 1305 ++
 target-tricore/tricore-opcodes.h |4 +-
 4 files changed, 1417 insertions(+), 27 deletions(-)

--
2.1.2




[Qemu-devel] [PATCH v3 3/5] target-tricore: Add instructions of B opcode format

2014-10-13 Thread Bastian Koppelmann
Add instructions of B opcode format.

Signed-off-by: Bastian Koppelmann kbast...@mail.uni-paderborn.de
Reviewed-by: Richard Henderson r...@twiddle.net
---
 target-tricore/translate.c | 27 +++
 1 file changed, 27 insertions(+)

diff --git a/target-tricore/translate.c b/target-tricore/translate.c
index fc89a43..830bcd0 100644
--- a/target-tricore/translate.c
+++ b/target-tricore/translate.c
@@ -116,6 +116,8 @@ void tricore_cpu_dump_state(CPUState *cs, FILE *f,
 } while (0)
 
 #define EA_ABS_FORMAT(con) (((con  0x3C000)  14) + (con  0x3FFF))
+#define EA_B_ABSOLUT(con) (((offset  0xf0)  8) | \
+   ((offset  0x0f)  1))
 
 /* Functions for load/save to/from memory */
 
@@ -492,6 +494,7 @@ static void gen_compute_branch(DisasContext *ctx, uint32_t 
opc, int r1,
 case OPC1_32_B_J:
 gen_goto_tb(ctx, 0, ctx-pc + offset * 2);
 break;
+case OPC1_32_B_CALL:
 case OPC1_16_SB_CALL:
 gen_helper_1arg(call, ctx-next_pc);
 gen_goto_tb(ctx, 0, ctx-pc + offset * 2);
@@ -567,6 +570,20 @@ static void gen_compute_branch(DisasContext *ctx, uint32_t 
opc, int r1,
 gen_helper_ret(cpu_env);
 tcg_gen_exit_tb(0);
 break;
+/* B-format */
+case OPC1_32_B_CALLA:
+gen_helper_1arg(call, ctx-next_pc);
+gen_goto_tb(ctx, 0, EA_B_ABSOLUT(offset));
+break;
+case OPC1_32_B_JLA:
+tcg_gen_movi_tl(cpu_gpr_a[11], ctx-next_pc);
+case OPC1_32_B_JA:
+gen_goto_tb(ctx, 0, EA_B_ABSOLUT(offset));
+break;
+case OPC1_32_B_JL:
+tcg_gen_movi_tl(cpu_gpr_a[11], ctx-next_pc);
+gen_goto_tb(ctx, 0, ctx-pc + offset * 2);
+break;
 default:
 printf(Branch Error at %x\n, ctx-pc);
 }
@@ -1403,6 +1420,16 @@ static void decode_32Bit_opc(CPUTriCoreState *env, 
DisasContext *ctx)
 tcg_temp_free(temp);
 tcg_temp_free(temp2);
 break;
+/* B-format */
+case OPC1_32_B_CALL:
+case OPC1_32_B_CALLA:
+case OPC1_32_B_J:
+case OPC1_32_B_JA:
+case OPC1_32_B_JL:
+case OPC1_32_B_JLA:
+address = MASK_OP_B_DISP24(ctx-opcode);
+gen_compute_branch(ctx, op1, 0, 0, 0, address);
+break;
 }
 }
 
-- 
2.1.2




[Qemu-devel] [PATCH v3 4/5] target-tricore: Add instructions of BIT opcode format

2014-10-13 Thread Bastian Koppelmann
Add instructions of BIT opcode format.
Add microcode generator functions gen_bit_1/2op to do 1/2 bit operations on the 
last bit.

Signed-off-by: Bastian Koppelmann kbast...@mail.uni-paderborn.de
---
v2 - v3:
- OR_NOR_T, AND_NOR_T: Now uses normal conditionals instead of preprocessor.

 target-tricore/translate.c | 312 +
 1 file changed, 312 insertions(+)

diff --git a/target-tricore/translate.c b/target-tricore/translate.c
index 830bcd0..ddbabc4 100644
--- a/target-tricore/translate.c
+++ b/target-tricore/translate.c
@@ -425,6 +425,49 @@ static inline void gen_subs(TCGv ret, TCGv r1, TCGv r2)
 gen_helper_sub_ssov(ret, cpu_env, r1, r2);
 }

+static inline void gen_bit_2op(TCGv ret, TCGv r1, TCGv r2,
+   int pos1, int pos2,
+   void(*op1)(TCGv, TCGv, TCGv),
+   void(*op2)(TCGv, TCGv, TCGv))
+{
+TCGv temp1, temp2;
+
+temp1 = tcg_temp_new();
+temp2 = tcg_temp_new();
+
+tcg_gen_shri_tl(temp2, r2, pos2);
+tcg_gen_shri_tl(temp1, r1, pos1);
+
+(*op1)(temp1, temp1, temp2);
+(*op2)(temp1 , ret, temp1);
+
+tcg_gen_deposit_tl(ret, ret, temp1, 0, 1);
+
+tcg_temp_free(temp1);
+tcg_temp_free(temp2);
+}
+
+/* ret = r1[pos1] op1 r2[pos2]; */
+static inline void gen_bit_1op(TCGv ret, TCGv r1, TCGv r2,
+   int pos1, int pos2,
+   void(*op1)(TCGv, TCGv, TCGv))
+{
+TCGv temp1, temp2;
+
+temp1 = tcg_temp_new();
+temp2 = tcg_temp_new();
+
+tcg_gen_shri_tl(temp2, r2, pos2);
+tcg_gen_shri_tl(temp1, r1, pos1);
+
+(*op1)(ret, temp1, temp2);
+
+tcg_gen_andi_tl(ret, ret, 0x1);
+
+tcg_temp_free(temp1);
+tcg_temp_free(temp2);
+}
+
 /* helpers for generating program flow micro-ops */

 static inline void gen_save_pc(target_ulong pc)
@@ -1345,6 +1388,253 @@ static void decode_abs_storeb_h(CPUTriCoreState *env, 
DisasContext *ctx)
 tcg_temp_free(temp);
 }

+/* Bit-format */
+
+static void decode_bit_andacc(CPUTriCoreState *env, DisasContext *ctx)
+{
+uint32_t op2;
+int r1, r2, r3;
+int pos1, pos2;
+
+r1 = MASK_OP_BIT_S1(ctx-opcode);
+r2 = MASK_OP_BIT_S2(ctx-opcode);
+r3 = MASK_OP_BIT_D(ctx-opcode);
+pos1 = MASK_OP_BIT_POS1(ctx-opcode);
+pos2 = MASK_OP_BIT_POS2(ctx-opcode);
+op2 = MASK_OP_BIT_OP2(ctx-opcode);
+
+
+switch (op2) {
+case OPC2_32_BIT_AND_AND_T:
+gen_bit_2op(cpu_gpr_d[r3], cpu_gpr_d[r1], cpu_gpr_d[r2],
+pos1, pos2, tcg_gen_and_tl, tcg_gen_and_tl);
+break;
+case OPC2_32_BIT_AND_ANDN_T:
+gen_bit_2op(cpu_gpr_d[r3], cpu_gpr_d[r1], cpu_gpr_d[r2],
+pos1, pos2, tcg_gen_andc_tl, tcg_gen_and_tl);
+break;
+case OPC2_32_BIT_AND_NOR_T:
+if (TCG_TARGET_HAS_andc_i32) {
+gen_bit_2op(cpu_gpr_d[r3], cpu_gpr_d[r1], cpu_gpr_d[r2],
+pos1, pos2, tcg_gen_or_tl, tcg_gen_andc_tl);
+} else {
+gen_bit_2op(cpu_gpr_d[r3], cpu_gpr_d[r1], cpu_gpr_d[r2],
+pos1, pos2, tcg_gen_nor_tl, tcg_gen_and_tl);
+}
+break;
+case OPC2_32_BIT_AND_OR_T:
+gen_bit_2op(cpu_gpr_d[r3], cpu_gpr_d[r1], cpu_gpr_d[r2],
+pos1, pos2, tcg_gen_or_tl, tcg_gen_and_tl);
+break;
+}
+}
+
+static void decode_bit_logical_t(CPUTriCoreState *env, DisasContext *ctx)
+{
+uint32_t op2;
+int r1, r2, r3;
+int pos1, pos2;
+r1 = MASK_OP_BIT_S1(ctx-opcode);
+r2 = MASK_OP_BIT_S2(ctx-opcode);
+r3 = MASK_OP_BIT_D(ctx-opcode);
+pos1 = MASK_OP_BIT_POS1(ctx-opcode);
+pos2 = MASK_OP_BIT_POS2(ctx-opcode);
+op2 = MASK_OP_BIT_OP2(ctx-opcode);
+
+switch (op2) {
+case OPC2_32_BIT_AND_T:
+gen_bit_1op(cpu_gpr_d[r3], cpu_gpr_d[r1], cpu_gpr_d[r2],
+pos1, pos2, tcg_gen_and_tl);
+break;
+case OPC2_32_BIT_ANDN_T:
+gen_bit_1op(cpu_gpr_d[r3], cpu_gpr_d[r1], cpu_gpr_d[r2],
+pos1, pos2, tcg_gen_andc_tl);
+break;
+case OPC2_32_BIT_NOR_T:
+gen_bit_1op(cpu_gpr_d[r3], cpu_gpr_d[r1], cpu_gpr_d[r2],
+pos1, pos2, tcg_gen_nor_tl);
+break;
+case OPC2_32_BIT_OR_T:
+gen_bit_1op(cpu_gpr_d[r3], cpu_gpr_d[r1], cpu_gpr_d[r2],
+pos1, pos2, tcg_gen_or_tl);
+break;
+}
+}
+
+static void decode_bit_insert(CPUTriCoreState *env, DisasContext *ctx)
+{
+uint32_t op2;
+int r1, r2, r3;
+int pos1, pos2;
+TCGv temp;
+op2 = MASK_OP_BIT_OP2(ctx-opcode);
+r1 = MASK_OP_BIT_S1(ctx-opcode);
+r2 = MASK_OP_BIT_S2(ctx-opcode);
+r3 = MASK_OP_BIT_D(ctx-opcode);
+pos1 = MASK_OP_BIT_POS1(ctx-opcode);
+pos2 = MASK_OP_BIT_POS2(ctx-opcode);
+
+temp = tcg_temp_new();
+
+tcg_gen_shri_tl(temp, cpu_gpr_d[r2], pos2);
+if (op2 == OPC2_32_BIT_INSN_T) {
+tcg_gen_not_tl(temp, temp);
+ 

[Qemu-devel] [PATCH v3 5/5] target-tricore: Add instructions of BO opcode format

2014-10-13 Thread Bastian Koppelmann
Add instructions of BO opcode format.
Add microcode generator functions gen_swap, gen_ldmst.
Add microcode generator functions gen_st/ld_preincr, which write back the 
address after the memory access.
Add helper for circular and bit reverse addr mode calculation.
Add sign extended bitmask for BO_OFF10 field.

Signed-off-by: Bastian Koppelmann kbast...@mail.uni-paderborn.de
---
v2 - v3:
- Add microcode generator functions gen_st/ld_preincr, which write back the
  address after the memory access.
- ST/LD_PREINC insn now use gen_st/ld_preincr or write back the address 
after
  after the memory access.

 target-tricore/helper.h  |   3 +
 target-tricore/op_helper.c   |  36 +++
 target-tricore/translate.c   | 663 +++
 target-tricore/tricore-opcodes.h |   2 +
 4 files changed, 704 insertions(+)

diff --git a/target-tricore/helper.h b/target-tricore/helper.h
index fbabbd5..b3fc33c 100644
--- a/target-tricore/helper.h
+++ b/target-tricore/helper.h
@@ -27,3 +27,6 @@ DEF_HELPER_2(ldlcx, void, env, i32)
 DEF_HELPER_2(lducx, void, env, i32)
 DEF_HELPER_2(stlcx, void, env, i32)
 DEF_HELPER_2(stucx, void, env, i32)
+/* Address mode helper */
+DEF_HELPER_1(br_update, i32, i32)
+DEF_HELPER_2(circ_update, i32, i32, i32)
diff --git a/target-tricore/op_helper.c b/target-tricore/op_helper.c
index 94b5d8e..a36988a 100644
--- a/target-tricore/op_helper.c
+++ b/target-tricore/op_helper.c
@@ -20,6 +20,42 @@
 #include exec/helper-proto.h
 #include exec/cpu_ldst.h

+/* Addressing mode helper */
+
+static uint16_t reverse16(uint16_t val)
+{
+uint8_t high = (uint8_t)(val  8);
+uint8_t low  = (uint8_t)(val  0xff);
+
+uint16_t rh, rl;
+
+rl = (uint16_t)((high * 0x0202020202ULL  0x010884422010ULL) % 1023);
+rh = (uint16_t)((low * 0x0202020202ULL  0x010884422010ULL) % 1023);
+
+return (rh  8) | rl;
+}
+
+uint32_t helper_br_update(uint32_t reg)
+{
+uint32_t index = reg  0x;
+uint32_t incr  = reg  16;
+uint32_t new_index = reverse16(reverse16(index) + reverse16(incr));
+return reg - index + new_index;
+}
+
+uint32_t helper_circ_update(uint32_t reg, uint32_t off)
+{
+uint32_t index = reg  0x;
+uint32_t length = reg  16;
+int32_t new_index = index + off;
+if (new_index  0) {
+new_index += length;
+} else {
+new_index %= length;
+}
+return reg - index + new_index;
+}
+
 #define SSOV(env, ret, arg, len) do {   \
 int64_t max_pos = INT##len ##_MAX;  \
 int64_t max_neg = INT##len ##_MIN;  \
diff --git a/target-tricore/translate.c b/target-tricore/translate.c
index ddbabc4..d5a9596 100644
--- a/target-tricore/translate.c
+++ b/target-tricore/translate.c
@@ -149,6 +149,15 @@ static void gen_st_2regs_64(TCGv rh, TCGv rl, TCGv 
address, DisasContext *ctx)
 tcg_temp_free_i64(temp);
 }

+static void gen_offset_st_2regs(TCGv rh, TCGv rl, TCGv base, int16_t con,
+DisasContext *ctx)
+{
+TCGv temp = tcg_temp_new();
+tcg_gen_addi_tl(temp, base, con);
+gen_st_2regs_64(rh, rl, temp, ctx);
+tcg_temp_free(temp);
+}
+
 static void gen_ld_2regs_64(TCGv rh, TCGv rl, TCGv address, DisasContext *ctx)
 {
 TCGv_i64 temp = tcg_temp_new_i64();
@@ -160,6 +169,35 @@ static void gen_ld_2regs_64(TCGv rh, TCGv rl, TCGv 
address, DisasContext *ctx)
 tcg_temp_free_i64(temp);
 }

+static void gen_offset_ld_2regs(TCGv rh, TCGv rl, TCGv base, int16_t con,
+DisasContext *ctx)
+{
+TCGv temp = tcg_temp_new();
+tcg_gen_addi_tl(temp, base, con);
+gen_ld_2regs_64(rh, rl, temp, ctx);
+tcg_temp_free(temp);
+}
+
+static void gen_st_preincr(DisasContext *ctx, TCGv r1, TCGv r2, int16_t off,
+   TCGMemOp mop)
+{
+TCGv temp = tcg_temp_new();
+tcg_gen_addi_tl(temp, r2, off);
+tcg_gen_qemu_st_tl(r1, temp, ctx-mem_idx, mop);
+tcg_gen_mov_tl(r2, temp);
+tcg_temp_free(temp);
+}
+
+static void gen_ld_preincr(DisasContext *ctx, TCGv r1, TCGv r2, int16_t off,
+   TCGMemOp mop)
+{
+TCGv temp = tcg_temp_new();
+tcg_gen_addi_tl(temp, r2, off);
+tcg_gen_qemu_ld_tl(r1, temp, ctx-mem_idx, mop);
+tcg_gen_mov_tl(r2, temp);
+tcg_temp_free(temp);
+}
+
 /* M(EA, word) = (M(EA, word)  ~E[a][63:32]) | (E[a][31:0]  E[a][63:32]); */
 static void gen_ldmst(DisasContext *ctx, int ereg, TCGv ea)
 {
@@ -1635,6 +1673,612 @@ static void decode_bit_sh_logic2(CPUTriCoreState *env, 
DisasContext *ctx)
 tcg_temp_free(temp);
 }

+/* BO-format */
+
+
+static void decode_bo_addrmode_post_pre_base(CPUTriCoreState *env,
+ DisasContext *ctx)
+{
+uint32_t op2;
+uint32_t off10;
+int32_t r1, r2;
+TCGv temp;
+
+r1 = MASK_OP_BO_S1D(ctx-opcode);
+r2  = MASK_OP_BO_S2(ctx-opcode);
+off10 = MASK_OP_BO_OFF10_SEXT(ctx-opcode);
+op2 = MASK_OP_BO_OP2(ctx-opcode);
+
+

Re: [Qemu-devel] [PATCH] target-ppc: kvm: Fix memory overflow issue about strncat()

2014-10-13 Thread Chen Gang
On 10/13/14 22:47, Alexander Graf wrote:
 
 Could you please instead rewrite it to use g_strdup_printf() rather than
 strncat()s? That way we resolve all string pitfalls automatically - and
 this code is not the fast path, so doing an extra memory allocation is ok.


I guess, it is a personal taste. For me, it may need additional variable
for g_strdup_printf(), and not save code lines, but *sprintf() is more
readable than str*cat().

The related code may like below (welcome any improvement for it):

diff --git a/target-ppc/kvm.c b/target-ppc/kvm.c
index 66e7ce5..cea6a87 100644
--- a/target-ppc/kvm.c
+++ b/target-ppc/kvm.c
@@ -1782,7 +1782,7 @@ static int kvmppc_find_cpu_dt(char *buf, int buf_len)
  * format) */
 static uint64_t kvmppc_read_int_cpu_dt(const char *propname)
 {
-char buf[PATH_MAX];
+char buf[PATH_MAX], *tmp;
 union {
 uint32_t v32;
 uint64_t v64;
@@ -1794,10 +1794,10 @@ static uint64_t kvmppc_read_int_cpu_dt(const char 
*propname)
 return -1;
 }
 
-strncat(buf, /, sizeof(buf) - strlen(buf) - 1);
-strncat(buf, propname, sizeof(buf) - strlen(buf) - 1);
+tmp = g_strdup_printf(%s/%s, buf, propname);
 
-f = fopen(buf, rb);
+f = fopen(tmp, rb);
+g_free(buf);
 if (!f) {
 return -1;
 }

For me, it is really a personal taste, so if the maintainer feels the
diff above is OK, I shall send patch v2 for it within 2 days.


Thanks.
-- 
Chen Gang

Open, share, and attitude like air, water, and life which God blessed



Re: [Qemu-devel] [PATCH v2 2/2] Xen: Use the ioreq-server API when available

2014-10-13 Thread Stefano Stabellini
On Mon, 13 Oct 2014, Paul Durrant wrote:
 The ioreq-server API added to Xen 4.5 offers better security than
 the existing Xen/QEMU interface because the shared pages that are
 used to pass emulation request/results back and forth are removed
 from the guest's memory space before any requests are serviced.
 This prevents the guest from mapping these pages (they are in a
 well known location) and attempting to attack QEMU by synthesizing
 its own request structures. Hence, this patch modifies configure
 to detect whether the API is available, and adds the necessary
 code to use the API if it is.
 
 Signed-off-by: Paul Durrant paul.durr...@citrix.com

I think the patch is pretty good, just one comment below.


 Cc: Stefano Stabellini stefano.stabell...@eu.citrix.com
 Cc: Peter Maydell peter.mayd...@linaro.org
 Cc: Paolo Bonzini pbonz...@redhat.com
 Cc: Michael Tokarev m...@tls.msk.ru
 Cc: Stefan Hajnoczi stefa...@redhat.com
 Cc: Stefan Weil s...@weilnetz.de
 Cc: Olaf Hering o...@aepfle.de
 Cc: Gerd Hoffmann kra...@redhat.com
 Cc: Alexey Kardashevskiy a...@ozlabs.ru
 Cc: Alexander Graf ag...@suse.de
 ---
  configure   |   29 ++
  include/hw/xen/xen_common.h |  222 
 +++
  trace-events|8 ++
  xen-hvm.c   |  163 +++
  4 files changed, 403 insertions(+), 19 deletions(-)
 
 diff --git a/configure b/configure
 index 9ac2600..c2db574 100755
 --- a/configure
 +++ b/configure
 @@ -1876,6 +1876,32 @@ int main(void) {
xc_gnttab_open(NULL, 0);
xc_domain_add_to_physmap(0, 0, XENMAPSPACE_gmfn, 0, 0);
xc_hvm_inject_msi(xc, 0, 0xf000, 0x);
 +  xc_hvm_create_ioreq_server(xc, 0, 0, NULL);
 +  return 0;
 +}
 +EOF
 +  compile_prog  $xen_libs
 +then
 +xen_ctrl_version=450
 +xen=yes
 +
 +  elif
 +  cat  $TMPC EOF 
 +#include xenctrl.h
 +#include xenstore.h
 +#include stdint.h
 +#include xen/hvm/hvm_info_table.h
 +#if !defined(HVM_MAX_VCPUS)
 +# error HVM_MAX_VCPUS not defined
 +#endif
 +int main(void) {
 +  xc_interface *xc;
 +  xs_daemon_open();
 +  xc = xc_interface_open(0, 0, 0);
 +  xc_hvm_set_mem_type(0, 0, HVMMEM_ram_ro, 0, 0);
 +  xc_gnttab_open(NULL, 0);
 +  xc_domain_add_to_physmap(0, 0, XENMAPSPACE_gmfn, 0, 0);
 +  xc_hvm_inject_msi(xc, 0, 0xf000, 0x);
return 0;
  }
  EOF
 @@ -4282,6 +4308,9 @@ if test -n $sparc_cpu; then
  echo Target Sparc Arch $sparc_cpu
  fi
  echo xen support   $xen
 +if test $xen = yes ; then
 +  echo xen ctrl version  $xen_ctrl_version
 +fi
  echo brlapi support$brlapi
  echo bluez  support$bluez
  echo Documentation $docs
 diff --git a/include/hw/xen/xen_common.h b/include/hw/xen/xen_common.h
 index 07731b9..7040506 100644
 --- a/include/hw/xen/xen_common.h
 +++ b/include/hw/xen/xen_common.h
 @@ -16,7 +16,9 @@
  
  #include hw/hw.h
  #include hw/xen/xen.h
 +#include hw/pci/pci.h
  #include qemu/queue.h
 +#include trace.h
  
  /*
   * We don't support Xen prior to 3.3.0.
 @@ -164,4 +166,224 @@ void destroy_hvm_domain(bool reboot);
  /* shutdown/destroy current domain because of an error */
  void xen_shutdown_fatal_error(const char *fmt, ...) GCC_FMT_ATTR(1, 2);
  
 +/* Xen before 4.5 */
 +#if CONFIG_XEN_CTRL_INTERFACE_VERSION  450
 +
 +#ifndef HVM_PARAM_BUFIOREQ_EVTCHN
 +#define HVM_PARAM_BUFIOREQ_EVTCHN 26
 +#endif
 +
 +#define IOREQ_TYPE_PCI_CONFIG 2
 +
 +typedef uint32_t ioservid_t;
 +
 +static inline void xen_map_memory_section(XenXC xc, domid_t dom,
 +  ioservid_t ioservid,
 +  MemoryRegionSection *section)
 +{
 +}
 +
 +static inline void xen_unmap_memory_section(XenXC xc, domid_t dom,
 +ioservid_t ioservid,
 +MemoryRegionSection *section)
 +{
 +}
 +
 +static inline void xen_map_io_section(XenXC xc, domid_t dom,
 +  ioservid_t ioservid,
 +  MemoryRegionSection *section)
 +{
 +}
 +
 +static inline void xen_unmap_io_section(XenXC xc, domid_t dom,
 +ioservid_t ioservid,
 +MemoryRegionSection *section)
 +{
 +}
 +
 +static inline void xen_map_pcidev(XenXC xc, domid_t dom,
 +  ioservid_t ioservid,
 +  PCIDevice *pci_dev)
 +{
 +}
 +
 +static inline void xen_unmap_pcidev(XenXC xc, domid_t dom,
 +ioservid_t ioservid,
 +PCIDevice *pci_dev)
 +{
 +}
 +
 +static inline int xen_create_ioreq_server(XenXC xc, domid_t dom,
 +  ioservid_t *ioservid)
 +{
 +return 0;
 +}
 +
 +static inline void xen_destroy_ioreq_server(XenXC xc, domid_t dom,
 +ioservid_t ioservid)
 +{
 +}
 +
 

Re: [Qemu-devel] [PATCH] virtio-pci: fix migration for pci bus master

2014-10-13 Thread Greg Kurz
On Mon, 13 Oct 2014 14:09:54 +0300
Michael S. Tsirkin m...@redhat.com wrote:

 On Mon, Oct 13, 2014 at 10:49:41AM +0200, Greg Kurz wrote:
  On Mon, 6 Oct 2014 20:25:04 +0300
  Michael S. Tsirkin m...@redhat.com wrote:
   [...]
   
   BTW I reverted that patch, and to fix migration, I'm thinking
   about applying the following patch on top of master.
   
  
  Michael,
  
  I could force the migration issue with a rhel65 guest thanks to the
  following patch, applied to hw/virtio/virtio-pci.c in QEMU v2.1.
  
  @@ -271,6 +272,16 @@ static void virtio_ioport_write(void *opaque, uint32_t 
  addr, uint32_t val)
   VirtIODevice *vdev = virtio_bus_get_device(proxy-bus);
   hwaddr pa;
   
  +if ((vdev-status == (VIRTIO_CONFIG_S_ACKNOWLEDGE | 
  VIRTIO_CONFIG_S_DRIVER))
  + (!(proxy-pci_dev.config[PCI_COMMAND]  PCI_COMMAND_MASTER))
  + getenv(MIG_BUG))
  +{
  +fprintf(stderr, \n\n\n\tMIGRATE !\n\n\n);
  +qmp_migrate(getenv(MIG_BUG), false, false, false, false, false,
  +false, NULL);
  +unsetenv(MIG_BUG);
  +}
  +
   switch (addr) {
   case VIRTIO_PCI_GUEST_FEATURES:
   /* Guest does not negotiate properly?  We have to assume nothing. 
  */
  
  
  Indeed, the destination QEMU master hangs because bus master isn't set.
  
   Would appreciate testing cross-version migration (2.1 to master)
   with this patch applied.
   
  
  I had first to to enable the property for pseries of course. I could then
  migrate QEMU v2.1 to QEMU master and back to QEMU v2.1, in the window
  where we have DRIVER enabled and MASTER disabled, without experiencing
  the hang.
  
  Your fix works as expected (just a remark below).
  
   
   
   diff --git a/hw/virtio/virtio-pci.h b/hw/virtio/virtio-pci.h
   index 1cea157..8873b6d 100644
   --- a/hw/virtio/virtio-pci.h
   +++ b/hw/virtio/virtio-pci.h
   @@ -53,6 +53,11 @@ typedef struct VirtioBusClass VirtioPCIBusClass;
#define VIRTIO_PCI_BUS_CLASS(klass) \
OBJECT_CLASS_CHECK(VirtioPCIBusClass, klass, TYPE_VIRTIO_PCI_BUS)
   
   +/* Need to activate work-arounds for buggy guests at vmstate load. */
   +#define VIRTIO_PCI_FLAG_BUS_MASTER_BUG_MIGRATION_BIT  0
   +#define VIRTIO_PCI_FLAG_BUS_MASTER_BUG_MIGRATION \
   +(1  VIRTIO_PCI_FLAG_BUS_MASTER_BUG_MIGRATION_BIT)
   +
/* Performance improves when virtqueue kick processing is decoupled from 
   the
 * vcpu thread using ioeventfd for some devices. */
#define VIRTIO_PCI_FLAG_USE_IOEVENTFD_BIT 1
   diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
   index bae023a..e07b6c4 100644
   --- a/include/hw/i386/pc.h
   +++ b/include/hw/i386/pc.h
   @@ -312,6 +312,11 @@ bool e820_get_entry(int, uint32_t, uint64_t *, 
   uint64_t *);
.driver   = intel-hda,\
.property = old_msi_addr,\
.value= on,\
   +},\
   +{\
   +.driver   = virtio-pci,\
   +.property = virtio-pci-bus-master-bug-migration,\
   +.value= on,\
}
   
#define PC_COMPAT_2_0 \
  
  FWIW, the issue does not occur with intel targets, at least not
  in my test case (booting rhel6 on a virtio-blk disk).
 
 This will reproduce with rhel5 though.
 
  I see bus
  master is set early (bios ?) and never unset...
 
 IIUC if you don't boot from device, it won't be set,
 and will not be set with older guests.
 

Not sure to follow... you mean that if I boot from memory ? Like,
passing -kernel and friends to QEMU ?

  If you decide to apply for intel anyway, shouldn't the enablement be
  in a separate patch ?
  
  Will you resend or would you like me to do it, along with the pseries
  enablement part ? In this case, I would need your SoB for the present
  patch.
  
  Thanks.
  
  --
  Greg
  
   diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c
   index a827cd4..a499a3c 100644
   --- a/hw/virtio/virtio-pci.c
   +++ b/hw/virtio/virtio-pci.c
   @@ -86,9 +86,6 @@
 * 12 is historical, and due to x86 page size. */
#define VIRTIO_PCI_QUEUE_ADDR_SHIFT12
   
   -/* Flags track per-device state like workarounds for quirks in older 
   guests. */
   -#define VIRTIO_PCI_FLAG_BUS_MASTER_BUG  (1  0)
   -
static void virtio_pci_bus_new(VirtioBusState *bus, size_t bus_size,
   VirtIOPCIProxy *dev);
   
   @@ -323,14 +320,6 @@ static void virtio_ioport_write(void *opaque, 
   uint32_t addr, uint32_t val)
 proxy-pci_dev.config[PCI_COMMAND] |
 PCI_COMMAND_MASTER, 1);
}
   -
   -/* Linux before 2.6.34 sets the device as OK without enabling
   -   the PCI device bus master bit. In this case we need to disable
   -   some safety checks. */
   -if ((val  VIRTIO_CONFIG_S_DRIVER_OK) 
   -!(proxy-pci_dev.config[PCI_COMMAND]  PCI_COMMAND_MASTER)) {
   -proxy-flags |= 

Re: [Qemu-devel] [PATCH v2 2/2] Xen: Use the ioreq-server API when available

2014-10-13 Thread Paul Durrant
 -Original Message-
 From: Stefano Stabellini [mailto:stefano.stabell...@eu.citrix.com]
 Sent: 13 October 2014 16:53
 To: Paul Durrant
 Cc: qemu-devel@nongnu.org; xen-de...@lists.xenproject.org; Stefano
 Stabellini; Peter Maydell; Paolo Bonzini; Michael Tokarev; Stefan Hajnoczi;
 Stefan Weil; Olaf Hering; Gerd Hoffmann; Alexey Kardashevskiy; Alexander
 Graf
 Subject: Re: [PATCH v2 2/2] Xen: Use the ioreq-server API when available
 
 On Mon, 13 Oct 2014, Paul Durrant wrote:
  The ioreq-server API added to Xen 4.5 offers better security than
  the existing Xen/QEMU interface because the shared pages that are
  used to pass emulation request/results back and forth are removed
  from the guest's memory space before any requests are serviced.
  This prevents the guest from mapping these pages (they are in a
  well known location) and attempting to attack QEMU by synthesizing
  its own request structures. Hence, this patch modifies configure
  to detect whether the API is available, and adds the necessary
  code to use the API if it is.
 
  Signed-off-by: Paul Durrant paul.durr...@citrix.com
 
 I think the patch is pretty good, just one comment below.
 
[snip]
  @@ -487,9 +494,52 @@ static void xen_region_del(MemoryListener
 *listener,
  MemoryRegionSection *section)
   {
   xen_set_memory(listener, section, false);
  +
  +if (section-mr != ram_memory) {
  +XenIOState *state = container_of(listener, XenIOState,
 memory_listener);
  +
  +xen_unmap_memory_section(xen_xc, xen_domid, state-ioservid,
 section);
  +}
  +
   memory_region_unref(section-mr);
   }
 
 I would prefer if you could move the xen_unmap_memory_section and
 xen_map_memory_section calls to xen_set_memory, where we already
 have a
 ram_memory check. Could you reuse it?
 

Sure, I can do that.

  Paul



Re: [Qemu-devel] [PATCH v3 4/5] target-tricore: Add instructions of BIT opcode format

2014-10-13 Thread Richard Henderson
On 10/13/2014 09:27 AM, Bastian Koppelmann wrote:
 Add instructions of BIT opcode format.
 Add microcode generator functions gen_bit_1/2op to do 1/2 bit operations on 
 the last bit.
 
 Signed-off-by: Bastian Koppelmann kbast...@mail.uni-paderborn.de
 ---
 v2 - v3:
 - OR_NOR_T, AND_NOR_T: Now uses normal conditionals instead of 
 preprocessor.
 
  target-tricore/translate.c | 312 
 +
  1 file changed, 312 insertions(+)

Reviewed-by: Richard Henderson r...@twiddle.net


r~



Re: [Qemu-devel] [PATCH v3 5/5] target-tricore: Add instructions of BO opcode format

2014-10-13 Thread Richard Henderson
On 10/13/2014 09:27 AM, Bastian Koppelmann wrote:
 Add instructions of BO opcode format.
 Add microcode generator functions gen_swap, gen_ldmst.
 Add microcode generator functions gen_st/ld_preincr, which write back the 
 address after the memory access.
 Add helper for circular and bit reverse addr mode calculation.
 Add sign extended bitmask for BO_OFF10 field.
 
 Signed-off-by: Bastian Koppelmann kbast...@mail.uni-paderborn.de
 ---
 v2 - v3:
 - Add microcode generator functions gen_st/ld_preincr, which write back 
 the
   address after the memory access.
 - ST/LD_PREINC insn now use gen_st/ld_preincr or write back the address 
 after
   after the memory access.

Reviewed-by: Richard Henderson r...@twiddle.net


r~



Re: [Qemu-devel] [PATCH v5 0/5] add description field in ObjectProperty and PropertyInfo struct

2014-10-13 Thread Andreas Färber
Am 13.10.2014 um 15:40 schrieb Andreas Färber:
 Already queued, not yet pushed. On my way to KVM Forum...

Fixed up the preceding series and pushed this one now:
https://github.com/afaerber/qemu-cpu/commits/qom-next

Thanks,
Andreas

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



Re: [Qemu-devel] [PATCH v2 00/36] complete conversion to hotplug-handler API

2014-10-13 Thread Andreas Färber
Hi,

All 37 patches should be applied now, in their latest version, with
Reviewed-bys and my Sob...
https://github.com/afaerber/qemu-cpu/commits/qom-next

This is a series size that I would ask to split in the future.

Thanks,
Andreas

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



Re: [Qemu-devel] [EXTERNAL] Re: how to dynamically add a block device using qmp?

2014-10-13 Thread Ken Chiang
I also tried to add a virt io device to the frontend but still no disc in the 
VM.  Am I missing anything else?


{QMP: {version: {qemu: {micro: 0, minor: 0, major: 2}, package:  
(Debian 2.0.0+dfsg-2ubuntu1.2)}, capabilities: []}}
{execute:qmp_capabilities}
{return: {}}
{ execute: blockdev-add,
   arguments: { options : {
id: tc1,
driver: qcow2,
file: { driver: file,
filename: test.qc2 } } } }

{return: {}}
{ execute: device_add, arguments: {
driver: virtio-blk-pci, id: vda, drive: tc1  } }
{return: {}}

Thanks!

Ken



---BeginMessage---
Markus,

Nevermind my previous question, I see now that you add a scsi bus prior to 
adding the scsi disk.  

However, I did this and qmp returned {[]} in both cases and I am still not 
seeing the disk in the vm.


here's the qmp session:

{execute:qmp_capabilities}
{return: {}}
{ execute: blockdev-add,
   arguments: { options : { 
id: tc1,
driver: qcow2,  
file: { driver: file, 
filename: /home/kchiang/kvmimages/xpsp2_b-0012.qc2 } } } } 
{return: {}}
{ execute: device_add, arguments: {
driver: virtio-scsi-pci, id: vs-hba } }
{return: {}}
{ execute: device_add, arguments: {
driver: scsi-disk, id: sdb, drive: tc1,
bus: vs-hba.0  } }
{return: {}}
{ execute: query-block }
{return: [{io-status: ok, device: ide0-hd0, locked: false, 
removable: false, inserted: {iops_rd: 0, image: {virtual-size: 
551872, filename: kvmimages/vts-6g-u12-inetsim.image, format: raw, 
actual-size: 555968, dirty-flag: false}, iops_wr: 0, ro: false, 
backing_file_depth: 0, drv: raw, iops: 0, bps_wr: 0, encrypted: 
false, bps: 0, bps_rd: 0, file: kvmimages/vts-6g-u12-inetsim.image, 
encryption_key_missing: false}, type: unknown}, {io-status: ok, 
device: ide1-cd0, locked: false, removable: true, tray_open: false, 
type: unknown}, {device: floppy0, locked: false, removable: true, 
tray_open: false, type: unknown}, {device: sd0, locked: false, 
removable: true, tray_open: false, type: unknown}, {io-status: ok, 
device: tc1, locked: false, removable: false, inserted: {iops_rd: 
0, image: {virtual-size: 5368709120, filename: 
/home/kchiang/kvmimages/xpsp2_b-0012.qc2, cluster-size: 65536, 
format: qcow2, actual-size: 1550454784, format-specific: {type: 
qcow2, data: {compat: 0.10}}, dirty-flag: false}, iops_wr: 0, ro: 
false, backing_file_depth: 0, drv: qcow2, iops: 0, bps_wr: 0, 
encrypted: false, bps: 0, bps_rd: 0, file: 
/home/kchiang/kvmimages/xpsp2_b-0012.qc2, encryption_key_missing: 
false}, type: unknown}]}

am I missing anything else?

I've also tried rebooting the VM and running rescan-scsi-bus to no avail.

Ken

On Sat, Oct 11, 2014 at 10:58:59AM +0200, Markus Armbruster wrote:
 Ken Chiang kchi...@sandia.gov writes:
 
  Hello,
 
  I am trying to add a block device dynamically using qmp and are having
  some issues.
 
  After successfully adding the block device using blockdev-add and
  verifying that it has been added using query-block, I am unable to
  see the block device in the VM under /dev/sdXX
 
 Block devices consist of a frontend (a.k.a. device model) and a backend.
 You added only a backend.  To add the frontend, use device_add.
 
  I am using ubuntu14.04LTS: qmp version: 
  {QMP: {version: {qemu: {micro: 0, minor: 0, major: 2}, 
  package:  (Debian 2.0.0+dfsg-2ubuntu1.2)}, capabilities: []}}
 
  Here's what I am doing:
  1.  /usr/bin/kvm-spice -hda kvmimages/ubuntu.image -m 768 -usbdevice tablet 
  -vnc :5 -qmp tcp:localhost:,server,nowait
 
  2.  telnet localhost 
 
  3.  In the telnet session run:
  {execute:qmp_capabilities}
  {return: {}}
  { execute: blockdev-add,
 arguments: { options : {
  id: disk1,
  driver: qcow2,
  file: { driver: file,
  filename: testvm.qc2 } } } }
  {return: {}}
 
  { execute: query-block }
 
  4. query-block returns the device disk1, but when I check the vm:
 # vncviewer :5
 
  there are no new devices.
 
 For a virtio-blk disk, try:
 
 { execute: device_add, arguments: {
 driver: virtio-blk-pci, id: vda, drive: disk1  } }
 
 For a SCSI disk, try:
 
 { execute: device_add, arguments: {
 driver: virtio-scsi-pci, id: vs-hba } }
 { execute: device_add, arguments: {
 driver: scsi-disk, id: sda, drive: disk1,
 bus: vs-hba.0  } }
 
 [...]

-- 
Ken Chiang
Information Assurance
Sandia National Labs - CA
925-294-2018
---End Message---


Re: [Qemu-devel] [question] is it possible that big-endian l1tableoffsetreferencedby other I/O while updating l1 table offset inqcow2_update_snapshot_refcount?

2014-10-13 Thread Zhang Haoyu
 Hi,
 I encounter a problem that after deleting snapshot, the qcow2 image 
 size is very larger than that it should be displayed by ls command,
 but the virtual disk size is okay via qemu-img info.
 I suspect that during updating l1 table offset, other I/O job 
 reference the big-endian l1 table offset (very large value),
 so the file is truncated to very large.
 Not quite.  Rather, all the data that the snapshot used to occupy is
 still consuming holes in the file; the maximum offset of the file is
 still unchanged, even if the file is no longer using as many 
 referenced
 clusters.  Recent changes have gone in to sparsify the file when
 possible (punching holes if your kernel and file system is new enough 
 to
 support that), so that it is not consuming the amount of disk space 
 that
 a mere ls reports.  But if what you are asking for is a way to compact
 the file back down, then you'll need to submit a patch.  The idea of
 having an online defragmenter for qcow2 files has been kicked around
 before, but it is complex enough that no one has attempted a patch 
 yet.
 Sorry, I didn't clarify the problem clearly.
 In qcow2_update_snapshot_refcount(), below code,
 /* Update L1 only if it isn't deleted anyway (addend = -1) */
 if (ret == 0  addend = 0  l1_modified) {
 for (i = 0; i  l1_size; i++) {
 cpu_to_be64s(l1_table[i]);
 }

 ret = bdrv_pwrite_sync(bs-file, l1_table_offset, 
 l1_table, l1_size2);

 for (i = 0; i  l1_size; i++) {
 be64_to_cpus(l1_table[i]);
 }
 }
 between cpu_to_be64s(l1_table[i]); and be64_to_cpus(l1_table[i]);,
 is it possible that there is other I/O reference this interim l1 table 
 whose entries contain the be64 l2 table offset?
 The be64 l2 table offset maybe a very large value, hundreds of TB is 
 possible,
 then the qcow2 file will be truncated to far larger than normal size.
 So we'll see the huge size of the qcow2 file by ls -hl, but the size 
 is still normal displayed by qemu-img info.

 If the possibility mentioned above exists, below raw code may fix it,
  if (ret == 0  addend = 0  l1_modified) {
 tmp_l1_table = g_malloc0(l1_size * sizeof(uint64_t))
 memcpy(tmp_l1_table, l1_table, l1_size * sizeof(uint64_t));
 for (i = 0; i  l1_size; i++) {
 cpu_to_be64s(tmp_l1_table[i]);
 }
 ret = bdrv_pwrite_sync(bs-file, l1_table_offset, 
 tmp_l1_table, l1_size2);

 free(tmp_l1_table);
 }
 l1_table is already a local variable (local to
 qcow2_update_snapshot_refcount()), so I can't really imagine how
 introducing another local buffer should mitigate the problem, if there
 is any.

 l1_table is not necessarily a local variable to 
 qcow2_update_snapshot_refcount,
 which depends on condition of if (l1_table_offset != 
 s-l1_table_offset),
 if the condition not true, l1_table = s-l1_table.
 Oh, yes, you're right. Okay, so in theory nothing should happen anyway,
 because qcow2 does not have to be reentrant (so s-l1_table will not be
 accessed while it's big endian and therefore possibly not in CPU order).
 Could you detail how qcow2 does not have to be reentrant?
 In below stack,
 qcow2_update_snapshot_refcount
 |- cpu_to_be64s(l1_table[i])
 |- bdrv_pwrite_sync
 This is executed on bs-file, not the qcow2 BDS.

 Yes, bs-file is passed to bdrv_pwrite_sync here,
 but aio_poll(aio_context) will poll all BDS's aio, not only that of 
 bs-file, doesn't it?
 Is it possible that there are pending aio which belong to this qcow2 BDS 
 still exist?

qcow2 is generally not reentrant, this is secured by locking 
(BDRVQcowState.lock). As long as one request for a BDS is still running, 
it will not be interrupted.

This problem can be reproduced with loop of 
savevm - delvm - savevm - delvm ..., for about half-hour,
but after applying above change of using local variable to sync l1_table,
this problem has not been occurred for more than 48 hours with loop of
savevm - delvm - savevm - delvm ...
Could you help analysing this problem, please?

And, because bdrv_co_do_rw is running in a coroutine context, not the other 
thread,
both bdrv_co_do_rw and qcow2_update_snapshot_refcount are performed in the same 
thread (main-thread),
how does BDRVQcowState.lock avoid the reentrant?

Thanks,
Zhang Haoyu
Max

 Thanks,
 Zhang Haoyu
 Max

 |-- bdrv_pwrite
 |--- bdrv_pwritev
 | bdrv_prwv_co
 |- aio_poll(aio_context) == this aio_context is qemu_aio_context
 |-- aio_dispatch
 |--- bdrv_co_io_em_complete
 | qemu_coroutine_enter(co-coroutine, NULL); == coroutine entry 
 is bdrv_co_do_rw
 bdrv_co_do_rw will access l1_table to perform I/O operation.

 Thanks,
 Zhang Haoyu
 But I find it rather ugly to convert the cached L1 table to big endian,
 so I'd be fine with the patch you proposed.

 Max




[Qemu-devel] [PATCH] hw/arm/virt: Replace memory_region_init_ram with memory_region_allocate_system_memory

2014-10-13 Thread zhuyijun
From: Zhu Yijun zhuyi...@huawei.com

Commit 0b183fc871:memory: move mem_path handling to
memory_region_allocate_system_memory split memory_region_init_ram and
memory_region_init_ram_from_file. Also it moved mem-path handling a step
up from memory_region_init_ram to memory_region_allocate_system_memory.

Therefore for any board that uses memory_region_init_ram directly,
-mem-path is not supported.

Commit e938ba0c35:ppc: memory: Replace memory_region_init_ram with
memory_region_allocate_system_memory have already fixed this on ppc.

arm/arm64 board also occurs, this patch is only for arm64 board(virt).

Signed-off-by: Zhu Yijun zhuyi...@huawei.com
---
 hw/arm/virt.c |5 ++---
 1 files changed, 2 insertions(+), 3 deletions(-)

diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index 8c6b171..32646a1 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -580,9 +580,8 @@ static void machvirt_init(MachineState *machine)
 fdt_add_cpu_nodes(vbi);
 fdt_add_psci_node(vbi);
 
-memory_region_init_ram(ram, NULL, mach-virt.ram, machine-ram_size,
-   error_abort);
-vmstate_register_ram_global(ram);
+memory_region_allocate_system_memory(ram, NULL, mach-virt.ram,
+ machine-ram_size);
 memory_region_add_subregion(sysmem, vbi-memmap[VIRT_MEM].base, ram);
 
 create_flash(vbi);
-- 
1.7.1





Re: [Qemu-devel] [PATCH] hw/arm/virt: Replace memory_region_init_ram with memory_region_allocate_system_memory

2014-10-13 Thread Peter Maydell
On 14 October 2014 04:36, zhuyijun zhuyi...@huawei.com wrote:
 From: Zhu Yijun zhuyi...@huawei.com

 Commit 0b183fc871:memory: move mem_path handling to
 memory_region_allocate_system_memory split memory_region_init_ram and
 memory_region_init_ram_from_file. Also it moved mem-path handling a step
 up from memory_region_init_ram to memory_region_allocate_system_memory.

 Therefore for any board that uses memory_region_init_ram directly,
 -mem-path is not supported.

 Commit e938ba0c35:ppc: memory: Replace memory_region_init_ram with
 memory_region_allocate_system_memory have already fixed this on ppc.

 arm/arm64 board also occurs, this patch is only for arm64 board(virt).

Why is this patch only changing this board? What's special
about virt that means we don't want to also make this
change for the other ARM boards? What about all the other
boards for the other architectures?

The commit message for 0b183fc871 suggests we decided to
just break -mem-path for all those boards, which seems an
odd thing to do... Paolo?

Incidentally I can't see anything that guards against
calling memory_region_allocate_system_memory() twice,
so I think you would end up with two blocks of RAM
both backed by the same file then. Or have I misread
the code?

-- PMM



Re: [Qemu-devel] [PATCH] hw/arm/virt: Replace memory_region_init_ram with memory_region_allocate_system_memory

2014-10-13 Thread Paolo Bonzini
Il 14/10/2014 06:54, Peter Maydell ha scritto:
 Why is this patch only changing this board? What's special
 about virt that means we don't want to also make this
 change for the other ARM boards? What about all the other
 boards for the other architectures?

-mem-path is not too useful without KVM (TCG is too slow to
notice the difference.  PPC and S390 have already been fixed.

For other boards, I have patches but I had no time to submit
them until now.

 The commit message for 0b183fc871 suggests we decided to
 just break -mem-path for all those boards, which seems an
 odd thing to do... Paolo?

The plan _was_ in fact to fix it up. :(

 Incidentally I can't see anything that guards against
 calling memory_region_allocate_system_memory() twice,
 so I think you would end up with two blocks of RAM
 both backed by the same file then. Or have I misread
 the code?

That would be a bug in the board, it is caught here:

if (memory_region_is_mapped(seg)) {
char *path = object_get_canonical_path_component(OBJECT(backend));
error_report(memory backend %s is used multiple times. Each 
 -numa option must use a different memdev value.,
 path);
exit(1);
}

The error message gives the common user error rather than
the less common developer error, but still you catch it.

Paolo



Re: [Qemu-devel] [PATCH] hw/arm/virt: Replace memory_region_init_ram with memory_region_allocate_system_memory

2014-10-13 Thread Peter Maydell
On 14 October 2014 07:10, Paolo Bonzini pbonz...@redhat.com wrote:
 Il 14/10/2014 06:54, Peter Maydell ha scritto:
 Why is this patch only changing this board? What's special
 about virt that means we don't want to also make this
 change for the other ARM boards? What about all the other
 boards for the other architectures?

 -mem-path is not too useful without KVM (TCG is too slow to
 notice the difference.  PPC and S390 have already been fixed.

MIPS has KVM too now...

 For other boards, I have patches but I had no time to submit
 them until now.

 The commit message for 0b183fc871 suggests we decided to
 just break -mem-path for all those boards, which seems an
 odd thing to do... Paolo?

 The plan _was_ in fact to fix it up. :(

 Incidentally I can't see anything that guards against
 calling memory_region_allocate_system_memory() twice,
 so I think you would end up with two blocks of RAM
 both backed by the same file then. Or have I misread
 the code?

 That would be a bug in the board, it is caught here:

 if (memory_region_is_mapped(seg)) {
 char *path = object_get_canonical_path_component(OBJECT(backend));
 error_report(memory backend %s is used multiple times. Each 
  -numa option must use a different memdev value.,
  path);
 exit(1);
 }

 The error message gives the common user error rather than
 the less common developer error, but still you catch it.

That's only in the NUMA code path though, isn't it?
I was looking at the non-numa codepath, which is what
all the boards I care about are going to be taking :-)

What's the right thing for a board where the system
RAM isn't contiguous, by the way?

thanks
-- PMM