Re: [Qemu-devel] [PATCH 1/2] ide: convert ide_sector_read() to asynchronous I/O

[Michael Tokarev]

On 29.03.2012 10:46, Michael Tokarev wrote:
> On 28.03.2012 19:43, Stefan Hajnoczi wrote:
> ...
> 
>> void ide_sector_read(IDEState *s)
>> {
> ...
>> +s->iov.iov_base = s->io_buffer;
>> +s->iov.iov_len  = n * BDRV_SECTOR_SIZE;
>> +qemu_iovec_init_external(&s->qiov, &s->iov, 1);
>> +bdrv_aio_readv(s->bs, sector_num, &s->qiov, n,
>> +   ide_sector_read_cb, s);
>>  }
>>  
>> @@ -383,6 +383,8 @@ struct IDEState {
>>  int cd_sector_size;
>>  int atapi_dma; /* true if dma is requested for the packet cmd */
>>  BlockAcctCookie acct;
>> +struct iovec iov;
>> +QEMUIOVector qiov;
>>  /* ATA DMA state */
>>  int io_buffer_size;
>>  QEMUSGList sg;
> 
> You don't actually need iov here, it can be a local variable
> just as well.  The same applies to ide_sector_write() in the
> next patch.  The state structure usually holds stuff which
> is actually needed to be keept across several calls, iov is
> not one of them.

Please ignore this one ;)  For qemu_iovec_init_external(), the
real iov is ofcourse needed.

Thanks,

/mjt

Re: [Qemu-devel] [PATCH v1 1/2] pl330: initial version

[Peter Maydell]

On 29 March 2012 03:54, Peter A. G. Crosthwaite
 wrote:
> Device model for Primecell PL330 dma controller.
>
> Signed-off-by: Peter A. G. Crosthwaite 
> ---

> +/*
> + * ARM PrimeCell PL330 DMA Controller
> + *
> + * Copyright (c) 2009 Samsung Electronics.
> + * Contributed by Kirill Batuzov 
> + */

This needs to state the license (presumably GPLv2-or-later
but you need to check with the authors of the code),
and a Signed-off-by: from somebody at Samsung would be good.

-- PMM

Re: [Qemu-devel] [PATCH] qemu tcg: Remove one entry of INDEX_op_ld_i64 from ppc_op_defs

[malc]

On Thu, 29 Mar 2012, David Gibson wrote:

