Re: [Qemu-devel] 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 tableoffsetreferenced by other I/O while updating l1 table offset in qcow2_update_snapshot_refcount?
>> 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?
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
>>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" À: "Alexandre DERUMIER" Cc: "Ceph Devel" , "qemu-devel" 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?
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
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 > > 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
On Mon, 6 Oct 2014 20:25:04 +0300
"Michael S. Tsirkin" 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[PC
Re: [Qemu-devel] [PATCH] virtio-pci: fix migration for pci bus master
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" 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_v
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?
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
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
On Mon, 13 Oct 2014 12:01:07 +0300
"Michael S. Tsirkin" 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" 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
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
Re: [Qemu-devel] [PATCH RFC 08/11] virtio_blk: use virtio v1.0 endian
On Mon, 13 Oct 2014 16:28:32 +1030 Rusty Russell wrote: > Cornelia Huck writes: > > Note that we care only about the fields still in use for virtio v1.0. > > > > Reviewed-by: Thomas Huth > > Reviewed-by: David Hildenbrand > > Signed-off-by: Cornelia Huck > > 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
On Mon, 13 Oct 2014 16:23:58 +1030 Rusty Russell wrote: > Cornelia Huck 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
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 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=link 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=child 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=link 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=child 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
[Qemu-devel] [Bug 921208] Re: win7/x64 installer hangs on startup with 0x0000005d.
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
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" 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
Re: [Qemu-devel] [PATCH v3 0/2] monitor: add peripheral device del completion support
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
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
On 13.10.14 12:42, Greg Kurz wrote:
> On Mon, 13 Oct 2014 12:01:07 +0300
> "Michael S. Tsirkin" 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" 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
On 10 October 2014 18:03, Greg Bellows wrote: > From: Fabian Aggeler > > Increasing banked_r13 and banked_r14 to store LR_mon and SP_mon (bank > index 7). > > Signed-off-by: Fabian Aggeler > Signed-off-by: Greg Bellows Reviewed-by: Peter Maydell thanks -- PMM
Re: [Qemu-devel] [PATCH] virtio-pci: fix migration for pci bus master
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" 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" 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
On 10 October 2014 18:03, Greg Bellows wrote: > From: Fabian Aggeler > > 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 > Signed-off-by: Fabian Aggeler > Signed-off-by: Greg Bellows > > == > > 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
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
Cc: Stefano Stabellini
Cc: Peter Maydell
Cc: Paolo Bonzini
Cc: Michael Tokarev
Cc: Stefan Hajnoczi
Cc: Stefan Weil
Cc: Olaf Hering
Cc: Gerd Hoffmann
Cc: Alexey Kardashevskiy
Cc: Alexander Graf
---
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 <
+#include
+#include
+#include
+#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 *bufioreq_pfn,
+evtchn_port_t *bufioreq_evtchn)
+{
+unsigned long param;
+int rc;
+
+rc = xc_get_hvm_param(xc, dom, HVM_PARAM_IOREQ_PFN, ¶m);
+if (rc < 0) {
+fprintf(stde
[Qemu-devel] [PATCH v2 0/2] Xen: Use ioreq-server API
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
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
Cc: Michael S. Tsirkin
Cc: Paolo Bonzini
Cc: Andreas Faerber
Cc: Thomas Huth
Cc: Peter Crosthwaite
Cc: Christian Borntraeger
---
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
+++ b/include/qemu/typedefs.h
@@ -54,6 +54,7 @@ typedef struct PCIHostState PCIHostState;
typedef struct PCIExpressHost PCIExpressHost;
ty
Re: [Qemu-devel] [PATCH v6 03/32] target-arm: reject switching to monitor mode
On 10 October 2014 18:03, Greg Bellows wrote: > From: Sergey Fedorov > > Reject switching to monitor mode from non-secure state. > > Signed-off-by: Sergey Fedorov > Signed-off-by: Fabian Aggeler > Reviewed-by: Edgar E. Iglesias > Signed-off-by: Greg Bellows > --- > target-arm/helper.c | 2 ++ > 1 file changed, 2 insertions(+) Reviewed-by: Peter Maydell thanks -- PMM
Re: [Qemu-devel] [PATCH 1/1] pci-host: add educational driver
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
> ---
> 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
> +S: Maintained
> +F: hw/pci-host/edu.c
> +
> IDE
> M: Kevin Wolf
> M: Stefan Hajnoczi
> 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 ds
Re: [Qemu-devel] [PATCH v6 04/32] target-arm: rename arm_current_pl to arm_current_el
On 10 October 2014 18:03, Greg Bellows 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 > > == > > 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 thanks -- PMM
Re: [Qemu-devel] [PATCH 1/1] pci-host: add educational driver
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
On 10 October 2014 18:03, Greg Bellows wrote:
> From: Fabian Aggeler
>
> 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
> Signed-off-by: Fabian Aggeler
> Signed-off-by: Greg Bellows
>
> ==
>
> 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_, 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
On 10 October 2014 18:03, Greg Bellows wrote:
> From: Fabian Aggeler
>
> 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
> Signed-off-by: Greg Bellows
>
> ==
>
> 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
thanks
-- PMM
Re: [Qemu-devel] [PATCH v6 06/32] target-arm: A32: Emulate the SMC instruction
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 wrote:
> On 10 October 2014 18:03, Greg Bellows wrote:
> > From: Fabian Aggeler
> >
> > 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
> > Signed-off-by: Fabian Aggeler
> > Signed-off-by: Greg Bellows
> >
> > ==
> >
> > 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_, 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.
Re: [Qemu-devel] [PATCH v6 06/32] target-arm: A32: Emulate the SMC instruction
On 13 October 2014 15:13, Greg Bellows 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
I have just few minor comments.
Reviewed-by: Yongbok Kim
regards,
Yongbok
On 08/10/2014 11:55, Leon Alrae wrote:
Signed-off-by: Leon Alrae
---
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-
Re: [Qemu-devel] [PATCH v5 0/5] add description field in ObjectProperty and PropertyInfo struct
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 > > 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=link > 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=child > 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=link > 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=child > 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 f
Re: [Qemu-devel] [PATCH v6 06/32] target-arm: A32: Emulate the SMC instruction
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 wrote: > On 13 October 2014 15:13, Greg Bellows 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()
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
---
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()
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
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
On 10.10.14 16:47, Peter Maydell wrote:
> On 10 October 2014 11:59, Antony Pavlov 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
>
> 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
On 10/12/2014 01:50 AM, Peter Maydell wrote: > On 12 October 2014 01:32, Chen Gang 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
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
On 13 October 2014 16:59, Eric Blake 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
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
Reviewed-by: Richard Henderson
---
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) + (e
[Qemu-devel] [PATCH v3 2/5] target-tricore: Add instructions of ABS, ABSB opcode format
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
Reviewed-by: Richard Henderson
---
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);
+
+tcg_temp_free(temp);
+tcg_temp_
[Qemu-devel] [PATCH v3 0/5] Add TriCore ABS, ABSB, B, BIT, BO instructions
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
Add instructions of B opcode format.
Signed-off-by: Bastian Koppelmann
Reviewed-by: Richard Henderson
---
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
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
---
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
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
---
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);
+
+switc
Re: [Qemu-devel] [PATCH] target-ppc: kvm: Fix memory overflow issue about strncat()
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
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
I think the patch is pretty good, just one comment below.
> Cc: Stefano Stabellini
> Cc: Peter Maydell
> Cc: Paolo Bonzini
> Cc: Michael Tokarev
> Cc: Stefan Hajnoczi
> Cc: Stefan Weil
> Cc: Olaf Hering
> Cc: Gerd Hoffmann
> Cc: Alexey Kardashevskiy
> Cc: Alexander Graf
> ---
> 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 < +#include
> +#include
> +#include
> +#include
> +#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,
> +
Re: [Qemu-devel] [PATCH] virtio-pci: fix migration for pci bus master
On Mon, 13 Oct 2014 14:09:54 +0300
"Michael S. Tsirkin" 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" 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
Re: [Qemu-devel] [PATCH v2 2/2] Xen: Use the ioreq-server API when available
> -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
>
> 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
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 > --- > 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~
Re: [Qemu-devel] [PATCH v3 5/5] target-tricore: Add instructions of BO opcode format
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 > --- > 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~
Re: [Qemu-devel] [PATCH v5 0/5] add description field in ObjectProperty and PropertyInfo struct
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
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?
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
--- Begin Message ---
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 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
Re: [Qemu-devel] [question] is it possible that big-endian l1tableoffsetreferencedby other I/O while updating l1 table offset inqcow2_update_snapshot_refcount?
>> 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 re
[Qemu-devel] [PATCH] hw/arm/virt: Replace memory_region_init_ram with memory_region_allocate_system_memory
From: Zhu Yijun 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 --- 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
On 14 October 2014 04:36, zhuyijun wrote: > From: Zhu Yijun > > 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
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
On 14 October 2014 07:10, Paolo Bonzini 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
Re: [Qemu-devel] [PATCH] hw/arm/virt: Replace memory_region_init_ram with memory_region_allocate_system_memory
Hi Peter, On 2014/10/14 12:54, Peter Maydell wrote: > On 14 October 2014 04:36, zhuyijun wrote: >> From: Zhu Yijun >> >> 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? > I think all of the other ARM boards should be changed. If changes in virt is ok, I will make the patch again including the modification of other ARM boards. Best regards, Yijun > 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] qxl: keep going if reaching guest bug on empty area
On Fr, 2014-10-10 at 19:05 +0200, Marc-André Lureau wrote: > a different patch series for the driver. However, it is simple enough > to keep qemu running on empty areas update, which is what this patch > provides. added to spice patch queue. thanks, Gerd
Re: [Qemu-devel] [PATCH] hw/display/vga: Remove unused arrays dmask4 and dmask16
On Fr, 2014-10-10 at 20:44 +0100, Peter Maydell wrote: > Following cleanup of the vga device code in commit d2e043a8041, > the arrays dmask4 and dmask16 are now unused. gcc doesn't warn > about this, but clang does; remove them. Queued up. thanks, Gerd
