Re: [Qemu-devel] [PATCHv2 1/3] ui/spice-display.c: add missing initialization for valgrind

[Gerd Hoffmann]

On 05/24/12 08:43, Alon Levy wrote:
> On Wed, May 23, 2012 at 08:59:22PM +0200, Gerd Hoffmann wrote:
>> On 05/22/12 17:29, Alon Levy wrote:
>>> We can't initialize QXLDevSurfaceCreate field by field because it has a
>>> pa hole, and so 4 bytes remain uninitialized when building on x86-64, so
>>> just memset.
>>
>> So you get valgrind warnings for the hole?  why?  nobody should ever
>> access the hole, so the missing initialization should not hurt in theory ...
> 
> Because we allocate this struct on the stack and then copy it over an fd
> to spice, through the dispatcher pipe.

Ah, yea, copying will make valgrind complain of course ...

I'll go queue it up.

cheers,
  Gerd

Re: [Qemu-devel] [PATCH] Support virtio-scsi-pci adapter hot-plug

[Michael S. Tsirkin]

On Thu, May 24, 2012 at 02:31:00PM +0800, Kelvin Wang wrote:
> On Wed, May 23, 2012 at 05:45:33PM +0300, Michael S. Tsirkin wrote:
> > On Wed, May 23, 2012 at 04:30:06PM +0200, Paolo Bonzini wrote:
> > > Il 23/05/2012 16:12, Michael S. Tsirkin ha scritto:
> > > >> > 2, Run qemu with the option -monitor.
> > > >> > 
> > > >> > 3, In the guest, insert necessary modules:
> > > >> > for m in acpiphp pci_hotplug; do sudo modprobe ${m}; done
> > > >> > 
> > > >> > 4, In the qemu monitor,hot add a virtio-scsi-pci adapter:
> > > >> > (qemu)pci_add auto storage if=virtio-scsi-pci
> > > >> > 
> > > >> > 5, Check whether the controller was added:
> > > >> > Guest: lspci
> > > >> > Qemu: (qemu)info qtree
> > > >> > 
> > > >> > Signed-off-by: Kelvin Wang 
> > > >> > Signed-off-by: Sheng Liu 
> > > > NAK
> > > > 
> > > > Do not use pci_add. It is a compatibility command.
> > > > Use the new style device_add.
> > > > Same for if=.
> > > > 
> > > > I think you won't need any changes then?
> > > > 
> > > 
> > > You don't.  You need to rescan the bus manually in the guest, that's all.
> > > 
> > > Paolo
> > 
> > If the point is to avoid need for manual bus rescans that's
> > good. But please do not touch the legacy commands.
> So, may I sent another patch to "avoid need for manual bus rescans"?

Let's separate bugfixes from adding new commands.

> device_add should be used by users, but another way supplied to users is not
> necessarily, but always harmless, right?

No, it has support costs.

> > If anyone wants to use new devices, new commands
> > drive_add and device_add should be used.
> > Same for command line flags.
> > 
> > -- 
> > MST
> > 

Re: [Qemu-devel] [PATCHv2 1/3] ui/spice-display.c: add missing initialization for valgrind

[Alon Levy]

On Wed, May 23, 2012 at 08:59:22PM +0200, Gerd Hoffmann wrote:
> On 05/22/12 17:29, Alon Levy wrote:
> > We can't initialize QXLDevSurfaceCreate field by field because it has a
> > pa hole, and so 4 bytes remain uninitialized when building on x86-64, so
> > just memset.
> 
> So you get valgrind warnings for the hole?  why?  nobody should ever
> access the hole, so the missing initialization should not hurt in theory ...

Because we allocate this struct on the stack and then copy it over an fd
to spice, through the dispatcher pipe.

echo quit | valgrind --log-file=/tmp/valgrind.log
--leak-check=full ./x86_64-softmmu/qemu-system-x86_64 -monitor stdio
-spice port=1234 -vga qxl

