Re: [Qemu-devel] [PULL 00/96] pci, pc, virtio fixes and cleanups

[Michael S. Tsirkin]

On Wed, Feb 18, 2015 at 11:03:53PM +0100, Michael S. Tsirkin wrote:
> On Wed, Feb 18, 2015 at 10:48:22PM +0100, Michael S. Tsirkin wrote:
> > On Wed, Feb 18, 2015 at 10:43:58PM +0100, Michael S. Tsirkin wrote:
> > > A huge patchset, but the scariest part is Igor's patches,
> > > and these have been used by multiple people by now.
> > > virtio header change is a bit rushed, but getting them upstream
> > > seems like the best way to give them the cross-platform
> > > testing that they need, and this takes us a step closer to
> > > virtio-1.0 support.
> > 
> > Ugh.
> > Missed aarch64 failures this triggers :(
> > Pls ignore for now, sorry about the noise.
> 
> OK, I think it's a false alarm - passes fine on all other machines,
> and on this one, current master gives the same error:
> 
> GTESTER check-qtest-aarch64
> qemu-system-aarch64: Unknown device 'gpex-pcihost' for default sysbus
> Broken pipe
> GTester: last random seed: R02Sf2b7ee633e245f2aa40572f893be1567
> 
> Peter, you'll run make check anyway - can you pls try merging,
> and running make check - if it passes for you too,
> then I think this pull is fine, and I'll figure out
> what's wrong with this box some other day.
> 
> Pls let me know.
> Thanks!

OK, this was because of some stale files apparently.
Regenerating config-devices made it go away.
Pls pull.

> > > The following changes since commit 
> > > cd2d5541271f1934345d8ca42f5fafff1744eee7:
> > > 
> > >   Merge remote-tracking branch 'remotes/rth/tags/pull-tcg-20150212' into 
> > > staging (2015-02-13 11:44:50 +)
> > > 
> > > are available in the git repository at:
> > > 
> > >   git://git.kernel.org/pub/scm/virt/kvm/mst/qemu.git tags/for_upstream
> > > 
> > > for you to fetch changes up to 6b5e5a8361bdff0e75629b1001236d78f27676b6:
> > > 
> > >   acpi-test: update expected files (2015-02-18 22:29:26 +0100)
> > > 
> > > 
> > > pci, pc, virtio fixes and cleanups
> > > 
> > > Last large pull for soft freeze.
> > > 
> > > A bunch of fixes all over the place.
> > > Most of ACPI refactoring has been merged.
> > > 
> > > virtio header cleanup has been merged,
> > > now that windows has been build-tested with it.
> > > 
> > > initial patches from virtio-1.0 branch have been merged.
> > > 
> > > Signed-off-by: Michael S. Tsirkin 
> > > 
> > > 
> > > Cornelia Huck (3):
> > >   virtio: cull virtio_bus_set_vdev_features
> > >   virtio: feature bit manipulation helpers
> > >   virtio: add feature checking helpers
> > > 
> > > Igor Mammedov (43):
> > >   acpi: move generic aml building helpers into dedictated file
> > >   acpi: add build_append_namestring() helper
> > >   acpi: drop min-bytes in build_package()
> > >   pc: acpi-build: update linker on guest access
> > >   pc: acpi-build: migrate RSDP table
> > >   pc: acpi: use local var for accessing ACPI tables blob in 
> > > acpi_build()
> > >   acpi: introduce AML composer aml_append()
> > >   acpi: add aml_scope() term
> > >   pc: acpi-build: use aml_scope() for \_SB scope
> > >   acpi: add aml_device() term
> > >   acpi: add aml_method() term
> > >   acpi: add aml_if() term
> > >   acpi: add aml_name() & aml_name_decl() term
> > >   acpi: add aml_int() term
> > >   acpi: add aml_return() term
> > >   acpi: add aml_arg() term
> > >   acpi: add aml_store() term
> > >   acpi: add aml_and() term
> > >   acpi: add aml_notify() term
> > >   acpi: add aml_call1(), aml_call2(), aml_call3(), aml_call4() helpers
> > >   acpi: add aml_package() term
> > >   pc: acpi-build: generate _S[345] packages dynamically
> > >   acpi: add aml_buffer() term
> > >   acpi: add aml_resource_template() helper
> > >   acpi: add aml_io() helper
> > >   acpi: include PkgLength size only when requested
> > >   acpi: add aml_operation_region() term
> > >   acpi: add aml_field() & aml_named_field() terms
> > >   acpi: add aml_local() term
> > >   acpi: add aml_string() term
> > >   pc: acpi-build: generate pvpanic device description dynamically
> > >   acpi: add aml_varpackage() term
> > >   acpi: add aml_equal() term
> > >   acpi: add aml_processor() term
> > >   acpi: add aml_eisaid() term
> > >   pc: acpi-build: drop template patching and CPU hotplug objects 
> > > dynamically
> > >   pc: acpi-build: create CPU hotplug IO region dynamically
> > >   acpi: add aml_reserved_field() term
> > >   pc: acpi-build: drop template patching and memory hotplug objects 
> > > dynamically
> > >   pc: acpi-build: create memory hotplug IO region dynamically
> > >   acpi: add aml_word_bus_number(), aml_word_io(), aml_dword_memory(), 
> > > aml_qword_memory() terms
> > >   pc: pcihp: expose MMIO base and len as properties
> > >   pc: acpi-build: reserve PCIHP MMIO resources
> > > 
> > > M

Re: [Qemu-devel] [RFC PATCH 5/7] target-arm/cpu: Add apic_id property for ARMCPU

[Hanjun Guo]

On 2015年02月19日 03:51, Igor Mammedov wrote:

On Wed, 18 Feb 2015 18:45:51 +0100
Andreas Färber  wrote:


Hi,

Am 17.02.2015 um 11:10 schrieb Shannon Zhao:

Add apic_id property for ARMCPU. It can be used for cpu hotplug.

Signed-off-by: Shannon Zhao 
---
  target-arm/cpu-qom.h |1 +
  target-arm/cpu.c |   77
++
target-arm/cpu.h |2 + 3 files changed, 80 insertions(+), 0
deletions(-)

diff --git a/target-arm/cpu-qom.h b/target-arm/cpu-qom.h
index ed5a644..d4560e2 100644
--- a/target-arm/cpu-qom.h
+++ b/target-arm/cpu-qom.h
@@ -59,6 +59,7 @@ typedef struct ARMCPU {
  /*< public >*/

  CPUARMState env;
+uint32_t apic_id;


Can you add a matching @apic_id: documentation entry above the struct?



  /* Coprocessor information */
  GHashTable *cp_regs;
diff --git a/target-arm/cpu.c b/target-arm/cpu.c
index 285947f..9202b07 100644
--- a/target-arm/cpu.c
+++ b/target-arm/cpu.c

[...]

@@ -343,6 +407,11 @@ static void arm_cpu_initfn(Object *obj)
  cpu->cp_regs = g_hash_table_new_full(g_int_hash, g_int_equal,
   g_free, g_free);

+object_property_add(obj, "apic-id", "int",
+arm_cpuid_get_apic_id,
+arm_cpuid_set_apic_id, NULL, NULL, NULL);


The property is correctly called apic-id. Please update the commit
message, which has it as apic_id (2x).

Is there such thing as apic-id on ARM?


Not apic-id, apic-id is x86 and ia64 dependent. On ARM, mpidr is
similar as apic-id to represent the CPU hardware ID, so I think
mpidr will be the one used in this patch, and another thing need
to consider that mpidr on ARM64 is 64 bits, not uint32_t.
Thanks
Hanjun

Re: [Qemu-devel] [PULL 00/96] pci, pc, virtio fixes and cleanups

[Peter Maydell]

On 19 February 2015 at 07:03, Michael S. Tsirkin  wrote:
> OK, I think it's a false alarm - passes fine on all other machines,
> and on this one, current master gives the same error:
>
> GTESTER check-qtest-aarch64
> qemu-system-aarch64: Unknown device 'gpex-pcihost' for default sysbus
> Broken pipe
> GTester: last random seed: R02Sf2b7ee633e245f2aa40572f893be1567

As you seem to have discovered, this is just because your
build tree has stale files in it:
http://lists.gnu.org/archive/html/qemu-devel/2015-02/msg02607.html
It's one of those things where we made a change to our makefiles
(in fact a change from over a year ago, I think), and the new
dependency rules are self-consistent but don't provide an
"upgrade" path for ensuring correct files are generated
starting from a pre-change build tree state.

As far as I can tell, once you've caused the make_device_config.sh
script to get run once, the .d files will all be present so that
future changes like this aarch64 one to pci.mak or other included
files will correctly trigger rebuilds.

thanks
-- PMM

[Qemu-devel] [PULL 0/1] seabios: update to 1.8.0 release

[Gerd Hoffmann]

  Hi,

seabios 1.8.0 was just released, and here comes the pull
request for the bios (and vgabios) update.

cheers,
  Gerd

The following changes since commit cd2d5541271f1934345d8ca42f5fafff1744eee7:

  Merge remote-tracking branch 'remotes/rth/tags/pull-tcg-20150212' into 
staging (2015-02-13 11:44:50 +)

are available in the git repository at:


  git://git.kraxel.org/qemu tags/pull-roms-20150219-1

for you to fetch changes up to 21f5826a04d38e19488f917e1eef22751490c769:

  seabios: update to 1.8.0 release (2015-02-19 09:33:03 +0100)


seabios: update to 1.8.0 release


Gerd Hoffmann (1):
  seabios: update to 1.8.0 release

 pc-bios/bios-256k.bin  | Bin 262144 -> 262144 bytes
 pc-bios/bios.bin   | Bin 131072 -> 131072 bytes
 pc-bios/vgabios-cirrus.bin | Bin 37376 -> 37888 bytes
 pc-bios/vgabios-qxl.bin| Bin 37376 -> 38400 bytes
 pc-bios/vgabios-stdvga.bin | Bin 37376 -> 38400 bytes
 pc-bios/vgabios-vmware.bin | Bin 37376 -> 38400 bytes
 pc-bios/vgabios.bin| Bin 37376 -> 38400 bytes
 roms/config.seabios-128k   |   1 +
 roms/seabios   |   2 +-
 9 files changed, 2 insertions(+), 1 deletion(-)

[Qemu-devel] [PULL 1/1] seabios: update to 1.8.0 release

[Gerd Hoffmann]

'git shortlog 8936dbb2..4c59f5d8' for seabios repo:

David Woodhouse (4):
  Update EFI_COMPATIBILITY16_TABLE to match 0.98 spec update
  build: use -m16 where available instead of asm(".code16gcc")
  romlayout: Use .code16 not .code16gcc
  vgabios: Use .code16 not .code16gcc

Gerd Hoffmann (2):
  add scripts/tarball.sh
  build: set LC_ALL=C

Hannes Reinecke (1):
  megasas: read addional PCI I/O bar

Ian Campbell (1):
  romlayout: Use "rep ; nop" not "rep nop".

Kevin O'Connor (139):
  vgabios: Return from handle_1011() if handler found.
  edd: Move EDD get drive parameters (int 1348) logic from disk.c to 
block.c.
  edd: Use sectors==-1 to detect removable media.
  edd: Separate out ATA and virtio specific parts of fill_edd().
  cdemu: store internal cdemu fields in standard "el-torito" spec format.
  Move cdemu call interface and disk_ret helper code to disk.c.
  smm: Replace SMI assembler code with C code.
  smm: Use a C struct to define the layout of the SMM area.
  smp: Replace QEMU SMP init assembler code with C; run only in 32bit mode.
  Don't enable thread preemption during S3 resume vga option rom execution.
  Remove old Bochs bios fixed address string at 0xfff00.
  Move most of the VAR16FIXED() defs to misc.c.
  build: Avoid absolute paths during "whole-program" compiling.
  Make sure handle_smi() and handle_smp() are compiled out if not enabled.
  Remove the TODO file.
  Abstract reset call (and possible 16bit mode switch) into reset() 
function.
  build: Remove unused function getSectionsStart() from layoutrom.py.
  build: Extract section visiting logic in layoutrom.py.
  build: Refactor layoutrom.py gc() function.
  build: Use customized entry point for each type of build.
  build: Refactor findInit() function.
  build: Rework getRelocs() to use a hash instead of categories in 
layoutrom.py
  build: Keep segmented sections separate until final link step.
  build: Use fileid instead of category to write sections in layoutrom.py.
  build: Only export needed fields in LayoutInfo in layoutrom.py.
  build: Get fixed address variables from 32bit compile pass (not 16bit)
  build: Minor - fix comments referring to old tools/ directory.
  xhci: Update the times for usb command timeouts.
  ehci: Update usb command timeouts to use usb_xfer_time()
  uhci: Update usb command timeouts to use usb_xfer_time()
  ohci: Update usb command timeouts to use usb_xfer_time()
  vgabios: Fix broken build resulting from e5749978.
  boot: Change ":rom%d" boot order rom instance to ":rom%x"
  Minor - remove stray tab from src/fw/smm.c.
  build: Update kconfig to version in Linux 3.16.
  usb: Fix usb_xfer_time() to work when called in 16bit mode.
  xhci: Call usb_desc2pipe() on xhci_update_pipe().
  xhci: Remove 16bit code wrappers.
  xhci: Use high memory instead of low memory for internal storage.
  xhci: Move root hub and setup code to top of file.
  xhci: Add xhci_check_ports() and xhci_free_pipes() functions.
  ehci: Move port power up from ehci_hub_detect() to check_ehci_ports().
  usb-hub: Enable power to all ports prior to calling usb_enumerate().
  xhci: Change xhci_hub_detect() to use connect status instead of link 
state.
  uhci: Repeatedly poll for device detect for 100ms.
  ohci: Repeatedly poll for device detect for 100ms.
  ehci: Stall uhci/ohci init only until default port routing is done.
  usb: Perform device detect polling on all usb controllers.
  ehci: Fix bug in hub port assignment
  Revert "Use the extra stack for 16bit USB and PS2 keyboard/mouse 
commands."
  pmm: Fix entry point to support non-zero %ss
  Move stack hop code below call32/call16 code in stacks.c
  Add need_hop_back() call that determines if stack_hop_back is needed
  Update invoke_mouse_handler() to use need_hop_back()
  Update stack_hop_back() to jump to 16bit mode if called in 32bit mode.
  Track when entering via call32() and use the same mode for 
stack_hop_back()
  Simplify farcall16 code
  Update reset() to use call16_back()
  build: Support declaring 32bit C functions that must reside in the 
f-segment
  Move call16() functions from romlayout.S to inline assembler in stacks.c
  Break up call32() into call32() and call32_sloppy()
  Fully restore 16bit state during call16_sloppy()
  Implement call32 mechanism using SMIs.
  Move a20 code from system.c and ps2port.h to x86.h
  Backup and restore a20 on call32_sloppy()
  usb: Rename ?hci_control() to ?hci_send_control()
  usb: Rename usb_getFrameExp() to usb_get_period()
  usb: Rename findEndPointDesc() to usb_find_desc()
  usb: Rename send_default_control() to usb_send_default_control()
  usb: Rename free_pipe() to usb_free_pipe()
  usb: Clarify usb freelist manipulations

[Qemu-devel] [PATCH] qmp: Provide qmp_query_vnc_servers stub for VNC-free build

[Jan Kiszka]

Signed-off-by: Jan Kiszka 
---
 qmp.c | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/qmp.c b/qmp.c
index 7f2d25a..ec070da 100644
--- a/qmp.c
+++ b/qmp.c
@@ -134,6 +134,12 @@ VncInfo *qmp_query_vnc(Error **errp)
 error_set(errp, QERR_FEATURE_DISABLED, "vnc");
 return NULL;
 };
+
+VncInfo2List *qmp_query_vnc_servers(Error **errp)
+{
+error_set(errp, QERR_FEATURE_DISABLED, "vnc");
+return NULL;
+};
 #endif
 
 #ifndef CONFIG_SPICE
-- 
2.1.4

Re: [Qemu-devel] [RESEND PATCH 1/2] balloon: call qdev_alias_all_properties for proxy dev in balloon class init

[Michael S. Tsirkin]

On Thu, Jan 29, 2015 at 05:24:41PM +0300, Denis V. Lunev wrote:
> The idea is that all other virtio devices are calling this helper
> to merge properties of the proxy device. This is the only difference
> in between this helper and code in inside virtio_instance_init_common.
> The patch should not cause any harm as property list in generic balloon
> code is empty.
> 
> This also allows to avoid some dummy errors like fixed by this
> commit 91ba21208839643603e7f7fa5864723c3f371ebe
> Author: Gonglei 
> Date:   Tue Sep 30 14:10:35 2014 +0800
> virtio-balloon: fix virtio-balloon child refcount in transports
> 
> Signed-off-by: Denis V. Lunev 
> Signed-off-by: Raushaniya Maksudova 
> Revieved-by: Cornelia Huck 
> CC: Christian Borntraeger 
> CC: Anthony Liguori 
> CC: Michael S. Tsirkin 
> ---
>  hw/s390x/virtio-ccw.c  | 5 ++---
>  hw/virtio/virtio-pci.c | 5 ++---
>  2 files changed, 4 insertions(+), 6 deletions(-)
> 
> diff --git a/hw/s390x/virtio-ccw.c b/hw/s390x/virtio-ccw.c
> index ea236c9..82da894 100644
> --- a/hw/s390x/virtio-ccw.c
> +++ b/hw/s390x/virtio-ccw.c
> @@ -899,9 +899,8 @@ static void balloon_ccw_stats_set_poll_interval(Object 
> *obj, struct Visitor *v,
>  static void virtio_ccw_balloon_instance_init(Object *obj)
>  {
>  VirtIOBalloonCcw *dev = VIRTIO_BALLOON_CCW(obj);
> -object_initialize(&dev->vdev, sizeof(dev->vdev), TYPE_VIRTIO_BALLOON);
> -object_property_add_child(obj, "virtio-backend", OBJECT(&dev->vdev), 
> NULL);
> -object_unref(OBJECT(&dev->vdev));
> +virtio_instance_init_common(obj, &dev->vdev, sizeof(dev->vdev),
> +TYPE_VIRTIO_BALLOON);
>  object_property_add(obj, "guest-stats", "guest statistics",
>  balloon_ccw_stats_get_all, NULL, NULL, dev, NULL);
>  
> diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c
> index dde1d73..745324b 100644
> --- a/hw/virtio/virtio-pci.c
> +++ b/hw/virtio/virtio-pci.c
> @@ -1316,9 +1316,8 @@ static void virtio_balloon_pci_class_init(ObjectClass 
> *klass, void *data)
>  static void virtio_balloon_pci_instance_init(Object *obj)
>  {
>  VirtIOBalloonPCI *dev = VIRTIO_BALLOON_PCI(obj);
> -object_initialize(&dev->vdev, sizeof(dev->vdev), TYPE_VIRTIO_BALLOON);
> -object_property_add_child(obj, "virtio-backend", OBJECT(&dev->vdev), 
> NULL);
> -object_unref(OBJECT(&dev->vdev));
> +virtio_instance_init_common(obj, &dev->vdev, sizeof(dev->vdev),
> +TYPE_VIRTIO_BALLOON);
>  object_property_add(obj, "guest-stats", "guest statistics",
>  balloon_pci_stats_get_all, NULL, NULL, dev,
>  NULL);

OK, but what about this guest-stats property?
Should it get the same treatment?

> -- 
> 1.9.1

Re: [Qemu-devel] [RESEND PATCH 1/2] balloon: call qdev_alias_all_properties for proxy dev in balloon class init

[Denis V. Lunev]

On 19/02/15 12:25, Michael S. Tsirkin wrote:

On Thu, Jan 29, 2015 at 05:24:41PM +0300, Denis V. Lunev wrote:

The idea is that all other virtio devices are calling this helper
to merge properties of the proxy device. This is the only difference
in between this helper and code in inside virtio_instance_init_common.
The patch should not cause any harm as property list in generic balloon
code is empty.

This also allows to avoid some dummy errors like fixed by this
 commit 91ba21208839643603e7f7fa5864723c3f371ebe
 Author: Gonglei 
 Date:   Tue Sep 30 14:10:35 2014 +0800
 virtio-balloon: fix virtio-balloon child refcount in transports

Signed-off-by: Denis V. Lunev 
Signed-off-by: Raushaniya Maksudova 
Revieved-by: Cornelia Huck 
CC: Christian Borntraeger 
CC: Anthony Liguori 
CC: Michael S. Tsirkin 
---
  hw/s390x/virtio-ccw.c  | 5 ++---
  hw/virtio/virtio-pci.c | 5 ++---
  2 files changed, 4 insertions(+), 6 deletions(-)

diff --git a/hw/s390x/virtio-ccw.c b/hw/s390x/virtio-ccw.c
index ea236c9..82da894 100644
--- a/hw/s390x/virtio-ccw.c
+++ b/hw/s390x/virtio-ccw.c
@@ -899,9 +899,8 @@ static void balloon_ccw_stats_set_poll_interval(Object 
*obj, struct Visitor *v,
  static void virtio_ccw_balloon_instance_init(Object *obj)
  {
  VirtIOBalloonCcw *dev = VIRTIO_BALLOON_CCW(obj);
-object_initialize(&dev->vdev, sizeof(dev->vdev), TYPE_VIRTIO_BALLOON);
-object_property_add_child(obj, "virtio-backend", OBJECT(&dev->vdev), NULL);
-object_unref(OBJECT(&dev->vdev));
+virtio_instance_init_common(obj, &dev->vdev, sizeof(dev->vdev),
+TYPE_VIRTIO_BALLOON);
  object_property_add(obj, "guest-stats", "guest statistics",
  balloon_ccw_stats_get_all, NULL, NULL, dev, NULL);
  
diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c

index dde1d73..745324b 100644
--- a/hw/virtio/virtio-pci.c
+++ b/hw/virtio/virtio-pci.c
@@ -1316,9 +1316,8 @@ static void virtio_balloon_pci_class_init(ObjectClass 
*klass, void *data)
  static void virtio_balloon_pci_instance_init(Object *obj)
  {
  VirtIOBalloonPCI *dev = VIRTIO_BALLOON_PCI(obj);
-object_initialize(&dev->vdev, sizeof(dev->vdev), TYPE_VIRTIO_BALLOON);
-object_property_add_child(obj, "virtio-backend", OBJECT(&dev->vdev), NULL);
-object_unref(OBJECT(&dev->vdev));
+virtio_instance_init_common(obj, &dev->vdev, sizeof(dev->vdev),
+TYPE_VIRTIO_BALLOON);
  object_property_add(obj, "guest-stats", "guest statistics",
  balloon_pci_stats_get_all, NULL, NULL, dev,
  NULL);

OK, but what about this guest-stats property?
Should it get the same treatment?


--
1.9.1

hmm, IMHO no. init_common is actually do the following

void virtio_instance_init_common(Object *proxy_obj, void *data,
 size_t vdev_size, const char *vdev_name)
{
DeviceState *vdev = data;

object_initialize(vdev, vdev_size, vdev_name);
object_property_add_child(proxy_obj, "virtio-backend", 
OBJECT(vdev), NULL);

object_unref(OBJECT(vdev));
qdev_alias_all_properties(vdev, proxy_obj);
}

on the other hand there is the following code in s390

static void s390_virtio_net_instance_init(Object *obj)
{
VirtIONetS390 *dev = VIRTIO_NET_S390(obj);

virtio_instance_init_common(obj, &dev->vdev, sizeof(dev->vdev),
TYPE_VIRTIO_NET);
object_property_add_alias(obj, "bootindex", OBJECT(&dev->vdev),
  "bootindex", &error_abort);
}

which does not contain guest-stats property.

Re: [Qemu-devel] [RESEND PATCH 1/2] balloon: call qdev_alias_all_properties for proxy dev in balloon class init

[Michael S. Tsirkin]

On Thu, Feb 19, 2015 at 12:36:37PM +0300, Denis V. Lunev wrote:
> On 19/02/15 12:25, Michael S. Tsirkin wrote:
> >On Thu, Jan 29, 2015 at 05:24:41PM +0300, Denis V. Lunev wrote:
> >>The idea is that all other virtio devices are calling this helper
> >>to merge properties of the proxy device. This is the only difference
> >>in between this helper and code in inside virtio_instance_init_common.
> >>The patch should not cause any harm as property list in generic balloon
> >>code is empty.
> >>
> >>This also allows to avoid some dummy errors like fixed by this
> >> commit 91ba21208839643603e7f7fa5864723c3f371ebe
> >> Author: Gonglei 
> >> Date:   Tue Sep 30 14:10:35 2014 +0800
> >> virtio-balloon: fix virtio-balloon child refcount in transports
> >>
> >>Signed-off-by: Denis V. Lunev 
> >>Signed-off-by: Raushaniya Maksudova 
> >>Revieved-by: Cornelia Huck 
> >>CC: Christian Borntraeger 
> >>CC: Anthony Liguori 
> >>CC: Michael S. Tsirkin 
> >>---
> >>  hw/s390x/virtio-ccw.c  | 5 ++---
> >>  hw/virtio/virtio-pci.c | 5 ++---
> >>  2 files changed, 4 insertions(+), 6 deletions(-)
> >>
> >>diff --git a/hw/s390x/virtio-ccw.c b/hw/s390x/virtio-ccw.c
> >>index ea236c9..82da894 100644
> >>--- a/hw/s390x/virtio-ccw.c
> >>+++ b/hw/s390x/virtio-ccw.c
> >>@@ -899,9 +899,8 @@ static void balloon_ccw_stats_set_poll_interval(Object 
> >>*obj, struct Visitor *v,
> >>  static void virtio_ccw_balloon_instance_init(Object *obj)
> >>  {
> >>  VirtIOBalloonCcw *dev = VIRTIO_BALLOON_CCW(obj);
> >>-object_initialize(&dev->vdev, sizeof(dev->vdev), TYPE_VIRTIO_BALLOON);
> >>-object_property_add_child(obj, "virtio-backend", OBJECT(&dev->vdev), 
> >>NULL);
> >>-object_unref(OBJECT(&dev->vdev));
> >>+virtio_instance_init_common(obj, &dev->vdev, sizeof(dev->vdev),
> >>+TYPE_VIRTIO_BALLOON);
> >>  object_property_add(obj, "guest-stats", "guest statistics",
> >>  balloon_ccw_stats_get_all, NULL, NULL, dev, NULL);
> >>diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c
> >>index dde1d73..745324b 100644
> >>--- a/hw/virtio/virtio-pci.c
> >>+++ b/hw/virtio/virtio-pci.c
> >>@@ -1316,9 +1316,8 @@ static void virtio_balloon_pci_class_init(ObjectClass 
> >>*klass, void *data)
> >>  static void virtio_balloon_pci_instance_init(Object *obj)
> >>  {
> >>  VirtIOBalloonPCI *dev = VIRTIO_BALLOON_PCI(obj);
> >>-object_initialize(&dev->vdev, sizeof(dev->vdev), TYPE_VIRTIO_BALLOON);
> >>-object_property_add_child(obj, "virtio-backend", OBJECT(&dev->vdev), 
> >>NULL);
> >>-object_unref(OBJECT(&dev->vdev));
> >>+virtio_instance_init_common(obj, &dev->vdev, sizeof(dev->vdev),
> >>+TYPE_VIRTIO_BALLOON);
> >>  object_property_add(obj, "guest-stats", "guest statistics",
> >>  balloon_pci_stats_get_all, NULL, NULL, dev,
> >>  NULL);
> >OK, but what about this guest-stats property?
> >Should it get the same treatment?
> >
> >>-- 
> >>1.9.1
> hmm, IMHO no. init_common is actually do the following
> 
> void virtio_instance_init_common(Object *proxy_obj, void *data,
>  size_t vdev_size, const char *vdev_name)
> {
> DeviceState *vdev = data;
> 
> object_initialize(vdev, vdev_size, vdev_name);
> object_property_add_child(proxy_obj, "virtio-backend", OBJECT(vdev),
> NULL);
> object_unref(OBJECT(vdev));
> qdev_alias_all_properties(vdev, proxy_obj);
> }
> 
> on the other hand there is the following code in s390
> 
> static void s390_virtio_net_instance_init(Object *obj)
> {
> VirtIONetS390 *dev = VIRTIO_NET_S390(obj);
> 
> virtio_instance_init_common(obj, &dev->vdev, sizeof(dev->vdev),
> TYPE_VIRTIO_NET);
> object_property_add_alias(obj, "bootindex", OBJECT(&dev->vdev),
>   "bootindex", &error_abort);
> }
> 
> which does not contain guest-stats property.

But why doesn't it?
Seems like an obvious omission?

-- 
MST

Re: [Qemu-devel] [RESEND PATCH 1/2] balloon: call qdev_alias_all_properties for proxy dev in balloon class init

[Denis V. Lunev]

On 19/02/15 12:39, Michael S. Tsirkin wrote:

On Thu, Feb 19, 2015 at 12:36:37PM +0300, Denis V. Lunev wrote:

On 19/02/15 12:25, Michael S. Tsirkin wrote:

On Thu, Jan 29, 2015 at 05:24:41PM +0300, Denis V. Lunev wrote:

The idea is that all other virtio devices are calling this helper
to merge properties of the proxy device. This is the only difference
in between this helper and code in inside virtio_instance_init_common.
The patch should not cause any harm as property list in generic balloon
code is empty.

This also allows to avoid some dummy errors like fixed by this
 commit 91ba21208839643603e7f7fa5864723c3f371ebe
 Author: Gonglei 
 Date:   Tue Sep 30 14:10:35 2014 +0800
 virtio-balloon: fix virtio-balloon child refcount in transports

Signed-off-by: Denis V. Lunev 
Signed-off-by: Raushaniya Maksudova 
Revieved-by: Cornelia Huck 
CC: Christian Borntraeger 
CC: Anthony Liguori 
CC: Michael S. Tsirkin 
---
  hw/s390x/virtio-ccw.c  | 5 ++---
  hw/virtio/virtio-pci.c | 5 ++---
  2 files changed, 4 insertions(+), 6 deletions(-)

diff --git a/hw/s390x/virtio-ccw.c b/hw/s390x/virtio-ccw.c
index ea236c9..82da894 100644
--- a/hw/s390x/virtio-ccw.c
+++ b/hw/s390x/virtio-ccw.c
@@ -899,9 +899,8 @@ static void balloon_ccw_stats_set_poll_interval(Object 
*obj, struct Visitor *v,
  static void virtio_ccw_balloon_instance_init(Object *obj)
  {
  VirtIOBalloonCcw *dev = VIRTIO_BALLOON_CCW(obj);
-object_initialize(&dev->vdev, sizeof(dev->vdev), TYPE_VIRTIO_BALLOON);
-object_property_add_child(obj, "virtio-backend", OBJECT(&dev->vdev), NULL);
-object_unref(OBJECT(&dev->vdev));
+virtio_instance_init_common(obj, &dev->vdev, sizeof(dev->vdev),
+TYPE_VIRTIO_BALLOON);
  object_property_add(obj, "guest-stats", "guest statistics",
  balloon_ccw_stats_get_all, NULL, NULL, dev, NULL);
diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c
index dde1d73..745324b 100644
--- a/hw/virtio/virtio-pci.c
+++ b/hw/virtio/virtio-pci.c
@@ -1316,9 +1316,8 @@ static void virtio_balloon_pci_class_init(ObjectClass 
*klass, void *data)
  static void virtio_balloon_pci_instance_init(Object *obj)
  {
  VirtIOBalloonPCI *dev = VIRTIO_BALLOON_PCI(obj);
-object_initialize(&dev->vdev, sizeof(dev->vdev), TYPE_VIRTIO_BALLOON);
-object_property_add_child(obj, "virtio-backend", OBJECT(&dev->vdev), NULL);
-object_unref(OBJECT(&dev->vdev));
+virtio_instance_init_common(obj, &dev->vdev, sizeof(dev->vdev),
+TYPE_VIRTIO_BALLOON);
  object_property_add(obj, "guest-stats", "guest statistics",
  balloon_pci_stats_get_all, NULL, NULL, dev,
  NULL);

OK, but what about this guest-stats property?
Should it get the same treatment?


--
1.9.1

hmm, IMHO no. init_common is actually do the following

void virtio_instance_init_common(Object *proxy_obj, void *data,
  size_t vdev_size, const char *vdev_name)
{
 DeviceState *vdev = data;

 object_initialize(vdev, vdev_size, vdev_name);
 object_property_add_child(proxy_obj, "virtio-backend", OBJECT(vdev),
NULL);
 object_unref(OBJECT(vdev));
 qdev_alias_all_properties(vdev, proxy_obj);
}

on the other hand there is the following code in s390

static void s390_virtio_net_instance_init(Object *obj)
{
 VirtIONetS390 *dev = VIRTIO_NET_S390(obj);

 virtio_instance_init_common(obj, &dev->vdev, sizeof(dev->vdev),
 TYPE_VIRTIO_NET);
 object_property_add_alias(obj, "bootindex", OBJECT(&dev->vdev),
   "bootindex", &error_abort);
}

which does not contain guest-stats property.

But why doesn't it?
Seems like an obvious omission?


no it is not

cfind . | xargs fgrep "guest-stats"
./hw/virtio/virtio-pci.c: object_property_get(OBJECT(&dev->vdev), v, 
"guest-stats", errp);
./hw/virtio/virtio-pci.c: object_property_get(OBJECT(&dev->vdev), v, 
"guest-stats-polling-interval",
./hw/virtio/virtio-pci.c: object_property_set(OBJECT(&dev->vdev), v, 
"guest-stats-polling-interval",
./hw/virtio/virtio-pci.c:object_property_add(obj, "guest-stats", 
"guest statistics",
./hw/virtio/virtio-pci.c:object_property_add(obj, 
"guest-stats-polling-interval", "int",
./hw/virtio/virtio-balloon.c:visit_start_struct(v, NULL, 
"guest-stats", name, 0, &err);
./hw/virtio/virtio-balloon.c:object_property_add(OBJECT(dev), 
"guest-stats", "guest statistics",
./hw/virtio/virtio-balloon.c:object_property_add(OBJECT(dev), 
"guest-stats-polling-interval", "int",
./hw/s390x/virtio-ccw.c: object_property_get(OBJECT(&dev->vdev), v, 
"guest-stats", errp);
./hw/s390x/virtio-ccw.c: object_property_get(OBJECT(&dev->vdev), v, 
"guest-stats-polling-interval",
./hw/s390x/virtio-ccw.c: object_property_set(OBJECT(&dev->vdev), v, 
"guest-stats-polling-interval",
./hw/s390x/virtio-ccw.c:object_property_add(obj, "guest-stats", 
"guest st

Re: [Qemu-devel] [PATCH] KVM: s390: Add MEMOP ioctls for reading/writing guest memory

[Cornelia Huck]

On Mon, 16 Feb 2015 13:16:41 +0100
Thomas Huth  wrote:

> On s390, we've got to make sure to hold the IPTE lock while accessing
> logical memory. So let's add an ioctl for reading and writing logical
> memory to provide this feature for userspace, too.
> 
> Signed-off-by: Thomas Huth 
> ---
>  Documentation/virtual/kvm/api.txt |   45 +++
>  arch/s390/kvm/gaccess.c   |   22 +++
>  arch/s390/kvm/gaccess.h   |2 +
>  arch/s390/kvm/kvm-s390.c  |   70 
> +
>  include/uapi/linux/kvm.h  |   20 ++
>  5 files changed, 159 insertions(+), 0 deletions(-)

Looks good, except for one minor thing:


> +static long kvm_s390_guest_mem_op(struct kvm_vcpu *vcpu,
> +   struct kvm_s390_mem_op *mop)
> +{
> + void __user *uaddr = (void __user *)mop->buf;
> + void *tmpbuf = NULL;
> + int r, srcu_idx;
> + const u64 supported_flags = KVM_S390_MEMOP_F_INJECT_EXCEPTION
> + | KVM_S390_MEMOP_F_CHECK_ONLY;
> +
> + if (mop->flags & ~supported_flags)
> + return -EINVAL;
> +
> + if (mop->size > 16 * PAGE_SIZE)
> + return -E2BIG;

Do you want to document this limit? Or make it discoverable by having
the capability check return the number of pages?

Re: [Qemu-devel] [PATCH] KVM: s390: Add MEMOP ioctls for reading/writing guest memory

[Thomas Huth]

On Thu, 19 Feb 2015 10:47:29 +0100
Cornelia Huck  wrote:

> On Mon, 16 Feb 2015 13:16:41 +0100
> Thomas Huth  wrote:
> 
> > On s390, we've got to make sure to hold the IPTE lock while accessing
> > logical memory. So let's add an ioctl for reading and writing logical
> > memory to provide this feature for userspace, too.
> > 
> > Signed-off-by: Thomas Huth 
> > ---
> >  Documentation/virtual/kvm/api.txt |   45 +++
> >  arch/s390/kvm/gaccess.c   |   22 +++
> >  arch/s390/kvm/gaccess.h   |2 +
> >  arch/s390/kvm/kvm-s390.c  |   70 
> > +
> >  include/uapi/linux/kvm.h  |   20 ++
> >  5 files changed, 159 insertions(+), 0 deletions(-)
> 
> Looks good, except for one minor thing:
> 
> > +static long kvm_s390_guest_mem_op(struct kvm_vcpu *vcpu,
> > + struct kvm_s390_mem_op *mop)
> > +{
> > +   void __user *uaddr = (void __user *)mop->buf;
> > +   void *tmpbuf = NULL;
> > +   int r, srcu_idx;
> > +   const u64 supported_flags = KVM_S390_MEMOP_F_INJECT_EXCEPTION
> > +   | KVM_S390_MEMOP_F_CHECK_ONLY;
> > +
> > +   if (mop->flags & ~supported_flags)
> > +   return -EINVAL;
> > +
> > +   if (mop->size > 16 * PAGE_SIZE)
> > +   return -E2BIG;
> 
> Do you want to document this limit? Or make it discoverable by having
> the capability check return the number of pages?

I like the idea with the capability ... I think I'll add that feature.

 Thomas

Re: [Qemu-devel] [PATCH] Makefile.target: set icon for binary file on Mac OS X

[Paolo Bonzini]

On 18/02/2015 22:09, Programmingkid wrote:
> + # Take an image and make the image its own icon:
> + sips -i ../pc-bios/qemu-nsis.ico
> + # Extract the icon to its own resource file:
> + DeRez -only icns ../pc-bios/qemu-nsis.ico > tmpicns.rsrc

IIUC sips modifies ../pc-bios/qemu-nsis.ico (adding a resource fork?),
so it's not possible to put it in Makefile.target.  If "sips" is invoked
twice by two different recursive invocations of Makefile.target, bad
things can happen.

I think we can simply distribute tmpicns.rsrc as pc-bios/qemu.rsrc instead.

> + # append this resource to the file you want to icon-ize.
> + Rez -append tmpicns.rsrc -o $(QEMU_PROG)
> +
> + # Use the resource to set the icon.
> + SetFile -a C $(QEMU_PROG)

These two commands can be added after

$(QEMU_PROG_BUILD): $(all-obj-y) ../libqemuutil.a ../libqemustub.a
$(call LINK,$^)

instead of adding a new rule.  There is no need for the comments.

Paolo

Re: [Qemu-devel] [PATCH] Make i82801b11 and ioh3420 x86 only by default

[Paolo Bonzini]

On 19/02/2015 04:45, Peter Maydell wrote:
> > I think it's quite likely that we'll use them, or at least ioh3420, on
> > any PCIe machine.  So you probably want to add ioh3420 to arm-softmmu
> > and aarch64-softmmu as well.  I don't know about i82801b11, but it
> > doesn't hurt to have it in ARM/AArch64 either.
>
> What's the motivation for narrowing these down to particular
> configs anyway?

They do not make sense if you do not have a PCIe bus.

Paolo

Re: [Qemu-devel] [PATCH 2/2] virtio: remove QEMU definition of VIRTIO_TRANSPORT_F_START/_END

[Cornelia Huck]

On Thu, 12 Feb 2015 15:53:49 +0100
Jens Freimann  wrote:

> On Thu, Feb 12, 2015 at 02:41:15PM +, Peter Maydell wrote:
> > On 12 February 2015 at 13:08, Jens Freimann  
> > wrote:
> > > We have defines for VIRTIO_TRANSPORT_F_START/_END in two places.
> > > In include/hw/virtio/virtio.h and in linux-headers/linux/virtio_config.h
> > >
> > > Since we already get virtio_config.h via update-linux-headers.sh,
> > > there's no need to have duplicate defines in QEMU headers files.
> > >
> > > Let's remove this define from include/hw/virtio/virtio.h
> > 
> > Isn't this going to break compilation on non-linux hosts?
> > They don't get linux-headers/ on their include path, so
> > our virtio.h is their only source for this define...
> 
> Ok, that's a fair point that I didn't think of. What's the correct way
> to fix this then? Change our virtio.h manually? In a separate commit
> or in the one generated by update-linux-headers (so we don't break bisect)?

I think this is fixed by MST's standard headers patch series.

[Qemu-devel] [PATCH] qapi-types: add C99 index names to arrays

[Michael S. Tsirkin]

It's not easy to figure out how monitor translates
strings: most QEMU code deals with translated indexes,
these are translated using _lookup arrays,
so you need to find the array name, and find the
appropriate offset.

This patch adds C99 indexes to lookup arrays, which makes it possible to
find the correct key using simple grep, and see that the matching is
correct at a glance.

Example:

Before:

const char *MigrationCapability_lookup[] = {
"xbzrle",
"rdma-pin-all",
"auto-converge",
"zero-blocks",
NULL,
};

After:

const char *MigrationCapability_lookup[] = {
[MIGRATION_CAPABILITY_XBZRLE] = "xbzrle",
[MIGRATION_CAPABILITY_RDMA_PIN_ALL] = "rdma-pin-all",
[MIGRATION_CAPABILITY_AUTO_CONVERGE] = "auto-converge",
[MIGRATION_CAPABILITY_ZERO_BLOCKS] = "zero-blocks",
[MIGRATION_CAPABILITY_MAX] = NULL,
};

Signed-off-by: Michael S. Tsirkin 
---
 scripts/qapi-types.py | 11 +++
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/scripts/qapi-types.py b/scripts/qapi-types.py
index 1eb272d..db87218 100644
--- a/scripts/qapi-types.py
+++ b/scripts/qapi-types.py
@@ -123,16 +123,19 @@ const char *%(name)s_lookup[] = {
  name=name)
 i = 0
 for value in values:
+index = generate_enum_full_value(name, value)
 ret += mcgen('''
-"%(value)s",
+[%(index)s] = "%(value)s",
 ''',
- value=value)
+ index = index, value = value)
 
+max_index = generate_enum_full_value(name, 'MAX')
 ret += mcgen('''
-NULL,
+[%(max_index)s] = NULL,
 };
 
-''')
+''',
+max_index=max_index)
 return ret
 
 def generate_enum(name, values):
-- 
MST

Re: [Qemu-devel] [RESEND PATCH 1/2] balloon: call qdev_alias_all_properties for proxy dev in balloon class init

[Michael S. Tsirkin]

On Thu, Feb 19, 2015 at 12:46:34PM +0300, Denis V. Lunev wrote:
> On 19/02/15 12:39, Michael S. Tsirkin wrote:
> >On Thu, Feb 19, 2015 at 12:36:37PM +0300, Denis V. Lunev wrote:
> >>On 19/02/15 12:25, Michael S. Tsirkin wrote:
> >>>On Thu, Jan 29, 2015 at 05:24:41PM +0300, Denis V. Lunev wrote:
> The idea is that all other virtio devices are calling this helper
> to merge properties of the proxy device. This is the only difference
> in between this helper and code in inside virtio_instance_init_common.
> The patch should not cause any harm as property list in generic balloon
> code is empty.
> 
> This also allows to avoid some dummy errors like fixed by this
>  commit 91ba21208839643603e7f7fa5864723c3f371ebe
>  Author: Gonglei 
>  Date:   Tue Sep 30 14:10:35 2014 +0800
>  virtio-balloon: fix virtio-balloon child refcount in transports
> 
> Signed-off-by: Denis V. Lunev 
> Signed-off-by: Raushaniya Maksudova 
> Revieved-by: Cornelia Huck 
> CC: Christian Borntraeger 
> CC: Anthony Liguori 
> CC: Michael S. Tsirkin 
> ---
>   hw/s390x/virtio-ccw.c  | 5 ++---
>   hw/virtio/virtio-pci.c | 5 ++---
>   2 files changed, 4 insertions(+), 6 deletions(-)
> 
> diff --git a/hw/s390x/virtio-ccw.c b/hw/s390x/virtio-ccw.c
> index ea236c9..82da894 100644
> --- a/hw/s390x/virtio-ccw.c
> +++ b/hw/s390x/virtio-ccw.c
> @@ -899,9 +899,8 @@ static void 
> balloon_ccw_stats_set_poll_interval(Object *obj, struct Visitor *v,
>   static void virtio_ccw_balloon_instance_init(Object *obj)
>   {
>   VirtIOBalloonCcw *dev = VIRTIO_BALLOON_CCW(obj);
> -object_initialize(&dev->vdev, sizeof(dev->vdev), 
> TYPE_VIRTIO_BALLOON);
> -object_property_add_child(obj, "virtio-backend", OBJECT(&dev->vdev), 
> NULL);
> -object_unref(OBJECT(&dev->vdev));
> +virtio_instance_init_common(obj, &dev->vdev, sizeof(dev->vdev),
> +TYPE_VIRTIO_BALLOON);
>   object_property_add(obj, "guest-stats", "guest statistics",
>   balloon_ccw_stats_get_all, NULL, NULL, dev, 
>  NULL);
> diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c
> index dde1d73..745324b 100644
> --- a/hw/virtio/virtio-pci.c
> +++ b/hw/virtio/virtio-pci.c
> @@ -1316,9 +1316,8 @@ static void 
> virtio_balloon_pci_class_init(ObjectClass *klass, void *data)
>   static void virtio_balloon_pci_instance_init(Object *obj)
>   {
>   VirtIOBalloonPCI *dev = VIRTIO_BALLOON_PCI(obj);
> -object_initialize(&dev->vdev, sizeof(dev->vdev), 
> TYPE_VIRTIO_BALLOON);
> -object_property_add_child(obj, "virtio-backend", OBJECT(&dev->vdev), 
> NULL);
> -object_unref(OBJECT(&dev->vdev));
> +virtio_instance_init_common(obj, &dev->vdev, sizeof(dev->vdev),
> +TYPE_VIRTIO_BALLOON);
>   object_property_add(obj, "guest-stats", "guest statistics",
>   balloon_pci_stats_get_all, NULL, NULL, dev,
>   NULL);
> >>>OK, but what about this guest-stats property?
> >>>Should it get the same treatment?
> >>>
> -- 
> 1.9.1
> >>hmm, IMHO no. init_common is actually do the following
> >>
> >>void virtio_instance_init_common(Object *proxy_obj, void *data,
> >>  size_t vdev_size, const char *vdev_name)
> >>{
> >> DeviceState *vdev = data;
> >>
> >> object_initialize(vdev, vdev_size, vdev_name);
> >> object_property_add_child(proxy_obj, "virtio-backend", OBJECT(vdev),
> >>NULL);
> >> object_unref(OBJECT(vdev));
> >> qdev_alias_all_properties(vdev, proxy_obj);
> >>}
> >>
> >>on the other hand there is the following code in s390
> >>
> >>static void s390_virtio_net_instance_init(Object *obj)
> >>{
> >> VirtIONetS390 *dev = VIRTIO_NET_S390(obj);
> >>
> >> virtio_instance_init_common(obj, &dev->vdev, sizeof(dev->vdev),
> >> TYPE_VIRTIO_NET);
> >> object_property_add_alias(obj, "bootindex", OBJECT(&dev->vdev),
> >>   "bootindex", &error_abort);
> >>}
> >>
> >>which does not contain guest-stats property.
> >But why doesn't it?
> >Seems like an obvious omission?
> >
> no it is not
> 
> cfind . | xargs fgrep "guest-stats"
> ./hw/virtio/virtio-pci.c: object_property_get(OBJECT(&dev->vdev), v,
> "guest-stats", errp);
> ./hw/virtio/virtio-pci.c: object_property_get(OBJECT(&dev->vdev), v,
> "guest-stats-polling-interval",
> ./hw/virtio/virtio-pci.c: object_property_set(OBJECT(&dev->vdev), v,
> "guest-stats-polling-interval",
> ./hw/virtio/virtio-pci.c:object_property_add(obj, "guest-stats", "guest
> statistics",
> ./hw/virtio/virtio-pci.c:object_property_add(obj,
> "guest-stats-polling-interval", "int",
> ./hw/virtio/virtio-balloon.c:visit_start_struct(v, NULL, "gu

Re: [Qemu-devel] [PATCH 1/2] target-mips: replace cpu_save/cpu_load with VMStateDescription

[Leon Alrae]

Hi,

On 18/02/2015 16:59, Andreas Färber wrote:
> Hi Leon,
> 
> Am 18.02.2015 um 15:51 schrieb Leon Alrae:
>> Create VMStateDescription for MIPS CPU. The new structure contains exactly 
>> the
>> same fields as before, therefore leaving existing version_id.
>>
>> Signed-off-by: Leon Alrae 
>> ---
>>  target-mips/cpu-qom.h |   4 +
>>  target-mips/cpu.c |   1 +
>>  target-mips/cpu.h |   2 -
>>  target-mips/machine.c | 567 
>> ++
>>  4 files changed, 257 insertions(+), 317 deletions(-)
> [...]
>> diff --git a/target-mips/cpu.c b/target-mips/cpu.c
>> index 98dc94e..cbdc463 100644
>> --- a/target-mips/cpu.c
>> +++ b/target-mips/cpu.c
>> @@ -148,6 +148,7 @@ static void mips_cpu_class_init(ObjectClass *c, void 
>> *data)
>>  cc->do_unassigned_access = mips_cpu_unassigned_access;
>>  cc->do_unaligned_access = mips_cpu_do_unaligned_access;
>>  cc->get_phys_page_debug = mips_cpu_get_phys_page_debug;
>> +dc->vmsd = &vmstate_mips_cpu;
> 
> This looks wrong. There's two ways to do a CPU VMSD, 1) via dc->vmsd,
> and 2) via cc->vmsd. When going for the new dc->vmsd, the common CPU
> state would need to be referenced from vmstate_mips_cpu below. Doing so
> would break backwards compatibility, so you probably want cc->vmsd,
> causing a separate VMSD for the common parts to be registered.

Ah, I see it now in cpu_exec_init() that vmstate_cpu_common won't get
registered if I set dc->vmsd. Thanks for pointing that out.

Regards,
Leon

Re: [Qemu-devel] [RESEND PATCH 1/2] balloon: call qdev_alias_all_properties for proxy dev in balloon class init

[Denis V. Lunev]

On 19/02/15 13:17, Michael S. Tsirkin wrote:

On Thu, Feb 19, 2015 at 12:46:34PM +0300, Denis V. Lunev wrote:

On 19/02/15 12:39, Michael S. Tsirkin wrote:

On Thu, Feb 19, 2015 at 12:36:37PM +0300, Denis V. Lunev wrote:

On 19/02/15 12:25, Michael S. Tsirkin wrote:

On Thu, Jan 29, 2015 at 05:24:41PM +0300, Denis V. Lunev wrote:

The idea is that all other virtio devices are calling this helper
to merge properties of the proxy device. This is the only difference
in between this helper and code in inside virtio_instance_init_common.
The patch should not cause any harm as property list in generic balloon
code is empty.

This also allows to avoid some dummy errors like fixed by this
 commit 91ba21208839643603e7f7fa5864723c3f371ebe
 Author: Gonglei 
 Date:   Tue Sep 30 14:10:35 2014 +0800
 virtio-balloon: fix virtio-balloon child refcount in transports

Signed-off-by: Denis V. Lunev 
Signed-off-by: Raushaniya Maksudova 
Revieved-by: Cornelia Huck 
CC: Christian Borntraeger 
CC: Anthony Liguori 
CC: Michael S. Tsirkin 
---
  hw/s390x/virtio-ccw.c  | 5 ++---
  hw/virtio/virtio-pci.c | 5 ++---
  2 files changed, 4 insertions(+), 6 deletions(-)

diff --git a/hw/s390x/virtio-ccw.c b/hw/s390x/virtio-ccw.c
index ea236c9..82da894 100644
--- a/hw/s390x/virtio-ccw.c
+++ b/hw/s390x/virtio-ccw.c
@@ -899,9 +899,8 @@ static void balloon_ccw_stats_set_poll_interval(Object 
*obj, struct Visitor *v,
  static void virtio_ccw_balloon_instance_init(Object *obj)
  {
  VirtIOBalloonCcw *dev = VIRTIO_BALLOON_CCW(obj);
-object_initialize(&dev->vdev, sizeof(dev->vdev), TYPE_VIRTIO_BALLOON);
-object_property_add_child(obj, "virtio-backend", OBJECT(&dev->vdev), NULL);
-object_unref(OBJECT(&dev->vdev));
+virtio_instance_init_common(obj, &dev->vdev, sizeof(dev->vdev),
+TYPE_VIRTIO_BALLOON);
  object_property_add(obj, "guest-stats", "guest statistics",
  balloon_ccw_stats_get_all, NULL, NULL, dev, NULL);
diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c
index dde1d73..745324b 100644
--- a/hw/virtio/virtio-pci.c
+++ b/hw/virtio/virtio-pci.c
@@ -1316,9 +1316,8 @@ static void virtio_balloon_pci_class_init(ObjectClass 
*klass, void *data)
  static void virtio_balloon_pci_instance_init(Object *obj)
  {
  VirtIOBalloonPCI *dev = VIRTIO_BALLOON_PCI(obj);
-object_initialize(&dev->vdev, sizeof(dev->vdev), TYPE_VIRTIO_BALLOON);
-object_property_add_child(obj, "virtio-backend", OBJECT(&dev->vdev), NULL);
-object_unref(OBJECT(&dev->vdev));
+virtio_instance_init_common(obj, &dev->vdev, sizeof(dev->vdev),
+TYPE_VIRTIO_BALLOON);
  object_property_add(obj, "guest-stats", "guest statistics",
  balloon_pci_stats_get_all, NULL, NULL, dev,
  NULL);

OK, but what about this guest-stats property?
Should it get the same treatment?


--
1.9.1

hmm, IMHO no. init_common is actually do the following

void virtio_instance_init_common(Object *proxy_obj, void *data,
  size_t vdev_size, const char *vdev_name)
{
 DeviceState *vdev = data;

 object_initialize(vdev, vdev_size, vdev_name);
 object_property_add_child(proxy_obj, "virtio-backend", OBJECT(vdev),
NULL);
 object_unref(OBJECT(vdev));
 qdev_alias_all_properties(vdev, proxy_obj);
}

on the other hand there is the following code in s390

static void s390_virtio_net_instance_init(Object *obj)
{
 VirtIONetS390 *dev = VIRTIO_NET_S390(obj);

 virtio_instance_init_common(obj, &dev->vdev, sizeof(dev->vdev),
 TYPE_VIRTIO_NET);
 object_property_add_alias(obj, "bootindex", OBJECT(&dev->vdev),
   "bootindex", &error_abort);
}

which does not contain guest-stats property.

But why doesn't it?
Seems like an obvious omission?


no it is not

cfind . | xargs fgrep "guest-stats"
./hw/virtio/virtio-pci.c: object_property_get(OBJECT(&dev->vdev), v,
"guest-stats", errp);
./hw/virtio/virtio-pci.c: object_property_get(OBJECT(&dev->vdev), v,
"guest-stats-polling-interval",
./hw/virtio/virtio-pci.c: object_property_set(OBJECT(&dev->vdev), v,
"guest-stats-polling-interval",
./hw/virtio/virtio-pci.c:object_property_add(obj, "guest-stats", "guest
statistics",
./hw/virtio/virtio-pci.c:object_property_add(obj,
"guest-stats-polling-interval", "int",
./hw/virtio/virtio-balloon.c:visit_start_struct(v, NULL, "guest-stats",
name, 0, &err);
./hw/virtio/virtio-balloon.c:object_property_add(OBJECT(dev),
"guest-stats", "guest statistics",
./hw/virtio/virtio-balloon.c:object_property_add(OBJECT(dev),
"guest-stats-polling-interval", "int",
./hw/s390x/virtio-ccw.c: object_property_get(OBJECT(&dev->vdev), v,
"guest-stats", errp);
./hw/s390x/virtio-ccw.c: object_property_get(OBJECT(&dev->vdev), v,
"guest-stats-polling-interval",
./hw/s390x/virtio-ccw.c: object_property_set(OBJECT(&dev->vdev), v,
"guest-sta

Re: [Qemu-devel] [PATCH] migration/rdma: clean up qemu_rdma_dest_init a bit

[Dr. David Alan Gilbert]

* Michael Tokarev (m...@tls.msk.ru) wrote:
> Do not check for rdma->host being empty twice.  This removes a large
> "if" block, so code indentation is changed.  While at it, remove an
> ugly goto from the loop, replacing it with a cleaner if logic.  And
> finally, there's no need to initialize `ret' variable since is always
> has a value.

This looks OK; have you got RDMA hardware to test with, if not I can
give it a go.

Dave

> 
> Signed-off-by: Michael Tokarev 
> ---
>  migration/rdma.c | 51 ++-
>  1 file changed, 22 insertions(+), 29 deletions(-)
> 
> diff --git a/migration/rdma.c b/migration/rdma.c
> index 6bee30c..76af724 100644
> --- a/migration/rdma.c
> +++ b/migration/rdma.c
> @@ -2366,10 +2366,10 @@ err_rdma_source_connect:
>  
>  static int qemu_rdma_dest_init(RDMAContext *rdma, Error **errp)
>  {
> -int ret = -EINVAL, idx;
> +int ret, idx;
>  struct rdma_cm_id *listen_id;
>  char ip[40] = "unknown";
> -struct rdma_addrinfo *res;
> +struct rdma_addrinfo *res, *e;
>  char port_str[16];
>  
>  for (idx = 0; idx < RDMA_WRID_MAX; idx++) {
> @@ -2377,7 +2377,7 @@ static int qemu_rdma_dest_init(RDMAContext *rdma, Error 
> **errp)
>  rdma->wr_data[idx].control_curr = NULL;
>  }
>  
> -if (rdma->host == NULL) {
> +if (!rdma->host || !rdma->host[0]) {
>  ERROR(errp, "RDMA host is not set!");
>  rdma->error_state = -EINVAL;
>  return -1;
> @@ -2400,40 +2400,33 @@ static int qemu_rdma_dest_init(RDMAContext *rdma, 
> Error **errp)
>  snprintf(port_str, 16, "%d", rdma->port);
>  port_str[15] = '\0';
>  
> -if (rdma->host && strcmp("", rdma->host)) {
> -struct rdma_addrinfo *e;
> +ret = rdma_getaddrinfo(rdma->host, port_str, NULL, &res);
> +if (ret < 0) {
> +ERROR(errp, "could not rdma_getaddrinfo address %s", rdma->host);
> +goto err_dest_init_bind_addr;
> +}
>  
> -ret = rdma_getaddrinfo(rdma->host, port_str, NULL, &res);
> -if (ret < 0) {
> -ERROR(errp, "could not rdma_getaddrinfo address %s", rdma->host);
> -goto err_dest_init_bind_addr;
> +for (e = res; e != NULL; e = e->ai_next) {
> +inet_ntop(e->ai_family,
> +&((struct sockaddr_in *) e->ai_dst_addr)->sin_addr, ip, sizeof 
> ip);
> +trace_qemu_rdma_dest_init_trying(rdma->host, ip);
> +ret = rdma_bind_addr(listen_id, e->ai_dst_addr);
> +if (ret) {
> +continue;
>  }
> -
> -for (e = res; e != NULL; e = e->ai_next) {
> -inet_ntop(e->ai_family,
> -&((struct sockaddr_in *) e->ai_dst_addr)->sin_addr, ip, 
> sizeof ip);
> -trace_qemu_rdma_dest_init_trying(rdma->host, ip);
> -ret = rdma_bind_addr(listen_id, e->ai_dst_addr);
> -if (!ret) {
> -if (e->ai_family == AF_INET6) {
> -ret = qemu_rdma_broken_ipv6_kernel(errp, 
> listen_id->verbs);
> -if (ret) {
> -continue;
> -}
> -}
> -
> -goto listen;
> +if (e->ai_family == AF_INET6) {
> +ret = qemu_rdma_broken_ipv6_kernel(errp, listen_id->verbs);
> +if (ret) {
> +continue;
>  }
>  }
> +break; 
> +}
>  
> +if (!e) {
>  ERROR(errp, "Error: could not rdma_bind_addr!");
>  goto err_dest_init_bind_addr;
> -} else {
> -ERROR(errp, "migration host and port not specified!");
> -ret = -EINVAL;
> -goto err_dest_init_bind_addr;
>  }
> -listen:
>  
>  rdma->listen_id = listen_id;
>  qemu_rdma_dump_gid("dest_init", listen_id);
> -- 
> 2.1.4
> 
> 
--
Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK

Re: [Qemu-devel] [RESEND PATCH 1/2] balloon: call qdev_alias_all_properties for proxy dev in balloon class init

[Michael S. Tsirkin]

On Thu, Feb 19, 2015 at 01:23:03PM +0300, Denis V. Lunev wrote:
> >The problem is code duplication: all transports need to know
> >about these balloon-specific property.
> >Why isn't it handled by virtio_instance_init_common?
> >
> why it should?
> 
> virtio_instance_init_common is common for all virtio devices
> including VirtIO net, VirtIO block, VirtIO SCSI. Thus the patch
> move initialization of all common stuff into the common
> code.

The problem seems common enough, virtio_instance_init_common
already works for all properties, why not for these ones?

Re: [Qemu-devel] [PATCH] Make i82801b11 and ioh3420 x86 only by default

[Michael S. Tsirkin]

On Thu, Feb 19, 2015 at 04:20:36PM +1100, David Gibson wrote:
> 
> On Thu, Feb 19, 2015 at 12:45:23PM +0900, Peter Maydell wrote:
> > On 18 February 2015 at 16:57, Paolo Bonzini  wrote:
> > > On 18/02/2015 05:59, David Gibson wrote:
> > >> that are very unlikely to appear on anything other than
> > >> an x86.  Therefore this patch gives them their own config options, 
> > >> enabled
> > >> only for x86 targets by default.
> > >
> > > I think it's quite likely that we'll use them, or at least ioh3420, on
> > > any PCIe machine.  So you probably want to add ioh3420 to arm-softmmu
> > > and aarch64-softmmu as well.  I don't know about i82801b11, but it
> > > doesn't hurt to have it in ARM/AArch64 either.
> > 
> > What's the motivation for narrowing these down to particular
> > configs anyway?
> 
> Simply to avoid probably-irrelevant devices showing up in qemu -device
> ?

And make the binary smaller.

-- 
MST

[Qemu-devel] bus_unparent() can go into infinite loop

[Markus Armbruster]

Reproducer (don't ask):

$ qemu-system-arm -M virt -S -display none -drive 
if=scsi,id=foo,bus=1,file=tmp.qcow2 -device nec-usb-xhci -device 
usb-storage,drive=foo -device virtio-scsi-pci
qemu-system-arm: -drive if=scsi,id=foo,bus=1,file=tmp.qcow2: Property 
'scsi-disk.drive' can't take value 'foo', it's in use
qemu-system-arm: -drive if=scsi,id=foo,bus=1,file=tmp.qcow2: Setting drive 
property failed
qemu-system-arm: -device virtio-scsi-pci: Setting drive property failed

Nevermind the silly error reporting, I got patches to clean that up.

Stuck in bus_unparent()'s loop:

while ((kid = QTAILQ_FIRST(&bus->children)) != NULL) {
DeviceState *dev = kid->child;
object_unparent(OBJECT(dev));
}

(gdb) bt
#0  bus_unparent (obj=)
at /home/armbru/work/qemu/hw/core/qdev.c:727
#1  0x5583a165 in object_finalize_child_property (obj=, 
name=, opaque=0x56450060)
at /home/armbru/work/qemu/qom/object.c:1079
#2  0x5583a40c in object_property_del (obj=0x5644ff50, 
name=0x56696450 "scsi.1", errp=)
at /home/armbru/work/qemu/qom/object.c:800
#3  0x5577f759 in device_unparent (obj=0x5644ff50)
at /home/armbru/work/qemu/hw/core/qdev.c:1230
#4  0x5583a165 in object_finalize_child_property (obj=, 
name=, opaque=0x5644ff50)
at /home/armbru/work/qemu/qom/object.c:1079
#5  0x5583a40c in object_property_del (obj=0x5644f540, 
name=0x566b9620 "virtio-backend", errp=)
at /home/armbru/work/qemu/qom/object.c:800
#6  0x557800c6 in qdev_init (dev=dev@entry=0x5644ff50)
at /home/armbru/work/qemu/hw/core/qdev.c:186
#7  0x5582440e in virtio_scsi_pci_init_pci (vpci_dev=0x5644f540)
at /home/armbru/work/qemu/hw/virtio/virtio-pci.c:1157
#8  0x55825fc8 in virtio_pci_init (pci_dev=)
at /home/armbru/work/qemu/hw/virtio/virtio-pci.c:1018
#9  0x557d6df7 in pci_qdev_init (qdev=0x5644f540)
at /home/armbru/work/qemu/hw/pci/pci.c:1775
#10 0x5577fbc4 in device_realize (dev=0x5644f540, 
errp=0x7fffda60) at /home/armbru/work/qemu/hw/core/qdev.c:247
#11 0x5578125d in device_set_realized (obj=0x5644f540, 
value=, errp=0x7fffdb98)
at /home/armbru/work/qemu/hw/core/qdev.c:1040
#12 0x5583927e in property_set_bool (obj=0x5644f540, 
v=, opaque=0x56698850, name=, 
errp=0x7fffdb98) at /home/armbru/work/qemu/qom/object.c:1514
#13 0x5583bc67 in object_property_set_qobject (obj=0x5644f540, 
value=, name=0x559482fd "realized", errp=0x7fffdb98)
at /home/armbru/work/qemu/qom/qom-qobject.c:24
#14 0x5583a7b0 in object_property_set_bool (
obj=obj@entry=0x5644f540, value=value@entry=true, 
name=name@entry=0x559482fd "realized", errp=errp@entry=0x7fffdb98)
at /home/armbru/work/qemu/qom/object.c:905
#15 0x5570d774 in qdev_device_add (opts=0x562a38c0)
at /home/armbru/work/qemu/qdev-monitor.c:574
#16 0x55716c79 in device_init_func (opts=, 
opaque=) at /home/armbru/work/qemu/vl.c:2127
#17 0x558f48eb in qemu_opts_foreach (list=, func=
0x55716c70 , opaque=0x0, 
abort_on_failure=)
at /home/armbru/work/qemu/util/qemu-option.c:1057
#18 0x5560e39d in main (argc=, argv=, 
envp=) at /home/armbru/work/qemu/vl.c:4244
(gdb) p dev->parent_obj.class->type->name
$5 = 0x5626c3b0 "scsi-disk"
(gdb) p bus->name
$8 = 0x56696430 "scsi.1"

Re: [Qemu-devel] [RESEND PATCH 1/2] balloon: call qdev_alias_all_properties for proxy dev in balloon class init

[Cornelia Huck]

On Thu, 19 Feb 2015 11:29:27 +0100
"Michael S. Tsirkin"  wrote:

> On Thu, Feb 19, 2015 at 01:23:03PM +0300, Denis V. Lunev wrote:
> > >The problem is code duplication: all transports need to know
> > >about these balloon-specific property.
> > >Why isn't it handled by virtio_instance_init_common?
> > >
> > why it should?
> > 
> > virtio_instance_init_common is common for all virtio devices
> > including VirtIO net, VirtIO block, VirtIO SCSI. Thus the patch
> > move initialization of all common stuff into the common
> > code.
> 
> The problem seems common enough, virtio_instance_init_common
> already works for all properties, why not for these ones?

It only works for the properties that are common amongst transports and
all devices.

Adding a virtio_balloon_init_common() that adds the guest_stats (or a
virtio_rng_init_common() that adds rng) is probably not a bad idea, but
I'd prefer it as patches on top of these.

Re: [Qemu-devel] [PATCH] target-mips: fix CP0.BadVAddr by stopping translation on Address error

[Leon Alrae]

On 28/01/2015 00:39, Maciej W. Rozycki wrote:
> On Mon, 26 Jan 2015, Leon Alrae wrote:
> 
>> BadVAddr is supposed to capture the most recent address that caused
>> the exception. Currently this is not happening as translation is not stopped
>> and BadVAddr is updated with subsequent addresses.
>>
>> Signed-off-by: Leon Alrae 
>> ---
> 
>  I think this deserves a better description as it is about the specific 
> case of an unaligned standard MIPS instruction fetch.  Address Error 
> exceptions can also happen for other reasons: unaligned data accesses or 
> any accesses outside memory segments the current execution mode is allowed 
> to reach.

I believe that the one line change in the patch makes that clear. I
agree however that the description itself could be more precise.

>  While at it I think it may be worth double-checking if the other places 
> that trigger this exception get it right.

Other places seem to look fine. Even decode_micromips_opc handles it
correctly whereas decode_opc -– which obviously was implemented before
microMIPS -- never got fixed.

Regards,
Leon

[Qemu-devel] [PATCH v3 1/3] Add -incoming defer

[Dr. David Alan Gilbert (git)]

From: "Dr. David Alan Gilbert" 

-incoming defer causes qemu to wait for an incoming migration
to be specified later.  The monitor can be used to set migration
capabilities that may affect the incoming connection process.

Signed-off-by: Dr. David Alan Gilbert 
Reviewed-by: Juan Quintela 
---
 migration/migration.c | 29 +++--
 1 file changed, 23 insertions(+), 6 deletions(-)

diff --git a/migration/migration.c b/migration/migration.c
index b3adbc6..f3d49d5 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -49,6 +49,8 @@ enum {
 static NotifierList migration_state_notifiers =
 NOTIFIER_LIST_INITIALIZER(migration_state_notifiers);
 
+static bool deferred_incoming;
+
 /* When we add fault tolerance, we could have several
migrations at once.  For now we don't need to add
dynamic creation of migration */
@@ -65,25 +67,40 @@ MigrationState *migrate_get_current(void)
 return ¤t_migration;
 }
 
+/*
+ * Called on -incoming with a defer: uri.
+ * The migration can be started later after any parameters have been
+ * changed.
+ */
+static void deferred_incoming_migration(Error **errp)
+{
+if (deferred_incoming) {
+error_setg(errp, "Incoming migration already deferred");
+}
+deferred_incoming = true;
+}
+
 void qemu_start_incoming_migration(const char *uri, Error **errp)
 {
 const char *p;
 
-if (strstart(uri, "tcp:", &p))
+if (!strcmp(uri, "defer")) {
+deferred_incoming_migration(errp);
+} else if (strstart(uri, "tcp:", &p)) {
 tcp_start_incoming_migration(p, errp);
 #ifdef CONFIG_RDMA
-else if (strstart(uri, "rdma:", &p))
+} else if (strstart(uri, "rdma:", &p)) {
 rdma_start_incoming_migration(p, errp);
 #endif
 #if !defined(WIN32)
-else if (strstart(uri, "exec:", &p))
+} else if (strstart(uri, "exec:", &p)) {
 exec_start_incoming_migration(p, errp);
-else if (strstart(uri, "unix:", &p))
+} else if (strstart(uri, "unix:", &p)) {
 unix_start_incoming_migration(p, errp);
-else if (strstart(uri, "fd:", &p))
+} else if (strstart(uri, "fd:", &p)) {
 fd_start_incoming_migration(p, errp);
 #endif
-else {
+} else {
 error_setg(errp, "unknown migration protocol: %s", uri);
 }
 }
-- 
2.1.0

[Qemu-devel] [PATCH v3 0/3] -incoming defer

[Dr. David Alan Gilbert (git)]

From: "Dr. David Alan Gilbert" 

This patchset provides a way of setting options on an incoming
migration before the fd/process/socket has been created.

   start qemu with   -incoming defer
   
   migrate_incoming theuri

v3:
  s/pause/defer/  (and associated fixups)
  Add ' around the '-incoming defer' in an error

v2:
  Create migrate_incoming/migrate-incoming rather than squashing the -u
  into the existing migrate command.

Dave

Dr. David Alan Gilbert (3):
  Add -incoming defer
  Add migrate_incoming
  Document -incoming options

 hmp-commands.hx   | 16 
 hmp.c | 14 ++
 hmp.h |  1 +
 migration/migration.c | 48 ++--
 qapi-schema.json  | 15 +++
 qemu-options.hx   | 29 ++---
 qmp-commands.hx   | 31 ++-
 7 files changed, 144 insertions(+), 10 deletions(-)

-- 
2.1.0

[Qemu-devel] [PATCH v3 2/3] Add migrate_incoming

[Dr. David Alan Gilbert (git)]

From: "Dr. David Alan Gilbert" 

Add migrate_incoming/migrate-incoming to start an incoming
migration.

Once a qemu has been started with
-incoming defer

the migration can be started by issuing:
migrate_incoming uri

Signed-off-by: Dr. David Alan Gilbert 
Reviewed-by: Eric Blake 
---
 hmp-commands.hx   | 16 
 hmp.c | 14 ++
 hmp.h |  1 +
 migration/migration.c | 19 +++
 qapi-schema.json  | 15 +++
 qmp-commands.hx   | 31 ++-
 6 files changed, 95 insertions(+), 1 deletion(-)

diff --git a/hmp-commands.hx b/hmp-commands.hx
index e37bc8b..5dcc556 100644
--- a/hmp-commands.hx
+++ b/hmp-commands.hx
@@ -922,6 +922,22 @@ Cancel the current VM migration.
 ETEXI
 
 {
+.name   = "migrate_incoming",
+.args_type  = "uri:s",
+.params = "uri",
+.help   = "Continue an incoming migration from an -incoming defer",
+.mhandler.cmd = hmp_migrate_incoming,
+},
+
+STEXI
+@item migrate_incoming @var{uri}
+@findex migrate_incoming
+Continue an incoming migration using the @var{uri} (that has the same syntax
+as the -incoming option).
+
+ETEXI
+
+{
 .name   = "migrate_set_cache_size",
 .args_type  = "value:o",
 .params = "value",
diff --git a/hmp.c b/hmp.c
index b47f331..f051896 100644
--- a/hmp.c
+++ b/hmp.c
@@ -1083,6 +1083,20 @@ void hmp_migrate_cancel(Monitor *mon, const QDict *qdict)
 qmp_migrate_cancel(NULL);
 }
 
+void hmp_migrate_incoming(Monitor *mon, const QDict *qdict)
+{
+Error *err = NULL;
+const char *uri = qdict_get_str(qdict, "uri");
+
+qmp_migrate_incoming(uri, &err);
+
+if (err) {
+monitor_printf(mon, "%s\n", error_get_pretty(err));
+error_free(err);
+return;
+}
+}
+
 void hmp_migrate_set_downtime(Monitor *mon, const QDict *qdict)
 {
 double value = qdict_get_double(qdict, "value");
diff --git a/hmp.h b/hmp.h
index 4bb5dca..95efe63 100644
--- a/hmp.h
+++ b/hmp.h
@@ -60,6 +60,7 @@ void hmp_snapshot_delete_blkdev_internal(Monitor *mon, const 
QDict *qdict);
 void hmp_drive_mirror(Monitor *mon, const QDict *qdict);
 void hmp_drive_backup(Monitor *mon, const QDict *qdict);
 void hmp_migrate_cancel(Monitor *mon, const QDict *qdict);
+void hmp_migrate_incoming(Monitor *mon, const QDict *qdict);
 void hmp_migrate_set_downtime(Monitor *mon, const QDict *qdict);
 void hmp_migrate_set_speed(Monitor *mon, const QDict *qdict);
 void hmp_migrate_set_capability(Monitor *mon, const QDict *qdict);
diff --git a/migration/migration.c b/migration/migration.c
index f3d49d5..2c805f1 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -432,6 +432,25 @@ void migrate_del_blocker(Error *reason)
 migration_blockers = g_slist_remove(migration_blockers, reason);
 }
 
+void qmp_migrate_incoming(const char *uri, Error **errp)
+{
+Error *local_err = NULL;
+
+if (!deferred_incoming) {
+error_setg(errp, "'-incoming defer' is required for migrate_incoming");
+return;
+}
+
+qemu_start_incoming_migration(uri, &local_err);
+
+if (local_err) {
+error_propagate(errp, local_err);
+return;
+}
+
+deferred_incoming = false;
+}
+
 void qmp_migrate(const char *uri, bool has_blk, bool blk,
  bool has_inc, bool inc, bool has_detach, bool detach,
  Error **errp)
diff --git a/qapi-schema.json b/qapi-schema.json
index e16f8eb..7a80081 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -1738,6 +1738,21 @@
 { 'command': 'migrate',
   'data': {'uri': 'str', '*blk': 'bool', '*inc': 'bool', '*detach': 'bool' } }
 
+##
+# @migrate-incoming
+#
+# Start an incoming migration, the qemu must have been started
+# with -incoming defer
+#
+# @uri: The Uniform Resource Identifier identifying the source or
+#   address to listen on
+#
+# Returns: nothing on success
+#
+# Since: 2.3
+##
+{ 'command': 'migrate-incoming', 'data': {'uri': 'str' } }
+
 # @xen-save-devices-state:
 #
 # Save the state of all devices to file. The RAM and the block devices
diff --git a/qmp-commands.hx b/qmp-commands.hx
index a85d847..60181c7 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -661,7 +661,36 @@ Example:
 <- { "return": {} }
 
 EQMP
-{
+
+{
+.name   = "migrate-incoming",
+.args_type  = "uri:s",
+.mhandler.cmd_new = qmp_marshal_input_migrate_incoming,
+},
+
+SQMP
+migrate-incoming
+
+
+Continue an incoming migration
+
+Arguments:
+
+- "uri": Source/listening URI (json-string)
+
+Example:
+
+-> { "execute": "migrate-incoming", "arguments": { "uri": "tcp::4446" } }
+<- { "return": {} }
+
+Notes:
+
+(1) QEMU must be started with -incoming defer to allow migrate-incoming to
+be used
+(2) The uri format is the same as to -incoming
+
+EQMP
+{
 .name   = "migrate-set-cache-size",
 .args_type  = "value:o",
 .mhand

[Qemu-devel] [PATCH v3 3/3] Document -incoming options

[Dr. David Alan Gilbert (git)]

From: "Dr. David Alan Gilbert" 

Document the various URI formats for -incoming, the previous
manpage and help text was wrong (out of date?)

Signed-off-by: Dr. David Alan Gilbert 
Reviewed-by: Juan Quintela 
---
 qemu-options.hx | 29 ++---
 1 file changed, 26 insertions(+), 3 deletions(-)

diff --git a/qemu-options.hx b/qemu-options.hx
index 85ca3ad..6d6d2a8 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -3169,12 +3169,35 @@ Set TB size.
 ETEXI
 
 DEF("incoming", HAS_ARG, QEMU_OPTION_incoming, \
-"-incoming p prepare for incoming migration, listen on port p\n",
+"-incoming uri   prepare for incoming migration, specifying source:\n" \
+"exec:commandExecute 'command' use the stdout as\n" \
+"the migration stream\n" \
+"fd:num  listen on the given fd\n" \
+"defer   wait for the URI to be specified by\n" \
+"the monitor (migrate_incoming)\n" \
+"rdma:addr:port  Listen on RDMA port on given address\n" \
+"tcp:addr:port   listen on TCP port (optional address)\n" \
+"unix:path   listen on the UNIX socket 'path'\n", \
 QEMU_ARCH_ALL)
 STEXI
-@item -incoming @var{port}
+@item -incoming @var{uri}
 @findex -incoming
-Prepare for incoming migration, listen on @var{port}.
+Prepare for incoming migration, specifying the source of the migration stream
+@table @option
+@item exec:@var{command}
+Execute 'command' and use the stdout as the migration stream.
+@item fd:@var{num}
+listen on the given fd
+@item defer
+wait for the URI to be specified by the monitor (migrate_incoming)
+@item rdma:@var{addr}:@var{port}
+Listen on RDMA port on given address
+@item tcp:@var{addr}:@var{port}[,ipv4][,ipv6][,to=to]
+Listen on TCP port @var{port} (optional @var{addr} to specify address to 
listen on).
+The options ,ipv4, ipv6 and ,to are used in the same manner as chardev TCP 
options.
+@item unix:@var{path}
+listen on the UNIX socket @var{path}
+@end table
 ETEXI
 
 DEF("nodefaults", 0, QEMU_OPTION_nodefaults, \
-- 
2.1.0

Re: [Qemu-devel] [PATCH RFC v3 05/14] block: add meta bitmaps

[Vladimir Sementsov-Ogievskiy]

On 19.02.2015 02:45, John Snow wrote:



On 02/18/2015 09:00 AM, Vladimir Sementsov-Ogievskiy wrote:

Meta bitmap is a 'dirty bitmap' for the BdrvDirtyBitmap. It tracks
changes (set/unset) of this BdrvDirtyBitmap. It is needed for live
migration of block dirty bitmaps.

Signed-off-by: Vladimir Sementsov-Ogievskiy 
---
  block.c   | 40 
  include/block/block.h |  5 +
  2 files changed, 45 insertions(+)

diff --git a/block.c b/block.c
index a127fd2..aaa08b8 100644
--- a/block.c
+++ b/block.c
@@ -58,9 +58,15 @@
   * (3) successor is set: frozen mode.
   * A frozen bitmap cannot be renamed, deleted, anonymized, 
cleared, set,

   * or enabled. A frozen bitmap can only abdicate() or reclaim().
+ *
+ * Meta bitmap:
+ * Meta bitmap is a 'dirty bitmap' for the BdrvDirtyBitmap. It 
tracks changes
+ * (set/unset) of this BdrvDirtyBitmap. It is needed for live 
migration of

+ * block dirty bitmaps.
   */
  struct BdrvDirtyBitmap {
  HBitmap *bitmap;/* Dirty sector bitmap 
implementation */

+HBitmap *meta_bitmap;   /* Meta bitmap */
  BdrvDirtyBitmap *successor; /* Anonymous child; implies frozen 
status */

  char *name; /* Optional non-empty unique ID */
  int64_t size;   /* Size of the bitmap (Number of 
sectors) */
@@ -5398,6 +5404,31 @@ void 
bdrv_dirty_bitmap_make_anon(BdrvDirtyBitmap *bitmap)

  bitmap->name = NULL;
  }

+HBitmap *bdrv_create_meta_bitmap(BdrvDirtyBitmap *bitmap,
+uint64_t granularity)
+{
+uint64_t sector_granularity;
+
+assert((granularity & (granularity - 1)) == 0);
+
+granularity *= 8 * bdrv_dirty_bitmap_granularity(bitmap);
+sector_granularity = granularity >> BDRV_SECTOR_BITS;
+assert(sector_granularity);
+


The maths here could use a comment, I think.

the "granularity" field here actually describes the desired 
serialization buffer size; e.g. CHUNK_SIZE (1 << 20) or 1 MiB. This 
parameter should be renamed to explain what it's actually for. 
Something like "chunk_size" and a comment explaining that it is in bytes.


...

That said, let's talk about the default chunk size you're using in 
correlation with this function.


a CHUNK_SIZE of 1MiB here is going to lead us to, if we have a bitmap 
with the default granularity of 128 sectors\64KiB bytes, a granularity 
for the meta_bitmap of one billion sectors (1 << 30) or 512GiB.


That's going to be bigger than most drives entirely, which will 
generally lead us to only using a single "chunk" per drive. Which 
means we won't really get a lot of mileage out of the bulk/dirty 
phases most of the time.


It's wild to think about that the first 1,000,000,000 sectors or 
512,000,000,000 bytes will all be represented by the first single bit 
in this bitmap. If a single hair on the drive changes, we resend the 
_entire_ bitmap, possibly over and over again. Will we ever make 
progress? Should we investigate a smaller chunk size?


Here's some quick mappings of chunk size (bytes) to effective 
meta_bitmap byte granularities, assuming the meta_bitmap is tracking a 
bitmap with the default granularity of 64KiB:


(1 << 20) 1MiB   -- 512GiB  // This is too high of a granularity
(1 << 17) 128KiB --  64GiB
(1 << 15) 32KiB  --  16GiB
(1 << 11) 2KiB   --   1GiB
(1 << 10) 1KiB   -- 512MiB
(1 << 9)  512B   -- 256MiB
(1 << 8)  256B   -- 128MiB
(1 << 5)  32 B   --  16MiB  // This is too small of a chunk size.
(1 << 1)   1 B   --   1MiB

We want to make the chunk sends efficient, but we also want to make 
sure that the dirty phase doesn't resend more data than it needs to, 
so we need to strike a balance here, no?


I think arguments could be made for most granularities between 128MiB 
through 1GiB. Anything outside of that is too lopsided, IMO.


What are your thoughts on this?

Ok, interesting thing to discuss.

My thoughts:
* the chunk size for block-migration is 1mb, than the bitmap (64kb 
granularity) for this chunk is 16bit=2bytes long. It's an intuitive 
reason for choosing the chunk size about 2 bytes. But in this case the 
data/metadata ratio is very bad (about 20bytes for the header of the 
chunk). So, taking the nearest value with adequate ratio gives (IMHO) 
'1kb -- 512mb': 20b/1k ~ 2%. Or 512b => 4%.


* for ndb+mirror migration scheme the default chunk is 64kb instead of 
1mb. So the bitmap is more smaller. But the same reason of data/metadata 
ratio leads to 1kb chunk for dirty bitmap migration.


So, what about default to 1kb and additional parameter for migration 
(migration capabilities) to give the user a possibility of chose?


* Yes, in most of user cases the bitmap (64kb granularity) will be small 
(< 1mb). In these cases, I think, it would be better to send the data 
only in complete step, only once. (for exmaple, if pending <= 1mb, 
dosn't send anything in incremental phase).
Live migration is actually needed only for migration of bitmaps for 
dis

Re: [Qemu-devel] [RFC] net: 'Remove vhostforce option in addition to vhost param'

[Pankaj Gupta]

Hi,

Could you please help/guide me here. As suggested by Jason I did other changes 
also.
But when I did testing still virtio-net.c functions like 'receive()' gets 
called when vhost is 'ON'.

I want to know is there anything I am missing here or is this expected 
behaviour?

I was also searching for "kvm eventfd support for injecting level-triggered 
interrupts", For non-MSIX
guests, can we remove vhost-force unless we have this feature?

Best regards,
Pankaj

- Original Message -
> From: "Jason Wang" 
> To: "Pankaj Gupta" 
> Cc: "Michael S. Tsirkin" , qemu-devel@nongnu.org, 
> stefa...@redhat.com, aligu...@amazon.com
> Sent: Sunday, February 15, 2015 8:11:29 AM
> Subject: Re: [Qemu-devel] [RFC] net: 'Remove vhostforce option in 
> additionto vhost param'
> 
> 
> 
> On Thu, Feb 12, 2015 at 11:41 PM, Pankaj Gupta 
> wrote:
> >  
> >>  
> >>  > On Thu, Feb 12, 2015 at 11:50:05AM +0530, Pankaj Gupta wrote:
> >>  > >  vhostforce was added to enable use of vhost when
> >>  > >  guest don't have MSI-X support.
> >>  > >  Now, we have scenarios which  dont use interrupts
> >>  > >  like DPDK and still use vhost. Also, performance of
> >>  > >  guests without MSI-X support is getting less popular.
> >>  > > 
> >>  > >  Its ok to remove this extra option and enable vhost
> >>  > >  on the basis of vhost=ON/OFF.
> >>  > > 
> >>  > > Signed-off-by: Pankaj Gupta 
> >>  > 
> >>  > The patch doesn't seem to do what it says.
> >>  > Did you try with a non MSIX guest and vhost=on, to check that
> >>  > it actually runs vhost and not userspace virtio?
> >>  
> >>  No, I have not. I just did basic tested a new guest without
> >> vhostforce.
> >>  I will test non-MSIX guest and share the result.
> > 
> > I tested this with RHEL 4 guest which don't have MSI-X. Though vhost
> > gets
> > created but still userspace virtio-net code executes.
> > 
> > So, vhostforce was added to disable vhost for non-MSI guest?
> 
> In fact to enable vhost.
> > 
> > I took the idea from KVM/Networking todo list.
> > 
> > Do we have some other dependency before we want to remove vhostforce?
> >   
> 
> You may want to take a look at the vhost_dev->force and
> vhost_dev_query().
> > 
> >>  
> >>  > 
> >>  > > ---
> >>  > >  net/tap.c|  4 +---
> >>  > >  net/vhost-user.c | 16 ++--
> >>  > >  2 files changed, 3 insertions(+), 17 deletions(-)
> >>  > > 
> >>  > > diff --git a/net/tap.c b/net/tap.c
> >>  > > index 1fe0edf..bd2efa9 100644
> >>  > > --- a/net/tap.c
> >>  > > +++ b/net/tap.c
> >>  > > @@ -634,13 +634,11 @@ static int net_init_tap_one(const
> >> NetdevTapOptions
> >>  > > *tap, NetClientState *peer,
> >>  > >  }
> >>  > >  }
> >>  > >  
> >>  > > -if (tap->has_vhost ? tap->vhost :
> >>  > > -vhostfdname || (tap->has_vhostforce &&
> >> tap->vhostforce)) {
> >>  > > +if (tap->has_vhost ? tap->vhost : vhostfdname) {
> >>  > >  VhostNetOptions options;
> >>  > >  
> >>  > >  options.backend_type = VHOST_BACKEND_TYPE_KERNEL;
> >>  > >  options.net_backend = &s->nc;
> >>  > > -options.force = tap->has_vhostforce && tap->vhostforce;
> >>  > >  
> >>  > >  if (tap->has_vhostfd || tap->has_vhostfds) {
> >>  > >  vhostfd = monitor_handle_fd_param(cur_mon,
> >> vhostfdname);
> >>  > > diff --git a/net/vhost-user.c b/net/vhost-user.c
> >>  > > index 24e050c..d2d7bf2 100644
> >>  > > --- a/net/vhost-user.c
> >>  > > +++ b/net/vhost-user.c
> >>  > > @@ -18,7 +18,6 @@
> >>  > >  typedef struct VhostUserState {
> >>  > >  NetClientState nc;
> >>  > >  CharDriverState *chr;
> >>  > > -bool vhostforce;
> >>  > >  VHostNetState *vhost_net;
> >>  > >  } VhostUserState;
> >>  > >  
> >>  > > @@ -51,7 +50,6 @@ static int vhost_user_start(VhostUserState *s)
> >>  > >  options.backend_type = VHOST_BACKEND_TYPE_USER;
> >>  > >  options.net_backend = &s->nc;
> >>  > >  options.opaque = s->chr;
> >>  > > -options.force = s->vhostforce;
> >>  > >  
> >>  > >  s->vhost_net = vhost_net_init(&options);
> >>  > >  
> >>  > > @@ -133,8 +131,7 @@ static void net_vhost_user_event(void
> >> *opaque, int
> >>  > > event)
> >>  > >  }
> >>  > >  
> >>  > >  static int net_vhost_user_init(NetClientState *peer, const
> >> char *device,
> >>  > > -   const char *name,
> >> CharDriverState *chr,
> >>  > > -   bool vhostforce)
> >>  > > +   const char *name,
> >> CharDriverState *chr)
> >>  > >  {
> >>  > >  NetClientState *nc;
> >>  > >  VhostUserState *s;
> >>  > > @@ -149,7 +146,6 @@ static int
> >> net_vhost_user_init(NetClientState *peer,
> >>  > > const char *device,
> >>  > >  /* We don't provide a receive callback */
> >>  > >  s->nc.receive_disabled = 1;
> >>  > >  s->chr = chr;
> >>  > > -s->vhostforce = vhostforce;
> >>  > >  
> >>  > >  qemu_chr_add_handlers(s->chr, NULL, NULL,
> >> net_vhost_user_event, 

Re: [Qemu-devel] [Fwd: [PATCH v2] vpc: Implement bdrv_co_get_block_status()]

[Kevin Wolf]

Am 18.02.2015 um 22:03 hat Peter Lieven geschrieben:
> Am 18.02.2015 um 21:57 schrieb Peter Lieven:
> > This implements bdrv_co_get_block_status() for VHD images. This can
> > significantly speed up qemu-img convert operation because only with this
> > function implemented sparseness can be considered. (Before, converting a
> > 1 TB empty image took several minutes for me, now it's instantaneous.)
> >
> > Signed-off-by: Kevin Wolf 
> > ---
> >  block/vpc.c | 50 --
> >  1 file changed, 48 insertions(+), 2 deletions(-)
> >
> > diff --git a/block/vpc.c b/block/vpc.c
> > index 7fddbf0..1533b6a 100644
> > --- a/block/vpc.c
> > +++ b/block/vpc.c
> > @@ -597,6 +597,51 @@ static coroutine_fn int vpc_co_write(BlockDriverState
> > *bs, int64_t sector_num,
> >  return ret;
> >  }
> >
> > +static int64_t coroutine_fn vpc_co_get_block_status(BlockDriverState *bs,
> > +int64_t sector_num, int nb_sectors, int *pnum)
> > +{
> > +BDRVVPCState *s = bs->opaque;
> > +VHDFooter *footer = (VHDFooter*) s->footer_buf;
> > +int64_t start, offset, next;
> > +bool allocated;
> > +int n;
> > +
> > +if (be32_to_cpu(footer->type) == VHD_FIXED) {
> > +*pnum = nb_sectors;
> > +return BDRV_BLOCK_RAW | BDRV_BLOCK_OFFSET_VALID | BDRV_BLOCK_DATA |
> > +   (sector_num << BDRV_SECTOR_BITS);
> > +}
> > +
> > +offset = get_sector_offset(bs, sector_num, 0);
> > +start = offset;
> > +allocated = (offset != -1);
> > +*pnum = 0;
> > +
> > +do {
> > +/* All sectors in a block are contiguous (without using the
> > bitmap) */
> > +n = ROUND_UP(sector_num + 1, s->block_size / BDRV_SECTOR_SIZE)
> > +  - sector_num;
> > +n = MIN(n, nb_sectors);
> > +
> > +*pnum += n;
> > +sector_num += n;
> > +nb_sectors -= n;
> > +next = start + (*pnum * BDRV_SECTOR_SIZE);
> > +
> > +if (nb_sectors == 0) {
> > +break;
> > +}
> > +
> > +offset = get_sector_offset(bs, sector_num, 0);
> > +} while ((allocated && offset == next) || (!allocated && offset == 
> > -1));
> > +
> > +if (allocated) {
> > +return BDRV_BLOCK_DATA | BDRV_BLOCK_OFFSET_VALID | start;
> > +} else {
> > +return 0;
> 
> Shouldn't this be
> 
>  return BDRV_BLOCK_ZERO;
> 
> ?
> 
> vpc_read memsets all blocks with offset == -1 to 0x00.

Yes, but the blocks are still unallocated, as opposed to allocated as
zero clusters, and this is indicated by 0.

vpc_get_info() sets bdi->unallocated_blocks_are_zero = true, so we end
up with bdrv_co_get_block_status() returning BDRV_BLOCK_ZERO, but not
BDRV_BLOCK_ALLOCATED (which would be set if we had BDRV_BLOCK_ZERO
here).

I'm not sure if a wrong allocated flag would cause problem currently,
but it's definitely necessary to get right once we add support for
differencing images (patches are on the list, pending review).

> Not for this patch, but couldn't we use your new function to signifincantly 
> speed up
> reading of continous allocated areas in vpc_read?

There aren't really contiguous blocks in VHD, you always have a bitmap
in between. In some cases it might be better to read the bitmap as well
as the two adjacent blocks and throw that buffer away in order to save
one read request, but with relatively large block sizes of VHD it's
probably not going to help that much.

It's also a question of whether we want to invest significant effort
into making vpc efficient enough for reasonably running a VM from it.
Our current assumption is that the support is mostly there for qemu-img
convert.

Kevin

[Qemu-devel] [PATCH] target-arm: modifying pc in tcg code for load/store multiple

[ild]

From: Ildar Isaev 

pc wasn't modified in tcg code for load/store multiple,
causing translation block to be executed in infinite loop forever

Signed-off-by: Ildar Isaev 
---
 target-arm/translate.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/target-arm/translate.c b/target-arm/translate.c
index 36868ed..622aa03 100644
--- a/target-arm/translate.c
+++ b/target-arm/translate.c
@@ -8973,7 +8973,7 @@ static void disas_arm_insn(DisasContext *s, unsigned int 
insn)
 tmp = load_cpu_field(spsr);
 gen_set_cpsr(tmp, CPSR_ERET_MASK);
 tcg_temp_free_i32(tmp);
-s->is_jmp = DISAS_UPDATE;
+gen_lookup_tb(s);
 }
 }
 break;
-- 
1.9.3

Re: [Qemu-devel] bus_unparent() can go into infinite loop

[Andreas Färber]

Am 19.02.2015 um 11:45 schrieb Markus Armbruster:
> Reproducer (don't ask):
> 
> $ qemu-system-arm -M virt -S -display none -drive 
> if=scsi,id=foo,bus=1,file=tmp.qcow2 -device nec-usb-xhci -device 
> usb-storage,drive=foo -device virtio-scsi-pci
> qemu-system-arm: -drive if=scsi,id=foo,bus=1,file=tmp.qcow2: Property 
> 'scsi-disk.drive' can't take value 'foo', it's in use
> qemu-system-arm: -drive if=scsi,id=foo,bus=1,file=tmp.qcow2: Setting 
> drive property failed
> qemu-system-arm: -device virtio-scsi-pci: Setting drive property failed
> 
> Nevermind the silly error reporting, I got patches to clean that up.

I'm actually more confused about the use of PCI devices with the virt
machine. Does that now feature Alex' PCI controller by default?
Otherwise there would be no bus for those devices.

Is this on master or on top of your PCI realize changes or anything?

> Stuck in bus_unparent()'s loop:
> 
> while ((kid = QTAILQ_FIRST(&bus->children)) != NULL) {
> DeviceState *dev = kid->child;
> object_unparent(OBJECT(dev));
> }

Logically speaking, unparenting on QOM level has nothing to with the bus
children list.
The parent may well be /machine/{unassigned,peripheral{,-anon}}
container objects rather than some BusState object, the latter usually
has link<> properties only for its qdev-level "children".

However I vaguely recall that we shoehorned the unparenting callback to
invoke unrealizing the device. What might happen here is that after
realizing the device failed, realized == false; realized = false in the
unparenting path becomes a no-op then. I.e., the realize error handling
may need to be reviewed to not just return but to undo any changes such
as attaching to some bus (or MemoryRegion, VMState, etc.).

Regards,
Andreas

> (gdb) bt
> #0  bus_unparent (obj=)
> at /home/armbru/work/qemu/hw/core/qdev.c:727
> #1  0x5583a165 in object_finalize_child_property (obj= out>, 
> name=, opaque=0x56450060)
> at /home/armbru/work/qemu/qom/object.c:1079
> #2  0x5583a40c in object_property_del (obj=0x5644ff50, 
> name=0x56696450 "scsi.1", errp=)
> at /home/armbru/work/qemu/qom/object.c:800
> #3  0x5577f759 in device_unparent (obj=0x5644ff50)
> at /home/armbru/work/qemu/hw/core/qdev.c:1230
> #4  0x5583a165 in object_finalize_child_property (obj= out>, 
> name=, opaque=0x5644ff50)
> at /home/armbru/work/qemu/qom/object.c:1079
> #5  0x5583a40c in object_property_del (obj=0x5644f540, 
> name=0x566b9620 "virtio-backend", errp=)
> at /home/armbru/work/qemu/qom/object.c:800
> #6  0x557800c6 in qdev_init (dev=dev@entry=0x5644ff50)
> at /home/armbru/work/qemu/hw/core/qdev.c:186
> #7  0x5582440e in virtio_scsi_pci_init_pci (vpci_dev=0x5644f540)
> at /home/armbru/work/qemu/hw/virtio/virtio-pci.c:1157
> #8  0x55825fc8 in virtio_pci_init (pci_dev=)
> at /home/armbru/work/qemu/hw/virtio/virtio-pci.c:1018
> #9  0x557d6df7 in pci_qdev_init (qdev=0x5644f540)
> at /home/armbru/work/qemu/hw/pci/pci.c:1775
> #10 0x5577fbc4 in device_realize (dev=0x5644f540, 
> errp=0x7fffda60) at /home/armbru/work/qemu/hw/core/qdev.c:247
> #11 0x5578125d in device_set_realized (obj=0x5644f540, 
> value=, errp=0x7fffdb98)
> at /home/armbru/work/qemu/hw/core/qdev.c:1040
> #12 0x5583927e in property_set_bool (obj=0x5644f540, 
> v=, opaque=0x56698850, name=, 
> errp=0x7fffdb98) at /home/armbru/work/qemu/qom/object.c:1514
> #13 0x5583bc67 in object_property_set_qobject (obj=0x5644f540, 
> value=, name=0x559482fd "realized", 
> errp=0x7fffdb98)
> at /home/armbru/work/qemu/qom/qom-qobject.c:24
> #14 0x5583a7b0 in object_property_set_bool (
> obj=obj@entry=0x5644f540, value=value@entry=true, 
> name=name@entry=0x559482fd "realized", errp=errp@entry=0x7fffdb98)
> at /home/armbru/work/qemu/qom/object.c:905
> #15 0x5570d774 in qdev_device_add (opts=0x562a38c0)
> at /home/armbru/work/qemu/qdev-monitor.c:574
> #16 0x55716c79 in device_init_func (opts=, 
> opaque=) at /home/armbru/work/qemu/vl.c:2127
> #17 0x558f48eb in qemu_opts_foreach (list=, func=
> 0x55716c70 , opaque=0x0, 
> abort_on_failure=)
> at /home/armbru/work/qemu/util/qemu-option.c:1057
> #18 0x5560e39d in main (argc=, argv=, 
> envp=) at /home/armbru/work/qemu/vl.c:4244
> (gdb) p dev->parent_obj.class->type->name
> $5 = 0x5626c3b0 "scsi-disk"
> (gdb) p bus->name
> $8 = 0x56696430 "scsi.1"

-- 
SUSE Linux GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Felix Imendörffer, Jane Smithard, Jennifer Guild, Dilip Upmanyu,
Graham Norton; HRB 21284 (AG Nürnberg)

[Qemu-devel] [PATCH] target-arm: always use user mode registers as operands for load/store multiple

[Ildar Isaev]

Pseudocode fragment for STM instruction in ARMv8 spec:

if registers == '1' then // Store User mode register
MemA[address,4] = Rmode[i, M32_User];

Signed-off-by: Ildar Isaev 
---
 target-arm/translate.c | 14 +-
 1 file changed, 5 insertions(+), 9 deletions(-)

diff --git a/target-arm/translate.c b/target-arm/translate.c
index 622aa03..ca0ce3f 100644
--- a/target-arm/translate.c
+++ b/target-arm/translate.c
@@ -8907,16 +8907,14 @@ static void disas_arm_insn(DisasContext *s, unsigned 
int insn)
 /* load */
 tmp = tcg_temp_new_i32();
 gen_aa32_ld32u(tmp, addr, get_mem_index(s));
-if (user) {
+if (!user && (i == rn)) {
+loaded_var = tmp;
+loaded_base = 1;
+} else {
 tmp2 = tcg_const_i32(i);
 gen_helper_set_user_reg(cpu_env, tmp2, tmp);
 tcg_temp_free_i32(tmp2);
 tcg_temp_free_i32(tmp);
-} else if (i == rn) {
-loaded_var = tmp;
-loaded_base = 1;
-} else {
-store_reg_from_load(s, i, tmp);
 }
 } else {
 /* store */
@@ -8925,13 +8923,11 @@ static void disas_arm_insn(DisasContext *s, unsigned 
int insn)
 val = (long)s->pc + 4;
 tmp = tcg_temp_new_i32();
 tcg_gen_movi_i32(tmp, val);
-} else if (user) {
+} else {
 tmp = tcg_temp_new_i32();
 tmp2 = tcg_const_i32(i);
 gen_helper_get_user_reg(tmp, cpu_env, tmp2);
 tcg_temp_free_i32(tmp2);
-} else {
-tmp = load_reg(s, i);
 }
 gen_aa32_st32(tmp, addr, get_mem_index(s));
 tcg_temp_free_i32(tmp);
-- 
1.9.3

Re: [Qemu-devel] bus_unparent() can go into infinite loop

[Markus Armbruster]

Andreas Färber  writes:

> Am 19.02.2015 um 11:45 schrieb Markus Armbruster:
>> Reproducer (don't ask):
>> 
>> $ qemu-system-arm -M virt -S -display none -drive
>> if=scsi,id=foo,bus=1,file=tmp.qcow2 -device nec-usb-xhci -device
>> usb-storage,drive=foo -device virtio-scsi-pci
>> qemu-system-arm: -drive if=scsi,id=foo,bus=1,file=tmp.qcow2:
>> Property 'scsi-disk.drive' can't take value 'foo', it's in use
>> qemu-system-arm: -drive if=scsi,id=foo,bus=1,file=tmp.qcow2:
>> Setting drive property failed
>> qemu-system-arm: -device virtio-scsi-pci: Setting drive property failed
>> 
>> Nevermind the silly error reporting, I got patches to clean that up.
>
> I'm actually more confused about the use of PCI devices with the virt
> machine. Does that now feature Alex' PCI controller by default?
> Otherwise there would be no bus for those devices.

"info qtree" shows a PCIE bus:

  dev: gpex-pcihost, id ""
gpio-out "sysbus-irq" 4
mmio /1000
mmio /
mmio 3eff/0001
bus: pcie.0
  type PCIE
  dev: gpex-root, id ""
addr = 00.0
romfile = ""
rombar = 1 (0x1)
multifunction = false
command_serr_enable = true
class Host bridge, addr 00:00.0, pci id 1b36:0008 (sub 1af4:1100)

> Is this on master or on top of your PCI realize changes or anything?

Unadulterated master (commit cd2d554).

>> Stuck in bus_unparent()'s loop:
>> 
>> while ((kid = QTAILQ_FIRST(&bus->children)) != NULL) {
>> DeviceState *dev = kid->child;
>> object_unparent(OBJECT(dev));
>> }
>
> Logically speaking, unparenting on QOM level has nothing to with the bus
> children list.
> The parent may well be /machine/{unassigned,peripheral{,-anon}}
> container objects rather than some BusState object, the latter usually
> has link<> properties only for its qdev-level "children".
>
> However I vaguely recall that we shoehorned the unparenting callback to
> invoke unrealizing the device. What might happen here is that after
> realizing the device failed, realized == false; realized = false in the
> unparenting path becomes a no-op then. I.e., the realize error handling
> may need to be reviewed to not just return but to undo any changes such
> as attaching to some bus (or MemoryRegion, VMState, etc.).

(gdb) p *dev
$2 = {parent_obj = {class = 0x56282140, free = 0x764dcf70 , 
properties = {tqh_first = 0x5669d860, tqh_last = 0x566a1d90}, 
ref = 1, parent = 0x0}, id = 0x0, realized = false, 
  pending_deleted_event = false, opts = 0x0, hotplugged = 0, 
  parent_bus = 0x56450060, gpios = {lh_first = 0x0}, child_bus = {
lh_first = 0x0}, num_child_bus = 0, instance_id_alias = -1, 
  alias_required_for_version = 0}

This is right before object_unparent().

Re: [Qemu-devel] [PATCH] target-arm: always use user mode registers as operands for load/store multiple

[Peter Maydell]

On 19 February 2015 at 21:55, Ildar Isaev  wrote:
> Pseudocode fragment for STM instruction in ARMv8 spec:
>
> if registers == '1' then // Store User mode register
> MemA[address,4] = Rmode[i, M32_User];

Again, do you have an example of the code that we mishandle
and what we get wrong?

thanks
-- PMM

Re: [Qemu-devel] [PATCH] target-arm: modifying pc in tcg code for load/store multiple

[Peter Maydell]

On 19 February 2015 at 21:26,   wrote:
> From: Ildar Isaev 
>
> pc wasn't modified in tcg code for load/store multiple,
> causing translation block to be executed in infinite loop forever
>
> Signed-off-by: Ildar Isaev 

It would be helpful if you gave an example of guest
code which we mishandle. Do you have a test case?

> ---
>  target-arm/translate.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/target-arm/translate.c b/target-arm/translate.c
> index 36868ed..622aa03 100644
> --- a/target-arm/translate.c
> +++ b/target-arm/translate.c
> @@ -8973,7 +8973,7 @@ static void disas_arm_insn(DisasContext *s, unsigned 
> int insn)
>  tmp = load_cpu_field(spsr);
>  gen_set_cpsr(tmp, CPSR_ERET_MASK);
>  tcg_temp_free_i32(tmp);
> -s->is_jmp = DISAS_UPDATE;
> +gen_lookup_tb(s);
>  }
>  }
>  break;

This doesn't look right. What if the load-multiple loaded PC?
Calling gen_lookup_tb() will overwrite that.

-- PMM

Re: [Qemu-devel] [PATCH RFC v3 08/14] migration: add migration/block-dirty-bitmap.c

[Vladimir Sementsov-Ogievskiy]

On 19.02.2015 02:47, John Snow wrote:



On 02/18/2015 09:00 AM, Vladimir Sementsov-Ogievskiy wrote:
Live migration of dirty bitmaps. Only named dirty bitmaps, associated 
with

root nodes and non-root named nodes are migrated.

If destination qemu is already containing a dirty bitmap with the 
same name
as a migrated bitmap (for the same node), than, if their 
granularities are
the same the migration will be done, otherwise the error will be 
generated.


If destination qemu doesn't contain such bitmap it will be created.

Signed-off-by: Vladimir Sementsov-Ogievskiy 
---
  include/migration/block.h  |   1 +
  migration/Makefile.objs|   2 +-
  migration/block-dirty-bitmap.c | 708 
+

  vl.c   |   1 +
  4 files changed, 711 insertions(+), 1 deletion(-)
  create mode 100644 migration/block-dirty-bitmap.c

diff --git a/include/migration/block.h b/include/migration/block.h
index ffa8ac0..566bb9f 100644
--- a/include/migration/block.h
+++ b/include/migration/block.h
@@ -14,6 +14,7 @@
  #ifndef BLOCK_MIGRATION_H
  #define BLOCK_MIGRATION_H

+void dirty_bitmap_mig_init(void);
  void blk_mig_init(void);
  int blk_mig_active(void);
  uint64_t blk_mig_bytes_transferred(void);
diff --git a/migration/Makefile.objs b/migration/Makefile.objs
index d929e96..128612d 100644
--- a/migration/Makefile.objs
+++ b/migration/Makefile.objs
@@ -6,5 +6,5 @@ common-obj-y += xbzrle.o
  common-obj-$(CONFIG_RDMA) += rdma.o
  common-obj-$(CONFIG_POSIX) += exec.o unix.o fd.o

-common-obj-y += block.o
+common-obj-y += block.o block-dirty-bitmap.o

diff --git a/migration/block-dirty-bitmap.c 
b/migration/block-dirty-bitmap.c

new file mode 100644
index 000..084ba22
--- /dev/null
+++ b/migration/block-dirty-bitmap.c
@@ -0,0 +1,708 @@
+/*
+ * QEMU dirty bitmap migration
+ *
+ * Live migration of dirty bitmaps. Only named dirty bitmaps, 
associated with

+ * root nodes and non-root named nodes are migrated.
+ *
+ * If destination qemu is already containing a dirty bitmap with the 
same name
+ * as a migrated bitmap (for the same node), than, if their 
granularities are
+ * the same the migration will be done, otherwise the error will be 
generated.

+ *
+ * If destination qemu doesn't contain such bitmap it will be created.
+ *
+ * format of migration:
+ *
+ * # Header (shared for different chunk types)
+ * 1 byte: flags


1, 2, or 4 bytes.


+ * [ 1 byte: node name size ] \  flags & DEVICE_NAME
+ * [ n bytes: node name ] /
+ * [ 1 byte: bitmap name size ] \  flags & BITMAP_NAME
+ * [ n bytes: bitmap name ] /
+ *
+ * # Start of bitmap migration (flags & START)
+ * header
+ * be64: granularity
+ *
+ * # Complete of bitmap migration (flags & COMPLETE)
+ * header
+ * 1 byte: bitmap enabled flag
+ *
+ * # Data chunk of bitmap migration
+ * header
+ * be64: start sector
+ * be32: number of sectors
+ * [ be64: buffer size  ] \ ! (flags & ZEROES)
+ * [ n bytes: buffer] /
+ *
+ * The last chunk in stream should contain flags & EOS. The chunk 
may skip
+ * device and/or bitmap names, assuming them to be the same with the 
previous

+ * chunk.
+ *
+ *
+ * This file is derived from migration/block.c
+ *
+ * Author:
+ * Vladimir Sementsov-Ogievskiy 
+ *
+ * original copyright message:
+ * 
=

+ * Copyright IBM, Corp. 2009
+ *
+ * Authors:
+ *  Liran Schour   
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2.  
See

+ * the COPYING file in the top-level directory.
+ *
+ * Contributions after 2012-01-13 are licensed under the terms of the
+ * GNU GPL, version 2 or (at your option) any later version.
+ * 
=

+ */
+
+#include "block/block.h"
+#include "block/block_int.h"
+#include "sysemu/block-backend.h"
+#include "qemu/main-loop.h"
+#include "qemu/error-report.h"
+#include "migration/block.h"
+#include "migration/migration.h"
+#include "qemu/hbitmap.h"
+#include 
+
+#define CHUNK_SIZE   (1 << 20)
+
+/* Flags occupy from one to four bytes. In all but one the 7-th 
(EXTRA_FLAGS)

+ * bit should be set. */
+#define DIRTY_BITMAP_MIG_FLAG_EOS   0x01
+#define DIRTY_BITMAP_MIG_FLAG_ZEROES0x02
+#define DIRTY_BITMAP_MIG_FLAG_BITMAP_NAME   0x04
+#define DIRTY_BITMAP_MIG_FLAG_DEVICE_NAME   0x08
+#define DIRTY_BITMAP_MIG_FLAG_START 0x10
+#define DIRTY_BITMAP_MIG_FLAG_COMPLETE  0x20
+#define DIRTY_BITMAP_MIG_FLAG_BITS  0x40
+
+#define DIRTY_BITMAP_MIG_EXTRA_FLAGS0x80
+#define DIRTY_BITMAP_MIG_FLAGS_SIZE_16  0x8000
+#define DIRTY_BITMAP_MIG_FLAGS_SIZE_32  0x8080
+
+#define DEBUG_DIRTY_BITMAP_MIGRATION
+
+#ifdef DEBUG_DIRTY_BITMAP_MIGRATION
+#define DPRINTF(fmt, ...) \
+do { printf("dirty_migration: " fmt, ## __VA_ARGS__); } while (0)
+#else
+#define DPRINTF(fmt, ...) \
+do { } while (0)
+#endif
+


Take a look at hw/ide/ahci.c, which has a DPRI

[Qemu-devel] [PATCH] pc: acpi: get rid of IASL warnings in MCRS method

[Igor Mammedov]

Newer IASL produces a bunch of false warnings like:
,, MW32, AddressRangeMemory, TypeStatic)
Object is not referenced ^  (Name is within method [MCRS])

Caused by local integer constant name DescriptorName
in DWordMemory resource macro, it appears that compiler creates
a name for each of the fields of DWordMemory resource macro
if optional DescriptorName is provided and the complains
about not used ones.

So silence compiler by getting rid of DescriptorName in
MW[64|23] resource macros and use numeric field offsets
for referencing fields in MW32 buffer as it's already done
for MW64.

Signed-off-by: Igor Mammedov 
---
 hw/i386/acpi-dsdt-mem-hotplug.dsl | 10 +-
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/hw/i386/acpi-dsdt-mem-hotplug.dsl 
b/hw/i386/acpi-dsdt-mem-hotplug.dsl
index 2a36c47..4d693e3 100644
--- a/hw/i386/acpi-dsdt-mem-hotplug.dsl
+++ b/hw/i386/acpi-dsdt-mem-hotplug.dsl
@@ -105,7 +105,7 @@
 0xFFFE,// Address Range Maximum
 0x,// Address Translation Offset
 0x,// Address Length
-,, MW64, AddressRangeMemory, TypeStatic)
+,,, AddressRangeMemory, TypeStatic)
 })
 
 CreateDWordField(MR64, 14, MINL)
@@ -140,11 +140,11 @@
 0xFFFE,// Address Range Maximum
 0x,// Address Translation Offset
 0x,// Address Length
-,, MW32, AddressRangeMemory, TypeStatic)
+,,, AddressRangeMemory, TypeStatic)
 })
-CreateDWordField(MR32, MW32._MIN, MIN)
-CreateDWordField(MR32, MW32._MAX, MAX)
-CreateDWordField(MR32, MW32._LEN, LEN)
+CreateDWordField(MR32, 10, MIN)
+CreateDWordField(MR32, 14, MAX)
+CreateDWordField(MR32, 22, LEN)
 Store(MINL, MIN)
 Store(MAXL, MAX)
 Store(LENL, LEN)
-- 
1.8.3.1

Re: [Qemu-devel] [PATCH v7 11/25] qcow2: refcount_order parameter for qcow2_create2

[Max Reitz]

On 2015-02-18 at 21:51, Eric Blake wrote:

On 02/18/2015 03:40 PM, Max Reitz wrote:

Add a refcount_order parameter to qcow2_create2(), use that value for
the image header and for calculating the size required for
preallocation.

For now, always pass 4.

This addition requires changes to the calculation of the file size for
the "full" and "falloc" preallocation modes. That in turn is a nice
opportunity to add a comment about that calculation not necessarily
being exact (and that being intentional).

Signed-off-by: Max Reitz 
---
  block/qcow2.c | 47 ---
  1 file changed, 36 insertions(+), 11 deletions(-)

@@ -2010,6 +2022,8 @@ static int qcow2_create(const char *filename, QemuOpts 
*opts, Error **errp)
  size_t cluster_size = DEFAULT_CLUSTER_SIZE;
  PreallocMode prealloc;
  int version = 3;
+uint64_t refcount_bits = 16;
+int refcount_order;
  Error *local_err = NULL;
  int ret;
  
@@ -2064,8 +2078,19 @@ static int qcow2_create(const char *filename, QemuOpts *opts, Error **errp)

  goto finish;
  }
  
+if (version < 3 && refcount_bits != 16) {

+error_setg(errp, "Different refcount widths than 16 bits require "
+   "compatibility level 1.1 or above (use compat=1.1 or "
+   "greater)");
+ret = -EINVAL;
+goto finish;
+}
+
+refcount_order = ffs(refcount_bits) - 1;

ffs() doesn't work on uint64_t (it gives the wrong answer for
0x1, for example); you want to use ffsll().  But ffsll() isn't
portable.  But we have include/qemu/host-utils.h that gives us ctz64()
which is what we want (where ctz==ffs-1 other than for the special case
of 0).  So this should be:

refcount_order = ctz64(refcount_bits);

With that change,
Reviewed-by: Eric Blake 


I intentionally left the ffs() here, because after this patch, 
refcount_bits is guaranteed to be 16, and after patch 14, it's 
guaranteed to be 64 or less (it's more obvious after patch 14 than 
here). So I would be fine with ctz64(), but in my opinion, ffs() is just 
fine, too.


Max


(Hmm, that means that at least qcow2.c:qcow2_create2() has a bug for
calling ffs(size_t), and we probably ought to eradicate other uses of
ffs from the tree - but as a separate followup).

Re: [Qemu-devel] [PATCH 2/3] coroutine: Clean up qemu_coroutine_enter()

[Stefan Hajnoczi]

On Wed, Feb 18, 2015 at 03:20:26PM +0100, Kevin Wolf wrote:
> Am 18.02.2015 um 14:50 hat Stefan Hajnoczi geschrieben:
> > On Tue, Feb 10, 2015 at 11:41:27AM +0100, Kevin Wolf wrote:
> > > qemu_coroutine_enter() is now the only user of coroutine_swap(). Both
> > > functions are short, so inline it.
> > > 
> > > Also, using COROUTINE_YIELD is now even more confusing because this code
> > > is never called during qemu_coroutine_yield() any more. In fact, this
> > > value is never read back, so we can just introduce a new COROUTINE_ENTER
> > > which documents the purpose of the task switch better.
> > > 
> > > Signed-off-by: Kevin Wolf 
> > > ---
> > >  include/block/coroutine_int.h |  1 +
> > >  qemu-coroutine.c  | 36 +++-
> > >  2 files changed, 16 insertions(+), 21 deletions(-)
> > > 
> > > diff --git a/include/block/coroutine_int.h b/include/block/coroutine_int.h
> > > index f133d65..69b83db 100644
> > > --- a/include/block/coroutine_int.h
> > > +++ b/include/block/coroutine_int.h
> > > @@ -29,6 +29,7 @@
> > >  #include "block/coroutine.h"
> > >  
> > >  typedef enum {
> > > +COROUTINE_ENTER = 0,
> > 
> > This makes the ucontext code harder to understand because
> > CoroutineAction values are used with setjmp()/longjmp() in
> > qemu_coroutine_switch().
> > 
> > The longjmp() man page says:
> > 
> >   If longjmp() is invoked with a second argument of 0, 1 will be
> >   returned instead.
> > 
> > I haven't checked whether or not this causes problems, but the code
> > would be simpler if we avoided using 0.
> 
> It doesn't, the value is unused where we pass COROUTINE_ENTER. But I can
> make it 3 instead.

Thanks, that would be good.

Stefan


pgpbtvYyKdqlR.pgp
Description: PGP signature

Re: [Qemu-devel] [RFC] net: 'Remove vhostforce option in addition to vhost param'

[Michael S. Tsirkin]

On Thu, Feb 19, 2015 at 07:02:43AM -0500, Pankaj Gupta wrote:
> Hi,
> 
> Could you please help/guide me here. As suggested by Jason I did other 
> changes also.
> But when I did testing still virtio-net.c functions like 'receive()' gets 
> called when vhost is 'ON'.
> 
> I want to know is there anything I am missing here or is this expected 
> behaviour?

It's a known bug.

> I was also searching for "kvm eventfd support for injecting level-triggered 
> interrupts", For non-MSIX
> guests, can we remove vhost-force unless we have this feature?
> 
> Best regards,
> Pankaj

This feature is unused by vhost ATM.


> > From: "Jason Wang" 
> > To: "Pankaj Gupta" 
> > Cc: "Michael S. Tsirkin" , qemu-devel@nongnu.org, 
> > stefa...@redhat.com, aligu...@amazon.com
> > Sent: Sunday, February 15, 2015 8:11:29 AM
> > Subject: Re: [Qemu-devel] [RFC] net: 'Remove vhostforce option in   
> > additionto vhost param'
> > 
> > 
> > 
> > On Thu, Feb 12, 2015 at 11:41 PM, Pankaj Gupta 
> > wrote:
> > >  
> > >>  
> > >>  > On Thu, Feb 12, 2015 at 11:50:05AM +0530, Pankaj Gupta wrote:
> > >>  > >  vhostforce was added to enable use of vhost when
> > >>  > >  guest don't have MSI-X support.
> > >>  > >  Now, we have scenarios which  dont use interrupts
> > >>  > >  like DPDK and still use vhost. Also, performance of
> > >>  > >  guests without MSI-X support is getting less popular.
> > >>  > > 
> > >>  > >  Its ok to remove this extra option and enable vhost
> > >>  > >  on the basis of vhost=ON/OFF.
> > >>  > > 
> > >>  > > Signed-off-by: Pankaj Gupta 
> > >>  > 
> > >>  > The patch doesn't seem to do what it says.
> > >>  > Did you try with a non MSIX guest and vhost=on, to check that
> > >>  > it actually runs vhost and not userspace virtio?
> > >>  
> > >>  No, I have not. I just did basic tested a new guest without
> > >> vhostforce.
> > >>  I will test non-MSIX guest and share the result.
> > > 
> > > I tested this with RHEL 4 guest which don't have MSI-X. Though vhost
> > > gets
> > > created but still userspace virtio-net code executes.
> > > 
> > > So, vhostforce was added to disable vhost for non-MSI guest?
> > 
> > In fact to enable vhost.
> > > 
> > > I took the idea from KVM/Networking todo list.
> > > 
> > > Do we have some other dependency before we want to remove vhostforce?
> > >   
> > 
> > You may want to take a look at the vhost_dev->force and
> > vhost_dev_query().
> > > 
> > >>  
> > >>  > 
> > >>  > > ---
> > >>  > >  net/tap.c|  4 +---
> > >>  > >  net/vhost-user.c | 16 ++--
> > >>  > >  2 files changed, 3 insertions(+), 17 deletions(-)
> > >>  > > 
> > >>  > > diff --git a/net/tap.c b/net/tap.c
> > >>  > > index 1fe0edf..bd2efa9 100644
> > >>  > > --- a/net/tap.c
> > >>  > > +++ b/net/tap.c
> > >>  > > @@ -634,13 +634,11 @@ static int net_init_tap_one(const
> > >> NetdevTapOptions
> > >>  > > *tap, NetClientState *peer,
> > >>  > >  }
> > >>  > >  }
> > >>  > >  
> > >>  > > -if (tap->has_vhost ? tap->vhost :
> > >>  > > -vhostfdname || (tap->has_vhostforce &&
> > >> tap->vhostforce)) {
> > >>  > > +if (tap->has_vhost ? tap->vhost : vhostfdname) {
> > >>  > >  VhostNetOptions options;
> > >>  > >  
> > >>  > >  options.backend_type = VHOST_BACKEND_TYPE_KERNEL;
> > >>  > >  options.net_backend = &s->nc;
> > >>  > > -options.force = tap->has_vhostforce && tap->vhostforce;
> > >>  > >  
> > >>  > >  if (tap->has_vhostfd || tap->has_vhostfds) {
> > >>  > >  vhostfd = monitor_handle_fd_param(cur_mon,
> > >> vhostfdname);
> > >>  > > diff --git a/net/vhost-user.c b/net/vhost-user.c
> > >>  > > index 24e050c..d2d7bf2 100644
> > >>  > > --- a/net/vhost-user.c
> > >>  > > +++ b/net/vhost-user.c
> > >>  > > @@ -18,7 +18,6 @@
> > >>  > >  typedef struct VhostUserState {
> > >>  > >  NetClientState nc;
> > >>  > >  CharDriverState *chr;
> > >>  > > -bool vhostforce;
> > >>  > >  VHostNetState *vhost_net;
> > >>  > >  } VhostUserState;
> > >>  > >  
> > >>  > > @@ -51,7 +50,6 @@ static int vhost_user_start(VhostUserState *s)
> > >>  > >  options.backend_type = VHOST_BACKEND_TYPE_USER;
> > >>  > >  options.net_backend = &s->nc;
> > >>  > >  options.opaque = s->chr;
> > >>  > > -options.force = s->vhostforce;
> > >>  > >  
> > >>  > >  s->vhost_net = vhost_net_init(&options);
> > >>  > >  
> > >>  > > @@ -133,8 +131,7 @@ static void net_vhost_user_event(void
> > >> *opaque, int
> > >>  > > event)
> > >>  > >  }
> > >>  > >  
> > >>  > >  static int net_vhost_user_init(NetClientState *peer, const
> > >> char *device,
> > >>  > > -   const char *name,
> > >> CharDriverState *chr,
> > >>  > > -   bool vhostforce)
> > >>  > > +   const char *name,
> > >> CharDriverState *chr)
> > >>  > >  {
> > >>  > >  NetClientState *nc;
> > >>  > >  VhostUserState *s;
> > >>  > > @@ -149,7 +146,6 @@ stati

[Qemu-devel] [Bug 1423528] [NEW] setting unsupported timeout for i6300esb watchdog causes hw reset

[Michael Biebl]

Public bug reported:

Bug-Debian: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=778291
Version: 2.1

systemd utilizes existing watchdog hardware and set's a 10min timer on reboot.
The i6300esb under qemu doesn't like such a timeout, and immediately resets the 
hardware:

The last message one gets is
[9.402243] i6300esb: Unexpected close, not stopping watchdog!


The linked bug report contains information how this bug can easily be 
reproduced.
With any image using a recent enough systemd as PID 1 you should be able to 
reproduce it by running

qemu-system-x86_64 -curses -enable-kvm -device i6300esb -watchdog-action
reset -hda 


I'm uncertain if this is a qemu or kernel/driver bug. If the latter, please 
re-assign the bug as necessary.

** 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/1423528

Title:
   setting unsupported timeout for i6300esb watchdog causes hw reset

Status in QEMU:
  New

Bug description:
  Bug-Debian: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=778291
  Version: 2.1

  systemd utilizes existing watchdog hardware and set's a 10min timer on reboot.
  The i6300esb under qemu doesn't like such a timeout, and immediately resets 
the hardware:

  The last message one gets is
  [9.402243] i6300esb: Unexpected close, not stopping watchdog!

  
  The linked bug report contains information how this bug can easily be 
reproduced.
  With any image using a recent enough systemd as PID 1 you should be able to 
reproduce it by running

  qemu-system-x86_64 -curses -enable-kvm -device i6300esb -watchdog-
  action reset -hda 

  
  I'm uncertain if this is a qemu or kernel/driver bug. If the latter, please 
re-assign the bug as necessary.

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

Re: [Qemu-devel] [PATCH 1/2] qemu-img convert: Rewrite copying logic

[Kevin Wolf]

Am 18.02.2015 um 18:18 hat Max Reitz geschrieben:
> On 2015-02-18 at 10:19, Kevin Wolf wrote:
> >The implementation of qemu-img convert is (a) messy, (b) buggy, and
> >(c) less efficient than possible. The changes required to beat some
> >sense into it are massive enough that incremental changes would only
> >make my and the reviewers' life harder. So throw it away and reimplement
> >it from scratch.
> >
> >Let me give some examples what I mean by messy, buggy and inefficient:
> >
> >(a) The copying logic of qemu-img convert has two separate branches for
> > compressed and normal target images, which roughly do the same -
> > except for a little code that handles actual differences between
> > compressed and uncompressed images, and much more code that
> > implements just a different set of optimisations and bugs. This is
> > unnecessary code duplication, and makes the code for compressed
> > output (unsurprisingly) suffer from bitrot.
> >
> > The code for uncompressed ouput is run twice to count the the total
> > length for the progress bar. In the first run it just takes a
> > shortcut and runs only half the loop, and when it's done, it toggles
> > a boolean, jumps out of the loop with a backwards goto and starts
> > over. Works, but pretty is something different.
> >
> >(b) Converting while keeping a backing file (-B option) is broken in
> > several ways. This includes not writing to the image file if the
> > input has zero clusters or data filled with zeros (ignoring that the
> > backing file will be visible instead).
> >
> > It also doesn't correctly limit every iteration of the copy loop to
> > sectors of the same status so that too many sectors may be copied to
> > in the target image. For -B this gives an unexpected result, for
> > other images it just does more work than necessary.
> >
> > Conversion with a compressed target completely ignores any target
> > backing file.
> >
> >(c) qemu-img convert skips reading and writing an area if it knows from
> > metadata that copying isn't needed (except for the bug mentioned
> > above that ignores a status change in some cases). It does, however,
> > read from the source even if it knows that it will read zeros, and
> > then search for non-zero bytes in the read buffer, if it's possible
> > that a write might be needed.
> >
> >This reimplementation of the copying core reorganises the code to remove
> >the duplication and have a much more obvious code flow, by essentially
> >splitting the copy iteration loop into three parts:
> >
> >1. Find the number of contiguous sectors of the same status at the
> >current offset (This can also be called in a separate loop for the
> >copying loop in order to determine the total sectors for the progress
> >bar.)
> >
> >2. Read sectors. If the status implies that there is no data there to
> >read (zero or unallocated cluster), don't do anything.
> >
> >3. Write sectors depending on the status. If it's data, write it. If
> >we want the backing file to be visible (with -B), don't write it. If
> >it's zeroed, skip it if you can, otherwise use bdrv_write_zeroes() to
> >optimise the write at least where possible.
> >
> >Signed-off-by: Kevin Wolf 
> >---
> >  qemu-img.c | 511 
> > -
> >  1 file changed, 305 insertions(+), 206 deletions(-)
> 
> You have been very careful not to write "Rewrite img_convert()" or
> something like that in the subject, so I can't really complain that
> there are still bugs in the function (which are not related to the
> copying logic), for instance:
> 
> $ ./qemu-img create -f qcow2 i1.qcow2 64M
> Formatting 'i1.qcow2', fmt=qcow2 size=67108864 encryption=off
> cluster_size=65536 lazy_refcounts=off
> $ ./qemu-img create -f qcow2 i2.qcow2 64M
> Formatting 'i2.qcow2', fmt=qcow2 size=67108864 encryption=off
> cluster_size=65536 lazy_refcounts=off
> $ ./qemu-img snapshot -c foo i1.qcow2
> $ ./qemu-img snapshot -c foo i2.qcow2
> $ ./qemu-io -c 'write 0 64M' i2.qcow2
> wrote 67108864/67108864 bytes at offset 0
> 64 MiB, 1 ops; 0:00:01.32 (48.152 MiB/sec and 0.7524 ops/sec)
> $ ./qemu-img convert -l snapshot.name=foo -O qcow2 i{1,2}.qcow2 o.qcow2
> qemu-img: error while reading sector 131072: Input/output error
> 
> ("No support for concatenating multiple snapshot" should be enforced
> for sn_opts != NULL)

Probably worth addressing, though not in this patch.

> >diff --git a/qemu-img.c b/qemu-img.c
> >index e148af8..5c8386e 100644
> >--- a/qemu-img.c
> >+++ b/qemu-img.c
> >@@ -1306,20 +1306,307 @@ out3:
> >  return ret;
> >  }
> >+enum ImgConvertBlockStatus {
> >+BLK_DATA,
> >+BLK_ZERO,
> >+BLK_BACKING_FILE,
> >+};
> >+
> >+typedef struct ImgConvertState {
> >+BlockDriverState **src;
> >+int64_t *src_sectors;
> >+int src_cur, src_num;
> >+int64_t src_cur_offset;
> >+int64_t total_sectors

Re: [Qemu-devel] [PATCH 2/2] target-mips: add missing MSA and correct FP in VMState

[Richard Henderson]

On 02/18/2015 06:51 AM, Leon Alrae wrote:
>  static VMStateField vmstate_fpu_fields[] = {
>  VMSTATE_FPR_ARRAY(fpr, CPUMIPSFPUContext, 32),
> -VMSTATE_INT8(fp_status.float_detect_tininess, CPUMIPSFPUContext),
> +VMSTATE_UINT8(fp_status.flush_to_zero, CPUMIPSFPUContext),
>  VMSTATE_INT8(fp_status.float_rounding_mode, CPUMIPSFPUContext),
>  VMSTATE_INT8(fp_status.float_exception_flags, CPUMIPSFPUContext),
>  VMSTATE_UINT32(fcr0, CPUMIPSFPUContext),
> @@ -70,6 +78,11 @@ static VMStateField vmstate_tc_fields[] = {
>  VMSTATE_UINTTL(CP0_TCScheFBack, TCState),
>  VMSTATE_INT32(CP0_Debug_tcstatus, TCState),
>  VMSTATE_UINTTL(CP0_UserLocal, TCState),
> +VMSTATE_INT32(msacsr, TCState),
> +VMSTATE_INT8(msa_fp_status.float_rounding_mode, TCState),
> +VMSTATE_INT8(msa_fp_status.float_exception_flags, TCState),
> +VMSTATE_UINT8(msa_fp_status.flush_to_zero, TCState),
> +VMSTATE_UINT8(msa_fp_status.flush_inputs_to_zero, TCState),
>  VMSTATE_END_OF_LIST()
>  };

Surely these fp_status fields are simply implementation of the architectural
CSR registers?

IMO you shouldn't store things related to TCG state, but always how the
architecture represents it.  That way you're free to change the TCG
implementation without breaking save/restore.


r~

Re: [Qemu-devel] [PATCH] qapi-types: add C99 index names to arrays

[Eric Blake]

On 02/19/2015 03:13 AM, Michael S. Tsirkin wrote:
> It's not easy to figure out how monitor translates
> strings: most QEMU code deals with translated indexes,
> these are translated using _lookup arrays,
> so you need to find the array name, and find the
> appropriate offset.
> 
> This patch adds C99 indexes to lookup arrays, which makes it possible to
> find the correct key using simple grep, and see that the matching is
> correct at a glance.
> 
> Example:
> 
> Before:
> 
> const char *MigrationCapability_lookup[] = {
> "xbzrle",
> "rdma-pin-all",
> "auto-converge",
> "zero-blocks",
> NULL,
> };
> 
> After:
> 
> const char *MigrationCapability_lookup[] = {
> [MIGRATION_CAPABILITY_XBZRLE] = "xbzrle",
> [MIGRATION_CAPABILITY_RDMA_PIN_ALL] = "rdma-pin-all",
> [MIGRATION_CAPABILITY_AUTO_CONVERGE] = "auto-converge",
> [MIGRATION_CAPABILITY_ZERO_BLOCKS] = "zero-blocks",
> [MIGRATION_CAPABILITY_MAX] = NULL,
> };

I like it :)

> 
> Signed-off-by: Michael S. Tsirkin 
> ---
>  scripts/qapi-types.py | 11 +++
>  1 file changed, 7 insertions(+), 4 deletions(-)

Reviewed-by: Eric Blake 

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



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] bus_unparent() can go into infinite loop

[Paolo Bonzini]

On 19/02/2015 14:05, Markus Armbruster wrote:
> Andreas Färber  writes:
> 
>> Am 19.02.2015 um 11:45 schrieb Markus Armbruster:
>>> Reproducer (don't ask):
>>>
>>> $ qemu-system-arm -M virt -S -display none -drive
>>> if=scsi,id=foo,bus=1,file=tmp.qcow2 -device nec-usb-xhci -device
>>> usb-storage,drive=foo -device virtio-scsi-pci
>>> qemu-system-arm: -drive if=scsi,id=foo,bus=1,file=tmp.qcow2:
>>> Property 'scsi-disk.drive' can't take value 'foo', it's in use
>>> qemu-system-arm: -drive if=scsi,id=foo,bus=1,file=tmp.qcow2:
>>> Setting drive property failed
>>> qemu-system-arm: -device virtio-scsi-pci: Setting drive property failed
>>>
>>> Nevermind the silly error reporting, I got patches to clean that up.
>>
>> I'm actually more confused about the use of PCI devices with the virt
>> machine. Does that now feature Alex' PCI controller by default?
>> Otherwise there would be no bus for those devices.
> 
> "info qtree" shows a PCIE bus:
> 
>   dev: gpex-pcihost, id ""
> gpio-out "sysbus-irq" 4
> mmio /1000
> mmio /
> mmio 3eff/0001
> bus: pcie.0
>   type PCIE
>   dev: gpex-root, id ""
> addr = 00.0
> romfile = ""
> rombar = 1 (0x1)
> multifunction = false
> command_serr_enable = true
> class Host bridge, addr 00:00.0, pci id 1b36:0008 (sub 1af4:1100)
> 
>> Is this on master or on top of your PCI realize changes or anything?
> 
> Unadulterated master (commit cd2d554).
> 
>>> Stuck in bus_unparent()'s loop:
>>>
>>> while ((kid = QTAILQ_FIRST(&bus->children)) != NULL) {
>>> DeviceState *dev = kid->child;
>>> object_unparent(OBJECT(dev));
>>> }
>>
>> Logically speaking, unparenting on QOM level has nothing to with the bus
>> children list.
>> The parent may well be /machine/{unassigned,peripheral{,-anon}}
>> container objects rather than some BusState object, the latter usually
>> has link<> properties only for its qdev-level "children".
>>
>> However I vaguely recall that we shoehorned the unparenting callback to
>> invoke unrealizing the device. What might happen here is that after
>> realizing the device failed, realized == false; realized = false in the
>> unparenting path becomes a no-op then. I.e., the realize error handling
>> may need to be reviewed to not just return but to undo any changes such
>> as attaching to some bus (or MemoryRegion, VMState, etc.).
> 
> (gdb) p *dev
> $2 = {parent_obj = {class = 0x56282140, free = 0x764dcf70 , 
> properties = {tqh_first = 0x5669d860, tqh_last = 0x566a1d90}, 
> ref = 1, parent = 0x0}, id = 0x0, realized = false, 
>   pending_deleted_event = false, opts = 0x0, hotplugged = 0, 
>   parent_bus = 0x56450060, gpios = {lh_first = 0x0}, child_bus = {
> lh_first = 0x0}, num_child_bus = 0, instance_id_alias = -1, 
>   alias_required_for_version = 0}
> 
> This is right before object_unparent().

The bug is that the device is not given a parent at all by
scsi_bus_legacy_add_drive, unlike for example qdev_device_add.

There is a "safety" net that adds the drive to the QOM tree in
qdev_init, but it isn't enough if you abort creation of the device
before qdev_init.  This applies to qdev_create and its callers
isa_create, usb_create, and also to qdev_try_create/isa_try_create.

I tried "git grep -lw FUNC_NAME | xargs grep object_unparent" and there
should be no other problematic case than scsi_bus_legacy_add_drive.

Patch will follow.

Paolo

Re: [Qemu-devel] [PATCH] target-arm: modifying pc in tcg code for load/store multiple

[Ildar Isaev]

> On 19 February 2015 at 21:26,   wrote:
> > From: Ildar Isaev 
> >
> > pc wasn't modified in tcg code for load/store multiple,
> > causing translation block to be executed in infinite loop forever
> >
> > Signed-off-by: Ildar Isaev 
> 
> It would be helpful if you gave an example of guest
> code which we mishandle. Do you have a test case?
> 

A bit clumsy, but something like that. Qemu never gets to the code past stmda.

-bash-4.1$ cat add.s


.text
mov   r0, #5 
mov   r1, #4
add   r2, r1, r0
stmda sp, {r1, r2, r5, sp, lr, pc}^
mov   r0, #26
mov   r1, #30


-bash-4.1$ arm-linux-gnueabihf-as -o add.o add.s

-bash-4.1$ arm-linux-gnueabihf-ld -Ttext=0x8000 -o add.elf add.o

-bash-4.1$ arm-linux-gnueabihf-objcopy -O binary add.elf add.bin

-bash-4.1$ dd if=/dev/zero of=test.bin bs=4096 count=4096

-bash-4.1$ dd if=add.bin of=test.bin bs=4096 conv=notrunc

-bash-4.1$ qemu-system-arm -M connex -pflash test.bin -nographic -serial 
/dev/null

QEMU 2.2.0 monitor - type 'help' for more information
(qemu) info registers
R00=0005 R01=0004 R02=0009 R03=
R04= R05= R06= R07=
R08= R09= R10= R11=
R12= R13= R14= R15=
PSR=0013  A svc32
FPSCR: 
(qemu) cont
(qemu) cont
(qemu) cont
(qemu) cont
(qemu) cont
(qemu) cont
(qemu) cont
(qemu) cont
(qemu) cont
(qemu) cont
(qemu) cont
(qemu) cont
(qemu) cont
(qemu) cont
(qemu) info registers
R00=0005 R01=0004 R02=0009 R03=
R04= R05= R06= R07=
R08= R09= R10= R11=
R12= R13= R14= R15=
PSR=0013  A svc32


add.s
Description: Binary data

Re: [Qemu-devel] [PATCH 1/2] qemu-img convert: Rewrite copying logic

[Max Reitz]

On 2015-02-19 at 10:47, Kevin Wolf wrote:

Am 18.02.2015 um 18:18 hat Max Reitz geschrieben:


[snip]


Actually, now that I'm looking at is_allocated_sectors_min(), if the
first sectors is not allocated, it will return false and thus both
the current qemu-img convert and the new one after this patch won't
write data. That's a bug, I think (because it is guaranteed by the
man page).

Or the description in the man page is wrong.

The intention with -S was that we avoid splitting up writes into too
many small chunks because that costs a lot of time. If you look at it
from that angle, it's doing exactly the right thing because skipping
zeros at the start doesn't increase the number of write requests.


Feel free to fix it. But I remember someone recently asking about 
preallocation for qemu-img convert in the #qemu IRC channel, and "-S 0" 
was one of the valid answers.


The nice thing about -S 0 is that it works with all image formats; I 
know that qcow2 is always our main concern (especially when it comes to 
the target of qemu-img convert), but I think using -S 0 for 
preallocation is justified.


Considering that in this case the man page was lying (because the code 
did not allocate everything), I'd be fine with fixing up the man page 
and leaving the behavior as-is, though, too.



[snip]


+
+if (s->compressed) {
+/* signal EOF to align */
+bdrv_write_compressed(s->target, 0, NULL, 0);

Is there a reason for ignoring the return value other than "That's
how img_convert() used to do it"?

No. Isn't that one good enough? ;-)


I don't know, your commit message states that the old implementation is 
buggy, so I don't think that's enough. :-)



So the code in qcow2 says this:

 if (nb_sectors == 0) {
 /* align end of file to a sector boundary to ease reading with
sector based I/Os */
 cluster_offset = bdrv_getlength(bs->file);
 return bdrv_truncate(bs->file, cluster_offset);
 }

I don't think we have any such restrictions any more, so it's mostly
useless. Perhaps ancient qemu versions would fail to read such an image,
but recent ones shouldn't.

In fact, our bdrv_pwrite() currently maps to sector-aligned functions in
the protocol driver, so I think at the moment we already get the
alignment automatically. This might change again once we convert block
drivers to byte offsets.


So you're intending to drop the bdrv_write_compressed() completely? I'd 
be fine with that as well.


Max

[Qemu-devel] [PATCH] scsi: give device a parent before setting properties

[Paolo Bonzini]

This mimics what is done in qdev_device_add, and lets the device be
freed in case something goes wrong.  Otherwise, object_unparent returns
immediately without freeing the device, which is on the other hand left
in the parent bus's list of children.

scsi_bus_legacy_handle_cmdline then returns an error, and the HBA is
destroyed as well with object_unparent.  But the lingering device that
was not removed in scsi_bus_legacy_add_drive cannot be removed now either,
and bus_unparent gets stuck in an infinite loop trying to empty the list
of children.

The right fix of course would be to assert in bus_add_child that the
device already has a bus, and remove the "safety net" that adds the
drive to the QOM tree in device_set_realized.  I am not yet sure whether
that would entail changing all callers to qdev_create (as well as
isa_create and usb_create and the corresponding _try_create versions).

Reported-by: Markus Armbruster 
Signed-off-by: Paolo Bonzini 
---
 hw/scsi/scsi-bus.c | 5 +
 1 file changed, 5 insertions(+)

diff --git a/hw/scsi/scsi-bus.c b/hw/scsi/scsi-bus.c
index db39ae0..dca9576 100644
--- a/hw/scsi/scsi-bus.c
+++ b/hw/scsi/scsi-bus.c
@@ -221,11 +221,16 @@ SCSIDevice *scsi_bus_legacy_add_drive(SCSIBus *bus, 
BlockBackend *blk,
   const char *serial, Error **errp)
 {
 const char *driver;
+char *name;
 DeviceState *dev;
 Error *err = NULL;
 
 driver = blk_is_sg(blk) ? "scsi-generic" : "scsi-disk";
 dev = qdev_create(&bus->qbus, driver);
+name = g_strdup_printf("legacy[%d]", unit);
+object_property_add_child(OBJECT(bus), name, OBJECT(dev), NULL);
+g_free(name);
+
 qdev_prop_set_uint32(dev, "scsi-id", unit);
 if (bootindex >= 0) {
 object_property_set_int(OBJECT(dev), bootindex, "bootindex",
-- 
2.3.0

Re: [Qemu-devel] [PATCH v3 0/2] block, virtio-scsi: Fix blk_set_aio_context

[Paolo Bonzini]

On 15/02/2015 04:06, Fam Zheng wrote:
> This is the simplified fix of:
> 
> [PATCH 0/3] virtio-scsi: Fix unsafe bdrv_set_aio_context calls
> 
> I included the original patch 1 - the function header comment update for
> bdrv_set_aio_context and added Paolo's rev-by.
> 
> Fam Zheng (2):
>   block: Forbid bdrv_set_aio_context outside BQL
>   virtio-scsi-dataplane: Call blk_set_aio_context within BQL
> 
>  hw/scsi/virtio-scsi.c | 15 +++
>  include/block/block.h |  3 +--
>  2 files changed, 8 insertions(+), 10 deletions(-)
> 

Applied, thanks.

Paolo

Re: [Qemu-devel] [PATCH 2/2] target-mips: add missing MSA and correct FP in VMState

[Leon Alrae]

On 19/02/2015 15:43, Richard Henderson wrote:
> On 02/18/2015 06:51 AM, Leon Alrae wrote:
>>  static VMStateField vmstate_fpu_fields[] = {
>>  VMSTATE_FPR_ARRAY(fpr, CPUMIPSFPUContext, 32),
>> -VMSTATE_INT8(fp_status.float_detect_tininess, CPUMIPSFPUContext),
>> +VMSTATE_UINT8(fp_status.flush_to_zero, CPUMIPSFPUContext),
>>  VMSTATE_INT8(fp_status.float_rounding_mode, CPUMIPSFPUContext),
>>  VMSTATE_INT8(fp_status.float_exception_flags, CPUMIPSFPUContext),
>>  VMSTATE_UINT32(fcr0, CPUMIPSFPUContext),
>> @@ -70,6 +78,11 @@ static VMStateField vmstate_tc_fields[] = {
>>  VMSTATE_UINTTL(CP0_TCScheFBack, TCState),
>>  VMSTATE_INT32(CP0_Debug_tcstatus, TCState),
>>  VMSTATE_UINTTL(CP0_UserLocal, TCState),
>> +VMSTATE_INT32(msacsr, TCState),
>> +VMSTATE_INT8(msa_fp_status.float_rounding_mode, TCState),
>> +VMSTATE_INT8(msa_fp_status.float_exception_flags, TCState),
>> +VMSTATE_UINT8(msa_fp_status.flush_to_zero, TCState),
>> +VMSTATE_UINT8(msa_fp_status.flush_inputs_to_zero, TCState),
>>  VMSTATE_END_OF_LIST()
>>  };
> 
> Surely these fp_status fields are simply implementation of the architectural
> CSR registers?
> 
> IMO you shouldn't store things related to TCG state, but always how the
> architecture represents it.  That way you're free to change the TCG
> implementation without breaking save/restore.

Good point. Saving fp_status and msa_fp_status doesn't seem to be needed
at all as they can be restored from FCSR and MSACSR respectively.
Presumably I can use vmstate post_load() for that.

Thanks,
Leon

Re: [Qemu-devel] [PATCH] scsi: give device a parent before setting properties

[Markus Armbruster]

Paolo Bonzini  writes:

> This mimics what is done in qdev_device_add, and lets the device be
> freed in case something goes wrong.  Otherwise, object_unparent returns
> immediately without freeing the device, which is on the other hand left
> in the parent bus's list of children.
>
> scsi_bus_legacy_handle_cmdline then returns an error, and the HBA is
> destroyed as well with object_unparent.  But the lingering device that
> was not removed in scsi_bus_legacy_add_drive cannot be removed now either,
> and bus_unparent gets stuck in an infinite loop trying to empty the list
> of children.
>
> The right fix of course would be to assert in bus_add_child that the
> device already has a bus, and remove the "safety net" that adds the
> drive to the QOM tree in device_set_realized.  I am not yet sure whether
> that would entail changing all callers to qdev_create (as well as
> isa_create and usb_create and the corresponding _try_create versions).
>
> Reported-by: Markus Armbruster 
> Signed-off-by: Paolo Bonzini 

Tested-by: Markus Armbruster 

[Qemu-devel] [PATCH 2/5] ui: Removed unused functions

[Thomas Huth]

Remove qemu_console_displaystate(), qemu_remove_kbd_event_handler(),
qemu_different_endianness_pixelformat() and cpkey(), since they are
completely unused.

Signed-off-by: Thomas Huth 
Cc: Anthony Liguori 
Cc: Gerd Hoffmann 
---
 include/ui/console.h |3 ---
 ui/console.c |   12 
 ui/d3des.c   |9 -
 ui/d3des.h   |6 --
 ui/input-legacy.c|6 --
 5 files changed, 0 insertions(+), 36 deletions(-)

diff --git a/include/ui/console.h b/include/ui/console.h
index 8a4d671..7c3b5d4 100644
--- a/include/ui/console.h
+++ b/include/ui/console.h
@@ -36,7 +36,6 @@ typedef struct QEMUPutLEDEntry QEMUPutLEDEntry;
 
 QEMUPutKbdEntry *qemu_add_kbd_event_handler(QEMUPutKBDEvent *func,
 void *opaque);
-void qemu_remove_kbd_event_handler(QEMUPutKbdEntry *entry);
 QEMUPutMouseEntry *qemu_add_mouse_event_handler(QEMUPutMouseEvent *func,
 void *opaque, int absolute,
 const char *name);
@@ -194,7 +193,6 @@ DisplaySurface *qemu_create_displaysurface_guestmem(int 
width, int height,
 pixman_format_code_t 
format,
 int linesize,
 uint64_t addr);
-PixelFormat qemu_different_endianness_pixelformat(int bpp);
 PixelFormat qemu_default_pixelformat(int bpp);
 
 DisplaySurface *qemu_create_displaysurface(int width, int height);
@@ -322,7 +320,6 @@ void qemu_console_resize(QemuConsole *con, int width, int 
height);
 void qemu_console_copy(QemuConsole *con, int src_x, int src_y,
int dst_x, int dst_y, int w, int h);
 DisplaySurface *qemu_console_surface(QemuConsole *con);
-DisplayState *qemu_console_displaystate(QemuConsole *console);
 
 /* sdl.c */
 void sdl_display_init(DisplayState *ds, int full_screen, int no_frame);
diff --git a/ui/console.c b/ui/console.c
index 87574a7..87af6b5 100644
--- a/ui/console.c
+++ b/ui/console.c
@@ -2005,18 +2005,6 @@ DisplaySurface *qemu_console_surface(QemuConsole 
*console)
 return console->surface;
 }
 
-DisplayState *qemu_console_displaystate(QemuConsole *console)
-{
-return console->ds;
-}
-
-PixelFormat qemu_different_endianness_pixelformat(int bpp)
-{
-pixman_format_code_t fmt = qemu_default_pixman_format(bpp, false);
-PixelFormat pf = qemu_pixelformat_from_pixman(fmt);
-return pf;
-}
-
 PixelFormat qemu_default_pixelformat(int bpp)
 {
 pixman_format_code_t fmt = qemu_default_pixman_format(bpp, true);
diff --git a/ui/d3des.c b/ui/d3des.c
index 60c840e..5bc99b8 100644
--- a/ui/d3des.c
+++ b/ui/d3des.c
@@ -121,15 +121,6 @@ static void cookey(register unsigned long *raw1)
return;
}
 
-void cpkey(register unsigned long *into)
-{
-   register unsigned long *from, *endp;
-
-   from = KnL, endp = &KnL[32];
-   while( from < endp ) *into++ = *from++;
-   return;
-   }
-
 void usekey(register unsigned long *from)
 {
register unsigned long *to, *endp;
diff --git a/ui/d3des.h b/ui/d3des.h
index 70cb6b5..773667e 100644
--- a/ui/d3des.h
+++ b/ui/d3des.h
@@ -36,12 +36,6 @@ void usekey(unsigned long *);
  * Loads the internal key register with the data in cookedkey.
  */
 
-void cpkey(unsigned long *);
-/*cookedkey[32]
- * Copies the contents of the internal key register into the storage
- * located at &cookedkey[0].
- */
-
 void des(unsigned char *, unsigned char *);
 /* from[8]   to[8]
  * Encrypts/Decrypts (according to the key currently loaded in the
diff --git a/ui/input-legacy.c b/ui/input-legacy.c
index a698a34..2d4ca19 100644
--- a/ui/input-legacy.c
+++ b/ui/input-legacy.c
@@ -143,12 +143,6 @@ QEMUPutKbdEntry 
*qemu_add_kbd_event_handler(QEMUPutKBDEvent *func, void *opaque)
 return entry;
 }
 
-void qemu_remove_kbd_event_handler(QEMUPutKbdEntry *entry)
-{
-qemu_input_handler_unregister(entry->s);
-g_free(entry);
-}
-
 static void legacy_mouse_event(DeviceState *dev, QemuConsole *src,
InputEvent *evt)
 {
-- 
1.7.1

[Qemu-devel] [PATCH 3/5] ui/vnc: Remove vnc_stop_worker_thread()

[Thomas Huth]

This function is not used anymore, let's remove it.

Signed-off-by: Thomas Huth 
Cc: Anthony Liguori 
Cc: Gerd Hoffmann 
---
 ui/vnc-jobs.c |   13 -
 ui/vnc-jobs.h |1 -
 2 files changed, 0 insertions(+), 14 deletions(-)

diff --git a/ui/vnc-jobs.c b/ui/vnc-jobs.c
index 68f3d77..c8ee203 100644
--- a/ui/vnc-jobs.c
+++ b/ui/vnc-jobs.c
@@ -342,16 +342,3 @@ void vnc_start_worker_thread(void)
QEMU_THREAD_DETACHED);
 queue = q; /* Set global queue */
 }
-
-void vnc_stop_worker_thread(void)
-{
-if (!vnc_worker_thread_running())
-return ;
-
-/* Remove all jobs and wake up the thread */
-vnc_lock_queue(queue);
-queue->exit = true;
-vnc_unlock_queue(queue);
-vnc_jobs_clear(NULL);
-qemu_cond_broadcast(&queue->cond);
-}
diff --git a/ui/vnc-jobs.h b/ui/vnc-jobs.h
index 31da103..044bf9f 100644
--- a/ui/vnc-jobs.h
+++ b/ui/vnc-jobs.h
@@ -40,7 +40,6 @@ void vnc_jobs_join(VncState *vs);
 
 void vnc_jobs_consume_buffer(VncState *vs);
 void vnc_start_worker_thread(void);
-void vnc_stop_worker_thread(void);
 
 /* Locks */
 static inline int vnc_trylock_display(VncDisplay *vd)
-- 
1.7.1

[Qemu-devel] [PATCH 1/5] migration: Remove unused functions

[Thomas Huth]

dup_mig_bytes_transferred(), skipped_mig_bytes_transferred(),
migrate_rdma_pin_all(), qsb_clone() and qsb_set_length()
are completely unused and thus can be deleted.

Signed-off-by: Thomas Huth 
Cc: Juan Quintela 
Cc: Amit Shah 
---
 arch_init.c   |   10 ---
 include/migration/migration.h |3 --
 include/migration/qemu-file.h |2 -
 migration/migration.c |9 ---
 migration/qemu-file-buf.c |   53 -
 5 files changed, 0 insertions(+), 77 deletions(-)

diff --git a/arch_init.c b/arch_init.c
index 89c8fa4..ad5ce28 100644
--- a/arch_init.c
+++ b/arch_init.c
@@ -249,21 +249,11 @@ static void acct_clear(void)
 memset(&acct_info, 0, sizeof(acct_info));
 }
 
-uint64_t dup_mig_bytes_transferred(void)
-{
-return acct_info.dup_pages * TARGET_PAGE_SIZE;
-}
-
 uint64_t dup_mig_pages_transferred(void)
 {
 return acct_info.dup_pages;
 }
 
-uint64_t skipped_mig_bytes_transferred(void)
-{
-return acct_info.skipped_pages * TARGET_PAGE_SIZE;
-}
-
 uint64_t skipped_mig_pages_transferred(void)
 {
 return acct_info.skipped_pages;
diff --git a/include/migration/migration.h b/include/migration/migration.h
index f37348b..7f6cdaa 100644
--- a/include/migration/migration.h
+++ b/include/migration/migration.h
@@ -115,9 +115,7 @@ void free_xbzrle_decoded_buf(void);
 
 void acct_update_position(QEMUFile *f, size_t size, bool zero);
 
-uint64_t dup_mig_bytes_transferred(void);
 uint64_t dup_mig_pages_transferred(void);
-uint64_t skipped_mig_bytes_transferred(void);
 uint64_t skipped_mig_pages_transferred(void);
 uint64_t norm_mig_bytes_transferred(void);
 uint64_t norm_mig_pages_transferred(void);
@@ -143,7 +141,6 @@ void migrate_add_blocker(Error *reason);
  */
 void migrate_del_blocker(Error *reason);
 
-bool migrate_rdma_pin_all(void);
 bool migrate_zero_blocks(void);
 
 bool migrate_auto_converge(void);
diff --git a/include/migration/qemu-file.h b/include/migration/qemu-file.h
index a923cec..45d7f71 100644
--- a/include/migration/qemu-file.h
+++ b/include/migration/qemu-file.h
@@ -133,9 +133,7 @@ bool qemu_file_mode_is_not_valid(const char *mode);
 bool qemu_file_is_writable(QEMUFile *f);
 
 QEMUSizedBuffer *qsb_create(const uint8_t *buffer, size_t len);
-QEMUSizedBuffer *qsb_clone(const QEMUSizedBuffer *);
 void qsb_free(QEMUSizedBuffer *);
-size_t qsb_set_length(QEMUSizedBuffer *qsb, size_t length);
 size_t qsb_get_length(const QEMUSizedBuffer *qsb);
 ssize_t qsb_get_buffer(const QEMUSizedBuffer *, off_t start, size_t count,
uint8_t *buf);
diff --git a/migration/migration.c b/migration/migration.c
index b3adbc6..6f1a490 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -540,15 +540,6 @@ void qmp_migrate_set_downtime(double value, Error **errp)
 max_downtime = (uint64_t)value;
 }
 
-bool migrate_rdma_pin_all(void)
-{
-MigrationState *s;
-
-s = migrate_get_current();
-
-return s->enabled_capabilities[MIGRATION_CAPABILITY_RDMA_PIN_ALL];
-}
-
 bool migrate_auto_converge(void)
 {
 MigrationState *s;
diff --git a/migration/qemu-file-buf.c b/migration/qemu-file-buf.c
index e97e0bd..3b79c09 100644
--- a/migration/qemu-file-buf.c
+++ b/migration/qemu-file-buf.c
@@ -124,28 +124,6 @@ size_t qsb_get_length(const QEMUSizedBuffer *qsb)
 }
 
 /**
- * Set the length of the buffer; the primary usage of this
- * function is to truncate the number of used bytes in the buffer.
- * The size will not be extended beyond the current number of
- * allocated bytes in the QEMUSizedBuffer.
- *
- * @qsb: A QEMUSizedBuffer
- * @new_len: The new length of bytes in the buffer
- *
- * Returns the number of bytes the buffer was truncated or extended
- * to.
- */
-size_t qsb_set_length(QEMUSizedBuffer *qsb, size_t new_len)
-{
-if (new_len <= qsb->size) {
-qsb->used = new_len;
-} else {
-qsb->used = qsb->size;
-}
-return qsb->used;
-}
-
-/**
  * Get the iovec that holds the data for a given position @pos.
  *
  * @qsb: A QEMUSizedBuffer
@@ -361,37 +339,6 @@ ssize_t qsb_write_at(QEMUSizedBuffer *qsb, const uint8_t 
*source,
 return count;
 }
 
-/**
- * Create a deep copy of the given QEMUSizedBuffer.
- *
- * @qsb: A QEMUSizedBuffer
- *
- * Returns a clone of @qsb or NULL on allocation failure
- */
-QEMUSizedBuffer *qsb_clone(const QEMUSizedBuffer *qsb)
-{
-QEMUSizedBuffer *out = qsb_create(NULL, qsb_get_length(qsb));
-size_t i;
-ssize_t res;
-off_t pos = 0;
-
-if (!out) {
-return NULL;
-}
-
-for (i = 0; i < qsb->n_iov; i++) {
-res =  qsb_write_at(out, qsb->iov[i].iov_base,
-pos, qsb->iov[i].iov_len);
-if (res < 0) {
-qsb_free(out);
-return NULL;
-}
-pos += res;
-}
-
-return out;
-}
-
 typedef struct QEMUBuffer {
 QEMUSizedBuffer *qsb;
 QEMUFile *file;
-- 
1.7.1

[Qemu-devel] [PATCH 5/5] xen: Remove xen_cmos_set_s3_resume()

[Thomas Huth]

The function is not used anymore, and thus can be deleted.

Signed-off-by: Thomas Huth 
Cc: Stefano Stabellini 
---
 include/hw/xen/xen.h |1 -
 xen-hvm-stub.c   |4 
 2 files changed, 0 insertions(+), 5 deletions(-)

diff --git a/include/hw/xen/xen.h b/include/hw/xen/xen.h
index b0ed04c..4356af4 100644
--- a/include/hw/xen/xen.h
+++ b/include/hw/xen/xen.h
@@ -32,7 +32,6 @@ int xen_pci_slot_get_pirq(PCIDevice *pci_dev, int irq_num);
 void xen_piix3_set_irq(void *opaque, int irq_num, int level);
 void xen_piix_pci_write_config_client(uint32_t address, uint32_t val, int len);
 void xen_hvm_inject_msi(uint64_t addr, uint32_t data);
-void xen_cmos_set_s3_resume(void *opaque, int irq, int level);
 
 qemu_irq *xen_interrupt_controller_init(void);
 
diff --git a/xen-hvm-stub.c b/xen-hvm-stub.c
index 2d98696..46867d8 100644
--- a/xen-hvm-stub.c
+++ b/xen-hvm-stub.c
@@ -30,10 +30,6 @@ void xen_hvm_inject_msi(uint64_t addr, uint32_t data)
 {
 }
 
-void xen_cmos_set_s3_resume(void *opaque, int irq, int level)
-{
-}
-
 void xen_ram_alloc(ram_addr_t ram_addr, ram_addr_t size, MemoryRegion *mr)
 {
 }
-- 
1.7.1

[Qemu-devel] [PATCH 4/5] util: Remove unused functions

[Thomas Huth]

Delete the unused functions qemu_signalfd_available(),
qemu_send_full() and qemu_recv_full().

Signed-off-by: Thomas Huth 
Cc: Markus Armbruster 
---
 include/qemu-common.h   |4 ---
 include/qemu/compatfd.h |1 -
 util/compatfd.c |   19 -
 util/osdep.c|   66 ---
 4 files changed, 0 insertions(+), 90 deletions(-)

diff --git a/include/qemu-common.h b/include/qemu-common.h
index 644b46d..0aac082 100644
--- a/include/qemu-common.h
+++ b/include/qemu-common.h
@@ -217,10 +217,6 @@ void *qemu_oom_check(void *ptr);
 
 ssize_t qemu_write_full(int fd, const void *buf, size_t count)
 QEMU_WARN_UNUSED_RESULT;
-ssize_t qemu_send_full(int fd, const void *buf, size_t count, int flags)
-QEMU_WARN_UNUSED_RESULT;
-ssize_t qemu_recv_full(int fd, void *buf, size_t count, int flags)
-QEMU_WARN_UNUSED_RESULT;
 
 #ifndef _WIN32
 int qemu_pipe(int pipefd[2]);
diff --git a/include/qemu/compatfd.h b/include/qemu/compatfd.h
index 6b04877..fc37915 100644
--- a/include/qemu/compatfd.h
+++ b/include/qemu/compatfd.h
@@ -39,6 +39,5 @@ struct qemu_signalfd_siginfo {
 };
 
 int qemu_signalfd(const sigset_t *mask);
-bool qemu_signalfd_available(void);
 
 #endif
diff --git a/util/compatfd.c b/util/compatfd.c
index 341ada6..e857150 100644
--- a/util/compatfd.c
+++ b/util/compatfd.c
@@ -108,22 +108,3 @@ int qemu_signalfd(const sigset_t *mask)
 
 return qemu_signalfd_compat(mask);
 }
-
-bool qemu_signalfd_available(void)
-{
-#ifdef CONFIG_SIGNALFD
-sigset_t mask;
-int fd;
-bool ok;
-sigemptyset(&mask);
-errno = 0;
-fd = syscall(SYS_signalfd, -1, &mask, _NSIG / 8);
-ok = (errno != ENOSYS);
-if (fd >= 0) {
-close(fd);
-}
-return ok;
-#else
-return false;
-#endif
-}
diff --git a/util/osdep.c b/util/osdep.c
index b2bd154..f938b69 100644
--- a/util/osdep.c
+++ b/util/osdep.c
@@ -310,72 +310,6 @@ int qemu_accept(int s, struct sockaddr *addr, socklen_t 
*addrlen)
 return ret;
 }
 
-/*
- * A variant of send(2) which handles partial write.
- *
- * Return the number of bytes transferred, which is only
- * smaller than `count' if there is an error.
- *
- * This function won't work with non-blocking fd's.
- * Any of the possibilities with non-bloking fd's is bad:
- *   - return a short write (then name is wrong)
- *   - busy wait adding (errno == EAGAIN) to the loop
- */
-ssize_t qemu_send_full(int fd, const void *buf, size_t count, int flags)
-{
-ssize_t ret = 0;
-ssize_t total = 0;
-
-while (count) {
-ret = send(fd, buf, count, flags);
-if (ret < 0) {
-if (errno == EINTR) {
-continue;
-}
-break;
-}
-
-count -= ret;
-buf += ret;
-total += ret;
-}
-
-return total;
-}
-
-/*
- * A variant of recv(2) which handles partial write.
- *
- * Return the number of bytes transferred, which is only
- * smaller than `count' if there is an error.
- *
- * This function won't work with non-blocking fd's.
- * Any of the possibilities with non-bloking fd's is bad:
- *   - return a short write (then name is wrong)
- *   - busy wait adding (errno == EAGAIN) to the loop
- */
-ssize_t qemu_recv_full(int fd, void *buf, size_t count, int flags)
-{
-ssize_t ret = 0;
-ssize_t total = 0;
-
-while (count) {
-ret = qemu_recv(fd, buf, count, flags);
-if (ret <= 0) {
-if (ret < 0 && errno == EINTR) {
-continue;
-}
-break;
-}
-
-count -= ret;
-buf += ret;
-total += ret;
-}
-
-return total;
-}
-
 void qemu_set_version(const char *version)
 {
 qemu_version = version;
-- 
1.7.1

[Qemu-devel] [PATCH v2 0/5] Remove unused functions

[Thomas Huth]

There are quite a lot of completely unused functions scattered
around in the QEMU sources - here are some patches to remove at
least some of them.

v2:
- Incorporated review feedback to keep the functions that still might
  be usefull (e.g. dropped the patch to remove unused functions in the
  block layer)
- Added a patch to remove xen_cmos_set_s3_resume()

Thomas Huth (5):
  migration: Remove unused functions
  ui: Removed unused functions
  ui/vnc: Remove vnc_stop_worker_thread()
  util: Remove unused functions
  xen: Remove xen_cmos_set_s3_resume()

 arch_init.c   |   10 --
 include/hw/xen/xen.h  |1 -
 include/migration/migration.h |3 --
 include/migration/qemu-file.h |2 -
 include/qemu-common.h |4 --
 include/qemu/compatfd.h   |1 -
 include/ui/console.h  |3 --
 migration/migration.c |9 -
 migration/qemu-file-buf.c |   53 -
 ui/console.c  |   12 ---
 ui/d3des.c|9 -
 ui/d3des.h|6 
 ui/input-legacy.c |6 
 ui/vnc-jobs.c |   13 
 ui/vnc-jobs.h |1 -
 util/compatfd.c   |   19 
 util/osdep.c  |   66 -
 xen-hvm-stub.c|4 --
 18 files changed, 0 insertions(+), 222 deletions(-)

Re: [Qemu-devel] [PATCH v4 0/5] libqos: Virtio MMIO driver

[Kevin Wolf]

Am 18.02.2015 um 18:07 hat Stefan Hajnoczi geschrieben:
> On Fri, Jan 23, 2015 at 05:38:48PM +0100, Marc Marí wrote:
> > Add virtio-mmio support to libqos and test case for virtio-blk.
> > 
> > Changes for version 3:
> >  - Fix leaks and minor bugs
> >  - Extract basic test case to a function
> > 
> > Changes for version 4:
> >  - Add format=raw to images, to avoid warnings
> >  - Solve bug with timeout in interrupt checking in virtio MMIO due to a race
> >condition
> > 
> > Marc Marí (5):
> >   libqos: Change use of pointers to uint64_t in virtio
> >   tests: Prepare virtio-blk-test for multi-arch implementation
> >   libqos: Remove PCI assumptions in constants of virtio driver
> >   libqos: Add malloc generic
> >   libqos: Add virtio MMIO support
> 
> CCed Kevin so this ARM virtio-blk (virtio-mmio) test can be merged
> through the block tree.
> 
> Reviewed-by: Stefan Hajnoczi 

Thanks, applied to the block branch.

Kevin


pgpVstZHloG3h.pgp
Description: PGP signature

Re: [Qemu-devel] [PATCH 03/10] guest agent: guest-file-open: refactoring

[Denis V. Lunev]

On 17/02/15 20:59, Eric Blake wrote:

On 02/17/2015 10:56 AM, Michael Roth wrote:

In any case, since I was actually the one who re-invented it,
and this code just moves it to another function, I think we
can address it as a seperate patch and leave the PULL
intact (unless there are other objections).

Agree that a separate cleanup patch would be okay.


OK. There is a portable way to detect whether
the descriptor is a pipe or socket on Windows.
Thus generic helper is possible.

We'll provide you with one. ETA is something around
next week.

[Qemu-devel] [PATCH v2 1/2] integrator/cp: Model CP control registers as sysbus device

[Jan Kiszka]

No new features yet, just encapsulation.

Signed-off-by: Jan Kiszka 
---
 hw/arm/integratorcp.c | 43 +++
 1 file changed, 35 insertions(+), 8 deletions(-)

diff --git a/hw/arm/integratorcp.c b/hw/arm/integratorcp.c
index 8c48b68..c5aa1f6 100644
--- a/hw/arm/integratorcp.c
+++ b/hw/arm/integratorcp.c
@@ -406,6 +406,18 @@ static int icp_pic_init(SysBusDevice *sbd)
 
 /* CP control registers.  */
 
+#define TYPE_ICP_CONTROL_REGS "icp_ctrl_regs"
+#define ICP_CONTROL_REGS(obj) \
+OBJECT_CHECK(ICPCtrlRegsState, (obj), TYPE_ICP_CONTROL_REGS)
+
+typedef struct ICPCtrlRegsState {
+/*< private >*/
+SysBusDevice parent_obj;
+/*< public >*/
+
+MemoryRegion iomem;
+} ICPCtrlRegsState;
+
 static uint64_t icp_control_read(void *opaque, hwaddr offset,
  unsigned size)
 {
@@ -444,15 +456,15 @@ static const MemoryRegionOps icp_control_ops = {
 .endianness = DEVICE_NATIVE_ENDIAN,
 };
 
-static void icp_control_init(hwaddr base)
+static int icp_control_init(SysBusDevice *sbd)
 {
-MemoryRegion *io;
+ICPCtrlRegsState *s = ICP_CONTROL_REGS(sbd);
 
-io = (MemoryRegion *)g_malloc0(sizeof(MemoryRegion));
-memory_region_init_io(io, NULL, &icp_control_ops, NULL,
-  "control", 0x0080);
-memory_region_add_subregion(get_system_memory(), base, io);
-/* ??? Save/restore.  */
+memory_region_init_io(&s->iomem, OBJECT(s), &icp_control_ops, s,
+  "icp_ctrl_regs", 0x0080);
+sysbus_init_mmio(sbd, &s->iomem);
+
+return 0;
 }
 
 
@@ -541,7 +553,7 @@ static void integratorcp_init(MachineState *machine)
 sysbus_create_simple("pl031", 0x1500, pic[8]);
 sysbus_create_simple("pl011", 0x1600, pic[1]);
 sysbus_create_simple("pl011", 0x1700, pic[2]);
-icp_control_init(0xcb00);
+sysbus_create_simple(TYPE_ICP_CONTROL_REGS, 0xcb00, NULL);
 sysbus_create_simple("pl050_keyboard", 0x1800, pic[3]);
 sysbus_create_simple("pl050_mouse", 0x1900, pic[4]);
 sysbus_create_simple(TYPE_INTEGRATOR_DEBUG, 0x1a00, 0);
@@ -606,10 +618,25 @@ static const TypeInfo icp_pic_info = {
 .class_init= icp_pic_class_init,
 };
 
+static void icp_class_init(ObjectClass *klass, void *data)
+{
+SysBusDeviceClass *k = SYS_BUS_DEVICE_CLASS(klass);
+
+k->init = icp_control_init;
+}
+
+static const TypeInfo icp_ctrl_regs_info = {
+.name  = TYPE_ICP_CONTROL_REGS,
+.parent= TYPE_SYS_BUS_DEVICE,
+.instance_size = sizeof(ICPCtrlRegsState),
+.class_init= icp_class_init,
+};
+
 static void integratorcp_register_types(void)
 {
 type_register_static(&icp_pic_info);
 type_register_static(&core_info);
+type_register_static(&icp_ctrl_regs_info);
 }
 
 type_init(integratorcp_register_types)
-- 
2.1.4

[Qemu-devel] [PATCH v2 0/2] integrator/cp: Working SD card support

[Jan Kiszka]

This addresses the review comments on the primitive first version.

Jan

Jan Kiszka (2):
  integrator/cp: Model CP control registers as sysbus device
  integrator/cp: Implement CARDIN and WPROT signals

 hw/arm/integratorcp.c | 99 ---
 1 file changed, 86 insertions(+), 13 deletions(-)

-- 
2.1.4

[Qemu-devel] [PATCH v2 2/2] integrator/cp: Implement CARDIN and WPROT signals

[Jan Kiszka]

This allows to use the SD card emulation of the board: Forward the
signals from the pl181 top the CP control register emulation, report the
current state via CP_INTREG, deliver CARDIN IRQ to the secondary
interrupt controller and also support clearing that line via CP_INTREG.

Signed-off-by: Jan Kiszka 
---
 hw/arm/integratorcp.c | 58 +--
 1 file changed, 52 insertions(+), 6 deletions(-)

diff --git a/hw/arm/integratorcp.c b/hw/arm/integratorcp.c
index c5aa1f6..39803a9 100644
--- a/hw/arm/integratorcp.c
+++ b/hw/arm/integratorcp.c
@@ -416,18 +416,29 @@ typedef struct ICPCtrlRegsState {
 /*< public >*/
 
 MemoryRegion iomem;
+
+qemu_irq mmc_irq;
+uint32_t intreg_state;
 } ICPCtrlRegsState;
 
+#define ICP_GPIO_MMC_WPROT  0
+#define ICP_GPIO_MMC_CARDIN 1
+
+#define ICP_INTREG_WPROT(1 << 0)
+#define ICP_INTREG_CARDIN   (1 << 3)
+
 static uint64_t icp_control_read(void *opaque, hwaddr offset,
  unsigned size)
 {
+ICPCtrlRegsState *s = opaque;
+
 switch (offset >> 2) {
 case 0: /* CP_IDFIELD */
 return 0x41034003;
 case 1: /* CP_FLASHPROG */
 return 0;
 case 2: /* CP_INTREG */
-return 0;
+return s->intreg_state;
 case 3: /* CP_DECODE */
 return 0x11;
 default:
@@ -439,9 +450,14 @@ static uint64_t icp_control_read(void *opaque, hwaddr 
offset,
 static void icp_control_write(void *opaque, hwaddr offset,
   uint64_t value, unsigned size)
 {
+ICPCtrlRegsState *s = opaque;
+
 switch (offset >> 2) {
-case 1: /* CP_FLASHPROG */
 case 2: /* CP_INTREG */
+s->intreg_state &= ~(value & ICP_INTREG_CARDIN);
+qemu_set_irq(s->mmc_irq, !!(s->intreg_state & ICP_INTREG_CARDIN));
+break;
+case 1: /* CP_FLASHPROG */
 case 3: /* CP_DECODE */
 /* Nothing interesting implemented yet.  */
 break;
@@ -456,14 +472,39 @@ static const MemoryRegionOps icp_control_ops = {
 .endianness = DEVICE_NATIVE_ENDIAN,
 };
 
+static void icp_control_gpio_set(void *opaque, int line, int level)
+{
+ICPCtrlRegsState *s = opaque;
+
+switch (line) {
+case ICP_GPIO_MMC_WPROT:
+s->intreg_state &= ~ICP_INTREG_WPROT;
+if (level) {
+s->intreg_state |= ICP_INTREG_WPROT;
+}
+break;
+case ICP_GPIO_MMC_CARDIN:
+/* line is released by writing to CP_INTREG */
+if (level) {
+s->intreg_state |= ICP_INTREG_CARDIN;
+qemu_set_irq(s->mmc_irq, 1);
+}
+break;
+}
+}
+
 static int icp_control_init(SysBusDevice *sbd)
 {
 ICPCtrlRegsState *s = ICP_CONTROL_REGS(sbd);
+DeviceState *dev = DEVICE(sbd);
 
 memory_region_init_io(&s->iomem, OBJECT(s), &icp_control_ops, s,
   "icp_ctrl_regs", 0x0080);
 sysbus_init_mmio(sbd, &s->iomem);
 
+qdev_init_gpio_in(dev, icp_control_gpio_set, 2);
+sysbus_init_irq(sbd, &s->mmc_irq);
+
 return 0;
 }
 
@@ -489,7 +530,7 @@ static void integratorcp_init(MachineState *machine)
 MemoryRegion *ram = g_new(MemoryRegion, 1);
 MemoryRegion *ram_alias = g_new(MemoryRegion, 1);
 qemu_irq pic[32];
-DeviceState *dev;
+DeviceState *dev, *sic, *icp;
 int i;
 Error *err = NULL;
 
@@ -547,17 +588,22 @@ static void integratorcp_init(MachineState *machine)
 for (i = 0; i < 32; i++) {
 pic[i] = qdev_get_gpio_in(dev, i);
 }
-sysbus_create_simple(TYPE_INTEGRATOR_PIC, 0xca00, pic[26]);
+sic = sysbus_create_simple(TYPE_INTEGRATOR_PIC, 0xca00, pic[26]);
 sysbus_create_varargs("integrator_pit", 0x1300,
   pic[5], pic[6], pic[7], NULL);
 sysbus_create_simple("pl031", 0x1500, pic[8]);
 sysbus_create_simple("pl011", 0x1600, pic[1]);
 sysbus_create_simple("pl011", 0x1700, pic[2]);
-sysbus_create_simple(TYPE_ICP_CONTROL_REGS, 0xcb00, NULL);
+icp = sysbus_create_simple(TYPE_ICP_CONTROL_REGS, 0xcb00,
+   qdev_get_gpio_in(sic, 3));
 sysbus_create_simple("pl050_keyboard", 0x1800, pic[3]);
 sysbus_create_simple("pl050_mouse", 0x1900, pic[4]);
 sysbus_create_simple(TYPE_INTEGRATOR_DEBUG, 0x1a00, 0);
-sysbus_create_varargs("pl181", 0x1c00, pic[23], pic[24], NULL);
+
+dev = sysbus_create_varargs("pl181", 0x1c00, pic[23], pic[24], NULL);
+qdev_connect_gpio_out(dev, 0, qdev_get_gpio_in(icp, ICP_GPIO_MMC_WPROT));
+qdev_connect_gpio_out(dev, 1, qdev_get_gpio_in(icp, ICP_GPIO_MMC_CARDIN));
+
 if (nd_table[0].used)
 smc91c111_init(&nd_table[0], 0xc800, pic[27]);
 
-- 
2.1.4

[Qemu-devel] [Bug 1422307] Re: qemu-nbd corrupts files

[Pierre Schweitzer]

Thanks Max & Stefan.

Could you please let me know when the commit makes it to the QEMU
repository? And give me its commit ID?

So that I can ask for a bugfix backport in Trusty.

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

Title:
  qemu-nbd corrupts files

Status in QEMU:
  New

Bug description:
  Dear all,

  On Trusty, in certain situations, try to copy files over a qemu-nbd
  mounted file system leads to write errors (and thus, file corruption).

  Here is the last example I tried:
  -> virtual disk is a VDI disk
  -> It has only one partition, in FAT

  Here is my mount process:
  # modprobe nbd max_part=63
  # qemu-nbd -c /dev/nbd0 "virtual_disk.vdi"
  # partprobe /dev/nbd0
  # mount /dev/nbd0p1 /tmp/mnt/

  Partition is properly mounted at that point:
  /dev/nbd0p1 on /tmp/mnt type vfat (rw)

  Now, when I copy a file (rather big, ~28MB):
  # cp file_to_copy /tmp/mnt/ ; sync
  # md5sum /tmp/mnt/file_to_copy
  2efc9f32e4267782b11d63d2f128a363  /tmp/mnt/file_to_copy
  # umount /tmp/mnt 
  # mount /dev/nbd0p1 /tmp/mnt/
  # md5sum /tmp/mnt/file_to_copy
  42b0a3bf73f704d03ce301716d7654de  /tmp/mnt/file_to_copy

  The first hash was obviously the right one.

  On a previous attempt I did, I spotted thanks to vbindiff that parts of the 
file were just filed with 0s instead of actual data.
  It will randomly work after several attempts to write.

  Version information:
  # qemu-nbd --version
  qemu-nbd version 0.0.1
  Written by Anthony Liguori.

  Cheers,

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

Re: [Qemu-devel] [PATCH RFC v3 11/14] iotests: add dirty bitmap migration test

[John Snow]

On 02/18/2015 09:00 AM, Vladimir Sementsov-Ogievskiy wrote:

The test starts two vms (vm_a, vm_b), create dirty bitmap in the first one, do
several writes to corresponding device and then migrate vm_a to vm_b
with dirty bitmaps.

Signed-off-by: Vladimir Sementsov-Ogievskiy 
---
  tests/qemu-iotests/117 | 88 ++
  tests/qemu-iotests/117.out |  5 +++
  tests/qemu-iotests/group   |  1 +
  3 files changed, 94 insertions(+)
  create mode 100755 tests/qemu-iotests/117
  create mode 100644 tests/qemu-iotests/117.out

diff --git a/tests/qemu-iotests/117 b/tests/qemu-iotests/117
new file mode 100755
index 000..61538cf
--- /dev/null
+++ b/tests/qemu-iotests/117
@@ -0,0 +1,88 @@
+#!/usr/bin/env python
+#
+# Tests for dirty bitmaps migration.
+#
+# (C) Vladimir Sementsov-Ogievskiy 2015
+#
+# 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 .
+#
+
+import os
+import iotests
+import time
+from iotests import qemu_img
+
+disk_a = os.path.join(iotests.test_dir, 'disk_a')
+disk_b = os.path.join(iotests.test_dir, 'disk_b')
+fifo   = os.path.join(iotests.test_dir, 'fifo')
+
+size   = 0x4000 # 1G
+sector_size = 512
+granularity = 0x1
+regions = [
+{ 'start': 0,  'count': 0x10 },
+{ 'start': 0x1000, 'count': 0x2  },
+{ 'start': 0x3999, 'count': 0x1  }
+]
+
+regions_in_sectors = [
+{ key: val / sector_size for (key, val) in el.items() } for el in regions]
+
+class TestDirtyBitmapMigration(iotests.QMPTestCase):
+
+def setUp(self):
+os.mkfifo(fifo)
+qemu_img('create', '-f', iotests.imgfmt, disk_a, str(size))
+qemu_img('create', '-f', iotests.imgfmt, disk_b, str(size))
+self.vm_a = iotests.VM().add_drive(disk_a)
+self.vm_b = iotests.VM().add_drive(disk_b)
+self.vm_b.add_incoming_migration("exec: cat " + fifo)
+self.vm_a.launch()
+self.vm_b.launch()
+
+def tearDown(self):
+self.vm_a.shutdown()
+self.vm_b.shutdown()
+os.remove(disk_a)
+os.remove(disk_b)
+os.remove(fifo)
+
+def test_migration(self):
+result = self.vm_a.qmp('block-dirty-bitmap-add', node='drive0',
+   name='bitmap', granularity=granularity)
+self.assert_qmp(result, 'return', {});
+
+for r in regions:
+  self.vm_a.hmp_qemu_io('drive0',
+'write %d %d' % (r['start'], r['count']))
+
+result = self.vm_a.qmp('query-block-dirty-bitmap', node='drive0',
+   name='bitmap')
+self.assert_qmp(result, 'return/dirty-regions', regions_in_sectors)
+
+result = self.vm_a.qmp('migrate-set-capabilities',
+   capabilities=[{'capability': 'dirty-bitmaps',
+  'state': True}])
+self.assert_qmp(result, 'return', {})
+result = self.vm_a.qmp('migrate', uri='exec:cat>' + fifo)
+while self.vm_a.qmp('query-migrate')['return']['status'] != 
'completed':
+  time.sleep(1)
+
+result = self.vm_b.qmp('query-block-dirty-bitmap', node='drive0',
+   name='bitmap')
+self.assert_qmp(result, 'return/dirty-regions', regions_in_sectors);
+
+
+if __name__ == '__main__':
+iotests.main()
diff --git a/tests/qemu-iotests/117.out b/tests/qemu-iotests/117.out
new file mode 100644
index 000..ae1213e
--- /dev/null
+++ b/tests/qemu-iotests/117.out
@@ -0,0 +1,5 @@
+.
+--
+Ran 1 tests
+
+OK
diff --git a/tests/qemu-iotests/group b/tests/qemu-iotests/group
index b4ddf1b..6ad5b55 100644
--- a/tests/qemu-iotests/group
+++ b/tests/qemu-iotests/group
@@ -118,3 +118,4 @@
  113 rw auto quick
  114 rw auto quick
  116 rw auto quick
+117 rw auto quick



Spoke with Stefan earlier today and I think my initial hunch of keeping 
the exhaustive QMP debug commands out of the upstream repo makes more 
sense right now.


Our view was that even if we check in code "to be used for debug only," 
there's no telling how people or other programs might come to rely on 
the data, so it's best not to introduce the interface to begin with.


I may pull these patches into one of my github repositories, though, 
since we'll probably need these patches for debugging in the near future.


So

Re: [Qemu-devel] [PATCH RFC v3 12/14] qapi: add md5 checksum of last dirty bitmap level to query-block

[John Snow]

On 02/18/2015 09:00 AM, Vladimir Sementsov-Ogievskiy wrote:

Signed-off-by: Vladimir Sementsov-Ogievskiy 
---
  block.c| 1 +
  include/qemu/hbitmap.h | 8 
  qapi/block-core.json   | 4 +++-
  util/hbitmap.c | 8 
  4 files changed, 20 insertions(+), 1 deletion(-)

diff --git a/block.c b/block.c
index 4cca55d..9532ccc 100644
--- a/block.c
+++ b/block.c
@@ -5600,6 +5600,7 @@ BlockDirtyInfoList 
*bdrv_query_dirty_bitmaps(BlockDriverState *bs)
  info->name = g_strdup(bm->name);
  info->disabled = bm->disabled;
  info->frozen = bdrv_dirty_bitmap_frozen(bm);
+info->md5 = hbitmap_md5(bm->bitmap);
  entry->value = info;
  *plist = entry;
  plist = &entry->next;
diff --git a/include/qemu/hbitmap.h b/include/qemu/hbitmap.h
index 10ce05b..2fb748a 100644
--- a/include/qemu/hbitmap.h
+++ b/include/qemu/hbitmap.h
@@ -188,6 +188,14 @@ void hbitmap_deserialize_zeroes(HBitmap *hb, uint64_t 
start, uint64_t count);
  void hbitmap_deserialize_finish(HBitmap *hb);

  /**
+ * hbitmap_md5:
+ * @bitmap: HBitmap to operate on.
+ *
+ * Returns md5 checksum of the last level.
+ */
+char *hbitmap_md5(const HBitmap *bitmap);
+
+/**
   * hbitmap_free:
   * @hb: HBitmap to operate on.
   *
diff --git a/qapi/block-core.json b/qapi/block-core.json
index 25dea80..2028d37 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -336,11 +336,13 @@
  #
  # @frozen: whether the dirty bitmap is frozen (Since 2.3)
  #
+# @md5: md5 checksum of the last bitmap level (since 2.3)
+#
  # Since: 1.3
  ##
  { 'type': 'BlockDirtyInfo',
'data': {'*name': 'str', 'count': 'int', 'granularity': 'uint32',
-   'disabled': 'bool', 'frozen': 'bool'} }
+   'disabled': 'bool', 'frozen': 'bool', 'md5': 'str'} }

  ##
  # @BlockInfo:
diff --git a/util/hbitmap.c b/util/hbitmap.c
index 1a736e7..8063dce 100644
--- a/util/hbitmap.c
+++ b/util/hbitmap.c
@@ -523,3 +523,11 @@ bool hbitmap_merge(HBitmap *a, const HBitmap *b)

  return true;
  }
+
+char *hbitmap_md5(const HBitmap *bitmap)
+{
+uint64_t size =
+MAX((bitmap->size + BITS_PER_LONG - 1) >> BITS_PER_LEVEL, 1);
+const guchar *data = (const guchar *)bitmap->levels[HBITMAP_LEVELS - 1];
+return g_compute_checksum_for_data(G_CHECKSUM_MD5, data, size);
+}



It strikes me as somewhat odd to introduce a feature for the explicit 
purpose of regression testing, but I can't think of how else we'd do it 
simply, so this makes the most sense to me right now.


Reviewed-by: John Snow 

[Qemu-devel] [PATCH] e1000: work around win 8.0 boot hang

[Radim Krčmář]

Window 8.0 driver has a particular behavior for a small time frame after
it enables rx interrupts:  the interrupt handler never clears
E1000_ICR_RXT0.  The handler does this something like this:
  set_imc(-1)   (1) disable all interrupts
  val = read_icr()  (2) clear ICR
  handled = magic(val)  (3) do nothing to E1000_ICR_RXT0
  set_ics(val & ~handled)   (4) set unhandled interrupts back to ICR
  set_ims(157)  (5) enable some interrupts

so if we started with RXT0, then every time the handler re-enables e1000
interrupts, it receives one.  This likely wouldn't matter in real
hardware, because it is slow enough to make some progress between
interrupts, but KVM instantly interrupts it, and boot hangs.
(If we have multiple VCPUs, the interrupt gets load-balanced and
 everything is fine.)

I haven't found any problem in earlier phase of initialization and
windows writes 0 to RADV and RDTR, so some workaround looks like the
only way if we want to support win8.0 on uniprocessors.  (I vote NO.)

This workaround uses the fact that a constant is cleared from ICR and
later set back to it.  After detecting this situation, we reuse the
mitigation framework to inject an interrupt 10 microseconds later.
(It's not exactly 10 microseconds, to keep the existing logic intact.)

The detection is done by checking at (1), (2), and (5).  (2) and (5)
require that the only bit in ICR is RXT0.  We could also check at (4),
and on writes to any other register, but it would most likely only add
more useless code, because normal operations shouldn't behave like that
anyway.  (An OS that deliberately keeps bits in ICR to notify itself
that there are more packets, or for more creative reasons, is nothing we
should care about.)

Signed-off-by: Radim Krčmář 
---
 The patch is still untested -- it only approximates the behavior of RHEL
 patches that worked, I'll try to get a reproducer ...

 hw/net/e1000.c | 29 ++---
 1 file changed, 22 insertions(+), 7 deletions(-)

diff --git a/hw/net/e1000.c b/hw/net/e1000.c
index a207e21bcf77..773aac47f0b2 100644
--- a/hw/net/e1000.c
+++ b/hw/net/e1000.c
@@ -138,6 +138,10 @@ typedef struct E1000State_st {
 #define E1000_FLAG_AUTONEG (1 << E1000_FLAG_AUTONEG_BIT)
 #define E1000_FLAG_MIT (1 << E1000_FLAG_MIT_BIT)
 uint32_t compat_flags;
+
+#define E1000_WIN8_WORKAROUND_ICR   E1000_ICR_RXT0
+#define E1000_WIN8_WORKAROUND_DELAY_US  10
+bool win8_workaround_needed;
 } E1000State;
 
 typedef struct E1000BaseClass {
@@ -288,7 +292,7 @@ set_interrupt_cause(E1000State *s, int index, uint32_t val)
 {
 PCIDevice *d = PCI_DEVICE(s);
 uint32_t pending_ints;
-uint32_t mit_delay;
+uint32_t mit_delay = 0;
 
 s->mac_reg[ICR] = val;
 
@@ -316,13 +320,17 @@ set_interrupt_cause(E1000State *s, int index, uint32_t 
val)
 if (s->mit_timer_on) {
 return;
 }
+
+if (s->win8_workround_needed) {
+mit_delay = E1000_WIN8_WORKAROUND_DELAY_US * 4;
+}
+
 if (s->compat_flags & E1000_FLAG_MIT) {
 /* Compute the next mitigation delay according to pending
  * interrupts and the current values of RADV (provided
  * RDTR!=0), TADV and ITR.
  * Then rearm the timer.
  */
-mit_delay = 0;
 if (s->mit_ide &&
 (pending_ints & (E1000_ICR_TXQE | E1000_ICR_TXDW))) {
 mit_update_delay(&mit_delay, s->mac_reg[TADV] * 4);
@@ -332,13 +340,14 @@ set_interrupt_cause(E1000State *s, int index, uint32_t 
val)
 }
 mit_update_delay(&mit_delay, s->mac_reg[ITR]);
 
-if (mit_delay) {
-s->mit_timer_on = 1;
-timer_mod(s->mit_timer, qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL) +
-  mit_delay * 256);
-}
 s->mit_ide = 0;
 }
+
+if (mit_delay) {
+s->mit_timer_on = 1;
+timer_mod(s->mit_timer, qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL) +
+  mit_delay * 256);
+}
 }
 
 s->mit_irq_level = (pending_ints != 0);
@@ -411,6 +420,7 @@ static void e1000_reset(void *opaque)
 d->mit_timer_on = 0;
 d->mit_irq_level = 0;
 d->mit_ide = 0;
+d->win8_workaround_needed = false;
 memset(d->phy_reg, 0, sizeof d->phy_reg);
 memmove(d->phy_reg, phy_reg_init, sizeof phy_reg_init);
 d->phy_reg[PHY_ID2] = edc->phy_id2;
@@ -1114,6 +1124,8 @@ mac_icr_read(E1000State *s, int index)
 {
 uint32_t ret = s->mac_reg[ICR];
 
+s->win8_workaround_needed &= ret == E1000_WIN8_WORKAROUND_ICR;
+
 DBGOUT(INTERRUPT, "ICR read: %x\n", ret);
 set_interrupt_cause(s, 0, 0);
 return ret;
@@ -1192,6 +1204,7 @@ static void
 set_imc(E1000State *s, int index, uint32_t val)
 {
 s->mac_reg[IMS] &= ~val;
+s->win8_workaround_needed = ~val == 0;
 set_ics(s, 0, 0);
 }
 
@@ -1199,7 +1212,9 @@ static void
 set_ims(E1000State 

Re: [Qemu-devel] [PATCH RFC v3 13/14] iotests: add dirty bitmap migration test

[John Snow]

On 02/18/2015 09:00 AM, Vladimir Sementsov-Ogievskiy wrote:

[the same test as 117, but not using qmp: query-block-dirty-bitmap.
only one test from {117, 118} will be in the next patch set version]

The test starts two vms (vm_a, vm_b), create dirty bitmap in
the first one, do several writes to corresponding device and
then migrate vm_a to vm_b with dirty bitmaps.

Signed-off-by: Vladimir Sementsov-Ogievskiy 
---
  tests/qemu-iotests/118 | 87 ++
  tests/qemu-iotests/118.out |  5 +++
  tests/qemu-iotests/group   |  1 +
  3 files changed, 93 insertions(+)
  create mode 100755 tests/qemu-iotests/118
  create mode 100644 tests/qemu-iotests/118.out

diff --git a/tests/qemu-iotests/118 b/tests/qemu-iotests/118
new file mode 100755
index 000..a85ea56
--- /dev/null
+++ b/tests/qemu-iotests/118
@@ -0,0 +1,87 @@
+#!/usr/bin/env python
+#
+# Tests for dirty bitmaps migration.
+#
+# (C) Vladimir Sementsov-Ogievskiy 2015
+#
+# 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 .
+#
+
+import os
+import iotests
+import time
+from iotests import qemu_img
+
+disk_a = os.path.join(iotests.test_dir, 'disk_a')
+disk_b = os.path.join(iotests.test_dir, 'disk_b')
+fifo   = os.path.join(iotests.test_dir, 'fifo')
+
+size   = 0x4000 # 1G
+sector_size = 512
+granularity = 0x1
+regions = [
+{ 'start': 0,  'count': 0x10 },
+{ 'start': 0x1000, 'count': 0x2  },
+{ 'start': 0x3999, 'count': 0x1  }
+]
+
+regions_in_sectors = [
+{ key: val / sector_size for (key, val) in el.items() } for el in regions]
+


Not used in this test.


+class TestDirtyBitmapMigration(iotests.QMPTestCase):
+
+def setUp(self):
+os.mkfifo(fifo)
+qemu_img('create', '-f', iotests.imgfmt, disk_a, str(size))
+qemu_img('create', '-f', iotests.imgfmt, disk_b, str(size))
+self.vm_a = iotests.VM().add_drive(disk_a)
+self.vm_b = iotests.VM().add_drive(disk_b)
+self.vm_b.add_incoming_migration("exec: cat " + fifo)
+self.vm_a.launch()
+self.vm_b.launch()
+
+def tearDown(self):
+self.vm_a.shutdown()
+self.vm_b.shutdown()
+os.remove(disk_a)
+os.remove(disk_b)
+os.remove(fifo)
+
+def test_migration(self):
+result = self.vm_a.qmp('block-dirty-bitmap-add', node='drive0',
+   name='bitmap', granularity=granularity)
+self.assert_qmp(result, 'return', {});
+
+for r in regions:
+  self.vm_a.hmp_qemu_io('drive0',
+'write %d %d' % (r['start'], r['count']))
+
+result = self.vm_a.qmp('query-block');
+md5 = result['return'][0]['dirty-bitmaps'][0]['md5']
+
+result = self.vm_a.qmp('migrate-set-capabilities',
+   capabilities=[{'capability': 'dirty-bitmaps',
+  'state': True}])
+self.assert_qmp(result, 'return', {})
+
+result = self.vm_a.qmp('migrate', uri='exec:cat>' + fifo)
+while self.vm_a.qmp('query-migrate')['return']['status'] != 
'completed':
+  time.sleep(1)
+


Might be possible to wait on a status completion here. Adding a method 
to VM:


def event_wait(self, name='BLOCK_JOB_COMPLETED', maxwait=10):
for _ in range(maxwait):
for event in self.get_qmp_events(wait=True):
if event['event'] == name:
return event
return None


And replacing the sleep and poll loop with:

result = self.vm_a.qmp('migrate', uri='exec:cat>' + fifo)
self.assertIsNotNone(self.vm_a.event_wait("STOP"))
self.assertIsNotNone(self.vm_b.event_wait("RESUME"))

works just as well without the poll/sleep.


+result = self.vm_b.qmp('query-block');
+self.assert_qmp(result, 'return[0]/dirty-bitmaps[0]/md5', md5);
+
+
+if __name__ == '__main__':
+iotests.main()
diff --git a/tests/qemu-iotests/118.out b/tests/qemu-iotests/118.out
new file mode 100644
index 000..ae1213e
--- /dev/null
+++ b/tests/qemu-iotests/118.out
@@ -0,0 +1,5 @@
+.
+--
+Ran 1 tests
+
+OK
diff --git a/tests/qemu-iotests/group b/tests/qemu-iotests/group
index 6ad5b55..1fd1983 100644
--- a/tests/qemu-iotests/group
+++ b/tests/qemu-iotests/group
@@ -119,3 +119,4 @@
  114 rw auto quick
  116 rw auto quick
  1

Re: [Qemu-devel] [PATCH] Makefile.target: set icon for binary file on Mac OS X

[Programmingkid]

On Feb 19, 2015, at 4:56 AM, Paolo Bonzini wrote:

> 
> 
> On 18/02/2015 22:09, Programmingkid wrote:
>> +# Take an image and make the image its own icon:
>> +sips -i ../pc-bios/qemu-nsis.ico
>> +# Extract the icon to its own resource file:
>> +DeRez -only icns ../pc-bios/qemu-nsis.ico > tmpicns.rsrc
> 
> IIUC sips modifies ../pc-bios/qemu-nsis.ico (adding a resource fork?),
> so it's not possible to put it in Makefile.target.  If "sips" is invoked
> twice by two different recursive invocations of Makefile.target, bad
> things can happen.
> 
> I think we can simply distribute tmpicns.rsrc as pc-bios/qemu.rsrc instead.
> 
>> +# append this resource to the file you want to icon-ize.
>> +Rez -append tmpicns.rsrc -o $(QEMU_PROG)
>> +
>> +# Use the resource to set the icon.
>> +SetFile -a C $(QEMU_PROG)
> 
> These two commands can be added after
> 
> $(QEMU_PROG_BUILD): $(all-obj-y) ../libqemuutil.a ../libqemustub.a
>$(call LINK,$^)
> 
> instead of adding a new rule.  There is no need for the comments.
> 
> Paolo

Ok. Will implement your suggested changes. Thank you.

Re: [Qemu-devel] [PATCH] e1000: work around win 8.0 boot hang

[Radim Krčmář]

2015-02-19 20:24+0100, Radim Krčmář:
> diff --git a/hw/net/e1000.c b/hw/net/e1000.c
> @@ -138,6 +138,10 @@ typedef struct E1000State_st {
> +#define E1000_WIN8_WORKAROUND_ICR   E1000_ICR_RXT0
> +#define E1000_WIN8_WORKAROUND_DELAY_US  10
> +bool win8_workaround_needed;
> @@ -288,7 +292,7 @@ set_interrupt_cause(E1000State *s, int index, uint32_t 
> val)
> @@ -316,13 +320,17 @@ set_interrupt_cause(E1000State *s, int index, uint32_t 
> val)
> +if (s->win8_workround_needed) {

So I read the patch again and noticed a typo here, which reminds me that
QEMU does not compile on rawhide for several reasons ...
I'll fix that to compensate.

[Qemu-devel] [PATCH 00/11] target-aarch64 fix and improvments

[Richard Henderson]

While doing the mechanics of a previous patch set converting
translators to use to TCGLabel pointers, I was reminded of
several outstanding OPTME comments in the aarch64 translator.

I had started with the csel change, which at first failed and
took quite some time to debug.  See the comment for patch 1.

Since this depends on the outstanding TCGLabel patch set, the
full tree is available at

  git://github.com/rth7680/qemu.git arm-movcond


r~


Richard Henderson (11):
  target-arm: Introduce DisasCompare
  target-arm: Extend NZCF to 64 bits
  target-arm: Handle always condition codes within arm_test_cc
  target-arm: Recognize SXTB, SXTH, SXTW, ASR
  target-arm: Recognize UXTB, UXTH, LSR, LSL
  target-arm: Eliminate unnecessary zero-extend in disas_bitfield
  target-arm: Recognize ROR
  target-arm: Use setcond and movcond for csel
  target-arm: Implement ccmp branchless
  target-arm: Implement fccmp branchless
  target-arm: Implement fcsel with movcond

 target-arm/cpu.h   |  21 +-
 target-arm/helper.c|  18 +-
 target-arm/translate-a64.c | 688 ++---
 target-arm/translate.c | 151 ++
 target-arm/translate.h |   2 -
 5 files changed, 524 insertions(+), 356 deletions(-)

-- 
2.1.0

[Qemu-devel] [PATCH 03/11] target-arm: Handle always condition codes within arm_test_cc

[Richard Henderson]

Handling this with TCG_COND_ALWAYS will allow these unlikely
cases to be handled without special cases in the rest of the
translator.  The TCG optimizer ought to be able to reduce
these ALWAYS conditions completely.

Signed-off-by: Richard Henderson 
---
 target-arm/translate-a64.c | 9 +
 target-arm/translate.c | 9 +
 2 files changed, 18 insertions(+)

diff --git a/target-arm/translate-a64.c b/target-arm/translate-a64.c
index 763bf35..219e257 100644
--- a/target-arm/translate-a64.c
+++ b/target-arm/translate-a64.c
@@ -1056,6 +1056,14 @@ static void arm_test_cc(DisasCompare *cmp, int cc)
 tcg_gen_andc_i64(value, cpu_ZF, value);
 break;
 
+case 14: /* always */
+case 15: /* always */
+/* Use the ALWAYS condition, which will fold early.
+   It doesn't matter what we use for the value.  */
+cond = TCG_COND_ALWAYS;
+value = cpu_ZF;
+goto no_invert;
+
 default:
 fprintf(stderr, "Bad condition code 0x%x\n", cc);
 abort();
@@ -1065,6 +1073,7 @@ static void arm_test_cc(DisasCompare *cmp, int cc)
 cond = tcg_invert_cond(cond);
 }
 
+ no_invert:
 cmp->cond = cond;
 cmp->value = value;
 cmp->value_global = global;
diff --git a/target-arm/translate.c b/target-arm/translate.c
index 0d0a4d1..54edc33 100644
--- a/target-arm/translate.c
+++ b/target-arm/translate.c
@@ -816,6 +816,14 @@ static void arm_test_cc(DisasCompare *cmp, int cc)
 tcg_gen_andc_i32(value, cpu_ZF, value);
 break;
 
+case 14: /* always */
+case 15: /* always */
+/* Use the ALWAYS condition, which will fold early.
+   It doesn't matter what we use for the value.  */
+cond = TCG_COND_ALWAYS;
+value = cpu_ZF;
+goto no_invert;
+
 default:
 fprintf(stderr, "Bad condition code 0x%x\n", cc);
 abort();
@@ -825,6 +833,7 @@ static void arm_test_cc(DisasCompare *cmp, int cc)
 cond = tcg_invert_cond(cond);
 }
 
+ no_invert:
 cmp->cond = cond;
 cmp->value = value;
 cmp->value_global = global;
-- 
2.1.0

[Qemu-devel] [PATCH 01/11] target-arm: Introduce DisasCompare

[Richard Henderson]

Splitting arm_gen_test_cc into 3 functions, so that it can be reused
for non-branch TCG comparisons.

Note that this is also a bug fix for aarch64.  At present, we have branches
using the 32-bit (translate.c) versions of cpu_[NZCV]F, but we set the flags
using the 64-bit (translate-a64.c) versions of cpu_[NZCV]F.  From the view
of the TCG code generator, these are unrelated variables.

The bug is hard to see because we currently only read these variables from
branches, and upon reaching a branch TCG will first spill live variables and
then reload the arguments of the branch.  Since the 32-bit versions were
never live until reaching the branch, we'd re-read the data that had just
been spilled from the 64-bit versions.

Accept the code duplication for now, as the 64-bit functions will diverge.

Signed-off-by: Richard Henderson 
---
 target-arm/translate-a64.c | 102 +++
 target-arm/translate.c | 116 +++--
 target-arm/translate.h |   2 -
 3 files changed, 172 insertions(+), 48 deletions(-)

diff --git a/target-arm/translate-a64.c b/target-arm/translate-a64.c
index 0b192a1..dbca12a 100644
--- a/target-arm/translate-a64.c
+++ b/target-arm/translate-a64.c
@@ -1036,6 +1036,108 @@ static inline void gen_check_sp_alignment(DisasContext 
*s)
  */
 }
 
+typedef struct DisasCompare {
+TCGCond cond;
+TCGv_i32 value;
+bool value_global;
+} DisasCompare;
+
+/*
+ * generate a conditional based on ARM condition code cc.
+ * This is common between ARM and Aarch64 targets.
+ */
+static void arm_test_cc(DisasCompare *cmp, int cc)
+{
+TCGv_i32 value;
+TCGCond cond;
+bool global = true;
+
+switch (cc) {
+case 0: /* eq: Z */
+case 1: /* ne: !Z */
+cond = TCG_COND_EQ;
+value = cpu_ZF;
+break;
+
+case 2: /* cs: C */
+case 3: /* cc: !C */
+cond = TCG_COND_NE;
+value = cpu_CF;
+break;
+
+case 4: /* mi: N */
+case 5: /* pl: !N */
+cond = TCG_COND_LT;
+value = cpu_NF;
+break;
+
+case 6: /* vs: V */
+case 7: /* vc: !V */
+cond = TCG_COND_LT;
+value = cpu_VF;
+break;
+
+case 8: /* hi: C && !Z */
+case 9: /* ls: !C || Z */
+cond = TCG_COND_NE;
+value = tcg_temp_new_i32();
+global = false;
+tcg_gen_neg_i32(value, cpu_CF);
+tcg_gen_and_i32(value, value, cpu_ZF);
+break;
+
+case 10: /* ge: N == V -> N ^ V == 0 */
+case 11: /* lt: N != V -> N ^ V != 0 */
+cond = TCG_COND_GE;
+value = tcg_temp_new_i32();
+global = false;
+tcg_gen_xor_i32(value, cpu_VF, cpu_NF);
+break;
+
+case 12: /* gt: !Z && N == V */
+case 13: /* le: Z || N != V */
+cond = TCG_COND_NE;
+value = tcg_temp_new_i32();
+global = false;
+tcg_gen_xor_i32(value, cpu_VF, cpu_NF);
+tcg_gen_sari_i32(value, value, 31);
+tcg_gen_andc_i32(value, cpu_ZF, value);
+break;
+
+default:
+fprintf(stderr, "Bad condition code 0x%x\n", cc);
+abort();
+}
+
+if (cc & 1) {
+cond = tcg_invert_cond(cond);
+}
+
+cmp->cond = cond;
+cmp->value = value;
+cmp->value_global = global;
+}
+
+static void arm_free_cc(DisasCompare *cmp)
+{
+if (!cmp->value_global) {
+tcg_temp_free_i32(cmp->value);
+}
+}
+
+static void arm_jump_cc(DisasCompare *cmp, TCGLabel *label)
+{
+tcg_gen_brcondi_i32(cmp->cond, cmp->value, 0, label);
+}
+
+static void arm_gen_test_cc(int cc, TCGLabel *label)
+{
+DisasCompare cmp;
+arm_test_cc(&cmp, cc);
+arm_jump_cc(&cmp, label);
+arm_free_cc(&cmp);
+}
+
 /*
  * This provides a simple table based table lookup decoder. It is
  * intended to be used when the relevant bits for decode are too
diff --git a/target-arm/translate.c b/target-arm/translate.c
index 381d896..dd4d80f 100644
--- a/target-arm/translate.c
+++ b/target-arm/translate.c
@@ -732,82 +732,106 @@ static void gen_thumb2_parallel_addsub(int op1, int op2, 
TCGv_i32 a, TCGv_i32 b)
 }
 #undef PAS_OP
 
+typedef struct DisasCompare {
+TCGCond cond;
+TCGv_i32 value;
+bool value_global;
+} DisasCompare;
+
 /*
- * generate a conditional branch based on ARM condition code cc.
+ * generate a conditional based on ARM condition code cc.
  * This is common between ARM and Aarch64 targets.
  */
-void arm_gen_test_cc(int cc, TCGLabel *label)
+static void arm_test_cc(DisasCompare *cmp, int cc)
 {
-TCGv_i32 tmp;
-TCGLabel *inv;
+TCGv_i32 value;
+TCGCond cond;
+bool global = true;
 
 switch (cc) {
 case 0: /* eq: Z */
-tcg_gen_brcondi_i32(TCG_COND_EQ, cpu_ZF, 0, label);
-break;
 case 1: /* ne: !Z */
-tcg_gen_brcondi_i32(TCG_COND_NE, cpu_ZF, 0, label);
+cond = TCG_COND_EQ;
+value = cpu_ZF;
 break;
+
 case 2: /* cs: C */
-tcg_gen_brcondi_i32(TCG_COND_NE, cpu_C

[Qemu-devel] [PATCH 04/11] target-arm: Recognize SXTB, SXTH, SXTW, ASR

[Richard Henderson]

... as aliases of SBFM, and special-case them.

Signed-off-by: Richard Henderson 
---
 target-arm/translate-a64.c | 24 +++-
 1 file changed, 23 insertions(+), 1 deletion(-)

diff --git a/target-arm/translate-a64.c b/target-arm/translate-a64.c
index 219e257..0cb60a2 100644
--- a/target-arm/translate-a64.c
+++ b/target-arm/translate-a64.c
@@ -3036,7 +3036,28 @@ static void disas_bitfield(DisasContext *s, uint32_t 
insn)
 tcg_rd = cpu_reg(s, rd);
 tcg_tmp = read_cpu_reg(s, rn, sf);
 
-/* OPTME: probably worth recognizing common cases of ext{8,16,32}{u,s} */
+/* Recognize the common aliases.  */
+if (opc == 0) { /* SBFM */
+if (ri == 0) {
+if (si == 7) { /* SXTB */
+tcg_gen_ext8s_i64(tcg_rd, tcg_tmp);
+goto done;
+} else if (si == 15) { /* SXTH */
+tcg_gen_ext16s_i64(tcg_rd, tcg_tmp);
+goto done;
+} else if (si == 31) { /* SXTW */
+tcg_gen_ext32s_i64(tcg_rd, tcg_tmp);
+goto done;
+}
+}
+if (si == 63 || (si == 31 && ri <= si)) { /* ASR */
+if (si == 31) {
+tcg_gen_ext32s_i64(tcg_tmp, tcg_tmp);
+}
+tcg_gen_sari_i64(tcg_rd, tcg_tmp, ri);
+goto done;
+}
+}
 
 if (opc != 1) { /* SBFM or UBFM */
 tcg_gen_movi_i64(tcg_rd, 0);
@@ -3061,6 +3082,7 @@ static void disas_bitfield(DisasContext *s, uint32_t insn)
 tcg_gen_sari_i64(tcg_rd, tcg_rd, 64 - (pos + len));
 }
 
+ done:
 if (!sf) { /* zero extend final result */
 tcg_gen_ext32u_i64(tcg_rd, tcg_rd);
 }
-- 
2.1.0

[Qemu-devel] [PATCH 05/11] target-arm: Recognize UXTB, UXTH, LSR, LSL

[Richard Henderson]

Signed-off-by: Richard Henderson 
---
 target-arm/translate-a64.c | 17 +
 1 file changed, 17 insertions(+)

diff --git a/target-arm/translate-a64.c b/target-arm/translate-a64.c
index 0cb60a2..54290ad 100644
--- a/target-arm/translate-a64.c
+++ b/target-arm/translate-a64.c
@@ -3057,6 +3057,23 @@ static void disas_bitfield(DisasContext *s, uint32_t 
insn)
 tcg_gen_sari_i64(tcg_rd, tcg_tmp, ri);
 goto done;
 }
+} else if (opc == 2) { /* UBFM */
+if (ri == 0) { /* UXTB, UXTH, plus non-canonical AND */
+tcg_gen_andi_i64(tcg_rd, tcg_tmp, bitmask64(si + 1));
+return;
+}
+if (si == 63 || (si == 31 && ri <= si)) { /* LSR */
+if (si == 31) {
+tcg_gen_ext32u_i64(tcg_tmp, tcg_tmp);
+}
+tcg_gen_shri_i64(tcg_rd, tcg_tmp, ri);
+return;
+}
+if (si + 1 == ri && si != bitsize - 1) { /* LSL */
+int shift = bitsize - 1 - si;
+tcg_gen_shli_i64(tcg_rd, tcg_tmp, shift);
+goto done;
+}
 }
 
 if (opc != 1) { /* SBFM or UBFM */
-- 
2.1.0

[Qemu-devel] [PATCH 06/11] target-arm: Eliminate unnecessary zero-extend in disas_bitfield

[Richard Henderson]

For !SF, this initial ext32u can't be optimized away by the
current TCG code generator.  (It would require backward bit
liveness propagation.)

But since the range of bits for !SF are already constrained by
unallocated_encoding, we'll never reference the high bits anyway.

Signed-off-by: Richard Henderson 
---
 target-arm/translate-a64.c | 6 +-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/target-arm/translate-a64.c b/target-arm/translate-a64.c
index 54290ad..ed97ed6 100644
--- a/target-arm/translate-a64.c
+++ b/target-arm/translate-a64.c
@@ -3034,7 +3034,11 @@ static void disas_bitfield(DisasContext *s, uint32_t 
insn)
 }
 
 tcg_rd = cpu_reg(s, rd);
-tcg_tmp = read_cpu_reg(s, rn, sf);
+
+/* Suppress the zero-extend for !sf.  Since RI and SI are constrained
+   to be smaller than bitsize, we'll never reference data outside the
+   low 32-bits anyway.  */
+tcg_tmp = read_cpu_reg(s, rn, 1);
 
 /* Recognize the common aliases.  */
 if (opc == 0) { /* SBFM */
-- 
2.1.0

[Qemu-devel] [PATCH 10/11] target-arm: Implement fccmp branchless

[Richard Henderson]

Signed-off-by: Richard Henderson 
---
 target-arm/translate-a64.c | 50 +++---
 1 file changed, 29 insertions(+), 21 deletions(-)

diff --git a/target-arm/translate-a64.c b/target-arm/translate-a64.c
index 8171a1f..5539ae3 100644
--- a/target-arm/translate-a64.c
+++ b/target-arm/translate-a64.c
@@ -4128,11 +4128,11 @@ static void disas_data_proc_reg(DisasContext *s, 
uint32_t insn)
 }
 }
 
-static void handle_fp_compare(DisasContext *s, bool is_double,
-  unsigned int rn, unsigned int rm,
-  bool cmp_with_zero, bool signal_all_nans)
+static void handle_fp_compare_1(DisasContext *s, TCGv_i64 tcg_flags,
+bool is_double, unsigned int rn,
+unsigned int rm, bool cmp_with_zero,
+bool signal_all_nans)
 {
-TCGv_i64 tcg_flags = tcg_temp_new_i64();
 TCGv_ptr fpst = get_fpstatus_ptr();
 
 if (is_double) {
@@ -4170,7 +4170,16 @@ static void handle_fp_compare(DisasContext *s, bool 
is_double,
 }
 
 tcg_temp_free_ptr(fpst);
+}
 
+static void handle_fp_compare(DisasContext *s, bool is_double,
+  unsigned int rn, unsigned int rm,
+  bool cmp_with_zero, bool signal_all_nans)
+{
+TCGv_i64 tcg_flags = tcg_temp_new_i64();
+
+handle_fp_compare_1(s, tcg_flags, is_double, rn, rm,
+cmp_with_zero, signal_all_nans);
 gen_set_nzcv(tcg_flags);
 
 tcg_temp_free_i64(tcg_flags);
@@ -4215,8 +4224,8 @@ static void disas_fp_compare(DisasContext *s, uint32_t 
insn)
 static void disas_fp_ccomp(DisasContext *s, uint32_t insn)
 {
 unsigned int mos, type, rm, cond, rn, op, nzcv;
-TCGv_i64 tcg_flags;
-TCGLabel *label_continue = NULL;
+TCGv_i64 t_flags, t_zero, t_nzcv;
+DisasCompare c;
 
 mos = extract32(insn, 29, 3);
 type = extract32(insn, 22, 2); /* 0 = single, 1 = double */
@@ -4235,23 +4244,22 @@ static void disas_fp_ccomp(DisasContext *s, uint32_t 
insn)
 return;
 }
 
-if (cond < 0x0e) { /* not always */
-TCGLabel *label_match = gen_new_label();
-label_continue = gen_new_label();
-arm_gen_test_cc(cond, label_match);
-/* nomatch: */
-tcg_flags = tcg_const_i64(nzcv << 28);
-gen_set_nzcv(tcg_flags);
-tcg_temp_free_i64(tcg_flags);
-tcg_gen_br(label_continue);
-gen_set_label(label_match);
-}
+/* Perform the new compare, but don't write the result back to flags. */
+t_flags = tcg_temp_new_i64();
+handle_fp_compare_1(s, t_flags, type, rn, rm, false, op);
 
-handle_fp_compare(s, type, rn, rm, false, op);
+/* If the condition is false, force the flags to #nzcv.  */
+arm_test_cc(&c, cond);
+t_zero = tcg_const_i64(0);
+t_nzcv = tcg_const_i64(nzcv << 28);
+tcg_gen_movcond_i64(c.cond, t_flags, c.value, t_zero, t_flags, t_nzcv);
+tcg_temp_free_i64(t_zero);
+tcg_temp_free_i64(t_nzcv);
+arm_free_cc(&c);
 
-if (cond < 0x0e) {
-gen_set_label(label_continue);
-}
+/* Write back the new flags.  */
+gen_set_nzcv(t_flags);
+tcg_temp_free_i64(t_flags);
 }
 
 /* copy src FP register to dst FP register; type specifies single or double */
-- 
2.1.0

[Qemu-devel] [PATCH 09/11] target-arm: Implement ccmp branchless

[Richard Henderson]

This can allow much of a ccmp to be elided when particular
flags are subsequently dead.

Signed-off-by: Richard Henderson 
---
 target-arm/translate-a64.c | 62 ++
 1 file changed, 41 insertions(+), 21 deletions(-)

diff --git a/target-arm/translate-a64.c b/target-arm/translate-a64.c
index 7549267..8171a1f 100644
--- a/target-arm/translate-a64.c
+++ b/target-arm/translate-a64.c
@@ -3641,8 +3641,8 @@ static void disas_adc_sbc(DisasContext *s, uint32_t insn)
 static void disas_cc(DisasContext *s, uint32_t insn)
 {
 unsigned int sf, op, y, cond, rn, nzcv, is_imm;
-TCGLabel *label_continue = NULL;
-TCGv_i64 tcg_tmp, tcg_y, tcg_rn;
+TCGv_i64 tcg_t0, tcg_t1, tcg_t2, tcg_y, tcg_rn;
+DisasCompare c;
 
 if (!extract32(insn, 29, 1)) {
 unallocated_encoding(s);
@@ -3660,19 +3660,13 @@ static void disas_cc(DisasContext *s, uint32_t insn)
 rn = extract32(insn, 5, 5);
 nzcv = extract32(insn, 0, 4);
 
-if (cond < 0x0e) { /* not always */
-TCGLabel *label_match = gen_new_label();
-label_continue = gen_new_label();
-arm_gen_test_cc(cond, label_match);
-/* nomatch: */
-tcg_tmp = tcg_temp_new_i64();
-tcg_gen_movi_i64(tcg_tmp, nzcv << 28);
-gen_set_nzcv(tcg_tmp);
-tcg_temp_free_i64(tcg_tmp);
-tcg_gen_br(label_continue);
-gen_set_label(label_match);
-}
-/* match, or condition is always */
+/* Set T0 = !COND.  */
+tcg_t0 = tcg_temp_new_i64();
+arm_test_cc(&c, cond);
+tcg_gen_setcondi_i64(tcg_invert_cond(c.cond), tcg_t0, c.value, 0);
+arm_free_cc(&c);
+
+/* Load the arguments for the new comparison.  */
 if (is_imm) {
 tcg_y = new_tmp_a64(s);
 tcg_gen_movi_i64(tcg_y, y);
@@ -3681,17 +3675,43 @@ static void disas_cc(DisasContext *s, uint32_t insn)
 }
 tcg_rn = cpu_reg(s, rn);
 
-tcg_tmp = tcg_temp_new_i64();
+/* Set the flags for the new comparison.  */
+tcg_t1 = tcg_temp_new_i64();
 if (op) {
-gen_sub_CC(sf, tcg_tmp, tcg_rn, tcg_y);
+gen_sub_CC(sf, tcg_t1, tcg_rn, tcg_y);
 } else {
-gen_add_CC(sf, tcg_tmp, tcg_rn, tcg_y);
+gen_add_CC(sf, tcg_t1, tcg_rn, tcg_y);
 }
-tcg_temp_free_i64(tcg_tmp);
 
-if (cond < 0x0e) { /* continue */
-gen_set_label(label_continue);
+/* If COND was false, force the flags to #nzcv.
+   Note that T1 = (COND ? 0 : -1), T2 = (COND ? -1 : 0).  */
+tcg_t2 = tcg_temp_new_i64();
+tcg_gen_neg_i64(tcg_t1, tcg_t0);
+tcg_gen_subi_i64(tcg_t2, tcg_t0, 1);
+
+if (nzcv & 8) { /* N */
+tcg_gen_or_i64(cpu_NF, cpu_NF, tcg_t1);
+} else {
+tcg_gen_and_i64(cpu_NF, cpu_NF, tcg_t2);
+}
+if (nzcv & 4) { /* Z */
+tcg_gen_and_i64(cpu_ZF, cpu_ZF, tcg_t2);
+} else {
+tcg_gen_or_i64(cpu_ZF, cpu_ZF, tcg_t0);
+}
+if (nzcv & 2) { /* C */
+tcg_gen_or_i64(cpu_CF, cpu_CF, tcg_t0);
+} else {
+tcg_gen_and_i64(cpu_CF, cpu_CF, tcg_t2);
+}
+if (nzcv & 1) { /* V */
+tcg_gen_or_i64(cpu_VF, cpu_VF, tcg_t1);
+} else {
+tcg_gen_and_i64(cpu_VF, cpu_VF, tcg_t2);
 }
+tcg_temp_free_i64(tcg_t0);
+tcg_temp_free_i64(tcg_t1);
+tcg_temp_free_i64(tcg_t2);
 }
 
 /* C3.5.6 Conditional select
-- 
2.1.0

[Qemu-devel] [PATCH 02/11] target-arm: Extend NZCF to 64 bits

[Richard Henderson]

The resulting aarch64 translation is a bit cleaner.
Sign-extending from 32-bits is simpler than having
to use setcond to narrow from 64-bits.

Signed-off-by: Richard Henderson 
---
 target-arm/cpu.h   |  21 ++--
 target-arm/helper.c|  18 ++-
 target-arm/translate-a64.c | 297 ++---
 target-arm/translate.c |  26 +++-
 4 files changed, 163 insertions(+), 199 deletions(-)

diff --git a/target-arm/cpu.h b/target-arm/cpu.h
index 11845a6..74835f4 100644
--- a/target-arm/cpu.h
+++ b/target-arm/cpu.h
@@ -167,10 +167,10 @@ typedef struct CPUARMState {
 uint32_t fiq_regs[5];
 
 /* cpsr flag cache for faster execution */
-uint32_t CF; /* 0 or 1 */
-uint32_t VF; /* V is the bit 31. All other bits are undefined */
-uint32_t NF; /* N is bit 31. All other bits are undefined.  */
-uint32_t ZF; /* Z set if zero.  */
+uint64_t CF; /* 0 or 1 */
+uint64_t VF; /* V is the bit 63. All other bits are undefined */
+uint64_t NF; /* N is bit 63. All other bits are undefined.  */
+uint64_t ZF; /* Z set if zero.  */
 uint32_t QF; /* 0 or 1 */
 uint32_t GE; /* cpsr[19:16] */
 uint32_t thumb; /* cpsr[5]. 0 = arm mode, 1 = thumb mode. */
@@ -666,20 +666,21 @@ static inline unsigned int aarch64_pstate_mode(unsigned 
int el, bool handler)
  */
 static inline uint32_t pstate_read(CPUARMState *env)
 {
-int ZF;
+unsigned ZF = (env->ZF == 0);
+unsigned NF = ((int64_t)env->NF < 0);
+unsigned VF = ((int64_t)env->VF < 0);
+unsigned CF = env->CF;
 
-ZF = (env->ZF == 0);
-return (env->NF & 0x8000) | (ZF << 30)
-| (env->CF << 29) | ((env->VF & 0x8000) >> 3)
+return (NF << 31) | (ZF << 30) | (CF << 29) | (VF << 28)
 | env->pstate | env->daif;
 }
 
 static inline void pstate_write(CPUARMState *env, uint32_t val)
 {
+env->NF = (uint64_t)val << 32;
 env->ZF = (~val) & PSTATE_Z;
-env->NF = val;
 env->CF = (val >> 29) & 1;
-env->VF = (val << 3) & 0x8000;
+env->VF = (uint64_t)val << (32 + (31 - 28));
 env->daif = val & PSTATE_DAIF;
 env->pstate = val & ~CACHED_PSTATE_BITS;
 }
diff --git a/target-arm/helper.c b/target-arm/helper.c
index 3bc20af..1b28108 100644
--- a/target-arm/helper.c
+++ b/target-arm/helper.c
@@ -3876,10 +3876,13 @@ static int bad_mode_switch(CPUARMState *env, int mode)
 
 uint32_t cpsr_read(CPUARMState *env)
 {
-int ZF;
-ZF = (env->ZF == 0);
-return env->uncached_cpsr | (env->NF & 0x8000) | (ZF << 30) |
-(env->CF << 29) | ((env->VF & 0x8000) >> 3) | (env->QF << 27)
+unsigned ZF = (env->ZF == 0);
+unsigned NF = ((int64_t)env->NF < 0);
+unsigned VF = ((int64_t)env->VF < 0);
+unsigned CF = env->CF;
+
+return env->uncached_cpsr | (NF << 31) | (ZF << 30)
+| (CF << 29) | (VF << 28) | (env->QF << 27)
 | (env->thumb << 5) | ((env->condexec_bits & 3) << 25)
 | ((env->condexec_bits & 0xfc) << 8)
 | (env->GE << 16) | (env->daif & CPSR_AIF);
@@ -3890,10 +3893,10 @@ void cpsr_write(CPUARMState *env, uint32_t val, 
uint32_t mask)
 uint32_t changed_daif;
 
 if (mask & CPSR_NZCV) {
+env->NF = (uint64_t)val << 32;
 env->ZF = (~val) & CPSR_Z;
-env->NF = val;
 env->CF = (val >> 29) & 1;
-env->VF = (val << 3) & 0x8000;
+env->VF = (uint64_t)val << (32 + (31 - 28));
 }
 if (mask & CPSR_Q)
 env->QF = ((val & CPSR_Q) != 0);
@@ -4545,6 +4548,9 @@ void aarch64_sync_64_to_32(CPUARMState *env)
 env->regs[i] = env->xregs[i];
 }
 
+/* Need to compress Z into the low bits.  */
+env->ZF = (env->ZF != 0);
+
 /* Unless we are in FIQ mode, r8-r12 come from the user registers x8-x12.
  * Otherwise, we copy x8-x12 into the banked user regs.
  */
diff --git a/target-arm/translate-a64.c b/target-arm/translate-a64.c
index dbca12a..763bf35 100644
--- a/target-arm/translate-a64.c
+++ b/target-arm/translate-a64.c
@@ -39,7 +39,7 @@
 
 static TCGv_i64 cpu_X[32];
 static TCGv_i64 cpu_pc;
-static TCGv_i32 cpu_NF, cpu_ZF, cpu_CF, cpu_VF;
+static TCGv_i64 cpu_NF, cpu_ZF, cpu_CF, cpu_VF;
 
 /* Load/store exclusive handling */
 static TCGv_i64 cpu_exclusive_addr;
@@ -104,10 +104,10 @@ void a64_translate_init(void)
   regnames[i]);
 }
 
-cpu_NF = tcg_global_mem_new_i32(TCG_AREG0, offsetof(CPUARMState, NF), 
"NF");
-cpu_ZF = tcg_global_mem_new_i32(TCG_AREG0, offsetof(CPUARMState, ZF), 
"ZF");
-cpu_CF = tcg_global_mem_new_i32(TCG_AREG0, offsetof(CPUARMState, CF), 
"CF");
-cpu_VF = tcg_global_mem_new_i32(TCG_AREG0, offsetof(CPUARMState, VF), 
"VF");
+cpu_NF = tcg_global_mem_new_i64(TCG_AREG0, offsetof(CPUARMState, NF), 
"NF");
+cpu_ZF = tcg_global_mem_new_i64(TCG_AREG0, offsetof(CPUARMState, ZF), 
"ZF");
+cpu_CF = tcg_global_mem_new_i64(TCG_AREG0, offsetof(CPUARMState, CF), 
"CF");
+cpu_VF = tcg_global_mem_new_i64(TCG_AREG0, offseto

[Qemu-devel] [PATCH 07/11] target-arm: Recognize ROR

[Richard Henderson]

Signed-off-by: Richard Henderson 
---
 target-arm/translate-a64.c | 33 +
 1 file changed, 21 insertions(+), 12 deletions(-)

diff --git a/target-arm/translate-a64.c b/target-arm/translate-a64.c
index ed97ed6..d139b2d 100644
--- a/target-arm/translate-a64.c
+++ b/target-arm/translate-a64.c
@@ -3136,17 +3136,7 @@ static void disas_extract(DisasContext *s, uint32_t insn)
 
 tcg_rd = cpu_reg(s, rd);
 
-if (imm) {
-/* OPTME: we can special case rm==rn as a rotate */
-tcg_rm = read_cpu_reg(s, rm, sf);
-tcg_rn = read_cpu_reg(s, rn, sf);
-tcg_gen_shri_i64(tcg_rm, tcg_rm, imm);
-tcg_gen_shli_i64(tcg_rn, tcg_rn, bitsize - imm);
-tcg_gen_or_i64(tcg_rd, tcg_rm, tcg_rn);
-if (!sf) {
-tcg_gen_ext32u_i64(tcg_rd, tcg_rd);
-}
-} else {
+if (unlikely(imm == 0)) {
 /* tcg shl_i32/shl_i64 is undefined for 32/64 bit shifts,
  * so an extract from bit 0 is a special case.
  */
@@ -3155,8 +3145,27 @@ static void disas_extract(DisasContext *s, uint32_t insn)
 } else {
 tcg_gen_ext32u_i64(tcg_rd, cpu_reg(s, rm));
 }
+} else if (rm == rn) { /* ROR */
+tcg_rm = cpu_reg(s, rm);
+if (sf) {
+tcg_gen_rotri_i64(tcg_rd, tcg_rm, imm);
+} else {
+TCGv_i32 tmp = tcg_temp_new_i32();
+tcg_gen_trunc_i64_i32(tmp, tcg_rm);
+tcg_gen_rotri_i32(tmp, tmp, imm);
+tcg_gen_extu_i32_i64(tcg_rd, tmp);
+tcg_temp_free_i32(tmp);
+}
+} else {
+tcg_rm = read_cpu_reg(s, rm, sf);
+tcg_rn = read_cpu_reg(s, rn, sf);
+tcg_gen_shri_i64(tcg_rm, tcg_rm, imm);
+tcg_gen_shli_i64(tcg_rn, tcg_rn, bitsize - imm);
+tcg_gen_or_i64(tcg_rd, tcg_rm, tcg_rn);
+if (!sf) {
+tcg_gen_ext32u_i64(tcg_rd, tcg_rd);
+}
 }
-
 }
 }
 
-- 
2.1.0

[Qemu-devel] [PATCH 08/11] target-arm: Use setcond and movcond for csel

[Richard Henderson]

Signed-off-by: Richard Henderson 
---
 target-arm/translate-a64.c | 60 +++---
 1 file changed, 24 insertions(+), 36 deletions(-)

diff --git a/target-arm/translate-a64.c b/target-arm/translate-a64.c
index d139b2d..7549267 100644
--- a/target-arm/translate-a64.c
+++ b/target-arm/translate-a64.c
@@ -3703,7 +3703,8 @@ static void disas_cc(DisasContext *s, uint32_t insn)
 static void disas_cond_select(DisasContext *s, uint32_t insn)
 {
 unsigned int sf, else_inv, rm, cond, else_inc, rn, rd;
-TCGv_i64 tcg_rd, tcg_src;
+TCGv_i64 tcg_rd, zero;
+DisasCompare c;
 
 if (extract32(insn, 29, 1) || extract32(insn, 11, 1)) {
 /* S == 1 or op2<1> == 1 */
@@ -3718,48 +3719,35 @@ static void disas_cond_select(DisasContext *s, uint32_t 
insn)
 rn = extract32(insn, 5, 5);
 rd = extract32(insn, 0, 5);
 
-if (rd == 31) {
-/* silly no-op write; until we use movcond we must special-case
- * this to avoid a dead temporary across basic blocks.
- */
-return;
-}
-
 tcg_rd = cpu_reg(s, rd);
 
-if (cond >= 0x0e) { /* condition "always" */
-tcg_src = read_cpu_reg(s, rn, sf);
-tcg_gen_mov_i64(tcg_rd, tcg_src);
-} else {
-/* OPTME: we could use movcond here, at the cost of duplicating
- * a lot of the arm_gen_test_cc() logic.
- */
-TCGLabel *label_match = gen_new_label();
-TCGLabel *label_continue = gen_new_label();
-
-arm_gen_test_cc(cond, label_match);
-/* nomatch: */
-tcg_src = cpu_reg(s, rm);
+arm_test_cc(&c, cond);
+zero = tcg_const_i64(0);
 
+if (rn == 31 && rm == 31 && (else_inc ^ else_inv)) {
+/* CSET & CSETM.  */
+tcg_gen_setcond_i64(tcg_invert_cond(c.cond), tcg_rd, c.value, zero);
+if (else_inv) {
+tcg_gen_neg_i64(tcg_rd, tcg_rd);
+}
+} else {
+TCGv_i64 t_true = cpu_reg(s, rn);
+TCGv_i64 t_false = read_cpu_reg(s, rm, 1);
 if (else_inv && else_inc) {
-tcg_gen_neg_i64(tcg_rd, tcg_src);
+tcg_gen_neg_i64(t_false, t_false);
 } else if (else_inv) {
-tcg_gen_not_i64(tcg_rd, tcg_src);
+tcg_gen_not_i64(t_false, t_false);
 } else if (else_inc) {
-tcg_gen_addi_i64(tcg_rd, tcg_src, 1);
-} else {
-tcg_gen_mov_i64(tcg_rd, tcg_src);
-}
-if (!sf) {
-tcg_gen_ext32u_i64(tcg_rd, tcg_rd);
+tcg_gen_addi_i64(t_false, t_false, 1);
 }
-tcg_gen_br(label_continue);
-/* match: */
-gen_set_label(label_match);
-tcg_src = read_cpu_reg(s, rn, sf);
-tcg_gen_mov_i64(tcg_rd, tcg_src);
-/* continue: */
-gen_set_label(label_continue);
+tcg_gen_movcond_i64(c.cond, tcg_rd, c.value, zero, t_true, t_false);
+}
+
+tcg_temp_free_i64(zero);
+arm_free_cc(&c);
+
+if (!sf) {
+tcg_gen_ext32u_i64(tcg_rd, tcg_rd);
 }
 }
 
-- 
2.1.0

[Qemu-devel] [PATCH 11/11] target-arm: Implement fcsel with movcond

[Richard Henderson]

Signed-off-by: Richard Henderson 
---
 target-arm/translate-a64.c | 48 --
 1 file changed, 21 insertions(+), 27 deletions(-)

diff --git a/target-arm/translate-a64.c b/target-arm/translate-a64.c
index 5539ae3..1302cec 100644
--- a/target-arm/translate-a64.c
+++ b/target-arm/translate-a64.c
@@ -4262,20 +4262,6 @@ static void disas_fp_ccomp(DisasContext *s, uint32_t 
insn)
 tcg_temp_free_i64(t_flags);
 }
 
-/* copy src FP register to dst FP register; type specifies single or double */
-static void gen_mov_fp2fp(DisasContext *s, int type, int dst, int src)
-{
-if (type) {
-TCGv_i64 v = read_fp_dreg(s, src);
-write_fp_dreg(s, dst, v);
-tcg_temp_free_i64(v);
-} else {
-TCGv_i32 v = read_fp_sreg(s, src);
-write_fp_sreg(s, dst, v);
-tcg_temp_free_i32(v);
-}
-}
-
 /* C3.6.24 Floating point conditional select
  *   31  30  29 28   24 23  22  21 20  16 15  12 11 10 95 40
  * +---+---+---+---+--+---+--+--+-+--+--+
@@ -4285,7 +4271,8 @@ static void gen_mov_fp2fp(DisasContext *s, int type, int 
dst, int src)
 static void disas_fp_csel(DisasContext *s, uint32_t insn)
 {
 unsigned int mos, type, rm, cond, rn, rd;
-TCGLabel *label_continue = NULL;
+TCGv_i64 t_true, t_false, t_zero;
+DisasCompare c;
 
 mos = extract32(insn, 29, 3);
 type = extract32(insn, 22, 2); /* 0 = single, 1 = double */
@@ -4303,21 +4290,28 @@ static void disas_fp_csel(DisasContext *s, uint32_t 
insn)
 return;
 }
 
-if (cond < 0x0e) { /* not always */
-TCGLabel *label_match = gen_new_label();
-label_continue = gen_new_label();
-arm_gen_test_cc(cond, label_match);
-/* nomatch: */
-gen_mov_fp2fp(s, type, rd, rm);
-tcg_gen_br(label_continue);
-gen_set_label(label_match);
+if (type) {
+t_true = read_fp_dreg(s, rn);
+t_false = read_fp_dreg(s, rm);
+} else {
+/* Zero-extend sreg inputs to 64-bits now.  */
+t_true = tcg_temp_new_i64();
+t_false = tcg_temp_new_i64();
+tcg_gen_ld32u_i64(t_true, cpu_env, fp_reg_offset(s, rn, MO_32));
+tcg_gen_ld32u_i64(t_false, cpu_env, fp_reg_offset(s, rm, MO_32));
 }
 
-gen_mov_fp2fp(s, type, rd, rn);
+arm_test_cc(&c, cond);
+t_zero = tcg_const_i64(0);
+tcg_gen_movcond_i64(c.cond, t_true, c.value, t_zero, t_true, t_false);
+tcg_temp_free_i64(t_zero);
+tcg_temp_free_i64(t_false);
+arm_free_cc(&c);
 
-if (cond < 0x0e) { /* continue */
-gen_set_label(label_continue);
-}
+/* Note that sregs write back zeros to the high bits,
+   and we've already done the zero-extension.  */
+write_fp_dreg(s, rd, t_true);
+tcg_temp_free_i64(t_true);
 }
 
 /* C3.6.25 Floating-point data-processing (1 source) - single precision */
-- 
2.1.0

[Qemu-devel] [PATCH] target-cris: Use movcond and setcond

[Richard Henderson]

Signed-off-by: Richard Henderson 
---
 target-cris/translate.c | 27 +++
 target-cris/translate_v10.c | 12 ++--
 2 files changed, 9 insertions(+), 30 deletions(-)
---
Note that this is relative to an outstanding TCG patch set.
The full tree is at

  git://github.com/rth7680/qemu.git cris-movcond


r~
---

diff --git a/target-cris/translate.c b/target-cris/translate.c
index 687c88b..d7fd2ef 100644
--- a/target-cris/translate.c
+++ b/target-cris/translate.c
@@ -311,7 +311,7 @@ static void t_gen_asr(TCGv d, TCGv a, TCGv b)
 
 static void t_gen_cris_dstep(TCGv d, TCGv a, TCGv b)
 {
-TCGLabel *l1 = gen_new_label();
+TCGv t = tcg_temp_new();
 
 /*
  * d <<= 1
@@ -319,9 +319,9 @@ static void t_gen_cris_dstep(TCGv d, TCGv a, TCGv b)
  *d -= s;
  */
 tcg_gen_shli_tl(d, a, 1);
-tcg_gen_brcond_tl(TCG_COND_LTU, d, b, l1);
-tcg_gen_sub_tl(d, d, b);
-gen_set_label(l1);
+tcg_gen_sub_tl(t, d, b);
+tcg_gen_movcond_tl(TCG_COND_GEU, d, d, b, t, d);
+tcg_temp_free(t);
 }
 
 static void t_gen_cris_mstep(TCGv d, TCGv a, TCGv b, TCGv ccs)
@@ -769,13 +769,7 @@ static void cris_alu_op_exec(DisasContext *dc, int op,
 t_gen_cris_mstep(dst, a, b, cpu_PR[PR_CCS]);
 break;
 case CC_OP_BOUND:
-{
-TCGLabel *l1 = gen_new_label();
-tcg_gen_mov_tl(dst, a);
-tcg_gen_brcond_tl(TCG_COND_LEU, a, b, l1);
-tcg_gen_mov_tl(dst, b);
-gen_set_label(l1);
-}
+tcg_gen_movcond_tl(TCG_COND_LEU, dst, a, b, a, b);
 break;
 case CC_OP_CMP:
 tcg_gen_sub_tl(dst, a, b);
@@ -1482,15 +1476,8 @@ static int dec_scc_r(CPUCRISState *env, DisasContext *dc)
 LOG_DIS("s%s $r%u\n",
 cc_name(cond), dc->op1);
 
-if (cond != CC_A) {
-TCGLabel *l1 = gen_new_label();
-gen_tst_cc(dc, cpu_R[dc->op1], cond);
-tcg_gen_brcondi_tl(TCG_COND_EQ, cpu_R[dc->op1], 0, l1);
-tcg_gen_movi_tl(cpu_R[dc->op1], 1);
-gen_set_label(l1);
-} else {
-tcg_gen_movi_tl(cpu_R[dc->op1], 1);
-}
+gen_tst_cc(dc, cpu_R[dc->op1], cond);
+tcg_gen_setcondi_tl(TCG_COND_NE, cpu_R[dc->op1], cpu_R[dc->op1], 0);
 
 cris_cc_mask(dc, 0);
 return 2;
diff --git a/target-cris/translate_v10.c b/target-cris/translate_v10.c
index b742c4c..618c234 100644
--- a/target-cris/translate_v10.c
+++ b/target-cris/translate_v10.c
@@ -535,16 +535,8 @@ static void dec10_reg_scc(DisasContext *dc)
 
 LOG_DIS("s%s $r%u\n", cc_name(cond), dc->src);
 
-if (cond != CC_A)
-{
-TCGLabel *l1 = gen_new_label();
-gen_tst_cc (dc, cpu_R[dc->src], cond);
-tcg_gen_brcondi_tl(TCG_COND_EQ, cpu_R[dc->src], 0, l1);
-tcg_gen_movi_tl(cpu_R[dc->src], 1);
-gen_set_label(l1);
-} else {
-tcg_gen_movi_tl(cpu_R[dc->src], 1);
-}
+gen_tst_cc(dc, cpu_R[dc->src], cond);
+tcg_gen_setcondi_tl(TCG_COND_NE, cpu_R[dc->src], cpu_R[dc->src], 0);
 
 cris_cc_mask(dc, 0);
 }
-- 
2.1.0

[Qemu-devel] [PATCH] target-microblaze: Use setcond for pcmp*

[Richard Henderson]

Signed-off-by: Richard Henderson 
---
 target-microblaze/translate.c | 23 ---
 1 file changed, 4 insertions(+), 19 deletions(-)
---
Note that this is relative to an outstanding TCG patch set.
The full tree is at

  git://github.com/rth7680/qemu.git mb-movcond


r~
---

diff --git a/target-microblaze/translate.c b/target-microblaze/translate.c
index 4068946..f3e9912 100644
--- a/target-microblaze/translate.c
+++ b/target-microblaze/translate.c
@@ -313,7 +313,6 @@ static void dec_sub(DisasContext *dc)
 static void dec_pattern(DisasContext *dc)
 {
 unsigned int mode;
-TCGLabel *l1;
 
 if ((dc->tb_flags & MSR_EE_FLAG)
   && (dc->cpu->env.pvr.regs[2] & PVR2_ILL_OPCODE_EXC_MASK)
@@ -333,29 +332,15 @@ static void dec_pattern(DisasContext *dc)
 case 2:
 LOG_DIS("pcmpeq r%d r%d r%d\n", dc->rd, dc->ra, dc->rb);
 if (dc->rd) {
-TCGv t0 = tcg_temp_local_new();
-l1 = gen_new_label();
-tcg_gen_movi_tl(t0, 1);
-tcg_gen_brcond_tl(TCG_COND_EQ,
-  cpu_R[dc->ra], cpu_R[dc->rb], l1);
-tcg_gen_movi_tl(t0, 0);
-gen_set_label(l1);
-tcg_gen_mov_tl(cpu_R[dc->rd], t0);
-tcg_temp_free(t0);
+tcg_gen_setcond_tl(TCG_COND_EQ, cpu_R[dc->rd],
+   cpu_R[dc->ra], cpu_R[dc->rb]);
 }
 break;
 case 3:
 LOG_DIS("pcmpne r%d r%d r%d\n", dc->rd, dc->ra, dc->rb);
-l1 = gen_new_label();
 if (dc->rd) {
-TCGv t0 = tcg_temp_local_new();
-tcg_gen_movi_tl(t0, 1);
-tcg_gen_brcond_tl(TCG_COND_NE,
-  cpu_R[dc->ra], cpu_R[dc->rb], l1);
-tcg_gen_movi_tl(t0, 0);
-gen_set_label(l1);
-tcg_gen_mov_tl(cpu_R[dc->rd], t0);
-tcg_temp_free(t0);
+tcg_gen_setcond_tl(TCG_COND_NE, cpu_R[dc->rd],
+   cpu_R[dc->ra], cpu_R[dc->rb]);
 }
 break;
 default:
-- 
2.1.0

[Qemu-devel] [Bug 1423668] [NEW] Unable to set scsi drive serial if it contains spaces.

[Alan Latteri]

Public bug reported:

I am virtualzing a physical server for which I need to set the SCSI/SATA
drive serial.  It is comprised of 12 " " spaces then 8 letter/digits.
If I exclude the spaces, the drive serial is not accurate.  If I include
the spaces I get the following error.

error: Failed to start domain test1
error: internal error: driver serial 'ABCD1234' contains unsafe 
characters

virsh edit
Centos 7.0
3.19.0-1.el7.elrepo.x86_64
QEMU emulator version 1.5.3 (qemu-kvm-1.5.3-60.el7.centos.7), Copyright (c) 
2003-2008 Fabrice Bellard

** Affects: qemu
 Importance: Undecided
 Status: New

** Affects: qemu-kvm
 Importance: Undecided
 Status: New

** Also affects: qemu-kvm
   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/1423668

Title:
  Unable to set scsi drive serial if it contains spaces.

Status in QEMU:
  New
Status in qemu-kvm:
  New

Bug description:
  I am virtualzing a physical server for which I need to set the
  SCSI/SATA drive serial.  It is comprised of 12 " " spaces then 8
  letter/digits.  If I exclude the spaces, the drive serial is not
  accurate.  If I include the spaces I get the following error.

  error: Failed to start domain test1
  error: internal error: driver serial 'ABCD1234' contains unsafe 
characters

  virsh edit
  Centos 7.0
  3.19.0-1.el7.elrepo.x86_64
  QEMU emulator version 1.5.3 (qemu-kvm-1.5.3-60.el7.centos.7), Copyright (c) 
2003-2008 Fabrice Bellard

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

[Qemu-devel] unable to set SATA serial with a spaces

[Alan Latteri]

I am virtualzing a physical server for which I need to set the SCSI/SATA drive 
serial. It is comprised of 12 " " spaces then 8 letter/digits. If I exclude the 
spaces, the drive serial is not accurate. If I include the spaces I get the 
following error.

error: Failed to start domain test1
error: internal error: driver serial ' ABCD1234' contains unsafe characters

virsh edit
Centos 7.0
3.19.0-1.el7.elrepo.x86_64
QEMU emulator version 1.5.3 (qemu-kvm-1.5.3-60.el7.centos.7), Copyright (c) 
2003-2008 Fabrice Bellard

Re: [Qemu-devel] unable to set SATA serial with a spaces

[John Snow]

On 02/19/2015 02:48 PM, Alan Latteri wrote:

I am virtualzing a physical server for which I need to set the SCSI/SATA
drive serial. It is comprised of 12 " " spaces then 8 letter/digits. If
I exclude the spaces, the drive serial is not accurate. If I include the
spaces I get the following error.

error: Failed to start domain test1
error: internal error: driver serial ' ABCD1234' contains unsafe characters

virsh edit
Centos 7.0
3.19.0-1.el7.elrepo.x86_64
QEMU emulator version 1.5.3 (qemu-kvm-1.5.3-60.el7.centos.7), Copyright
(c) 2003-2008 Fabrice Bellard



Are you sure you need to input the spaces? the standard inquiry reply 
that outputs the serial uses spaces as padding, so even if you use 
serial "ABCD1234" the inquiry command is going to tell you

"ABCD1234".

Do you have software that relies on the serial number and can tell the 
difference between spaces present and no spaces present?


--js

Re: [Qemu-devel] unable to set SATA serial with a spaces

[Alan Latteri]

I have a software that I was able to virtualize on VirtualBox by setting the HD 
serial using the spaces easily.  Using the same parameters minus the spaces 
with QEMU, the software will not license.  Only different I can see is this.


> On Feb 19, 2015, at 2:12 PM, John Snow  wrote:
> 
> 
> 
> On 02/19/2015 02:48 PM, Alan Latteri wrote:
>> I am virtualzing a physical server for which I need to set the SCSI/SATA
>> drive serial. It is comprised of 12 " " spaces then 8 letter/digits. If
>> I exclude the spaces, the drive serial is not accurate. If I include the
>> spaces I get the following error.
>> 
>> error: Failed to start domain test1
>> error: internal error: driver serial ' ABCD1234' contains unsafe characters
>> 
>> virsh edit
>> Centos 7.0
>> 3.19.0-1.el7.elrepo.x86_64
>> QEMU emulator version 1.5.3 (qemu-kvm-1.5.3-60.el7.centos.7), Copyright
>> (c) 2003-2008 Fabrice Bellard
>> 
> 
> Are you sure you need to input the spaces? the standard inquiry reply that 
> outputs the serial uses spaces as padding, so even if you use serial 
> "ABCD1234" the inquiry command is going to tell you
> "ABCD1234".
> 
> Do you have software that relies on the serial number and can tell the 
> difference between spaces present and no spaces present?
> 
> --js

Re: [Qemu-devel] [PATCH v18 1/2] sPAPR: Implement EEH RTAS calls

[Gavin Shan]

On Mon, Feb 16, 2015 at 04:32:09PM +1100, Gavin Shan wrote:
>On Mon, Feb 16, 2015 at 12:52:48PM +1100, David Gibson wrote:
>>On Mon, Feb 16, 2015 at 10:16:01AM +1100, Gavin Shan wrote:
>>> The emulation for EEH RTAS requests from guest isn't covered
>>> by QEMU yet and the patch implements them.
>>> 
>>> The patch defines constants used by EEH RTAS calls and adds
>>> callbacks sPAPRPHBClass::{eeh_set_option, eeh_get_state, eeh_reset,
>>> eeh_configure}, which are going to be used as follows:
>>> 
>>>   * RTAS calls are received in spapr_pci.c, sanity check is done
>>> there.
>>>   * RTAS handlers handle what they can. If there is something it
>>> cannot handle and the corresponding sPAPRPHBClass callback is
>>> defined, it is called.
>>>   * Those callbacks are only implemented for VFIO now. They do ioctl()
>>> to the IOMMU container fd to complete the calls. Error codes from
>>> that ioctl() are transferred back to the guest.
>>> 
>>> [aik: defined RTAS tokens for EEH RTAS calls]
>>> Signed-off-by: Gavin Shan 
>>> ---
>>>  hw/ppc/spapr_pci.c  | 281 
>>> 
>>>  include/hw/pci-host/spapr.h |   4 +
>>>  include/hw/ppc/spapr.h  |  43 ++-
>>>  3 files changed, 326 insertions(+), 2 deletions(-)
>>> 
>>> diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c
>>> index cebdeb3..29b071d 100644
>>> --- a/hw/ppc/spapr_pci.c
>>> +++ b/hw/ppc/spapr_pci.c
>>> @@ -406,6 +406,268 @@ static void 
>>> rtas_ibm_query_interrupt_source_number(PowerPCCPU *cpu,
>>>  rtas_st(rets, 2, 1);/* 0 == level; 1 == edge */
>>>  }
>>>  
>>> +static void rtas_ibm_set_eeh_option(PowerPCCPU *cpu,
>>> +sPAPREnvironment *spapr,
>>> +uint32_t token, uint32_t nargs,
>>> +target_ulong args, uint32_t nret,
>>> +target_ulong rets)
>>> +{
>>> +sPAPRPHBState *sphb;
>>> +sPAPRPHBClass *spc;
>>> +uint32_t addr, option;
>>> +uint64_t buid;
>>> +int ret;
>>> +
>>> +if ((nargs != 4) || (nret != 1)) {
>>> +goto param_error_exit;
>>> +}
>>> +
>>> +buid = ((uint64_t)rtas_ld(args, 1) << 32) | rtas_ld(args, 2);
>>> +addr = rtas_ld(args, 0);
>>> +option = rtas_ld(args, 3);
>>> +
>>> +sphb = find_phb(spapr, buid);
>>> +if (!sphb) {
>>> +goto param_error_exit;
>>> +}
>>> +
>>> +spc = SPAPR_PCI_HOST_BRIDGE_GET_CLASS(sphb);
>>> +if (!spc->eeh_set_option) {
>>> +goto param_error_exit;
>>> +}
>>> +
>>> +/*
>>> + * The EEH functionality is enabled on basis of PCI device,
>>> + * instead of PE. We need check the validity of the PCI
>>> + * device address.
>>> + */
>>> +if (option == RTAS_EEH_ENABLE &&
>>> +!find_dev(spapr, buid, addr)) {
>>> +goto param_error_exit;
>>> +}
>>
>>You're still breaking your layering by doing checks dependent on the
>>specific option both here and in the callback.
>>
>>What I meant by my comments on the previous version was that this
>>find_dev() test should also move into the eeh_set_option callback.
>>Obviously that means adding addr into the parameters - but surely if
>>the addr has any meaning whatsoever, it must be at least potentially
>>needed by the callback anyway.
>>
>
>Ok. Either simply dropping the check here, or moving find_dev() to
>sPAPRPHBClass::eeh_set_option() as you suggested. However, there're more
>things needed for sPAPRPHBClass::eeh_set_option() to do the check as follows.
>David, could you help to confirm which way you prefer?
>
>- Rename find_dev() to spapr_find_pci_dev() and make it public. It will be
>  called in spapr_pci_vfio.c
>- Add one field sPAPRPHBState::spapr to reference the associated 
>sPAPREnvironment,
>  which is required by spapr_find_pci_dev(). Otherwise, we have to pass 
> sPAPREnvironment
>  to sPAPRPHBClass::eeh_set_option().
>

Ping, David? If you don't object, I'll drop the check simply in next revision.
Note that I dropped public find_phb()/find_dev() in v15 and I don't want change
the code back.

Thanks,
Gavin

>>Apart from that,
>>
>>Reviewed-by: David Gibson 
>>
>>-- 
>>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 3/8] libqos/ahci: add ahci command helpers

[John Snow]

ahci_command_set_flags:  Set additional flags in the command header.
ahci_command_clr_flags:  Clear flags from the command header.
ahci_command_set_offset: Change the IO sector from 0.
ahci_command_adjust: Adjust many values simultaneously.

To be used to adjust the command header if the default values/guesses
were incorrect or undesirable.

Signed-off-by: John Snow 
---
 tests/libqos/ahci.c | 42 ++
 tests/libqos/ahci.h |  5 +
 2 files changed, 47 insertions(+)

diff --git a/tests/libqos/ahci.c b/tests/libqos/ahci.c
index 6f6652c..1d92438 100644
--- a/tests/libqos/ahci.c
+++ b/tests/libqos/ahci.c
@@ -713,6 +713,40 @@ void ahci_command_free(AHCICommand *cmd)
 g_free(cmd);
 }
 
+void ahci_command_set_flags(AHCICommand *cmd, uint16_t cmdh_flags)
+{
+cmd->header.flags |= cmdh_flags;
+}
+
+void ahci_command_clr_flags(AHCICommand *cmd, uint16_t cmdh_flags)
+{
+cmd->header.flags &= ~cmdh_flags;
+}
+
+void ahci_command_set_offset(AHCICommand *cmd, uint64_t lba_sect)
+{
+RegH2DFIS *fis = &(cmd->fis);
+if (cmd->props->lba28) {
+g_assert_cmphex(lba_sect, <=, 0xFFF);
+} else if (cmd->props->lba48) {
+g_assert_cmphex(lba_sect, <=, 0x);
+} else {
+/* Can't set offset if we don't know the format. */
+g_assert_not_reached();
+}
+
+/* LBA28 uses the low nibble of the device/control register for LBA24:27 */
+fis->lba_lo[0] = (lba_sect & 0xFF);
+fis->lba_lo[1] = (lba_sect >> 8) & 0xFF;
+fis->lba_lo[2] = (lba_sect >> 16) & 0xFF;
+if (cmd->props->lba28) {
+fis->device = (fis->device & 0xF0) || (lba_sect >> 24) & 0x0F;
+}
+fis->lba_hi[0] = (lba_sect >> 24) & 0xFF;
+fis->lba_hi[1] = (lba_sect >> 32) & 0xFF;
+fis->lba_hi[2] = (lba_sect >> 40) & 0xFF;
+}
+
 void ahci_command_set_buffer(AHCICommand *cmd, uint64_t buffer)
 {
 cmd->buffer = buffer;
@@ -740,6 +774,14 @@ void ahci_command_set_prd_size(AHCICommand *cmd, unsigned 
prd_size)
 ahci_command_set_sizes(cmd, cmd->xbytes, prd_size);
 }
 
+void ahci_command_adjust(AHCICommand *cmd, uint64_t offset, uint64_t buffer,
+ size_t xbytes, unsigned prd_size)
+{
+ahci_command_set_sizes(cmd, xbytes, prd_size);
+ahci_command_set_buffer(cmd, buffer);
+ahci_command_set_offset(cmd, offset);
+}
+
 void ahci_command_commit(AHCIQState *ahci, AHCICommand *cmd, uint8_t port)
 {
 uint16_t i, prdtl;
diff --git a/tests/libqos/ahci.h b/tests/libqos/ahci.h
index 39b99d3..888545d 100644
--- a/tests/libqos/ahci.h
+++ b/tests/libqos/ahci.h
@@ -537,11 +537,16 @@ void ahci_command_verify(AHCIQState *ahci, AHCICommand 
*cmd);
 void ahci_command_free(AHCICommand *cmd);
 
 /* Command adjustments */
+void ahci_command_set_flags(AHCICommand *cmd, uint16_t cmdh_flags);
+void ahci_command_clr_flags(AHCICommand *cmd, uint16_t cmdh_flags);
+void ahci_command_set_offset(AHCICommand *cmd, uint64_t lba_sect);
 void ahci_command_set_buffer(AHCICommand *cmd, uint64_t buffer);
 void ahci_command_set_size(AHCICommand *cmd, uint64_t xbytes);
 void ahci_command_set_prd_size(AHCICommand *cmd, unsigned prd_size);
 void ahci_command_set_sizes(AHCICommand *cmd, uint64_t xbytes,
 unsigned prd_size);
+void ahci_command_adjust(AHCICommand *cmd, uint64_t lba_sect, uint64_t gbuffer,
+ uint64_t xbytes, unsigned prd_size);
 
 /* Command Misc */
 uint8_t ahci_command_slot(AHCICommand *cmd);
-- 
1.9.3

[Qemu-devel] [PATCH 2/8] qtest/ahci: Add a macro bootup routine

[John Snow]

Add a routine that can be used to engage the AHCI
device at a not-granular level so that bringing up
the functionality of the HBA is easy in future tests
that are not concerned with testing the bring-up process.

Signed-off-by: John Snow 
---
 tests/ahci-test.c | 23 +--
 1 file changed, 17 insertions(+), 6 deletions(-)

diff --git a/tests/ahci-test.c b/tests/ahci-test.c
index 53fd068..9fe9fb5 100644
--- a/tests/ahci-test.c
+++ b/tests/ahci-test.c
@@ -107,6 +107,21 @@ static void ahci_shutdown(AHCIQState *ahci)
 qtest_shutdown(qs);
 }
 
+/**
+ * Boot and fully enable the HBA device.
+ * @see ahci_boot, ahci_pci_enable and ahci_hba_enable.
+ */
+static AHCIQState *ahci_boot_and_enable(void)
+{
+AHCIQState *ahci;
+ahci = ahci_boot();
+
+ahci_pci_enable(ahci);
+ahci_hba_enable(ahci);
+
+return ahci;
+}
+
 /*** Specification Adherence Tests ***/
 
 /**
@@ -831,9 +846,7 @@ static void test_identify(void)
 {
 AHCIQState *ahci;
 
-ahci = ahci_boot();
-ahci_pci_enable(ahci);
-ahci_hba_enable(ahci);
+ahci = ahci_boot_and_enable();
 ahci_test_identify(ahci);
 ahci_shutdown(ahci);
 }
@@ -845,9 +858,7 @@ static void test_dma_rw_simple(void)
 {
 AHCIQState *ahci;
 
-ahci = ahci_boot();
-ahci_pci_enable(ahci);
-ahci_hba_enable(ahci);
+ahci = ahci_boot_and_enable();
 ahci_test_dma_rw_simple(ahci);
 ahci_shutdown(ahci);
 }
-- 
1.9.3

[Qemu-devel] [PATCH 7/8] qtest/ahci: add qcow2 support to ahci-test

[John Snow]

This will enable the testing of high offsets without
wasting a lot of disk space, and does not impact the
previous tests.

Signed-off-by: John Snow 
---
 tests/ahci-test.c | 15 +--
 tests/libqos/libqos.c | 30 ++
 tests/libqos/libqos.h |  1 +
 3 files changed, 36 insertions(+), 10 deletions(-)

diff --git a/tests/ahci-test.c b/tests/ahci-test.c
index 0d4a3df..38550c6 100644
--- a/tests/ahci-test.c
+++ b/tests/ahci-test.c
@@ -39,8 +39,8 @@
 #include "hw/pci/pci_ids.h"
 #include "hw/pci/pci_regs.h"
 
-/* Test-specific defines. */
-#define TEST_IMAGE_SIZE(64 * 1024 * 1024)
+/* Test-specific defines -- in MiB */
+#define TEST_IMAGE_SIZE_MB (200 * 1024)
 
 /*** Globals ***/
 static char tmp_path[] = "/tmp/qtest.XX";
@@ -81,7 +81,7 @@ static AHCIQState *ahci_boot(void)
 s = g_malloc0(sizeof(AHCIQState));
 
 cli = "-drive if=none,id=drive0,file=%s,cache=writeback,serial=%s"
-",format=raw"
+",format=qcow2"
 " -M q35 "
 "-device ide-hd,drive=drive0 "
 "-global ide-hd.ver=%s";
@@ -1050,7 +1050,6 @@ static void create_ahci_io_test(enum IOMode type, enum 
AddrMode addr,
 int main(int argc, char **argv)
 {
 const char *arch;
-int fd;
 int ret;
 int c;
 int i, j, k;
@@ -1087,12 +1086,8 @@ int main(int argc, char **argv)
 return 0;
 }
 
-/* Create a temporary raw image */
-fd = mkstemp(tmp_path);
-g_assert(fd >= 0);
-ret = ftruncate(fd, TEST_IMAGE_SIZE);
-g_assert(ret == 0);
-close(fd);
+/* Create a temporary qcow2 image */
+mkqcow2(tmp_path, TEST_IMAGE_SIZE_MB);
 
 /* Run the tests */
 qtest_add_func("/ahci/sanity", test_sanity);
diff --git a/tests/libqos/libqos.c b/tests/libqos/libqos.c
index bc8beb2..3577401 100644
--- a/tests/libqos/libqos.c
+++ b/tests/libqos/libqos.c
@@ -61,3 +61,33 @@ void qtest_shutdown(QOSState *qs)
 qtest_quit(qs->qts);
 g_free(qs);
 }
+
+int mkqcow2(const char *file, unsigned size_mb)
+{
+pid_t pid;
+int rc;
+char buff[32];
+
+snprintf(buff, 32, "%uM", size_mb);
+
+pid = fork();
+switch (pid) {
+case -1:
+perror("fork failed");
+return -1;
+case 0:
+close(fileno(stdout));
+rc = open("/dev/null", O_WRONLY);
+g_assert_cmpint(rc, ==, fileno(stdout));
+execl("./qemu-img", "qemu-img", "create", "-f", "qcow2",
+  file, buff, NULL);
+perror("execl failed");
+exit(-1);
+default:
+wait(&rc);
+g_assert_cmpint(rc, ==, 0);
+return rc;
+}
+
+g_assert_not_reached();
+}
diff --git a/tests/libqos/libqos.h b/tests/libqos/libqos.h
index 612d41e..5abd2bd 100644
--- a/tests/libqos/libqos.h
+++ b/tests/libqos/libqos.h
@@ -19,6 +19,7 @@ typedef struct QOSState {
 QOSState *qtest_vboot(QOSOps *ops, const char *cmdline_fmt, va_list ap);
 QOSState *qtest_boot(QOSOps *ops, const char *cmdline_fmt, ...);
 void qtest_shutdown(QOSState *qs);
+int mkqcow2(const char *file, unsigned size_mb);
 
 static inline uint64_t qmalloc(QOSState *q, size_t bytes)
 {
-- 
1.9.3

[Qemu-devel] [PATCH 8/8] qtest/ahci: test different disk sectors

[John Snow]

Test sector offset 0, 1, and the last sector(s)
in LBA28 and LBA48 modes.

Signed-off-by: John Snow 
---
 tests/ahci-test.c   | 69 +++--
 tests/libqos/ahci.c | 10 
 tests/libqos/ahci.h |  4 ++--
 3 files changed, 64 insertions(+), 19 deletions(-)

diff --git a/tests/ahci-test.c b/tests/ahci-test.c
index 38550c6..f536b19 100644
--- a/tests/ahci-test.c
+++ b/tests/ahci-test.c
@@ -41,6 +41,8 @@
 
 /* Test-specific defines -- in MiB */
 #define TEST_IMAGE_SIZE_MB (200 * 1024)
+#define TEST_IMAGE_SECTORS ((TEST_IMAGE_SIZE_MB / AHCI_SECTOR_SIZE) \
+* 1024 * 1024)
 
 /*** Globals ***/
 static char tmp_path[] = "/tmp/qtest.XX";
@@ -712,7 +714,7 @@ static void ahci_test_identify(AHCIQState *ahci)
 ahci_port_clear(ahci, px);
 
 /* "Read" 512 bytes using CMD_IDENTIFY into the host buffer. */
-ahci_io(ahci, px, CMD_IDENTIFY, &buff, buffsize);
+ahci_io(ahci, px, CMD_IDENTIFY, &buff, buffsize, 0);
 
 /* Check serial number/version in the buffer */
 /* NB: IDENTIFY strings are packed in 16bit little endian chunks.
@@ -728,11 +730,12 @@ static void ahci_test_identify(AHCIQState *ahci)
 g_assert_cmphex(rc, ==, 0);
 
 sect_size = le16_to_cpu(*((uint16_t *)(&buff[5])));
-g_assert_cmphex(sect_size, ==, 0x200);
+g_assert_cmphex(sect_size, ==, AHCI_SECTOR_SIZE);
 }
 
 static void ahci_test_io_rw_simple(AHCIQState *ahci, unsigned bufsize,
-   uint8_t read_cmd, uint8_t write_cmd)
+   uint64_t sector, uint8_t read_cmd,
+   uint8_t write_cmd)
 {
 uint64_t ptr;
 uint8_t port;
@@ -758,9 +761,9 @@ static void ahci_test_io_rw_simple(AHCIQState *ahci, 
unsigned bufsize,
 memwrite(ptr, tx, bufsize);
 
 /* Write this buffer to disk, then read it back to the DMA buffer. */
-ahci_guest_io(ahci, port, write_cmd, ptr, bufsize);
+ahci_guest_io(ahci, port, write_cmd, ptr, bufsize, sector);
 qmemset(ptr, 0x00, bufsize);
-ahci_guest_io(ahci, port, read_cmd, ptr, bufsize);
+ahci_guest_io(ahci, port, read_cmd, ptr, bufsize, sector);
 
 /*** Read back the Data ***/
 memread(ptr, rx, bufsize);
@@ -948,12 +951,45 @@ enum IOOps {
 NUM_IO_OPS
 };
 
+enum OffsetType {
+OFFSET_BEGIN = 0,
+OFFSET_ZERO = OFFSET_BEGIN,
+OFFSET_LOW,
+OFFSET_HIGH,
+NUM_OFFSETS
+};
+
+static const char *offset_str[NUM_OFFSETS] = { "zero", "low", "high" };
+
 typedef struct AHCIIOTestOptions {
 enum BuffLen length;
 enum AddrMode address_type;
 enum IOMode io_type;
+enum OffsetType offset;
 } AHCIIOTestOptions;
 
+static uint64_t offset_sector(enum OffsetType ofst,
+  enum AddrMode addr_type,
+  uint64_t buffsize)
+{
+uint64_t ceil;
+uint64_t nsectors;
+
+switch (ofst) {
+case OFFSET_ZERO:
+return 0;
+case OFFSET_LOW:
+return 1;
+case OFFSET_HIGH:
+ceil = (addr_type == ADDR_MODE_LBA28) ? 0xfff : 0x;
+ceil = MIN(ceil, TEST_IMAGE_SECTORS - 1);
+nsectors = buffsize / AHCI_SECTOR_SIZE;
+return ceil - nsectors + 1;
+default:
+g_assert_not_reached();
+}
+}
+
 /**
  * Table of possible I/O ATA commands given a set of enumerations.
  */
@@ -981,12 +1017,12 @@ static const uint8_t 
io_cmds[NUM_MODES][NUM_ADDR_MODES][NUM_IO_OPS] = {
  * transfer modes, and buffer sizes.
  */
 static void test_io_rw_interface(enum AddrMode lba48, enum IOMode dma,
- unsigned bufsize)
+ unsigned bufsize, uint64_t sector)
 {
 AHCIQState *ahci;
 
 ahci = ahci_boot_and_enable();
-ahci_test_io_rw_simple(ahci, bufsize,
+ahci_test_io_rw_simple(ahci, bufsize, sector,
io_cmds[dma][lba48][IO_READ],
io_cmds[dma][lba48][IO_WRITE]);
 ahci_shutdown(ahci);
@@ -999,6 +1035,7 @@ static void test_io_interface(gconstpointer opaque)
 {
 AHCIIOTestOptions *opts = (AHCIIOTestOptions *)opaque;
 unsigned bufsize;
+uint64_t sector;
 
 switch (opts->length) {
 case LEN_SIMPLE:
@@ -1017,13 +1054,14 @@ static void test_io_interface(gconstpointer opaque)
 g_assert_not_reached();
 }
 
-test_io_rw_interface(opts->address_type, opts->io_type, bufsize);
+sector = offset_sector(opts->offset, opts->address_type, bufsize);
+test_io_rw_interface(opts->address_type, opts->io_type, bufsize, sector);
 g_free(opts);
 return;
 }
 
 static void create_ahci_io_test(enum IOMode type, enum AddrMode addr,
-enum BuffLen len)
+enum BuffLen len, enum OffsetType offset)
 {
 static const char *arch;
 char *name;
@@ -1032,17 +1070,20 @@ static void create_ahci_io_test(enum IOMode type, enum 
AddrMode addr,
 opts->length = len;
 o

[Qemu-devel] [PATCH 6/8] qtest/ahci: add fragmented dma test

[John Snow]

Test what happens when we try to use extremely short PRDTs
to accomplish a small data transfer.

Signed-off-by: John Snow 
---
 tests/ahci-test.c | 59 +++
 1 file changed, 59 insertions(+)

diff --git a/tests/ahci-test.c b/tests/ahci-test.c
index cef8e2f..0d4a3df 100644
--- a/tests/ahci-test.c
+++ b/tests/ahci-test.c
@@ -851,6 +851,63 @@ static void test_identify(void)
 ahci_shutdown(ahci);
 }
 
+/**
+ * Fragmented DMA test: Perform a standard 4K DMA read/write
+ * test, but make sure the physical regions are fragmented to
+ * be very small, each just 32 bytes, to see how AHCI performs
+ * with chunks defined to be much less than a sector.
+ */
+static void test_dma_fragmented(void)
+{
+AHCIQState *ahci;
+AHCICommand *cmd;
+uint8_t px;
+size_t bufsize = 4096;
+unsigned char *tx = g_malloc(bufsize);
+unsigned char *rx = g_malloc0(bufsize);
+unsigned i;
+uint64_t ptr;
+
+ahci = ahci_boot_and_enable();
+px = ahci_port_select(ahci);
+ahci_port_clear(ahci, px);
+
+/* create pattern */
+for (i = 0; i < bufsize; i++) {
+tx[i] = (bufsize - i);
+}
+
+/* Create a DMA buffer in guest memory, and write our pattern to it. */
+ptr = guest_alloc(ahci->parent->alloc, bufsize);
+g_assert(ptr);
+memwrite(ptr, tx, bufsize);
+
+cmd = ahci_command_create(CMD_WRITE_DMA);
+ahci_command_adjust(cmd, 0, ptr, bufsize, 32);
+ahci_command_commit(ahci, cmd, px);
+ahci_command_issue(ahci, cmd);
+ahci_command_verify(ahci, cmd);
+g_free(cmd);
+
+cmd = ahci_command_create(CMD_READ_DMA);
+ahci_command_adjust(cmd, 0, ptr, bufsize, 32);
+ahci_command_commit(ahci, cmd, px);
+ahci_command_issue(ahci, cmd);
+ahci_command_verify(ahci, cmd);
+g_free(cmd);
+
+/* Read back the guest's receive buffer into local memory */
+memread(ptr, rx, bufsize);
+guest_free(ahci->parent->alloc, ptr);
+
+g_assert_cmphex(memcmp(tx, rx, bufsize), ==, 0);
+
+ahci_shutdown(ahci);
+
+g_free(rx);
+g_free(tx);
+}
+
 
/**/
 /* AHCI I/O Test Matrix Definitions   
*/
 
@@ -1053,6 +1110,8 @@ int main(int argc, char **argv)
 }
 }
 
+qtest_add_func("/ahci/io/dma/lba28/fragmented", test_dma_fragmented);
+
 ret = g_test_run();
 
 /* Cleanup */
-- 
1.9.3

[Qemu-devel] [PATCH 1/8] libqos/ahci: Zero-fill AHCI headers

[John Snow]

Even though it's just the reserved space, make sure they're zeroes.

Signed-off-by: John Snow 
---
 tests/libqos/ahci.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/libqos/ahci.c b/tests/libqos/ahci.c
index 170ec5a..6f6652c 100644
--- a/tests/libqos/ahci.c
+++ b/tests/libqos/ahci.c
@@ -487,7 +487,7 @@ void ahci_get_command_header(AHCIQState *ahci, uint8_t port,
 void ahci_set_command_header(AHCIQState *ahci, uint8_t port,
  uint8_t slot, AHCICommandHeader *cmd)
 {
-AHCICommandHeader tmp;
+AHCICommandHeader tmp = { .flags = 0 };
 uint64_t ba = ahci->port[port].clb;
 ba += slot * sizeof(AHCICommandHeader);
 
-- 
1.9.3

[Qemu-devel] [PATCH 0/8] ahci: add more IO tests

[John Snow]

This series is based on top of my ahci DMA test series, which is in
turn based on the ahci preliminary refactoring series. Both are currently
pending on stefanha/block.

This series adds many variations that expand on the existing trivial
DMA I/O case. These pathways check different PRDT configurations,
different I/O commands for PIO and DMA, different address scheme
combinations for LBA28 and LBA48, and different sector offsets.

John Snow (8):
  libqos/ahci: Zero-fill AHCI headers
  qtest/ahci: Add a macro bootup routine
  libqos/ahci: add ahci command helpers
  qtest/ahci: Add DMA test variants
  qtest/ahci: Add PIO and LBA48 tests
  qtest/ahci: add fragmented dma test
  qtest/ahci: add qcow2 support to ahci-test
  qtest/ahci: test different disk sectors

 tests/ahci-test.c | 295 +-
 tests/libqos/ahci.c   |  54 -
 tests/libqos/ahci.h   |   9 +-
 tests/libqos/libqos.c |  30 +
 tests/libqos/libqos.h |   1 +
 5 files changed, 355 insertions(+), 34 deletions(-)

-- 
1.9.3

[Qemu-devel] [PATCH 5/8] qtest/ahci: Add PIO and LBA48 tests

[John Snow]

In addition to DMA tests, test PIO and LBA48 command pathways in AHCI.
To accomplish this, a primitive multiplexer for gtest is added.

Though guests may prefer not to issue PIO commands directly except
for single sector cases during early boot and shutdown, these pathways
are still used for the transfer of ATAPI commands as well, and should
be behaving well.

Signed-off-by: John Snow 
---
 tests/ahci-test.c | 155 ++
 1 file changed, 133 insertions(+), 22 deletions(-)

diff --git a/tests/ahci-test.c b/tests/ahci-test.c
index 9394d85..cef8e2f 100644
--- a/tests/ahci-test.c
+++ b/tests/ahci-test.c
@@ -731,7 +731,8 @@ static void ahci_test_identify(AHCIQState *ahci)
 g_assert_cmphex(sect_size, ==, 0x200);
 }
 
-static void ahci_test_dma_rw_simple(AHCIQState *ahci, unsigned bufsize)
+static void ahci_test_io_rw_simple(AHCIQState *ahci, unsigned bufsize,
+   uint8_t read_cmd, uint8_t write_cmd)
 {
 uint64_t ptr;
 uint8_t port;
@@ -757,9 +758,9 @@ static void ahci_test_dma_rw_simple(AHCIQState *ahci, 
unsigned bufsize)
 memwrite(ptr, tx, bufsize);
 
 /* Write this buffer to disk, then read it back to the DMA buffer. */
-ahci_guest_io(ahci, port, CMD_WRITE_DMA, ptr, bufsize);
+ahci_guest_io(ahci, port, write_cmd, ptr, bufsize);
 qmemset(ptr, 0x00, bufsize);
-ahci_guest_io(ahci, port, CMD_READ_DMA, ptr, bufsize);
+ahci_guest_io(ahci, port, read_cmd, ptr, bufsize);
 
 /*** Read back the Data ***/
 memread(ptr, rx, bufsize);
@@ -850,36 +851,141 @@ static void test_identify(void)
 ahci_shutdown(ahci);
 }
 
+/**/
+/* AHCI I/O Test Matrix Definitions   
*/
+
+enum BuffLen {
+LEN_BEGIN = 0,
+LEN_SIMPLE = LEN_BEGIN,
+LEN_DOUBLE,
+LEN_LONG,
+LEN_SHORT,
+NUM_LENGTHS
+};
+
+static const char *buff_len_str[NUM_LENGTHS] = { "simple", "double",
+ "long", "short" };
+
+enum AddrMode {
+ADDR_MODE_BEGIN = 0,
+ADDR_MODE_LBA28 = ADDR_MODE_BEGIN,
+ADDR_MODE_LBA48,
+NUM_ADDR_MODES
+};
+
+static const char *addr_mode_str[NUM_ADDR_MODES] = { "lba28", "lba48" };
+
+enum IOMode {
+MODE_BEGIN = 0,
+MODE_PIO = MODE_BEGIN,
+MODE_DMA,
+NUM_MODES
+};
+
+static const char *io_mode_str[NUM_MODES] = { "pio", "dma" };
+
+enum IOOps {
+IO_BEGIN = 0,
+IO_READ = IO_BEGIN,
+IO_WRITE,
+NUM_IO_OPS
+};
+
+typedef struct AHCIIOTestOptions {
+enum BuffLen length;
+enum AddrMode address_type;
+enum IOMode io_type;
+} AHCIIOTestOptions;
+
 /**
- * Perform a simple DMA R/W test using non-NCQ commands.
+ * Table of possible I/O ATA commands given a set of enumerations.
  */
-static void test_dma_rw_interface(unsigned bufsize)
+static const uint8_t io_cmds[NUM_MODES][NUM_ADDR_MODES][NUM_IO_OPS] = {
+[MODE_PIO] = {
+[ADDR_MODE_LBA28] = {
+[IO_READ] = CMD_READ_PIO,
+[IO_WRITE] = CMD_WRITE_PIO },
+[ADDR_MODE_LBA48] = {
+[IO_READ] = CMD_READ_PIO_EXT,
+[IO_WRITE] = CMD_WRITE_PIO_EXT }
+},
+[MODE_DMA] = {
+[ADDR_MODE_LBA28] = {
+[IO_READ] = CMD_READ_DMA,
+[IO_WRITE] = CMD_WRITE_DMA },
+[ADDR_MODE_LBA48] = {
+[IO_READ] = CMD_READ_DMA_EXT,
+[IO_WRITE] = CMD_WRITE_DMA_EXT }
+}
+};
+
+/**
+ * Test a Read/Write pattern using various commands, addressing modes,
+ * transfer modes, and buffer sizes.
+ */
+static void test_io_rw_interface(enum AddrMode lba48, enum IOMode dma,
+ unsigned bufsize)
 {
 AHCIQState *ahci;
 
 ahci = ahci_boot_and_enable();
-ahci_test_dma_rw_simple(ahci, bufsize);
+ahci_test_io_rw_simple(ahci, bufsize,
+   io_cmds[dma][lba48][IO_READ],
+   io_cmds[dma][lba48][IO_WRITE]);
 ahci_shutdown(ahci);
 }
 
-static void test_dma_rw_simple(void)
+/**
+ * Demultiplex the test data and invoke the actual test routine.
+ */
+static void test_io_interface(gconstpointer opaque)
 {
-test_dma_rw_interface(4096);
-}
+AHCIIOTestOptions *opts = (AHCIIOTestOptions *)opaque;
+unsigned bufsize;
 
-static void test_dma_rw_double(void)
-{
-test_dma_rw_interface(8192);
-}
+switch (opts->length) {
+case LEN_SIMPLE:
+bufsize = 4096;
+break;
+case LEN_DOUBLE:
+bufsize = 8192;
+break;
+case LEN_LONG:
+bufsize = 4096 * 64;
+break;
+case LEN_SHORT:
+bufsize = 512;
+break;
+default:
+g_assert_not_reached();
+}
 
-static void test_dma_rw_long(void)
-{
-test_dma_rw_interface(4096 * 64);
+test_io_rw_interface(opts->address_type, opts->io_type, bufsize);
+g_free(opts);
+return;
 }
 
-static void test_dma_rw_short(void)
+static void create_a

[Qemu-devel] [PATCH 4/8] qtest/ahci: Add DMA test variants

[John Snow]

These test a few different pathways in the AHCI code.

short:  Test the minimum transfer size, exactly one sector.
simple: Test a transfer using a single PRD, in this case, 4K.
double: Test transferring 8K, which we will split up as two PRDs.
long:   Test transferring a lot of data using many PRDs, 256K.
Signed-off-by: John Snow 
---
 tests/ahci-test.c | 38 ++
 1 file changed, 30 insertions(+), 8 deletions(-)

diff --git a/tests/ahci-test.c b/tests/ahci-test.c
index 9fe9fb5..9394d85 100644
--- a/tests/ahci-test.c
+++ b/tests/ahci-test.c
@@ -731,12 +731,11 @@ static void ahci_test_identify(AHCIQState *ahci)
 g_assert_cmphex(sect_size, ==, 0x200);
 }
 
-static void ahci_test_dma_rw_simple(AHCIQState *ahci)
+static void ahci_test_dma_rw_simple(AHCIQState *ahci, unsigned bufsize)
 {
 uint64_t ptr;
 uint8_t port;
 unsigned i;
-const unsigned bufsize = 4096;
 unsigned char *tx = g_malloc(bufsize);
 unsigned char *rx = g_malloc0(bufsize);
 
@@ -751,7 +750,7 @@ static void ahci_test_dma_rw_simple(AHCIQState *ahci)
 ptr = ahci_alloc(ahci, bufsize);
 g_assert(ptr);
 
-/* Write some indicative pattern to our 4K buffer. */
+/* Write some indicative pattern to our buffer. */
 for (i = 0; i < bufsize; i++) {
 tx[i] = (bufsize - i);
 }
@@ -852,15 +851,35 @@ static void test_identify(void)
 }
 
 /**
- * Perform a simple DMA R/W test, using a single PRD and non-NCQ commands.
+ * Perform a simple DMA R/W test using non-NCQ commands.
  */
+static void test_dma_rw_interface(unsigned bufsize)
+{
+AHCIQState *ahci;
+
+ahci = ahci_boot_and_enable();
+ahci_test_dma_rw_simple(ahci, bufsize);
+ahci_shutdown(ahci);
+}
+
 static void test_dma_rw_simple(void)
 {
-AHCIQState *ahci;
+test_dma_rw_interface(4096);
+}
 
-ahci = ahci_boot_and_enable();
-ahci_test_dma_rw_simple(ahci);
-ahci_shutdown(ahci);
+static void test_dma_rw_double(void)
+{
+test_dma_rw_interface(8192);
+}
+
+static void test_dma_rw_long(void)
+{
+test_dma_rw_interface(4096 * 64);
+}
+
+static void test_dma_rw_short(void)
+{
+test_dma_rw_interface(512);
 }
 
 
/**/
@@ -919,6 +938,9 @@ int main(int argc, char **argv)
 qtest_add_func("/ahci/hba_enable", test_hba_enable);
 qtest_add_func("/ahci/identify",   test_identify);
 qtest_add_func("/ahci/dma/simple", test_dma_rw_simple);
+qtest_add_func("/ahci/dma/double", test_dma_rw_double);
+qtest_add_func("/ahci/dma/long",   test_dma_rw_long);
+qtest_add_func("/ahci/dma/short",  test_dma_rw_short);
 
 ret = g_test_run();
 
-- 
1.9.3