Re: [Qemu-devel] [question] Will qemu-2.0.0 be a longterm stablebranch?
>> Hi, all >> >> Which version is best for commercial product, qemu-2.0.0 or other versions? >> Any advices? >> >> Thanks, >> Zhang Haoyu > >Use one of the downstreams: Red Hat, Fedora, Debian all ship QEMU and >have active QEMU maintainers developing the distribution. > How to get the latest qemu distribution shipped by RedHat? Thanks, Zhang Haoyu >-- >MST
Re: [Qemu-devel] [PATCH] migration: static variables will not be reset at second migration
> -Original Message-
> From: Juan Quintela [mailto:quint...@redhat.com]
> Sent: Friday, March 21, 2014 9:26 PM
> To: Gonglei (Arei)
> Cc: qemu-devel@nongnu.org; owass...@redhat.com; pbonz...@redhat.com;
> ebl...@redhat.com; dgilb...@redhat.com; chenliang (T)
> Subject: Re: [PATCH] migration: static variables will not be reset at second
> migration
>
> wrote:
> > From: ChenLiang
> >
> > The static variables in migration_bitmap_sync will not be reset in
> > the case of a second attempted migration.
> >
> > Signed-off-by: ChenLiang
> > Signed-off-by: Gonglei
>
> Good catch. Applied..
>
Hi, Juan? Ping... please :)
Best regards,
-Gonglei
> > ---
> > arch_init.c | 15 ---
> > 1 file changed, 12 insertions(+), 3 deletions(-)
> >
> > diff --git a/arch_init.c b/arch_init.c
> > index 60c975d..10516cb 100644
> > --- a/arch_init.c
> > +++ b/arch_init.c
> > @@ -468,15 +468,23 @@ static void
> migration_bitmap_sync_range(ram_addr_t start, ram_addr_t length)
> >
> >
> > /* Needs iothread lock! */
> > +/* Fix me: there are too many global variables used in migration process.
> > */
> > +static int64_t start_time;
> > +static int64_t bytes_xfer_prev;
> > +static int64_t num_dirty_pages_period;
> > +
> > +static void migration_bitmap_sync_init(void)
> > +{
> > +start_time = 0;
> > +bytes_xfer_prev = 0;
> > +num_dirty_pages_period = 0;
> > +}
> >
> > static void migration_bitmap_sync(void)
> > {
> > RAMBlock *block;
> > uint64_t num_dirty_pages_init = migration_dirty_pages;
> > MigrationState *s = migrate_get_current();
> > -static int64_t start_time;
> > -static int64_t bytes_xfer_prev;
> > -static int64_t num_dirty_pages_period;
> > int64_t end_time;
> > int64_t bytes_xfer_now;
> >
> > @@ -733,6 +741,7 @@ static int ram_save_setup(QEMUFile *f, void
> *opaque)
> > migration_dirty_pages = ram_pages;
> > mig_throttle_on = false;
> > dirty_rate_high_cnt = 0;
> > +migration_bitmap_sync_init();
> >
> > if (migrate_use_xbzrle()) {
> > qemu_mutex_lock_iothread();
Re: [Qemu-devel] [PATCH 3/7] spapr: Refactor spapr_populate_memory()
On 06/24/2014 01:08 PM, Nishanth Aravamudan wrote:
> On 21.06.2014 [13:06:53 +1000], Alexey Kardashevskiy wrote:
>> On 06/21/2014 08:55 AM, Nishanth Aravamudan wrote:
>>> On 16.06.2014 [17:53:49 +1000], Alexey Kardashevskiy wrote:
Current QEMU does not support memoryless NUMA nodes.
This prepares SPAPR for that.
This moves 2 calls of spapr_populate_memory_node() into
the existing loop which handles nodes other than than
the first one.
Signed-off-by: Alexey Kardashevskiy
---
hw/ppc/spapr.c | 31 +++
1 file changed, 11 insertions(+), 20 deletions(-)
diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index cb3a10a..666b676 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -689,28 +689,13 @@ static void spapr_populate_memory_node(void *fdt,
int nodeid, hwaddr start,
static int spapr_populate_memory(sPAPREnvironment *spapr, void *fdt)
{
-hwaddr node0_size, mem_start, node_size;
+hwaddr mem_start, node_size;
int i;
-/* memory node(s) */
-if (nb_numa_nodes > 1 && node_mem[0] < ram_size) {
-node0_size = node_mem[0];
-} else {
-node0_size = ram_size;
-}
-
-/* RMA */
-spapr_populate_memory_node(fdt, 0, 0, spapr->rma_size);
-
-/* RAM: Node 0 */
-if (node0_size > spapr->rma_size) {
-spapr_populate_memory_node(fdt, 0, spapr->rma_size,
- node0_size - spapr->rma_size);
-}
-
-/* RAM: Node 1 and beyond */
-mem_start = node0_size;
-for (i = 1; i < nb_numa_nodes; i++) {
+for (i = 0, mem_start = 0; i < nb_numa_nodes; ++i) {
+if (!node_mem[i]) {
+continue;
+}
>>>
>>> Doesn't this skip memoryless nodes? What actually puts the memoryless
>>> node in the device-tree?
>>
>> It does skip.
>>
>>> And if you were to put them in, wouldn't spapr_populate_memory_node()
>>> fail because we'd be creating two nodes with memory@XXX where XXX is the
>>> same (starting address) for both?
>>
>> I cannot do this now - there is no way to tell from the command line where
>> I want NUMA node memory start from so I'll end up with multiple nodes with
>> the same name and QEMU won't start. When NUMA fixes reach upstream, I'll
>> try to work out something on top of that.
>
> Ah I got something here. With the patches I just sent to enable sparse
> NUMA nodes, plus your series rebased on top, here's what I see in a
> Linux LPAR:
>
> qemu-system-ppc64 -machine pseries,accel=kvm,usb=off -m 4096 -realtime
> mlock=off -numa node,nodeid=3,mem=4096,cpus=2-3 -numa
> node,nodeid=2,mem=0,cpus=0-1 -smp 4
>
> info numa
> 2 nodes
> node 2 cpus: 0 1
> node 2 size: 0 MB
> node 3 cpus: 2 3
> node 3 size: 4096 MB
>
> numactl --hardware
> available: 3 nodes (0,2-3)
> node 0 cpus:
> node 0 size: 0 MB
> node 0 free: 0 MB
> node 2 cpus: 0 1
> node 2 size: 0 MB
> node 2 free: 0 MB
> node 3 cpus: 2 3
> node 3 size: 4073 MB
> node 3 free: 3830 MB
> node distances:
> node 0 2 3
> 0: 10 40 40
> 2: 40 10 40
> 3: 40 40 10
>
> The trick, it seems, is if you have a memoryless node, it needs to
> have CPUs assigned to it.
Yep. The device tree does not have NUMA nodes, it only has CPUs and
memory@xxx (memory banks?) and the guest kernel has to parse
ibm,associativity and reconstruct the NUMA topology. If some node is not
mentioned in any ibm,associativity, it does not exist.
> The CPU's "ibm,associativity" property will
> make Linux set up the proper NUMA topology.
>
> Thoughts? Should there be a check that every "present" NUMA node at
> least either has CPUs or memory.
May be, I'll wait for NUMA stuff in upstream, apply your patch(es), my
patches and see what I get :)
> It seems like if neither are present,
> we can just hotplug them later?
hotplug what? NUMA nodes?
> Does qemu support topology for PCI devices?
Nope.
--
Alexey
Re: [Qemu-devel] [PATCH 3/7] spapr: Refactor spapr_populate_memory()
On 06/24/2014 03:40 AM, Nishanth Aravamudan wrote:
> On 21.06.2014 [13:06:53 +1000], Alexey Kardashevskiy wrote:
>> On 06/21/2014 08:55 AM, Nishanth Aravamudan wrote:
>>> On 16.06.2014 [17:53:49 +1000], Alexey Kardashevskiy wrote:
Current QEMU does not support memoryless NUMA nodes.
This prepares SPAPR for that.
This moves 2 calls of spapr_populate_memory_node() into
the existing loop which handles nodes other than than
the first one.
Signed-off-by: Alexey Kardashevskiy
---
hw/ppc/spapr.c | 31 +++
1 file changed, 11 insertions(+), 20 deletions(-)
diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index cb3a10a..666b676 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -689,28 +689,13 @@ static void spapr_populate_memory_node(void *fdt,
int nodeid, hwaddr start,
static int spapr_populate_memory(sPAPREnvironment *spapr, void *fdt)
{
-hwaddr node0_size, mem_start, node_size;
+hwaddr mem_start, node_size;
int i;
-/* memory node(s) */
-if (nb_numa_nodes > 1 && node_mem[0] < ram_size) {
-node0_size = node_mem[0];
-} else {
-node0_size = ram_size;
-}
-
-/* RMA */
-spapr_populate_memory_node(fdt, 0, 0, spapr->rma_size);
-
-/* RAM: Node 0 */
-if (node0_size > spapr->rma_size) {
-spapr_populate_memory_node(fdt, 0, spapr->rma_size,
- node0_size - spapr->rma_size);
-}
-
-/* RAM: Node 1 and beyond */
-mem_start = node0_size;
-for (i = 1; i < nb_numa_nodes; i++) {
+for (i = 0, mem_start = 0; i < nb_numa_nodes; ++i) {
+if (!node_mem[i]) {
+continue;
+}
>>>
>>> Doesn't this skip memoryless nodes? What actually puts the memoryless
>>> node in the device-tree?
>>
>> It does skip.
>>
>>> And if you were to put them in, wouldn't spapr_populate_memory_node()
>>> fail because we'd be creating two nodes with memory@XXX where XXX is the
>>> same (starting address) for both?
>>
>> I cannot do this now - there is no way to tell from the command line
>> where I want NUMA node memory start from so I'll end up with multiple
>> nodes with the same name and QEMU won't start. When NUMA fixes reach
>> upstream, I'll try to work out something on top of that.
>
> So in mst's tree, which I've rebased your patches, we have a struct
> defining each NUMA node, which has a size (and the index is the nodeid).
> I've got patches working that allow for sparse indexing, but I'm curious
> what you think we should do for the naming.
There should be no nodes for memory@xxx in the device tree for memoryless
NUMA nodes so no problem with naming.
> I can send out the patches,
> with the caveat that architectures still need to fix the remaining
> issues for memoryless nodes?
? If we do not change the existing behavior and just extending it, why will
there be a problem?
--
Alexey
[Qemu-devel] [PATCH v1 1/1] char: cadence_uart: Convert to realize()
SysBusDevice::init is deprecated. Convert to Object::init and
Device::realize as prescribed by QOM conventions.
Signed-off-by: Alistair Francis
---
hw/char/cadence_uart.c | 29 -
1 files changed, 16 insertions(+), 13 deletions(-)
diff --git a/hw/char/cadence_uart.c b/hw/char/cadence_uart.c
index bf0c853..5a22a72 100644
--- a/hw/char/cadence_uart.c
+++ b/hw/char/cadence_uart.c
@@ -468,27 +468,30 @@ static void cadence_uart_reset(DeviceState *dev)
uart_update_status(s);
}
-static int cadence_uart_init(SysBusDevice *dev)
+static void candence_uart_realize(DeviceState *dev, Error **errp)
{
UartState *s = CADENCE_UART(dev);
-memory_region_init_io(&s->iomem, OBJECT(s), &uart_ops, s, "uart", 0x1000);
-sysbus_init_mmio(dev, &s->iomem);
-sysbus_init_irq(dev, &s->irq);
-
-s->fifo_trigger_handle = timer_new_ns(QEMU_CLOCK_VIRTUAL,
-(QEMUTimerCB *)fifo_trigger_update, s);
-
-s->char_tx_time = (get_ticks_per_sec() / 9600) * 10;
-
s->chr = qemu_char_get_next_serial();
if (s->chr) {
qemu_chr_add_handlers(s->chr, uart_can_receive, uart_receive,
uart_event, s);
}
+}
-return 0;
+static void cadence_uart_init(Object *obj)
+{
+UartState *s = CADENCE_UART(obj);
+
+memory_region_init_io(&s->iomem, obj, &uart_ops, s, "uart", 0x1000);
+sysbus_init_mmio(SYS_BUS_DEVICE(obj), &s->iomem);
+sysbus_init_irq(SYS_BUS_DEVICE(obj), &s->irq);
+
+s->fifo_trigger_handle = timer_new_ns(QEMU_CLOCK_VIRTUAL,
+(QEMUTimerCB *)fifo_trigger_update, s);
+
+s->char_tx_time = (get_ticks_per_sec() / 9600) * 10;
}
static int cadence_uart_post_load(void *opaque, int version_id)
@@ -520,9 +523,8 @@ static const VMStateDescription vmstate_cadence_uart = {
static void cadence_uart_class_init(ObjectClass *klass, void *data)
{
DeviceClass *dc = DEVICE_CLASS(klass);
-SysBusDeviceClass *sdc = SYS_BUS_DEVICE_CLASS(klass);
-sdc->init = cadence_uart_init;
+dc->realize = candence_uart_realize;
dc->vmsd = &vmstate_cadence_uart;
dc->reset = cadence_uart_reset;
}
@@ -531,6 +533,7 @@ static const TypeInfo cadence_uart_info = {
.name = TYPE_CADENCE_UART,
.parent= TYPE_SYS_BUS_DEVICE,
.instance_size = sizeof(UartState),
+.instance_init = cadence_uart_init,
.class_init= cadence_uart_class_init,
};
--
1.7.1
Re: [Qemu-devel] [PATCH] openrisc: fix comment
On Tue, 24 Jun 2014 07:46:43 +0300 "Michael S. Tsirkin" wrote: > Fix English in comment: > > s/the each/each/ > > s/ \*\// \*\// > > Signed-off-by: Michael S. Tsirkin > --- > target-openrisc/translate.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/target-openrisc/translate.c b/target-openrisc/translate.c > index b728718..55ff935 100644 > --- a/target-openrisc/translate.c > +++ b/target-openrisc/translate.c > @@ -531,14 +531,14 @@ static void dec_calc(DisasContext *dc, uint32_t insn) > TCGv_i64 high = tcg_temp_new_i64(); > TCGv_i32 sr_ove = tcg_temp_local_new_i32(); > int lab = gen_new_label(); > -/* Calculate the each result. */ > +/* Calculate each result. */ > tcg_gen_extu_i32_i64(tra, cpu_R[ra]); > tcg_gen_extu_i32_i64(trb, cpu_R[rb]); > tcg_gen_mul_i64(result, tra, trb); > tcg_temp_free_i64(tra); > tcg_temp_free_i64(trb); > tcg_gen_shri_i64(high, result, TARGET_LONG_BITS); > -/* Overflow or not. */ > +/* Overflow or not. */ > tcg_gen_brcondi_i64(TCG_COND_EQ, high, 0x, lab); > tcg_gen_ori_tl(cpu_sr, cpu_sr, (SR_OV | SR_CY)); > tcg_gen_andi_tl(sr_ove, cpu_sr, SR_OVE); Reviewed-by: Igor Mammedov
Re: [Qemu-devel] [PATCH] numa: fix comment
On Tue, 24 Jun 2014 08:51:23 +0300
"Michael S. Tsirkin" wrote:
> s/if given for/is given for/;
>
> Reported-by: Hu Tao
> Signed-off-by: Michael S. Tsirkin
> ---
> numa.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/numa.c b/numa.c
> index 47049a5..6c2eae7 100644
> --- a/numa.c
> +++ b/numa.c
> @@ -161,7 +161,7 @@ void set_numa_nodes(void)
> nb_numa_nodes = MAX_NODES;
> }
>
> -/* If no memory size if given for any node, assume the default case
> +/* If no memory size is given for any node, assume the default case
> * and distribute the available memory equally across all nodes
> */
> for (i = 0; i < nb_numa_nodes; i++) {
Reviewed-by: Igor Mammedov
Re: [Qemu-devel] [RFC] Functions bus_foreach and device_find from libqos virtio API
On Mon, 06/23 16:55, Marc Marí wrote:
> diff --git a/tests/Makefile b/tests/Makefile
> index 4caf7de..b85a036 100644
> --- a/tests/Makefile
> +++ b/tests/Makefile
> @@ -282,6 +282,7 @@ libqos-obj-y += tests/libqos/i2c.o
> libqos-pc-obj-y = $(libqos-obj-y) tests/libqos/pci-pc.o
> libqos-pc-obj-y += tests/libqos/malloc-pc.o
> libqos-omap-obj-y = $(libqos-obj-y) tests/libqos/i2c-omap.o
> +libqos-virtio-obj-y = $(libqos-obj-y) $(libqos-pc-obj-y)
> tests/libqos/virtio-pci.o
$(libqos-pc-obj-y) has $(libqos-obj-y) itself, two lines above. So no need to
add again.
> diff --git a/tests/libqos/virtio-pci.c b/tests/libqos/virtio-pci.c
> new file mode 100644
> index 000..62c238f
> --- /dev/null
> +++ b/tests/libqos/virtio-pci.c
> @@ -0,0 +1,123 @@
> +/*
> + * libqos virtio definitions
PCI
> + *
> + * Copyright (c) 2014 Marc Marí
> + *
> + * This work is licensed under the terms of the GNU GPL, version 2 or later.
> + * See the COPYING file in the top-level directory.
> + */
> +
> diff --git a/tests/libqos/virtio-pci.h b/tests/libqos/virtio-pci.h
> new file mode 100644
> index 000..d92bcf2
> --- /dev/null
> +++ b/tests/libqos/virtio-pci.h
> @@ -0,0 +1,40 @@
> +/*
> + * libqos virtio PCI definitions
> + *
> + * Copyright (c) 2014 Marc Marí
> + *
> + * This work is licensed under the terms of the GNU GPL, version 2 or later.
> + * See the COPYING file in the top-level directory.
> + */
> +
> +#ifndef LIBQOS_VIRTIO_PCI_H
> +#define LIBQOS_VIRTIO_PCI_H
> +
> +#include "libqos/virtio.h"
> +
> +void qvirtio_pci_notify(QVirtioDevice *d, uint16_t vector);
> +void qvirtio_pci_get_config(QVirtioDevice *d, void *config);
> +void qvirtio_pci_set_config(QVirtioDevice *d, void *config);
> +uint32_t qvirtio_pci_get_features(QVirtioDevice *d);
> +uint8_t qvirtio_pci_get_status(QVirtioDevice *d);
> +void qvirtio_pci_set_status(QVirtioDevice *d, uint8_t val);
> +void qvirtio_pci_reset(QVirtioDevice *d);
> +uint8_t qvirtio_pci_query_isr(QVirtioDevice *d);
> +void qvirtio_pci_foreach(uint16_t device_id,
> +void (*func)(QVirtioDevice *d, void *data), void *data);
> +QVirtioDevice *qvirtio_pci_device_find(uint16_t device_id);
> +
> +const QVirtioBus qvirtio_pci = {
> +.notify = qvirtio_pci_notify,
> +.get_config = qvirtio_pci_get_config,
> +.set_config = qvirtio_pci_set_config,
> +.get_features = qvirtio_pci_get_features,
> +.get_status = qvirtio_pci_get_status,
> +.set_status = qvirtio_pci_set_status,
> +.reset = qvirtio_pci_reset,
> +.query_isr = qvirtio_pci_query_isr,
> +.bus_foreach = qvirtio_pci_foreach,
> +.device_find = qvirtio_pci_device_find,
> +};
Each source file to include this header will define its own copy of
qvirtio_pci. Please move this to tests/libqos/virtio-pci.c. And can you make
the implementation functions static, because they are already accessed through
members of qvirtio_pci?
> +
> +#endif
> diff --git a/tests/libqos/virtio.h b/tests/libqos/virtio.h
> new file mode 100644
> index 000..838aae4
> --- /dev/null
> +++ b/tests/libqos/virtio.h
> +/*QVirtioBus *qvirtio_pci_init();
> +QVirtioBus *qvirtio_mmio_init();
> +QVirtioBus *qvirtio_ccw_init();*/
When you drop RFC, please drop this,
> +
> +/*
> +I'm not sure what implementation of VirtQueue is better, QEMU's or Linux's. I
> +think QEMU implementation is better, because it will be easier to connect the
> +QEMU Virtqueue with the Libaos VirtQueue.
> +
> +The functions to use the VirtQueue are the ones I think necessary in Libqos,
> but
> +probably there are some missing and some others that are not necessary.
> +*/
and this,
> diff --git a/tests/virtio-blk-test.c b/tests/virtio-blk-test.c
> index d53f875..4f5b1e0 100644
> --- a/tests/virtio-blk-test.c
> +++ b/tests/virtio-blk-test.c
> @@ -2,6 +2,7 @@
> * QTest testcase for VirtIO Block Device
> *
> * Copyright (c) 2014 SUSE LINUX Products GmbH
> + * Copyright (c) 2014 Marc Marí
> *
> * This work is licensed under the terms of the GNU GPL, version 2 or later.
> * See the COPYING file in the top-level directory.
> @@ -9,26 +10,51 @@
>
> #include
> #include
> +#include
> +#include
> +#include
> #include "libqtest.h"
> -#include "qemu/osdep.h"
> +#include "libqos/virtio.h"
> +/*#include "qemu/osdep.h"*/
and this.
>
> -/* Tests only initialization so far. TODO: Replace with functional tests */
> -static void pci_nop(void)
> +#define TEST_IMAGE_SIZE (64 * 1024 * 1024)
> +
> +static char tmp_path[] = "/tmp/qtest.XX";
> +extern QVirtioBus qvirtio_pci;
> +
> +static void pci_basic(void)
> {
> +QVirtioDevice *dev;
> +dev = qvirtio_pci.device_find(VIRTIO_BLK_DEVICE_ID);
> +fprintf(stderr, "Device: %x %x %x\n",
> +dev->device_id, dev->location, dev->device_type);
> }
>
> int main(int argc, char **argv)
> {
> int ret;
> +int fd;
> +char test_start[100];
Depending on length of tmp_path, this looks quite close to an overflow ...
>
> g_test_init(&argc, &a
Re: [Qemu-devel] [PATCH] numa: fix comment
On Tue, Jun 24, 2014 at 01:33:35PM +0800, Hu Tao wrote:
> On Tue, Jun 24, 2014 at 07:45:15AM +0300, Michael S. Tsirkin wrote:
> > Fix up English in comments:
> > s/the each/each/
> >
> > Signed-off-by: Michael S. Tsirkin
> > ---
> > numa.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/numa.c b/numa.c
> > index e471afe..47049a5 100644
> > --- a/numa.c
> > +++ b/numa.c
> > @@ -172,7 +172,7 @@ void set_numa_nodes(void)
> > if (i == nb_numa_nodes) {
> > uint64_t usedmem = 0;
> >
> > -/* On Linux, the each node's border has to be 8MB aligned,
> > +/* On Linux, each node's border has to be 8MB aligned,
> > * the final node gets the rest.
> > */
> > for (i = 0; i < nb_numa_nodes - 1; i++) {
> > --
> > MST
>
> There is another one in the same function that can be fixed togehter:
>
> /* If no memory size if given for any node
>
> s/if/is/
>
> Hu
Fixed, thanks.
[Qemu-devel] [PATCH] numa: fix comment
s/if given for/is given for/;
Reported-by: Hu Tao
Signed-off-by: Michael S. Tsirkin
---
numa.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/numa.c b/numa.c
index 47049a5..6c2eae7 100644
--- a/numa.c
+++ b/numa.c
@@ -161,7 +161,7 @@ void set_numa_nodes(void)
nb_numa_nodes = MAX_NODES;
}
-/* If no memory size if given for any node, assume the default case
+/* If no memory size is given for any node, assume the default case
* and distribute the available memory equally across all nodes
*/
for (i = 0; i < nb_numa_nodes; i++) {
--
MST
Re: [Qemu-devel] [question] Will qemu-2.0.0 be a longterm stable branch?
On Tue, Jun 24, 2014 at 09:08:56AM +0800, Zhang Haoyu wrote: > Hi, all > > Which version is best for commercial product, qemu-2.0.0 or other versions? > Any advices? > > Thanks, > Zhang Haoyu Use one of the downstreams: Red Hat, Fedora, Debian all ship QEMU and have active QEMU maintainers developing the distribution. -- MST
Re: [Qemu-devel] machines and versions - why so many?
On 06/24/2014 03:15 PM, Paolo Bonzini wrote: > Il 23/06/2014 23:35, Alexey Kardashevskiy ha scritto: >> nec-usb-xhci is off for PC_COMPAT_1_3 because of what? PIIX emulation was >> broken in 1.3? Or nec-usb-xhci? > > nec-usb-xhci is not "off". Yes, my bad, I understood (but explained this wrong) that XHCI is enabled but without MSI. > > .driver = "nec-usb-xhci",\ > .property = "msi",\ > .value= "off",\ > > What's off is MSI (and MSI-X), because it wasn't implemented. > >> If it is the latter, why is the tweak limited by PC? > > Because at the time nobody cared about migration compatibility (even now > only pSeries is starting to care). "only pSeries"? There are plenty of x86 machines already and if it is not for migration support, then what are they all for? :) >> Looks like I must copy PC_COMPAT_X_X as PSERIES_COMPAT_X_X starting 1.6 (or >> 1.7 - whichever starts supporting migration well enough on pseries) because >> pretty much of what they do is tweaking PCI devices and we can have all of >> these devices on pseries. > > It's not necessary to do this retroactively, unless you're planning to test > forward migration from 1.6 to 2.1. Just start with 2.2 (and add > pseries-2.1 now as an alias). > > Note that firmware interfaces and CPU features should also be versioned if > they're guest visible. That's the point of functions like pc_compat_1_6. > >> And then keep an eye on what is happening in PC >> world to copy same tweaks to pseries as they come. Is that correct? > > Yes. We should come up sooner or later with a way to share the tweaks though. > >> Others (ARM, s390) do not do that because they do not support PCI? Or >> migration? Or some other reason? > > This is not limited to PCI. Others do not support migration across > different QEMU versions. Yeah, I got that now. Thanks! -- Alexey
Re: [Qemu-devel] [PATCH] numa: fix comment
On Tue, Jun 24, 2014 at 07:45:15AM +0300, Michael S. Tsirkin wrote:
> Fix up English in comments:
> s/the each/each/
>
> Signed-off-by: Michael S. Tsirkin
> ---
> numa.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/numa.c b/numa.c
> index e471afe..47049a5 100644
> --- a/numa.c
> +++ b/numa.c
> @@ -172,7 +172,7 @@ void set_numa_nodes(void)
> if (i == nb_numa_nodes) {
> uint64_t usedmem = 0;
>
> -/* On Linux, the each node's border has to be 8MB aligned,
> +/* On Linux, each node's border has to be 8MB aligned,
> * the final node gets the rest.
> */
> for (i = 0; i < nb_numa_nodes - 1; i++) {
> --
> MST
There is another one in the same function that can be fixed togehter:
/* If no memory size if given for any node
s/if/is/
Hu
Re: [Qemu-devel] broken win32 coroutines (was Re: qemu 1.6.1)
Il 24/06/2014 03:41, Michael W. Bombardieri ha scritto: Hi, Thanks for adding me to this thread. I am not familiar with the qemu source code but I am aware of the coroutine crash and I can test a patch if you have one. qemu 1.5.1 was the last version I was able to build and use on win32. Later versions build without error but exhibit the coroutine crash. qemu binaries built with the coroutine feature disabled are to slow to use. Peter posted one here: http://article.gmane.org/gmane.comp.emulators.qemu/282189/raw Paolo
Re: [Qemu-devel] machines and versions - why so many?
Il 24/06/2014 00:33, Alexey Kardashevskiy ha scritto: I actually wonder if it is not going to be "-machine pseries-2.0" then what will it look like? "-machine pseries,qemucompat=2.0"? I would keep using pseries-2.0. There might be some class hierarchy, like spapr-machine spapr-machine-2.2 spapr-machine-2.1 I don't know what Andreas had in mind though, but you can check out Eduardo's patches for QOMifying the PC machines and take inspiration from there. That's probably a safe bet. Paolo
Re: [Qemu-devel] [question] Will qemu-2.0.0 be a longterm stable branch?
Il 24/06/2014 03:08, Zhang Haoyu ha scritto: Hi, all Which version is best for commercial product, qemu-2.0.0 or other versions? Any advices? The QEMU probject is not providing long term stable branches. In fact, we lack the manpower to provide short term stable branches too. :( Paolo
Re: [Qemu-devel] machines and versions - why so many?
Il 23/06/2014 23:35, Alexey Kardashevskiy ha scritto: nec-usb-xhci is off for PC_COMPAT_1_3 because of what? PIIX emulation was broken in 1.3? Or nec-usb-xhci? nec-usb-xhci is not "off". .driver = "nec-usb-xhci",\ .property = "msi",\ .value= "off",\ What's off is MSI (and MSI-X), because it wasn't implemented. If it is the latter, why is the tweak limited by PC? Because at the time nobody cared about migration compatibility (even now only pSeries is starting to care). Looks like I must copy PC_COMPAT_X_X as PSERIES_COMPAT_X_X starting 1.6 (or 1.7 - whichever starts supporting migration well enough on pseries) because pretty much of what they do is tweaking PCI devices and we can have all of these devices on pseries. It's not necessary to do this retroactively, unless you're planning to test forward migration from 1.6 to 2.1. Just start with 2.2 (and add pseries-2.1 now as an alias). Note that firmware interfaces and CPU features should also be versioned if they're guest visible. That's the point of functions like pc_compat_1_6. And then keep an eye on what is happening in PC world to copy same tweaks to pseries as they come. Is that correct? Yes. We should come up sooner or later with a way to share the tweaks though. Others (ARM, s390) do not do that because they do not support PCI? Or migration? Or some other reason? This is not limited to PCI. Others do not support migration across different QEMU versions. Paolo
Re: [Qemu-devel] [Bug 1308542] Re: hang in qemu_gluster_init
Verified that this fixes the hang and no change is required in gluster driver of QEMU after this fix in glusterfs code. On Mon, Jun 23, 2014 at 11:59 PM, nixpanic wrote: > A complete fix has been included in the glusterfs master-branch. It has > not (yet) been requested or marked for backporting to a stable (3.5.x) > branch. > > * https://bugzilla.redhat.com/1091335 with > http://review.gluster.org/7857 > > The issue with glfs_set_logging is fixed in the almost released > glusterfs-3.5.1 (https://bugzilla.redhat.com/1103413). > > -- > You received this bug notification because you are a member of qemu- > devel-ml, which is subscribed to QEMU. > https://bugs.launchpad.net/bugs/1308542 > > Title: > hang in qemu_gluster_init > > Status in QEMU: > New > > Bug description: > In qemu_gluster_init, if the call to either glfs_set_volfile_server or > glfs_set_logging fails into the "out" case, glfs_fini is called > without having first calling glfs_init. This causes glfs_lock to spin > forever on this bit: > > while (!fs->init) > pthread_cond_wait (&fs->cond, &fs->mutex); > > And here's the bottom part of the backtrace when hung: > > #0 pthread_cond_wait@@GLIBC_2.3.2 () at > ../nptl/sysdeps/unix/sysv/linux/x86_64/pthread_cond_wait.S:183 > #1 0x7feceebf58c3 in glfs_lock (fs=0x7fecf15660b0) at > glfs-internal.h:156 > #2 glfs_active_subvol (fs=0x7fecf15660b0) at glfs-resolve.c:799 > #3 0x7feceebeb5b4 in glfs_fini (fs=0x7fecf15660b0) at glfs.c:652 > #4 0x7fecf0043c73 in qemu_gluster_init (gconf= out>, filename=) at > /usr/src/debug/qemu-kvm-0.12.1.2/block/gluster.c:229 > > I believe this can be fixed by simply moving the call to glfs_init > after the call to glfs_new but before the calls to > glfs_set_volfile_server or glfs_set_logging. > > To manage notifications about this bug go to: > https://bugs.launchpad.net/qemu/+bug/1308542/+subscriptions > > -- http://raobharata.wordpress.com/
[Qemu-devel] [PATCH] openrisc: fix comment
Fix English in comment: s/the each/each/ s/ \*\// \*\// Signed-off-by: Michael S. Tsirkin --- target-openrisc/translate.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/target-openrisc/translate.c b/target-openrisc/translate.c index b728718..55ff935 100644 --- a/target-openrisc/translate.c +++ b/target-openrisc/translate.c @@ -531,14 +531,14 @@ static void dec_calc(DisasContext *dc, uint32_t insn) TCGv_i64 high = tcg_temp_new_i64(); TCGv_i32 sr_ove = tcg_temp_local_new_i32(); int lab = gen_new_label(); -/* Calculate the each result. */ +/* Calculate each result. */ tcg_gen_extu_i32_i64(tra, cpu_R[ra]); tcg_gen_extu_i32_i64(trb, cpu_R[rb]); tcg_gen_mul_i64(result, tra, trb); tcg_temp_free_i64(tra); tcg_temp_free_i64(trb); tcg_gen_shri_i64(high, result, TARGET_LONG_BITS); -/* Overflow or not. */ +/* Overflow or not. */ tcg_gen_brcondi_i64(TCG_COND_EQ, high, 0x, lab); tcg_gen_ori_tl(cpu_sr, cpu_sr, (SR_OV | SR_CY)); tcg_gen_andi_tl(sr_ove, cpu_sr, SR_OVE); -- MST
[Qemu-devel] [PATCH] numa: fix comment
Fix up English in comments:
s/the each/each/
Signed-off-by: Michael S. Tsirkin
---
numa.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/numa.c b/numa.c
index e471afe..47049a5 100644
--- a/numa.c
+++ b/numa.c
@@ -172,7 +172,7 @@ void set_numa_nodes(void)
if (i == nb_numa_nodes) {
uint64_t usedmem = 0;
-/* On Linux, the each node's border has to be 8MB aligned,
+/* On Linux, each node's border has to be 8MB aligned,
* the final node gets the rest.
*/
for (i = 0; i < nb_numa_nodes - 1; i++) {
--
MST
Re: [Qemu-devel] [RFC] Functions bus_foreach and device_find from libqos virtio API
On Mon, Jun 23, 2014 at 04:55:22PM +0200, Marc Marí wrote:
> +static QVirtioDevice *qpcidevice_to_qvirtiodevice(QPCIDevice *dpci)
> +{
> +QVirtioDevice *dvirtio;
> +dvirtio = g_malloc0(sizeof(*dvirtio));
> +
> +if (dpci) {
> +dvirtio->device_id = qpci_config_readw(dpci, PCI_DEVICE_ID);
> +dvirtio->device_type = qpci_config_readw(dpci, PCI_SUBSYSTEM_ID);
> +dvirtio->location = dpci->devfn;
> +dvirtio->vq = g_malloc0(sizeof(QVirtQueue));
QVirtioDevice doesn't know about PCI, so device_id and location are not
appropriate - at least as public fields that callers are expected to
access. It's fine to stash away the PCI-specific information, probably
by defining a QVirtioPCIDevice struct that embeds QVirtioDevice:
typedef struct {
QVirtioDevice vdev;
QPCIDevice *pdev;
} QVirtioPCIDevice;
Then virtio-pci functions can cast from the generic QVirtioDevice back
to the PCI-specific QVirtioPCIDevice:
static void qvirtio_pci_notify(QVirtioDevice *vdev)
{
QVirtioPCIDevice *dev = (QVirtioPCIDevice*)vdev;
qpci_io_writew(dev->pdev, ...);
}
> +void qvirtio_pci_foreach(uint16_t device_type,
> +void (*func)(QVirtioDevice *d, void *data), void *data)
> +{
> +QPCIBus *bus;
> +QPCIDevice *dev;
> +int slot;
> +int fn;
> +
> +bus = qpci_init_pc();
This might be a design flaw in qpci_init_pc(). Not sure it's a good
idea to have a local PCI bus for every qvirtio_pci_foreach() call.
Tests should be able to operate on different PCI busses, if the machine
has several, but devices on the same bus should probably share a QPCIBus
instance.
> +for (slot = 0; slot < 32; slot++) {
> +for (fn = 0; fn < 8; fn++) {
> +dev = g_malloc0(sizeof(*dev));
> +dev->bus = bus;
> +dev->devfn = QPCI_DEVFN(slot, fn);
> +
> +if (qpci_config_readw(dev, PCI_VENDOR_ID) == 0x) {
> +g_free(dev);
> +continue;
> +}
> +
> +if (device_type != (uint16_t)-1 &&
> +qpci_config_readw(dev, PCI_SUBSYSTEM_ID) != device_type) {
> +continue;
> +}
> +
> +func(qpcidevice_to_qvirtiodevice(dev), data);
> +}
> +}
Can you reuse qpci_device_foreach() instead of duplicating this code?
> +uint64_t qvring_desc_addr(uint64_t desc_pa, int i);
> +uint32_t qvring_desc_len(uint64_t desc_pa, int i);
> +uint16_t qvring_desc_flags(uint64_t desc_pa, int i);
> +uint16_t qvring_desc_next(uint64_t desc_pa, int i);
> +uint16_t qvring_avail_flags(QVirtQueue *vq);
> +uint16_t qvring_avail_idx(QVirtQueue *vq);
> +uint16_t qvring_avail_ring(QVirtQueue *vq, int i);
> +uint16_t qvring_used_event(QVirtQueue *vq);
> +void qvring_used_ring_id(QVirtQueue *vq, int i, uint32_t val);
> +void qvring_used_ring_len(QVirtQueue *vq, int i, uint32_t val);
> +uint16_t qvring_used_idx(QVirtQueue *vq);
> +void qvring_used_idx_set(QVirtQueue *vq, uint16_t val);
> +void qvring_used_flags_set_bit(QVirtQueue *vq, int mask);
> +void qvring_used_flags_unset_bit(QVirtQueue *vq, int mask);
> +void qvring_avail_event(QVirtQueue *vq, uint16_t val);
> +
> +void qvirtqueue_push(QVirtQueue *vq, const QVirtQueueElement *elem,
> +unsigned int len);
> +void qvirtqueue_flush(QVirtQueue *vq, unsigned int count);
> +void qvirtqueue_fill(QVirtQueue *vq, const QVirtQueueElement *elem,
> +unsigned int len, unsigned int idx);
> +void qvirtqueue_pop(QVirtQueue *vq, QVirtQueueElement *elem);
> +void qvirtqueue_map_sg(/*struct iovec *sg,*/ uint64_t *addr,
> +size_t num_sg, int is_write);
> +void qvirtio_notify(QVirtioDevice *vdev, QVirtQueue *vq);
> +int qvirtio_queue_ready(QVirtQueue *vq);
> +int qvirtio_queue_empty(QVirtQueue *vq);
Please drop all the unimplemented functions and unused structs from the
next patch revision. Let's focus on one step at a time:
1. Finding devices
2. Device reset, setting status field bits, feature bit negotiation
3. Notification (qtest->QEMU kick, QEMU->qtest interrupt)
4. Virtqueues
5. Simple virtio-blk tests (e.g. read a block from the disk) to prove it
works
I'm suggesting you send the next patch revision without RFC and just
with a test case for finding devices (no unimplemented stuff). That way
you can send new patches each week and have them merged incrementally,
rather than pushing along one big series that keeps growing until the
very end.
> +
> +#endif
> diff --git a/tests/virtio-blk-test.c b/tests/virtio-blk-test.c
> index d53f875..4f5b1e0 100644
> --- a/tests/virtio-blk-test.c
> +++ b/tests/virtio-blk-test.c
> @@ -2,6 +2,7 @@
> * QTest testcase for VirtIO Block Device
> *
> * Copyright (c) 2014 SUSE LINUX Products GmbH
> + * Copyright (c) 2014 Marc Marí
> *
> * This work is licensed under the terms of the GNU GPL, version 2 or later.
> * See the COPYING file in the top-level directory.
> @@ -9,26
Re: [Qemu-devel] [PATCH v1 1/1] timer: candence_ttc: Convert to realize()
Subject should read:
"convert to Object::init()"
On Tue, Jun 24, 2014 at 2:24 PM, Alistair Francis
wrote:
> SysBusDevice::init is deprecated. Convert to Object::init
> as prescribed by QOM conventions.
>
> Signed-off-by: Alistair Francis
Otherwise:
Reviewed-by: Peter Crosthwaite
Peter has been taking misc Zynq devs patches via the ARM queue, so
perhaps this can go via target-arm?
Regards,
Peter
> ---
>
> hw/timer/cadence_ttc.c | 15 ++-
> 1 files changed, 6 insertions(+), 9 deletions(-)
>
> diff --git a/hw/timer/cadence_ttc.c b/hw/timer/cadence_ttc.c
> index 52c..d46db3c 100644
> --- a/hw/timer/cadence_ttc.c
> +++ b/hw/timer/cadence_ttc.c
> @@ -406,21 +406,19 @@ static void cadence_timer_init(uint32_t freq,
> CadenceTimerState *s)
> s->timer = timer_new_ns(QEMU_CLOCK_VIRTUAL, cadence_timer_tick, s);
> }
>
> -static int cadence_ttc_init(SysBusDevice *dev)
> +static void cadence_ttc_init(Object *obj)
> {
> -CadenceTTCState *s = CADENCE_TTC(dev);
> +CadenceTTCState *s = CADENCE_TTC(obj);
> int i;
>
> for (i = 0; i < 3; ++i) {
> cadence_timer_init(13300, &s->timer[i]);
> -sysbus_init_irq(dev, &s->timer[i].irq);
> +sysbus_init_irq(SYS_BUS_DEVICE(obj), &s->timer[i].irq);
> }
>
> -memory_region_init_io(&s->iomem, OBJECT(s), &cadence_ttc_ops, s,
> +memory_region_init_io(&s->iomem, obj, &cadence_ttc_ops, s,
>"timer", 0x1000);
> -sysbus_init_mmio(dev, &s->iomem);
> -
> -return 0;
> +sysbus_init_mmio(SYS_BUS_DEVICE(obj), &s->iomem);
> }
>
> static void cadence_timer_pre_save(void *opaque)
> @@ -474,9 +472,7 @@ static const VMStateDescription vmstate_cadence_ttc = {
> static void cadence_ttc_class_init(ObjectClass *klass, void *data)
> {
> DeviceClass *dc = DEVICE_CLASS(klass);
> -SysBusDeviceClass *sdc = SYS_BUS_DEVICE_CLASS(klass);
>
> -sdc->init = cadence_ttc_init;
> dc->vmsd = &vmstate_cadence_ttc;
> }
>
> @@ -484,6 +480,7 @@ static const TypeInfo cadence_ttc_info = {
> .name = TYPE_CADENCE_TTC,
> .parent = TYPE_SYS_BUS_DEVICE,
> .instance_size = sizeof(CadenceTTCState),
> +.instance_init = cadence_ttc_init,
> .class_init = cadence_ttc_class_init,
> };
>
> --
> 1.7.1
>
>
[Qemu-devel] [RFC] alpha qemu arithmetic exceptions
First of all, kudos - with current qemu tree qemu-alpha-system is
working pretty well - debian install and a *lot* of builds work just fine.
As in, getting from lenny to pretty complete squeeze toolchain, including gcj,
openjdk6 and a lot of crap needed to satisfy build-deps of those, plus all
priority:required and most of priority:important ones. It's a _lot_ of
beating and the damn thing survives - the problems had been with debian
packages themselves (fstatat() bug in lenny libc, epically buggered build-deps
in gcc-defaults, etc.). As it is, one core of 6-way 3.3GHz phenom II is quite
capable of running a home-grown autobuilder. Feels like ~250-300MHz alpha
with a very fast disk...
Remaining problems, AFAICS, are around floating point traps.
I've found one in glibc testsuite (math/tests-misc.c; overflow in
ADDS/SU ends up with wrong results from fetestexcept() - only FE_OVERFLOW is
set, while the sucker expects FE_INEXACT as well and actual hardware sets both)
and another in gcc one (with -funsafe-math-optimizations CVTST/S on denorms
triggers SIGFPE/FPE_FLTINV).
The libc one is a bug in gen_fp_exc_raise_ignore() - the difference
between ADDS/SU and ADDS/SUI is only in trapping, not storing results in
FPCR.INE and friends. Both will have the same effect on those and
if (ignore) {
tcg_gen_andi_i32(exc, exc, ~ignore);
}
in gen_fp_exc_raise_ignore() leads to exc & ignore not reaching the
update of env->fpcr_exc_status in helper_fp_exc_raise_s(). See 4.7.8:
[quote]
In addition, the FPCR gives a summary of each exception type for the
exception conditions detected by all IEEE floating-point operates thus
far, as well as an overall summary bit that indicates whether any of
these exception conditions has been detected. The indiividual exception
bits match exactly in purpose and order the exception bits found in the
exception summary quadword that is pushed for arithmetic traps. However,
for each instruction, these exception bits arse set independent of the
trapping mode specified for the instruction. Therefore, even though
trapping may be disabled for a certain exceptional condition, the fact
that the exceptional condition was encountered by an instruction is
still recorded in the FPCR.
[end quote]
And yes, on actual hardware both ADDS/SU and ADDS/SUI set FPCR.INE the same
way - verified by direct experiment.
While we are at it, I'm not quite sure what plain ADDS will leave in FPCR.INE
if it traps on overflow. It's probably entirely academical, but it might be
worth checking if ELF ABI for Alpha has anything to say about the state seen
by fetestexcept() in SIGFPE handler...
Not sure what's the decent way to fix that - we could, of course, follow that
tcg_gen_ld8u_i32(exc, cpu_env,
offsetof(CPUAlphaState, fp_status.float_exception_flags));
with generating code that would do |= into ->fpcr_exc_status, but I don't
know if we'd blow the footprint to hell by doing so. Alternatively, we could
do that in helpers called before we start raising exceptions, and I really
wonder what happens with plain CVTQS - do we get FPCR.INE set there anyway?
If so, we really have to do it at least in that helper... Comments?
The gcc one comes from the fact that we never set EXC_M_SWC,
whether we come from helper_fp_exc_raise() or from helper_fp_exc_raise_s(),
so kernel-side do_entArith() skips
if (summary & 1) {
/* Software-completion summary bit is set, so try to
emulate the instruction. If the processor supports
precise exceptions, we don't have to search. */
if (!amask(AMASK_PRECISE_TRAP))
si_code = alpha_fp_emul(regs->pc - 4);
else
si_code = alpha_fp_emul_imprecise(regs, write_mask);
if (si_code == 0)
return;
}
and buggers off to raise SIGFPE. That's easy to fix, but... we also
get trap PC pointing to offending instruction, not the next one after it,
as 4.7.7.3 would require and as the kernel expects.
I'm not sure what to do with trap PC - bumping env->pc by 4 after
cpu_restore_state() in dynamic_excp() seems to work, but I'm not at all sure
it's correct; I don't know qemu well enough to tell.
Anyway, delta that seems to fix the gcc one (gcc.dg/pr28796-2.c from
gcc-4.3 and later) follows. Again, I'm not at all sure if handling of
env->pc in there is safe from qemu POV and I'd like like to get comments on
that from somebody more familiar with qemu guts.
diff --git a/target-alpha/fpu_helper.c b/target-alpha/fpu_helper.c
index d2d776c..a24535b 100644
--- a/target-alpha/fpu_helper.c
+++ b/target-alpha/fpu_helper.c
@@ -45,10 +45,10 @@ uint32_t helper_fp_exc_get(CPUAlphaState *env)
}
static inline void inline_fp_exc_raise(CPUAlphaState *env, uintptr_
Re: [Qemu-devel] [PATCH v1 1/1] timer: candence_ttc: Convert to realize()
CC Andreas
On Tue, Jun 24, 2014 at 2:24 PM, Alistair Francis
wrote:
> SysBusDevice::init is deprecated. Convert to Object::init
> as prescribed by QOM conventions.
>
> Signed-off-by: Alistair Francis
> ---
>
> hw/timer/cadence_ttc.c | 15 ++-
> 1 files changed, 6 insertions(+), 9 deletions(-)
>
> diff --git a/hw/timer/cadence_ttc.c b/hw/timer/cadence_ttc.c
> index 52c..d46db3c 100644
> --- a/hw/timer/cadence_ttc.c
> +++ b/hw/timer/cadence_ttc.c
> @@ -406,21 +406,19 @@ static void cadence_timer_init(uint32_t freq,
> CadenceTimerState *s)
> s->timer = timer_new_ns(QEMU_CLOCK_VIRTUAL, cadence_timer_tick, s);
> }
>
> -static int cadence_ttc_init(SysBusDevice *dev)
> +static void cadence_ttc_init(Object *obj)
> {
> -CadenceTTCState *s = CADENCE_TTC(dev);
> +CadenceTTCState *s = CADENCE_TTC(obj);
> int i;
>
> for (i = 0; i < 3; ++i) {
> cadence_timer_init(13300, &s->timer[i]);
> -sysbus_init_irq(dev, &s->timer[i].irq);
> +sysbus_init_irq(SYS_BUS_DEVICE(obj), &s->timer[i].irq);
> }
>
> -memory_region_init_io(&s->iomem, OBJECT(s), &cadence_ttc_ops, s,
> +memory_region_init_io(&s->iomem, obj, &cadence_ttc_ops, s,
>"timer", 0x1000);
> -sysbus_init_mmio(dev, &s->iomem);
> -
> -return 0;
> +sysbus_init_mmio(SYS_BUS_DEVICE(obj), &s->iomem);
> }
>
> static void cadence_timer_pre_save(void *opaque)
> @@ -474,9 +472,7 @@ static const VMStateDescription vmstate_cadence_ttc = {
> static void cadence_ttc_class_init(ObjectClass *klass, void *data)
> {
> DeviceClass *dc = DEVICE_CLASS(klass);
> -SysBusDeviceClass *sdc = SYS_BUS_DEVICE_CLASS(klass);
>
> -sdc->init = cadence_ttc_init;
> dc->vmsd = &vmstate_cadence_ttc;
> }
>
> @@ -484,6 +480,7 @@ static const TypeInfo cadence_ttc_info = {
> .name = TYPE_CADENCE_TTC,
> .parent = TYPE_SYS_BUS_DEVICE,
> .instance_size = sizeof(CadenceTTCState),
> +.instance_init = cadence_ttc_init,
> .class_init = cadence_ttc_class_init,
> };
>
> --
> 1.7.1
>
Re: [Qemu-devel] [PATCH v2 0/4] mirror: Fix behavior for zero byte image
On Mon, 06/09 10:56, Fam Zheng wrote: > The current behavior of mirroring zero byte image is slightly different from > non-zero image: the BLOCK_JOB_READY event is skipped and job is completed > immediately. This is not a big problem for human user but only makes > management > software harder to write. Since we are focusing on an good API instead of UI, > let's make the behavior more consistent and predictable. > > The first patch fixes the behavior. The following two patches add test case > for > the involved code path. > > Thanks for Eric Blake to report this! Ping. Shall we merge this before it rots? Thanks, Fam > > > v2: Address Stefan's comments. > > - Added patch 01: block_job_yield. > - Use block_job_yield in 02. > - Fix test case updates. > > Thanks to Stefan, Paolo and Eric for reviewing! > > Fam > > > Fam Zheng (4): > blockjob: Add block_job_yield() > mirror: Go through ready -> complete process for 0 len image > qemu-iotests: Test BLOCK_JOB_READY event for 0Kb image active commit > qemu-iotests: Test 0-length image for mirror > > block/mirror.c | 11 ++- > blockjob.c | 14 ++ > include/block/blockjob.h | 8 > tests/qemu-iotests/040 | 12 +--- > tests/qemu-iotests/040.out | 4 ++-- > tests/qemu-iotests/041 | 9 ++--- > tests/qemu-iotests/041.out | 4 ++-- > 7 files changed, 51 insertions(+), 11 deletions(-) > > -- > 2.0.0 >
Re: [Qemu-devel] [PATCH 0/3] target-sparc: fixed unused function warnings
On 06/23/2014 04:01 PM, Peter Maydell wrote: > These patchsets fix clang 3.4 warnings about unused static inline > functions (clang now warns about these if they're defined in a > .c file but then not used; gcc doesn't). The first patch just > removes two totally unused functions; the second two patches > use ifdeffery to avoid defining the functions in non-TARGET_SPARC64 > builds. I think I would be happier if you changed the functions to not be marked as "inline". I think that there's a large body of code that marks things inline exactly to prevent unused warnings with gcc. If we're going to play with the ifdeffery, we might as well just let any compiler warn if its unused. r~
[Qemu-devel] [PATCH v1 1/1] timer: candence_ttc: Convert to realize()
SysBusDevice::init is deprecated. Convert to Object::init
as prescribed by QOM conventions.
Signed-off-by: Alistair Francis
---
hw/timer/cadence_ttc.c | 15 ++-
1 files changed, 6 insertions(+), 9 deletions(-)
diff --git a/hw/timer/cadence_ttc.c b/hw/timer/cadence_ttc.c
index 52c..d46db3c 100644
--- a/hw/timer/cadence_ttc.c
+++ b/hw/timer/cadence_ttc.c
@@ -406,21 +406,19 @@ static void cadence_timer_init(uint32_t freq,
CadenceTimerState *s)
s->timer = timer_new_ns(QEMU_CLOCK_VIRTUAL, cadence_timer_tick, s);
}
-static int cadence_ttc_init(SysBusDevice *dev)
+static void cadence_ttc_init(Object *obj)
{
-CadenceTTCState *s = CADENCE_TTC(dev);
+CadenceTTCState *s = CADENCE_TTC(obj);
int i;
for (i = 0; i < 3; ++i) {
cadence_timer_init(13300, &s->timer[i]);
-sysbus_init_irq(dev, &s->timer[i].irq);
+sysbus_init_irq(SYS_BUS_DEVICE(obj), &s->timer[i].irq);
}
-memory_region_init_io(&s->iomem, OBJECT(s), &cadence_ttc_ops, s,
+memory_region_init_io(&s->iomem, obj, &cadence_ttc_ops, s,
"timer", 0x1000);
-sysbus_init_mmio(dev, &s->iomem);
-
-return 0;
+sysbus_init_mmio(SYS_BUS_DEVICE(obj), &s->iomem);
}
static void cadence_timer_pre_save(void *opaque)
@@ -474,9 +472,7 @@ static const VMStateDescription vmstate_cadence_ttc = {
static void cadence_ttc_class_init(ObjectClass *klass, void *data)
{
DeviceClass *dc = DEVICE_CLASS(klass);
-SysBusDeviceClass *sdc = SYS_BUS_DEVICE_CLASS(klass);
-sdc->init = cadence_ttc_init;
dc->vmsd = &vmstate_cadence_ttc;
}
@@ -484,6 +480,7 @@ static const TypeInfo cadence_ttc_info = {
.name = TYPE_CADENCE_TTC,
.parent = TYPE_SYS_BUS_DEVICE,
.instance_size = sizeof(CadenceTTCState),
+.instance_init = cadence_ttc_init,
.class_init = cadence_ttc_class_init,
};
--
1.7.1
Re: [Qemu-devel] [PATCH] target-s390x: Remove unused ld_code6() function
On 06/23/2014 04:06 PM, Peter Maydell wrote: > The ld_code6() function is unused; remove it. > > Signed-off-by: Peter Maydell > --- > We could use this for loads of 6-byte insns, but that would > imply a pointless reload of the first 2 bytes, which is > presumably why this function isn't actually used. Exactly so. Reviewed-by: Richard Henderson r~
Re: [Qemu-devel] [RFC PATCH] ppc/spapr: support sparse NUMA node numbering
Hi David,
On 23.06.2014 [18:08:33 -0700], Nishanth Aravamudan wrote:
> With generic sparse NUMA node parsing in place ("numa: enable sparse
> node numbering"), ppc can be updated to iterate over only the
> user-specified nodes.
>
> qemu-system-ppc64 -machine pseries,accel=kvm,usb=off -m 4096
> -realtime mlock=off -numa node,nodeid=3 -numa node,nodeid=2 -smp 4
I'd like to do so something similar for x86 as a follow-on patch, but I
don't really know what the SRAT should look like, or what the APIC
mapping should be to have sparse NUMA numbering? Some cursory googling
indicates you've run into this in practice? Possibly with a bad APIC?
Thanks,
Nish
Re: [Qemu-devel] [Xen-devel] [RFC PATCH V3 1/2] xen: pass kernel initrd to qemu
>>> On 6/23/2014 at 10:22 PM, in message
<21416.14398.695327.278...@mariner.uk.xensource.com>, Ian Jackson
wrote:
> Chunyan Liu writes ("[RFC PATCH V3 1/2] xen: pass kernel initrd to qemu"):
> > xen side patch to support xen HVM direct kernel boot:
> > support 'kernel', 'ramdisk', 'cmdline' (and 'root', 'extra' as well
> > which would be deprecated later) in HVM config file, parse config file,
> > pass -kernel, -initrd, -append parameters to qemu.
> ...
> > +Currently, direct kernel boot can be supported by PV guests, and HVM
> guests
> > +with limitations. For HVM guests, in case of stubdom-dm and old rombios,
> > +direct kernel boot is not supported.
>
> What does "PV guests" mean here ? Do you mean guest kernels which
> support the Xen PV environment ?
Yes. I mean Paravirtualized Guest.
Since before this patch these options are supported by PV guest only (to do
direct kernel boot), now HVM guest also supports them, so I move them to the
general options. What I want to say here is:
PV guests can support direct kernel boot (as always), now HVM guests can
support direct kernel boot too but with limitations (qemu-xen with default
seabios).
>
> If so, what is the benefit of booting them "direct" but HVM, rather
> than simply booting them PV ?
>
> > =item B
> >
> > Run C to find the kernel image and ramdisk to use. Normally
> > C would be C, which is an emulation of
> > -grub/grub2/syslinux.
> > +grub/grub2/syslinux. Either B or B must be specified
> > +for PV guests.
>
> Surely using this should be possible for direct kernel boot too.
>
> > +/*
> > + * LIBXL_HAVE_BUILDINFO_HVM_DIRECT_KERNEL_BOOT
> > + *
> > + * If this is defined, then the libxl_domain_build_info structure will
> > + * contain hvm.kernel, hvm.cmdline and hvm.ramdisk. hvm.kernel is a string
> > + * to indicate kernel image location, hvm.ramdisk is a string to indicate
> > + * ramdisk location, hvm.cmdline is a string to indicate the paramters
> which
> > + * would be appened to kernel image.
>
> If we are going to do this then I think the kernel, cmdline and
> ramdisk (and bootloader) parameters shoudl be moved into the main part
> of the domain_build_info struct. This will involve a compatibility
> layer: temporarily (for at least one release) both will be supported
> so the IDL will have to have both and code inside libxl will have to
> honour either.
I see.
>
> The #define should then be
> LIBXL_HAVE_BUILD_INFO_CMDLINE
> or some such - because all libxl callers will want to update to the
> new API eventually.
>
> > +static char *parse_cmdline(XLU_Config *config)
> > +{
> > +char *cmdline = NULL;
> > +const char *root = NULL, *extra = NULL, *buf = NULL;
> > +
> > +xlu_cfg_get_string (config, "cmdline", &buf, 0);
> > +xlu_cfg_get_string (config, "root", &root, 0);
> > +xlu_cfg_get_string (config, "extra", &extra, 0);
> > +
> > +if (buf) {
> > +cmdline = strdup(buf);
> > +if (root || extra)
> > +fprintf(stderr, "Warning: ignoring deprecated root= and extra=
> >
> "
> > +"in favour of cmdline=\n");
>
> Why are you deprecating root= and extra= ?
Just following Ian Campbell's suggestion:
http://lists.gnu.org/archive/html/qemu-devel/2014-06/msg02909.html
Now, cmdline is actually 'root' plus 'extra', if there is no special future
usage,
one 'cmdline' seems to be better.
>
> They seem likely to be a useful distinction if we decide to add
> further default options in the future.
Don't know how they will be used. But it we do need them, sure we can keep
the current way.
Thanks,
Chunyan
>
> If you do want to deprecate them, you should make this very clear -
> probably by putting that change in a separate patch in your series.
>
> Thanks,
> Ian.
>
> ___
> Xen-devel mailing list
> xen-de...@lists.xen.org
> http://lists.xen.org/xen-devel
>
>
Re: [Qemu-devel] [PATCH 3/7] spapr: Refactor spapr_populate_memory()
On 21.06.2014 [13:06:53 +1000], Alexey Kardashevskiy wrote:
> On 06/21/2014 08:55 AM, Nishanth Aravamudan wrote:
> > On 16.06.2014 [17:53:49 +1000], Alexey Kardashevskiy wrote:
> >> Current QEMU does not support memoryless NUMA nodes.
> >> This prepares SPAPR for that.
> >>
> >> This moves 2 calls of spapr_populate_memory_node() into
> >> the existing loop which handles nodes other than than
> >> the first one.
> >>
> >> Signed-off-by: Alexey Kardashevskiy
> >> ---
> >> hw/ppc/spapr.c | 31 +++
> >> 1 file changed, 11 insertions(+), 20 deletions(-)
> >>
> >> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> >> index cb3a10a..666b676 100644
> >> --- a/hw/ppc/spapr.c
> >> +++ b/hw/ppc/spapr.c
> >> @@ -689,28 +689,13 @@ static void spapr_populate_memory_node(void *fdt,
> >> int nodeid, hwaddr start,
> >>
> >> static int spapr_populate_memory(sPAPREnvironment *spapr, void *fdt)
> >> {
> >> -hwaddr node0_size, mem_start, node_size;
> >> +hwaddr mem_start, node_size;
> >> int i;
> >>
> >> -/* memory node(s) */
> >> -if (nb_numa_nodes > 1 && node_mem[0] < ram_size) {
> >> -node0_size = node_mem[0];
> >> -} else {
> >> -node0_size = ram_size;
> >> -}
> >> -
> >> -/* RMA */
> >> -spapr_populate_memory_node(fdt, 0, 0, spapr->rma_size);
> >> -
> >> -/* RAM: Node 0 */
> >> -if (node0_size > spapr->rma_size) {
> >> -spapr_populate_memory_node(fdt, 0, spapr->rma_size,
> >> - node0_size - spapr->rma_size);
> >> -}
> >> -
> >> -/* RAM: Node 1 and beyond */
> >> -mem_start = node0_size;
> >> -for (i = 1; i < nb_numa_nodes; i++) {
> >> +for (i = 0, mem_start = 0; i < nb_numa_nodes; ++i) {
> >> +if (!node_mem[i]) {
> >> +continue;
> >> +}
> >
> > Doesn't this skip memoryless nodes? What actually puts the memoryless
> > node in the device-tree?
>
> It does skip.
>
> > And if you were to put them in, wouldn't spapr_populate_memory_node()
> > fail because we'd be creating two nodes with memory@XXX where XXX is the
> > same (starting address) for both?
>
> I cannot do this now - there is no way to tell from the command line where
> I want NUMA node memory start from so I'll end up with multiple nodes with
> the same name and QEMU won't start. When NUMA fixes reach upstream, I'll
> try to work out something on top of that.
Ah I got something here. With the patches I just sent to enable sparse
NUMA nodes, plus your series rebased on top, here's what I see in a
Linux LPAR:
qemu-system-ppc64 -machine pseries,accel=kvm,usb=off -m 4096 -realtime
mlock=off -numa node,nodeid=3,mem=4096,cpus=2-3 -numa
node,nodeid=2,mem=0,cpus=0-1 -smp 4
info numa
2 nodes
node 2 cpus: 0 1
node 2 size: 0 MB
node 3 cpus: 2 3
node 3 size: 4096 MB
numactl --hardware
available: 3 nodes (0,2-3)
node 0 cpus:
node 0 size: 0 MB
node 0 free: 0 MB
node 2 cpus: 0 1
node 2 size: 0 MB
node 2 free: 0 MB
node 3 cpus: 2 3
node 3 size: 4073 MB
node 3 free: 3830 MB
node distances:
node 0 2 3
0: 10 40 40
2: 40 10 40
3: 40 40 10
The trick, it seems, is if you have a memoryless node, it needs to
have CPUs assigned to it. The CPU's "ibm,associativity" property will
make Linux set up the proper NUMA topology.
Thoughts? Should there be a check that every "present" NUMA node at
least either has CPUs or memory. It seems like if neither are present,
we can just hotplug them later? Does qemu support topology for PCI
devices?
Thanks,
Nish
Re: [Qemu-devel] [PATCH v6 for 2.1 00/10] Modify block jobs to use node-names
On Mon, 06/23 21:08, Stefan Hajnoczi wrote: > On Thu, Jun 19, 2014 at 12:26:00PM -0400, Jeff Cody wrote: > > On Thu, Jun 19, 2014 at 05:17:16PM +0800, Stefan Hajnoczi wrote: > > > On Tue, Jun 17, 2014 at 05:53:48PM -0400, Jeff Cody wrote: > > > Let's discuss this topic in a sub-thread and figure out what to do for > > > QEMU 2.1. This is an important issue to solve before the release > > > because we can't change QMP command semantics easily later. > > > > > > My questions are: > > > a. How do we fix resize, snapshot-sync, etc? It seems like we need to > > >propagate child op blockers. > > > > > > b. Is it a good idea to perform op blocker checks on the root node? > > >It's inconsistent with resize, snapshot-sync, etc. Permissions in > > >BDS graphs with multiple root nodes (e.g. guest device and NBD > > >run-time server) will be different depending on which root you > > >specify. > > > > I don't think (b) is the ultimate solution. It is used as a stop-gap > > because op blockers in the current implementation is essentially > > analogous to the in-use flag. But is it good enough for 2.1? If > > *everything* checks the topmost node in 2.1, then I think we are OK in > > all cases except where images files share a common BDS. > > Checking op blockers on the root node as a stop-gap is a good idea. > Let's apply it across all commands (e.g. snapshot-sync, resize). > > Fam pointed out that this approach is vulnerable to blockdev-add, where > blockers could be set/checked on an incomplete BDS graph (since you can > add new nodes on top). Do we need to move the blockers up the graph if > a new root node is inserted? My concern is if we allow adding new root on top, it's not easy to know the real root then. To give an example: If we have [base id=""] <- [active id="drive0" blockers=...] When user does (QMP) block-commit device="drive0" ... We should check drive0, which is OK. Then, assume user adds a new root on top, we would take care of moving the blockers: [base id=""] <- [active id="drive0"] <- [active id="drive1" blockers=] At this point, what if user does something on drive0 again? (QMP) block-commit device="drive0" ... The right thing to do is to check blockers on "drive1", since it's the real root now. But how do we know? Do we need to add a back reference pointer ->overlap_hd in BDS, or do we maintain a look up table, or do we search all BDS graphs to figure out? None is easier than if we put the blockers in the bottom BDS, in the first place: [base id="" blockers=...] <- [active id="drive0"] Even if user adds a new root, we don't need to worry about moving blockers, because the bottom is not changed. [base id="" blockers=...] <- [active id="drive0"] <- [active id="drive1"] Checking the blockers are easy, either for drive0 or drive1: just follow the backing chain until getting to the end. Fam
Re: [Qemu-devel] broken win32 coroutines (was Re: qemu 1.6.1)
Hi,
Thanks for adding me to this thread.
I am not familiar with the qemu source code but I am aware
of the coroutine crash and I can test a patch if you have
one.
qemu 1.5.1 was the last version I was able to build and use
on win32. Later versions build without error but exhibit the
coroutine crash.
qemu binaries built with the coroutine feature disabled are
to slow to use.
- Michael
On Mon, Jun 23, 2014 at 04:39:30PM +0200, Paolo Bonzini wrote:
> Il 26/10/2013 11:51, Stefan Weil ha scritto:
> > unpatched QEMU, crash with assertion
> >
> > 00448670 <_qemu_coroutine_switch>:
> > 448670: 53 push %ebx
> > 448671: 83 ec 18sub$0x18,%esp
> > 448674: c7 04 24 a8 62 6d 00movl $0x6d62a8,(%esp)
> > 44867b: 8b 5c 24 24 mov0x24(%esp),%ebx
> > 44867f: e8 ec 9e 27 00 call 6c2570
> > <___emutls_get_address>
> > 448684: 89 18 mov%ebx,(%eax)
> > 448686: 8b 44 24 28 mov0x28(%esp),%eax
> > 44868a: 89 43 24mov%eax,0x24(%ebx)
> > 44868d: 8b 43 20mov0x20(%ebx),%eax
> > 448690: 89 04 24mov%eax,(%esp)
> > 448693: ff 15 c0 5f 83 00 call *0x835fc0
> > 448699: 83 ec 04sub$0x4,%esp
> > 44869c: 8b 44 24 20 mov0x20(%esp),%eax
> > 4486a0: 8b 40 24mov0x24(%eax),%eax
> > 4486a3: 83 c4 18add$0x18,%esp
> > 4486a6: 5b pop%ebx
> > 4486a7: c3 ret
> >
> >
> > patched, works
> >
> > 00448620 <_qemu_coroutine_switch>:
> > 448620: 83 ec 1csub$0x1c,%esp
> > 448623: c7 04 24 a8 62 6d 00movl $0x6d62a8,(%esp)
> > 44862a: 89 5c 24 14 mov%ebx,0x14(%esp)
> > 44862e: 8b 5c 24 24 mov0x24(%esp),%ebx
> > 448632: 89 74 24 18 mov%esi,0x18(%esp)
> > 448636: e8 25 9f 27 00 call 6c2560
> > <___emutls_get_address>
> > 44863b: 8b 30 mov(%eax),%esi
> > 44863d: 89 18 mov%ebx,(%eax)
> > 44863f: 8b 44 24 28 mov0x28(%esp),%eax
> > 448643: 89 43 24mov%eax,0x24(%ebx)
> > 448646: 8b 43 20mov0x20(%ebx),%eax
> > 448649: 89 04 24mov%eax,(%esp)
> > 44864c: ff 15 c0 5f 83 00 call *0x835fc0
> > 448652: 8b 46 24mov0x24(%esi),%eax
> > 448655: 83 ec 04sub$0x4,%esp
> > 448658: 8b 5c 24 14 mov0x14(%esp),%ebx
> > 44865c: 8b 74 24 18 mov0x18(%esp),%esi
> > 448660: 83 c4 1cadd$0x1c,%esp
> > 448663: c3 ret
>
> The only difference here is basically that "from" is being saved in
> %esi across the calls to __emutls_get_address and SwitchToFiber. But
> as Peter found out, the fix really happens because now the compiler
> will not inline qemu_coroutine_switch anymore.
>
> Peter provided another dump, this time from Win64. It is the
> coroutine_trampoline with inlined qemu_coroutine_switch:
>
>0: push %rdi
>1: push %rsi
>2: push %rbx
>3: sub$0x30,%rsp
>7: mov%rcx,%rbx
>a: lea...(%rip),%rcx
> 11: mov...(%rip),%rax
> 18: mov%rax,0x28(%rsp)
> 1d: xor%eax,%eax
> 1f: callq ___emutls_get_address
> 24: mov...(%rip),%rsi
> 2b: mov%rax,%rdi
> 2e: nop2
> 30: mov0x8(%rbx),%rcx# ecx = co->entry_arg
> 34: callq *(%rbx) # co->entry(co->entry_arg);
> 36: mov0x10(%rbx),%rax # load co->caller
> 3a: mov%rax,(%rdi) # "current" = co->caller; (wrong
> current!)
> 3d: movl $0x2,0x48(%rax) # co->caller->action =
> COROUTINE_TERMINATE;
> 44: mov0x40(%rax),%rcx # load co->caller->fiber
> 48: callq *%rsi
> 4a: jmp30
> 4c: nopl 0x0(%rax)
>
> Some offsets are incomplete because this is from a .o, so not
> linked, but it's already enough to see that ___emutls_get_address
> (from "current = to_;" in qemu_coroutine_switch) is being hoisted
> outside the loop, which becomes:
>
> Coroutine *co = co_;
> Coroutine **p_current = ¤t;
>
> while (true) {
> co->entry(co->entry_arg);
>
> CoroutineWin32 *from = DO_UPCAST(CoroutineWin32, base, co);
> CoroutineWin32 *to = DO_UPCAST(CoroutineWin32, base, co->caller);
>
> *p_current = to_;
>
> to->action = action;
> SwitchToFiber(to->fiber);
> return from->action;
> }
>
> This is wrong if co->entry yields out of the coroutine which is then
> restarted on another th
Re: [Qemu-devel] [PATCH trivial v2] block.c: Add return value for bdrv_append_temp_snapshot() to avoid incorrect failure processing issue
On 06/24/2014 10:25 AM, Fam Zheng wrote:
> On Mon, 06/23 23:28, Chen Gang wrote:
>> When failure occurs, 'ret' need be set, or may return 0 to indicate success.
>
> s/need/needs/
>
>> And error_propagate() also need be called only one time within a function.
>
> s/need/needs/
>
For a grammar in my learning, in this situation, 'need' is not a verb,
"be set" and "be called" are the verb used with passive mode.
>>
>> It is abnormal to prevent bdrv_append_temp_snapshot() return value but still
>> set errp when error occurs -- although it contents return value internally.
>
> s/contents/has/
>
OK, thanks, and excuse me for my poor English. If necessary I shall send
patch v2 for it.
>>
>> So let bdrv_append_temp_snapshot() internal return value outside, and let
>> all things normal, then fix the issue too.
>>
>>
>> Signed-off-by: Chen Gang
>> ---
>> block.c | 7 ---
>> include/block/block.h | 2 +-
>> 2 files changed, 5 insertions(+), 4 deletions(-)
>>
>> diff --git a/block.c b/block.c
>> index 74af8d7..5e4fb60 100644
>> --- a/block.c
>> +++ b/block.c
>> @@ -1240,7 +1240,7 @@ done:
>> return ret;
>> }
>>
>> -void bdrv_append_temp_snapshot(BlockDriverState *bs, int flags, Error
>> **errp)
>> +int bdrv_append_temp_snapshot(BlockDriverState *bs, int flags, Error **errp)
>> {
>> /* TODO: extra byte is a hack to ensure MAX_PATH space on Windows. */
>> char *tmp_filename = g_malloc0(PATH_MAX + 1);
>> @@ -1258,6 +1258,7 @@ void bdrv_append_temp_snapshot(BlockDriverState *bs,
>> int flags, Error **errp)
>> /* Get the required size from the image */
>> total_size = bdrv_getlength(bs);
>> if (total_size < 0) {
>> +ret = total_size;
>> error_setg_errno(errp, -total_size, "Could not get image size");
>> goto out;
>> }
>> @@ -1304,6 +1305,7 @@ void bdrv_append_temp_snapshot(BlockDriverState *bs,
>> int flags, Error **errp)
>>
>> out:
>> g_free(tmp_filename);
>> +return ret;
>> }
>>
>> static QDict *parse_json_filename(const char *filename, Error **errp)
>> @@ -1495,9 +1497,8 @@ int bdrv_open(BlockDriverState **pbs, const char
>> *filename,
>> /* For snapshot=on, create a temporary qcow2 overlay. bs points to the
>> * temporary snapshot afterwards. */
>> if (snapshot_flags) {
>> -bdrv_append_temp_snapshot(bs, snapshot_flags, &local_err);
>> +ret = bdrv_append_temp_snapshot(bs, snapshot_flags, &local_err);
>> if (local_err) {
>> -error_propagate(errp, local_err);
>> goto close_and_fail;
>> }
>> }
>> diff --git a/include/block/block.h b/include/block/block.h
>> index f15b99b..7b3381c 100644
>> --- a/include/block/block.h
>> +++ b/include/block/block.h
>> @@ -217,7 +217,7 @@ int bdrv_open_image(BlockDriverState **pbs, const char
>> *filename,
>> bool allow_none, Error **errp);
>> void bdrv_set_backing_hd(BlockDriverState *bs, BlockDriverState
>> *backing_hd);
>> int bdrv_open_backing_file(BlockDriverState *bs, QDict *options, Error
>> **errp);
>> -void bdrv_append_temp_snapshot(BlockDriverState *bs, int flags, Error
>> **errp);
>> +int bdrv_append_temp_snapshot(BlockDriverState *bs, int flags, Error
>> **errp);
>
> Not used outside block.c, but pre-existing.
>
Yeah, originally, I considered about it, and finally still remained it
as pre-existing.
> Reviewed-by: Fam Zheng
>
Thank you for your work.
Thanks.
--
Chen Gang
Open, share, and attitude like air, water, and life which God blessed
Re: [Qemu-devel] [Xen-devel] [RFC PATCH V3 2/2] qemu: support xen hvm direct kernel boot
>>> On 6/23/2014 at 06:14 PM, in message
, Stefano
Stabellini wrote:
> On Sun, 22 Jun 2014, Chun Yan Liu wrote:
> > >>> On 6/20/2014 at 08:08 PM, in message
> > , Stefano
> > Stabellini wrote:
> > > On Fri, 20 Jun 2014, Chunyan Liu wrote:
> > > > qemu side patch to support xen HVM direct kernel boot:
> > > > if -kernel exists, calls xen_load_linux(), which will read
> > > > kernel/initrd
> > > > and add a linuxboot.bin or multiboot.bin option rom. The
> > > > linuxboot.bin/multiboot.bin will load kernel/initrd and jump to execute
> > > >
> > > > kernel directly. It's working when xen uses seabios.
> > > >
> > > > Signed-off-by: Chunyan Liu
> > > > ---
> > > > hw/i386/pc.c | 22 ++
> > > > hw/i386/pc_piix.c | 7 +++
> > > > hw/i386/xen/xen_apic.c | 1 +
> > > > include/hw/i386/pc.h | 5 +
> > > > 4 files changed, 35 insertions(+)
> > > >
> > > > diff --git a/hw/i386/pc.c b/hw/i386/pc.c
> > > > index 3e0ecf1..e005136 100644
> > > > --- a/hw/i386/pc.c
> > > > +++ b/hw/i386/pc.c
> > > > @@ -1183,6 +1183,28 @@ void pc_acpi_init(const char *default_dsdt)
> > > > }
> > > > }
> > > >
> > > > +FWCfgState *xen_load_linux(const char *kernel_filename,
> > > > + const char *kernel_cmdline,
> > > > + const char *initrd_filename,
> > > > + ram_addr_t below_4g_mem_size,
> > > > + PcGuestInfo *guest_info)
> > > > +{
> > > > +int i;
> > > > +FWCfgState *fw_cfg;
> > > > +
> > > > +assert(kernel_filename != NULL);
> > > > +
> > > > +fw_cfg = fw_cfg_init(BIOS_CFG_IOPORT, BIOS_CFG_IOPORT + 1, 0, 0);
> > >
> > > Is it actually OK to initialize just BIOS_CFG_IOPORT and avoid
> > > everything else currently done in bochs_bios_init?
> > > Would it make sense to simply call bochs_bios_init?
> > >
> > >
> > > > +rom_set_fw(fw_cfg);
> > > > +
> > > > +load_linux(fw_cfg, kernel_filename, initrd_filename,
> > > > kernel_cmdline,
>
> > > below_4g_mem_size);
> > > > +for (i = 0; i < nb_option_roms; i++) {
> > > > +rom_add_option(option_rom[i].name, option_rom[i].bootindex);
> > > > +}
> > >
> > > Wouldn't this have the unintended consequence of possibly loading other
> > > option_roms into the guest memory?
> >
> > For xen, no.
> >
> > Before this patch, indeed there is another option_rom "kvmvapic" in the
> list,
> > but since the option_rom list is never loaded in xen case, that won't harm.
> >
> But
> > as expected, I think in xen case, it should not be in option_rom list at
> all, since
> > we won't expect to load it. In v1, I tried to check the option_rom name to
> > bypass it and load multiboot.bin/linuxboot.bin only.
> >
> > In comments, Paolo Bonzini suggests a better way, that is to add following
> line
> > in xen_apic_realize():
> > + s->vapic_control = 0;
> > In this way, "kvmvapic" won't be added to the option_rom list. I think that
> > should be the normal way for xen.
> >
> > So, since V2, I updated in this way.
> > http://lists.gnu.org/archive/html/qemu-devel/2014-06/msg00704.html
>
> I see. Paolo's suggestion is a good one.
>
> I would add an assert here to ensure that there is only one option
> rom named "linuxboot.bin", so that we can easily spot if something in
> the future breaks our assumptions.
Yes, that's better. Will update. Thanks!
>
>
> > >
> > >
> > > > +guest_info->fw_cfg = fw_cfg;
> > > > +return fw_cfg;
> > > > +}
> > > > +
> > > > FWCfgState *pc_memory_init(MemoryRegion *system_memory,
> > > > const char *kernel_filename,
> > > > const char *kernel_cmdline,
> > > > diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
> > > > index a48e263..b737868 100644
> > > > --- a/hw/i386/pc_piix.c
> > > > +++ b/hw/i386/pc_piix.c
> > > > @@ -158,6 +158,13 @@ static void pc_init1(MachineState *machine,
> > > > machine->initrd_filename,
> > > > below_4g_mem_size, above_4g_mem_size,
> > > > rom_memory, &ram_memory, guest_info);
> > > > +} else if (machine->kernel_filename != NULL) {
> > > > +/* For xen HVM direct kernel boot, load linux here */
> > > > +fw_cfg = xen_load_linux(machine->kernel_filename,
> > > > +machine->kernel_cmdline,
> > > > +machine->initrd_filename,
> > > > +below_4g_mem_size,
> > > > +guest_info);
> > > > }
> > > >
> > > > gsi_state = g_malloc0(sizeof(*gsi_state));
> > > > diff --git a/hw/i386/xen/xen_apic.c b/hw/i386/xen/xen_apic.c
> > > > index 63bb7f7..f5acd6a 100644
> > > > --- a/hw/i386
Re: [Qemu-devel] [PATCH trivial v2] block.c: Add return value for bdrv_append_temp_snapshot() to avoid incorrect failure processing issue
On Mon, 06/23 23:28, Chen Gang wrote:
> When failure occurs, 'ret' need be set, or may return 0 to indicate success.
s/need/needs/
> And error_propagate() also need be called only one time within a function.
s/need/needs/
>
> It is abnormal to prevent bdrv_append_temp_snapshot() return value but still
> set errp when error occurs -- although it contents return value internally.
s/contents/has/
>
> So let bdrv_append_temp_snapshot() internal return value outside, and let
> all things normal, then fix the issue too.
>
>
> Signed-off-by: Chen Gang
> ---
> block.c | 7 ---
> include/block/block.h | 2 +-
> 2 files changed, 5 insertions(+), 4 deletions(-)
>
> diff --git a/block.c b/block.c
> index 74af8d7..5e4fb60 100644
> --- a/block.c
> +++ b/block.c
> @@ -1240,7 +1240,7 @@ done:
> return ret;
> }
>
> -void bdrv_append_temp_snapshot(BlockDriverState *bs, int flags, Error **errp)
> +int bdrv_append_temp_snapshot(BlockDriverState *bs, int flags, Error **errp)
> {
> /* TODO: extra byte is a hack to ensure MAX_PATH space on Windows. */
> char *tmp_filename = g_malloc0(PATH_MAX + 1);
> @@ -1258,6 +1258,7 @@ void bdrv_append_temp_snapshot(BlockDriverState *bs,
> int flags, Error **errp)
> /* Get the required size from the image */
> total_size = bdrv_getlength(bs);
> if (total_size < 0) {
> +ret = total_size;
> error_setg_errno(errp, -total_size, "Could not get image size");
> goto out;
> }
> @@ -1304,6 +1305,7 @@ void bdrv_append_temp_snapshot(BlockDriverState *bs,
> int flags, Error **errp)
>
> out:
> g_free(tmp_filename);
> +return ret;
> }
>
> static QDict *parse_json_filename(const char *filename, Error **errp)
> @@ -1495,9 +1497,8 @@ int bdrv_open(BlockDriverState **pbs, const char
> *filename,
> /* For snapshot=on, create a temporary qcow2 overlay. bs points to the
> * temporary snapshot afterwards. */
> if (snapshot_flags) {
> -bdrv_append_temp_snapshot(bs, snapshot_flags, &local_err);
> +ret = bdrv_append_temp_snapshot(bs, snapshot_flags, &local_err);
> if (local_err) {
> -error_propagate(errp, local_err);
> goto close_and_fail;
> }
> }
> diff --git a/include/block/block.h b/include/block/block.h
> index f15b99b..7b3381c 100644
> --- a/include/block/block.h
> +++ b/include/block/block.h
> @@ -217,7 +217,7 @@ int bdrv_open_image(BlockDriverState **pbs, const char
> *filename,
> bool allow_none, Error **errp);
> void bdrv_set_backing_hd(BlockDriverState *bs, BlockDriverState *backing_hd);
> int bdrv_open_backing_file(BlockDriverState *bs, QDict *options, Error
> **errp);
> -void bdrv_append_temp_snapshot(BlockDriverState *bs, int flags, Error
> **errp);
> +int bdrv_append_temp_snapshot(BlockDriverState *bs, int flags, Error **errp);
Not used outside block.c, but pre-existing.
Reviewed-by: Fam Zheng
Fam
> int bdrv_open(BlockDriverState **pbs, const char *filename,
>const char *reference, QDict *options, int flags,
>BlockDriver *drv, Error **errp);
> --
> 1.7.11.7
[Qemu-devel] [PATCH v1 7/7] target-arm: Call the pmccntr_sync function when swapping ELs
Call the new pmccntr_sync() function when there is a possibility
of swapping ELs (I.E. when there is an exception)
Signed-off-by: Alistair Francis
---
target-arm/helper-a64.c |5 +
target-arm/helper.c |7 +++
target-arm/op_helper.c |6 ++
3 files changed, 18 insertions(+), 0 deletions(-)
diff --git a/target-arm/helper-a64.c b/target-arm/helper-a64.c
index 2b4ce6a..b61174f 100644
--- a/target-arm/helper-a64.c
+++ b/target-arm/helper-a64.c
@@ -446,6 +446,8 @@ void aarch64_cpu_do_interrupt(CPUState *cs)
target_ulong addr = env->cp15.vbar_el[1];
int i;
+pmccntr_sync(env);
+
if (arm_current_pl(env) == 0) {
if (env->aarch64) {
addr += 0x400;
@@ -484,6 +486,7 @@ void aarch64_cpu_do_interrupt(CPUState *cs)
addr += 0x100;
break;
default:
+pmccntr_sync(env);
cpu_abort(cs, "Unhandled exception 0x%x\n", cs->exception_index);
}
@@ -511,4 +514,6 @@ void aarch64_cpu_do_interrupt(CPUState *cs)
env->pc = addr;
cs->interrupt_request |= CPU_INTERRUPT_EXITTB;
+
+pmccntr_sync(env);
}
diff --git a/target-arm/helper.c b/target-arm/helper.c
index 3e0a9a1..7c0a57f 100644
--- a/target-arm/helper.c
+++ b/target-arm/helper.c
@@ -3417,6 +3417,8 @@ void arm_cpu_do_interrupt(CPUState *cs)
assert(!IS_M(env));
+pmccntr_sync(env);
+
arm_log_exception(cs->exception_index);
/* TODO: Vectored interrupt controller. */
@@ -3447,6 +3449,7 @@ void arm_cpu_do_interrupt(CPUState *cs)
&& (env->uncached_cpsr & CPSR_M) != ARM_CPU_MODE_USR) {
env->regs[0] = do_arm_semihosting(env);
qemu_log_mask(CPU_LOG_INT, "...handled as semihosting call\n");
+pmccntr_sync(env);
return;
}
}
@@ -3465,6 +3468,7 @@ void arm_cpu_do_interrupt(CPUState *cs)
env->regs[15] += 2;
env->regs[0] = do_arm_semihosting(env);
qemu_log_mask(CPU_LOG_INT, "...handled as semihosting call\n");
+pmccntr_sync(env);
return;
}
}
@@ -3508,6 +3512,7 @@ void arm_cpu_do_interrupt(CPUState *cs)
offset = 4;
break;
default:
+pmccntr_sync(env);
cpu_abort(cs, "Unhandled exception 0x%x\n", cs->exception_index);
return; /* Never happens. Keep compiler happy. */
}
@@ -3540,6 +3545,8 @@ void arm_cpu_do_interrupt(CPUState *cs)
env->regs[14] = env->regs[15] + offset;
env->regs[15] = addr;
cs->interrupt_request |= CPU_INTERRUPT_EXITTB;
+
+pmccntr_sync(env);
}
/* Check section/page access permissions.
diff --git a/target-arm/op_helper.c b/target-arm/op_helper.c
index 9c1ef52..07ab30b 100644
--- a/target-arm/op_helper.c
+++ b/target-arm/op_helper.c
@@ -376,6 +376,8 @@ void HELPER(exception_return)(CPUARMState *env)
uint32_t spsr = env->banked_spsr[spsr_idx];
int new_el, i;
+pmccntr_sync(env);
+
if (env->pstate & PSTATE_SP) {
env->sp_el[cur_el] = env->xregs[31];
} else {
@@ -418,6 +420,8 @@ void HELPER(exception_return)(CPUARMState *env)
env->pc = env->elr_el[cur_el];
}
+pmccntr_sync(env);
+
return;
illegal_return:
@@ -433,6 +437,8 @@ illegal_return:
spsr &= PSTATE_NZCV | PSTATE_DAIF;
spsr |= pstate_read(env) & ~(PSTATE_NZCV | PSTATE_DAIF);
pstate_write(env, spsr);
+
+pmccntr_sync(env);
}
/* ??? Flag setting arithmetic is awkward because we need to do comparisons.
--
1.7.1
[Qemu-devel] [PATCH v1 6/7] target-arm: Implement pmccfiltr_write function
This is the function that is called when writing to the
PMCCFILTR_EL0 register
Signed-off-by: Alistair Francis
---
target-arm/helper.c |9 +
1 files changed, 9 insertions(+), 0 deletions(-)
diff --git a/target-arm/helper.c b/target-arm/helper.c
index bbd4b23..3e0a9a1 100644
--- a/target-arm/helper.c
+++ b/target-arm/helper.c
@@ -624,6 +624,14 @@ static void pmccntr_write(CPUARMState *env, const
ARMCPRegInfo *ri,
}
#endif
+static void pmccfiltr_write(CPUARMState *env, const ARMCPRegInfo *ri,
+uint64_t value)
+{
+pmccntr_sync(env);
+env->cp15.pmccfiltr_el0 = value & 0x7E00;
+pmccntr_sync(env);
+}
+
static void pmcntenset_write(CPUARMState *env, const ARMCPRegInfo *ri,
uint64_t value)
{
@@ -786,6 +794,7 @@ static const ARMCPRegInfo v7_cp_reginfo[] = {
#endif
{ .name = "PMCCFILTR_EL0", .state = ARM_CP_STATE_AA64,
.opc0 = 3, .opc1 = 3, .crn = 14, .crm = 15, .opc2 = 7,
+ .writefn = pmccfiltr_write,
.access = PL0_RW, .accessfn = pmreg_access,
.state = ARM_CP_STATE_AA64,
.resetvalue = 0,
--
1.7.1
[Qemu-devel] [PATCH v1 5/7] target-arm: Remove old code and replace with new functions
Remove the old PMCCNTR code and replace it with calls to the new
pmccntr_sync() function and the CCNT_ENABLED macro
Signed-off-by: Alistair Francis
---
target-arm/helper.c | 27 ---
1 files changed, 4 insertions(+), 23 deletions(-)
diff --git a/target-arm/helper.c b/target-arm/helper.c
index 15169c4..bbd4b23 100644
--- a/target-arm/helper.c
+++ b/target-arm/helper.c
@@ -569,20 +569,7 @@ void pmccntr_sync(CPUARMState *env)
static void pmcr_write(CPUARMState *env, const ARMCPRegInfo *ri,
uint64_t value)
{
-uint64_t temp_ticks;
-
-temp_ticks = muldiv64(qemu_clock_get_us(QEMU_CLOCK_VIRTUAL),
- get_ticks_per_sec(), 100);
-
-if (env->cp15.c9_pmcr & PMCRE) {
-/* If the counter is enabled */
-if (env->cp15.c9_pmcr & PMCRD) {
-/* Increment once every 64 processor clock cycles */
-env->cp15.c15_ccnt = (temp_ticks/64) - env->cp15.c15_ccnt;
-} else {
-env->cp15.c15_ccnt = temp_ticks - env->cp15.c15_ccnt;
-}
-}
+pmccntr_sync(env);
if (value & PMCRC) {
/* The counter has been reset */
@@ -593,20 +580,14 @@ static void pmcr_write(CPUARMState *env, const
ARMCPRegInfo *ri,
env->cp15.c9_pmcr &= ~0x39;
env->cp15.c9_pmcr |= (value & 0x39);
-if (env->cp15.c9_pmcr & PMCRE) {
-if (env->cp15.c9_pmcr & PMCRD) {
-/* Increment once every 64 processor clock cycles */
-temp_ticks /= 64;
-}
-env->cp15.c15_ccnt = temp_ticks - env->cp15.c15_ccnt;
-}
+pmccntr_sync(env);
}
static uint64_t pmccntr_read(CPUARMState *env, const ARMCPRegInfo *ri)
{
uint64_t total_ticks;
-if (!(env->cp15.c9_pmcr & PMCRE)) {
+if (!CCNT_ENABLED(env)) {
/* Counter is disabled, do not change value */
return env->cp15.c15_ccnt;
}
@@ -626,7 +607,7 @@ static void pmccntr_write(CPUARMState *env, const
ARMCPRegInfo *ri,
{
uint64_t total_ticks;
-if (!(env->cp15.c9_pmcr & PMCRE)) {
+if (!CCNT_ENABLED(env)) {
/* Counter is disabled, set the absolute value */
env->cp15.c15_ccnt = value;
return;
--
1.7.1
[Qemu-devel] [PATCH v1 2/7] target-arm: Implement PMCCNTR_EL0 and related registers
This patch adds support for the ARMv8 version of the PMCCNTR and
related registers. It also starts to implement the PMCCFILTR_EL0
register.
Signed-off-by: Peter Crosthwaite
Signed-off-by: Alistair Francis
---
target-arm/cpu.h|1 +
target-arm/helper.c | 39 +++
2 files changed, 40 insertions(+), 0 deletions(-)
diff --git a/target-arm/cpu.h b/target-arm/cpu.h
index cd1c7b6..6a2efd8 100644
--- a/target-arm/cpu.h
+++ b/target-arm/cpu.h
@@ -224,6 +224,7 @@ typedef struct CPUARMState {
* was reset. Otherwise it stores the counter value
*/
uint64_t c15_ccnt;
+uint64_t pmccfiltr_el0; /* Performance Monitor Filter Register */
} cp15;
struct {
diff --git a/target-arm/helper.c b/target-arm/helper.c
index ac10564..ce986ee 100644
--- a/target-arm/helper.c
+++ b/target-arm/helper.c
@@ -738,7 +738,22 @@ static const ARMCPRegInfo v7_cp_reginfo[] = {
.writefn = pmcntenset_write,
.accessfn = pmreg_access,
.raw_writefn = raw_write },
+{ .name = "PMCNTENSET_EL0", .crn = 9, .crm = 12, .opc0 = 3, .opc1 = 3,
+ .opc2 = 1, .access = PL0_RW, .resetvalue = 0,
+ .state = ARM_CP_STATE_AA64,
+ .fieldoffset = offsetof(CPUARMState, cp15.c9_pmcnten),
+ .writefn = pmcntenset_write,
+ .accessfn = pmreg_access,
+ .raw_writefn = raw_write },
{ .name = "PMCNTENCLR", .cp = 15, .crn = 9, .crm = 12, .opc1 = 0, .opc2 =
2,
+ .access = PL0_RW,
+ .fieldoffset = offsetof(CPUARMState, cp15.c9_pmcnten),
+ .accessfn = pmreg_access,
+ .writefn = pmcntenclr_write,
+ .type = ARM_CP_NO_MIGRATE },
+{ .name = "PMCNTENCLR_EL0", .crn = 9, .crm = 12, .opc0 = 3, .opc1 = 3,
+ .opc2 = 2,
+ .state = ARM_CP_STATE_AA64,
.access = PL0_RW, .fieldoffset = offsetof(CPUARMState, cp15.c9_pmcnten),
.accessfn = pmreg_access,
.writefn = pmcntenclr_write,
@@ -762,11 +777,25 @@ static const ARMCPRegInfo v7_cp_reginfo[] = {
.access = PL0_RW, .resetvalue = 0, .type = ARM_CP_IO,
.readfn = pmccntr_read, .writefn = pmccntr_write,
.accessfn = pmreg_access },
+{ .name = "PMCCNTR_EL0", .cp = 15, .crn = 9, .crm = 13, .opc1 = 3,
+ .opc2 = 0,
+ .access = PL0_RW, .resetvalue = 0, .type = ARM_CP_IO,
+ .state = ARM_CP_STATE_AA64,
+ .readfn = pmccntr_read, .writefn = pmccntr_write,
+ .accessfn = pmreg_access },
#endif
+{ .name = "PMCCFILTR_EL0", .state = ARM_CP_STATE_AA64,
+ .opc0 = 3, .opc1 = 3, .crn = 14, .crm = 15, .opc2 = 7,
+ .access = PL0_RW, .accessfn = pmreg_access,
+ .state = ARM_CP_STATE_AA64,
+ .resetvalue = 0,
+ .type = ARM_CP_IO,
+ .fieldoffset = offsetof(CPUARMState, cp15.pmccfiltr_el0), },
{ .name = "PMXEVTYPER", .cp = 15, .crn = 9, .crm = 13, .opc1 = 0, .opc2 =
1,
.access = PL0_RW,
.fieldoffset = offsetof(CPUARMState, cp15.c9_pmxevtyper),
.accessfn = pmreg_access, .writefn = pmxevtyper_write,
+ .resetvalue = 0,
.raw_writefn = raw_write },
/* Unimplemented, RAZ/WI. */
{ .name = "PMXEVCNTR", .cp = 15, .crn = 9, .crm = 13, .opc1 = 0, .opc2 = 2,
@@ -2324,6 +2353,16 @@ void register_cp_regs_for_features(ARMCPU *cpu)
.raw_writefn = raw_write,
};
define_one_arm_cp_reg(cpu, &pmcr);
+ARMCPRegInfo pmcr64 = {
+.name = "PMCR_EL0", .crn = 9, .crm = 12, .opc0 = 3, .opc1 = 3,
+.opc2 = 0,
+.state = ARM_CP_STATE_AA64,
+.access = PL0_RW, .resetvalue = cpu->midr & 0xff00,
+.fieldoffset = offsetof(CPUARMState, cp15.c9_pmcr),
+.accessfn = pmreg_access, .writefn = pmcr_write,
+.raw_writefn = raw_write,
+};
+define_one_arm_cp_reg(cpu, &pmcr64);
#endif
ARMCPRegInfo clidr = {
.name = "CLIDR", .state = ARM_CP_STATE_BOTH,
--
1.7.1
[Qemu-devel] [PATCH v1 4/7] target-arm: Implement pmccntr_sync function
This is used to synchronise the PMCCNTR counter and swap its
state between enabled and disabled if required. It must always
be called twice, both before and after any logic that could
change the state of the PMCCNTR counter.
Signed-off-by: Alistair Francis
---
Remembering that the c15_ccnt register stores the last time
the counter was reset if enabled. If disabled it stores the
counter value (when it was disabled).
The three use cases are as below:
--
Starts enabled/disabled and is staying enabled/disabled
--
The two calls to pmccntr_sync cancel each other out.
Each call implements this logic:
env->cp15.c15_ccnt = temp_ticks - env->cp15.c15_ccnt
Which expands to:
env->cp15.c15_ccnt = temp_ticks -
(temp_ticks - env->cp15.c15_ccnt)
env->cp15.c15_ccnt = env->cp15.c15_ccnt
--
Starts enabled, gets disabled
--
The logic is run during the first call while during
the second call it is not. That means that c15_ccnt
changes from storing the last time the counter was
reset, to storing the absolute value of the counter.
--
Starts disabled, gets enabled
--
During the fist call no changes are made, while during
the second call the register is changed. This changes it
from storing the absolute value to storing the last time
the counter was reset.
target-arm/cpu.h| 11 +++
target-arm/helper.c | 19 +++
2 files changed, 30 insertions(+), 0 deletions(-)
diff --git a/target-arm/cpu.h b/target-arm/cpu.h
index 31aa09c..0984eda 100644
--- a/target-arm/cpu.h
+++ b/target-arm/cpu.h
@@ -1051,6 +1051,17 @@ static inline bool cp_access_ok(int current_pl,
}
/**
+ * pmccntr_sync
+ * @cpu: ARMCPU
+ *
+ * Syncronises the counter in the PMCCNTR. This must always be called twice,
+ * once before any action that might effect the timer and again afterwards.
+ * The fucntion is used to swap the state of the register if required.
+ * This only happens when not in user mode (!CONFIG_USER_ONLY)
+ */
+void pmccntr_sync(CPUARMState *env);
+
+/**
* write_list_to_cpustate
* @cpu: ARMCPU
*
diff --git a/target-arm/helper.c b/target-arm/helper.c
index ce986ee..15169c4 100644
--- a/target-arm/helper.c
+++ b/target-arm/helper.c
@@ -546,6 +546,25 @@ static CPAccessResult pmreg_access(CPUARMState *env, const
ARMCPRegInfo *ri)
return CP_ACCESS_OK;
}
+void pmccntr_sync(CPUARMState *env)
+{
+#ifndef CONFIG_USER_ONLY
+uint64_t temp_ticks;
+
+temp_ticks = muldiv64(qemu_clock_get_us(QEMU_CLOCK_VIRTUAL),
+ get_ticks_per_sec(), 100);
+
+if (env->cp15.c9_pmcr & PMCRD) {
+/* Increment once every 64 processor clock cycles */
+temp_ticks /= 64;
+}
+
+if (CCNT_ENABLED(env)) {
+env->cp15.c15_ccnt = temp_ticks - env->cp15.c15_ccnt;
+}
+#endif
+}
+
#ifndef CONFIG_USER_ONLY
static void pmcr_write(CPUARMState *env, const ARMCPRegInfo *ri,
uint64_t value)
--
1.7.1
[Qemu-devel] [PATCH v1 1/7] target-arm: Make the ARM PMCCNTR register 64-bit
This makes the PMCCNTR register 64-bit to allow for the
64-bit ARMv8 version
Signed-off-by: Peter Crosthwaite
Signed-off-by: Alistair Francis
---
target-arm/cpu.h|2 +-
target-arm/helper.c | 19 +--
2 files changed, 10 insertions(+), 11 deletions(-)
diff --git a/target-arm/cpu.h b/target-arm/cpu.h
index 369d472..cd1c7b6 100644
--- a/target-arm/cpu.h
+++ b/target-arm/cpu.h
@@ -223,7 +223,7 @@ typedef struct CPUARMState {
/* If the counter is enabled, this stores the last time the counter
* was reset. Otherwise it stores the counter value
*/
-uint32_t c15_ccnt;
+uint64_t c15_ccnt;
} cp15;
struct {
diff --git a/target-arm/helper.c b/target-arm/helper.c
index ed4d2bb..ac10564 100644
--- a/target-arm/helper.c
+++ b/target-arm/helper.c
@@ -550,11 +550,10 @@ static CPAccessResult pmreg_access(CPUARMState *env,
const ARMCPRegInfo *ri)
static void pmcr_write(CPUARMState *env, const ARMCPRegInfo *ri,
uint64_t value)
{
-/* Don't computer the number of ticks in user mode */
-uint32_t temp_ticks;
+uint64_t temp_ticks;
-temp_ticks = qemu_clock_get_us(QEMU_CLOCK_VIRTUAL) *
- get_ticks_per_sec() / 100;
+temp_ticks = muldiv64(qemu_clock_get_us(QEMU_CLOCK_VIRTUAL),
+ get_ticks_per_sec(), 100);
if (env->cp15.c9_pmcr & PMCRE) {
/* If the counter is enabled */
@@ -586,15 +585,15 @@ static void pmcr_write(CPUARMState *env, const
ARMCPRegInfo *ri,
static uint64_t pmccntr_read(CPUARMState *env, const ARMCPRegInfo *ri)
{
-uint32_t total_ticks;
+uint64_t total_ticks;
if (!(env->cp15.c9_pmcr & PMCRE)) {
/* Counter is disabled, do not change value */
return env->cp15.c15_ccnt;
}
-total_ticks = qemu_clock_get_us(QEMU_CLOCK_VIRTUAL) *
- get_ticks_per_sec() / 100;
+total_ticks = muldiv64(qemu_clock_get_us(QEMU_CLOCK_VIRTUAL),
+ get_ticks_per_sec(), 100);
if (env->cp15.c9_pmcr & PMCRD) {
/* Increment once every 64 processor clock cycles */
@@ -606,7 +605,7 @@ static uint64_t pmccntr_read(CPUARMState *env, const
ARMCPRegInfo *ri)
static void pmccntr_write(CPUARMState *env, const ARMCPRegInfo *ri,
uint64_t value)
{
-uint32_t total_ticks;
+uint64_t total_ticks;
if (!(env->cp15.c9_pmcr & PMCRE)) {
/* Counter is disabled, set the absolute value */
@@ -614,8 +613,8 @@ static void pmccntr_write(CPUARMState *env, const
ARMCPRegInfo *ri,
return;
}
-total_ticks = qemu_clock_get_us(QEMU_CLOCK_VIRTUAL) *
- get_ticks_per_sec() / 100;
+total_ticks = muldiv64(qemu_clock_get_us(QEMU_CLOCK_VIRTUAL),
+ get_ticks_per_sec(), 100);
if (env->cp15.c9_pmcr & PMCRD) {
/* Increment once every 64 processor clock cycles */
--
1.7.1
[Qemu-devel] [PATCH v1 3/7] target-arm: Add helper macros and defines for CCNT register
Include a helper function to determine if the CCNT counter
is enabled as well as the constants used to mask the pmccfiltr_el0
register.
Signed-off-by: Alistair Francis
---
target-arm/cpu.h | 19 +++
1 files changed, 19 insertions(+), 0 deletions(-)
diff --git a/target-arm/cpu.h b/target-arm/cpu.h
index 6a2efd8..31aa09c 100644
--- a/target-arm/cpu.h
+++ b/target-arm/cpu.h
@@ -111,6 +111,25 @@ typedef struct ARMGenericTimer {
#define GTIMER_VIRT 1
#define NUM_GTIMERS 2
+#ifndef CONFIG_USER_ONLY
+/* Definitions for the PMCCFILTR_EL0 and PMXEVTYPER registers */
+#define PMCP 0x8000
+#define PMCU 0x4000
+
+/* This implements the PMCCFILTR_EL0:P and U bits; the PMXEVTYPER:P and U
+ * bits and the c9_pmcr:E bit.
+ *
+ * It does not suppor the secure/non-secure componenets of the
+ * PMCCFILTR_EL0 register
+ */
+#define CCNT_ENABLED(env) \
+((env->cp15.c9_pmcr & PMCRE) && \
+!(env->cp15.pmccfiltr_el0 & PMCP && arm_current_pl(env) == 1) && \
+!(env->cp15.pmccfiltr_el0 & PMCU && arm_current_pl(env) == 0) && \
+!(env->cp15.c9_pmxevtyper & PMCP && arm_current_pl(env) == 1) && \
+!(env->cp15.c9_pmxevtyper & PMCU && arm_current_pl(env) == 0))
+#endif
+
typedef struct CPUARMState {
/* Regs for current mode. */
uint32_t regs[16];
--
1.7.1
[Qemu-devel] [PATCH v1 0/7] target-arm: Extend PMCCNTR for ARMv8
This patch series continues on from my original PMCCNTR patch work to extend the counter to be 64-bit and support for the PMCCFILTR_EL0 register which allows the counter to be disabled based on the current EL Alistair Francis (7): target-arm: Make the ARM PMCCNTR register 64-bit target-arm: Implement PMCCNTR_EL0 and related registers target-arm: Add helper macros and defines for CCNT register target-arm: Implement pmccntr_sync function target-arm: Remove old code and replace with new functions target-arm: Implement pmccfiltr_write function target-arm: Call the pmccntr_sync function when swapping ELs target-arm/cpu.h| 33 +- target-arm/helper-a64.c |5 ++ target-arm/helper.c | 114 ++ target-arm/op_helper.c |6 +++ 4 files changed, 127 insertions(+), 31 deletions(-)
[Qemu-devel] [question] Will qemu-2.0.0 be a longterm stable branch?
Hi, all Which version is best for commercial product, qemu-2.0.0 or other versions? Any advices? Thanks, Zhang Haoyu
[Qemu-devel] [RFC PATCH] ppc/spapr: support sparse NUMA node numbering
With generic sparse NUMA node parsing in place ("numa: enable sparse
node numbering"), ppc can be updated to iterate over only the
user-specified nodes.
qemu-system-ppc64 -machine pseries,accel=kvm,usb=off -m 4096
-realtime mlock=off -numa node,nodeid=3 -numa node,nodeid=2 -smp 4
Before:
info numa:
node 0 cpus: 0 2
node 0 size: 2048 MB
node 1 cpus: 1 3
node 1 size: 2048 MB
numactl --hardware:
available: 2 nodes (0-1)
node 0 cpus: 0 2
node 0 size: 2027 MB
node 0 free: 1875 MB
node 1 cpus: 1 3
node 1 size: 2045 MB
node 1 free: 1980 MB
node distances:
node 0 1
0: 10 40
1: 40 10
After:
info numa:
node 2 cpus: 0 2
node 2 size: 2048 MB
node 3 cpus: 1 3
node 3 size: 2048 MB
numactl --hardware:
available: 3 nodes (0,2-3)
node 0 cpus:
node 0 size: 0 MB
node 0 free: 0 MB
node 2 cpus: 0 2
node 2 size: 2027 MB
node 2 free: 1943 MB
node 3 cpus: 1 3
node 3 size: 2045 MB
node 3 free: 1904 MB
node distances:
node 0 2 3
0: 10 40 40
2: 40 10 40
3: 40 40 10
Note, the empty node 0 is due to the Linux kernel.
Signed-off-by: Nishanth Aravamudan
diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index 82f183f..d07857a 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -707,7 +707,10 @@ static int spapr_populate_memory(sPAPREnvironment *spapr,
void *fdt)
/* RAM: Node 1 and beyond */
mem_start = node0_size;
-for (i = 1; i < nb_numa_nodes; i++) {
+for (i = 1; i < max_numa_node; i++) {
+if (!numa_info[i].present) {
+continue;
+}
mem_reg_property[0] = cpu_to_be64(mem_start);
if (mem_start >= ram_size) {
node_size = 0;
Re: [Qemu-devel] machines and versions - why so many?
On 06/24/2014 01:16 AM, Markus Armbruster wrote: > Alexey Kardashevskiy writes: > >> Hi! >> >> I have been hearing recently that we (server PPC) should have more that >> just one pseries machine in QEMU because this is what everybody else does :) >> >> My current understanding is that multiple machines (like >> pc-i440fx-1.4..2.1, and many others) are needed: >> >> 1) for the -nodefaults case when a lot of devices are still created and >> there is no other way to configure them; >> >> in "pseries", only CPU + empty VIO + empty PCI buses are created, >> everything else can be created explicitly; nothing to tweak; >> >> 2) to enable/disable CPUID_EXT_xxx bits (saw in x86); >> >> in "pseries", there is a "compat" property on CPU and that seems to be >> enough; >> >> 3) for devices which are created explicitly and for which we want some >> capabilities be disabled and we do not want to bother about this every time >> we run QEMU; >> >> ok, this one makes some sense for "pseries" to have (and upcoming >> endianness register on VGA seems to be the case) but it seems that adding a >> "compat" or "feature" property to the VGA device (and other devices which >> deal with this kind of compatibility) is still more architecturally correct >> thing to do, and let libvirt deal with the rest. >> >> Since I (almost) always miss the bigger picture, what do I miss now? :) >> Thanks! > > Actually, the true reasons for versioned machine types are stable guest > ABI and migration compatibility. > > All the stuff you mentioned above (presence of devices, CPUID bits, > optional device capabilities) is guest ABI. > > For some machines such as PCs, we want to keep the guest ABI stable. A > release freezes the guest ABI. When we do something that affects it, we > take care to change only the current, unfrozen ABI, and not the > previously frozen ABIs. > > Versioned machine types let you pick a guest ABI. Typical usage is to > create a guest with the latest machine type, then stick to that machine > type forever. > > Migration turns "want to keep guest ABI stable" into "must keep guest > ABI stable", and adds "must keep migration format compatible". Ensured > by using the same versioned machine type on source and destination. As I mentioned in another mail, I am aware of migration :) It just does not seem right that when we choose some PC version, some device properties get tweaked, and these are not even devices which are always present on PC machine (like NEC USB) and this tweak would make perfect sense for other architectures but it is limited to PC anyway. > If you don't care for migration, you probably don't need to bother with > versioned machine types. > > If all you want is configure the board, check out machine options (the > prop=value,... you can specify in addition to the machine type). > -- Alexey
Re: [Qemu-devel] [PATCH v1 1/3] sPAPR: Implement PCI error injection RTAS calls
On Mon, Jun 23, 2014 at 06:18:37PM +0200, Alexander Graf wrote:
>
>On 23.06.14 04:22, Gavin Shan wrote:
>>The patch implements PCI error injection RTAS calls for sPAPR
>>platform, which are defined PAPR spec as follows. Those RTAS
>>calls are expected to be invoked in strict sequence of
>>"ibm,open-errinjct", "ibm,errinjct", "ibm,close-errinjct".
>>
>>- "ibm,open-errinjct": Apply for token to do error injection
>>- "ibm,close-errinjct": Free the token assigned previously
>>- "ibm,errinjct": Do error injection
>>
>>The patch defines constants used by error injection RTAS calls
>>and adds callback sPAPRPHBClass::format_errinjct_cmd, which is
>>going to be used like this way:
>>
>>- Error injection RTAS calls are received in spapr_rtas.c. If that's
>> "ibm,open-errinjct" or "ibm,close-errinjct", we handle it there
>> with the help of static counter "sPAPREnvironment::errinjct_token_cnt".
>> If the RTAS call is "ibm,errinjct", sanity check is done there and
>> just returns failure if the error type isn't PCI related. Otherwise,
>> the input parameters are figured out and route the request to
>> sPAPRPHBClass::format_errinjct_cmd.
>>- sPAPRPHBClass::format_errinjct_cmd translates the address (BUID, PE
>> number) to IOMMU group ID and then return the formated string back to
>> spapr_rtas.c where the string is writen to firmware sysfs file
>> "/sys/firmware/opal/errinjct" to do error injection.
>>
>>Signed-off-by: Gavin Shan
>>---
>> hw/ppc/spapr_rtas.c | 198
>>
>> include/hw/pci-host/spapr.h | 11 +++
>> include/hw/ppc/spapr.h | 35
>> 3 files changed, 244 insertions(+)
>>
>>diff --git a/hw/ppc/spapr_rtas.c b/hw/ppc/spapr_rtas.c
>>index a7233e4..94fb866 100644
>>--- a/hw/ppc/spapr_rtas.c
>>+++ b/hw/ppc/spapr_rtas.c
>>@@ -32,6 +32,7 @@
>> #include "hw/ppc/spapr.h"
>> #include "hw/ppc/spapr_vio.h"
>>+#include "hw/pci-host/spapr.h"
>> #include
>>@@ -268,6 +269,196 @@ static void rtas_ibm_set_system_parameter(PowerPCCPU
>>*cpu,
>> rtas_st(rets, 0, ret);
>> }
>>+static sPAPRErrInjct *rtas_find_errinjct_token(sPAPREnvironment *spapr,
>>+ uint32_t token_num)
>>+{
>>+sPAPRErrInjct *open_token;
>>+
>>+QLIST_FOREACH(open_token, &spapr->errinjct_open_tokens, list) {
>>+if (open_token->token == token_num) {
>>+return open_token;
>>+}
>>+}
>>+
>>+return NULL;
>>+}
>>+
>>+static void rtas_ibm_open_errinjct(PowerPCCPU *cpu,
>>+ sPAPREnvironment *spapr,
>>+ uint32_t token, uint32_t nargs,
>>+ target_ulong args, uint32_t nret,
>>+ target_ulong rets)
>>+{
>>+sPAPRErrInjct *open_token;
>>+int32_t ret = RTAS_OUT_HW_ERROR;
>>+
>>+/* Sanity check on number of arguments */
>>+if ((nargs != 0) || (nret != 2)) {
>>+goto out;
>>+}
>>+
>>+/* Allocate token */
>>+open_token = g_malloc(sizeof(*open_token));
>
>With this hypercall the guest can OOM us by simply allocating useless
>tokens.
>
Yeah, that's possible. I'll have the limitation that each QEMU process
only can have one token. Then we don't need allocate memory blocks to
maintain the tokens.
Thanks,
Gavin
>>+if (!open_token) {
>>+goto out;
>>+}
>>+open_token->token = spapr->errinjct_open_token_cnt++;
>>+QLIST_INSERT_HEAD(&spapr->errinjct_open_tokens, open_token, list);
>>+
>>+/* Success */
>>+rtas_st(rets, 0, open_token->token);
>>+ret = RTAS_OUT_SUCCESS;
>>+out:
>>+rtas_st(rets, 1, ret);
>>+}
>>+
>>+static char *rtas_do_errinjct_ioa(sPAPREnvironment *spapr,
>>+sPAPRErrInjctIOA *ioa)
>>+{
>>+sPAPRPHBState *sphb;
>>+sPAPRPHBClass *info;
>>+
>>+/* Sanity check on argument */
>>+if (!spapr || !ioa) {
>>+return NULL;
>>+}
>>+
>>+/* Invalid option ? */
>>+if (ioa->func > RTAS_ERRINJCT_IOA_DMA_WR_MEM_TARGET) {
>>+return NULL;
>>+}
>>+
>>+/* Find PHB */
>>+sphb = spapr_find_phb(spapr, ioa->buid);
>>+if (!sphb) {
>>+return NULL;
>>+}
>>+
>>+/* PHB doesn't support error injection ? */
>>+info = SPAPR_PCI_HOST_BRIDGE_GET_CLASS(sphb);
>>+if (!info->format_errinjct_cmd) {
>>+return NULL;
>>+}
>>+
>>+return info->format_errinjct_cmd(sphb, ioa);
>>+}
>>+
>>+static void rtas_ibm_errinjct(PowerPCCPU *cpu,
>>+ sPAPREnvironment *spapr,
>>+ uint32_t token, uint32_t nargs,
>>+ target_ulong args, uint32_t nret,
>>+ target_ulong rets)
>>+{
>>+sPAPRErrInjctIOA ioa;
>>+sPAPRErrInjct *open_token;
>>+uint32_t token_num, ei_token;
>>+target_ulong param_buf;
>>+char *pbuf;
>>+int fd;
>>+int32_t ret = RTAS_OUT_PARAM_ERROR;
>>+
>>+/* Sani
[Qemu-devel] [RFC PATCH v2] numa: enable sparse node numbering
On 23.06.2014 [12:33:10 -0700], Nishanth Aravamudan wrote:
> Add a present field to NodeInfo which indicates if a given nodeid was
> present on the command-line or not. Current code relies on a memory
> value being passed for a node to indicate presence, which is
> insufficient in the presence of memoryless nodes or sparse numbering.
> Adjust the iteration of various NUMA loops to use the maximum known NUMA
> ID rather than the number of NUMA nodes.
>
> numa.c::set_numa_nodes() has become a bit more convoluted for
> round-robin'ing the CPUs over known nodes when not specified by the
> user.
Sorry for the noise, but after staring at it for a bit, I think this
version is a bit cleaner for the set_numa_nodes() change.
Add a present field to NodeInfo which indicates if a given nodeid was
present on the command-line or not. Current code relies on a memory
value being passed for a node to indicate presence, which is
insufficient in the presence of memoryless nodes or sparse numbering.
Adjust the iteration of various NUMA loops to use the maximum known NUMA
ID rather than the number of NUMA nodes.
numa.c::set_numa_nodes() has become a bit more convoluted for
round-robin'ing the CPUs over known nodes when not specified by the
user.
Note that architecture-specific code still needs changes (forthcoming)
for both sparse node numbering and memoryless nodes.
Examples:
(1) qemu-system-x86_64 -enable-kvm -m 4096 -numa node,nodeid=3 -numa
node,nodeid=2 -smp 16
Before:
node 0 cpus: 0 2 4 6 8 10 12 14
node 0 size: 2048 MB
node 1 cpus: 1 3 5 7 9 11 13 15
node 1 size: 2048 MB
After:
node 2 cpus: 0 2 4 6 8 10 12 14 |
node 2 size: 2048 MB |
node 3 cpus: 1 3 5 7 9 11 13 15 |
node 3 size: 2048 MB
(2) qemu-system-x86_64 -enable-kvm -m 4096 -numa node,nodeid=0 -numa
node,nodeid=1 -numa node,nodeid=2 -numa node,nodeid=3 -smp 16
node 0 cpus: 0 4 8 12 |
node 0 size: 1024 MB |
node 1 cpus: 1 5 9 13 |
node 1 size: 1024 MB |
node 2 cpus: 2 6 10 14|
node 2 size: 1024 MB |
node 3 cpus: 3 7 11 15|
node 3 size: 1024 MB
(qemu) info numa
4 nodes
node 0 cpus: 0 4 8 12
node 0 size: 1024 MB
node 1 cpus: 1 5 9 13
node 1 size: 1024 MB
node 2 cpus: 2 6 10 14
node 2 size: 1024 MB
node 3 cpus: 3 7 11 15
node 3 size: 1024 MB
Signed-off-by: Nishanth Aravamudan
Cc: Eduardo Habkost
Cc: Alexey Kardashevskiy
Cc: Michael S. Tsirkin
Cc: qemu-devel@nongnu.org
---
Based off mst's for_upstream tag, which has the NodeInfo changes.
v1 -> v2:
Modify set_numa_nodes loop for round-robin'ing CPUs.
diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h
index 277230d..b90bf66 100644
--- a/include/sysemu/sysemu.h
+++ b/include/sysemu/sysemu.h
@@ -145,11 +145,13 @@ extern int mem_prealloc;
*/
#define MAX_CPUMASK_BITS 255
-extern int nb_numa_nodes;
+extern int nb_numa_nodes; /* Number of NUMA nodes */
+extern int max_numa_node; /* Highest specified NUMA node ID */
typedef struct node_info {
uint64_t node_mem;
DECLARE_BITMAP(node_cpu, MAX_CPUMASK_BITS);
struct HostMemoryBackend *node_memdev;
+bool present;
} NodeInfo;
extern NodeInfo numa_info[MAX_NODES];
void set_numa_nodes(void);
diff --git a/monitor.c b/monitor.c
index c7f8797..4721996 100644
--- a/monitor.c
+++ b/monitor.c
@@ -2003,7 +2003,10 @@ static void do_info_numa(Monitor *mon, const QDict
*qdict)
CPUState *cpu;
monitor_printf(mon, "%d nodes\n", nb_numa_nodes);
-for (i = 0; i < nb_numa_nodes; i++) {
+for (i = 0; i < max_numa_node; i++) {
+if (!numa_info[i].present) {
+continue;
+}
monitor_printf(mon, "node %d cpus:", i);
CPU_FOREACH(cpu) {
if (cpu->numa_node == i) {
diff --git a/numa.c b/numa.c
index e471afe..2a4c577 100644
--- a/numa.c
+++ b/numa.c
@@ -106,6 +106,10 @@ static void numa_node_parse(NumaNodeOptions *node,
QemuOpts *opts, Error **errp)
numa_info[nodenr].node_mem = object_property_get_int(o, "size", NULL);
numa_info[nodenr].node_memdev = MEMORY_BACKEND(o);
}
+numa_info[nodenr].present = true;
+if (nodenr >= max_numa_node) {
+max_numa_node = nodenr + 1;
+}
}
int numa_init_func(QemuOpts *opts, void *opaque)
@@ -155,7 +159,7 @@ void set_numa_nodes(void)
{
if (nb_numa_nodes > 0) {
uint64_t numa_total;
-int i;
+int i, j = -1;
if (nb_numa_nodes > MAX_NODES) {
nb_numa_nodes = MAX_NODES;
@@ -164,27 +168,29 @@ void set_numa_nodes(void)
[Qemu-devel] [PATCH 1/4] spapr: Add rtas_st_buffer utility function
Add a function to write lengh + data into a buffer as required for the
emulation of the RTAS ibm,get-system-parameter call.
If the destination is smaller than the source, the write is truncated
and success is returned. This matches the behaviour of pHyp.
This will be used in following patches.
Signed-off-by: Sam Bobroff
---
include/hw/ppc/spapr.h | 17 +
1 file changed, 17 insertions(+)
diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h
index 08c301f..39a7764 100644
--- a/include/hw/ppc/spapr.h
+++ b/include/hw/ppc/spapr.h
@@ -373,6 +373,23 @@ static inline void rtas_st(target_ulong phys, int n,
uint32_t val)
stl_be_phys(&address_space_memory, ppc64_phys_to_real(phys + 4*n), val);
}
+
+static inline void rtas_st_buffer(target_ulong phys, target_ulong phys_len,
+ uint8_t *buffer, uint16_t buffer_len)
+{
+uint16_t i;
+
+if (phys_len < 2) {
+return;
+}
+stw_be_phys(&address_space_memory,
+ppc64_phys_to_real(phys), buffer_len);
+for (i = 0; i < MIN(buffer_len, phys_len - 2); i++) {
+stb_phys(&address_space_memory,
+ ppc64_phys_to_real(phys + 2 + i), buffer[i]);
+}
+}
+
typedef void (*spapr_rtas_fn)(PowerPCCPU *cpu, sPAPREnvironment *spapr,
uint32_t token,
uint32_t nargs, target_ulong args,
--
1.9.0
[Qemu-devel] [PATCH 4/4] spapr: Add RTAS sysparm SPLPAR Characteristics
Add support for the SPLPAR Characteristics parameter to the emulated
RTAS call ibm,get-system-parameter.
The support provides just enough information to allow "cat
/proc/powerpc/lparcfg" to succeed without generating a kernel error
message.
Without this patch the above command will produce the following kernel
message: arch/powerpc/platforms/pseries/lparcfg.c \
parse_system_parameter_string Error calling get-system-parameter \
(0xfffd)
Signed-off-by: Sam Bobroff
---
hw/ppc/spapr_rtas.c | 13 +
1 file changed, 13 insertions(+)
diff --git a/hw/ppc/spapr_rtas.c b/hw/ppc/spapr_rtas.c
index 8d94845..4270e7a 100644
--- a/hw/ppc/spapr_rtas.c
+++ b/hw/ppc/spapr_rtas.c
@@ -224,6 +224,7 @@ static void rtas_stop_self(PowerPCCPU *cpu,
sPAPREnvironment *spapr,
env->msr = 0;
}
+#define SPLPAR_CHARACTERISTICS 20
#define DIAGNOSTICS_RUN_MODE42
#define UUID48
@@ -238,8 +239,20 @@ static void rtas_ibm_get_system_parameter(PowerPCCPU *cpu,
target_ulong length = rtas_ld(args, 2);
target_ulong ret = RTAS_OUT_SUCCESS;
uint8_t zero = 0;
+uint8_t param_buf[64];
+int param_len;
switch (parameter) {
+case SPLPAR_CHARACTERISTICS:
+param_len = snprintf((char *)param_buf, sizeof param_buf,
+ "MaxEntCap=%d,MaxPlatProcs=%d",
+ max_cpus, smp_cpus);
+if (param_len >= 0) {
+rtas_st_buffer(buffer, length, param_buf, param_len);
+} else {
+ret = RTAS_OUT_HW_ERROR;
+}
+break;
case DIAGNOSTICS_RUN_MODE:
rtas_st_buffer(buffer, length, &zero, sizeof zero);
break;
--
1.9.0
[Qemu-devel] [PATCH 3/4] spapr: Fix RTAS sysparm DIAGNOSTICS_RUN_MODE
This allows the ibm,get-system-parameter RTAS call to succeed for the
DIAGNOSTICS_RUN_MODE system parameter.
The problem can be seen with "ppc64_cpu --run-mode" from the
powerpc-utils package which fails before this patch with "Machine does
not support diagnostic run mode".
This is corrected by using the rtas_st_buffer() function to write to
the buffer.
The function return value code is also slightly simplified.
Signed-off-by: Sam Bobroff
---
hw/ppc/spapr_rtas.c | 11 +--
1 file changed, 5 insertions(+), 6 deletions(-)
diff --git a/hw/ppc/spapr_rtas.c b/hw/ppc/spapr_rtas.c
index 4f87673..8d94845 100644
--- a/hw/ppc/spapr_rtas.c
+++ b/hw/ppc/spapr_rtas.c
@@ -236,19 +236,18 @@ static void rtas_ibm_get_system_parameter(PowerPCCPU *cpu,
target_ulong parameter = rtas_ld(args, 0);
target_ulong buffer = rtas_ld(args, 1);
target_ulong length = rtas_ld(args, 2);
-target_ulong ret = RTAS_OUT_NOT_SUPPORTED;
+target_ulong ret = RTAS_OUT_SUCCESS;
+uint8_t zero = 0;
switch (parameter) {
case DIAGNOSTICS_RUN_MODE:
-if (length == 1) {
-rtas_st(buffer, 0, 0);
-ret = RTAS_OUT_SUCCESS;
-}
+rtas_st_buffer(buffer, length, &zero, sizeof zero);
break;
case UUID:
rtas_st_buffer(buffer, length, qemu_uuid, (qemu_uuid_set ? 16 : 0));
-ret = RTAS_OUT_SUCCESS;
break;
+default:
+ret = RTAS_OUT_NOT_SUPPORTED;
}
rtas_st(rets, 0, ret);
--
1.9.0
[Qemu-devel] [PATCH 0/4] spapr: improve RTAS get_system_parameter
This series of patches improves QEMU's emulation of the RTAS call "ibm,get-system-parameter" by adding support for the "UUID" and "SPLPAR Characteristics" parameters as well as fixing a problem in the the implementation of the "Platform Processor Diagnostics Run Mode" parameter. It also improves the surrounding code a little to support the addition of additional system parameters. Sam Bobroff (4): spapr: Add rtas_st_buffer utility function spapr: Add RTAS sysparm UUID spapr: Fix RTAS sysparm DIAGNOSTICS_RUN_MODE spapr: Add RTAS sysparm SPLPAR Characteristics hw/ppc/spapr_rtas.c| 28 +++- include/hw/ppc/spapr.h | 17 + 2 files changed, 40 insertions(+), 5 deletions(-) -- 1.9.0
[Qemu-devel] [PATCH 2/4] spapr: Add RTAS sysparm UUID
Add support for the UUID parameter to the emulated RTAS call
ibm,get-system-parameter.
Return the guest's UUID as the value for the RTAS UUID system
parameter, or null (a zero length result) if it is not set.
Signed-off-by: Sam Bobroff
---
hw/ppc/spapr_rtas.c | 6 ++
1 file changed, 6 insertions(+)
diff --git a/hw/ppc/spapr_rtas.c b/hw/ppc/spapr_rtas.c
index ea4a2b2..4f87673 100644
--- a/hw/ppc/spapr_rtas.c
+++ b/hw/ppc/spapr_rtas.c
@@ -225,6 +225,7 @@ static void rtas_stop_self(PowerPCCPU *cpu,
sPAPREnvironment *spapr,
}
#define DIAGNOSTICS_RUN_MODE42
+#define UUID48
static void rtas_ibm_get_system_parameter(PowerPCCPU *cpu,
sPAPREnvironment *spapr,
@@ -244,6 +245,10 @@ static void rtas_ibm_get_system_parameter(PowerPCCPU *cpu,
ret = RTAS_OUT_SUCCESS;
}
break;
+case UUID:
+rtas_st_buffer(buffer, length, qemu_uuid, (qemu_uuid_set ? 16 : 0));
+ret = RTAS_OUT_SUCCESS;
+break;
}
rtas_st(rets, 0, ret);
@@ -260,6 +265,7 @@ static void rtas_ibm_set_system_parameter(PowerPCCPU *cpu,
switch (parameter) {
case DIAGNOSTICS_RUN_MODE:
+case UUID:
ret = RTAS_OUT_NOT_AUTHORIZED;
break;
}
--
1.9.0
[Qemu-devel] [RFC 06/14] vga: 15 and 16bpp draw functions are "swapping" only
We now only use vga_draw_line15() and vga_draw_line16() when
byteswapping. Otherwise we have a shared surface. Make this
explicit and remove the reliance on lduw_p which uses
TARGET_WORDS_ENDIAN. We now only rely on the "byteswap" variable
in vga_draw_graphic().
Signed-off-by: Benjamin Herrenschmidt
---
hw/display/vga.c | 22 +-
hw/display/vga_template.h | 23 ---
2 files changed, 29 insertions(+), 16 deletions(-)
diff --git a/hw/display/vga.c b/hw/display/vga.c
index e98f7da..198e192 100644
--- a/hw/display/vga.c
+++ b/hw/display/vga.c
@@ -1342,8 +1342,8 @@ enum {
VGA_DRAW_LINE4D2,
VGA_DRAW_LINE8D2,
VGA_DRAW_LINE8,
-VGA_DRAW_LINE15,
-VGA_DRAW_LINE16,
+VGA_DRAW_LINE15_SWAP,
+VGA_DRAW_LINE16_SWAP,
VGA_DRAW_LINE_NB,
};
@@ -1354,8 +1354,8 @@ static vga_draw_line_func * const
vga_draw_line_table[VGA_DRAW_LINE_NB] = {
vga_draw_line4d2,
vga_draw_line8d2,
vga_draw_line8,
-vga_draw_line15,
-vga_draw_line16,
+vga_draw_line15_swap,
+vga_draw_line16_swap,
};
static int vga_get_bpp(VGACommonState *s)
@@ -1537,12 +1537,15 @@ static void vga_draw_graphic(VGACommonState *s, int
full_update)
bits = 8;
break;
case 15:
-v = VGA_DRAW_LINE15;
-bits = 16;
-break;
case 16:
-v = VGA_DRAW_LINE16;
-bits = 16;
+if (byteswap) {
+v = depth == 15 ? VGA_DRAW_LINE15_SWAP : VGA_DRAW_LINE16_SWAP;
+} else if (!is_buffer_shared(surface)) {
+fprintf(stderr,
+"vga: Non-shared surface at %dbpp unsupported !\n",
depth);
+return;
+}
+bits = depth;
break;
case 24:
case 32:
@@ -1552,6 +1555,7 @@ static void vga_draw_graphic(VGACommonState *s, int
full_update)
return;
}
bits = depth;
+break;
}
}
if (!is_buffer_shared(surface)) {
diff --git a/hw/display/vga_template.h b/hw/display/vga_template.h
index e7cd9e0..7a44771 100644
--- a/hw/display/vga_template.h
+++ b/hw/display/vga_template.h
@@ -281,18 +281,28 @@ static void vga_draw_line8(VGACommonState *s1, uint8_t *d,
/* XXX: optimize */
+/* We have lduw_he_p, lduw_be_p, lduw_le_p but we don't have
+ * lduw_sw_p, as in unconditionally swap :) So let's make it
+ * up here
+ */
+#if defined(HOST_WORDS_BIGENDIAN)
+#define ld_sw_pixel16 lduw_le_p
+#else
+#define ld_sw_pixel16 lduw_be_p
+#endif
+
/*
* 15 bit color
*/
-static void vga_draw_line15(VGACommonState *s1, uint8_t *d,
-const uint8_t *s, int width)
+static void vga_draw_line15_swap(VGACommonState *s1, uint8_t *d,
+ const uint8_t *s, int width)
{
int w;
uint32_t v, r, g, b;
w = width;
do {
-v = lduw_p((void *)s);
+v = ld_sw_pixel16((void *)s);
r = (v >> 7) & 0xf8;
g = (v >> 2) & 0xf8;
b = (v << 3) & 0xf8;
@@ -305,15 +315,15 @@ static void vga_draw_line15(VGACommonState *s1, uint8_t
*d,
/*
* 16 bit color
*/
-static void vga_draw_line16(VGACommonState *s1, uint8_t *d,
-const uint8_t *s, int width)
+static void vga_draw_line16_swap(VGACommonState *s1, uint8_t *d,
+ const uint8_t *s, int width)
{
int w;
uint32_t v, r, g, b;
w = width;
do {
-v = lduw_p((void *)s);
+v = ld_sw_pixel16((void *)s);
r = (v >> 8) & 0xf8;
g = (v >> 3) & 0xfc;
b = (v << 3) & 0xf8;
@@ -322,4 +332,3 @@ static void vga_draw_line16(VGACommonState *s1, uint8_t *d,
d += 4;
} while (--w != 0);
}
-
--
1.9.1
[Qemu-devel] [RFC 02/14] ui: Remove unused QEMU_BIG_ENDIAN_FLAG
If we need to, we should use the pixman formats instead but for
now this is unused except in commented out code so take it out
to avoid further confusion about surface endianness.
Signed-off-by: Benjamin Herrenschmidt
---
include/ui/console.h | 3 +--
ui/console.c | 7 ---
ui/vnc-enc-tight.c | 12
3 files changed, 5 insertions(+), 17 deletions(-)
diff --git a/include/ui/console.h b/include/ui/console.h
index 0a59b8d..e4487b5 100644
--- a/include/ui/console.h
+++ b/include/ui/console.h
@@ -102,8 +102,7 @@ struct QemuConsoleClass {
ObjectClass parent_class;
};
-#define QEMU_BIG_ENDIAN_FLAG0x01
-#define QEMU_ALLOCATED_FLAG 0x02
+#define QEMU_ALLOCATED_FLAG 0x01
struct PixelFormat {
uint8_t bits_per_pixel;
diff --git a/ui/console.c b/ui/console.c
index bb304fc..cd31375 100644
--- a/ui/console.c
+++ b/ui/console.c
@@ -1236,9 +1236,6 @@ static void qemu_alloc_display(DisplaySurface *surface,
int width, int height)
assert(surface->image != NULL);
surface->flags = QEMU_ALLOCATED_FLAG;
-#ifdef HOST_WORDS_BIGENDIAN
-surface->flags |= QEMU_BIG_ENDIAN_FLAG;
-#endif
}
DisplaySurface *qemu_create_displaysurface(int width, int height)
@@ -1263,10 +1260,6 @@ DisplaySurface *qemu_create_displaysurface_from(int
width, int height,
(void *)data, linesize);
assert(surface->image != NULL);
-#ifdef HOST_WORDS_BIGENDIAN
-surface->flags = QEMU_BIG_ENDIAN_FLAG;
-#endif
-
return surface;
}
diff --git a/ui/vnc-enc-tight.c b/ui/vnc-enc-tight.c
index f02352c..3d1b5cd 100644
--- a/ui/vnc-enc-tight.c
+++ b/ui/vnc-enc-tight.c
@@ -220,8 +220,7 @@ tight_detect_smooth_image24(VncState *vs, int w, int h)
unsigned int errors;\
unsigned char *buf = vs->tight.tight.buffer;\
\
-endian = 0; /* FIXME: ((vs->clientds.flags & QEMU_BIG_ENDIAN_FLAG) != \
- (vs->ds->surface->flags & QEMU_BIG_ENDIAN_FLAG)); */ \
+endian = 0; /* FIXME */ \
\
\
max[0] = vs->client_pf.rmax; \
@@ -563,8 +562,7 @@ tight_filter_gradient24(VncState *vs, uint8_t *buf, int w,
int h)
buf32 = (uint32_t *)buf;
memset(vs->tight.gradient.buffer, 0, w * 3 * sizeof(int));
-if (1 /* FIXME: (vs->clientds.flags & QEMU_BIG_ENDIAN_FLAG) ==
- (vs->ds->surface->flags & QEMU_BIG_ENDIAN_FLAG) */) {
+if (1 /* FIXME */) {
shift[0] = vs->client_pf.rshift;
shift[1] = vs->client_pf.gshift;
shift[2] = vs->client_pf.bshift;
@@ -621,8 +619,7 @@ tight_filter_gradient24(VncState *vs, uint8_t *buf, int w,
int h)
\
memset (vs->tight.gradient.buffer, 0, w * 3 * sizeof(int)); \
\
-endian = 0; /* FIXME: ((vs->clientds.flags & QEMU_BIG_ENDIAN_FLAG) != \
- (vs->ds->surface->flags & QEMU_BIG_ENDIAN_FLAG)); */ \
+endian = 0; /* FIXME */ \
\
max[0] = vs->client_pf.rmax;\
max[1] = vs->client_pf.gmax;\
@@ -898,8 +895,7 @@ static void tight_pack24(VncState *vs, uint8_t *buf, size_t
count, size_t *ret)
buf32 = (uint32_t *)buf;
-if (1 /* FIXME: (vs->clientds.flags & QEMU_BIG_ENDIAN_FLAG) ==
- (vs->ds->surface->flags & QEMU_BIG_ENDIAN_FLAG) */) {
+if (1 /* FIXME */) {
rshift = vs->client_pf.rshift;
gshift = vs->client_pf.gshift;
bshift = vs->client_pf.bshift;
--
1.9.1
Re: [Qemu-devel] [PATCH v1 1/3] sPAPR: Implement PCI error injection RTAS calls
On Mon, Jun 23, 2014 at 11:13:45PM +0200, Alexander Graf wrote: > >> Am 23.06.2014 um 23:03 schrieb Benjamin Herrenschmidt >> : >> >>> On Mon, 2014-06-23 at 18:18 +0200, Alexander Graf wrote: >>> Device emulation code shouldn't even remotely have an idea what host >>> it's running on. Also semantically there are a few issues with this approach >>> >>> 1) QEMU is usually running with user privileges, so it doesn't have >>> access to the file above >> >> Right, this needs to go via VFIO like the rest of the EEH stuff >> >>> 2) QEMU's channel to hardware devices is via normal kernel API. For >>> physical devices that's VFIO. No side channels please. >> >> Indeed. If the user gets access to that file, suddenly qemu can >> "manufacture" a bad string and error inject in other devices it doesn't >> own which isn't great. >> >> Gavin, this needs to go via the same path as normal EEH and be limited >> to injecting errors that are completely bounded to the PE. >> >> I don't think this is very high priority. We should first write a good >> host side error injection tool and sort out the reporting of the EEH log >> from host to guest. >> >>> 3) Ownership of the question whether a PE is in error mode is >>> responsibility of the PHB. In the emulated case, the PHB would have to >>> set itself into a mode where it behaves as if it's blocked. >> >> We don't have to support error injection for emulated since we don't >> support (yet) the rest oF EEH for them. We could one day but it's >> really not urgent. > >I agree, but the layers are the same ;) > Thanks, Ben and Alex. Yes, it's fair enough for VFIO (ioctl cmd) to routing the PCI error injection. Thanks, Gavin >Alex > >> >> Cheers, >> Ben. >>
Re: [Qemu-devel] [PATCH] linux-user: fix ipc(SEMCTL, ...) argument handling
On Mon, Jun 23, 2014 at 11:42:14PM +0100, Peter Maydell wrote: > On 23 June 2014 23:36, Paul Burton wrote: > > Actually no, I don't think you're right about that afterall. The > > argument union itself is never modified. I imagine if it were then it > > would be painful in the case of the semctl syscall where the union is > > passed directly as an argument, rather than as a pointer as it is for > > the ipc syscall. > > > > What may be modified is the data pointed to by the pointers within union > > semun. That is already handled by do_semctl & the translate functions it > > calls. > > Except if you look at do_semctl you see code like: > case GETVAL: > case SETVAL: > arg.val = tswap32(target_su.val); > ret = get_errno(semctl(semid, semnum, cmd, arg)); > target_su.val = tswap32(arg.val); > break; > > which clearly is just modifying fields in the target_semun union. > So something's wrong (probably that code)... Yes, both Linux & man semctl agree that GETVAL returns the value of the semaphore as the return value of the syscall. So I believe the assignment to target_su.val there is (functionally harmless) garbage. Thanks, Paul signature.asc Description: Digital signature
[Qemu-devel] [RFC 08/14] vga: Simplify vga_draw_blank() a bit
The test for surface_bits_per_pixel() isn't necessary anymore,
the 8bpp case never happens.
Signed-off-by: Benjamin Herrenschmidt
---
hw/display/vga.c | 9 ++---
1 file changed, 2 insertions(+), 7 deletions(-)
diff --git a/hw/display/vga.c b/hw/display/vga.c
index 70405e6..3b2cca5 100644
--- a/hw/display/vga.c
+++ b/hw/display/vga.c
@@ -1639,7 +1639,7 @@ static void vga_draw_graphic(VGACommonState *s, int
full_update)
static void vga_draw_blank(VGACommonState *s, int full_update)
{
DisplaySurface *surface = qemu_console_surface(s->con);
-int i, w, val;
+int i, w;
uint8_t *d;
if (!full_update)
@@ -1647,15 +1647,10 @@ static void vga_draw_blank(VGACommonState *s, int
full_update)
if (s->last_scr_width <= 0 || s->last_scr_height <= 0)
return;
-if (surface_bits_per_pixel(surface) == 8) {
-val = rgb_to_pixel32(0, 0, 0);
-} else {
-val = 0;
-}
w = s->last_scr_width * surface_bytes_per_pixel(surface);
d = surface_data(surface);
for(i = 0; i < s->last_scr_height; i++) {
-memset(d, val, w);
+memset(d, 0, w);
d += surface_stride(surface);
}
dpy_gfx_update(s->con, 0, 0,
--
1.9.1
[Qemu-devel] [RFC 07/14] vga: Remove rgb_to_pixel indirection
We always use rgb_to_pixel32 nowadays.
Signed-off-by: Benjamin Herrenschmidt
---
hw/display/cirrus_vga.c | 15 +--
hw/display/vga.c| 31 ++-
hw/display/vga_int.h| 2 --
3 files changed, 19 insertions(+), 29 deletions(-)
diff --git a/hw/display/cirrus_vga.c b/hw/display/cirrus_vga.c
index 6fbe39d..ec465e3 100644
--- a/hw/display/cirrus_vga.c
+++ b/hw/display/cirrus_vga.c
@@ -29,6 +29,7 @@
#include "hw/hw.h"
#include "hw/pci/pci.h"
#include "ui/console.h"
+#include "ui/pixel_ops.h"
#include "vga_int.h"
#include "hw/loader.h"
@@ -2212,6 +2213,8 @@ static void cirrus_cursor_draw_line(VGACommonState *s1,
uint8_t *d1, int scr_y)
} else {
src += (s->vga.sr[0x13] & 0x3f) * 256;
src += (scr_y - s->hw_cursor_y) * 4;
+
+
poffset = 128;
content = ((uint32_t *)src)[0] |
((uint32_t *)(src + 128))[0];
@@ -2229,12 +2232,12 @@ static void cirrus_cursor_draw_line(VGACommonState *s1,
uint8_t *d1, int scr_y)
x2 = s->vga.last_scr_width;
w = x2 - x1;
palette = s->cirrus_hidden_palette;
-color0 = s->vga.rgb_to_pixel(c6_to_8(palette[0x0 * 3]),
- c6_to_8(palette[0x0 * 3 + 1]),
- c6_to_8(palette[0x0 * 3 + 2]));
-color1 = s->vga.rgb_to_pixel(c6_to_8(palette[0xf * 3]),
- c6_to_8(palette[0xf * 3 + 1]),
- c6_to_8(palette[0xf * 3 + 2]));
+color0 = rgb_to_pixel32(c6_to_8(palette[0x0 * 3]),
+c6_to_8(palette[0x0 * 3 + 1]),
+c6_to_8(palette[0x0 * 3 + 2]));
+color1 = rgb_to_pixel32(c6_to_8(palette[0xf * 3]),
+c6_to_8(palette[0xf * 3 + 1]),
+c6_to_8(palette[0xf * 3 + 2]));
bpp = surface_bytes_per_pixel(surface);
d1 += x1 * bpp;
switch (surface_bits_per_pixel(surface)) {
diff --git a/hw/display/vga.c b/hw/display/vga.c
index 198e192..70405e6 100644
--- a/hw/display/vga.c
+++ b/hw/display/vga.c
@@ -979,13 +979,6 @@ typedef void vga_draw_line_func(VGACommonState *s1,
uint8_t *d,
#include "vga_template.h"
-static unsigned int rgb_to_pixel32_dup(unsigned int r, unsigned int g,
unsigned b)
-{
-unsigned int col;
-col = rgb_to_pixel32(r, g, b);
-return col;
-}
-
/* return true if the palette was modified */
static int update_palette16(VGACommonState *s)
{
@@ -1002,9 +995,9 @@ static int update_palette16(VGACommonState *s)
v = ((s->ar[VGA_ATC_COLOR_PAGE] & 0xc) << 4) | (v & 0x3f);
}
v = v * 3;
-col = s->rgb_to_pixel(c6_to_8(s->palette[v]),
- c6_to_8(s->palette[v + 1]),
- c6_to_8(s->palette[v + 2]));
+col = rgb_to_pixel32(c6_to_8(s->palette[v]),
+ c6_to_8(s->palette[v + 1]),
+ c6_to_8(s->palette[v + 2]));
if (col != palette[i]) {
full_update = 1;
palette[i] = col;
@@ -1024,13 +1017,13 @@ static int update_palette256(VGACommonState *s)
v = 0;
for(i = 0; i < 256; i++) {
if (s->dac_8bit) {
- col = s->rgb_to_pixel(s->palette[v],
-s->palette[v + 1],
-s->palette[v + 2]);
+ col = rgb_to_pixel32(s->palette[v],
+ s->palette[v + 1],
+ s->palette[v + 2]);
} else {
- col = s->rgb_to_pixel(c6_to_8(s->palette[v]),
-c6_to_8(s->palette[v + 1]),
-c6_to_8(s->palette[v + 2]));
+ col = rgb_to_pixel32(c6_to_8(s->palette[v]),
+ c6_to_8(s->palette[v + 1]),
+ c6_to_8(s->palette[v + 2]));
}
if (col != palette[i]) {
full_update = 1;
@@ -1213,7 +1206,6 @@ static void vga_draw_text(VGACommonState *s, int
full_update)
s->last_cw = cw;
full_update = 1;
}
-s->rgb_to_pixel = rgb_to_pixel32_dup;
full_update |= update_palette16(s);
palette = s->last_palette;
x_incr = cw * surface_bytes_per_pixel(surface);
@@ -1505,8 +1497,6 @@ static void vga_draw_graphic(VGACommonState *s, int
full_update)
dpy_gfx_replace_surface(s->con, surface);
}
-s->rgb_to_pixel = rgb_to_pixel32_dup;
-
if (shift_control == 0) {
full_update |= update_palette16(s);
if (s->sr[VGA_SEQ_CLOCK_MODE] & 8) {
@@ -1657,9 +1647,8 @@ static void vga_draw_blank(VGACommonState *s, int
full_update)
if (s->last_scr_width <= 0 || s->last_scr_height <= 0)
return;
-s->rgb_to_pixel = rgb_to_pixel32_dup;
if (surface_bits_per_pixel(surface) == 8) {
-val = s->rgb_to_pixel(0, 0, 0);
+val = rgb_to_pixel32(0, 0, 0);
} else {
val = 0;
}
diff --
[Qemu-devel] [RFC 09/14] cirrus: Remove non-32bpp cursor drawing
We only draw cursor on non-shared surfaces (so it seems...) and
these are always 32bpp
Signed-off-by: Benjamin Herrenschmidt
---
hw/display/cirrus_vga.c | 64 +---
hw/display/cirrus_vga_template.h | 102 ---
2 files changed, 36 insertions(+), 130 deletions(-)
delete mode 100644 hw/display/cirrus_vga_template.h
diff --git a/hw/display/cirrus_vga.c b/hw/display/cirrus_vga.c
index ec465e3..2e49a93 100644
--- a/hw/display/cirrus_vga.c
+++ b/hw/display/cirrus_vga.c
@@ -2171,20 +2171,44 @@ static void cirrus_cursor_invalidate(VGACommonState *s1)
}
}
-#define DEPTH 8
-#include "cirrus_vga_template.h"
-
-#define DEPTH 16
-#include "cirrus_vga_template.h"
-
-#define DEPTH 32
-#include "cirrus_vga_template.h"
+static void vga_draw_cursor_line(uint8_t *d1,
+ const uint8_t *src1,
+ int poffset, int w,
+ unsigned int color0,
+ unsigned int color1,
+ unsigned int color_xor)
+{
+const uint8_t *plane0, *plane1;
+int x, b0, b1;
+uint8_t *d;
+
+d = d1;
+plane0 = src1;
+plane1 = src1 + poffset;
+for (x = 0; x < w; x++) {
+b0 = (plane0[x >> 3] >> (7 - (x & 7))) & 1;
+b1 = (plane1[x >> 3] >> (7 - (x & 7))) & 1;
+switch (b0 | (b1 << 1)) {
+case 0:
+break;
+case 1:
+((uint32_t *)d)[0] ^= color_xor;
+break;
+case 2:
+((uint32_t *)d)[0] = color0;
+break;
+case 3:
+((uint32_t *)d)[0] = color1;
+break;
+}
+d += 4;
+}
+}
static void cirrus_cursor_draw_line(VGACommonState *s1, uint8_t *d1, int scr_y)
{
CirrusVGAState *s = container_of(s1, CirrusVGAState, vga);
-DisplaySurface *surface = qemu_console_surface(s->vga.con);
-int w, h, bpp, x1, x2, poffset;
+int w, h, x1, x2, poffset;
unsigned int color0, color1;
const uint8_t *palette, *src;
uint32_t content;
@@ -2238,24 +2262,8 @@ static void cirrus_cursor_draw_line(VGACommonState *s1,
uint8_t *d1, int scr_y)
color1 = rgb_to_pixel32(c6_to_8(palette[0xf * 3]),
c6_to_8(palette[0xf * 3 + 1]),
c6_to_8(palette[0xf * 3 + 2]));
-bpp = surface_bytes_per_pixel(surface);
-d1 += x1 * bpp;
-switch (surface_bits_per_pixel(surface)) {
-default:
-break;
-case 8:
-vga_draw_cursor_line_8(d1, src, poffset, w, color0, color1, 0xff);
-break;
-case 15:
-vga_draw_cursor_line_16(d1, src, poffset, w, color0, color1, 0x7fff);
-break;
-case 16:
-vga_draw_cursor_line_16(d1, src, poffset, w, color0, color1, 0x);
-break;
-case 32:
-vga_draw_cursor_line_32(d1, src, poffset, w, color0, color1, 0xff);
-break;
-}
+d1 += x1 * 4;
+vga_draw_cursor_line(d1, src, poffset, w, color0, color1, 0xff);
}
/***
diff --git a/hw/display/cirrus_vga_template.h b/hw/display/cirrus_vga_template.h
deleted file mode 100644
index 3b28280..000
--- a/hw/display/cirrus_vga_template.h
+++ /dev/null
@@ -1,102 +0,0 @@
-/*
- * QEMU Cirrus VGA Emulator templates
- *
- * Copyright (c) 2003 Fabrice Bellard
- *
- * 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.
- */
-
-#if DEPTH == 8
-#define BPP 1
-#elif DEPTH == 15 || DEPTH == 16
-#define BPP 2
-#elif DEPTH == 32
-#define BPP 4
-#else
-#error unsupported depth
-#endif
-
-static void glue(vga_draw_cursor_line_, DEPTH)(uint8_t *d1,
- const uint8_t *src1,
- int poffset, int w,
- unsigned int color0,
- unsigned int co
Re: [Qemu-devel] [PATCH] linux-user: fix ipc(SEMCTL, ...) argument handling
On Tue, Jun 24, 2014 at 12:21:42AM +0100, Peter Maydell wrote: > On 24 June 2014 00:06, Paul Burton wrote: > > On Mon, Jun 23, 2014 at 11:35:17PM +0100, Peter Maydell wrote: > >> and so I'm dubious about a patch that's > >> trying to make a very small change to it > > > > Isn't that precisely how good bisectable bug fixes should be made? > > The key is in the second half of this sentence: > > >> without looking > >> at the larger picture and testing and fixing bugs on more > >> than one architecture. > > > > I pointed you to the kernel code which dereferences the pointer & it's > > quite clearly architecture neutral, so I'm not sure what you're getting > > at with the architecture comment. > > > > There's quite clearly a bug in QEMU here, and this patch fixes it. I > > hope you're not saying you don't want it merged because it doesn't fix > > some other hypothetical bugs in the same area. > > What I mean is that I would expect that any attempt to fix > behaviour in this area ought to result in a series of three > or four patches which fix various bugs (of which this > would just be one). When an area of code is pretty > clearly bogus like this one, then in my experience an > attempt to make a small fix to it without just going ahead > and overhauling it is likely to result in accidentally > breaking existing working uses which happened to work > more or less by fluke. This is particularly true if you only > test one guest architecture; you can reasonably make that > level of limited testing in areas where the codebase is > sane, but not where it is clearly dubious. > > So yes, I would prefer this not to be merged unless either > (a) it comes as part of a series that cleans up the code for > handling semctl so it's not obviously broken > (b) it has been tested to confirm that it doesn't obviously > regress any guest architecture (or at least not any of the > more important ones), > and ideally both. > > I don't think this is an enormous amount of work (maybe a > couple of days?); I'm really just recommending the approach > and amount of cleanup that I would do if I found I needed > to make a fix in this area myself. Well I disagree with your logic, but perhaps that's primarily because of your claim that the semctl code is "clearly bogus" and "obviously broken". Could you back that up? I know there's the one bogus line in the GETVAL/SETVAL case that was mentioned in another email, but is there anything else you consider broken? I see your point on testing, but frankly this code is generic for all architectures in Linux. I don't have the time to test each architecture and I don't have the time to test all software that may have previously been working by fluke. So what's the bar you'd like to set here? I traced the issue with fakeroot then compared the code & behaviour of QEMU with that of Linux. I found a difference. I fixed it. I checked that the kernel code for this is architecture neutral. So as far as I'm aware this patch fixes a bug and QEMU would be better with this patch than it is without it. But anyway, please do enlighten me: where are the bugs of which you speak? I'd like to see them fixed too :) Thanks, Paul signature.asc Description: Digital signature
Re: [Qemu-devel] [RFC 13/14] vga: Add endian control register
On Tue, 2014-06-24 at 09:11 +1000, Benjamin Herrenschmidt wrote:
> Include the endian state in the migration stream as an optional
> subsection which we only include when the endian isn't the default,
> thus enabling backward compatibility of the common case.
Note: This description is a bit too terse, sorry about that
This is based on the proposed register addition that I've discussed
in a separate email, adding an "extended capabilities" register and
using it to expose the endian control.
Arguably, this could be split into 2 patches...
At this point however, I'd first want to make sure we all agree on the
approach before going down into the details.
I haven't been successful contacting the Bochs folks, either on their
mailing list or on IRC.
Ben.
> Signed-off-by: Benjamin Herrenschmidt
> ---
> hw/display/vga.c | 55
> ++--
> hw/display/vga_int.h | 15 +-
> 2 files changed, 63 insertions(+), 7 deletions(-)
>
> diff --git a/hw/display/vga.c b/hw/display/vga.c
> index 29d57cf..54b1fbe 100644
> --- a/hw/display/vga.c
> +++ b/hw/display/vga.c
> @@ -615,6 +615,11 @@ uint32_t vbe_ioport_read_data(void *opaque, uint32_t
> addr)
> }
> } else if (s->vbe_index == VBE_DISPI_INDEX_VIDEO_MEMORY_64K) {
> val = s->vram_size / (64 * 1024);
> +} else if (s->vbe_index == VBE_DISPI_INDEX_EXTENDED_CAPS) {
> +val = VBE_DISPI_HAS_ENDIAN_CTRL;
> +} else if (s->vbe_index == VBE_DISPI_INDEX_ENDIAN_CTRL) {
> +val = s->big_endian_fb ? VBE_DISPI_BIG_ENDIAN :
> +VBE_DISPI_LITTLE_ENDIAN;
> } else {
> val = 0;
> }
> @@ -634,7 +639,8 @@ void vbe_ioport_write_data(void *opaque, uint32_t addr,
> uint32_t val)
> {
> VGACommonState *s = opaque;
>
> -if (s->vbe_index <= VBE_DISPI_INDEX_NB) {
> +if (s->vbe_index <= VBE_DISPI_INDEX_NB ||
> +s->vbe_index == VBE_DISPI_INDEX_ENDIAN_CTRL) {
> #ifdef DEBUG_BOCHS_VBE
> printf("VBE: write index=0x%x val=0x%x\n", s->vbe_index, val);
> #endif
> @@ -737,7 +743,7 @@ void vbe_ioport_write_data(void *opaque, uint32_t addr,
> uint32_t val)
> s->bank_offset = 0;
> }
> s->dac_8bit = (val & VBE_DISPI_8BIT_DAC) > 0;
> -s->vbe_regs[s->vbe_index] = val;
> +s->vbe_regs[s->vbe_index] = val | VBE_DISPI_EXTCAPS;
> vga_update_memory_access(s);
> break;
> case VBE_DISPI_INDEX_VIRT_WIDTH:
> @@ -774,6 +780,9 @@ void vbe_ioport_write_data(void *opaque, uint32_t addr,
> uint32_t val)
> s->vbe_start_addr >>= 2;
> }
> break;
> + case VBE_DISPI_INDEX_ENDIAN_CTRL:
> +s->big_endian_fb = !!(val & VBE_DISPI_BIG_ENDIAN);
> +break;
> default:
> break;
> }
> @@ -1464,7 +1473,8 @@ static void vga_draw_graphic(VGACommonState *s, int
> full_update)
> if (s->line_offset != s->last_line_offset ||
> disp_width != s->last_width ||
> height != s->last_height ||
> -s->last_depth != depth) {
> +s->last_depth != depth ||
> +s->last_byteswap != byteswap) {
> if (depth == 32 || depth == 24 ||
> ((depth == 16 || depth == 15) && !byteswap)) {
> pixman_format_code_t format =
> @@ -1483,6 +1493,7 @@ static void vga_draw_graphic(VGACommonState *s, int
> full_update)
> s->last_height = height;
> s->last_line_offset = s->line_offset;
> s->last_depth = depth;
> +s->last_byteswap = byteswap;
> full_update = 1;
> } else if (is_buffer_shared(surface) &&
> (full_update || surface_data(surface) != s->vram_ptr
> @@ -1731,6 +1742,7 @@ void vga_common_reset(VGACommonState *s)
> s->vbe_index = 0;
> memset(s->vbe_regs, '\0', sizeof(s->vbe_regs));
> s->vbe_regs[VBE_DISPI_INDEX_ID] = VBE_DISPI_ID5;
> +s->vbe_regs[VBE_DISPI_INDEX_ENABLE] = VBE_DISPI_EXTCAPS;
> s->vbe_start_addr = 0;
> s->vbe_line_offset = 0;
> s->vbe_bank_mask = (s->vram_size >> 16) - 1;
> @@ -1751,6 +1763,7 @@ void vga_common_reset(VGACommonState *s)
> s->cursor_start = 0;
> s->cursor_end = 0;
> s->cursor_offset = 0;
> +s->big_endian_fb = s->default_endian_fb;
> memset(s->invalidated_y_table, '\0', sizeof(s->invalidated_y_table));
> memset(s->last_palette, '\0', sizeof(s->last_palette));
> memset(s->last_ch_attr, '\0', sizeof(s->last_ch_attr));
> @@ -1982,6 +1995,28 @@ static int vga_common_post_load(void *opaque, int
> version_id)
> return 0;
> }
>
> +static bool vga_endian_state_needed(void *opaque)
> +{
> +VGACommonState *s = opaque;
> +
> +/*
> + * Only send the endian state if it's different from the
> + * default one, thus ensuring backward compatibility for
> + * migration of the common case
> + */
> +return s->default_endian_fb != s->big_endian_fb;
>
Re: [Qemu-devel] [RFC 11/14] vga: Make fb endian a common state variable
On Tue, 2014-06-24 at 00:24 +0100, Peter Maydell wrote:
> On 24 June 2014 00:11, Benjamin Herrenschmidt
> wrote:
> > And initialize it based on target endian
> > @@ -155,6 +155,7 @@ typedef struct VGACommonState {
> > const GraphicHwOps *hw_ops;
> > bool full_update_text;
> > bool full_update_gfx;
> > +bool big_endian_fb;
>
> Don't we need to migrate this new state somehow?
Only when it can change, which is done in patch 13
This patch (11) just moves it around to the state but its value is still
fixed at this point in the series.
Cheers,
Ben.
[Qemu-devel] [RFC 13/14] vga: Add endian control register
Include the endian state in the migration stream as an optional
subsection which we only include when the endian isn't the default,
thus enabling backward compatibility of the common case.
Signed-off-by: Benjamin Herrenschmidt
---
hw/display/vga.c | 55 ++--
hw/display/vga_int.h | 15 +-
2 files changed, 63 insertions(+), 7 deletions(-)
diff --git a/hw/display/vga.c b/hw/display/vga.c
index 29d57cf..54b1fbe 100644
--- a/hw/display/vga.c
+++ b/hw/display/vga.c
@@ -615,6 +615,11 @@ uint32_t vbe_ioport_read_data(void *opaque, uint32_t addr)
}
} else if (s->vbe_index == VBE_DISPI_INDEX_VIDEO_MEMORY_64K) {
val = s->vram_size / (64 * 1024);
+} else if (s->vbe_index == VBE_DISPI_INDEX_EXTENDED_CAPS) {
+val = VBE_DISPI_HAS_ENDIAN_CTRL;
+} else if (s->vbe_index == VBE_DISPI_INDEX_ENDIAN_CTRL) {
+val = s->big_endian_fb ? VBE_DISPI_BIG_ENDIAN :
+VBE_DISPI_LITTLE_ENDIAN;
} else {
val = 0;
}
@@ -634,7 +639,8 @@ void vbe_ioport_write_data(void *opaque, uint32_t addr,
uint32_t val)
{
VGACommonState *s = opaque;
-if (s->vbe_index <= VBE_DISPI_INDEX_NB) {
+if (s->vbe_index <= VBE_DISPI_INDEX_NB ||
+s->vbe_index == VBE_DISPI_INDEX_ENDIAN_CTRL) {
#ifdef DEBUG_BOCHS_VBE
printf("VBE: write index=0x%x val=0x%x\n", s->vbe_index, val);
#endif
@@ -737,7 +743,7 @@ void vbe_ioport_write_data(void *opaque, uint32_t addr,
uint32_t val)
s->bank_offset = 0;
}
s->dac_8bit = (val & VBE_DISPI_8BIT_DAC) > 0;
-s->vbe_regs[s->vbe_index] = val;
+s->vbe_regs[s->vbe_index] = val | VBE_DISPI_EXTCAPS;
vga_update_memory_access(s);
break;
case VBE_DISPI_INDEX_VIRT_WIDTH:
@@ -774,6 +780,9 @@ void vbe_ioport_write_data(void *opaque, uint32_t addr,
uint32_t val)
s->vbe_start_addr >>= 2;
}
break;
+ case VBE_DISPI_INDEX_ENDIAN_CTRL:
+s->big_endian_fb = !!(val & VBE_DISPI_BIG_ENDIAN);
+break;
default:
break;
}
@@ -1464,7 +1473,8 @@ static void vga_draw_graphic(VGACommonState *s, int
full_update)
if (s->line_offset != s->last_line_offset ||
disp_width != s->last_width ||
height != s->last_height ||
-s->last_depth != depth) {
+s->last_depth != depth ||
+s->last_byteswap != byteswap) {
if (depth == 32 || depth == 24 ||
((depth == 16 || depth == 15) && !byteswap)) {
pixman_format_code_t format =
@@ -1483,6 +1493,7 @@ static void vga_draw_graphic(VGACommonState *s, int
full_update)
s->last_height = height;
s->last_line_offset = s->line_offset;
s->last_depth = depth;
+s->last_byteswap = byteswap;
full_update = 1;
} else if (is_buffer_shared(surface) &&
(full_update || surface_data(surface) != s->vram_ptr
@@ -1731,6 +1742,7 @@ void vga_common_reset(VGACommonState *s)
s->vbe_index = 0;
memset(s->vbe_regs, '\0', sizeof(s->vbe_regs));
s->vbe_regs[VBE_DISPI_INDEX_ID] = VBE_DISPI_ID5;
+s->vbe_regs[VBE_DISPI_INDEX_ENABLE] = VBE_DISPI_EXTCAPS;
s->vbe_start_addr = 0;
s->vbe_line_offset = 0;
s->vbe_bank_mask = (s->vram_size >> 16) - 1;
@@ -1751,6 +1763,7 @@ void vga_common_reset(VGACommonState *s)
s->cursor_start = 0;
s->cursor_end = 0;
s->cursor_offset = 0;
+s->big_endian_fb = s->default_endian_fb;
memset(s->invalidated_y_table, '\0', sizeof(s->invalidated_y_table));
memset(s->last_palette, '\0', sizeof(s->last_palette));
memset(s->last_ch_attr, '\0', sizeof(s->last_ch_attr));
@@ -1982,6 +1995,28 @@ static int vga_common_post_load(void *opaque, int
version_id)
return 0;
}
+static bool vga_endian_state_needed(void *opaque)
+{
+VGACommonState *s = opaque;
+
+/*
+ * Only send the endian state if it's different from the
+ * default one, thus ensuring backward compatibility for
+ * migration of the common case
+ */
+return s->default_endian_fb != s->big_endian_fb;
+}
+
+const VMStateDescription vmstate_vga_endian = {
+.name = "vga.endian",
+.version_id = 1,
+.minimum_version_id = 1,
+.fields = (VMStateField[]) {
+VMSTATE_UINT8_EQUAL(big_endian_fb, VGACommonState),
+VMSTATE_END_OF_LIST()
+}
+};
+
const VMStateDescription vmstate_vga_common = {
.name = "vga",
.version_id = 2,
@@ -2018,6 +2053,14 @@ const VMStateDescription vmstate_vga_common = {
VMSTATE_UINT32(vbe_line_offset, VGACommonState),
VMSTATE_UINT32(vbe_bank_mask, VGACommonState),
VMSTATE_END_OF_LIST()
+},
+.subsections = (VMStateSubsection []) {
+{
+.vmsd = &vmstate_vga_endian,
+.needed = vga_endian_state_needed,
+}, {
+/* empty */
+}
}
}
Re: [Qemu-devel] [RFC 11/14] vga: Make fb endian a common state variable
On 24 June 2014 00:11, Benjamin Herrenschmidt wrote:
> And initialize it based on target endian
> @@ -155,6 +155,7 @@ typedef struct VGACommonState {
> const GraphicHwOps *hw_ops;
> bool full_update_text;
> bool full_update_gfx;
> +bool big_endian_fb;
Don't we need to migrate this new state somehow?
thanks
-- PMM
Re: [Qemu-devel] [PATCH] linux-user: fix ipc(SEMCTL, ...) argument handling
On 24 June 2014 00:06, Paul Burton wrote: > On Mon, Jun 23, 2014 at 11:35:17PM +0100, Peter Maydell wrote: >> and so I'm dubious about a patch that's >> trying to make a very small change to it > > Isn't that precisely how good bisectable bug fixes should be made? The key is in the second half of this sentence: >> without looking >> at the larger picture and testing and fixing bugs on more >> than one architecture. > > I pointed you to the kernel code which dereferences the pointer & it's > quite clearly architecture neutral, so I'm not sure what you're getting > at with the architecture comment. > > There's quite clearly a bug in QEMU here, and this patch fixes it. I > hope you're not saying you don't want it merged because it doesn't fix > some other hypothetical bugs in the same area. What I mean is that I would expect that any attempt to fix behaviour in this area ought to result in a series of three or four patches which fix various bugs (of which this would just be one). When an area of code is pretty clearly bogus like this one, then in my experience an attempt to make a small fix to it without just going ahead and overhauling it is likely to result in accidentally breaking existing working uses which happened to work more or less by fluke. This is particularly true if you only test one guest architecture; you can reasonably make that level of limited testing in areas where the codebase is sane, but not where it is clearly dubious. So yes, I would prefer this not to be merged unless either (a) it comes as part of a series that cleans up the code for handling semctl so it's not obviously broken (b) it has been tested to confirm that it doesn't obviously regress any guest architecture (or at least not any of the more important ones), and ideally both. I don't think this is an enormous amount of work (maybe a couple of days?); I'm really just recommending the approach and amount of cleanup that I would do if I found I needed to make a fix in this area myself. thanks -- PMM
[Qemu-devel] [PATCH v5 2/3] qga: Add guest-get-fsinfo command
Add command to get mounted filesystems information in the guest.
The returned value contains a list of mountpoint paths and
corresponding disks info such as disk bus type, drive address,
and the disk controllers' PCI addresses, so that management layer
such as libvirt can resolve the disk backends.
For example, when `lsblk' result is:
NAME MAJ:MIN RM SIZE RO TYPE MOUNTPOINT
sdb 8:16 01G 0 disk
`-sdb1 8:17 0 1024M 0 part
`-vg0-lv0253:10 1.4G 0 lvm /mnt/test
sdc 8:32 01G 0 disk
`-sdc1 8:33 0 512M 0 part
`-vg0-lv0253:10 1.4G 0 lvm /mnt/test
vda252:00 25G 0 disk
`-vda1 252:10 25G 0 part /
where sdb is a SCSI disk with PCI controller :00:0a.0 and ID=1,
sdc is an IDE disk with PCI controller :00:01.1, and
vda is a virtio-blk disk with PCI device :00:06.0,
guest-get-fsinfo command will return the following result:
{"return":
[{"name":"dm-1",
"mountpoint":"/mnt/test",
"disk":[
{"bus-type":"scsi","bus":0,"unit":1,"target":0,
"pci-controller":{"bus":0,"slot":10,"domain":0,"function":0}},
{"bus-type":"ide","bus":0,"unit":0,"target":0,
"pci-controller":{"bus":0,"slot":1,"domain":0,"function":1}}],
"type":"xfs"},
{"name":"vda1", "mountpoint":"/",
"disk":[
{"bus-type":"virtio","bus":0,"unit":0,"target":0,
"pci-controller":{"bus":0,"slot":6,"domain":0,"function":0}}],
"type":"ext4"}]}
In Linux guest, the disk information is resolved from sysfs. So far,
it only supports virtio-blk, virtio-scsi, IDE, SATA, SCSI disks on x86
hosts, and "disk" parameter may be empty for unsupported disk types.
Signed-off-by: Tomoki Sekiyama
---
qga/commands-posix.c | 438 ++
qga/commands-win32.c |6 +
qga/qapi-schema.json | 79 +
3 files changed, 522 insertions(+), 1 deletion(-)
diff --git a/qga/commands-posix.c b/qga/commands-posix.c
index 212988f..9198b9f 100644
--- a/qga/commands-posix.c
+++ b/qga/commands-posix.c
@@ -18,6 +18,7 @@
#include
#include
#include
+#include
#include
#include
#include
@@ -575,6 +576,7 @@ static void guest_file_init(void)
typedef struct FsMount {
char *dirname;
char *devtype;
+unsigned int devmajor, devminor;
QTAILQ_ENTRY(FsMount) next;
} FsMount;
@@ -596,15 +598,40 @@ static void free_fs_mount_list(FsMountList *mounts)
}
}
+static int dev_major_minor(const char *devpath,
+ unsigned int *devmajor, unsigned int *devminor)
+{
+struct stat st;
+
+*devmajor = 0;
+*devminor = 0;
+
+if (stat(devpath, &st) < 0) {
+slog("failed to stat device file '%s': %s", devpath, strerror(errno));
+return -1;
+}
+if (S_ISDIR(st.st_mode)) {
+/* It is bind mount */
+return -2;
+}
+if (S_ISBLK(st.st_mode)) {
+*devmajor = major(st.st_rdev);
+*devminor = minor(st.st_rdev);
+return 0;
+}
+return -1;
+}
+
/*
* Walk the mount table and build a list of local file systems
*/
-static void build_fs_mount_list(FsMountList *mounts, Error **errp)
+static void build_fs_mount_list_from_mtab(FsMountList *mounts, Error **errp)
{
struct mntent *ment;
FsMount *mount;
char const *mtab = "/proc/self/mounts";
FILE *fp;
+unsigned int devmajor, devminor;
fp = setmntent(mtab, "r");
if (!fp) {
@@ -624,20 +651,423 @@ static void build_fs_mount_list(FsMountList *mounts,
Error **errp)
(strcmp(ment->mnt_type, "cifs") == 0)) {
continue;
}
+if (dev_major_minor(ment->mnt_fsname, &devmajor, &devminor) == -2) {
+/* Skip bind mounts */
+continue;
+}
mount = g_malloc0(sizeof(FsMount));
mount->dirname = g_strdup(ment->mnt_dir);
mount->devtype = g_strdup(ment->mnt_type);
+mount->devmajor = devmajor;
+mount->devminor = devminor;
QTAILQ_INSERT_TAIL(mounts, mount, next);
}
endmntent(fp);
}
+
+static void decode_mntname(char *name, int len)
+{
+int i, j = 0;
+for (i = 0; i <= len; i++) {
+if (name[i] != '\\') {
+name[j++] = name[i];
+} else if (name[i + 1] == '\\') {
+name[j++] = '\\';
+i++;
+} else if (name[i + 1] >= '0' && name[i + 1] <= '3' &&
+ name[i + 2] >= '0' && name[i + 2] <= '7' &&
+ name[i + 3] >= '0' && name[i + 3] <= '7') {
+name[j++] = (name[i + 1] - '0') * 64 +
+(name[i + 2] - '0') * 8 +
+(name[i + 3] - '0');
+i += 3;
+} else {
+name[j++] = name[i];
+}
+}
+}
+
+static void build_fs_mount_list(FsMountList *mounts, Error **errp)
+{
+FsMount *mou
[Qemu-devel] [PATCH v5 1/3] qga: Add guest-fsfreeze-freeze-list command
If an array of mount point paths is specified as 'mountpoints' argument
of guest-fsfreeze-freeze-list, qemu-ga will only freeze the file systems
mounted on specified paths in Linux guests. Otherwise, it works as the
same way as guest-fsfreeze-freeze.
This would be useful when the host wants to create partial disk snapshots.
Signed-off-by: Tomoki Sekiyama
Reviewed-by: Eric Blake
---
qga/commands-posix.c | 32 +++-
qga/commands-win32.c |9 +
qga/qapi-schema.json | 17 +
3 files changed, 57 insertions(+), 1 deletion(-)
diff --git a/qga/commands-posix.c b/qga/commands-posix.c
index 34ddba0..212988f 100644
--- a/qga/commands-posix.c
+++ b/qga/commands-posix.c
@@ -710,13 +710,21 @@ GuestFsfreezeStatus qmp_guest_fsfreeze_status(Error
**errp)
return GUEST_FSFREEZE_STATUS_THAWED;
}
+int64_t qmp_guest_fsfreeze_freeze(Error **errp)
+{
+return qmp_guest_fsfreeze_freeze_list(false, NULL, errp);
+}
+
/*
* Walk list of mounted file systems in the guest, and freeze the ones which
* are real local file systems.
*/
-int64_t qmp_guest_fsfreeze_freeze(Error **errp)
+int64_t qmp_guest_fsfreeze_freeze_list(bool has_mountpoints,
+ strList *mountpoints,
+ Error **errp)
{
int ret = 0, i = 0;
+strList *list;
FsMountList mounts;
struct FsMount *mount;
Error *local_err = NULL;
@@ -741,6 +749,19 @@ int64_t qmp_guest_fsfreeze_freeze(Error **errp)
ga_set_frozen(ga_state);
QTAILQ_FOREACH_REVERSE(mount, &mounts, FsMountList, next) {
+/* To issue fsfreeze in the reverse order of mounts, check if the
+ * mount is listed in the list here */
+if (has_mountpoints) {
+for (list = mountpoints; list; list = list->next) {
+if (strcmp(list->value, mount->dirname) == 0) {
+break;
+}
+}
+if (!list) {
+continue;
+}
+}
+
fd = qemu_open(mount->dirname, O_RDONLY);
if (fd == -1) {
error_setg_errno(errp, errno, "failed to open %s", mount->dirname);
@@ -1474,6 +1495,15 @@ int64_t qmp_guest_fsfreeze_freeze(Error **errp)
return 0;
}
+int64_t qmp_guest_fsfreeze_freeze_list(bool has_mountpoints,
+ strList *mountpoints,
+ Error **errp)
+{
+error_set(errp, QERR_UNSUPPORTED);
+
+return 0;
+}
+
int64_t qmp_guest_fsfreeze_thaw(Error **errp)
{
error_set(errp, QERR_UNSUPPORTED);
diff --git a/qga/commands-win32.c b/qga/commands-win32.c
index e769396..94877e9 100644
--- a/qga/commands-win32.c
+++ b/qga/commands-win32.c
@@ -206,6 +206,15 @@ error:
return 0;
}
+int64_t qmp_guest_fsfreeze_freeze_list(bool has_mountpoints,
+ strList *mountpoints,
+ Error **errp)
+{
+error_set(errp, QERR_UNSUPPORTED);
+
+return 0;
+}
+
/*
* Thaw local file systems using Volume Shadow-copy Service.
*/
diff --git a/qga/qapi-schema.json b/qga/qapi-schema.json
index a8cdcb3..caa4612 100644
--- a/qga/qapi-schema.json
+++ b/qga/qapi-schema.json
@@ -387,6 +387,23 @@
'returns': 'int' }
##
+# @guest-fsfreeze-freeze-list:
+#
+# Sync and freeze specified guest filesystems
+#
+# @mountpoints: #optional an array of mountpoints of filesystems to be frozen.
+# If omitted, every mounted filesystem is frozen.
+#
+# Returns: Number of file systems currently frozen. On error, all filesystems
+# will be thawed.
+#
+# Since: 2.1
+##
+{ 'command': 'guest-fsfreeze-freeze-list',
+ 'data':{ '*mountpoints': ['str'] },
+ 'returns': 'int' }
+
+##
# @guest-fsfreeze-thaw:
#
# Unfreeze all frozen guest filesystems
[Qemu-devel] [PATCH v5 0/3] qga: Add guest-fsfreeze-freeze-list command
Hi, This is v5 patch for qemu-ga to add functions to freeze specific file systems mounted in a guest. Changes since v4: - PATCH 2: fix coding styles (spaces around operators) make decode_mntname() more generic rename functions to avoid leading '__' improve compatibility with older Linux - PATCH 3 (new in v5): disable unsupported commands by default (v4: http://lists.gnu.org/archive/html/qemu-devel/2014-06/msg01357.html) --- Tomoki Sekiyama (3): qga: Add guest-fsfreeze-freeze-list command qga: Add guest-get-fsinfo command qga: Disable unsupported commands by default qga/commands-posix.c | 508 qga/commands-win32.c | 47 qga/guest-agent-core.h |1 qga/main.c |1 qga/qapi-schema.json | 96 + 5 files changed, 650 insertions(+), 3 deletions(-) -- Regards, Tomoki Sekiyama
[Qemu-devel] [RFC 11/14] vga: Make fb endian a common state variable
And initialize it based on target endian
Signed-off-by: Benjamin Herrenschmidt
---
hw/display/vga.c | 17 ++---
hw/display/vga_int.h | 1 +
2 files changed, 15 insertions(+), 3 deletions(-)
diff --git a/hw/display/vga.c b/hw/display/vga.c
index 909518c..e0c8dc7 100644
--- a/hw/display/vga.c
+++ b/hw/display/vga.c
@@ -1418,10 +1418,10 @@ static void vga_draw_graphic(VGACommonState *s, int
full_update)
uint8_t *d;
uint32_t v, addr1, addr;
vga_draw_line_func *vga_draw_line = NULL;
-#if defined(HOST_WORDS_BIGENDIAN) == defined(TARGET_WORDS_BIGENDIAN)
-static const bool byteswap = false;
+#ifdef HOST_WORDS_BIGENDIAN
+bool byteswap = !s->big_endian_fb;
#else
-static const bool byteswap = true;
+bool byteswap = s->big_endian_fb;
#endif
full_update |= update_basic_params(s);
@@ -2082,6 +2082,17 @@ void vga_common_init(VGACommonState *s, Object *obj,
bool global_vmstate)
s->update_retrace_info = vga_precise_update_retrace_info;
break;
}
+
+/*
+ * Set default fb endian based on target, should probably be turned
+ * into a device attribute set by the machine/platform to remove
+ * all target endian dependencies from this file.
+ */
+#ifdef TARGET_WORDS_BIGENDIAN
+s->big_endian_fb = true;
+#else
+s->big_endian_fb = false;
+#endif
vga_dirty_log_start(s);
}
diff --git a/hw/display/vga_int.h b/hw/display/vga_int.h
index 14e777a..ae64321 100644
--- a/hw/display/vga_int.h
+++ b/hw/display/vga_int.h
@@ -155,6 +155,7 @@ typedef struct VGACommonState {
const GraphicHwOps *hw_ops;
bool full_update_text;
bool full_update_gfx;
+bool big_endian_fb;
/* hardware mouse cursor support */
uint32_t invalidated_y_table[VGA_MAX_HEIGHT / 32];
void (*cursor_invalidate)(struct VGACommonState *s);
--
1.9.1
[Qemu-devel] [RFC 14/14] ppc/spapr/vga: Switch VGA endian on H_SET_MODE
When the guest switches the interrupt endian mode, which essentially
means a global machine endian switch, we want to change the VGA
framebuffer endian mode as well in order to be backward compatible
with existing guests who don't know about the new endian control
register.
Signed-off-by: Benjamin Herrenschmidt
---
hw/display/vga-pci.c | 8
hw/ppc/spapr_hcall.c | 2 ++
hw/ppc/spapr_pci.c | 26 ++
include/hw/pci/pci.h | 1 +
include/hw/ppc/spapr.h | 1 +
5 files changed, 38 insertions(+)
diff --git a/hw/display/vga-pci.c b/hw/display/vga-pci.c
index 0351d94..e2abc8c 100644
--- a/hw/display/vga-pci.c
+++ b/hw/display/vga-pci.c
@@ -212,6 +212,14 @@ static void pci_secondary_vga_reset(DeviceState *dev)
vga_common_reset(&d->vga);
}
+void pci_vga_switch_fb_endian(DeviceState *dev, bool big_endian)
+{
+PCIVGAState *d = DO_UPCAST(PCIVGAState, dev.qdev, dev);
+VGACommonState *s = &d->vga;
+
+s->big_endian_fb = big_endian;
+}
+
static Property vga_pci_properties[] = {
DEFINE_PROP_UINT32("vgamem_mb", PCIVGAState, vga.vram_size_mb, 16),
DEFINE_PROP_BIT("mmio", PCIVGAState, flags, PCI_VGA_FLAG_ENABLE_MMIO,
true),
diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c
index 7952077..e52f22d 100644
--- a/hw/ppc/spapr_hcall.c
+++ b/hw/ppc/spapr_hcall.c
@@ -731,12 +731,14 @@ static target_ulong h_set_mode_resouce_le(PowerPCCPU *cpu,
CPU_FOREACH(cs) {
set_spr(cs, SPR_LPCR, 0, LPCR_ILE);
}
+spapr_pci_switch_vga(true);
return H_SUCCESS;
case H_SET_MODE_ENDIAN_LITTLE:
CPU_FOREACH(cs) {
set_spr(cs, SPR_LPCR, LPCR_ILE, LPCR_ILE);
}
+spapr_pci_switch_vga(false);
return H_SUCCESS;
}
diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c
index 988f8cb..99ae3cd 100644
--- a/hw/ppc/spapr_pci.c
+++ b/hw/ppc/spapr_pci.c
@@ -926,3 +926,29 @@ static void spapr_pci_register_types(void)
}
type_init(spapr_pci_register_types)
+
+static int spapr_switch_one_vga(DeviceState *dev, void *opaque)
+{
+bool be = *(bool *)opaque;
+
+if (!strcmp(object_get_typename(OBJECT(dev)), "VGA")) {
+pci_vga_switch_fb_endian(dev, be);
+}
+return 0;
+}
+
+void spapr_pci_switch_vga(bool big_endian)
+{
+sPAPRPHBState *sphb;
+
+/*
+ * For backward compatibility with existing guests, we switch
+ * the endianness of the VGA controller when changing the guest
+ * interrupt mode
+ */
+QLIST_FOREACH(sphb, &spapr->phbs, list) {
+BusState *bus = &PCI_HOST_BRIDGE(sphb)->bus->qbus;
+qbus_walk_children(bus, spapr_switch_one_vga, NULL, NULL, NULL,
+ &big_endian);
+}
+}
diff --git a/include/hw/pci/pci.h b/include/hw/pci/pci.h
index 8c25ae5..bea4aff 100644
--- a/include/hw/pci/pci.h
+++ b/include/hw/pci/pci.h
@@ -373,6 +373,7 @@ PCIDevice *pci_nic_init_nofail(NICInfo *nd, PCIBus *rootbus,
const char *default_devaddr);
PCIDevice *pci_vga_init(PCIBus *bus);
+void pci_vga_switch_fb_endian(DeviceState *dev, bool big_endian);
int pci_bus_num(PCIBus *s);
void pci_for_each_device(PCIBus *bus, int bus_num,
diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h
index 08c301f..e3e7aff 100644
--- a/include/hw/ppc/spapr.h
+++ b/include/hw/ppc/spapr.h
@@ -425,5 +425,6 @@ int spapr_dma_dt(void *fdt, int node_off, const char
*propname,
uint32_t liobn, uint64_t window, uint32_t size);
int spapr_tcet_dma_dt(void *fdt, int node_off, const char *propname,
sPAPRTCETable *tcet);
+void spapr_pci_switch_vga(bool big_endian);
#endif /* !defined (__HW_SPAPR_H__) */
--
1.9.1
[Qemu-devel] [RFC 05/14] vga: Remove unused vga_draw_line24() and vga_draw_line32()
24 and 32bpp surfaces are always shared, we never "draw" them anymore
Signed-off-by: Benjamin Herrenschmidt
---
hw/display/vga.c | 29 +++-
hw/display/vga_template.h | 56 ---
2 files changed, 13 insertions(+), 72 deletions(-)
diff --git a/hw/display/vga.c b/hw/display/vga.c
index b46e42d..e98f7da 100644
--- a/hw/display/vga.c
+++ b/hw/display/vga.c
@@ -1344,8 +1344,6 @@ enum {
VGA_DRAW_LINE8,
VGA_DRAW_LINE15,
VGA_DRAW_LINE16,
-VGA_DRAW_LINE24,
-VGA_DRAW_LINE32,
VGA_DRAW_LINE_NB,
};
@@ -1358,8 +1356,6 @@ static vga_draw_line_func * const
vga_draw_line_table[VGA_DRAW_LINE_NB] = {
vga_draw_line8,
vga_draw_line15,
vga_draw_line16,
-vga_draw_line24,
-vga_draw_line32,
};
static int vga_get_bpp(VGACommonState *s)
@@ -1431,7 +1427,7 @@ static void vga_draw_graphic(VGACommonState *s, int
full_update)
int disp_width, multi_scan, multi_run;
uint8_t *d;
uint32_t v, addr1, addr;
-vga_draw_line_func *vga_draw_line;
+vga_draw_line_func *vga_draw_line = NULL;
#if defined(HOST_WORDS_BIGENDIAN) == defined(TARGET_WORDS_BIGENDIAN)
static const bool byteswap = false;
#else
@@ -1528,7 +1524,7 @@ static void vga_draw_graphic(VGACommonState *s, int
full_update)
}
bits = 4;
} else {
-switch(s->get_bpp(s)) {
+switch(depth) {
default:
case 0:
full_update |= update_palette256(s);
@@ -1549,19 +1545,20 @@ static void vga_draw_graphic(VGACommonState *s, int
full_update)
bits = 16;
break;
case 24:
-v = VGA_DRAW_LINE24;
-bits = 24;
-break;
case 32:
-v = VGA_DRAW_LINE32;
-bits = 32;
-break;
+if (!is_buffer_shared(surface)) {
+fprintf(stderr,
+"vga: Non-shared surface at %dbpp unsupported !\n",
depth);
+return;
+}
+bits = depth;
}
}
-vga_draw_line = vga_draw_line_table[v];
-
-if (!is_buffer_shared(surface) && s->cursor_invalidate) {
-s->cursor_invalidate(s);
+if (!is_buffer_shared(surface)) {
+vga_draw_line = vga_draw_line_table[v];
+if (s->cursor_invalidate) {
+s->cursor_invalidate(s);
+}
}
line_offset = s->line_offset;
diff --git a/hw/display/vga_template.h b/hw/display/vga_template.h
index 41fc764..e7cd9e0 100644
--- a/hw/display/vga_template.h
+++ b/hw/display/vga_template.h
@@ -323,59 +323,3 @@ static void vga_draw_line16(VGACommonState *s1, uint8_t *d,
} while (--w != 0);
}
-/*
- * 24 bit color
- */
-static void vga_draw_line24(VGACommonState *s1, uint8_t *d,
-const uint8_t *s, int width)
-{
-int w;
-uint32_t r, g, b;
-
-w = width;
-do {
-#if defined(TARGET_WORDS_BIGENDIAN)
-r = s[0];
-g = s[1];
-b = s[2];
-#else
-b = s[0];
-g = s[1];
-r = s[2];
-#endif
-((uint32_t *)d)[0] = rgb_to_pixel32(r, g, b);
-s += 3;
-d += 4;
-} while (--w != 0);
-}
-
-/*
- * 32 bit color
- */
-static void vga_draw_line32(VGACommonState *s1, uint8_t *d,
-const uint8_t *s, int width)
-{
-#if defined(HOST_WORDS_BIGENDIAN) == defined(TARGET_WORDS_BIGENDIAN)
-memcpy(d, s, width * 4);
-#else
-int w;
-uint32_t r, g, b;
-
-w = width;
-do {
-#if defined(TARGET_WORDS_BIGENDIAN)
-r = s[1];
-g = s[2];
-b = s[3];
-#else
-b = s[0];
-g = s[1];
-r = s[2];
-#endif
-((uint32_t *)d)[0] = rgb_to_pixel32(r, g, b);
-s += 4;
-d += 4;
-} while (--w != 0);
-#endif
-}
-
--
1.9.1
[Qemu-devel] [RFC 03/14] vga: Start cutting out non-32bpp conversion support
Nowadays, we either share a surface with the host, or we create
a 32bpp ARGB console surface.
So we only need to draw/convert to 32bpp, enabling us to remove
all but one instance of vga_template.h inclusion (to be further
cleaned up), rgb_to_pixel_* etc...
Signed-off-by: Benjamin Herrenschmidt
---
hw/display/vga.c | 258 +--
1 file changed, 22 insertions(+), 236 deletions(-)
diff --git a/hw/display/vga.c b/hw/display/vga.c
index 4674576..fbe2ddc 100644
--- a/hw/display/vga.c
+++ b/hw/display/vga.c
@@ -974,81 +974,12 @@ void vga_mem_writeb(VGACommonState *s, hwaddr addr,
uint32_t val)
}
}
-typedef void vga_draw_glyph8_func(uint8_t *d, int linesize,
- const uint8_t *font_ptr, int h,
- uint32_t fgcol, uint32_t bgcol);
-typedef void vga_draw_glyph9_func(uint8_t *d, int linesize,
- const uint8_t *font_ptr, int h,
- uint32_t fgcol, uint32_t bgcol, int dup9);
typedef void vga_draw_line_func(VGACommonState *s1, uint8_t *d,
const uint8_t *s, int width);
-#define DEPTH 8
-#include "vga_template.h"
-
-#define DEPTH 15
-#include "vga_template.h"
-
-#define BGR_FORMAT
-#define DEPTH 15
-#include "vga_template.h"
-
-#define DEPTH 16
-#include "vga_template.h"
-
-#define BGR_FORMAT
-#define DEPTH 16
-#include "vga_template.h"
-
#define DEPTH 32
#include "vga_template.h"
-#define BGR_FORMAT
-#define DEPTH 32
-#include "vga_template.h"
-
-static unsigned int rgb_to_pixel8_dup(unsigned int r, unsigned int g, unsigned
b)
-{
-unsigned int col;
-col = rgb_to_pixel8(r, g, b);
-col |= col << 8;
-col |= col << 16;
-return col;
-}
-
-static unsigned int rgb_to_pixel15_dup(unsigned int r, unsigned int g,
unsigned b)
-{
-unsigned int col;
-col = rgb_to_pixel15(r, g, b);
-col |= col << 16;
-return col;
-}
-
-static unsigned int rgb_to_pixel15bgr_dup(unsigned int r, unsigned int g,
- unsigned int b)
-{
-unsigned int col;
-col = rgb_to_pixel15bgr(r, g, b);
-col |= col << 16;
-return col;
-}
-
-static unsigned int rgb_to_pixel16_dup(unsigned int r, unsigned int g,
unsigned b)
-{
-unsigned int col;
-col = rgb_to_pixel16(r, g, b);
-col |= col << 16;
-return col;
-}
-
-static unsigned int rgb_to_pixel16bgr_dup(unsigned int r, unsigned int g,
- unsigned int b)
-{
-unsigned int col;
-col = rgb_to_pixel16bgr(r, g, b);
-col |= col << 16;
-return col;
-}
static unsigned int rgb_to_pixel32_dup(unsigned int r, unsigned int g,
unsigned b)
{
@@ -1057,13 +988,6 @@ static unsigned int rgb_to_pixel32_dup(unsigned int r,
unsigned int g, unsigned
return col;
}
-static unsigned int rgb_to_pixel32bgr_dup(unsigned int r, unsigned int g,
unsigned b)
-{
-unsigned int col;
-col = rgb_to_pixel32bgr(r, g, b);
-return col;
-}
-
/* return true if the palette was modified */
static int update_palette16(VGACommonState *s)
{
@@ -1170,56 +1094,6 @@ static int update_basic_params(VGACommonState *s)
return full_update;
}
-#define NB_DEPTHS 7
-
-static inline int get_depth_index(DisplaySurface *s)
-{
-switch (surface_bits_per_pixel(s)) {
-default:
-case 8:
-return 0;
-case 15:
-return 1;
-case 16:
-return 2;
-case 32:
-if (is_surface_bgr(s)) {
-return 4;
-} else {
-return 3;
-}
-}
-}
-
-static vga_draw_glyph8_func * const vga_draw_glyph8_table[NB_DEPTHS] = {
-vga_draw_glyph8_8,
-vga_draw_glyph8_16,
-vga_draw_glyph8_16,
-vga_draw_glyph8_32,
-vga_draw_glyph8_32,
-vga_draw_glyph8_16,
-vga_draw_glyph8_16,
-};
-
-static vga_draw_glyph8_func * const vga_draw_glyph16_table[NB_DEPTHS] = {
-vga_draw_glyph16_8,
-vga_draw_glyph16_16,
-vga_draw_glyph16_16,
-vga_draw_glyph16_32,
-vga_draw_glyph16_32,
-vga_draw_glyph16_16,
-vga_draw_glyph16_16,
-};
-
-static vga_draw_glyph9_func * const vga_draw_glyph9_table[NB_DEPTHS] = {
-vga_draw_glyph9_8,
-vga_draw_glyph9_16,
-vga_draw_glyph9_16,
-vga_draw_glyph9_32,
-vga_draw_glyph9_32,
-vga_draw_glyph9_16,
-vga_draw_glyph9_16,
-};
static const uint8_t cursor_glyph[32 * 4] = {
0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff,
@@ -1271,18 +1145,6 @@ static void vga_get_text_resolution(VGACommonState *s,
int *pwidth, int *pheight
*pcheight = cheight;
}
-typedef unsigned int rgb_to_pixel_dup_func(unsigned int r, unsigned int g,
unsigned b);
-
-static rgb_to_pixel_dup_func * const rgb_to_pixel_dup_table[NB_DEPTHS] = {
-rgb_to_pixel8_dup,
-rgb_to_pixel15_dup,
-rgb_to_pixel16_dup,
-rgb_to_pixel32_dup,
-rgb_to_pixel32bgr_dup,
-rgb_to_pixel15bgr_dup,
-rgb_to_pixel16bgr_dup,
-};
-
/*
* Text mode updat
[Qemu-devel] [RFC 12/14] vga: Rename vga_template.h to vga-helpers.h
It's no longer a template, we only instanciate the file once.
Keep it a #included file so the functions remain static.
Signed-off-by: Benjamin Herrenschmidt
---
hw/display/vga-helpers.h | 334 ++
hw/display/vga.c | 2 +-
hw/display/vga_template.h | 334 --
3 files changed, 335 insertions(+), 335 deletions(-)
create mode 100644 hw/display/vga-helpers.h
delete mode 100644 hw/display/vga_template.h
diff --git a/hw/display/vga-helpers.h b/hw/display/vga-helpers.h
new file mode 100644
index 000..7a44771
--- /dev/null
+++ b/hw/display/vga-helpers.h
@@ -0,0 +1,334 @@
+/*
+ * QEMU VGA Emulator templates
+ *
+ * Copyright (c) 2003 Fabrice Bellard
+ *
+ * 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.
+ */
+
+static inline void vga_draw_glyph_line(uint8_t *d, uint32_t font_data,
+ uint32_t xorcol, uint32_t bgcol)
+{
+((uint32_t *)d)[0] = (-((font_data >> 7)) & xorcol) ^ bgcol;
+((uint32_t *)d)[1] = (-((font_data >> 6) & 1) & xorcol) ^ bgcol;
+((uint32_t *)d)[2] = (-((font_data >> 5) & 1) & xorcol) ^ bgcol;
+((uint32_t *)d)[3] = (-((font_data >> 4) & 1) & xorcol) ^ bgcol;
+((uint32_t *)d)[4] = (-((font_data >> 3) & 1) & xorcol) ^ bgcol;
+((uint32_t *)d)[5] = (-((font_data >> 2) & 1) & xorcol) ^ bgcol;
+((uint32_t *)d)[6] = (-((font_data >> 1) & 1) & xorcol) ^ bgcol;
+((uint32_t *)d)[7] = (-((font_data >> 0) & 1) & xorcol) ^ bgcol;
+}
+
+static void vga_draw_glyph8(uint8_t *d, int linesize,
+const uint8_t *font_ptr, int h,
+uint32_t fgcol, uint32_t bgcol)
+{
+uint32_t font_data, xorcol;
+
+xorcol = bgcol ^ fgcol;
+do {
+font_data = font_ptr[0];
+vga_draw_glyph_line(d, font_data, xorcol, bgcol);
+font_ptr += 4;
+d += linesize;
+} while (--h);
+}
+
+static void vga_draw_glyph16(uint8_t *d, int linesize,
+ const uint8_t *font_ptr, int h,
+ uint32_t fgcol, uint32_t bgcol)
+{
+uint32_t font_data, xorcol;
+
+xorcol = bgcol ^ fgcol;
+do {
+font_data = font_ptr[0];
+vga_draw_glyph_line(d, expand4to8[font_data >> 4],
+xorcol, bgcol);
+vga_draw_glyph_line(d + 32, expand4to8[font_data & 0x0f],
+xorcol, bgcol);
+font_ptr += 4;
+d += linesize;
+} while (--h);
+}
+
+static void vga_draw_glyph9(uint8_t *d, int linesize,
+const uint8_t *font_ptr, int h,
+uint32_t fgcol, uint32_t bgcol, int dup9)
+{
+uint32_t font_data, xorcol, v;
+
+xorcol = bgcol ^ fgcol;
+do {
+font_data = font_ptr[0];
+((uint32_t *)d)[0] = (-((font_data >> 7)) & xorcol) ^ bgcol;
+((uint32_t *)d)[1] = (-((font_data >> 6) & 1) & xorcol) ^ bgcol;
+((uint32_t *)d)[2] = (-((font_data >> 5) & 1) & xorcol) ^ bgcol;
+((uint32_t *)d)[3] = (-((font_data >> 4) & 1) & xorcol) ^ bgcol;
+((uint32_t *)d)[4] = (-((font_data >> 3) & 1) & xorcol) ^ bgcol;
+((uint32_t *)d)[5] = (-((font_data >> 2) & 1) & xorcol) ^ bgcol;
+((uint32_t *)d)[6] = (-((font_data >> 1) & 1) & xorcol) ^ bgcol;
+v = (-((font_data >> 0) & 1) & xorcol) ^ bgcol;
+((uint32_t *)d)[7] = v;
+if (dup9)
+((uint32_t *)d)[8] = v;
+else
+((uint32_t *)d)[8] = bgcol;
+font_ptr += 4;
+d += linesize;
+} while (--h);
+}
+
+/*
+ * 4 color mode
+ */
+static void vga_draw_line2(VGACommonState *s1, uint8_t *d,
+ const uint8_t *s, int width)
+{
+uint32_t plane_mask, *palette, data, v;
+int x;
+
+palette = s1->last_palette;
+plane_mask = mask16[s1->ar[VGA_ATC_PLANE
[Qemu-devel] [RFC 10/14] vga: Remove some "should be done in BIOS" comments
Not all platforms have a VGA BIOS, powerpc typically relies on
using the DISPI interface to initialize the card.
Signed-off-by: Benjamin Herrenschmidt
---
hw/display/vga.c | 6 ++
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/hw/display/vga.c b/hw/display/vga.c
index 3b2cca5..909518c 100644
--- a/hw/display/vga.c
+++ b/hw/display/vga.c
@@ -695,14 +695,13 @@ void vbe_ioport_write_data(void *opaque, uint32_t addr,
uint32_t val)
((s->vbe_regs[VBE_DISPI_INDEX_BPP] + 7) >> 3);
s->vbe_start_addr = 0;
-/* clear the screen (should be done in BIOS) */
+/* clear the screen */
if (!(val & VBE_DISPI_NOCLEARMEM)) {
memset(s->vram_ptr, 0,
s->vbe_regs[VBE_DISPI_INDEX_YRES] *
s->vbe_line_offset);
}
-/* we initialize the VGA graphic mode (should be done
- in BIOS) */
+/* we initialize the VGA graphic mode */
/* graphic mode + memory map 1 */
s->gr[VGA_GFX_MISC] = (s->gr[VGA_GFX_MISC] & ~0x0c) | 0x04 |
VGA_GR06_GRAPHICS_MODE;
@@ -735,7 +734,6 @@ void vbe_ioport_write_data(void *opaque, uint32_t addr,
uint32_t val)
(shift_control << 5);
s->cr[VGA_CRTC_MAX_SCAN] &= ~0x9f; /* no double scan */
} else {
-/* XXX: the bios should do that */
s->bank_offset = 0;
}
s->dac_8bit = (val & VBE_DISPI_8BIT_DAC) > 0;
--
1.9.1
[Qemu-devel] [RFC 01/14] vga: Create direct sufaces for depth 24 too
pixman supports 24bpp directly, let's share surfaces
For both endians even !
Signed-off-by: Benjamin Herrenschmidt
---
hw/display/vga.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/hw/display/vga.c b/hw/display/vga.c
index e4cd206..4674576 100644
--- a/hw/display/vga.c
+++ b/hw/display/vga.c
@@ -1692,7 +1692,8 @@ static void vga_draw_graphic(VGACommonState *s, int
full_update)
disp_width != s->last_width ||
height != s->last_height ||
s->last_depth != depth) {
-if (depth == 32 || ((depth == 16 || depth == 15) && !byteswap)) {
+ if (depth == 32 || depth == 24 ||
+ ((depth == 16 || depth == 15) && !byteswap)) {
pixman_format_code_t format =
qemu_default_pixman_format(depth, !byteswap);
surface = qemu_create_displaysurface_from(disp_width,
--
1.9.1
[Qemu-devel] [RFC 00/14] VGA cleanups and endian control
This series cleans up VGA and a bit of cirrus to remove all the now unused conversions to non-32bpp surfaces. Then the last two patches add a proposed variant of the endian control register and the (still somewhat controversial) trick to auto-switch VGA endian on powerpc SPAPR platforms. They apply on top of Gerd pixel format series
[Qemu-devel] [RFC 04/14] vga: Remove remainder of old conversion cruft
All the macros used to generate different versions of vga_template.h
are now unnecessary, take them all out and remove the _32 suffix from
most functions.
Signed-off-by: Benjamin Herrenschmidt
---
hw/display/vga.c | 46 +-
hw/display/vga_template.h | 226 +++---
2 files changed, 95 insertions(+), 177 deletions(-)
diff --git a/hw/display/vga.c b/hw/display/vga.c
index fbe2ddc..b46e42d 100644
--- a/hw/display/vga.c
+++ b/hw/display/vga.c
@@ -977,10 +977,8 @@ void vga_mem_writeb(VGACommonState *s, hwaddr addr,
uint32_t val)
typedef void vga_draw_line_func(VGACommonState *s1, uint8_t *d,
const uint8_t *s, int width);
-#define DEPTH 32
#include "vga_template.h"
-
static unsigned int rgb_to_pixel32_dup(unsigned int r, unsigned int g,
unsigned b)
{
unsigned int col;
@@ -1279,19 +1277,19 @@ static void vga_draw_text(VGACommonState *s, int
full_update)
bgcol = palette[cattr >> 4];
fgcol = palette[cattr & 0x0f];
if (cw == 16) {
-vga_draw_glyph16_32(d1, linesize,
-font_ptr, cheight, fgcol, bgcol);
+vga_draw_glyph16(d1, linesize,
+ font_ptr, cheight, fgcol, bgcol);
} else if (cw != 9) {
-vga_draw_glyph8_32(d1, linesize,
- font_ptr, cheight, fgcol, bgcol);
+vga_draw_glyph8(d1, linesize,
+font_ptr, cheight, fgcol, bgcol);
} else {
dup9 = 0;
if (ch >= 0xb0 && ch <= 0xdf &&
(s->ar[VGA_ATC_MODE] & 0x04)) {
dup9 = 1;
}
-vga_draw_glyph9_32(d1, linesize,
- font_ptr, cheight, fgcol, bgcol, dup9);
+vga_draw_glyph9(d1, linesize,
+font_ptr, cheight, fgcol, bgcol, dup9);
}
if (src == cursor_ptr &&
!(s->cr[VGA_CRTC_CURSOR_START] & 0x20) &&
@@ -1307,14 +1305,14 @@ static void vga_draw_text(VGACommonState *s, int
full_update)
h = line_last - line_start + 1;
d = d1 + linesize * line_start;
if (cw == 16) {
-vga_draw_glyph16_32(d, linesize,
- cursor_glyph, h, fgcol, bgcol);
+vga_draw_glyph16(d, linesize,
+ cursor_glyph, h, fgcol, bgcol);
} else if (cw != 9) {
-vga_draw_glyph8_32(d, linesize,
- cursor_glyph, h, fgcol, bgcol);
+vga_draw_glyph8(d, linesize,
+cursor_glyph, h, fgcol, bgcol);
} else {
-vga_draw_glyph9_32(d, linesize,
- cursor_glyph, h, fgcol, bgcol,
1);
+vga_draw_glyph9(d, linesize,
+cursor_glyph, h, fgcol, bgcol, 1);
}
}
}
@@ -1352,16 +1350,16 @@ enum {
};
static vga_draw_line_func * const vga_draw_line_table[VGA_DRAW_LINE_NB] = {
-vga_draw_line2_32,
-vga_draw_line2d2_32,
-vga_draw_line4_32,
-vga_draw_line4d2_32,
-vga_draw_line8d2_32,
-vga_draw_line8_32,
-vga_draw_line15_32,
-vga_draw_line16_32,
-vga_draw_line24_32,
-vga_draw_line32_32,
+vga_draw_line2,
+vga_draw_line2d2,
+vga_draw_line4,
+vga_draw_line4d2,
+vga_draw_line8d2,
+vga_draw_line8,
+vga_draw_line15,
+vga_draw_line16,
+vga_draw_line24,
+vga_draw_line32,
};
static int vga_get_bpp(VGACommonState *s)
diff --git a/hw/display/vga_template.h b/hw/display/vga_template.h
index 90ec9c2..41fc764 100644
--- a/hw/display/vga_template.h
+++ b/hw/display/vga_template.h
@@ -22,41 +22,9 @@
* THE SOFTWARE.
*/
-#if DEPTH == 8
-#define BPP 1
-#define PIXEL_TYPE uint8_t
-#elif DEPTH == 15 || DEPTH == 16
-#define BPP 2
-#define PIXEL_TYPE uint16_t
-#elif DEPTH == 32
-#define BPP 4
-#define PIXEL_TYPE uint32_t
-#else
-#error unsupport depth
-#endif
-
-#ifdef BGR_FORMAT
-#define PIXEL_NAME glue(DEPTH, bgr)
-#else
-#define PIXEL_NAME DEPTH
-#endif /* BGR_FORMAT */
-
-#if DEPTH != 15 && !defined(BGR_FORMAT)
-
-static inline void glue(vga_draw_glyph_line_, DEPTH)(uint8_t *d,
- uint32_t font_data,
- uint32_t xorcol,
- uint32_t bgcol)
+stat
[Qemu-devel] [PATCH] hw/moxie/moxiesim.c: Remove unused moxie_intc_create()
The function moxie_intc_create() is unused; remove it.
Signed-off-by: Peter Maydell
---
In any case the recommendation these days would be to do this
kind of simple create-configure-init-wire-up inline in the
machine model code...
hw/moxie/moxiesim.c | 13 -
1 file changed, 13 deletions(-)
diff --git a/hw/moxie/moxiesim.c b/hw/moxie/moxiesim.c
index f4357cf..430f841 100644
--- a/hw/moxie/moxiesim.c
+++ b/hw/moxie/moxiesim.c
@@ -94,19 +94,6 @@ static void main_cpu_reset(void *opaque)
cpu_reset(CPU(cpu));
}
-static inline DeviceState *
-moxie_intc_create(hwaddr base, qemu_irq irq, int kind_of_intr)
-{
-DeviceState *dev;
-
-dev = qdev_create(NULL, "moxie,intc");
-qdev_prop_set_uint32(dev, "kind-of-intr", kind_of_intr);
-qdev_init_nofail(dev);
-sysbus_mmio_map(SYS_BUS_DEVICE(dev), 0, base);
-sysbus_connect_irq(SYS_BUS_DEVICE(dev), 0, irq);
-return dev;
-}
-
static void moxiesim_init(MachineState *machine)
{
MoxieCPU *cpu = NULL;
--
2.0.0
[Qemu-devel] [PATCH 1/3] target-sparc: Remove unused gen_op_subi_cc and gen_op_addi_cc
The functions gen_op_addi_cc() and gen_op_subi_cc() are unused; remove them.
Signed-off-by: Peter Maydell
---
target-sparc/translate.c | 24
1 file changed, 24 deletions(-)
diff --git a/target-sparc/translate.c b/target-sparc/translate.c
index 1ab07a1..b19673a 100644
--- a/target-sparc/translate.c
+++ b/target-sparc/translate.c
@@ -360,14 +360,6 @@ static inline void gen_mov_reg_C(TCGv reg, TCGv_i32 src)
tcg_gen_andi_tl(reg, reg, 0x1);
}
-static inline void gen_op_addi_cc(TCGv dst, TCGv src1, target_long src2)
-{
-tcg_gen_mov_tl(cpu_cc_src, src1);
-tcg_gen_movi_tl(cpu_cc_src2, src2);
-tcg_gen_addi_tl(cpu_cc_dst, cpu_cc_src, src2);
-tcg_gen_mov_tl(dst, cpu_cc_dst);
-}
-
static inline void gen_op_add_cc(TCGv dst, TCGv src1, TCGv src2)
{
tcg_gen_mov_tl(cpu_cc_src, src1);
@@ -499,22 +491,6 @@ static void gen_op_addx_int(DisasContext *dc, TCGv dst,
TCGv src1,
}
}
-static inline void gen_op_subi_cc(TCGv dst, TCGv src1, target_long src2,
DisasContext *dc)
-{
-tcg_gen_mov_tl(cpu_cc_src, src1);
-tcg_gen_movi_tl(cpu_cc_src2, src2);
-if (src2 == 0) {
-tcg_gen_mov_tl(cpu_cc_dst, src1);
-tcg_gen_movi_i32(cpu_cc_op, CC_OP_LOGIC);
-dc->cc_op = CC_OP_LOGIC;
-} else {
-tcg_gen_subi_tl(cpu_cc_dst, cpu_cc_src, src2);
-tcg_gen_movi_i32(cpu_cc_op, CC_OP_SUB);
-dc->cc_op = CC_OP_SUB;
-}
-tcg_gen_mov_tl(dst, cpu_cc_dst);
-}
-
static inline void gen_op_sub_cc(TCGv dst, TCGv src1, TCGv src2)
{
tcg_gen_mov_tl(cpu_cc_src, src1);
--
2.0.0
[Qemu-devel] [PATCH 0/2] target-ppc: fix unused-function warnings
These patches for target-ppc fix clang 3.4 warnings about 'static inline' functions which are defined but never used. (Incidentally, "d" is a terrible name for a function, so we're well rid of that one :-)) thanks -- PMM Peter Maydell (2): target-ppc: Remove unused IMM and d extract helpers target-ppc: Remove unused gen_qemu_ld8s() target-ppc/translate.c | 8 1 file changed, 8 deletions(-) -- 2.0.0
[Qemu-devel] [PATCH] target-unicore: Remove unused functions
The functions gen_st64, gen_ld64, gen_mulxy, ucf64_itod and
ucf64_dtoi are all unused; remove them.
Signed-off-by: Peter Maydell
---
target-unicore32/translate.c| 28
target-unicore32/ucf64_helper.c | 22 --
2 files changed, 50 deletions(-)
diff --git a/target-unicore32/translate.c b/target-unicore32/translate.c
index 5a8c7c8..e3643c2 100644
--- a/target-unicore32/translate.c
+++ b/target-unicore32/translate.c
@@ -576,13 +576,6 @@ static inline TCGv gen_ld32(TCGv addr, int index)
return tmp;
}
-static inline TCGv_i64 gen_ld64(TCGv addr, int index)
-{
-TCGv_i64 tmp = tcg_temp_new_i64();
-tcg_gen_qemu_ld64(tmp, addr, index);
-return tmp;
-}
-
static inline void gen_st8(TCGv val, TCGv addr, int index)
{
tcg_gen_qemu_st8(val, addr, index);
@@ -601,12 +594,6 @@ static inline void gen_st32(TCGv val, TCGv addr, int index)
dead_tmp(val);
}
-static inline void gen_st64(TCGv_i64 val, TCGv addr, int index)
-{
-tcg_gen_qemu_st64(val, addr, index);
-tcg_temp_free_i64(val);
-}
-
static inline void gen_set_pc_im(uint32_t val)
{
tcg_gen_movi_i32(cpu_R[31], val);
@@ -1128,21 +1115,6 @@ static inline void gen_jmp(DisasContext *s, uint32_t
dest)
}
}
-static inline void gen_mulxy(TCGv t0, TCGv t1, int x, int y)
-{
-if (x) {
-tcg_gen_sari_i32(t0, t0, 16);
-} else {
-gen_sxth(t0);
-}
-if (y) {
-tcg_gen_sari_i32(t1, t1, 16);
-} else {
-gen_sxth(t1);
-}
-tcg_gen_mul_i32(t0, t0, t1);
-}
-
/* Returns nonzero if access to the PSR is not permitted. Marks t0 as dead. */
static int gen_set_psr(DisasContext *s, uint32_t mask, int bsr, TCGv t0)
{
diff --git a/target-unicore32/ucf64_helper.c b/target-unicore32/ucf64_helper.c
index 0c7ea26..5af008f 100644
--- a/target-unicore32/ucf64_helper.c
+++ b/target-unicore32/ucf64_helper.c
@@ -290,28 +290,6 @@ static inline uint32_t ucf64_stoi(float32 s)
return v.i;
}
-static inline float64 ucf64_itod(uint64_t i)
-{
-union {
-uint64_t i;
-float64 d;
-} v;
-
-v.i = i;
-return v.d;
-}
-
-static inline uint64_t ucf64_dtoi(float64 d)
-{
-union {
-uint64_t i;
-float64 d;
-} v;
-
-v.d = d;
-return v.i;
-}
-
/* Integer to float conversion. */
float32 HELPER(ucf64_si2sf)(float32 x, CPUUniCore32State *env)
{
--
2.0.0
[Qemu-devel] [PATCH 0/3] target-sparc: fixed unused function warnings
These patchsets fix clang 3.4 warnings about unused static inline functions (clang now warns about these if they're defined in a .c file but then not used; gcc doesn't). The first patch just removes two totally unused functions; the second two patches use ifdeffery to avoid defining the functions in non-TARGET_SPARC64 builds. Incidentally, Mark, if you wanted to send a patch to MAINTAINERS to add yourself as a listed comaintainer for SPARC I'd be happy to apply it... Peter Maydell (3): target-sparc: Remove unused gen_op_subi_cc and gen_op_addi_cc target-sparc: address_mask(), asi_address_mask() are TARGET_SPARC64 only target-sparc: is_translating_asi() is TARGET_SPARC64 only target-sparc/ldst_helper.c | 10 +- target-sparc/translate.c | 24 2 files changed, 5 insertions(+), 29 deletions(-) -- 2.0.0
[Qemu-devel] [PATCH 2/2] target-ppc: Remove unused gen_qemu_ld8s()
The gen_qemu_ld8s() function is unused; remove it.
Signed-off-by: Peter Maydell
---
target-ppc/translate.c | 5 -
1 file changed, 5 deletions(-)
diff --git a/target-ppc/translate.c b/target-ppc/translate.c
index 4099cdc..aa835a2 100644
--- a/target-ppc/translate.c
+++ b/target-ppc/translate.c
@@ -2662,11 +2662,6 @@ static inline void gen_qemu_ld8u(DisasContext *ctx, TCGv
arg1, TCGv arg2)
tcg_gen_qemu_ld8u(arg1, arg2, ctx->mem_idx);
}
-static inline void gen_qemu_ld8s(DisasContext *ctx, TCGv arg1, TCGv arg2)
-{
-tcg_gen_qemu_ld8s(arg1, arg2, ctx->mem_idx);
-}
-
static inline void gen_qemu_ld16u(DisasContext *ctx, TCGv arg1, TCGv arg2)
{
TCGMemOp op = MO_UW | ctx->default_tcg_memop_mask;
--
2.0.0
[Qemu-devel] [PATCH 2/3] target-sparc: address_mask(), asi_address_mask() are TARGET_SPARC64 only
The address_mask() and asi_address_mask() functions are only used in
TARGET_SPARC64 configs, so guard with ifdefs to avoid warnings about
unused functions in 32-bit builds.
Signed-off-by: Peter Maydell
---
target-sparc/ldst_helper.c | 4
1 file changed, 4 insertions(+)
diff --git a/target-sparc/ldst_helper.c b/target-sparc/ldst_helper.c
index 03bd9f9..f4be811 100644
--- a/target-sparc/ldst_helper.c
+++ b/target-sparc/ldst_helper.c
@@ -250,6 +250,7 @@ static void replace_tlb_1bit_lru(SparcTLBEntry *tlb,
#endif
+#if defined(TARGET_SPARC64) || defined(CONFIG_USER_ONLY)
static inline target_ulong address_mask(CPUSPARCState *env1, target_ulong addr)
{
#ifdef TARGET_SPARC64
@@ -259,6 +260,7 @@ static inline target_ulong address_mask(CPUSPARCState
*env1, target_ulong addr)
#endif
return addr;
}
+#endif
/* returns true if access using this ASI is to have address translated by MMU
otherwise access is to raw physical address */
@@ -287,6 +289,7 @@ static inline int is_translating_asi(int asi)
#endif
}
+#ifdef TARGET_SPARC64
static inline target_ulong asi_address_mask(CPUSPARCState *env,
int asi, target_ulong addr)
{
@@ -296,6 +299,7 @@ static inline target_ulong asi_address_mask(CPUSPARCState
*env,
return addr;
}
}
+#endif
void helper_check_align(CPUSPARCState *env, target_ulong addr, uint32_t align)
{
--
2.0.0
[Qemu-devel] [PATCH] target-s390x: Remove unused ld_code6() function
The ld_code6() function is unused; remove it.
Signed-off-by: Peter Maydell
---
We could use this for loads of 6-byte insns, but that would
imply a pointless reload of the first 2 bytes, which is
presumably why this function isn't actually used.
---
target-s390x/translate.c | 5 -
1 file changed, 5 deletions(-)
diff --git a/target-s390x/translate.c b/target-s390x/translate.c
index 8ca4824..e2a1d05 100644
--- a/target-s390x/translate.c
+++ b/target-s390x/translate.c
@@ -264,11 +264,6 @@ static inline uint64_t ld_code4(CPUS390XState *env,
uint64_t pc)
return (uint64_t)(uint32_t)cpu_ldl_code(env, pc);
}
-static inline uint64_t ld_code6(CPUS390XState *env, uint64_t pc)
-{
-return (ld_code2(env, pc) << 32) | ld_code4(env, pc + 2);
-}
-
static int get_mem_index(DisasContext *s)
{
switch (s->tb->flags & FLAG_MASK_ASC) {
--
2.0.0
Re: [Qemu-devel] [PATCH] linux-user: fix ipc(SEMCTL, ...) argument handling
On Mon, Jun 23, 2014 at 11:35:17PM +0100, Peter Maydell wrote: > >> Have you checked this on other architectures than MIPS? > >> I have a vague recollection that there are between-arch > >> differences regarding handling of the semctl argument... > > > > I haven't tried running code for any other targets, but the pointer is > > dereferenced from generic code in Linux, see ipc/syscall.c: > > > > > > http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/ipc/syscall.c#n39 > > I see that code has a NULL pointer check which your patch doesn't... True, a NULL pointer would lead to EFAULT with my patch where the kernel will give EINVAL. I'll fix it. > >> Also, VERIFY_READ doesn't seem right for some of the > >> semctl operations which will modify the target_semun. > > > That part I think you're right about, I'll switch to VERIFY_WRITE. > > On the other hand that doesn't line up with the kernel code, > which just does a get_user() of a single target_ulong and > passes that to the sys_semctl function (which then might > interpret it as a user pointer to a thing that needs locking > and might be written to). We've crossed emails, I just noted the same thing :) > That would suggest that you > should be using get_user_ual() here, passing an abi_ulong > into do_semctl() and probably fixing up that function and its > callers. Well as far as I can tell the union will always be the size of a long anyway, so I see no harm in using lock_user(_struct) as I did. I'll switch if you insist, but the result will be the same. > Basically I think the semctl code is probably buggy in a > number of ways Perhaps, I suspect you know better than I. > and so I'm dubious about a patch that's > trying to make a very small change to it Isn't that precisely how good bisectable bug fixes should be made? > without looking > at the larger picture and testing and fixing bugs on more > than one architecture. I pointed you to the kernel code which dereferences the pointer & it's quite clearly architecture neutral, so I'm not sure what you're getting at with the architecture comment. There's quite clearly a bug in QEMU here, and this patch fixes it. I hope you're not saying you don't want it merged because it doesn't fix some other hypothetical bugs in the same area. > (http://patchwork.ozlabs.org/patch/217249/ is a previous > attempt at fixing up semctl; on reflection I may have been > wrong about the relevance of compat_sys_semctl, though.) In terms of the compat_ wrappers, note that compat_sys_ipc also dereferences the argument as a pointer: http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/ipc/compat.c#n350 And that compat_sys_semctl does not. As far as I can see they both match the behaviour of the non-compat versions with regards to this, so that seems irrelevant. I do agree that the patch you link to is wrong though, I'm guessing the author had confused semctl(...) and ipc(SEMCTL, ...). Thanks, Paul signature.asc Description: Digital signature
[Qemu-devel] [PATCH 1/2] target-ppc: Remove unused IMM and d extract helpers
Remove the definition of the IMM and d extract helpers; these seem to have
been added as part of the initial PPC support in 2003 but never actually
used.
Signed-off-by: Peter Maydell
---
target-ppc/translate.c | 3 ---
1 file changed, 3 deletions(-)
diff --git a/target-ppc/translate.c b/target-ppc/translate.c
index 4801721..4099cdc 100644
--- a/target-ppc/translate.c
+++ b/target-ppc/translate.c
@@ -426,7 +426,6 @@ static inline uint32_t SPR(uint32_t opcode)
return ((sprn >> 5) & 0x1F) | ((sprn & 0x1F) << 5);
}
/*** Get constants***/
-EXTRACT_HELPER(IMM, 12, 8);
/* 16 bits signed immediate value */
EXTRACT_SHELPER(SIMM, 0, 16);
/* 16 bits unsigned immediate value */
@@ -459,8 +458,6 @@ EXTRACT_HELPER(FPFLM, 17, 8);
EXTRACT_HELPER(FPW, 16, 1);
/***Jump target decoding ***/
-/* Displacement */
-EXTRACT_SHELPER(d, 0, 16);
/* Immediate address */
static inline target_ulong LI(uint32_t opcode)
{
--
2.0.0
[Qemu-devel] [PATCH 0/3] target-sparc: fixed unused function warnings
These patchsets fix clang 3.4 warnings about unused static inline functions (clang now warns about these if they're defined in a .c file but then not used; gcc doesn't). The first patch just removes two totally unused functions; the second two patches use ifdeffery to avoid defining the functions in non-TARGET_SPARC64 builds. Peter Maydell (3): target-sparc: Remove unused gen_op_subi_cc and gen_op_addi_cc target-sparc: address_mask(), asi_address_mask() are TARGET_SPARC64 only target-sparc: is_translating_asi() is TARGET_SPARC64 only target-sparc/ldst_helper.c | 10 +- target-sparc/translate.c | 24 2 files changed, 5 insertions(+), 29 deletions(-) -- 2.0.0
[Qemu-devel] [PATCH 3/3] target-sparc: is_translating_asi() is TARGET_SPARC64 only
Move the is_translating_asi() inside the TARGET_SPARC64 ifdef (and remove
the unimplemented 32-bit codepath), as it is only called from TARGET_SPARC64
code. This fixes a clang 3.4 unused-function warning.
Signed-off-by: Peter Maydell
---
target-sparc/ldst_helper.c | 8 ++--
1 file changed, 2 insertions(+), 6 deletions(-)
diff --git a/target-sparc/ldst_helper.c b/target-sparc/ldst_helper.c
index f4be811..e0aee86 100644
--- a/target-sparc/ldst_helper.c
+++ b/target-sparc/ldst_helper.c
@@ -262,11 +262,12 @@ static inline target_ulong address_mask(CPUSPARCState
*env1, target_ulong addr)
}
#endif
+#ifdef TARGET_SPARC64
/* returns true if access using this ASI is to have address translated by MMU
otherwise access is to raw physical address */
+/* TODO: check sparc32 bits */
static inline int is_translating_asi(int asi)
{
-#ifdef TARGET_SPARC64
/* Ultrasparc IIi translating asi
- note this list is defined by cpu implementation
*/
@@ -283,13 +284,8 @@ static inline int is_translating_asi(int asi)
default:
return 0;
}
-#else
-/* TODO: check sparc32 bits */
-return 0;
-#endif
}
-#ifdef TARGET_SPARC64
static inline target_ulong asi_address_mask(CPUSPARCState *env,
int asi, target_ulong addr)
{
--
2.0.0
Re: [Qemu-devel] [Qemu-ppc] [PATCH] mac99: Add motherboard devices before PCI cards
On 23/06/14 23:03, BALATON Zoltan wrote: Change the order of creating devices for New World Mac emulation so that devices on the motherboard are added first and PCI cards (VGA and NIC) come later. As a side effect, this also causes OpenBIOS to map the motherboard devices into the MMIO space to the same addresses as on real hardware and allow clients that hardcode these addresses (e.g. MorphOS) to find and use them until OpenBIOS is tought to map devices to specific addresses. (On real hardware the graphics and network cards are really on separate buses but we don't model that yet.) This brings the memory map closer to what is found on PowerMac3,1. Signed-off-by: BALATON Zoltan --- hw/ppc/mac_newworld.c | 14 +++--- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/hw/ppc/mac_newworld.c b/hw/ppc/mac_newworld.c index e493dc1..1a1e305 100644 --- a/hw/ppc/mac_newworld.c +++ b/hw/ppc/mac_newworld.c @@ -373,18 +373,11 @@ static void ppc_core99_init(MachineState *machine) machine_arch = ARCH_MAC99; } /* init basic PC hardware */ -pci_vga_init(pci_bus); - escc_mem = escc_init(0, pic[0x25], pic[0x24], serial_hds[0], serial_hds[1], ESCC_CLOCK, 4); memory_region_init_alias(escc_bar, NULL, "escc-bar", escc_mem, 0, memory_region_size(escc_mem)); -for(i = 0; i < nb_nics; i++) -pci_nic_init_nofail(&nd_table[i], pci_bus, "ne2k_pci", NULL); - -ide_drive_get(hd, MAX_IDE_BUS); - macio = pci_create(pci_bus, -1, TYPE_NEWWORLD_MACIO); dev = DEVICE(macio); qdev_connect_gpio_out(dev, 0, pic[0x19]); /* CUDA */ @@ -395,6 +388,8 @@ static void ppc_core99_init(MachineState *machine) macio_init(macio, pic_mem, escc_bar); /* We only emulate 2 out of 3 IDE controllers for now */ +ide_drive_get(hd, MAX_IDE_BUS); + macio_ide = MACIO_IDE(object_resolve_path_component(OBJECT(macio), "ide[0]")); macio_ide_init_drives(macio_ide, hd); @@ -420,9 +415,14 @@ static void ppc_core99_init(MachineState *machine) } } +pci_vga_init(pci_bus); + if (graphic_depth != 15 && graphic_depth != 32 && graphic_depth != 8) graphic_depth = 15; Missing braces here. +for(i = 0; i < nb_nics; i++) +pci_nic_init_nofail(&nd_table[i], pci_bus, "ne2k_pci", NULL); + And here too. /* The NewWorld NVRAM is not located in the MacIO device */ dev = qdev_create(NULL, TYPE_MACIO_NVRAM); qdev_prop_set_uint32(dev, "size", 0x2000); Generally the rule with QEMU is that as you change parts of the code that haven't been touched for a while, you should update them to meet the new style guidelines. Did you run the diffs through scripts/checkpatch.pl at all as that should catch styling points like this? ATB, Mark.
Re: [Qemu-devel] [PULL 075/118] macio: handle non-block ATAPI DMA transfers
On 23/06/14 20:26, BALATON Zoltan wrote: (add Kevin to CC) I'm afraid as you're the only person that can boot MorphOS this far then we need you to diagnose and suggest a suitable alternative by comparing the before and after output. Since MacOS is already a supported client then if no solution can be found then it is likely that this patch will be reverted :( So should I revert the patch for now? We're already in soft freeze. Well let's see if Zoltan can make any headway with debugging over the next few days; if there's no progress by the weekend then sadly my recommendation would be to revert in time for -rc0 as this definitely causes intermittent boot failures in Darwin for me. It would be nicer if it could be fixed instead of reverting. You could help detangling the macio.c code for a start. Just to clarify here: the macio/DBDMA code is quite complicated, but this is because this device has to work around to the fact that currently the DMA I/O routines currently need sector alignment whereas macio requires byte-level alignment. There has been quite a lot of work at the lower levels to support byte-level alignment (see Kevin's series at http://lists.gnu.org/archive/html/qemu-devel/2014-01/msg02163.html) but until we can specify transfers to byte granularity in the dma_bdrv_*() APIs then there isn't much we can do to clean up the macio.c code. Kevin, are there any plans to bubble the byte-granularity block layer changes up to the dma_bdrv_*() APIs in the near future? Please bear in mind that QEMU supports a large number of OSs, and there is already an enthusiastic group of people using Alex's OS X work (see emaculation for many examples) so introducing an intermittent fault on a supported OS is not an option here. I should also re-emphasise that Alex/Andreas work on many different parts of QEMU, and my work is currently unsponsored so while we are all keen to improve QEMU to the point where it can emulate new OSs such as MorphOS, it's not the case that we can simply drop what we are doing at the time to focus on an issue that affects a single OS which is new and currently unsupported. Now I think it's fair to say that I've spent quite a few hours helping you and coming up with the original version of this patch, and I'm glad that you are now seeing success with this. But what is important to us right now heading towards a release is that patches don't cause any regressions. All I can say is that debugging this stuff isn't easy, particularly with MorphOS which has some rather unusual behaviours. But what we really need from you now over the next few days is for you to compare the debug output between the working and non-working cases and figure out if we can fix this in time for the 2.1 release. You have everything you need (including my acceptance test of booting both MorphOS and Darwin ISOs), so time to take a deep breath and begin what should be a challenging yet ultimately rewarding debugging process :) ATB, Mark.
Re: [Qemu-devel] [PATCH] linux-user: fix ipc(SEMCTL, ...) argument handling
On 23 June 2014 23:36, Paul Burton wrote: > Actually no, I don't think you're right about that afterall. The > argument union itself is never modified. I imagine if it were then it > would be painful in the case of the semctl syscall where the union is > passed directly as an argument, rather than as a pointer as it is for > the ipc syscall. > > What may be modified is the data pointed to by the pointers within union > semun. That is already handled by do_semctl & the translate functions it > calls. Except if you look at do_semctl you see code like: case GETVAL: case SETVAL: arg.val = tswap32(target_su.val); ret = get_errno(semctl(semid, semnum, cmd, arg)); target_su.val = tswap32(arg.val); break; which clearly is just modifying fields in the target_semun union. So something's wrong (probably that code)... thanks -- PMM
Re: [Qemu-devel] [PATCH] linux-user: fix ipc(SEMCTL, ...) argument handling
On Mon, Jun 23, 2014 at 11:18:25PM +0100, Paul Burton wrote: > > Also, VERIFY_READ doesn't seem right for some of the > > semctl operations which will modify the target_semun. > > > > thanks > > -- PMM > > That part I think you're right about, I'll switch to VERIFY_WRITE. Actually no, I don't think you're right about that afterall. The argument union itself is never modified. I imagine if it were then it would be painful in the case of the semctl syscall where the union is passed directly as an argument, rather than as a pointer as it is for the ipc syscall. What may be modified is the data pointed to by the pointers within union semun. That is already handled by do_semctl & the translate functions it calls. /me is not fond of this API... So anyway, I believe the patch is good as-is. Thanks, Paul signature.asc Description: Digital signature
Re: [Qemu-devel] [PATCH v2] hw/net/eepro100: Implement read-only bits in MDI registers
Ping! Stefan, you said you were going to take this via the net queue?
thanks
-- PMM
On 9 June 2014 16:03, Peter Maydell wrote:
> Although we defined an eepro100_mdi_mask[] array indicating which bits
> in the registers are read-only, we weren't actually doing anything with
> it. Make the MDI register-write code use it rather than manually making
> register 1 read-only and leaving the rest as reads-as-written. (The
> special-case handling of register 0 remains as before since its mask is
> all-zeros and the special casing happens before we apply the masking.)
>
> Signed-off-by: Peter Maydell
> Message-id: 1402159924-13853-1-git-send-email-peter.mayd...@linaro.org
> ---
> No code change, but I fixed the errors in the commit message.
>
> hw/net/eepro100.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/hw/net/eepro100.c b/hw/net/eepro100.c
> index 3b891ca..9c70cce 100644
> --- a/hw/net/eepro100.c
> +++ b/hw/net/eepro100.c
> @@ -1217,7 +1217,6 @@ static void eepro100_write_mdi(EEPRO100State *s)
> break;
> case 1:/* Status Register */
> missing("not writable");
> -data = s->mdimem[reg];
> break;
> case 2:/* PHY Identification Register (Word 1) */
> case 3:/* PHY Identification Register (Word 2) */
> @@ -1230,7 +1229,8 @@ static void eepro100_write_mdi(EEPRO100State *s)
> default:
> missing("not implemented");
> }
> -s->mdimem[reg] = data;
> +s->mdimem[reg] &= eepro100_mdi_mask[reg];
> +s->mdimem[reg] |= data & ~eepro100_mdi_mask[reg];
> } else if (opcode == 2) {
> /* MDI read */
> switch (reg) {
> --
> 1.9.2
Re: [Qemu-devel] [PATCH] linux-user: fix ipc(SEMCTL, ...) argument handling
On 23 June 2014 23:18, Paul Burton wrote: > On Mon, Jun 23, 2014 at 11:12:42PM +0100, Peter Maydell wrote: >> On 23 June 2014 22:40, Paul Burton wrote: >> > The ptr argument to the ipc syscall was incorrectly being used as the >> > value of the argument union for the SEMCTL call. It is actually, as its >> > name would suggest, a pointer to that union. >> >> Have you checked this on other architectures than MIPS? >> I have a vague recollection that there are between-arch >> differences regarding handling of the semctl argument... > > I haven't tried running code for any other targets, but the pointer is > dereferenced from generic code in Linux, see ipc/syscall.c: > > > http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/ipc/syscall.c#n39 I see that code has a NULL pointer check which your patch doesn't... >> Also, VERIFY_READ doesn't seem right for some of the >> semctl operations which will modify the target_semun. > That part I think you're right about, I'll switch to VERIFY_WRITE. On the other hand that doesn't line up with the kernel code, which just does a get_user() of a single target_ulong and passes that to the sys_semctl function (which then might interpret it as a user pointer to a thing that needs locking and might be written to). That would suggest that you should be using get_user_ual() here, passing an abi_ulong into do_semctl() and probably fixing up that function and its callers. Basically I think the semctl code is probably buggy in a number of ways and so I'm dubious about a patch that's trying to make a very small change to it without looking at the larger picture and testing and fixing bugs on more than one architecture. (http://patchwork.ozlabs.org/patch/217249/ is a previous attempt at fixing up semctl; on reflection I may have been wrong about the relevance of compat_sys_semctl, though.) thanks -- PMM
Re: [Qemu-devel] machines and versions - why so many?
On 06/24/2014 07:41 AM, Andreas Färber wrote: > Am 23.06.2014 23:35, schrieb Alexey Kardashevskiy: >> Looks like I must copy PC_COMPAT_X_X as PSERIES_COMPAT_X_X starting 1.6 (or >> 1.7 - whichever starts supporting migration well enough on pseries) because >> pretty much of what they do is tweaking PCI devices and we can have all of >> these devices on pseries. And then keep an eye on what is happening in PC >> world to copy same tweaks to pseries as they come. Is that correct? > > Please don't. There's a series by Marcel on the list converting those PC > macros to QOM. You already have a QOM sPAPR machine, so you should just > derive new legacy types as needed and override things there. I failed to find the series in patchworks, was it long time ago? What was the subject? I actually wonder if it is not going to be "-machine pseries-2.0" then what will it look like? "-machine pseries,qemucompat=2.0"? I would think there will be TYPE_MACHINE_X_Y types which I would use as a parent TYPE_SPAPR_MACHINE (dynamically, as we do for the "host" CPU type) but this is not what you are saying, correct? > Also, -machine *is* the global mechanism we have to tell QEMU which > version you want, it's a shorthand for setting a list of global > properties. Don't forget that QEMU can be used without libvirt, so the > knowledge of which properties to set for which version is kept in QEMU. I do not forget, I use libvirt once a month :) -- Alexey
Re: [Qemu-devel] [PATCH v2] mac99: Change memory layout to better match PowerMac3, 1
On Mon, 23 Jun 2014, Mark Cave-Ayland wrote: On 23/06/14 20:25, BALATON Zoltan wrote: It's how OpenBIOS assigns MMIO addresses to pci devices. It does it by going through them in order and map them starting from the base address (with some allingment). I guess you could look at drivers/pci.c I think it's in there somewhere. I think it'd make more sense to just bolt the PCI devices to their respective devfns that they also have on real hardware. Depending on ordering magic that happens to give us different BAR maps by firmware doesn't really give me a lot of confidence. I don't understand what you mean. I don't want to rewrite PCI handling code in OpenBIOS as that would have a higher chance of breaking something else (OpenBIOS is used by other archs as well). Also it would require more knowledge about the emulated hardware in OpenBIOS while it aims to be a generic implementation and wants to reduce special cases already in it. So I don't see a cleaner and easy way to do this. If I'm missing something please tell me. On the other hand you've said before that the mac99 machine is mostly a hack to be enough that some OS-es can run with it. Why reordering some devices to get the right BAR maps not fit in this hack? Since the MMIO address is calculated by rounding up to the next start address based on region size (where size tends to be more static compared to start address) then I'd be okay with this as a short term hack. Thanks. I've sent the other half of the patch then, that could be taken until a better solution will be available (it does not have to be reverted then either). Regards, BALATON Zoltan
[Qemu-devel] [PATCH] mac99: Add motherboard devices before PCI cards
Change the order of creating devices for New World Mac emulation so that devices on the motherboard are added first and PCI cards (VGA and NIC) come later. As a side effect, this also causes OpenBIOS to map the motherboard devices into the MMIO space to the same addresses as on real hardware and allow clients that hardcode these addresses (e.g. MorphOS) to find and use them until OpenBIOS is tought to map devices to specific addresses. (On real hardware the graphics and network cards are really on separate buses but we don't model that yet.) This brings the memory map closer to what is found on PowerMac3,1. Signed-off-by: BALATON Zoltan --- hw/ppc/mac_newworld.c | 14 +++--- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/hw/ppc/mac_newworld.c b/hw/ppc/mac_newworld.c index e493dc1..1a1e305 100644 --- a/hw/ppc/mac_newworld.c +++ b/hw/ppc/mac_newworld.c @@ -373,18 +373,11 @@ static void ppc_core99_init(MachineState *machine) machine_arch = ARCH_MAC99; } /* init basic PC hardware */ -pci_vga_init(pci_bus); - escc_mem = escc_init(0, pic[0x25], pic[0x24], serial_hds[0], serial_hds[1], ESCC_CLOCK, 4); memory_region_init_alias(escc_bar, NULL, "escc-bar", escc_mem, 0, memory_region_size(escc_mem)); -for(i = 0; i < nb_nics; i++) -pci_nic_init_nofail(&nd_table[i], pci_bus, "ne2k_pci", NULL); - -ide_drive_get(hd, MAX_IDE_BUS); - macio = pci_create(pci_bus, -1, TYPE_NEWWORLD_MACIO); dev = DEVICE(macio); qdev_connect_gpio_out(dev, 0, pic[0x19]); /* CUDA */ @@ -395,6 +388,8 @@ static void ppc_core99_init(MachineState *machine) macio_init(macio, pic_mem, escc_bar); /* We only emulate 2 out of 3 IDE controllers for now */ +ide_drive_get(hd, MAX_IDE_BUS); + macio_ide = MACIO_IDE(object_resolve_path_component(OBJECT(macio), "ide[0]")); macio_ide_init_drives(macio_ide, hd); @@ -420,9 +415,14 @@ static void ppc_core99_init(MachineState *machine) } } +pci_vga_init(pci_bus); + if (graphic_depth != 15 && graphic_depth != 32 && graphic_depth != 8) graphic_depth = 15; +for(i = 0; i < nb_nics; i++) +pci_nic_init_nofail(&nd_table[i], pci_bus, "ne2k_pci", NULL); + /* The NewWorld NVRAM is not located in the MacIO device */ dev = qdev_create(NULL, TYPE_MACIO_NVRAM); qdev_prop_set_uint32(dev, "size", 0x2000); -- 1.8.1.5