==2677== Syscall param write(buf) points to uninitialised byte(s)
==2677==at 0x659504D: ??? (syscall-template.S:82)
==2677==by 0x91BCD54: write_safe (dispatcher.c:103)
==2677==by 0x91BD168: dispatcher_send_message (dispatcher.c:182)
==2677==by 0x91BEC24: red_dispatcher_create_primary_surface_sync 
(red_dispatcher.c:489)
==2677==by 0x91BECAD: red_dispatcher_create_primary_surface 
(red_dispatcher.c:502)
==2677==by 0x91BED03: qxl_worker_create_primary_surface 
(red_dispatcher.c:509)
==2677==by 0x3403CC: qemu_spice_create_primary_surface (spice-display.c:106)
==2677==by 0x340BAB: qemu_spice_create_host_primary (spice-display.c:260)
==2677==by 0x40EC50: qxl_enter_vga_mode (qxl.c:931)
==2677==by 0x40EFCA: qxl_soft_reset (qxl.c:982)
==2677==by 0x40F088: qxl_hard_reset (qxl.c:1004)
==2677==by 0x40F0DA: qxl_reset_handler (qxl.c:1011)
==2677==  Address 0x7feffef98 is on thread 1's stack
> 
> > Signed-off-by: Alon Levy 
> > ---
> >  ui/spice-display.c |2 ++
> >  1 file changed, 2 insertions(+)
> > 
> > diff --git a/ui/spice-display.c b/ui/spice-display.c
> > index 5418eb3..3e8f0b3 100644
> > --- a/ui/spice-display.c
> > +++ b/ui/spice-display.c
> > @@ -244,6 +244,8 @@ void qemu_spice_create_host_primary(SimpleSpiceDisplay 
> > *ssd)
> >  {
> >  QXLDevSurfaceCreate surface;
> >  
> > +memset(&surface, 0, sizeof(surface));
> > +
> >  dprint(1, "%s: %dx%d\n", __FUNCTION__,
> > ds_get_width(ssd->ds), ds_get_height(ssd->ds));
> >  
> 

Re: [Qemu-devel] [PATCH] Support virtio-scsi-pci adapter hot-plug

[Kelvin Wang]

On Wed, May 23, 2012 at 05:45:33PM +0300, Michael S. Tsirkin wrote:
> On Wed, May 23, 2012 at 04:30:06PM +0200, Paolo Bonzini wrote:
> > Il 23/05/2012 16:12, Michael S. Tsirkin ha scritto:
> > >> > 2, Run qemu with the option -monitor.
> > >> > 
> > >> > 3, In the guest, insert necessary modules:
> > >> > for m in acpiphp pci_hotplug; do sudo modprobe ${m}; done
> > >> > 
> > >> > 4, In the qemu monitor,hot add a virtio-scsi-pci adapter:
> > >> > (qemu)pci_add auto storage if=virtio-scsi-pci
> > >> > 
> > >> > 5, Check whether the controller was added:
> > >> > Guest: lspci
> > >> > Qemu: (qemu)info qtree
> > >> > 
> > >> > Signed-off-by: Kelvin Wang 
> > >> > Signed-off-by: Sheng Liu 
> > > NAK
> > > 
> > > Do not use pci_add. It is a compatibility command.
> > > Use the new style device_add.
> > > Same for if=.
> > > 
> > > I think you won't need any changes then?
> > > 
> > 
> > You don't.  You need to rescan the bus manually in the guest, that's all.
> > 
> > Paolo
> 
> If the point is to avoid need for manual bus rescans that's
> good. But please do not touch the legacy commands.
So, may I sent another patch to "avoid need for manual bus rescans"?
device_add should be used by users, but another way supplied to users is not
necessarily, but always harmless, right?
> If anyone wants to use new devices, new commands
> drive_add and device_add should be used.
> Same for command line flags.
> 
> -- 
> MST
> 

Re: [Qemu-devel] [PATCH] Support virtio-scsi-pci adapter hot-plug

[Kelvin Wang]

On Wed, May 23, 2012 at 05:12:16PM +0300, Michael S. Tsirkin wrote:
> On Wed, May 23, 2012 at 09:52:06PM +0800, Kelvin Wang wrote:
> > Support the virtio-scsi-pci adapter hot-plug. However, this patch can only 
> > make
> > adapter hot-plugable. More effort is needed for LUN hot-plug. Actually, 
> > that is 
> > the most valuable feature to virtio-scsi.
> > 
> > Following the steps as follows can give an intuitive understanding of this 
> > feature after applying the patch to QEMU v1.1.0-rc3.
> > 
> > 1, Generate a image file:
> > qemu-img create -f qcow2 test.img 2G
> > 
> > 2, Run qemu with the option -monitor.
> > 
> > 3, In the guest, insert necessary modules:
> > for m in acpiphp pci_hotplug; do sudo modprobe ${m}; done
> > 
> > 4, In the qemu monitor,hot add a virtio-scsi-pci adapter:
> > (qemu)pci_add auto storage if=virtio-scsi-pci
> > 
> > 5, Check whether the controller was added:
> > Guest: lspci
> > Qemu: (qemu)info qtree
> > 
> > Signed-off-by: Kelvin Wang 
> > Signed-off-by: Sheng Liu 
> 
> NAK
> 
> Do not use pci_add. It is a compatibility command.
> Use the new style device_add.
> Same for if=.
> 
> I think you won't need any changes then?
OK, Thanks!
> 
> > ---
> >  blockdev.c   |   19 +++
> >  blockdev.h   |3 ++-
> >  hw/pci-hotplug.c |   15 +++
> >  3 files changed, 32 insertions(+), 5 deletions(-)
> > 
> > diff --git a/blockdev.c b/blockdev.c
> > index 67895b2..ecb82bf 100644
> > --- a/blockdev.c
> > +++ b/blockdev.c
> > @@ -31,6 +31,7 @@ static const char *const if_name[IF_COUNT] = {
> >  [IF_MTD] = "mtd",
> >  [IF_SD] = "sd",
> >  [IF_VIRTIO] = "virtio",
> > +   [IF_VIRTIO_SCSI] = "virtio-scsi",
> 
> weird indentation
> 
> >  [IF_XEN] = "xen",
> >  };
> > 
> 
> I think blockdev is a legacy thing. You are
> supposed to use the new style device-add. No?
> 
> > @@ -436,7 +437,8 @@ DriveInfo *drive_init(QemuOpts *opts, int 
> > default_to_scsi)
> > 
> >  on_write_error = BLOCK_ERR_STOP_ENOSPC;
> >  if ((buf = qemu_opt_get(opts, "werror")) != NULL) {
> > -if (type != IF_IDE && type != IF_SCSI && type != IF_VIRTIO && type 
> > != IF_NONE) {
> > +if (type != IF_IDE && type != IF_SCSI && type != IF_VIRTIO && 
> > +   type != IF_VIRTIO_SCSI && type != IF_NONE) {
> 
> code dupicated here and below. add a function?
> 
> >  error_report("werror is not supported by this bus type");
> >  return NULL;
> >  }
> > @@ -449,7 +451,8 @@ DriveInfo *drive_init(QemuOpts *opts, int 
> > default_to_scsi)
> > 
> >  on_read_error = BLOCK_ERR_REPORT;
> >  if ((buf = qemu_opt_get(opts, "rerror")) != NULL) {
> > -if (type != IF_IDE && type != IF_VIRTIO && type != IF_SCSI && type 
> > != IF_NONE) {
> > +if (type != IF_IDE && type != IF_VIRTIO && type != IF_SCSI && 
> > +   IF_VIRTIO_SCSI && type != IF_NONE) {
> 
> typo? IF_VIRTIO_SCSI is always != 0 ...
Ouch, ashamed for my carelessness.
> 
> >  error_report("rerror is not supported by this bus type");
> >  return NULL;
> >  }
> > @@ -461,7 +464,7 @@ DriveInfo *drive_init(QemuOpts *opts, int 
> > default_to_scsi)
> >  }
> > 
> >  if ((devaddr = qemu_opt_get(opts, "addr")) != NULL) {
> > -if (type != IF_VIRTIO) {
> > +if (type != IF_VIRTIO || type != IF_VIRTIO_SCSI) {
> >  error_report("addr is not supported by this bus type");
> >  return NULL;
> >  }
> > @@ -579,6 +582,14 @@ DriveInfo *drive_init(QemuOpts *opts, int 
> > default_to_scsi)
> >  if (devaddr)
> >  qemu_opt_set(opts, "addr", devaddr);
> >  break;
> > +   case IF_VIRTIO_SCSI:
> > +   opts = qemu_opts_create(qemu_find_opts("device"), NULL, 0);
> > +   qemu_opt_set(opts, "driver", "virtio-scsi-pci");
> > +   qemu_opt_set(opts, "drive", dinfo->id);
> > +   if (devaddr) {
> > +   qemu_opt_set(opts, "addr", devaddr);
> > +   }
> > +   break;
> >  default:
> >  abort();
> >  }
> > @@ -604,7 +615,7 @@ DriveInfo *drive_init(QemuOpts *opts, int 
> > default_to_scsi)
> >  ro = 1;
> >  } else if (ro == 1) {
> >  if (type != IF_SCSI && type != IF_VIRTIO && type != IF_FLOPPY &&
> > -type != IF_NONE && type != IF_PFLASH) {
> > +type != IF_NONE && type != IF_PFLASH && IF_VIRTIO_SCSI) {
> >  error_report("readonly not supported by this bus type");
> >  goto err;
> >  }
> > diff --git a/blockdev.h b/blockdev.h
> > index 260e16b..96f40a5 100644
> > --- a/blockdev.h
> > +++ b/blockdev.h
> > @@ -22,7 +22,8 @@ void blockdev_auto_del(BlockDriverState *bs);
> >  typedef enum {
> >  IF_DEFAULT = -1,/* for use with drive_add() only */
> >  IF_NONE,
> > -IF_IDE, IF_SCSI, IF_FLOPPY, IF_PFLASH, IF_MTD, IF_SD, IF_VIRTIO, 
> > IF_XEN,
> > +IF_IDE, IF_SCSI, IF_FL

Re: [Qemu-devel] [PATCH v2 15/15] net: invoke qemu_can_send_packet only before net queue sending function

[Zhi Yong Wu]

On Thu, May 24, 2012 at 12:00 AM, Paolo Bonzini  wrote:
> Il 23/05/2012 17:14, zwu.ker...@gmail.com ha scritto:
>> From: Zhi Yong Wu 
>>
>> Signed-off-by: Zhi Yong Wu 
>> ---
>>  net/queue.c      |    4 ++--
>>  net/slirp.c      |    7 ---
>>  net/tap.c        |    2 +-
>>  slirp/if.c       |    5 -
>>  slirp/libslirp.h |    1 -
>>  5 files changed, 3 insertions(+), 16 deletions(-)
>>
>> diff --git a/net/queue.c b/net/queue.c
>> index 0afd783..d2e57de 100644
>> --- a/net/queue.c
>> +++ b/net/queue.c
>> @@ -176,7 +176,7 @@ ssize_t qemu_net_queue_send(NetQueue *queue,
>>  {
>>      ssize_t ret;
>>
>> -    if (queue->delivering) {
>> +    if (queue->delivering || !qemu_can_send_packet(sender)) {
>>          return qemu_net_queue_append(queue, sender, flags, data, size, 
>> NULL);
>>      }
>>
>> @@ -200,7 +200,7 @@ ssize_t qemu_net_queue_send_iov(NetQueue *queue,
>>  {
>>      ssize_t ret;
>>
>> -    if (queue->delivering) {
>> +    if (queue->delivering || !qemu_can_send_packet(sender)) {
>>          return qemu_net_queue_append_iov(queue, sender, flags, iov, iovcnt, 
>> NULL);
>>      }
>>
>> diff --git a/net/slirp.c b/net/slirp.c
>> index a6ede2b..248f7ff 100644
>> --- a/net/slirp.c
>> +++ b/net/slirp.c
>> @@ -96,13 +96,6 @@ static void slirp_smb_cleanup(SlirpState *s);
>>  static inline void slirp_smb_cleanup(SlirpState *s) { }
>>  #endif
>>
>> -int slirp_can_output(void *opaque)
>> -{
>> -    SlirpState *s = opaque;
>> -
>> -    return qemu_can_send_packet(&s->nc);
>> -}
>> -
>>  void slirp_output(void *opaque, const uint8_t *pkt, int pkt_len)
>>  {
>>      SlirpState *s = opaque;
>> diff --git a/net/tap.c b/net/tap.c
>> index 65f45b8..7b1992b 100644
>> --- a/net/tap.c
>> +++ b/net/tap.c
>> @@ -210,7 +210,7 @@ static void tap_send(void *opaque)
>>          if (size == 0) {
>>              tap_read_poll(s, 0);
>>          }
>> -    } while (size > 0 && qemu_can_send_packet(&s->nc));
>> +    } while (size > 0);
>
> Can you explain this?  Also, have you benchmarked the change to see what
Its code execution flow is like below:
tap_send --> qemu_send_packet_async
->qemu_send_packet_async_with_flags ->qemu_net_queue_send

So it will finally invoke qemu_can_send_packet to determine if it can
send packets. this code change delay this determination.

> effect it has?
No, i have not done benchmark testing. When a lot of packets will go
to the guest from outside and this guest' NIC can_receive return
false, this change will cause CPU to execute some additional codes.

>
> Also, can you explain why you didn't implement this?
Hub can now do its own flow control if it provides its can_recieve.
Why need we add some counts to track in-flight packets? Do you think
that it can speed up the packets delivery? otherwise i think that it
complex the code. To be honest, i prefer current solution. Do you
think of this? :)

>
>>> If they did, hubs could then do their own flow control via can_receive.
>>> When qemu_send_packet returns zero you increment a count of in-flight
>>> packets, and a sent-packet callback would decrement the same count. When the
>>> count is non-zero, can_receive returns false (and vice versa).  The sent_cb
>>> also needs to call qemu_flush_queued_packets when the count drop to zero.
>>> With this in place, I think the other TODO about the return value is easily
>>> solved; receive/receive_iov callbacks can simply return immediate success,
>>> and later block further sends.
>
> Paolo
>
>>  }
>>
>>  int tap_has_ufo(NetClientState *nc)
>> diff --git a/slirp/if.c b/slirp/if.c
>> index 096cf6f..533295d 100644
>> --- a/slirp/if.c
>> +++ b/slirp/if.c
>> @@ -177,11 +177,6 @@ void if_start(Slirp *slirp)
>>      }
>>
>>      while (ifm_next) {
>> -        /* check if we can really output */
>> -        if (!slirp_can_output(slirp->opaque)) {
>> -            break;
>> -        }
>> -
>>          ifm = ifm_next;
>>          from_batchq = next_from_batchq;
>>
>> diff --git a/slirp/libslirp.h b/slirp/libslirp.h
>> index 77527ad..9b471b5 100644
>> --- a/slirp/libslirp.h
>> +++ b/slirp/libslirp.h
>> @@ -25,7 +25,6 @@ void slirp_select_poll(fd_set *readfds, fd_set *writefds, 
>> fd_set *xfds,
>>  void slirp_input(Slirp *slirp, const uint8_t *pkt, int pkt_len);
>>
>>  /* you must provide the following functions: */
>> -int slirp_can_output(void *opaque);
>>  void slirp_output(void *opaque, const uint8_t *pkt, int pkt_len);
>>
>>  int slirp_add_hostfwd(Slirp *slirp, int is_udp,
>



-- 
Regards,

Zhi Yong Wu

Re: [Qemu-devel] Virtio-pci issue

[Evgeny Voevodin]

On 23.05.2012 17:36, Stefan Hajnoczi wrote:

On Thu, May 10, 2012 at 12:24 PM, Evgeny Voevodin
  wrote:

Hi, guys!

While trying to refactor virtio-pci as continuation of my RFC
virtio-mmio patch series:
http://lists.gnu.org/archive/html/qemu-devel/2012-04/msg03665.html
I've faced a problem.

I try to create virtio-pci-transport device to which virtio-blk,net,etc
could be connected. Any board could create as many transport devices as
it wants and then any actual back-end could be connected to that
transport through a provided virtio-pci.x bus. virtio-pci-transport is
derived from TYPE_PCI_DEVICE.
The problem is that if I create transport devices and wish to connect
back-ends to them, each back-end would set its properties to the class,
not to the object.
For example, vendor_id property is different for blk and net, but since
they are both have same class PCIDeviceClass, they share that property.

How to deal with this?

I think you are asking how a virtio-pci adapter can have device_id
value that depends on its VirtioDevice (not known at compile-time and
different between virtio-pci adapters).

You should be able to set the PCI device ID at runtime:

pci_config_set_device_id(pci_dev->config, device_id_from_virtio_device);

Take a look at hw/pci.c:pci_qdev_init().  The order of operations is important:

1. do_pci_register_device() sets up default values from PCIDeviceClass.
2. ->init(pci_dev) gets called, this is where your init code gets executed.

So it should be fine to set the PCI device ID at runtime from
VirtioPCIAdapter::init().

Stefan



Oh, thanks a lot, Stefan! As soon as I have a time to continue 
refactoring, I'll check this functionality.
And also there is another problem that I've faced with. It is the 
ability to plug as many pci back-ends as board wants.
I mean that if for each back-end board should create a transport, then 
user have to know maximum number of transport instances
created by board. In the case of mmio transport I think that it's a 
correct behaviour, but for pci transport seems not.


--
Kind regards,
Evgeny Voevodin,
Leading Software Engineer,
ASWG, Moscow R&D center, Samsung Electronics
e-mail: e.voevo...@samsung.com

[Qemu-devel] [PATCH] vfio-powerpc: enabled and supported on power

[Alexey Kardashevskiy]

The patch introduces support of VFIO on POWER.

The patch consists of:

1. IOMMU driver for VFIO.
It does not use IOMMU API at all, instead it calls POWER
IOMMU API directly (ppc_md callbacks).

2. A piece of code (module_init) which creates IOMMU groups.
TBD: what is a better place for it?

The patch is made on top of
git://github.com/awilliam/linux-vfio.git iommu-group-vfio-20120523
(which is iommu-group-vfio-20120521 + some fixes)

Signed-off-by: Alexey Kardashevskiy 
---
 arch/powerpc/Kconfig |6 +
 arch/powerpc/include/asm/iommu.h |3 +
 arch/powerpc/kernel/Makefile |1 +
 arch/powerpc/kernel/iommu_vfio.c |  371 ++
 4 files changed, 381 insertions(+), 0 deletions(-)
 create mode 100644 arch/powerpc/kernel/iommu_vfio.c

diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
index feab3ba..13d12ac 100644
--- a/arch/powerpc/Kconfig
+++ b/arch/powerpc/Kconfig
@@ -319,6 +319,12 @@ config 8XX_MINIMAL_FPEMU
 config IOMMU_HELPER
def_bool PPC64
 
+config IOMMU_VFIO
+   select IOMMU_API
+   depends on PPC64
+   tristate "Enable IOMMU chardev to support user-space PCI"
+   default n
+
 config SWIOTLB
bool "SWIOTLB support"
default n
diff --git a/arch/powerpc/include/asm/iommu.h b/arch/powerpc/include/asm/iommu.h
index 957a83f..c64bce7 100644
--- a/arch/powerpc/include/asm/iommu.h
+++ b/arch/powerpc/include/asm/iommu.h
@@ -66,6 +66,9 @@ struct iommu_table {
unsigned long  it_halfpoint; /* Breaking point for small/large allocs */
spinlock_t it_lock;  /* Protects it_map */
unsigned long *it_map;   /* A simple allocation bitmap for now */
+#ifdef CONFIG_IOMMU_API
+   struct iommu_group *it_group;
+#endif
 };
 
 struct scatterlist;
diff --git a/arch/powerpc/kernel/Makefile b/arch/powerpc/kernel/Makefile
index f5808a3..7cfd68e 100644
--- a/arch/powerpc/kernel/Makefile
+++ b/arch/powerpc/kernel/Makefile
@@ -90,6 +90,7 @@ obj-$(CONFIG_RELOCATABLE_PPC32)   += reloc_32.o
 
 obj-$(CONFIG_PPC32)+= entry_32.o setup_32.o
 obj-$(CONFIG_PPC64)+= dma-iommu.o iommu.o
+obj-$(CONFIG_IOMMU_VFIO)   += iommu_vfio.o
 obj-$(CONFIG_KGDB) += kgdb.o
 obj-$(CONFIG_PPC_OF_BOOT_TRAMPOLINE)   += prom_init.o
 obj-$(CONFIG_MODULES)  += ppc_ksyms.o
diff --git a/arch/powerpc/kernel/iommu_vfio.c b/arch/powerpc/kernel/iommu_vfio.c
new file mode 100644
index 000..68a93dd
--- /dev/null
+++ b/arch/powerpc/kernel/iommu_vfio.c
@@ -0,0 +1,371 @@
+/*
+ * VFIO: IOMMU DMA mapping support for TCE on POWER
+ *
+ * Copyright (C) 2012 IBM Corp.  All rights reserved.
+ * Author: Alexey Kardashevskiy 
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ * Derived from original vfio_iommu_x86.c:
+ * Copyright (C) 2012 Red Hat, Inc.  All rights reserved.
+ * Author: Alex Williamson 
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#define DRIVER_VERSION  "0.1"
+#define DRIVER_AUTHOR   "a...@ozlabs.ru"
+#define DRIVER_DESC "POWER IOMMU chardev for VFIO"
+
+#define IOMMU_CHECK_EXTENSION  _IO(VFIO_TYPE, VFIO_BASE + 1)
+
+/*  API for POWERPC IOMMU  */
+
+#define POWERPC_IOMMU  2
+
+struct tce_iommu_info {
+   __u32 argsz;
+   __u32 dma32_window_start;
+   __u32 dma32_window_size;
+};
+
+#define POWERPC_IOMMU_GET_INFO _IO(VFIO_TYPE, VFIO_BASE + 12)
+
+struct tce_iommu_dma_map {
+   __u32 argsz;
+   __u64 va;
+   __u64 dmaaddr;
+};
+
+#define POWERPC_IOMMU_MAP_DMA  _IO(VFIO_TYPE, VFIO_BASE + 13)
+#define POWERPC_IOMMU_UNMAP_DMA_IO(VFIO_TYPE, VFIO_BASE + 14)
+
+/* * */
+
+struct tce_iommu {
+   struct iommu_table *tbl;
+};
+
+static int tce_iommu_attach_group(void *iommu_data,
+   struct iommu_group *iommu_group)
+{
+   struct tce_iommu *tceiommu = iommu_data;
+
+   if (tceiommu->tbl) {
+   printk(KERN_ERR "Only one group per IOMMU instance is 
allowed\n");
+   return -EFAULT;
+   }
+   tceiommu->tbl = iommu_group_get_iommudata(iommu_group);
+
+   return 0;
+}
+
+static void tce_iommu_detach_group(void *iommu_data,
+   struct iommu_group *iommu_group)
+{
+   struct tce_iommu *tceiommu = iommu_data;
+
+   if (!tceiommu->tbl) {
+   printk(KERN_ERR "IOMMU already released\n");
+   return;
+   }
+   tceiommu->tbl = NULL;
+}
+
+static void *tce_iommu_open(unsigned long arg)
+{
+   struct tce_iommu *tceiommu;
+
+   if (arg != POWERPC_IOMMU)
+   return ERR_PTR(-EINVAL);
+
+   tceiommu = kzalloc(sizeof(*tceiommu), GFP_KERNEL);
+   if (!tceiommu

[Qemu-devel] [Bug 393430] Re: kvm: use PulseAudio instead of ALSA

[Serge Hallyn]

** Changed in: kvm (Ubuntu)
   Status: Incomplete => Fix Released

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

Title:
  kvm: use PulseAudio instead of ALSA

Status in QEMU:
  Incomplete
Status in “kvm” package in Ubuntu:
  Fix Released
Status in “kvm” package in Debian:
  Fix Released

Bug description:
  Binary package hint: kvm

  kvm in jaunty prefers to use OSS drivers rather than ALSA:
  # dpkg -l | grep kvm
  ii  kvm1:84+dfsg-0ubuntu11Full virtualization on i386 
and amd64 hardwa
  # kvm -audio-help | grep ^Name
  Name: oss
  Name: alsa
  Name: sdl
  Name: pa
  Name: none
  Name: wav
  #

  Because of that, once a virtual machine with working audio card
  starts, the kvm grabs the audio card entirely for itself. Any
  subsequent operations on audio card either from the host machine, or
  from the other audio-enabled virtual machines lock up (apparently
  awaiting on on access to the audio card).

  The bug is echo of the corresponding debian bug:
  http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=508018

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

[Qemu-devel] [PATCH] version: update version info

[zwu . kernel]

From: Zhi Yong Wu 

Signed-off-by: Zhi Yong Wu 
---
 VERSION |2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/VERSION b/VERSION
index 87903b6..9084fa2 100644
--- a/VERSION
+++ b/VERSION
@@ -1 +1 @@
-1.0.93
+1.1.0
-- 
1.7.6

Re: [Qemu-devel] [PATCH] TCG: Fix TB invalidation after breakpoint insertion/deletion

[Jan Kiszka]

On 2012-05-23 23:34, Jan Kiszka wrote:
> From: Jan Kiszka 
> 
> tb_invalidate_phys_addr has to called with the exact physical address of
> the breakpoint we add/remove, not just the page's base address.
> Otherwise we easily fail to flush the right TB.
> 
> Regression of 1e7855a558.

Sorry, forgot the tag: this should go in before 1.1 of course.

> 
> Reported-by: TeLeMan 
> Signed-off-by: Jan Kiszka 
> ---
>  exec.c |3 ++-
>  1 files changed, 2 insertions(+), 1 deletions(-)
> 
> diff --git a/exec.c b/exec.c
> index a0494c7..efa1345 100644
> --- a/exec.c
> +++ b/exec.c
> @@ -1492,7 +1492,8 @@ void tb_invalidate_phys_addr(target_phys_addr_t addr)
>  
>  static void breakpoint_invalidate(CPUArchState *env, target_ulong pc)
>  {
> -tb_invalidate_phys_addr(cpu_get_phys_page_debug(env, pc));
> +tb_invalidate_phys_addr(cpu_get_phys_page_debug(env, pc) +
> +(pc & ~TARGET_PAGE_MASK));
>  }
>  #endif
>  #endif /* TARGET_HAS_ICE */



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH v2 13/15] net: Remove obsolete vlan info

[Zhi Yong Wu]

On Wed, May 23, 2012 at 11:41 PM, Jan Kiszka  wrote:
> On 2012-05-23 12:14, zwu.ker...@gmail.com wrote:
>> From: Zhi Yong Wu 
>>
>> Signed-off-by: Zhi Yong Wu 
>> ---
>>  net.c |    1 -
>>  1 files changed, 0 insertions(+), 1 deletions(-)
>>
>> diff --git a/net.c b/net.c
>> index 61dc28d..8c8e703 100644
>> --- a/net.c
>> +++ b/net.c
>> @@ -1079,7 +1079,6 @@ void do_info_network(Monitor *mon)
>>      NetClientState *nc, *peer;
>>      net_client_type type;
>>
>> -    monitor_printf(mon, "Devices not on any VLAN:\n");
>>      QTAILQ_FOREACH(nc, &net_clients, next) {
>>          peer = nc->peer;
>>          type = nc->info->type;
>
> This looks suspicious - or the patch description is improvable. This is
> really just about removing that headline? And what about the indention
> of the lines printed afterward?
As you have known, vlan concept is replaced with hub. So i think that
it is more reasonable to remove this in monitor.
>
> It also leads me to the question how hub-based networks will be
> visualized on "info network", specifically when there are multiple hubs.
> Can you provide some more complex example of an info network output?

(qemu) info network
  virtio-net-pci.0: type=nic,model=virtio-net-pci,macaddr=52:54:00:12:34:56
   \ hub0port0: type=(null),
  virtio-net-pci.1: type=nic,model=virtio-net-pci,macaddr=52:54:00:12:34:57
   \ hub1port0: type=(null),
hub 1
port 1 peer user.1
port 0 peer virtio-net-pci.1
hub 0
port 1 peer user.0
port 0 peer virtio-net-pci.0
>
> Jan
>
> --
> Siemens AG, Corporate Technology, CT T DE IT 1
> Corporate Competence Center Embedded Linux



-- 
Regards,

Zhi Yong Wu

[Qemu-devel] [PATCH] Clarify comments of tb_invalidate_phys_[page_]range

[Jan Kiszka]

From: Jan Kiszka 

They could suggest that all TBs of the page containing the range would
be invalidated.

Signed-off-by: Jan Kiszka 
---
 exec.c |   22 --
 1 files changed, 12 insertions(+), 10 deletions(-)

diff --git a/exec.c b/exec.c
index efa1345..a1c12ec 100644
--- a/exec.c
+++ b/exec.c
@@ -1076,11 +1076,11 @@ TranslationBlock *tb_gen_code(CPUArchState *env,
 }
 
 /*
- * invalidate all TBs which intersect with the target physical pages
- * starting in range [start;end[. NOTE: start and end may refer to
- * different physical pages. 'is_cpu_write_access' should be true if called
- * from a real cpu write access: the virtual CPU will exit the current
- * TB if code is modified inside this TB.
+ * Invalidate all TBs which intersect with the target physical address range
+ * [start;end[. NOTE: start and end may refer to *different* physical pages.
+ * 'is_cpu_write_access' should be true if called from a real cpu write
+ * access: the virtual CPU will exit the current TB if code is modified inside
+ * this TB.
  */
 void tb_invalidate_phys_range(tb_page_addr_t start, tb_page_addr_t end,
   int is_cpu_write_access)
@@ -1092,11 +1092,13 @@ void tb_invalidate_phys_range(tb_page_addr_t start, 
tb_page_addr_t end,
 }
 }
 
-/* invalidate all TBs which intersect with the target physical page
-   starting in range [start;end[. NOTE: start and end must refer to
-   the same physical page. 'is_cpu_write_access' should be true if called
-   from a real cpu write access: the virtual CPU will exit the current
-   TB if code is modified inside this TB. */
+/*
+ * Invalidate all TBs which intersect with the target physical address range
+ * [start;end[. NOTE: start and end must refer to the *same* physical page.
+ * 'is_cpu_write_access' should be true if called from a real cpu write
+ * access: the virtual CPU will exit the current TB if code is modified inside
+ * this TB.
+ */
 void tb_invalidate_phys_page_range(tb_page_addr_t start, tb_page_addr_t end,
int is_cpu_write_access)
 {



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] The image size of instance VM keeps growing

[Charles . Tsai-蔡清海-研究發展部]

Thanks. Good alternative view.

-Original Message-
From: Michael Tokarev [mailto:m...@tls.msk.ru] 
Sent: Wednesday, May 23, 2012 9:11 PM
To: Stefan Hajnoczi
Cc: Charles.Tsai-蔡清海-研究發展部; Jonah.Wu-吳君勉-研究發展部; qemu-devel@nongnu.org
Subject: Re: [Qemu-devel] The image size of instance VM keeps growing

On 23.05.2012 16:57, Stefan Hajnoczi wrote:
[]
> However, the qcow2 file should not grow much beyond its virtual disk 
> size.  So if you find the qcow2 is 3G but the virtual disk size is 1G, 
> then there is a bug.  But from what you've posted so far I suspect the 
> guest is simply writing to the disk.

Dont't forget about possible snapshots inside the qcow2 file.  So it is quite 
well possible to have qcow2 file sized much larger than virtual disk size :)

/mjt

[Qemu-devel] [PATCH] TCG: Fix TB invalidation after breakpoint insertion/deletion

[Jan Kiszka]

From: Jan Kiszka 

tb_invalidate_phys_addr has to called with the exact physical address of
the breakpoint we add/remove, not just the page's base address.
Otherwise we easily fail to flush the right TB.

Regression of 1e7855a558.

Reported-by: TeLeMan 
Signed-off-by: Jan Kiszka 
---
 exec.c |3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/exec.c b/exec.c
index a0494c7..efa1345 100644
--- a/exec.c
+++ b/exec.c
@@ -1492,7 +1492,8 @@ void tb_invalidate_phys_addr(target_phys_addr_t addr)
 
 static void breakpoint_invalidate(CPUArchState *env, target_ulong pc)
 {
-tb_invalidate_phys_addr(cpu_get_phys_page_debug(env, pc));
+tb_invalidate_phys_addr(cpu_get_phys_page_debug(env, pc) +
+(pc & ~TARGET_PAGE_MASK));
 }
 #endif
 #endif /* TARGET_HAS_ICE */
-- 
1.7.3.4



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] The image size of instance VM keeps growing

[Charles . Tsai-蔡清海-研究發展部]

Stefan,

Thank you for information. What we worried is an endless growth of the image 
size.
If virtual disk size is the ceiling of the max. image size to be grew, we 
should not have to worry about this issue.


-Original Message-
From: Stefan Hajnoczi [mailto:stefa...@gmail.com] 
Sent: Wednesday, May 23, 2012 8:57 PM
To: Charles.Tsai-蔡清海-研究發展部
Cc: qemu-devel@nongnu.org; Jonah.Wu-吳君勉-研究發展部
Subject: Re: [Qemu-devel] The image size of instance VM keeps growing

On Wed, May 23, 2012 at 11:47 AM, Charles.Tsai-蔡清海-研究發展部
 wrote:
>After the VM ran for overnight, the image size for the running VM grew 
> up to 3G bytes.

What is the size of the backing file?  3G is small for a Windows guest.  Any 
sector that is written to could cause 64 KB to be allocated in the image file.

If qemu-img check sees no inconsistencies then it seems the guest is writing to 
previously untouched regions of the disk...causing the
qcow2 to grow.

However, the qcow2 file should not grow much beyond its virtual disk size.  So 
if you find the qcow2 is 3G but the virtual disk size is 1G, then there is a 
bug.  But from what you've posted so far I suspect the guest is simply writing 
to the disk.

Stefan

Re: [Qemu-devel] [PATCH] exec: fix breakpoint_invalidate() breakage

[Jan Kiszka]

On 2012-05-23 23:00, Jan Kiszka wrote:
> On 2012-05-23 22:29, TeLeMan wrote:
>> On Thu, May 24, 2012 at 1:36 AM, Jan Kiszka  wrote:
>>> On 2012-05-23 13:02, Jan Kiszka wrote:
 On 2012-05-23 11:11, TeLeMan wrote:
> On Wed, May 23, 2012 at 7:22 PM, Jan Kiszka  
> wrote:
>> On 2012-05-23 04:09, TeLeMan wrote:
>>> On Wed, May 23, 2012 at 11:41 AM, Andreas Färber  
>>> wrote:
 Am 18.05.2012 11:49, schrieb TeLeMan:
> This breakage was introduced by the commit "memory: make
> phys_page_find() return an unadjusted".

 You seem to have found the origin of your problem. If you also mention
 the commit hash in your commit message then certain frontends (gitk,
 repo.or.cz) will display it as a handy hyperlink to that commit.

>
> Signed-off-by: TeLeMan 

 Signed-off-by is a legal statement of origin and must not be a 
 pseudonym.
>>> Ok, please ignore this patch. I won't submit any patch just report bugs.
>>
>> Then please describe this bug in more details, e.g. how to reproduce.
> I think its evident. cpu_get_phys_page_debug(env, pc) is not the
> physical address of pc but the physical page base address of pc.

 ...so this bites us if the instruction spans two pages as
 tb_invalidate_phys_addr requests invalidation on a page granularity.
>>>
>>> In fact, this is irrelevant. We only need to flush the address at which
>>> the instruction starts, and that is achieved by flushing all TB that
>>> relate to that page as the current code does.
>>
>> But the instruction start is wrong and its TB may not be found. For example,
>> the pc is 0x1234 and its physical address is 0x1234. The correct
>> "start" and "end" of tb_invalidate_phys_page_range() is 0x1234 and
>> 0x1235. But now the "start" and "end" is 0x1000 and 0x1001.
>> If 0x1000 is not translated yet, the real TB won't be invalidated.
> 
> The tb containing 0x1234 would be linked to the list of TBs that are
> related to the 0x1000 page. As we declare that page invalid, all
> affected TBs are dropped, not just the one containing the breakpoint.
> See tb_invalidate_phys_page_range.

Oops, too fast: in fact the introductory comment of
tb_invalidate_phys_page_range is misleading, there is a sub-page-level
range check. And now my test also actually triggers. Was probably
running the wrong qemu version before.

Jan



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH] exec: fix breakpoint_invalidate() breakage

[TeLeMan]

On Thu, May 24, 2012 at 10:00 AM, Jan Kiszka  wrote:
> On 2012-05-23 22:29, TeLeMan wrote:
>> On Thu, May 24, 2012 at 1:36 AM, Jan Kiszka  wrote:
>>> On 2012-05-23 13:02, Jan Kiszka wrote:
 On 2012-05-23 11:11, TeLeMan wrote:
> On Wed, May 23, 2012 at 7:22 PM, Jan Kiszka  
> wrote:
>> On 2012-05-23 04:09, TeLeMan wrote:
>>> On Wed, May 23, 2012 at 11:41 AM, Andreas Färber  
>>> wrote:
 Am 18.05.2012 11:49, schrieb TeLeMan:
> This breakage was introduced by the commit "memory: make
> phys_page_find() return an unadjusted".

 You seem to have found the origin of your problem. If you also mention
 the commit hash in your commit message then certain frontends (gitk,
 repo.or.cz) will display it as a handy hyperlink to that commit.

>
> Signed-off-by: TeLeMan 

 Signed-off-by is a legal statement of origin and must not be a 
 pseudonym.
>>> Ok, please ignore this patch. I won't submit any patch just report bugs.
>>
>> Then please describe this bug in more details, e.g. how to reproduce.
> I think its evident. cpu_get_phys_page_debug(env, pc) is not the
> physical address of pc but the physical page base address of pc.

 ...so this bites us if the instruction spans two pages as
 tb_invalidate_phys_addr requests invalidation on a page granularity.
>>>
>>> In fact, this is irrelevant. We only need to flush the address at which
>>> the instruction starts, and that is achieved by flushing all TB that
>>> relate to that page as the current code does.
>>
>> But the instruction start is wrong and its TB may not be found. For example,
>> the pc is 0x1234 and its physical address is 0x1234. The correct
>> "start" and "end" of tb_invalidate_phys_page_range() is 0x1234 and
>> 0x1235. But now the "start" and "end" is 0x1000 and 0x1001.
>> If 0x1000 is not translated yet, the real TB won't be invalidated.
>
> The tb containing 0x1234 would be linked to the list of TBs that are
> related to the 0x1000 page. As we declare that page invalid, all
> affected TBs are dropped, not just the one containing the breakpoint.
> See tb_invalidate_phys_page_range.

 if (!(tb_end <= start || tb_start >= end)) {
...
  tb_phys_invalidate(tb, -1);
...
 }

If start is wrong, tb_phys_invalidate() may not be called.

> Jan
>

Re: [Qemu-devel] [PATCH] exec: fix breakpoint_invalidate() breakage

[TeLeMan]

On Thu, May 24, 2012 at 4:44 AM, Anthony Liguori  wrote:
> On 05/23/2012 03:28 PM, Blue Swirl wrote:
>>
>> On Wed, May 23, 2012 at 8:04 PM, Andreas Färber  wrote:
>>>
>>> Am 23.05.2012 21:40, schrieb Blue Swirl:

 On Wed, May 23, 2012 at 3:41 AM, Andreas Färber
  wrote:
>
> Am 18.05.2012 11:49, schrieb TeLeMan:
>>
>> This breakage was introduced by the commit "memory: make
>> phys_page_find() return an unadjusted".
>
>
> You seem to have found the origin of your problem. If you also mention
> the commit hash in your commit message then certain frontends (gitk,
> repo.or.cz) will display it as a handy hyperlink to that commit.
>
>>
>> Signed-off-by: TeLeMan
>
>
> Signed-off-by is a legal statement of origin and must not be a
> pseudonym.


 $ git log --author=.*eleman
 commit c62f6d1d76aea587556c85b6b7b5c44167006264
 Author: TeLeMan
 Date:   Mon Jul 25 16:29:14 2011 +0800

     monitor: fix build breakage with --disable-vnc

     The breakage was introduced by the commit
 13661089810d3e59931f3e80d7cb541b99af7071

     Signed-off-by: TeLeMan
     Signed-off-by: Anthony Liguori

 and several others.
>>>
>>>
>>> Funny, since I learned that from Anthony. :-)
>
>
> Signed-off-by is a contractual statement and your real name is required:
>
> http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=blob;f=Documentation/SubmittingPatches#l339
>
> But I'm generally going to assume that if you SoB something, it's your real
> name.  I have challenged people in the past but only when it's very
> obviously fake.  If TeLeMan is not your real name, you cannot Signed-off-by
> with it.

Sorry, I did't notice this rule before. Now I don't want to violate
it. But I confuse myself if TeLeMan can be as my real name. Actually
TeLeMan is not my Chinese name. I used TeLeMan as my nickname and
English name. In other words TeLeMan can identify myself. I won't use
my Chinese name because I think it's my privacy. I used QEMU to do my
researching tools not as Andreas imagined.

> Regards,
>
> Anthony Liguori

Re: [Qemu-devel] [PATCH] exec: fix breakpoint_invalidate() breakage

[Jan Kiszka]

On 2012-05-23 22:29, TeLeMan wrote:
> On Thu, May 24, 2012 at 1:36 AM, Jan Kiszka  wrote:
>> On 2012-05-23 13:02, Jan Kiszka wrote:
>>> On 2012-05-23 11:11, TeLeMan wrote:
 On Wed, May 23, 2012 at 7:22 PM, Jan Kiszka  wrote:
> On 2012-05-23 04:09, TeLeMan wrote:
>> On Wed, May 23, 2012 at 11:41 AM, Andreas Färber  
>> wrote:
>>> Am 18.05.2012 11:49, schrieb TeLeMan:
 This breakage was introduced by the commit "memory: make
 phys_page_find() return an unadjusted".
>>>
>>> You seem to have found the origin of your problem. If you also mention
>>> the commit hash in your commit message then certain frontends (gitk,
>>> repo.or.cz) will display it as a handy hyperlink to that commit.
>>>

 Signed-off-by: TeLeMan 
>>>
>>> Signed-off-by is a legal statement of origin and must not be a 
>>> pseudonym.
>> Ok, please ignore this patch. I won't submit any patch just report bugs.
>
> Then please describe this bug in more details, e.g. how to reproduce.
 I think its evident. cpu_get_phys_page_debug(env, pc) is not the
 physical address of pc but the physical page base address of pc.
>>>
>>> ...so this bites us if the instruction spans two pages as
>>> tb_invalidate_phys_addr requests invalidation on a page granularity.
>>
>> In fact, this is irrelevant. We only need to flush the address at which
>> the instruction starts, and that is achieved by flushing all TB that
>> relate to that page as the current code does.
> 
> But the instruction start is wrong and its TB may not be found. For example,
> the pc is 0x1234 and its physical address is 0x1234. The correct
> "start" and "end" of tb_invalidate_phys_page_range() is 0x1234 and
> 0x1235. But now the "start" and "end" is 0x1000 and 0x1001.
> If 0x1000 is not translated yet, the real TB won't be invalidated.

The tb containing 0x1234 would be linked to the list of TBs that are
related to the 0x1000 page. As we declare that page invalid, all
affected TBs are dropped, not just the one containing the breakpoint.
See tb_invalidate_phys_page_range.

Jan



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH 10/21] qdev: do not propagate properties to subclasses

[Andreas Färber]

Am 24.05.2012 01:46, schrieb Andreas Färber:
> Am 02.05.2012 13:31, schrieb Paolo Bonzini:
>> As soon as we'll look up properties along the inheritance chain, we
>> will have duplicates if class A defines some properties and its
>> subclass B does not define any, because class_b->props will be
>> left equal to class_a->props.
>>
>> The solution here is to reintroduce the class_base_init TypeInfo
>> callback, that was present in one of the early QOM versions but
>> removed (on my request...) before committing.
>>
>> Signed-off-by: Paolo Bonzini 
> 
> This breaks `make check` once again:
> 
> GTESTER check-qtest-i386
> qemu-system-i386: Property '.id' not found

This was caused by the APIC 'id' property (dot in error message is
misleading). IIUC it's because it's defined for the base class. Solved
by squashing into the following commit. Applied to qom-next:
http://repo.or.cz/w/qemu/afaerber.git/shortlog/refs/heads/qom-next

Andreas

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

Re: [Qemu-devel] [PATCH] exec: fix breakpoint_invalidate() breakage

[TeLeMan]

On Thu, May 24, 2012 at 1:36 AM, Jan Kiszka  wrote:
> On 2012-05-23 13:02, Jan Kiszka wrote:
>> On 2012-05-23 11:11, TeLeMan wrote:
>>> On Wed, May 23, 2012 at 7:22 PM, Jan Kiszka  wrote:
 On 2012-05-23 04:09, TeLeMan wrote:
> On Wed, May 23, 2012 at 11:41 AM, Andreas Färber  wrote:
>> Am 18.05.2012 11:49, schrieb TeLeMan:
>>> This breakage was introduced by the commit "memory: make
>>> phys_page_find() return an unadjusted".
>>
>> You seem to have found the origin of your problem. If you also mention
>> the commit hash in your commit message then certain frontends (gitk,
>> repo.or.cz) will display it as a handy hyperlink to that commit.
>>
>>>
>>> Signed-off-by: TeLeMan 
>>
>> Signed-off-by is a legal statement of origin and must not be a pseudonym.
> Ok, please ignore this patch. I won't submit any patch just report bugs.

 Then please describe this bug in more details, e.g. how to reproduce.
>>> I think its evident. cpu_get_phys_page_debug(env, pc) is not the
>>> physical address of pc but the physical page base address of pc.
>>
>> ...so this bites us if the instruction spans two pages as
>> tb_invalidate_phys_addr requests invalidation on a page granularity.
>
> In fact, this is irrelevant. We only need to flush the address at which
> the instruction starts, and that is achieved by flushing all TB that
> relate to that page as the current code does.

But the instruction start is wrong and its TB may not be found. For example,
the pc is 0x1234 and its physical address is 0x1234. The correct
"start" and "end" of tb_invalidate_phys_page_range() is 0x1234 and
0x1235. But now the "start" and "end" is 0x1000 and 0x1001.
If 0x1000 is not translated yet, the real TB won't be invalidated.

> So, again my question: How can I reproduce the issue you see?
>
> Jan
>
> --
> Siemens AG, Corporate Technology, CT T DE IT 1
> Corporate Competence Center Embedded Linux

[Qemu-devel] buildbot failure in qemu on block_openbsd_current

[qemu]

The Buildbot has detected a new failure on builder block_openbsd_current while 
building qemu.
Full details are available at:
 http://buildbot.b1-systems.de/qemu/builders/block_openbsd_current/builds/234

Buildbot URL: http://buildbot.b1-systems.de/qemu/

Buildslave for this Build: brad_openbsd_current

Build Reason: The Nightly scheduler named 'nightly_block' triggered this build
Build Source Stamp: [branch block] HEAD
Blamelist: 

BUILD FAILED: failed compile

sincerely,
 -The Buildbot

[Qemu-devel] buildbot failure in qemu on block_openbsd_4.9

[qemu]

The Buildbot has detected a new failure on builder block_openbsd_4.9 while 
building qemu.
Full details are available at:
 http://buildbot.b1-systems.de/qemu/builders/block_openbsd_4.9/builds/222

Buildbot URL: http://buildbot.b1-systems.de/qemu/

Buildslave for this Build: kraxel_openbsd49

Build Reason: The Nightly scheduler named 'nightly_block' triggered this build
Build Source Stamp: [branch block] HEAD
Blamelist: 

BUILD FAILED: failed compile

sincerely,
 -The Buildbot

Re: [Qemu-devel] [PATCH 10/21] qdev: do not propagate properties to subclasses

[Andreas Färber]

Am 02.05.2012 13:31, schrieb Paolo Bonzini:
> As soon as we'll look up properties along the inheritance chain, we
> will have duplicates if class A defines some properties and its
> subclass B does not define any, because class_b->props will be
> left equal to class_a->props.
> 
> The solution here is to reintroduce the class_base_init TypeInfo
> callback, that was present in one of the early QOM versions but
> removed (on my request...) before committing.
> 
> Signed-off-by: Paolo Bonzini 

This breaks `make check` once again:

GTESTER check-qtest-i386
qemu-system-i386: Property '.id' not found

Andreas

> ---
>  hw/qdev.c |   11 +++
>  1 file changed, 11 insertions(+)
> 
> diff --git a/hw/qdev.c b/hw/qdev.c
> index 94fb32e..67d7770 100644
> --- a/hw/qdev.c
> +++ b/hw/qdev.c
> @@ -658,6 +658,16 @@ static void device_finalize(Object *obj)
>  QTAILQ_REMOVE(&dev->parent_bus->children, dev, sibling);
>  }
>  
> +static void device_class_base_init(ObjectClass *class, void *data)
> +{
> +DeviceClass *klass = DEVICE_CLASS(class);
> +
> +/* We explicitly look up properties in the superclasses,
> + * so do not propagate them to the subclasses.
> + */
> +klass->props = NULL;
> +}
> +
>  void device_reset(DeviceState *dev)
>  {
>  DeviceClass *klass = DEVICE_GET_CLASS(dev);
> @@ -684,6 +694,7 @@ static TypeInfo device_type_info = {
>  .instance_size = sizeof(DeviceState),
>  .instance_init = device_initfn,
>  .instance_finalize = device_finalize,
> +.class_base_init = device_class_base_init,
>  .abstract = true,
>  .class_size = sizeof(DeviceClass),
>  };

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

Re: [Qemu-devel] [PATCH 09/21] qdev: move bus properties to a separate global

[Andreas Färber]

Am 02.05.2012 13:31, schrieb Paolo Bonzini:
> Simple code movement in order to simplify future refactoring.
> 
> Signed-off-by: Paolo Bonzini 

Thanks, re-verified all movements on qdev-props-4 and applied to qom-next:
http://repo.or.cz/w/qemu/afaerber.git/shortlog/refs/heads/qom-next

Andreas

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

Re: [Qemu-devel] [PATCH 1.1 0/2]: qemu-ga: fix build on openbsd

[Michael Roth]

On Thu, May 24, 2012 at 12:15:18AM +0200, Andreas Färber wrote:
> Am 24.05.2012 00:02, schrieb Michael Roth:
> > On Wed, May 23, 2012 at 06:25:36PM -0300, Luiz Capitulino wrote:
> >> All the -Wno-redundant-decls warnings are still there on 5.1, if I remove 
> >> that
> >> flag I still get the following ones. Michael, do you want me to fix the 
> >> qemu-ga
> >> one for 1.1?
> [...]
> > The sprintf() has been there for a while now so I think it's okay to fix
> > later since openbsd does seem to have enough coverage to catch qemu-ga
> > breakages and it hasn't been mentioned yet.
> 
> The buildbot did catch it and so did I, unfortunately. The issue here is

An actual breakage due to the sprintf()? Or are you referring to the
original breakage due to environ not being declared? I agree the latter
needs to be in for 1.1

WRT to the sprintf(), there seem to be other offenders in the tree atm, which
is why I'm not sure it makes sense to target a qemu-ga specific fix for 1.1. It
seems like that kind breakage would've been present for a long time now,
not just for qemu-ga but other targets as well.

For example, here's the latest successful default_openbsd build (just prior to
the environ breakage):

http://buildbot.b1-systems.de/qemu/builders/default_openbsd_4.9/builds/271/steps/compile/logs/stdio

  LINK  alpha-softmmu/qemu-system-alpha
/usr/local/lib/libglib-2.0.so.2992.0: warning: vsprintf() is often misused, 
please use vsnprintf()
/usr/local/lib/libglib-2.0.so.2992.0: warning: stpcpy() is dangerous GNU crap; 
don't use it
../libdis/i386-dis.o(.text+0x325): In function `print_displacement':
: warning: strcpy() is almost always misused, please use strlcpy()
/usr/local/lib/libcurl.so.22.0: warning: strcat() is almost always misused, 
please use strlcat()

translate.o(.text+0x744): In function `cpu_alpha_init':
: warning: sprintf() is often misused, please use snprintf()

I'm not adamantly opposed or anything, though.

> rather that
> a) your qemu-ga branch is still not covered by the buildbots (Stefan
> told me the other day we need to contact Daniel Gollub for that), and
> b) Anthony tested all PULLs locally instead of pushing early so that the
> existing build bots could've detect any breakage on master before rc3
> was set in stone.
> 
> Andreas
> 
> -- 
> SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
> GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
> 

Re: [Qemu-devel] Signal management in qemu-user

[Peter Maydell]

On 23 May 2012 23:38, Alex Barcelo  wrote:
> This *always* goes wrong without calling the signal handler

I haven't looked too closely, but I suspect we're just not
paying any attention to whether memory does or doesn't have
the PROT_EXEC permission when we translate code from it.

This is the kind of corner case that the linux-user code is
often not very good at, because not many guest programs play
this sort of game.

-- PMM

Re: [Qemu-devel] [PATCH 1.1 0/2]: qemu-ga: fix build on openbsd

[Michael Roth]

On Thu, May 24, 2012 at 12:15:18AM +0200, Andreas Färber wrote:
> Am 24.05.2012 00:02, schrieb Michael Roth:
> > On Wed, May 23, 2012 at 06:25:36PM -0300, Luiz Capitulino wrote:
> >> All the -Wno-redundant-decls warnings are still there on 5.1, if I remove 
> >> that
> >> flag I still get the following ones. Michael, do you want me to fix the 
> >> qemu-ga
> >> one for 1.1?
> [...]
> > The sprintf() has been there for a while now so I think it's okay to fix
> > later since openbsd does seem to have enough coverage to catch qemu-ga
> > breakages and it hasn't been mentioned yet.
> 
> The buildbot did catch it and so did I, unfortunately. The issue here is
> rather that
> a) your qemu-ga branch is still not covered by the buildbots (Stefan
> told me the other day we need to contact Daniel Gollub for that), and

Hi Daniel,

Would it be possible to get QEMU Guest Agent tree covered by the QEMU build
bots? We've had a recent string of BSD breakages we'd like to start catching
in advance.

The staging branch is available at:

git://github.com/mdroth/qemu.git qga

> b) Anthony tested all PULLs locally instead of pushing early so that the
> existing build bots could've detect any breakage on master before rc3
> was set in stone.
> 
> Andreas
> 
> -- 
> SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
> GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
> 

Re: [Qemu-devel] Signal management in qemu-user

[Andreas Färber]

Am 24.05.2012 00:38, schrieb Alex Barcelo:
>>> Running it in a i386 machine works and gives an output of "0x0d\n0x20".
>>> Running it in a qemu-i386 segfaults. Because the self-modifying code
>>> raises a SIGSEGV in the qemu (I understand that it is the method used by
>>> qemu to handle self-modifying code). But the sigprocmask disables the
>>> SIGSEGV and the qemu-user... does nothing to avoid it. So the SIGSEGV is
>>> unmanaged and breaks the program.
>>
>> Alex has the following SIGSEGV workaround queued for our openSUSE package:
>> http://repo.or.cz/w/qemu/agraf.git/commit/0760e24b52ff20a328f168ed23b52c9b9c0fd28f
>>
>> Don't know if it fixes your specific problem. Peter had some ideas how
>> to refactor signal handling but iirc didn't have time to work on it himself.
> 
> Is it similar at all?

Peter answered that already: No, it isn't. Sorry for the confusion.

Andreas

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

Re: [Qemu-devel] Signal management in qemu-user

[Alex Barcelo]

>> Running it in a i386 machine works and gives an output of "0x0d\n0x20".
>> Running it in a qemu-i386 segfaults. Because the self-modifying code
>> raises a SIGSEGV in the qemu (I understand that it is the method used by
>> qemu to handle self-modifying code). But the sigprocmask disables the
>> SIGSEGV and the qemu-user... does nothing to avoid it. So the SIGSEGV is
>> unmanaged and breaks the program.
>
> Alex has the following SIGSEGV workaround queued for our openSUSE package:
> http://repo.or.cz/w/qemu/agraf.git/commit/0760e24b52ff20a328f168ed23b52c9b9c0fd28f
>
> Don't know if it fixes your specific problem. Peter had some ideas how
> to refactor signal handling but iirc didn't have time to work on it himself.

Is it similar at all? (it's easy for me to believe everyone who knows
more than me about qemu internal and patches --everybody in this
mailing list fits this xD). But this bug seems more subtle. I have a
modified test case, maybe a bit more natural:

#include 
#include 
#include 
#include 
#include 
#include 

unsigned char *testfun;

void sighandler(int signum) {
printf("Hello, I'm in a signal handler\n");
mprotect(testfun, 1024, PROT_READ|PROT_EXEC|PROT_WRITE);
testfun[ 1]=0x20;
}

int main ( void )
{
unsigned int ra;
signal(SIGSEGV, sighandler);
testfun=memalign(getpagesize(),1024);
/*
// We block the SIGSEGV signal, used by qemu-user
sigset_t set;
sigemptyset(&set);
sigaddset(&set, 11);
sigprocmask(SIG_BLOCK, &set, NULL);
*/
mprotect(testfun, 1024, PROT_READ|PROT_EXEC|PROT_WRITE);

//400687: b8 0d 00 00 00  mov$0xd,%eax
//40068d: c3  retq
testfun[ 0]=0xb8;
testfun[ 1]=0x0d;
testfun[ 2]=0x00;
testfun[ 3]=0x00;
testfun[ 4]=0x00;
testfun[ 5]=0xc3;
printf ( "0x%02X\n",
 ((unsigned int (*)())testfun)() );

mprotect(testfun, 1024, PROT_READ);

//400687: b8 20 00 00 00  mov$0x20,%eax
//40068d: c3  retq
// This self-modifying code will break because of the sigsegv signal block
printf ( "0x%02X\n",
 ((unsigned int (*)())testfun)() );
}

// --

This *always* goes wrong without calling the signal handler The output
in usermode emulation is "0x0D\n0x0D" when it should be:
---
0x0D
Hello, I'm in a signal handler
0x20
---
Alexander's patch hasn't any impact on the test case.

I'm a bit lost, and not sure what is the responsible of this. But it's
some kind of bug. I am eager to follow any leads and to think and
generate more test cases, but (maybe) I'm failing to see some basic
logic on signal flow.

Re: [Qemu-devel] [PATCH 1.1 0/2]: qemu-ga: fix build on openbsd

[Andreas Färber]

Am 24.05.2012 00:02, schrieb Michael Roth:
> On Wed, May 23, 2012 at 06:25:36PM -0300, Luiz Capitulino wrote:
>> All the -Wno-redundant-decls warnings are still there on 5.1, if I remove 
>> that
>> flag I still get the following ones. Michael, do you want me to fix the 
>> qemu-ga
>> one for 1.1?
[...]
> The sprintf() has been there for a while now so I think it's okay to fix
> later since openbsd does seem to have enough coverage to catch qemu-ga
> breakages and it hasn't been mentioned yet.

The buildbot did catch it and so did I, unfortunately. The issue here is
rather that
a) your qemu-ga branch is still not covered by the buildbots (Stefan
told me the other day we need to contact Daniel Gollub for that), and
b) Anthony tested all PULLs locally instead of pushing early so that the
existing build bots could've detect any breakage on master before rc3
was set in stone.

Andreas

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

Re: [Qemu-devel] [PATCH 1.1] es1370: Fix debug code

[malc]

On Wed, 23 May 2012, Stefan Weil wrote:

> When DEBUG_ES1370 is defined, the compiler shows these warnings:
> 

[..snip..]

Thanks, applied.

-- 
mailto:av1...@comtv.ru

Re: [Qemu-devel] [PATCH 1.1 0/2]: qemu-ga: fix build on openbsd

[Michael Roth]

On Wed, May 23, 2012 at 06:25:36PM -0300, Luiz Capitulino wrote:
> On Wed, 23 May 2012 15:08:43 -0500
> Michael Roth  wrote:
> 
> > On Wed, May 23, 2012 at 03:48:03PM -0300, Luiz Capitulino wrote:
> > > Build error found by buildbot:
> > > 
> > >  qga/commands-posix.c: In function 'qmp_guest_shutdown':
> > >  qga/commands-posix.c:65: error: 'environ' undeclared (first use in this 
> > > function)
> > >  qga/commands-posix.c:65: error: (Each undeclared identifier is reported 
> > > only once
> > >  qga/commands-posix.c:65: error: for each function it appears in.)
> > > 
> > > On F16 environ is declared in , but that obviously doesn't
> > > happen on all systems.
> > > 
> > > Tested on OpenBSD 4.9.
> 
> Just tested on 5.1 too.
> 
> > > PS: I get zillions of -Wno-redundant-decls errors on OpenBSD 4.8 and still
> > > get other warnnigs if I drop that flag.
> 
> All the -Wno-redundant-decls warnings are still there on 5.1, if I remove that
> flag I still get the following ones. Michael, do you want me to fix the 
> qemu-ga
> one for 1.1?

I'll take a patch, but unless it constitutes a build fix or an actual
overrun I don't think it's worth targetting for 1.1

The sprintf() has been there for a while now so I think it's okay to fix
later since openbsd does seem to have enough coverage to catch qemu-ga
breakages and it hasn't been mentioned yet.

The usage will also be safe for any architecture where pid_t is
defined to be to be 2^64 or less, so we should be good there as well.

> 
>   LINK  qemu-ga
> /usr/local/lib/libglib-2.0.so.2992.0: warning: vsprintf() is often misused, 
> please use vsnprintf()
> /usr/local/lib/libglib-2.0.so.2992.0: warning: stpcpy() is dangerous GNU 
> crap; don't use it
> /usr/local/lib/libglib-2.0.so.2992.0: warning: strcpy() is almost always 
> misused, please use strlcpy()
> qemu-ga.o(.text+0x358): In function `ga_open_pidfile':
> /home/lcapitulino/qmp-unstable/qemu-ga.c:258: warning: sprintf() is often 
> misused, please use snprintf()
> 

Re: [Qemu-devel] [PATCH qom-next 5/6] target-i386: make initialize CPU in QOM way

[Andreas Färber]

Am 23.05.2012 18:39, schrieb Igor Mammedov:
> Make CPU creation/initialization consistent with QOM object
> behavior in this, by moving tcg and apic initialization from board
> level into CPU's initfn/realize calls and cpu_model property setter.
> 
> Which makes CPU object self-sufficient in respect of creation/initialization
> and matches a typical object creation sequence, i.e.:
>   - create CPU instance
>   - set properties
>   - realize object - (x86_cpu_realize will be converted into realize
>   property setter, when it is implemented)
> 
> v2:
>   - fix moving of tcg_* initialization into cpu.c from helper.c
>   spotted-by: 
>   - make it compile/work on i386-linux-user target
> 
> Signed-off-by: Igor Mammedov 
> ---
>  hw/pc.c  |   45 
>  target-i386/cpu.c|   81 -
>  target-i386/helper.c |   39 
>  3 files changed, 85 insertions(+), 80 deletions(-)
> 
> diff --git a/hw/pc.c b/hw/pc.c
> index 1ccfc6e..d7845ea 100644
> --- a/hw/pc.c
> +++ b/hw/pc.c

> @@ -911,30 +891,17 @@ static void pc_cpu_reset(void *opaque)
>  cpu_reset(CPU(cpu));
>  }
>  
> -static X86CPU *pc_new_cpu(const char *cpu_model)
> -{
> -X86CPU *cpu;
> -CPUX86State *env;
> -
> -cpu = cpu_x86_init(cpu_model);
> -if (cpu == NULL) {
> -exit(1);
> -}
> -env = &cpu->env;
> -if ((env->cpuid_features & CPUID_APIC) || smp_cpus > 1) {
> -env->apic_state = apic_init(env, env->cpuid_apic_id);
> -}
> -qemu_register_reset(pc_cpu_reset, cpu);
> -pc_cpu_reset(cpu);
> -return cpu;
> -}
> -
>  void pc_cpus_init(const char *cpu_model)
>  {
> +X86CPU *cpu;
>  int i;
>  
>  for(i = 0; i < smp_cpus; i++) {

If we do changes here, please add the missing space. :)

> -pc_new_cpu(cpu_model);
> +cpu = cpu_x86_init(cpu_model);
> +if (cpu == NULL) {
> +exit(1);
> +}
> +qemu_register_reset(pc_cpu_reset, cpu);
>  }
>  }
>  
> diff --git a/target-i386/cpu.c b/target-i386/cpu.c
> index e655129..99ef891 100644
> --- a/target-i386/cpu.c
> +++ b/target-i386/cpu.c

> @@ -1749,24 +1753,89 @@ static void x86_set_cpu_model(Object *obj, const char 
> *value, Error **errp)
>  if (cpu_x86_register(cpu, env->cpu_model_str) < 0) {
>  fprintf(stderr, "Unable to find x86 CPU definition\n");
>  error_set(errp, QERR_INVALID_PARAMETER_COMBINATION);
> +return;
> +}
> +
> +#ifndef CONFIG_USER_ONLY
> +if (((env->cpuid_features & CPUID_APIC) || smp_cpus > 1)) {
> +if (kvm_irqchip_in_kernel()) {
> +env->apic_state = qdev_create(NULL, "kvm-apic");
> +} else if (xen_enabled()) {
> +env->apic_state = qdev_create(NULL, "xen-apic");
> +} else {
> +env->apic_state = qdev_create(NULL, "apic");
> +}
> +object_property_add_child(OBJECT(cpu), "apic",
> +OBJECT(env->apic_state), NULL);
> +
> +qdev_prop_set_uint8(env->apic_state, "id", env->cpuid_apic_id);
> +qdev_prop_set_ptr(env->apic_state, "cpu_env", env);

I'd like to avoid re-adding this set_ptr(). We can cherry-pick my
link property from QOM CPUState part 4 series.

> +}
> +#endif
> +}
> +
> +#ifndef CONFIG_USER_ONLY
> +static CPUDebugExcpHandler *prev_debug_excp_handler;
> +
> +static void breakpoint_handler(CPUX86State *env)
> +{
> +CPUBreakpoint *bp;
> +
> +if (env->watchpoint_hit) {
> +if (env->watchpoint_hit->flags & BP_CPU) {
> +env->watchpoint_hit = NULL;
> +if (check_hw_breakpoints(env, 0)) {
> +raise_exception_env(EXCP01_DB, env);
> +} else {
> +cpu_resume_from_signal(env, NULL);
> +}
> +}
> +} else {
> +QTAILQ_FOREACH(bp, &env->breakpoints, entry)
> +if (bp->pc == env->eip) {
> +if (bp->flags & BP_CPU) {
> +check_hw_breakpoints(env, 1);
> +raise_exception_env(EXCP01_DB, env);
> +}
> +break;
> +}
> +}
> +if (prev_debug_excp_handler) {
> +prev_debug_excp_handler(env);
>  }
>  }
> +#endif
>  
>  void x86_cpu_realize(Object *obj, Error **errp)
>  {
>  X86CPU *cpu = X86_CPU(obj);
> +#ifndef CONFIG_USER_ONLY
> +CPUX86State *env = &cpu->env;
> +
> +if (env->apic_state) {
> +if (qdev_init(env->apic_state) < 0) {
> +error_set(errp, QERR_DEVICE_INIT_FAILED,
> +object_get_typename(OBJECT(env->apic_state)));
> +return;
> +}
> +}
> +#endif
>  
>  mce_init(cpu);
> -qemu_init_vcpu(&cpu->env);
> +qemu_init_vcpu(env);

This only works because currently qemu_init_vcpu() is a no-op macro that
doesn't use the parameter. Please don't change it back, I guess it's a
mismerge.

We can avoid the env variable if I finish merging Paolo's seri

Re: [Qemu-devel] [PATCH qom-next 4/6] pc: move apic_mapped initialization into common apic init code

[Peter Maydell]

On 23 May 2012 22:09, Igor Mammedov  wrote:
> For cpu-hotplug it was suggested to use device_add/del
> interface for it. To do so in a generalized way hot-plugged cpu
> should follow general QOM object creation sequence, i.e.
>  - create new cpu instance
>  - set properties
>  - realize instance
> without creating precedent of special case for cpus in device_add/del
> if possible. So goal is to have a self-sufficient cpu object that
> doesn't require external hooks to create/initialize it. It looks
> possible do so for target-i386 at least.

I think your self-sufficient CPU object should probably be a
container QOM object which contains the CPU core itself and
the APIC device. Then the container object's initialisation
can map the APIC device.

-- PMM

[Qemu-devel] [PATCH 1.1] es1370: Fix debug code

[Stefan Weil]

When DEBUG_ES1370 is defined, the compiler shows these warnings:

hw/es1370.c: In function ‘es1370_update_voices’:
hw/es1370.c:414: warning: format ‘%d’ expects type ‘int’, but argument 3 has 
type ‘size_t’
hw/es1370.c: In function ‘es1370_writel’:
hw/es1370.c:582: warning: format ‘%d’ expects type ‘int’, but argument 3 has 
type ‘long int’
hw/es1370.c:592: warning: format ‘%d’ expects type ‘int’, but argument 3 has 
type ‘long int’
hw/es1370.c:609: warning: format ‘%d’ expects type ‘int’, but argument 3 has 
type ‘long int’
hw/es1370.c: In function ‘es1370_readl’:
hw/es1370.c:751: warning: suggest braces around empty body in an ‘if’ statement

Fix the format strings and add the missing braces.

Signed-off-by: Stefan Weil 
---
 hw/es1370.c |   11 ++-
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/hw/es1370.c b/hw/es1370.c
index f19cef3..573f747 100644
--- a/hw/es1370.c
+++ b/hw/es1370.c
@@ -410,7 +410,7 @@ static void es1370_update_voices (ES1370State *s, uint32_t 
ctl, uint32_t sctl)
 
 if ((old_fmt != new_fmt) || (old_freq != new_freq)) {
 d->shift = (new_fmt & 1) + (new_fmt >> 1);
-ldebug ("channel %d, freq = %d, nchannels %d, fmt %d, shift %d\n",
+ldebug ("channel %zu, freq = %d, nchannels %d, fmt %d, shift %d\n",
 i,
 new_freq,
 1 << (new_fmt & 1),
@@ -578,7 +578,7 @@ IO_WRITE_PROTO (es1370_writel)
 d++;
 case ES1370_REG_DAC1_SCOUNT:
 d->scount = (val & 0x) | (d->scount & ~0x);
-ldebug ("chan %d CURR_SAMP_CT %d, SAMP_CT %d\n",
+ldebug ("chan %td CURR_SAMP_CT %d, SAMP_CT %d\n",
 d - &s->chan[0], val >> 16, (val & 0x));
 break;
 
@@ -588,7 +588,7 @@ IO_WRITE_PROTO (es1370_writel)
 d++;
 case ES1370_REG_DAC1_FRAMEADR:
 d->frame_addr = val;
-ldebug ("chan %d frame address %#x\n", d - &s->chan[0], val);
+ldebug ("chan %td frame address %#x\n", d - &s->chan[0], val);
 break;
 
 case ES1370_REG_PHANTOM_FRAMECNT:
@@ -605,7 +605,7 @@ IO_WRITE_PROTO (es1370_writel)
 case ES1370_REG_DAC1_FRAMECNT:
 d->frame_cnt = val;
 d->leftover = 0;
-ldebug ("chan %d frame count %d, buffer size %d\n",
+ldebug ("chan %td frame count %d, buffer size %d\n",
 d - &s->chan[0], val >> 16, val & 0x);
 break;
 
@@ -745,9 +745,10 @@ IO_READ_PROTO (es1370_readl)
 {
 uint32_t size = ((d->frame_cnt & 0x) + 1) << 2;
 uint32_t curr = ((d->frame_cnt >> 16) + 1) << 2;
-if (curr > size)
+if (curr > size) {
 dolog ("read framecnt curr %d, size %d %d\n", curr, size,
curr > size);
+}
 }
 #endif
 break;
-- 
1.7.10

Re: [Qemu-devel] [PATCH 1.1 0/2]: qemu-ga: fix build on openbsd

[Luiz Capitulino]

On Wed, 23 May 2012 15:08:43 -0500
Michael Roth  wrote:

> On Wed, May 23, 2012 at 03:48:03PM -0300, Luiz Capitulino wrote:
> > Build error found by buildbot:
> > 
> >  qga/commands-posix.c: In function 'qmp_guest_shutdown':
> >  qga/commands-posix.c:65: error: 'environ' undeclared (first use in this 
> > function)
> >  qga/commands-posix.c:65: error: (Each undeclared identifier is reported 
> > only once
> >  qga/commands-posix.c:65: error: for each function it appears in.)
> > 
> > On F16 environ is declared in , but that obviously doesn't
> > happen on all systems.
> > 
> > Tested on OpenBSD 4.9.

Just tested on 5.1 too.

> > PS: I get zillions of -Wno-redundant-decls errors on OpenBSD 4.8 and still
> > get other warnnigs if I drop that flag.

All the -Wno-redundant-decls warnings are still there on 5.1, if I remove that
flag I still get the following ones. Michael, do you want me to fix the qemu-ga
one for 1.1?

  LINK  qemu-ga
/usr/local/lib/libglib-2.0.so.2992.0: warning: vsprintf() is often misused, 
please use vsnprintf()
/usr/local/lib/libglib-2.0.so.2992.0: warning: stpcpy() is dangerous GNU crap; 
don't use it
/usr/local/lib/libglib-2.0.so.2992.0: warning: strcpy() is almost always 
misused, please use strlcpy()
qemu-ga.o(.text+0x358): In function `ga_open_pidfile':
/home/lcapitulino/qmp-unstable/qemu-ga.c:258: warning: sprintf() is often 
misused, please use snprintf()

Re: [Qemu-devel] [PATCH qom-next 4/6] pc: move apic_mapped initialization into common apic init code

[Igor Mammedov]

- Original Message -
> From: "Peter Maydell" 
> To: "Igor Mammedov" 
> Cc: aligu...@us.ibm.com, "wei liu2" , 
> ehabk...@redhat.com, "stefano stabellini"
> , s...@weilnetz.de, mtosa...@redhat.com, 
> qemu-devel@nongnu.org, ag...@suse.de,
> blauwir...@gmail.com, a...@redhat.com, "jan kiszka" , 
> "anthony perard"
> , pbonz...@redhat.com, afaer...@suse.de
> Sent: Wednesday, May 23, 2012 6:44:30 PM
> Subject: Re: [Qemu-devel] [PATCH qom-next 4/6] pc: move apic_mapped 
> initialization into common apic init code
>
> On 23 May 2012 17:39, Igor Mammedov  wrote:
> > @@ -295,6 +297,15 @@ static int apic_init_common(SysBusDevice *dev)
> >
> >     sysbus_init_mmio(dev, &s->io_memory);
> >
> > +    /* XXX: mapping more APICs at the same memory location */
> > +    if (apic_mapped == 0) {
> > +        /* NOTE: the APIC is directly connected to the CPU - it is
> > not
> > +           on the global memory bus. */
> > +        /* XXX: what if the base changes? */
> > +        sysbus_mmio_map(sysbus_from_qdev(&s->busdev.qdev), 0,
> > MSI_ADDR_BASE);
> > +        apic_mapped = 1;
> > +    }
> > +
> >     if (!vapic && s->vapic_control & VAPIC_ENABLE_MASK) {
> >         vapic = sysbus_create_simple("kvmvapic", -1, NULL);
> >     }
>
> This looks wrong -- sysbus device init functions shouldn't
> be mapping MMIO regions themselves, in general. They should
> expose MMIO regions to be mapped by whichever device or board
> model creates them. Which is what the code before this patch
> was doing -- why do you want to move this code?

Why:
For cpu-hotplug it was suggested to use device_add/del
interface for it. To do so in a generalized way hot-plugged cpu
should follow general QOM object creation sequence, i.e.
  - create new cpu instance
  - set properties
  - realize instance
without creating precedent of special case for cpus in device_add/del
if possible. So goal is to have a self-sufficient cpu object that
doesn't require external hooks to create/initialize it. It looks
possible do so for target-i386 at least.

Some thoughts why mapping should be inside of apic object initfn:

LAPIC is a part of cpu so it's created and initialized by cpu. (not
true for 486 but for later cpus should be)
I've looked in Intel SDM and chapter "10.4.4 Local APIC Status and Location"
says:

