[Qemu-devel] question about vioscsi queue depth patch

2014-12-09 Thread Wangting (Kathy)
Hi Vadim,

I test the vioscsi 0001-queue-depth254.patch for virtio-win-0.1-74 with 
minor modifications about

virtio api, because it have not updated to use the linux api. When a VM with 
four disks in one controller

and 4kb 32depth sequential read for each disk at the same time, the tool of 
iometer which simulates

the IO model become no response. If fit the patch into virtio-win-0.1-94, it 
works well.

Is there some bug with the old virtio api, or is there some reason that the 
patch can not work normally

in virtio-win-0.1-74?



Best wishes,

Ting Wang





Re: [Qemu-devel] question about vioscsi queue depth patch

2014-12-09 Thread Vadim Rozenfeld
On Tue, 2014-12-09 at 16:20 +0800, Wangting (Kathy) wrote:
 Hi Vadim,
 
 I test the vioscsi 0001-queue-depth254.patch for virtio-win-0.1-74 with 
 minor modifications about
 
 virtio api, because it have not updated to use the linux api. When a VM with 
 four disks in one controller
 
 and 4kb 32depth sequential read for each disk at the same time, the tool of 
 iometer which simulates
 
 the IO model become no response. If fit the patch into virtio-win-0.1-94, it 
 works well.
 
 Is there some bug with the old virtio api, or is there some reason that 
 the patch can not work normally
 
 in virtio-win-0.1-74?
There were a lot of bug-fixes / changes introduced since build 74
(November 2013). Some of them did affect SRB completion and queue depth
logic.
So do you see any performance improvement after applying the patch?

Cheers,
Vadim.

 
 
 
 Best wishes,
 
 Ting Wang
 
 





Re: [Qemu-devel] [PATCH 0/7] MIPS: IEEE 754-2008 features support

2014-12-09 Thread Peter Maydell
On 9 December 2014 at 01:54, Maciej W. Rozycki ma...@codesourcery.com wrote:
 Hi,

  This patch series comprises changes to QEMU, both the MIPS backend and
 generic SoftFloat support code, to support IEEE 754-2008 features
 introduced to revision 3.50 of the MIPS Architecture as follows

I really really want to hold this patchset off until the softfloat
relicensing has gone through, because repeating that work would
be really painful...

thanks
-- PMM



Re: [Qemu-devel] suggestion for the QEMU advent calendar

2014-12-09 Thread Kevin Wolf
Am 08.12.2014 um 17:54 hat Fabian Greffrath geschrieben:
 PS: I am a bit shocked by the fact how complicated it is to install GRUB
 on a disk image. And, John, even if the approach you outlined worked, I
 guess GRUB will find its device.map puzzled when it will get rebooted
 from inside an image file instead of an actual hard-drive partition the
 next time.

As long as you don't say which version of GRUB, it's easy. Install GRUB
Legacy, copy stage[12] and menu.lst to the image and do this in the GRUB
shell:

device (hd0) $IMAGE_PATH
root (hd0,0)
install /boot/grub/stage1 (hd0) /boot/grub/stage2 p /boot/grub/menu.lst

Or boot from a floppy/CD with GRUB in the VM and run the same thing
without the device command.

Syslinux is relatively easy as well. Just avoid GRUB 2 if you want to
set up disk images from the host. I've done it before, but it's ugly...

Kevin



Re: [Qemu-devel] [PATCH v2] arm: add fw_cfg to virt board

2014-12-09 Thread Gerd Hoffmann
  Hi,

 So... after playing with this thing for some time, it's become clear
 that MMIO traps are painfully slow on the aarch64 platform we've been
 working on (using KVM).

So, as we don't have compatibility requirements, and we also can't play
tricks like using x86 string instructions:  How about a completely
different, dma-style interface for fw_cfg access?

One register for the (physical) target address.
One register for the transfer size.
One register for the fw_cfg entry.
Possibly one register for the fw_cfg offset (not really needed, but
avoids the need for side effects such as writing fw_cfg entry register
clearing the offset).
One register to kick the transfer.

cheers,
  Gerd





Re: [Qemu-devel] Redundant VDE network

2014-12-09 Thread Mark Cave-Ayland
On 09/12/14 06:29, Dmitry Antipov wrote:

 On 12/08/2014 08:53 PM, Stefan Hajnoczi wrote:
 
 Try:

 qemu-system-sparc -m 256 \
  -netdev vde,sock=/tmp/vde0,id=vde0 \
  -device ne2k_pci,netdev=vde0 \
  -netdev vde,sock=/tmp/vde1,id=vde1 \
  -device ne2k_pci,netdev=vde1 \
  -hda vm0.img
 
 This works just fine for qemu-system-x86, but not for SPARC with lance NIC:
 
 qemu-system-sparc -m 256 \
   -netdev vde,sock=/tmp/vde0,id=vde0 -device lance,netdev=vde0 \
   -netdev vde,sock=/tmp/vde1,id=vde1 -device lance,netdev=vde1 \
   -hda vm0.img
 
 qemu-system-sparc: -device lance,netdev=vde0: Parameter 'driver' expects
 pluggable device type
 
 It looks like PCI/sysbus emulation is incomplete/broken on SPARC.
 Assuming that x86 is OK, what SPARC-specific stuff should I check?

It's not that the emulation is broken (it's actually working as
expected), it's due to the way in which the BIOS probes sbus devices.

On real hardware, the device tree is generated by running bytecode
within the ROM contained upon each card by probing a known offset within
each slot, which is exactly how the cg3 and tcx cards are detected upon
boot.

However the since the lance device is on-board, it is hard-coded at a
fixed address in both Sun's OBP and OpenBIOS rather than being probed
via execution of an external ROM.

It should be a fairly trivial exercise to tweak QEMU and OpenBIOS to add
a second lance instance at a spare sbus slot address, but as you can see
it would require some extra smarts to dynamically patch a compiled ROM
within QEMU to insert the relevant properties from the device tree when
trying to add another network card via -device.


ATB,

Mark.




Re: [Qemu-devel] [PATCH v2] virtio: remove useless declaration of virtio_net_init()

2014-12-09 Thread Markus Armbruster
arei.gong...@huawei.com writes:

 From: Gonglei arei.gong...@huawei.com

 commit 1773d9ee (virtio-net: cleanup: init and exit function)
 removed the definition of virtio_net_init(), but didn't remove its
 declaration in the header.  Clean that up.

 Cc: Markus Armbruster arm...@redhat.com
 Signed-off-by: Gonglei arei.gong...@huawei.com

Reviewed-by: Markus Armbruster arm...@redhat.com



Re: [Qemu-devel] Redundant VDE network

2014-12-09 Thread Stefan Hajnoczi
On Tue, Dec 9, 2014 at 6:29 AM, Dmitry Antipov dmanti...@yandex.ru wrote:
 On 12/08/2014 08:53 PM, Stefan Hajnoczi wrote:

 Try:

 qemu-system-sparc -m 256 \
  -netdev vde,sock=/tmp/vde0,id=vde0 \
  -device ne2k_pci,netdev=vde0 \
  -netdev vde,sock=/tmp/vde1,id=vde1 \
  -device ne2k_pci,netdev=vde1 \
  -hda vm0.img


 This works just fine for qemu-system-x86, but not for SPARC with lance NIC:

 qemu-system-sparc -m 256 \
   -netdev vde,sock=/tmp/vde0,id=vde0 -device lance,netdev=vde0 \
   -netdev vde,sock=/tmp/vde1,id=vde1 -device lance,netdev=vde1 \
   -hda vm0.img

 qemu-system-sparc: -device lance,netdev=vde0: Parameter 'driver' expects
 pluggable device type

 It looks like PCI/sysbus emulation is incomplete/broken on SPARC.
 Assuming that x86 is OK, what SPARC-specific stuff should I check?

Is there a reason why you are using the old 32-bit SPARC machine types
instead of the more modern 64-bit SPARC machine types?

$ sparc-softmmu/qemu-system-sparc -M help
Supported machines are:
LX   Sun4m platform, SPARCstation LX
SPARCClassic Sun4m platform, SPARCClassic
SPARCbookSun4m platform, SPARCbook
SS-10Sun4m platform, SPARCstation 10
SS-20Sun4m platform, SPARCstation 20
SS-4 Sun4m platform, SPARCstation 4
SS-5 Sun4m platform, SPARCstation 5 (default)
SS-600MP Sun4m platform, SPARCserver 600MP
Voyager  Sun4m platform, SPARCstation Voyager
leon3_genericLeon-3 generic
none empty machine

$ sparc64-softmmu/qemu-system-sparc64 -M help
Supported machines are:
Niagara  Sun4v platform, Niagara
none empty machine
sun4uSun4u platform (default)
sun4vSun4v platform

I guess the SS-5 default machine type you are running doesn't have a PCI bus?

By the way, I have not tested the command-line because Fedora does not
have a vde package.  You are in niche territory with both SPARC and
vde, expect to either debug things yourself or file bugs (they may not
be addressed in a timely fashion).  Just want to set expectations for
what you will find.

Stefan



[Qemu-devel] [Bug 1363641] Re: Build of v2.1.0 fails on armv7l due to undeclared __NR_select

2014-12-09 Thread Eduardo Otubo
Hello Ben, you're completely right on what regards the version on the
error message. I'll fix it as soon as possible. Sorry for the trouble on
that :( (and sorry for the late reply I was on vacations)

Regarding the if statement, as Peter said here --
http://lists.gnu.org/archive/html/qemu-devel/2014-11/msg00960.html --
shell evaluates the OR before the AND, so that's Ok.

Thanks.

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

Title:
  Build of v2.1.0 fails on armv7l due to undeclared __NR_select

Status in QEMU:
  New

Bug description:
  After `make clean` and `git clean -x -f -d` `git checkout v2.1.0 
  configure --prefix=/home/user/prefix-qemu-2.1.0  make` fails due to
  missing declarations

  CCqemu-seccomp.o
  qemu-seccomp.c:28:1: error: '__NR_select' undeclared here (not in a 
function)
  qemu-seccomp.c:36:1: error: '__NR_mmap' undeclared here (not in a 
function)
  qemu-seccomp.c:57:1: error: '__NR_getrlimit' undeclared here (not in a 
function)
  qemu-seccomp.c:96:1: error: '__NR_time' undeclared here (not in a 
function)
    GEN   qmp-marshal.c
  qemu-seccomp.c:186:1: error: '__NR_alarm' undeclared here (not in a 
function)
  make: *** [qemu-seccomp.o] Error 1

  Same errors for master 8b3030114a449e66c68450acaac4b66f26d91416.
  `configure`should not succeed for a failing build if the error occurs
  due to missing dependencies, if it's a bug it needs to be fixed.
  `config.log` for v2.1.0 and 8b303011... attached. The content is
  mostly compiler output which I think is unusual for `config.log`, but
  see for yourself.

  I'm building on a debian 7.6 chroot on Synology DSM 5.0. `uname -a`
  says `Linux diskstatation 3.2.40 #4493 SMP Thu Aug 21 21:43:02 CST
  2014 armv7l GNU/Linux`.

  After installing some of the missing dependencies, i.e. `apt-get
  install liblzo2-dev libbsd-dev syslinux-common libhwloc-dev librdmacm-
  dev libsnappy-dev libibverbs-dev valgrind linux-
  headers-3.2.0-4-common` I'm getting

   CCmigration-rdma.o
  migration-rdma.c: In function 'ram_chunk_start':
  migration-rdma.c:523:12: error: cast to pointer from integer of different 
size [-Werror=int-to-pointer-cast]
  migration-rdma.c: In function '__qemu_rdma_add_block':
  migration-rdma.c:556:49: error: cast to pointer from integer of different 
size [-Werror=int-to-pointer-cast]
  migration-rdma.c:557:49: error: cast to pointer from integer of different 
size [-Werror=int-to-pointer-cast]
  migration-rdma.c: In function '__qemu_rdma_delete_block':
  migration-rdma.c:664:45: error: cast to pointer from integer of different 
size [-Werror=int-to-pointer-cast]
  migration-rdma.c:699:49: error: cast to pointer from integer of different 
size [-Werror=int-to-pointer-cast]
  migration-rdma.c: In function 'qemu_rdma_search_ram_block':
  migration-rdma.c:1113:49: error: cast to pointer from integer of 
different size [-Werror=int-to-pointer-cast]
  migration-rdma.c: In function 'qemu_rdma_register_and_get_keys':
  migration-rdma.c:1176:50: error: cast from pointer to integer of 
different size [-Werror=pointer-to-int-cast]
  migration-rdma.c:1177:29: error: cast from pointer to integer of 
different size [-Werror=pointer-to-int-cast]
  migration-rdma.c:1177:51: error: cast from pointer to integer of 
different size [-Werror=pointer-to-int-cast]
  migration-rdma.c:1178:29: error: cast from pointer to integer of 
different size [-Werror=pointer-to-int-cast]
  migration-rdma.c: In function 'qemu_rdma_post_send_control':
  migration-rdma.c:1562:36: error: cast from pointer to integer of 
different size [-Werror=pointer-to-int-cast]
  migration-rdma.c: In function 'qemu_rdma_post_recv_control':
  migration-rdma.c:1616:37: error: cast from pointer to integer of 
different size [-Werror=pointer-to-int-cast]
  migration-rdma.c: In function 'qemu_rdma_write_one':
  migration-rdma.c:1864:16: error: cast from pointer to integer of 
different size [-Werror=pointer-to-int-cast]
  migration-rdma.c:1868:53: error: cast to pointer from integer of 
different size [-Werror=int-to-pointer-cast]
  migration-rdma.c:1922:52: error: cast to pointer from integer of 
different size [-Werror=int-to-pointer-cast]
  migration-rdma.c:1923:50: error: cast to pointer from integer of 
different size [-Werror=int-to-pointer-cast]
  migration-rdma.c:1977:49: error: cast to pointer from integer of 
different size [-Werror=int-to-pointer-cast]
  migration-rdma.c:1998:49: error: cast to pointer from integer of 
different size [-Werror=int-to-pointer-cast]
  migration-rdma.c:2010:58: error: cast to pointer from integer of 
different size [-Werror=int-to-pointer-cast]
  migration-rdma.c: In function 'qemu_rdma_registration_handle':
  migration-rdma.c:3027:21: error: cast from pointer to integer of 

Re: [Qemu-devel] qcow2: Can create qcow2 image format on rbd server

2014-12-09 Thread Stefan Hajnoczi
On Tue, Dec 9, 2014 at 3:52 AM, Josh Durgin josh.dur...@inktank.com wrote:
 On 12/08/2014 05:58 AM, Jun Li wrote:

 On Fri, 12/05 18:01, Max Reitz wrote:

 On 2014-12-05 at 16:32, Jun Li wrote:

 Currently, qemu-img can not create qcow2 image format on rbd server.
 Analysis
 the code as followings:
 when create qcow2 format image:
 qcow2_create2
bdrv_create_file(filename, opts, local_err);  -- Here will create a
 0 size
file(e.g: file1) on rbd server.
...
ret = bdrv_pwrite(bs, 0, header, cluster_size); -- So here can not
 write
qcow2 header into file1 due to the file1's length is 0. Seems
qemu_rbd_aio_writev can not write beyond EOF.
...

 As above analysis, there are two methods to solve the above bz as
 followings:
 1, When create file1, just create a fixed-size file1 on rbd server(not 0
 size).


 Should be possible by using -o preallocation=falloc or -o
 preallocation=full.


 Although -o preallocation=falloc or -o preallocation=full can create a
 qcow2
 format image successfully, but when perform qemu-img resize file.qcow2
 +500M, then use the extend 500M disk image still hit the same issue(as
 rbd
 block driver does not support growable file).


 Why not use 'rbd resize' and raw images instead?
 rbd already supports snapshots, cloning, thin provisioning, and
 differential backup natively, so putting qcow2 on top tends to just add
 overhead.

In general, I don't expect many people to use qcow2 on rbd either.

However, qcow2 does work on top of iSCSI, LVM, or host block devices
and some users actually use this feature (even though there is LVM
thin provisioning, for example).  So the failure on rbd is unexpected
and inconsistent.

The way it works on other protocols is that the user must first create
an adequately sized volume before running qemu-img create.  With rbd
this doesn't work because we truncate the volume to 0 bytes during
create.

Either we need to fix this (without losing the ability to qemu-img
create -f raw rbd:... 10G) or we should have a clear error message.

The simplest way would just be to detect rbd create with size 0 and
print a clear error message like image formats that grow on demand
are not supported on rbd.

Stefan



Re: [Qemu-devel] suggestion for the QEMU advent calendar

2014-12-09 Thread Stefan Hajnoczi
On Tue, Dec 9, 2014 at 9:44 AM, Fabian Greffrath fab...@greffrath.com wrote:
 Am Dienstag, den 09.12.2014, 10:28 +0100 schrieb Kevin Wolf:
 Syslinux is relatively easy as well. Just avoid GRUB 2 if you want to
 set up disk images from the host. I've done it before, but it's ugly...

 Yep, I was talking about GRUB 2. However, in the meantime I have figured
 out what to do in order to create a minimal booting image. After all
 the image creation, partitioning, formatting, device mapping and finally
 mounting you only have to call grub-install with the right parameters
 (!), copy the actual kernel into the image and create a minimal grub.cfg
 file.

 By right parameters I meant that grub-install per default copies GRUB
 2 in its entirety onto the image, including the whole menu system,
 graphical support, file system drivers and localization, etc. This will
 take up about 10MB, which is why I put the minimal word in quotation
 marks. You can, of course, manually select which modules you want to
 install, but this must be a perfect guess. In my simple case it worked
 with '--install-modules=part_msdos ext2 multiboot normal --locales='
 passed to grub-install on the command line.

 Anyway, it's easier to just boot the multiboot kernel image and be done
 with it. ;)

The multiboot image is fine.

10 MB for a boot loader?  Just wow, at some point it's better to slap
a boot sector onto the Linux kernel and be done with it, kexec already
exists.

Stefan



Re: [Qemu-devel] suggestion for the QEMU advent calendar

2014-12-09 Thread Fabian Greffrath
Am Dienstag, den 09.12.2014, 09:51 + schrieb Stefan Hajnoczi: 
 The multiboot image is fine.

I think so.

 10 MB for a boot loader?  Just wow, at some point it's better to slap
 a boot sector onto the Linux kernel and be done with it, kexec already
 exists.

Well, it's a bit more than just a boot loader, but yeah:

$ du -h /boot/grub/
3,2M /boot/grub/locale
2,4M /boot/grub/i386-pc
2,3M /boot/grub/fonts
11M /boot/grub/

- Fabian





Re: [Qemu-devel] [PATCH 1/9] pc: acpi: fix WindowsXP BSOD when memory hotplug is enabled

2014-12-09 Thread Igor Mammedov
On Mon, 8 Dec 2014 22:57:05 +0200
Michael S. Tsirkin m...@redhat.com wrote:

 On Mon, Dec 08, 2014 at 04:08:00PM +, Igor Mammedov wrote:
  ACPI parser in XP considers PNP0A06 devices of CPU and
  memory hotplug as duplicates. Adding unique _UID
  to CPU hotplug device fixes BSOD.
  
  Signed-off-by: Igor Mammedov imamm...@redhat.com
 
 And let's add them for memory hotplug as well?
XP doesn't support it.

 Also, if we do stable branch release, we probably
 want to only do it for memory hotplug in a separate
 patch, right?
 This way users who don't enable memory hotplug
 are unaffected, reduces risk slightly.
Memory hotplug device already has _UID, so it doesn't need patching.
This patch just fixes BSOD if QEMU has been started with
hotplug enabled i.e. for example -m 2G,slots=2,maxmem=4G,
and prevents clashing between memory hotplug and
cpu hotplug devices on XP (i.e. XP specific quirk).

