[Qemu-devel] question about vioscsi queue depth patch

[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

[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

[Peter Maydell]

On 9 December 2014 at 01:54, Maciej W. Rozycki  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

[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

[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

[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()

[Markus Armbruster]

 writes:

> From: Gonglei 
>
> 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 
> Signed-off-by: Gonglei 

Reviewed-by: Markus Armbruster 

Re: [Qemu-devel] Redundant VDE network

[Stefan Hajnoczi]

On Tue, Dec 9, 2014 at 6:29 AM, 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?

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

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

[Fabian Greffrath]

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. ;)

Cheers,

Fabian

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

[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 
d

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

[Stefan Hajnoczi]

On Tue, Dec 9, 2014 at 3:52 AM, Josh Durgin  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

[Stefan Hajnoczi]

On Tue, Dec 9, 2014 at 9:44 AM, Fabian Greffrath  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

[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

[Igor Mammedov]

On Mon, 8 Dec 2014 22:57:05 +0200
"Michael S. Tsirkin"  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 
> 
> 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

[Igor Mammedov]

On Mon, 8 Dec 2014 21:13:25 +0200
"Michael S. Tsirkin"  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 
> 
> 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

[Igor Mammedov]

On Mon, 8 Dec 2014 23:03:02 +0200
"Michael S. Tsirkin"  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 
> 
> 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

[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

[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 
Signed-off-by: Eric Auger 

---
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 
+ *  Eric Auger 
+ *
+ * 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 .
+ *
+ */
+
+#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);
+} NodeCreationPair;
+
+/* list of supported dynamic sysbus devices

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

[Eric Auger]

On 12/05/2014 05:36 PM, Peter Maydell wrote:
> On 30 November 2014 at 18:19, Eric Auger  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 
>> Signed-off-by: Eric Auger 
>>
>> ---
>> 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

[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 

---

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

[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 
Signed-off-by: Eric Auger 

---
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, 30ULL * 1024 * 1024 

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

[Igor Mammedov]

On Mon, 8 Dec 2014 23:15:32 +0200
"Michael S. Tsirkin"  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 
> 
> 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

[Eric Auger]

On 12/05/2014 07:16 PM, Peter Maydell wrote:
> On 5 December 2014 at 16:38, Peter Maydell  wrote:
>> On 30 November 2014 at 18:19, Eric Auger  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

[Eric Auger]

On 12/05/2014 05:40 PM, Peter Maydell wrote:
> On 30 November 2014 at 18:19, Eric Auger  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 
>> + *  Eric Auger 
>> + *
>> + * 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 
>> + *  Eric Auger 
>> + *
>> + * 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

[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"  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 
> > 
> > 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

[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"  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 
> > 
> > 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

[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

[Igor Mammedov]

On Mon, 8 Dec 2014 22:43:40 +0200
"Michael S. Tsirkin"  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 
> 
> 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 
> > +#include 
> > +#include 
> > +#include 
> > +#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

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

[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"  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 
> > 
> > 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

[Igor Mammedov]

On Mon, 8 Dec 2014 22:21:20 +0200
"Michael S. Tsirkin"  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 
> 
> 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

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

[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"

[Peter Maydell]

On 9 December 2014 at 10:50, 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;

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

[Igor Mammedov]

On Mon, 8 Dec 2014 22:54:03 +0200
"Michael S. Tsirkin"  wrote:

> On Mon, Dec 08, 2014 at 04:08:06PM +, Igor Mammedov wrote:
> > Signed-off-by: Igor Mammedov 
> 
> 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 
> >  #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

[Pranavkumar Sawargaonkar]

Hi PMM,

On 5 December 2014 at 19:41, Peter Maydell  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"

[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"

[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  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.

[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"

[Peter Maydell]

On 9 December 2014 at 11:25, Richard W.M. Jones  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

[Peter Maydell]

On 9 December 2014 at 11:07, Pranavkumar Sawargaonkar
 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

[Igor Mammedov]

On Tue, 9 Dec 2014 12:34:02 +0200
"Michael S. Tsirkin"  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"  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 
> > > 
> > > 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

[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"  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 
> > 
> > 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(meth

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

[Dr. David Alan Gilbert (git)]

From: "Dr. David Alan Gilbert" 

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 
---
 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 b/migration/xbzrle.c
similari

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

[Dr. David Alan Gilbert (git)]

From: "Dr. David Alan Gilbert" 

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

Signed-off-by: Dr. David Alan Gilbert 
---
 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"

[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  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

[Dr. David Alan Gilbert (git)]

From: "Dr. David Alan Gilbert" 

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

[Dr. David Alan Gilbert (git)]

From: "Dr. David Alan Gilbert" 

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 
---
 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

[Dr. David Alan Gilbert (git)]

From: "Dr. David Alan Gilbert" 

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 
---
 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

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

[Maciej W. Rozycki]

On Tue, 9 Dec 2014, Peter Maydell wrote:

> >  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...

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

  Maciej

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

[Peter Maydell]

On 9 December 2014 at 12:28, Maciej W. Rozycki  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

[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"  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"  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 
> > > > 
> > > > 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

[Igor Mammedov]

On Tue, 9 Dec 2014 12:38:12 +0200
"Michael S. Tsirkin"  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"  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 
> > > 
> > > 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,

Re: [Qemu-devel] Redundant VDE network

[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

[Igor Mammedov]

On Tue, 9 Dec 2014 14:02:17 +0200
"Michael S. Tsirkin"  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"  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 
> > > 
> > > 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 87

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

[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"  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"  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"  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 
> > > > > > 
> > > > > > 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

[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"

[Dr. David Alan Gilbert]

* Peter Maydell (peter.mayd...@linaro.org) wrote:
> On 9 December 2014 at 11:25, Richard W.M. Jones  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

[Cornelia Huck]

On Fri,  5 Dec 2014 10:19:58 +0100
Frank Blaschka  wrote:

> From: Frank Blaschka 
> 
> 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 
> ---
>  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

[Cornelia Huck]

On Fri,  5 Dec 2014 10:19:57 +0100
Frank Blaschka  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

[Cornelia Huck]

On Mon,  8 Dec 2014 09:18:59 +0100
Frank Blaschka  wrote:

> Remove all unnecessary calls to cpu_synchronize_state
> 
> Signed-off-by: Frank Blaschka 
> ---
>  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

[Juan Quintela]

Alexander Spyridakis  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

[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

[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 

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 


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"

[Peter Maydell]

On 9 December 2014 at 12:33, Dr. David Alan Gilbert  wrote:
> * Peter Maydell (peter.mayd...@linaro.org) wrote:
>> On 9 December 2014 at 11:25, Richard W.M. Jones  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

[Igor Mammedov]

On Mon, 8 Dec 2014 22:43:24 +0200
"Michael S. Tsirkin"  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 
> > ---
> >  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; /

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

[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

[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 
> ---
>  - 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

[Chrysostomos Nanakos]

Remove first email address and let the one from which I am contributing.

Signed-off-by: Chrysostomos Nanakos 
---
 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 
 M: Chrysostomos Nanakos 
 S: Maintained
 F: block/archipelago.c
-- 
1.7.10.4

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

[Igor Mammedov]

On Tue, 9 Dec 2014 14:51:59 +0200
"Michael S. Tsirkin"  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"  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"  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 
> > > > > 
> > > > > 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

[Igor Mammedov]

On Tue, 9 Dec 2014 14:51:59 +0200
"Michael S. Tsirkin"  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"  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"  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 
> > > > > 
> > > > > 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

[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 
---
 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

[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

[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 
---
 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

[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 
---
 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

[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 
---
 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 SELFT

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

[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 
---
 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

[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"  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 
> > > ---
> > >  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)
> > >  {
> > > -Acp

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

[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

[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

[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

[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()

[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 
>> Reviewed-by: Paolo Bonzini 
>> ---
>> 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

[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

[Stefan Hajnoczi]

On Tue, Nov 25, 2014 at 05:30:02PM -0600, grhookatw...@gmail.com wrote:
> From: Gary R Hook 
> 
> 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 
> 
> ---
>  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

[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 
> > --
> > 
> > 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()

[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 
> Reviewed-by: Paolo Bonzini 
> ---
> 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

[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

[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 

-- 
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

[Eric Blake]

On 12/08/2014 11:38 PM, zhanghailiang wrote:
> Signed-off-by: zhanghailiang 
> ---
>  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 

-- 
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

[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

[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 
---
  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

[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

[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= XMM01=000

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

[Peter Maydell]

On 9 December 2014 at 15:41, Laszlo Ersek  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

[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

[#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()

[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

[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

[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 

---
 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.

[Gal Hammer]

Signed-off-by: Gal Hammer 

---
 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 
+#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

[Gal Hammer]

Signed-off-by: Gal Hammer 

---
 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