"APIC Base field, bits 12 through 35 ⎯ Specifies the base address of the APIC
registers. ---cut--- Following a power-up or reset, the field is set to FEE0 
H."

it suggests that apic base is mapped right after cpu power-on and power-on 
could be
roughly mapped to object_new(),set_prop,realize sequence for cpu. So when cpu
creates child object "apic", then mapping of default apic base inside apic/cpu
objects looks more appropriate. (depending on cpu model, but we could chose a 
more
convenient case to implement).

Thanks,
  Igor

Re: [Qemu-devel] [PATCH qom-next 00/59] QOM CPUState, part 4: CPU_COMMON

[Blue Swirl]

On Wed, May 23, 2012 at 3:07 AM, Andreas Färber  wrote:
> Hello,
>
> This series, based on qom-next and the two pending ARM cleanup patches, starts
> moving fields from CPUArchState (CPU_COMMON) to QOM CPUState. It stops short
> of moving all easily possible fields (i.e., those not depending on 
> target_ulong
> or target_phys_addr_t) since the series got too long already and is expected 
> to
> spark some controversies due to collisions with several other series.
>
> The series is structured as preparatory refactorings interwoven with the 
> actual
> touch-all movement of one field ("cpu: Move ... to CPUState"), optionally
> followed by type signature cleanups, culminating in the movement of two fields
> that are tied together by VMState.
> Thus, unlike part 3, this series cannot randomly be cherry-picked to
> -next trees, only select parts thereof (e.g., use of cpu_s390x_init()).
>
> Please review and test.
>
> The use of cpu_index vs. cpuid_apic_id for x86 cpu[n] still needs some 
> thought.
>
> The question was brought up whether adding the CPUs a child properties
> should be generalized outside the machine scope - I don't think so, since CPU
> hotplug seems highly architecture-specific and not applicable everywhere 
> (SoCs).
>
> Blue will likely have a superb idea how to avoid the cpu_tlb_flush() 
> indirection
> that I needed for VMState, but apart from having been a lot of dumb typing, it
> works fine as interim solution. "Blah." wasn't terribly helpful as a comment.

Unfortunately I don't have superb ideas today (as if I had them any
other day...), only second rate jokes (as if they could be called
jokes...). With 'Blah' I obviously meant that I didn't have a solution
for that particular target_ulong/target_phys_addr_t problem. I'll try
to improve on all these areas, if you know what I mean.

>
> I have checked this to compile on ...
> * openSUSE 12.1 x86_64 w/KVM,
> * openSUSE Factory ppc w/KVM,
> * SLES 11 SP2 s390x w/KVM,
> * mingw32/64 cross-builds,
> * OpenBSD 5.1 amd64 (not for final version though, master doesn't build).
> Untested: Xen.
> Only some targets including i386 were lightly runtime-tested.
>
> Available for testing and cherry-picking (not pulling!) from:
> git://github.com/afaerber/qemu-cpu.git qom-cpu-common.v1
> https://github.com/afaerber/qemu-cpu/commits/qom-cpu-common.v1
>
> Regards,
> Andreas
>
> Cc: Anthony Liguori 
> Cc: Paolo Bonzini 
> Cc: Igor Mammedov 
>
> Cc: Richard Henderson 
> Cc: Peter Maydell 
> Cc: Edgar E. Iglesias 
> Cc: Michael Walle 
> Cc: Aurélien Jarno 
> Cc: Alexander Graf 
> Cc: David Gibson 
> Cc: qemu-ppc 
> Cc: Blue Swirl 
> Cc: Guan Xuetao 
> Cc: Max Filippov 
>
> Cc: Avi Kivity 
> Cc: Marcelo Tosatti 
> Cc: Jan Kiszka 
> Cc: kvm 
>
> Cc: Stefano Stabellini 
> Cc: xen-devel 
>
> Changes from preview in Igor's apic thread:
> * Use g_strdup_printf() for "cpu[x]" to be safe wrt length and nul 
> termination.
> * Clean up removal of x86 version 5 load/save support.
> * Convert use of env->halted in s390x KVM code.
> * Convert some uses of env->halted/interrupt_request in ppc KVM code.
> * Convert some uses of env->halted in Xen code, prepend cpu_x86_init() patch.
> * Avoid using POWERPC_CPU() / SPARC_CPU() macros inside *_set_irq() functions.
>
> Andreas Färber (59):
>  qemu-thread: Let qemu_thread_is_self() return bool
>  cpu: Move CPU_COMMON_THREAD into CPUState
>  cpu: Move thread field into CPUState
>  pc: Add CPU as /machine/cpu[n]
>  apic: Replace cpu_env pointer by X86CPU link
>  pc: Pass X86CPU to cpu_is_bsp()
>  cpu: Move thread_kicked to CPUState
>  Makefile.dis: Add include/ to include path
>  cpus: Pass CPUState to qemu_cpu_is_self()
>  cpus: Pass CPUState to qemu_cpu_kick_thread()
>  cpu: Move created field to CPUState
>  cpu: Move stop field to CPUState
>  ppce500_spin: Store PowerPCCPU in SpinKick
>  cpu: Move stopped field to CPUState
>  cpus: Pass CPUState to cpu_is_stopped()
>  cpus: Pass CPUState to cpu_can_run()
>  cpu: Move halt_cond to CPUState
>  cpus: Pass CPUState to qemu_tcg_cpu_thread_fn
>  cpus: Pass CPUState to qemu_tcg_init_vcpu()
>  ppc: Pass PowerPCCPU to ppc6xx_set_irq()
>  ppc: Pass PowerPCCPU to ppc970_set_irq()
>  ppc: Pass PowerPCCPU to power7_set_irq()
>  ppc: Pass PowerPCCPU to ppc40x_set_irq()
>  ppc: Pass PowerPCCPU to ppce500_set_irq()
>  sun4m: Pass SPARCCPU to cpu_set_irq()
>  sun4m: Pass SPARCCPU to cpu_kick_irq()
>  sun4u: Pass SPARCCPU to {,s,hs}tick_irq() and cpu_timer_create()
>  sun4u: Pass SPARCCPU to cpu_kick_irq()
>  target-ppc: Rename kvm_kick_{env => cpu} and pass PowerPCCPU
>  target-s390x: Let cpu_s390x_init() return S390CPU
>  s390-virtio: Use cpu_s390x_init() to obtain S390CPU
>  s390-virtio: Let s390_cpu_addr2state() return S390CPU
>  target-s390x: Pass S390CPU to s390_cpu_restart()
>  cpus: Pass CPUState to qemu_cpu_kick()
>  cpu: Move queued_work_{first,last} to CPUState
>  cpus: Pass CPUState to flush_queued_work()
>  cpus: Pass CPUState to qemu_wait_io_event_common()
>  target-ppc: Pass Pow

Re: [Qemu-devel] [PATCH 1/1 v4] Allow machines to configure the QEMU_VERSION that's exposed via hardware

[Peter Maydell]

On 23 May 2012 21:06, Crístian Viana  wrote:
> This would be the new code:
>
> snprintf((void *) w, 12, "QEMU %s", qemu_get_version()); /* char version[12] 
> */
>
> I'm not sure of what value the pointer contains at that moment,
> concatenating doesn't seem safe to me. What if w already contains a string?
> The result won't be the same.

The point is that your snprintf is not actually using the full power of
a format string parser, it's just concatenating two strings ("QEMU "
and the version). The simple way to put two strings into a buffer
one after the other is to copy string A and then concatenate string
B on the end.

> I don't understand the Nokia code, so I prefer
> to leave it as it was before (with snprintf).

The Nokia code as it was before doesn't use sprintf or snprintf.

We're just filling in a buffer in memory, and we have a char[12] array,
as the comment says. (NB that w is an int16_t*, which is why w+=6 moves
us over the array.)

What you want is something like:

   strcpy((void*)w, "QEMU ");
   pstrcat((void*)w, 12, qemu_get_version());

instead of the current single strcpy().

> I don't understand the Nokia code, so I prefer
> to leave it as it was before (with snprintf).

The Nokia code as it was before doesn't use sprintf or snprintf.

-- PMM

[Qemu-devel] [PATCH 1.1] vga: Initialise VRAM with 0

[Stefan Weil]

The VNC code reads this memory before it is written by BIOS or
other code. Avoid random values by setting the VRAM to 0.

This bug was reported by Valgrind.

Signed-off-by: Stefan Weil 
---
 hw/vga.c |1 +
 1 file changed, 1 insertion(+)

diff --git a/hw/vga.c b/hw/vga.c
index 1469680..80b8ec3 100644
--- a/hw/vga.c
+++ b/hw/vga.c
@@ -2283,6 +2283,7 @@ void vga_common_init(VGACommonState *s, int vga_ram_size)
 s->update_retrace_info = vga_precise_update_retrace_info;
 break;
 }
+memset(s->vram_ptr, 0, s->vram_size);
 vga_dirty_log_start(s);
 }
 
-- 
1.7.10

Re: [Qemu-devel] [PATCH] exec: fix breakpoint_invalidate() breakage

[Anthony Liguori]

On 05/23/2012 03:28 PM, Blue Swirl wrote:

On Wed, May 23, 2012 at 8:04 PM, Andreas Färber  wrote:

Am 23.05.2012 21:40, schrieb Blue Swirl:

On Wed, May 23, 2012 at 3:41 AM, Andreas Färber  wrote:

Am 18.05.2012 11:49, schrieb TeLeMan:

This breakage was introduced by the commit "memory: make
phys_page_find() return an unadjusted".


You seem to have found the origin of your problem. If you also mention
the commit hash in your commit message then certain frontends (gitk,
repo.or.cz) will display it as a handy hyperlink to that commit.



Signed-off-by: TeLeMan


Signed-off-by is a legal statement of origin and must not be a pseudonym.


$ git log --author=.*eleman
commit c62f6d1d76aea587556c85b6b7b5c44167006264
Author: TeLeMan
Date:   Mon Jul 25 16:29:14 2011 +0800

 monitor: fix build breakage with --disable-vnc

 The breakage was introduced by the commit
13661089810d3e59931f3e80d7cb541b99af7071

 Signed-off-by: TeLeMan
 Signed-off-by: Anthony Liguori

and several others.


Funny, since I learned that from Anthony. :-)


Signed-off-by is a contractual statement and your real name is required:

http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=blob;f=Documentation/SubmittingPatches#l339

But I'm generally going to assume that if you SoB something, it's your real 
name.  I have challenged people in the past but only when it's very obviously 
fake.  If TeLeMan is not your real name, you cannot Signed-off-by with it.


Regards,

Anthony Liguori

Re: [Qemu-devel] [PATCH] exec: fix breakpoint_invalidate() breakage

[Blue Swirl]

On Wed, May 23, 2012 at 8:04 PM, Andreas Färber  wrote:
> Am 23.05.2012 21:40, schrieb Blue Swirl:
>> On Wed, May 23, 2012 at 3:41 AM, Andreas Färber  wrote:
>>> Am 18.05.2012 11:49, schrieb TeLeMan:
 This breakage was introduced by the commit "memory: make
 phys_page_find() return an unadjusted".
>>>
>>> You seem to have found the origin of your problem. If you also mention
>>> the commit hash in your commit message then certain frontends (gitk,
>>> repo.or.cz) will display it as a handy hyperlink to that commit.
>>>

 Signed-off-by: TeLeMan 
>>>
>>> Signed-off-by is a legal statement of origin and must not be a pseudonym.
>>
>> $ git log --author=.*eleman
>> commit c62f6d1d76aea587556c85b6b7b5c44167006264
>> Author: TeLeMan 
>> Date:   Mon Jul 25 16:29:14 2011 +0800
>>
>>     monitor: fix build breakage with --disable-vnc
>>
>>     The breakage was introduced by the commit
>> 13661089810d3e59931f3e80d7cb541b99af7071
>>
>>     Signed-off-by: TeLeMan 
>>     Signed-off-by: Anthony Liguori 
>>
>> and several others.
>
> Funny, since I learned that from Anthony. :-)
>
> Anyway, the only non-childish reason to contribute under a pseudonym (as
> opposed to a known alias*) I can imagine is a contribution from someone
> working on a competing commercial emulation/virtualization solution who
> doesn't want her employer finding out, in which case it is questionable
> whether she can actually sign off her own code as it may conflict with
> non-free company IP. Any comments, Mr. Blauwirbel? ;)

I don't think the reason you imagined is very probable and I can
imagine other reasons. ;-)

>
> Andreas
>
> * e.g., malc, kraxel, mmu_man
>
> --
> SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
> GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

Re: [Qemu-devel] [PATCH 1.1 0/2]: qemu-ga: fix build on openbsd

[Michael Roth]

On Wed, May 23, 2012 at 03:48:03PM -0300, Luiz Capitulino wrote:
> Build error found by buildbot:
> 
>  qga/commands-posix.c: In function 'qmp_guest_shutdown':
>  qga/commands-posix.c:65: error: 'environ' undeclared (first use in this 
> function)
>  qga/commands-posix.c:65: error: (Each undeclared identifier is reported only 
> once
>  qga/commands-posix.c:65: error: for each function it appears in.)
> 
> On F16 environ is declared in , but that obviously doesn't
> happen on all systems.
> 
> Tested on OpenBSD 4.9.
> 
> PS: I get zillions of -Wno-redundant-decls errors on OpenBSD 4.8 and still
> get other warnnigs if I drop that flag.
> 
>  configure|   19 +++
>  qga/commands-posix.c |6 +-
>  2 files changed, 24 insertions(+), 1 deletion(-)
> 

Thanks, applied to qga branch. Will send a 1.1 pull request for these.

Re: [Qemu-devel] [PATCH qom-next 4/6] pc: move apic_mapped initialization into common apic init code

[Jan Kiszka]

On 2012-05-23 13:44, Peter Maydell wrote:
> On 23 May 2012 17:39, Igor Mammedov  wrote:
>> @@ -295,6 +297,15 @@ static int apic_init_common(SysBusDevice *dev)
>>
>> sysbus_init_mmio(dev, &s->io_memory);
>>
>> +/* XXX: mapping more APICs at the same memory location */
>> +if (apic_mapped == 0) {
>> +/* NOTE: the APIC is directly connected to the CPU - it is not
>> +   on the global memory bus. */
>> +/* XXX: what if the base changes? */
>> +sysbus_mmio_map(sysbus_from_qdev(&s->busdev.qdev), 0, 
>> MSI_ADDR_BASE);
>> +apic_mapped = 1;
>> +}
>> +
>> if (!vapic && s->vapic_control & VAPIC_ENABLE_MASK) {
>> vapic = sysbus_create_simple("kvmvapic", -1, NULL);
>> }
> 
> This looks wrong -- sysbus device init functions shouldn't
> be mapping MMIO regions themselves, in general. They should
> expose MMIO regions to be mapped by whichever device or board
> model creates them. Which is what the code before this patch
> was doing -- why do you want to move this code?

I fact, the CPU normally decides about where the APIC is mapped,
specifically when it is remapped via the MSR (which QEMU cannot support
yet). Well, unless we are talking about an external APIC and a 486 CPU.
Then the board decides. But I guess we could live with ignoring that
corner case and stick the mapping setup into the CPU initialization code.

Jan

-- 
Siemens AG, Corporate Technology, CT T DE IT 1
Corporate Competence Center Embedded Linux

Re: [Qemu-devel] [PATCH 1/1 v4] Allow machines to configure the QEMU_VERSION that's exposed via hardware

[Crístian Viana]

On 23-05-2012 13:11, Eric Blake wrote:

pstrcat is more efficient than snprintf() - the former is dedicated to a
single task, while the latter has to parse a format string and decode
that it is doing a single %s expansion.  In other words, just because
*printf can do string concatenation doesn't make it the best tool for
the job.


This would be the new code:

snprintf((void *) w, 12, "QEMU %s", qemu_get_version()); /* char 
version[12] */