> From: Li Zhang 
> 
> There two entries of INDEX_op_ld_i64 in the ppc_op_defs.  That causes an
> assertion failure in tcg_add_target_add_op_defs() when --enable-debug is
> used on a ppc64 backend (that's ppc64 host, not target).
> 

Thanks, applied.

[..snip..]

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

Re: [Qemu-devel] [PATCH RFC] piix: fix up/down races

[Gleb Natapov]

On Tue, Mar 27, 2012 at 07:59:08PM +0200, Michael S. Tsirkin wrote:
> piix acpi interface suffers from the following 2 issues:
> 
> 1.
> - delete device a
> - quickly add device b in another slot
> 
> if we do this before guest reads the down register,
> the down event is discarded and device will never
> be deleted.
> 
> 2.
> - delete device a
> - quickly reset before guest can respond
> 
> interrupt is reset and guest will never eject the device.
> 
> To fix this, we implement two changes:
> 
> 1. Add two new registers:
> CLR_UP
> CLR_DOWN
> bios will now write to these the value it read from UP/DOWN
> 
> 2. on reset, remove all devices which have DOWN bit set
> 
> For compatibility with old guests, we also clear
> the DOWN bit on write to EJ0 for a device.
> 
> Signed-off-by: Michael S. Tsirkin 
> 
> Warning: untested.
> Posting for early feedback/flames.
> 
> ---
>  hw/acpi_piix4.c |   72 --
docs/specs/acpi_pci_hotplug.txt +++

>  1 files changed, 58 insertions(+), 14 deletions(-)
> 
> diff --git a/hw/acpi_piix4.c b/hw/acpi_piix4.c
> index 797ed24..a155358 100644
> --- a/hw/acpi_piix4.c
> +++ b/hw/acpi_piix4.c
> @@ -43,6 +43,8 @@
>  #define PCI_BASE 0xae00
>  #define PCI_EJ_BASE 0xae08
>  #define PCI_RMV_BASE 0xae0c
> +#define PCI_CLR_UP_BASE 0xae10
> +#define PCI_CLR_DOWN_BASE 0xae14
>  
>  #define PIIX4_PCI_HOTPLUG_STATUS 2
>  
> @@ -287,6 +289,25 @@ static void piix4_update_hotplug(PIIX4PMState *s)
>  }
>  }
>  
> +static void acpi_piix_eject_slot(PIIX4PMState *s, unsigned slots)
> +{
> +DeviceState *qdev, *next;
> +BusState *bus = qdev_get_parent_bus(&s->dev.qdev);
> +int slot = ffs(slots) - 1;
> +
Funny how AML code makes bitmap from a slot number and here we do the
opposite. I wish we could fix it.

> +/* Clear down register here too - this is good for
> + * compatibility with old guests which do not have CLR_DOWN. */
> +s->pci0_status.down &= ~(1U << slot);
Strictly speaking the comment is incorrect since reset handler below
relies on this too.

> +
> +QTAILQ_FOREACH_SAFE(qdev, &bus->children, sibling, next) {
> +PCIDevice *dev = PCI_DEVICE(qdev);
> +PCIDeviceClass *pc = PCI_DEVICE_GET_CLASS(dev);
> +if (PCI_SLOT(dev->devfn) == slot && !pc->no_hotplug) {
> +qdev_free(qdev);
> +}
> +}
> +}
> +
>  static void piix4_reset(void *opaque)
>  {
>  PIIX4PMState *s = opaque;
> @@ -302,6 +323,19 @@ static void piix4_reset(void *opaque)
>  pci_conf[0x5B] = 0x02;
>  }
>  piix4_update_hotplug(s);
> +/*
> + * Guest lost remove events if any.
> + * As it's being reset it's safe to remove the device now.
> + */
> +while (s->pci0_status.down) {
> +acpi_piix_eject_slot(s, s->pci0_status.down);
> +}
> +/*
> + * Guest lost add events if any.
> + * As it's being reset and will rescan the bus we cann discard
> + * past events now.
> + */
> +s->pci0_status.up = 0;
>  }
>  
>  static void piix4_powerdown(void *opaque, int irq, int power_failing)
> @@ -490,22 +524,31 @@ static uint32_t pciej_read(void *opaque, uint32_t addr)
>  
>  static void pciej_write(void *opaque, uint32_t addr, uint32_t val)
>  {
> -BusState *bus = opaque;
> -DeviceState *qdev, *next;
> -int slot = ffs(val) - 1;
> +PIIX4PMState *s = opaque;
>  
> -QTAILQ_FOREACH_SAFE(qdev, &bus->children, sibling, next) {
> -PCIDevice *dev = PCI_DEVICE(qdev);
> -PCIDeviceClass *pc = PCI_DEVICE_GET_CLASS(dev);
> -if (PCI_SLOT(dev->devfn) == slot && !pc->no_hotplug) {
> -qdev_free(qdev);
> -}
> +if (val) {
> +acpi_piix_eject_slot(s, val);
>  }
>  
> -
>  PIIX4_DPRINTF("pciej write %x <== %d\n", addr, val);
>  }
>  
> +static void pci_clr_up_write(void *opaque, uint32_t addr, uint32_t val)
> +{
> +PIIX4PMState *s = opaque;
> +s->pci0_status.up &= ~val;
> +
> +PIIX4_DPRINTF("pci_clr_up write %x <== %d\n", addr, val);
> +}
> +
> +static void pci_clr_down_write(void *opaque, uint32_t addr, uint32_t val)
> +{
> +PIIX4PMState *s = opaque;
> +s->pci0_status.down &= ~val;
> +
> +PIIX4_DPRINTF("pci_clr_down write %x <== %d\n", addr, val);
> +}
> +
>  static uint32_t pcirmv_read(void *opaque, uint32_t addr)
>  {
>  PIIX4PMState *s = opaque;
> @@ -532,12 +575,15 @@ static void piix4_acpi_system_hot_add_init(PCIBus *bus, 
> PIIX4PMState *s)
>  register_ioport_write(PCI_BASE, 8, 4, pcihotplug_write, pci0_status);
>  register_ioport_read(PCI_BASE, 8, 4,  pcihotplug_read, pci0_status);
>  
> -register_ioport_write(PCI_EJ_BASE, 4, 4, pciej_write, bus);
> -register_ioport_read(PCI_EJ_BASE, 4, 4,  pciej_read, bus);
> +register_ioport_write(PCI_EJ_BASE, 4, 4, pciej_write, s);
> +register_ioport_read(PCI_EJ_BASE, 4, 4,  pciej_read, s);
>  
>  register_ioport_write(PCI_RMV_BASE, 4, 4, pcirmv_write, s);
>  register_ioport_read(PCI_RMV_BAS

Re: [Qemu-devel] [PATCH 1/2] ide: convert ide_sector_read() to asynchronous I/O

[Stefan Hajnoczi]

On Thu, Mar 29, 2012 at 7:42 AM, Michael Tokarev  wrote:
> On 28.03.2012 19:43, Stefan Hajnoczi wrote:
>>  void ide_sector_read(IDEState *s)
>>  {
> []
>> +    s->iov.iov_base = s->io_buffer;
>> +    s->iov.iov_len  = n * BDRV_SECTOR_SIZE;
>> +    qemu_iovec_init_external(&s->qiov, &s->iov, 1);
>> +
>> +    bdrv_acct_start(s->bs, &s->acct, n * BDRV_SECTOR_SIZE, BDRV_ACCT_READ);
>> +    bdrv_aio_readv(s->bs, sector_num, &s->qiov, n,
>> +                   ide_sector_read_cb, s);
>>  }
>
> Shouldn't this function be returning something and
> check the return value of bdrv_aio_readv() ?

I think you are right that something is missing here.  If the IDE
controller is reset then we need to cancel the pending aio request.
Otherwise we could end up with a dangling request across reset that
overwrites io_buffer.

About checking the return value: it used to be the case that the
return value needed to be checked.

This was kind of a wart in the interface because it created 2 error
handling paths: one immediate return and one callback with error.  I
think Paolo and Kevin were the ones to address this.  You can check
out this patch for starters but there are a few more related ones:

ad54ae80c7 ("block: bdrv_aio_* do not return NULL")

Today the return value does not need to be checked for NULL.

Stefan

Re: [Qemu-devel] [PATCH 1/2] qdev: qdev_unplug(): Use error_set()

[Stefan Hajnoczi]

On Wed, Mar 28, 2012 at 05:50:53PM -0300, Luiz Capitulino wrote:
> @@ -268,7 +270,14 @@ static int pci_device_hot_remove(Monitor *mon, const 
> char *pci_addr)
>  monitor_printf(mon, "slot %d empty\n", slot);
>  return -1;
>  }
> -return qdev_unplug(&d->qdev);
> +
> +ret = qdev_unplug(&d->qdev, &errp);
> +if (error_is_set(&errp)) {
> +monitor_printf(mon, "%s\n", error_get_pretty(errp));
> +error_free(errp);
> +}

Minor thing if you respin: this if statement could be replaced with 
hmp_handle_error(mon, &errp).

Stefan

Re: [Qemu-devel] [PATCH 2/2] qapi: convert device_del

[Stefan Hajnoczi]

On Wed, Mar 28, 2012 at 05:50:54PM -0300, Luiz Capitulino wrote:
>  ret = qdev_unplug(dev, &local_err);
>  if (error_is_set(&local_err)) {
> -qerror_report_err(local_err);
> -error_free(local_err);
> +error_propagate(errp, local_err);
> +} else if (ret) {
> +error_set(errp, QERR_UNDEFINED_ERROR);

Can we make qdev_unplug() void?  I can find no case in QEMU where we
return != 0 without setting error.  If we fix the function prototype
this invalid state can be eliminated forever.

(Other functions that take Error **errp are usually void.)

> diff --git a/qapi-schema.json b/qapi-schema.json
> index 0d11d6e..ace55f3 100644
> --- a/qapi-schema.json
> +++ b/qapi-schema.json
> @@ -1701,3 +1701,23 @@
>  # Since: 1.1
>  ##
>  { 'command': 'xen-save-devices-state', 'data': {'filename': 'str'} }
> +
> +##
> +# @device_del:
> +#
> +# Remove a device from a guest
> +#
> +# @id: the name of the device
> +#
> +# Returns: Nothing on success
> +#  If @id is not a valid device, DeviceNotFound
> +#  If the device does not support unplug, BusNoHotplug
> +#
> +# Notes: When this command completes, the device may not be removed from the
> +#guest.  Hot removal is an operation that requires guest cooperation.
> +#This command merely requests that the guest begin the hot removal
> +#process.

I have not peeked at the implementation in QEMU or libvirt, but is there
a QMP event for actual removal or would the user need to poll?  This bit
of information would be useful in the documentation.

Stefan

Re: [Qemu-devel] [Qemu-trivial] [PATCH] w32: Undefine error constants before their redefinition

[Stefan Hajnoczi]

On Wed, Mar 28, 2012 at 08:56:38PM +0200, Jan Kiszka wrote:
> Avoids lots of warnings.
> 
> Signed-off-by: Jan Kiszka 
> ---
>  qemu_socket.h |2 ++
>  1 files changed, 2 insertions(+), 0 deletions(-)

Thanks, applied to the trivial patches tree:
https://github.com/stefanha/qemu/commits/trivial-patches

Stefan

[Qemu-devel] [PATCH] buildfix: check for old pod2man versions

[Gerd Hoffmann]

Older pod2man don't have a --utf8 switch, check for this in conffigure
and use it only when present.  Fixes build on RHEL-5.

Signed-off-by: Gerd Hoffmann 
---
 Makefile  |1 -
 configure |8 
 2 files changed, 8 insertions(+), 1 deletions(-)

diff --git a/Makefile b/Makefile
index d8e1f36..35c7a2a 100644
--- a/Makefile
+++ b/Makefile
@@ -348,7 +348,6 @@ QMP/qmp-commands.txt: $(SRC_PATH)/qmp-commands.hx
 qemu-img-cmds.texi: $(SRC_PATH)/qemu-img-cmds.hx
$(call quiet-command,sh $(SRC_PATH)/scripts/hxtool -t < $< > $@,"  GEN  
 $@")
 
-POD2MAN = pod2man --utf8
 qemu.1: qemu-doc.texi qemu-options.texi qemu-monitor.texi
$(call quiet-command, \
  perl -Ww -- $(SRC_PATH)/scripts/texi2pod.pl $< qemu.pod && \
diff --git a/configure b/configure
index 14ef738..6e870ad 100755
--- a/configure
+++ b/configure
@@ -2821,6 +2821,13 @@ if test "$solaris" = "no" ; then
 fi
 fi
 
+# test if pod2man has --utf8 option
+if pod2man --help | grep -q utf8; then
+POD2MAN="pod2man --utf8"
+else
+POD2MAN="pod2man"
+fi
+
 # Use ASLR, no-SEH and DEP if available
 if test "$mingw32" = "yes" ; then
 for flag in --dynamicbase --no-seh --nxcompat; do
@@ -3358,6 +3365,7 @@ echo "LIBS+=$LIBS" >> $config_host_mak
 echo "LIBS_TOOLS+=$libs_tools" >> $config_host_mak
 echo "EXESUF=$EXESUF" >> $config_host_mak
 echo "LIBS_QGA+=$libs_qga" >> $config_host_mak
+echo "POD2MAN=$POD2MAN" >> $config_host_mak
 
 # generate list of library paths for linker script
 
-- 
1.7.1

Re: [Qemu-devel] [PATCH] usb-ehci: frindex always is a 14 bits counter

[Gerd Hoffmann]

On 03/28/12 20:47, Hans de Goede wrote:
> frindex always is a 14 bits counter, and not a 13 bits one as we were
> emulating. There are some subtle hints to this in the spec, first of all
> "Table 2-12. FRINDEX - Frame Index Register" says:
> "Bit 13:0 Frame Index. The value in this register increments at the end of
> each time frame (e.g. micro-frame). Bits [N:3] are used for the Frame List
> current index. This means that each location of the frame list is accessed
> 8 times (frames or micro-frames) before moving to the next index. The
> following illustrates values of N based on the value of the Frame List
> Size field in the USBCMD register.
> 
> USBCMD[Frame List Size]   Number Elements  N
> 00b   102412
> 01b51211
> 10b25610
> 11b   Reserved"
> 
> Notice how the text talks about "Bits [N:3]" are used ..., it does
> NOT say that when N == 12 (our case) the counter will wrap from 8191 to 0,
> or in otherwords that it is a 13 bits counter (bits 0 - 12).
> 
> The other hint is in "Table 2-10. USBSTS USB Status Register Bit Definitions":
> 
> "Bit 3 Frame List Rollover - R/WC. The Host Controller sets this bit to a one
> when the Frame List Index (see Section 2.3.4) rolls over from its maximum 
> value
> to zero. The exact value at which the rollover occurs depends on the frame
> list size. For example, if the frame list size (as programmed in the Frame
> List Size field of the USBCMD register) is 1024, the Frame Index Register
> rolls over every time FRINDEX[13] toggles. Similarly, if the size is 512,
> the Host Controller sets this bit to a one every time FRINDEX[12] toggles."
> 
> Notice how this text talks about setting bit 3 when bit 13 of frindex toggles
> (when there are 1024 entries, so our case), so this indicates that frindex
> has a bit 13 making it a 14 bit counter.
> 
> Besides these clear hints the real proof is in the pudding. Before this
> patch I could not stream data from a USB2 webcam under Windows XP, after
> this cam using a USB2 webcam under Windows XP works fine, and no regressions
> with other operating systems were seen.
> 
> Signed-off-by: Hans de Goede 
> ---
>  hw/usb-ehci.c |8 ++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/hw/usb-ehci.c b/hw/usb-ehci.c
> index b5d7037..3934bf0 100644
> --- a/hw/usb-ehci.c
> +++ b/hw/usb-ehci.c
> @@ -2157,11 +2157,15 @@ static void ehci_frame_timer(void *opaque)
>  if ( !(ehci->usbsts & USBSTS_HALT)) {
>  ehci->frindex += 8;
>  
> -if (ehci->frindex > 0x1fff) {
> -ehci->frindex = 0;
> +if (ehci->frindex == 0x2000) {
>  ehci_set_interrupt(ehci, USBSTS_FLR);
>  }
>  
> +if (ehci->frindex == 0x4000) {
> +ehci_set_interrupt(ehci, USBSTS_FLR);
> +ehci->frindex = 0;
> +}
> +
>  ehci->sofv = (ehci->frindex - 1) >> 3;
>  ehci->sofv &= 0x03ff;
>  }

Patch added to usb patch queue.

thanks,
  Gerd

Re: [Qemu-devel] [PATCH v2] spice_info: add mouse_mode

[Gerd Hoffmann]

On 03/26/12 16:15, Alon Levy wrote:
> Add mouse_mode, either server or mouse, to qmp and hmp commands, based
> on spice_server_is_server_mouse added in spice-server 0.10.3.

Looks good.  What is the status of the spice-server patch?  Committed?

cheers,
  Gerd

Re: [Qemu-devel] [PATCH 1/2] ide: convert ide_sector_read() to asynchronous I/O

[Paolo Bonzini]

Il 29/03/2012 10:29, Stefan Hajnoczi ha scritto:
> This was kind of a wart in the interface because it created 2 error
> handling paths: one immediate return and one callback with error.  I
> think Paolo and Kevin were the ones to address this.

I think you did that when adding the coroutines.  I just cleaned up the
result.

Paolo

Re: [Qemu-devel] [Qemu-ppc] [PATCH 1/2] PPC: Fix interrupt MSR value within the PPC interrupt handler.

[Mark Cave-Ayland]

On 28/03/12 01:46, David Gibson wrote:

Hi David,


If we're going to make this specific to MSRs, might as well cut down on
the user's verbosity:

#define MSR_BIT(x) ((target_ulong)1<<  MSR_##x)

...and move it to a header file.

Or possibly have the header file define a set of MSRBIT_IR, MSRBIT_DR, etc.


I think I prefer your macro above and move it to a relevant part of 
target-ppc/cpu.h with the other MSR defines.



  static inline void powerpc_excp(CPUPPCState *env, int excp_model, int excp)
  {
  target_ulong msr, new_msr, vector;
@@ -2478,11 +2480,26 @@ static inline void powerpc_excp(CPUPPCState *env, int 
excp_model, int excp)
  qemu_log_mask(CPU_LOG_INT, "Raise exception at " TARGET_FMT_lx
" =>  %08x (%02x)\n", env->nip, excp, env->error_code);

-/* new srr1 value excluding must-be-zero bits */
+/* new srr1 value with interrupt-specific bits defaulting to zero */
  msr = env->msr&  ~0x783fULL;

-/* new interrupt handler msr */
-new_msr = env->msr&  ((target_ulong)1<<  MSR_ME);
+switch (excp_model) {
+case POWERPC_EXCP_BOOKE:
+/* new interrupt handler msr */
+new_msr = env->msr&  ((target_ulong)1<<  MSR_ME);
+break;
+
+default:
+/* new interrupt handler msr (as per PowerISA 2.06B p.811 and p.814):
+   1) force the following bits to zero
+  IR, DR, FE0, FE1, EE, BE, FP, PMM, PR, SE
+   2) default the following bits to zero (can be overidden later on)
+  RI */
+new_msr = env->msr&  ~(MSR_BIT(MSR_IR) | MSR_BIT(MSR_DR)
+  | MSR_BIT(MSR_FE0)| MSR_BIT(MSR_FE1) | MSR_BIT(MSR_EE)
+  | MSR_BIT(MSR_BE) | MSR_BIT(MSR_FP) | MSR_BIT(MSR_PMM)
+  | MSR_BIT(MSR_PR) | MSR_BIT(MSR_SE) | MSR_BIT(MSR_RI));
+}


What about POWERPC_EXCP_40x?  And are all the classic chips OK with the
2.06B implementation?


Hrm, yeah.  I think what you ought to do is to use the new logic just
for the "classic" exception models.  Have the default branch remain
the one that just masks ME.  That's wrong, but it's the same wrong as
we have already, and we can fix it later once we've verified what the
right thing to do is for 40x and BookE.


I'm actually coming at this from a fixing what was potentially an 
OpenBIOS bug rather than a PPC angle, so I have to admit I have no I 
idea which ones are the "classic" exception models. Would you consider 
this to be just EXCP_STD, EXCP_6* and EXCP_7*?



Many thanks,

Mark.

Re: [Qemu-devel] [PATCH] pci: Factor out bounds checking on config space accesses

[Michael S. Tsirkin]

On Thu, Mar 29, 2012 at 02:53:52PM +1100, David Gibson wrote:
> On Wed, Mar 28, 2012 at 11:30:56AM +0200, Michael S. Tsirkin wrote:
> > On Wed, Mar 28, 2012 at 12:11:52PM +1100, David Gibson wrote:
> > > Michael,
> > > 
> > > Any chance of an ack or nack on this one?
> > > 
> > > On Mon, Mar 19, 2012 at 03:58:11PM +1100, David Gibson wrote:
> > > > There are several paths into the code to emulate PCI config space 
> > > > accesses:
> > > > one for MMIO to a plain old PCI bridge one for MMIO to a PCIe bridge and
> > > > one for the pseries machine which provides para-virtualized access to 
> > > > PCI
> > > > config space.  Each of these functions does their own bounds checking
> > > > against the size of config space to check for addresses outside the
> > > > size of config space.  The pci_host_config_{read,write}_common() (sort
> > > > of) checks for partial overruns, that is where the address is within
> > > > the size of config space, but address + length is not, it takes a
> > > > limit parameter for this purpose.
> > > > 
> > > > As well as being a small code duplication, and it being weird to
> > > > separate the checks for partial and total overruns, this checking
> > > > currently has a few buglets:
> > > > 
> > > > * For non PCI-Express we assume that the size of config space is
> > > >   PCI_CONFIG_SPACE_SIZE.  That's true for everything we emulate
> > > >   now, but is not necessarily true (e.g. PCI-X devices can have
> > > >   extended config space)
> > > > 
> > > > * The limit parameter is not necessary, since the size of config
> > > >   space can be obtained using pci_config_size()
> > > > 
> > > > * Partial overruns could only occur with a misaligned access,
> > > >   which should have already been dealt with by this point
> > > > 
> > > > * Partial overruns are handled as a partial read or write, which
> > > >   is very unlikely behaviour for real hardware
> > > > 
> > > > * Furthermore, partial reads are 0x0 padded, whereas returning
> > > >   0xff for unimplemented addresses us much more common.
> > > > 
> > > > * The partial reads/writes only work correctly by assuming
> > > >   little-endian byte layout.  While that is always true for PCI
> > > >   config space, it's an awfully subtle thing to rely on without
> > > >   comment.
> > 
> > This last point can be addressed by adding a comment?
> > Patch welcome.
> 
> Well, it could.  But why comment on the subtle assumptions of code
> which implements a totally bizarre behaviour, rather than just
> removing the bizarre behaviour.
> 
> > 
> > > > This patch, therefore, moves the bounds checking wholly into
> > > > pci_host_config_{read,write}_common().  No partial reads or writes are
> > > > performed, instead any out-of-bounds write is simply ignored and an
> > > > out-of-bounds read returns 0xff.
> > > > 
> > > > This simplifies all the callers, and makes the overall semantics saner
> > > > for edge cases.
> > > > 
> > > > Cc: Michael S. Tsirkin 
> > > > 
> > > > Signed-off-by: David Gibson 
> > 
> > Sorry, I didn't reply because I have no idea whether this patch is
> > correct.
> 
> Well, what do you need to decide one way or the other?
> 
> Would it help to split this up into micro-patches which address single
> aspects of the points covered in the patch description?

That's always good.
But most importantly, I'd like to get the motivation straight.

> > Couldn't figure out from the description whether there's a test case
> > where we differ from real hardware in our behaviour.
> 
> Not sure what you mean by a testcase here.  Do you mean a formal
> automated testcase somewhere, or just are there cases in which the
> existing behaviour differs from hardware.

No, not formal. Simply
- what these cases are
- what is the actual versus expected behaviour
- how to observe the difference from the guest

>  If the latter, then yes,
> absolutely.  In fact I'd be astonished if there is *any* hardware
> which implements partial writes (or reads) the way the existing code
> does.

We could classify such a difference as a minor bug.
The fix might turn out to be different for different platforms.

> > The change affects lots of platforms and there's no mention of which
> > ones were tested.
> 
> Only pseries, I'm afraid, because that's all I've really got guest
> setups available to try.

Then it would be better to find a way to limit the change to that
platform.

Alternatively need to get others interested enough to spend cycles
on testing your patches.

> -- 
> David Gibson  | I'll have my music baroque, and my code
> david AT gibson.dropbear.id.au| minimalist, thank you.  NOT _the_ 
> _other_
>   | _way_ _around_!
> http://www.ozlabs.org/~dgibson

[Qemu-devel] [PATCH v2 2/2] ide: convert ide_sector_write() to asynchronous I/O

[Stefan Hajnoczi]

The IDE PIO write sector code path uses bdrv_write() and hence can make
the guest unresponsive while the I/O request is in progress.  This patch
converts ide_sector_write() to use bdrv_aio_writev() by using the
BUSY_STAT bit to tell the guest that the request is in progress.

Signed-off-by: Stefan Hajnoczi 
---
 hw/ide/core.c |   61 +++-
 1 files changed, 42 insertions(+), 19 deletions(-)

diff --git a/hw/ide/core.c b/hw/ide/core.c
index 47bc958..dc21e7c 100644
--- a/hw/ide/core.c
+++ b/hw/ide/core.c
@@ -673,40 +673,39 @@ static void ide_sector_write_timer_cb(void *opaque)
 ide_set_irq(s->bus);
 }
 
-void ide_sector_write(IDEState *s)
+static void ide_sector_write_cb(void *opaque, int ret)
 {
-int64_t sector_num;
-int ret, n, n1;
-
-s->status = READY_STAT | SEEK_STAT;
-sector_num = ide_get_sector(s);
-#if defined(DEBUG_IDE)
-printf("write sector=%" PRId64 "\n", sector_num);
-#endif
-n = s->nsector;
-if (n > s->req_nb_sectors)
-n = s->req_nb_sectors;
+IDEState *s = opaque;
+int n;
 
-bdrv_acct_start(s->bs, &s->acct, n * BDRV_SECTOR_SIZE, BDRV_ACCT_READ);
-ret = bdrv_write(s->bs, sector_num, s->io_buffer, n);
 bdrv_acct_done(s->bs, &s->acct);
 
+s->pio_aiocb = NULL;
+s->status &= ~BUSY_STAT;
+
 if (ret != 0) {
-if (ide_handle_rw_error(s, -ret, BM_STATUS_PIO_RETRY))
+if (ide_handle_rw_error(s, -ret, BM_STATUS_PIO_RETRY)) {
 return;
+}
 }
 
+n = s->nsector;
+if (n > s->req_nb_sectors) {
+n = s->req_nb_sectors;
+}
 s->nsector -= n;
 if (s->nsector == 0) {
 /* no more sectors to write */
 ide_transfer_stop(s);
 } else {
-n1 = s->nsector;
-if (n1 > s->req_nb_sectors)
+int n1 = s->nsector;
+if (n1 > s->req_nb_sectors) {
 n1 = s->req_nb_sectors;
-ide_transfer_start(s, s->io_buffer, 512 * n1, ide_sector_write);
+}
+ide_transfer_start(s, s->io_buffer, n1 * BDRV_SECTOR_SIZE,
+   ide_sector_write);
 }
-ide_set_sector(s, sector_num + n);
+ide_set_sector(s, ide_get_sector(s) + n);
 
 if (win2k_install_hack && ((++s->irq_count % 16) == 0)) {
 /* It seems there is a bug in the Windows 2000 installer HDD
@@ -722,6 +721,30 @@ void ide_sector_write(IDEState *s)
 }
 }
 
+void ide_sector_write(IDEState *s)
+{
+int64_t sector_num;
+int n;
+
+s->status = READY_STAT | SEEK_STAT | BUSY_STAT;
+sector_num = ide_get_sector(s);
+#if defined(DEBUG_IDE)
+printf("sector=%" PRId64 "\n", sector_num);
+#endif
+n = s->nsector;
+if (n > s->req_nb_sectors) {
+n = s->req_nb_sectors;
+}
+
+s->iov.iov_base = s->io_buffer;
+s->iov.iov_len  = n * BDRV_SECTOR_SIZE;
+qemu_iovec_init_external(&s->qiov, &s->iov, 1);
+
+bdrv_acct_start(s->bs, &s->acct, n * BDRV_SECTOR_SIZE, BDRV_ACCT_READ);
+s->pio_aiocb = bdrv_aio_writev(s->bs, sector_num, &s->qiov, n,
+   ide_sector_write_cb, s);
+}
+
 static void ide_flush_cb(void *opaque, int ret)
 {
 IDEState *s = opaque;
-- 
1.7.9.1

[Qemu-devel] [PATCH v2 1/2] ide: convert ide_sector_read() to asynchronous I/O

[Stefan Hajnoczi]

The IDE PIO interface currently uses bdrv_read() to perform reads
synchronously.  Synchronous I/O in the vcpu thread is bad because it
prevents the guest from executing code - it makes the guest
unresponsive.

This patch converts IDE PIO to use bdrv_aio_readv().  We simply need to
use the BUSY_STAT status so the guest knows to wait while we are busy.

The only external user of ide_sector_read() is restart behavior on I/O
errors and it is not affected by this change.  We still need to restart
I/O in the same way.

Migration is also unaffected if I understand the code correctly.  We
continue to use the same transfer function and the BUSY_STAT status
should never be migrated since we flush I/O before migrating device
state.

Signed-off-by: Stefan Hajnoczi 
---
 hw/ide/core.c |   76 ++--
 hw/ide/internal.h |3 ++
 2 files changed, 58 insertions(+), 21 deletions(-)

diff --git a/hw/ide/core.c b/hw/ide/core.c
index 4d568ac..47bc958 100644
--- a/hw/ide/core.c
+++ b/hw/ide/core.c
@@ -457,40 +457,68 @@ static void ide_rw_error(IDEState *s) {
 ide_set_irq(s->bus);
 }
 
+static void ide_sector_read_cb(void *opaque, int ret)
+{
+IDEState *s = opaque;
+int n;
+
+s->pio_aiocb = NULL;
+s->status &= ~BUSY_STAT;
+
+bdrv_acct_done(s->bs, &s->acct);
+if (ret != 0) {
+if (ide_handle_rw_error(s, -ret, BM_STATUS_PIO_RETRY |
+BM_STATUS_RETRY_READ)) {
+return;
+}
+}
+
+n = s->nsector;
+if (n > s->req_nb_sectors) {
+n = s->req_nb_sectors;
+}
+
+/* Allow the guest to read the io_buffer */
+ide_transfer_start(s, s->io_buffer, n * BDRV_SECTOR_SIZE, ide_sector_read);
+
+ide_set_irq(s->bus);
+
+ide_set_sector(s, ide_get_sector(s) + n);
+s->nsector -= n;
+}
+
 void ide_sector_read(IDEState *s)
 {
 int64_t sector_num;
-int ret, n;
+int n;
 
 s->status = READY_STAT | SEEK_STAT;
 s->error = 0; /* not needed by IDE spec, but needed by Windows */
 sector_num = ide_get_sector(s);
 n = s->nsector;
+
 if (n == 0) {
-/* no more sector to read from disk */
 ide_transfer_stop(s);
-} else {
+return;
+}
+
+s->status |= BUSY_STAT;
+
+if (n > s->req_nb_sectors) {
+n = s->req_nb_sectors;
+}
+
 #if defined(DEBUG_IDE)
-printf("read sector=%" PRId64 "\n", sector_num);
+printf("sector=%" PRId64 "\n", sector_num);
 #endif
-if (n > s->req_nb_sectors)
-n = s->req_nb_sectors;
 
-bdrv_acct_start(s->bs, &s->acct, n * BDRV_SECTOR_SIZE, BDRV_ACCT_READ);
-ret = bdrv_read(s->bs, sector_num, s->io_buffer, n);
-bdrv_acct_done(s->bs, &s->acct);
-if (ret != 0) {
-if (ide_handle_rw_error(s, -ret,
-BM_STATUS_PIO_RETRY | BM_STATUS_RETRY_READ))
-{
-return;
-}
-}
-ide_transfer_start(s, s->io_buffer, 512 * n, ide_sector_read);
-ide_set_irq(s->bus);
-ide_set_sector(s, sector_num + n);
-s->nsector -= n;
-}
+s->iov.iov_base = s->io_buffer;
+s->iov.iov_len  = n * BDRV_SECTOR_SIZE;
+qemu_iovec_init_external(&s->qiov, &s->iov, 1);
+
+bdrv_acct_start(s->bs, &s->acct, n * BDRV_SECTOR_SIZE, BDRV_ACCT_READ);
+s->pio_aiocb = bdrv_aio_readv(s->bs, sector_num, &s->qiov, n,
+  ide_sector_read_cb, s);
 }
 
 static void dma_buf_commit(IDEState *s)
@@ -1750,6 +1778,12 @@ static void ide_reset(IDEState *s)
 #ifdef DEBUG_IDE
 printf("ide: reset\n");
 #endif
+
+if (s->pio_aiocb) {
+bdrv_aio_cancel(s->pio_aiocb);
+s->pio_aiocb = NULL;
+}
+
 if (s->drive_kind == IDE_CFATA)
 s->mult_sectors = 0;
 else
diff --git a/hw/ide/internal.h b/hw/ide/internal.h
index c808a0d..8f9259b 100644
--- a/hw/ide/internal.h
+++ b/hw/ide/internal.h
@@ -383,6 +383,9 @@ struct IDEState {
 int cd_sector_size;
 int atapi_dma; /* true if dma is requested for the packet cmd */
 BlockAcctCookie acct;
+BlockDriverAIOCB *pio_aiocb;
+struct iovec iov;
+QEMUIOVector qiov;
 /* ATA DMA state */
 int io_buffer_size;
 QEMUSGList sg;
-- 
1.7.9.1

[Qemu-devel] [PATCH v2 0/2] ide: convert pio code path to asynchronous I/O

[Stefan Hajnoczi]

IDE PIO mode is currently implemented using synchronous I/O functions.  There's
no need to do this because the IDE interface is actually designed with polling
and interrupts in mind - we can do asynchronous I/O and let the guest know when
the operation has completed.  The benefit of asynchronous I/O is that the guest
can continue executing code and is more responsive.

The second aim of this conversion is to avoid calling bdrv_read()/bdrv_write()
since they do not work with I/O throttling.  This means guests should now boot
IDE drives successfully when I/O throttling is enabled.

Note that ATAPI is not converted yet and still uses bdrv_read() in two
locations.  A future patch will have to convert ATAPI so CD-ROMs also do
asynchronous I/O.

I have tested both Windows 7 Home Premium and Red Hat Enterprise Linux 6.0
guests with these patches.  In Windows, use the device manager to disable DMA
on the IDE channels.  Under recent Linux kernels, use the libata.dma=0 kernel
parameter.

Chris and Richard: Please test this to confirm that it fixes the hang you
reported.

v2:
 * Keep aiocb and cancel request on reset [mjt]

Stefan Hajnoczi (2):
  ide: convert ide_sector_read() to asynchronous I/O
  ide: convert ide_sector_write() to asynchronous I/O

 hw/ide/core.c |  137 +---
 hw/ide/internal.h |3 +
 2 files changed, 100 insertions(+), 40 deletions(-)

-- 
1.7.9.1

Re: [Qemu-devel] Moniter The Ram Access On QEMU

[陳韋任]

> Now I want to moniter memory access on QEMU (guest virtual or guest
> physical).
> I found that QEMU will translate arm instructions into TCG instructions,
> and the TCG instructions will be translated into X86 instructions.
> 
> The function "tcg_out_op"(tcg/i386/tcg_target.c) will translate the TCG
> instructions into X86 instructions eventually.
> Does the case statement just like "OP_32_64(ld8u)" and
> "INDEX_op_qemu_ld8u".etc includes all memory access routines on X86 view.
> Is it proper for me to add some codes in TCG -> X86 to moniter the ram
> access on QEMU?

  TGC ops like qemu_ld/qemu_st is for address translations (guest virtual to
host virtual) in system mode, that's all I can tell.

Regards,
chenwj

-- 
Wei-Ren Chen (陳韋任)
Computer Systems Lab, Institute of Information Science,
Academia Sinica, Taiwan (R.O.C.)
Tel:886-2-2788-3799 #1667
Homepage: http://people.cs.nctu.edu.tw/~chenwj

[Qemu-devel] [Bug 524447] Re: virsh save is very slow

[BenLake]

Just wanted to say thanks to everyone who got this fix out. Works great!

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

Title:
  virsh save is very slow

Status in libvirt virtualization API:
  Unknown
Status in QEMU:
  Fix Released
Status in “libvirt” package in Ubuntu:
  Invalid
Status in “qemu-kvm” package in Ubuntu:
  Fix Released
Status in “libvirt” source package in Lucid:
  Won't Fix
Status in “qemu-kvm” source package in Lucid:
  Fix Released
Status in “libvirt” source package in Maverick:
  Won't Fix
Status in “qemu-kvm” source package in Maverick:
  Won't Fix
Status in “qemu-kvm” package in Debian:
  Fix Released

Bug description:
  ==
  SRU Justification:
  1. impact: 'qemu save' is slow
  2. how addressed: a patch upstream fixes the case when a file does not 
announce when it is ready.
  3. patch: see the patch in linked bzr trees
  4. TEST CASE: see comment #4 for a specific recipe
  5. regression potential:  this patch only touches the vm save path.
  ==

  As reported here: http://www.redhat.com/archives/libvir-
  list/2009-December/msg00203.html

  "virsh save" is very slow - it writes the image at around 1MB/sec on
  my test system.

  (I think I saw a bug report for this issue on Fedora's bugzilla, but I
  can't find it now...)

  Confirmed under Karmic.

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

Re: [Qemu-devel] [SeaBIOS] [PATCH 1/4] Add basic linked list operations

[Ian Campbell]

On Wed, 2012-03-28 at 22:27 -0400, Kevin O'Connor wrote:
> On Wed, Mar 28, 2012 at 04:39:07PM +0200, Gerd Hoffmann wrote:
> > On 03/28/12 06:28, Alexey Korolev wrote:
> > > This linked list implementation is partially based on kernel code. So it
> > > should be quite stable
> > 
> > How about just copying the file?
> > 
> > I've used the linux kernel list implementation elsewhere too and it
> > worked just fine with only minor tweaks (remove some likely()/unlikely()
> > macros IIRC).
> 
> Linux is GPLv2 while SeaBIOS is LGPLv3, so some care should be taken.

FWIW In Xen we decided to go with the BSD list macros in our LGPL
library interfaces to avoid a similar sort issue.
http://xenbits.xen.org/hg/xen-unstable.hg/file/74d2af0b56ea/tools/include/xen-external
has that plus a handy sed script to namespace the interface, although I
don't suppose that will matter inside seabios.

Ian.

Re: [Qemu-devel] Moniter The Ram Access On QEMU

[stefan weids]

Hi Chenwj,

Thanks for your kindly help. Would you mind give me more help?

Now I have some probem with the exception handle. I found the funciton
"do_interrupt" in "target-arm/helper.c" and "gen_exception" in
"target-arm/translate.c" seems to handle the exceptions. But I am comfused
with those two functions, which function will do the really work about
generating an exception. As my understanding, I think “do_interrupt” seems
to do some preparations before enter exceptions just like operate some
registers and accumulate the exception handler address. And "gen_exception"
seems to do the read job to generate a exception. But I'm not sure.

Any comments are appreciated.

Thanks.

BR,
Stefan


在 2012年3月29日 上午11:28，陳韋任 写道：

> > Now I want to moniter memory access on QEMU (guest virtual or guest
> > physical).
> > I found that QEMU will translate arm instructions into TCG instructions,
> > and the TCG instructions will be translated into X86 instructions.
> >
> > The function "tcg_out_op"(tcg/i386/tcg_target.c) will translate the TCG
> > instructions into X86 instructions eventually.
> > Does the case statement just like "OP_32_64(ld8u)" and
> > "INDEX_op_qemu_ld8u".etc includes all memory access routines on X86 view.
> > Is it proper for me to add some codes in TCG -> X86 to moniter the ram
> > access on QEMU?
>
>   TGC ops like qemu_ld/qemu_st is for address translations (guest virtual
> to
> host virtual) in system mode, that's all I can tell.
>
> Regards,
> chenwj
>
> --
> Wei-Ren Chen (陳韋任)
> Computer Systems Lab, Institute of Information Science,
> Academia Sinica, Taiwan (R.O.C.)
> Tel:886-2-2788-3799 #1667
> Homepage: http://people.cs.nctu.edu.tw/~chenwj
>

Re: [Qemu-devel] [PATCH V3 01/12] add MIPS DSP internal functions

[Richard Henderson]

On 03/27/2012 10:03 PM, Jia Liu wrote:
> Thanks.
> do you mean, I should write like this?
> helper.h:
> DEF_HELPER_FLAGS_3(addq_ph, 0, i32, env, i32, i32)
> 
> dsp_helper.c:
> uint32_t helper_addq_ph(CPUMIPSState *env, uint32_t rs, uint32_t rt)
> {}

Yes.  Although the 0 flags argument probably warrants DEF_HELPER_3 instead.
Then, of course, you have to pass down the env parameter to all of the other
inline helpers that use it.

>>> +if (len == 2)
>>> +env->active_tc.DSPControl &= 0xFCFF;
>>> +else if (len == 4)
>>> +env->active_tc.DSPControl &= 0xF0FF;
>> Run all your patches through ./scripts/checkpatch.pl and fix the
>> errors that will report.
>>
> I've run ./scripts/checkpatch.pl, but I didn't get a ERR here...

You should have gotten an error about missing {.

  if (len == 2) {
...
  } else if (len == 4) {
...
  }


r~

Re: [Qemu-devel] [PATCH 0/2] ide: convert pio code path to asynchronous I/O

[Chris Webb]

Stefan Hajnoczi  writes:

> Chris and Richard: Please test this to confirm that it fixes the hang you
> reported.

We've been testing this (v1 against qemu-kvm 1.0) today, and it's looking
very good. Thanks!

The lock-ups during boot no longer happen, and if you severely throttle
(1MB/s, 100 req/s) a guest in the middle of a big dd, there are lots of
nasty kernel messages as it falls back to pio mode, but the guest doesn't
lock up and qemu remains responsive.

Best wishes,

Chris.

Re: [Qemu-devel] live migration between qemu-kvm 1.0 and 0.15

[Jan Kiszka]

On 2012-03-27 18:39, Anthony Liguori wrote:
> On 03/27/2012 11:22 AM, Jan Kiszka wrote:
>> On 2012-03-27 17:59, Avi Kivity wrote:
>>> On 03/27/2012 11:55 AM, Jan Kiszka wrote:
 On 2012-03-27 10:55, Vasilis Liaskovitis wrote:
> Hi,
>
> is live migration between qemu-kvm stable-0.15 and stable-1.0 trees 
> possible?
> When I live migrate a VM from 1.0 to 0.15, the destination side 0.15 
> qemu-kvm
> exits with:
> (qemu) Unknown savevm section or instance 'i8259' 0
>
> That's expected, since commit "i8259:convert to qdev" 
> 747c70af78f7088f182c87e683bdf847beead1e4
> introduces the i8259 device in the qdev tree.
>
> The other direction (live migrate from 0.15.1 to 1.0.0) seems to work 
> fine.
> Are any other issues expected in this direction?
>
> The vmstate for i8259 has not changed between these trees afaict. If the
> qdev-ified i8259 is reverted from stable-1.0 tree (to restore 
> live-migration
> compatibility between the versions), would you expect problems?

 The legacy instance IDs used in old versions are not written out by
 newer ones. They are just accepted on reading to allow moving forward.
 There are more devices in the tree that used those instance IDs, not
 only the i8259. Reverting the qdev conversion may reactivate backward
 migratability for 1.0->0.15 (unless there are others as well). But that
 will definitely be over after 1.1 as inrevertible code depends on the
 qdev conversion.
>>>
>>> Is this true even for -M pc-0.15?
>>
>> Yes. Alias IDs enable modeling according to qdev (back then) for devices
>> that wrote oddly numbered states in the past and porting them over to
>> the new format. Adding some compat write-out mode would probably be
>> feasible but would also require some thoughts and quite a bit work to
>> integrate this cleanly in vmstate.
>>
>> I guess this just remained unnoticed because the introduction of the
>> alias ID concept and first conversions happened at a time when lots of
>> devices increased their vmstate version anyway.
> 
> So, since we're approaching 1.1, we should really discuss release criteria 
> for 
> 1.1 with respect to live migration.  I'd prefer to avoid surprises in this 
> release.
> 
> My expectation is that migration works from:
> 
> qemu-1.0 -M 1.0 =>qemu-1.1 -M 1.1
> qemu-1.1 -M 1.0 <=qemu-1.1 -M 1.0

Besides the instance ID thing, I found two further blockers:

 - kvm-tpr-op (kvmvapic), easy to disable in non-1.1 machines

 - vmstate fix for i8254 which involved a version bump from 2 to 3.
   That is actually now compatible with qemu-kvm and should not cause
   troubles there. But it breaks the backward-migration scenario for
   QEMU. I have no good idea how to resolve this while pleasing all
   consumers we care about. Any suggestions?

Jan

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

[Qemu-devel] [PATCH] vapic: Disable for pre-1.1 machines

[Jan Kiszka]

The kvmvapic was not present in older QEMU versions, thus must be
disabled in compat machines.

Signed-off-by: Jan Kiszka 
---
 hw/pc_piix.c |   43 +--
 1 files changed, 33 insertions(+), 10 deletions(-)

diff --git a/hw/pc_piix.c b/hw/pc_piix.c
index 3f99f9a..8061960 100644
--- a/hw/pc_piix.c
+++ b/hw/pc_piix.c
@@ -386,6 +386,10 @@ static QEMUMachine pc_machine_v1_0 = {
 .driver   = "isa-fdc",
 .property = "check_media_rate",
 .value= "off",
+},{
+.driver   = "apic",
+.property = "vapic",
+.value= "off",
 },
 { /* end of list */ }
 },
@@ -405,6 +409,10 @@ static QEMUMachine pc_machine_v0_15 = {
 .driver   = "isa-fdc",
 .property = "check_media_rate",
 .value= "off",
+},{
+.driver   = "apic",
+.property = "vapic",
+.value= "off",
 },
 { /* end of list */ }
 },
@@ -444,11 +452,14 @@ static QEMUMachine pc_machine_v0_14 = {
 .driver   = "isa-fdc",
 .property = "check_media_rate",
 .value= "off",
-},
-{
+},{
 .driver   = "pc-sysfw",
 .property = "rom_only",
 .value= stringify(1),
+},{
+.driver   = "apic",
+.property = "vapic",
+.value= "off",
 },
 { /* end of list */ }
 },
@@ -500,11 +511,14 @@ static QEMUMachine pc_machine_v0_13 = {
 .driver   = "isa-fdc",
 .property = "check_media_rate",
 .value= "off",
-},
-{
+},{
 .driver   = "pc-sysfw",
 .property = "rom_only",
 .value= stringify(1),
+},{
+.driver   = "apic",
+.property = "vapic",
+.value= "off",
 },
 { /* end of list */ }
 },
@@ -560,11 +574,14 @@ static QEMUMachine pc_machine_v0_12 = {
 .driver   = "isa-fdc",
 .property = "check_media_rate",
 .value= "off",
-},
-{
+},{
 .driver   = "pc-sysfw",
 .property = "rom_only",
 .value= stringify(1),
+},{
+.driver   = "apic",
+.property = "vapic",
+.value= "off",
 },
 { /* end of list */ }
 }
@@ -628,11 +645,14 @@ static QEMUMachine pc_machine_v0_11 = {
 .driver   = "isa-fdc",
 .property = "check_media_rate",
 .value= "off",
-},
-{
+},{
 .driver   = "pc-sysfw",
 .property = "rom_only",
 .value= stringify(1),
+},{
+.driver   = "apic",
+.property = "vapic",
+.value= "off",
 },
 { /* end of list */ }
 }
@@ -708,11 +728,14 @@ static QEMUMachine pc_machine_v0_10 = {
 .driver   = "isa-fdc",
 .property = "check_media_rate",
 .value= "off",
-},
-{
+},{
 .driver   = "pc-sysfw",
 .property = "rom_only",
 .value= stringify(1),
+},{
+.driver   = "apic",
+.property = "vapic",
+.value= "off",
 },
 { /* end of list */ }
 },
-- 
1.7.3.4

Re: [Qemu-devel] [PATCH] Kick io-thread on qemu_chr_accept_input

[Jan Kiszka]

On 2012-03-16 14:18, Jan Kiszka wrote:
> On 2012-03-16 14:16, Anthony Liguori wrote:
>> On 03/16/2012 07:25 AM, Jan Kiszka wrote:
>>> Once a chr frontend is able to receive input again, we need to inform
>>> the io-thread about this fact. Otherwise, main_loop_wait may continue to
>>> select without the related backend file descriptor in its set. This can
>>> cause high input latencies if only low-rate events arrive otherwise.
>>>
>>> Signed-off-by: Jan Kiszka
>>
>> I'm not nacking this patch, but please note that this is a band-aid as not 
>> all 
>> char devices actually use qemu_chr_accept_input().
> 
> Then they have to be fixed. :)

Could you apply the patch in the meantime? It is not band-aid, but a
required base to fix such issues.

Jan

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

[Qemu-devel] [PATCH v2] ARM: Permit any ARMv6K CPU to read the MVFR0 and MVFR1 VFP registers.

[Andrew Towers]

This patch replaces the ARM_FEATURE_VFP3 test when reading MVFR registers
with a test for a new feature flag ARM_FEATURE_MVFR, and sets this feature
for all ARMv6K cores (ARM1156 is not a v6K core, yet supports MVFR; qemu
does not support ARM1156 at this time.)

MVFR0 and MVFR1 were introduced in ARM1136JF-S r1p0 (ARMv6K, VFPv2) and are
present in ARM1156T2F-S (non-v6K), ARM1176JZF-S, ARM11MPCore and newer cores.
Reference: ARM DDI 0211H, 0290G, 0301H, 0360E.
http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.ddi0211h/Ffbefjag.html

Without this change, the linux kernel will not boot with VFP support enabled
under ARM1176 system emulation, due to the unconditional use of MVFR1 at the
end of vfp_init() in arch/arm/vfp/vfpmodule.c:

  VFP support v0.3: implemetor 41 architecture 1 part 20 variant b rev 5
  Internal error: Oops - undefined instruction: 0 [#1]

Signed-off-by: Andrew Towers 
---

v2:
 * introduced ARM_FEATURE_MVFR, implied by ARM_FEATURE_V6K.

Paul: I'd love to work on an rPi board model, and I'll see what I can put
together, but much of the hardware is under NDA and I'm not in the loop..


 target-arm/cpu.h   |1 +
 target-arm/helper.c|1 +
 target-arm/translate.c |2 +-
 3 files changed, 3 insertions(+), 1 deletions(-)

diff --git a/target-arm/cpu.h b/target-arm/cpu.h
index 26c114b..4cbd389 100644
--- a/target-arm/cpu.h
+++ b/target-arm/cpu.h
@@ -383,6 +383,7 @@ enum arm_features {
 ARM_FEATURE_ARM_DIV, /* divide supported in ARM encoding */
 ARM_FEATURE_VFP4, /* VFPv4 (implies that NEON is v2) */
 ARM_FEATURE_GENERIC_TIMER,
+ARM_FEATURE_MVFR, /* Media and VFP Feature Registers 0 and 1 */
 };
 
 static inline int arm_feature(CPUARMState *env, int feature)
diff --git a/target-arm/helper.c b/target-arm/helper.c
index 1314f23..837e9b0 100644
--- a/target-arm/helper.c
+++ b/target-arm/helper.c
@@ -254,6 +254,7 @@ static void cpu_reset_model_id(CPUARMState *env, uint32_t 
id)
 }
 if (arm_feature(env, ARM_FEATURE_V6K)) {
 set_feature(env, ARM_FEATURE_V6);
+set_feature(env, ARM_FEATURE_MVFR);
 }
 if (arm_feature(env, ARM_FEATURE_V6)) {
 set_feature(env, ARM_FEATURE_V5);
diff --git a/target-arm/translate.c b/target-arm/translate.c
index b5861c8..46d1d3e 100644
--- a/target-arm/translate.c
+++ b/target-arm/translate.c
@@ -2906,7 +2906,7 @@ static int disas_vfp_insn(CPUARMState * env, DisasContext 
*s, uint32_t insn)
 case ARM_VFP_MVFR0:
 case ARM_VFP_MVFR1:
 if (IS_USER(s)
-|| !arm_feature(env, ARM_FEATURE_VFP3))
+|| !arm_feature(env, ARM_FEATURE_MVFR))
 return 1;
 tmp = load_cpu_field(vfp.xregs[rn]);
 break;
-- 
1.7.5.4

Re: [Qemu-devel] Moniter The Ram Access On QEMU

[陳韋任]

> Now I have some probem with the exception handle. I found the funciton
> "do_interrupt" in "target-arm/helper.c" and "gen_exception" in
> "target-arm/translate.c" seems to handle the exceptions. But I am comfused
> with those two functions, which function will do the really work about
> generating an exception. As my understanding, I think “do_interrupt” seems
> to do some preparations before enter exceptions just like operate some
> registers and accumulate the exception handler address. And "gen_exception"
> seems to do the read job to generate a exception. But I'm not sure.

  Interrupt is a "external" source which breaks the current execution flow of
the CPU. For example, a device might raise an interrupt so that CPU can know the
device has completed its task. On the other hand, exception is an "internal"
source which breaks the current execution flow of the CPU. Take divide by zero
as an example, CPU will detect this error (occurred in the CPU itself). So
depends on what you mean "exception", do_interrupt or gen_exception might does
the real work.

Regards,
chenwj
  
-- 
Wei-Ren Chen (陳韋任)
Computer Systems Lab, Institute of Information Science,
Academia Sinica, Taiwan (R.O.C.)
Tel:886-2-2788-3799 #1667
Homepage: http://people.cs.nctu.edu.tw/~chenwj

[Qemu-devel] [PATCHv2] piix: fix up/down races + document

[Michael S. Tsirkin]

piix acpi interface suffers from the following 2 issues:

1.
- delete device a
- quickly add device b in another slot

if we do this before guest reads the down register,
the down event is discarded and device will never
be deleted.

2.
- delete device a
- quickly reset before guest can respond

interrupt is reset and guest will never eject the device.

To fix this, we implement two changes:

1. Add two new registers:
CLR_UP
CLR_DOWN
bios will now write to these the value it read from UP/DOWN

2. on reset, remove all devices which have DOWN bit set

For compatibility with old guests, we also clear
the DOWN bit on write to EJ0 for a device.

This patch also adds more detailed documentation
for the ACPI interface, including the semantics
of both old and new registers.

Signed-off-by: Michael S. Tsirkin 
---

Changes from v1:
- tweaked a comment about clearing down register on reset
- documentation update

 docs/specs/acpi_pci_hotplug.txt |   68 +++
 hw/acpi_piix4.c |   75 +++---
 2 files changed, 121 insertions(+), 22 deletions(-)

diff --git a/docs/specs/acpi_pci_hotplug.txt b/docs/specs/acpi_pci_hotplug.txt
index f0f74a7..a8398f9 100644
--- a/docs/specs/acpi_pci_hotplug.txt
+++ b/docs/specs/acpi_pci_hotplug.txt
@@ -7,31 +7,83 @@ describes the interface between QEMU and the ACPI BIOS.
 ACPI GPE block (IO ports 0xafe0-0xafe3, byte access):
 -
 
-Generic ACPI GPE block. Bit 1 (GPE.1) used to notify PCI hotplug/eject
+Generic ACPI GPE block. Bit 1 (GPE.1) is used to notify PCI hotplug/hotunplug
 event to ACPI BIOS, via SCI interrupt.
 
-PCI slot injection notification pending (IO port 0xae00-0xae03, 4-byte access):
+Semantics: this event occurs each time a bit in either Pending PCI slot
+injection notification or Pending PCI slot removal notification registers
+changes status from 0 to 1.
+
+Pending PCI slot injection notification (IO port 0xae00-0xae03, 4-byte access):
 ---
-Slot injection notification pending. One bit per slot.
+Slot injection notification is pending. One bit per slot.
+Guests should not write this register, in practice the value
+written overwrites the register.
 
 Read by ACPI BIOS GPE.1 handler to notify OS of injection
 events.
 
-PCI slot removal notification (IO port 0xae04-0xae07, 4-byte access):
+Semantics: after a slot is populated by hotplug, it is immediately enabled
+and a bit in this register is set.
+Reset value: 0
+
+Pending PCI slot removal notification (IO port 0xae04-0xae07, 4-byte access):
 -
-Slot removal notification pending. One bit per slot.
+Slot removal notification is pending. One bit per slot.
+Guests should not write this register, in practice the value
+written overwrites the register.
 
 Read by ACPI BIOS GPE.1 handler to notify OS of removal
 events.
 
+Semantics: upon hotunplug request, a bit in this register is set
+and the OSPM is notified about hotunplug request, but a slot remains enabled
+until eject.
+Reset value: 0
+
 PCI device eject (IO port 0xae08-0xae0b, 4-byte access):
 
 
 Used by ACPI BIOS _EJ0 method to request device removal. One bit per slot.
-Reads return 0.
+Guests should not read this register. In practice reads return 0.
+
+Semantics: selected hotunpluggable slots are disabled and powered off.
+Has no effect on non-hotunpluggable slots.
+
+For compatibility with old BIOS, writes to this register
+clear bits in pending slot injection notification register.
 
 PCI removability status (IO port 0xae0c-0xae0f, 4-byte access):
 ---
 
-Used by ACPI BIOS _RMV method to indicate removability status to OS. One
-bit per slot.
+Used by ACPI BIOS _RMV method to detect removability status
+and report to OS. One bit per slot.
+Guests should not write this register, in practice the value
+written is discarded.
+
+Semantics: selected slots support hotplug. Contents of this register
+do not change while guest is running.
+
+Clear PCI slot injection notification (IO port 0xae10-0xae14, 4-byte access):
+---
+Clears bits in pending slot injection notification register. One bit per slot.
+Guests should not read this register. In practice reads return 0.
+
+Written to by ACPI BIOS GPE.1 handler after reading the
+pending slot injection notification register and before
+notifying OS of injection events.
+
+Semantics: used to acknowledge the injection notification.
+Does not affect slot status.
+
+Clear PCI slot removal notification (IO port 0xae04-0xae07, 4-byte access):
+-
+Clears bits in slot removal notification pending register. One bit per slot.
+Guests should not read this register. In practice reads return 0.
+
+Written to by ACPI BIOS 

Re: [Qemu-devel] [PATCHv2] piix: fix up/down races + document

[Gleb Natapov]

On Thu, Mar 29, 2012 at 02:51:44PM +0200, Michael S. Tsirkin wrote:
> piix acpi interface suffers from the following 2 issues:
> 
> 1.
> - delete device a
> - quickly add device b in another slot
> 
> if we do this before guest reads the down register,
> the down event is discarded and device will never
> be deleted.
> 
> 2.
> - delete device a
> - quickly reset before guest can respond
> 
> interrupt is reset and guest will never eject the device.
> 
> To fix this, we implement two changes:
> 
> 1. Add two new registers:
> CLR_UP
> CLR_DOWN
> bios will now write to these the value it read from UP/DOWN
> 
> 2. on reset, remove all devices which have DOWN bit set
> 
> For compatibility with old guests, we also clear
> the DOWN bit on write to EJ0 for a device.
> 
> This patch also adds more detailed documentation
> for the ACPI interface, including the semantics
> of both old and new registers.
> 
> Signed-off-by: Michael S. Tsirkin 
> ---
> 
> Changes from v1:
> - tweaked a comment about clearing down register on reset
> - documentation update
> 
>  docs/specs/acpi_pci_hotplug.txt |   68 +++
>  hw/acpi_piix4.c |   75 +++---
>  2 files changed, 121 insertions(+), 22 deletions(-)
> 
> diff --git a/docs/specs/acpi_pci_hotplug.txt b/docs/specs/acpi_pci_hotplug.txt
> index f0f74a7..a8398f9 100644
> --- a/docs/specs/acpi_pci_hotplug.txt
> +++ b/docs/specs/acpi_pci_hotplug.txt
> @@ -7,31 +7,83 @@ describes the interface between QEMU and the ACPI BIOS.
>  ACPI GPE block (IO ports 0xafe0-0xafe3, byte access):
>  -
>  
> -Generic ACPI GPE block. Bit 1 (GPE.1) used to notify PCI hotplug/eject
> +Generic ACPI GPE block. Bit 1 (GPE.1) is used to notify PCI hotplug/hotunplug
>  event to ACPI BIOS, via SCI interrupt.
>  
> -PCI slot injection notification pending (IO port 0xae00-0xae03, 4-byte 
> access):
> +Semantics: this event occurs each time a bit in either Pending PCI slot
> +injection notification or Pending PCI slot removal notification registers
> +changes status from 0 to 1.
> +
> +Pending PCI slot injection notification (IO port 0xae00-0xae03, 4-byte 
> access):
>  ---
> -Slot injection notification pending. One bit per slot.
> +Slot injection notification is pending. One bit per slot.
> +Guests should not write this register, in practice the value
> +written overwrites the register.
>  
>  Read by ACPI BIOS GPE.1 handler to notify OS of injection
>  events.
>  
> -PCI slot removal notification (IO port 0xae04-0xae07, 4-byte access):
> +Semantics: after a slot is populated by hotplug, it is immediately enabled
> +and a bit in this register is set.
> +Reset value: 0
> +
> +Pending PCI slot removal notification (IO port 0xae04-0xae07, 4-byte access):
>  -
> -Slot removal notification pending. One bit per slot.
> +Slot removal notification is pending. One bit per slot.
> +Guests should not write this register, in practice the value
> +written overwrites the register.
>  
>  Read by ACPI BIOS GPE.1 handler to notify OS of removal
>  events.
>  
> +Semantics: upon hotunplug request, a bit in this register is set
> +and the OSPM is notified about hotunplug request, but a slot remains enabled
> +until eject.
> +Reset value: 0
> +
>  PCI device eject (IO port 0xae08-0xae0b, 4-byte access):
>  
>  
>  Used by ACPI BIOS _EJ0 method to request device removal. One bit per slot.
> -Reads return 0.
> +Guests should not read this register. In practice reads return 0.
> +
> +Semantics: selected hotunpluggable slots are disabled and powered off.
> +Has no effect on non-hotunpluggable slots.
As it's implemented right now only one slot should be ejected at a time.

Looks good otherwise.

> +
> +For compatibility with old BIOS, writes to this register
> +clear bits in pending slot injection notification register.
>  
>  PCI removability status (IO port 0xae0c-0xae0f, 4-byte access):
>  ---
>  
> -Used by ACPI BIOS _RMV method to indicate removability status to OS. One
> -bit per slot.
> +Used by ACPI BIOS _RMV method to detect removability status
> +and report to OS. One bit per slot.
> +Guests should not write this register, in practice the value
> +written is discarded.
> +
> +Semantics: selected slots support hotplug. Contents of this register
> +do not change while guest is running.
> +
> +Clear PCI slot injection notification (IO port 0xae10-0xae14, 4-byte access):
> +---
> +Clears bits in pending slot injection notification register. One bit per 
> slot.
> +Guests should not read this register. In practice reads return 0.
> +
> +Written to by ACPI BIOS GPE.1 handler after reading the
> +pending slot injection notification register and before
> +notifying 

Re: [Qemu-devel] [PATCH v2] ARM: Permit any ARMv6K CPU to read the MVFR0 and MVFR1 VFP registers.

[Andreas Färber]

Am 29.03.2012 14:41, schrieb Andrew Towers:
> This patch replaces the ARM_FEATURE_VFP3 test when reading MVFR registers
> with a test for a new feature flag ARM_FEATURE_MVFR, and sets this feature
> for all ARMv6K cores (ARM1156 is not a v6K core, yet supports MVFR; qemu
> does not support ARM1156 at this time.)
> 
> MVFR0 and MVFR1 were introduced in ARM1136JF-S r1p0 (ARMv6K, VFPv2) and are
> present in ARM1156T2F-S (non-v6K), ARM1176JZF-S, ARM11MPCore and newer cores.
> Reference: ARM DDI 0211H, 0290G, 0301H, 0360E.
> http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.ddi0211h/Ffbefjag.html
> 
> Without this change, the linux kernel will not boot with VFP support enabled
> under ARM1176 system emulation, due to the unconditional use of MVFR1 at the
> end of vfp_init() in arch/arm/vfp/vfpmodule.c:
> 
>   VFP support v0.3: implemetor 41 architecture 1 part 20 variant b rev 5
>   Internal error: Oops - undefined instruction: 0 [#1]
> 
> Signed-off-by: Andrew Towers 

Feature inference looks good,

Reviewed-by: Andreas Färber 

> ---
> 
> v2:
>  * introduced ARM_FEATURE_MVFR, implied by ARM_FEATURE_V6K.
> 
> Paul: I'd love to work on an rPi board model, and I'll see what I can put
> together, but much of the hardware is under NDA and I'm not in the loop..

You might find the partial bcm2835 manual helpful:
http://www.raspberrypi.org/archives/615

Open-source drivers (esp. Linux) are also often a helpful source for
emulating NDA'ed hardware.

Regards,
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/2] qdev: qdev_unplug(): Use error_set()

[Luiz Capitulino]

On Thu, 29 Mar 2012 08:00:15 +0100
Stefan Hajnoczi  wrote:

> On Wed, Mar 28, 2012 at 05:50:53PM -0300, Luiz Capitulino wrote:
> > @@ -268,7 +270,14 @@ static int pci_device_hot_remove(Monitor *mon, const 
> > char *pci_addr)
> >  monitor_printf(mon, "slot %d empty\n", slot);
> >  return -1;
> >  }
> > -return qdev_unplug(&d->qdev);
> > +
> > +ret = qdev_unplug(&d->qdev, &errp);
> > +if (error_is_set(&errp)) {
> > +monitor_printf(mon, "%s\n", error_get_pretty(errp));
> > +error_free(errp);
> > +}
> 
> Minor thing if you respin: this if statement could be replaced with 
> hmp_handle_error(mon, &errp).