I've tested it with XPsp3 and all later version upto WS2012R2.


 
  ---
   hw/i386/acpi-dsdt-cpu-hotplug.dsl | 1 +
   1 file changed, 1 insertion(+)
  
  diff --git a/hw/i386/acpi-dsdt-cpu-hotplug.dsl 
  b/hw/i386/acpi-dsdt-cpu-hotplug.dsl
  index 34aab5a..268d870 100644
  --- a/hw/i386/acpi-dsdt-cpu-hotplug.dsl
  +++ b/hw/i386/acpi-dsdt-cpu-hotplug.dsl
  @@ -94,6 +94,7 @@ Scope(\_SB) {
   
   Device(CPU_HOTPLUG_RESOURCE_DEVICE) {
   Name(_HID, EisaId(PNP0A06))
  +Name(_UID, CPU hotplug resources)
   
   Name(_CRS, ResourceTemplate() {
   IO(Decode16, CPU_STATUS_BASE, CPU_STATUS_BASE, 0, 
  CPU_STATUS_LEN)
  -- 
  1.8.3.1




Re: [Qemu-devel] [PATCH 2/9] pc: acpi: decribe bridge device as not hotpluggable

2014-12-09 Thread Igor Mammedov
On Mon, 8 Dec 2014 21:13:25 +0200
Michael S. Tsirkin m...@redhat.com wrote:

 On Mon, Dec 08, 2014 at 04:08:01PM +, Igor Mammedov wrote:
  when bridge hotplug is disabled, i.e. for machine
  types less then 2.0, bridge device was created as
  hotpluggable by mistake since commit 133a2da.
  
  Fix it by just creating it as a present device.
  
  Signed-off-by: Igor Mammedov imamm...@redhat.com
 
 What exactly is the problem here?
 It seems that such bridge is hotpluggable, even though
 e.g. windows guests lacks drivers to support this.
before 133a2da slot with existing at startup bridge weren't
marked as hotpluggable nor described in SSDT. But after
133a2da it's described as hotpluggable slot for compat
machines (2.0 and lower) which doesn't match with original
behavior.

Also Marcel mentioned that bridges could be hotpluggable
but that they should not be hot-UNpluggable.

 
 
  ---
   hw/i386/acpi-build.c | 2 +-
   1 file changed, 1 insertion(+), 1 deletion(-)
  
  diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
  index b37a397..1fb92e5 100644
  --- a/hw/i386/acpi-build.c
  +++ b/hw/i386/acpi-build.c
  @@ -913,7 +913,7 @@ static void build_pci_bus_end(PCIBus *bus, void 
  *bus_state)
   }
   }
   
  -if (!dc-hotpluggable || bridge_in_acpi) {
  +if (!dc-hotpluggable || pc-is_bridge) {
   clear_bit(slot, slot_hotplug_enable);
   }
   }
  -- 
  1.8.3.1




Re: [Qemu-devel] [PATCH 3/9] pc: acpi-build: cleanup AcpiPmInfo initialization

2014-12-09 Thread Igor Mammedov
On Mon, 8 Dec 2014 23:03:02 +0200
Michael S. Tsirkin m...@redhat.com wrote:

 On Mon, Dec 08, 2014 at 04:08:02PM +, Igor Mammedov wrote:
  zero initialize AcpiPmInfo struct to reduce code bloat
  a little bit.
  
  Signed-off-by: Igor Mammedov imamm...@redhat.com
 
 I generally prefer explicit initialization, but
 it's a matter of taste.
I did this since it saves some LOCs and will save even
more later with patches for dynamic _CRS.

  ---
   hw/i386/acpi-build.c | 8 ++--
   1 file changed, 2 insertions(+), 6 deletions(-)
  
  diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
  index 1fb92e5..f5ec66a 100644
  --- a/hw/i386/acpi-build.c
  +++ b/hw/i386/acpi-build.c
  @@ -161,6 +161,8 @@ static void acpi_get_pm_info(AcpiPmInfo *pm)
   Object *obj = NULL;
   QObject *o;
   
  +memset(pm, 0, sizeof(*pm));
  +
   if (piix) {
   obj = piix;
   }
  @@ -173,22 +175,16 @@ static void acpi_get_pm_info(AcpiPmInfo *pm)
   o = object_property_get_qobject(obj, ACPI_PM_PROP_S3_DISABLED, NULL);
   if (o) {
   pm-s3_disabled = qint_get_int(qobject_to_qint(o));
  -} else {
  -pm-s3_disabled = false;
   }
   qobject_decref(o);
   o = object_property_get_qobject(obj, ACPI_PM_PROP_S4_DISABLED, NULL);
   if (o) {
   pm-s4_disabled = qint_get_int(qobject_to_qint(o));
  -} else {
  -pm-s4_disabled = false;
   }
   qobject_decref(o);
   o = object_property_get_qobject(obj, ACPI_PM_PROP_S4_VAL, NULL);
   if (o) {
   pm-s4_val = qint_get_int(qobject_to_qint(o));
  -} else {
  -pm-s4_val = false;
   }
   qobject_decref(o);
   
  -- 
  1.8.3.1
 




[Qemu-devel] [PATCH v6 0/3] machvirt dynamic sysbus device instantiation

2014-12-09 Thread Eric Auger
This patch series enables machvirt to dynamically instantiate sysbus
devices from command line (using -device option).

All those sysbus devices are plugged onto a platform bus. This latter
device is instantiated in machvirt and takes care of the binding of
children sysbus devices on a machine init done notifier. The device
tree node generation for children dynamic sysbus device also happens
on a subsequent notifier that must be executed after the above one.
machvirt registers that notifier before the platform bus creation to
make sure notifiers are executed in the right order: dt generation after
actual QOM binding.

Very few sysbus devices are supposed to be instantiated that
way. VFIO devices belong to them.

Node creation really is architecture specific. On ARM the dynamic
sysbus device node creation is implemented in a new C module,
hw/arm/sysbus-fdt.c and not in the machine file.

Machvirt transformations and sysbus-fdt are largely inspired from Alex work.

The patch series can be found at:
http://git.linaro.org/people/eric.auger/qemu.git
branch vfio_integ_v9_rc6_official_dynsysbus_v6

Best Regards

Eric

v5 - v6:
Take into account Peter's comments:
- dtb overload mechanism rewritten: arm_load_kernel code is moved into a
  machine init done notifier notify instead. arm_load_kernel only registers
  that notifier. As a consequence the dtb is loaded once.
- v5 1-4 patch files are removed and replaced by a single patch file moving
  arm_load_kernel in the notifier (2).
- as a consequence arm_load_kernel must be called before sysbus-fdt
  arm_register_platform_bus_fdt_creator.
- In virt, platform_bus_params not a const anymore since its fields are
  initialized from vbi-memmap and vbi-irqmap. Hence create_platform_bus
  proto can be simplified.
- In sysbus-fdt add_all_platform_bus_fdt_nodes now takes a handle to
  an ARMPlatformBusFdtParams. This is not a modify_dtb function anymore
  fdt pointer is checked in case the callback is called after the load_dtb
  (this latter deallocated fdt pointer). Check of fdt_filename moved in here.
  upgrade_dtb is removed. copyright aligned between .h and .c.

v4 - v5:
- in virt.c: platform_bus_params becomes static const
- sysbus-fdt: change indentation in add_fdt_node_functions array init
- s/load_dtb/arm_load_dtb in one boot.c comment

v3 - v4:
- dyn_sysbus_binding removed since binding stuff now are implemented by
  the platform bus device
- due to a change in ARM load_dtb implementation using rom_add_blob_fixed,
  the dt no more is generated in a reset notifier but is generated on a
  machine init done notifier
- the augmented device tree is not generated from scratch anymore but is
  added using a modify_dtb function. This required some small change in
  boot.c
- the case where the user provides a dtb file now is handled
- some cleanup in virt additions
- implement a list of dyanmically instantiable devices in sysbus-fdt

v2 - v3:
- patch now applies on top of Alex full patchset
- dyn_sysbus_devtree: add arm_prefix to emphasize the fact those
  functions are arm specific; arm_sysbus_device_create_devtree
  becomes static
- load_dtb renamed into arm_load_dtb
- add copyright in hw/arm/dyn_sysbus_devtree.c

v1 - v2:
- device node generation no more in sysbus device but in
  dyn_sysbus_devtree
- VFIO region shrinked to 4MB and relocated in machvirt to avoid PCI
  shrink (dynamic vfio-mmio support might come latter)
- platform_bus_base removed from PlatformDevtreeData


Eric Auger (3):
  hw/arm/sysbus-fdt: helpers for platform bus nodes addition
  hw/arm/boot: arm_load_kernel implemented as a machine init done
notifier
  hw/arm/virt: add dynamic sysbus device support

 hw/arm/Makefile.objs|   1 +
 hw/arm/boot.c   |  14 +++-
 hw/arm/sysbus-fdt.c | 173 
 hw/arm/virt.c   |  97 -
 include/hw/arm/arm.h|  28 +++
 include/hw/arm/sysbus-fdt.h |  60 +++
 6 files changed, 355 insertions(+), 18 deletions(-)
 create mode 100644 hw/arm/sysbus-fdt.c
 create mode 100644 include/hw/arm/sysbus-fdt.h

-- 
1.8.3.2




[Qemu-devel] [PATCH v6 1/3] hw/arm/sysbus-fdt: helpers for platform bus nodes addition

2014-12-09 Thread Eric Auger
This new C module will be used by ARM machine files to generate
platform bus node and their dynamic sysbus device tree nodes.

Dynamic sysbus device node addition is done in a machine init
done notifier. arm_register_platform_bus_fdt_creator does the
registration of this latter and is supposed to be called by
ARM machine files that support platform bus and their dynamic
sysbus. Addition of dynamic sysbus nodes is done only if the
user did not provide any dtb.

Signed-off-by: Alexander Graf ag...@suse.de
Signed-off-by: Eric Auger eric.au...@linaro.org

---
v5 - v6:
- add_all_platform_bus_fdt_nodes is not a modify_dtb function anymore
- it now takes a handle to an ARMPlatformBusFdtParams.
- fdt pointer is checked in case this notifier is executed after the
  one that executes the load_dtb (this latter deallocates the fdt pointer)
- check of fdt_filename moved in here.
- upgrade_dtb is removed
- copyright aligned between .h and .c

v4 - v5:
- change indentation in add_fdt_node_functions. Also becomes a
  static const.
- ARMPlatformBusFdtParams.system_params becomes a pointer to
  a const ARMPlatformBusSystemParams
- removes platform-bus.h second inclusion

v3 - v4:
- dyn_sysbus_devtree.c renamed into sysbus-fdt.c
- use new PlatformBusDevice object
- the dtb upgrade is done through modify_dtb. Before the fdt
  was recreated from scratch. When the user provided a dtb this
  latter was overwritten which was not correct.
- an array contains the association between device type names
  and their node creation function
- I must aknowledge I did not find any cleaner way to implement
  a FDT_BUILDER interface, as suggested by Paolo. The class method
  would need to be initialized somewhere and since it cannot
  happen in the device itself - according to Alex  Peter comments -,
  I don't see when I shall associate the device type and its
  interface implementation.

v2 - v3:
- add arm_ prefix
- arm_sysbus_device_create_devtree becomes static

v1 - v2:
- Code moved in an arch specific file to accomodate architecture
  dependent specificities.
- remove platform_bus_base from PlatformDevtreeData

v1: code originally written by Alex Graf in e500.c and reused for
ARM [Eric Auger]
---
 hw/arm/Makefile.objs|   1 +
 hw/arm/sysbus-fdt.c | 173 
 include/hw/arm/sysbus-fdt.h |  60 +++
 3 files changed, 234 insertions(+)
 create mode 100644 hw/arm/sysbus-fdt.c
 create mode 100644 include/hw/arm/sysbus-fdt.h

diff --git a/hw/arm/Makefile.objs b/hw/arm/Makefile.objs
index 6088e53..0cc63e1 100644
--- a/hw/arm/Makefile.objs
+++ b/hw/arm/Makefile.objs
@@ -3,6 +3,7 @@ obj-$(CONFIG_DIGIC) += digic_boards.o
 obj-y += integratorcp.o kzm.o mainstone.o musicpal.o nseries.o
 obj-y += omap_sx1.o palm.o realview.o spitz.o stellaris.o
 obj-y += tosa.o versatilepb.o vexpress.o virt.o xilinx_zynq.o z2.o
+obj-y += sysbus-fdt.o
 
 obj-y += armv7m.o exynos4210.o pxa2xx.o pxa2xx_gpio.o pxa2xx_pic.o
 obj-$(CONFIG_DIGIC) += digic.o
diff --git a/hw/arm/sysbus-fdt.c b/hw/arm/sysbus-fdt.c
new file mode 100644
index 000..843405b
--- /dev/null
+++ b/hw/arm/sysbus-fdt.c
@@ -0,0 +1,173 @@
+/*
+ * ARM Platform Bus device tree generation helpers
+ *
+ * Copyright (c) 2014 Linaro Limited
+ *
+ * Authors:
+ *  Alex Graf ag...@suse.de
+ *  Eric Auger eric.au...@linaro.org
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2 or later, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope 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 http://www.gnu.org/licenses/.
+ *
+ */
+
+#include hw/arm/sysbus-fdt.h
+#include qemu/error-report.h
+#include sysemu/device_tree.h
+#include hw/platform-bus.h
+#include sysemu/sysemu.h
+
+/*
+ * internal struct that contains the information to create dynamic
+ * sysbus device node
+ */
+typedef struct PlatformBusFdtData {
+void *fdt; /* device tree handle */
+int irq_start; /* index of the first IRQ usable by platform bus devices */
+const char *pbus_node_name; /* name of the platform bus node */
+PlatformBusDevice *pbus;
+} PlatformBusFdtData;
+
+/*
+ * struct used when calling the machine init done notifier
+ * that constructs the fdt nodes of platform bus devices
+ */
+typedef struct PlatformBusFdtNotifierParams {
+ARMPlatformBusFdtParams *fdt_params;
+Notifier notifier;
+} PlatformBusFdtNotifierParams;
+
+/* struct that associates a device type name and a node creation function */
+typedef struct NodeCreationPair {
+const char *typename;
+int (*add_fdt_node_fn)(SysBusDevice *sbdev, void *opaque);
+} 

Re: [Qemu-devel] [PATCH v5 6/6] hw/arm/virt: add dynamic sysbus device support

2014-12-09 Thread Eric Auger
On 12/05/2014 05:36 PM, Peter Maydell wrote:
 On 30 November 2014 at 18:19, Eric Auger eric.au...@linaro.org wrote:
 Allows sysbus devices to be instantiated from command line by
 using -device option. Machvirt creates a platform bus at init.
 The dynamic sysbus devices are attached to this platform bus device.

 The platform bus device registers a machine init done notifier
 whose role will be to bind the dynamic sysbus devices. Indeed
 dynamic sysbus devices are created after machine init.

 machvirt also registers a notifier that will build the device
 tree nodes for the platform bus and its children dynamic sysbus
 devices.

 Signed-off-by: Alexander Graf ag...@suse.de
 Signed-off-by: Eric Auger eric.au...@linaro.org

 ---
 v4 - v5:
 - platform_bus_params becomes static const
 - reword comment in create_platform_bus
 - reword the commit message

 v3 - v4:
 - use platform bus object, instantiated in create_platform_bus
 - device tree generation for platform bus and children dynamic
   sysbus devices is no more handled at reset but in a
   machine_init_done_notifier (due to the change in implementaion
   of ARM load dtb using rom_add_blob_fixed).
 - device tree enhancement now takes into account the case of
   user provided dtb. Before the user dtb was overwritten which
   was wrong. However in case the dtb is provided by the user,
   dynamic sysbus nodes are not added there.
 - renaming of MACHVIRT_PLATFORM defines
 - MACHVIRT_PLATFORM_PAGE_SHIFT and SIZE_PAGES not needed anymore,
   hence removed.
 - DynSysbusParams struct renamed into ARMPlatformBusSystemParams
   and above params removed.
 - separation of dt creation and QEMU binding is not mandated anymore
   since the device tree is not created from scratch anymore. Instead
   the modify_dtb function is used.
 - create_platform_bus registers another machine init done notifier
   to start VFIO IRQ handling. This latter executes after the
   dynamic sysbus device binding.

 v2 - v3:
 - renaming of arm_platform_bus_create_devtree and arm_load_dtb
 - add copyright in hw/arm/dyn_sysbus_devtree.c

 v1 - v2:
 - remove useless vfio-platform.h include file
 - s/MACHVIRT_PLATFORM_HOLE/MACHVIRT_PLATFORM_SIZE
 - use dyn_sysbus_binding and dyn_sysbus_devtree
 - dynamic sysbus platform buse size shrinked to 4MB and
   moved between RTC and MMIO

 v1:

 Inspired from what Alex Graf did in ppc e500
 https://lists.gnu.org/archive/html/qemu-ppc/2014-07/msg00012.html

 Conflicts:
 hw/arm/sysbus-fdt.c
 ---
  hw/arm/virt.c | 57 +
  1 file changed, 57 insertions(+)

 diff --git a/hw/arm/virt.c b/hw/arm/virt.c
 index 314e55b..37326a9 100644
 --- a/hw/arm/virt.c
 +++ b/hw/arm/virt.c
 @@ -42,6 +42,8 @@
  #include exec/address-spaces.h
  #include qemu/bitops.h
  #include qemu/error-report.h
 +#include hw/arm/sysbus-fdt.h
 +#include hw/platform-bus.h

  #define NUM_VIRTIO_TRANSPORTS 32

 @@ -59,6 +61,11 @@
  #define GIC_FDT_IRQ_PPI_CPU_START 8
  #define GIC_FDT_IRQ_PPI_CPU_WIDTH 8

 +#define PLATFORM_BUS_BASE 0x940
 +#define PLATFORM_BUS_SIZE (4ULL * 1024 * 1024)
 +#define PLATFORM_BUS_FIRST_IRQ48
 +#define PLATFORM_BUS_NUM_IRQS 20
 +
  enum {
  VIRT_FLASH,
  VIRT_MEM,
 @@ -68,6 +75,7 @@ enum {
  VIRT_UART,
  VIRT_MMIO,
  VIRT_RTC,
 +VIRT_PLATFORM_BUS,
  };

  typedef struct MemMapEntry {
 @@ -107,6 +115,7 @@ static const MemMapEntry a15memmap[] = {
  [VIRT_GIC_CPU] ={ 0x0801, 0x0001 },
  [VIRT_UART] =   { 0x0900, 0x1000 },
  [VIRT_RTC] ={ 0x0901, 0x1000 },
 +[VIRT_PLATFORM_BUS] = {PLATFORM_BUS_BASE , PLATFORM_BUS_SIZE},
 
 This makes it pretty hard to read this -- you should use
 the raw 0x numbers here. Anywhere else that wants to know
 the base address etc should fish it out of the memory
 map at runtime, as we do with the other devices.
Hi Peter

ok. As a side effect platform_bus_params will not be a const anymore,
previously recommended by Alex.
 
  [VIRT_MMIO] =   { 0x0a00, 0x0200 },
  /* ...repeating for a total of NUM_VIRTIO_TRANSPORTS, each of that size 
 */
  /* 0x1000 .. 0x4000 reserved for PCI */
 @@ -117,6 +126,14 @@ static const int a15irqmap[] = {
  [VIRT_UART] = 1,
  [VIRT_RTC] = 2,
  [VIRT_MMIO] = 16, /* ...to 16 + NUM_VIRTIO_TRANSPORTS - 1 */
 +[VIRT_PLATFORM_BUS] = PLATFORM_BUS_FIRST_IRQ,
 
 Similarly with interrupt numbers.
ok
Thanks
Eric
 
 -- PMM
 




[Qemu-devel] [PATCH v6 2/3] hw/arm/boot: arm_load_kernel implemented as a machine init done notifier

2014-12-09 Thread Eric Auger
Device tree nodes for the platform bus and its children dynamic sysbus
devices are added in a machine init done notifier. To load the dtb once,
after those latter nodes are built and before ROM freeze, the actual
arm_load_kernel existing code is moved into a notifier notify function,
arm_load_kernel_notify. arm_load_kernel now only registers the
corresponding notifier.

Machine files that do not support platform bus stay unchanged. Machine
files willing to support dynamic sysbus devices must call arm_load_kernel
before sysbus-fdt arm_register_platform_bus_fdt_creator to make sure
dynamic sysbus device nodes are integrated in the dtb.

Signed-off-by: Eric Auger eric.au...@linaro.org

---

v6: creation of this patch file
---
 hw/arm/boot.c| 14 +-
 include/hw/arm/arm.h | 28 
 2 files changed, 41 insertions(+), 1 deletion(-)

diff --git a/hw/arm/boot.c b/hw/arm/boot.c
index 0014c34..9d8aaa0 100644
--- a/hw/arm/boot.c
+++ b/hw/arm/boot.c
@@ -476,7 +476,7 @@ static void do_cpu_reset(void *opaque)
 }
 }
 
-void arm_load_kernel(ARMCPU *cpu, struct arm_boot_info *info)
+static void arm_load_kernel_notify(Notifier *notifier, void *data)
 {
 CPUState *cs;
 int kernel_size;
@@ -487,6 +487,11 @@ void arm_load_kernel(ARMCPU *cpu, struct arm_boot_info 
*info)
 hwaddr entry, kernel_load_offset;
 int big_endian;
 static const ARMInsnFixup *primary_loader;
+ArmLoadKernelNotifier *n =
+container_of(notifier, ArmLoadKernelNotifier, notifier);
+ARMCPU *cpu = n-cpu;
+struct arm_boot_info *info =
+container_of(n, struct arm_boot_info, load_kernel_notifier);
 
 /* CPU objects (unlike devices) are not automatically reset on system
  * reset, so we must always register a handler to do so. If we're
@@ -665,3 +670,10 @@ void arm_load_kernel(ARMCPU *cpu, struct arm_boot_info 
*info)
 ARM_CPU(cs)-env.boot_info = info;
 }
 }
+
+void arm_load_kernel(ARMCPU *cpu, struct arm_boot_info *info)
+{
+info-load_kernel_notifier.cpu = cpu;
+info-load_kernel_notifier.notifier.notify = arm_load_kernel_notify;
+qemu_add_machine_init_done_notifier(info-load_kernel_notifier.notifier);
+}
diff --git a/include/hw/arm/arm.h b/include/hw/arm/arm.h
index cefc9e6..88cf80b 100644
--- a/include/hw/arm/arm.h
+++ b/include/hw/arm/arm.h
@@ -19,6 +19,16 @@ qemu_irq *armv7m_init(MemoryRegion *system_memory,
   int flash_size, int sram_size,
   const char *kernel_filename, const char *cpu_model);
 
+/*
+ * struct used as a parameter of the arm_load_kernel machine init
+ * done notifier
+ */
+typedef struct {
+ARMCPU *cpu; /* handle to the first cpu object */
+Notifier notifier; /* actual notifier */
+} ArmLoadKernelNotifier;
+
+
 /* arm_boot.c */
 struct arm_boot_info {
 uint64_t ram_size;
@@ -61,12 +71,30 @@ struct arm_boot_info {
  * the user it should implement this hook.
  */
 void (*modify_dtb)(const struct arm_boot_info *info, void *fdt);
+/* machine init done notifier executing arm_load_dtb */
+ArmLoadKernelNotifier load_kernel_notifier;
 /* Used internally by arm_boot.c */
 int is_linux;
 hwaddr initrd_start;
 hwaddr initrd_size;
 hwaddr entry;
 };
+
+/**
+ * arm_load_kernel - Loads memory with everything needed to boot
+ *
+ * @cpu: handle to the first CPU object
+ * @info: handle to the boot info struct
+ * Registers a machine init done notifier that copies to memory
+ * everything needed to boot, depending on machine and user options:
+ * kernel image, boot loaders, initrd, dtb. Also registers the CPU
+ * reset handler.
+ *
+ * In case the machine file supports the platform bus device and its
+ * dynamically instantiable sysbus devices, this function must be called
+ * before sysbus-fdt arm_register_platform_bus_fdt_creator. Indeed the
+ * machine init done notifiers are called in registration reverse order.
+ */
 void arm_load_kernel(ARMCPU *cpu, struct arm_boot_info *info);
 
 /* Multiplication factor to convert from system clock ticks to qemu timer
-- 
1.8.3.2




[Qemu-devel] [PATCH v6 3/3] hw/arm/virt: add dynamic sysbus device support

2014-12-09 Thread Eric Auger
Allows sysbus devices to be instantiated from command line by
using -device option. Machvirt creates a platform bus at init.
The dynamic sysbus devices are attached to this platform bus device.

The platform bus device registers a machine init done notifier
whose role will be to bind the dynamic sysbus devices. Indeed
dynamic sysbus devices are created after machine init.

machvirt also registers a notifier that will build the device
tree nodes for the platform bus and its children dynamic sysbus
devices.

Signed-off-by: Alexander Graf ag...@suse.de
Signed-off-by: Eric Auger eric.au...@linaro.org

---
v5 - v6:
- Take into account Peter's comments:
  - platform_bus_params initialized from vbi-memmap and vbi-irqmap.
As a consequence, const is removed. Also alignment in a15memmap
is slightly changed.
  - ARMPlatformBusSystemParams handle removed from create_platform_bus
prototype
- arm_load_kernel has become a machine init done notifier registration.
  It must be called before platform_bus_create to guarantee the correct
  notifier execution sequence

v4 - v5:
- platform_bus_params becomes static const
- reword comment in create_platform_bus
- reword the commit message

v3 - v4:
- use platform bus object, instantiated in create_platform_bus
- device tree generation for platform bus and children dynamic
  sysbus devices is no more handled at reset but in a
  machine_init_done_notifier (due to the change in implementaion
  of ARM load dtb using rom_add_blob_fixed).
- device tree enhancement now takes into account the case of
  user provided dtb. Before the user dtb was overwritten which
  was wrong. However in case the dtb is provided by the user,
  dynamic sysbus nodes are not added there.
- renaming of MACHVIRT_PLATFORM defines
- MACHVIRT_PLATFORM_PAGE_SHIFT and SIZE_PAGES not needed anymore,
  hence removed.
- DynSysbusParams struct renamed into ARMPlatformBusSystemParams
  and above params removed.
- separation of dt creation and QEMU binding is not mandated anymore
  since the device tree is not created from scratch anymore. Instead
  the modify_dtb function is used.
- create_platform_bus registers another machine init done notifier
  to start VFIO IRQ handling. This latter executes after the
  dynamic sysbus device binding.

v2 - v3:
- renaming of arm_platform_bus_create_devtree and arm_load_dtb
- add copyright in hw/arm/dyn_sysbus_devtree.c

v1 - v2:
- remove useless vfio-platform.h include file
- s/MACHVIRT_PLATFORM_HOLE/MACHVIRT_PLATFORM_SIZE
- use dyn_sysbus_binding and dyn_sysbus_devtree
- dynamic sysbus platform buse size shrinked to 4MB and
  moved between RTC and MMIO

v1:

Inspired from what Alex Graf did in ppc e500
https://lists.gnu.org/archive/html/qemu-ppc/2014-07/msg00012.html

Conflicts:
hw/arm/sysbus-fdt.c
---
 hw/arm/virt.c | 97 ---
 1 file changed, 80 insertions(+), 17 deletions(-)

diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index 314e55b..366083a 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -42,6 +42,8 @@
 #include exec/address-spaces.h
 #include qemu/bitops.h
 #include qemu/error-report.h
+#include hw/arm/sysbus-fdt.h
+#include hw/platform-bus.h
 
 #define NUM_VIRTIO_TRANSPORTS 32
 
@@ -59,6 +61,9 @@
 #define GIC_FDT_IRQ_PPI_CPU_START 8
 #define GIC_FDT_IRQ_PPI_CPU_WIDTH 8
 
+#define PLATFORM_BUS_FIRST_IRQ48
+#define PLATFORM_BUS_NUM_IRQS 20
+
 enum {
 VIRT_FLASH,
 VIRT_MEM,
@@ -68,8 +73,11 @@ enum {
 VIRT_UART,
 VIRT_MMIO,
 VIRT_RTC,
+VIRT_PLATFORM_BUS,
 };
 
+static ARMPlatformBusSystemParams platform_bus_params;
+
 typedef struct MemMapEntry {
 hwaddr base;
 hwaddr size;
@@ -100,23 +108,25 @@ typedef struct VirtBoardInfo {
  */
 static const MemMapEntry a15memmap[] = {
 /* Space up to 0x800 is reserved for a boot ROM */
-[VIRT_FLASH] =  {  0, 0x0800 },
-[VIRT_CPUPERIPHS] = { 0x0800, 0x0002 },
+[VIRT_FLASH] =  {  0, 0x0800 },
+[VIRT_CPUPERIPHS] = { 0x0800, 0x0002 },
 /* GIC distributor and CPU interfaces sit inside the CPU peripheral space 
*/
-[VIRT_GIC_DIST] =   { 0x0800, 0x0001 },
-[VIRT_GIC_CPU] ={ 0x0801, 0x0001 },
-[VIRT_UART] =   { 0x0900, 0x1000 },
-[VIRT_RTC] ={ 0x0901, 0x1000 },
-[VIRT_MMIO] =   { 0x0a00, 0x0200 },
+[VIRT_GIC_DIST] =   { 0x0800, 0x0001 },
+[VIRT_GIC_CPU] ={ 0x0801, 0x0001 },
+[VIRT_UART] =   { 0x0900, 0x1000 },
+[VIRT_RTC] ={ 0x0901, 0x1000 },
+[VIRT_PLATFORM_BUS] =   { 0x0940, 0x0400 },
+[VIRT_MMIO] =   { 0x0a00, 0x0200 },
 /* ...repeating for a total of NUM_VIRTIO_TRANSPORTS, each of that size */
 /* 0x1000 .. 0x4000 reserved for PCI */
-[VIRT_MEM] ={ 0x4000, 30ULL * 1024 * 1024 * 1024 },
+[VIRT_MEM] ={ 0x4000, 

Re: [Qemu-devel] [PATCH 4/9] acpi: build_append_nameseg(): add padding if necessary

2014-12-09 Thread Igor Mammedov
On Mon, 8 Dec 2014 23:15:32 +0200
Michael S. Tsirkin m...@redhat.com wrote:

 On Mon, Dec 08, 2014 at 04:08:03PM +, Igor Mammedov wrote:
  According to ACPI spec NameSeg shorter than 4 characters
  must be padded up to 4 characters with _ symbol.
  ACPI 5.0:  20.2.2 Name Objects Encoding
  
  Do it in build_append_nameseg() so that caller shouldn't know
  or care about it.
  
  Signed-off-by: Igor Mammedov imamm...@redhat.com
 
 To me just doing it right in callers seems just as easy, but
 I guess you disagree :)
That means, author MUST know about padding, if he/she doesn't
or forget about it that would introduce error usually resulting
in BSOD. This patch allows to avoid such mistakes.

 
  ---
   hw/i386/acpi-build.c | 18 +-
   1 file changed, 13 insertions(+), 5 deletions(-)
  
  diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
  index f5ec66a..a8b7a2b 100644
  --- a/hw/i386/acpi-build.c
  +++ b/hw/i386/acpi-build.c
  @@ -292,6 +292,8 @@ static inline void build_append_array(GArray *array, 
  GArray *val)
   g_array_append_vals(array, val-data, val-len);
   }
   
  +#define ACPI_NAMESEG_LEN 4
  +
   static void GCC_FMT_ATTR(2, 3)
   build_append_nameseg(GArray *array, const char *format, ...)
   {
  @@ -299,13 +301,19 @@ build_append_nameseg(GArray *array, const char 
  *format, ...)
   char s[] = ;
   int len;
   va_list args;
  +const char padding = '_';
   
   va_start(args, format);
   len = vsnprintf(s, sizeof s, format, args);
   va_end(args);
   
  -assert(len == 4);
  +g_assert(len = ACPI_NAMESEG_LEN);
 
 I'm not sure when is g_assert preferable to assert.
 What's the motivation here?
 
 
  +
   g_array_append_vals(array, s, len);
  +while (len != ACPI_NAMESEG_LEN) {
  +g_array_append_val(array, padding);
  +++len;
  +}
 
 Easier
 
   /* Pad up to 4 characters if necessary. */
   g_array_append_vals(array, , 4 - len);
 
 
 
   }
   
   /* 5.4 Definition Block Encoding */
  @@ -846,7 +854,7 @@ static void build_pci_bus_end(PCIBus *bus, void 
  *bus_state)
   
   if (bus-parent_dev) {
   op = 0x82; /* DeviceOp */
  -build_append_nameseg(bus_table, S%.02X_,
  +build_append_nameseg(bus_table, S%.02X,
bus-parent_dev-devfn);
   build_append_byte(bus_table, 0x08); /* NameOp */
   build_append_nameseg(bus_table, _SUN);
  @@ -966,7 +974,7 @@ static void build_pci_bus_end(PCIBus *bus, void 
  *bus_state)
   build_append_int(notify, 0x1U  i);
   build_append_byte(notify, 0x00); /* NullName */
   build_append_byte(notify, 0x86); /* NotifyOp */
  -build_append_nameseg(notify, S%.02X_, PCI_DEVFN(i, 0));
  +build_append_nameseg(notify, S%.02X, PCI_DEVFN(i, 0));
   build_append_byte(notify, 0x69); /* Arg1Op */
   
   /* Pack it up */
  @@ -1023,7 +1031,7 @@ static void build_pci_bus_end(PCIBus *bus, void 
  *bus_state)
   if (bus-parent_dev) {
   build_append_byte(parent-notify_table, '^'); /* 
  ParentPrefixChar */
   build_append_byte(parent-notify_table, 0x2E); /* 
  DualNamePrefix */
  -build_append_nameseg(parent-notify_table, S%.02X_,
  +build_append_nameseg(parent-notify_table, S%.02X,
bus-parent_dev-devfn);
   build_append_nameseg(parent-notify_table, PCNT);
   }
  @@ -1093,7 +1101,7 @@ build_ssdt(GArray *table_data, GArray *linker,
   GArray *sb_scope = build_alloc_array();
   uint8_t op = 0x10; /* ScopeOp */
   
  -build_append_nameseg(sb_scope, _SB_);
  +build_append_nameseg(sb_scope, _SB);
   
   /* build Processor object for each processor */
   for (i = 0; i  acpi_cpus; i++) {
  -- 
  1.8.3.1




Re: [Qemu-devel] [PATCH v5 1/6] hw/arm/boot: load_dtb becomes non static arm_load_dtb

2014-12-09 Thread Eric Auger
On 12/05/2014 07:16 PM, Peter Maydell wrote:
 On 5 December 2014 at 16:38, Peter Maydell peter.mayd...@linaro.org wrote:
 On 30 November 2014 at 18:19, Eric Auger eric.au...@linaro.org wrote:
 load_dtb is renamed into arm_load_dtb and becomes non static.
 it will be used by machvirt for dynamic instantiation of
 platform devices

 'virt' shouldn't be a special case -- we should always
 handle setting up the DTB in guest memory in the same
 way, whether there happens to be a vfio platform device
 available or not.
 
 ...this probably means that a bunch of the work currently
 done in arm_load_kernel() should be deferred to a 'machine
 init complete' hook (perhaps all of it?).

Hi Peter,

OK

I moved the arm_load_kernel code into a machine init done notify and
arm_load_kernel now only registers the notifier.
For machine files willing to support platform bus, the arm_load_kernel
must happen before the registration of the notifier that adds platform
bus nodes and after CPU init (notifiers are executed in registration
reverse order).

Best Regards

Eric
 
 -- PMM
 




Re: [Qemu-devel] [PATCH v5 5/6] hw/arm/sysbus-fdt: helpers for platform bus nodes addition

2014-12-09 Thread Eric Auger
On 12/05/2014 05:40 PM, Peter Maydell wrote:
 On 30 November 2014 at 18:19, Eric Auger eric.au...@linaro.org wrote:
 --- /dev/null
 +++ b/hw/arm/sysbus-fdt.c
 @@ -0,0 +1,180 @@
 +/*
 + * ARM Platform Bus device tree generation helpers
 + *
 + * Copyright (c) 2014 Linaro Limited
 + *
 + * Authors:
 + *  Alex Graf ag...@suse.de
 + *  Eric Auger eric.au...@linaro.org
 + *
 + * This program is free software; you can redistribute it and/or modify it
 + * under the terms and conditions of the GNU General Public License,
 + * version 2 or later, as published by the Free Software Foundation.
 
 but
 
 diff --git a/include/hw/arm/sysbus-fdt.h b/include/hw/arm/sysbus-fdt.h
 new file mode 100644
 index 000..d4bec85
 --- /dev/null
 +++ b/include/hw/arm/sysbus-fdt.h
 @@ -0,0 +1,50 @@
 +/*
 + * Dynamic sysbus device tree node generation API
 + *
 + * Copyright Linaro Limited, 2014
 + *
 + * Authors:
 + *  Alex Graf ag...@suse.de
 + *  Eric Auger eric.au...@linaro.org
 + *
 + * This work is licensed under the terms of the GNU GPL, version 2.  See
 + * the COPYING file in the top-level directory.
 + */
 
 ...shouldn't the .c file and the .h file be under the same license?
OK copyrights are now aligned

Thanks

Eric
 
 -- PMM
 




Re: [Qemu-devel] [PATCH 1/9] pc: acpi: fix WindowsXP BSOD when memory hotplug is enabled

2014-12-09 Thread Michael S. Tsirkin
On Tue, Dec 09, 2014 at 11:05:37AM +0100, Igor Mammedov wrote:
 On Mon, 8 Dec 2014 22:57:05 +0200
 Michael S. Tsirkin m...@redhat.com wrote:
 
  On Mon, Dec 08, 2014 at 04:08:00PM +, Igor Mammedov wrote:
   ACPI parser in XP considers PNP0A06 devices of CPU and
   memory hotplug as duplicates. Adding unique _UID
   to CPU hotplug device fixes BSOD.
   
   Signed-off-by: Igor Mammedov imamm...@redhat.com
  
  And let's add them for memory hotplug as well?
 XP doesn't support it.
  Also, if we do stable branch release, we probably
  want to only do it for memory hotplug in a separate
  patch, right?
  This way users who don't enable memory hotplug
  are unaffected, reduces risk slightly.
 Memory hotplug device already has _UID, so it doesn't need patching.
 This patch just fixes BSOD if QEMU has been started with
 hotplug enabled i.e. for example -m 2G,slots=2,maxmem=4G,
 and prevents clashing between memory hotplug and
 cpu hotplug devices on XP (i.e. XP specific quirk).
 
 I've tested it with XPsp3 and all later version upto WS2012R2.


I see, I misunderstood.
Thanks, I'll apply this one.

 
  
   ---
hw/i386/acpi-dsdt-cpu-hotplug.dsl | 1 +
1 file changed, 1 insertion(+)
   
   diff --git a/hw/i386/acpi-dsdt-cpu-hotplug.dsl 
   b/hw/i386/acpi-dsdt-cpu-hotplug.dsl
   index 34aab5a..268d870 100644
   --- a/hw/i386/acpi-dsdt-cpu-hotplug.dsl
   +++ b/hw/i386/acpi-dsdt-cpu-hotplug.dsl
   @@ -94,6 +94,7 @@ Scope(\_SB) {

Device(CPU_HOTPLUG_RESOURCE_DEVICE) {
Name(_HID, EisaId(PNP0A06))
   +Name(_UID, CPU hotplug resources)

Name(_CRS, ResourceTemplate() {
IO(Decode16, CPU_STATUS_BASE, CPU_STATUS_BASE, 0, 
   CPU_STATUS_LEN)
   -- 
   1.8.3.1



Re: [Qemu-devel] [PATCH 2/9] pc: acpi: decribe bridge device as not hotpluggable

2014-12-09 Thread Michael S. Tsirkin
On Tue, Dec 09, 2014 at 11:27:16AM +0100, Igor Mammedov wrote:
 On Mon, 8 Dec 2014 21:13:25 +0200
 Michael S. Tsirkin m...@redhat.com wrote:
 
  On Mon, Dec 08, 2014 at 04:08:01PM +, Igor Mammedov wrote:
   when bridge hotplug is disabled, i.e. for machine
   types less then 2.0, bridge device was created as
   hotpluggable by mistake since commit 133a2da.
   
   Fix it by just creating it as a present device.
   
   Signed-off-by: Igor Mammedov imamm...@redhat.com
  
  What exactly is the problem here?
  It seems that such bridge is hotpluggable, even though
  e.g. windows guests lacks drivers to support this.
 before 133a2da slot with existing at startup bridge weren't
 marked as hotpluggable nor described in SSDT. But after
 133a2da it's described as hotpluggable slot for compat
 machines (2.0 and lower) which doesn't match with original
 behavior.
 
 Also Marcel mentioned that bridges could be hotpluggable
 but that they should not be hot-UNpluggable.

OK so is there some guest that's confused?
What's the bug?

  
  
   ---
hw/i386/acpi-build.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
   
   diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
   index b37a397..1fb92e5 100644
   --- a/hw/i386/acpi-build.c
   +++ b/hw/i386/acpi-build.c
   @@ -913,7 +913,7 @@ static void build_pci_bus_end(PCIBus *bus, void 
   *bus_state)
}
}

   -if (!dc-hotpluggable || bridge_in_acpi) {
   +if (!dc-hotpluggable || pc-is_bridge) {
clear_bit(slot, slot_hotplug_enable);
}
}
   -- 
   1.8.3.1



Re: [Qemu-devel] [PATCH] ide: Implement VPD response for ATAPI

2014-12-09 Thread Stefan Hajnoczi
On Thu, Dec 04, 2014 at 01:59:19PM -0500, John Snow wrote:
 diff --git a/hw/ide/atapi.c b/hw/ide/atapi.c
 index c63b7e5..d27c34b 100644
 --- a/hw/ide/atapi.c
 +++ b/hw/ide/atapi.c
 @@ -622,19 +622,98 @@ static void cmd_request_sense(IDEState *s, uint8_t *buf)
  static void cmd_inquiry(IDEState *s, uint8_t *buf)
  {
  int max_len = buf[4];

What guarantees do we have about the size of buf in this function?

We can't trust max_len because it comes from the guest.

 +int idx = 0;
  
 -buf[0] = 0x05; /* CD-ROM */
 -buf[1] = 0x80; /* removable */
 -buf[2] = 0x00; /* ISO */
 -buf[3] = 0x21; /* ATAPI-2 (XXX: put ATAPI-4 ?) */
 -buf[4] = 31; /* additional length */
 -buf[5] = 0; /* reserved */
 -buf[6] = 0; /* reserved */
 -buf[7] = 0; /* reserved */
 -padstr8(buf + 8, 8, QEMU);
 -padstr8(buf + 16, 16, QEMU DVD-ROM);
 -padstr8(buf + 32, 4, s-version);
 -ide_atapi_cmd_reply(s, 36, max_len);
 +/* If the EVPD (Enable Vital Product Data) bit is set in byte 1,
 + * we are being asked for a specific page of info indicated by byte 2. */
 +if (buf[1]  0x01) {
 +switch (buf[2]) {
 +case 0x00:
 +/* Supported Pages */
 +buf[idx++] = 0x05; /* CD-ROM */
 +buf[idx++] = 0x00; /* Page Code (0x00) */
 +buf[idx++] = 0x00; /* reserved */
 +buf[idx++] = 0x02; /* Two pages supported: */
 +buf[idx++] = 0x00; /* 0x00: Supported Pages, and: */
 +buf[idx++] = 0x83; /* 0x83: Device Identification. */
 +break;
 +case 0x83:
 +/* Device Identification. Each entry is optional, but the entries
 + * included here are modeled after libata's VPD responses. */
 +buf[idx++] = 0x05; /* CD-ROM */
 +buf[idx++] = 0x83; /* Page Code */
 +buf[idx++] = 0x00;
 +buf[idx++] = 0x00; /* Length of encapsulated structure: */
 +
 +/* Entry 1: Serial */
 +if (idx + 24  max_len) {
 +/* Not enough room for even the first entry: */
 +/* 4 byte header + 20 byte string */
 +ide_atapi_cmd_error(s, ILLEGAL_REQUEST,
 +ASC_DATA_PHASE_ERROR);

Missing return

 +}
 +buf[idx++] = 0x02; /* Ascii */
 +buf[idx++] = 0x00; /* Vendor Specific */
 +buf[idx++] = 0x00;
 +buf[idx++] = 20;   /* Remaining length */
 +padstr8(buf + idx, 20, s-drive_serial_str);
 +idx += 20;
 +
 +/* Entry 2: Drive Model and Serial */
 +if (idx + 72  max_len) {
 +/* 4 (header) + 8 (vendor) + 60 (model  serial) */
 +goto out;
 +}

I guess this time it's okay to return early when there is not enough
space left due to optional fields?

Do we need to set buf[3] (which is currently 0)?


pgpg9v5wlC00F.pgp
Description: PGP signature


Re: [Qemu-devel] [PATCH 5/9] acpi: move generic aml building helpers into dedictated file

2014-12-09 Thread Igor Mammedov
On Mon, 8 Dec 2014 22:43:40 +0200
Michael S. Tsirkin m...@redhat.com wrote:

 On Mon, Dec 08, 2014 at 04:08:04PM +, Igor Mammedov wrote:
  the will be later used for composing AML primitives
  and all that could be reused later for ARM machines
  as well.
  
  Signed-off-by: Igor Mammedov imamm...@redhat.com
 
 It will be easy to move when needed.
 Why do it now?
I'm trying to reduce already huge dynamic AML series,
and moving this now is not any way worse than moving it
later. Since it should be moved anyway for ARM machines
to reuse.

 
 
  ---
   hw/acpi/Makefile.objs|   1 +
   hw/acpi/acpi_gen_utils.c | 169 
  +++
   hw/i386/acpi-build.c | 165 
  +-
   include/hw/acpi/acpi_gen_utils.h |  23 ++
   4 files changed, 195 insertions(+), 163 deletions(-)
   create mode 100644 hw/acpi/acpi_gen_utils.c
   create mode 100644 include/hw/acpi/acpi_gen_utils.h
  
  diff --git a/hw/acpi/Makefile.objs b/hw/acpi/Makefile.objs
  index acd2389..4237232 100644
  --- a/hw/acpi/Makefile.objs
  +++ b/hw/acpi/Makefile.objs
  @@ -1,3 +1,4 @@
   common-obj-$(CONFIG_ACPI) += core.o piix4.o ich9.o pcihp.o cpu_hotplug.o
   common-obj-$(CONFIG_ACPI) += memory_hotplug.o
   common-obj-$(CONFIG_ACPI) += acpi_interface.o
  +common-obj-$(CONFIG_ACPI) += acpi_gen_utils.o
  diff --git a/hw/acpi/acpi_gen_utils.c b/hw/acpi/acpi_gen_utils.c
  new file mode 100644
  index 000..1583b35
  --- /dev/null
  +++ b/hw/acpi/acpi_gen_utils.c
  @@ -0,0 +1,169 @@
  +#include stdio.h
  +#include stdarg.h
  +#include assert.h
  +#include stdbool.h
  +#include hw/acpi/acpi_gen_utils.h
  +
  +GArray *build_alloc_array(void)
  +{
  +return g_array_new(false, true /* clear */, 1);
  +}
  +
  +void build_free_array(GArray *array)
  +{
  +g_array_free(array, true);
  +}
  +
  +void build_prepend_byte(GArray *array, uint8_t val)
  +{
  +g_array_prepend_val(array, val);
  +}
  +
  +void build_append_byte(GArray *array, uint8_t val)
  +{
  +g_array_append_val(array, val);
  +}
  +
  +void build_append_array(GArray *array, GArray *val)
  +{
  +g_array_append_vals(array, val-data, val-len);
  +}
  +
  +#define ACPI_NAMESEG_LEN 4
  +
  +void GCC_FMT_ATTR(2, 3)
  +build_append_nameseg(GArray *array, const char *format, ...)
  +{
  +/* It would be nicer to use g_string_vprintf but it's only there in 
  2.22 */
  +char s[] = ;
  +int len;
  +va_list args;
  +const char padding = '_';
  +
  +va_start(args, format);
  +len = vsnprintf(s, sizeof s, format, args);
  +va_end(args);
  +
  +g_assert(len = ACPI_NAMESEG_LEN);
  +
  +g_array_append_vals(array, s, len);
  +while (len != ACPI_NAMESEG_LEN) {
  +g_array_append_val(array, padding);
  +++len;
  +}
  +}
  +
  +/* 5.4 Definition Block Encoding */
  +enum {
  +PACKAGE_LENGTH_1BYTE_SHIFT = 6, /* Up to 63 - use extra 2 bits. */
  +PACKAGE_LENGTH_2BYTE_SHIFT = 4,
  +PACKAGE_LENGTH_3BYTE_SHIFT = 12,
  +PACKAGE_LENGTH_4BYTE_SHIFT = 20,
  +};
  +
  +void build_prepend_package_length(GArray *package, unsigned min_bytes)
  +{
  +uint8_t byte;
  +unsigned length = package-len;
  +unsigned length_bytes;
  +
  +if (length + 1  (1  PACKAGE_LENGTH_1BYTE_SHIFT)) {
  +length_bytes = 1;
  +} else if (length + 2  (1  PACKAGE_LENGTH_3BYTE_SHIFT)) {
  +length_bytes = 2;
  +} else if (length + 3  (1  PACKAGE_LENGTH_4BYTE_SHIFT)) {
  +length_bytes = 3;
  +} else {
  +length_bytes = 4;
  +}
  +
  +/* Force length to at least min_bytes.
  + * This wastes memory but that's how bios did it.
  + */
  +length_bytes = MAX(length_bytes, min_bytes);
  +
  +/* PkgLength is the length of the inclusive length of the data. */
  +length += length_bytes;
  +
  +switch (length_bytes) {
  +case 1:
  +byte = length;
  +build_prepend_byte(package, byte);
  +return;
  +case 4:
  +byte = length  PACKAGE_LENGTH_4BYTE_SHIFT;
  +build_prepend_byte(package, byte);
  +length = (1  PACKAGE_LENGTH_4BYTE_SHIFT) - 1;
  +/* fall through */
  +case 3:
  +byte = length  PACKAGE_LENGTH_3BYTE_SHIFT;
  +build_prepend_byte(package, byte);
  +length = (1  PACKAGE_LENGTH_3BYTE_SHIFT) - 1;
  +/* fall through */
  +case 2:
  +byte = length  PACKAGE_LENGTH_2BYTE_SHIFT;
  +build_prepend_byte(package, byte);
  +length = (1  PACKAGE_LENGTH_2BYTE_SHIFT) - 1;
  +/* fall through */
  +}
  +/*
  + * Most significant two bits of byte zero indicate how many following 
  bytes
  + * are in PkgLength encoding.
  + */
  +byte = ((length_bytes - 1)  PACKAGE_LENGTH_1BYTE_SHIFT) | length;
  +build_prepend_byte(package, byte);
  +}
  +
  +void build_package(GArray *package, uint8_t op, unsigned min_bytes)
  +{
  +

Re: [Qemu-devel] [PATCH 4/9] acpi: build_append_nameseg(): add padding if necessary

2014-12-09 Thread Michael S. Tsirkin
On Tue, Dec 09, 2014 at 11:32:45AM +0100, Igor Mammedov wrote:
 On Mon, 8 Dec 2014 23:15:32 +0200
 Michael S. Tsirkin m...@redhat.com wrote:
 
  On Mon, Dec 08, 2014 at 04:08:03PM +, Igor Mammedov wrote:
   According to ACPI spec NameSeg shorter than 4 characters
   must be padded up to 4 characters with _ symbol.
   ACPI 5.0:  20.2.2 Name Objects Encoding
   
   Do it in build_append_nameseg() so that caller shouldn't know
   or care about it.
   
   Signed-off-by: Igor Mammedov imamm...@redhat.com
  
  To me just doing it right in callers seems just as easy, but
  I guess you disagree :)
 That means, author MUST know about padding, if he/she doesn't
 or forget about it that would introduce error usually resulting
 in BSOD.

Not really. assert will trigger on qemu start.

 This patch allows to avoid such mistakes.

Even the most basic testing (e.g. make check) will find these mistakes.
It's more a question of which API we prefer.

Anyway, could you respond on g_assert vs assert please?

  
   ---
hw/i386/acpi-build.c | 18 +-
1 file changed, 13 insertions(+), 5 deletions(-)
   
   diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
   index f5ec66a..a8b7a2b 100644
   --- a/hw/i386/acpi-build.c
   +++ b/hw/i386/acpi-build.c
   @@ -292,6 +292,8 @@ static inline void build_append_array(GArray *array, 
   GArray *val)
g_array_append_vals(array, val-data, val-len);
}

   +#define ACPI_NAMESEG_LEN 4
   +
static void GCC_FMT_ATTR(2, 3)
build_append_nameseg(GArray *array, const char *format, ...)
{
   @@ -299,13 +301,19 @@ build_append_nameseg(GArray *array, const char 
   *format, ...)
char s[] = ;
int len;
va_list args;
   +const char padding = '_';

va_start(args, format);
len = vsnprintf(s, sizeof s, format, args);
va_end(args);

   -assert(len == 4);
   +g_assert(len = ACPI_NAMESEG_LEN);
  
  I'm not sure when is g_assert preferable to assert.
  What's the motivation here?
  
  
   +
g_array_append_vals(array, s, len);
   +while (len != ACPI_NAMESEG_LEN) {
   +g_array_append_val(array, padding);
   +++len;
   +}
  
  Easier
  
  /* Pad up to 4 characters if necessary. */
  g_array_append_vals(array, , 4 - len);
  
  
  
}

/* 5.4 Definition Block Encoding */
   @@ -846,7 +854,7 @@ static void build_pci_bus_end(PCIBus *bus, void 
   *bus_state)

if (bus-parent_dev) {
op = 0x82; /* DeviceOp */
   -build_append_nameseg(bus_table, S%.02X_,
   +build_append_nameseg(bus_table, S%.02X,
 bus-parent_dev-devfn);
build_append_byte(bus_table, 0x08); /* NameOp */
build_append_nameseg(bus_table, _SUN);
   @@ -966,7 +974,7 @@ static void build_pci_bus_end(PCIBus *bus, void 
   *bus_state)
build_append_int(notify, 0x1U  i);
build_append_byte(notify, 0x00); /* NullName */
build_append_byte(notify, 0x86); /* NotifyOp */
   -build_append_nameseg(notify, S%.02X_, PCI_DEVFN(i, 0));
   +build_append_nameseg(notify, S%.02X, PCI_DEVFN(i, 0));
build_append_byte(notify, 0x69); /* Arg1Op */

/* Pack it up */
   @@ -1023,7 +1031,7 @@ static void build_pci_bus_end(PCIBus *bus, void 
   *bus_state)
if (bus-parent_dev) {
build_append_byte(parent-notify_table, '^'); /* 
   ParentPrefixChar */
build_append_byte(parent-notify_table, 0x2E); /* 
   DualNamePrefix */
   -build_append_nameseg(parent-notify_table, S%.02X_,
   +build_append_nameseg(parent-notify_table, S%.02X,
 bus-parent_dev-devfn);
build_append_nameseg(parent-notify_table, PCNT);
}
   @@ -1093,7 +1101,7 @@ build_ssdt(GArray *table_data, GArray *linker,
GArray *sb_scope = build_alloc_array();
uint8_t op = 0x10; /* ScopeOp */

   -build_append_nameseg(sb_scope, _SB_);
   +build_append_nameseg(sb_scope, _SB);

/* build Processor object for each processor */
for (i = 0; i  acpi_cpus; i++) {
   -- 
   1.8.3.1



Re: [Qemu-devel] [PATCH 6/9] acpi: add build_append_namestring() helper

2014-12-09 Thread Igor Mammedov
On Mon, 8 Dec 2014 22:21:20 +0200
Michael S. Tsirkin m...@redhat.com wrote:

 On Mon, Dec 08, 2014 at 04:08:05PM +, Igor Mammedov wrote:
  Use build_append_namestring() instead of build_append_nameseg()
  So user won't have to care whether name is NameSeg, NamePath or
  NameString.
  
  See for refernce ACPI 5.0: 20.2.2 Name Objects Encoding
  
  Signed-off-by: Igor Mammedov imamm...@redhat.com
 
 Well that's wasteful since most places do have simple 4 byte names.
What do you mean under wasteful?

 How about we replace where this is needed?
 
 
  ---
   hw/acpi/acpi_gen_utils.c | 86 
  +++-
   hw/i386/acpi-build.c | 35 
   include/hw/acpi/acpi_gen_utils.h |  2 +-
   3 files changed, 102 insertions(+), 21 deletions(-)
  
  diff --git a/hw/acpi/acpi_gen_utils.c b/hw/acpi/acpi_gen_utils.c
  index 1583b35..31e2694 100644
  --- a/hw/acpi/acpi_gen_utils.c
  +++ b/hw/acpi/acpi_gen_utils.c
  @@ -31,7 +31,7 @@ void build_append_array(GArray *array, GArray *val)
   
   #define ACPI_NAMESEG_LEN 4
   
  -void GCC_FMT_ATTR(2, 3)
  +static void GCC_FMT_ATTR(2, 3)
   build_append_nameseg(GArray *array, const char *format, ...)
   {
   /* It would be nicer to use g_string_vprintf but it's only there in 
  2.22 */
  @@ -53,6 +53,90 @@ build_append_nameseg(GArray *array, const char *format, 
  ...)
   }
   }
   
  +static const uint8_t ACPI_DualNamePrefix  = 0x2E;
  +static const uint8_t ACPI_MultiNamePrefix = 0x2F;
  +
  +static void
  +build_append_namestringv(GArray *array, const char *format, va_list ap)
  +{
  +/* It would be nicer to use g_string_vprintf but it's only there in 
  2.22 */
  +char *s;
  +int len;
  +va_list va_len;
  +char **segs;
  +char **segs_iter;
  +int seg_count = 0;
  +
  +va_copy(va_len, ap);
  +len = vsnprintf(NULL, 0, format, va_len);
  +va_end(va_len);
  +len += 1;
  +s = g_new(typeof(*s), len);
  +
  +len = vsnprintf(s, len, format, ap);
  +
  +segs = g_strsplit(s, ., 0);
  +g_free(s);
  +
  +/* count segments */
  +segs_iter = segs;
  +while (*segs_iter) {
  +++segs_iter;
  +++seg_count;
  +}
  +
  +/* handle RootPath || PrefixPath */
  +s = *segs;
  +while (*s == '\\' || *s == '^') {
  +g_array_append_val(array, *s);
  +++s;
  +}
  +
  +switch (seg_count) {
  +case 1:
  +if (!*s) { /* NullName */
  +const uint8_t nullpath = 0;
  +g_array_append_val(array, nullpath);
  +} else {
  +build_append_nameseg(array, %s, s);
  +}
  +break;
  +
  +case 2:
  +g_array_append_val(array, ACPI_DualNamePrefix);
  +build_append_nameseg(array, %s, s);
  +build_append_nameseg(array, %s, segs[1]);
  +break;
  +
  +default:
  +g_array_append_val(array, ACPI_MultiNamePrefix);
  +assert(seg_count = 0xFF); /* ByteData Max */
  +g_array_append_val(array, seg_count);
  +
  +/* handle the 1st segment manually due to prefix/root path */
  +build_append_nameseg(array, %s, s);
  +
  +/* add the rest of segments */
  +segs_iter = segs + 1;
  +while (*segs_iter) {
  +build_append_nameseg(array, %s, *segs_iter);
  +++segs_iter;
  +}
  +break;
  +}
  +g_strfreev(segs);
  +}
  +
  +void GCC_FMT_ATTR(2, 3)
  +build_append_namestring(GArray *array, const char *format, ...)
  +{
  +va_list ap;
  +
  +va_start(ap, format);
  +build_append_namestringv(array, format, ap);
  +va_end(ap);
  +}
  +
   /* 5.4 Definition Block Encoding */
   enum {
   PACKAGE_LENGTH_1BYTE_SHIFT = 6, /* Up to 63 - use extra 2 bits. */
  diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
  index 870e4a0..0f6202d 100644
  --- a/hw/i386/acpi-build.c
  +++ b/hw/i386/acpi-build.c
  @@ -273,7 +273,7 @@ static GArray *build_alloc_method(const char *name, 
  uint8_t arg_count)
   {
   GArray *method = build_alloc_array();
   
  -build_append_nameseg(method, %s, name);
  +build_append_namestring(method, %s, name);
   build_append_byte(method, arg_count); /* MethodFlags: ArgCount */
   
   return method;
  @@ -565,7 +565,7 @@ build_append_notify_method(GArray *device, const char 
  *name,
   
   for (i = 0; i  count; i++) {
   GArray *target = build_alloc_array();
  -build_append_nameseg(target, format, i);
  +build_append_namestring(target, format, i);
   assert(i  256); /* Fits in 1 byte */
   build_append_notify_target_ifequal(method, target, i, 1);
   build_free_array(target);
  @@ -693,24 +693,24 @@ static void build_pci_bus_end(PCIBus *bus, void 
  *bus_state)
   
   if (bus-parent_dev) {
   op = 0x82; /* DeviceOp */
  -build_append_nameseg(bus_table, S%.02X,
  +build_append_namestring(bus_table, S%.02X,

[Qemu-devel] Cubietruck: cannot create KVM guests: kvm_init_vcpu failed: Invalid argument

2014-12-09 Thread Kashyap Chamarthy
Booting a minimal KVM guest throws the below error on Cubietruck:

  kvm_init_vcpu failed: Invalid argument

More context and an easy reproducer in this QEMU bug[1] for Fedora.

Context quoting Rich Jones from comment #2:

For some reason I thought this had been fixed upstream, but
now that I've finally got my CT working again, I see that I
am still carrying that patch in my custom qemu.

diff --git a/target-arm/cpu.c b/target-arm/cpu.c
index 5ce7350..04d69d1 100644
--- a/target-arm/cpu.c
+++ b/target-arm/cpu.c
@@ -858,7 +858,7 @@ static void cortex_a15_initfn(Object *obj)
 set_feature(cpu-env, ARM_FEATURE_DUMMY_C15_REGS);
 set_feature(cpu-env, ARM_FEATURE_CBAR_RO);
 set_feature(cpu-env, ARM_FEATURE_LPAE);
-cpu-kvm_target = QEMU_KVM_ARM_TARGET_CORTEX_A15;
+cpu-kvm_target = QEMU_KVM_ARM_TARGET_CORTEX_A7;
 cpu-midr = 0x412fc0f1;
 cpu-reset_fpsid = 0x410430f0;
 cpu-mvfr0 = 0x10110222;

So that's the answer really, it's a qemu bug.  Actually it looks as
if qemu contains some code to try to get the host CPU type, but it
doesn't work, or maybe we need to pass a -cpu option ...



  [1] https://bugzilla.redhat.com/show_bug.cgi?id=1171501 -- Cubietruck:
  cannot create KVM guests: kvm_init_vcpu failed: Invalid argument

-- 
/kashyap



Re: [Qemu-devel] Cubietruck: cannot create KVM guests: kvm_init_vcpu failed: Invalid argument

2014-12-09 Thread Peter Maydell
On 9 December 2014 at 10:50, Kashyap Chamarthy kcham...@redhat.com wrote:
 Booting a minimal KVM guest throws the below error on Cubietruck:

   kvm_init_vcpu failed: Invalid argument

 More context and an easy reproducer in this QEMU bug[1] for Fedora.

 Context quoting Rich Jones from comment #2:

 For some reason I thought this had been fixed upstream, but
 now that I've finally got my CT working again, I see that I
 am still carrying that patch in my custom qemu.

 diff --git a/target-arm/cpu.c b/target-arm/cpu.c
 index 5ce7350..04d69d1 100644
 --- a/target-arm/cpu.c
 +++ b/target-arm/cpu.c
 @@ -858,7 +858,7 @@ static void cortex_a15_initfn(Object *obj)
  set_feature(cpu-env, ARM_FEATURE_DUMMY_C15_REGS);
  set_feature(cpu-env, ARM_FEATURE_CBAR_RO);
  set_feature(cpu-env, ARM_FEATURE_LPAE);
 -cpu-kvm_target = QEMU_KVM_ARM_TARGET_CORTEX_A15;
 +cpu-kvm_target = QEMU_KVM_ARM_TARGET_CORTEX_A7;
  cpu-midr = 0x412fc0f1;
  cpu-reset_fpsid = 0x410430f0;
  cpu-mvfr0 = 0x10110222;

This is obviously a bogus patch -- this is the initfn
for Cortex-A15 so telling it to be a Cortex-A7 is wrong
(and would break working setups on A15 hosts).

 So that's the answer really, it's a qemu bug.  Actually it looks as
 if qemu contains some code to try to get the host CPU type, but it
 doesn't work, or maybe we need to pass a -cpu option ...

Yes, you need to pass a -cpu option. For KVM on ARM, you
always need to either:
 * pass a -cpu option matching the host CPU
 * pass -cpu host

The CubieTruck is a Cortex-A7, which we don't have specific
support for in QEMU, so you will need -cpu host.

thanks
-- PMM



Re: [Qemu-devel] [PATCH 7/9] acpi: replace opencoded notify codes with named values

2014-12-09 Thread Igor Mammedov
On Mon, 8 Dec 2014 22:54:03 +0200
Michael S. Tsirkin m...@redhat.com wrote:

 On Mon, Dec 08, 2014 at 04:08:06PM +, Igor Mammedov wrote:
  Signed-off-by: Igor Mammedov imamm...@redhat.com
 
 I'm not sure this makes sense for these constants.
 
 Device Check seems more readable than ACPI_DEV_CHK.
 If your object here is readability, please do not
 abbreviate.
 
 Generally ability to match spec names exactly is
 what made me prefer code comments to enums for one-off constants.
 Looking up Device Check works in any spec version,
 I don't have to dig up the exact one, find table by name,
 break my eyes trying to locate the correct line in
 a huge table.
 Just text search, and the correct line is highlighted.
 
 Why look in another spec version you might ask?
 Well it's definitely helpful to understand legacy
 guest quirks.
 
 

sure, lets drop it.

  ---
   hw/i386/acpi-build.c | 4 ++--
   include/hw/acpi/acpi_gen_utils.h | 6 ++
   2 files changed, 8 insertions(+), 2 deletions(-)
  
  diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
  index 0f6202d..a33d130 100644
  --- a/hw/i386/acpi-build.c
  +++ b/hw/i386/acpi-build.c
  @@ -840,10 +840,10 @@ static void build_pci_bus_end(PCIBus *bus, void 
  *bus_state)
   build_append_namestring(method, BNUM);
   build_append_namestring(method, DVNT);
   build_append_namestring(method, PCIU);
  -build_append_int(method, 1); /* Device Check */
  +build_append_int(method, ACPI_DEV_CHK);
   build_append_namestring(method, DVNT);
   build_append_namestring(method, PCID);
  -build_append_int(method, 3); /* Eject Request */
  +build_append_int(method, ACPI_DEV_EJ);
   }
   
   /* Notify about child bus events in any case */
  diff --git a/include/hw/acpi/acpi_gen_utils.h 
  b/include/hw/acpi/acpi_gen_utils.h
  index fd50625..ce76dc1 100644
  --- a/include/hw/acpi/acpi_gen_utils.h
  +++ b/include/hw/acpi/acpi_gen_utils.h
  @@ -5,6 +5,12 @@
   #include glib.h
   #include qemu/compiler.h
   
  +/* ACPI 5.0: table Device Object Notification Values */
  +enum {
  +ACPI_DEV_CHK = 1,
  +ACPI_DEV_EJ = 3,
  +};
  +
   GArray *build_alloc_array(void);
   void build_free_array(GArray *array);
   void build_prepend_byte(GArray *array, uint8_t val);
  -- 
  1.8.3.1
 




Re: [Qemu-devel] [PATCH 0/2] support migration/save/load on AArch64 CPUs

2014-12-09 Thread Pranavkumar Sawargaonkar
Hi PMM,

On 5 December 2014 at 19:41, Peter Maydell peter.mayd...@linaro.org wrote:
 These patches implement support for migration/save/load on AArch64
 CPUs. The first one from Alex (with some mangling from me) just
 moves the sysreg sync code we have for 32-bit across to 64-bit.
 The second fills in the gaps in the main CPU vmstate struct.

 Notes:
  * currently doing 'from cold' incoming migration with -loadvm
or -incoming command line arguments is broken if using the VGIC;
this is a kernel bug, fixed by this patch:
http://www.spinics.net/lists/arm-kernel/msg383736.html
  * Pranav: patch 1 here is what you need to rebase the bigendian
virtio patch on so it can use a single common hook for 32 and
64 bit without hacks to work around missing register sync

Thanks for the patch.
I have applied this patch and called cpu_synchronize_state() to get
sctlr_el1 value in env-cp15.c1_sys  during virtio endianness
determination while ARM64 guest was booting , but value getting read
is 0.
Do I need to do anything else ?

Thanks,
Pranav

  * Christoffer: patch 1 is also the one which moves reset around

 Alex Bennée (1):
   target-arm/kvm: make reg sync code common between kvm32/64

 Peter Maydell (1):
   target-arm: Support save/load for 64 bit CPUs

  target-arm/kvm.c | 98 
 
  target-arm/kvm32.c   | 94 ++---
  target-arm/kvm64.c   | 24 +++--
  target-arm/kvm_arm.h | 22 
  target-arm/machine.c | 22 ++--
  5 files changed, 156 insertions(+), 104 deletions(-)

 --
 1.9.1




Re: [Qemu-devel] Cubietruck: cannot create KVM guests: kvm_init_vcpu failed: Invalid argument

2014-12-09 Thread Richard W.M. Jones
On Tue, Dec 09, 2014 at 11:50:31AM +0100, Kashyap Chamarthy wrote:
 Booting a minimal KVM guest throws the below error on Cubietruck:
 
   kvm_init_vcpu failed: Invalid argument
 
 More context and an easy reproducer in this QEMU bug[1] for Fedora.
 
 Context quoting Rich Jones from comment #2:
 
 For some reason I thought this had been fixed upstream, but
 now that I've finally got my CT working again, I see that I
 am still carrying that patch in my custom qemu.
 
 diff --git a/target-arm/cpu.c b/target-arm/cpu.c
 index 5ce7350..04d69d1 100644
 --- a/target-arm/cpu.c
 +++ b/target-arm/cpu.c
 @@ -858,7 +858,7 @@ static void cortex_a15_initfn(Object *obj)
  set_feature(cpu-env, ARM_FEATURE_DUMMY_C15_REGS);
  set_feature(cpu-env, ARM_FEATURE_CBAR_RO);
  set_feature(cpu-env, ARM_FEATURE_LPAE);
 -cpu-kvm_target = QEMU_KVM_ARM_TARGET_CORTEX_A15;
 +cpu-kvm_target = QEMU_KVM_ARM_TARGET_CORTEX_A7;
  cpu-midr = 0x412fc0f1;
  cpu-reset_fpsid = 0x410430f0;
  cpu-mvfr0 = 0x10110222;
 
 So that's the answer really, it's a qemu bug.  Actually it looks as
 if qemu contains some code to try to get the host CPU type, but it
 doesn't work, or maybe we need to pass a -cpu option ...

Kashyap,

Can you try modifying src/launch-direct.c to see if we can pass

  -cpu cortex-a7

and if that fixes the problem?

If that does solve the problem, the question becomes how to detect the
right CPU (either cortex-a7, cortex-a15, cortex-a57, ...)  This
information isn't easily available to libguestfs AFAIK.

I really think that qemu should just do the right thing though.

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-p2v converts physical machines to virtual machines.  Boot with a
live CD or over the network (PXE) and turn machines into KVM guests.
http://libguestfs.org/virt-v2v



Re: [Qemu-devel] Cubietruck: cannot create KVM guests: kvm_init_vcpu failed: Invalid argument

2014-12-09 Thread Richard W.M. Jones
On Tue, Dec 09, 2014 at 10:53:41AM +, Peter Maydell wrote:
 On 9 December 2014 at 10:50, Kashyap Chamarthy kcham...@redhat.com wrote:
  Booting a minimal KVM guest throws the below error on Cubietruck:
 
kvm_init_vcpu failed: Invalid argument
 
  More context and an easy reproducer in this QEMU bug[1] for Fedora.
 
  Context quoting Rich Jones from comment #2:
 
  For some reason I thought this had been fixed upstream, but
  now that I've finally got my CT working again, I see that I
  am still carrying that patch in my custom qemu.
 
  diff --git a/target-arm/cpu.c b/target-arm/cpu.c
  index 5ce7350..04d69d1 100644
  --- a/target-arm/cpu.c
  +++ b/target-arm/cpu.c
  @@ -858,7 +858,7 @@ static void cortex_a15_initfn(Object *obj)
   set_feature(cpu-env, ARM_FEATURE_DUMMY_C15_REGS);
   set_feature(cpu-env, ARM_FEATURE_CBAR_RO);
   set_feature(cpu-env, ARM_FEATURE_LPAE);
  -cpu-kvm_target = QEMU_KVM_ARM_TARGET_CORTEX_A15;
  +cpu-kvm_target = QEMU_KVM_ARM_TARGET_CORTEX_A7;
   cpu-midr = 0x412fc0f1;
   cpu-reset_fpsid = 0x410430f0;
   cpu-mvfr0 = 0x10110222;
 
 This is obviously a bogus patch -- this is the initfn
 for Cortex-A15 so telling it to be a Cortex-A7 is wrong
 (and would break working setups on A15 hosts).

Yes, I definitely was not proposing this as a serious patch!  In my
copy of qemu it has HACK in all capital letters in the summary ...

  So that's the answer really, it's a qemu bug.  Actually it looks as
  if qemu contains some code to try to get the host CPU type, but it
  doesn't work, or maybe we need to pass a -cpu option ...
 
 Yes, you need to pass a -cpu option. For KVM on ARM, you
 always need to either:
  * pass a -cpu option matching the host CPU
  * pass -cpu host
 
 The CubieTruck is a Cortex-A7, which we don't have specific
 support for in QEMU, so you will need -cpu host.

Kashyap ^^ can you try this?  Should be a trivial one-liner change
in src/launch-direct.c.

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-builder quickly builds VMs from scratch
http://libguestfs.org/virt-builder.1.html



[Qemu-devel] New Bee : What is the canonical path.

2014-12-09 Thread boddu pavan
Hi, 
I am new to qemu, And i need help in understanding a part of code,  Can any one 
tell the use of Canonical paths of the Object.
Thanks,Sai Pavan

Re: [Qemu-devel] Cubietruck: cannot create KVM guests: kvm_init_vcpu failed: Invalid argument

2014-12-09 Thread Peter Maydell
On 9 December 2014 at 11:25, Richard W.M. Jones rjo...@redhat.com wrote:
 I really think that qemu should just do the right thing though.

Tricky, because you have to use the same CPU as the host if
you want to use KVM, and that conflicts with wanting to do the
same thing on all host platforms for a given set of command line
options. If you have some concrete suggestions for improving
things that don't break backwards-compatibility I'm happy to
hear them, though.

-- PMM



Re: [Qemu-devel] [PATCH 0/2] support migration/save/load on AArch64 CPUs

2014-12-09 Thread Peter Maydell
On 9 December 2014 at 11:07, Pranavkumar Sawargaonkar
pranavku...@linaro.org wrote:
 Thanks for the patch.
 I have applied this patch and called cpu_synchronize_state() to get
 sctlr_el1 value in env-cp15.c1_sys  during virtio endianness
 determination while ARM64 guest was booting , but value getting read
 is 0.
 Do I need to do anything else ?

Hmm, on a 64 bit host that should work. (There's an issue with
32-bit hosts that means we won't sync registers properly which
I'm working on, but it shouldn't affect 64-bit.) I suggest you
debug further to see why the sync doesn't get the right SCTLR
value across.

-- PMM



Re: [Qemu-devel] [PATCH 2/9] pc: acpi: decribe bridge device as not hotpluggable

2014-12-09 Thread Igor Mammedov
On Tue, 9 Dec 2014 12:34:02 +0200
Michael S. Tsirkin m...@redhat.com wrote:

 On Tue, Dec 09, 2014 at 11:27:16AM +0100, Igor Mammedov wrote:
  On Mon, 8 Dec 2014 21:13:25 +0200
  Michael S. Tsirkin m...@redhat.com wrote:
  
   On Mon, Dec 08, 2014 at 04:08:01PM +, Igor Mammedov wrote:
when bridge hotplug is disabled, i.e. for machine
types less then 2.0, bridge device was created as
hotpluggable by mistake since commit 133a2da.

Fix it by just creating it as a present device.

Signed-off-by: Igor Mammedov imamm...@redhat.com
   
   What exactly is the problem here?
   It seems that such bridge is hotpluggable, even though
   e.g. windows guests lacks drivers to support this.
  before 133a2da slot with existing at startup bridge weren't
  marked as hotpluggable nor described in SSDT. But after
  133a2da it's described as hotpluggable slot for compat
  machines (2.0 and lower) which doesn't match with original
  behavior.
  
  Also Marcel mentioned that bridges could be hotpluggable
  but that they should not be hot-UNpluggable.
 
 OK so is there some guest that's confused?
 What's the bug?
It allows to do eject on bridge when it shouldn't.
I don't know about consequences of it,
it's better not to allow invalid action in the first place.




 
   
   
---
 hw/i386/acpi-build.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
index b37a397..1fb92e5 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -913,7 +913,7 @@ static void build_pci_bus_end(PCIBus *bus, void 
*bus_state)
 }
 }
 
-if (!dc-hotpluggable || bridge_in_acpi) {
+if (!dc-hotpluggable || pc-is_bridge) {
 clear_bit(slot, slot_hotplug_enable);
 }
 }
-- 
1.8.3.1




Re: [Qemu-devel] [PATCH 6/9] acpi: add build_append_namestring() helper

2014-12-09 Thread Michael S. Tsirkin
On Tue, Dec 09, 2014 at 11:39:39AM +0100, Igor Mammedov wrote:
 On Mon, 8 Dec 2014 22:21:20 +0200
 Michael S. Tsirkin m...@redhat.com wrote:
 
  On Mon, Dec 08, 2014 at 04:08:05PM +, Igor Mammedov wrote:
   Use build_append_namestring() instead of build_append_nameseg()
   So user won't have to care whether name is NameSeg, NamePath or
   NameString.
   
   See for refernce ACPI 5.0: 20.2.2 Name Objects Encoding
   
   Signed-off-by: Igor Mammedov imamm...@redhat.com
  
  Well that's wasteful since most places do have simple 4 byte names.
 What do you mean under wasteful?

I think I misunderstood what this patch does:
you mean select the correct name type based on
the string used?

I'll have to re-read it, sorry about the noise.

  How about we replace where this is needed?
  
  
   ---
hw/acpi/acpi_gen_utils.c | 86 
   +++-
hw/i386/acpi-build.c | 35 
include/hw/acpi/acpi_gen_utils.h |  2 +-
3 files changed, 102 insertions(+), 21 deletions(-)
   
   diff --git a/hw/acpi/acpi_gen_utils.c b/hw/acpi/acpi_gen_utils.c
   index 1583b35..31e2694 100644
   --- a/hw/acpi/acpi_gen_utils.c
   +++ b/hw/acpi/acpi_gen_utils.c
   @@ -31,7 +31,7 @@ void build_append_array(GArray *array, GArray *val)

#define ACPI_NAMESEG_LEN 4

   -void GCC_FMT_ATTR(2, 3)
   +static void GCC_FMT_ATTR(2, 3)
build_append_nameseg(GArray *array, const char *format, ...)
{
/* It would be nicer to use g_string_vprintf but it's only there in 
   2.22 */
   @@ -53,6 +53,90 @@ build_append_nameseg(GArray *array, const char 
   *format, ...)
}
}

   +static const uint8_t ACPI_DualNamePrefix  = 0x2E;
   +static const uint8_t ACPI_MultiNamePrefix = 0x2F;
   +
   +static void
   +build_append_namestringv(GArray *array, const char *format, va_list ap)
   +{
   +/* It would be nicer to use g_string_vprintf but it's only there in 
   2.22 */
   +char *s;
   +int len;
   +va_list va_len;
   +char **segs;
   +char **segs_iter;
   +int seg_count = 0;
   +
   +va_copy(va_len, ap);
   +len = vsnprintf(NULL, 0, format, va_len);
   +va_end(va_len);
   +len += 1;
   +s = g_new(typeof(*s), len);
   +
   +len = vsnprintf(s, len, format, ap);
   +
   +segs = g_strsplit(s, ., 0);
   +g_free(s);
   +
   +/* count segments */
   +segs_iter = segs;
   +while (*segs_iter) {
   +++segs_iter;
   +++seg_count;
   +}
   +
   +/* handle RootPath || PrefixPath */
   +s = *segs;
   +while (*s == '\\' || *s == '^') {
   +g_array_append_val(array, *s);
   +++s;
   +}
   +
   +switch (seg_count) {
   +case 1:
   +if (!*s) { /* NullName */
   +const uint8_t nullpath = 0;
   +g_array_append_val(array, nullpath);
   +} else {
   +build_append_nameseg(array, %s, s);
   +}
   +break;
   +
   +case 2:
   +g_array_append_val(array, ACPI_DualNamePrefix);
   +build_append_nameseg(array, %s, s);
   +build_append_nameseg(array, %s, segs[1]);
   +break;
   +
   +default:
   +g_array_append_val(array, ACPI_MultiNamePrefix);
   +assert(seg_count = 0xFF); /* ByteData Max */
   +g_array_append_val(array, seg_count);
   +
   +/* handle the 1st segment manually due to prefix/root path */
   +build_append_nameseg(array, %s, s);
   +
   +/* add the rest of segments */
   +segs_iter = segs + 1;
   +while (*segs_iter) {
   +build_append_nameseg(array, %s, *segs_iter);
   +++segs_iter;
   +}
   +break;
   +}
   +g_strfreev(segs);
   +}
   +
   +void GCC_FMT_ATTR(2, 3)
   +build_append_namestring(GArray *array, const char *format, ...)
   +{
   +va_list ap;
   +
   +va_start(ap, format);
   +build_append_namestringv(array, format, ap);
   +va_end(ap);
   +}
   +
/* 5.4 Definition Block Encoding */
enum {
PACKAGE_LENGTH_1BYTE_SHIFT = 6, /* Up to 63 - use extra 2 bits. */
   diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
   index 870e4a0..0f6202d 100644
   --- a/hw/i386/acpi-build.c
   +++ b/hw/i386/acpi-build.c
   @@ -273,7 +273,7 @@ static GArray *build_alloc_method(const char *name, 
   uint8_t arg_count)
{
GArray *method = build_alloc_array();

   -build_append_nameseg(method, %s, name);
   +build_append_namestring(method, %s, name);
build_append_byte(method, arg_count); /* MethodFlags: ArgCount */

return method;
   @@ -565,7 +565,7 @@ build_append_notify_method(GArray *device, const char 
   *name,

for (i = 0; i  count; i++) {
GArray *target = build_alloc_array();
   -build_append_nameseg(target, format, i);
   +build_append_namestring(target, format, i);
assert(i  256); /* Fits in 1 

[Qemu-devel] [PATCH v2 1/4] Start migrating migration code into a migration directory

2014-12-09 Thread Dr. David Alan Gilbert (git)
From: Dr. David Alan Gilbert dgilb...@redhat.com

The migration code now occupies a fair chunk of the top level .c
files, it seems time to give it it's own directory.

I've not touched:
   arch_init.c - that's mostly RAM migration but has a few random other
 bits
   savevm.c- because it's built target specific

This is purely a code move; no code has changed.
   - it fails checkpatch because of old violations, it feels safer
 to keep this as purely a move and fix those at some mythical future
 date.

Signed-off-by: Dr. David Alan Gilbert dgilb...@redhat.com
---
 Makefile.objs| 10 ++
 migration/Makefile.objs  | 10 ++
 block-migration.c = migration/block-migration.c |  0
 migration-exec.c = migration/migration-exec.c   |  0
 migration-fd.c = migration/migration-fd.c   |  0
 migration-rdma.c = migration/migration-rdma.c   |  0
 migration-tcp.c = migration/migration-tcp.c |  0
 migration-unix.c = migration/migration-unix.c   |  0
 migration.c = migration/migration.c |  0
 qemu-file-stdio.c = migration/qemu-file-stdio.c |  0
 qemu-file-unix.c = migration/qemu-file-unix.c   |  0
 qemu-file.c = migration/qemu-file.c |  0
 vmstate.c = migration/vmstate.c |  0
 xbzrle.c = migration/xbzrle.c   |  0
 tests/Makefile   |  6 +++---
 15 files changed, 15 insertions(+), 11 deletions(-)
 create mode 100644 migration/Makefile.objs
 rename block-migration.c = migration/block-migration.c (100%)
 rename migration-exec.c = migration/migration-exec.c (100%)
 rename migration-fd.c = migration/migration-fd.c (100%)
 rename migration-rdma.c = migration/migration-rdma.c (100%)
 rename migration-tcp.c = migration/migration-tcp.c (100%)
 rename migration-unix.c = migration/migration-unix.c (100%)
 rename migration.c = migration/migration.c (100%)
 rename qemu-file-stdio.c = migration/qemu-file-stdio.c (100%)
 rename qemu-file-unix.c = migration/qemu-file-unix.c (100%)
 rename qemu-file.c = migration/qemu-file.c (100%)
 rename vmstate.c = migration/vmstate.c (100%)
 rename xbzrle.c = migration/xbzrle.c (100%)

diff --git a/Makefile.objs b/Makefile.objs
index 18fd35c..abeb902 100644
--- a/Makefile.objs
+++ b/Makefile.objs
@@ -48,15 +48,9 @@ common-obj-$(CONFIG_POSIX) += os-posix.o
 
 common-obj-$(CONFIG_LINUX) += fsdev/
 
-common-obj-y += migration.o migration-tcp.o
-common-obj-y += vmstate.o
-common-obj-y += qemu-file.o qemu-file-unix.o qemu-file-stdio.o
-common-obj-$(CONFIG_RDMA) += migration-rdma.o
+common-obj-y += migration/
 common-obj-y += qemu-char.o #aio.o
-common-obj-y += block-migration.o
-common-obj-y += page_cache.o xbzrle.o
-
-common-obj-$(CONFIG_POSIX) += migration-exec.o migration-unix.o migration-fd.o
+common-obj-y += page_cache.o
 
 common-obj-$(CONFIG_SPICE) += spice-qemu-char.o
 
diff --git a/migration/Makefile.objs b/migration/Makefile.objs
new file mode 100644
index 000..63dbe93
--- /dev/null
+++ b/migration/Makefile.objs
@@ -0,0 +1,10 @@
+common-obj-y += migration.o migration-tcp.o
+common-obj-y += vmstate.o
+common-obj-y += qemu-file.o qemu-file-unix.o qemu-file-stdio.o
+common-obj-$(CONFIG_RDMA) += migration-rdma.o
+common-obj-y += xbzrle.o
+
+common-obj-$(CONFIG_POSIX) += migration-exec.o migration-unix.o migration-fd.o
+
+common-obj-y += block-migration.o
+
diff --git a/block-migration.c b/migration/block-migration.c
similarity index 100%
rename from block-migration.c
rename to migration/block-migration.c
diff --git a/migration-exec.c b/migration/migration-exec.c
similarity index 100%
rename from migration-exec.c
rename to migration/migration-exec.c
diff --git a/migration-fd.c b/migration/migration-fd.c
similarity index 100%
rename from migration-fd.c
rename to migration/migration-fd.c
diff --git a/migration-rdma.c b/migration/migration-rdma.c
similarity index 100%
rename from migration-rdma.c
rename to migration/migration-rdma.c
diff --git a/migration-tcp.c b/migration/migration-tcp.c
similarity index 100%
rename from migration-tcp.c
rename to migration/migration-tcp.c
diff --git a/migration-unix.c b/migration/migration-unix.c
similarity index 100%
rename from migration-unix.c
rename to migration/migration-unix.c
diff --git a/migration.c b/migration/migration.c
similarity index 100%
rename from migration.c
rename to migration/migration.c
diff --git a/qemu-file-stdio.c b/migration/qemu-file-stdio.c
similarity index 100%
rename from qemu-file-stdio.c
rename to migration/qemu-file-stdio.c
diff --git a/qemu-file-unix.c b/migration/qemu-file-unix.c
similarity index 100%
rename from qemu-file-unix.c
rename to migration/qemu-file-unix.c
diff --git a/qemu-file.c b/migration/qemu-file.c
similarity index 100%
rename from qemu-file.c
rename to migration/qemu-file.c
diff --git a/vmstate.c b/migration/vmstate.c
similarity index 100%
rename from vmstate.c
rename to migration/vmstate.c
diff --git a/xbzrle.c 

[Qemu-devel] [PATCH v2 2/4] Remove migration- pre/post fixes off files in migration/ dir

2014-12-09 Thread Dr. David Alan Gilbert (git)
From: Dr. David Alan Gilbert dgilb...@redhat.com

The general feeling is that having migration/migration-blah
is overkill.

Signed-off-by: Dr. David Alan Gilbert dgilb...@redhat.com
---
 migration/Makefile.objs  | 8 
 migration/{block-migration.c = block.c} | 0
 migration/{migration-exec.c = exec.c}   | 0
 migration/{migration-fd.c = fd.c}   | 0
 migration/{migration-rdma.c = rdma.c}   | 0
 migration/{migration-tcp.c = tcp.c} | 0
 migration/{migration-unix.c = unix.c}   | 0
 7 files changed, 4 insertions(+), 4 deletions(-)
 rename migration/{block-migration.c = block.c} (100%)
 rename migration/{migration-exec.c = exec.c} (100%)
 rename migration/{migration-fd.c = fd.c} (100%)
 rename migration/{migration-rdma.c = rdma.c} (100%)
 rename migration/{migration-tcp.c = tcp.c} (100%)
 rename migration/{migration-unix.c = unix.c} (100%)

diff --git a/migration/Makefile.objs b/migration/Makefile.objs
index 63dbe93..ce1e3c7 100644
--- a/migration/Makefile.objs
+++ b/migration/Makefile.objs
@@ -1,10 +1,10 @@
-common-obj-y += migration.o migration-tcp.o
+common-obj-y += migration.o tcp.o
 common-obj-y += vmstate.o
 common-obj-y += qemu-file.o qemu-file-unix.o qemu-file-stdio.o
-common-obj-$(CONFIG_RDMA) += migration-rdma.o
 common-obj-y += xbzrle.o
 
-common-obj-$(CONFIG_POSIX) += migration-exec.o migration-unix.o migration-fd.o
+common-obj-$(CONFIG_RDMA) += rdma.o
+common-obj-$(CONFIG_POSIX) += exec.o unix.o fd.o
 
-common-obj-y += block-migration.o
+common-obj-y += block.o
 
diff --git a/migration/block-migration.c b/migration/block.c
similarity index 100%
rename from migration/block-migration.c
rename to migration/block.c
diff --git a/migration/migration-exec.c b/migration/exec.c
similarity index 100%
rename from migration/migration-exec.c
rename to migration/exec.c
diff --git a/migration/migration-fd.c b/migration/fd.c
similarity index 100%
rename from migration/migration-fd.c
rename to migration/fd.c
diff --git a/migration/migration-rdma.c b/migration/rdma.c
similarity index 100%
rename from migration/migration-rdma.c
rename to migration/rdma.c
diff --git a/migration/migration-tcp.c b/migration/tcp.c
similarity index 100%
rename from migration/migration-tcp.c
rename to migration/tcp.c
diff --git a/migration/migration-unix.c b/migration/unix.c
similarity index 100%
rename from migration/migration-unix.c
rename to migration/unix.c
-- 
2.1.0




Re: [Qemu-devel] Cubietruck: cannot create KVM guests: kvm_init_vcpu failed: Invalid argument

2014-12-09 Thread Kashyap Chamarthy
On Tue, Dec 09, 2014 at 11:27:02AM +, Richard W.M. Jones wrote:
 On Tue, Dec 09, 2014 at 10:53:41AM +, Peter Maydell wrote:
  On 9 December 2014 at 10:50, Kashyap Chamarthy kcham...@redhat.com wrote:
   Booting a minimal KVM guest throws the below error on Cubietruck:
  
 kvm_init_vcpu failed: Invalid argument
  
   More context and an easy reproducer in this QEMU bug[1] for Fedora.
  
   Context quoting Rich Jones from comment #2:
  
   For some reason I thought this had been fixed upstream, but
   now that I've finally got my CT working again, I see that I
   am still carrying that patch in my custom qemu.
  
   diff --git a/target-arm/cpu.c b/target-arm/cpu.c
   index 5ce7350..04d69d1 100644
   --- a/target-arm/cpu.c
   +++ b/target-arm/cpu.c
   @@ -858,7 +858,7 @@ static void cortex_a15_initfn(Object *obj)
set_feature(cpu-env, ARM_FEATURE_DUMMY_C15_REGS);
set_feature(cpu-env, ARM_FEATURE_CBAR_RO);
set_feature(cpu-env, ARM_FEATURE_LPAE);
   -cpu-kvm_target = QEMU_KVM_ARM_TARGET_CORTEX_A15;
   +cpu-kvm_target = QEMU_KVM_ARM_TARGET_CORTEX_A7;
cpu-midr = 0x412fc0f1;
cpu-reset_fpsid = 0x410430f0;
cpu-mvfr0 = 0x10110222;
  
  This is obviously a bogus patch -- this is the initfn
  for Cortex-A15 so telling it to be a Cortex-A7 is wrong
  (and would break working setups on A15 hosts).
 
 Yes, I definitely was not proposing this as a serious patch!  In my
 copy of qemu it has HACK in all capital letters in the summary ...
 
   So that's the answer really, it's a qemu bug.  Actually it looks as
   if qemu contains some code to try to get the host CPU type, but it
   doesn't work, or maybe we need to pass a -cpu option ...
  
  Yes, you need to pass a -cpu option. For KVM on ARM, you
  always need to either:
   * pass a -cpu option matching the host CPU
   * pass -cpu host
  
  The CubieTruck is a Cortex-A7, which we don't have specific
  support for in QEMU, so you will need -cpu host.
 
 Kashyap ^^ can you try this?  Should be a trivial one-liner change
 in src/launch-direct.c.

Yeah, libguestfs build in progress on Cubietruck (it's slow), will test.

-- 
/kashyap



[Qemu-devel] [PATCH v2 0/4] More migration file cleanup

2014-12-09 Thread Dr. David Alan Gilbert (git)
From: Dr. David Alan Gilbert dgilb...@redhat.com

This is a small set of patches that shuffle migration code
around, but doesn't change the behaviour:

  1) Move a lot of the migration source into a separate 'migration' directory
 Note this moves a lot of files around, git format-patch -M -B spots the
 renames
  2) Now they're in a migration directory, rename them to remove the migration-
  3) Split the 'struct QEMUFile' out into a private header
 Because the QEMU Buffered file code wants to access fields, and it's about
 to be in a separate file from QEMUFile.
  4) Split the QEMU buffered file code out in the same way as the stdio and
 unix/socket code has been split out.

Dave

v2:
  Also move block-migration.c
  Shorten file names now they're all in the migration/ directory

Dr. David Alan Gilbert (4):
  Start migrating migration code into a migration directory
  Remove migration- pre/post fixes off files in migration/ dir
  Split struct QEMUFile out
  Split the QEMU buffered file code out

 Makefile.objs|  10 +-
 migration/Makefile.objs  |  10 +
 block-migration.c = migration/block.c   |   0
 migration-exec.c = migration/exec.c |   0
 migration-fd.c = migration/fd.c |   0
 migration.c = migration/migration.c |   0
 qemu-file.c = migration/qemu-file-buf.c | 511 +-
 migration/qemu-file-internal.h   |  53 +++
 qemu-file-stdio.c = migration/qemu-file-stdio.c |   0
 qemu-file-unix.c = migration/qemu-file-unix.c   |   0
 migration/qemu-file.c| 519 +++
 migration-rdma.c = migration/rdma.c |   0
 migration-tcp.c = migration/tcp.c   |   0
 migration-unix.c = migration/unix.c |   0
 vmstate.c = migration/vmstate.c |   0
 xbzrle.c = migration/xbzrle.c   |   0
 tests/Makefile   |   7 +-
 17 files changed, 589 insertions(+), 521 deletions(-)
 create mode 100644 migration/Makefile.objs
 rename block-migration.c = migration/block.c (100%)
 rename migration-exec.c = migration/exec.c (100%)
 rename migration-fd.c = migration/fd.c (100%)
 rename migration.c = migration/migration.c (100%)
 rename qemu-file.c = migration/qemu-file-buf.c (51%)
 create mode 100644 migration/qemu-file-internal.h
 rename qemu-file-stdio.c = migration/qemu-file-stdio.c (100%)
 rename qemu-file-unix.c = migration/qemu-file-unix.c (100%)
 create mode 100644 migration/qemu-file.c
 rename migration-rdma.c = migration/rdma.c (100%)
 rename migration-tcp.c = migration/tcp.c (100%)
 rename migration-unix.c = migration/unix.c (100%)
 rename vmstate.c = migration/vmstate.c (100%)
 rename xbzrle.c = migration/xbzrle.c (100%)

-- 
2.1.0




[Qemu-devel] [PATCH v2 3/4] Split struct QEMUFile out

2014-12-09 Thread Dr. David Alan Gilbert (git)
From: Dr. David Alan Gilbert dgilb...@redhat.com

Now we've got multiple QEMUFile source files, some of them need
access to things that were defined in qemu-file.c, so create
a -internal header for them.

Signed-off-by: Dr. David Alan Gilbert dgilb...@redhat.com
---
 migration/qemu-file-internal.h | 53 ++
 migration/qemu-file.c  | 23 +-
 2 files changed, 54 insertions(+), 22 deletions(-)
 create mode 100644 migration/qemu-file-internal.h

diff --git a/migration/qemu-file-internal.h b/migration/qemu-file-internal.h
new file mode 100644
index 000..d95e853
--- /dev/null
+++ b/migration/qemu-file-internal.h
@@ -0,0 +1,53 @@
+/*
+ * QEMU System Emulator
+ *
+ * Copyright (c) 2003-2008 Fabrice Bellard
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the Software), to 
deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED AS IS, WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING 
FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ */
+
+#ifndef QEMU_FILE_INTERNAL_H
+#define QEMU_FILE_INTERNAL_H 1
+
+#include qemu-common.h
+#include qemu/iov.h
+
+#define IO_BUF_SIZE 32768
+#define MAX_IOV_SIZE MIN(IOV_MAX, 64)
+
+struct QEMUFile {
+const QEMUFileOps *ops;
+void *opaque;
+
+int64_t bytes_xfer;
+int64_t xfer_limit;
+
+int64_t pos; /* start of buffer when writing, end of buffer
+when reading */
+int buf_index;
+int buf_size; /* 0 when writing */
+uint8_t buf[IO_BUF_SIZE];
+
+struct iovec iov[MAX_IOV_SIZE];
+unsigned int iovcnt;
+
+int last_error;
+};
+
+#endif
diff --git a/migration/qemu-file.c b/migration/qemu-file.c
index f938e36..671fba9 100644
--- a/migration/qemu-file.c
+++ b/migration/qemu-file.c
@@ -27,30 +27,9 @@
 #include block/coroutine.h
 #include migration/migration.h
 #include migration/qemu-file.h
+#include migration/qemu-file-internal.h
 #include trace.h
 
-#define IO_BUF_SIZE 32768
-#define MAX_IOV_SIZE MIN(IOV_MAX, 64)
-
-struct QEMUFile {
-const QEMUFileOps *ops;
-void *opaque;
-
-int64_t bytes_xfer;
-int64_t xfer_limit;
-
-int64_t pos; /* start of buffer when writing, end of buffer
-when reading */
-int buf_index;
-int buf_size; /* 0 when writing */
-uint8_t buf[IO_BUF_SIZE];
-
-struct iovec iov[MAX_IOV_SIZE];
-unsigned int iovcnt;
-
-int last_error;
-};
-
 bool qemu_file_mode_is_not_valid(const char *mode)
 {
 if (mode == NULL ||
-- 
2.1.0




[Qemu-devel] [PATCH v2 4/4] Split the QEMU buffered file code out

2014-12-09 Thread Dr. David Alan Gilbert (git)
From: Dr. David Alan Gilbert dgilb...@redhat.com

The splitting of qemu-file and addition of the buffered file landed
at the same time; so now split the buffered file code out.

Signed-off-by: Dr. David Alan Gilbert dgilb...@redhat.com
---
 migration/Makefile.objs   |   2 +-
 migration/qemu-file-buf.c | 486 ++
 migration/qemu-file.c | 455 ---
 tests/Makefile|   3 +-
 4 files changed, 489 insertions(+), 457 deletions(-)
 create mode 100644 migration/qemu-file-buf.c

diff --git a/migration/Makefile.objs b/migration/Makefile.objs
index ce1e3c7..d929e96 100644
--- a/migration/Makefile.objs
+++ b/migration/Makefile.objs
@@ -1,6 +1,6 @@
 common-obj-y += migration.o tcp.o
 common-obj-y += vmstate.o
-common-obj-y += qemu-file.o qemu-file-unix.o qemu-file-stdio.o
+common-obj-y += qemu-file.o qemu-file-buf.o qemu-file-unix.o qemu-file-stdio.o
 common-obj-y += xbzrle.o
 
 common-obj-$(CONFIG_RDMA) += rdma.o
diff --git a/migration/qemu-file-buf.c b/migration/qemu-file-buf.c
new file mode 100644
index 000..d33dd44
--- /dev/null
+++ b/migration/qemu-file-buf.c
@@ -0,0 +1,486 @@
+/*
+ * QEMU System Emulator
+ *
+ * Copyright (c) 2003-2008 Fabrice Bellard
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the Software), to 
deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED AS IS, WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING 
FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ */
+#include qemu-common.h
+#include qemu/iov.h
+#include qemu/sockets.h
+#include block/coroutine.h
+#include migration/migration.h
+#include migration/qemu-file.h
+#include migration/qemu-file-internal.h
+#include trace.h
+
+#define QSB_CHUNK_SIZE  (1  10)
+#define QSB_MAX_CHUNK_SIZE  (16 * QSB_CHUNK_SIZE)
+
+/**
+ * Create a QEMUSizedBuffer
+ * This type of buffer uses scatter-gather lists internally and
+ * can grow to any size. Any data array in the scatter-gather list
+ * can hold different amount of bytes.
+ *
+ * @buffer: Optional buffer to copy into the QSB
+ * @len: size of initial buffer; if @buffer is given, buffer must
+ *   hold at least len bytes
+ *
+ * Returns a pointer to a QEMUSizedBuffer or NULL on allocation failure
+ */
+QEMUSizedBuffer *qsb_create(const uint8_t *buffer, size_t len)
+{
+QEMUSizedBuffer *qsb;
+size_t alloc_len, num_chunks, i, to_copy;
+size_t chunk_size = (len  QSB_MAX_CHUNK_SIZE)
+? QSB_MAX_CHUNK_SIZE
+: QSB_CHUNK_SIZE;
+
+num_chunks = DIV_ROUND_UP(len ? len : QSB_CHUNK_SIZE, chunk_size);
+alloc_len = num_chunks * chunk_size;
+
+qsb = g_try_new0(QEMUSizedBuffer, 1);
+if (!qsb) {
+return NULL;
+}
+
+qsb-iov = g_try_new0(struct iovec, num_chunks);
+if (!qsb-iov) {
+g_free(qsb);
+return NULL;
+}
+
+qsb-n_iov = num_chunks;
+
+for (i = 0; i  num_chunks; i++) {
+qsb-iov[i].iov_base = g_try_malloc0(chunk_size);
+if (!qsb-iov[i].iov_base) {
+/* qsb_free is safe since g_free can cope with NULL */
+qsb_free(qsb);
+return NULL;
+}
+
+qsb-iov[i].iov_len = chunk_size;
+if (buffer) {
+to_copy = (len - qsb-used)  chunk_size
+  ? chunk_size : (len - qsb-used);
+memcpy(qsb-iov[i].iov_base, buffer[qsb-used], to_copy);
+qsb-used += to_copy;
+}
+}
+
+qsb-size = alloc_len;
+
+return qsb;
+}
+
+/**
+ * Free the QEMUSizedBuffer
+ *
+ * @qsb: The QEMUSizedBuffer to free
+ */
+void qsb_free(QEMUSizedBuffer *qsb)
+{
+size_t i;
+
+if (!qsb) {
+return;
+}
+
+for (i = 0; i  qsb-n_iov; i++) {
+g_free(qsb-iov[i].iov_base);
+}
+g_free(qsb-iov);
+g_free(qsb);
+}
+
+/**
+ * Get the number of used bytes in the QEMUSizedBuffer
+ *
+ * @qsb: A QEMUSizedBuffer
+ *
+ * Returns the number of bytes currently used in this buffer
+ */
+size_t qsb_get_length(const QEMUSizedBuffer *qsb)
+{
+return qsb-used;
+}
+
+/**
+ * Set the length of the buffer; the primary usage of this

Re: [Qemu-devel] [PATCH 0/7] MIPS: IEEE 754-2008 features support

2014-12-09 Thread Peter Maydell
On 9 December 2014 at 12:28, Maciej W. Rozycki ma...@codesourcery.com wrote:
 On Tue, 9 Dec 2014, Peter Maydell wrote:
 I really really want to hold this patchset off until the softfloat
 relicensing has gone through, because repeating that work would
 be really painful...

  Understood.  Do you have an ETA for this to happen?

It's waiting for people to review the relicensing patchset
(which includes a couple of reimplementations of functions,
so you might want to review and test if you have any handy
test setups.)

thanks
-- PMM



Re: [Qemu-devel] [PATCH 2/9] pc: acpi: decribe bridge device as not hotpluggable

2014-12-09 Thread Michael S. Tsirkin
On Tue, Dec 09, 2014 at 12:45:27PM +0100, Igor Mammedov wrote:
 On Tue, 9 Dec 2014 12:34:02 +0200
 Michael S. Tsirkin m...@redhat.com wrote:
 
  On Tue, Dec 09, 2014 at 11:27:16AM +0100, Igor Mammedov wrote:
   On Mon, 8 Dec 2014 21:13:25 +0200
   Michael S. Tsirkin m...@redhat.com wrote:
   
On Mon, Dec 08, 2014 at 04:08:01PM +, Igor Mammedov wrote:
 when bridge hotplug is disabled, i.e. for machine
 types less then 2.0, bridge device was created as
 hotpluggable by mistake since commit 133a2da.
 
 Fix it by just creating it as a present device.
 
 Signed-off-by: Igor Mammedov imamm...@redhat.com

What exactly is the problem here?
It seems that such bridge is hotpluggable, even though
e.g. windows guests lacks drivers to support this.
   before 133a2da slot with existing at startup bridge weren't
   marked as hotpluggable nor described in SSDT. But after
   133a2da it's described as hotpluggable slot for compat
   machines (2.0 and lower) which doesn't match with original
   behavior.
   
   Also Marcel mentioned that bridges could be hotpluggable
   but that they should not be hot-UNpluggable.
  
  OK so is there some guest that's confused?
  What's the bug?
 It allows to do eject on bridge when it shouldn't.

I'm not sure it shouldn't. Assuming guest supports SHPC, eject on such a
bridge might well work correctly.


 I don't know about consequences of it,
 it's better not to allow invalid action in the first place.
 
 
  


 ---
  hw/i386/acpi-build.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)
 
 diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
 index b37a397..1fb92e5 100644
 --- a/hw/i386/acpi-build.c
 +++ b/hw/i386/acpi-build.c
 @@ -913,7 +913,7 @@ static void build_pci_bus_end(PCIBus *bus, void 
 *bus_state)
  }
  }
  
 -if (!dc-hotpluggable || bridge_in_acpi) {
 +if (!dc-hotpluggable || pc-is_bridge) {
  clear_bit(slot, slot_hotplug_enable);
  }
  }
 -- 
 1.8.3.1



Re: [Qemu-devel] [PATCH 4/9] acpi: build_append_nameseg(): add padding if necessary

2014-12-09 Thread Igor Mammedov
On Tue, 9 Dec 2014 12:38:12 +0200
Michael S. Tsirkin m...@redhat.com wrote:

 On Tue, Dec 09, 2014 at 11:32:45AM +0100, Igor Mammedov wrote:
  On Mon, 8 Dec 2014 23:15:32 +0200
  Michael S. Tsirkin m...@redhat.com wrote:
  
   On Mon, Dec 08, 2014 at 04:08:03PM +, Igor Mammedov wrote:
According to ACPI spec NameSeg shorter than 4 characters
must be padded up to 4 characters with _ symbol.
ACPI 5.0:  20.2.2 Name Objects Encoding

Do it in build_append_nameseg() so that caller shouldn't know
or care about it.

Signed-off-by: Igor Mammedov imamm...@redhat.com
   
   To me just doing it right in callers seems just as easy, but
   I guess you disagree :)
  That means, author MUST know about padding, if he/she doesn't
  or forget about it that would introduce error usually resulting
  in BSOD.
 
 Not really. assert will trigger on qemu start.
it sure will, but a bit confusing according to ASL spec name is up to
4 charactes and only AML spec specifies that there should be padding
if necessary.
I'm trying to get away from constructing AML by hands and the next step
for API would be ASL like one which is my eventual target in the end.

 
  This patch allows to avoid such mistakes.
 
 Even the most basic testing (e.g. make check) will find these mistakes.
 It's more a question of which API we prefer.
 
 Anyway, could you respond on g_assert vs assert please?
Ah, I'm sorry about that, haven't noticed comments below.

 
   
---
 hw/i386/acpi-build.c | 18 +-
 1 file changed, 13 insertions(+), 5 deletions(-)

diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
index f5ec66a..a8b7a2b 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -292,6 +292,8 @@ static inline void build_append_array(GArray 
*array, GArray *val)
 g_array_append_vals(array, val-data, val-len);
 }
 
+#define ACPI_NAMESEG_LEN 4
+
 static void GCC_FMT_ATTR(2, 3)
 build_append_nameseg(GArray *array, const char *format, ...)
 {
@@ -299,13 +301,19 @@ build_append_nameseg(GArray *array, const char 
*format, ...)
 char s[] = ;
 int len;
 va_list args;
+const char padding = '_';
 
 va_start(args, format);
 len = vsnprintf(s, sizeof s, format, args);
 va_end(args);
 
-assert(len == 4);
+g_assert(len = ACPI_NAMESEG_LEN);
   
   I'm not sure when is g_assert preferable to assert.
   What's the motivation here?
None, I'm not sure what policy on it either
I can keep assert here if preferable.

   
   
+
 g_array_append_vals(array, s, len);
+while (len != ACPI_NAMESEG_LEN) {
+g_array_append_val(array, padding);
+++len;
+}
   
   Easier
   
 /* Pad up to 4 characters if necessary. */
 g_array_append_vals(array, , 4 - len);
Thanks, I'll fix it up.

   
   
   
 }
 
 /* 5.4 Definition Block Encoding */
@@ -846,7 +854,7 @@ static void build_pci_bus_end(PCIBus *bus, void 
*bus_state)
 
 if (bus-parent_dev) {
 op = 0x82; /* DeviceOp */
-build_append_nameseg(bus_table, S%.02X_,
+build_append_nameseg(bus_table, S%.02X,
  bus-parent_dev-devfn);
 build_append_byte(bus_table, 0x08); /* NameOp */
 build_append_nameseg(bus_table, _SUN);
@@ -966,7 +974,7 @@ static void build_pci_bus_end(PCIBus *bus, void 
*bus_state)
 build_append_int(notify, 0x1U  i);
 build_append_byte(notify, 0x00); /* NullName */
 build_append_byte(notify, 0x86); /* NotifyOp */
-build_append_nameseg(notify, S%.02X_, PCI_DEVFN(i, 0));
+build_append_nameseg(notify, S%.02X, PCI_DEVFN(i, 0));
 build_append_byte(notify, 0x69); /* Arg1Op */
 
 /* Pack it up */
@@ -1023,7 +1031,7 @@ static void build_pci_bus_end(PCIBus *bus, void 
*bus_state)
 if (bus-parent_dev) {
 build_append_byte(parent-notify_table, '^'); /* 
ParentPrefixChar */
 build_append_byte(parent-notify_table, 0x2E); /* 
DualNamePrefix */
-build_append_nameseg(parent-notify_table, S%.02X_,
+build_append_nameseg(parent-notify_table, S%.02X,
  bus-parent_dev-devfn);
 build_append_nameseg(parent-notify_table, PCNT);
 }
@@ -1093,7 +1101,7 @@ build_ssdt(GArray *table_data, GArray *linker,
 GArray *sb_scope = build_alloc_array();
 uint8_t op = 0x10; /* ScopeOp */
 
-build_append_nameseg(sb_scope, _SB_);
+build_append_nameseg(sb_scope, _SB);
 
 /* build Processor object for each processor */
 for (i = 0; i  acpi_cpus; i++) {
-- 
1.8.3.1




Re: [Qemu-devel] Redundant VDE network

2014-12-09 Thread Dmitry Antipov

On 12/09/2014 12:37 PM, Stefan Hajnoczi wrote:


Is there a reason why you are using the old 32-bit SPARC machine types
instead of the more modern 64-bit SPARC machine types?


Unfortunately yes, this is the closest match to the real (legacy)
hardware I need to use :-(.


I guess the SS-5 default machine type you are running doesn't have a PCI bus?


Yes.


By the way, I have not tested the command-line because Fedora does not
have a vde package.


FYI, there is one I build myself: 
http://37.139.80.10/tmp/vde-2.3.2-0.587.fc20.src.rpm

Dmitry




Re: [Qemu-devel] [PATCH 6/9] acpi: add build_append_namestring() helper

2014-12-09 Thread Igor Mammedov
On Tue, 9 Dec 2014 14:02:17 +0200
Michael S. Tsirkin m...@redhat.com wrote:

 On Tue, Dec 09, 2014 at 11:39:39AM +0100, Igor Mammedov wrote:
  On Mon, 8 Dec 2014 22:21:20 +0200
  Michael S. Tsirkin m...@redhat.com wrote:
  
   On Mon, Dec 08, 2014 at 04:08:05PM +, Igor Mammedov wrote:
Use build_append_namestring() instead of build_append_nameseg()
So user won't have to care whether name is NameSeg, NamePath or
NameString.

See for refernce ACPI 5.0: 20.2.2 Name Objects Encoding

Signed-off-by: Igor Mammedov imamm...@redhat.com
   
   Well that's wasteful since most places do have simple 4 byte names.
  What do you mean under wasteful?
 
 I think I misunderstood what this patch does:
 you mean select the correct name type based on
 the string used?
Yep, it chooses appropriate type depending on input.

 
 I'll have to re-read it, sorry about the noise.
 
   How about we replace where this is needed?
   
   
---
 hw/acpi/acpi_gen_utils.c | 86 
+++-
 hw/i386/acpi-build.c | 35 
 include/hw/acpi/acpi_gen_utils.h |  2 +-
 3 files changed, 102 insertions(+), 21 deletions(-)

diff --git a/hw/acpi/acpi_gen_utils.c b/hw/acpi/acpi_gen_utils.c
index 1583b35..31e2694 100644
--- a/hw/acpi/acpi_gen_utils.c
+++ b/hw/acpi/acpi_gen_utils.c
@@ -31,7 +31,7 @@ void build_append_array(GArray *array, GArray *val)
 
 #define ACPI_NAMESEG_LEN 4
 
-void GCC_FMT_ATTR(2, 3)
+static void GCC_FMT_ATTR(2, 3)
 build_append_nameseg(GArray *array, const char *format, ...)
 {
 /* It would be nicer to use g_string_vprintf but it's only there 
in 2.22 */
@@ -53,6 +53,90 @@ build_append_nameseg(GArray *array, const char 
*format, ...)
 }
 }
 
+static const uint8_t ACPI_DualNamePrefix  = 0x2E;
+static const uint8_t ACPI_MultiNamePrefix = 0x2F;
+
+static void
+build_append_namestringv(GArray *array, const char *format, va_list ap)
+{
+/* It would be nicer to use g_string_vprintf but it's only there 
in 2.22 */
+char *s;
+int len;
+va_list va_len;
+char **segs;
+char **segs_iter;
+int seg_count = 0;
+
+va_copy(va_len, ap);
+len = vsnprintf(NULL, 0, format, va_len);
+va_end(va_len);
+len += 1;
+s = g_new(typeof(*s), len);
+
+len = vsnprintf(s, len, format, ap);
+
+segs = g_strsplit(s, ., 0);
+g_free(s);
+
+/* count segments */
+segs_iter = segs;
+while (*segs_iter) {
+++segs_iter;
+++seg_count;
+}
+
+/* handle RootPath || PrefixPath */
+s = *segs;
+while (*s == '\\' || *s == '^') {
+g_array_append_val(array, *s);
+++s;
+}
+
+switch (seg_count) {
+case 1:
+if (!*s) { /* NullName */
+const uint8_t nullpath = 0;
+g_array_append_val(array, nullpath);
+} else {
+build_append_nameseg(array, %s, s);
+}
+break;
+
+case 2:
+g_array_append_val(array, ACPI_DualNamePrefix);
+build_append_nameseg(array, %s, s);
+build_append_nameseg(array, %s, segs[1]);
+break;
+
+default:
+g_array_append_val(array, ACPI_MultiNamePrefix);
+assert(seg_count = 0xFF); /* ByteData Max */
+g_array_append_val(array, seg_count);
+
+/* handle the 1st segment manually due to prefix/root path */
+build_append_nameseg(array, %s, s);
+
+/* add the rest of segments */
+segs_iter = segs + 1;
+while (*segs_iter) {
+build_append_nameseg(array, %s, *segs_iter);
+++segs_iter;
+}
+break;
+}
+g_strfreev(segs);
+}
+
+void GCC_FMT_ATTR(2, 3)
+build_append_namestring(GArray *array, const char *format, ...)
+{
+va_list ap;
+
+va_start(ap, format);
+build_append_namestringv(array, format, ap);
+va_end(ap);
+}
+
 /* 5.4 Definition Block Encoding */
 enum {
 PACKAGE_LENGTH_1BYTE_SHIFT = 6, /* Up to 63 - use extra 2 bits. */
diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
index 870e4a0..0f6202d 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -273,7 +273,7 @@ static GArray *build_alloc_method(const char *name, 
uint8_t arg_count)
 {
 GArray *method = build_alloc_array();
 
-build_append_nameseg(method, %s, name);
+build_append_namestring(method, %s, name);
 build_append_byte(method, arg_count); /* MethodFlags: ArgCount */
 
 return method;
@@ -565,7 +565,7 @@ 

Re: [Qemu-devel] [PATCH 2/9] pc: acpi: decribe bridge device as not hotpluggable

2014-12-09 Thread Michael S. Tsirkin
On Tue, Dec 09, 2014 at 01:57:51PM +0100, Igor Mammedov wrote:
 On Tue, 9 Dec 2014 14:51:59 +0200
 Michael S. Tsirkin m...@redhat.com wrote:
 
  On Tue, Dec 09, 2014 at 12:45:27PM +0100, Igor Mammedov wrote:
   On Tue, 9 Dec 2014 12:34:02 +0200
   Michael S. Tsirkin m...@redhat.com wrote:
   
On Tue, Dec 09, 2014 at 11:27:16AM +0100, Igor Mammedov wrote:
 On Mon, 8 Dec 2014 21:13:25 +0200
 Michael S. Tsirkin m...@redhat.com wrote:
 
  On Mon, Dec 08, 2014 at 04:08:01PM +, Igor Mammedov wrote:
   when bridge hotplug is disabled, i.e. for machine
   types less then 2.0, bridge device was created as
   hotpluggable by mistake since commit 133a2da.
   
   Fix it by just creating it as a present device.
   
   Signed-off-by: Igor Mammedov imamm...@redhat.com
  
  What exactly is the problem here?
  It seems that such bridge is hotpluggable, even though
  e.g. windows guests lacks drivers to support this.
 before 133a2da slot with existing at startup bridge weren't
 marked as hotpluggable nor described in SSDT. But after
 133a2da it's described as hotpluggable slot for compat
 machines (2.0 and lower) which doesn't match with original
 behavior.
 
 Also Marcel mentioned that bridges could be hotpluggable
 but that they should not be hot-UNpluggable.

OK so is there some guest that's confused?
What's the bug?
   It allows to do eject on bridge when it shouldn't.
  
  I'm not sure it shouldn't. Assuming guest supports SHPC, eject on such a
  bridge might well work correctly.
 If I remember correctly SHPC hotplug doesn't need ASL parts at all.

Right. And then presumably bridge itself can be removed.

  
  
   I don't know about consequences of it,
   it's better not to allow invalid action in the first place.
   
   

  
  
   ---
hw/i386/acpi-build.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
   
   diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
   index b37a397..1fb92e5 100644
   --- a/hw/i386/acpi-build.c
   +++ b/hw/i386/acpi-build.c
   @@ -913,7 +913,7 @@ static void build_pci_bus_end(PCIBus *bus, 
   void *bus_state)
}
}

   -if (!dc-hotpluggable || bridge_in_acpi) {
   +if (!dc-hotpluggable || pc-is_bridge) {
clear_bit(slot, slot_hotplug_enable);
}
}
   -- 
   1.8.3.1



Re: [Qemu-devel] KVM call for agenda for 2014-12-08

2014-12-09 Thread Alexander Spyridakis
Hello,

Is this call still going to happen today? I haven't yet received the
contact details.

Thanks and best regards.



Re: [Qemu-devel] Cubietruck: cannot create KVM guests: kvm_init_vcpu failed: Invalid argument

2014-12-09 Thread Dr. David Alan Gilbert
* Peter Maydell (peter.mayd...@linaro.org) wrote:
 On 9 December 2014 at 11:25, Richard W.M. Jones rjo...@redhat.com wrote:
  I really think that qemu should just do the right thing though.
 
 Tricky, because you have to use the same CPU as the host if
 you want to use KVM

That's an unusally strict requirement; is there no concept
of a common subset that will work on multiple CPUs?

Is that still true for v8? I can see it making it tricky for users
with piles of newer/older hosts.

Dave

 and that conflicts with wanting to do the
 same thing on all host platforms for a given set of command line
 options. If you have some concrete suggestions for improving
 things that don't break backwards-compatibility I'm happy to
 hear them, though.
 
 -- PMM
 
--
Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK



Re: [Qemu-devel] [PATCH 1/2] s390/pci: add error event support

2014-12-09 Thread Cornelia Huck
On Fri,  5 Dec 2014 10:19:58 +0100
Frank Blaschka blasc...@linux.vnet.ibm.com wrote:

 From: Frank Blaschka frank.blasc...@de.ibm.com
 
 This patch adds support to generate s390 pci error events

It also uses the new infrastructure for iota error handling (which
confused me when I read this), so I modified the patch description to
read:

This patch adds support to generate s390 pci error events and makes
use of it to support iota error handling.

 
 Signed-off-by: Frank Blaschka frank.blasc...@de.ibm.com
 ---
  hw/s390x/s390-pci-bus.c  | 50 +---
  hw/s390x/s390-pci-bus.h  | 35 +++-
  hw/s390x/s390-pci-inst.c | 86 
 ++--
  3 files changed, 155 insertions(+), 16 deletions(-)




Re: [Qemu-devel] [PATCH 0/2] s390/pci: add 2 more features

2014-12-09 Thread Cornelia Huck
On Fri,  5 Dec 2014 10:19:57 +0100
Frank Blaschka blasc...@linux.vnet.ibm.com wrote:

 Coni, Alex, Christian,
 
 here are 2 more s390/pci features on top of the base pci support.
 Thx!
 
 Frank
 
 Frank Blaschka (2):
   s390/pci: add error event support
   s390/pci: implement stpcifc instruction
 
  hw/s390x/s390-pci-bus.c  |  50 ++--
  hw/s390x/s390-pci-bus.h  |  36 +++-
  hw/s390x/s390-pci-inst.c | 148 
 ---
  hw/s390x/s390-pci-inst.h |   1 +
  target-s390x/kvm.c   |   9 ++-
  5 files changed, 227 insertions(+), 17 deletions(-)
 

Applied 1-2 (patch 1 with slightly modified description) to s390-next.




Re: [Qemu-devel] [PATCH] s390/pci: remove unnecessary cpu_synchronize_state

2014-12-09 Thread Cornelia Huck
On Mon,  8 Dec 2014 09:18:59 +0100
Frank Blaschka blasc...@linux.vnet.ibm.com wrote:

 Remove all unnecessary calls to cpu_synchronize_state
 
 Signed-off-by: Frank Blaschka blasc...@linux.vnet.ibm.com
 ---
  hw/s390x/s390-pci-inst.c | 6 --
  1 file changed, 6 deletions(-)

Applied to s390-next.




Re: [Qemu-devel] KVM call for agenda for 2014-12-08

2014-12-09 Thread Juan Quintela
Alexander Spyridakis a.spyrida...@virtualopensystems.com wrote:
 Hello,

 Is this call still going to happen today? I haven't yet received the
 contact details.

Just sent it, it would be in 20 mins.

Later, Juan.



[Qemu-devel] KVM call for 2014-12-09

2014-12-09 Thread Juan Quintela

Hi

Just a gentle reminder that Today we have a call to continue talking
about multithread.

Thanks, Juan.



Re: [Qemu-devel] [PATCH RFC v5 14/19] s390x/virtio-ccw: enable virtio 1.0

2014-12-09 Thread Michael S. Tsirkin
On Tue, Dec 02, 2014 at 02:00:22PM +0100, Cornelia Huck wrote:
 virtio-ccw should now have everything in place to operate virtio 1.0
 devices, so let's enable revision 1.
 
 Signed-off-by: Cornelia Huck cornelia.h...@de.ibm.com

Looks like this will allow revision 1 for all devices,
we only want this for virtio 1 devices.

The following should fix it I think:

Signed-off-by: Michael S. Tsirkin m...@redhat.com


diff --git a/hw/s390x/virtio-ccw.h b/hw/s390x/virtio-ccw.h
index d40e3be..f5a1d3e 100644
--- a/hw/s390x/virtio-ccw.h
+++ b/hw/s390x/virtio-ccw.h
@@ -69,9 +69,6 @@ typedef struct VirtIOCCWDeviceClass {
 int (*exit)(VirtioCcwDevice *dev);
 } VirtIOCCWDeviceClass;
 
-/* The maximum virtio revision we support. */
-#define VIRTIO_CCW_REV_MAX 1
-
 /* Performance improves when virtqueue kick processing is decoupled from the
  * vcpu thread using ioeventfd for some devices. */
 #define VIRTIO_CCW_FLAG_USE_IOEVENTFD_BIT 1
@@ -104,6 +101,12 @@ struct VirtioCcwDevice {
 uint64_t ind_bit;
 };
 
+/* The maximum virtio revision we support. */
+static int virtio_ccw_rev_max(VirtioCcwDevice *dev)
+{
+return dev-host_features  (1ULL  VIRTIO_F_VERSION_1) ? 1 : 0;
+}
+
 /* virtual css bus type */
 typedef struct VirtualCssBus {
 BusState parent_obj;
diff --git a/hw/s390x/virtio-ccw.c b/hw/s390x/virtio-ccw.c
index 3826074..922b021 100644
--- a/hw/s390x/virtio-ccw.c
+++ b/hw/s390x/virtio-ccw.c
@@ -693,7 +693,7 @@ static int virtio_ccw_cb(SubchDev *sch, CCW1 ccw)
 }
 cpu_physical_memory_read(ccw.cda, revinfo, len);
 if (dev-revision = 0 ||
-revinfo.revision  VIRTIO_CCW_REV_MAX) {
+revinfo.revision  virtio_ccw_rev_max(dev)) {
 ret = -ENOSYS;
 break;
 }



Re: [Qemu-devel] Cubietruck: cannot create KVM guests: kvm_init_vcpu failed: Invalid argument

2014-12-09 Thread Peter Maydell
On 9 December 2014 at 12:33, Dr. David Alan Gilbert dgilb...@redhat.com wrote:
 * Peter Maydell (peter.mayd...@linaro.org) wrote:
 On 9 December 2014 at 11:25, Richard W.M. Jones rjo...@redhat.com wrote:
  I really think that qemu should just do the right thing though.

 Tricky, because you have to use the same CPU as the host if
 you want to use KVM

 That's an unusally strict requirement; is there no concept
 of a common subset that will work on multiple CPUs?

 Is that still true for v8? I can see it making it tricky for users
 with piles of newer/older hosts.

It's a limitation of the kernel implementation -- we don't
attempt to trap things like ID registers, so what the guest
sees is just the host CPU. The architecture provides the means
to trap these and the KVM API allows userspace to ask the
kernel to provide an emulation specifically of a particular
CPU. But the kernel will currently always refuse to emulate
anything other than the host CPU type it's running on.

thanks
-- PMM



Re: [Qemu-devel] [PATCH 9/9] pc: acpi-build: replace recursive PCI bus tree generation with loop based

2014-12-09 Thread Igor Mammedov
On Mon, 8 Dec 2014 22:43:24 +0200
Michael S. Tsirkin m...@redhat.com wrote:

 On Mon, Dec 08, 2014 at 04:08:08PM +, Igor Mammedov wrote:
  it replaces PCI tree structure in SSDT with a set of scopes
  describing each PCI bus as a separate scope with a child devices.
  It makes code easier to follow and a little bit smaller.
  
  In addition it makes simplier to convert current template
  patching approach to completely dynamically generated SSDT
  without dependency on IASL, which would allow to drop
  template binary blobs from source tree.
  
  Signed-off-by: Igor Mammedov imamm...@redhat.com
  ---
   hw/i386/acpi-build.c | 362 
  +++
   1 file changed, 165 insertions(+), 197 deletions(-)
 
 I like it that your patch makes code smaller and simpler,
 but I think we do need to generate hierarchical AML.
If only reason for hierarchical AML is table size then
I don't see how scopes could possibly double the table size,
Compared to tree version scope adds only:

 ~5 bytes for package + NamePath(~[0-2] + 4 * tree depth)

Which is almost nothing compared to slots description
per a bridge.
One needs to configure insanely deep bridge nesting
for scope cost to significantly affect table size, which
doesn't sound as practical thing to do anyway except of
testing limitation on how many nested bridges QEMU would
stomach.

 
 I think this can still be done with some modifications to
 the logic you have here.
 
 Basically current code does all work in build_pci_bus_end.
 It follows that you can do the same by simply walking
 the list in reverse order.
I've thought about it already, but tree creation means
creating temporary contexts for each scope, copying them
to parent context, keeping track of contexts to build
correct notifiers. It's basically the same or even worse
as current code just other way around.

And all that makes code quite hard to follow and
maintain, that is the main reason to drop recursion in
favor of simple flat scope sets. And it makes easier to
switch from AML+template patching to ASL API later
and hide from user direct access to table array, forcing
him to work only with ASL constructs when building table.

 
 
 
 
 
  diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
  index 97ff245..7606a73 100644
  --- a/hw/i386/acpi-build.c
  +++ b/hw/i386/acpi-build.c
  @@ -641,196 +641,186 @@ static void acpi_set_pci_info(void)
   }
   }
   
  -static void build_pci_bus_state_init(AcpiBuildPciBusHotplugState *state,
  - AcpiBuildPciBusHotplugState *parent,
  - bool pcihp_bridge_en)
  +static char *pci_bus_get_scope_name(PCIBus *bus)
   {
  -state-parent = parent;
  -state-device_table = build_alloc_array();
  -state-notify_table = build_alloc_array();
  -state-pcihp_bridge_en = pcihp_bridge_en;
  -}
  +char *name = NULL;
  +char *last = name;
   
  -static void build_pci_bus_state_cleanup(AcpiBuildPciBusHotplugState *state)
  -{
  -build_free_array(state-device_table);
  -build_free_array(state-notify_table);
  -}
  +while (bus-parent_dev) {
  +last = name;
  +name = g_strdup_printf(%s.S%.02X_,
  +   name ? name : , bus-parent_dev-devfn);
  +g_free(last);
   
  -static void *build_pci_bus_begin(PCIBus *bus, void *parent_state)
  -{
  -AcpiBuildPciBusHotplugState *parent = parent_state;
  -AcpiBuildPciBusHotplugState *child = g_malloc(sizeof *child);
  +bus = bus-parent_dev-bus;
  +}
   
  -build_pci_bus_state_init(child, parent, parent-pcihp_bridge_en);
  +last = name;
  +name = g_strdup_printf(PCI0%s, name ? name : );
  +g_free(last);
   
  -return child;
  +return name;
   }
   
  -static void build_pci_bus_end(PCIBus *bus, void *bus_state)
  +static GQueue *get_pci_bus_list(PCIBus *bus, bool pcihp_bridge_en)
   {
  -AcpiBuildPciBusHotplugState *child = bus_state;
  -AcpiBuildPciBusHotplugState *parent = child-parent;
  -GArray *bus_table = build_alloc_array();
  -DECLARE_BITMAP(slot_hotplug_enable, PCI_SLOT_MAX);
  -DECLARE_BITMAP(slot_device_present, PCI_SLOT_MAX);
  -DECLARE_BITMAP(slot_device_system, PCI_SLOT_MAX);
  -DECLARE_BITMAP(slot_device_vga, PCI_SLOT_MAX);
  -DECLARE_BITMAP(slot_device_qxl, PCI_SLOT_MAX);
  -uint8_t op;
  -int i;
  -QObject *bsel;
  -GArray *method;
  -bool bus_hotplug_support = false;
  -
  -/*
  - * Skip bridge subtree creation if bridge hotplug is disabled
  - * to make acpi tables compatible with legacy machine types.
  - */
  -if (!child-pcihp_bridge_en  bus-parent_dev) {
  -return;
  -}
  -
  -if (bus-parent_dev) {
  -op = 0x82; /* DeviceOp */
  -build_append_namestring(bus_table, S%.02X,
  - bus-parent_dev-devfn);
  -build_append_byte(bus_table, 0x08); /* NameOp */
  -  

[Qemu-devel] [PATCH 0/1] Update email addresses for Chrysostomos Nanakos

2014-12-09 Thread Chrysostomos Nanakos
Remove first email address and let the one from which I am contributing.


Chrysostomos Nanakos (1):
  MAINTAINERS: Update email addresses for Chrysostomos Nanakos

 MAINTAINERS |1 -
 1 file changed, 1 deletion(-)

-- 
1.7.10.4




Re: [Qemu-devel] [PATCH v2] vt82c686: fix coverity warning about out-of-bounds write

2014-12-09 Thread Paolo Bonzini


On 09/12/2014 08:15, zhanghailiang wrote:
 Refactor superio_ioport_writeb to fix the out of bounds write warning.
 
 In addition, fix two typos: s/chage/change/
 
 Signed-off-by: zhanghailiang zhang.zhanghaili...@huawei.com
 ---
  - using bool instead of int for can_write as suggested by Stefan Weil
  - fix two typos: s/chage/change
 ---
  hw/isa/vt82c686.c | 41 +++--
  1 file changed, 19 insertions(+), 22 deletions(-)
 
 diff --git a/hw/isa/vt82c686.c b/hw/isa/vt82c686.c
 index e0c235c..6c00f22 100644
 --- a/hw/isa/vt82c686.c
 +++ b/hw/isa/vt82c686.c
 @@ -50,13 +50,13 @@ typedef struct VT82C686BState {
  static void superio_ioport_writeb(void *opaque, hwaddr addr, uint64_t data,
unsigned size)
  {
 -int can_write;
  SuperIOConfig *superio_conf = opaque;
  
  DPRINTF(superio_ioport_writeb  address 0x%x  val 0x%x\n, addr, data);
  if (addr == 0x3f0) {
  superio_conf-index = data  0xff;
  } else {
 +bool can_write = true;
  /* 0x3f1 */
  switch (superio_conf-index) {
  case 0x00 ... 0xdf:
 @@ -68,30 +68,27 @@ static void superio_ioport_writeb(void *opaque, hwaddr 
 addr, uint64_t data,
  case 0xf7:
  case 0xf9 ... 0xfb:
  case 0xfd ... 0xff:
 -can_write = 0;
 +can_write = false;
  break;
 -default:
 -can_write = 1;
 -
 -if (can_write) {
 -switch (superio_conf-index) {
 -case 0xe7:
 -if ((data  0xff) != 0xfe) {
 -DPRINTF(chage uart 1 base. unsupported yet\n);
 -}
 -break;
 -case 0xe8:
 -if ((data  0xff) != 0xbe) {
 -DPRINTF(chage uart 2 base. unsupported yet\n);
 -}
 -break;
 -
 -default:
 -superio_conf-config[superio_conf-index] = data  0xff;
 -}
 +case 0xe7:
 +if ((data  0xff) != 0xfe) {
 +DPRINTF(change uart 1 base. unsupported yet\n);
 +can_write = false;
 +}
 +break;
 +case 0xe8:
 +if ((data  0xff) != 0xbe) {
 +DPRINTF(change uart 2 base. unsupported yet\n);
 +can_write = false;
  }
 +break;
 +default:
 +break;
 +
 +}
 +if (can_write == true) {

Michael, can you remove == true when applying this patch?

Thanks,

Paolo

 +superio_conf-config[superio_conf-index] = data  0xff;
  }
 -superio_conf-config[superio_conf-index] = data  0xff;
  }
  }
  
 



[Qemu-devel] [PATCH 1/1] MAINTAINERS: Update email addresses for Chrysostomos Nanakos

2014-12-09 Thread Chrysostomos Nanakos
Remove first email address and let the one from which I am contributing.

Signed-off-by: Chrysostomos Nanakos ch...@include.gr
---
 MAINTAINERS |1 -
 1 file changed, 1 deletion(-)

diff --git a/MAINTAINERS b/MAINTAINERS
index bcb69e8..a33d8f7 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1089,7 +1089,6 @@ S: Supported
 F: block/ssh.c
 
 ARCHIPELAGO
-M: Chrysostomos Nanakos cnana...@grnet.gr
 M: Chrysostomos Nanakos ch...@include.gr
 S: Maintained
 F: block/archipelago.c
-- 
1.7.10.4




Re: [Qemu-devel] [PATCH 2/9] pc: acpi: decribe bridge device as not hotpluggable

2014-12-09 Thread Igor Mammedov
On Tue, 9 Dec 2014 14:51:59 +0200
Michael S. Tsirkin m...@redhat.com wrote:

 On Tue, Dec 09, 2014 at 12:45:27PM +0100, Igor Mammedov wrote:
  On Tue, 9 Dec 2014 12:34:02 +0200
  Michael S. Tsirkin m...@redhat.com wrote:
  
   On Tue, Dec 09, 2014 at 11:27:16AM +0100, Igor Mammedov wrote:
On Mon, 8 Dec 2014 21:13:25 +0200
Michael S. Tsirkin m...@redhat.com wrote:

 On Mon, Dec 08, 2014 at 04:08:01PM +, Igor Mammedov wrote:
  when bridge hotplug is disabled, i.e. for machine
  types less then 2.0, bridge device was created as
  hotpluggable by mistake since commit 133a2da.
  
  Fix it by just creating it as a present device.
  
  Signed-off-by: Igor Mammedov imamm...@redhat.com
 
 What exactly is the problem here?
 It seems that such bridge is hotpluggable, even though
 e.g. windows guests lacks drivers to support this.
before 133a2da slot with existing at startup bridge weren't
marked as hotpluggable nor described in SSDT. But after
133a2da it's described as hotpluggable slot for compat
machines (2.0 and lower) which doesn't match with original
behavior.

Also Marcel mentioned that bridges could be hotpluggable
but that they should not be hot-UNpluggable.
   
   OK so is there some guest that's confused?
   What's the bug?
  It allows to do eject on bridge when it shouldn't.
 
 I'm not sure it shouldn't. Assuming guest supports SHPC, eject on such a
 bridge might well work correctly.
If I remember correctly SHPC hotplug doesn't need ASL parts at all.

 
 
  I don't know about consequences of it,
  it's better not to allow invalid action in the first place.
  
  
   
 
 
  ---
   hw/i386/acpi-build.c | 2 +-
   1 file changed, 1 insertion(+), 1 deletion(-)
  
  diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
  index b37a397..1fb92e5 100644
  --- a/hw/i386/acpi-build.c
  +++ b/hw/i386/acpi-build.c
  @@ -913,7 +913,7 @@ static void build_pci_bus_end(PCIBus *bus, void 
  *bus_state)
   }
   }
   
  -if (!dc-hotpluggable || bridge_in_acpi) {
  +if (!dc-hotpluggable || pc-is_bridge) {
   clear_bit(slot, slot_hotplug_enable);
   }
   }
  -- 
  1.8.3.1




Re: [Qemu-devel] [PATCH 2/9] pc: acpi: decribe bridge device as not hotpluggable

2014-12-09 Thread Igor Mammedov
On Tue, 9 Dec 2014 14:51:59 +0200
Michael S. Tsirkin m...@redhat.com wrote:

 On Tue, Dec 09, 2014 at 12:45:27PM +0100, Igor Mammedov wrote:
  On Tue, 9 Dec 2014 12:34:02 +0200
  Michael S. Tsirkin m...@redhat.com wrote:
  
   On Tue, Dec 09, 2014 at 11:27:16AM +0100, Igor Mammedov wrote:
On Mon, 8 Dec 2014 21:13:25 +0200
Michael S. Tsirkin m...@redhat.com wrote:

 On Mon, Dec 08, 2014 at 04:08:01PM +, Igor Mammedov wrote:
  when bridge hotplug is disabled, i.e. for machine
  types less then 2.0, bridge device was created as
  hotpluggable by mistake since commit 133a2da.
  
  Fix it by just creating it as a present device.
  
  Signed-off-by: Igor Mammedov imamm...@redhat.com
 
 What exactly is the problem here?
 It seems that such bridge is hotpluggable, even though
 e.g. windows guests lacks drivers to support this.
before 133a2da slot with existing at startup bridge weren't
marked as hotpluggable nor described in SSDT. But after
133a2da it's described as hotpluggable slot for compat
machines (2.0 and lower) which doesn't match with original
behavior.

Also Marcel mentioned that bridges could be hotpluggable
but that they should not be hot-UNpluggable.
   
   OK so is there some guest that's confused?
   What's the bug?
  It allows to do eject on bridge when it shouldn't.
 
 I'm not sure it shouldn't. Assuming guest supports SHPC, eject on such a
 bridge might well work correctly.
Question here is should we keep compat mode as it was for old machine types
(i.e. not hot-removable), for new machines bridge is not hot-removable now
or just ignore silent change made by 133a2da?

 
 
  I don't know about consequences of it,
  it's better not to allow invalid action in the first place.
  
  
   
 
 
  ---
   hw/i386/acpi-build.c | 2 +-
   1 file changed, 1 insertion(+), 1 deletion(-)
  
  diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
  index b37a397..1fb92e5 100644
  --- a/hw/i386/acpi-build.c
  +++ b/hw/i386/acpi-build.c
  @@ -913,7 +913,7 @@ static void build_pci_bus_end(PCIBus *bus, void 
  *bus_state)
   }
   }
   
  -if (!dc-hotpluggable || bridge_in_acpi) {
  +if (!dc-hotpluggable || pc-is_bridge) {
   clear_bit(slot, slot_hotplug_enable);
   }
   }
  -- 
  1.8.3.1




[Qemu-devel] [PATCH 2/5] tpm: Allow 32 16 bit accesses to the registers

2014-12-09 Thread Stefan Berger
Improve the access to the registers with 32 and 16 bit reads and writes.
Also enable access to a non-base register address, such as reads of the
2nd byte of a register. Map the FIFO byte access to any byte within
its 4 byte register (following specs).

Signed-off-by: Stefan Berger stef...@linux.vnet.ibm.com
---
 hw/tpm/tpm_tis.c | 60 
 1 file changed, 47 insertions(+), 13 deletions(-)

diff --git a/hw/tpm/tpm_tis.c b/hw/tpm/tpm_tis.c
index c0e7cd7..6170693 100644
--- a/hw/tpm/tpm_tis.c
+++ b/hw/tpm/tpm_tis.c
@@ -427,6 +427,7 @@ static uint64_t tpm_tis_mmio_read(void *opaque, hwaddr addr,
 uint32_t val = 0x;
 uint8_t locty = tpm_tis_locality_from_addr(addr);
 uint32_t avail;
+uint8_t v;
 
 if (tpm_backend_had_startup_error(s-be_driver)) {
 return val;
@@ -476,14 +477,26 @@ static uint64_t tpm_tis_mmio_read(void *opaque, hwaddr 
addr,
 break;
 case TPM_TIS_REG_DATA_FIFO:
 if (tis-active_locty == locty) {
-switch (tis-loc[locty].state) {
-case TPM_TIS_STATE_COMPLETION:
-val = tpm_tis_data_read(s, locty);
-break;
-default:
-val = TPM_TIS_NO_DATA_BYTE;
-break;
+if (size  4 - (addr  0x3)) {
+/* prevent access beyond FIFO */
+size = 4 - (addr  0x3);
+}
+val = 0;
+shift = 0;
+while (size  0) {
+switch (tis-loc[locty].state) {
+case TPM_TIS_STATE_COMPLETION:
+v = tpm_tis_data_read(s, locty);
+break;
+default:
+v = TPM_TIS_NO_DATA_BYTE;
+break;
+}
+val |= (v  shift);
+shift += 8;
+size--;
 }
+shift = 0; /* no more adjustments */
 }
 break;
 case TPM_TIS_REG_DID_VID:
@@ -518,11 +531,13 @@ static void tpm_tis_mmio_write_intern(void *opaque, 
hwaddr addr,
 {
 TPMState *s = opaque;
 TPMTISEmuState *tis = s-s.tis;
-uint16_t off = addr  0xfff;
+uint16_t off = addr  0xffc;
+uint8_t shift = (addr  0x3) * 8;
 uint8_t locty = tpm_tis_locality_from_addr(addr);
 uint8_t active_locty, l;
 int c, set_new_locty = 1;
 uint16_t len;
+uint32_t mask = (size == 1) ? 0xff : ((size == 2) ? 0x : ~0);
 
 DPRINTF(tpm_tis: write.%u(%08x) = %08x\n, size, (int)addr, 
(uint32_t)val);
 
@@ -535,6 +550,15 @@ static void tpm_tis_mmio_write_intern(void *opaque, hwaddr 
addr,
 return;
 }
 
+val = mask;
+
+if (shift) {
+val = shift;
+mask = shift;
+}
+
+mask ^= 0x;
+
 switch (off) {
 case TPM_TIS_REG_ACCESS:
 
@@ -646,9 +670,10 @@ static void tpm_tis_mmio_write_intern(void *opaque, hwaddr 
addr,
 break;
 }
 
-tis-loc[locty].inte = (val  (TPM_TIS_INT_ENABLED |
-   TPM_TIS_INT_POLARITY_MASK |
-   TPM_TIS_INTERRUPTS_SUPPORTED));
+tis-loc[locty].inte = mask;
+tis-loc[locty].inte |= (val  (TPM_TIS_INT_ENABLED |
+TPM_TIS_INT_POLARITY_MASK |
+TPM_TIS_INTERRUPTS_SUPPORTED));
 break;
 case TPM_TIS_REG_INT_VECTOR:
 /* hard wired -- ignore */
@@ -747,16 +772,25 @@ static void tpm_tis_mmio_write_intern(void *opaque, 
hwaddr addr,
 tis-loc[locty].state == TPM_TIS_STATE_COMPLETION) {
 /* drop the byte */
 } else {
-DPRINTF(tpm_tis: Byte to send to TPM: %02x\n, (uint8_t)val);
+DPRINTF(tpm_tis: Data to send to TPM: %08x (size=%d)\n,
+val, size);
 if (tis-loc[locty].state == TPM_TIS_STATE_READY) {
 tis-loc[locty].state = TPM_TIS_STATE_RECEPTION;
 tis-loc[locty].sts = TPM_TIS_STS_EXPECT | TPM_TIS_STS_VALID;
 }
 
-if ((tis-loc[locty].sts  TPM_TIS_STS_EXPECT)) {
+val = shift;
+if (size  4 - (addr  0x3)) {
+/* prevent access beyond FIFO */
+size = 4 - (addr  0x3);
+}
+
+while ((tis-loc[locty].sts  TPM_TIS_STS_EXPECT)  size  0) {
 if (tis-loc[locty].w_offset  tis-loc[locty].w_buffer.size) {
 tis-loc[locty].w_buffer.
 buffer[tis-loc[locty].w_offset++] = (uint8_t)val;
+val = 8;
+size--;
 } else {
 tis-loc[locty].sts = TPM_TIS_STS_VALID;
 }
-- 
1.9.3




[Qemu-devel] [PATCH 0/5] tpm: Extend the TPM TIS implementation

2014-12-09 Thread Stefan Berger
The following series of patches extends the TPM TIS implementation to
version 1.3. This will lead to a TIS version that supports TPM 2.
For this I would post relatively small patches afterwards.

Regards,
   Stefan


Stefan Berger (5):
  tpm: Extend sts register to 32 bit
  tpm: Allow 32  16 bit accesses to the registers
  tpm: Support for XFIFO register
  tpm: Support for TIS selftest done flag
  tpm: Support for capability flags of TIS 1.3

 hw/tpm/tpm_int.h |   1 +
 hw/tpm/tpm_passthrough.c |  37 ++--
 hw/tpm/tpm_tis.c | 131 ++-
 hw/tpm/tpm_tis.h |   2 +-
 include/sysemu/tpm_backend.h |   2 +-
 5 files changed, 139 insertions(+), 34 deletions(-)

-- 
1.9.3




[Qemu-devel] [PATCH 5/5] tpm: Support for capability flags of TIS 1.3

2014-12-09 Thread Stefan Berger
Provide the TIS 1.3 capability flags.
The interface now looks like a TIS 1.3 interface. It's fully
compatible with previous TIS 1.2 and drivers written for
TIS 1.2 continue to work.

Signed-off-by: Stefan Berger stef...@linux.vnet.ibm.com
---
 hw/tpm/tpm_tis.c | 9 -
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/hw/tpm/tpm_tis.c b/hw/tpm/tpm_tis.c
index 61186c5..d0bb97f 100644
--- a/hw/tpm/tpm_tis.c
+++ b/hw/tpm/tpm_tis.c
@@ -14,7 +14,7 @@
  *
  * Implementation of the TIS interface according to specs found at
  * http://www.trustedcomputinggroup.org. This implementation currently
- * supports version 1.21, revision 1.0.
+ * supports version 1.3, 21 March 2013
  * In the developers menu choose the PC Client section then find the TIS
  * specification.
  */
@@ -103,8 +103,15 @@
 
 #endif
 
+#define TPM_TIS_CAP_INTERFACE_VERSION1_3 (2  28)
+#define TPM_TIS_CAP_DATA_TRANSFER_64B(3  9)
+#define TPM_TIS_CAP_DATA_TRANSFER_LEGACY (0  9)
+#define TPM_TIS_CAP_BURST_COUNT_DYNAMIC  (0  8)
 #define TPM_TIS_CAP_INTERRUPT_LOW_LEVEL  (1  4) /* support is mandatory */
 #define TPM_TIS_CAPABILITIES_SUPPORTED   (TPM_TIS_CAP_INTERRUPT_LOW_LEVEL | \
+  TPM_TIS_CAP_BURST_COUNT_DYNAMIC | \
+  TPM_TIS_CAP_DATA_TRANSFER_64B | \
+  TPM_TIS_CAP_INTERFACE_VERSION1_3 | \
   TPM_TIS_INTERRUPTS_SUPPORTED)
 
 #define TPM_TIS_TPM_DID   0x0001
-- 
1.9.3




[Qemu-devel] [PATCH 3/5] tpm: Support for XFIFO register

2014-12-09 Thread Stefan Berger
Support for the XFIFO register (range) of the TIS 1.3 specification.
We support a range of 64 bytes.

Signed-off-by: Stefan Berger stef...@linux.vnet.ibm.com
---
 hw/tpm/tpm_tis.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/hw/tpm/tpm_tis.c b/hw/tpm/tpm_tis.c
index 6170693..a37c7ce 100644
--- a/hw/tpm/tpm_tis.c
+++ b/hw/tpm/tpm_tis.c
@@ -51,6 +51,8 @@
 #define TPM_TIS_REG_INTF_CAPABILITY   0x14
 #define TPM_TIS_REG_STS   0x18
 #define TPM_TIS_REG_DATA_FIFO 0x24
+#define TPM_TIS_REG_DATA_XFIFO0x80
+#define TPM_TIS_REG_DATA_XFIFO_END0xbc
 #define TPM_TIS_REG_DID_VID   0xf00
 #define TPM_TIS_REG_RID   0xf04
 
@@ -476,6 +478,7 @@ static uint64_t tpm_tis_mmio_read(void *opaque, hwaddr addr,
 }
 break;
 case TPM_TIS_REG_DATA_FIFO:
+case TPM_TIS_REG_DATA_XFIFO ... TPM_TIS_REG_DATA_XFIFO_END:
 if (tis-active_locty == locty) {
 if (size  4 - (addr  0x3)) {
 /* prevent access beyond FIFO */
@@ -762,6 +765,7 @@ static void tpm_tis_mmio_write_intern(void *opaque, hwaddr 
addr,
 }
 break;
 case TPM_TIS_REG_DATA_FIFO:
+case TPM_TIS_REG_DATA_XFIFO ... TPM_TIS_REG_DATA_XFIFO_END:
 /* data fifo */
 if (tis-active_locty != locty) {
 break;
-- 
1.9.3




[Qemu-devel] [PATCH 4/5] tpm: Support for TIS selftest done flag

2014-12-09 Thread Stefan Berger
Extend the backend to check whether the TPM_ContinueSelfTest
finished successfully and provide a flag to the TIS front-end
if it successfully finished. The TIS then sets a flag in
all localities in the STS register and keeps it until the next
reset.

Signed-off-by: Stefan Berger stef...@linux.vnet.ibm.com
---
 hw/tpm/tpm_int.h |  1 +
 hw/tpm/tpm_passthrough.c | 37 
 hw/tpm/tpm_tis.c | 58 ++--
 include/sysemu/tpm_backend.h |  2 +-
 4 files changed, 79 insertions(+), 19 deletions(-)

diff --git a/hw/tpm/tpm_int.h b/hw/tpm/tpm_int.h
index 2f582ca..2b35fe2 100644
--- a/hw/tpm/tpm_int.h
+++ b/hw/tpm/tpm_int.h
@@ -62,6 +62,7 @@ struct tpm_resp_hdr {
 
 #define TPM_FAIL  9
 
+#define TPM_ORD_ContinueSelfTest  0x53
 #define TPM_ORD_GetTicks  0xf1
 
 #endif /* TPM_TPM_INT_H */
diff --git a/hw/tpm/tpm_passthrough.c b/hw/tpm/tpm_passthrough.c
index 56e9e0f..29063cf 100644
--- a/hw/tpm/tpm_passthrough.c
+++ b/hw/tpm/tpm_passthrough.c
@@ -112,14 +112,31 @@ static void tpm_write_fatal_error_response(uint8_t *out, 
uint32_t out_len)
 }
 }
 
+static bool tpm_passthrough_is_selftest(const uint8_t *in, uint32_t in_len)
+{
+struct tpm_req_hdr *hdr = (struct tpm_req_hdr *)in;
+
+if (in_len = sizeof(*hdr)) {
+return (be32_to_cpu(hdr-ordinal) == TPM_ORD_ContinueSelfTest);
+}
+
+return false;
+}
+
 static int tpm_passthrough_unix_tx_bufs(TPMPassthruState *tpm_pt,
 const uint8_t *in, uint32_t in_len,
-uint8_t *out, uint32_t out_len)
+uint8_t *out, uint32_t out_len,
+bool *selftest_done)
 {
 int ret;
+bool is_selftest;
+const struct tpm_resp_hdr *hdr;
 
 tpm_pt-tpm_op_canceled = false;
 tpm_pt-tpm_executing = true;
+*selftest_done = false;
+
+is_selftest = tpm_passthrough_is_selftest(in, in_len);
 
 ret = tpm_passthrough_unix_write(tpm_pt-tpm_fd, in, in_len);
 if (ret != in_len) {
@@ -149,6 +166,11 @@ static int tpm_passthrough_unix_tx_bufs(TPMPassthruState 
*tpm_pt,
  packet from TPM\n);
 }
 
+if (is_selftest  (ret = sizeof(struct tpm_resp_hdr))) {
+hdr = (struct tpm_resp_hdr *)out;
+*selftest_done = (be32_to_cpu(hdr-errcode) == 0);
+}
+
 err_exit:
 if (ret  0) {
 tpm_write_fatal_error_response(out, out_len);
@@ -160,13 +182,15 @@ err_exit:
 }
 
 static int tpm_passthrough_unix_transfer(TPMPassthruState *tpm_pt,
- const TPMLocality *locty_data)
+ const TPMLocality *locty_data,
+ bool *selftest_done)
 {
 return tpm_passthrough_unix_tx_bufs(tpm_pt,
 locty_data-w_buffer.buffer,
 locty_data-w_offset,
 locty_data-r_buffer.buffer,
-locty_data-r_buffer.size);
+locty_data-r_buffer.size,
+selftest_done);
 }
 
 static void tpm_passthrough_worker_thread(gpointer data,
@@ -175,16 +199,19 @@ static void tpm_passthrough_worker_thread(gpointer data,
 TPMPassthruThreadParams *thr_parms = user_data;
 TPMPassthruState *tpm_pt = TPM_PASSTHROUGH(thr_parms-tb);
 TPMBackendCmd cmd = (TPMBackendCmd)data;
+bool selftest_done = false;
 
 DPRINTF(tpm_passthrough: processing command type %d\n, cmd);
 
 switch (cmd) {
 case TPM_BACKEND_CMD_PROCESS_CMD:
 tpm_passthrough_unix_transfer(tpm_pt,
-  thr_parms-tpm_state-locty_data);
+  thr_parms-tpm_state-locty_data,
+  selftest_done);
 
 thr_parms-recv_data_callback(thr_parms-tpm_state,
-  thr_parms-tpm_state-locty_number);
+  thr_parms-tpm_state-locty_number,
+  selftest_done);
 break;
 case TPM_BACKEND_CMD_INIT:
 case TPM_BACKEND_CMD_END:
diff --git a/hw/tpm/tpm_tis.c b/hw/tpm/tpm_tis.c
index a37c7ce..61186c5 100644
--- a/hw/tpm/tpm_tis.c
+++ b/hw/tpm/tpm_tis.c
@@ -64,6 +64,7 @@
 #define TPM_TIS_STS_TPM_GO(1  5)
 #define TPM_TIS_STS_DATA_AVAILABLE(1  4)
 #define TPM_TIS_STS_EXPECT(1  3)
+#define TPM_TIS_STS_SELFTEST_DONE (1  2)
 #define TPM_TIS_STS_RESPONSE_RETRY(1  1)
 
 #define TPM_TIS_BURST_COUNT_SHIFT 8
@@ -147,6 +148,24 @@ static void tpm_tis_show_buffer(const TPMSizedBuffer *sb, 
const char *string)
 }
 
 /*
+ * Set the given flags in the STS register by clearing the register but
+ * preserving the SELFTEST_DONE flag 

[Qemu-devel] [PATCH 1/5] tpm: Extend sts register to 32 bit

2014-12-09 Thread Stefan Berger
More recent TIS specs extend the STS register to 32 bit. While
we don't store the TIS interface state, yet, we can extend it
without sideeffects.

Signed-off-by: Stefan Berger stef...@linux.vnet.ibm.com
---
 hw/tpm/tpm_tis.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/tpm/tpm_tis.h b/hw/tpm/tpm_tis.h
index 1a0db23..db78d51 100644
--- a/hw/tpm/tpm_tis.h
+++ b/hw/tpm/tpm_tis.h
@@ -41,7 +41,7 @@ typedef enum {
 typedef struct TPMLocality {
 TPMTISState state;
 uint8_t access;
-uint8_t sts;
+uint32_t sts;
 uint32_t inte;
 uint32_t ints;
 
-- 
1.9.3




Re: [Qemu-devel] [PATCH 9/9] pc: acpi-build: replace recursive PCI bus tree generation with loop based

2014-12-09 Thread Michael S. Tsirkin
On Tue, Dec 09, 2014 at 03:01:00PM +0100, Igor Mammedov wrote:
 On Mon, 8 Dec 2014 22:43:24 +0200
 Michael S. Tsirkin m...@redhat.com wrote:
 
  On Mon, Dec 08, 2014 at 04:08:08PM +, Igor Mammedov wrote:
   it replaces PCI tree structure in SSDT with a set of scopes
   describing each PCI bus as a separate scope with a child devices.
   It makes code easier to follow and a little bit smaller.
   
   In addition it makes simplier to convert current template
   patching approach to completely dynamically generated SSDT
   without dependency on IASL, which would allow to drop
   template binary blobs from source tree.
   
   Signed-off-by: Igor Mammedov imamm...@redhat.com
   ---
hw/i386/acpi-build.c | 362 
   +++
1 file changed, 165 insertions(+), 197 deletions(-)
  
  I like it that your patch makes code smaller and simpler,
  but I think we do need to generate hierarchical AML.
 If only reason for hierarchical AML is table size then
 I don't see how scopes could possibly double the table size,
 Compared to tree version scope adds only:
 
  ~5 bytes for package + NamePath(~[0-2] + 4 * tree depth)

No, tree depth squared since you generate this per bridge.

 Which is almost nothing compared to slots description
 per a bridge.
 One needs to configure insanely deep bridge nesting
 for scope cost to significantly affect table size, which
 doesn't sound as practical thing to do anyway except of
 testing limitation on how many nested bridges QEMU would
 stomach.

pci supports up to 256 nested bridges.
So that's 256K which isn't the end of the world but
not negligeable for a bios.

  
  I think this can still be done with some modifications to
  the logic you have here.
  
  Basically current code does all work in build_pci_bus_end.
  It follows that you can do the same by simply walking
  the list in reverse order.
 I've thought about it already, but tree creation means
 creating temporary contexts for each scope, copying them
 to parent context, keeping track of contexts to build
 correct notifiers. It's basically the same or even worse
 as current code just other way around.

Well, the way I see it, the main simplification is
that you can pass data from child to parent explicitly.

Maybe I'm not explaining this properly, I'll try to
find the time to send a patch for comparison.

 And all that makes code quite hard to follow and
 maintain, that is the main reason to drop recursion in
 favor of simple flat scope sets.

I agree the approach of unrolling the tree, then walking the list has
advantages, but if it's forcing a specific AML structure then that
really means it's too limited.

 And it makes easier to
 switch from AML+template patching to ASL API later
 and hide from user direct access to table array, forcing
 him to work only with ASL constructs when building table.

I don't necessarily see how these two ideas are related.

  
  
  
  
  
   diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
   index 97ff245..7606a73 100644
   --- a/hw/i386/acpi-build.c
   +++ b/hw/i386/acpi-build.c
   @@ -641,196 +641,186 @@ static void acpi_set_pci_info(void)
}
}

   -static void build_pci_bus_state_init(AcpiBuildPciBusHotplugState *state,
   - AcpiBuildPciBusHotplugState *parent,
   - bool pcihp_bridge_en)
   +static char *pci_bus_get_scope_name(PCIBus *bus)
{
   -state-parent = parent;
   -state-device_table = build_alloc_array();
   -state-notify_table = build_alloc_array();
   -state-pcihp_bridge_en = pcihp_bridge_en;
   -}
   +char *name = NULL;
   +char *last = name;

   -static void build_pci_bus_state_cleanup(AcpiBuildPciBusHotplugState 
   *state)
   -{
   -build_free_array(state-device_table);
   -build_free_array(state-notify_table);
   -}
   +while (bus-parent_dev) {
   +last = name;
   +name = g_strdup_printf(%s.S%.02X_,
   +   name ? name : , bus-parent_dev-devfn);
   +g_free(last);

   -static void *build_pci_bus_begin(PCIBus *bus, void *parent_state)
   -{
   -AcpiBuildPciBusHotplugState *parent = parent_state;
   -AcpiBuildPciBusHotplugState *child = g_malloc(sizeof *child);
   +bus = bus-parent_dev-bus;
   +}

   -build_pci_bus_state_init(child, parent, parent-pcihp_bridge_en);
   +last = name;
   +name = g_strdup_printf(PCI0%s, name ? name : );
   +g_free(last);

   -return child;
   +return name;
}

   -static void build_pci_bus_end(PCIBus *bus, void *bus_state)
   +static GQueue *get_pci_bus_list(PCIBus *bus, bool pcihp_bridge_en)
{
   -AcpiBuildPciBusHotplugState *child = bus_state;
   -AcpiBuildPciBusHotplugState *parent = child-parent;
   -GArray *bus_table = build_alloc_array();
   -DECLARE_BITMAP(slot_hotplug_enable, PCI_SLOT_MAX);
   -

Re: [Qemu-devel] [PATCH 0/5] tpm: Extend the TPM TIS implementation

2014-12-09 Thread Michael S. Tsirkin
On Tue, Dec 09, 2014 at 09:18:54AM -0500, Stefan Berger wrote:
 The following series of patches extends the TPM TIS implementation to
 version 1.3. This will lead to a TIS version that supports TPM 2.
 For this I would post relatively small patches afterwards.
 
 Regards,
Stefan


Since this is guest visible, should this be limited
to new machine types, to avoid breaking migrating
guests across hypervisor versions?

 
 Stefan Berger (5):
   tpm: Extend sts register to 32 bit
   tpm: Allow 32  16 bit accesses to the registers
   tpm: Support for XFIFO register
   tpm: Support for TIS selftest done flag
   tpm: Support for capability flags of TIS 1.3
 
  hw/tpm/tpm_int.h |   1 +
  hw/tpm/tpm_passthrough.c |  37 ++--
  hw/tpm/tpm_tis.c | 131 
 ++-
  hw/tpm/tpm_tis.h |   2 +-
  include/sysemu/tpm_backend.h |   2 +-
  5 files changed, 139 insertions(+), 34 deletions(-)
 
 -- 
 1.9.3



Re: [Qemu-devel] [PATCH 0/5] tpm: Extend the TPM TIS implementation

2014-12-09 Thread Stefan Berger

On 12/09/2014 09:35 AM, Michael S. Tsirkin wrote:

On Tue, Dec 09, 2014 at 09:18:54AM -0500, Stefan Berger wrote:

The following series of patches extends the TPM TIS implementation to
version 1.3. This will lead to a TIS version that supports TPM 2.
For this I would post relatively small patches afterwards.

Regards,
Stefan


Since this is guest visible, should this be limited
to new machine types, to avoid breaking migrating
guests across hypervisor versions?


Migration is not currently possible with TPM, since TPM (passthrough) is 
preventing it.


Stefan




Re: [Qemu-devel] [PATCH 0/5] tpm: Extend the TPM TIS implementation

2014-12-09 Thread Michael S. Tsirkin
On Tue, Dec 09, 2014 at 09:40:28AM -0500, Stefan Berger wrote:
 On 12/09/2014 09:35 AM, Michael S. Tsirkin wrote:
 On Tue, Dec 09, 2014 at 09:18:54AM -0500, Stefan Berger wrote:
 The following series of patches extends the TPM TIS implementation to
 version 1.3. This will lead to a TIS version that supports TPM 2.
 For this I would post relatively small patches afterwards.
 
 Regards,
 Stefan
 
 Since this is guest visible, should this be limited
 to new machine types, to avoid breaking migrating
 guests across hypervisor versions?
 
 Migration is not currently possible with TPM, since TPM (passthrough) is
 preventing it.
 
 Stefan

OK, for live migration, fair enough.

For off-line migration: will the following still work:
- boot guest with tpm 1.3. use tpm in some way
- halt, start guest on old qemu with tpm 1.2
?




Re: [Qemu-devel] [PATCH 0/5] tpm: Extend the TPM TIS implementation

2014-12-09 Thread Stefan Berger

On 12/09/2014 09:45 AM, Michael S. Tsirkin wrote:

On Tue, Dec 09, 2014 at 09:40:28AM -0500, Stefan Berger wrote:

On 12/09/2014 09:35 AM, Michael S. Tsirkin wrote:

On Tue, Dec 09, 2014 at 09:18:54AM -0500, Stefan Berger wrote:

The following series of patches extends the TPM TIS implementation to
version 1.3. This will lead to a TIS version that supports TPM 2.
For this I would post relatively small patches afterwards.

Regards,
Stefan

Since this is guest visible, should this be limited
to new machine types, to avoid breaking migrating
guests across hypervisor versions?

Migration is not currently possible with TPM, since TPM (passthrough) is
preventing it.

 Stefan

OK, for live migration, fair enough.

For off-line migration: will the following still work:
- boot guest with tpm 1.3. use tpm in some way
- halt, start guest on old qemu with tpm 1.2
?


So this is not a suspend resume operation ? Sure, it will work if the 
driver the OS is using works with a TPM TIS 1.3 interface and also with 
a TPM TIS 1.2 interface.


  Stefan




Re: [Qemu-devel] [PATCH v3] qdev: Avoid type assertion in qdev_build_hotpluggable_device_list()

2014-12-09 Thread Andreas Färber
Am 08.12.2014 um 15:07 schrieb Jun Li:
 Ping, why does this patch has not been merged until now? Could anyone give
 some explanations? Thx.

I had already applied the previous version of the patch to qom-next.
I'll drop it and replace it with this version then.

Regards,
Andreas

 On Wed, 11/05 15:03, Jun Li wrote:
 Currently when *obj is not a TYPE_DEVICE, qemu will abort. This patch just
 fixed it. When *obj is not a TYPE_DEVICE, just do not add it to hotpluggable
 device list.

 This patch also fixed the following issue:
 1, boot qemu using cli:
 $ /opt/qemu-git-arm/bin/qemu-system-x86_64 -monitor stdio -enable-kvm \
 -device virtio-scsi-pci,id=scsi0

 2, device_del scsi0 via hmp using tab key(first input device_del, then press
 Tab key).
 (qemu) device_del

 After step2, qemu will abort.
 (qemu) device_del hw/core/qdev.c:930:qdev_build_hotpluggable_device_list:
 Object 0x563a2460 is not an instance of type device

 Signed-off-by: Jun Li junm...@gmail.com
 Reviewed-by: Paolo Bonzini pbonz...@redhat.com
 ---
 v3:
   According to Andreas's suggestion, do some changes. As followings:
   1, change the Subject to more meaningful.
   2, use two return 0 to return early avoid reindentation. And I have 
 found in qcow2_grow_l1_table has also used two return 0. So accept 
 Andreas's suggestion. Thanks.

 v2:
   This version just do a little changes for the commit message.
 As following show:
 In v1,
 1, boot qemu using cli:
 virtio-scsi-pci,id=scsi0 -enable-kvm

 In v2,
 1, boot qemu using cli:
 $ /opt/qemu-git-arm/bin/qemu-system-x86_64 -monitor stdio -enable-kvm \
 -device virtio-scsi-pci,id=scsi0
 ---
  hw/core/qdev.c | 7 ++-
  1 file changed, 6 insertions(+), 1 deletion(-)

 diff --git a/hw/core/qdev.c b/hw/core/qdev.c
 index b3d5196..7db3e13 100644
 --- a/hw/core/qdev.c
 +++ b/hw/core/qdev.c
 @@ -927,7 +927,12 @@ void qdev_alias_all_properties(DeviceState *target, 
 Object *source)
  int qdev_build_hotpluggable_device_list(Object *obj, void *opaque)
  {
  GSList **list = opaque;
 -DeviceState *dev = DEVICE(obj);
 +DeviceState *dev = (DeviceState *)object_dynamic_cast(OBJECT(obj),
 +  device);
 +
 +if (dev == NULL) {
 +return 0;
 +}
  
  if (dev-realized  object_property_get_bool(obj, hotpluggable, 
 NULL)) {
  *list = g_slist_append(*list, dev);
 -- 
 1.9.3

 


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



Re: [Qemu-devel] [PATCH] qmp: extend QMP to provide read/write access to physical memory

2014-12-09 Thread Bryan D. Payne

 By evidence, I mean actual numbers for actual QEMU code.  Nothing
 sophisticated, just use your new interface in a way you consider
 relevant for your own use case, then approximate this use with existing
 interfaces.  The approximation can be very rough.  For instance, showing
 that doing the whole job with your approach is a much faster than doing
 a necessary part of the job with existing commands would do.


Sure, I can put together some numbers to help with this discussion.



 The QMP overhead could be too high.  QMP is control plane, not data
 plane.  How much data do you envisage flowing here?

 In theory, even the character device overhead could be too high.
 Measuring it shouldn't be too hard.


This discussion has me thinking about whether QMP would be viable.  I think
I'll take a little time to explore that approach in a little more depth
before proceeding here.  I can report back with what I find.


If you control a QMP monitor, you own the guest.


Good point.  So I'll explore the performance aspects and let that drive the
decision.



 I feel code comment is find for internal interfaces, but for external
 interfaces, a separate interface document is more appropriate.


Sounds good.

Cheers,
-bryan


Re: [Qemu-devel] [PATCH v2] Block migration: fix return value

2014-12-09 Thread Stefan Hajnoczi
On Tue, Nov 25, 2014 at 05:30:02PM -0600, grhookatw...@gmail.com wrote:
 From: Gary R Hook gary.h...@nimboxx.com
 
 Modify block_save_iterate() to return positive/zero/negative
 (success/not done/failure) return status. The computation of
 the blocks transferred (an int64_t) exceeds the size of an
 int return value.
 
 Signed-off-by: Gary R Hook gary.h...@nimboxx.com
 
 ---
  block-migration.c | 10 +-
  1 file changed, 9 insertions(+), 1 deletion(-)

Thanks, applied to my block-next tree:
https://github.com/stefanha/qemu/commits/block-next

Stefan


pgpc9EA4vVOfw.pgp
Description: PGP signature


Re: [Qemu-devel] [PATCH] nvme: 64kB page size fixes

2014-12-09 Thread Stefan Hajnoczi
On Tue, Dec 02, 2014 at 09:18:59PM +0100, Paolo Bonzini wrote:
 
 
 On 27/11/2014 04:39, Anton Blanchard wrote:
  Initialise our maximum page size capability to 64kB and increase
  the page_size variable from 16 to 32 bits.
  
  Signed-off-by: Anton Blanchard an...@samba.org
  --
  
  diff --git a/hw/block/nvme.c b/hw/block/nvme.c
  index 1327658..aa1ed98 100644
  --- a/hw/block/nvme.c
  +++ b/hw/block/nvme.c
  @@ -811,6 +811,7 @@ static int nvme_init(PCIDevice *pci_dev)
   NVME_CAP_SET_AMS(n-bar.cap, 1);
   NVME_CAP_SET_TO(n-bar.cap, 0xf);
   NVME_CAP_SET_CSS(n-bar.cap, 1);
  +NVME_CAP_SET_MPSMAX(n-bar.cap, 4);
   
   n-bar.vs = 0x00010001;
   n-bar.intmc = n-bar.intms = 0;
  diff --git a/hw/block/nvme.h b/hw/block/nvme.h
  index 993c511..b6ccb65 100644
  --- a/hw/block/nvme.h
  +++ b/hw/block/nvme.h
  @@ -688,7 +688,7 @@ typedef struct NvmeCtrl {
   NvmeBar  bar;
   BlockConfconf;
   
  -uint16_tpage_size;
  +uint32_tpage_size;
   uint16_tpage_bits;
   uint16_tmax_prp_ents;
   uint16_tcqe_size;
  
  
 
 This should probably be a property of the device instead.  If you want
 to change the default, you need to preserve a backwards-compatible value
 for pre-2.3 machine types (-M pc-i440fx-2.2, -M pc-i440fx-2.1 etc.).

Hi Anton,
Any update on this?

Paolo has a point that changing this value would make a guest-visible
hardware change.  Therefore older machine types, which users might rely
on, should continue to show the old value.

This is especially important with live migration because hardware should
not change underneath the guest due to migration.

Stefan


pgp3F0WRrpdGt.pgp
Description: PGP signature


Re: [Qemu-devel] [PATCH v3] qdev: Avoid type assertion in qdev_build_hotpluggable_device_list()

2014-12-09 Thread Andreas Färber
Am 05.11.2014 um 08:03 schrieb Jun Li:
 Currently when *obj is not a TYPE_DEVICE, qemu will abort. This patch just
 fixed it. When *obj is not a TYPE_DEVICE, just do not add it to hotpluggable
 device list.
 
 This patch also fixed the following issue:
 1, boot qemu using cli:
 $ /opt/qemu-git-arm/bin/qemu-system-x86_64 -monitor stdio -enable-kvm \
 -device virtio-scsi-pci,id=scsi0
 
 2, device_del scsi0 via hmp using tab key(first input device_del, then press
 Tab key).
 (qemu) device_del
 
 After step2, qemu will abort.
 (qemu) device_del hw/core/qdev.c:930:qdev_build_hotpluggable_device_list:
 Object 0x563a2460 is not an instance of type device
 
 Signed-off-by: Jun Li junm...@gmail.com
 Reviewed-by: Paolo Bonzini pbonz...@redhat.com
 ---
 v3:
   According to Andreas's suggestion, do some changes. As followings:
   1, change the Subject to more meaningful.
   2, use two return 0 to return early avoid reindentation. And I have found 
 in qcow2_grow_l1_table has also used two return 0. So accept Andreas's 
 suggestion. Thanks.

Thanks, replaced the queued version on qom-next with a few edits and the
following change:
https://github.com/afaerber/qemu-cpu/commits/qom-next

diff --git a/hw/core/qdev.c b/hw/core/qdev.c
index 51ab59e..ef06aa4 100644
--- a/hw/core/qdev.c
+++ b/hw/core/qdev.c
@@ -939,7 +939,7 @@ static int qdev_add_hotpluggable_device(Object *obj,
void *opaque)
 {
 GSList **list = opaque;
 DeviceState *dev = (DeviceState *)object_dynamic_cast(OBJECT(obj),
-  device);
+  TYPE_DEVICE);

 if (dev == NULL) {
 return 0;

Regards,
Andreas

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



Re: [Qemu-devel] about tracetool

2014-12-09 Thread Stefan Hajnoczi
On Tue, Dec 02, 2014 at 09:46:50AM +0900, Ady Wahyudi Paundu wrote:
 I know that simpletrace records go to memory buffer before it flushed
 to file by a writer thread.  My question is how to call this thread
 (preferably using python), so I can make the flush process periodical?

Look at the source code: trace/simple.c

Why do you want to do this?

Stefan


pgpBjnddPk6v8.pgp
Description: PGP signature


Re: [Qemu-devel] [PATCH for-2.3 0/2] block: Fix for trailing whitespace in qemu-img create and its tests

2014-12-09 Thread Eric Blake
On 12/09/2014 12:38 AM, Fam Zheng wrote:
 This will make it easier to keep checkpatch.pl happy.
 
 Fam Zheng (2):
   qemu-iotests: Remove traling whitespaces in *.out
   block: Don't add trailing space in Formating... message

Series:
Reviewed-by: Eric Blake ebl...@redhat.com

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



signature.asc
Description: OpenPGP digital signature


Re: [Qemu-devel] [PATCH] qmp-command.hx: add missing docs for migration capabilites

2014-12-09 Thread Eric Blake
On 12/08/2014 11:38 PM, zhanghailiang wrote:
 Signed-off-by: zhanghailiang zhang.zhanghaili...@huawei.com
 ---
  qmp-commands.hx | 6 ++
  1 file changed, 6 insertions(+)


  - xbzrle: XBZRLE support
 +- rdma-pin-all: pin all pages when use of RDMA during migration

s/use of/using/

With the grammar fix,
Reviewed-by: Eric Blake ebl...@redhat.com

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



signature.asc
Description: OpenPGP digital signature


Re: [Qemu-devel] [PATCH v2] arm: add fw_cfg to virt board

2014-12-09 Thread Laszlo Ersek
On 12/09/14 10:31, Gerd Hoffmann wrote:
   Hi,
 
 So... after playing with this thing for some time, it's become clear
 that MMIO traps are painfully slow on the aarch64 platform we've been
 working on (using KVM).
 
 So, as we don't have compatibility requirements, and we also can't play
 tricks like using x86 string instructions:  How about a completely
 different, dma-style interface for fw_cfg access?
 
 One register for the (physical) target address.
 One register for the transfer size.
 One register for the fw_cfg entry.
 Possibly one register for the fw_cfg offset (not really needed, but
 avoids the need for side effects such as writing fw_cfg entry register
 clearing the offset).
 One register to kick the transfer.

Again, this was the idea that Rich had in 2010 (see the links in the
discussion thus far). It was rejected back then (which is why I didn't
even try to resurrect it now), and Peter has now asked if anything has
changed that would make that approach more acceptable now.

Avi and Gleb not hanging around so they can't block the approach now
might or might not be such a change; I don't know. But, I tend to trust
their opinion, even if dates back to 2010 in this case.

Anyway I'm certainly not opposed to performance, so if someone can
thoroughly refute everything they said in that thread, go ahead.

Thanks!
Laszlo



Re: [Qemu-devel] [RFC PATCH V2 1/4] block: add accounting for merged requests

2014-12-09 Thread Peter Lieven

On 05.12.2014 15:58, Eric Blake wrote:

On 12/05/2014 04:50 AM, Peter Lieven wrote:

Signed-off-by: Peter Lieven p...@kamp.de
---
  block.c|2 ++
  block/accounting.c |7 +++
  block/qapi.c   |2 ++
  hmp.c  |6 +-
  include/block/accounting.h |3 +++
  qapi/block-core.json   |9 -
  qmp-commands.hx|   22 ++
  7 files changed, 45 insertions(+), 6 deletions(-)
@@ -2284,6 +2288,8 @@ Example:
rd_total_times_ns:3465673657
flush_total_times_ns:49653
flush_operations:61,
+  rd_merged:0,
+  wr_merged:0

Oh my - we previously had invalid JSON in our example.  Yay that this
fixes it :)


There is actually more invalid JSON, but I would fix that in a separate
patch apart from this series.

Peter



Re: [Qemu-devel] [RFC PATCH V2 4/4] virtio-blk: introduce multiread

2014-12-09 Thread Peter Lieven

On 05.12.2014 16:05, Eric Blake wrote:

On 12/05/2014 04:50 AM, Peter Lieven wrote:

this patch finally introduce multiread support to virtio-blk while

s/introduce/introduces/
s/virtio-blk while/virtio-blk. While/


multiwrite support was there for a long time read support was missing.

s/time/time,/


To achieve this the patch does serveral things which might need futher

s/serveral/several/
s/futher/further/


explaination:

s/explaination/explanation/


  - the whole merge and multireq logic is moved from block.c into
virtio-blk. This is move is a preparation for directly creating a
coroutine out of virtio-blk.

Can this move be done as a separate prerequisite patch? Mixing code
motion and new features in the same patch is harder to review.


The issue is that the code is not movable. I could artifically use
the new code in a new patch only for write requests and add
another patch to add read merging. But I doubt that it would make
it easier to review but even harder.

Peter



[Qemu-devel] [Bug 1400768] [NEW] Fatal error when running with '-machine isapc' on 2.1.2

2014-12-09 Thread Matthew Thode
Public bug reported:

all I have are the traces, should hopefully be easy to reproduce.

# qemu-system-i386 -machine isapc
VNC server running on `::1:5900'
qemu: fatal: Trying to execute code outside RAM or ROM at 0x1a0dff44

EAX=000f0f88 EBX=0010 ECX=07fc EDX=002c
ESI=6f5c EDI=0800 EBP=07fc ESP=fffe0014
EIP=1a0dff44 EFL=0046 [---Z-P-] CPL=0 II=0 A20=1 SMM=0 HLT=0
ES =0010   00cf9300 DPL=0 DS   [-WA]
CS =0008   00cf9b00 DPL=0 CS32 [-RA]
SS =0010   00cf9300 DPL=0 DS   [-WA]
DS =0010   00cf9300 DPL=0 DS   [-WA]
FS =0010   00cf9300 DPL=0 DS   [-WA]
GS =0010   00cf9300 DPL=0 DS   [-WA]
LDT=   8200 DPL=0 LDT
TR =   8b00 DPL=0 TSS32-busy
GDT= 000f6be8 0037
IDT= 000f6c26 
CR0=6011 CR2= CR3= CR4=
DR0= DR1= DR2= DR3= 
DR6=0ff0 DR7=0400
CCS= CCD= CCO=ADDB
EFER=
FCW=037f FSW= [ST=0] FTW=00 MXCSR=1f80
FPR0=  FPR1= 
FPR2=  FPR3= 
FPR4=  FPR5= 
FPR6=  FPR7= 
XMM00= XMM01=
XMM02= XMM03=
XMM04= XMM05=
XMM06= XMM07=
Aborted


# qemu-system-x86_64 -machine isapc
VNC server running on `::1:5900'
qemu: fatal: Trying to execute code outside RAM or ROM at 0x1a0dff44

EAX=000f0f88 EBX=0010 ECX=07fc EDX=002c
ESI=6f5c EDI=0800 EBP=07fc ESP=fffe0014
EIP=1a0dff44 EFL=0046 [---Z-P-] CPL=0 II=0 A20=1 SMM=0 HLT=0
ES =0010   00cf9300 DPL=0 DS   [-WA]
CS =0008   00cf9b00 DPL=0 CS32 [-RA]
SS =0010   00cf9300 DPL=0 DS   [-WA]
DS =0010   00cf9300 DPL=0 DS   [-WA]
FS =0010   00cf9300 DPL=0 DS   [-WA]
GS =0010   00cf9300 DPL=0 DS   [-WA]
LDT=   8200 DPL=0 LDT
TR =   8b00 DPL=0 TSS32-busy
GDT= 000f6be8 0037
IDT= 000f6c26 
CR0=6011 CR2= CR3= CR4=
DR0= DR1= DR2= 
DR3= 
DR6=0ff0 DR7=0400
CCS= CCD= CCO=ADDB
EFER=
FCW=037f FSW= [ST=0] FTW=00 MXCSR=1f80
FPR0=  FPR1= 
FPR2=  FPR3= 
FPR4=  FPR5= 
FPR6=  FPR7= 
XMM00= XMM01=
XMM02= XMM03=
XMM04= XMM05=
XMM06= XMM07=
Aborted

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

Title:
  Fatal error when running with '-machine isapc' on 2.1.2

Status in QEMU:
  New

Bug description:
  all I have are the traces, should hopefully be easy to reproduce.

  # qemu-system-i386 -machine isapc
  VNC server running on `::1:5900'
  qemu: fatal: Trying to execute code outside RAM or ROM at 0x1a0dff44

  EAX=000f0f88 EBX=0010 ECX=07fc EDX=002c
  ESI=6f5c EDI=0800 EBP=07fc ESP=fffe0014
  EIP=1a0dff44 EFL=0046 [---Z-P-] CPL=0 II=0 A20=1 SMM=0 HLT=0
  ES =0010   00cf9300 DPL=0 DS   [-WA]
  CS =0008   00cf9b00 DPL=0 CS32 [-RA]
  SS =0010   00cf9300 DPL=0 DS   [-WA]
  DS =0010   00cf9300 DPL=0 DS   [-WA]
  FS =0010   00cf9300 DPL=0 DS   [-WA]
  GS =0010   00cf9300 DPL=0 DS   [-WA]
  LDT=   8200 DPL=0 LDT
  TR =   8b00 DPL=0 TSS32-busy
  GDT= 000f6be8 0037
  IDT= 000f6c26 
  CR0=6011 CR2= CR3= CR4=
  DR0= DR1= DR2= DR3= 
  DR6=0ff0 DR7=0400
  CCS= CCD= CCO=ADDB
  EFER=
  FCW=037f FSW= [ST=0] FTW=00 MXCSR=1f80
  FPR0=  FPR1= 
  FPR2=  FPR3= 
  FPR4=  FPR5= 
  FPR6=  FPR7= 
  XMM00= 

Re: [Qemu-devel] [PATCH v2] arm: add fw_cfg to virt board

2014-12-09 Thread Peter Maydell
On 9 December 2014 at 15:41, Laszlo Ersek ler...@redhat.com wrote:
 Again, this was the idea that Rich had in 2010 (see the links in the
 discussion thus far). It was rejected back then (which is why I didn't
 even try to resurrect it now), and Peter has now asked if anything has
 changed that would make that approach more acceptable now.

 Avi and Gleb not hanging around so they can't block the approach now
 might or might not be such a change; I don't know. But, I tend to trust
 their opinion, even if dates back to 2010 in this case.

Yeah, this is about my point of view. A direct-write-to-memory
fw_cfg doesn't seem too unreasonable to me, but it would be nice
to see a solidly argued case from its proponents to counterbalance
the previous rejection.

 Anyway I'm certainly not opposed to performance, so if someone can
 thoroughly refute everything they said in that thread, go ahead.

As far as I can tell the main line of argument was if you're
using fw_cfg to transfer huge amounts of data you're doing
something wrong.

Is ARM much higher overhead than x86 for these accesses?

(Added Alex on cc as one of the few people from the previous
round of argument who's still active...)

-- PMM



Re: [Qemu-devel] [PATCH v9 02/10] qmp: Add block-dirty-bitmap-add and block-dirty-bitmap-remove

2014-12-09 Thread Stefan Hajnoczi
On Mon, Dec 01, 2014 at 03:30:08PM -0500, John Snow wrote:
 diff --git a/block.c b/block.c
 index e5c6ccf..3f27519 100644
 --- a/block.c
 +++ b/block.c
 @@ -5420,6 +5420,25 @@ int bdrv_get_dirty(BlockDriverState *bs, 
 BdrvDirtyBitmap *bitmap, int64_t sector
  }
  }
  
 +#define BDB_MIN_DEF_GRANULARITY 4096
 +#define BDB_MAX_DEF_GRANULARITY 65536
 +#define BDB_DEFAULT_GRANULARITY BDB_MAX_DEF_GRANULARITY
 +
 +uint64_t bdrv_dbm_calc_def_granularity(BlockDriverState *bs)

Long names are unwieldy but introducing multiple abbreviations is not a
good solution, it makes the code more confusing (BDB vs dbm).

I would call the function bdrv_get_default_bitmap_granularity().

The constants weren't necessary since the point of this function is to
capture the default value.  No one else should use the constants -
otherwise there is a high probability that they are reimplementing this
logic.  I would just leave them as literals in the code.

 diff --git a/blockdev.c b/blockdev.c
 index 5651a8e..4d30b09 100644
 --- a/blockdev.c
 +++ b/blockdev.c
 @@ -1894,6 +1894,60 @@ void qmp_block_set_io_throttle(const char *device, 
 int64_t bps, int64_t bps_rd,
  aio_context_release(aio_context);
  }
  
 +void qmp_block_dirty_bitmap_add(const char *device, const char *name,
 +bool has_granularity, int64_t granularity,
 +Error **errp)
 +{
 +BlockDriverState *bs;
 +
 +bs = bdrv_lookup_bs(device, NULL, errp);

Markus: I think we need to support node-name here so dirty bitmaps can
be applied at any node in the graph?

 +if (!bs) {
 +return;
 +}

These new monitor commands need to acquire/release the
BlockDriverState's AioContext like the other monitor commands.  This
ensures that BDS accesses in other threads are synchronized (stopped
while we run).

AioContext *aio_context;
aio_context = bdrv_get_aio_context(bs);
aio_context_acquire(aio_context);
...
out:
aio_context_release(aio_context);


pgp__INvPK1Kj.pgp
Description: PGP signature


[Qemu-devel] [Bug 1400768] Re: Fatal error when running with '-machine isapc' on 2.1.2

2014-12-09 Thread #1
Hello,

I too have the same results.
Below is an additional snippet where the call is made through valgrind.

While valgrind grinds, the SDL window displays Guest has not
initialized the display (yet).

==16648== Memcheck, a memory error detector
==16648== Copyright (C) 2002-2013, and GNU GPL'd, by Julian Seward et al.
==16648== Using Valgrind-3.10.1 and LibVEX; rerun with -h for copyright info

==16648== Command: qemu-system-i386 -machine isapc -fda bootdisk.img

==16648==   

qemu: fatal: Trying to execute code outside RAM or ROM at 0x1a0dff44



EAX=000f0f88 EBX=0010 ECX=07fc EDX=002c 

ESI=6f5c EDI=0800 EBP=07fc ESP=fffe0014 

EIP=1a0dff44 EFL=0046 [---Z-P-] CPL=0 II=0 A20=1 SMM=0 HLT=0

ES =0010   00cf9300 DPL=0 DS   [-WA]

CS =0008   00cf9b00 DPL=0 CS32 [-RA]

SS =0010   00cf9300 DPL=0 DS   [-WA]

DS =0010   00cf9300 DPL=0 DS   [-WA]

FS =0010   00cf9300 DPL=0 DS   [-WA]

GS =0010   00cf9300 DPL=0 DS   [-WA]

LDT=   8200 DPL=0 LDT   

TR =   8b00 DPL=0 TSS32-busy

GDT= 000f6be8 0037  

IDT= 000f6c26   

CR0=6011 CR2= CR3= CR4= 

DR0= DR1= DR2= DR3= 

DR6=0ff0 DR7=0400   

CCS= CCD= CCO=ADDB  

EFER=   

FCW=037f FSW= [ST=0] FTW=00 MXCSR=1f80
FPR0=  FPR1= 
FPR2=  FPR3= 
FPR4=  FPR5= 
FPR6=  FPR7= 
XMM00= XMM01=
XMM02= XMM03=
XMM04= XMM05=
XMM06= XMM07=
==16648== 
==16648== HEAP SUMMARY:
==16648== in use at exit: 36,820,878 bytes in 32,195 blocks
==16648==   total heap usage: 198,107 allocs, 165,912 frees, 1,828,399,692 
bytes allocated
==16648== 
==16648== LEAK SUMMARY:
==16648==definitely lost: 8,462 bytes in 29 blocks
==16648==indirectly lost: 55,605 bytes in 1,550 blocks
==16648==  possibly lost: 316,286 bytes in 773 blocks
==16648==still reachable: 36,304,789 bytes in 29,208 blocks
==16648== suppressed: 0 bytes in 0 blocks
==16648== Rerun with --leak-check=full to see details of leaked memory
==16648== 
==16648== For counts of detected and suppressed errors, rerun with: -v
==16648== ERROR SUMMARY: 0 errors from 0 contexts (suppressed: 0 from 0)
Killed

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

Title:
  Fatal error when running with '-machine isapc' on 2.1.2

Status in QEMU:
  New

Bug description:
  

Re: [Qemu-devel] [PATCH v9 03/10] block: Introduce bdrv_dirty_bitmap_granularity()

2014-12-09 Thread Stefan Hajnoczi
On Mon, Dec 01, 2014 at 03:30:09PM -0500, John Snow wrote:
 diff --git a/include/block/block.h b/include/block/block.h
 index 066ded6..f180f93 100644
 --- a/include/block/block.h
 +++ b/include/block/block.h
 @@ -440,6 +440,8 @@ void bdrv_dirty_bitmap_make_anon(BlockDriverState *bs, 
 BdrvDirtyBitmap *bitmap);
  void bdrv_release_dirty_bitmap(BlockDriverState *bs, BdrvDirtyBitmap 
 *bitmap);
  BlockDirtyInfoList *bdrv_query_dirty_bitmaps(BlockDriverState *bs);
  uint64_t bdrv_dbm_calc_def_granularity(BlockDriverState *bs);
 +int64_t bdrv_dirty_bitmap_granularity(BlockDriverState *bs,
 +  BdrvDirtyBitmap *bitmap);

This looks weird now.  The default granularity has uint64_t type but the
actual granularity has int64_t.

Please make them consistent.


pgpHmbIpEuyvW.pgp
Description: PGP signature


[Qemu-devel] [PATCH V9 0/3] Virtual Machine Generation ID

2014-12-09 Thread Gal Hammer
Hi,

Resending patches after the release of version 2.2.

Please note that this patch set doesn't include the *.hex.generated
files and the binary ACPI tables (make check will fail).

Thanks,

Gal.

V9 - Add a unit test.
   - Rebased to version 2.2.
   - Removed hex.generated the binary files from patch.

V8 - Add a device's description file.
   - GUID is stored in fw cfg file and the guest writes the
 physical address to the device (reduces vmexits).

V7 - Move the device's description back to the static SSDT table.
   - The GUID is store in a hard coded physical address and not
 in the ACPI table itself.
   - ACPI notification is triggered when the GUID is changed.

V6 - include the pre-compiled ASL file
   - remove an empty line at end of files.

V5 - Move device's description to SSDT table (dynamic).

V4 - Fix a typo in error message string.
   - Move device's description from DSDT back to SSDT table.

V3 - Remove -uuid command line parameter.
   - Move device's description from SSDT to DSDT table.
   - Add new vmgenid sysbus device.

Gal Hammer (3):
  docs: vm generation id device's description
  i386: Add a Virtual Machine Generation ID device
  tests: add a unit test for the vmgenid device.

 default-configs/i386-softmmu.mak |  1 +
 default-configs/x86_64-softmmu.mak   |  1 +
 docs/specs/vmgenid.txt   | 27 
 hw/acpi/core.c   |  8 ++
 hw/acpi/ich9.c   |  8 ++
 hw/acpi/piix4.c  |  8 ++
 hw/i386/acpi-build.c | 26 +++
 hw/i386/acpi-dsdt.dsl|  4 ++-
 hw/i386/pc.c |  8 ++
 hw/i386/q35-acpi-dsdt.dsl|  5 +++-
 hw/i386/ssdt-misc.dsl| 43 
 hw/isa/lpc_ich9.c|  1 +
 hw/misc/Makefile.objs|  1 +
 include/hw/acpi/acpi.h   |  2 ++
 include/hw/acpi/acpi_dev_interface.h |  4 +++
 include/hw/acpi/ich9.h   |  2 ++
 include/hw/i386/pc.h |  3 +++
 tests/Makefile   |  2 ++
 tests/vmgenid-test.c | 48 
 19 files changed, 200 insertions(+), 2 deletions(-)
 create mode 100644 docs/specs/vmgenid.txt
 create mode 100644 tests/vmgenid-test.c

-- 
1.9.3




[Qemu-devel] [PATCH V9 2/3] i386: Add a Virtual Machine Generation ID device

2014-12-09 Thread Gal Hammer
Based on Microsoft's sepecifications (paper can be dowloaded from
http://go.microsoft.com/fwlink/?LinkId=260709), add a device
description to the SSDT ACPI table and its implementation.

The GUID is set using a global vmgenid.uuid parameter.

Signed-off-by: Gal Hammer gham...@redhat.com

---
 default-configs/i386-softmmu.mak |  1 +
 default-configs/x86_64-softmmu.mak   |  1 +
 hw/acpi/core.c   |  8 +++
 hw/acpi/ich9.c   |  8 +++
 hw/acpi/piix4.c  |  8 +++
 hw/i386/acpi-build.c | 26 ++
 hw/i386/acpi-dsdt.dsl|  4 +++-
 hw/i386/pc.c |  8 +++
 hw/i386/q35-acpi-dsdt.dsl|  5 -
 hw/i386/ssdt-misc.dsl| 43 
 hw/isa/lpc_ich9.c|  1 +
 hw/misc/Makefile.objs|  1 +
 include/hw/acpi/acpi.h   |  2 ++
 include/hw/acpi/acpi_dev_interface.h |  4 
 include/hw/acpi/ich9.h   |  2 ++
 include/hw/i386/pc.h |  3 +++
 16 files changed, 123 insertions(+), 2 deletions(-)

diff --git a/default-configs/i386-softmmu.mak b/default-configs/i386-softmmu.mak
index 8e08841..bd33c75 100644
--- a/default-configs/i386-softmmu.mak
+++ b/default-configs/i386-softmmu.mak
@@ -45,3 +45,4 @@ CONFIG_IOAPIC=y
 CONFIG_ICC_BUS=y
 CONFIG_PVPANIC=y
 CONFIG_MEM_HOTPLUG=y
+CONFIG_VMGENID=y
diff --git a/default-configs/x86_64-softmmu.mak 
b/default-configs/x86_64-softmmu.mak
index 66557ac..006fc7c 100644
--- a/default-configs/x86_64-softmmu.mak
+++ b/default-configs/x86_64-softmmu.mak
@@ -45,3 +45,4 @@ CONFIG_IOAPIC=y
 CONFIG_ICC_BUS=y
 CONFIG_PVPANIC=y
 CONFIG_MEM_HOTPLUG=y
+CONFIG_VMGENID=y
diff --git a/hw/acpi/core.c b/hw/acpi/core.c
index 51913d6..d4597c6 100644
--- a/hw/acpi/core.c
+++ b/hw/acpi/core.c
@@ -28,6 +28,8 @@
 #include qapi-visit.h
 #include qapi-event.h
 
+#define ACPI_VM_GENERATION_ID_CHANGED_STATUS 1
+
 struct acpi_table_header {
 uint16_t _length; /* our length, not actual part of the hdr */
   /* allows easier parsing for fw_cfg clients */
@@ -683,3 +685,9 @@ void acpi_update_sci(ACPIREGS *regs, qemu_irq irq)
(regs-pm1.evt.en  ACPI_BITMASK_TIMER_ENABLE) 
!(pm1a_sts  ACPI_BITMASK_TIMER_STATUS));
 }
+
+void acpi_vm_generation_id_changed(ACPIREGS *acpi_regs, qemu_irq irq)
+{
+acpi_regs-gpe.sts[0] |= ACPI_VM_GENERATION_ID_CHANGED_STATUS;
+acpi_update_sci(acpi_regs, irq);
+}
diff --git a/hw/acpi/ich9.c b/hw/acpi/ich9.c
index ea991a3..12a9387 100644
--- a/hw/acpi/ich9.c
+++ b/hw/acpi/ich9.c
@@ -307,3 +307,11 @@ void ich9_pm_ospm_status(AcpiDeviceIf *adev, 
ACPIOSTInfoList ***list)
 
 acpi_memory_ospm_status(s-pm.acpi_memory_hotplug, list);
 }
+
+void ich9_vm_generation_id_changed(AcpiDeviceIf *adev)
+{
+ICH9LPCState *s = ICH9_LPC_DEVICE(adev);
+ICH9LPCPMRegs *pm = s-pm;
+
+acpi_vm_generation_id_changed(pm-acpi_regs, pm-irq);
+}
diff --git a/hw/acpi/piix4.c b/hw/acpi/piix4.c
index 481a16c..41b6eb6 100644
--- a/hw/acpi/piix4.c
+++ b/hw/acpi/piix4.c
@@ -574,6 +574,13 @@ static void piix4_ospm_status(AcpiDeviceIf *adev, 
ACPIOSTInfoList ***list)
 acpi_memory_ospm_status(s-acpi_memory_hotplug, list);
 }
 
+static void piix4_vm_generation_id_changed(AcpiDeviceIf *adev)
+{
+PIIX4PMState *s = PIIX4_PM(adev);
+
+acpi_vm_generation_id_changed(s-ar, s-irq);
+}
+
 static Property piix4_pm_properties[] = {
 DEFINE_PROP_UINT32(smb_io_base, PIIX4PMState, smb_io_base, 0),
 DEFINE_PROP_UINT8(ACPI_PM_PROP_S3_DISABLED, PIIX4PMState, disable_s3, 0),
@@ -611,6 +618,7 @@ static void piix4_pm_class_init(ObjectClass *klass, void 
*data)
 hc-plug = piix4_device_plug_cb;
 hc-unplug_request = piix4_device_unplug_request_cb;
 adevc-ospm_status = piix4_ospm_status;
+adevc-vm_generation_id_changed = piix4_vm_generation_id_changed;
 }
 
 static const TypeInfo piix4_pm_info = {
diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
index b37a397..bb53182 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -251,6 +251,7 @@ static void acpi_get_pci_info(PcPciInfo *info)
 #define ACPI_BUILD_TABLE_FILE etc/acpi/tables
 #define ACPI_BUILD_RSDP_FILE etc/acpi/rsdp
 #define ACPI_BUILD_TPMLOG_FILE etc/tpm/log
+#define ACPI_BUILD_VMGENID_FILE etc/vm-generation-id
 
 static void
 build_header(GArray *linker, GArray *table_data,
@@ -1062,6 +1063,8 @@ build_ssdt(GArray *table_data, GArray *linker,
 {
 MachineState *machine = MACHINE(qdev_get_machine());
 uint32_t nr_mem = machine-ram_slots;
+uint32_t vm_gid_physical_address;
+uint32_t vm_gid_offset = 0;
 unsigned acpi_cpus = guest_info-apic_id_limit;
 int ssdt_start = table_data-len;
 uint8_t *ssdt_ptr;
@@ -1090,6 +1093,21 @@ build_ssdt(GArray *table_data, GArray *linker,
 ACPI_BUILD_SET_LE(ssdt_ptr, sizeof(ssdp_misc_aml),
   

[Qemu-devel] [PATCH V9 3/3] tests: add a unit test for the vmgenid device.

2014-12-09 Thread Gal Hammer
Signed-off-by: Gal Hammer gham...@redhat.com

---
 tests/Makefile   |  2 ++
 tests/vmgenid-test.c | 48 
 2 files changed, 50 insertions(+)
 create mode 100644 tests/vmgenid-test.c

diff --git a/tests/Makefile b/tests/Makefile
index 16f0e4c..612441a 100644
--- a/tests/Makefile
+++ b/tests/Makefile
@@ -64,6 +64,7 @@ gcov-files-check-qom-interface-y = qom/object.c
 check-unit-$(CONFIG_POSIX) += tests/test-vmstate$(EXESUF)
 check-unit-y += tests/test-qemu-opts$(EXESUF)
 gcov-files-test-qemu-opts-y = qom/test-qemu-opts.c
+check-qtest-i386-y += tests/vmgenid-test$(EXESUF)
 
 check-block-$(CONFIG_POSIX) += tests/qemu-iotests-quick.sh
 
@@ -351,6 +352,7 @@ tests/usb-hcd-xhci-test$(EXESUF): tests/usb-hcd-xhci-test.o 
$(libqos-usb-obj-y)
 tests/vhost-user-test$(EXESUF): tests/vhost-user-test.o qemu-char.o 
qemu-timer.o $(qtest-obj-y)
 tests/qemu-iotests/socket_scm_helper$(EXESUF): 
tests/qemu-iotests/socket_scm_helper.o
 tests/test-qemu-opts$(EXESUF): tests/test-qemu-opts.o libqemuutil.a 
libqemustub.a
+tests/vmgenid-test$(EXESUF): tests/vmgenid-test.o
 
 ifeq ($(CONFIG_POSIX),y)
 LIBS += -lutil
diff --git a/tests/vmgenid-test.c b/tests/vmgenid-test.c
new file mode 100644
index 000..d9c3e29
--- /dev/null
+++ b/tests/vmgenid-test.c
@@ -0,0 +1,48 @@
+/*
+ * QTest testcase for VM Generation ID
+ *
+ * Copyright (c) 2014 Red Hat, Inc.
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ */
+
+#include string.h
+#include libqtest.h
+
+static void vmgenid_test(void)
+{
+static const uint8_t expected[16] = {
+0x32, 0x4e, 0x6e, 0xaf, 0xd1, 0xd1, 0x4b, 0xf6,
+0xbf, 0x41, 0xb9, 0xbb, 0x6c, 0x91, 0xfb, 0x87
+};
+uint8_t guid[16];
+uint32_t i;
+
+// Emulate the ACPI _INI method (tells the device where the physical
+// memory was allocated.
+writel(0xfedf, 0x0001f000);
+   
+// Skip the ACPI ADDR method and read the GUID directly from memory.
+for (i = 0; i  16; i++) {
+guid[i] = readb(0x0001f000 + i);
+} 
+
+g_assert_cmpuint(sizeof(guid), ==, sizeof(expected));
+g_assert(memcmp(guid, expected, sizeof(guid)) == 0);
+}
+
+int main(int argc, char **argv)
+{
+int ret;
+
+g_test_init(argc, argv, NULL);
+qtest_add_func(/vmgenid/vmgenid, vmgenid_test);
+
+qtest_start(-global vmgenid.uuid=324e6eaf-d1d1-4bf6-bf41-b9bb6c91fb87);
+ret = g_test_run();
+
+qtest_end();
+
+return ret;
+}
-- 
1.9.3




[Qemu-devel] [PATCH V9 1/3] docs: vm generation id device's description

2014-12-09 Thread Gal Hammer
Signed-off-by: Gal Hammer gham...@redhat.com

---
 docs/specs/vmgenid.txt | 27 +++
 1 file changed, 27 insertions(+)
 create mode 100644 docs/specs/vmgenid.txt

diff --git a/docs/specs/vmgenid.txt b/docs/specs/vmgenid.txt
new file mode 100644
index 000..9a09d11
--- /dev/null
+++ b/docs/specs/vmgenid.txt
@@ -0,0 +1,27 @@
+VIRTUAL MACHINE GENERATION ID
+=
+
+The VM generation ID (vmgenid) device is an emulated device which
+expose a 128-bit, cryptographically random, integer value identifier.
+This allows management applications (e.g. libvirt) to notify the guest
+operating system when the virtual machine is executed with a different
+configuration (e.g. snapshot execution or creation from a template).
+
+Specs is on the web at: http://go.microsoft.com/fwlink/?LinkId=260709
+
+---
+
+The vmgenid device is a sysbus device with the following ACPI ID:
+QEMU0002.
+
+The device adds a vmgenid.uuid property, which can be modifed using
+the -global command line argument or the QMP interface.
+
+The device uses a fixed memory resource: 0xfedf-0xfedf0003. The
+guest is expected to write the physical address of the GUID's buffer
+to that memory resource. This allows the device to modify the GUID if
+requested by the management application.
+
+According to the specification, any change to the GUID executes an
+ACPI notification. The vmgenid device triggers the GPE._E00 which
+executes the ACPI Notify operation.
-- 
1.9.3




[Qemu-devel] [PATCH 2/4] hw/virtio-blk: add a constant for max number of merged requests

2014-12-09 Thread Peter Lieven
As it was not obvious (at least for me) where the 32 comes from;
add a constant for it.

Signed-off-by: Peter Lieven p...@kamp.de
Reviewed-by: Eric Blake ebl...@redhat.com
---
 hw/block/virtio-blk.c  |2 +-
 include/hw/virtio/virtio-blk.h |4 +++-
 2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c
index b19b102..490f961 100644
--- a/hw/block/virtio-blk.c
+++ b/hw/block/virtio-blk.c
@@ -326,7 +326,7 @@ static void virtio_blk_handle_write(VirtIOBlockReq *req, 
MultiReqBuffer *mrb)
 block_acct_start(blk_get_stats(req-dev-blk), req-acct, req-qiov.size,
  BLOCK_ACCT_WRITE);
 
-if (mrb-num_writes == 32) {
+if (mrb-num_writes == VIRTIO_BLK_MAX_MERGE_REQS) {
 virtio_submit_multiwrite(req-dev-blk, mrb);
 }
 
diff --git a/include/hw/virtio/virtio-blk.h b/include/hw/virtio/virtio-blk.h
index 3979dc4..3f2652f 100644
--- a/include/hw/virtio/virtio-blk.h
+++ b/include/hw/virtio/virtio-blk.h
@@ -134,8 +134,10 @@ typedef struct VirtIOBlock {
 struct VirtIOBlockDataPlane *dataplane;
 } VirtIOBlock;
 
+#define VIRTIO_BLK_MAX_MERGE_REQS 32
+
 typedef struct MultiReqBuffer {
-BlockRequestblkreq[32];
+BlockRequestblkreq[VIRTIO_BLK_MAX_MERGE_REQS];
 unsigned intnum_writes;
 } MultiReqBuffer;
 
-- 
1.7.9.5




[Qemu-devel] [PATCH 4/4] virtio-blk: introduce multiread

2014-12-09 Thread Peter Lieven
this patch finally introduces multiread support to virtio-blk. While
multiwrite support was there for a long time, read support was missing.

To achieve this the patch does several things which might need further
explanation:

 - the whole merge and multireq logic is moved from block.c into
   virtio-blk. This is move is a preparation for directly creating a
   coroutine out of virtio-blk.

 - requests are only merged if they are strictly sequential, and no
   longer sorted. This simplification decreases overhead and reduces
   latency. It will also merge some requests which were unmergable before.

   The old algorithm took up to 32 requests, sorted them and tried to merge
   them. The outcome was anything between 1 and 32 requests. In case of
   32 requests there were 31 requests unnecessarily delayed.

   On the other hand let's imagine e.g. 16 unmergeable requests followed
   by 32 mergable requests. The latter 32 requests would have been split
   into two 16 byte requests.

   Last the simplified logic allows for a fast path if we have only a
   single request in the multirequest. In this case the request is sent as
   ordinary request without multireq callbacks.

As a first benchmark I installed Ubuntu 14.04.1 on a local SSD. The number of
merged requests is in the same order while the write latency is obviously
decreased by several percent.

cmdline:
qemu-system-x86_64 -m 1024 -smp 2 -enable-kvm -cdrom 
ubuntu-14.04.1-server-amd64.iso \
 -drive if=virtio,file=/dev/ssd/ubuntu1404,aio=native,cache=none -monitor stdio

Before:
virtio0:
 rd_bytes=151056896 wr_bytes=2683947008 rd_operations=18614 wr_operations=67979
 flush_operations=15335 wr_total_time_ns=540428034217 
rd_total_time_ns=0520068
 flush_total_time_ns=40673685006 rd_merged=0 wr_merged=15531

After:
virtio0:
 rd_bytes=149487104 wr_bytes=2701344768 rd_operations=18148 wr_operations=68578
 flush_operations=15368 wr_total_time_ns=437030089565 
rd_total_time_ns=9836288815
 flush_total_time_ns=40597981121 rd_merged=690 wr_merged=14615

Some first numbers of improved read performance while booting:

The Ubuntu 14.04.1 vServer from above:
virtio0:
 rd_bytes=97545216 wr_bytes=119808 rd_operations=5071 wr_operations=26
 flush_operations=2 wr_total_time_ns=8847669 rd_total_time_ns=13952575478
 flush_total_time_ns=3075496 rd_merged=742 wr_merged=0

Windows 2012R2 (booted from iSCSI):
virtio0: rd_bytes=176559104 wr_bytes=61859840 rd_operations=7200 
wr_operations=360
 flush_operations=68 wr_total_time_ns=34344992718 rd_total_time_ns=134386844669
 flush_total_time_ns=18115517 rd_merged=641 wr_merged=216

Signed-off-by: Peter Lieven p...@kamp.de
---
 hw/block/dataplane/virtio-blk.c |6 +-
 hw/block/virtio-blk.c   |  239 ---
 include/hw/virtio/virtio-blk.h  |   22 ++--
 trace-events|1 +
 4 files changed, 162 insertions(+), 106 deletions(-)

diff --git a/hw/block/dataplane/virtio-blk.c b/hw/block/dataplane/virtio-blk.c
index 1222a37..30bf2e7 100644
--- a/hw/block/dataplane/virtio-blk.c
+++ b/hw/block/dataplane/virtio-blk.c
@@ -96,9 +96,7 @@ static void handle_notify(EventNotifier *e)
 event_notifier_test_and_clear(s-host_notifier);
 blk_io_plug(s-conf-conf.blk);
 for (;;) {
-MultiReqBuffer mrb = {
-.num_writes = 0,
-};
+MultiReqBuffer mrb = {};
 int ret;
 
 /* Disable guest-host notifies to avoid unnecessary vmexits */
@@ -120,7 +118,7 @@ static void handle_notify(EventNotifier *e)
 virtio_blk_handle_request(req, mrb);
 }
 
-virtio_submit_multiwrite(s-conf-conf.blk, mrb);
+virtio_submit_multireq(s-conf-conf.blk, mrb);
 
 if (likely(ret == -EAGAIN)) { /* vring emptied */
 /* Re-enable guest-host notifies and stop processing the vring.
diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c
index 490f961..cc0076a 100644
--- a/hw/block/virtio-blk.c
+++ b/hw/block/virtio-blk.c
@@ -34,6 +34,8 @@ VirtIOBlockReq *virtio_blk_alloc_request(VirtIOBlock *s)
 req-dev = s;
 req-qiov.size = 0;
 req-next = NULL;
+req-mr_next = NULL;
+req-mr_qiov.nalloc = 0;
 return req;
 }
 
@@ -84,20 +86,29 @@ static int virtio_blk_handle_rw_error(VirtIOBlockReq *req, 
int error,
 
 static void virtio_blk_rw_complete(void *opaque, int ret)
 {
-VirtIOBlockReq *req = opaque;
+VirtIOBlockReq *next = opaque;
 
-trace_virtio_blk_rw_complete(req, ret);
+while (next) {
+VirtIOBlockReq *req = next;
+next = req-mr_next;
+trace_virtio_blk_rw_complete(req, ret);
 
-if (ret) {
-int p = virtio_ldl_p(VIRTIO_DEVICE(req-dev), req-out.type);
-bool is_read = !(p  VIRTIO_BLK_T_OUT);
-if (virtio_blk_handle_rw_error(req, -ret, is_read))
-return;
-}
+if (req-mr_qiov.nalloc) {
+qemu_iovec_destroy(req-mr_qiov);
+}
 
-virtio_blk_req_complete(req, VIRTIO_BLK_S_OK);
-

  1   2   >