I'm not sure of what value the pointer contains at that moment, 
concatenating doesn't seem safe to me. What if w already contains a 
string? The result won't be the same. I don't understand the Nokia code, 
so I prefer to leave it as it was before (with snprintf).


Best regards,
Crístian.

Re: [Qemu-devel] [PATCH] exec: fix breakpoint_invalidate() breakage

[Andreas Färber]

Am 23.05.2012 21:40, schrieb Blue Swirl:
> On Wed, May 23, 2012 at 3:41 AM, Andreas Färber  wrote:
>> Am 18.05.2012 11:49, schrieb TeLeMan:
>>> This breakage was introduced by the commit "memory: make
>>> phys_page_find() return an unadjusted".
>>
>> You seem to have found the origin of your problem. If you also mention
>> the commit hash in your commit message then certain frontends (gitk,
>> repo.or.cz) will display it as a handy hyperlink to that commit.
>>
>>>
>>> Signed-off-by: TeLeMan 
>>
>> Signed-off-by is a legal statement of origin and must not be a pseudonym.
> 
> $ git log --author=.*eleman
> commit c62f6d1d76aea587556c85b6b7b5c44167006264
> Author: TeLeMan 
> Date:   Mon Jul 25 16:29:14 2011 +0800
> 
> monitor: fix build breakage with --disable-vnc
> 
> The breakage was introduced by the commit
> 13661089810d3e59931f3e80d7cb541b99af7071
> 
> Signed-off-by: TeLeMan 
> Signed-off-by: Anthony Liguori 
> 
> and several others.

Funny, since I learned that from Anthony. :-)

Anyway, the only non-childish reason to contribute under a pseudonym (as
opposed to a known alias*) I can imagine is a contribution from someone
working on a competing commercial emulation/virtualization solution who
doesn't want her employer finding out, in which case it is questionable
whether she can actually sign off her own code as it may conflict with
non-free company IP. Any comments, Mr. Blauwirbel? ;)

Andreas

* e.g., malc, kraxel, mmu_man

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

Re: [Qemu-devel] [PATCH] exec: fix breakpoint_invalidate() breakage

[Blue Swirl]

On Wed, May 23, 2012 at 3:41 AM, Andreas Färber  wrote:
> Am 18.05.2012 11:49, schrieb TeLeMan:
>> This breakage was introduced by the commit "memory: make
>> phys_page_find() return an unadjusted".
>
> You seem to have found the origin of your problem. If you also mention
> the commit hash in your commit message then certain frontends (gitk,
> repo.or.cz) will display it as a handy hyperlink to that commit.
>
>>
>> Signed-off-by: TeLeMan 
>
> Signed-off-by is a legal statement of origin and must not be a pseudonym.

$ git log --author=.*eleman
commit c62f6d1d76aea587556c85b6b7b5c44167006264
Author: TeLeMan 
Date:   Mon Jul 25 16:29:14 2011 +0800

monitor: fix build breakage with --disable-vnc

The breakage was introduced by the commit
13661089810d3e59931f3e80d7cb541b99af7071

Signed-off-by: TeLeMan 
Signed-off-by: Anthony Liguori 

and several others.

>
> /-F
>
>> ---
>>  exec.c |    3 ++-
>>  1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/exec.c b/exec.c
>> index 0607c9b..ad99476 100644
>> --- a/exec.c
>> +++ b/exec.c
>> @@ -1475,7 +1475,8 @@ void tb_invalidate_phys_addr(target_phys_addr_t addr)
>>
>>  static void breakpoint_invalidate(CPUArchState *env, target_ulong pc)
>>  {
>> -    tb_invalidate_phys_addr(cpu_get_phys_page_debug(env, pc));
>> +    tb_invalidate_phys_addr(cpu_get_phys_page_debug(env, pc)
>> +                            | (pc & ~TARGET_PAGE_MASK));
>>  }
>>  #endif
>>  #endif /* TARGET_HAS_ICE */
>
> --
> SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
> GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
>

Re: [Qemu-devel] [PATCH 10/15] Openrisc: add a simulation board

[Blue Swirl]