I'm not sure I'd like to see hmp_handle_error() spread over the tree. It uses
the monitor object and I've added it just because having the same code
duplicated among HMP functions bothered me... I think it's better to
restrict it to hmp.c.

Re: [Qemu-devel] [PATCH 2/2] qapi: convert device_del

[Luiz Capitulino]

On Thu, 29 Mar 2012 08:08:51 +0100
Stefan Hajnoczi  wrote:

> On Wed, Mar 28, 2012 at 05:50:54PM -0300, Luiz Capitulino wrote:
> >  ret = qdev_unplug(dev, &local_err);
> >  if (error_is_set(&local_err)) {
> > -qerror_report_err(local_err);
> > -error_free(local_err);
> > +error_propagate(errp, local_err);
> > +} else if (ret) {
> > +error_set(errp, QERR_UNDEFINED_ERROR);
> 
> Can we make qdev_unplug() void?  I can find no case in QEMU where we
> return != 0 without setting error.  If we fix the function prototype
> this invalid state can be eliminated forever.

Good point, I'll change it.

> 
> (Other functions that take Error **errp are usually void.)
> 
> > diff --git a/qapi-schema.json b/qapi-schema.json
> > index 0d11d6e..ace55f3 100644
> > --- a/qapi-schema.json
> > +++ b/qapi-schema.json
> > @@ -1701,3 +1701,23 @@
> >  # Since: 1.1
> >  ##
> >  { 'command': 'xen-save-devices-state', 'data': {'filename': 'str'} }
> > +
> > +##
> > +# @device_del:
> > +#
> > +# Remove a device from a guest
> > +#
> > +# @id: the name of the device
> > +#
> > +# Returns: Nothing on success
> > +#  If @id is not a valid device, DeviceNotFound
> > +#  If the device does not support unplug, BusNoHotplug
> > +#
> > +# Notes: When this command completes, the device may not be removed from 
> > the
> > +#guest.  Hot removal is an operation that requires guest 
> > cooperation.
> > +#This command merely requests that the guest begin the hot removal
> > +#process.
> 
> I have not peeked at the implementation in QEMU or libvirt, but is there
> a QMP event for actual removal or would the user need to poll?  This bit
> of information would be useful in the documentation.

There's no event, I'll document it. Is there any preferred method for
polling? query-pci?

Re: [Qemu-devel] [PATCH V3 06/12] Add helper functions for MIPS DSP GPR-Based Shift instructions

[Richard Henderson]

On 03/27/2012 09:43 PM, Jia Liu wrote:
> On Wed, Mar 28, 2012 at 12:11 AM, Richard Henderson  wrote:
>> On 03/27/12 02:24, Jia Liu wrote:
>>> +DEF_HELPER_FLAGS_2(shll_qb, TCG_CALL_CONST | TCG_CALL_PURE, i32, int, i32)
>>
> 
> It should be DEF_HELPER_2(shll_qb, i32, int, i32). Is it?
> Sorry I'm not sure about it.

Exactly.


r~

Re: [Qemu-devel] [PATCH V3 11/12] Handle MIPS DSP instructions in target-mips/translate.c

[Richard Henderson]

On 03/27/2012 09:36 PM, Jia Liu wrote:
>> > This patch should have been split as well, adding translations of the insns
>> > at the same time you add the helpers.  That said, the actual code looks ok.
>> >
> helpers can be grouped by MIPS DSP manual, BUT translations have to
> grouped by opcode.
> There is some difference between them, I'll try to do this.
> 

One possibility is to add the bare skeleton of the decoding by opcode first,
leaving out the actual case statements.  Then add the

  case OPCODE:
gen_helper();
break;

in the patch adding the helpers.


r~

Re: [Qemu-devel] [PATCHv2] piix: fix up/down races + document

[Michael S. Tsirkin]

On Thu, Mar 29, 2012 at 03:04:12PM +0200, Gleb Natapov wrote:
> On Thu, Mar 29, 2012 at 02:51:44PM +0200, Michael S. Tsirkin wrote:
> > piix acpi interface suffers from the following 2 issues:
> > 
> > 1.
> > - delete device a
> > - quickly add device b in another slot
> > 
> > if we do this before guest reads the down register,
> > the down event is discarded and device will never
> > be deleted.
> > 
> > 2.
> > - delete device a
> > - quickly reset before guest can respond
> > 
> > interrupt is reset and guest will never eject the device.
> > 
> > To fix this, we implement two changes:
> > 
> > 1. Add two new registers:
> > CLR_UP
> > CLR_DOWN
> > bios will now write to these the value it read from UP/DOWN
> > 
> > 2. on reset, remove all devices which have DOWN bit set
> > 
> > For compatibility with old guests, we also clear
> > the DOWN bit on write to EJ0 for a device.
> > 
> > This patch also adds more detailed documentation
> > for the ACPI interface, including the semantics
> > of both old and new registers.
> > 
> > Signed-off-by: Michael S. Tsirkin 
> > ---
> > 
> > Changes from v1:
> > - tweaked a comment about clearing down register on reset
> > - documentation update
> > 
> >  docs/specs/acpi_pci_hotplug.txt |   68 +++
> >  hw/acpi_piix4.c |   75 
> > +++---
> >  2 files changed, 121 insertions(+), 22 deletions(-)
> > 
> > diff --git a/docs/specs/acpi_pci_hotplug.txt 
> > b/docs/specs/acpi_pci_hotplug.txt
> > index f0f74a7..a8398f9 100644
> > --- a/docs/specs/acpi_pci_hotplug.txt
> > +++ b/docs/specs/acpi_pci_hotplug.txt
> > @@ -7,31 +7,83 @@ describes the interface between QEMU and the ACPI BIOS.
> >  ACPI GPE block (IO ports 0xafe0-0xafe3, byte access):
> >  -
> >  
> > -Generic ACPI GPE block. Bit 1 (GPE.1) used to notify PCI hotplug/eject
> > +Generic ACPI GPE block. Bit 1 (GPE.1) is used to notify PCI 
> > hotplug/hotunplug
> >  event to ACPI BIOS, via SCI interrupt.
> >  
> > -PCI slot injection notification pending (IO port 0xae00-0xae03, 4-byte 
> > access):
> > +Semantics: this event occurs each time a bit in either Pending PCI slot
> > +injection notification or Pending PCI slot removal notification registers
> > +changes status from 0 to 1.
> > +
> > +Pending PCI slot injection notification (IO port 0xae00-0xae03, 4-byte 
> > access):
> >  ---
> > -Slot injection notification pending. One bit per slot.
> > +Slot injection notification is pending. One bit per slot.
> > +Guests should not write this register, in practice the value
> > +written overwrites the register.
> >  
> >  Read by ACPI BIOS GPE.1 handler to notify OS of injection
> >  events.
> >  
> > -PCI slot removal notification (IO port 0xae04-0xae07, 4-byte access):
> > +Semantics: after a slot is populated by hotplug, it is immediately enabled
> > +and a bit in this register is set.
> > +Reset value: 0
> > +
> > +Pending PCI slot removal notification (IO port 0xae04-0xae07, 4-byte 
> > access):
> >  -
> > -Slot removal notification pending. One bit per slot.
> > +Slot removal notification is pending. One bit per slot.
> > +Guests should not write this register, in practice the value
> > +written overwrites the register.
> >  
> >  Read by ACPI BIOS GPE.1 handler to notify OS of removal
> >  events.
> >  
> > +Semantics: upon hotunplug request, a bit in this register is set
> > +and the OSPM is notified about hotunplug request, but a slot remains 
> > enabled
> > +until eject.
> > +Reset value: 0
> > +
> >  PCI device eject (IO port 0xae08-0xae0b, 4-byte access):
> >  
> >  
> >  Used by ACPI BIOS _EJ0 method to request device removal. One bit per slot.
> > -Reads return 0.
> > +Guests should not read this register. In practice reads return 0.
> > +
> > +Semantics: selected hotunpluggable slots are disabled and powered off.
> > +Has no effect on non-hotunpluggable slots.
> As it's implemented right now only one slot should be ejected at a time.

Right. I'll document that as well.

> Looks good otherwise.
> 
> > +
> > +For compatibility with old BIOS, writes to this register
> > +clear bits in pending slot injection notification register.
> >  
> >  PCI removability status (IO port 0xae0c-0xae0f, 4-byte access):
> >  ---
> >  
> > -Used by ACPI BIOS _RMV method to indicate removability status to OS. One
> > -bit per slot.
> > +Used by ACPI BIOS _RMV method to detect removability status
> > +and report to OS. One bit per slot.
> > +Guests should not write this register, in practice the value
> > +written is discarded.
> > +
> > +Semantics: selected slots support hotplug. Contents of this register
> > +do not change while guest is running.
> > +
> > +Clear PCI slot injection notification (IO port 0xae10-0xae14, 4-b

Re: [Qemu-devel] [PATCH 0/2] ide: convert pio code path to asynchronous I/O

[Stefan Hajnoczi]

On Thu, Mar 29, 2012 at 12:30 PM, Chris Webb  wrote:
> Stefan Hajnoczi  writes:
>
>> Chris and Richard: Please test this to confirm that it fixes the hang you
>> reported.
>
> We've been testing this (v1 against qemu-kvm 1.0) today, and it's looking
> very good. Thanks!
>
> The lock-ups during boot no longer happen, and if you severely throttle
> (1MB/s, 100 req/s) a guest in the middle of a big dd, there are lots of
> nasty kernel messages as it falls back to pio mode, but the guest doesn't
> lock up and qemu remains responsive.

Thanks for trying it out.

Regarding the guest kernel errors, they may be timeouts.  The guest
may consider the IDE controller buggy/dead due to how long requests
take to complete under severe throttling.  There's not much that can
be done about that, I think.  If you think the guest errors are due to
something else though, please post the error messages.

Stefan

Re: [Qemu-devel] [PATCH 2/2] qapi: convert device_del

[Stefan Hajnoczi]

On Thu, Mar 29, 2012 at 2:17 PM, Luiz Capitulino  wrote:
> On Thu, 29 Mar 2012 08:08:51 +0100
> Stefan Hajnoczi  wrote:
>
>> On Wed, Mar 28, 2012 at 05:50:54PM -0300, Luiz Capitulino wrote:
>> >      ret = qdev_unplug(dev, &local_err);
>> >      if (error_is_set(&local_err)) {
>> > -        qerror_report_err(local_err);
>> > -        error_free(local_err);
>> > +        error_propagate(errp, local_err);
>> > +    } else if (ret) {
>> > +        error_set(errp, QERR_UNDEFINED_ERROR);
>>
>> Can we make qdev_unplug() void?  I can find no case in QEMU where we
>> return != 0 without setting error.  If we fix the function prototype
>> this invalid state can be eliminated forever.
>
> Good point, I'll change it.
>
>>
>> (Other functions that take Error **errp are usually void.)
>>
>> > diff --git a/qapi-schema.json b/qapi-schema.json
>> > index 0d11d6e..ace55f3 100644
>> > --- a/qapi-schema.json
>> > +++ b/qapi-schema.json
>> > @@ -1701,3 +1701,23 @@
>> >  # Since: 1.1
>> >  ##
>> >  { 'command': 'xen-save-devices-state', 'data': {'filename': 'str'} }
>> > +
>> > +##
>> > +# @device_del:
>> > +#
>> > +# Remove a device from a guest
>> > +#
>> > +# @id: the name of the device
>> > +#
>> > +# Returns: Nothing on success
>> > +#          If @id is not a valid device, DeviceNotFound
>> > +#          If the device does not support unplug, BusNoHotplug
>> > +#
>> > +# Notes: When this command completes, the device may not be removed from 
>> > the
>> > +#        guest.  Hot removal is an operation that requires guest 
>> > cooperation.
>> > +#        This command merely requests that the guest begin the hot removal
>> > +#        process.
>>
>> I have not peeked at the implementation in QEMU or libvirt, but is there
>> a QMP event for actual removal or would the user need to poll?  This bit
>> of information would be useful in the documentation.
>
> There's no event, I'll document it. Is there any preferred method for
> polling? query-pci?

I took a quick peek at libvirt and am none the wiser.  If we don't
know what the recommended approach is then let's leave it.

Stefan

Re: [Qemu-devel] [PATCH 1/2] qdev: qdev_unplug(): Use error_set()

[Stefan Hajnoczi]

On Thu, Mar 29, 2012 at 2:15 PM, Luiz Capitulino  wrote:
> On Thu, 29 Mar 2012 08:00:15 +0100
> Stefan Hajnoczi  wrote:
>
>> On Wed, Mar 28, 2012 at 05:50:53PM -0300, Luiz Capitulino wrote:
>> > @@ -268,7 +270,14 @@ static int pci_device_hot_remove(Monitor *mon, const 
>> > char *pci_addr)
>> >          monitor_printf(mon, "slot %d empty\n", slot);
>> >          return -1;
>> >      }
>> > -    return qdev_unplug(&d->qdev);
>> > +
>> > +    ret = qdev_unplug(&d->qdev, &errp);
>> > +    if (error_is_set(&errp)) {
>> > +        monitor_printf(mon, "%s\n", error_get_pretty(errp));
>> > +        error_free(errp);
>> > +    }
>>
>> Minor thing if you respin: this if statement could be replaced with 
>> hmp_handle_error(mon, &errp).
>
> I'm not sure I'd like to see hmp_handle_error() spread over the tree. It uses
> the monitor object and I've added it just because having the same code
> duplicated among HMP functions bothered me... I think it's better to
> restrict it to hmp.c.

Okay.  I mentioned it because I noticed that there are several
different ways to do essentially the same thing.

Stefan

Re: [Qemu-devel] [PATCH v2] ARM: Permit any ARMv6K CPU to read the MVFR0 and MVFR1 VFP registers.

[Peter Maydell]

On 29 March 2012 13:41, Andrew Towers  wrote:
> This patch replaces the ARM_FEATURE_VFP3 test when reading MVFR registers
> with a test for a new feature flag ARM_FEATURE_MVFR, and sets this feature
> for all ARMv6K cores (ARM1156 is not a v6K core, yet supports MVFR; qemu
> does not support ARM1156 at this time.)

Reviewed-by: Peter Maydell 
and put into target-arm.next. I'll probably do a pullreq tomorrow.

Thanks!
-- PMM

Re: [Qemu-devel] [PATCH v1 1/2] pl330: initial version

[Kirill Batuzov]

On Thu, 29 Mar 2012, Peter A. G. Crosthwaite wrote:

> Device model for Primecell PL330 dma controller.
> 
> Signed-off-by: Peter A. G. Crosthwaite 
> ---

Signed-off-by: Kirill Batuzov 

> +static int PL330Fifo_get(PL330Fifo *s, uint8_t *buf, int len, uint8_t tag)
> +{
> +int i, ret;
> +int old_s, old_t;
> +
> +old_s = s->s;
> +old_t = s->t;
> +for (i = 0; i < len; i++) {
> +if (s->t != s->s && s->tag[s->s] == tag) {
> +buf[i] = s->buf[s->s];
> +s->s = PL330Fifo_inc(s->s, s->buf_size);
> +} else {
> +/* Rollback transaction */
> +ret = (s->t == s->s) ? ret = PL330FIFO_STALL : PL330FIFO_ERR;
Minor issue - double assignement of ret in this line.
> +s->s = old_s;
> +s->t = old_t;
> +return ret;
> +}
> +}
> +return PL330FIFO_OK;
> +}

-- 
Kirill Batuzov

[Qemu-devel] [PATCH 3/6] usb: use USBDescriptor for config descriptors.

[Gerd Hoffmann]

Add config descriptor substruct to USBDescriptor,
use it in the descriptor generator code.

Signed-off-by: Gerd Hoffmann 
---
 hw/usb/desc.c |   22 --
 hw/usb/desc.h |9 +
 2 files changed, 21 insertions(+), 10 deletions(-)

diff --git a/hw/usb/desc.c b/hw/usb/desc.c
index 5cecb25..3466968 100644
--- a/hw/usb/desc.c
+++ b/hw/usb/desc.c
@@ -78,22 +78,24 @@ int usb_desc_config(const USBDescConfig *conf, uint8_t 
*dest, size_t len)
 {
 uint8_t  bLength = 0x09;
 uint16_t wTotalLength = 0;
+USBDescriptor *d = (void *)dest;
 int i, rc;
 
 if (len < bLength) {
 return -1;
 }
 
-dest[0x00] = bLength;
-dest[0x01] = USB_DT_CONFIG;
-dest[0x04] = conf->bNumInterfaces;
-dest[0x05] = conf->bConfigurationValue;
-dest[0x06] = conf->iConfiguration;
-dest[0x07] = conf->bmAttributes;
-dest[0x08] = conf->bMaxPower;
+d->bLength  = bLength;
+d->bDescriptorType  = USB_DT_CONFIG;
+
+d->u.config.bNumInterfaces  = conf->bNumInterfaces;
+d->u.config.bConfigurationValue = conf->bConfigurationValue;
+d->u.config.iConfiguration  = conf->iConfiguration;
+d->u.config.bmAttributes= conf->bmAttributes;
+d->u.config.bMaxPower   = conf->bMaxPower;
 wTotalLength += bLength;
 
-/* handle grouped interfaces if any*/
+/* handle grouped interfaces if any */
 for (i = 0; i < conf->nif_groups; i++) {
 rc = usb_desc_iface_group(&(conf->if_groups[i]),
   dest + wTotalLength,
@@ -113,8 +115,8 @@ int usb_desc_config(const USBDescConfig *conf, uint8_t 
*dest, size_t len)
 wTotalLength += rc;
 }
 
-dest[0x02] = usb_lo(wTotalLength);
-dest[0x03] = usb_hi(wTotalLength);
+d->u.config.wTotalLength_lo = usb_lo(wTotalLength);
+d->u.config.wTotalLength_hi = usb_hi(wTotalLength);
 return wTotalLength;
 }
 
diff --git a/hw/usb/desc.h b/hw/usb/desc.h
index 15d0780..298e050 100644
--- a/hw/usb/desc.h
+++ b/hw/usb/desc.h
@@ -36,6 +36,15 @@ typedef struct USBDescriptor {
 uint8_t   bNumConfigurations;
 uint8_t   bReserved;
 } device_qualifier;
+struct {
+uint8_t   wTotalLength_lo;
+uint8_t   wTotalLength_hi;
+uint8_t   bNumInterfaces;
+uint8_t   bConfigurationValue;
+uint8_t   iConfiguration;
+uint8_t   bmAttributes;
+uint8_t   bMaxPower;
+} config;
 } u;
 } QEMU_PACKED USBDescriptor;
 
-- 
1.7.1

[Qemu-devel] [PATCH 1/6] usb: add USBDescriptor, use for device descriptors.

[Gerd Hoffmann]

This patch adds a new type for the binary representation of usb
descriptors.  It is put into use for the descriptor generator code
where the struct replaces the hard-coded offsets.

Signed-off-by: Gerd Hoffmann 
---
 hw/usb/desc.c |   37 +++--
 hw/usb/desc.h |   26 ++
 2 files changed, 45 insertions(+), 18 deletions(-)

diff --git a/hw/usb/desc.c b/hw/usb/desc.c
index 9847a75..de7e204 100644
--- a/hw/usb/desc.c
+++ b/hw/usb/desc.c
@@ -18,32 +18,33 @@ int usb_desc_device(const USBDescID *id, const 
USBDescDevice *dev,
 uint8_t *dest, size_t len)
 {
 uint8_t bLength = 0x12;
+USBDescriptor *d = (void *)dest;
 
 if (len < bLength) {
 return -1;
 }
 
-dest[0x00] = bLength;
-dest[0x01] = USB_DT_DEVICE;
+d->bLength = bLength;
+d->bDescriptorType = USB_DT_DEVICE;
 
-dest[0x02] = usb_lo(dev->bcdUSB);
-dest[0x03] = usb_hi(dev->bcdUSB);
-dest[0x04] = dev->bDeviceClass;
-dest[0x05] = dev->bDeviceSubClass;
-dest[0x06] = dev->bDeviceProtocol;
-dest[0x07] = dev->bMaxPacketSize0;
+d->u.device.bcdUSB_lo  = usb_lo(dev->bcdUSB);
+d->u.device.bcdUSB_hi  = usb_hi(dev->bcdUSB);
+d->u.device.bDeviceClass   = dev->bDeviceClass;
+d->u.device.bDeviceSubClass= dev->bDeviceSubClass;
+d->u.device.bDeviceProtocol= dev->bDeviceProtocol;
+d->u.device.bMaxPacketSize0= dev->bMaxPacketSize0;
+
+d->u.device.idVendor_lo= usb_lo(id->idVendor);
+d->u.device.idVendor_hi= usb_hi(id->idVendor);
+d->u.device.idProduct_lo   = usb_lo(id->idProduct);
+d->u.device.idProduct_hi   = usb_hi(id->idProduct);
+d->u.device.bcdDevice_lo   = usb_lo(id->bcdDevice);
+d->u.device.bcdDevice_hi   = usb_hi(id->bcdDevice);
+d->u.device.iManufacturer  = id->iManufacturer;
+d->u.device.iProduct   = id->iProduct;
+d->u.device.iSerialNumber  = id->iSerialNumber;
 
-dest[0x08] = usb_lo(id->idVendor);
-dest[0x09] = usb_hi(id->idVendor);
-dest[0x0a] = usb_lo(id->idProduct);
-dest[0x0b] = usb_hi(id->idProduct);
-dest[0x0c] = usb_lo(id->bcdDevice);
-dest[0x0d] = usb_hi(id->bcdDevice);
-dest[0x0e] = id->iManufacturer;
-dest[0x0f] = id->iProduct;
-dest[0x10] = id->iSerialNumber;
-
-dest[0x11] = dev->bNumConfigurations;
+d->u.device.bNumConfigurations = dev->bNumConfigurations;
 
 return bLength;
 }
diff --git a/hw/usb/desc.h b/hw/usb/desc.h
index d6e07ea..c5a242e 100644
--- a/hw/usb/desc.h
+++ b/hw/usb/desc.h
@@ -3,6 +3,32 @@
 
 #include 
 
+/* binary representation */
+typedef struct USBDescriptor {
+uint8_t   bLength;
+uint8_t   bDescriptorType;
+union {
+struct {
+uint8_t   bcdUSB_lo;
+uint8_t   bcdUSB_hi;
+uint8_t   bDeviceClass;
+uint8_t   bDeviceSubClass;
+uint8_t   bDeviceProtocol;
+uint8_t   bMaxPacketSize0;
+uint8_t   idVendor_lo;
+uint8_t   idVendor_hi;
+uint8_t   idProduct_lo;
+uint8_t   idProduct_hi;
+uint8_t   bcdDevice_lo;
+uint8_t   bcdDevice_hi;
+uint8_t   iManufacturer;
+uint8_t   iProduct;
+uint8_t   iSerialNumber;
+uint8_t   bNumConfigurations;
+} device;
+} u;
+} QEMU_PACKED USBDescriptor;
+
 struct USBDescID {
 uint16_t  idVendor;
 uint16_t  idProduct;
-- 
1.7.1

[Qemu-devel] [PATCH 0/6] usb: descriptor rework.

[Gerd Hoffmann]

  Hi,

This patch series reworks the usb descriptor handling in qemu.  It adds
a struct for the binary representation of usb descriptors.  It is put
into use for both generating usb descriptors (for emulated devices) and
parsing usb descriptors (usb-host driver).  Additionally the usb-host
parser code has been completely rewritten to simplify the logic and to
make it more robust.

please review,
  Gerd

Gerd Hoffmann (6):
  usb: add USBDescriptor, use for device descriptors.
  usb: use USBDescriptor for device qualifier descriptors.
  usb: use USBDescriptor for config descriptors.
  usb: use USBDescriptor for interface descriptors.
  usb: use USBDescriptor for endpoint descriptors.
  usb-host: rewrite usb_linux_update_endp_table

 hw/usb/desc.c   |  126 ++
 hw/usb/desc.h   |   63 +
 hw/usb/host-linux.c |  194 ++
 trace-events|6 ++
 4 files changed, 237 insertions(+), 152 deletions(-)

[Qemu-devel] [PATCH 5/6] usb: use USBDescriptor for endpoint descriptors.

[Gerd Hoffmann]

Add endpoint descriptor substruct to USBDescriptor,
use it in the descriptor generator code.

Signed-off-by: Gerd Hoffmann 
---
 hw/usb/desc.c |   20 +++-
 hw/usb/desc.h |9 +
 2 files changed, 20 insertions(+), 9 deletions(-)

diff --git a/hw/usb/desc.c b/hw/usb/desc.c
index 2b8febb..3c77368 100644
--- a/hw/usb/desc.c
+++ b/hw/usb/desc.c
@@ -200,21 +200,23 @@ int usb_desc_endpoint(const USBDescEndpoint *ep, uint8_t 
*dest, size_t len)
 {
 uint8_t bLength = ep->is_audio ? 0x09 : 0x07;
 uint8_t extralen = ep->extra ? ep->extra[0] : 0;
+USBDescriptor *d = (void *)dest;
 
 if (len < bLength + extralen) {
 return -1;
 }
 
-dest[0x00] = bLength;
-dest[0x01] = USB_DT_ENDPOINT;
-dest[0x02] = ep->bEndpointAddress;
-dest[0x03] = ep->bmAttributes;
-dest[0x04] = usb_lo(ep->wMaxPacketSize);
-dest[0x05] = usb_hi(ep->wMaxPacketSize);
-dest[0x06] = ep->bInterval;
+d->bLength  = bLength;
+d->bDescriptorType  = USB_DT_ENDPOINT;
+
+d->u.endpoint.bEndpointAddress  = ep->bEndpointAddress;
+d->u.endpoint.bmAttributes  = ep->bmAttributes;
+d->u.endpoint.wMaxPacketSize_lo = usb_lo(ep->wMaxPacketSize);
+d->u.endpoint.wMaxPacketSize_hi = usb_hi(ep->wMaxPacketSize);
+d->u.endpoint.bInterval = ep->bInterval;
 if (ep->is_audio) {
-dest[0x07] = ep->bRefresh;
-dest[0x08] = ep->bSynchAddress;
+d->u.endpoint.bRefresh  = ep->bRefresh;
+d->u.endpoint.bSynchAddress = ep->bSynchAddress;
 }
 if (ep->extra) {
 memcpy(dest + bLength, ep->extra, extralen);
diff --git a/hw/usb/desc.h b/hw/usb/desc.h
index 6f42eae..d164e8f 100644
--- a/hw/usb/desc.h
+++ b/hw/usb/desc.h
@@ -54,6 +54,15 @@ typedef struct USBDescriptor {
 uint8_t   bInterfaceProtocol;
 uint8_t   iInterface;
 } interface;
+struct {
+uint8_t   bEndpointAddress;
+uint8_t   bmAttributes;
+uint8_t   wMaxPacketSize_lo;
+uint8_t   wMaxPacketSize_hi;
+uint8_t   bInterval;
+uint8_t   bRefresh;/* only audio ep */
+uint8_t   bSynchAddress;   /* only audio ep */
+} endpoint;
 } u;
 } QEMU_PACKED USBDescriptor;
 
-- 
1.7.1

[Qemu-devel] [PATCH v7 1/2] target-arm: Drop cpu_arm_close()

[Andreas Färber]

It's unused, so no need to QOM'ify it later.

Signed-off-by: Andreas Färber 
Reviewed-by: Peter Maydell 
---
 target-arm/cpu.h|1 -
 target-arm/helper.c |5 -
 2 files changed, 0 insertions(+), 6 deletions(-)

diff --git a/target-arm/cpu.h b/target-arm/cpu.h
index 26c114b..69ef142 100644
--- a/target-arm/cpu.h
+++ b/target-arm/cpu.h
@@ -238,7 +238,6 @@ typedef struct CPUARMState {
 CPUARMState *cpu_arm_init(const char *cpu_model);
 void arm_translate_init(void);
 int cpu_arm_exec(CPUARMState *s);
-void cpu_arm_close(CPUARMState *s);
 void do_interrupt(CPUARMState *);
 void switch_mode(CPUARMState *, int);
 uint32_t do_arm_semihosting(CPUARMState *env);
diff --git a/target-arm/helper.c b/target-arm/helper.c
index 1314f23..1ce8105 100644
--- a/target-arm/helper.c
+++ b/target-arm/helper.c
@@ -493,11 +493,6 @@ static uint32_t cpu_arm_find_by_name(const char *name)
 return id;
 }
 
-void cpu_arm_close(CPUARMState *env)
-{
-g_free(env);
-}
-
 static int bad_mode_switch(CPUARMState *env, int mode)
 {
 /* Return true if it is not valid for us to switch to
-- 
1.7.7

[Qemu-devel] [PATCH 6/6] usb-host: rewrite usb_linux_update_endp_table

[Gerd Hoffmann]

This patch carries a complete rewrite of the usb descriptor parser.
Changes / improvements:

 * We are using the USBDescriptor struct instead of hard-coded offsets
   now to access descriptor data.
 * (debug) printfs are all gone, tracepoints have been added instead.
 * We don't try (and fail) to skip over unneeded descriptors.  We parse
   them all one by one.  We keep track of which configuration, interface
   and altsetting we are looking at and use this information to figure
   which desciptors are in use and which we can ignore.
 * On parse errors we clear all endpoint information, which will
   disallow any communication with the device, except control endpoint
   messages.  This makes sure we don't end up with a silly device state
   where half of the endpoints got enabled and the other half was left
   disabled.
 * Some sanity checks have been added.

The new parser is more robust and also leaves complete device
information in the trace log if you enable the ush_host_parse_*
tracepoints.

Signed-off-by: Gerd Hoffmann 
---
 hw/usb/host-linux.c |  194 ++
 trace-events|6 ++
 2 files changed, 107 insertions(+), 93 deletions(-)

diff --git a/hw/usb/host-linux.c b/hw/usb/host-linux.c
index a382f0a..061a1b7 100644
--- a/hw/usb/host-linux.c
+++ b/hw/usb/host-linux.c
@@ -42,6 +42,7 @@
 #include 
 #include 
 #include "hw/usb.h"
+#include "hw/usb/desc.h"
 
 /* We redefine it to avoid version problems */
 struct usb_ctrltransfer {
@@ -1108,121 +1109,128 @@ static int usb_host_handle_control(USBDevice *dev, 
USBPacket *p,
 return USB_RET_ASYNC;
 }
 
-static uint8_t usb_linux_get_alt_setting(USBHostDevice *s,
-uint8_t configuration, uint8_t interface)
-{
-char device_name[64], line[1024];
-int alt_setting;
-
-sprintf(device_name, "%d-%s:%d.%d", s->bus_num, s->port,
-(int)configuration, (int)interface);
-
-if (!usb_host_read_file(line, sizeof(line), "bAlternateSetting",
-device_name)) {
-/* Assume alt 0 on error */
-return 0;
-}
-if (sscanf(line, "%d", &alt_setting) != 1) {
-/* Assume alt 0 on error */
-return 0;
-}
-return alt_setting;
-}
-
 /* returns 1 on problem encountered or 0 for success */
 static int usb_linux_update_endp_table(USBHostDevice *s)
 {
-uint8_t *descriptors;
-uint8_t devep, type, alt_interface;
-uint16_t raw;
-int interface, length, i, ep, pid;
+static const char *tname[] = {
+[USB_ENDPOINT_XFER_CONTROL] = "control",
+[USB_ENDPOINT_XFER_ISOC]= "isoc",
+[USB_ENDPOINT_XFER_BULK]= "bulk",
+[USB_ENDPOINT_XFER_INT] = "int",
+};
+uint8_t devep, type;
+uint16_t mps, v, p;
+int ep, pid;
+unsigned int i, configuration = -1, interface = -1, altsetting = -1;
 struct endp_data *epd;
+USBDescriptor *d;
+bool active = false;
 
 usb_ep_init(&s->dev);
 
-if (s->dev.configuration == 0) {
-/* not configured yet -- leave all endpoints disabled */
-return 0;
-}
-
-/* get the desired configuration, interface, and endpoint descriptors
- * from device description */
-descriptors = &s->descr[18];
-length = s->descr_len - 18;
-i = 0;
-
-while (i < length) {
-if (descriptors[i + 1] != USB_DT_CONFIG) {
-fprintf(stderr, "invalid descriptor data\n");
-return 1;
-} else if (descriptors[i + 5] != s->dev.configuration) {
-DPRINTF("not requested configuration %d\n", s->dev.configuration);
-i += (descriptors[i + 3] << 8) + descriptors[i + 2];
-continue;
-}
-i += descriptors[i];
-
-if (descriptors[i + 1] != USB_DT_INTERFACE ||
-(descriptors[i + 1] == USB_DT_INTERFACE &&
- descriptors[i + 4] == 0)) {
-i += descriptors[i];
-continue;
+for (i = 0;; i += d->bLength) {
+if (i+2 >= s->descr_len) {
+break;
 }
-
-interface = descriptors[i + 2];
-alt_interface = usb_linux_get_alt_setting(s, s->dev.configuration,
-  interface);
-
-/* the current interface descriptor is the active interface
- * and has endpoints */
-if (descriptors[i + 3] != alt_interface) {
-i += descriptors[i];
-continue;
+d = (void *)(s->descr + i);
+if (d->bLength < 2) {
+trace_usb_host_parse_error(s->bus_num, s->addr,
+   "descriptor too short");
+goto error;
 }
-
-/* advance to the endpoints */
-while (i < length && descriptors[i +1] != USB_DT_ENDPOINT) {
-i += descriptors[i];
+if (i + d->bLength > s->descr_len) {
+trace_usb_host_parse_error(s->bus_num, s->addr,
+   "descriptor too long");
+

Re: [Qemu-devel] [PATCH v7 0/2] QOM'ify ARM CPU

[Peter Maydell]

On 29 March 2012 15:50, Andreas Färber  wrote:
> Hello Peter,
>
> Here's an improved mini-conversion. Please apply to target-arm.next tree.

Thanks. Both patches
Reviewed-by: Peter Maydell 
and put into target-arm.next for tomorrow's pullreq.

-- PMM

[Qemu-devel] [PATCH 2/6] usb: use USBDescriptor for device qualifier descriptors.

[Gerd Hoffmann]

Add device qualifier substruct to USBDescriptor,
use it in the descriptor generator code.

Signed-off-by: Gerd Hoffmann 
---
 hw/usb/desc.c |   23 ---
 hw/usb/desc.h |   10 ++
 2 files changed, 22 insertions(+), 11 deletions(-)

diff --git a/hw/usb/desc.c b/hw/usb/desc.c
index de7e204..5cecb25 100644
--- a/hw/usb/desc.c
+++ b/hw/usb/desc.c
@@ -53,22 +53,23 @@ int usb_desc_device_qualifier(const USBDescDevice *dev,
   uint8_t *dest, size_t len)
 {
 uint8_t bLength = 0x0a;
+USBDescriptor *d = (void *)dest;
 
 if (len < bLength) {
 return -1;
 }
 
-dest[0x00] = bLength;
-dest[0x01] = USB_DT_DEVICE_QUALIFIER;
-
-dest[0x02] = usb_lo(dev->bcdUSB);
-dest[0x03] = usb_hi(dev->bcdUSB);
-dest[0x04] = dev->bDeviceClass;
-dest[0x05] = dev->bDeviceSubClass;
-dest[0x06] = dev->bDeviceProtocol;
-dest[0x07] = dev->bMaxPacketSize0;
-dest[0x08] = dev->bNumConfigurations;
-dest[0x09] = 0; /* reserved */
+d->bLength   = bLength;
+d->bDescriptorType   = USB_DT_DEVICE_QUALIFIER;
+
+d->u.device_qualifier.bcdUSB_lo  = usb_lo(dev->bcdUSB);
+d->u.device_qualifier.bcdUSB_hi  = usb_hi(dev->bcdUSB);
+d->u.device_qualifier.bDeviceClass   = dev->bDeviceClass;
+d->u.device_qualifier.bDeviceSubClass= dev->bDeviceSubClass;
+d->u.device_qualifier.bDeviceProtocol= dev->bDeviceProtocol;
+d->u.device_qualifier.bMaxPacketSize0= dev->bMaxPacketSize0;
+d->u.device_qualifier.bNumConfigurations = dev->bNumConfigurations;
+d->u.device_qualifier.bReserved  = 0;
 
 return bLength;
 }
diff --git a/hw/usb/desc.h b/hw/usb/desc.h
index c5a242e..15d0780 100644
--- a/hw/usb/desc.h
+++ b/hw/usb/desc.h
@@ -26,6 +26,16 @@ typedef struct USBDescriptor {
 uint8_t   iSerialNumber;
 uint8_t   bNumConfigurations;
 } device;
+struct {
+uint8_t   bcdUSB_lo;
+uint8_t   bcdUSB_hi;
+uint8_t   bDeviceClass;
+uint8_t   bDeviceSubClass;
+uint8_t   bDeviceProtocol;
+uint8_t   bMaxPacketSize0;
+uint8_t   bNumConfigurations;
+uint8_t   bReserved;
+} device_qualifier;
 } u;
 } QEMU_PACKED USBDescriptor;
 
-- 
1.7.1

[Qemu-devel] [PATCH 4/6] usb: use USBDescriptor for interface descriptors.

[Gerd Hoffmann]

Add interface descriptor substruct to USBDescriptor,
use it in the descriptor generator code.

Signed-off-by: Gerd Hoffmann 
---
 hw/usb/desc.c |   20 +++-
 hw/usb/desc.h |9 +
 2 files changed, 20 insertions(+), 9 deletions(-)

diff --git a/hw/usb/desc.c b/hw/usb/desc.c
index 3466968..2b8febb 100644
--- a/hw/usb/desc.c
+++ b/hw/usb/desc.c
@@ -159,20 +159,22 @@ int usb_desc_iface(const USBDescIface *iface, uint8_t 
*dest, size_t len)
 {
 uint8_t bLength = 0x09;
 int i, rc, pos = 0;
+USBDescriptor *d = (void *)dest;
 
 if (len < bLength) {
 return -1;
 }
 
-dest[0x00] = bLength;
-dest[0x01] = USB_DT_INTERFACE;
-dest[0x02] = iface->bInterfaceNumber;
-dest[0x03] = iface->bAlternateSetting;
-dest[0x04] = iface->bNumEndpoints;
-dest[0x05] = iface->bInterfaceClass;
-dest[0x06] = iface->bInterfaceSubClass;
-dest[0x07] = iface->bInterfaceProtocol;
-dest[0x08] = iface->iInterface;
+d->bLength= bLength;
+d->bDescriptorType= USB_DT_INTERFACE;
+
+d->u.interface.bInterfaceNumber   = iface->bInterfaceNumber;
+d->u.interface.bAlternateSetting  = iface->bAlternateSetting;
+d->u.interface.bNumEndpoints  = iface->bNumEndpoints;
+d->u.interface.bInterfaceClass= iface->bInterfaceClass;
+d->u.interface.bInterfaceSubClass = iface->bInterfaceSubClass;
+d->u.interface.bInterfaceProtocol = iface->bInterfaceProtocol;
+d->u.interface.iInterface = iface->iInterface;
 pos += bLength;
 
 for (i = 0; i < iface->ndesc; i++) {
diff --git a/hw/usb/desc.h b/hw/usb/desc.h
index 298e050..6f42eae 100644
--- a/hw/usb/desc.h
+++ b/hw/usb/desc.h
@@ -45,6 +45,15 @@ typedef struct USBDescriptor {
 uint8_t   bmAttributes;
 uint8_t   bMaxPower;
 } config;
+struct {
+uint8_t   bInterfaceNumber;
+uint8_t   bAlternateSetting;
+uint8_t   bNumEndpoints;
+uint8_t   bInterfaceClass;
+uint8_t   bInterfaceSubClass;
+uint8_t   bInterfaceProtocol;
+uint8_t   iInterface;
+} interface;
 } u;
 } QEMU_PACKED USBDescriptor;
 
-- 
1.7.1

Re: [Qemu-devel] live migration between qemu-kvm 1.0 and 0.15

[Avi Kivity]

On 03/29/2012 01:56 PM, Jan Kiszka wrote:
> On 2012-03-27 18:39, Anthony Liguori wrote:
> > On 03/27/2012 11:22 AM, Jan Kiszka wrote:
> >> On 2012-03-27 17:59, Avi Kivity wrote:
> >>> On 03/27/2012 11:55 AM, Jan Kiszka wrote:
>  On 2012-03-27 10:55, Vasilis Liaskovitis wrote:
> > Hi,
> >
> > is live migration between qemu-kvm stable-0.15 and stable-1.0 trees 
> > possible?
> > When I live migrate a VM from 1.0 to 0.15, the destination side 0.15 
> > qemu-kvm
> > exits with:
> > (qemu) Unknown savevm section or instance 'i8259' 0
> >
> > That's expected, since commit "i8259:convert to qdev" 
> > 747c70af78f7088f182c87e683bdf847beead1e4
> > introduces the i8259 device in the qdev tree.
> >
> > The other direction (live migrate from 0.15.1 to 1.0.0) seems to work 
> > fine.
> > Are any other issues expected in this direction?
> >
> > The vmstate for i8259 has not changed between these trees afaict. If the
> > qdev-ified i8259 is reverted from stable-1.0 tree (to restore 
> > live-migration
> > compatibility between the versions), would you expect problems?
> 
>  The legacy instance IDs used in old versions are not written out by
>  newer ones. They are just accepted on reading to allow moving forward.
>  There are more devices in the tree that used those instance IDs, not
>  only the i8259. Reverting the qdev conversion may reactivate backward
>  migratability for 1.0->0.15 (unless there are others as well). But that
>  will definitely be over after 1.1 as inrevertible code depends on the
>  qdev conversion.
> >>>
> >>> Is this true even for -M pc-0.15?
> >>
> >> Yes. Alias IDs enable modeling according to qdev (back then) for devices
> >> that wrote oddly numbered states in the past and porting them over to
> >> the new format. Adding some compat write-out mode would probably be
> >> feasible but would also require some thoughts and quite a bit work to
> >> integrate this cleanly in vmstate.
> >>
> >> I guess this just remained unnoticed because the introduction of the
> >> alias ID concept and first conversions happened at a time when lots of
> >> devices increased their vmstate version anyway.
> > 
> > So, since we're approaching 1.1, we should really discuss release criteria 
> > for 
> > 1.1 with respect to live migration.  I'd prefer to avoid surprises in this 
> > release.
> > 
> > My expectation is that migration works from:
> > 
> > qemu-1.0 -M 1.0 =>qemu-1.1 -M 1.1
> > qemu-1.1 -M 1.0 <=qemu-1.1 -M 1.0
>
> Besides the instance ID thing, I found two further blockers:
>
>  - kvm-tpr-op (kvmvapic), easy to disable in non-1.1 machines
>
>  - vmstate fix for i8254 which involved a version bump from 2 to 3.
>That is actually now compatible with qemu-kvm and should not cause
>troubles there. But it breaks the backward-migration scenario for
>QEMU. I have no good idea how to resolve this while pleasing all
>consumers we care about. Any suggestions?

Option 1: make -M old force an old vmstate to be written out.  Sounds
like a generally useful thing.
Option 2: ask those consumers to issue updates that bring their code up
to version 3.  Require fully updated qemus on both sides.  Easy to
achieve, result is less flexible but reasonable IMO (especially with a
long lead time, which we have).

-- 
error compiling committee.c: too many arguments to function

[Qemu-devel] [PATCH v7 2/2] target-arm: Minimalistic CPU QOM'ification

[Andreas Färber]

Introduce only one non-abstract type TYPE_ARM_CPU and do not touch
cp15 registers to not interfere with Peter's ongoing remodelling.
Embed CPUARMState as first (additional) field of ARMCPU.

Let CPUClass::reset() call cpu_state_reset() for now.

Signed-off-by: Andreas Färber 
---
 Makefile.target  |1 +
 target-arm/cpu-qom.h |   70 ++
 target-arm/cpu.c |   60 ++
 target-arm/cpu.h |1 +
 target-arm/helper.c  |8 +-
 5 files changed, 139 insertions(+), 1 deletions(-)
 create mode 100644 target-arm/cpu-qom.h
 create mode 100644 target-arm/cpu.c

diff --git a/Makefile.target b/Makefile.target
index 44b2e83..6e8b997 100644
--- a/Makefile.target
+++ b/Makefile.target
@@ -92,6 +92,7 @@ endif
 libobj-$(TARGET_SPARC64) += vis_helper.o
 libobj-$(CONFIG_NEED_MMU) += mmu.o
 libobj-$(TARGET_ARM) += neon_helper.o iwmmxt_helper.o
+libobj-$(TARGET_ARM) += cpu.o
 ifeq ($(TARGET_BASE_ARCH), sparc)
 libobj-y += fop_helper.o cc_helper.o win_helper.o mmu_helper.o ldst_helper.o
 libobj-y += cpu_init.o
diff --git a/target-arm/cpu-qom.h b/target-arm/cpu-qom.h
new file mode 100644
index 000..cf107c9
--- /dev/null
+++ b/target-arm/cpu-qom.h
@@ -0,0 +1,70 @@
+/*
+ * QEMU ARM CPU
+ *
+ * Copyright (c) 2012 SUSE LINUX Products GmbH
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; either version 2
+ * of the License, or (at your option) any later version.
+ *
+ * This program 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 General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, see
+ * 
+ */
+#ifndef QEMU_ARM_CPU_QOM_H
+#define QEMU_ARM_CPU_QOM_H
+
+#include "qemu/cpu.h"
+#include "cpu.h"
+
+#define TYPE_ARM_CPU "arm-cpu"
+
+#define ARM_CPU_CLASS(klass) \
+OBJECT_CLASS_CHECK(ARMCPUClass, (klass), TYPE_ARM_CPU)
+#define ARM_CPU(obj) \
+OBJECT_CHECK(ARMCPU, (obj), TYPE_ARM_CPU)
+#define ARM_CPU_GET_CLASS(obj) \
+OBJECT_GET_CLASS(ARMCPUClass, (obj), TYPE_ARM_CPU)
+
+/**
+ * ARMCPUClass:
+ *
+ * An ARM CPU model.
+ */
+typedef struct ARMCPUClass {
+/*< private >*/
+CPUClass parent_class;
+/*< public >*/
+
+void (*parent_reset)(CPUState *cpu);
+} ARMCPUClass;
+
+/**
+ * ARMCPU:
+ * @env: #CPUARMState
+ *
+ * An ARM CPU core.
+ */
+typedef struct ARMCPU {
+/*< private >*/
+CPUState parent_obj;
+/*< public >*/
+
+CPUARMState env;
+} ARMCPU;
+
+static inline ARMCPU *arm_env_get_cpu(CPUARMState *env)
+{
+return ARM_CPU(container_of(env, ARMCPU, env));
+}
+
+#define ENV_GET_CPU(e) CPU(arm_env_get_cpu(e))
+
+
+#endif
diff --git a/target-arm/cpu.c b/target-arm/cpu.c
new file mode 100644
index 000..c3ed45b
--- /dev/null
+++ b/target-arm/cpu.c
@@ -0,0 +1,60 @@
+/*
+ * QEMU ARM CPU
+ *
+ * Copyright (c) 2012 SUSE LINUX Products GmbH
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; either version 2
+ * of the License, or (at your option) any later version.
+ *
+ * This program 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 General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, see
+ * 
+ */
+
+#include "cpu-qom.h"
+#include "qemu-common.h"
+
+/* CPUClass::reset() */
+static void arm_cpu_reset(CPUState *s)
+{
+ARMCPU *cpu = ARM_CPU(s);
+ARMCPUClass *acc = ARM_CPU_GET_CLASS(cpu);
+
+acc->parent_reset(s);
+
+/* TODO Inline the current contents of cpu_state_reset(),
+once cpu_reset_model_id() is eliminated. */
+cpu_state_reset(&cpu->env);
+}
+
+static void arm_cpu_class_init(ObjectClass *oc, void *data)
+{
+ARMCPUClass *acc = ARM_CPU_CLASS(oc);
+CPUClass *cc = CPU_CLASS(acc);
+
+acc->parent_reset = cc->reset;
+cc->reset = arm_cpu_reset;
+}
+
+static const TypeInfo arm_cpu_type_info = {
+.name = TYPE_ARM_CPU,
+.parent = TYPE_CPU,
+.instance_size = sizeof(ARMCPU),
+.abstract = false,
+.class_size = sizeof(ARMCPUClass),
+.class_init = arm_cpu_class_init,
+};
+
+static void arm_cpu_register_types(void)
+{
+type_register_static(&arm_cpu_type_info);
+}
+
+type_init(arm_cpu_register_types)
diff --git a/target-arm/cpu.h b/target-arm/cpu.h
index 69ef142..a68df61 100644
--- a/target-arm/cpu.h
+++ b/target

Re: [Qemu-devel] live migration between qemu-kvm 1.0 and 0.15

[Anthony Liguori]

On 03/29/2012 10:17 AM, Avi Kivity wrote:

On 03/29/2012 01:56 PM, Jan Kiszka wrote:

On 2012-03-27 18:39, Anthony Liguori wrote:

On 03/27/2012 11:22 AM, Jan Kiszka wrote:

On 2012-03-27 17:59, Avi Kivity wrote:

On 03/27/2012 11:55 AM, Jan Kiszka wrote:

On 2012-03-27 10:55, Vasilis Liaskovitis wrote:

Hi,

is live migration between qemu-kvm stable-0.15 and stable-1.0 trees possible?
When I live migrate a VM from 1.0 to 0.15, the destination side 0.15 qemu-kvm
exits with:
(qemu) Unknown savevm section or instance 'i8259' 0

That's expected, since commit "i8259:convert to qdev" 
747c70af78f7088f182c87e683bdf847beead1e4
introduces the i8259 device in the qdev tree.

The other direction (live migrate from 0.15.1 to 1.0.0) seems to work fine.
Are any other issues expected in this direction?

The vmstate for i8259 has not changed between these trees afaict. If the
qdev-ified i8259 is reverted from stable-1.0 tree (to restore live-migration
compatibility between the versions), would you expect problems?


The legacy instance IDs used in old versions are not written out by
newer ones. They are just accepted on reading to allow moving forward.
There are more devices in the tree that used those instance IDs, not
only the i8259. Reverting the qdev conversion may reactivate backward
migratability for 1.0->0.15 (unless there are others as well). But that
will definitely be over after 1.1 as inrevertible code depends on the
qdev conversion.


Is this true even for -M pc-0.15?


Yes. Alias IDs enable modeling according to qdev (back then) for devices
that wrote oddly numbered states in the past and porting them over to
the new format. Adding some compat write-out mode would probably be
feasible but would also require some thoughts and quite a bit work to
integrate this cleanly in vmstate.

I guess this just remained unnoticed because the introduction of the
alias ID concept and first conversions happened at a time when lots of
devices increased their vmstate version anyway.


So, since we're approaching 1.1, we should really discuss release criteria for
1.1 with respect to live migration.  I'd prefer to avoid surprises in this 
release.

My expectation is that migration works from:

qemu-1.0 -M 1.0 => qemu-1.1 -M 1.1
qemu-1.1 -M 1.0<=qemu-1.1 -M 1.0


Besides the instance ID thing, I found two further blockers:

  - kvm-tpr-op (kvmvapic), easy to disable in non-1.1 machines

  - vmstate fix for i8254 which involved a version bump from 2 to 3.
That is actually now compatible with qemu-kvm and should not cause
troubles there. But it breaks the backward-migration scenario for
QEMU. I have no good idea how to resolve this while pleasing all
consumers we care about. Any suggestions?


Option 1: make -M old force an old vmstate to be written out.  Sounds
like a generally useful thing.
Option 2: ask those consumers to issue updates that bring their code up
to version 3.  Require fully updated qemus on both sides.  Easy to
achieve, result is less flexible but reasonable IMO (especially with a
long lead time, which we have).


I prefer Option 2 presuming the bug is a legitimate bug fix.

If it couldn't be done as a subsection, then there's really no choice IMHO.

Regards,

Anthony Liguori

Re: [Qemu-devel] live migration between qemu-kvm 1.0 and 0.15

[Anthony Liguori]

On 03/27/2012 12:42 PM, Jan Kiszka wrote:

On 2012-03-27 18:49, Anthony Liguori wrote:

On 03/27/2012 11:46 AM, Avi Kivity wrote:

On 03/27/2012 06:39 PM, Anthony Liguori wrote:


So, since we're approaching 1.1, we should really discuss release
criteria for 1.1 with respect to live migration.  I'd prefer to avoid
surprises in this release.


Agree strongly.



My expectation is that migration works from:

qemu-1.0 -M 1.0 =>  qemu-1.1 -M 1.1


Why do you expect that?  Maybe you meant -M 1.0 at the end?


Sorry, I did mean -M 1.0.




qemu-1.1 -M 1.0<=qemu-1.1 -M 1.0

I would expect that migration works from:

qemu-0.15 -M 0.15   => qemu-1.1 -M 0.15



Ack.


I'm okay if this fails gracefully:

qemu-1.1 -M 0.15<=   qemu-0.15 -M 0.15


RHEL has more stringent requirements (going back to its heavily patched
0.12).  I think we should have the infrastructure that allow one to add
the hacks to make this work, even if we don't actually do the compat
work for the release (I think it's fine for qemu to support just one
version going back; and unreasonable to require it to go as far back as
RHEL).


This is reasonable to me.


Here is a draft to get things written in the old format. Totally
untested and likely borken (written in a hurry). I'll split up if it
works fine.


I don't really like this as a matter of principle.

Knowingly migrating when the result may be a broken guest is a bug, it's not a 
feature.


It's one thing if we're changing formats for other reasons, but if we're 
changing the format to send what's effectively broken migration state, then 
that's an evil thing to do.


Subsections are the compromise.  We send a subsection when we think migration 
can work and fail gracefully when it can't.  Presumably there's a reason we're 
not using subsections here.


Regards,

Anthony Liguori

Re: [Qemu-devel] live migration between qemu-kvm 1.0 and 0.15

[Avi Kivity]

On 03/29/2012 05:21 PM, Anthony Liguori wrote:
>> Option 1: make -M old force an old vmstate to be written out.  Sounds
>> like a generally useful thing.
>> Option 2: ask those consumers to issue updates that bring their code up
>> to version 3.  Require fully updated qemus on both sides.  Easy to
>> achieve, result is less flexible but reasonable IMO (especially with a
>> long lead time, which we have).
>
>
> I prefer Option 2 presuming the bug is a legitimate bug fix.
>
> If it couldn't be done as a subsection, then there's really no choice
> IMHO.

qemu-kvm already has version 3, so our hand is forced.  I too think
option 2 is the best here.

-- 
error compiling committee.c: too many arguments to function

[Qemu-devel] [PATCH v7 0/2] QOM'ify ARM CPU

[Andreas Färber]

Hello Peter,

Here's an improved mini-conversion. Please apply to target-arm.next tree.

Available at:
https://github.com/afaerber/qemu-cpu/commits/qom-cpu-arm.v7

Regards,
Andreas

Cc: Peter Maydell 
Cc: Paul Brook 
Cc: Andrzej Zaborowski 
Cc: Max Filippov 

v6 -> v7:
* Fix TODO comment and add a corresponding TODO in helper.c.
* Avoid klass/class by using oc for ObjectClass, cc for CPUClass, acc for 
ARMCPUClass.

v5 -> v6:
* By dropping some of the patches in the series, cpu_reset() would no longer
  reset the CPUARMState. Fix this by re-adding a reset handler and calling
  cpu_state_reset() for now.

v4 -> v5:
* Use only one non-abstract CPU type for now, leave everything else as is.
* Drop cpu_arm_close() instead of converting it.
* Still make available cpu-qom.h through cpu.h for convenience.

v3 -> v4:
* Rebased on top of type_init() v2, object_class_get_list() v2, qom-cpu v4.

* Rename cpu-core.h to cpu-qom.h. While the term "ARM core" is quite common,
  it is less so for other architectures like s390x so use a neutral term.
* Use container_of() for CPUState -> CPU macros (suggested by Anthony).
* Rework arm_env_get_object() -> arm_env_get_cpu(), avoids some casts
  (suggested by Anthony). Also rename ENV_GET_OBJECT() -> ENV_GET_CPU().
* Sort -cpu ? list.
* Use object_class_get_list() and sort ourselves rather than using
  object_class_foreach_ordered() with callbacks (suggested by Anthony).
* Drop ARMCPUClass jtag_id since it turned out unneeded in QEMU (Peter+Andrzej).

* Drop experimental "halted" property since that should be in common code.
* Introduce "cpuid-variant" and "cpuid-revision" properties.
* Use CPU properties to drop unneeded pxa270-* classes.
* Move "/cpu" child property to integratorcp machine.

v2 -> v3:
* Rebased against qom-upstream.14 branch (and that against master).

* Rename target-arm/cpu-core.c to cpu.c now that we no longer need VPATH.
* Leave cpu-core.h as is to separate from legacy cpu.h.
* Fix -cpu alias "pxa270": handled in cpu_arm_init().
* Use proper GPL headers.

* Start removing CPUID uses in cpu_reset_model_id() and cpu.h.
* Fully convert cpu_reset_model_id() to ARMCPUInfo or per-model code.

* Experiment with adding properties ("halted").
* For testing, add a "/cpu" child property (HACK).

v1 -> v2:

* Cherry-pick Anthony's object_class_foreach() patch.

* Fix ARMCPUClass type name (arm-cpu-core -> arm-cpu).
* Add documentation.
* Rename ARMCPUDef to ARMCPUInfo.
* Use a C99-style table for initializing the classes through class_data
  instead of individual class_init functions (suggested by Anthony).
* Prepare reset callback.

* Make ENV_GET_OBJECT() use an inline function for readability.
* Invoke the CPU's reset method from cpu_reset().

* Do feature initialization via table where sensible.
* Add feature flags to ARMCPU as well (suggested by PMM for future tweaking,
  also simplifies load/save a bit) and initialize them from ARMCPUClass.
* Make feature inference work for ARMCPU as well by not passing the ARMCPUClass.
  Use function-local macros to avoid the ugliness of deferencing the features 
pointer.

Andreas Färber (2):
  target-arm: Drop cpu_arm_close()
  target-arm: Minimalistic CPU QOM'ification

 Makefile.target  |1 +
 target-arm/cpu-qom.h |   70 ++
 target-arm/cpu.c |   60 ++
 target-arm/cpu.h |2 +-
 target-arm/helper.c  |   13 +
 5 files changed, 139 insertions(+), 7 deletions(-)
 create mode 100644 target-arm/cpu-qom.h
 create mode 100644 target-arm/cpu.c

-- 
1.7.7

Re: [Qemu-devel] [PATCH v2 0/6] ARM: AREG0 conversion

[Laurent Desnogues]

On Tue, Mar 27, 2012 at 9:59 PM, Artyom Tarasenko  wrote:
> On Tue, Mar 27, 2012 at 7:01 PM, Laurent Desnogues
>  wrote:
>> On Tue, Mar 27, 2012 at 6:48 PM, Blue Swirl  wrote:
>>> On Tue, Mar 27, 2012 at 13:40, Laurent Desnogues
>>>  wrote:
 On Mon, Mar 26, 2012 at 7:02 PM, Blue Swirl  wrote:
 [...]
> At least stack protector is protecting more code than before (for
> example TLB miss handler), but could overhead from that amount to 5%?
>
> Otherwise there should be just a few extra register moves here and
> there, that should be cheap on modern processors.

 The extra moves might be cheap but their cost is obviously not 0:
 on top of using extra CPU core resources, code size is increased
 which results in more instruction cache misses.

 I didn't like the idea when we discussed it back in May, now it
 looks like we have concrete evidence the speed impact is
 measurable (though I'd like some more numbers than the rough
 5% estimate I gave).
>>>
>>> A clearly defined test case running on a host that does not adjust
>>> clock frequencies would be nice. It would be interesting to find out
>>> where exactly the slowdown comes from.
>>>
>>> Perhaps the access helpers ({helper,_}_{ld,st}{b,w,l}_mmu) generated
>>> by softmmu_template.h are the culprit. If so, they could be split from
>>> other code and moved to TCG back ends. That way the interface could be
>>> improved while keeping all other cleanups.
>>
>> I also get a slowdown running in user mode, so I don't think
>> improving the mmu ld/st will completely remove the issue.
>> In that case the slowdown comes from the extra move
>> instructions for helper calls.  The ARM target uses way too
>> many helpers, but that's another discussion :-)
>>
>
> Have you tried compiling without -fstack-protector-all as suggested by Lluís?
> I observe a similar slowdown on a sparc target, and there compiling
> without stack protection definitely helps.

That will indeed probably make the real problem, which is that
this patch increases the size of generated code, less obvious
on small benchmarks that don't put pressure on instruction
cache.  But the fact is that generated code is larger and will
have to execute more instructions, so no matter what you do,
this will have an impact on speed.


Laurent

Re: [Qemu-devel] live migration between qemu-kvm 1.0 and 0.15

[Jan Kiszka]

On 2012-03-29 17:23, Anthony Liguori wrote:
> On 03/27/2012 12:42 PM, Jan Kiszka wrote:
>> On 2012-03-27 18:49, Anthony Liguori wrote:
>>> On 03/27/2012 11:46 AM, Avi Kivity wrote:
 On 03/27/2012 06:39 PM, Anthony Liguori wrote:
>
> So, since we're approaching 1.1, we should really discuss release
> criteria for 1.1 with respect to live migration.  I'd prefer to avoid
> surprises in this release.

 Agree strongly.

>
> My expectation is that migration works from:
>
> qemu-1.0 -M 1.0 =>  qemu-1.1 -M 1.1

 Why do you expect that?  Maybe you meant -M 1.0 at the end?
>>>
>>> Sorry, I did mean -M 1.0.
>>>

> qemu-1.1 -M 1.0<=qemu-1.1 -M 1.0
>
> I would expect that migration works from:
>
> qemu-0.15 -M 0.15   => qemu-1.1 -M 0.15
>

 Ack.

> I'm okay if this fails gracefully:
>
> qemu-1.1 -M 0.15<=   qemu-0.15 -M 0.15

 RHEL has more stringent requirements (going back to its heavily patched
 0.12).  I think we should have the infrastructure that allow one to add
 the hacks to make this work, even if we don't actually do the compat
 work for the release (I think it's fine for qemu to support just one
 version going back; and unreasonable to require it to go as far back as
 RHEL).
>>>
>>> This is reasonable to me.
>>
>> Here is a draft to get things written in the old format. Totally
>> untested and likely borken (written in a hurry). I'll split up if it
>> works fine.
> 
> I don't really like this as a matter of principle.
> 
> Knowingly migrating when the result may be a broken guest is a bug, it's not 
> a 
> feature.
> 
> It's one thing if we're changing formats for other reasons, but if we're 
> changing the format to send what's effectively broken migration state, then 
> that's an evil thing to do.
> 
> Subsections are the compromise.  We send a subsection when we think migration 
> can work and fail gracefully when it can't.  Presumably there's a reason 
> we're 
> not using subsections here.

In this case (instance ID), it's actually not about a bug fix but a
consolidation of the vmstate format. So I think it's an exception,
though I don't like the code changes it requires as well.

Jan

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

Re: [Qemu-devel] [PATCH 1/4 v3] block: add image fragmentation statistics to qemu-img

[Stefan Hajnoczi]

On Thu, Mar 15, 2012 at 12:45 PM, Stefan Hajnoczi
 wrote:
> On Thu, Mar 15, 2012 at 08:13:31PM +0800, Dong Xu Wang wrote:
>> From: Dong Xu Wang 
>>
>> Discussion can be found at:
>> http://patchwork.ozlabs.org/patch/128730/
>>
>> This patch add image fragmentation statistics while using qemu-img check.
>>
>> Signed-off-by: Dong Xu Wang 
>> ---
>>  block.h    |    7 +++
>>  qemu-img.c |    9 -
>>  2 files changed, 15 insertions(+), 1 deletions(-)
>
> A minor comment that doesn't require resending:
>
> Please avoid introducing whitespace or reformatting changes in a patch
> that makes other code changes.  Patch 1 changes whitespace in a place
> that is not touched by the rest of your patch, so that change is not
> essential and it's best to leave it out.
>
> Reviewed-by: Stefan Hajnoczi 

Ping?

Stefan

Re: [Qemu-devel] [PATCH 1/4 v3] block: add image fragmentation statistics to qemu-img

[Stefan Hajnoczi]

On Thu, Mar 15, 2012 at 12:13 PM, Dong Xu Wang
 wrote:
> From: Dong Xu Wang 
>
> Discussion can be found at:
> http://patchwork.ozlabs.org/patch/128730/
>
> This patch add image fragmentation statistics while using qemu-img check.
>
> Signed-off-by: Dong Xu Wang 
> ---
>  block.h    |    7 +++
>  qemu-img.c |    9 -
>  2 files changed, 15 insertions(+), 1 deletions(-)

In the future please send a 0/4 email when sending a patch series:

git format-patch --cover-letter ...

The cover letter introduces the patch series and also acts as the
single place where reviewers can reply to say they are happy with the
series.

Kevin: my Reviewed-by: applies to the whole series.

Stefan

[Qemu-devel] [PATCH] block: fix streaming/closing race

[Paolo Bonzini]

Streaming can issue I/O while qcow2_close is running.  This causes the
L2 caches to become very confused or, alternatively, could cause a
segfault when the streaming coroutine is reentered after closing its
block device.  The fix is to cancel streaming jobs when closing their
underlying device.  The cancellation must be synchronous, so add a flag
saying whether streaming has in-flight I/O.

Signed-off-by: Paolo Bonzini 
---
 block.c|   14 ++
 block/stream.c |4 +++-
 block_int.h|2 ++
 3 files changed, 19 insertions(+), 1 deletions(-)

diff --git a/block.c b/block.c
index b88ee90..1aab4bf 100644
--- a/block.c
+++ b/block.c
@@ -813,6 +813,9 @@ unlink_and_fail:
 void bdrv_close(BlockDriverState *bs)
 {
 if (bs->drv) {
+if (bs->job) {
+block_job_cancel_sync(bs->job);
+}
 if (bs == bs_snapshots) {
 bs_snapshots = NULL;
 }
@@ -4069,3 +4072,14 @@ bool block_job_is_cancelled(BlockJob *job)
 {
 return job->cancelled;
 }
+
+void block_job_cancel_sync(BlockJob *job)
+{
+BlockDriverState *bs = job->bs;
+
+assert(bs->job == job);
+block_job_cancel(job);
+while (bs->job != NULL && bs->job->busy) {
+qemu_aio_wait();
+}
+}
diff --git a/block/stream.c b/block/stream.c
index d1b3986..b8a1628 100644
--- a/block/stream.c
+++ b/block/stream.c
@@ -175,7 +175,7 @@ retry:
 break;
 }
 
-
+s->common.busy = true;
 if (base) {
 ret = is_allocated_base(bs, base, sector_num,
 STREAM_BUFFER_SIZE / BDRV_SECTOR_SIZE, &n);
@@ -189,6 +189,7 @@ retry:
 if (s->common.speed) {
 uint64_t delay_ns = ratelimit_calculate_delay(&s->limit, n);
 if (delay_ns > 0) {
+s->common.busy = false;
 co_sleep_ns(rt_clock, delay_ns);
 
 /* Recheck cancellation and that sectors are unallocated */
@@ -208,6 +209,7 @@ retry:
 /* Note that even when no rate limit is applied we need to yield
  * with no pending I/O here so that qemu_aio_flush() returns.
  */
+s->common.busy = false;
 co_sleep_ns(rt_clock, 0);
 }
 
diff --git a/block_int.h b/block_int.h
index b460c36..f5c3dff 100644
--- a/block_int.h
+++ b/block_int.h
@@ -89,6 +89,7 @@ struct BlockJob {
 const BlockJobType *job_type;
 BlockDriverState *bs;
 bool cancelled;
+bool busy;
 
 /* These fields are published by the query-block-jobs QMP API */
 int64_t offset;
@@ -329,6 +330,7 @@ void block_job_complete(BlockJob *job, int ret);
 int block_job_set_speed(BlockJob *job, int64_t value);
 void block_job_cancel(BlockJob *job);
 bool block_job_is_cancelled(BlockJob *job);
+void block_job_cancel_sync(BlockJob *job);
 
 int stream_start(BlockDriverState *bs, BlockDriverState *base,
  const char *base_id, BlockDriverCompletionFunc *cb,
-- 
1.7.9.1

Re: [Qemu-devel] [PATCH 0/2] ide: convert pio code path to asynchronous I/O

[Chris Webb]

Stefan Hajnoczi  writes:

> Thanks for trying it out.
> 
> Regarding the guest kernel errors, they may be timeouts.  The guest
> may consider the IDE controller buggy/dead due to how long requests
> take to complete under severe throttling.  There's not much that can
> be done about that, I think.  If you think the guest errors are due to
> something else though, please post the error messages.

I'm sure you're right that they're standard timeouts. We see exactly the
same behaviour when a guest is on a very heavily contended disk with slow
access because of that.

Best wishes,

Chris.

Re: [Qemu-devel] [PATCH v2 0/6] ARM: AREG0 conversion

[Richard Henderson]

On 03/29/2012 11:42 AM, Laurent Desnogues wrote:
> That will indeed probably make the real problem, which is that
> this patch increases the size of generated code, less obvious
> on small benchmarks that don't put pressure on instruction
> cache.  But the fact is that generated code is larger and will
> have to execute more instructions, so no matter what you do,
> this will have an impact on speed.

While this is true, the benefit of using a more standard calling
convention on reliability and debug-ability is enormous.

Consider the i686 host, where we currently obscond with EBP.
While I'm not aware of any current problems with -O0 or spill
failures under optimization, it's not inconceivable.

Consider sparc-linux host, where we have *no* call-saved global
register at all, and (currently) try very hard to use a call-
clobbered global register, with occasionally disastrous results.
See the patch set I posted recently where I give up on this entirely
and make sparc use a TLS variable instead of a hard register at all.
This regresses the Sparc host on speed for a progression in reliability.
The conversion to explicit env arguments fixes essentially all of
the speed regression since we then receive ENV in %o0 instead of
having to read from TLS.


r~

Re: [Qemu-devel] [PATCHv2] piix: fix up/down races + document

[Alex Williamson]

On Thu, 2012-03-29 at 14:51 +0200, Michael S. Tsirkin wrote:
> piix acpi interface suffers from the following 2 issues:
> 
> 1.
> - delete device a
> - quickly add device b in another slot
> 
> if we do this before guest reads the down register,
> the down event is discarded and device will never
> be deleted.
> 
> 2.
> - delete device a
> - quickly reset before guest can respond
> 
> interrupt is reset and guest will never eject the device.
> 
> To fix this, we implement two changes:
> 
> 1. Add two new registers:
> CLR_UP
> CLR_DOWN
> bios will now write to these the value it read from UP/DOWN
> 
> 2. on reset, remove all devices which have DOWN bit set
> 
> For compatibility with old guests, we also clear
> the DOWN bit on write to EJ0 for a device.
> 
> This patch also adds more detailed documentation
> for the ACPI interface, including the semantics
> of both old and new registers.
> 
> Signed-off-by: Michael S. Tsirkin 
> ---
> 
> Changes from v1:
> - tweaked a comment about clearing down register on reset
> - documentation update
> 
>  docs/specs/acpi_pci_hotplug.txt |   68 +++
>  hw/acpi_piix4.c |   75 +++---
>  2 files changed, 121 insertions(+), 22 deletions(-)
> 
> diff --git a/docs/specs/acpi_pci_hotplug.txt b/docs/specs/acpi_pci_hotplug.txt
> index f0f74a7..a8398f9 100644
> --- a/docs/specs/acpi_pci_hotplug.txt
> +++ b/docs/specs/acpi_pci_hotplug.txt
> @@ -7,31 +7,83 @@ describes the interface between QEMU and the ACPI BIOS.
>  ACPI GPE block (IO ports 0xafe0-0xafe3, byte access):
>  -
>  
> -Generic ACPI GPE block. Bit 1 (GPE.1) used to notify PCI hotplug/eject
> +Generic ACPI GPE block. Bit 1 (GPE.1) is used to notify PCI hotplug/hotunplug
>  event to ACPI BIOS, via SCI interrupt.
>  
> -PCI slot injection notification pending (IO port 0xae00-0xae03, 4-byte 
> access):
> +Semantics: this event occurs each time a bit in either Pending PCI slot
> +injection notification or Pending PCI slot removal notification registers
> +changes status from 0 to 1.
> +
> +Pending PCI slot injection notification (IO port 0xae00-0xae03, 4-byte 
> access):
>  ---
> -Slot injection notification pending. One bit per slot.
> +Slot injection notification is pending. One bit per slot.
> +Guests should not write this register, in practice the value
> +written overwrites the register.

Sorry if I'm behind on the discussion, but why do we need to define
0xae0c & 0xae10 below for write-only access when these registers (0xae00
& 0xae04) are effectively read-only (yes, write is there, but it should
*never* be used in the current implementation)?  Thanks,

Alex

>  Read by ACPI BIOS GPE.1 handler to notify OS of injection
>  events.
>  
> -PCI slot removal notification (IO port 0xae04-0xae07, 4-byte access):
> +Semantics: after a slot is populated by hotplug, it is immediately enabled
> +and a bit in this register is set.
> +Reset value: 0
> +
> +Pending PCI slot removal notification (IO port 0xae04-0xae07, 4-byte access):
>  -
> -Slot removal notification pending. One bit per slot.
> +Slot removal notification is pending. One bit per slot.
> +Guests should not write this register, in practice the value
> +written overwrites the register.
>  
>  Read by ACPI BIOS GPE.1 handler to notify OS of removal
>  events.
>  
> +Semantics: upon hotunplug request, a bit in this register is set
> +and the OSPM is notified about hotunplug request, but a slot remains enabled
> +until eject.
> +Reset value: 0
> +
>  PCI device eject (IO port 0xae08-0xae0b, 4-byte access):
>  
>  
>  Used by ACPI BIOS _EJ0 method to request device removal. One bit per slot.
> -Reads return 0.
> +Guests should not read this register. In practice reads return 0.
> +
> +Semantics: selected hotunpluggable slots are disabled and powered off.
> +Has no effect on non-hotunpluggable slots.
> +
> +For compatibility with old BIOS, writes to this register
> +clear bits in pending slot injection notification register.
>  
>  PCI removability status (IO port 0xae0c-0xae0f, 4-byte access):
>  ---
>  
> -Used by ACPI BIOS _RMV method to indicate removability status to OS. One
> -bit per slot.
> +Used by ACPI BIOS _RMV method to detect removability status
> +and report to OS. One bit per slot.
> +Guests should not write this register, in practice the value
> +written is discarded.
> +
> +Semantics: selected slots support hotplug. Contents of this register
> +do not change while guest is running.
> +
> +Clear PCI slot injection notification (IO port 0xae10-0xae14, 4-byte access):
> +---
> +Clears bits in pending slot injection notification register. One bit per 
> slot.
> +Guests should not read this reg

[Qemu-devel] [PATCH v2 0/2]: convert device_del to the qapi

[Luiz Capitulino]

V2:

o change qdev_unplug() to return void (this allows some simplifications)
o rename pci_device_hot_remove() 'errp' var to 'local_err'
o minor changelog fixes

 hmp-commands.hx  |3 +--
 hmp.c|9 +
 hmp.h|1 +
 hw/pci-hotplug.c |   14 --
 hw/qdev-monitor.c|   11 ++-
 hw/qdev.c|   12 
 hw/qdev.h|3 ++-
 hw/usb/dev-storage.c |2 +-
 hw/xen_platform.c|4 ++--
 qapi-schema.json |   20 
 qmp-commands.hx  |5 +
 11 files changed, 63 insertions(+), 21 deletions(-)

[Qemu-devel] [PATCH 1/2] qdev: qdev_unplug(): use error_set()

[Luiz Capitulino]

It currently uses qerror_report(), but next commit will convert
the drive_del command to the QAPI and this requires using
error_set().

One particularity of qerror_report() is that it knows when it's
running on monitor context or command-line context and prints the
error message accordingly. error_set() doesn't do this, so we
have to be careful not to drop error messages.

qdev_unplug() has three kinds of usages:

 1. It's called when hot adding a device fails, to undo anything
that has been done before hitting the error

 2. It's called by function monitor functions like device_del(),
to unplug a device

 3. It's used by xen_platform.c in a way that doesn't _seem_ to
be in monitor context

Only item 2 can print an error message to the user, this commit
maintains that.

Signed-off-by: Luiz Capitulino 
---
 hw/pci-hotplug.c |   14 --
 hw/qdev-monitor.c|   11 ++-
 hw/qdev.c|   12 
 hw/qdev.h|3 ++-
 hw/usb/dev-storage.c |2 +-
 hw/xen_platform.c|4 ++--
 6 files changed, 35 insertions(+), 11 deletions(-)

diff --git a/hw/pci-hotplug.c b/hw/pci-hotplug.c
index 5c6307f..c55d8b9 100644
--- a/hw/pci-hotplug.c
+++ b/hw/pci-hotplug.c
@@ -32,6 +32,7 @@
 #include "virtio-blk.h"
 #include "qemu-config.h"
 #include "blockdev.h"
+#include "error.h"
 
 #if defined(TARGET_I386)
 static PCIDevice *qemu_pci_hot_add_nic(Monitor *mon,
@@ -191,7 +192,7 @@ static PCIDevice *qemu_pci_hot_add_storage(Monitor *mon,
 dev = NULL;
 if (dev && dinfo) {
 if (scsi_hot_add(mon, &dev->qdev, dinfo, 0) != 0) {
-qdev_unplug(&dev->qdev);
+qdev_unplug(&dev->qdev, NULL);
 dev = NULL;
 }
 }
@@ -258,6 +259,7 @@ static int pci_device_hot_remove(Monitor *mon, const char 
*pci_addr)
 PCIDevice *d;
 int dom, bus;
 unsigned slot;
+Error *local_err = NULL;
 
 if (pci_read_devaddr(mon, pci_addr, &dom, &bus, &slot)) {
 return -1;
@@ -268,7 +270,15 @@ static int pci_device_hot_remove(Monitor *mon, const char 
*pci_addr)
 monitor_printf(mon, "slot %d empty\n", slot);
 return -1;
 }
-return qdev_unplug(&d->qdev);
+
+qdev_unplug(&d->qdev, &local_err);
+if (error_is_set(&local_err)) {
+monitor_printf(mon, "%s\n", error_get_pretty(local_err));
+error_free(local_err);
+return -1;
+}
+
+return 0;
 }
 
 void do_pci_device_hot_remove(Monitor *mon, const QDict *qdict)
diff --git a/hw/qdev-monitor.c b/hw/qdev-monitor.c
index a310cc7..e952238 100644
--- a/hw/qdev-monitor.c
+++ b/hw/qdev-monitor.c
@@ -577,6 +577,7 @@ int do_device_add(Monitor *mon, const QDict *qdict, QObject 
**ret_data)
 int do_device_del(Monitor *mon, const QDict *qdict, QObject **ret_data)
 {
 const char *id = qdict_get_str(qdict, "id");
+Error *local_err = NULL;
 DeviceState *dev;
 
 dev = qdev_find_recursive(sysbus_get_default(), id);
@@ -584,7 +585,15 @@ int do_device_del(Monitor *mon, const QDict *qdict, 
QObject **ret_data)
 qerror_report(QERR_DEVICE_NOT_FOUND, id);
 return -1;
 }
-return qdev_unplug(dev);
+
+qdev_unplug(dev, &local_err);
+if (error_is_set(&local_err)) {
+qerror_report_err(local_err);
+error_free(local_err);
+return -1;
+}
+
+return 0;
 }
 
 void qdev_machine_init(void)
diff --git a/hw/qdev.c b/hw/qdev.c
index ee21d90..8b0ad19 100644
--- a/hw/qdev.c
+++ b/hw/qdev.c
@@ -28,6 +28,7 @@
 #include "net.h"
 #include "qdev.h"
 #include "sysemu.h"
+#include "error.h"
 
 int qdev_hotplug = 0;
 static bool qdev_hot_added = false;
@@ -172,19 +173,22 @@ void qdev_set_legacy_instance_id(DeviceState *dev, int 
alias_id,
 dev->alias_required_for_version = required_for_version;
 }
 
-int qdev_unplug(DeviceState *dev)
+void qdev_unplug(DeviceState *dev, Error **errp)
 {
 DeviceClass *dc = DEVICE_GET_CLASS(dev);
 
 if (!dev->parent_bus->allow_hotplug) {
-qerror_report(QERR_BUS_NO_HOTPLUG, dev->parent_bus->name);
-return -1;
+error_set(errp, QERR_BUS_NO_HOTPLUG, dev->parent_bus->name);
+return;
 }
 assert(dc->unplug != NULL);
 
 qdev_hot_removed = true;
 
-return dc->unplug(dev);
+if (dc->unplug(dev) < 0) {
+error_set(errp, QERR_UNDEFINED_ERROR);
+return;
+}
 }
 
 static int qdev_reset_one(DeviceState *dev, void *opaque)
diff --git a/hw/qdev.h b/hw/qdev.h
index 9cc3f98..11f77cf 100644
--- a/hw/qdev.h
+++ b/hw/qdev.h
@@ -7,6 +7,7 @@
 #include "qemu-option.h"
 #include "qapi/qapi-visit-core.h"
 #include "qemu/object.h"
+#include "error.h"
 
 typedef struct Property Property;
 
@@ -148,7 +149,7 @@ int qdev_init(DeviceState *dev) QEMU_WARN_UNUSED_RESULT;
 void qdev_init_nofail(DeviceState *dev);
 void qdev_set_legacy_instance_id(DeviceState *dev, int alias_id,
  int required_for_version);
-int qdev_unplug(DeviceState *dev);
+void

[Qemu-devel] [PATCH 2/2] qapi: convert device_del

[Luiz Capitulino]

Signed-off-by: Anthony Liguori 
Signed-off-by: Luiz Capitulino 
---
 hmp-commands.hx   |3 +--
 hmp.c |9 +
 hmp.h |1 +
 hw/qdev-monitor.c |   18 +-
 qapi-schema.json  |   20 
 qmp-commands.hx   |5 +
 6 files changed, 37 insertions(+), 19 deletions(-)

diff --git a/hmp-commands.hx b/hmp-commands.hx
index bd35a3e..a6f5a84 100644
--- a/hmp-commands.hx
+++ b/hmp-commands.hx
@@ -622,8 +622,7 @@ ETEXI
 .args_type  = "id:s",
 .params = "device",
 .help   = "remove device",
-.user_print = monitor_user_noop,
-.mhandler.cmd_new = do_device_del,
+.mhandler.cmd = hmp_device_del,
 },
 
 STEXI
diff --git a/hmp.c b/hmp.c
index 9cf2d13..f3e5163 100644
--- a/hmp.c
+++ b/hmp.c
@@ -934,3 +934,12 @@ void hmp_migrate(Monitor *mon, const QDict *qdict)
 qemu_mod_timer(status->timer, qemu_get_clock_ms(rt_clock));
 }
 }
+
+void hmp_device_del(Monitor *mon, const QDict *qdict)
+{
+const char *id = qdict_get_str(qdict, "id");
+Error *err = NULL;
+
+qmp_device_del(id, &err);
+hmp_handle_error(mon, &err);
+}
diff --git a/hmp.h b/hmp.h
index 8807853..443b812 100644
--- a/hmp.h
+++ b/hmp.h
@@ -60,5 +60,6 @@ void hmp_block_stream(Monitor *mon, const QDict *qdict);
 void hmp_block_job_set_speed(Monitor *mon, const QDict *qdict);
 void hmp_block_job_cancel(Monitor *mon, const QDict *qdict);
 void hmp_migrate(Monitor *mon, const QDict *qdict);
+void hmp_device_del(Monitor *mon, const QDict *qdict);
 
 #endif
diff --git a/hw/qdev-monitor.c b/hw/qdev-monitor.c
index e952238..7e3c925 100644
--- a/hw/qdev-monitor.c
+++ b/hw/qdev-monitor.c
@@ -19,6 +19,7 @@
 
 #include "qdev.h"
 #include "monitor.h"
+#include "qmp-commands.h"
 
 /*
  * Aliases were a bad idea from the start.  Let's keep them
@@ -574,26 +575,17 @@ int do_device_add(Monitor *mon, const QDict *qdict, 
QObject **ret_data)
 return 0;
 }
 
-int do_device_del(Monitor *mon, const QDict *qdict, QObject **ret_data)
+void qmp_device_del(const char *id, Error **errp)
 {
-const char *id = qdict_get_str(qdict, "id");
-Error *local_err = NULL;
 DeviceState *dev;
 
 dev = qdev_find_recursive(sysbus_get_default(), id);
 if (NULL == dev) {
-qerror_report(QERR_DEVICE_NOT_FOUND, id);
-return -1;
-}
-
-qdev_unplug(dev, &local_err);
-if (error_is_set(&local_err)) {
-qerror_report_err(local_err);
-error_free(local_err);
-return -1;
+error_set(errp, QERR_DEVICE_NOT_FOUND, id);
+return;
 }
 
-return 0;
+qdev_unplug(dev, errp);
 }
 
 void qdev_machine_init(void)
diff --git a/qapi-schema.json b/qapi-schema.json
index 0d11d6e..ace55f3 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -1701,3 +1701,23 @@
 # Since: 1.1
 ##
 { 'command': 'xen-save-devices-state', 'data': {'filename': 'str'} }
+
+##
+# @device_del:
+#
+# Remove a device from a guest
+#
+# @id: the name of the device
+#
+# Returns: Nothing on success
+#  If @id is not a valid device, DeviceNotFound
+#  If the device does not support unplug, BusNoHotplug
+#
+# Notes: When this command completes, the device may not be removed from the
+#guest.  Hot removal is an operation that requires guest cooperation.
+#This command merely requests that the guest begin the hot removal
+#process.
+#
+# Since: 0.14.0
+##
+{ 'command': 'device_del', 'data': {'id': 'str'} }
diff --git a/qmp-commands.hx b/qmp-commands.hx
index c626ba8..c878b54 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -314,10 +314,7 @@ EQMP
 {
 .name   = "device_del",
 .args_type  = "id:s",
-.params = "device",
-.help   = "remove device",
-.user_print = monitor_user_noop,
-.mhandler.cmd_new = do_device_del,
+.mhandler.cmd_new = qmp_marshal_input_device_del,
 },
 
 SQMP
-- 
1.7.9.2.384.g4a92a

[Qemu-devel] [PATCH v2 5/5] target-unicore32: Move CPU-dependent init into initfn

[Andreas Färber]

Instead of setting values in a CPUID switch, do so in initfn functions.

Signed-off-by: Andreas Färber 
---
 target-unicore32/cpu.c|   14 ++
 target-unicore32/helper.c |   23 ---
 2 files changed, 14 insertions(+), 23 deletions(-)

diff --git a/target-unicore32/cpu.c b/target-unicore32/cpu.c
index 189b6f6..de63f58 100644
--- a/target-unicore32/cpu.c
+++ b/target-unicore32/cpu.c
@@ -15,6 +15,11 @@
 #include "cpu-qom.h"
 #include "qemu-common.h"
 
+static inline void set_feature(CPUUniCore32State *env, int feature)
+{
+env->features |= feature;
+}
+
 /* CPU models */
 
 typedef struct UniCore32CPUInfo {
@@ -28,6 +33,12 @@ static void unicore_ii_cpu_initfn(Object *obj)
 CPUUniCore32State *env = &cpu->env;
 
 env->cp0.c0_cpuid = 0x40010863;
+
+set_feature(env, UC32_HWCAP_CMOV);
+set_feature(env, UC32_HWCAP_UCF64);
+env->ucf64.xregs[UC32_UCF64_FPSCR] = 0;
+env->cp0.c0_cachetype = 0x1dd20d2;
+env->cp0.c1_sys = 0x00090078;
 }
 
 static void uc32_any_cpu_initfn(Object *obj)
@@ -36,6 +47,9 @@ static void uc32_any_cpu_initfn(Object *obj)
 CPUUniCore32State *env = &cpu->env;
 
 env->cp0.c0_cpuid = 0x;
+
+set_feature(env, UC32_HWCAP_CMOV);
+set_feature(env, UC32_HWCAP_UCF64);
 }
 
 static const UniCore32CPUInfo uc32_cpus[] = {
diff --git a/target-unicore32/helper.c b/target-unicore32/helper.c
index 0f23a40..9fe4a37 100644
--- a/target-unicore32/helper.c
+++ b/target-unicore32/helper.c
@@ -14,16 +14,10 @@
 #include "helper.h"
 #include "host-utils.h"
 
-static inline void set_feature(CPUUniCore32State *env, int feature)
-{
-env->features |= feature;
-}
-
 CPUUniCore32State *uc32_cpu_init(const char *cpu_model)
 {
 UniCore32CPU *cpu;
 CPUUniCore32State *env;
-uint32_t id;
 static int inited = 1;
 
 if (object_class_by_name(cpu_model) == NULL) {
@@ -32,23 +26,6 @@ CPUUniCore32State *uc32_cpu_init(const char *cpu_model)
 cpu = UNICORE32_CPU(object_new(cpu_model));
 env = &cpu->env;
 
-id = env->cp0.c0_cpuid;
-switch (id) {
-case UC32_CPUID_UCV2:
-set_feature(env, UC32_HWCAP_CMOV);
-set_feature(env, UC32_HWCAP_UCF64);
-env->ucf64.xregs[UC32_UCF64_FPSCR] = 0;
-env->cp0.c0_cachetype = 0x1dd20d2;
-env->cp0.c1_sys = 0x00090078;
-break;
-case UC32_CPUID_ANY: /* For userspace emulation.  */
-set_feature(env, UC32_HWCAP_CMOV);
-set_feature(env, UC32_HWCAP_UCF64);
-break;
-default:
-cpu_abort(env, "Bad CPU ID: %x\n", id);
-}
-
 if (inited) {
 inited = 0;
 uc32_translate_init();
-- 
1.7.7

[Qemu-devel] [PATCH v2 3/5] target-unicore32: License future contributions under GPLv2+

[Andreas Färber]

This is to limit relicensing obstacles to the pending IBM investigation.

Signed-off-by: Andreas Färber 
---
 target-unicore32/helper.c |3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/target-unicore32/helper.c b/target-unicore32/helper.c
index 6af492d..18a9cbb 100644
--- a/target-unicore32/helper.c
+++ b/target-unicore32/helper.c
@@ -4,6 +4,9 @@
  * 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.
+ *
+ * Contributions from 2012-04-01 on are considered under GPL version 2,
+ * or (at your option) any later version.
  */
 
 #include "cpu.h"
-- 
1.7.7

[Qemu-devel] [PATCH v2 0/5] QOM'ify UniCore32 CPU

[Andreas Färber]

Hello Xuetao,

This updated series converts the UniCore32 CPU to QOM. In addition to
addressing review comments from v1, a new approach for CPU-dependent
values has been adopted: We should keep setting of features imperative
and can avoid adding new class fields as storage for default values
by moving the existing imperative code into new initfn functions.

Patch 1 adds a "UniCore32 CPU guest core (TCG)" section to MAINTAINERS,
so that the target-unicore32 author gets notified of patches against his code.

Patch 2, based on feedback from Guan Xuetao, changes the license of most
target-unicore32 files from GPLv2 to GPLv2+. Anthony had contributed a
qemu_malloc() -> g_malloc() substitution that he can't relicense at this time,
so leave that as GPLv2 and declare my following patches explicitly as GPLv2+.
Patch 3 adds a notice to license any following contributions under GPLv2+ 
already.

Patch 4 embeds CPUUniCore32State into UniCore32CPU. My new cpu-qom.h header
can be GPLv2+, but into cpu.c we're moving helper.c code so make it GPLv2 for 
now.

Patch 5 cleans up uc32_cpu_init() function by moving initializations into 
initfn.

Could you test this please and, if happy, provide your Acked-by for patches 1 
and 3-5?

Available from:
git://github.com/afaerber/qemu-cpu.git qom-cpu-unicore32.v2
https://github.com/afaerber/qemu-cpu/commits/qom-cpu-unicore32.v2

Regards,
Andreas

Cc: Guan Xue-tao 
Cc: Anthony Liguori 
Cc: Peter Maydell 

v1 -> v2:
* Change license notice to match the previous one more closely.
* Adopt that license notice for new cpu-qom.h as well.
* Add notices to helper.c and cpu.c to license future contributions under 
GPLv2+.
* Adopt Peter's approach of using per-CPU initfn functions for default values.
* Squash init code movement together (no new UniCore32CPUClass members needed).
* Various naming updates (e.g., ..._type_info).

Changes from former repo.or.cz qom-cpu[-wip] branch:
* Drop duplicate .instance_init.

Andreas Färber (5):
  MAINTAINERS: Add entry for UniCore32
  target-unicore32: Relicense to GPLv2+
  target-unicore32: License future contributions under GPLv2+
  target-unicore32: QOM'ify CPU
  target-unicore32: Move CPU-dependent init into initfn

 MAINTAINERS  |5 ++
 Makefile.target  |1 +
 target-unicore32/cpu-qom.h   |   59 
 target-unicore32/cpu.c   |  104 ++
 target-unicore32/cpu.h   |4 +-
 target-unicore32/helper.c|   65 +++---
 target-unicore32/helper.h|3 +-
 target-unicore32/op_helper.c |3 +-
 target-unicore32/translate.c |3 +-
 9 files changed, 186 insertions(+), 61 deletions(-)
 create mode 100644 target-unicore32/cpu-qom.h
 create mode 100644 target-unicore32/cpu.c

-- 
1.7.7

[Qemu-devel] [PATCH v2 4/5] target-unicore32: QOM'ify CPU

[Andreas Färber]

Embed CPUUniCore32State as first member of UniCore32CPU.

Contributed under GPLv2+.

Signed-off-by: Andreas Färber 
---
 Makefile.target|1 +
 target-unicore32/cpu-qom.h |   59 +
 target-unicore32/cpu.c |   90 
 target-unicore32/cpu.h |1 +
 target-unicore32/helper.c  |   43 +++-
 5 files changed, 158 insertions(+), 36 deletions(-)
 create mode 100644 target-unicore32/cpu-qom.h
 create mode 100644 target-unicore32/cpu.c

diff --git a/Makefile.target b/Makefile.target
index 6e8b997..9b0cf74 100644
--- a/Makefile.target
+++ b/Makefile.target
@@ -99,6 +99,7 @@ libobj-y += cpu_init.o
 endif
 libobj-$(TARGET_SPARC) += int32_helper.o
 libobj-$(TARGET_SPARC64) += int64_helper.o
+libobj-$(TARGET_UNICORE32) += cpu.o
 libobj-$(TARGET_ALPHA) += int_helper.o fpu_helper.o sys_helper.o mem_helper.o
 
 libobj-y += disas.o
diff --git a/target-unicore32/cpu-qom.h b/target-unicore32/cpu-qom.h
new file mode 100644
index 000..342d85e
--- /dev/null
+++ b/target-unicore32/cpu-qom.h
@@ -0,0 +1,59 @@
+/*
+ * QEMU UniCore32 CPU
+ *
+ * Copyright (c) 2012 SUSE LINUX Products GmbH
+ *
+ * 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, or (at your option) any
+ * later version. See the COPYING file in the top-level directory.
+ */
+#ifndef QEMU_UC32_CPU_QOM_H
+#define QEMU_UC32_CPU_QOM_H
+
+#include "qemu/cpu.h"
+#include "cpu.h"
+
+#define TYPE_UNICORE32_CPU "unicore32-cpu"
+
+#define UNICORE32_CPU_CLASS(klass) \
+OBJECT_CLASS_CHECK(UniCore32CPUClass, (klass), TYPE_UNICORE32_CPU)
+#define UNICORE32_CPU(obj) \
+OBJECT_CHECK(UniCore32CPU, (obj), TYPE_UNICORE32_CPU)
+#define UNICORE32_CPU_GET_CLASS(obj) \
+OBJECT_GET_CLASS(UniCore32CPUClass, (obj), TYPE_UNICORE32_CPU)
+
+/**
+ * UniCore32CPUClass:
+ *
+ * A UniCore32 CPU model.
+ */
+typedef struct UniCore32CPUClass {
+/*< private >*/
+CPUClass parent_class;
+/*< public >*/
+} UniCore32CPUClass;
+
+/**
+ * UniCore32CPU:
+ * @env: #CPUUniCore32State
+ *
+ * A UniCore32 CPU.
+ */
+typedef struct UniCore32CPU {
+/*< private >*/
+CPUState parent_obj;
+/*< public >*/
+
+CPUUniCore32State env;
+} UniCore32CPU;
+
+static inline UniCore32CPU *uc32_env_get_cpu(CPUUniCore32State *env)
+{
+return UNICORE32_CPU(container_of(env, UniCore32CPU, env));
+}
+
+#define ENV_GET_CPU(e) CPU(uc32_env_get_cpu(e))
+
+
+#endif
diff --git a/target-unicore32/cpu.c b/target-unicore32/cpu.c
new file mode 100644
index 000..189b6f6
--- /dev/null
+++ b/target-unicore32/cpu.c
@@ -0,0 +1,90 @@
+/*
+ * QEMU UniCore32 CPU
+ *
+ * Copyright (c) 2010-2011 GUAN Xue-tao
+ * Copyright (c) 2012 SUSE LINUX Products GmbH
+ *
+ * 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.
+ *
+ * Contributions from 2012-04-01 on are considered under GPL version 2,
+ * or (at your option) any later version.
+ */
+
+#include "cpu-qom.h"
+#include "qemu-common.h"
+
+/* CPU models */
+
+typedef struct UniCore32CPUInfo {
+const char *name;
+void (*instance_init)(Object *obj);
+} UniCore32CPUInfo;
+
+static void unicore_ii_cpu_initfn(Object *obj)
+{
+UniCore32CPU *cpu = UNICORE32_CPU(obj);
+CPUUniCore32State *env = &cpu->env;
+
+env->cp0.c0_cpuid = 0x40010863;
+}
+
+static void uc32_any_cpu_initfn(Object *obj)
+{
+UniCore32CPU *cpu = UNICORE32_CPU(obj);
+CPUUniCore32State *env = &cpu->env;
+
+env->cp0.c0_cpuid = 0x;
+}
+
+static const UniCore32CPUInfo uc32_cpus[] = {
+{ .name = "UniCore-II", .instance_init = unicore_ii_cpu_initfn },
+{ .name = "any",.instance_init = uc32_any_cpu_initfn },
+};
+
+static void uc32_cpu_initfn(Object *obj)
+{
+UniCore32CPU *cpu = UNICORE32_CPU(obj);
+CPUUniCore32State *env = &cpu->env;
+
+cpu_exec_init(env);
+env->cpu_model_str = object_get_typename(obj);
+
+env->uncached_asr = ASR_MODE_USER;
+env->regs[31] = 0;
+
+tlb_flush(env, 1);
+}
+
+static void uc32_register_cpu_type(const UniCore32CPUInfo *info)
+{
+TypeInfo type_info = {
+.name = info->name,
+.parent = TYPE_UNICORE32_CPU,
+.instance_init = info->instance_init,
+};
+
+type_register_static(&type_info);
+}
+
+static const TypeInfo uc32_cpu_type_info = {
+.name = TYPE_UNICORE32_CPU,
+.parent = TYPE_CPU,
+.instance_size = sizeof(UniCore32CPU),
+.instance_init = uc32_cpu_initfn,
+.abstract = true,
+.class_size = sizeof(UniCore32CPUClass),
+};
+
+static void uc32_cpu_register_types(void)
+{
+int i;
+
+type_register_static(&uc32_cpu_type_info);
+for (i = 0; i < ARRAY_SIZE(uc32_cpus); i++) {
+uc32_register_cpu_type(&uc32_cpus[i]);
+}
+}
+
+type_init(uc32_cpu_register_t

[Qemu-devel] [PATCH v2 2/5] target-unicore32: Relicense to GPLv2+

[Andreas Färber]

Adopt the license text suggested by Guan Xue-tao (with a minor
simplification) for all target-unicore/ files except helper.c.

To helper.c Anthony Liguori contributed a qemu_malloc() -> g_malloc()
conversion, still pending IBM relicensing approval, so that remains
GPLv2 for now.

By relicensing all possible parts now, we avoid having to formally
relicense new, e.g., QOM code.

Signed-off-by: Andreas Färber 
Signed-off-by: Guan Xuetao 
Signed-off-by: Stefan Weil 
Signed-off-by: Blue Swirl 
Signed-off-by: Dor Laor 
---
 target-unicore32/cpu.h   |3 ++-
 target-unicore32/helper.h|3 ++-
 target-unicore32/op_helper.c |3 ++-
 target-unicore32/translate.c |3 ++-
 4 files changed, 8 insertions(+), 4 deletions(-)

diff --git a/target-unicore32/cpu.h b/target-unicore32/cpu.h
index a3f8589..8ec0d2f 100644
--- a/target-unicore32/cpu.h
+++ b/target-unicore32/cpu.h
@@ -5,7 +5,8 @@
  *
  * 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.
+ * published by the Free Software Foundation, or (at your option) any
+ * later version. See the COPYING file in the top-level directory.
  */
 #ifndef __CPU_UC32_H__
 #define __CPU_UC32_H__
diff --git a/target-unicore32/helper.h b/target-unicore32/helper.h
index 615de2a..5a3b8a4 100644
--- a/target-unicore32/helper.h
+++ b/target-unicore32/helper.h
@@ -3,7 +3,8 @@
  *
  * 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.
+ * published by the Free Software Foundation, or (at your option) any
+ * later version. See the COPYING file in the top-level directory.
  */
 #include "def-helper.h"
 
diff --git a/target-unicore32/op_helper.c b/target-unicore32/op_helper.c
index 638a020..b954c30 100644
--- a/target-unicore32/op_helper.c
+++ b/target-unicore32/op_helper.c
@@ -5,7 +5,8 @@
  *
  * 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.
+ * published by the Free Software Foundation, or (at your option) any
+ * later version. See the COPYING file in the top-level directory.
  */
 #include "cpu.h"
 #include "dyngen-exec.h"
diff --git a/target-unicore32/translate.c b/target-unicore32/translate.c
index 3b3ba16..9793d14 100644
--- a/target-unicore32/translate.c
+++ b/target-unicore32/translate.c
@@ -5,7 +5,8 @@
  *
  * 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.
+ * published by the Free Software Foundation, or (at your option) any
+ * later version. See the COPYING file in the top-level directory.
  */
 #include 
 #include 
-- 
1.7.7

[Qemu-devel] [RFC 00/13]: convert device_add to the qapi

[Luiz Capitulino]

This is an RFC because it's not 100% finished yet and I'm not sure if some
of the changes are the way to go.

Here's a summary of the series its issues, more details can be found in
the patches:

 o Patches 1 to 11 convert several qemu-option and qemu-config functions
   from qerror_report() to error_set(). Note that not all users of those
   functions are converted, so it might be needed to call qerror_report_err()
   somewhere in the call stack to keep QError semantics.

   Also, new versions of qemu_opt_set() and qemu_find_opts() that take an
   Error parameter are introduced. This allows for incremental conversion
   (vs. converting 100% of their users in one series)

 o Patch 12 introduces support for allowing QAPI functions to accept a
   variable list of arguments, which are unchecked and passed "as-is"
   to the QAPI function. My implementation here is very hacky, please
   check the patch for details

 o Patch 13 does the QAPI conversion itself, but the HMP version of
   device_add lacks its help text (ie. '?'). The problem here is that hmp.c
   is really a QMP client and can only use what's provided by QMP. There are
   two ways to solve this:

1. Anthony told me that he has this implemented in his tree, but
   I couldn't find it. Anthony, can you point me to your
   implementation?

2. Add a query-drivers command or similar to get a list of drivers,
   so that the HMP command can use it to implement the help text

 blockdev.c   |2 +-
 hmp-commands.hx  |3 +-
 hmp.c|   24 +++
 hmp.h|1 +
 hw/qdev-monitor.c|   70 ++-
 hw/qdev.h|2 +-
 hw/usb/dev-storage.c |2 +-
 hw/watchdog.c|2 +-
 qapi-schema.json |   38 ++
 qemu-char.c  |8 ++-
 qemu-config.c|   43 +---
 qemu-config.h|3 +
 qemu-option.c|  173 +++---
 qemu-option.h|6 +-
 qemu-sockets.c   |8 +--
 qerror.c |4 ++
 qerror.h |3 +
 qmp-commands.hx  |3 +-
 scripts/qapi-commands.py |   31 -
 scripts/qapi.py  |2 +
 vl.c |   25 ---
 21 files changed, 342 insertions(+), 111 deletions(-)

[Qemu-devel] [PATCH 05/13] qemu-option: qemu_opt_parse(): use error_set()

[Luiz Capitulino]

The functions opt_set() and qemu_opts_validate() both call qemu_opt_parse(),
but their callers expect QError semantics. Thus, both functions call
qerro_report_err() to keep the expected semantics.

Signed-off-by: Luiz Capitulino 
---
 qemu-option.c |   54 +-
 1 file changed, 29 insertions(+), 25 deletions(-)

diff --git a/qemu-option.c b/qemu-option.c
index 61354af..4829de5 100644
--- a/qemu-option.c
+++ b/qemu-option.c
@@ -584,38 +584,27 @@ uint64_t qemu_opt_get_size(QemuOpts *opts, const char 
*name, uint64_t defval)
 return opt->value.uint;
 }
 
-static int qemu_opt_parse(QemuOpt *opt)
+static void qemu_opt_parse(QemuOpt *opt, Error **errp)
 {
-Error *local_err = NULL;
-
 if (opt->desc == NULL)
-return 0;
+return;
 
 switch (opt->desc->type) {
 case QEMU_OPT_STRING:
 /* nothing */
-return 0;
+return;
 case QEMU_OPT_BOOL:
-parse_option_bool(opt->name, opt->str, &opt->value.boolean, 
&local_err);
+parse_option_bool(opt->name, opt->str, &opt->value.boolean, errp);
 break;
 case QEMU_OPT_NUMBER:
-parse_option_number(opt->name, opt->str, &opt->value.uint,
-&local_err);
+parse_option_number(opt->name, opt->str, &opt->value.uint, errp);
 break;
 case QEMU_OPT_SIZE:
-parse_option_size(opt->name, opt->str, &opt->value.uint, &local_err);
+parse_option_size(opt->name, opt->str, &opt->value.uint, errp);
 break;
 default:
 abort();
 }
-
-if (error_is_set(&local_err)) {
-qerror_report_err(local_err);
-error_free(local_err);
-return -1;
-}
-
-return 0;
 }
 
 static void qemu_opt_del(QemuOpt *opt)
@@ -631,6 +620,7 @@ static int opt_set(QemuOpts *opts, const char *name, const 
char *value,
 {
 QemuOpt *opt;
 const QemuOptDesc *desc = opts->list->desc;
+Error *local_err = NULL;
 int i;
 
 for (i = 0; desc[i].name != NULL; i++) {
@@ -642,8 +632,8 @@ static int opt_set(QemuOpts *opts, const char *name, const 
char *value,
 if (i == 0) {
 /* empty list -> allow any */;
 } else {
-qerror_report(QERR_INVALID_PARAMETER, name);
-return -1;
+error_set(&local_err, QERR_INVALID_PARAMETER, name);
+goto exit_err;
 }
 }
 
@@ -661,11 +651,18 @@ static int opt_set(QemuOpts *opts, const char *name, 
const char *value,
 if (value) {
 opt->str = g_strdup(value);
 }
-if (qemu_opt_parse(opt) < 0) {
+qemu_opt_parse(opt, &local_err);
+if (error_is_set(&local_err)) {
 qemu_opt_del(opt);
-return -1;
+goto exit_err;
 }
+
 return 0;
+
+exit_err:
+qerror_report_err(local_err);
+error_free(local_err);
+return -1;
 }
 
 int qemu_opt_set(QemuOpts *opts, const char *name, const char *value)
@@ -1053,6 +1050,7 @@ QDict *qemu_opts_to_qdict(QemuOpts *opts, QDict *qdict)
 int qemu_opts_validate(QemuOpts *opts, const QemuOptDesc *desc)
 {
 QemuOpt *opt;
+Error *local_err = NULL;
 
 assert(opts->list->desc[0].name == NULL);
 
@@ -1065,18 +1063,24 @@ int qemu_opts_validate(QemuOpts *opts, const 
QemuOptDesc *desc)
 }
 }
 if (desc[i].name == NULL) {
-qerror_report(QERR_INVALID_PARAMETER, opt->name);
-return -1;
+error_set(&local_err, QERR_INVALID_PARAMETER, opt->name);
+goto exit_err;
 }
 
 opt->desc = &desc[i];
 
-if (qemu_opt_parse(opt) < 0) {
-return -1;
+qemu_opt_parse(opt, &local_err);
+if (error_is_set(&local_err)) {
+goto exit_err;
 }
 }
 
 return 0;
+
+exit_err:
+qerror_report_err(local_err);
+error_free(local_err);
+return -1;
 }
 
 int qemu_opts_foreach(QemuOptsList *list, qemu_opts_loopfunc func, void 
*opaque,
-- 
1.7.9.2.384.g4a92a

[Qemu-devel] [PATCH 10/13] qemu-config: find_list(): use error_set()

[Luiz Capitulino]

Note that qemu_find_opts() callers still expect automatic error reporting
with QError, so qemu_find_opts() calls qerror_report_err() to keep the
same semantics.

Signed-off-by: Luiz Capitulino 
---
 qemu-config.c |   32 +---
 1 file changed, 25 insertions(+), 7 deletions(-)

diff --git a/qemu-config.c b/qemu-config.c
index f876646..bdb381d 100644
--- a/qemu-config.c
+++ b/qemu-config.c
@@ -3,6 +3,7 @@
 #include "qemu-option.h"
 #include "qemu-config.h"
 #include "hw/qdev.h"
+#include "error.h"
 
 static QemuOptsList qemu_drive_opts = {
 .name = "drive",
@@ -631,7 +632,8 @@ static QemuOptsList *vm_config_groups[32] = {
 NULL,
 };
 
-static QemuOptsList *find_list(QemuOptsList **lists, const char *group)
+static QemuOptsList *find_list(QemuOptsList **lists, const char *group,
+   Error **errp)
 {
 int i;
 
@@ -640,14 +642,23 @@ static QemuOptsList *find_list(QemuOptsList **lists, 
const char *group)
 break;
 }
 if (lists[i] == NULL) {
-error_report("there is no option group \"%s\"", group);
+error_set(errp, QERR_INVALID_OPTION_GROUP, group);
 }
 return lists[i];
 }
 
 QemuOptsList *qemu_find_opts(const char *group)
 {
-return find_list(vm_config_groups, group);
+QemuOptsList *ret;
+Error *local_err = NULL;
+
+ret = find_list(vm_config_groups, group, &local_err);
+if (error_is_set(&local_err)) {
+error_report("%s\n", error_get_pretty(local_err));
+error_free(local_err);
+}
+
+return ret;
 }
 
 void qemu_add_opts(QemuOptsList *list)
@@ -762,6 +773,7 @@ int qemu_config_parse(FILE *fp, QemuOptsList **lists, const 
char *fname)
 char line[1024], group[64], id[64], arg[64], value[1024];
 Location loc;
 QemuOptsList *list = NULL;
+Error *local_err = NULL;
 QemuOpts *opts = NULL;
 int res = -1, lno = 0;
 
@@ -778,17 +790,23 @@ int qemu_config_parse(FILE *fp, QemuOptsList **lists, 
const char *fname)
 }
 if (sscanf(line, "[%63s \"%63[^\"]\"]", group, id) == 2) {
 /* group with id */
-list = find_list(lists, group);
-if (list == NULL)
+list = find_list(lists, group, &local_err);
+if (error_is_set(&local_err)) {
+error_report("%s\n", error_get_pretty(local_err));
+error_free(local_err);
 goto out;
+}
 opts = qemu_opts_create(list, id, 1, NULL);
 continue;
 }
 if (sscanf(line, "[%63[^]]]", group) == 1) {
 /* group without id */
-list = find_list(lists, group);
-if (list == NULL)
+list = find_list(lists, group, &local_err);
+if (error_is_set(&local_err)) {
+error_report("%s\n", error_get_pretty(local_err));
+error_free(local_err);
 goto out;
+}
 opts = qemu_opts_create(list, NULL, 0, NULL);
 continue;
 }
-- 
1.7.9.2.384.g4a92a

Re: [Qemu-devel] [PATCH v2] signrom: Rewrite as python script

[Jan Kiszka]

On 2012-03-05 15:09, Jan Kiszka wrote:
> Now that we have a hard dependency on python anyway, we can replace the
> slow shell script to calculate the option ROM checksum with a fast AND
> portable python version. Tested both with python 2.7 and 3.1.
> 
> Signed-off-by: Jan Kiszka 
> ---
> 
> Changes in v2:
>  - Switch to GPLv2+.
>The tool replaces an inefficient GPLv2+ implementation, so want to
>avoid any license "regression". As it is self-contained, it doesn't
>affect the QEMU core license anyway (which least common denominator
>remains GPLv2).
> 

Ping?

>  pc-bios/optionrom/Makefile |2 +-
>  scripts/signrom.py |   40 +++
>  scripts/signrom.sh |   45 
> 
>  3 files changed, 41 insertions(+), 46 deletions(-)
>  create mode 100644 scripts/signrom.py
>  delete mode 100755 scripts/signrom.sh
> 
> diff --git a/pc-bios/optionrom/Makefile b/pc-bios/optionrom/Makefile
> index f6b4027..57d8bd0 100644
> --- a/pc-bios/optionrom/Makefile
> +++ b/pc-bios/optionrom/Makefile
> @@ -26,7 +26,7 @@ build-all: multiboot.bin linuxboot.bin kvmvapic.bin
>   $(call quiet-command,$(OBJCOPY) -O binary -j .text $< $@,"  Building 
> $(TARGET_DIR)$@")
>  
>  %.bin: %.raw
> - $(call quiet-command,$(SHELL) $(SRC_PATH)/scripts/signrom.sh $< $@,"  
> Signing $(TARGET_DIR)$@")
> + $(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/signrom.py $< $@,"  
> Signing $(TARGET_DIR)$@")
>  
>  clean:
>   rm -f *.o *.d *.raw *.img *.bin *~
> diff --git a/scripts/signrom.py b/scripts/signrom.py
> new file mode 100644
> index 000..f9c35cc
> --- /dev/null
> +++ b/scripts/signrom.py
> @@ -0,0 +1,40 @@
> +#
> +# Option ROM signing utility
> +#
> +# Authors:
> +#  Jan Kiszka 
> +#
> +# This work is licensed under the terms of the GNU GPL, version 2 or later.
> +# See the COPYING file in the top-level directory.
> +
> +import sys
> +import struct
> +
> +if len(sys.argv) < 3:
> +print('usage: signrom.py input output')
> +sys.exit(1)
> +
> +fin = open(sys.argv[1], 'rb')
> +fout = open(sys.argv[2], 'wb')
> +
> +fin.seek(2)
> +size = ord(fin.read(1)) * 512 - 1
> +
> +fin.seek(0)
> +data = fin.read(size)
> +fout.write(data)
> +
> +checksum = 0
> +for b in data:
> +# catch Python 2 vs. 3 differences
> +if isinstance(b, int):
> +checksum += b
> +else:
> +checksum += ord(b)
> +checksum = (256 - checksum) % 256
> +
> +# Python 3 no longer allows chr(checksum)
> +fout.write(struct.pack('B', checksum))
> +
> +fin.close()
> +fout.close()
> diff --git a/scripts/signrom.sh b/scripts/signrom.sh
> deleted file mode 100755
> index 9dc5c63..000
> --- a/scripts/signrom.sh
> +++ /dev/null
> @@ -1,45 +0,0 @@
> -#!/bin/sh
> -
> -# Option ROM Signing utility
> -#
> -# This program is free software; you can redistribute it and/or modify
> -# it under the terms of the GNU General Public License as published by
> -# the Free Software Foundation; either version 2 of the License, or
> -# (at your option) any later version.
> -#
> -# This program 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 General Public License for more details.
> -#
> -# You should have received a copy of the GNU General Public License
> -# along with this program; if not, see .
> -#
> -# Copyright Novell Inc, 2009
> -#   Authors: Alexander Graf 
> -#
> -# Syntax: signrom.sh  
> -
> -# did we get proper arguments?
> -test "$1" -a "$2" || exit 1
> -
> -sum=0
> -
> -# find out the file size
> -x=`dd if="$1" bs=1 count=1 skip=2 2>/dev/null | od -t u1 -A n`
> -#size=`expr $x \* 512 - 1`
> -size=$(( $x * 512 - 1 ))
> -
> -# now get the checksum
> -nums=`od -A n -t u1 -v -N $size "$1"`
> -for i in ${nums}; do
> -# add each byte's value to sum
> -sum=`expr \( $sum + $i \) % 256`
> -done
> -
> -sum=$(( (256 - $sum) % 256 ))
> -sum_octal=$( printf "%o" $sum )
> -
> -# and write the output file
> -cp "$1" "$2"
> -printf "\\$sum_octal" | dd of="$2" bs=1 count=1 seek=$size conv=notrunc 
> 2>/dev/null

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

[Qemu-devel] [PATCH 03/13] qemu-option: parse_option_bool(): use error_set()

[Luiz Capitulino]

Note that set_option_parameter() callers still expect automatic error
reporting with QError, so set_option_parameter() calls
qerror_report_err() to keep the same semantics.

Signed-off-by: Luiz Capitulino 
---
 qemu-option.c |   22 +++---
 1 file changed, 15 insertions(+), 7 deletions(-)

diff --git a/qemu-option.c b/qemu-option.c
index 72dcb56..a8b50af 100644
--- a/qemu-option.c
+++ b/qemu-option.c
@@ -169,7 +169,8 @@ QEMUOptionParameter 
*get_option_parameter(QEMUOptionParameter *list,
 return NULL;
 }
 
-static int parse_option_bool(const char *name, const char *value, bool *ret)
+static void parse_option_bool(const char *name, const char *value, bool *ret,
+  Error **errp)
 {
 if (value != NULL) {
 if (!strcmp(value, "on")) {
@@ -177,13 +178,11 @@ static int parse_option_bool(const char *name, const char 
*value, bool *ret)
 } else if (!strcmp(value, "off")) {
 *ret = 0;
 } else {
-qerror_report(QERR_INVALID_PARAMETER_VALUE, name, "'on' or 'off'");
-return -1;
+error_set(errp,QERR_INVALID_PARAMETER_VALUE, name, "'on' or 
'off'");
 }
 } else {
 *ret = 1;
 }
-return 0;
 }
 
 static void parse_option_number(const char *name, const char *value,
@@ -263,6 +262,7 @@ int set_option_parameter(QEMUOptionParameter *list, const 
char *name,
 const char *value)
 {
 bool flag;
+Error *local_err = NULL;
 
 // Find a matching parameter
 list = get_option_parameter(list, name);
@@ -274,8 +274,10 @@ int set_option_parameter(QEMUOptionParameter *list, const 
char *name,
 // Process parameter
 switch (list->type) {
 case OPT_FLAG:
-if (parse_option_bool(name, value, &flag) == -1)
-return -1;
+parse_option_bool(name, value, &flag, &local_err);
+if (error_is_set(&local_err)) {
+goto exit_err;
+}
 list->value.n = flag;
 break;
 
@@ -299,6 +301,11 @@ int set_option_parameter(QEMUOptionParameter *list, const 
char *name,
 }
 
 return 0;
+
+exit_err:
+qerror_report_err(local_err);
+error_free(local_err);
+return -1;
 }
 
 /*
@@ -588,7 +595,8 @@ static int qemu_opt_parse(QemuOpt *opt)
 /* nothing */
 return 0;
 case QEMU_OPT_BOOL:
-return parse_option_bool(opt->name, opt->str, &opt->value.boolean);
+parse_option_bool(opt->name, opt->str, &opt->value.boolean, 
&local_err);
+break;
 case QEMU_OPT_NUMBER:
 parse_option_number(opt->name, opt->str, &opt->value.uint,
 &local_err);
-- 
1.7.9.2.384.g4a92a

[Qemu-devel] [PATCH 01/13] qemu-option: qemu_opts_create(): use error_set()

[Luiz Capitulino]

This commit converts qemu_opts_create() from qerror_report() to
error_set().

This means that qemu_opts_create() now takes an Error argument and
callers need to pass it if they're interested in getting error
information.

Currently, most calls to qemu_opts_create() can't fail, so most
callers don't need any changes.

The two cases where code checks for qemu_opts_create() erros are:

 1. Initialization code in vl.c. All of them print their own
error messages directly to stderr, no need to pass the Error
object

 2. The functions opts_parse(), qemu_opts_from_qdict() and
qemu_chr_parse_compat() make use of the error information and
they can be called from HMP or QMP. In this case, to allow for
incremental conversion, we propagate the error up using
qerror_report_err(), which keep the QError semantics

Signed-off-by: Luiz Capitulino 
---
 blockdev.c   |2 +-
 hw/usb/dev-storage.c |2 +-
 hw/watchdog.c|2 +-
 qemu-char.c  |8 ++--
 qemu-config.c|6 +++---
 qemu-option.c|   37 +++--
 qemu-option.h|4 +++-
 qemu-sockets.c   |8 
 vl.c |   22 +-
 9 files changed, 59 insertions(+), 32 deletions(-)

diff --git a/blockdev.c b/blockdev.c
index 1a500b8..77be66a 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -565,7 +565,7 @@ DriveInfo *drive_init(QemuOpts *opts, int default_to_scsi)
 break;
 case IF_VIRTIO:
 /* add virtio block device */
-opts = qemu_opts_create(qemu_find_opts("device"), NULL, 0);
+opts = qemu_opts_create(qemu_find_opts("device"), NULL, 0, NULL);
 if (arch_type == QEMU_ARCH_S390X) {
 qemu_opt_set(opts, "driver", "virtio-blk-s390");
 } else {
diff --git a/hw/usb/dev-storage.c b/hw/usb/dev-storage.c
index bdbe7bd..c9c59be 100644
--- a/hw/usb/dev-storage.c
+++ b/hw/usb/dev-storage.c
@@ -580,7 +580,7 @@ static USBDevice *usb_msd_init(USBBus *bus, const char 
*filename)
 
 /* parse -usbdevice disk: syntax into drive opts */
 snprintf(id, sizeof(id), "usb%d", nr++);
-opts = qemu_opts_create(qemu_find_opts("drive"), id, 0);
+opts = qemu_opts_create(qemu_find_opts("drive"), id, 0, NULL);
 
 p1 = strchr(filename, ':');
 if (p1++) {
diff --git a/hw/watchdog.c b/hw/watchdog.c
index 4c18965..a42124d 100644
--- a/hw/watchdog.c
+++ b/hw/watchdog.c
@@ -66,7 +66,7 @@ int select_watchdog(const char *p)
 QLIST_FOREACH(model, &watchdog_list, entry) {
 if (strcasecmp(model->wdt_name, p) == 0) {
 /* add the device */
-opts = qemu_opts_create(qemu_find_opts("device"), NULL, 0);
+opts = qemu_opts_create(qemu_find_opts("device"), NULL, 0, NULL);
 qemu_opt_set(opts, "driver", p);
 return 0;
 }
diff --git a/qemu-char.c b/qemu-char.c
index bb9e3f5..67a5ff0 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -2581,10 +2581,14 @@ QemuOpts *qemu_chr_parse_compat(const char *label, 
const char *filename)
 int pos;
 const char *p;
 QemuOpts *opts;
+Error *local_err = NULL;
 
-opts = qemu_opts_create(qemu_find_opts("chardev"), label, 1);
-if (NULL == opts)
+opts = qemu_opts_create(qemu_find_opts("chardev"), label, 1, &local_err);
+if (error_is_set(&local_err)) {
+qerror_report_err(local_err);
+error_free(local_err);
 return NULL;
+}
 
 if (strstart(filename, "mon:", &p)) {
 filename = p;
diff --git a/qemu-config.c b/qemu-config.c
index be84a03..f876646 100644
--- a/qemu-config.c
+++ b/qemu-config.c
@@ -709,7 +709,7 @@ int qemu_global_option(const char *str)
 return -1;
 }
 
-opts = qemu_opts_create(&qemu_global_opts, NULL, 0);
+opts = qemu_opts_create(&qemu_global_opts, NULL, 0, NULL);
 qemu_opt_set(opts, "driver", driver);
 qemu_opt_set(opts, "property", property);
 qemu_opt_set(opts, "value", str+offset+1);
@@ -781,7 +781,7 @@ int qemu_config_parse(FILE *fp, QemuOptsList **lists, const 
char *fname)
 list = find_list(lists, group);
 if (list == NULL)
 goto out;
-opts = qemu_opts_create(list, id, 1);
+opts = qemu_opts_create(list, id, 1, NULL);
 continue;
 }
 if (sscanf(line, "[%63[^]]]", group) == 1) {
@@ -789,7 +789,7 @@ int qemu_config_parse(FILE *fp, QemuOptsList **lists, const 
char *fname)
 list = find_list(lists, group);
 if (list == NULL)
 goto out;
-opts = qemu_opts_create(list, NULL, 0);
+opts = qemu_opts_create(list, NULL, 0, NULL);
 continue;
 }
 if (sscanf(line, " %63s = \"%1023[^\"]\"", arg, value) == 2) {
diff --git a/qemu-option.c b/qemu-option.c
index 35cd609..9f531c8 100644
--- a/qemu-option.c
+++ b/qemu-option.c
@@ -30,6 +30,7 @@
 #include "qemu-error.h"
 #include "qemu-objects.h"
 #include "qemu-option.

Re: [Qemu-devel] [PATCH 2/2] qapi: convert device_del

[Eric Blake]

On 03/29/2012 10:56 AM, Luiz Capitulino wrote:
> Signed-off-by: Anthony Liguori 
> Signed-off-by: Luiz Capitulino 
> ---
>  hmp-commands.hx   |3 +--
>  hmp.c |9 +
>  hmp.h |1 +
>  hw/qdev-monitor.c |   18 +-
>  qapi-schema.json  |   20 
>  qmp-commands.hx   |5 +
>  6 files changed, 37 insertions(+), 19 deletions(-)

> +# @device_del:
> +#
> +# Remove a device from a guest
> +#
> +# @id: the name of the device
> +#
> +# Returns: Nothing on success
> +#  If @id is not a valid device, DeviceNotFound
> +#  If the device does not support unplug, BusNoHotplug
> +#
> +# Notes: When this command completes, the device may not be removed from the
> +#guest.  Hot removal is an operation that requires guest cooperation.
> +#This command merely requests that the guest begin the hot removal
> +#process.

Nothing against your patch itself, but is there any way we could enhance
things in future patches to actually notify the management app when the
guest has cooperated and the devices is actually removed?  A new event
would be helpful, as well as a way to detect whether the new event
exists and should be waited for.

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



signature.asc
Description: OpenPGP digital signature

[Qemu-devel] [PATCH 06/13] qemu-option: opt_set(): use error_set()

[Luiz Capitulino]

The functions qemu_opt_set() and opts_do_parse() both call opt_set(),
but their callers expect QError semantics. Thus, both functions call
qerro_report_err() to keep the expected semantics.

Signed-off-by: Luiz Capitulino 
---
 qemu-option.c |   38 +-
 1 file changed, 21 insertions(+), 17 deletions(-)

diff --git a/qemu-option.c b/qemu-option.c
index 4829de5..38461d4 100644
--- a/qemu-option.c
+++ b/qemu-option.c
@@ -615,12 +615,11 @@ static void qemu_opt_del(QemuOpt *opt)
 g_free(opt);
 }
 
-static int opt_set(QemuOpts *opts, const char *name, const char *value,
-   bool prepend)
+static void opt_set(QemuOpts *opts, const char *name, const char *value,
+bool prepend, Error **errp)
 {
 QemuOpt *opt;
 const QemuOptDesc *desc = opts->list->desc;
-Error *local_err = NULL;
 int i;
 
 for (i = 0; desc[i].name != NULL; i++) {
@@ -632,8 +631,8 @@ static int opt_set(QemuOpts *opts, const char *name, const 
char *value,
 if (i == 0) {
 /* empty list -> allow any */;
 } else {
-error_set(&local_err, QERR_INVALID_PARAMETER, name);
-goto exit_err;
+error_set(errp, QERR_INVALID_PARAMETER, name);
+return;
 }
 }
 
@@ -651,23 +650,24 @@ static int opt_set(QemuOpts *opts, const char *name, 
const char *value,
 if (value) {
 opt->str = g_strdup(value);
 }
-qemu_opt_parse(opt, &local_err);
-if (error_is_set(&local_err)) {
+qemu_opt_parse(opt, errp);
+if (error_is_set(errp)) {
 qemu_opt_del(opt);
-goto exit_err;
 }
-
-return 0;
-
-exit_err:
-qerror_report_err(local_err);
-error_free(local_err);
-return -1;
 }
 
 int qemu_opt_set(QemuOpts *opts, const char *name, const char *value)
 {
-return opt_set(opts, name, value, false);
+Error *local_err = NULL;
+
+opt_set(opts, name, value, false, &local_err);
+if (error_is_set(&local_err)) {
+qerror_report_err(local_err);
+error_free(local_err);
+return -1;
+}
+
+return 0;
 }
 
 int qemu_opt_set_bool(QemuOpts *opts, const char *name, bool val)
@@ -853,6 +853,7 @@ static int opts_do_parse(QemuOpts *opts, const char *params,
 {
 char option[128], value[1024];
 const char *p,*pe,*pc;
+Error *local_err = NULL;
 
 for (p = params; *p != '\0'; p++) {
 pe = strchr(p, '=');
@@ -884,7 +885,10 @@ static int opts_do_parse(QemuOpts *opts, const char 
*params,
 }
 if (strcmp(option, "id") != 0) {
 /* store and parse */
-if (opt_set(opts, option, value, prepend) == -1) {
+opt_set(opts, option, value, prepend, &local_err);
+if (error_is_set(&local_err)) {
+qerror_report_err(local_err);
+error_free(local_err);
 return -1;
 }
 }
-- 
1.7.9.2.384.g4a92a

Re: [Qemu-devel] [RFC PATCH v3 0/8] Rewrite tracetool using python modules

[Stefan Hajnoczi]

On Tue, Mar 27, 2012 at 09:49:12PM +0200, Lluís Vilanova wrote:
> A full rewrite of the tracetool script using per-format and per-backend 
> modules,
> so that it's easier to read and extend it in the future.
> 
> Signed-off-by: Lluís Vilanova 
> ---
> NOTE: This series applies in current master, ignoring the "Rewrite tracetool
>   using python" series.
> 
> Changes in v3:
> 
> * Some minor fixes according to comments from Alon Levy:
> ** Fixed some rogue tabs.
> ** Fixed definition of '_SCRIPT' in "tracetool.py".
> ** Fixed some documentation.
> ** Defensively treat results from 'try_import' in 'get_list'.
> ** Fixed the no-arguments case in 'tracetool.backend.dtrace.d'.
> ** Fixed the commandline checks for the "stap" format.
> ** Added '__repr__' methods for both Event and Arguments objects.
> 
> 
> Changes in v2:
> 
> * Fixed a strange import error.
> * Add a pointer to 'tracetool.out' in the format and backend documentation.
> 
> Lluís Vilanova (8):
>   tracetool: Rewrite infrastructure as python modules
>   tracetool: Add module for the 'c' format
>   tracetool: Add module for the 'h' format
>   tracetool: Add support for the 'stderr' backend
>   tracetool: Add support for the 'simple' backend
>   tracetool: Add support for the 'ust' backend
>   tracetool: Add support for the 'dtrace' backend
>   tracetool: Add MAINTAINERS info
> 
> 
>  MAINTAINERS   |2 
>  Makefile.objs |6 
>  Makefile.target   |   13 -
>  configure |4 
>  scripts/tracetool |  648 
> -
>  scripts/tracetool.py  |  141 +++
>  scripts/tracetool/__init__.py |  223 +++
>  scripts/tracetool/backend/__init__.py |  119 ++
>  scripts/tracetool/backend/dtrace.py   |  104 +
>  scripts/tracetool/backend/simple.py   |   60 +++
>  scripts/tracetool/backend/stderr.py   |   61 +++
>  scripts/tracetool/backend/ust.py  |  102 +
>  scripts/tracetool/format/__init__.py  |   96 +
>  scripts/tracetool/format/c.py |   20 +
>  scripts/tracetool/format/d.py |   20 +
>  scripts/tracetool/format/h.py |   45 ++
>  scripts/tracetool/format/stap.py  |   20 +
>  17 files changed, 1025 insertions(+), 659 deletions(-)
>  delete mode 100755 scripts/tracetool
>  create mode 100755 scripts/tracetool.py
>  create mode 100644 scripts/tracetool/__init__.py
>  create mode 100644 scripts/tracetool/backend/__init__.py
>  create mode 100644 scripts/tracetool/backend/dtrace.py
>  create mode 100644 scripts/tracetool/backend/simple.py
>  create mode 100644 scripts/tracetool/backend/stderr.py
>  create mode 100644 scripts/tracetool/backend/ust.py
>  create mode 100644 scripts/tracetool/format/__init__.py
>  create mode 100644 scripts/tracetool/format/c.py
>  create mode 100644 scripts/tracetool/format/d.py
>  create mode 100644 scripts/tracetool/format/h.py
>  create mode 100644 scripts/tracetool/format/stap.py

Only one point: please don't introduce PUBLIC yet.  Let's add it when
it's needed.  At the moment nothing uses it.

I have tested this series with all backends and looked at the diff
between the old tracetool and tracetool.py.

I'm happy with this series.  We need to test it hard for 1.1 to make
sure there are no hickups for tracing users.

Stefan

[Qemu-devel] [PATCH 04/13] qemu-option: parse_option_size(): use error_set()

[Luiz Capitulino]

Signed-off-by: Luiz Capitulino 
---
 qemu-option.c |   20 +++-
 1 file changed, 11 insertions(+), 9 deletions(-)

diff --git a/qemu-option.c b/qemu-option.c
index a8b50af..61354af 100644
--- a/qemu-option.c
+++ b/qemu-option.c
@@ -203,7 +203,8 @@ static void parse_option_number(const char *name, const 
char *value,
 }
 }
 
-static int parse_option_size(const char *name, const char *value, uint64_t 
*ret)
+static void parse_option_size(const char *name, const char *value,
+  uint64_t *ret, Error **errp)
 {
 char *postfix;
 double sizef;
@@ -229,16 +230,14 @@ static int parse_option_size(const char *name, const char 
*value, uint64_t *ret)
 *ret = (uint64_t) sizef;
 break;
 default:
-qerror_report(QERR_INVALID_PARAMETER_VALUE, name, "a size");
+error_set(errp, QERR_INVALID_PARAMETER_VALUE, name, "a size");
 error_printf_unless_qmp("You may use k, M, G or T suffixes for "
 "kilobytes, megabytes, gigabytes and terabytes.\n");
-return -1;
+return;
 }
 } else {
-qerror_report(QERR_INVALID_PARAMETER_VALUE, name, "a size");
-return -1;
+error_set(errp, QERR_INVALID_PARAMETER_VALUE, name, "a size");
 }
-return 0;
 }
 
 /*
@@ -291,8 +290,10 @@ int set_option_parameter(QEMUOptionParameter *list, const 
char *name,
 break;
 
 case OPT_SIZE:
-if (parse_option_size(name, value, &list->value.n) == -1)
-return -1;
+parse_option_size(name, value, &list->value.n, &local_err);
+if (error_is_set(&local_err)) {
+goto exit_err;
+}
 break;
 
 default:
@@ -602,7 +603,8 @@ static int qemu_opt_parse(QemuOpt *opt)
 &local_err);
 break;
 case QEMU_OPT_SIZE:
-return parse_option_size(opt->name, opt->str, &opt->value.uint);
+parse_option_size(opt->name, opt->str, &opt->value.uint, &local_err);
+break;
 default:
 abort();
 }
-- 
1.7.9.2.384.g4a92a

[Qemu-devel] [PATCH 08/13] qemu-option: introduce qemu_opt_set_err()

[Luiz Capitulino]

This is like qemu_opt_set(), except that it takes an Error argument.

This new function allows for a incremental conversion of code using
qemu_opt_set().

Signed-off-by: Luiz Capitulino 
---
 qemu-option.c |6 ++
 qemu-option.h |2 ++
 2 files changed, 8 insertions(+)

diff --git a/qemu-option.c b/qemu-option.c
index 666eacd..881467d 100644
--- a/qemu-option.c
+++ b/qemu-option.c
@@ -670,6 +670,12 @@ int qemu_opt_set(QemuOpts *opts, const char *name, const 
char *value)
 return 0;
 }
 
+void qemu_opt_set_err(QemuOpts *opts, const char *name, const char *value,
+  Error **errp)
+{
+opt_set(opts, name, value, false, errp);
+}
+
 int qemu_opt_set_bool(QemuOpts *opts, const char *name, bool val)
 {
 QemuOpt *opt;
diff --git a/qemu-option.h b/qemu-option.h
index 4d5b3d3..52e9ec1 100644
--- a/qemu-option.h
+++ b/qemu-option.h
@@ -111,6 +111,8 @@ bool qemu_opt_get_bool(QemuOpts *opts, const char *name, 
bool defval);
 uint64_t qemu_opt_get_number(QemuOpts *opts, const char *name, uint64_t 
defval);
 uint64_t qemu_opt_get_size(QemuOpts *opts, const char *name, uint64_t defval);
 int qemu_opt_set(QemuOpts *opts, const char *name, const char *value);
+void qemu_opt_set_err(QemuOpts *opts, const char *name, const char *value,
+  Error **errp);
 int qemu_opt_set_bool(QemuOpts *opts, const char *name, bool val);
 typedef int (*qemu_opt_loopfunc)(const char *name, const char *value, void 
*opaque);
 int qemu_opt_foreach(QemuOpts *opts, qemu_opt_loopfunc func, void *opaque,
-- 
1.7.9.2.384.g4a92a

[Qemu-devel] [PATCH 07/13] qemu-option: opts_do_parse(): use error_set()

[Luiz Capitulino]

The functions qemu_opts_do_parse() and opts_parse() both call
opts_do_parse(), but their callers expect QError semantics. Thus,
both functions call qerro_report_err() to keep the expected semantics.

Signed-off-by: Luiz Capitulino 
---
 qemu-option.c |   38 --
 1 file changed, 24 insertions(+), 14 deletions(-)

diff --git a/qemu-option.c b/qemu-option.c
index 38461d4..666eacd 100644
--- a/qemu-option.c
+++ b/qemu-option.c
@@ -848,12 +848,11 @@ int qemu_opts_print(QemuOpts *opts, void *dummy)
 return 0;
 }
 
-static int opts_do_parse(QemuOpts *opts, const char *params,
- const char *firstname, bool prepend)
+static void opts_do_parse(QemuOpts *opts, const char *params,
+  const char *firstname, bool prepend, Error **errp)
 {
 char option[128], value[1024];
 const char *p,*pe,*pc;
-Error *local_err = NULL;
 
 for (p = params; *p != '\0'; p++) {
 pe = strchr(p, '=');
@@ -885,23 +884,29 @@ static int opts_do_parse(QemuOpts *opts, const char 
*params,
 }
 if (strcmp(option, "id") != 0) {
 /* store and parse */
-opt_set(opts, option, value, prepend, &local_err);
-if (error_is_set(&local_err)) {
-qerror_report_err(local_err);
-error_free(local_err);
-return -1;
+opt_set(opts, option, value, prepend, errp);
+if (error_is_set(errp)) {
+return;
 }
 }
 if (*p != ',') {
 break;
 }
 }
-return 0;
 }
 
 int qemu_opts_do_parse(QemuOpts *opts, const char *params, const char 
*firstname)
 {
-return opts_do_parse(opts, params, firstname, false);
+Error *local_err = NULL;
+
+opts_do_parse(opts, params, firstname, false, &local_err);
+if (error_is_set(&local_err)) {
+qerror_report_err(local_err);
+error_free(local_err);
+return -1;
+}
+
+return 0;
 }
 
 static QemuOpts *opts_parse(QemuOptsList *list, const char *params,
@@ -934,18 +939,23 @@ static QemuOpts *opts_parse(QemuOptsList *list, const 
char *params,
 }
 if (opts == NULL) {
 if (error_is_set(&local_err)) {
-qerror_report_err(local_err);
-error_free(local_err);
+goto exit_err;
 }
 return NULL;
 }
 
-if (opts_do_parse(opts, params, firstname, defaults) != 0) {
+opts_do_parse(opts, params, firstname, defaults, &local_err);
+if (error_is_set(&local_err)) {
 qemu_opts_del(opts);
-return NULL;
+goto exit_err;
 }
 
 return opts;
+
+exit_err:
+qerror_report_err(local_err);
+error_free(local_err);
+return NULL;
 }
 
 QemuOpts *qemu_opts_parse(QemuOptsList *list, const char *params,
-- 
1.7.9.2.384.g4a92a

[Qemu-devel] [PATCH 12/13] qapi: support for keyworded variable-length argument list

[Luiz Capitulino]

This allows for QAPI functions to receive a variable-length argument
list. This is going to be used by device_add and netdev_add commands.

In the schema, the argument list is represented by type name '**',
like this example:

{ 'command': 'foo', 'data': { 'arg-list': '**' } }

Each argument is represented by the KeyValues type and the C
implementation should expect a KeyValuesList, like:

void qmp_foo(KeyValuesList *values_list, Error **errp);

XXX: This implementation is simple but very hacky. We just iterate
 through all arguments and build the KeyValuesList list to be
 passed to the QAPI function.

 Maybe we could have a kwargs type, that does exactly this but
 through a visitor instead?

Signed-off-by: Luiz Capitulino 
---
 qapi-schema.json |   15 +++
 scripts/qapi-commands.py |   31 ---
 scripts/qapi.py  |2 ++
 3 files changed, 45 insertions(+), 3 deletions(-)

diff --git a/qapi-schema.json b/qapi-schema.json
index 0d11d6e..25bd487 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -1701,3 +1701,18 @@
 # Since: 1.1
 ##
 { 'command': 'xen-save-devices-state', 'data': {'filename': 'str'} }
+
+##
+# @KeyValues:
+#
+# A generic representation of a key value pair.
+#
+# @key: the name of the item
+#
+# @value: the string representation of the item value.  This typically follows
+# QEMU's command line parsing format.  See the man pages for more
+# information.
+#
+# Since: 0.14.0
+##
+{ 'type': 'KeyValues', 'data': {'key': 'str', 'value': 'str'} }
diff --git a/scripts/qapi-commands.py b/scripts/qapi-commands.py
index 30a24d2..75a6e81 100644
--- a/scripts/qapi-commands.py
+++ b/scripts/qapi-commands.py
@@ -146,19 +146,44 @@ v = qmp_input_get_visitor(mi);
  obj=obj)
 
 for argname, argtype, optional, structured in parse_args(args):
-if optional:
+if optional and not '**':
 ret += mcgen('''
 visit_start_optional(v, &has_%(c_name)s, "%(name)s", errp);
 if (has_%(c_name)s) {
 ''',
  c_name=c_var(argname), name=argname)
 push_indent()
-ret += mcgen('''
+if argtype == '**':
+if dealloc:
+ret += mcgen('''
+qapi_free_KeyValuesList(%(obj)s);
+''',
+obj=c_var(argname))
+else:
+ret += mcgen('''
+{
+const QDictEntry *entry;
+v = v; /* fix me baby */
+
+for (entry = qdict_first(args); entry; entry = qdict_next(qdict, entry)) {
+KeyValuesList *item = g_malloc0(sizeof(*item));
+item->value = g_malloc0(sizeof(*item->value));
+item->value->key = g_strdup(qdict_entry_key(entry));
+item->value->value = 
g_strdup(qstring_get_str(qobject_to_qstring(qdict_entry_value(entry;
+
+item->next = %(obj)s;
+%(obj)s = item;
+}
+}
+''',
+obj=c_var(argname))
+else:
+ret += mcgen('''
 %(visitor)s(v, &%(c_name)s, "%(name)s", errp);
 ''',
  c_name=c_var(argname), name=argname, argtype=argtype,
  visitor=type_visitor(argtype))
-if optional:
+if optional and not '**':
 pop_indent()
 ret += mcgen('''
 }
diff --git a/scripts/qapi.py b/scripts/qapi.py
index e062336..87b9ee6 100644
--- a/scripts/qapi.py
+++ b/scripts/qapi.py
@@ -163,6 +163,8 @@ def c_type(name):
 return 'bool'
 elif name == 'number':
 return 'double'
+elif name == '**':
+return 'KeyValuesList *'
 elif type(name) == list:
 return '%s *' % c_list_type(name[0])
 elif is_enum(name):
-- 
1.7.9.2.384.g4a92a

[Qemu-devel] [PATCH 11/13] qemu-config: introduce qemu_find_opts_err()

[Luiz Capitulino]

This is like qemu_find_opts(), except that it takes an Error argument.

This new function allows for a incremental conversion of code using
qemu_find_opts().

Signed-off-by: Luiz Capitulino 
---
 qemu-config.c |5 +
 qemu-config.h |3 +++
 2 files changed, 8 insertions(+)

diff --git a/qemu-config.c b/qemu-config.c
index bdb381d..bb3bff4 100644
--- a/qemu-config.c
+++ b/qemu-config.c
@@ -661,6 +661,11 @@ QemuOptsList *qemu_find_opts(const char *group)
 return ret;
 }
 
+QemuOptsList *qemu_find_opts_err(const char *group, Error **errp)
+{
+return find_list(vm_config_groups, group, errp);
+}
+
 void qemu_add_opts(QemuOptsList *list)
 {
 int entries, i;
diff --git a/qemu-config.h b/qemu-config.h
index 20d707f..a093e3f 100644
--- a/qemu-config.h
+++ b/qemu-config.h
@@ -1,11 +1,14 @@
 #ifndef QEMU_CONFIG_H
 #define QEMU_CONFIG_H
 
+#include "error.h"
+
 extern QemuOptsList qemu_fsdev_opts;
 extern QemuOptsList qemu_virtfs_opts;
 extern QemuOptsList qemu_spice_opts;
 
 QemuOptsList *qemu_find_opts(const char *group);
+QemuOptsList *qemu_find_opts_err(const char *group, Error **errp);
 void qemu_add_opts(QemuOptsList *list);
 int qemu_set_option(const char *str);
 int qemu_global_option(const char *str);
-- 
1.7.9.2.384.g4a92a

[Qemu-devel] [PATCH 13/13] qapi: convert device_add

[Luiz Capitulino]

FIXME: HMP's help text (device_add ?) is not implemented. We need a way to
   export this information in QMP.

Signed-off-by: Anthony Liguori 
Signed-off-by: Luiz Capitulino 
---
 hmp-commands.hx   |3 +--
 hmp.c |   24 ++
 hmp.h |1 +
 hw/qdev-monitor.c |   70 +++--
 hw/qdev.h |2 +-
 qapi-schema.json  |   23 ++
 qmp-commands.hx   |3 +--
 vl.c  |3 ++-
 8 files changed, 105 insertions(+), 24 deletions(-)

diff --git a/hmp-commands.hx b/hmp-commands.hx
index bd35a3e..376dc4d 100644
--- a/hmp-commands.hx
+++ b/hmp-commands.hx
@@ -606,8 +606,7 @@ ETEXI
 .args_type  = "device:O",
 .params = "driver[,prop=value][,...]",
 .help   = "add device, like -device on the command line",
-.user_print = monitor_user_noop,
-.mhandler.cmd_new = do_device_add,
+.mhandler.cmd = hmp_device_add,
 },
 
 STEXI
diff --git a/hmp.c b/hmp.c
index 9cf2d13..23c06ec 100644
--- a/hmp.c
+++ b/hmp.c
@@ -934,3 +934,27 @@ void hmp_migrate(Monitor *mon, const QDict *qdict)
 qemu_mod_timer(status->timer, qemu_get_clock_ms(rt_clock));
 }
 }
+
+void hmp_device_add(Monitor *mon, const QDict *qdict)
+{
+KeyValuesList *opts_list = NULL;
+const QDictEntry *entry;
+Error *err = NULL;
+
+for (entry = qdict_first(qdict); entry; entry = qdict_next(qdict, entry)) {
+const char *key = qdict_entry_key(entry);
+const char *value = 
qstring_get_str(qobject_to_qstring(qdict_entry_value(entry)));
+KeyValuesList *kv;
+
+kv = g_malloc0(sizeof(*kv));
+kv->value = g_malloc0(sizeof(*kv->value));
+kv->value->key = g_strdup(key);
+kv->value->value = g_strdup(value);
+kv->next = opts_list;
+opts_list = kv;
+}
+
+qmp_device_add(opts_list, &err);
+qapi_free_KeyValuesList(opts_list);
+hmp_handle_error(mon, &err);
+}
diff --git a/hmp.h b/hmp.h
index 8807853..3e75d4c 100644
--- a/hmp.h
+++ b/hmp.h
@@ -60,5 +60,6 @@ void hmp_block_stream(Monitor *mon, const QDict *qdict);
 void hmp_block_job_set_speed(Monitor *mon, const QDict *qdict);
 void hmp_block_job_cancel(Monitor *mon, const QDict *qdict);
 void hmp_migrate(Monitor *mon, const QDict *qdict);
+void hmp_device_add(Monitor *mon, const QDict *qdict);
 
 #endif
diff --git a/hw/qdev-monitor.c b/hw/qdev-monitor.c
index a310cc7..0661f55 100644
--- a/hw/qdev-monitor.c
+++ b/hw/qdev-monitor.c
@@ -19,6 +19,7 @@
 
 #include "qdev.h"
 #include "monitor.h"
+#include "qmp-commands.h"
 
 /*
  * Aliases were a bad idea from the start.  Let's keep them
@@ -388,7 +389,7 @@ static BusState *qbus_find(const char *path)
 }
 }
 
-DeviceState *qdev_device_add(QemuOpts *opts)
+DeviceState *qdev_device_add(QemuOpts *opts, Error **errp)
 {
 ObjectClass *obj;
 DeviceClass *k;
@@ -398,7 +399,7 @@ DeviceState *qdev_device_add(QemuOpts *opts)
 
 driver = qemu_opt_get(opts, "driver");
 if (!driver) {
-qerror_report(QERR_MISSING_PARAMETER, "driver");
+error_set(errp, QERR_MISSING_PARAMETER, "driver");
 return NULL;
 }
 
@@ -414,7 +415,7 @@ DeviceState *qdev_device_add(QemuOpts *opts)
 }
 
 if (!obj) {
-qerror_report(QERR_INVALID_PARAMETER_VALUE, "driver", "device type");
+error_set(errp, QERR_INVALID_PARAMETER_VALUE, "driver", "device type");
 return NULL;
 }
 
@@ -428,20 +429,18 @@ DeviceState *qdev_device_add(QemuOpts *opts)
 return NULL;
 }
 if (bus->info != k->bus_info) {
-qerror_report(QERR_BAD_BUS_FOR_DEVICE,
-   driver, bus->info->name);
+error_set(errp, QERR_BAD_BUS_FOR_DEVICE, driver, bus->info->name);
 return NULL;
 }
 } else {
 bus = qbus_find_recursive(sysbus_get_default(), NULL, k->bus_info);
 if (!bus) {
-qerror_report(QERR_NO_BUS_FOR_DEVICE,
-  driver, k->bus_info->name);
+error_set(errp, QERR_NO_BUS_FOR_DEVICE, driver, k->bus_info->name);
 return NULL;
 }
 }
 if (qdev_hotplug && !bus->allow_hotplug) {
-qerror_report(QERR_BUS_NO_HOTPLUG, bus->name);
+error_set(errp, QERR_BUS_NO_HOTPLUG, bus->name);
 return NULL;
 }
 
@@ -463,7 +462,7 @@ DeviceState *qdev_device_add(QemuOpts *opts)
 return NULL;
 }
 if (qdev_init(qdev) < 0) {
-qerror_report(QERR_DEVICE_INIT_FAILED, driver);
+error_set(errp, QERR_DEVICE_INIT_FAILED, driver);
 return NULL;
 }
 if (qdev->id) {
@@ -555,23 +554,58 @@ void do_info_qdm(Monitor *mon)
 object_class_foreach(qdev_print_devinfo, TYPE_DEVICE, false, NULL);
 }
 
-int do_device_add(Monitor *mon, const QDict *qdict, QObject **ret_data)
+void qmp_device_add(KeyValuesList *opts, Error **errp)
 {
-QemuOpts *opts;
+Error *local_err = NULL;
+QemuO

[Qemu-devel] [PATCH 09/13] qerror: introduce QERR_INVALID_OPTION_GROUP

[Luiz Capitulino]

Signed-off-by: Luiz Capitulino 
---
 qerror.c |4 
 qerror.h |3 +++
 2 files changed, 7 insertions(+)

diff --git a/qerror.c b/qerror.c
index 41c729a..02e3a14 100644
--- a/qerror.c
+++ b/qerror.c
@@ -156,6 +156,10 @@ static const QErrorStringTable qerror_table[] = {
 .desc  = "Invalid block format '%(name)'",
 },
 {
+.error_fmt = QERR_INVALID_OPTION_GROUP,
+.desc  = "There is no option group '%(group)'",
+},
+{
 .error_fmt = QERR_INVALID_PARAMETER,
 .desc  = "Invalid parameter '%(name)'",
 },
diff --git a/qerror.h b/qerror.h
index e16f9c2..c79 100644
--- a/qerror.h
+++ b/qerror.h
@@ -139,6 +139,9 @@ QError *qobject_to_qerror(const QObject *obj);
 #define QERR_INVALID_BLOCK_FORMAT \
 "{ 'class': 'InvalidBlockFormat', 'data': { 'name': %s } }"
 
+#define QERR_INVALID_OPTION_GROUP \
+"{ 'class': 'InvalidOptionGroup', 'data': { 'group': %s } }"
+
 #define QERR_INVALID_PARAMETER \
 "{ 'class': 'InvalidParameter', 'data': { 'name': %s } }"
 
-- 
1.7.9.2.384.g4a92a

[Qemu-devel] [PATCH v2 1/5] MAINTAINERS: Add entry for UniCore32

[Andreas Färber]

Signed-off-by: Andreas Färber 
Acked-by: Guan Xuetao 
---
 MAINTAINERS |5 +
 1 files changed, 5 insertions(+), 0 deletions(-)

diff --git a/MAINTAINERS b/MAINTAINERS
index f83d07c2..922945c 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -112,6 +112,11 @@ M: Blue Swirl 
 S: Maintained
 F: target-sparc/
 
+UniCore32
+M: Guan Xuetao 
+S: Maintained
+F: target-unicore32/
+
 X86
 M: qemu-devel@nongnu.org
 S: Odd Fixes
-- 
1.7.7

Re: [Qemu-devel] [PATCH 12/13] qapi: support for keyworded variable-length argument list

[Anthony Liguori]

On 03/29/2012 12:26 PM, Luiz Capitulino wrote:

This allows for QAPI functions to receive a variable-length argument
list. This is going to be used by device_add and netdev_add commands.

In the schema, the argument list is represented by type name '**',
like this example:

 { 'command': 'foo', 'data': { 'arg-list': '**' } }

Each argument is represented by the KeyValues type and the C
implementation should expect a KeyValuesList, like:

 void qmp_foo(KeyValuesList *values_list, Error **errp);

XXX: This implementation is simple but very hacky. We just iterate
  through all arguments and build the KeyValuesList list to be
  passed to the QAPI function.

  Maybe we could have a kwargs type, that does exactly this but
  through a visitor instead?

Signed-off-by: Luiz Capitulino


What about just treating '**' as "marshal remaining arguments to a string" and 
then pass that string to device_add?  qmp_device_add can then parse that string 
with QemuOpts.


It's a bit ugly, but that's how things worked.  When we introduce qom_add, this 
problem goes away because you would make multiple calls to qom_set to set all of 
the properties.


Regards,

Anthony Liguori


---
  qapi-schema.json |   15 +++
  scripts/qapi-commands.py |   31 ---
  scripts/qapi.py  |2 ++
  3 files changed, 45 insertions(+), 3 deletions(-)

diff --git a/qapi-schema.json b/qapi-schema.json
index 0d11d6e..25bd487 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -1701,3 +1701,18 @@
  # Since: 1.1
  ##
  { 'command': 'xen-save-devices-state', 'data': {'filename': 'str'} }
+
+##
+# @KeyValues:
+#
+# A generic representation of a key value pair.
+#
+# @key: the name of the item
+#
+# @value: the string representation of the item value.  This typically follows
+# QEMU's command line parsing format.  See the man pages for more
+# information.
+#
+# Since: 0.14.0
+##
+{ 'type': 'KeyValues', 'data': {'key': 'str', 'value': 'str'} }
diff --git a/scripts/qapi-commands.py b/scripts/qapi-commands.py
index 30a24d2..75a6e81 100644
--- a/scripts/qapi-commands.py
+++ b/scripts/qapi-commands.py
@@ -146,19 +146,44 @@ v = qmp_input_get_visitor(mi);
   obj=obj)

  for argname, argtype, optional, structured in parse_args(args):
-if optional:
+if optional and not '**':
  ret += mcgen('''
  visit_start_optional(v,&has_%(c_name)s, "%(name)s", errp);
  if (has_%(c_name)s) {
  ''',
   c_name=c_var(argname), name=argname)
  push_indent()
-ret += mcgen('''
+if argtype == '**':
+if dealloc:
+ret += mcgen('''
+qapi_free_KeyValuesList(%(obj)s);
+''',
+obj=c_var(argname))
+else:
+ret += mcgen('''
+{
+const QDictEntry *entry;
+v = v; /* fix me baby */
+
+for (entry = qdict_first(args); entry; entry = qdict_next(qdict, entry)) {
+KeyValuesList *item = g_malloc0(sizeof(*item));
+item->value = g_malloc0(sizeof(*item->value));
+item->value->key = g_strdup(qdict_entry_key(entry));
+item->value->value = 
g_strdup(qstring_get_str(qobject_to_qstring(qdict_entry_value(entry;
+
+item->next = %(obj)s;
+%(obj)s = item;
+}
+}
+''',
+obj=c_var(argname))
+else:
+ret += mcgen('''
  %(visitor)s(v,&%(c_name)s, "%(name)s", errp);
  ''',
   c_name=c_var(argname), name=argname, argtype=argtype,
   visitor=type_visitor(argtype))
-if optional:
+if optional and not '**':
  pop_indent()
  ret += mcgen('''
  }
diff --git a/scripts/qapi.py b/scripts/qapi.py
index e062336..87b9ee6 100644
--- a/scripts/qapi.py
+++ b/scripts/qapi.py
@@ -163,6 +163,8 @@ def c_type(name):
  return 'bool'
  elif name == 'number':
  return 'double'
+elif name == '**':
+return 'KeyValuesList *'
  elif type(name) == list:
  return '%s *' % c_list_type(name[0])
  elif is_enum(name):

[Qemu-devel] [PATCH 02/13] qemu-option: parse_option_number(): use error_set()

[Luiz Capitulino]

Note that qemu_opt_parse() callers still expect automatic error reporting
with QError, so qemu_opts_parse() calls qerror_report_err() to keep the
same semantics.

Signed-off-by: Luiz Capitulino 
---
 qemu-option.c |   26 +++---
 1 file changed, 19 insertions(+), 7 deletions(-)

diff --git a/qemu-option.c b/qemu-option.c
index 9f531c8..72dcb56 100644
--- a/qemu-option.c
+++ b/qemu-option.c
@@ -186,7 +186,8 @@ static int parse_option_bool(const char *name, const char 
*value, bool *ret)
 return 0;
 }
 
-static int parse_option_number(const char *name, const char *value, uint64_t 
*ret)
+static void parse_option_number(const char *name, const char *value,
+uint64_t *ret, Error **errp)
 {
 char *postfix;
 uint64_t number;
@@ -194,15 +195,13 @@ static int parse_option_number(const char *name, const 
char *value, uint64_t *re
 if (value != NULL) {
 number = strtoull(value, &postfix, 0);
 if (*postfix != '\0') {
-qerror_report(QERR_INVALID_PARAMETER_VALUE, name, "a number");
-return -1;
+error_set(errp, QERR_INVALID_PARAMETER_VALUE, name, "a number");
+return;
 }
 *ret = number;
 } else {
-qerror_report(QERR_INVALID_PARAMETER_VALUE, name, "a number");
-return -1;
+error_set(errp, QERR_INVALID_PARAMETER_VALUE, name, "a number");
 }
-return 0;
 }
 
 static int parse_option_size(const char *name, const char *value, uint64_t 
*ret)
@@ -579,8 +578,11 @@ uint64_t qemu_opt_get_size(QemuOpts *opts, const char 
*name, uint64_t defval)
 
 static int qemu_opt_parse(QemuOpt *opt)
 {
+Error *local_err = NULL;
+
 if (opt->desc == NULL)
 return 0;
+
 switch (opt->desc->type) {
 case QEMU_OPT_STRING:
 /* nothing */
@@ -588,12 +590,22 @@ static int qemu_opt_parse(QemuOpt *opt)
 case QEMU_OPT_BOOL:
 return parse_option_bool(opt->name, opt->str, &opt->value.boolean);
 case QEMU_OPT_NUMBER:
-return parse_option_number(opt->name, opt->str, &opt->value.uint);
+parse_option_number(opt->name, opt->str, &opt->value.uint,
+&local_err);
+break;
 case QEMU_OPT_SIZE:
 return parse_option_size(opt->name, opt->str, &opt->value.uint);
 default:
 abort();
 }
+
+if (error_is_set(&local_err)) {
+qerror_report_err(local_err);
+error_free(local_err);
+return -1;
+}
+
+return 0;
 }
 
 static void qemu_opt_del(QemuOpt *opt)
-- 
1.7.9.2.384.g4a92a

Re: [Qemu-devel] [PATCH] w32: Undefine error constants before their redefinition

[Jan Kiszka]

On 2012-03-29 20:02, Stefan Weil wrote:
> Am 28.03.2012 20:56, schrieb Jan Kiszka:
>> Avoids lots of warnings.
>>
>> Signed-off-by: Jan Kiszka 
>> ---
>> qemu_socket.h | 2 ++
>> 1 files changed, 2 insertions(+), 0 deletions(-)
>>
>> diff --git a/qemu_socket.h b/qemu_socket.h
>> index fe4cf6c..51ad210 100644
>> --- a/qemu_socket.h
>> +++ b/qemu_socket.h
>> @@ -8,7 +8,9 @@
>> #include 
>>
>> #define socket_error() WSAGetLastError()
>> +#undef EWOULDBLOCK
>> #undef EINTR
>> +#undef EINPROGRESS
>> #define EWOULDBLOCK WSAEWOULDBLOCK
>> #define EINTR WSAEINTR
>> #define EINPROGRESS WSAEINPROGRESS
> 
> Hi,
> 
> I am curious: with which version of MinGW or Cygwin do you get warnings?
> I don't see them in my native and cross MinGW / MinGW-w64 builds.

Not sure if that is what you are looking for, but my runtime rpm claims
to be mingw32-runtime-20120126-1.4.

> 
> Where do the original definitions come from, and are they compatible with
> the redefined values? If yes, it might be possible to put the new 
> definitions
> in a conditionally compiled code block (#if !defined(EWOULDBLOCK) ... 
> #endif).

/usr/i686-w64-mingw32/sys-root/mingw/include/errno.h:158:0: note: this
is the location of the previous definition

And there we have e.g.

#ifndef EINPROGRESS
#define EINPROGRESS 112
#endif

vs.

#define WSAEINPROGRESS 10036L

> 
> Could slirp/slirp.h also use qemu_socket.h? That would simplify the code.
> Is it possible to move those definitions to qemu-os-win32.h? I'd prefer
> to have them in some w32 specific header file instead of qemu_socket.h and
> slirp/slirp.h.

/me too. But as I cannot test w32, I would welcome to receive a tested
patch for the slirp queue. :)

Jan

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

Re: [Qemu-devel] [PATCH 12/13] qapi: support for keyworded variable-length argument list

[Luiz Capitulino]

On Thu, 29 Mar 2012 13:26:34 -0500
Anthony Liguori  wrote:

> On 03/29/2012 12:26 PM, Luiz Capitulino wrote:
> > This allows for QAPI functions to receive a variable-length argument
> > list. This is going to be used by device_add and netdev_add commands.
> >
> > In the schema, the argument list is represented by type name '**',
> > like this example:
> >
> >  { 'command': 'foo', 'data': { 'arg-list': '**' } }
> >
> > Each argument is represented by the KeyValues type and the C
> > implementation should expect a KeyValuesList, like:
> >
> >  void qmp_foo(KeyValuesList *values_list, Error **errp);
> >
> > XXX: This implementation is simple but very hacky. We just iterate
> >   through all arguments and build the KeyValuesList list to be
> >   passed to the QAPI function.
> >
> >   Maybe we could have a kwargs type, that does exactly this but
> >   through a visitor instead?
> >
> > Signed-off-by: Luiz Capitulino
> 
> What about just treating '**' as "marshal remaining arguments to a string" 
> and 
> then pass that string to device_add?  qmp_device_add can then parse that 
> string 
> with QemuOpts.

If this turns out to be simple enough, I'm fine with it.

> It's a bit ugly, but that's how things worked.  When we introduce qom_add, 
> this 
> problem goes away because you would make multiple calls to qom_set to set all 
> of 
> the properties.

Just out of curiosity, is qom_add going to supersede device_add?

Re: [Qemu-devel] [PATCH 03/14] tcg-sparc: Assume v9 cpu always, i.e. force v8plus in 32-bit mode.

[Blue Swirl]

On Wed, Mar 28, 2012 at 00:32, Richard Henderson  wrote:
> Current code doesn't actually work in 32-bit mode at all.  Since
> no one really noticed, drop the complication of v7 and v8 cpus.
> Eliminate the --sparc_cpu configure option and standardize macro
> testing on TCG_TARGET_REG_BITS / HOST_LONG_BITS
>
> Signed-off-by: Richard Henderson 
> ---
>  configure              |   41 -
>  disas.c                |    6 --
>  dyngen-exec.h          |    4 +---
>  exec.c                 |   12 +---
>  qemu-timer.h           |    8 +---
>  tcg/sparc/tcg-target.c |   20 +---
>  tcg/sparc/tcg-target.h |    7 ---
>  tcg/tcg.c              |    3 ++-
>  8 files changed, 26 insertions(+), 75 deletions(-)
>
> diff --git a/configure b/configure
> index 80ca430..7741ba9 100755
> --- a/configure
> +++ b/configure
> @@ -86,7 +86,6 @@ source_path=`dirname "$0"`
>  cpu=""
>  interp_prefix="/usr/gnemul/qemu-%M"
>  static="no"
> -sparc_cpu=""
>  cross_prefix=""
>  audio_drv_list=""
>  audio_card_list="ac97 es1370 sb16 hda"
> @@ -216,21 +215,6 @@ for opt do
>   ;;
>   --disable-debug-info) debug_info="no"
>   ;;
> -  --sparc_cpu=*)
> -    sparc_cpu="$optarg"
> -    case $sparc_cpu in
> -    v7|v8|v8plus|v8plusa)
> -      cpu="sparc"
> -    ;;
> -    v9)
> -      cpu="sparc64"
> -    ;;
> -    *)
> -      echo "undefined SPARC architecture. Exiting";
> -      exit 1
> -    ;;
> -    esac
> -  ;;
>   esac
>  done
>  # OS specific
> @@ -284,8 +268,6 @@ elif check_define __i386__ ; then
>  elif check_define __x86_64__ ; then
>   cpu="x86_64"
>  elif check_define __sparc__ ; then
> -  # We can't check for 64 bit (when gcc is biarch) or V8PLUSA
> -  # They must be specified using --sparc_cpu
>   if check_define __arch64__ ; then
>     cpu="sparc64"
>   else
> @@ -749,8 +731,6 @@ for opt do
>   ;;
>   --enable-uname-release=*) uname_release="$optarg"
>   ;;
> -  --sparc_cpu=*)
> -  ;;
>   --enable-werror) werror="yes"
>   ;;
>   --disable-werror) werror="no"
> @@ -830,32 +810,19 @@ for opt do
>   esac
>  done
>
> -#
> -# If cpu ~= sparc and  sparc_cpu hasn't been defined, plug in the right
> -# QEMU_CFLAGS/LDFLAGS (assume sparc_v8plus for 32-bit and sparc_v9 for 
> 64-bit)
> -#
>  host_guest_base="no"
>  case "$cpu" in
> -    sparc) case $sparc_cpu in
> -           v7|v8)
> -             QEMU_CFLAGS="-mcpu=${sparc_cpu} -D__sparc_${sparc_cpu}__ 
> $QEMU_CFLAGS"
> -           ;;
> -           v8plus|v8plusa)
> -             QEMU_CFLAGS="-mcpu=ultrasparc -D__sparc_${sparc_cpu}__ 
> $QEMU_CFLAGS"
> -           ;;
> -           *) # sparc_cpu not defined in the command line
> -             QEMU_CFLAGS="-mcpu=ultrasparc -D__sparc_v8plus__ $QEMU_CFLAGS"
> -           esac
> +    sparc)
>            LDFLAGS="-m32 $LDFLAGS"
> -           QEMU_CFLAGS="-m32 -ffixed-g2 -ffixed-g3 $QEMU_CFLAGS"
> +           QEMU_CFLAGS="-m32 -mcpu=ultrasparc $QEMU_CFLAGS"
> +           QEMU_CFLAGS="-ffixed-g2 -ffixed-g3 $QEMU_CFLAGS"
>            if test "$solaris" = "no" ; then
>              QEMU_CFLAGS="-ffixed-g1 -ffixed-g6 $QEMU_CFLAGS"
> -             helper_cflags="-ffixed-i0"
>            fi
>            ;;
>     sparc64)
> -           QEMU_CFLAGS="-m64 -mcpu=ultrasparc -D__sparc_v9__ $QEMU_CFLAGS"
>            LDFLAGS="-m64 $LDFLAGS"
> +           QEMU_CFLAGS="-m64 -mcpu=ultrasparc $QEMU_CFLAGS"
>            QEMU_CFLAGS="-ffixed-g5 -ffixed-g6 -ffixed-g7 $QEMU_CFLAGS"
>            if test "$solaris" != "no" ; then
>              QEMU_CFLAGS="-ffixed-g1 $QEMU_CFLAGS"
> diff --git a/disas.c b/disas.c
> index 4945c44..b3434fa 100644
> --- a/disas.c
> +++ b/disas.c
> @@ -175,9 +175,7 @@ void target_disas(FILE *out, target_ulong code, 
> target_ulong size, int flags)
>        print_insn = print_insn_arm;
>  #elif defined(TARGET_SPARC)
>     print_insn = print_insn_sparc;
> -#ifdef TARGET_SPARC64
>     disasm_info.mach = bfd_mach_sparc_v9b;
> -#endif

This is not OK, it would change ASI printout for V8 guest code.

>  #elif defined(TARGET_PPC)
>     if (flags >> 16)
>         disasm_info.endian = BFD_ENDIAN_LITTLE;
> @@ -287,9 +285,7 @@ void disas(FILE *out, void *code, unsigned long size)
>     print_insn = print_insn_alpha;
>  #elif defined(__sparc__)
>     print_insn = print_insn_sparc;
> -#if defined(__sparc_v8plus__) || defined(__sparc_v8plusa__) || 
> defined(__sparc_v9__)
>     disasm_info.mach = bfd_mach_sparc_v9b;
> -#endif

This change is OK, it's for Sparc V9 host.

>  #elif defined(__arm__)
>     print_insn = print_insn_arm;
>  #elif defined(__MIPSEB__)
> @@ -397,9 +393,7 @@ void monitor_disas(Monitor *mon, CPUArchState *env,
>     print_insn = print_insn_alpha;
>  #elif defined(TARGET_SPARC)
>     print_insn = print_insn_sparc;
> -#ifdef TARGET_SPARC64
>     disasm_info.mach = bfd_mach_sparc_v9b;
> -#endif

This is again for the guest code disassembly (from monitor) which
could be V8, so not OK.

>  #elif defined(TARGET_PPC)
>  #ifdef TARGET_PPC64
>     disasm_info.mach = b

Re: [Qemu-devel] [PATCH 05/14] tcg-sparc: Simplify qemu_ld/st direct memory paths.

[Blue Swirl]

On Wed, Mar 28, 2012 at 00:32, Richard Henderson  wrote:
> Given that we have an opcode for all sizes, all endianness,
> turn the functions into a simple table lookup.
>
> Signed-off-by: Richard Henderson 
> ---
>  tcg/sparc/tcg-target.c |  384 
> +++-
>  1 files changed, 150 insertions(+), 234 deletions(-)
>
> diff --git a/tcg/sparc/tcg-target.c b/tcg/sparc/tcg-target.c
> index c74fc2c..5cea5a8 100644
> --- a/tcg/sparc/tcg-target.c
> +++ b/tcg/sparc/tcg-target.c
> @@ -294,6 +294,16 @@ static inline int tcg_target_const_match(tcg_target_long 
> val,
>  #define ASI_PRIMARY_LITTLE 0x88
>  #endif
>
> +#define LDUH_LE    (LDUHA | INSN_ASI(ASI_PRIMARY_LITTLE))
> +#define LDSH_LE    (LDSHA | INSN_ASI(ASI_PRIMARY_LITTLE))
> +#define LDUW_LE    (LDUWA | INSN_ASI(ASI_PRIMARY_LITTLE))
> +#define LDSW_LE    (LDSWA | INSN_ASI(ASI_PRIMARY_LITTLE))
> +#define LDX_LE     (LDXA  | INSN_ASI(ASI_PRIMARY_LITTLE))
> +
> +#define STH_LE     (STHA  | INSN_ASI(ASI_PRIMARY_LITTLE))
> +#define STW_LE     (STWA  | INSN_ASI(ASI_PRIMARY_LITTLE))
> +#define STX_LE     (STXA  | INSN_ASI(ASI_PRIMARY_LITTLE))
> +
>  static inline void tcg_out_arith(TCGContext *s, int rd, int rs1, int rs2,
>                                  int op)
>  {
> @@ -366,66 +376,46 @@ static inline void tcg_out_movi(TCGContext *s, TCGType 
> type,
>     }
>  }
>
> -static inline void tcg_out_ld_raw(TCGContext *s, int ret,
> -                                  tcg_target_long arg)
> +static inline void tcg_out_ldst_rr(TCGContext *s, int data, int a1,
> +                                   int a2, int op)
>  {
> -    tcg_out_sethi(s, ret, arg);
> -    tcg_out32(s, LDUW | INSN_RD(ret) | INSN_RS1(ret) |
> -              INSN_IMM13(arg & 0x3ff));
> +    tcg_out32(s, op | INSN_RD(data) | INSN_RS1(a1) | INSN_RS2(a2));
>  }
>
> -static inline void tcg_out_ld_ptr(TCGContext *s, int ret,
> -                                  tcg_target_long arg)
> +static inline void tcg_out_ldst(TCGContext *s, int ret, int addr,
> +                                int offset, int op)
>  {
> -    if (!check_fit_tl(arg, 10))
> -        tcg_out_movi(s, TCG_TYPE_PTR, ret, arg & ~0x3ffULL);
> -    if (TCG_TARGET_REG_BITS == 64) {
> -        tcg_out32(s, LDX | INSN_RD(ret) | INSN_RS1(ret) |
> -                  INSN_IMM13(arg & 0x3ff));
> -    } else {
> -        tcg_out32(s, LDUW | INSN_RD(ret) | INSN_RS1(ret) |
> -                  INSN_IMM13(arg & 0x3ff));
> -    }
> -}
> -
> -static inline void tcg_out_ldst(TCGContext *s, int ret, int addr, int 
> offset, int op)
> -{
> -    if (check_fit_tl(offset, 13))
> +    if (check_fit_tl(offset, 13)) {
>         tcg_out32(s, op | INSN_RD(ret) | INSN_RS1(addr) |
>                   INSN_IMM13(offset));
> -    else {
> +    } else {
>         tcg_out_movi(s, TCG_TYPE_PTR, TCG_REG_I5, offset);
> -        tcg_out32(s, op | INSN_RD(ret) | INSN_RS1(TCG_REG_I5) |
> -                  INSN_RS2(addr));
> +        tcg_out_ldst_rr(s, ret, addr, TCG_REG_I5, op);
>     }
>  }
>
> -static inline void tcg_out_ldst_asi(TCGContext *s, int ret, int addr,
> -                                    int offset, int op, int asi)
> -{
> -    tcg_out_movi(s, TCG_TYPE_PTR, TCG_REG_I5, offset);
> -    tcg_out32(s, op | INSN_RD(ret) | INSN_RS1(TCG_REG_I5) |
> -              INSN_ASI(asi) | INSN_RS2(addr));
> -}
> -
>  static inline void tcg_out_ld(TCGContext *s, TCGType type, TCGReg ret,
>                               TCGReg arg1, tcg_target_long arg2)
>  {
> -    if (type == TCG_TYPE_I32)
> -        tcg_out_ldst(s, ret, arg1, arg2, LDUW);
> -    else
> -        tcg_out_ldst(s, ret, arg1, arg2, LDX);
> +    tcg_out_ldst(s, ret, arg1, arg2, (type == TCG_TYPE_I32 ? LDUW : LDX));
>  }
>
>  static inline void tcg_out_st(TCGContext *s, TCGType type, TCGReg arg,
>                               TCGReg arg1, tcg_target_long arg2)
>  {
> -    if (type == TCG_TYPE_I32)
> -        tcg_out_ldst(s, arg, arg1, arg2, STW);
> -    else
> -        tcg_out_ldst(s, arg, arg1, arg2, STX);
> +    tcg_out_ldst(s, arg, arg1, arg2, (type == TCG_TYPE_I32 ? STW : STX));
> +}
> +
> +static inline void tcg_out_ld_ptr(TCGContext *s, int ret,
> +                                  tcg_target_long arg)
> +{
> +    if (!check_fit_tl(arg, 10)) {
> +        tcg_out_movi(s, TCG_TYPE_PTR, ret, arg & ~0x3ff);
> +    }
> +    tcg_out_ld(s, TCG_TYPE_PTR, ret, ret, arg & 0x3ff);
>  }
>
> +
>  static inline void tcg_out_sety(TCGContext *s, int rs)
>  {
>     tcg_out32(s, WRY | INSN_RS1(TCG_REG_G0) | INSN_RS2(rs));
> @@ -757,22 +747,16 @@ static const void * const qemu_st_helpers[4] = {
>    WHICH is the offset into the CPUTLBEntry structure of the slot to read.
>    This should be offsetof addr_read or addr_write.
>
> -   Outputs:
> -   LABEL_PTRS is filled with the position of the forward jumps to the
> -   TLB miss case.  This will always be a ,PN insn, so a 19-bit offset.
> -
> -   Returns a register loaded with the low part of the address, adjusted
> -   as indicated by the TLB 

Re: [Qemu-devel] [PATCH 03/14] tcg-sparc: Assume v9 cpu always, i.e. force v8plus in 32-bit mode.

[Richard Henderson]

On 03/29/2012 02:45 PM, Blue Swirl wrote:
>> >  #elif defined(TARGET_SPARC)
>> > print_insn = print_insn_sparc;
>> > -#ifdef TARGET_SPARC64
>> > disasm_info.mach = bfd_mach_sparc_v9b;
>> > -#endif
> This is not OK, it would change ASI printout for V8 guest code.
> 

Ah, right.  Ok, will fix for the next revision.


r~

Re: [Qemu-devel] [PATCH 12/13] qapi: support for keyworded variable-length argument list

[Anthony Liguori]

On 03/29/2012 01:42 PM, Luiz Capitulino wrote:

On Thu, 29 Mar 2012 13:26:34 -0500
Anthony Liguori  wrote:


On 03/29/2012 12:26 PM, Luiz Capitulino wrote:

This allows for QAPI functions to receive a variable-length argument
list. This is going to be used by device_add and netdev_add commands.

In the schema, the argument list is represented by type name '**',
like this example:

  { 'command': 'foo', 'data': { 'arg-list': '**' } }

Each argument is represented by the KeyValues type and the C
implementation should expect a KeyValuesList, like:

  void qmp_foo(KeyValuesList *values_list, Error **errp);

XXX: This implementation is simple but very hacky. We just iterate
   through all arguments and build the KeyValuesList list to be
   passed to the QAPI function.

   Maybe we could have a kwargs type, that does exactly this but
   through a visitor instead?

Signed-off-by: Luiz Capitulino


What about just treating '**' as "marshal remaining arguments to a string" and
then pass that string to device_add?  qmp_device_add can then parse that string
with QemuOpts.


If this turns out to be simple enough, I'm fine with it.


I don't love doing this sort of double conversion but it's really the only 
practical way to do it I think.  device_add has a weird semantic where 
printing/parsing is implied so I think it's unavoidable.



It's a bit ugly, but that's how things worked.  When we introduce qom_add, this
problem goes away because you would make multiple calls to qom_set to set all of
the properties.


Just out of curiosity, is qom_add going to supersede device_add?


Eventually...

I'd like to see qom_add as the low level interface but then I'd like to see nice 
high level interfaces like 'block_add' which took a parameter of virtio-blk and 
did all of the magic to create a block device in such a way that's compliant to 
the current machine type.


Regards,

Anthony Liguori

Re: [Qemu-devel] [PATCH 12/14] tcg-sparc: Use defines for temporaries.

[Blue Swirl]

On Wed, Mar 28, 2012 at 00:32, Richard Henderson  wrote:
> And change from %i4/%i5 to %g1/%o7 to remove a v8plus fixme.
>
> Signed-off-by: Richard Henderson 
> ---
>  tcg/sparc/tcg-target.c |  114 ---
>  1 files changed, 58 insertions(+), 56 deletions(-)
>
> diff --git a/tcg/sparc/tcg-target.c b/tcg/sparc/tcg-target.c
> index 5b3cde4..9e822f3 100644
> --- a/tcg/sparc/tcg-target.c
> +++ b/tcg/sparc/tcg-target.c
> @@ -59,8 +59,12 @@ static const char * const 
> tcg_target_reg_names[TCG_TARGET_NB_REGS] = {
>  };
>  #endif
>
> +/* Define some temporary registers.  T2 is used for constant generation.  */
> +#define TCG_REG_T1  TCG_REG_G1
> +#define TCG_REG_T2  TCG_REG_O7
> +
>  #ifdef CONFIG_USE_GUEST_BASE
> -# define TCG_GUEST_BASE_REG TCG_REG_I3
> +# define TCG_GUEST_BASE_REG TCG_REG_I5
>  #else
>  # define TCG_GUEST_BASE_REG TCG_REG_G0
>  #endif
> @@ -85,6 +89,7 @@ static const int tcg_target_reg_alloc_order[] = {
>     TCG_REG_I2,
>     TCG_REG_I3,
>     TCG_REG_I4,
> +    TCG_REG_I5,
>  };
>
>  static const int tcg_target_call_iarg_regs[6] = {
> @@ -372,10 +377,10 @@ static inline void tcg_out_movi(TCGContext *s, TCGType 
> type,
>         tcg_out_sethi(s, ret, ~arg);
>         tcg_out_arithi(s, ret, ret, (arg & 0x3ff) | -0x400, ARITH_XOR);
>     } else {
> -        tcg_out_movi_imm32(s, TCG_REG_I4, arg >> (TCG_TARGET_REG_BITS / 2));
> -        tcg_out_arithi(s, TCG_REG_I4, TCG_REG_I4, 32, SHIFT_SLLX);
> -        tcg_out_movi_imm32(s, ret, arg);
> -        tcg_out_arith(s, ret, ret, TCG_REG_I4, ARITH_OR);
> +        tcg_out_movi_imm32(s, ret, arg >> (TCG_TARGET_REG_BITS / 2));
> +        tcg_out_arithi(s, ret, ret, 32, SHIFT_SLLX);
> +        tcg_out_movi_imm32(s, TCG_REG_T2, arg);
> +        tcg_out_arith(s, ret, ret, TCG_REG_T2, ARITH_OR);
>     }
>  }
>
> @@ -392,8 +397,8 @@ static inline void tcg_out_ldst(TCGContext *s, int ret, 
> int addr,
>         tcg_out32(s, op | INSN_RD(ret) | INSN_RS1(addr) |
>                   INSN_IMM13(offset));
>     } else {
> -        tcg_out_movi(s, TCG_TYPE_PTR, TCG_REG_I5, offset);
> -        tcg_out_ldst_rr(s, ret, addr, TCG_REG_I5, op);
> +        tcg_out_movi(s, TCG_TYPE_PTR, TCG_REG_T1, offset);
> +        tcg_out_ldst_rr(s, ret, addr, TCG_REG_T1, op);
>     }
>  }
>
> @@ -435,8 +440,8 @@ static inline void tcg_out_addi(TCGContext *s, int reg, 
> tcg_target_long val)
>         if (check_fit_tl(val, 13))
>             tcg_out_arithi(s, reg, reg, val, ARITH_ADD);
>         else {
> -            tcg_out_movi(s, TCG_TYPE_PTR, TCG_REG_I5, val);
> -            tcg_out_arith(s, reg, reg, TCG_REG_I5, ARITH_ADD);
> +            tcg_out_movi(s, TCG_TYPE_PTR, TCG_REG_T1, val);
> +            tcg_out_arith(s, reg, reg, TCG_REG_T1, ARITH_ADD);
>         }
>     }
>  }
> @@ -448,8 +453,8 @@ static inline void tcg_out_andi(TCGContext *s, int rd, 
> int rs,
>         if (check_fit_tl(val, 13))
>             tcg_out_arithi(s, rd, rs, val, ARITH_AND);
>         else {
> -            tcg_out_movi(s, TCG_TYPE_I32, TCG_REG_I5, val);
> -            tcg_out_arith(s, rd, rs, TCG_REG_I5, ARITH_AND);
> +            tcg_out_movi(s, TCG_TYPE_I32, TCG_REG_T1, val);
> +            tcg_out_arith(s, rd, rs, TCG_REG_T1, ARITH_AND);
>         }
>     }
>  }
> @@ -461,8 +466,8 @@ static void tcg_out_div32(TCGContext *s, int rd, int rs1,
>     if (uns) {
>         tcg_out_sety(s, TCG_REG_G0);
>     } else {
> -        tcg_out_arithi(s, TCG_REG_I5, rs1, 31, SHIFT_SRA);
> -        tcg_out_sety(s, TCG_REG_I5);
> +        tcg_out_arithi(s, TCG_REG_T1, rs1, 31, SHIFT_SRA);
> +        tcg_out_sety(s, TCG_REG_T1);

By the way, since we assume V9+, this 32 bit division which uses the
register y could be changed (in some later patch) to use nicer 64 bit
division.

>     }
>
>     tcg_out_arithc(s, rd, rs1, val2, val2const,
> @@ -608,8 +613,8 @@ static void tcg_out_setcond_i32(TCGContext *s, TCGCond 
> cond, TCGArg ret,
>     case TCG_COND_GTU:
>     case TCG_COND_GEU:
>         if (c2const && c2 != 0) {
> -            tcg_out_movi_imm13(s, TCG_REG_I5, c2);
> -            c2 = TCG_REG_I5;
> +            tcg_out_movi_imm13(s, TCG_REG_T1, c2);
> +            c2 = TCG_REG_T1;
>         }
>         t = c1, c1 = c2, c2 = t, c2const = 0;
>         cond = tcg_swap_cond(cond);
> @@ -656,15 +661,15 @@ static void tcg_out_setcond2_i32(TCGContext *s, TCGCond 
> cond, TCGArg ret,
>
>     switch (cond) {
>     case TCG_COND_EQ:
> -        tcg_out_setcond_i32(s, TCG_COND_EQ, TCG_REG_I5, al, bl, blconst);
> +        tcg_out_setcond_i32(s, TCG_COND_EQ, TCG_REG_T1, al, bl, blconst);
>         tcg_out_setcond_i32(s, TCG_COND_EQ, ret, ah, bh, bhconst);
> -        tcg_out_arith(s, ret, ret, TCG_REG_I5, ARITH_AND);
> +        tcg_out_arith(s, ret, ret, TCG_REG_T1, ARITH_AND);
>         break;
>
>     case TCG_COND_NE:
> -        tcg_out_setcond_i32(s, TCG_COND_NE, TCG_REG_I5, al, al, blconst);
> +        tcg_out_setcond_i32(s, TCG_COND_NE, TCG_REG_T1, al, al, blconst);
>         tcg_out

Re: [Qemu-devel] [PATCH 07/14] Avoid declaring the env variable at all if CONFIG_TCG_PASS_AREG0.

[Blue Swirl]

On Wed, Mar 28, 2012 at 00:32, Richard Henderson  wrote:
> At the same time, remove use of the global ENV from user-exec.c.
>
> Signed-off-by: Richard Henderson 
> ---
>  Makefile.target |    5 -
>  dyngen-exec.h   |    5 +
>  user-exec.c     |   17 ++---
>  3 files changed, 11 insertions(+), 16 deletions(-)
>
> diff --git a/Makefile.target b/Makefile.target
> index aa53e28..81fdf9e 100644
> --- a/Makefile.target
> +++ b/Makefile.target
> @@ -110,11 +110,6 @@ $(libobj-y): $(GENERATED_HEADERS)
>  ifndef CONFIG_TCG_PASS_AREG0
>  op_helper.o: QEMU_CFLAGS += $(HELPER_CFLAGS)
>  endif
> -user-exec.o: QEMU_CFLAGS += $(HELPER_CFLAGS)
> -
> -# Note: this is a workaround. The real fix is to avoid compiling
> -# cpu_signal_handler() in user-exec.c.
> -signal.o: QEMU_CFLAGS += $(HELPER_CFLAGS)

The patch does not apply, please rebase.
Applying: Avoid declaring the env variable at all if CONFIG_TCG_PASS_AREG0.
error: patch failed: Makefile.target:110
error: Makefile.target: patch does not apply

>
>  #
>  # Linux user emulator target
> diff --git a/dyngen-exec.h b/dyngen-exec.h
> index cfeef99..65fcb43 100644
> --- a/dyngen-exec.h
> +++ b/dyngen-exec.h
> @@ -19,6 +19,10 @@
>  #if !defined(__DYNGEN_EXEC_H__)
>  #define __DYNGEN_EXEC_H__
>
> +/* If the target has indicated that it does not need an AREG0,
> +   don't declare the env variable at all, much less as a register.  */
> +#if !defined(CONFIG_TCG_PASS_AREG0)
> +
>  #if defined(CONFIG_TCG_INTERPRETER)
>  /* The TCG interpreter does not need a special register AREG0,
>  * but it is possible to use one by defining AREG0.
> @@ -65,4 +69,5 @@ register CPUArchState *env asm(AREG0);
>  extern CPUArchState *env;
>  #endif
>
> +#endif /* !CONFIG_TCG_PASS_AREG0 */
>  #endif /* !defined(__DYNGEN_EXEC_H__) */
> diff --git a/user-exec.c b/user-exec.c
> index cd905ff..9691f09 100644
> --- a/user-exec.c
> +++ b/user-exec.c
> @@ -18,7 +18,6 @@
>  */
>  #include "config.h"
>  #include "cpu.h"
> -#include "dyngen-exec.h"
>  #include "disas.h"
>  #include "tcg.h"
>
> @@ -58,8 +57,6 @@ void cpu_resume_from_signal(CPUArchState *env1, void *puc)
>     struct sigcontext *uc = puc;
>  #endif
>
> -    env = env1;
> -
>     /* XXX: restore cpu registers saved in host registers */
>
>     if (puc) {
> @@ -74,8 +71,8 @@ void cpu_resume_from_signal(CPUArchState *env1, void *puc)
>         sigprocmask(SIG_SETMASK, &uc->sc_mask, NULL);
>  #endif
>     }
> -    env->exception_index = -1;
> -    longjmp(env->jmp_env, 1);
> +    env1->exception_index = -1;
> +    longjmp(env1->jmp_env, 1);
>  }
>
>  /* 'pc' is the host PC at which the exception was raised. 'address' is
> @@ -86,12 +83,10 @@ static inline int handle_cpu_signal(unsigned long pc, 
> unsigned long address,
>                                     int is_write, sigset_t *old_set,
>                                     void *puc)
>  {
> +    CPUArchState *env1 = cpu_single_env;
>     TranslationBlock *tb;
>     int ret;
>
> -    if (cpu_single_env) {
> -        env = cpu_single_env; /* XXX: find a correct solution for 
> multithread */
> -    }
>  #if defined(DEBUG_SIGNAL)
>     qemu_printf("qemu: SIGSEGV pc=0x%08lx address=%08lx w=%d 
> oldset=0x%08lx\n",
>                 pc, address, is_write, *(unsigned long *)old_set);
> @@ -102,7 +97,7 @@ static inline int handle_cpu_signal(unsigned long pc, 
> unsigned long address,
>     }
>
>     /* see if it is an MMU fault */
> -    ret = cpu_handle_mmu_fault(env, address, is_write, MMU_USER_IDX);
> +    ret = cpu_handle_mmu_fault(env1, address, is_write, MMU_USER_IDX);
>     if (ret < 0) {
>         return 0; /* not an MMU fault */
>     }
> @@ -114,13 +109,13 @@ static inline int handle_cpu_signal(unsigned long pc, 
> unsigned long address,
>     if (tb) {
>         /* the PC is inside the translated code. It means that we have
>            a virtual CPU fault */
> -        cpu_restore_state(tb, env, pc);
> +        cpu_restore_state(tb, env1, pc);
>     }
>
>     /* we restore the process signal mask as the sigreturn should
>        do it (XXX: use sigsetjmp) */
>     sigprocmask(SIG_SETMASK, old_set, NULL);
> -    exception_action(env);
> +    exception_action(env1);
>
>     /* never comes here */
>     return 1;
> --
> 1.7.7.6
>

Re: [Qemu-devel] [Bug 965133] [NEW] Sparc64 crash on start

[Blue Swirl]

On Mon, Mar 26, 2012 at 10:27, Tiziano Vecchi  wrote:
> Public bug reported:
>
> qemu version 1.0.1 compiled on a Ubuntu live on a HP laptop win a x64
> architecture.
>
> With more than 4G of memory sparc64 machine crash on start.
>
> command line: qemu-system-sparc64 -m 4G
>
> output:
> VNC server running on `127.0.0.1:5900'
> qemu: fatal: Trap 0x0064 while trap level (5) >= MAXTL (5), Error state
> pc: ffd04c80  npc: ffd04c84
> General Registers:
> %g0-3:    
> %g4-7:    
>
> Current Register Window:
> %o0-3: ffd0 0008 0008 
> %o4-7:   fff754e1 ffd144d4
> %l0-3: 0001 fff75c4d  
> %l4-7:    
> %i0-3:   0001 0036
> %i4-7: ffe87418 ffe87648 fff75591 ffd0bf54
>
> Floating Point Registers:
> %f00:    
> %f08:    
> %f16:    
> %f24:    
> %f32:    
> %f40:    
> %f48:    
> %f56:    
> pstate: 0414 ccr: 99 (icc: N--C xcc: N--C) asi: 00 tl: 5 pil: 0
> cansave: 5 canrestore: 1 otherwin: 0 wstate: 0 cleanwin: 6 cwp: 3
> fsr:  y:  fprs: 
> Aborted (core dumped)

This is actually a bug in OpenBIOS. It is mapped at 0xffd0 (below
4G) for compatibility with 32 bit code, but the code does not handle
this case where top of RAM overlaps OpenBIOS.

>
> ** Affects: qemu
>     Importance: Undecided
>         Status: New
>
> --
> You received this bug notification because you are a member of qemu-
> devel-ml, which is subscribed to QEMU.
> https://bugs.launchpad.net/bugs/965133
>
> Title:
>  Sparc64 crash on start
>
> Status in QEMU:
>  New
>
> Bug description:
>  qemu version 1.0.1 compiled on a Ubuntu live on a HP laptop win a x64
>  architecture.
>
>  With more than 4G of memory sparc64 machine crash on start.
>
>  command line: qemu-system-sparc64 -m 4G
>
>  output:
>  VNC server running on `127.0.0.1:5900'
>  qemu: fatal: Trap 0x0064 while trap level (5) >= MAXTL (5), Error state
>  pc: ffd04c80  npc: ffd04c84
>  General Registers:
>  %g0-3:    
>  %g4-7:    
>
>  Current Register Window:
>  %o0-3: ffd0 0008 0008 
>  %o4-7:   fff754e1 ffd144d4
>  %l0-3: 0001 fff75c4d  
>  %l4-7:    
>  %i0-3:   0001 0036
>  %i4-7: ffe87418 ffe87648 fff75591 ffd0bf54
>
>  Floating Point Registers:
>  %f00:    
>  %f08:    
>  %f16:    
>  %f24:    
>  %f32:    
>  %f40:    
>  %f48:    
>  %f56:    
>  pstate: 0414 ccr: 99 (icc: N--C xcc: N--C) asi: 00 tl: 5 pil: 0
>  cansave: 5 canrestore: 1 otherwin: 0 wstate: 0 cleanwin: 6 cwp: 3
>  fsr:  y:  fprs: 
>  Aborted (core dumped)
>
> To manage notifications about this bug go to:
> https://bugs.launchpad.net/qemu/+bug/965133/+subscriptions
>