On Wed, May 23, 2012 at 7:54 AM, Jia Liu  wrote:
> Hi Blue,
>
> On Sat, May 19, 2012 at 3:51 PM, Blue Swirl  wrote:
>> On Thu, May 17, 2012 at 8:35 AM, Jia Liu  wrote:
>>> add a simulation board for openrisc.
>>>
>>> Signed-off-by: Jia Liu 
>>> ---
>>>  Makefile.target   |    1 +
>>>  hw/openrisc_sim.c |  140 
>>> +
>>>  2 files changed, 141 insertions(+)
>>>  create mode 100644 hw/openrisc_sim.c
>>>
>>> diff --git a/Makefile.target b/Makefile.target
>>> index 6b1d2d0..433dd3c 100644
>>> --- a/Makefile.target
>>> +++ b/Makefile.target
>>> @@ -391,6 +391,7 @@ obj-xtensa-y += core-dc233c.o
>>>  obj-xtensa-y += core-fsf.o
>>>
>>>  obj-openrisc-y += openrisc_pic.o
>>> +obj-openrisc-y += openrisc_sim.o
>>>  obj-openrisc-y += openrisc_timer.o
>>>
>>>  main.o: QEMU_CFLAGS+=$(GPROF_CFLAGS)
>>> diff --git a/hw/openrisc_sim.c b/hw/openrisc_sim.c
>>> new file mode 100644
>>> index 000..869a02e
>>> --- /dev/null
>>> +++ b/hw/openrisc_sim.c
>>> @@ -0,0 +1,140 @@
>>> +/*
>>> + * Openrisc simulator for use as an ISS.
>>> + *
>>> + *  Copyright (c) 2011-2012 Jia Liu 
>>> + *                          Feng Gao 
>>> + *
>>> + * This library is free software; you can redistribute it and/or
>>> + * modify it under the terms of the GNU Library General Public
>>> + * License as published by the Free Software Foundation; either
>>> + * version 2 of the License, or (at your option) any later version.
>>> + *
>>> + * This library is distributed in the hope that it will be useful,
>>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
>>> + * Library General Public License for more details.
>>> + *
>>> + * You should have received a copy of the GNU Library General Public
>>> + * License along with this library; if not, write to the Free Software
>>> + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston MA  
>>> 02110-1301 USA
>>> + */
>>> +
>>> +#include "hw.h"
>>> +#include "fdc.h"
>>> +#include "net.h"
>>> +#include "openrisc_cpudev.h"
>>> +#include "boards.h"
>>> +#include "pci.h"
>>> +#include "elf.h"
>>> +#include "smbus.h"
>>> +#include "memory.h"
>>> +#include "pc.h"
>>> +#include "pci.h"
>>> +#include "sysbus.h"
>>> +#include "flash.h"
>>> +#include "loader.h"
>>> +#include "exec-memory.h"
>>> +#include "sysemu.h"
>>> +#include "isa.h"
>>> +#include "mc146818rtc.h"
>>
>> There's no rtc, so this shouldn't be needed. Please trim other includes as 
>> well.
>>
>
> deleted, thanks.
>
>>> +#include "blockdev.h"
>>> +#include "qemu-log.h"
>>> +
>>> +#define KERNEL_LOAD_ADDR 0x100
>>> +
>>> +static uint64_t translate_phys_addr(void *env, uint64_t addr)
>>> +{
>>> +    return cpu_get_phys_page_debug(env, addr);
>>
>> There's no virtual to physical mapping during CPU init, I suppose. Then just
>> return addr;
>>
>
> this func is useless, deleted, thanks.
>
>>> +}
>>> +
>>> +static void main_cpu_reset(void *opaque)
>>> +{
>>> +    CPUOPENRISCState *env = opaque;
>>> +    openrisc_reset(env);
>>> +}
>>> +
>>> +static void openrisc_sim_init(ram_addr_t ram_size,
>>> +                              const char *boot_device,
>>> +                              const char *kernel_filename,
>>> +                              const char *kernel_cmdline,
>>> +                              const char *initrd_filename,
>>> +                              const char *cpu_model)
>>> +{
>>> +    CPUOPENRISCState *env;
>>> +    MemoryRegion *ram = g_new(MemoryRegion, 1);
>>> +    int kernel_size;
>>
>> target_ulong, target_phys_addr_t or uint64_t?
>>
>
> we need it int here, for load error, it return -1.

Well, you could use for example (uint64_t)-1 in the comparison. My
point with this change is to prepare for a 64 bit architecture while
int could be 32 bits. Kernels larger than 4G are still a bit rare...

>
>>> +    uint64_t elf_entry;
>>> +    target_phys_addr_t entry;
>>
>> This is not really used.
>
> it is used by somewhere.

It's written to in a few places, but there are no readers.

>
>>
>>> +    qemu_irq *i8259;
>>> +    ISABus *isa_bus;
>>> +
>>> +    if (!cpu_model) {
>>> +        cpu_model = "or1200";
>>> +    }
>>> +    env = cpu_init(cpu_model);
>>> +    if (!env) {
>>> +        fprintf(stderr, "Unable to find CPU definition!\n");
>>> +        exit(1);
>>> +    }
>>> +
>>> +    qemu_register_reset(main_cpu_reset, env);
>>> +    main_cpu_reset(env);
>>> +
>>> +    memory_region_init_ram(ram, "openrisc.ram", ram_size);
>>> +    memory_region_add_subregion(get_system_memory(), 0, ram);
>>> +
>>> +    if (kernel_filename) {
>>
>> Please add && !qtest_enabled().
>>
>
> Thanks, added.
>
>>> +        kernel_size = load_elf(kernel_filename, translate_phys_addr, env,
>>> +                               &elf_entry, NULL, NULL, 1, ELF_MACHINE, 1);
>>> +        entry = elf_entry;
>>> +        if (kernel_size < 0) {
>>> +            kernel_size = load_uimage(kernel_filename, &entry, NULL, NULL);
>>> +

[Qemu-devel] [PATCH 1.1 0/2]: qemu-ga: fix build on openbsd

[Luiz Capitulino]

Build error found by buildbot:

 qga/commands-posix.c: In function 'qmp_guest_shutdown':
 qga/commands-posix.c:65: error: 'environ' undeclared (first use in this 
function)
 qga/commands-posix.c:65: error: (Each undeclared identifier is reported only 
once
 qga/commands-posix.c:65: error: for each function it appears in.)

On F16 environ is declared in , but that obviously doesn't
happen on all systems.

Tested on OpenBSD 4.9.

PS: I get zillions of -Wno-redundant-decls errors on OpenBSD 4.8 and still
get other warnnigs if I drop that flag.

 configure|   19 +++
 qga/commands-posix.c |6 +-
 2 files changed, 24 insertions(+), 1 deletion(-)

Re: [Qemu-devel] [PATCH 05/15] Openrisc: add exception support

[Blue Swirl]

On Wed, May 23, 2012 at 7:09 AM, Jia Liu  wrote:
> Hi Blue,
>
> On Sat, May 19, 2012 at 3:22 PM, Blue Swirl  wrote:
>> On Thu, May 17, 2012 at 8:35 AM, Jia Liu  wrote:
>>> add the openrisc exception support.
>>>
>>> Signed-off-by: Jia Liu 
>>> ---
>>>  Makefile.target               |    2 +-
>>>  target-openrisc/excp.c        |   27 +++
>>>  target-openrisc/excp.h        |   28 
>>>  target-openrisc/excp_helper.c |   28 
>>>  target-openrisc/helper.h      |    3 +++
>>>  target-openrisc/translate.c   |   25 -
>>>  6 files changed, 103 insertions(+), 10 deletions(-)
>>>  create mode 100644 target-openrisc/excp.c
>>>  create mode 100644 target-openrisc/excp.h
>>>  create mode 100644 target-openrisc/excp_helper.c
>>>
>>> diff --git a/Makefile.target b/Makefile.target
>>> index 975c9a8..ed5f0b0 100644
>>> --- a/Makefile.target
>>> +++ b/Makefile.target
>>> @@ -101,7 +101,7 @@ endif
>>>  libobj-$(TARGET_SPARC) += int32_helper.o
>>>  libobj-$(TARGET_SPARC64) += int64_helper.o
>>>  libobj-$(TARGET_ALPHA) += int_helper.o fpu_helper.o sys_helper.o 
>>> mem_helper.o
>>> -libobj-$(TARGET_OPENRISC) += intrp_helper.o mem.o mem_helper.o
>>> +libobj-$(TARGET_OPENRISC) += excp.o excp_helper.o intrp_helper.o mem.o 
>>> mem_helper.o
>>>
>>>  libobj-y += disas.o
>>>  libobj-$(CONFIG_TCI_DIS) += tci-dis.o
>>> diff --git a/target-openrisc/excp.c b/target-openrisc/excp.c
>>> new file mode 100644
>>> index 000..fc9391a
>>> --- /dev/null
>>> +++ b/target-openrisc/excp.c
>>> @@ -0,0 +1,27 @@
>>> +/*
>>> + *  Openrisc exception.
>>> + *
>>> + *  Copyright (c) 2011-2012 Jia Liu 
>>> + *
>>> + * This library is free software; you can redistribute it and/or
>>> + * modify it under the terms of the GNU Lesser General Public
>>> + * License as published by the Free Software Foundation; either
>>> + * version 2 of the License, or (at your option) any later version.
>>> + *
>>> + * This library is distributed in the hope that it will be useful,
>>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
>>> + * Lesser General Public License for more details.
>>> + *
>>> + * You should have received a copy of the GNU Lesser General Public
>>> + * License along with this library; if not, see 
>>> .
>>> + */
>>> +
>>> +#include "cpu.h"
>>> +#include "excp.h"
>>> +
>>> +void raise_exception(CPUOPENRISCState *env, uint32_t excp)
>>
>> QEMU_NORETURN?
>>
>
> Does it have to return?

No, QEMU_NORETURN just tells the compiler that the function does not
return, because it executes a longjmp() etc. That way the compiler can
optimize the code.

Here we could propagate QEMU_NORETURN from cpu_loop_exit() which has this flag:
void QEMU_NORETURN cpu_loop_exit(CPUArchState *env1);

>
>>> +{
>>> +    env->exception_index = excp;
>>> +    cpu_loop_exit(env);
>>> +}
>>
>> I'd just merge this file with excp_helper.c for simplicity, though
>> it's not incorrect either.
>>
>>> diff --git a/target-openrisc/excp.h b/target-openrisc/excp.h
>>> new file mode 100644
>>> index 000..4c32e46
>>> --- /dev/null
>>> +++ b/target-openrisc/excp.h
>>> @@ -0,0 +1,28 @@
>>> +/*
>>> + *  Openrisc exception header.
>>> + *
>>> + *  Copyright (c) 2011-2012 Jia Liu 
>>> + *
>>> + * This library is free software; you can redistribute it and/or
>>> + * modify it under the terms of the GNU Lesser General Public
>>> + * License as published by the Free Software Foundation; either
>>> + * version 2 of the License, or (at your option) any later version.
>>> + *
>>> + * This library is distributed in the hope that it will be useful,
>>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
>>> + * Lesser General Public License for more details.
>>> + *
>>> + * You should have received a copy of the GNU Lesser General Public
>>> + * License along with this library; if not, see 
>>> .
>>> + */
>>> +
>>> +#ifndef QEMU_OPENRISC_EXCP_H
>>> +#define QEMU_OPENRISC_EXCP_H
>>> +
>>> +#include "cpu.h"
>>> +#include "qemu-common.h"
>>> +
>>> +void raise_exception(CPUOPENRISCState *env, uint32_t excp);
>>> +
>>> +#endif /* QEMU_OPENRISC_EXCP_H */
>>> diff --git a/target-openrisc/excp_helper.c b/target-openrisc/excp_helper.c
>>> new file mode 100644
>>> index 000..0cad14b
>>> --- /dev/null
>>> +++ b/target-openrisc/excp_helper.c
>>> @@ -0,0 +1,28 @@
>>> +/*
>>> + * OpenRISC exception helper routines
>>> + *
>>> + *  Copyright (c) 2011-2012 Jia Liu 
>>> + *
>>> + * This library is free software; you can redistribute it and/or
>>> + * modify it under the terms of the GNU Library General Public
>>> + * License as published by the Free Software Foundation; either
>>> + * version 2 of the License, or (at your option) any later version.
>>> + *
>>> + * This library

Re: [Qemu-devel] How to create new target port?

[Michael Eager]

On 05/23/2012 08:59 AM, Stefan Weil wrote:

Am 23.05.2012 16:37, schrieb Michael Eager:

On 05/22/2012 11:18 PM, 陳韋任 wrote:

I'm investigating adding a new target architecture
to QEMU. Are there documents, how-to's, or other
guidance on how to approach this? Or any advice?

I noticed that there are a number of directories for
architectures like target-arm and target-mips. There
are also definitions under tcg for arm and mips. I
noticed that target-microblaze exists, but there is
no microblaze directory under tcg. What does this
mean?


Depends on what you'd like to add, a guest or a host support. If you want to
add a new guest, take target-xxx/* as an example. Otherwise, looks at tcg/xxx/*.
The term "target" could be a little MISLEADING here. :)


I'm interested in adding a new emulated architecture,
not a new host. So adding a new target- sounds
like the plan.


Yes, that's the place for new target architectures.

Which architecture are you thinking of? Maybe someone else
is already working on it. http://wiki.qemu.org/Links has an
incomplete list of links to unofficial versions of QEMU which
support additional targets.


Thanks for the pointer.  It's a proprietary architecture,
not one of those mentioned.  I'll add it to the list as we
move forward with the project.

--
Michael Eagerea...@eagercon.com
1960 Park Blvd., Palo Alto, CA 94306  650-325-8077

[Qemu-devel] [PATCH 2/2] qemu-ga: Fix missing environ declaration

[Luiz Capitulino]

Commit 3674838cd05268954bb6473239cd7f700a79bf0f uses the environ global
variable, but is relying on environ to be declared somewhere else.

This worked for me because on F16 environ is declared in , but
that doesn't happen in OpenBSD for example, causing a build failure.

This commit fixes the build error by declaring environ if it hasn't
being declared yet.

Also fixes a build warning due to a missing  include.

Signed-off-by: Luiz Capitulino 
---
 qga/commands-posix.c |6 +-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/qga/commands-posix.c b/qga/commands-posix.c
index 7664be1..dab3bf9 100644
--- a/qga/commands-posix.c
+++ b/qga/commands-posix.c
@@ -14,12 +14,17 @@
 #include 
 #include 
 #include 
+#include 
 #include "qga/guest-agent-core.h"
 #include "qga-qmp-commands.h"
 #include "qerror.h"
 #include "qemu-queue.h"
 #include "host-utils.h"
 
+#ifndef CONFIG_HAS_ENVIRON
+extern char **environ;
+#endif
+
 #if defined(__linux__)
 #include 
 #include 
@@ -27,7 +32,6 @@
 #include 
 #include 
 #include 
-#include 
 
 #if defined(__linux__) && defined(FIFREEZE)
 #define CONFIG_FSFREEZE
-- 
1.7.9.2.384.g4a92a

Re: [Qemu-devel] [PATCHv2 1/3] ui/spice-display.c: add missing initialization for valgrind

[Gerd Hoffmann]

On 05/22/12 17:29, Alon Levy wrote:
> We can't initialize QXLDevSurfaceCreate field by field because it has a
> pa hole, and so 4 bytes remain uninitialized when building on x86-64, so
> just memset.

So you get valgrind warnings for the hole?  why?  nobody should ever
access the hole, so the missing initialization should not hurt in theory ...

> Signed-off-by: Alon Levy 
> ---
>  ui/spice-display.c |2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/ui/spice-display.c b/ui/spice-display.c
> index 5418eb3..3e8f0b3 100644
> --- a/ui/spice-display.c
> +++ b/ui/spice-display.c
> @@ -244,6 +244,8 @@ void qemu_spice_create_host_primary(SimpleSpiceDisplay 
> *ssd)
>  {
>  QXLDevSurfaceCreate surface;
>  
> +memset(&surface, 0, sizeof(surface));
> +
>  dprint(1, "%s: %dx%d\n", __FUNCTION__,
> ds_get_width(ssd->ds), ds_get_height(ssd->ds));
>  

Re: [Qemu-devel] [PATCH next v2 00/74] QOM CPUState, part 3: CPU reset

[Blue Swirl]

On Tue, May 22, 2012 at 12:34 AM, Andreas Färber  wrote:
> Am 21.05.2012 20:20, schrieb Blue Swirl:
>> On Mon, May 21, 2012 at 9:09 AM, Andreas Färber  wrote:
>>> Am 14.05.2012 23:22, schrieb Blue Swirl:
 On Mon, May 14, 2012 at 8:59 PM, Andreas Färber  wrote:
> Am 14.05.2012 21:54, schrieb Blue Swirl:
>> On Mon, May 14, 2012 at 4:15 PM, Andreas Färber  wrote:
>>> Am 10.05.2012 02:13, schrieb Andreas Färber:
 Andreas Färber (74):
>>> [...]
   target-sparc: Let cpu_sparc_init() return SPARCCPU
   sun4m: Use cpu_sparc_init() to obtain SPARCCPU
   sun4m: Pass SPARCCPU to {main,secondary}_cpu_reset()
   sun4u: Use cpu_sparc_init() to obtain SPARCCPU
   sun4u: Let cpu_devinit() return SPARCCPU
   sun4u: Store SPARCCPU in ResetData
   leon3: Use cpu_sparc_init() to obtain SPARCCPU
   leon3: Store SPARCCPU in ResetData
>>> [...]
>
> Forgot to mention the bsd-user patch.
>
   Kill off cpu_state_reset()
>>>
>>> Ping! Blue, can you ack please?
>>
>> Otherwise the patches look pretty safe ("if it compiles, it works").
>>>
>>> Was I supposed to interpret that as an Acked-by? ;)
>>
>> No, but you may do so if it compiles. ;-)
>
> Joking, are we? My cover letter indicated pretty clearly on what
> platforms I had already compile-tested on, and that meant each patch.
> I had also run `make check` on each and the sparc test image for sun4m,
> HelenOS for sun4u and "crible" for leon3.

OK,
Acked-by: Blue Swirl 

>
> Applied the remainder of the series to qom-next:
> http://repo.or.cz/w/qemu/afaerber.git/shortlog/refs/heads/qom-next
>
> Tristan/Fabien, could you please submit a patch to document the Leon3
> machine in MAINTAINERS? No one got cc'ed on those two patches. Thanks.
>
> Andreas
>
> --
> SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
> GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

[Qemu-devel] [PATCH 1/2] configure: check if environ is declared

[Luiz Capitulino]

Some systems may declare environ automatically, others don't. Check for it.

Signed-off-by: Luiz Capitulino 
---
 configure |   19 +++
 1 file changed, 19 insertions(+)

diff --git a/configure b/configure
index b55a792..1f338f8 100755
--- a/configure
+++ b/configure
@@ -2831,6 +2831,21 @@ if compile_prog "" "" ; then
 linux_magic_h=yes
 fi
 
+
+# check if environ is declared
+
+has_environ=no
+cat > $TMPC << EOF
+#include 
+int main(void) {
+environ = environ;
+return 0;
+}
+EOF
+if compile_prog "" "" ; then
+has_environ=yes
+fi
+
 ##
 # End of CC checks
 # After here, no more $cc or $ld runs
@@ -3342,6 +3357,10 @@ if test "$linux_magic_h" = "yes" ; then
   echo "CONFIG_LINUX_MAGIC_H=y" >> $config_host_mak
 fi
 
+if test "$has_environ" = "yes" ; then
+  echo "CONFIG_HAS_ENVIRON=y" >> $config_host_mak
+fi
+
 # USB host support
 case "$usb" in
 linux)
-- 
1.7.9.2.384.g4a92a

Re: [Qemu-devel] buildbot failure in qemu on default_openbsd_current

[Luiz Capitulino]

On Wed, 23 May 2012 10:01:02 -0600
Eric Blake  wrote:

> >  #include 
> > +#include 
> >  #include 
> >  #include 
> >  #include "qga/guest-agent-core.h"
> > @@ -20,6 +21,10 @@
> >  #include "qemu-queue.h"
> >  #include "host-utils.h"
> >  
> > +#ifndef _GNU_SOURCE
> > +extern char **environ;
> > +#endif
> 
> Looks reasonable.

But that's not the cause of problem. What's happening is that on F16 environ
is declared automatically in . I'll post a different fix right now.

Re: [Qemu-devel] [PATCH 06/21] qdev: push "type" property up to Object

[Paolo Bonzini]

Il 23/05/2012 19:40, Andreas Färber ha scritto:
> Paolo, please let me know if you're okay with my commit messages, then I
> can override the author to be you as well (same for preceding one).

Don't worry, you have carte blanche. :)

Paolo

Re: [Qemu-devel] [PATCH 06/21] qdev: push "type" property up to Object

[Andreas Färber]

Am 23.05.2012 19:18, schrieb Paolo Bonzini:
> Il 23/05/2012 19:06, Andreas Färber ha scritto:
>> Am 02.05.2012 13:30, schrieb Paolo Bonzini:
>>> Now that Object is a type, add an instance_init function and push
>>> the "type" property from qdev to there.
>>>
>>> Signed-off-by: Paolo Bonzini 
>>
>> The rebased version from qdev-props-4 breaks and hangs `make check`:
>>
>> GTESTER check-qtest-sparc
>> qemu-system-sparc: Insufficient permission to perform this operation
>>
>> Paolo, can you please investigate?
> 
> Please try this patch (untested; please add my signed-off-by if you end
> up committing it).

Tested make check and PReP. I added some braces missing in the original
code to please checkpatch.pl and applied both to qom-next:
http://repo.or.cz/w/qemu/afaerber.git/shortlog/refs/heads/qom-next

Paolo, please let me know if you're okay with my commit messages, then I
can override the author to be you as well (same for preceding one).

BTW note that I usually update subjects to start with a capital letter
to visually separate the lowercase topic from the actual summary. ;)

> There is also a "type" property in arm_l2x0.c, but it seems unused.

Peter, can we drop that property? I'd insert such a patch before then.

Thanks,
Andreas

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

Re: [Qemu-devel] [PATCH] exec: fix breakpoint_invalidate() breakage

[Jan Kiszka]

On 2012-05-23 13:02, Jan Kiszka wrote:
> On 2012-05-23 11:11, TeLeMan wrote:
>> On Wed, May 23, 2012 at 7:22 PM, Jan Kiszka  wrote:
>>> On 2012-05-23 04:09, TeLeMan wrote:
 On Wed, May 23, 2012 at 11:41 AM, Andreas Färber  wrote:
> Am 18.05.2012 11:49, schrieb TeLeMan:
>> This breakage was introduced by the commit "memory: make
>> phys_page_find() return an unadjusted".
>
> You seem to have found the origin of your problem. If you also mention
> the commit hash in your commit message then certain frontends (gitk,
> repo.or.cz) will display it as a handy hyperlink to that commit.
>
>>
>> Signed-off-by: TeLeMan 
>
> Signed-off-by is a legal statement of origin and must not be a pseudonym.
 Ok, please ignore this patch. I won't submit any patch just report bugs.
>>>
>>> Then please describe this bug in more details, e.g. how to reproduce.
>> I think its evident. cpu_get_phys_page_debug(env, pc) is not the
>> physical address of pc but the physical page base address of pc.
> 
> ...so this bites us if the instruction spans two pages as
> tb_invalidate_phys_addr requests invalidation on a page granularity.

In fact, this is irrelevant. We only need to flush the address at which
the instruction starts, and that is achieved by flushing all TB that
relate to that page as the current code does.

So, again my question: How can I reproduce the issue you see?

Jan

-- 
Siemens AG, Corporate Technology, CT T DE IT 1
Corporate Competence Center Embedded Linux

Re: [Qemu-devel] [PATCH 06/21] qdev: push "type" property up to Object

[Paolo Bonzini]

Il 23/05/2012 19:06, Andreas Färber ha scritto:
> Am 02.05.2012 13:30, schrieb Paolo Bonzini:
>> Now that Object is a type, add an instance_init function and push
>> the "type" property from qdev to there.
>>
>> Signed-off-by: Paolo Bonzini 
> 
> The rebased version from qdev-props-4 breaks and hangs `make check`:
> 
> GTESTER check-qtest-sparc
> qemu-system-sparc: Insufficient permission to perform this operation
> 
> Paolo, can you please investigate?

Please try this patch (untested; please add my signed-off-by if you end
up committing it).

There is also a "type" property in arm_l2x0.c, but it seems unused.

Paolo

diff --git a/hw/m48t59.c b/hw/m48t59.c
index 24dcbfa..3cc2178 100644
--- a/hw/m48t59.c
+++ b/hw/m48t59.c
@@ -65,7 +65,7 @@ struct M48t59State {
 /* NVRAM storage */
 uint8_t *buffer;
 /* Model parameters */
-uint32_t type; /* 2 = m48t02, 8 = m48t08, 59 = m48t59 */
+uint32_t model; /* 2 = m48t02, 8 = m48t08, 59 = m48t59 */
 /* NVRAM storage */
 uint16_t addr;
 uint8_t  lock;
@@ -197,9 +197,9 @@ void m48t59_write (void *opaque, uint32_t addr, uint32_t val)
 	NVRAM_PRINTF("%s: 0x%08x => 0x%08x\n", __func__, addr, val);
 
 /* check for NVRAM access */
-if ((NVRAM->type == 2 && addr < 0x7f8) ||
-(NVRAM->type == 8 && addr < 0x1ff8) ||
-(NVRAM->type == 59 && addr < 0x1ff0))
+if ((NVRAM->model == 2 && addr < 0x7f8) ||
+(NVRAM->model == 8 && addr < 0x1ff8) ||
+(NVRAM->model == 59 && addr < 0x1ff0))
 goto do_write;
 
 /* TOD access */
@@ -334,7 +334,7 @@ void m48t59_write (void *opaque, uint32_t addr, uint32_t val)
 	tmp = from_bcd(val);
 	if (tmp >= 0 && tmp <= 99) {
 	get_time(NVRAM, &tm);
-if (NVRAM->type == 8)
+if (NVRAM->model == 8)
 tm.tm_year = from_bcd(val) + 68; // Base year is 1968
 else
 tm.tm_year = from_bcd(val);
@@ -362,9 +362,9 @@ uint32_t m48t59_read (void *opaque, uint32_t addr)
 uint32_t retval = 0xFF;
 
 /* check for NVRAM access */
-if ((NVRAM->type == 2 && addr < 0x078f) ||
-(NVRAM->type == 8 && addr < 0x1ff8) ||
-(NVRAM->type == 59 && addr < 0x1ff0))
+if ((NVRAM->model == 2 && addr < 0x078f) ||
+(NVRAM->model == 8 && addr < 0x1ff8) ||
+(NVRAM->model == 59 && addr < 0x1ff0))
 goto do_read;
 
 /* TOD access */
@@ -439,7 +439,7 @@ uint32_t m48t59_read (void *opaque, uint32_t addr)
 case 0x07FF:
 /* year */
 get_time(NVRAM, &tm);
-if (NVRAM->type == 8)
+if (NVRAM->model == 8)
 retval = to_bcd(tm.tm_year - 68); // Base year is 1968
 else
 retval = to_bcd(tm.tm_year);
@@ -633,7 +633,7 @@ static const MemoryRegionOps m48t59_io_ops = {
 
 /* Initialisation routine */
 M48t59State *m48t59_init(qemu_irq IRQ, target_phys_addr_t mem_base,
- uint32_t io_base, uint16_t size, int type)
+ uint32_t io_base, uint16_t size, int model)
 {
 DeviceState *dev;
 SysBusDevice *s;
@@ -641,7 +641,7 @@ M48t59State *m48t59_init(qemu_irq IRQ, target_phys_addr_t mem_base,
 M48t59State *state;
 
 dev = qdev_create(NULL, "m48t59");
-qdev_prop_set_uint32(dev, "type", type);
+qdev_prop_set_uint32(dev, "model", model);
 qdev_prop_set_uint32(dev, "size", size);
 qdev_prop_set_uint32(dev, "io_base", io_base);
 qdev_init_nofail(dev);
@@ -661,14 +661,14 @@ M48t59State *m48t59_init(qemu_irq IRQ, target_phys_addr_t mem_base,
 }
 
 M48t59State *m48t59_init_isa(ISABus *bus, uint32_t io_base, uint16_t size,
- int type)
+ int model)
 {
 M48t59ISAState *d;
 ISADevice *dev;
 M48t59State *s;
 
 dev = isa_create(bus, "m48t59_isa");
-qdev_prop_set_uint32(&dev->qdev, "type", type);
+qdev_prop_set_uint32(&dev->qdev, "model", model);
 qdev_prop_set_uint32(&dev->qdev, "size", size);
 qdev_prop_set_uint32(&dev->qdev, "io_base", io_base);
 qdev_init_nofail(&dev->qdev);
@@ -686,7 +686,7 @@ M48t59State *m48t59_init_isa(ISABus *bus, uint32_t io_base, uint16_t size,
 static void m48t59_init_common(M48t59State *s)
 {
 s->buffer = g_malloc0(s->size);
-if (s->type == 59) {
+if (s->model == 59) {
 s->alrm_timer = qemu_new_timer_ns(rtc_clock, &alarm_cb, s);
 s->wd_timer = qemu_new_timer_ns(vm_clock, &watchdog_cb, s);
 }
@@ -722,7 +722,7 @@ static int m48t59_init1(SysBusDevice *dev)
 
 static Property m48t59_isa_properties[] = {
 DEFINE_PROP_UINT32("size",M48t59ISAState, state.size,-1),
-DEFINE_PROP_UINT32("type",M48t59ISAState, state.type,-1),
+DEFINE_PROP_UINT32("model",   M48t59ISAState, state.model,-1),
 DEFINE_PROP_HEX32( "io_base", M48t59ISAState, state.io_base,  0),
 DEFINE_PROP_END_OF_LIST(),
 };
@@ -746,7 +746,7 @@ static TypeInfo m48t59_isa_info = {
 
 static Property m48t59_properties[] = {
 DEFINE_PROP_U

[Qemu-devel] [PATCH qom-next 3/6] target-i386: add cpu-model property to x86_cpu

[Igor Mammedov]

it's probably intermidiate step till cpu modeled as
sub-classes. After then we probably could drop it.

However it still could be used for overiding default
cpu subclasses definition, and probably renamed to
something like 'features'.

v2:
 - remove accidential tcg_* init code move

Signed-off-by: Igor Mammedov 
---
 cpu-defs.h   |2 +-
 hw/pc.c  |   10 --
 target-i386/cpu.c|   24 
 target-i386/helper.c |   16 
 4 files changed, 37 insertions(+), 15 deletions(-)

diff --git a/cpu-defs.h b/cpu-defs.h
index f49e950..8f4623c 100644
--- a/cpu-defs.h
+++ b/cpu-defs.h
@@ -221,7 +221,7 @@ typedef struct CPUWatchpoint {
 struct QemuCond *halt_cond; \
 int thread_kicked;  \
 struct qemu_work_item *queued_work_first, *queued_work_last;\
-const char *cpu_model_str;  \
+char *cpu_model_str;\
 struct KVMState *kvm_state; \
 struct kvm_run *kvm_run;\
 int kvm_fd; \
diff --git a/hw/pc.c b/hw/pc.c
index c609770..1aa90a2 100644
--- a/hw/pc.c
+++ b/hw/pc.c
@@ -930,7 +930,6 @@ static X86CPU *pc_new_cpu(const char *cpu_model)
 
 cpu = cpu_x86_init(cpu_model);
 if (cpu == NULL) {
-fprintf(stderr, "Unable to find x86 CPU definition\n");
 exit(1);
 }
 env = &cpu->env;
@@ -946,15 +945,6 @@ void pc_cpus_init(const char *cpu_model)
 {
 int i;
 
-/* init CPUs */
-if (cpu_model == NULL) {
-#ifdef TARGET_X86_64
-cpu_model = "qemu64";
-#else
-cpu_model = "qemu32";
-#endif
-}
-
 for(i = 0; i < smp_cpus; i++) {
 pc_new_cpu(cpu_model);
 }
diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index 14c0f64..e655129 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -1731,6 +1731,27 @@ static void mce_init(X86CPU *cpu)
 }
 }
 
+static char *x86_get_cpu_model(Object *obj, Error **errp)
+{
+X86CPU *cpu = X86_CPU(obj);
+CPUX86State *env = &cpu->env;
+return g_strdup(env->cpu_model_str);
+}
+
+static void x86_set_cpu_model(Object *obj, const char *value, Error **errp)
+{
+X86CPU *cpu = X86_CPU(obj);
+CPUX86State *env = &cpu->env;
+
+g_free((gpointer)env->cpu_model_str);
+env->cpu_model_str = g_strdup(value);
+
+if (cpu_x86_register(cpu, env->cpu_model_str) < 0) {
+fprintf(stderr, "Unable to find x86 CPU definition\n");
+error_set(errp, QERR_INVALID_PARAMETER_COMBINATION);
+}
+}
+
 void x86_cpu_realize(Object *obj, Error **errp)
 {
 X86CPU *cpu = X86_CPU(obj);
@@ -1771,6 +1792,9 @@ static void x86_cpu_initfn(Object *obj)
 x86_cpuid_get_tsc_freq,
 x86_cpuid_set_tsc_freq, NULL, NULL, NULL);
 
+object_property_add_str(obj, "cpu-model",
+x86_get_cpu_model, x86_set_cpu_model, NULL);
+
 env->cpuid_apic_id = env->cpu_index;
 }
 
diff --git a/target-i386/helper.c b/target-i386/helper.c
index 3ceefad..fbaeeea 100644
--- a/target-i386/helper.c
+++ b/target-i386/helper.c
@@ -1154,12 +1154,10 @@ int cpu_x86_get_descr_debug(CPUX86State *env, unsigned 
int selector,
 X86CPU *cpu_x86_init(const char *cpu_model)
 {
 X86CPU *cpu;
-CPUX86State *env;
+Error *errp = NULL;
 static int inited;
 
 cpu = X86_CPU(object_new(TYPE_X86_CPU));
-env = &cpu->env;
-env->cpu_model_str = cpu_model;
 
 /* init various static tables used in TCG mode */
 if (tcg_enabled() && !inited) {
@@ -1170,7 +1168,17 @@ X86CPU *cpu_x86_init(const char *cpu_model)
 cpu_set_debug_excp_handler(breakpoint_handler);
 #endif
 }
-if (cpu_x86_register(cpu, cpu_model) < 0) {
+
+if (cpu_model) {
+object_property_set_str(OBJECT(cpu), cpu_model, "cpu-model", &errp);
+} else {
+#ifdef TARGET_X86_64
+object_property_set_str(OBJECT(cpu), "qemu64", "cpu-model", &errp);
+#else
+object_property_set_str(OBJECT(cpu), "qemu32", "cpu-model", &errp);
+#endif
+}
+if (errp) {
 object_delete(OBJECT(cpu));
 return NULL;
 }
-- 
1.7.7.6

[Qemu-devel] [PATCH qom-next 5/6] target-i386: make initialize CPU in QOM way

[Igor Mammedov]

Make CPU creation/initialization consistent with QOM object
behavior in this, by moving tcg and apic initialization from board
level into CPU's initfn/realize calls and cpu_model property setter.

Which makes CPU object self-sufficient in respect of creation/initialization
and matches a typical object creation sequence, i.e.:
  - create CPU instance
  - set properties
  - realize object - (x86_cpu_realize will be converted into realize
  property setter, when it is implemented)

v2:
  - fix moving of tcg_* initialization into cpu.c from helper.c
  spotted-by: 
  - make it compile/work on i386-linux-user target

Signed-off-by: Igor Mammedov 
---
 hw/pc.c  |   45 
 target-i386/cpu.c|   81 -
 target-i386/helper.c |   39 
 3 files changed, 85 insertions(+), 80 deletions(-)

diff --git a/hw/pc.c b/hw/pc.c
index 1ccfc6e..d7845ea 100644
--- a/hw/pc.c
+++ b/hw/pc.c
@@ -42,7 +42,6 @@
 #include "sysbus.h"
 #include "sysemu.h"
 #include "kvm.h"
-#include "xen.h"
 #include "blockdev.h"
 #include "ui/qemu-spice.h"
 #include "memory.h"
@@ -877,25 +876,6 @@ DeviceState *cpu_get_current_apic(void)
 }
 }
 
-static DeviceState *apic_init(void *env, uint8_t apic_id)
-{
-DeviceState *dev;
-
-if (kvm_irqchip_in_kernel()) {
-dev = qdev_create(NULL, "kvm-apic");
-} else if (xen_enabled()) {
-dev = qdev_create(NULL, "xen-apic");
-} else {
-dev = qdev_create(NULL, "apic");
-}
-
-qdev_prop_set_uint8(dev, "id", apic_id);
-qdev_prop_set_ptr(dev, "cpu_env", env);
-qdev_init_nofail(dev);
-
-return dev;
-}
-
 void pc_acpi_smi_interrupt(void *opaque, int irq, int level)
 {
 CPUX86State *s = opaque;
@@ -911,30 +891,17 @@ static void pc_cpu_reset(void *opaque)
 cpu_reset(CPU(cpu));
 }
 
-static X86CPU *pc_new_cpu(const char *cpu_model)
-{
-X86CPU *cpu;
-CPUX86State *env;
-
-cpu = cpu_x86_init(cpu_model);
-if (cpu == NULL) {
-exit(1);
-}
-env = &cpu->env;
-if ((env->cpuid_features & CPUID_APIC) || smp_cpus > 1) {
-env->apic_state = apic_init(env, env->cpuid_apic_id);
-}
-qemu_register_reset(pc_cpu_reset, cpu);
-pc_cpu_reset(cpu);
-return cpu;
-}
-
 void pc_cpus_init(const char *cpu_model)
 {
+X86CPU *cpu;
 int i;
 
 for(i = 0; i < smp_cpus; i++) {
-pc_new_cpu(cpu_model);
+cpu = cpu_x86_init(cpu_model);
+if (cpu == NULL) {
+exit(1);
+}
+qemu_register_reset(pc_cpu_reset, cpu);
 }
 }
 
diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index e655129..99ef891 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -23,6 +23,7 @@
 
 #include "cpu.h"
 #include "kvm.h"
+#include "hw/xen.h"
 
 #include "qemu-option.h"
 #include "qemu-config.h"
@@ -31,6 +32,9 @@
 
 #include "hyperv.h"
 
+#include "hw/qdev.h"
+#include "sysemu.h"
+
 /* feature flags taken from "Intel Processor Identification and the CPUID
  * Instruction" and AMD's "CPUID Specification".  In cases of disagreement
  * between feature naming conventions, aliases may be added.
@@ -1749,24 +1753,89 @@ static void x86_set_cpu_model(Object *obj, const char 
*value, Error **errp)
 if (cpu_x86_register(cpu, env->cpu_model_str) < 0) {
 fprintf(stderr, "Unable to find x86 CPU definition\n");
 error_set(errp, QERR_INVALID_PARAMETER_COMBINATION);
+return;
+}
+
+#ifndef CONFIG_USER_ONLY
+if (((env->cpuid_features & CPUID_APIC) || smp_cpus > 1)) {
+if (kvm_irqchip_in_kernel()) {
+env->apic_state = qdev_create(NULL, "kvm-apic");
+} else if (xen_enabled()) {
+env->apic_state = qdev_create(NULL, "xen-apic");
+} else {
+env->apic_state = qdev_create(NULL, "apic");
+}
+object_property_add_child(OBJECT(cpu), "apic",
+OBJECT(env->apic_state), NULL);
+
+qdev_prop_set_uint8(env->apic_state, "id", env->cpuid_apic_id);
+qdev_prop_set_ptr(env->apic_state, "cpu_env", env);
+}
+#endif
+}
+
+#ifndef CONFIG_USER_ONLY
+static CPUDebugExcpHandler *prev_debug_excp_handler;
+
+static void breakpoint_handler(CPUX86State *env)
+{
+CPUBreakpoint *bp;
+
+if (env->watchpoint_hit) {
+if (env->watchpoint_hit->flags & BP_CPU) {
+env->watchpoint_hit = NULL;
+if (check_hw_breakpoints(env, 0)) {
+raise_exception_env(EXCP01_DB, env);
+} else {
+cpu_resume_from_signal(env, NULL);
+}
+}
+} else {
+QTAILQ_FOREACH(bp, &env->breakpoints, entry)
+if (bp->pc == env->eip) {
+if (bp->flags & BP_CPU) {
+check_hw_breakpoints(env, 1);
+raise_exception_env(EXCP01_DB, env);
+}
+break;
+}
+}
+if (prev_debug_excp_handler) {
+prev_debug_excp_han

Re: [Qemu-devel] [PATCH 06/21] qdev: push "type" property up to Object

[Andreas Färber]

Am 02.05.2012 13:30, schrieb Paolo Bonzini:
> Now that Object is a type, add an instance_init function and push
> the "type" property from qdev to there.
> 
> Signed-off-by: Paolo Bonzini 

The rebased version from qdev-props-4 breaks and hangs `make check`:

GTESTER check-qtest-sparc
qemu-system-sparc: Insufficient permission to perform this operation

Paolo, can you please investigate?

Thanks,
Andreas

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

[Qemu-devel] [PATCH v2 06/15] net: Remove vlan qdev property

[zwu . kernel]

From: Stefan Hajnoczi 

The vlan feature is implemented using hubs and no longer uses
special-purpose VLANState structs that are accessible as qdev
properties.

Signed-off-by: Stefan Hajnoczi 
Signed-off-by: Zhi Yong Wu 
---
 hw/qdev-properties.c |   72 --
 hw/qdev.c|2 -
 hw/qdev.h|4 ---
 net.h|3 --
 4 files changed, 0 insertions(+), 81 deletions(-)

diff --git a/hw/qdev-properties.c b/hw/qdev-properties.c
index b7b5597..d2e2afb 100644
--- a/hw/qdev-properties.c
+++ b/hw/qdev-properties.c
@@ -623,71 +623,6 @@ PropertyInfo qdev_prop_netdev = {
 .set   = set_netdev,
 };
 
-/* --- vlan --- */
-
-static int print_vlan(DeviceState *dev, Property *prop, char *dest, size_t len)
-{
-VLANState **ptr = qdev_get_prop_ptr(dev, prop);
-
-if (*ptr) {
-return snprintf(dest, len, "%d", (*ptr)->id);
-} else {
-return snprintf(dest, len, "");
-}
-}
-
-static void get_vlan(Object *obj, Visitor *v, void *opaque,
- const char *name, Error **errp)
-{
-DeviceState *dev = DEVICE(obj);
-Property *prop = opaque;
-VLANState **ptr = qdev_get_prop_ptr(dev, prop);
-int64_t id;
-
-id = *ptr ? (*ptr)->id : -1;
-visit_type_int(v, &id, name, errp);
-}
-
-static void set_vlan(Object *obj, Visitor *v, void *opaque,
- const char *name, Error **errp)
-{
-DeviceState *dev = DEVICE(obj);
-Property *prop = opaque;
-VLANState **ptr = qdev_get_prop_ptr(dev, prop);
-Error *local_err = NULL;
-int64_t id;
-VLANState *vlan;
-
-if (dev->state != DEV_STATE_CREATED) {
-error_set(errp, QERR_PERMISSION_DENIED);
-return;
-}
-
-visit_type_int(v, &id, name, &local_err);
-if (local_err) {
-error_propagate(errp, local_err);
-return;
-}
-if (id == -1) {
-*ptr = NULL;
-return;
-}
-vlan = qemu_find_vlan(id, 1);
-if (!vlan) {
-error_set(errp, QERR_INVALID_PARAMETER_VALUE,
-  name, prop->info->name);
-return;
-}
-*ptr = vlan;
-}
-
-PropertyInfo qdev_prop_vlan = {
-.name  = "vlan",
-.print = print_vlan,
-.get   = get_vlan,
-.set   = set_vlan,
-};
-
 /* --- pointer --- */
 
 /* Not a proper property, just for dirty hacks.  TODO Remove it!  */
@@ -1094,13 +1029,6 @@ void qdev_prop_set_netdev(DeviceState *dev, const char 
*name, VLANClientState *v
 assert_no_error(errp);
 }
 
-void qdev_prop_set_vlan(DeviceState *dev, const char *name, VLANState *value)
-{
-Error *errp = NULL;
-object_property_set_int(OBJECT(dev), value ? value->id : -1, name, &errp);
-assert_no_error(errp);
-}
-
 void qdev_prop_set_macaddr(DeviceState *dev, const char *name, uint8_t *value)
 {
 Error *errp = NULL;
diff --git a/hw/qdev.c b/hw/qdev.c
index 6a8f6bd..49dd303 100644
--- a/hw/qdev.c
+++ b/hw/qdev.c
@@ -316,8 +316,6 @@ void qdev_connect_gpio_out(DeviceState * dev, int n, 
qemu_irq pin)
 void qdev_set_nic_properties(DeviceState *dev, NICInfo *nd)
 {
 qdev_prop_set_macaddr(dev, "mac", nd->macaddr.a);
-if (nd->vlan)
-qdev_prop_set_vlan(dev, "vlan", nd->vlan);
 if (nd->netdev)
 qdev_prop_set_netdev(dev, "netdev", nd->netdev);
 if (nd->nvectors != DEV_NVECTORS_UNSPECIFIED &&
diff --git a/hw/qdev.h b/hw/qdev.h
index 4e90119..0a50a40 100644
--- a/hw/qdev.h
+++ b/hw/qdev.h
@@ -222,7 +222,6 @@ extern PropertyInfo qdev_prop_macaddr;
 extern PropertyInfo qdev_prop_losttickpolicy;
 extern PropertyInfo qdev_prop_drive;
 extern PropertyInfo qdev_prop_netdev;
-extern PropertyInfo qdev_prop_vlan;
 extern PropertyInfo qdev_prop_pci_devfn;
 extern PropertyInfo qdev_prop_blocksize;
 
@@ -277,8 +276,6 @@ extern PropertyInfo qdev_prop_blocksize;
 DEFINE_PROP(_n, _s, _f, qdev_prop_string, char*)
 #define DEFINE_PROP_NETDEV(_n, _s, _f) \
 DEFINE_PROP(_n, _s, _f, qdev_prop_netdev, VLANClientState*)
-#define DEFINE_PROP_VLAN(_n, _s, _f) \
-DEFINE_PROP(_n, _s, _f, qdev_prop_vlan, VLANState*)
 #define DEFINE_PROP_DRIVE(_n, _s, _f) \
 DEFINE_PROP(_n, _s, _f, qdev_prop_drive, BlockDriverState *)
 #define DEFINE_PROP_MACADDR(_n, _s, _f) \
@@ -305,7 +302,6 @@ void qdev_prop_set_uint64(DeviceState *dev, const char 
*name, uint64_t value);
 void qdev_prop_set_string(DeviceState *dev, const char *name, char *value);
 void qdev_prop_set_chr(DeviceState *dev, const char *name, CharDriverState 
*value);
 void qdev_prop_set_netdev(DeviceState *dev, const char *name, VLANClientState 
*value);
-void qdev_prop_set_vlan(DeviceState *dev, const char *name, VLANState *value);
 int qdev_prop_set_drive(DeviceState *dev, const char *name, BlockDriverState 
*value) QEMU_WARN_UNUSED_RESULT;
 void qdev_prop_set_drive_nofail(DeviceState *dev, const char *name, 
BlockDriverState *value);
 void qdev_prop_set_macaddr(DeviceState *dev, const char *name, uint8_t *value);
diff --git a/net.h b/n

Re: [Qemu-devel] [PATCH v2 15/15] net: invoke qemu_can_send_packet only before net queue sending function

[Paolo Bonzini]

Il 23/05/2012 17:14, zwu.ker...@gmail.com ha scritto:
> From: Zhi Yong Wu 
> 
> Signed-off-by: Zhi Yong Wu 
> ---
>  net/queue.c  |4 ++--
>  net/slirp.c  |7 ---
>  net/tap.c|2 +-
>  slirp/if.c   |5 -
>  slirp/libslirp.h |1 -
>  5 files changed, 3 insertions(+), 16 deletions(-)
> 
> diff --git a/net/queue.c b/net/queue.c
> index 0afd783..d2e57de 100644
> --- a/net/queue.c
> +++ b/net/queue.c
> @@ -176,7 +176,7 @@ ssize_t qemu_net_queue_send(NetQueue *queue,
>  {
>  ssize_t ret;
>  
> -if (queue->delivering) {
> +if (queue->delivering || !qemu_can_send_packet(sender)) {
>  return qemu_net_queue_append(queue, sender, flags, data, size, NULL);
>  }
>  
> @@ -200,7 +200,7 @@ ssize_t qemu_net_queue_send_iov(NetQueue *queue,
>  {
>  ssize_t ret;
>  
> -if (queue->delivering) {
> +if (queue->delivering || !qemu_can_send_packet(sender)) {
>  return qemu_net_queue_append_iov(queue, sender, flags, iov, iovcnt, 
> NULL);
>  }
>  
> diff --git a/net/slirp.c b/net/slirp.c
> index a6ede2b..248f7ff 100644
> --- a/net/slirp.c
> +++ b/net/slirp.c
> @@ -96,13 +96,6 @@ static void slirp_smb_cleanup(SlirpState *s);
>  static inline void slirp_smb_cleanup(SlirpState *s) { }
>  #endif
>  
> -int slirp_can_output(void *opaque)
> -{
> -SlirpState *s = opaque;
> -
> -return qemu_can_send_packet(&s->nc);
> -}
> -
>  void slirp_output(void *opaque, const uint8_t *pkt, int pkt_len)
>  {
>  SlirpState *s = opaque;
> diff --git a/net/tap.c b/net/tap.c
> index 65f45b8..7b1992b 100644
> --- a/net/tap.c
> +++ b/net/tap.c
> @@ -210,7 +210,7 @@ static void tap_send(void *opaque)
>  if (size == 0) {
>  tap_read_poll(s, 0);
>  }
> -} while (size > 0 && qemu_can_send_packet(&s->nc));
> +} while (size > 0);

Can you explain this?  Also, have you benchmarked the change to see what
effect it has?

Also, can you explain why you didn't implement this?

>> If they did, hubs could then do their own flow control via can_receive.
>> When qemu_send_packet returns zero you increment a count of in-flight
>> packets, and a sent-packet callback would decrement the same count. When the
>> count is non-zero, can_receive returns false (and vice versa).  The sent_cb
>> also needs to call qemu_flush_queued_packets when the count drop to zero.
>> With this in place, I think the other TODO about the return value is easily
>> solved; receive/receive_iov callbacks can simply return immediate success,
>> and later block further sends.

Paolo

>  }
>  
>  int tap_has_ufo(NetClientState *nc)
> diff --git a/slirp/if.c b/slirp/if.c
> index 096cf6f..533295d 100644
> --- a/slirp/if.c
> +++ b/slirp/if.c
> @@ -177,11 +177,6 @@ void if_start(Slirp *slirp)
>  }
>  
>  while (ifm_next) {
> -/* check if we can really output */
> -if (!slirp_can_output(slirp->opaque)) {
> -break;
> -}
> -
>  ifm = ifm_next;
>  from_batchq = next_from_batchq;
>  
> diff --git a/slirp/libslirp.h b/slirp/libslirp.h
> index 77527ad..9b471b5 100644
> --- a/slirp/libslirp.h
> +++ b/slirp/libslirp.h
> @@ -25,7 +25,6 @@ void slirp_select_poll(fd_set *readfds, fd_set *writefds, 
> fd_set *xfds,
>  void slirp_input(Slirp *slirp, const uint8_t *pkt, int pkt_len);
>  
>  /* you must provide the following functions: */
> -int slirp_can_output(void *opaque);
>  void slirp_output(void *opaque, const uint8_t *pkt, int pkt_len);
>  
>  int slirp_add_hostfwd(Slirp *slirp, int is_udp,

Re: [Qemu-devel] [PATCH 05/21] qom: assert that public types have a non-NULL parent field

[Andreas Färber]

Am 02.05.2012 14:35, schrieb Andreas Färber:
> Am 02.05.2012 13:30, schrieb Paolo Bonzini:
>> This protects against unwanted effects of changing TYPE_OBJECT from
>> NULL to a string.  Suggested by Andreas Faerber.
>>
>> Signed-off-by: Paolo Bonzini 
>> ---
>>  include/qemu/object.h |1 -
>>  qom/object.c  |   14 ++
>>  2 files changed, 10 insertions(+), 5 deletions(-)
>>
>> diff --git a/include/qemu/object.h b/include/qemu/object.h
>> index 9c49d99..ec2b382 100644
>> --- a/include/qemu/object.h
>> +++ b/include/qemu/object.h
>> @@ -527,7 +527,6 @@ const char *object_get_typename(Object *obj);
>>   */
>>  Type type_register_static(const TypeInfo *info);
>>  
>> -#define type_register_static_alias(info, name) do { } while (0)
> 
> Unrelated removal of unused code?
> 
> Other than that
> 
> Reviewed-by: Andreas Färber 

Split in two and applied to qom-next:
http://repo.or.cz/w/qemu/afaerber.git/shortlog/refs/heads/qom-next

/-F

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

[Qemu-devel] [PATCH 07/10] qdev: move bulk of qdev-properties.c to qom/object.c

[Paolo Bonzini]

Signed-off-by: Paolo Bonzini 
---
 hw/qdev-properties.c  |  487 +
 hw/qdev.c |   47 +
 hw/qdev.h |   87 -
 include/qemu/object.h |   98 ++
 qom/object.c  |  459 ++
 5 files changed, 604 insertions(+), 574 deletions(-)

diff --git a/hw/qdev-properties.c b/hw/qdev-properties.c
index 4665d60..58f73f9 100644
--- a/hw/qdev-properties.c
+++ b/hw/qdev-properties.c
@@ -3,392 +3,27 @@
 #include "qerror.h"
 #include "blockdev.h"
 
-void *object_get_prop_ptr(Object *obj, Property *prop)
-{
-void *ptr = obj;
-ptr += prop->offset;
-return ptr;
-}
-
-static uint32_t get_prop_mask(Property *prop)
-{
-assert(prop->info == &qdev_prop_bit);
-return 0x1 << prop->bitnr;
-}
-
-static void bit_prop_set(Object *obj, Property *props, bool val)
-{
-uint32_t *p = object_get_prop_ptr(obj, props);
-uint32_t mask = get_prop_mask(props);
-if (val)
-*p |= mask;
-else
-*p &= ~mask;
-}
-
-/* Bit */
-
-static int print_bit(Object *obj, Property *prop, char *dest, size_t len)
-{
-uint32_t *p = object_get_prop_ptr(obj, prop);
-return snprintf(dest, len, (*p & get_prop_mask(prop)) ? "on" : "off");
-}
-
-static void get_bit(Object *obj, Visitor *v, void *opaque,
-const char *name, Error **errp)
-{
-Property *prop = opaque;
-uint32_t *p = object_get_prop_ptr(obj, prop);
-bool value = (*p & get_prop_mask(prop)) != 0;
-
-visit_type_bool(v, &value, name, errp);
-}
-
-static void set_bit(Object *obj, Visitor *v, void *opaque,
-const char *name, Error **errp)
-{
-Property *prop = opaque;
-Error *local_err = NULL;
-bool value;
-
-if (object_is_realized(obj)) {
-error_set(errp, QERR_PERMISSION_DENIED);
-return;
-}
-
-visit_type_bool(v, &value, name, &local_err);
-if (local_err) {
-error_propagate(errp, local_err);
-return;
-}
-bit_prop_set(obj, prop, value);
-}
-
-PropertyInfo qdev_prop_bit = {
-.name  = "boolean",
-.legacy_name  = "on/off",
-.print = print_bit,
-.get   = get_bit,
-.set   = set_bit,
-};
-
-/* --- 8bit integer --- */
-
-static void get_uint8(Object *obj, Visitor *v, void *opaque,
-  const char *name, Error **errp)
-{
-Property *prop = opaque;
-uint8_t *ptr = object_get_prop_ptr(obj, prop);
-
-visit_type_uint8(v, ptr, name, errp);
-}
-
-static void set_uint8(Object *obj, Visitor *v, void *opaque,
-  const char *name, Error **errp)
-{
-Property *prop = opaque;
-uint8_t *ptr = object_get_prop_ptr(obj, prop);
-
-if (object_is_realized(obj)) {
-error_set(errp, QERR_PERMISSION_DENIED);
-return;
-}
-
-visit_type_uint8(v, ptr, name, errp);
-}
-
-PropertyInfo qdev_prop_uint8 = {
-.name  = "uint8",
-.get   = get_uint8,
-.set   = set_uint8,
-};
-
-/* --- 8bit hex value --- */
-
-static int parse_hex8(Object *obj, Property *prop, const char *str)
-{
-uint8_t *ptr = object_get_prop_ptr(obj, prop);
-char *end;
-
-if (str[0] != '0' || str[1] != 'x') {
-return -EINVAL;
-}
-
-*ptr = strtoul(str, &end, 16);
-if ((*end != '\0') || (end == str)) {
-return -EINVAL;
-}
-
-return 0;
-}
-
-static int print_hex8(Object *obj, Property *prop, char *dest, size_t len)
-{
-uint8_t *ptr = object_get_prop_ptr(obj, prop);
-return snprintf(dest, len, "0x%" PRIx8, *ptr);
-}
-
-PropertyInfo qdev_prop_hex8 = {
-.name  = "uint8",
-.legacy_name  = "hex8",
-.parse = parse_hex8,
-.print = print_hex8,
-.get   = get_uint8,
-.set   = set_uint8,
-};
-
-/* --- 16bit integer --- */
-
-static void get_uint16(Object *obj, Visitor *v, void *opaque,
-   const char *name, Error **errp)
-{
-Property *prop = opaque;
-uint16_t *ptr = object_get_prop_ptr(obj, prop);
-
-visit_type_uint16(v, ptr, name, errp);
-}
-
-static void set_uint16(Object *obj, Visitor *v, void *opaque,
-   const char *name, Error **errp)
-{
-Property *prop = opaque;
-uint16_t *ptr = object_get_prop_ptr(obj, prop);
-
-if (object_is_realized(obj)) {
-error_set(errp, QERR_PERMISSION_DENIED);
-return;
-}
-
-visit_type_uint16(v, ptr, name, errp);
-}
-
-PropertyInfo qdev_prop_uint16 = {
-.name  = "uint16",
-.get   = get_uint16,
-.set   = set_uint16,
-};
-
-/* --- 32bit integer --- */
-
-static void get_uint32(Object *obj, Visitor *v, void *opaque,
-   const char *name, Error **errp)
-{
-Property *prop = opaque;
-uint32_t *ptr = object_get_prop_ptr(obj, prop);
-
-visit_type_uint32(v, ptr, name, errp);
-}
-
-static void set_uint32(Object *obj, Visitor *v, void *opaque,
-   const char *name, Error **errp)
-{
-Property *prop = opaque;
-uint32_t *ptr = ob

[Qemu-devel] [PATCH qom-next 1/6] pc: Enable MSI support at APIC level

[Igor Mammedov]

From: Jan Kiszka 

Push msi_supported enabling to the APIC implementations where we can
encapsulate the decision more cleanly, hiding the details from the
generic code.

Acked-by: Stefano Stabellini 
Signed-off-by: Jan Kiszka 
Signed-off-by: Marcelo Tosatti 
---
 hw/apic.c |3 +++
 hw/pc.c   |9 -
 hw/xen.h  |   10 --
 hw/xen_apic.c |5 +
 4 files changed, 8 insertions(+), 19 deletions(-)

diff --git a/hw/apic.c b/hw/apic.c
index 4eeaf88..5fbf01c 100644
--- a/hw/apic.c
+++ b/hw/apic.c
@@ -19,6 +19,7 @@
 #include "apic_internal.h"
 #include "apic.h"
 #include "ioapic.h"
+#include "msi.h"
 #include "host-utils.h"
 #include "trace.h"
 #include "pc.h"
@@ -862,6 +863,8 @@ static void apic_init(APICCommonState *s)
 
 s->timer = qemu_new_timer_ns(vm_clock, apic_timer, s);
 local_apics[s->idx] = s;
+
+msi_supported = true;
 }
 
 static void apic_class_init(ObjectClass *klass, void *data)
diff --git a/hw/pc.c b/hw/pc.c
index 4167782..ad27f36 100644
--- a/hw/pc.c
+++ b/hw/pc.c
@@ -911,15 +911,6 @@ static DeviceState *apic_init(void *env, uint8_t apic_id)
 apic_mapped = 1;
 }
 
-/* KVM does not support MSI yet. */
-if (!kvm_irqchip_in_kernel()) {
-msi_supported = true;
-}
-
-if (xen_msi_support()) {
-msi_supported = true;
-}
-
 return dev;
 }
 
diff --git a/hw/xen.h b/hw/xen.h
index 3ae4cd0..e5926b7 100644
--- a/hw/xen.h
+++ b/hw/xen.h
@@ -57,14 +57,4 @@ void xen_register_framebuffer(struct MemoryRegion *mr);
 #  define HVM_MAX_VCPUS 32
 #endif
 
-static inline int xen_msi_support(void)
-{
-#if defined(CONFIG_XEN_CTRL_INTERFACE_VERSION) \
-&& CONFIG_XEN_CTRL_INTERFACE_VERSION >= 420
-return xen_enabled();
-#else
-return 0;
-#endif
-}
-
 #endif /* QEMU_HW_XEN_H */
diff --git a/hw/xen_apic.c b/hw/xen_apic.c
index 1725ff6..a9e101f 100644
--- a/hw/xen_apic.c
+++ b/hw/xen_apic.c
@@ -40,6 +40,11 @@ static void xen_apic_init(APICCommonState *s)
 {
 memory_region_init_io(&s->io_memory, &xen_apic_io_ops, s, "xen-apic-msi",
   MSI_SPACE_SIZE);
+
+#if defined(CONFIG_XEN_CTRL_INTERFACE_VERSION) \
+&& CONFIG_XEN_CTRL_INTERFACE_VERSION >= 420
+msi_supported = true;
+#endif
 }
 
 static void xen_apic_set_base(APICCommonState *s, uint64_t val)
-- 
1.7.7.6

[Qemu-devel] [PATCH 06/10] qdev: generalize properties to Objects

[Paolo Bonzini]

The property machinery uses DeviceState arguments in a few places.
Replace this with Object so that we can push properties up.

Signed-off-by: Paolo Bonzini 
---
 hw/qdev-addr.c   |   19 +++---
 hw/qdev-properties.c |  180 +-
 hw/qdev.c|8 +--
 hw/qdev.h|   10 +--
 4 files changed, 93 insertions(+), 124 deletions(-)

diff --git a/hw/qdev-addr.c b/hw/qdev-addr.c
index a3796bd..99ca116 100644
--- a/hw/qdev-addr.c
+++ b/hw/qdev-addr.c
@@ -5,26 +5,25 @@
 
 /* --- target physical address --- */
 
-static int parse_taddr(DeviceState *dev, Property *prop, const char *str)
+static int parse_taddr(Object *obj, Property *prop, const char *str)
 {
-target_phys_addr_t *ptr = qdev_get_prop_ptr(dev, prop);
+target_phys_addr_t *ptr = object_get_prop_ptr(obj, prop);
 
 *ptr = strtoull(str, NULL, 16);
 return 0;
 }
 
-static int print_taddr(DeviceState *dev, Property *prop, char *dest, size_t 
len)
+static int print_taddr(Object *obj, Property *prop, char *dest, size_t len)
 {
-target_phys_addr_t *ptr = qdev_get_prop_ptr(dev, prop);
+target_phys_addr_t *ptr = object_get_prop_ptr(obj, prop);
 return snprintf(dest, len, "0x" TARGET_FMT_plx, *ptr);
 }
 
 static void get_taddr(Object *obj, Visitor *v, void *opaque,
   const char *name, Error **errp)
 {
-DeviceState *dev = DEVICE(obj);
 Property *prop = opaque;
-target_phys_addr_t *ptr = qdev_get_prop_ptr(dev, prop);
+target_phys_addr_t *ptr = object_get_prop_ptr(obj, prop);
 int64_t value;
 
 value = *ptr;
@@ -34,9 +33,8 @@ static void get_taddr(Object *obj, Visitor *v, void *opaque,
 static void set_taddr(Object *obj, Visitor *v, void *opaque,
   const char *name, Error **errp)
 {
-DeviceState *dev = DEVICE(obj);
 Property *prop = opaque;
-target_phys_addr_t *ptr = qdev_get_prop_ptr(dev, prop);
+target_phys_addr_t *ptr = object_get_prop_ptr(obj, prop);
 Error *local_err = NULL;
 int64_t value;
 
@@ -53,9 +51,8 @@ static void set_taddr(Object *obj, Visitor *v, void *opaque,
 if ((uint64_t)value <= (uint64_t) ~(target_phys_addr_t)0) {
 *ptr = value;
 } else {
-error_set(errp, QERR_PROPERTY_VALUE_OUT_OF_RANGE,
-  dev->id?:"", name, value, (uint64_t) 0,
-  (uint64_t) ~(target_phys_addr_t)0);
+error_set(errp, QERR_INVALID_PARAMETER_VALUE,
+ name, "target_phys_addr_t");
 }
 }
 
diff --git a/hw/qdev-properties.c b/hw/qdev-properties.c
index 7121759..4665d60 100644
--- a/hw/qdev-properties.c
+++ b/hw/qdev-properties.c
@@ -3,23 +3,23 @@
 #include "qerror.h"
 #include "blockdev.h"
 
-void *qdev_get_prop_ptr(DeviceState *dev, Property *prop)
+void *object_get_prop_ptr(Object *obj, Property *prop)
 {
-void *ptr = dev;
+void *ptr = obj;
 ptr += prop->offset;
 return ptr;
 }
 
-static uint32_t qdev_get_prop_mask(Property *prop)
+static uint32_t get_prop_mask(Property *prop)
 {
 assert(prop->info == &qdev_prop_bit);
 return 0x1 << prop->bitnr;
 }
 
-static void bit_prop_set(DeviceState *dev, Property *props, bool val)
+static void bit_prop_set(Object *obj, Property *props, bool val)
 {
-uint32_t *p = qdev_get_prop_ptr(dev, props);
-uint32_t mask = qdev_get_prop_mask(props);
+uint32_t *p = object_get_prop_ptr(obj, props);
+uint32_t mask = get_prop_mask(props);
 if (val)
 *p |= mask;
 else
@@ -28,19 +28,18 @@ static void bit_prop_set(DeviceState *dev, Property *props, 
bool val)
 
 /* Bit */
 
-static int print_bit(DeviceState *dev, Property *prop, char *dest, size_t len)
+static int print_bit(Object *obj, Property *prop, char *dest, size_t len)
 {
-uint32_t *p = qdev_get_prop_ptr(dev, prop);
-return snprintf(dest, len, (*p & qdev_get_prop_mask(prop)) ? "on" : "off");
+uint32_t *p = object_get_prop_ptr(obj, prop);
+return snprintf(dest, len, (*p & get_prop_mask(prop)) ? "on" : "off");
 }
 
 static void get_bit(Object *obj, Visitor *v, void *opaque,
 const char *name, Error **errp)
 {
-DeviceState *dev = DEVICE(obj);
 Property *prop = opaque;
-uint32_t *p = qdev_get_prop_ptr(dev, prop);
-bool value = (*p & qdev_get_prop_mask(prop)) != 0;
+uint32_t *p = object_get_prop_ptr(obj, prop);
+bool value = (*p & get_prop_mask(prop)) != 0;
 
 visit_type_bool(v, &value, name, errp);
 }
@@ -48,7 +47,6 @@ static void get_bit(Object *obj, Visitor *v, void *opaque,
 static void set_bit(Object *obj, Visitor *v, void *opaque,
 const char *name, Error **errp)
 {
-DeviceState *dev = DEVICE(obj);
 Property *prop = opaque;
 Error *local_err = NULL;
 bool value;
@@ -63,7 +61,7 @@ static void set_bit(Object *obj, Visitor *v, void *opaque,
 error_propagate(errp, local_err);
 return;
 }
-bit_prop_set(dev, prop, value);
+bit_prop_set(obj, prop, value);
 }
 
 Proper

Re: [Qemu-devel] [PATCH 04/21] qom: make Object a type

[Andreas Färber]

Am 16.05.2012 10:16, schrieb Andreas Färber:
> Am 02.05.2012 13:30, schrieb Paolo Bonzini:
>> Right now the base Object class has a special NULL type.  Change this so
>> that we will be able to add class_init and class_base_init callbacks.
>> To do this, remove some special casing of ObjectClass that is not really
>> necessary.
>>
>> Signed-off-by: Paolo Bonzini 
> 
> Anthony, you had Reviewed-by an earlier version of this patch but then
> did it differently in your series. Should I apply this to qom-next or is
> there some issue with it? The issue of unintended NULL .parent that I
> raised is being addressed with a followup patch.

Anthony agreed to this version on IRC so I've applied this to qom-next:
http://repo.or.cz/w/qemu/afaerber.git/shortlog/refs/heads/qom-next

Andreas

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

[Qemu-devel] [PATCH qom-next 4/6] pc: move apic_mapped initialization into common apic init code

[Igor Mammedov]

Move from apic_init in pc.c, the code that belongs to apic_init_common.

Signed-off-by: Igor Mammedov 
---
 hw/apic_common.c |   11 +++
 hw/msi.h |2 ++
 hw/pc.c  |   12 
 3 files changed, 13 insertions(+), 12 deletions(-)

diff --git a/hw/apic_common.c b/hw/apic_common.c
index 23d51e8..c3fa66b 100644
--- a/hw/apic_common.c
+++ b/hw/apic_common.c
@@ -21,6 +21,7 @@
 #include "apic_internal.h"
 #include "trace.h"
 #include "kvm.h"
+#include "msi.h"
 
 static int apic_irq_delivered;
 bool apic_report_tpr_access;
@@ -284,6 +285,7 @@ static int apic_init_common(SysBusDevice *dev)
 APICCommonClass *info;
 static DeviceState *vapic;
 static int apic_no;
+static int apic_mapped;
 
 if (apic_no >= MAX_APICS) {
 return -1;
@@ -295,6 +297,15 @@ static int apic_init_common(SysBusDevice *dev)
 
 sysbus_init_mmio(dev, &s->io_memory);
 
+/* XXX: mapping more APICs at the same memory location */
+if (apic_mapped == 0) {
+/* NOTE: the APIC is directly connected to the CPU - it is not
+   on the global memory bus. */
+/* XXX: what if the base changes? */
+sysbus_mmio_map(sysbus_from_qdev(&s->busdev.qdev), 0, MSI_ADDR_BASE);
+apic_mapped = 1;
+}
+
 if (!vapic && s->vapic_control & VAPIC_ENABLE_MASK) {
 vapic = sysbus_create_simple("kvmvapic", -1, NULL);
 }
diff --git a/hw/msi.h b/hw/msi.h
index 3040bb0..abd52b6 100644
--- a/hw/msi.h
+++ b/hw/msi.h
@@ -40,4 +40,6 @@ static inline bool msi_present(const PCIDevice *dev)
 return dev->cap_present & QEMU_PCI_CAP_MSI;
 }
 
+#define MSI_ADDR_BASE 0xfee0
+
 #endif /* QEMU_MSI_H */
diff --git a/hw/pc.c b/hw/pc.c
index 1aa90a2..1ccfc6e 100644
--- a/hw/pc.c
+++ b/hw/pc.c
@@ -70,8 +70,6 @@
 #define FW_CFG_E820_TABLE (FW_CFG_ARCH_LOCAL + 3)
 #define FW_CFG_HPET (FW_CFG_ARCH_LOCAL + 4)
 
-#define MSI_ADDR_BASE 0xfee0
-
 #define E820_NR_ENTRIES16
 
 struct e820_entry {
@@ -882,7 +880,6 @@ DeviceState *cpu_get_current_apic(void)
 static DeviceState *apic_init(void *env, uint8_t apic_id)
 {
 DeviceState *dev;
-static int apic_mapped;
 
 if (kvm_irqchip_in_kernel()) {
 dev = qdev_create(NULL, "kvm-apic");
@@ -896,15 +893,6 @@ static DeviceState *apic_init(void *env, uint8_t apic_id)
 qdev_prop_set_ptr(dev, "cpu_env", env);
 qdev_init_nofail(dev);
 
-/* XXX: mapping more APICs at the same memory location */
-if (apic_mapped == 0) {
-/* NOTE: the APIC is directly connected to the CPU - it is not
-   on the global memory bus. */
-/* XXX: what if the base changes? */
-sysbus_mmio_map(sysbus_from_qdev(dev), 0, MSI_ADDR_BASE);
-apic_mapped = 1;
-}
-
 return dev;
 }
 
-- 
1.7.7.6

[Qemu-devel] [PATCH qom-next 2/6] target-i386: move cpu halted decision into x86_cpu_reset

[Igor Mammedov]

From: Igor Mammedov 

MP initialization protocol differs between cpu families, and for P6 and
onward models it is up to CPU to decide if it will be BSP using this
protocol, so try to model this. However there is no point in implementing
MP initialization protocol in qemu. Thus first CPU is always marked as BSP.

This patch:
 - moves decision to designate BSP from board into cpu, making cpu
self-sufficient in this regard. Later it will allow to cleanup hw/pc.c
and remove cpu_reset and wrappers from there.
 - stores flag that CPU is BSP in IA32_APIC_BASE to model behavior
described in Inted SDM vol 3a part 1 chapter 8.4.1
 - uses MSR_IA32_APICBASE_BSP flag in apic_base for checking if cpu is BSP

patch is based on Jan Kiszka's proposal:
http://thread.gmane.org/gmane.comp.emulators.qemu/100806

v2:
  - fix build for i386-linux-user
  spotted-by: Peter Maydell 

Signed-off-by: Igor Mammedov 
---
 hw/apic.h|2 +-
 hw/apic_common.c |   18 --
 hw/pc.c  |9 -
 target-i386/cpu.c|9 +
 target-i386/helper.c |1 -
 target-i386/kvm.c|5 +++--
 6 files changed, 25 insertions(+), 19 deletions(-)

diff --git a/hw/apic.h b/hw/apic.h
index 62179ce..d961ed4 100644
--- a/hw/apic.h
+++ b/hw/apic.h
@@ -20,9 +20,9 @@ void apic_init_reset(DeviceState *s);
 void apic_sipi(DeviceState *s);
 void apic_handle_tpr_access_report(DeviceState *d, target_ulong ip,
TPRAccess access);
+void apic_designate_bsp(DeviceState *d);
 
 /* pc.c */
-int cpu_is_bsp(CPUX86State *env);
 DeviceState *cpu_get_current_apic(void);
 
 #endif
diff --git a/hw/apic_common.c b/hw/apic_common.c
index 60b8259..23d51e8 100644
--- a/hw/apic_common.c
+++ b/hw/apic_common.c
@@ -43,8 +43,8 @@ uint64_t cpu_get_apic_base(DeviceState *d)
 trace_cpu_get_apic_base((uint64_t)s->apicbase);
 return s->apicbase;
 } else {
-trace_cpu_get_apic_base(0);
-return 0;
+trace_cpu_get_apic_base(MSR_IA32_APICBASE_BSP);
+return MSR_IA32_APICBASE_BSP;
 }
 }
 
@@ -201,22 +201,28 @@ void apic_init_reset(DeviceState *d)
 s->timer_expiry = -1;
 }
 
+void apic_designate_bsp(DeviceState *d)
+{
+if (d) {
+APICCommonState *s = APIC_COMMON(d);
+s->apicbase |= MSR_IA32_APICBASE_BSP;
+}
+}
+
 static void apic_reset_common(DeviceState *d)
 {
 APICCommonState *s = DO_UPCAST(APICCommonState, busdev.qdev, d);
 APICCommonClass *info = APIC_COMMON_GET_CLASS(s);
-bool bsp;
 
-bsp = cpu_is_bsp(s->cpu_env);
 s->apicbase = 0xfee0 |
-(bsp ? MSR_IA32_APICBASE_BSP : 0) | MSR_IA32_APICBASE_ENABLE;
+(s->apicbase & MSR_IA32_APICBASE_BSP) | MSR_IA32_APICBASE_ENABLE;
 
 s->vapic_paddr = 0;
 info->vapic_base_update(s);
 
 apic_init_reset(d);
 
-if (bsp) {
+if (s->apicbase & MSR_IA32_APICBASE_BSP) {
 /*
  * LINT0 delivery mode on CPU #0 is set to ExtInt at initialization
  * time typically by BIOS, so PIC interrupt can be delivered to the
diff --git a/hw/pc.c b/hw/pc.c
index ad27f36..c609770 100644
--- a/hw/pc.c
+++ b/hw/pc.c
@@ -870,12 +870,6 @@ void pc_init_ne2k_isa(ISABus *bus, NICInfo *nd)
 nb_ne2k++;
 }
 
-int cpu_is_bsp(CPUX86State *env)
-{
-/* We hard-wire the BSP to the first CPU. */
-return env->cpu_index == 0;
-}
-
 DeviceState *cpu_get_current_apic(void)
 {
 if (cpu_single_env) {
@@ -926,10 +920,7 @@ void pc_acpi_smi_interrupt(void *opaque, int irq, int 
level)
 static void pc_cpu_reset(void *opaque)
 {
 X86CPU *cpu = opaque;
-CPUX86State *env = &cpu->env;
-
 cpu_reset(CPU(cpu));
-env->halted = !cpu_is_bsp(env);
 }
 
 static X86CPU *pc_new_cpu(const char *cpu_model)
diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index 89b4ac7..14c0f64 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -1704,6 +1704,15 @@ static void x86_cpu_reset(CPUState *s)
 env->dr[7] = DR7_FIXED_1;
 cpu_breakpoint_remove_all(env, BP_CPU);
 cpu_watchpoint_remove_all(env, BP_CPU);
+
+#if !defined(CONFIG_USER_ONLY)
+/* We hard-wire the BSP to the first CPU. */
+if (env->cpu_index == 0) {
+apic_designate_bsp(env->apic_state);
+}
+
+env->halted = !(cpu_get_apic_base(env->apic_state) & 
MSR_IA32_APICBASE_BSP);
+#endif
 }
 
 static void mce_init(X86CPU *cpu)
diff --git a/target-i386/helper.c b/target-i386/helper.c
index 2cc8097..3ceefad 100644
--- a/target-i386/helper.c
+++ b/target-i386/helper.c
@@ -1191,7 +1191,6 @@ void do_cpu_init(X86CPU *cpu)
 env->interrupt_request = sipi;
 env->pat = pat;
 apic_init_reset(env->apic_state);
-env->halted = !cpu_is_bsp(env);
 }
 
 void do_cpu_sipi(X86CPU *cpu)
diff --git a/target-i386/kvm.c b/target-i386/kvm.c
index 0d0d8f6..09621e5 100644
--- a/target-i386/kvm.c
+++ b/target-i386/kvm.c
@@ -583,8 +583,9 @@ void kvm_arch_reset_vcpu(CPUX86State *env)
 env->interrupt_injected = -1;
 env->xcr0 = 1;
 if (kvm_irqchip_i

Re: [Qemu-devel] [PATCH qom-next 4/6] pc: move apic_mapped initialization into common apic init code

[Peter Maydell]

On 23 May 2012 17:39, Igor Mammedov  wrote:
> @@ -295,6 +297,15 @@ static int apic_init_common(SysBusDevice *dev)
>
>     sysbus_init_mmio(dev, &s->io_memory);
>
> +    /* XXX: mapping more APICs at the same memory location */
> +    if (apic_mapped == 0) {
> +        /* NOTE: the APIC is directly connected to the CPU - it is not
> +           on the global memory bus. */
> +        /* XXX: what if the base changes? */
> +        sysbus_mmio_map(sysbus_from_qdev(&s->busdev.qdev), 0, MSI_ADDR_BASE);
> +        apic_mapped = 1;
> +    }
> +
>     if (!vapic && s->vapic_control & VAPIC_ENABLE_MASK) {
>         vapic = sysbus_create_simple("kvmvapic", -1, NULL);
>     }

This looks wrong -- sysbus device init functions shouldn't
be mapping MMIO regions themselves, in general. They should
expose MMIO regions to be mapped by whichever device or board
model creates them. Which is what the code before this patch
was doing -- why do you want to move this code?

-- PMM

[Qemu-devel] [PATCH qom-next 6/6] target-i386: move reset callback to cpu.c

[Igor Mammedov]

Moving reset callback into cpu object from board level will allow
properly create object during run-time (hotplug).

When reset over QOM hierarchy is implemented, this reset callback
should be removed.

v2:
  - fix build for i386-linux-target

Signed-off-by: Igor Mammedov 
---
 hw/pc.c   |7 ---
 target-i386/cpu.c |   11 +++
 2 files changed, 11 insertions(+), 7 deletions(-)

diff --git a/hw/pc.c b/hw/pc.c
index d7845ea..868dbe7 100644
--- a/hw/pc.c
+++ b/hw/pc.c
@@ -885,12 +885,6 @@ void pc_acpi_smi_interrupt(void *opaque, int irq, int 
level)
 }
 }
 
-static void pc_cpu_reset(void *opaque)
-{
-X86CPU *cpu = opaque;
-cpu_reset(CPU(cpu));
-}
-
 void pc_cpus_init(const char *cpu_model)
 {
 X86CPU *cpu;
@@ -901,7 +895,6 @@ void pc_cpus_init(const char *cpu_model)
 if (cpu == NULL) {
 exit(1);
 }
-qemu_register_reset(pc_cpu_reset, cpu);
 }
 }
 
diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index 99ef891..ad39f71 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -1775,6 +1775,13 @@ static void x86_set_cpu_model(Object *obj, const char 
*value, Error **errp)
 }
 
 #ifndef CONFIG_USER_ONLY
+/* TODO: remove me, when reset over QOM tree is implemented */
+static void x86_cpu_machine_reset_cb(void *opaque)
+{
+X86CPU *cpu = opaque;
+cpu_reset(CPU(cpu));
+}
+
 static CPUDebugExcpHandler *prev_debug_excp_handler;
 
 static void breakpoint_handler(CPUX86State *env)
@@ -1823,6 +1830,10 @@ void x86_cpu_realize(Object *obj, Error **errp)
 
 mce_init(cpu);
 qemu_init_vcpu(env);
+
+#ifndef CONFIG_USER_ONLY
+qemu_register_reset(x86_cpu_machine_reset_cb, cpu);
+#endif
 cpu_reset(CPU(cpu));
 }
 
-- 
1.7.7.6

[Qemu-devel] [PATCH qom-next v2 0/6] target-i386: re-factor CPU creation/initialization to QOM

[Igor Mammedov]

Moving code related to CPU creation and initialization internal parts
from board level into apic and cpu objects will allow X86CPU to better
model QOM object life-cycle.
It will allow to create X86CPU as any other object by creating it with
object_new() then setting properties and then calling x86_cpu_realize()
to make it running. Later x86_cpu_realize() should become realize property.


 1  pc: Enable MSI support at APIC level
 http://thread.gmane.org/gmane.comp.emulators.kvm.devel/91186
 included here to prevent confilict since qom-next doesn't have it
 but patches 4, 5 are depending on it. I guess it would be best way to
 avoid confilicts, correct me if I'm wrong.

 2  target-i386: move cpu halted decision into x86_cpu_reset
 3  target-i386: add cpu-model property to x86_cpu
 4  pc: move apic_mapped initialization into common apic init code
 5  target-i386: make initialize CPU in QOM way
 6  target-i386: move reset callback to cpu.c


git tree for testing: 
  https://github.com/imammedo/qemu/tree/x86-cpu-realize-v2

Compile & Run tested:
  target-i386: tcg and kvm mode
  i386-linux-user: running of /bin/ls and /usr/bin/make on qemu tree

[Qemu-devel] [PATCH 05/10] qdev: push state up to Object

[Paolo Bonzini]

qdev properties use the state member (an embryo of the "realized"
property) in order to disable setting them after a device has been
initialized.  So, in order to push qdev properties up to Object
we need to push this bit there too.

Signed-off-by: Paolo Bonzini 
---
 hw/qdev-addr.c|3 ++-
 hw/qdev-properties.c  |   26 +-
 hw/qdev.c |   11 +--
 hw/qdev.h |6 --
 include/qemu/object.h |   14 ++
 qom/object.c  |6 ++
 6 files changed, 40 insertions(+), 26 deletions(-)

diff --git a/hw/qdev-addr.c b/hw/qdev-addr.c
index b711b6b..a3796bd 100644
--- a/hw/qdev-addr.c
+++ b/hw/qdev-addr.c
@@ -1,3 +1,4 @@
+#include "qemu/object.h"
 #include "qdev.h"
 #include "qdev-addr.h"
 #include "targphys.h"
@@ -39,7 +40,7 @@ static void set_taddr(Object *obj, Visitor *v, void *opaque,
 Error *local_err = NULL;
 int64_t value;
 
-if (dev->state != DEV_STATE_CREATED) {
+if (object_is_realized(obj)) {
 error_set(errp, QERR_PERMISSION_DENIED);
 return;
 }
diff --git a/hw/qdev-properties.c b/hw/qdev-properties.c
index 9a6c04a..7121759 100644
--- a/hw/qdev-properties.c
+++ b/hw/qdev-properties.c
@@ -53,7 +53,7 @@ static void set_bit(Object *obj, Visitor *v, void *opaque,
 Error *local_err = NULL;
 bool value;
 
-if (dev->state != DEV_STATE_CREATED) {
+if (object_is_realized(obj)) {
 error_set(errp, QERR_PERMISSION_DENIED);
 return;
 }
@@ -93,7 +93,7 @@ static void set_uint8(Object *obj, Visitor *v, void *opaque,
 Property *prop = opaque;
 uint8_t *ptr = qdev_get_prop_ptr(dev, prop);
 
-if (dev->state != DEV_STATE_CREATED) {
+if (object_is_realized(obj)) {
 error_set(errp, QERR_PERMISSION_DENIED);
 return;
 }
@@ -160,7 +160,7 @@ static void set_uint16(Object *obj, Visitor *v, void 
*opaque,
 Property *prop = opaque;
 uint16_t *ptr = qdev_get_prop_ptr(dev, prop);
 
-if (dev->state != DEV_STATE_CREATED) {
+if (object_is_realized(obj)) {
 error_set(errp, QERR_PERMISSION_DENIED);
 return;
 }
@@ -193,7 +193,7 @@ static void set_uint32(Object *obj, Visitor *v, void 
*opaque,
 Property *prop = opaque;
 uint32_t *ptr = qdev_get_prop_ptr(dev, prop);
 
-if (dev->state != DEV_STATE_CREATED) {
+if (object_is_realized(obj)) {
 error_set(errp, QERR_PERMISSION_DENIED);
 return;
 }
@@ -218,7 +218,7 @@ static void set_int32(Object *obj, Visitor *v, void *opaque,
 Property *prop = opaque;
 int32_t *ptr = qdev_get_prop_ptr(dev, prop);
 
-if (dev->state != DEV_STATE_CREATED) {
+if (object_is_realized(obj)) {
 error_set(errp, QERR_PERMISSION_DENIED);
 return;
 }
@@ -291,7 +291,7 @@ static void set_uint64(Object *obj, Visitor *v, void 
*opaque,
 Property *prop = opaque;
 uint64_t *ptr = qdev_get_prop_ptr(dev, prop);
 
-if (dev->state != DEV_STATE_CREATED) {
+if (object_is_realized(obj)) {
 error_set(errp, QERR_PERMISSION_DENIED);
 return;
 }
@@ -379,7 +379,7 @@ static void set_string(Object *obj, Visitor *v, void 
*opaque,
 Error *local_err = NULL;
 char *str;
 
-if (dev->state != DEV_STATE_CREATED) {
+if (object_is_realized(obj)) {
 error_set(errp, QERR_PERMISSION_DENIED);
 return;
 }
@@ -457,7 +457,7 @@ static void set_pointer(Object *obj, Visitor *v, Property 
*prop,
 char *str;
 int ret;
 
-if (dev->state != DEV_STATE_CREATED) {
+if (object_is_realized(obj)) {
 error_set(errp, QERR_PERMISSION_DENIED);
 return;
 }
@@ -626,7 +626,7 @@ static void set_vlan(Object *obj, Visitor *v, void *opaque,
 int64_t id;
 VLANState *vlan;
 
-if (dev->state != DEV_STATE_CREATED) {
+if (object_is_realized(obj)) {
 error_set(errp, QERR_PERMISSION_DENIED);
 return;
 }
@@ -696,7 +696,7 @@ static void set_mac(Object *obj, Visitor *v, void *opaque,
 int i, pos;
 char *str, *p;
 
-if (dev->state != DEV_STATE_CREATED) {
+if (object_is_realized(obj)) {
 error_set(errp, QERR_PERMISSION_DENIED);
 return;
 }
@@ -764,7 +764,7 @@ static void set_enum(Object *obj, Visitor *v, void *opaque,
 Property *prop = opaque;
 int *ptr = qdev_get_prop_ptr(dev, prop);
 
-if (dev->state != DEV_STATE_CREATED) {
+if (object_is_realized(obj)) {
 error_set(errp, QERR_PERMISSION_DENIED);
 return;
 }
@@ -795,7 +795,7 @@ static void set_pci_devfn(Object *obj, Visitor *v, void 
*opaque,
 Error *local_err = NULL;
 char *str = (char *)"";
 
-if (dev->state != DEV_STATE_CREATED) {
+if (object_is_realized(obj)) {
 error_set(errp, QERR_PERMISSION_DENIED);
 return;
 }
@@ -861,7 +861,7 @@ static void set_blocksize(Object *obj, Visitor *v, void 
*opaque,
 uint16_t value, *ptr = qdev_get_prop_ptr(dev, prop);
 Error *local_err = NULL;
 
-if (d

[Qemu-devel] [PATCH 10/10] qom: Add QERR_PROPERTY_SET_AFTER_REALIZE

[Paolo Bonzini]

From: Peter Maydell 

Add a new QError QERR_PROPERTY_SET_AFTER_REALIZE for attempts
to set a QOM or qdev property after the object/device has been
realized. This allows a slightly more informative diagnostic
than the previous "Insufficient permission" message.

Signed-off-by: Peter Maydell 
Signed-off-by: Paolo Bonzini 
---
 hw/qdev-properties.c |   15 ++-
 qerror.c |4 
 qerror.h |3 +++
 qom/object.c |   30 --
 4 files changed, 37 insertions(+), 15 deletions(-)

diff --git a/hw/qdev-properties.c b/hw/qdev-properties.c
index 6766abc..29499bf 100644
--- a/hw/qdev-properties.c
+++ b/hw/qdev-properties.c
@@ -76,7 +76,8 @@ static void set_pointer(Object *obj, Visitor *v, Property 
*prop,
 int ret;
 
 if (object_is_realized(obj)) {
-error_set(errp, QERR_PERMISSION_DENIED);
+error_set(errp, QERR_PROPERTY_SET_AFTER_REALIZE,
+  object_get_id(obj), name);
 return;
 }
 
@@ -242,7 +243,8 @@ static void set_vlan(Object *obj, Visitor *v, void *opaque,
 VLANState *vlan;
 
 if (object_is_realized(obj)) {
-error_set(errp, QERR_PERMISSION_DENIED);
+error_set(errp, QERR_PROPERTY_SET_AFTER_REALIZE,
+  object_get_id(obj), name);
 return;
 }
 
@@ -303,7 +305,8 @@ static void set_mac(Object *obj, Visitor *v, void *opaque,
 char *str, *p;
 
 if (object_is_realized(obj)) {
-error_set(errp, QERR_PERMISSION_DENIED);
+error_set(errp, QERR_PROPERTY_SET_AFTER_REALIZE,
+  object_get_id(obj), name);
 return;
 }
 
@@ -389,7 +392,8 @@ static void set_pci_devfn(Object *obj, Visitor *v, void 
*opaque,
 char *str = (char *)"";
 
 if (object_is_realized(obj)) {
-error_set(errp, QERR_PERMISSION_DENIED);
+error_set(errp, QERR_PROPERTY_SET_AFTER_REALIZE,
+  object_get_id(obj), name);
 return;
 }
 
@@ -463,7 +467,8 @@ static void set_blocksize(Object *obj, Visitor *v, void 
*opaque,
 Error *local_err = NULL;
 
 if (object_is_realized(obj)) {
-error_set(errp, QERR_PERMISSION_DENIED);
+error_set(errp, QERR_PROPERTY_SET_AFTER_REALIZE,
+  object_get_id(obj), name);
 return;
 }
 
diff --git a/qerror.c b/qerror.c
index 5092fe7..860b3cb 100644
--- a/qerror.c
+++ b/qerror.c
@@ -229,6 +229,10 @@ static const QErrorStringTable qerror_table[] = {
 .desc  = "Property '%(device).%(property)' not found",
 },
 {
+.error_fmt = QERR_PROPERTY_SET_AFTER_REALIZE,
+.desc  = "Property '%(device).%(property)' cannot be set after 
realize",
+},
+{
 .error_fmt = QERR_PROPERTY_VALUE_BAD,
 .desc  = "Property '%(device).%(property)' doesn't take value 
'%(value)'",
 },
diff --git a/qerror.h b/qerror.h
index 4cbba48..6f7b1fb 100644
--- a/qerror.h
+++ b/qerror.h
@@ -193,6 +193,9 @@ QError *qobject_to_qerror(const QObject *obj);
 #define QERR_PROPERTY_NOT_FOUND \
 "{ 'class': 'PropertyNotFound', 'data': { 'device': %s, 'property': %s } }"
 
+#define QERR_PROPERTY_SET_AFTER_REALIZE \
+"{ 'class': 'PropertySetAfterRealize', 'data': { 'device': %s, 'property': 
%s } }"
+
 #define QERR_PROPERTY_VALUE_BAD \
 "{ 'class': 'PropertyValueBad', 'data': { 'device': %s, 'property': %s, 
'value': %s } }"
 
diff --git a/qom/object.c b/qom/object.c
index b113e66..e131366 100644
--- a/qom/object.c
+++ b/qom/object.c
@@ -809,7 +809,8 @@ void object_property_get(Object *obj, Visitor *v, const 
char *name,
 }
 
 if (!prop->get) {
-error_set(errp, QERR_PERMISSION_DENIED);
+error_set(errp, QERR_PROPERTY_SET_AFTER_REALIZE,
+  object_get_id(obj), name);
 } else {
 prop->get(obj, v, prop->opaque, name, errp);
 }
@@ -824,7 +825,8 @@ void object_property_set(Object *obj, Visitor *v, const 
char *name,
 }
 
 if (!prop->set) {
-error_set(errp, QERR_PERMISSION_DENIED);
+error_set(errp, QERR_PROPERTY_SET_AFTER_REALIZE,
+  object_get_id(obj), name);
 } else {
 prop->set(obj, v, prop->opaque, name, errp);
 }
@@ -1445,7 +1447,8 @@ static void set_bit(Object *obj, Visitor *v, void *opaque,
 bool value;
 
 if (object_is_realized(obj)) {
-error_set(errp, QERR_PERMISSION_DENIED);
+error_set(errp, QERR_PROPERTY_SET_AFTER_REALIZE,
+  object_get_id(obj), name);
 return;
 }
 
@@ -1483,7 +1486,8 @@ static void set_uint8(Object *obj, Visitor *v, void 
*opaque,
 uint8_t *ptr = object_get_prop_ptr(obj, prop);
 
 if (object_is_realized(obj)) {
-error_set(errp, QERR_PERMISSION_DENIED);
+error_set(errp, QERR_PROPERTY_SET_AFTER_REALIZE,
+  object_get_id(obj), name);
 return;
 }
 
@@ -1548,7 +1552,8 @@ static void set_uint16(Object *obj, Visitor *v, void 
*opaque,
 uint16_t *ptr = object_get

Re: [Qemu-devel] buildbot failure in qemu on default_openbsd_current

[Michael Roth]

On Wed, May 23, 2012 at 12:57:11PM -0300, Luiz Capitulino wrote:
> On Wed, 23 May 2012 11:35:49 -0300
> Luiz Capitulino  wrote:
> 
> > > Maybe we need a patch to declare environ for openbsd
> > 
> > Yes, I have the patch already but am installing openbsd on a VM to test it.
> 
> I'm getting lots of make errors on openbsd 4.9:
> 
> "Makefile", line 7: Missing dependency operator
> "/root/qemu.a/rules.mak", line 20: Missing dependency operator
> "/root/qemu.a/rules.mak", line 23: Need an operator
> "/root/qemu.a/rules.mak", line 26: Need an operator
> "Makefile", line 15: Need an operator
> "Makefile", line 19: Need an operator
> "Makefile", line 22: Missing dependency operator
> "Makefile", line 24: Need an operator
> Bad modifier: $(SRC_PATH)/hw)
> 
> Bad modifier: $(SRC_PATH)/hw)
> 
> "Makefile", line 36: Need an operator
> 
> Anyone knows what I did wrong? Or, if anybody could test the attached fix...

Had to nuke my openbsd VM a while back to free up space, but I vagulely recall
the fix for this being to use `gmake` instead of `make`

> From 4a3f4cff8aa27fe3810d621d20bf90f18ca8e2d5 Mon Sep 17 00:00:00 2001
> From: Luiz Capitulino 
> Date: Wed, 23 May 2012 11:33:51 -0300
> Subject: [PATCH] qemu-ga: Fix missing environ declarion
> 
> Commit 3674838cd05268954bb6473239cd7f700a79bf0f uses the environ
> global variable, but is relying on it to be declared somewhere else.
> 
> This works for Linux because _GNU_SOURCE declares it, but it brakes
> for system where _GNU_SOURCE is not declared, such as OpenBSD.
> 
> Fix it by declaring environ when _GNU_SOURCE is not defined.
> 
> Signed-off-by: Luiz Capitulino 
> ---
>  qga/commands-posix.c |5 +
>  1 file changed, 5 insertions(+)
> 
> diff --git a/qga/commands-posix.c b/qga/commands-posix.c
> index 7664be1..304ffa8 100644
> --- a/qga/commands-posix.c
> +++ b/qga/commands-posix.c
> @@ -12,6 +12,7 @@
>   */
> 
>  #include 
> +#include 
>  #include 
>  #include 
>  #include "qga/guest-agent-core.h"
> @@ -20,6 +21,10 @@
>  #include "qemu-queue.h"
>  #include "host-utils.h"
> 
> +#ifndef _GNU_SOURCE
> +extern char **environ;
> +#endif
> +
>  #if defined(__linux__)
>  #include 
>  #include 
> -- 
> 1.7.9.2.384.g4a92a
> 

Re: [Qemu-devel] [PATCH 1/2] sched: add virt sched domain for the guest

[Dave Hansen]

On 05/23/2012 01:48 AM, Peter Zijlstra wrote:
> On Wed, 2012-05-23 at 16:34 +0800, Liu ping fan wrote:
>> > so we need to migrate some of vcpus from node-B to node-A, or to
>> > node-C.
> This is absolutely broken, you cannot do that.
> 
> A guest task might want to be node affine, it looks at the topology sets
> a cpu affinity mask and expects to stay on that node.
> 
> But then you come along, and flip one of those cpus to another node. The
> guest task will now run on another node and get remote memory accesses.

Insane, sure.  But, if the node has physically gone away, what do we do?
 I think we've got to either kill the guest, or let it run somewhere
suboptimal.  Sounds like you're advocating killing it. ;)

[Qemu-devel] [PATCH 02/10] qdev: remove qdev_prop_exists

[Paolo Bonzini]

Can be replaced everywhere with object_property_find.

Signed-off-by: Paolo Bonzini 
---
 hw/qdev-properties.c  |5 -
 hw/qdev.c |2 +-
 hw/qdev.h |1 -
 hw/scsi-bus.c |2 +-
 include/qemu/object.h |9 +
 qom/object.c  |2 +-
 6 files changed, 12 insertions(+), 9 deletions(-)

diff --git a/hw/qdev-properties.c b/hw/qdev-properties.c
index af68ca5..eeb950d 100644
--- a/hw/qdev-properties.c
+++ b/hw/qdev-properties.c
@@ -925,11 +925,6 @@ static Property *qdev_prop_find(DeviceState *dev, const 
char *name)
 return NULL;
 }
 
-int qdev_prop_exists(DeviceState *dev, const char *name)
-{
-return qdev_prop_find(dev, name) ? true : false;
-}
-
 void error_set_from_qdev_prop_error(Error **errp, int ret, DeviceState *dev,
 Property *prop, const char *value)
 {
diff --git a/hw/qdev.c b/hw/qdev.c
index b5efb16..fc1dfbf 100644
--- a/hw/qdev.c
+++ b/hw/qdev.c
@@ -323,7 +323,7 @@ void qdev_set_nic_properties(DeviceState *dev, NICInfo *nd)
 if (nd->netdev)
 qdev_prop_set_netdev(dev, "netdev", nd->netdev);
 if (nd->nvectors != DEV_NVECTORS_UNSPECIFIED &&
-qdev_prop_exists(dev, "vectors")) {
+object_property_find(OBJECT(dev), "vectors")) {
 qdev_prop_set_uint32(dev, "vectors", nd->nvectors);
 }
 nd->instantiated = 1;
diff --git a/hw/qdev.h b/hw/qdev.h
index 224875d..212ca24 100644
--- a/hw/qdev.h
+++ b/hw/qdev.h
@@ -299,7 +299,6 @@ extern PropertyInfo qdev_prop_blocksize;
 
 /* Set properties between creation and init.  */
 void *qdev_get_prop_ptr(DeviceState *dev, Property *prop);
-int qdev_prop_exists(DeviceState *dev, const char *name);
 int qdev_prop_parse(DeviceState *dev, const char *name, const char *value);
 void qdev_prop_set_bit(DeviceState *dev, const char *name, bool value);
 void qdev_prop_set_uint8(DeviceState *dev, const char *name, uint8_t value);
diff --git a/hw/scsi-bus.c b/hw/scsi-bus.c
index 2bb19e9..bdb3ca5 100644
--- a/hw/scsi-bus.c
+++ b/hw/scsi-bus.c
@@ -214,7 +214,7 @@ SCSIDevice *scsi_bus_legacy_add_drive(SCSIBus *bus, 
BlockDriverState *bdrv,
 if (bootindex >= 0) {
 qdev_prop_set_int32(dev, "bootindex", bootindex);
 }
-if (qdev_prop_exists(dev, "removable")) {
+if (object_property_find(OBJECT(dev), "removable")) {
 qdev_prop_set_bit(dev, "removable", removable);
 }
 if (qdev_prop_set_drive(dev, "drive", bdrv) < 0) {
diff --git a/include/qemu/object.h b/include/qemu/object.h
index 5fd2270..b7efc63 100644
--- a/include/qemu/object.h
+++ b/include/qemu/object.h
@@ -636,6 +636,15 @@ void object_property_add(Object *obj, const char *name, 
const char *type,
 
 void object_property_del(Object *obj, const char *name, struct Error **errp);
 
+/**
+ * object_property_find:
+ * @obj: the object
+ * @name: the name of the property
+ *
+ * Look up a property for an object and return its #ObjectProperty if found.
+ */
+ObjectProperty *object_property_find(Object *obj, const char *name);
+
 void object_unparent(Object *obj);
 
 /**
diff --git a/qom/object.c b/qom/object.c
index 4b410f1..13fd157 100644
--- a/qom/object.c
+++ b/qom/object.c
@@ -662,7 +662,7 @@ void object_property_add(Object *obj, const char *name, 
const char *type,
 QTAILQ_INSERT_TAIL(&obj->properties, prop, node);
 }
 
-static ObjectProperty *object_property_find(Object *obj, const char *name)
+ObjectProperty *object_property_find(Object *obj, const char *name)
 {
 ObjectProperty *prop;
 
-- 
1.7.10.1

Re: [Qemu-devel] [PATCH 1/1 v4] Allow machines to configure the QEMU_VERSION that's exposed via hardware

[Eric Blake]

On 05/23/2012 10:08 AM, Crístian Viana wrote:

>> So when you posted the previous version of your patch it was pointed
>> out that this is a buffer overflow:
>> http://lists.gnu.org/archive/html/qemu-devel/2012-04/msg01657.html
>>
>> You need to fix this.
> 
> I have sent a reply to that thread explaining that the user actually
> doesn't have control of that string, that is only used internally in the
> code (just like the QEMU_VERSION macro).
> I fixed the code now with snprintf copying at most 12 chars to the
> string (the array size). I can't think of why pstrcat would be better in
> this case, as suggested by Erik.

s/Erik/Eric/, but you're not the first to make that typo.

pstrcat is more efficient than snprintf() - the former is dedicated to a
single task, while the latter has to parse a format string and decode
that it is doing a single %s expansion.  In other words, just because
*printf can do string concatenation doesn't make it the best tool for
the job.

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



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] buildbot failure in qemu on default_openbsd_current

[Luiz Capitulino]

On Wed, 23 May 2012 18:06:18 +0200
Paolo Bonzini  wrote:

> Il 23/05/2012 17:57, Luiz Capitulino ha scritto:
> > On Wed, 23 May 2012 11:35:49 -0300
> > Luiz Capitulino  wrote:
> > 
> >>> Maybe we need a patch to declare environ for openbsd
> >>
> >> Yes, I have the patch already but am installing openbsd on a VM to test it.
> > 
> > I'm getting lots of make errors on openbsd 4.9:
> > 
> > "Makefile", line 7: Missing dependency operator
> > "/root/qemu.a/rules.mak", line 20: Missing dependency operator
> > "/root/qemu.a/rules.mak", line 23: Need an operator
> > "/root/qemu.a/rules.mak", line 26: Need an operator
> > "Makefile", line 15: Need an operator
> > "Makefile", line 19: Need an operator
> > "Makefile", line 22: Missing dependency operator
> > "Makefile", line 24: Need an operator
> > Bad modifier: $(SRC_PATH)/hw)
> > 
> > Bad modifier: $(SRC_PATH)/hw)
> > 
> > "Makefile", line 36: Need an operator
> > 
> > Anyone knows what I did wrong? Or, if anybody could test the attached fix...
> 
> Probably you need to install gnumake.

Yeah, Andreas helped by irc, but now I get zillions of -Wno-redundant-decls 
errors:

In file included from /root/qemu.a/qemu-common.h:33,
 from /root/qemu.a/module.c:16:
/usr//include/unistd.h:99: warning: redundant redeclaration of 'lseek'
/usr//include/sys/types.h:210: warning: previous declaration of 'lseek' was here
/usr//include/unistd.h:138: warning: redundant redeclaration of 'ftruncate'
/usr//include/sys/types.h:211: warning: previous declaration of 'ftruncate' was 
here

I've removed -Wno-redundant-decls and I finally get the environ build error,
but I wonder why build bot doesn't get it.

Re: [Qemu-devel] [PATCH 1/1 v4] Allow machines to configure the QEMU_VERSION that's exposed via hardware

[Crístian Viana]

Hi Peter,

Thanks for all your tips!


OK, this has been bugging me for the last three versions, and
since I'm complaining about other things anyway: can you reword
this commit message, please, so that it is a standalone paragraph
explaining (a) what the commit does and (b) why it is doing it, rather
than being a combination of an unattributed quote from Anthony and some
new text from you. Commit messages aren't emails.
Explanatory remarks like the URL to the previous discussion can go
below the '---' line where they won't appear in the formal git commit
message.


Ok, I rewrote the commit message.


When you're posting a new version of a patch please include a
summary of changes since the previous version below the '---'
line.


Ok.


So when you posted the previous version of your patch it was pointed
out that this is a buffer overflow:
http://lists.gnu.org/archive/html/qemu-devel/2012-04/msg01657.html

You need to fix this.


I have sent a reply to that thread explaining that the user actually 
doesn't have control of that string, that is only used internally in the 
code (just like the QEMU_VERSION macro).
I fixed the code now with snprintf copying at most 12 chars to the 
string (the array size). I can't think of why pstrcat would be better in 
this case, as suggested by Erik.



The questions about the usb-redir version still apply too:
  http://lists.gnu.org/archive/html/qemu-devel/2012-04/msg01700.html


I have also sent a reply to that thread asking if the usb-redir version 
appears to the guest but I haven't got no answer.



Don't just leave things unchanged from previous versions that were objected
to without explicitly mentioning them in the below-the-'---' commentary (ie
explaining why you decided not to change them), please. Otherwise reviewers
have to go through the whole thing with a fine tooth comb rechecking whether
you've actually fixed anything.
I didn't know there was "patch changelog protocol" here, I'll do it from 
now on.


Best regards,
Crístian.

Re: [Qemu-devel] buildbot failure in qemu on default_openbsd_current

[Paolo Bonzini]

Il 23/05/2012 17:57, Luiz Capitulino ha scritto:
> On Wed, 23 May 2012 11:35:49 -0300
> Luiz Capitulino  wrote:
> 
>>> Maybe we need a patch to declare environ for openbsd
>>
>> Yes, I have the patch already but am installing openbsd on a VM to test it.
> 
> I'm getting lots of make errors on openbsd 4.9:
> 
> "Makefile", line 7: Missing dependency operator
> "/root/qemu.a/rules.mak", line 20: Missing dependency operator
> "/root/qemu.a/rules.mak", line 23: Need an operator
> "/root/qemu.a/rules.mak", line 26: Need an operator
> "Makefile", line 15: Need an operator
> "Makefile", line 19: Need an operator
> "Makefile", line 22: Missing dependency operator
> "Makefile", line 24: Need an operator
> Bad modifier: $(SRC_PATH)/hw)
> 
> Bad modifier: $(SRC_PATH)/hw)
> 
> "Makefile", line 36: Need an operator
> 
> Anyone knows what I did wrong? Or, if anybody could test the attached fix...

Probably you need to install gnumake.

Paolo

Re: [Qemu-devel] [PATCH 1.1] virtio: Fix compiler warning for non Linux hosts

[Stefan Weil]

Am 23.05.2012 17:32, schrieb Kevin Wolf:

Am 23.05.2012 17:29, schrieb Stefan Weil:

Am 23.05.2012 10:09, schrieb Stefan Hajnoczi:

On Tue, May 22, 2012 at 10:23 PM, Stefan Weil   wrote:

The local variables ret, i are only used if __linux__ is defined.

Signed-off-by: Stefan Weil
---
   hw/virtio-blk.c |4 +++-
   1 file changed, 3 insertions(+), 1 deletion(-)

The #ifdef __linux__ further down in the function declares the local
hdr variable.  We could move ret and i down into the #ifdef instead of
adding a new one.

I noticed that, but declaring variables anywhere is C++, not C code.

It's called C99.

Kevin



Maybe, but I already had patches rejected because of that style.
Did this policy change? I'd appreciate that!

Stefan

Re: [Qemu-devel] [PATCH] exec: fix breakpoint_invalidate() breakage

[Jan Kiszka]

On 2012-05-23 11:11, TeLeMan wrote:
> On Wed, May 23, 2012 at 7:22 PM, Jan Kiszka  wrote:
>> On 2012-05-23 04:09, TeLeMan wrote:
>>> On Wed, May 23, 2012 at 11:41 AM, Andreas Färber  wrote:
 Am 18.05.2012 11:49, schrieb TeLeMan:
> This breakage was introduced by the commit "memory: make
> phys_page_find() return an unadjusted".

 You seem to have found the origin of your problem. If you also mention
 the commit hash in your commit message then certain frontends (gitk,
 repo.or.cz) will display it as a handy hyperlink to that commit.

>
> Signed-off-by: TeLeMan 

 Signed-off-by is a legal statement of origin and must not be a pseudonym.
>>> Ok, please ignore this patch. I won't submit any patch just report bugs.
>>
>> Then please describe this bug in more details, e.g. how to reproduce.
> I think its evident. cpu_get_phys_page_debug(env, pc) is not the
> physical address of pc but the physical page base address of pc.

...so this bites us if the instruction spans two pages as
tb_invalidate_phys_addr requests invalidation on a page granularity.
Such information would have been helpful to understand when it actually
breaks - not in the common case.

Thanks,
Jan

-- 
Siemens AG, Corporate Technology, CT T DE IT 1
Corporate Competence Center Embedded Linux

Re: [Qemu-devel] buildbot failure in qemu on default_openbsd_current

[Eric Blake]

On 05/23/2012 09:57 AM, Luiz Capitulino wrote:

> 
> Anyone knows what I did wrong? Or, if anybody could test the attached fix...

Sorry, I'm not in a position to test the patch, but I can at least
review it.

> 
> 
> 0001-qemu-ga-Fix-missing-environ-declarion.patch
> 
> 
>>From 4a3f4cff8aa27fe3810d621d20bf90f18ca8e2d5 Mon Sep 17 00:00:00 2001
> From: Luiz Capitulino 
> Date: Wed, 23 May 2012 11:33:51 -0300
> Subject: [PATCH] qemu-ga: Fix missing environ declarion

s/declarion/declaration/

> 
> Commit 3674838cd05268954bb6473239cd7f700a79bf0f uses the environ
> global variable, but is relying on it to be declared somewhere else.
> 
> This works for Linux because _GNU_SOURCE declares it, but it brakes

s/brakes/breaks/

> for system where _GNU_SOURCE is not declared, such as OpenBSD.
> 
> Fix it by declaring environ when _GNU_SOURCE is not defined.
> 
> Signed-off-by: Luiz Capitulino 
> ---
>  qga/commands-posix.c |5 +
>  1 file changed, 5 insertions(+)
> 
> diff --git a/qga/commands-posix.c b/qga/commands-posix.c
> index 7664be1..304ffa8 100644
> --- a/qga/commands-posix.c
> +++ b/qga/commands-posix.c
> @@ -12,6 +12,7 @@
>   */
>  
>  #include 
> +#include 
>  #include 
>  #include 
>  #include "qga/guest-agent-core.h"
> @@ -20,6 +21,10 @@
>  #include "qemu-queue.h"
>  #include "host-utils.h"
>  
> +#ifndef _GNU_SOURCE
> +extern char **environ;
> +#endif

Looks reasonable.

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



signature.asc
Description: OpenPGP digital signature

[Qemu-devel] [PATCH v2 08/15] net: Remove VLANState

[zwu . kernel]

From: Stefan Hajnoczi 

VLANState is no longer used and can be removed.

Signed-off-by: Stefan Hajnoczi 
Signed-off-by: Zhi Yong Wu 
---
 net.c |  127 ++---
 net.h |8 
 net/socket.c  |6 +-
 net/tap.c |6 +-
 net/tap.h |2 +-
 qemu-common.h |1 -
 6 files changed, 29 insertions(+), 121 deletions(-)

diff --git a/net.c b/net.c
index abf5a3d..eb2ad06 100644
--- a/net.c
+++ b/net.c
@@ -44,7 +44,6 @@
 # define CONFIG_NET_BRIDGE
 #endif
 
-static QTAILQ_HEAD(, VLANState) vlans;
 static QTAILQ_HEAD(, VLANClientState) non_vlan_clients;
 
 int default_net = 1;
@@ -249,11 +248,7 @@ NICState *qemu_new_nic(NetClientInfo *info,
 
 static void qemu_cleanup_vlan_client(VLANClientState *vc)
 {
-if (vc->vlan) {
-QTAILQ_REMOVE(&vc->vlan->clients, vc, next);
-} else {
-QTAILQ_REMOVE(&non_vlan_clients, vc, next);
-}
+QTAILQ_REMOVE(&non_vlan_clients, vc, next);
 
 if (vc->info->cleanup) {
 vc->info->cleanup(vc);
@@ -262,13 +257,11 @@ static void qemu_cleanup_vlan_client(VLANClientState *vc)
 
 static void qemu_free_vlan_client(VLANClientState *vc)
 {
-if (!vc->vlan) {
-if (vc->send_queue) {
-qemu_del_net_queue(vc->send_queue);
-}
-if (vc->peer) {
-vc->peer->peer = NULL;
-}
+if (vc->send_queue) {
+qemu_del_net_queue(vc->send_queue);
+}
+if (vc->peer) {
+vc->peer->peer = NULL;
 }
 g_free(vc->name);
 g_free(vc->model);
@@ -278,7 +271,7 @@ static void qemu_free_vlan_client(VLANClientState *vc)
 void qemu_del_vlan_client(VLANClientState *vc)
 {
 /* If there is a peer NIC, delete and cleanup client, but do not free. */
-if (!vc->vlan && vc->peer && vc->peer->info->type == NET_CLIENT_TYPE_NIC) {
+if (vc->peer && vc->peer->info->type == NET_CLIENT_TYPE_NIC) {
 NICState *nic = DO_UPCAST(NICState, nc, vc->peer);
 if (nic->peer_deleted) {
 return;
@@ -294,7 +287,7 @@ void qemu_del_vlan_client(VLANClientState *vc)
 }
 
 /* If this is a peer NIC and peer has already been deleted, free it now. */
-if (!vc->vlan && vc->peer && vc->info->type == NET_CLIENT_TYPE_NIC) {
+if (vc->peer && vc->info->type == NET_CLIENT_TYPE_NIC) {
 NICState *nic = DO_UPCAST(NICState, nc, vc);
 if (nic->peer_deleted) {
 qemu_free_vlan_client(vc->peer);
@@ -308,52 +301,25 @@ void qemu_del_vlan_client(VLANClientState *vc)
 void qemu_foreach_nic(qemu_nic_foreach func, void *opaque)
 {
 VLANClientState *nc;
-VLANState *vlan;
 
 QTAILQ_FOREACH(nc, &non_vlan_clients, next) {
 if (nc->info->type == NET_CLIENT_TYPE_NIC) {
 func(DO_UPCAST(NICState, nc, nc), opaque);
 }
 }
-
-QTAILQ_FOREACH(vlan, &vlans, next) {
-QTAILQ_FOREACH(nc, &vlan->clients, next) {
-if (nc->info->type == NET_CLIENT_TYPE_NIC) {
-func(DO_UPCAST(NICState, nc, nc), opaque);
-}
-}
-}
 }
 
 int qemu_can_send_packet(VLANClientState *sender)
 {
-VLANState *vlan = sender->vlan;
-VLANClientState *vc;
-
-if (sender->peer) {
-if (sender->peer->receive_disabled) {
-return 0;
-} else if (sender->peer->info->can_receive &&
-   !sender->peer->info->can_receive(sender->peer)) {
-return 0;
-} else {
-return 1;
-}
-}
-
-if (!sender->vlan) {
+if (!sender->peer) {
 return 1;
 }
 
-QTAILQ_FOREACH(vc, &vlan->clients, next) {
-if (vc == sender) {
-continue;
-}
-
-/* no can_receive() handler, they can always receive */
-if (vc->info->can_receive && !vc->info->can_receive(vc)) {
-return 0;
-}
+if (sender->peer->receive_disabled) {
+return 0;
+} else if (sender->peer->info->can_receive &&
+   !sender->peer->info->can_receive(sender->peer)) {
+return 0;
 }
 return 1;
 }
@@ -390,34 +356,18 @@ static ssize_t qemu_deliver_packet(VLANClientState 
*sender,
 
 void qemu_purge_queued_packets(VLANClientState *vc)
 {
-NetQueue *queue;
-
-if (!vc->peer && !vc->vlan) {
+if (!vc->peer) {
 return;
 }
 
-if (vc->peer) {
-queue = vc->peer->send_queue;
-} else {
-queue = vc->vlan->send_queue;
-}
-
-qemu_net_queue_purge(queue, vc);
+qemu_net_queue_purge(vc->peer->send_queue, vc);
 }
 
 void qemu_flush_queued_packets(VLANClientState *vc)
 {
-NetQueue *queue;
-
 vc->receive_disabled = 0;
 
-if (vc->vlan) {
-queue = vc->vlan->send_queue;
-} else {
-queue = vc->send_queue;
-}
-
-qemu_net_queue_flush(queue);
+qemu_net_queue_flush(vc->send_queue);
 }
 
 static ssize_t qemu_send_packet_async_with_flags(VLANClientState *sender,
@@ -432,15 +382,11 @@ static ssize_t 
qemu_send_packet_async_with_flags(V

Re: [Qemu-devel] How to create new target port?

[Stefan Weil]

Am 23.05.2012 16:37, schrieb Michael Eager:

On 05/22/2012 11:18 PM, 陳韋任 wrote:

I'm investigating adding a new target architecture
to QEMU.  Are there documents, how-to's, or other
guidance on how to approach this?  Or any advice?

I noticed that there are a number of directories for
architectures like target-arm and target-mips.  There
are also definitions under tcg for arm and mips.  I
noticed that target-microblaze exists, but there is
no microblaze directory under tcg.  What does this
mean?


   Depends on what you'd like to add, a guest or a host support. If 
you want to
add a new guest, take target-xxx/* as an example. Otherwise, looks at 
tcg/xxx/*.

The term "target" could be a little MISLEADING here. :)


I'm interested in adding a new emulated architecture,
not a new host.  So adding a new target- sounds
like the plan.


Yes, that's the place for new target architectures.

Which architecture are you thinking of? Maybe someone else
is already working on it. http://wiki.qemu.org/Links has an
incomplete list of links to unofficial versions of QEMU which
support additional targets.

Regards,
Stefan Weil

[Qemu-devel] [PATCH v2 05/15] net: Drop vlan argument to qemu_new_net_client()

[zwu . kernel]

From: Stefan Hajnoczi 

Since hubs are now used to implement the 'vlan' feature and the vlan
argument is always NULL, remove the argument entirely and update all net
clients that use qemu_new_net_client().

Signed-off-by: Stefan Hajnoczi 
Signed-off-by: Zhi Yong Wu 
---
 net.c   |   27 ++-
 net.h   |1 -
 net/dump.c  |2 +-
 net/hub.c   |2 +-
 net/slirp.c |2 +-
 net/socket.c|4 ++--
 net/tap-win32.c |2 +-
 net/tap.c   |2 +-
 net/vde.c   |2 +-
 9 files changed, 18 insertions(+), 26 deletions(-)

diff --git a/net.c b/net.c
index 88b9e1f..96252f9 100644
--- a/net.c
+++ b/net.c
@@ -194,7 +194,6 @@ static ssize_t qemu_deliver_packet_iov(VLANClientState 
*sender,
void *opaque);
 
 VLANClientState *qemu_new_net_client(NetClientInfo *info,
- VLANState *vlan,
  VLANClientState *peer,
  const char *model,
  const char *name)
@@ -213,22 +212,16 @@ VLANClientState *qemu_new_net_client(NetClientInfo *info,
 vc->name = assign_name(vc, model);
 }
 
-if (vlan) {
-assert(!peer);
-vc->vlan = vlan;
-QTAILQ_INSERT_TAIL(&vc->vlan->clients, vc, next);
-} else {
-if (peer) {
-assert(!peer->peer);
-vc->peer = peer;
-peer->peer = vc;
-}
-QTAILQ_INSERT_TAIL(&non_vlan_clients, vc, next);
-
-vc->send_queue = qemu_new_net_queue(qemu_deliver_packet,
-qemu_deliver_packet_iov,
-vc);
+if (peer) {
+assert(!peer->peer);
+vc->peer = peer;
+peer->peer = vc;
 }
+QTAILQ_INSERT_TAIL(&non_vlan_clients, vc, next);
+
+vc->send_queue = qemu_new_net_queue(qemu_deliver_packet,
+qemu_deliver_packet_iov,
+vc);
 
 return vc;
 }
@@ -245,7 +238,7 @@ NICState *qemu_new_nic(NetClientInfo *info,
 assert(info->type == NET_CLIENT_TYPE_NIC);
 assert(info->size >= sizeof(NICState));
 
-nc = qemu_new_net_client(info, conf->vlan, conf->peer, model, name);
+nc = qemu_new_net_client(info, conf->peer, model, name);
 
 nic = DO_UPCAST(NICState, nc, nc);
 nic->conf = conf;
diff --git a/net.h b/net.h
index 50c55ad..d3d6e4c 100644
--- a/net.h
+++ b/net.h
@@ -92,7 +92,6 @@ struct VLANState {
 VLANState *qemu_find_vlan(int id, int allocate);
 VLANClientState *qemu_find_netdev(const char *id);
 VLANClientState *qemu_new_net_client(NetClientInfo *info,
- VLANState *vlan,
  VLANClientState *peer,
  const char *model,
  const char *name);
diff --git a/net/dump.c b/net/dump.c
index 37cec3c..621f4e7 100644
--- a/net/dump.c
+++ b/net/dump.c
@@ -129,7 +129,7 @@ static int net_dump_init(VLANClientState *peer, const char 
*device,
 return -1;
 }
 
-nc = qemu_new_net_client(&net_dump_info, NULL, peer, device, name);
+nc = qemu_new_net_client(&net_dump_info, peer, device, name);
 
 snprintf(nc->info_str, sizeof(nc->info_str),
  "dump to %s (len=%d)", filename, len);
diff --git a/net/hub.c b/net/hub.c
index 19c1169..af3de9c 100644
--- a/net/hub.c
+++ b/net/hub.c
@@ -124,7 +124,7 @@ static NetHubPort *net_hub_port_new(NetHub *hub)
 
 snprintf(name, sizeof name, "hub%uport%u", hub->id, id);
 
-nc = qemu_new_net_client(&net_hub_port_info, NULL, NULL, "hub", name);
+nc = qemu_new_net_client(&net_hub_port_info, NULL, "hub", name);
 port = DO_UPCAST(NetHubPort, nc, nc);
 port->id = id;
 port->hub = hub;
diff --git a/net/slirp.c b/net/slirp.c
index edb4621..5ed7036 100644
--- a/net/slirp.c
+++ b/net/slirp.c
@@ -238,7 +238,7 @@ static int net_slirp_init(VLANClientState *peer, const char 
*model,
 }
 #endif
 
-nc = qemu_new_net_client(&net_slirp_info, NULL, peer, model, name);
+nc = qemu_new_net_client(&net_slirp_info, peer, model, name);
 
 snprintf(nc->info_str, sizeof(nc->info_str),
  "net=%s,restrict=%s", inet_ntoa(net),
diff --git a/net/socket.c b/net/socket.c
index ed28cbd..bf7a793 100644
--- a/net/socket.c
+++ b/net/socket.c
@@ -286,7 +286,7 @@ static NetSocketState 
*net_socket_fd_init_dgram(VLANClientState *peer,
 }
 }
 
-nc = qemu_new_net_client(&net_dgram_socket_info, NULL, peer, model, name);
+nc = qemu_new_net_client(&net_dgram_socket_info, peer, model, name);
 
 snprintf(nc->info_str, sizeof(nc->info_str),
 "socket: fd=%d (%s mcast=%s:%d)",
@@ -330,7 +330,7 @@ static NetSocketState 
*net_socket_fd_init_stream(VLANClientState *peer,
 VLANClientState *nc;
 NetSocketState *s;
 
-nc = qemu_new_net_

Re: [Qemu-devel] buildbot failure in qemu on default_openbsd_current

[Luiz Capitulino]

On Wed, 23 May 2012 11:35:49 -0300
Luiz Capitulino  wrote:

> > Maybe we need a patch to declare environ for openbsd
> 
> Yes, I have the patch already but am installing openbsd on a VM to test it.

I'm getting lots of make errors on openbsd 4.9:

"Makefile", line 7: Missing dependency operator
"/root/qemu.a/rules.mak", line 20: Missing dependency operator
"/root/qemu.a/rules.mak", line 23: Need an operator
"/root/qemu.a/rules.mak", line 26: Need an operator
"Makefile", line 15: Need an operator
"Makefile", line 19: Need an operator
"Makefile", line 22: Missing dependency operator
"Makefile", line 24: Need an operator
Bad modifier: $(SRC_PATH)/hw)

Bad modifier: $(SRC_PATH)/hw)

"Makefile", line 36: Need an operator

Anyone knows what I did wrong? Or, if anybody could test the attached fix...
>From 4a3f4cff8aa27fe3810d621d20bf90f18ca8e2d5 Mon Sep 17 00:00:00 2001
From: Luiz Capitulino 
Date: Wed, 23 May 2012 11:33:51 -0300
Subject: [PATCH] qemu-ga: Fix missing environ declarion

Commit 3674838cd05268954bb6473239cd7f700a79bf0f uses the environ
global variable, but is relying on it to be declared somewhere else.

This works for Linux because _GNU_SOURCE declares it, but it brakes
for system where _GNU_SOURCE is not declared, such as OpenBSD.

Fix it by declaring environ when _GNU_SOURCE is not defined.

Signed-off-by: Luiz Capitulino 
---
 qga/commands-posix.c |5 +
 1 file changed, 5 insertions(+)

diff --git a/qga/commands-posix.c b/qga/commands-posix.c
index 7664be1..304ffa8 100644
--- a/qga/commands-posix.c
+++ b/qga/commands-posix.c
@@ -12,6 +12,7 @@
  */
 
 #include 
+#include 
 #include 
 #include 
 #include "qga/guest-agent-core.h"
@@ -20,6 +21,10 @@
 #include "qemu-queue.h"
 #include "host-utils.h"
 
+#ifndef _GNU_SOURCE
+extern char **environ;
+#endif
+
 #if defined(__linux__)
 #include 
 #include 
-- 
1.7.9.2.384.g4a92a

[Qemu-devel] [PATCH 04/10] qom: add get_id

[Paolo Bonzini]

Some classes may present objects differently in errors, for example if they
are not part of the composition tree or if they are not assigned an id by
the user.  Let them do this with a get_id method on Object, and use the
method consistently where a %(device) appears in the error.

Signed-off-by: Paolo Bonzini 
---
 hw/qdev-properties.c  |6 +++---
 hw/qdev.c |   15 ++-
 include/qemu/object.h |   11 +++
 qom/object.c  |   26 +-
 4 files changed, 53 insertions(+), 5 deletions(-)

diff --git a/hw/qdev-properties.c b/hw/qdev-properties.c
index eeb950d..9a6c04a 100644
--- a/hw/qdev-properties.c
+++ b/hw/qdev-properties.c
@@ -931,16 +931,16 @@ void error_set_from_qdev_prop_error(Error **errp, int 
ret, DeviceState *dev,
 switch (ret) {
 case -EEXIST:
 error_set(errp, QERR_PROPERTY_VALUE_IN_USE,
-  object_get_typename(OBJECT(dev)), prop->name, value);
+  object_get_id(OBJECT(dev)), prop->name, value);
 break;
 default:
 case -EINVAL:
 error_set(errp, QERR_PROPERTY_VALUE_BAD,
-  object_get_typename(OBJECT(dev)), prop->name, value);
+  object_get_id(OBJECT(dev)), prop->name, value);
 break;
 case -ENOENT:
 error_set(errp, QERR_PROPERTY_VALUE_NOT_FOUND,
-  object_get_typename(OBJECT(dev)), prop->name, value);
+  object_get_id(OBJECT(dev)), prop->name, value);
 break;
 case 0:
 break;
diff --git a/hw/qdev.c b/hw/qdev.c
index e909f3b..5d6dc1f 100644
--- a/hw/qdev.c
+++ b/hw/qdev.c
@@ -259,7 +259,7 @@ void qdev_init_nofail(DeviceState *dev)
 {
 if (qdev_init(dev) < 0) {
 error_report("Initialization of device %s failed",
- object_get_typename(OBJECT(dev)));
+ object_get_id(OBJECT(dev)));
 exit(1);
 }
 }
@@ -714,6 +714,13 @@ static void device_finalize(Object *obj)
 }
 }
 
+static const char *qdev_get_id(Object *obj)
+{
+DeviceState *dev = DEVICE(obj);
+
+return dev->id?:object_get_typename(obj);
+}
+
 static void device_class_base_init(ObjectClass *class, void *data)
 {
 DeviceClass *klass = DEVICE_CLASS(class);
@@ -744,6 +751,11 @@ Object *qdev_get_machine(void)
 return dev;
 }
 
+static void device_class_init(ObjectClass *class, void *data)
+{
+class->get_id = qdev_get_id;
+}
+
 static TypeInfo device_type_info = {
 .name = TYPE_DEVICE,
 .parent = TYPE_OBJECT,
@@ -751,6 +763,7 @@ static TypeInfo device_type_info = {
 .instance_init = device_initfn,
 .instance_finalize = device_finalize,
 .class_base_init = device_class_base_init,
+.class_init = device_class_init,
 .abstract = true,
 .class_size = sizeof(DeviceClass),
 };
diff --git a/include/qemu/object.h b/include/qemu/object.h
index e714c2c..cb08cfa 100644
--- a/include/qemu/object.h
+++ b/include/qemu/object.h
@@ -239,6 +239,9 @@ struct ObjectClass
 {
 /*< private >*/
 Type type;
+
+/*< public >*/
+const char *(*get_id)(Object *);
 };
 
 /**
@@ -501,6 +504,14 @@ Object *object_dynamic_cast(Object *obj, const char 
*typename);
 Object *object_dynamic_cast_assert(Object *obj, const char *typename);
 
 /**
+ * object_get_id:
+ * @obj: A derivative of #Object
+ *
+ * Returns: A string that can be used to refer to @obj.
+ */
+const char *object_get_id(Object *obj);
+
+/**
  * object_get_class:
  * @obj: A derivative of #Object
  *
diff --git a/qom/object.c b/qom/object.c
index 68a4c57..b19ef94 100644
--- a/qom/object.c
+++ b/qom/object.c
@@ -334,6 +334,24 @@ static void object_property_del_child(Object *obj, Object 
*child, Error **errp)
 }
 }
 
+static const char *_object_get_id(Object *obj)
+{
+ObjectProperty *prop;
+
+QTAILQ_FOREACH(prop, &obj->properties, node) {
+if (strstart(prop->type, "child<", NULL) && prop->opaque == obj) {
+return prop->name;
+}
+}
+
+return "";
+}
+
+const char *object_get_id(Object *obj)
+{
+return obj->class->get_id(obj);
+}
+
 void object_unparent(Object *obj)
 {
 if (obj->parent) {
@@ -672,7 +690,7 @@ ObjectProperty *object_property_find(Object *obj, const 
char *name, Error **errp
 }
 }
 
-error_set(errp, QERR_PROPERTY_NOT_FOUND, "", name);
+error_set(errp, QERR_PROPERTY_NOT_FOUND, object_get_id(obj), name);
 return NULL;
 }
 
@@ -1236,6 +1254,11 @@ static void object_instance_init(Object *obj)
 object_property_add_str(obj, "type", qdev_get_type, NULL, NULL);
 }
 
+static void object_class_init(ObjectClass *klass, void *class_data)
+{
+klass->get_id = _object_get_id;
+}
+
 static void register_types(void)
 {
 static TypeInfo interface_info = {
@@ -1248,6 +1271,7 @@ static void register_types(void)
 .name = TYPE_OBJECT,
 .instance_size = sizeof(Object),
 .instance_init = object_instance_init,
+.class_init = object_class_init,
 .abstra

[Qemu-devel] [PATCH v2 15/15] net: invoke qemu_can_send_packet only before net queue sending function

[zwu . kernel]

From: Zhi Yong Wu 

Signed-off-by: Zhi Yong Wu 
---
 net/queue.c  |4 ++--
 net/slirp.c  |7 ---
 net/tap.c|2 +-
 slirp/if.c   |5 -
 slirp/libslirp.h |1 -
 5 files changed, 3 insertions(+), 16 deletions(-)

diff --git a/net/queue.c b/net/queue.c
index 0afd783..d2e57de 100644
--- a/net/queue.c
+++ b/net/queue.c
@@ -176,7 +176,7 @@ ssize_t qemu_net_queue_send(NetQueue *queue,
 {
 ssize_t ret;
 
-if (queue->delivering) {
+if (queue->delivering || !qemu_can_send_packet(sender)) {
 return qemu_net_queue_append(queue, sender, flags, data, size, NULL);
 }
 
@@ -200,7 +200,7 @@ ssize_t qemu_net_queue_send_iov(NetQueue *queue,
 {
 ssize_t ret;
 
-if (queue->delivering) {
+if (queue->delivering || !qemu_can_send_packet(sender)) {
 return qemu_net_queue_append_iov(queue, sender, flags, iov, iovcnt, 
NULL);
 }
 
diff --git a/net/slirp.c b/net/slirp.c
index a6ede2b..248f7ff 100644
--- a/net/slirp.c
+++ b/net/slirp.c
@@ -96,13 +96,6 @@ static void slirp_smb_cleanup(SlirpState *s);
 static inline void slirp_smb_cleanup(SlirpState *s) { }
 #endif
 
-int slirp_can_output(void *opaque)
-{
-SlirpState *s = opaque;
-
-return qemu_can_send_packet(&s->nc);
-}
-
 void slirp_output(void *opaque, const uint8_t *pkt, int pkt_len)
 {
 SlirpState *s = opaque;
diff --git a/net/tap.c b/net/tap.c
index 65f45b8..7b1992b 100644
--- a/net/tap.c
+++ b/net/tap.c
@@ -210,7 +210,7 @@ static void tap_send(void *opaque)
 if (size == 0) {
 tap_read_poll(s, 0);
 }
-} while (size > 0 && qemu_can_send_packet(&s->nc));
+} while (size > 0);
 }
 
 int tap_has_ufo(NetClientState *nc)
diff --git a/slirp/if.c b/slirp/if.c
index 096cf6f..533295d 100644
--- a/slirp/if.c
+++ b/slirp/if.c
@@ -177,11 +177,6 @@ void if_start(Slirp *slirp)
 }
 
 while (ifm_next) {
-/* check if we can really output */
-if (!slirp_can_output(slirp->opaque)) {
-break;
-}
-
 ifm = ifm_next;
 from_batchq = next_from_batchq;
 
diff --git a/slirp/libslirp.h b/slirp/libslirp.h
index 77527ad..9b471b5 100644
--- a/slirp/libslirp.h
+++ b/slirp/libslirp.h
@@ -25,7 +25,6 @@ void slirp_select_poll(fd_set *readfds, fd_set *writefds, 
fd_set *xfds,
 void slirp_input(Slirp *slirp, const uint8_t *pkt, int pkt_len);
 
 /* you must provide the following functions: */
-int slirp_can_output(void *opaque);
 void slirp_output(void *opaque, const uint8_t *pkt, int pkt_len);
 
 int slirp_add_hostfwd(Slirp *slirp, int is_udp,
-- 
1.7.6

Re: [Qemu-devel] [PATCH 1/2] sched: add virt sched domain for the guest

[Peter Zijlstra]

On Wed, 2012-05-23 at 08:23 -0700, Dave Hansen wrote:
> On 05/23/2012 01:48 AM, Peter Zijlstra wrote:
> > On Wed, 2012-05-23 at 16:34 +0800, Liu ping fan wrote:
> >> > so we need to migrate some of vcpus from node-B to node-A, or to
> >> > node-C.
> > This is absolutely broken, you cannot do that.
> > 
> > A guest task might want to be node affine, it looks at the topology sets
> > a cpu affinity mask and expects to stay on that node.
> > 
> > But then you come along, and flip one of those cpus to another node. The
> > guest task will now run on another node and get remote memory accesses.
> 
> Insane, sure.  But, if the node has physically gone away, what do we do?
>  I think we've got to either kill the guest, or let it run somewhere
> suboptimal.  Sounds like you're advocating killing it. ;)

You all seem terribly confused. If you want a guest that 100% mirrors
the host topology you need hard-binding of all vcpu threads and clearly
you're in trouble if you unplug a host cpu while there's still a vcpu
expecting to run there.

That's an administrator error and you get to keep the pieces, I don't
care.

In case you want simple virt-numa where a number of vcpus constitute a
vnode and have their memory all on the same node the vcpus are ran on,
what does it matter if you unplug something in the host? Just migrate
everything -- including memory.

But what Liu was proposing is completely insane and broken. You cannot
simply remap cpu:node relations. Wanting to do that shows a profound
lack of understanding.

Our kernel assumes that a cpu remains on the same node. All userspace
that does anything with NUMA assumes the same. You cannot change this.

[Qemu-devel] [PATCH v2 14/15] net: cleanup deliver/deliver_iov func pointers

[zwu . kernel]

From: Zhi Yong Wu 

Signed-off-by: Zhi Yong Wu 
---
 net.c   |   35 +++
 net.h   |   11 +++
 net/queue.c |   13 -
 net/queue.h |   17 ++---
 4 files changed, 28 insertions(+), 48 deletions(-)

diff --git a/net.c b/net.c
index 8c8e703..7360d26 100644
--- a/net.c
+++ b/net.c
@@ -181,17 +181,6 @@ static char *assign_name(NetClientState *nc1, const char 
*model)
 return g_strdup(buf);
 }
 
-static ssize_t qemu_deliver_packet(NetClientState *sender,
-   unsigned flags,
-   const uint8_t *data,
-   size_t size,
-   void *opaque);
-static ssize_t qemu_deliver_packet_iov(NetClientState *sender,
-   unsigned flags,
-   const struct iovec *iov,
-   int iovcnt,
-   void *opaque);
-
 NetClientState *qemu_new_net_client(NetClientInfo *info,
 NetClientState *peer,
 const char *model,
@@ -218,9 +207,7 @@ NetClientState *qemu_new_net_client(NetClientInfo *info,
 }
 QTAILQ_INSERT_TAIL(&net_clients, nc, next);
 
-nc->send_queue = qemu_new_net_queue(qemu_deliver_packet,
-qemu_deliver_packet_iov,
-nc);
+nc->send_queue = qemu_new_net_queue(nc);
 
 return nc;
 }
@@ -324,11 +311,11 @@ int qemu_can_send_packet(NetClientState *sender)
 return 1;
 }
 
-static ssize_t qemu_deliver_packet(NetClientState *sender,
-   unsigned flags,
-   const uint8_t *data,
-   size_t size,
-   void *opaque)
+ssize_t qemu_deliver_packet(NetClientState *sender,
+unsigned flags,
+const uint8_t *data,
+size_t size,
+void *opaque)
 {
 NetClientState *nc = opaque;
 ssize_t ret;
@@ -421,11 +408,11 @@ static ssize_t nc_sendv_compat(NetClientState *nc, const 
struct iovec *iov,
 return nc->info->receive(nc, buffer, offset);
 }
 
-static ssize_t qemu_deliver_packet_iov(NetClientState *sender,
-   unsigned flags,
-   const struct iovec *iov,
-   int iovcnt,
-   void *opaque)
+ssize_t qemu_deliver_packet_iov(NetClientState *sender,
+unsigned flags,
+const struct iovec *iov,
+int iovcnt,
+void *opaque)
 {
 NetClientState *nc = opaque;
 
diff --git a/net.h b/net.h
index 250669a..7779b6a 100644
--- a/net.h
+++ b/net.h
@@ -112,6 +112,17 @@ void qemu_check_nic_model(NICInfo *nd, const char *model);
 int qemu_find_nic_model(NICInfo *nd, const char * const *models,
 const char *default_model);
 
+ssize_t qemu_deliver_packet(NetClientState *sender,
+unsigned flags,
+const uint8_t *data,
+size_t size,
+void *opaque);
+ssize_t qemu_deliver_packet_iov(NetClientState *sender,
+unsigned flags,
+const struct iovec *iov,
+int iovcnt,
+void *opaque);
+
 void do_info_network(Monitor *mon);
 
 /* NIC info */
diff --git a/net/queue.c b/net/queue.c
index 35c3463..0afd783 100644
--- a/net/queue.c
+++ b/net/queue.c
@@ -23,6 +23,7 @@
 
 #include "net/queue.h"
 #include "qemu-queue.h"
+#include "net.h"
 
 /* The delivery handler may only return zero if it will call
  * qemu_net_queue_flush() when it determines that it is once again able
@@ -48,8 +49,6 @@ struct NetPacket {
 };
 
 struct NetQueue {
-NetPacketDeliver *deliver;
-NetPacketDeliverIOV *deliver_iov;
 void *opaque;
 
 QTAILQ_HEAD(packets, NetPacket) packets;
@@ -57,16 +56,12 @@ struct NetQueue {
 unsigned delivering : 1;
 };
 
-NetQueue *qemu_new_net_queue(NetPacketDeliver *deliver,
- NetPacketDeliverIOV *deliver_iov,
- void *opaque)
+NetQueue *qemu_new_net_queue(void *opaque)
 {
 NetQueue *queue;
 
 queue = g_malloc0(sizeof(NetQueue));
 
-queue->deliver = deliver;
-queue->deliver_iov = deliver_iov;
 queue->opaque = opaque;
 
 QTAILQ_INIT(&queue->packets);
@@ -151,7 +146,7 @@ static ssize_t qemu_net_queue_deliver(NetQueue *queue,
 ssize_t ret = -1;
 
 queue->delivering = 1;
-ret = queue->deliver(sender, flags, data, size, queue->opaque);
+   

[Qemu-devel] [PATCH v2 07/15] net: Remove vlan code from net.c

[zwu . kernel]

From: Stefan Hajnoczi 

The vlan implementation in net.c has been replaced by hubs so we can
remove the code.

Signed-off-by: Stefan Hajnoczi 
Signed-off-by: Zhi Yong Wu 
---
 hw/xen_nic.c |1 -
 net.c|  108 --
 net.h|1 -
 3 files changed, 0 insertions(+), 110 deletions(-)

diff --git a/hw/xen_nic.c b/hw/xen_nic.c
index 9a59bda..85526fe 100644
--- a/hw/xen_nic.c
+++ b/hw/xen_nic.c
@@ -328,7 +328,6 @@ static int net_init(struct XenDevice *xendev)
 return -1;
 }
 
-netdev->conf.vlan = qemu_find_vlan(netdev->xendev.dev, 1);
 netdev->conf.peer = NULL;
 
 netdev->nic = qemu_new_nic(&net_xen_info, &netdev->conf,
diff --git a/net.c b/net.c
index 96252f9..abf5a3d 100644
--- a/net.c
+++ b/net.c
@@ -388,50 +388,6 @@ static ssize_t qemu_deliver_packet(VLANClientState *sender,
 return ret;
 }
 
-static ssize_t qemu_vlan_deliver_packet(VLANClientState *sender,
-unsigned flags,
-const uint8_t *buf,
-size_t size,
-void *opaque)
-{
-VLANState *vlan = opaque;
-VLANClientState *vc;
-ssize_t ret = -1;
-
-QTAILQ_FOREACH(vc, &vlan->clients, next) {
-ssize_t len;
-
-if (vc == sender) {
-continue;
-}
-
-if (vc->link_down) {
-ret = size;
-continue;
-}
-
-if (vc->receive_disabled) {
-ret = 0;
-continue;
-}
-
-if (flags & QEMU_NET_PACKET_FLAG_RAW && vc->info->receive_raw) {
-len = vc->info->receive_raw(vc, buf, size);
-} else {
-len = vc->info->receive(vc, buf, size);
-}
-
-if (len == 0) {
-vc->receive_disabled = 1;
-}
-
-ret = (ret >= 0) ? ret : len;
-
-}
-
-return ret;
-}
-
 void qemu_purge_queued_packets(VLANClientState *vc)
 {
 NetQueue *queue;
@@ -538,42 +494,6 @@ static ssize_t qemu_deliver_packet_iov(VLANClientState 
*sender,
 }
 }
 
-static ssize_t qemu_vlan_deliver_packet_iov(VLANClientState *sender,
-unsigned flags,
-const struct iovec *iov,
-int iovcnt,
-void *opaque)
-{
-VLANState *vlan = opaque;
-VLANClientState *vc;
-ssize_t ret = -1;
-
-QTAILQ_FOREACH(vc, &vlan->clients, next) {
-ssize_t len;
-
-if (vc == sender) {
-continue;
-}
-
-if (vc->link_down) {
-ret = iov_size(iov, iovcnt);
-continue;
-}
-
-assert(!(flags & QEMU_NET_PACKET_FLAG_RAW));
-
-if (vc->info->receive_iov) {
-len = vc->info->receive_iov(vc, iov, iovcnt);
-} else {
-len = vc_sendv_compat(vc, iov, iovcnt);
-}
-
-ret = (ret >= 0) ? ret : len;
-}
-
-return ret;
-}
-
 ssize_t qemu_sendv_packet_async(VLANClientState *sender,
 const struct iovec *iov, int iovcnt,
 NetPacketSent *sent_cb)
@@ -601,34 +521,6 @@ qemu_sendv_packet(VLANClientState *vc, const struct iovec 
*iov, int iovcnt)
 return qemu_sendv_packet_async(vc, iov, iovcnt, NULL);
 }
 
-/* find or alloc a new VLAN */
-VLANState *qemu_find_vlan(int id, int allocate)
-{
-VLANState *vlan;
-
-QTAILQ_FOREACH(vlan, &vlans, next) {
-if (vlan->id == id) {
-return vlan;
-}
-}
-
-if (!allocate) {
-return NULL;
-}
-
-vlan = g_malloc0(sizeof(VLANState));
-vlan->id = id;
-QTAILQ_INIT(&vlan->clients);
-
-vlan->send_queue = qemu_new_net_queue(qemu_vlan_deliver_packet,
-  qemu_vlan_deliver_packet_iov,
-  vlan);
-
-QTAILQ_INSERT_TAIL(&vlans, vlan, next);
-
-return vlan;
-}
-
 VLANClientState *qemu_find_netdev(const char *id)
 {
 VLANClientState *vc;
diff --git a/net.h b/net.h
index 7d18b10..a4ac48d 100644
--- a/net.h
+++ b/net.h
@@ -87,7 +87,6 @@ struct VLANState {
 NetQueue *send_queue;
 };
 
-VLANState *qemu_find_vlan(int id, int allocate);
 VLANClientState *qemu_find_netdev(const char *id);
 VLANClientState *qemu_new_net_client(NetClientInfo *info,
  VLANClientState *peer,
-- 
1.7.6

[Qemu-devel] [PATCH v2 04/15] hub: Check that hubs are configured correctly

[zwu . kernel]

From: Stefan Hajnoczi 

Checks can be performed to make sure that hubs have at least one NIC and
one host device, warning the user if this is not the case.
Configurations which do not meet this rule tend to be broken but just
emit a warning.  This patch preserves compatibility with the checks
performed by net core on vlans.

Signed-off-by: Stefan Hajnoczi 
Signed-off-by: Zhi Yong Wu 
---
 net.c |   25 +
 net/hub.c |   45 +
 net/hub.h |1 +
 3 files changed, 47 insertions(+), 24 deletions(-)

diff --git a/net.c b/net.c
index d9c7eac..88b9e1f 100644
--- a/net.c
+++ b/net.c
@@ -1337,7 +1337,6 @@ void net_cleanup(void)
 
 void net_check_clients(void)
 {
-VLANState *vlan;
 VLANClientState *vc;
 int i;
 
@@ -1353,30 +1352,8 @@ void net_check_clients(void)
 return;
 }
 
-QTAILQ_FOREACH(vlan, &vlans, next) {
-int has_nic = 0, has_host_dev = 0;
+net_hub_check_clients();
 
-QTAILQ_FOREACH(vc, &vlan->clients, next) {
-switch (vc->info->type) {
-case NET_CLIENT_TYPE_NIC:
-has_nic = 1;
-break;
-case NET_CLIENT_TYPE_USER:
-case NET_CLIENT_TYPE_TAP:
-case NET_CLIENT_TYPE_SOCKET:
-case NET_CLIENT_TYPE_VDE:
-has_host_dev = 1;
-break;
-default: ;
-}
-}
-if (has_host_dev && !has_nic)
-fprintf(stderr, "Warning: vlan %d with no nics\n", vlan->id);
-if (has_nic && !has_host_dev)
-fprintf(stderr,
-"Warning: vlan %d is not connected to host network\n",
-vlan->id);
-}
 QTAILQ_FOREACH(vc, &non_vlan_clients, next) {
 if (!vc->peer) {
 fprintf(stderr, "Warning: %s %s has no peer\n",
diff --git a/net/hub.c b/net/hub.c
index 3cd1249..19c1169 100644
--- a/net/hub.c
+++ b/net/hub.c
@@ -222,3 +222,48 @@ int net_hub_id_for_client(VLANClientState *nc, unsigned 
int *id)
 }
 return -ENOENT;
 }
+
+/**
+ * Warn if hub configurations are likely wrong
+ */
+void net_hub_check_clients(void)
+{
+NetHub *hub;
+NetHubPort *port;
+VLANClientState *peer;
+
+QLIST_FOREACH(hub, &hubs, next) {
+int has_nic = 0, has_host_dev = 0;
+
+QLIST_FOREACH(port, &hub->ports, next) {
+peer = port->nc.peer;
+if (!peer) {
+fprintf(stderr, "Warning: hub port %s has no peer\n",
+port->nc.name);
+continue;
+}
+
+switch (peer->info->type) {
+case NET_CLIENT_TYPE_NIC:
+has_nic = 1;
+break;
+case NET_CLIENT_TYPE_USER:
+case NET_CLIENT_TYPE_TAP:
+case NET_CLIENT_TYPE_SOCKET:
+case NET_CLIENT_TYPE_VDE:
+has_host_dev = 1;
+break;
+default:
+break;
+}
+}
+if (has_host_dev && !has_nic) {
+fprintf(stderr, "Warning: vlan %u with no nics\n", hub->id);
+}
+if (has_nic && !has_host_dev) {
+fprintf(stderr,
+"Warning: vlan %u is not connected to host network\n",
+hub->id);
+}
+}
+}
diff --git a/net/hub.h b/net/hub.h
index 60d4cae..caa3b16 100644
--- a/net/hub.h
+++ b/net/hub.h
@@ -21,5 +21,6 @@ VLANClientState *net_hub_find_client_by_name(unsigned int 
hub_id,
  const char *name);
 void net_hub_info(Monitor *mon);
 int net_hub_id_for_client(VLANClientState *nc, unsigned int *id);
+void net_hub_check_clients(void);
 
 #endif /* NET_HUB_H */
-- 
1.7.6

Re: [Qemu-devel] [PATCH] ahci: SATA FIS is 20 bytes, not 0x20

[Stefan Weil]

Am 23.05.2012 01:26, schrieb Daniel Verkamp:

As in the SATA and AHCI specifications, a FIS is 5 Dwords of 4 bytes
each, which comes to 20 bytes (decimal), not 0x20.

Signed-off-by: Daniel Verkamp
---
  hw/ide/ahci.c |4 ++--
  1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/hw/ide/ahci.c b/hw/ide/ahci.c
index a883a92..2d7d03d 100644
--- a/hw/ide/ahci.c
+++ b/hw/ide/ahci.c
@@ -462,7 +462,7 @@ static void ahci_check_cmd_bh(void *opaque)

  static void ahci_init_d2h(AHCIDevice *ad)
  {
-uint8_t init_fis[0x20];
+uint8_t init_fis[20];
  IDEState *ide_state =&ad->port.ifs[0];


The current code only uses 14 elements, so 20 elements
still waste some local memory (and 0x20 elements waste
even more).




  memset(init_fis, 0, sizeof(init_fis));
@@ -619,7 +619,7 @@ static void ahci_write_fis_d2h(AHCIDevice *ad, uint8_t 
*cmd_fis)
  d2h_fis[11] = cmd_fis[11];
  d2h_fis[12] = cmd_fis[12];
  d2h_fis[13] = cmd_fis[13];
-for (i = 14; i<  0x20; i++) {
+for (i = 14; i<  20; i++) {
  d2h_fis[i] = 0;
  }


I am not sure whether this change is correct.
This code does _not_ access the array which was allocated above:

d2h_fis = &ad->res_fis[RES_FIS_RFIS];

Regards,
Stefan W.