Re: [Qemu-devel] [PULL 0/4] update to 35c53797 to 4e03af8

[Peter Maydell]

On 3 September 2015 at 06:02, Gerd Hoffmann  wrote:
>   Hi,
>
> Here comes the ipxe update pull.  Thanks to Stefan all bits are upstream
> now.  Also upstream has a named configuration for qemu.  That simplifies
> the whole process, I could drop some patches and so there are only 4 of
> them left, two of them being the actual ipxe update (one submodule, one
> binaries).
>
> Note that this is NOT current git master.  Michael Brown added support
> for EFI_PXE_BASE_CODE_PROTOCOL recently, and I tried to pick the last
> commit before that work started to have a stable and known-good base.
>
> I'll have a look a the new code shortly though, so stay tuned for more
> ipxe updates.
>
> cheers,
>   Gerd
>
> The following changes since commit f8b8091d2779d956011a3fb83ff60dbf7465c71d:
>
>   Merge remote-tracking branch 
> 'remotes/mdroth/tags/qga-pull-2015-09-01-v2-tag' into staging (2015-09-01 
> 19:42:43 +0100)
>
> are available in the git repository at:
>
>
>   git://git.kraxel.org/qemu tags/pull-ipxe-20150903-1
>
> for you to fetch changes up to f4798320144245da66128edb840bd940fd287d28:
>
>   ipxe: update binaries (2015-09-03 14:46:24 +0200)
>
> 
> ipxe: update to 35c53797 to 4e03af8, build tweaks.
>
> 
> Gerd Hoffmann (4):
>   ipxe: update from 35c53797 to 4e03af8
>   ipxe: don't override GITVERSION
>   ipxe: use upstream configuration
>   ipxe: update binaries
>
>  pc-bios/efi-e1000.rom  | Bin 197120 -> 196608 bytes
>  pc-bios/efi-eepro100.rom   | Bin 197632 -> 197120 bytes
>  pc-bios/efi-ne2k_pci.rom   | Bin 195584 -> 195584 bytes
>  pc-bios/efi-pcnet.rom  | Bin 195584 -> 195584 bytes
>  pc-bios/efi-rtl8139.rom| Bin 200192 -> 199168 bytes
>  pc-bios/efi-virtio.rom | Bin 194048 -> 193024 bytes
>  roms/Makefile  |  11 ---
>  roms/config.ipxe.general.h |   4 
>  roms/ipxe  |   2 +-
>  9 files changed, 5 insertions(+), 12 deletions(-)
>  delete mode 100644 roms/config.ipxe.general.h

Applied, thanks.

-- PMM

Re: [Qemu-devel] [PATCH 01/16] block: Introduce BDS.file_child

[Max Reitz]

On 22.09.2015 19:14, Max Reitz wrote:
> On 17.09.2015 15:48, Kevin Wolf wrote:
>> Store the BdrvChild for bs->file. At this point, bs->file_child->bs just
>> duplicates the bs->file pointer. Later, it will completely replace it.
>>
>> Signed-off-by: Kevin Wolf 
>> ---
>>  block.c   | 12 +---
>>  include/block/block_int.h |  1 +
>>  2 files changed, 10 insertions(+), 3 deletions(-)
> 
> Reviewed-by: Max Reitz 
> 
> Although I have a small comment below:
> 
>>
>> diff --git a/block.c b/block.c
>> index 6268e37..2e9e5e2 100644
>> --- a/block.c
>> +++ b/block.c
>> @@ -1487,11 +1487,17 @@ static int bdrv_open_inherit(BlockDriverState **pbs, 
>> const char *filename,
>>  
>>  assert(file == NULL);
> 
> This looks strange now, without the direct connection to
> bdrv_open_image(). I'd drop this...
> 
>>  bs->open_flags = flags;
>> -ret = bdrv_open_image(, filename, options, "file",
>> -  bs, _file, true, _err);
>> -if (ret < 0) {
>> +
>> +bs->file_child = bdrv_open_child(filename, options, "file", bs,
>> + _file, true, _err);
>> +if (local_err) {
>> +ret = -EINVAL;
>>  goto fail;
>>  }
>> +
>> +if (bs->file_child) {
>> +file = bs->file_child->bs;
>> +}
> 
> And make this file = bs->file_child ? bs->file_child->bs : NULL (or the
> respective long form, i.e.
> if (bs->file_child) { ... } else { file = NULL; }).
> 
> We could even put this after this if block and drop the NULL
> initialization of file, but that might go overboard.

And expecting something like this, I skipped ahead to patch 5, which
drops this altogether. An R-b without any comments it is, then. :-)

Max

>>  }
>>  
>>  /* Image format probing */
>> diff --git a/include/block/block_int.h b/include/block/block_int.h
>> index 14ad4c3..d0dd93e 100644
>> --- a/include/block/block_int.h
>> +++ b/include/block/block_int.h
>> @@ -381,6 +381,7 @@ struct BlockDriverState {
>>  BlockDriverState *backing_hd;
>>  BdrvChild *backing_child;
>>  BlockDriverState *file;
>> +BdrvChild *file_child;
>>  
>>  NotifierList close_notifiers;
>>  
>>
> 
> 




signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH 03/16] blkverify: Convert s->test_file to BdrvChild

[Max Reitz]

On 17.09.2015 15:48, Kevin Wolf wrote:
> Signed-off-by: Kevin Wolf 
> ---
>  block/blkverify.c | 41 +
>  1 file changed, 21 insertions(+), 20 deletions(-)

Reviewed-by: Max Reitz 



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH] docs: describe the QEMU build system structure / design

[Laszlo Ersek]

meta review of your review:

On 09/22/15 20:11, John Snow wrote:
> Reviewed from an en_US perspective, though I left alone things that are
> clearly regional (e.g. 'behaviour' vs 'behavior')
> 
> On 09/22/2015 12:35 PM, Daniel P. Berrange wrote:

>> + - Add information to the help output message to report on the new
>> +   feature flag.
>> +
> 
> Remove period, or add to the other list items for consistency. My
> personal preference is to use the period for any sentences with proper
> grammatical structure, omitting it for simple list items.

Then:

>> +which create binaries must include the $(EXESUF) variable on the binary
>> +name. eg
> 
> 'e.g.' here and everywhere subsequent.

Self-contradiction found!!!eleven

:)

Honestly I'm surprised (or not) how many typos you've found that I
blissfully slid over.

>> +Each system/userspace emulation target needs to have a slightly
>> +different set of make rules / variables. Thus, make will be recursively
>> +invoked for each of the emulation targets.
>> +
>> +The recursive invokation will end up processing the toplevel
> 
> invocation again.

Self-contradictory period again! :)

> Thanks for writing this!

Yes!

> Pretending to be Eric,

Yes. :)

Cheers
Laszlo

Re: [Qemu-devel] [PATCH v14 0/5] vGICv3 support

[Peter Maydell]

On 17 September 2015 at 10:27, Peter Maydell  wrote:
> On 9 September 2015 at 08:49, Pavel Fedin  wrote:
>> This series introduces support for GICv3 by KVM. Software emulation is
>> currently not supported.
>>
>> v13 => v14
>>
>> - Rebased on the latest master, fixed conflicts in hw/arm/virt.c
>>
>
> You'll need to fix the acpi related bits Shannon has review comments
> on in patch 5, but otherwise this looks good to me.

Having discussed this with Shannon yesterday, the fixes required
are trivial. Shannon kindly sent me this tiny patch, so I'm just
going to squash that into your patch 5 and put the whole lot into
target-arm.next, rather than make you have to go through another
round.


 hw/arm/virt-acpi-build.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
index 0dd7fce..59c84ff 100644
--- a/hw/arm/virt-acpi-build.c
+++ b/hw/arm/virt-acpi-build.c
@@ -454,8 +454,8 @@ build_madt(GArray *table_data, GArray *linker,
VirtGuestInfo *guest_info,

 gicr->type = ACPI_APIC_GENERIC_REDISTRIBUTOR;
 gicr->length = sizeof(*gicr);
-gicr->base_address = memmap[VIRT_GIC_REDIST].base;
-gicr->range_length = memmap[VIRT_GIC_REDIST].size;
+gicr->base_address = cpu_to_le64(memmap[VIRT_GIC_REDIST].base);
+gicr->range_length = cpu_to_le32(memmap[VIRT_GIC_REDIST].size);
 } else {
 for (i = 0; i < guest_info->smp_cpus; i++) {
 AcpiMadtGenericInterrupt *gicc = acpi_data_push(table_data,

thanks
-- PMM

Re: [Qemu-devel] [PATCH 06/16] block: Remove bdrv_open_image()

[Max Reitz]

On 17.09.2015 15:48, Kevin Wolf wrote:
> It is unused now.
> 
> Signed-off-by: Kevin Wolf 
> ---
>  block.c   | 34 --
>  include/block/block.h |  4 
>  2 files changed, 38 deletions(-)

Reviewed-by: Max Reitz 



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH 07/16] block: Convert bs->backing_hd to BdrvChild

[Max Reitz]

On 17.09.2015 15:48, Kevin Wolf wrote:
> This is the final step in converting all of the BlockDriverState
> pointers that block drivers use to BdrvChild.
> 
> After this patch, bs->children contains the full list of child nodes
> that are referenced by a given BDS, and these children are only
> referenced through BdrvChild, so that updating the pointer in there is
> enough for changing edges in the graph.
> 
> Signed-off-by: Kevin Wolf 
> ---
>  block.c   | 116 
> +-
>  block/io.c|  24 +-
>  block/mirror.c|   7 +--
>  block/qapi.c  |   8 ++--
>  block/qcow.c  |   4 +-
>  block/qcow2-cluster.c |   4 +-
>  block/qcow2.c |   6 +--
>  block/qed.c   |  12 ++---
>  block/stream.c|  10 ++--
>  block/vmdk.c  |  21 +
>  block/vvfat.c |   6 +--
>  blockdev.c|   6 +--
>  include/block/block_int.h |  12 +++--
>  qemu-img.c|   8 ++--
>  14 files changed, 130 insertions(+), 114 deletions(-)
> 

[...]

> diff --git a/block/io.c b/block/io.c
> index 8a27efa..d7e742a 100644
> --- a/block/io.c
> +++ b/block/io.c

[...]

> @@ -1604,7 +1604,7 @@ int64_t bdrv_get_block_status(BlockDriverState *bs,
>int64_t sector_num,
>int nb_sectors, int *pnum)
>  {
> -return bdrv_get_block_status_above(bs, bs->backing_hd,
> +return bdrv_get_block_status_above(bs, backing_bs(bs),
> sector_num, nb_sectors, pnum);
>  }
>  
> @@ -1662,7 +1662,7 @@ int bdrv_is_allocated_above(BlockDriverState *top,
>  n = pnum_inter;
>  }
>  
> -intermediate = intermediate->backing_hd;
> +intermediate = intermediate->backing ? intermediate->backing->bs : 
> NULL;

backing_bs(intermediate)?

>  }
>  
>  *pnum = n;
> diff --git a/block/mirror.c b/block/mirror.c
> index a258926..259e11a 100644
> --- a/block/mirror.c
> +++ b/block/mirror.c
> @@ -371,7 +371,8 @@ static void mirror_exit(BlockJob *job, void *opaque)
>  if (s->common.driver->job_type == BLOCK_JOB_TYPE_COMMIT) {
>  /* drop the bs loop chain formed by the swap: break the loop then
>   * trigger the unref from the top one */
> -BlockDriverState *p = s->base->backing_hd;
> +BlockDriverState *p = s->base->backing
> +? s->base->backing->bs : NULL;

Maybe you don't want to use backing_bs() outside of the core block
layer, but it could be used here, too.

(There are two similar expressions in block/stream.c, and maybe
elsewhere, too)

>  bdrv_set_backing_hd(s->base, NULL);
>  bdrv_unref(p);
>  }

[...]

> diff --git a/blockdev.c b/blockdev.c
> index 32b04b4..bc158ff 100644
> --- a/blockdev.c
> +++ b/blockdev.c
> @@ -2508,10 +2508,10 @@ void qmp_drive_backup(const char *device, const char 
> *target,
>  /* See if we have a backing HD we can use to create our new image
>   * on top of. */
>  if (sync == MIRROR_SYNC_MODE_TOP) {
> -source = bs->backing_hd;
> -if (!source) {
> +if (!bs->backing) {
>  sync = MIRROR_SYNC_MODE_FULL;
>  }
> +source = bs->backing->bs;

That doesn't seem right. In case of !bs->backing, this won't abort but
just continue and run into bs->backing->bs, which should therefore
probably be backing_bs(bs) instead.

>  }
>  if (sync == MIRROR_SYNC_MODE_NONE) {
>  source = bs;
> @@ -2716,7 +2716,7 @@ void qmp_drive_mirror(const char *device, const char 
> *target,
>  }
>  
>  flags = bs->open_flags | BDRV_O_RDWR;
> -source = bs->backing_hd;
> +source = bs->backing ? bs->backing->bs : NULL;

Why not source = backing_bs(bs)?

>  if (!source && sync == MIRROR_SYNC_MODE_TOP) {
>  sync = MIRROR_SYNC_MODE_FULL;
>  }

[...]

> diff --git a/qemu-img.c b/qemu-img.c
> index 6ff4e85..c4454da 100644
> --- a/qemu-img.c
> +++ b/qemu-img.c

[...]

> @@ -2206,11 +2206,11 @@ static int get_block_status(BlockDriverState *bs, 
> int64_t sector_num,
>  if (ret & (BDRV_BLOCK_ZERO|BDRV_BLOCK_DATA)) {
>  break;
>  }
> -bs = bs->backing_hd;
> -if (bs == NULL) {
> +if (bs->backing == NULL) {
>  ret = 0;
>  break;
>  }
> +bs = bs->backing->bs;

This changes behavior. bs needs to be set to NULL in the
if (bs->backing == NULL) block, or the break will break: Before, if
bs->backing_hd == NULL, the loop was left with bs == NULL. Now, bs won't
be NULL anymore (but its value is used after the loop and stored in e->bs).

Max

>  
>  depth++;
>  }
> 




signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] MAINTAINERS leaves too many files uncovered

[John Snow]

On 09/22/2015 05:13 AM, Markus Armbruster wrote:
> Paolo asked for an update.
> 
> Markus Armbruster  writes:
>>
>> Full list of unmaintained files now:
> [...]
> 

Full list edited down to 'sections' with replies in-line...

[ top level confetti/detritus ]
[ backends/* ]
[ coroutine-* ]
[ default-configs/* ]
[ disas* ]

> dma-helpers.c

Block?

[ docs/* ]

> docs/bitmaps.md

Mine, or otherwise at least 'block.'

> docs/blkdebug.txt
> docs/blkverify.txt
> docs/live-block-ops.txt
> docs/qcow2-cache.txt
> docs/specs/qcow2.txt
> docs/specs/qed_spec.txt

block?

[ fpu/* ]
[ hw/bt ]
[ hw/char ]
[ hw/core ]
[ hw/cpu ]
[ hw/display ]
[ hw/dma ]
[ hw/gpio ]
[ hw/i2c ]
[ hw/input ]
[ hw/intc ]
[ hw/ipack ]
[ hw/isa ]
[ hw/m68k ]
[ hw/misc ]
[ hw/moxie ]
[ hw/net ]
[ hw/nvram ]
[ hw/pci-bridge ]
[ hw/pci-host ]
[ hw/pcmcia ]
[ hw/sd ]
[ hw/timer ]
[ hw/tpm ]
[ hw/virtio ]
[ hw/watchdog ]
[ hw/xen ]
[ hw/xenpv ]

[ include/* ]
[ include/disas/* ]
[ include/exec/* ]
[ include/fpu/* ]
[ include/hw/{...see hw list above...} ]

[ libcacard/* ] forward to libcacard project?
[ pc-bios/* ]
[ po/* ]

> qemu-coroutine-io.c
> qemu-coroutine-lock.c
> qemu-coroutine-sleep.c
> qemu-coroutine.c

Stefan ... ?

[ roms/* ]
[ scripts/* ] -- pretty much a grab-bag of random stuff.
[ stubs/* ] -- I think we need a designated "build system" maintainer to
take this and other build-system related infrastructure pieces.

[ target-m68k/* ]
[ target-tilegx/* ]
[ tests/acpi-test-data/* ]
[ tests/* ]

> tests/fdc-test.c

Mine, as part of the FDC stanza.

> tests/libqos/ahci.c
> tests/libqos/ahci.h

Mine again, as part of the IDE stanza.

> tests/libqos/fw_cfg.c
> tests/libqos/fw_cfg.h
> tests/libqos/i2c-imx.c
> tests/libqos/i2c-omap.c
> tests/libqos/i2c.c
> tests/libqos/i2c.h
> tests/libqos/libqos-pc.c
> tests/libqos/libqos-pc.h
> tests/libqos/libqos.c
> tests/libqos/libqos.h
> tests/libqos/malloc-generic.c
> tests/libqos/malloc-generic.h
> tests/libqos/malloc-pc.c
> tests/libqos/malloc-pc.h
> tests/libqos/malloc.c
> tests/libqos/malloc.h
> tests/libqos/pci-pc.c
> tests/libqos/pci-pc.h
> tests/libqos/pci.c
> tests/libqos/pci.h
> tests/libqos/usb.c
> tests/libqos/usb.h
> tests/libqos/virtio-mmio.c
> tests/libqos/virtio-mmio.h
> tests/libqos/virtio-pci.c
> tests/libqos/virtio-pci.h
> tests/libqos/virtio.c
> tests/libqos/virtio.h
> tests/libqtest.c
> tests/libqtest.h

I wouldn't mind taking these, if that helped instead of being a burden.
I expect most of the time that changes to these files will go through
other trees.

Perhaps as a 'weak' assignment, but allowing e.g. virtio, i2c, etc to be
more strongly associated with those subsystems and their maintainers
instead; like I will group tests/libqos/ahci.[ch] with my IDE stanza.


[ tests/multiboot/* ]
[ tests/rocker/* ]
[ tests/tcg/* ]

> tests/test-hbitmap.c

Either me or Paolo, I guess, though I'm not sure if it's worth creating
stanzas for each single test file here. We have bigger fish to fry,
probably.

[ tests/test-qmp-* ]

Our good friend Markus.

[ util/* ] Another giant grab-bag.


--js

Re: [Qemu-devel] [PATCH] docs: describe the QEMU build system structure / design

[Eric Blake]

On 09/22/2015 12:28 PM, Laszlo Ersek wrote:
> meta review of your review:
> 

> 
>>> +Each system/userspace emulation target needs to have a slightly
>>> +different set of make rules / variables. Thus, make will be recursively
>>> +invoked for each of the emulation targets.
>>> +
>>> +The recursive invokation will end up processing the toplevel
>>
>> invocation again.
> 
> Self-contradictory period again! :)
> 
>> Thanks for writing this!

Heartily seconded - in spite of our review comments, the document looks
very useful.

> 
> Yes!
> 
>> Pretending to be Eric,
> 
> Yes. :)

Don't know if I should be pleased or shocked :)


-- 
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] docs: describe the QEMU build system structure / design

[Laszlo Ersek]

On 09/22/15 20:51, Eric Blake wrote:
> On 09/22/2015 12:28 PM, Laszlo Ersek wrote:
>> meta review of your review:
>>
> 
>>
 +Each system/userspace emulation target needs to have a slightly
 +different set of make rules / variables. Thus, make will be recursively
 +invoked for each of the emulation targets.
 +
 +The recursive invokation will end up processing the toplevel
>>>
>>> invocation again.
>>
>> Self-contradictory period again! :)
>>
>>> Thanks for writing this!
> 
> Heartily seconded - in spite of our review comments, the document looks
> very useful.
> 
>>
>> Yes!
>>
>>> Pretending to be Eric,
>>
>> Yes. :)
> 
> Don't know if I should be pleased or shocked :)

Pleased, certainly. :)

Laszlo

Re: [Qemu-devel] MAINTAINERS leaves too many files uncovered

[Paolo Bonzini]

-BEGIN PGP SIGNED MESSAGE-
Hash: SHA256



On 22/09/2015 20:07, Eric Blake wrote:
> On 09/22/2015 03:13 AM, Markus Armbruster wrote:
> 
>>> Full list of unmaintained files now:
>> [...]
>> 
> 
>> docs/bitmaps.md docs/blkdebug.txt docs/blkverify.txt
> 
>> docs/live-block-ops.txt
> 
>> docs/qcow2-cache.txt docs/qdev-device-use.txt
> 
>> docs/specs/qcow2.txt docs/specs/qed_spec.txt
> 
> Kevin, Stefan - should these be added to Block maintainers?

Possibly moved to docs/block while at it?

Paolo

Re: [Qemu-devel] [PULL 0/2] Monitor patches

[Peter Maydell]

On 22 September 2015 at 05:01, Markus Armbruster  wrote:
> The following changes since commit 9e72681d16792d0ffc42bab634b1753ff299bdfd:
>
>   Merge remote-tracking branch 'remotes/armbru/tags/pull-qapi-2015-09-21' 
> into staging (2015-09-21 22:33:51 +0100)
>
> are available in the git repository at:
>
>   git://repo.or.cz/qemu/armbru.git tags/pull-monitor-2015-09-22
>
> for you to fetch changes up to abadcbc838a3b256819fd8932c34a4af41ec187f:
>
>   hmp: Restore "info pci" (2015-09-22 11:32:37 +0200)
>
> 
> Monitor patches
>
> 
> Daniel P. Berrange (1):
>   monitor: allow device_del to accept QOM paths
>
> Paolo Bonzini (1):
>   hmp: Restore "info pci"
>

Applied, thanks.

-- PMM

Re: [Qemu-devel] [PATCH] docs: describe the QEMU build system structure / design

[Eric Blake]

On 09/22/2015 10:35 AM, Daniel P. Berrange wrote:
> Developers who are new to QEMU, or have a background familiarity
> with GNU autotools can have trouble getting their head around the

s/autotools/autotools,/

> home-grown QEMU build system. This document attempts to explain
> the structure / design of the configure script and the various
> Makefile pieces that live across the source tree.
> 
> Signed-off-by: Daniel P. Berrange 
> ---
>  docs/build-system.txt | 493 
> ++
>  1 file changed, 493 insertions(+)
>  create mode 100644 docs/build-system.txt
> 

> +Stage 1: configure

> +
> +In contrast to autoconf scripts, QEMU's configure is expected to be
> +silent while it is checking for features. It will only display output
> +when an error occurrs, or to show the final feature enablement summary

s/occurrs/occurs/

> +on completion.
> +
> +Adding new checks to the configure script usually comprises the
> +following tasks
> +
> + - Initialize one or more variables with the default feature state.
> +
> +   Ideally features should auto-detect whether they are present,
> +   so try to avoid hardcoding the initial state to either enabled
> +   or disabled, as that forces the user to pass a --{dis,en}able-XXX
> +   flag on every invokation of configure

s/invokation/invocation/


> +Stage 2: makefiles
> +==
> +
> +Although the source code is spread across multiple subdirectories, the
> +build system should be considered largely non-recursive in nature, in
> +contrast to common practices seen with automake. There is some recursive
> +invokation of make, but this is related to the things being built,
> +rather than the source directory structure.

s/invokation/invocation/

Mention that we require GNU make.

> +Module structure
> +
> +
> +There are a number of key outputs of the QEMU build system
> +
> + - Tools - qemu-img, qemu-nbd, qga (guest agent), etc
> + - System emulators - qemu-system-$ARCH
> + - Userspace emulators - qemu-$ARCH
> + - Unit tests
> +
> +The source code is highly modularized, split across many files to
> +facilitate building of all of these components with as little duplicated
> +compilation as possible. There can be considered to be two distinct
> +groups of files, those which are independant of the QEMU emulation
> +target and those which are dependant on the QEMU emulation target.

Throughout this section: s/(in)?dependant/dependent/

> +
> +Statically defined files
> +

> +The recursive invokation will end up processing the toplevel

s/invokation/invocation/


> +
> +Dynamically created files
> +-
> +
> +The following files are generated dynamically by configure in order to
> +control the behaviour of the staticaly defined makefiles. This avoids

s/staticaly/statically/


> +
> +It is also used as a dependancy checking mechanism. If make sees that

s/dependancy/dependency/

-- 
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 v10 6/7] vhost-user: add multiple queue support

[Eduardo Habkost]

On Fri, Sep 18, 2015 at 10:58:43PM +0800, Yuanhan Liu wrote:
[...]
> diff --git a/net/vhost-user.c b/net/vhost-user.c
> index 93dcecd..4fa3d64 100644
> --- a/net/vhost-user.c
> +++ b/net/vhost-user.c
[...]
> +if (i == 0) {
> +max_queues = vhost_net_get_max_queues(s->vhost_net);

This breaks arm-softmmu (and any target that doesn't have CONFIG_VHOST_NET):

  $ make subdir-arm-softmmu
CCarm-softmmu/hw/net/vhost_net.o
LINK  arm-softmmu/qemu-system-arm
  ../net/vhost-user.o: In function `vhost_user_start':
  /home/ehabkost/rh/proj/virt/qemu/net/vhost-user.c:88: undefined reference to 
`vhost_net_get_max_queues'
  collect2: error: ld returned 1 exit status
  Makefile:193: recipe for target 'qemu-system-arm' failed
  make[1]: *** [qemu-system-arm] Error 1
  Makefile:184: recipe for target 'subdir-arm-softmmu' failed
  make: *** [subdir-arm-softmmu] Error 2

-- 
Eduardo

Re: [Qemu-devel] [PULL 00/48] Misc patches for 2015-09-22

[Peter Maydell]

On 22 September 2015 at 08:05, Paolo Bonzini  wrote:
> The following changes since commit 9e72681d16792d0ffc42bab634b1753ff299bdfd:
>
>   Merge remote-tracking branch 'remotes/armbru/tags/pull-qapi-2015-09-21' 
> into staging (2015-09-21 22:33:51 +0100)
>
> are available in the git repository at:
>
>   git://github.com/bonzini/qemu.git tags/for-upstream
>
> for you to fetch changes up to 6beb990c835097c402876b04075c6af533d00949:
>
>   ppc: Rename ELF_MACHINE to be PPC specific (2015-09-22 15:55:02 +0200)
>
> 
> * First batch of MAINTAINERS updates
> * IOAPIC fixes (to pass kvm-unit-tests with -machine kernel_irqchip=off)
> * NBD API upgrades from Daniel
> * strtosz fixes from Marc-André
> * improved support for readonly=on on scsi-generic devices
> * new "info ioapic" and "info lapic" monitor commands
> * Peter Crosthwaite's ELF_MACHINE cleanups

I'm afraid this doesn't build on 32-bit:

/root/qemu/hw/intc/ioapic_common.c: In function ‘ioapic_print_redtbl’:
/root/qemu/hw/intc/ioapic_common.c:80:24: error: format ‘%lu’ expects
argument of type ‘long unsigned int’, but argument 6 has type
‘uint64_t’ [-Werror=format]

thanks
-- PMM

Re: [Qemu-devel] [PATCH] docs: describe the QEMU build system structure / design

[Paolo Bonzini]

On 22/09/2015 20:11, John Snow wrote:
>> +such as error handling infrastructure, standard data structures,
>> +platform portability wrapper functions, etc. This code can be compiled
>> +once only and the .o files linked into all output binaries.
>> +
>> +In the target dependant set lives CPU emulation, device emulation and
>> +much glue code. This code is generally compiled multiple times, once for

s/is generally compiled/sometimes also has to be compiled/

>> +each target architecture being built.

... while in other cases can be compiled once for each architecture.

Do not use "target architecture", since you're using "target" to mean a
Makefile target, i.e. binary.

>> +The target independant code that is used by all binaries is built into a

s/The target independant/Utlity/

>> +static archive called libqemuutil.a, which is then linked to all the
>> +binaries. Due to ongoing incomplete refactoring, some of the code in

s/Due to ongoing incomplete refactoring/In order to provide hooks that
are only needed by some of the binaries/

>> +libqemuutil.a depends on other functions that are not available in all

s/available in/fully implemented by/

>> +QEMU binaries. To deal with this there is a second library called
>> +libqemustub.a which provide dummy stubs for all these functions. These
>> +will get lazy linked into the binary if the real implementation is not
>> +present. Thus any time a binary links to libqemuutil.a, it should also
>> +be made to link libqemustub.a. eg
>> +
>> + qemu-img$(EXESUF): qemu-img.o ..snip.. libqemuutil.a libqemustub.a

Really all binaries should link libqemustub.a.

Perhaps add a note that libqemustub symbols effectively work as weak
symbols, but a static library is more portable?

>> +
>> +Windows platform portability
>> +
>> +
>> +On Windows all binaries have a .exe suffix, so all the Makefile rules
> 
> I guess you pronounce the 'dot' :)
> 
>> +which create binaries must include the $(EXESUF) variable on the binary
>> +name. eg
> 
> 'e.g.' here and everywhere subsequent.
> 
>> +
>> + qemu-img$(EXESUF): qemu-img.o ..snip..
>> +
>> +This expands to '.exe' on Windows, or '' on other platforms.
>> +
>> +A further complication for the system and userspace emulator binaries is
>> +that two separate binaries need to be generated.
>> +
>> +The main binary (eg qemu-system-x86_64.exe) is linked against the
>> +Windows console runtime subsystem. These are expected to be run from a
>> +command prompt window, and so will print stderr to the console that
>> +launched them.
>> +
>> +The second binary generated has a 'w' on the end of its name (eg
>> +qemu-system-x86_64w.exe) and is linked against the Windows graphical
>> +runtime subsystem. These are expected to be run directly from the
>> +desktop and will open up a dedicated console window for stderr output.
>> +
>> +The Makefile.target will generate the binary for the graphical subsystem
>> +first, and then use objcopy to relink it against the console subsystem
>> +to generate the second binary.
>> +
>> +
>> +Object variable naming
>> +--
>> +
>> +The QEMU convention is to define variables to list different groups of
>> +object files. These are named with the convention $PREFIX-y. For example

$PREFIX-obj-y

>> +the libqemuutil.a file will be linked with all objects listed in a
>> +variable 'util-y'.

util-obj-y.

>> +- */Makefile.objs
>> +
>> +Since the source code is spread across multiple directories, the rules
>> +for each file are similarly modularized. Thus each subdirectory
>> +containing .c files will usually also contain a Makefile.objs file.
>> +These files are not directly invoked by a recursive make, but instead
>> +they are imported by the top level Makefile and/or Makefile.target
>> +
>> +Each Makefile.objs usually just declares a set of variables listing the
>> +.o files that need building from the source files in the directory. They
>> +will also define any custom linker or compiler flags. For example in
>> +block/Makefile.objs
>> +
>> +  block-obj-$(CONFIG_LIBISCSI) += iscsi.o
>> +  block-obj-$(CONFIG_CURL) += curl.o
>> +
>> +  ..snip...
>> +
>> +  iscsi.o-cflags := $(LIBISCSI_CFLAGS)
>> +  iscsi.o-libs   := $(LIBISCSI_LIBS)
>> +  curl.o-cflags  := $(CURL_CFLAGS)
>> +  curl.o-libs:= $(CURL_LIBS)

You may want to mention that rules in */Makefile.objs should use $(obj)
as a prefix to the target, for example:

$(obj)/generated-tcg-tracers.h: $(obj)/generated-tcg-tracers.h-timestamp

I'll join the choir: awesome job.

Paolo

Re: [Qemu-devel] [PATCH v7 09/14] block: Add block job transactions

[John Snow]

On 09/21/2015 10:46 PM, Fam Zheng wrote:
> Sometimes block jobs must execute as a transaction group.  Finishing
> jobs wait until all other jobs are ready to complete successfully.
> Failure or cancellation of one job cancels the other jobs in the group.
> 
> Signed-off-by: Stefan Hajnoczi 
> [Rewrite the implementation which is now contained in block_job_completed.
> --Fam]
> Signed-off-by: Fam Zheng 
> Reviewed-by: Max Reitz 
> ---
>  blockjob.c   | 135 
> ++-
>  include/block/block.h|   1 +
>  include/block/blockjob.h |  38 +
>  3 files changed, 172 insertions(+), 2 deletions(-)
> 
> diff --git a/blockjob.c b/blockjob.c
> index 36c18e0..91e8d3c 100644
> --- a/blockjob.c
> +++ b/blockjob.c
> @@ -36,6 +36,19 @@
>  #include "qemu/timer.h"
>  #include "qapi-event.h"
>  
> +/* Transactional group of block jobs */
> +struct BlockJobTxn {
> +
> +/* Is this txn being cancelled? */
> +bool aborting;
> +
> +/* List of jobs */
> +QLIST_HEAD(, BlockJob) jobs;
> +
> +/* Reference count */
> +int refcnt;
> +};
> +
>  void *block_job_create(const BlockJobDriver *driver, BlockDriverState *bs,
> int64_t speed, BlockCompletionFunc *cb,
> void *opaque, Error **errp)
> @@ -90,6 +103,86 @@ void block_job_unref(BlockJob *job)
>  }
>  }
>  
> +static void block_job_completed_single(BlockJob *job)
> +{
> +if (!job->ret) {
> +if (job->driver->commit) {
> +job->driver->commit(job);
> +}
> +} else {
> +if (job->driver->abort) {
> +job->driver->abort(job);
> +}
> +}
> +job->cb(job->opaque, job->ret);
> +if (job->txn) {
> +block_job_txn_unref(job->txn);
> +}
> +block_job_unref(job);
> +}
> +
> +static void block_job_completed_txn_abort(BlockJob *job)
> +{
> +AioContext *ctx;
> +BlockJobTxn *txn = job->txn;
> +BlockJob *other_job, *next;
> +
> +if (txn->aborting) {
> +/*
> + * We are cancelled by another job, which will handle everything.
> + */
> +return;
> +}
> +txn->aborting = true;
> +/* We are the first failed job. Cancel other jobs. */
> +QLIST_FOREACH(other_job, >jobs, txn_list) {
> +ctx = bdrv_get_aio_context(other_job->bs);
> +aio_context_acquire(ctx);
> +}
> +QLIST_FOREACH(other_job, >jobs, txn_list) {
> +if (other_job == job || other_job->completed) {
> +/* Other jobs are "effectively" cancelled by us, set the status 
> for
> + * them; this job, however, may or may not be cancelled, 
> depending
> + * on the caller, so leave it. */
> +if (other_job != job) {
> +other_job->cancelled = true;
> +}
> +continue;
> +}
> +block_job_cancel_sync(other_job);
> +assert(other_job->completed);
> +}
> +QLIST_FOREACH_SAFE(other_job, >jobs, txn_list, next) {
> +ctx = bdrv_get_aio_context(other_job->bs);
> +block_job_completed_single(other_job);
> +aio_context_release(ctx);
> +}
> +}
> +
> +static void block_job_completed_txn_success(BlockJob *job)
> +{
> +AioContext *ctx;
> +BlockJobTxn *txn = job->txn;
> +BlockJob *other_job, *next;
> +/*
> + * Successful completion, see if there are other running jobs in this
> + * txn.
> + */
> +QLIST_FOREACH(other_job, >jobs, txn_list) {
> +if (!other_job->completed) {
> +return;
> +}
> +}
> +/* We are the last completed job, commit the transaction. */
> +QLIST_FOREACH_SAFE(other_job, >jobs, txn_list, next) {
> +ctx = bdrv_get_aio_context(other_job->bs);
> +aio_context_acquire(ctx);
> +assert(other_job->ret == 0);

Sorry for being a dense noggin about this, but is it documented anywhere
(through an assertion or otherwise) that we will never return a
positive, non-zero return code for a block job?

I just don't want to get into a situation where, in the future, someone
decides to do so and then mysteriously something breaks a version or two
later.

> +block_job_completed_single(other_job);
> +aio_context_release(ctx);
> +}
> +}
> +
>  void block_job_completed(BlockJob *job, int ret)
>  {
>  BlockDriverState *bs = job->bs;
> @@ -98,8 +191,13 @@ void block_job_completed(BlockJob *job, int ret)
>  assert(!job->completed);
>  job->completed = true;
>  job->ret = ret;
> -job->cb(job->opaque, ret);
> -block_job_unref(job);
> +if (!job->txn) {
> +block_job_completed_single(job);
> +} else if (ret < 0 || block_job_is_cancelled(job)) {
> +block_job_completed_txn_abort(job);
> +} else {
> +block_job_completed_txn_success(job);
> +}
>  }
>  
>  void block_job_set_speed(BlockJob *job, int64_t speed, 

Re: [Qemu-devel] [PATCH 05/16] block: Convert bs->file to BdrvChild

[Max Reitz]

On 17.09.2015 15:48, Kevin Wolf wrote:
> This patch removes the temporary duplication between bs->file and
> bs->file_child by converting everything to BdrvChild.
> 
> Signed-off-by: Kevin Wolf 
> ---
>  block.c   | 61 ++
>  block/blkdebug.c  | 32 
>  block/blkverify.c | 33 ++---
>  block/bochs.c |  8 +++---
>  block/cloop.c | 10 
>  block/dmg.c   | 20 +++
>  block/io.c| 50 +++---
>  block/parallels.c | 38 +++--
>  block/qapi.c  |  2 +-
>  block/qcow.c  | 42 +---
>  block/qcow2-cache.c   | 11 +
>  block/qcow2-cluster.c | 38 +
>  block/qcow2-refcount.c| 45 ++
>  block/qcow2-snapshot.c| 30 ---
>  block/qcow2.c | 62 
> ---
>  block/qed-table.c |  4 +--
>  block/qed.c   | 32 
>  block/raw_bsd.c   | 40 +++---
>  block/snapshot.c  | 12 -
>  block/vdi.c   | 17 +++--
>  block/vhdx-log.c  | 25 ++-
>  block/vhdx.c  | 36 ++-
>  block/vmdk.c  | 27 +++--
>  block/vpc.c   | 34 ++
>  include/block/block.h |  8 +-
>  include/block/block_int.h |  3 +--
>  26 files changed, 377 insertions(+), 343 deletions(-)

Reviewed-by: Max Reitz 

> diff --git a/block.c b/block.c
> index 2e9e5e2..93d713b 100644
> --- a/block.c
> +++ b/block.c

[...]

> @@ -1929,6 +1925,11 @@ void bdrv_close(BlockDriverState *bs)
>  bdrv_unref(backing_hd);
>  }
>  
> +if (bs->file != NULL) {
> +bdrv_unref(bs->file->bs);

I think we can use bdrv_unref_child(bs->file) here. I'd personally like
it more, at least (because using bdrv_unref() and relying on the loop
below is basically what the comment inside of the loop advises against).

Yes, I know, eventually, we want to drop this block anyway and let the
loop below handle everything using bdrv_unref_child(). But when we do
that conversion, it'll be obvious to drop a bdrv_unref_child() call,
whereas dropping bdrv_unref() alone isn't so obvious.

Max

> +bs->file = NULL;
> +}
> +
>  QLIST_FOREACH_SAFE(child, >children, next, next) {
>  /* TODO Remove bdrv_unref() from drivers' close function and use
>   * bdrv_unref_child() here */
> @@ -1953,11 +1954,6 @@ void bdrv_close(BlockDriverState *bs)
>  bs->options = NULL;
>  QDECREF(bs->full_open_options);
>  bs->full_open_options = NULL;
> -
> -if (bs->file != NULL) {
> -bdrv_unref(bs->file);
> -bs->file = NULL;
> -}
>  }
>  
>  if (bs->blk) {



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH v7 10/14] blockdev: make BlockJobTxn available to qmp 'transaction'

[John Snow]

On 09/21/2015 10:46 PM, Fam Zheng wrote:
> From: Stefan Hajnoczi 
> 
> Provide a BlockJobTxn to actions executed in a qmp 'transaction'
> command.  This allows actions to make their block jobs either complete
> as a group or fail/cancel together.
> 
> The next patch adds the first user.
> 
> Signed-off-by: Stefan Hajnoczi 
> Reviewed-by: Fam Zheng 
> Reviewed-by: Max Reitz 
> Signed-off-by: Fam Zheng 
> ---
>  blockdev.c | 6 ++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/blockdev.c b/blockdev.c
> index 796bc64..ed50cb4 100644
> --- a/blockdev.c
> +++ b/blockdev.c
> @@ -1285,6 +1285,7 @@ typedef struct BlkActionOps {
>  struct BlkActionState {
>  TransactionAction *action;
>  const BlkActionOps *ops;
> +BlockJobTxn *block_job_txn;
>  QSIMPLEQ_ENTRY(BlkActionState) entry;
>  };
>  

I know it's obvious, but for consistency we should update the
documentation sitting right above this structure.

> @@ -1883,12 +1884,15 @@ static const BlkActionOps actions[] = {
>  void qmp_transaction(TransactionActionList *dev_list, Error **errp)
>  {
>  TransactionActionList *dev_entry = dev_list;
> +BlockJobTxn *block_job_txn;
>  BlkActionState *state, *next;
>  Error *local_err = NULL;
>  
>  QSIMPLEQ_HEAD(snap_bdrv_states, BlkActionState) snap_bdrv_states;
>  QSIMPLEQ_INIT(_bdrv_states);
>  
> +block_job_txn = block_job_txn_new();
> +
>  /* drain all i/o before any operations */
>  bdrv_drain_all();
>  
> @@ -1908,6 +1912,7 @@ void qmp_transaction(TransactionActionList *dev_list, 
> Error **errp)
>  state = g_malloc0(ops->instance_size);
>  state->ops = ops;
>  state->action = dev_info;
> +state->block_job_txn = block_job_txn;
>  QSIMPLEQ_INSERT_TAIL(_bdrv_states, state, entry);
>  
>  state->ops->prepare(state, _err);
> @@ -1940,6 +1945,7 @@ exit:
>  }
>  g_free(state);
>  }
> +block_job_txn_unref(block_job_txn);
>  }
>  
>  
> 

With or without my recommendation:

Reviewed-by: John Snow 

Re: [Qemu-devel] [Bug 1498144] [NEW] Failure booting hurd with qemu-system-i386 on ARM

[Laszlo Ersek]

On 09/21/15 21:04, PeteVine wrote:
> Public bug reported:
> 
> Trying to boot debian-hurd-20150320.img ends with:
> 
> qemu-system-i386: qemu-coroutine-lock.c:91: qemu_co_queue_restart_all:
> Assertion `qemu_in_coroutine()' failed.
> 
> Program received signal SIGABRT, Aborted.
> __libc_do_syscall ()
> at ../ports/sysdeps/unix/sysv/linux/arm/libc-do-syscall.S:44
> 44  ../ports/sysdeps/unix/sysv/linux/arm/libc-do-syscall.S: No such file 
> or directory.
> (gdb) bt
> #0  __libc_do_syscall ()
> at ../ports/sysdeps/unix/sysv/linux/arm/libc-do-syscall.S:44
> #1  0xb6ef8f0e in __GI_raise (sig=sig@entry=6)
> at ../nptl/sysdeps/unix/sysv/linux/raise.c:56
> #2  0xb6efb766 in __GI_abort () at abort.c:89
> #3  0xb6ef4150 in __assert_fail_base (
> fmt=0x1 , 
> assertion=0x7f89a234 "qemu_in_coroutine()", assertion@entry=0x0, 
> file=0x7f89da58 "qemu-coroutine-lock.c", file@entry=0xb566 "\001", 
> line=91, line@entry=3069931692, 
> function=function@entry=0x7f89ab78 "qemu_co_queue_restart_all")
> at assert.c:92
> #4  0xb6ef41e6 in __GI___assert_fail (assertion=0x0, file=0xb566 "\001", 
> line=3069931692, function=0x7f89ab78 "qemu_co_queue_restart_all")
> at assert.c:101
> #5  0x7f59a6b4 in ?? ()
> 
> I was using the same setup as in Bug 893208 (i.e git checkout from
> 2015-09-15)
> 
> ** Affects: qemu
>  Importance: Undecided
>  Status: New
> 

This backtrace is next to useless I believe, but it's not your fault. I
think "scripts/qemu-gdb.py" might be helpful (it has a little bit of
documentation too). See also commit 9eddd6a4.

Thanks
Laszlo

[Qemu-devel] [PATCH V5 0/2] Move sdhci.h to include/hw/sd

[Sai Pavan Boddu]

Move sdhci.h splitting it into common and internal.
Create a new directory for sd in include/hw/.
Correct paths of sd.h in at every instance of #include.

Sai Pavan Boddu (2):
  sd.h: Move sd.h to include/hw/sd/
  sdhci: Split sdhci.h for public and internal device usage

 hw/sd/milkymist-memcard.c   |  2 +-
 hw/sd/omap_mmc.c|  2 +-
 hw/sd/pl181.c   |  2 +-
 hw/sd/pxa2xx_mmci.c |  2 +-
 hw/sd/sd.c  |  2 +-
 hw/sd/{sdhci.h => sdhci-internal.h} | 67 +--
 hw/sd/sdhci.c   |  3 +-
 hw/sd/ssi-sd.c  |  2 +-
 include/hw/{ => sd}/sd.h|  0
 include/hw/sd/sdhci.h   | 92 +
 10 files changed, 101 insertions(+), 73 deletions(-)
 rename hw/sd/{sdhci.h => sdhci-internal.h} (75%)
 rename include/hw/{ => sd}/sd.h (100%)
 create mode 100644 include/hw/sd/sdhci.h

-- 
2.1.4

[Qemu-devel] [PATCH V5 1/2] sd.h: Move sd.h to include/hw/sd/

[Sai Pavan Boddu]

Create a sd director under include/hw/ and move sd.h to same.

Signed-off-by: Sai Pavan Boddu 
Reviewed-by: Alistair Francis 
---
Changes for V5:
None
Changes for V4:
Fix commit message.
Changes for V3:
None.
---
 hw/sd/milkymist-memcard.c | 2 +-
 hw/sd/omap_mmc.c  | 2 +-
 hw/sd/pl181.c | 2 +-
 hw/sd/pxa2xx_mmci.c   | 2 +-
 hw/sd/sd.c| 2 +-
 hw/sd/sdhci.h | 2 +-
 hw/sd/ssi-sd.c| 2 +-
 include/hw/{ => sd}/sd.h  | 0
 8 files changed, 7 insertions(+), 7 deletions(-)
 rename include/hw/{ => sd}/sd.h (100%)

diff --git a/hw/sd/milkymist-memcard.c b/hw/sd/milkymist-memcard.c
index 2209ef1..b430d56 100644
--- a/hw/sd/milkymist-memcard.c
+++ b/hw/sd/milkymist-memcard.c
@@ -28,7 +28,7 @@
 #include "qemu/error-report.h"
 #include "sysemu/block-backend.h"
 #include "sysemu/blockdev.h"
-#include "hw/sd.h"
+#include "hw/sd/sd.h"
 
 enum {
 ENABLE_CMD_TX   = (1<<0),
diff --git a/hw/sd/omap_mmc.c b/hw/sd/omap_mmc.c
index 35d8033..5bc4719 100644
--- a/hw/sd/omap_mmc.c
+++ b/hw/sd/omap_mmc.c
@@ -18,7 +18,7 @@
  */
 #include "hw/hw.h"
 #include "hw/arm/omap.h"
-#include "hw/sd.h"
+#include "hw/sd/sd.h"
 
 struct omap_mmc_s {
 qemu_irq irq;
diff --git a/hw/sd/pl181.c b/hw/sd/pl181.c
index 11fcd47..ddd9b6f 100644
--- a/hw/sd/pl181.c
+++ b/hw/sd/pl181.c
@@ -10,7 +10,7 @@
 #include "sysemu/block-backend.h"
 #include "sysemu/blockdev.h"
 #include "hw/sysbus.h"
-#include "hw/sd.h"
+#include "hw/sd/sd.h"
 
 //#define DEBUG_PL181 1
 
diff --git a/hw/sd/pxa2xx_mmci.c b/hw/sd/pxa2xx_mmci.c
index d1fe6d5..b217080 100644
--- a/hw/sd/pxa2xx_mmci.c
+++ b/hw/sd/pxa2xx_mmci.c
@@ -12,7 +12,7 @@
 
 #include "hw/hw.h"
 #include "hw/arm/pxa.h"
-#include "hw/sd.h"
+#include "hw/sd/sd.h"
 #include "hw/qdev.h"
 
 struct PXA2xxMMCIState {
diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index a1ff465..0787e33 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -31,7 +31,7 @@
 
 #include "hw/hw.h"
 #include "sysemu/block-backend.h"
-#include "hw/sd.h"
+#include "hw/sd/sd.h"
 #include "qemu/bitmap.h"
 
 //#define DEBUG_SD 1
diff --git a/hw/sd/sdhci.h b/hw/sd/sdhci.h
index 3352d23..a45593f 100644
--- a/hw/sd/sdhci.h
+++ b/hw/sd/sdhci.h
@@ -28,7 +28,7 @@
 #include "qemu-common.h"
 #include "hw/pci/pci.h"
 #include "hw/sysbus.h"
-#include "hw/sd.h"
+#include "hw/sd/sd.h"
 
 /* R/W SDMA System Address register 0x0 */
 #define SDHC_SYSAD 0x00
diff --git a/hw/sd/ssi-sd.c b/hw/sd/ssi-sd.c
index e4b2d4f..c49ff62 100644
--- a/hw/sd/ssi-sd.c
+++ b/hw/sd/ssi-sd.c
@@ -13,7 +13,7 @@
 #include "sysemu/block-backend.h"
 #include "sysemu/blockdev.h"
 #include "hw/ssi.h"
-#include "hw/sd.h"
+#include "hw/sd/sd.h"
 
 //#define DEBUG_SSI_SD 1
 
diff --git a/include/hw/sd.h b/include/hw/sd/sd.h
similarity index 100%
rename from include/hw/sd.h
rename to include/hw/sd/sd.h
-- 
2.1.4

Re: [Qemu-devel] [wiki] New wiki page - vhost-user setup with ovs/dpdk backend

[Marcel Apfelbaum]

On 09/22/2015 08:43 AM, Star Chang wrote:

Thanks Marcel!!!

After tailing the logs and basic troubleshooting, we finally can have 2 VMs to 
ping to each other. One dpdkvhostuser interface is for each VM and we have to 
new add flows based on dpdkvhostuser ofport
numbers.


Hi,

Good to hear this!
If you think I should add something to the wiki please do tell.



Another issue we might post it to proper mail loop is that we run pktgen-dpdk 
2.9.1 inside of VM but no packets counted from pktgen terminal display. It is 
so weird that we only see 128 tx packets
bursted out and then nothing is continue working. After that, we try to dump-flows via 
ovs-ofctl utility but it shows "connection refued". I am not sure have any one 
of you guys ideas here? Thanks!!!



I am sorry but my OVS knowledge is rather limited. I think this is a question 
for the OVS mailing list.

Thanks,
Marcel



Bridge "ovsbr0"

Port "ovsbr0"

Interface "ovsbr0"

type: internal

Port "dpdk1"

Interface "dpdk1"

type: dpdk

Port "dpdk0"

Interface "dpdk0"

type: dpdk

Port "vhost-user1"

Interface "vhost-user1"

type: dpdkvhostuser

Port "vhost-user0"

Interface "vhost-user0"

type: dpdkvhostuser


On Tue, Sep 22, 2015 at 2:34 AM, Marcel Apfelbaum > wrote:

On 09/21/2015 11:25 AM, Marcel Apfelbaum wrote:

On 09/21/2015 08:01 AM, Star Chang wrote:

Hi Marcel:

Many thanks for you to contribute wiki page in how to configuring 
vhost-user type with openvsitch/qemu in VM environment.

We bring 2 VMs up with vhost-user type. We can see eth0 interfaces 
created in 2 VMs with proper mac address we assign. After IP address 
assignment, 2 VMs could not PING to each other when
they are in
the same subnet. There are not packets at all in count when running 
ovs-ofctl dump-ports :(

However, we check link up state via ovs-ofctl utility but LINK_DOWN 
as below. Have any one with experiences and give some helps … Thanks!!


Hi,

I would look into OVS log for specific issues:

 journalctl  --since `date +%T --date="-10 minutes"`

The above command will show you the last 10 minutes log.
You are welcomed to show us the log.


I forgot to CC the list.

Thanks,
Marcel


Thanks,
Marcel

$ sudo ./utilities/ovs-ofctl show ovsbr0
OFPT_FEATURES_REPLY (xid=0x2): dpid:0670da615e4a
n_tables:254, n_buffers:256
capabilities: FLOW_STATS TABLE_STATS PORT_STATS QUEUE_STATS 
ARP_MATCH_IP
actions: output enqueue set_vlan_vid set_vlan_pcp strip_vlan 
mod_dl_src mod_dl_dst mod_nw_src mod_nw_dst mod_nw_tos mod_tp_src mod_tp_dst
1(vhost-user1): addr:00:00:00:00:00:00
config: PORT_DOWN
state: LINK_DOWN
speed: 0 Mbps now, 0 Mbps max
2(vhost-user2): addr:00:00:00:00:00:00
config: PORT_DOWN
state: LINK_DOWN
speed: 0 Mbps now, 0 Mbps max
LOCAL(ovsbr0): addr:06:70:da:61:5e:4a
config: PORT_DOWN
state: LINK_DOWN
current: 10MB-FD COPPER
speed: 10 Mbps now, 0 Mbps max
OFPT_GET_CONFIG_REPLY (xid=0x4): frags=normal miss_send_len=0



Thanks,
Star

--
The future belongs to those who believe in the beauty of their 
dreams.







--
The future belongs to those who believe in the beauty of their dreams.

[Qemu-devel] [PATCH V5 2/2] sdhci: Split sdhci.h for public and internal device usage

[Sai Pavan Boddu]

Split sdhci.h into pubilc Version (i.e include/hw/sd/sdhci.h) and
internal version (i.e hw/sd/sdhci-interna.h) base on register
declarations and object declaration.

Signed-off-by: Sai Pavan Boddu 
---
Chagdes for V5:
Rename pubilc header version as sdhci.h and internal version to
sdhci-internal.h
Changes for V4:
Remain the name of internal version of sdchi.h as same. And change
Re-Adding qemu-common.h header.
the one which is in includes/ to sdhci-common.h
Changes for V2:
Create new area in includes for sd. And move sdhci.h to same.
Changes for V3:
Split the headers to public and common.
---
 hw/sd/{sdhci.h => sdhci-internal.h} | 67 +--
 hw/sd/sdhci.c   |  3 +-
 include/hw/sd/sdhci.h   | 92 +
 3 files changed, 95 insertions(+), 67 deletions(-)
 rename hw/sd/{sdhci.h => sdhci-internal.h} (75%)
 create mode 100644 include/hw/sd/sdhci.h

diff --git a/hw/sd/sdhci.h b/hw/sd/sdhci-internal.h
similarity index 75%
rename from hw/sd/sdhci.h
rename to hw/sd/sdhci-internal.h
index a45593f..c40ae2b 100644
--- a/hw/sd/sdhci.h
+++ b/hw/sd/sdhci-internal.h
@@ -21,14 +21,10 @@
  * You should have received a copy of the GNU General Public License along
  * with this program; if not, see .
  */
-
 #ifndef SDHCI_H
 #define SDHCI_H
 
-#include "qemu-common.h"
-#include "hw/pci/pci.h"
-#include "hw/sysbus.h"
-#include "hw/sd/sd.h"
+#include "hw/sd/sdhci.h"
 
 /* R/W SDMA System Address register 0x0 */
 #define SDHC_SYSAD 0x00
@@ -231,65 +227,6 @@ enum {
 sdhc_gap_write  = 2   /* SDHC stopped at block gap during write operation 
*/
 };
 
-/* SD/MMC host controller state */
-typedef struct SDHCIState {
-union {
-PCIDevice pcidev;
-SysBusDevice busdev;
-};
-SDState *card;
-MemoryRegion iomem;
-
-QEMUTimer *insert_timer;   /* timer for 'changing' sd card. */
-QEMUTimer *transfer_timer;
-qemu_irq eject_cb;
-qemu_irq ro_cb;
-qemu_irq irq;
-
-uint32_t sdmasysad;/* SDMA System Address register */
-uint16_t blksize;  /* Host DMA Buff Boundary and Transfer BlkSize Reg 
*/
-uint16_t blkcnt;   /* Blocks count for current transfer */
-uint32_t argument; /* Command Argument Register */
-uint16_t trnmod;   /* Transfer Mode Setting Register */
-uint16_t cmdreg;   /* Command Register */
-uint32_t rspreg[4];/* Response Registers 0-3 */
-uint32_t prnsts;   /* Present State Register */
-uint8_t  hostctl;  /* Host Control Register */
-uint8_t  pwrcon;   /* Power control Register */
-uint8_t  blkgap;   /* Block Gap Control Register */
-uint8_t  wakcon;   /* WakeUp Control Register */
-uint16_t clkcon;   /* Clock control Register */
-uint8_t  timeoutcon;   /* Timeout Control Register */
-uint8_t  admaerr;  /* ADMA Error Status Register */
-uint16_t norintsts;/* Normal Interrupt Status Register */
-uint16_t errintsts;/* Error Interrupt Status Register */
-uint16_t norintstsen;  /* Normal Interrupt Status Enable Register */
-uint16_t errintstsen;  /* Error Interrupt Status Enable Register */
-uint16_t norintsigen;  /* Normal Interrupt Signal Enable Register */
-uint16_t errintsigen;  /* Error Interrupt Signal Enable Register */
-uint16_t acmd12errsts; /* Auto CMD12 error status register */
-uint64_t admasysaddr;  /* ADMA System Address Register */
-
-uint32_t capareg;  /* Capabilities Register */
-uint32_t maxcurr;  /* Maximum Current Capabilities Register */
-uint8_t  *fifo_buffer; /* SD host i/o FIFO buffer */
-uint32_t buf_maxsz;
-uint16_t data_count;   /* current element in FIFO buffer */
-uint8_t  stopped_state;/* Current SDHC state */
-/* Buffer Data Port Register - virtual access point to R and W buffers */
-/* Software Reset Register - always reads as 0 */
-/* Force Event Auto CMD12 Error Interrupt Reg - write only */
-/* Force Event Error Interrupt Register- write only */
-/* RO Host Controller Version Register always reads as 0x2401 */
-} SDHCIState;
-
 extern const VMStateDescription sdhci_vmstate;
 
-#define TYPE_PCI_SDHCI "sdhci-pci"
-#define PCI_SDHCI(obj) OBJECT_CHECK(SDHCIState, (obj), TYPE_PCI_SDHCI)
-
-#define TYPE_SYSBUS_SDHCI "generic-sdhci"
-#define SYSBUS_SDHCI(obj)   \
- OBJECT_CHECK(SDHCIState, (obj), TYPE_SYSBUS_SDHCI)
-
-#endif /* SDHCI_H */
+#endif
diff --git a/hw/sd/sdhci.c b/hw/sd/sdhci.c
index e63367b..34018fd 100644
--- a/hw/sd/sdhci.c
+++ b/hw/sd/sdhci.c
@@ -28,8 +28,7 @@
 #include "sysemu/dma.h"
 #include "qemu/timer.h"
 #include "qemu/bitops.h"
-
-#include "sdhci.h"
+#include "sdhci-internal.h"
 
 /* host controller debug messages */
 #ifndef SDHC_DEBUG
diff --git a/include/hw/sd/sdhci.h b/include/hw/sd/sdhci.h
new file mode 100644
index 

[Qemu-devel] [PATCH V6 1/2] sd.h: Move sd.h to include/hw/sd/

[Sai Pavan Boddu]

Create a sd directory under include/hw/ and move sd.h to same.

Signed-off-by: Sai Pavan Boddu 
Reviewed-by: Alistair Francis 
---
Changes for V6:
Fix commit message.
Changes for V5:
None
Changes for V4:
Fix commit message.
Changes for V3:
None.
---
 hw/sd/milkymist-memcard.c | 2 +-
 hw/sd/omap_mmc.c  | 2 +-
 hw/sd/pl181.c | 2 +-
 hw/sd/pxa2xx_mmci.c   | 2 +-
 hw/sd/sd.c| 2 +-
 hw/sd/sdhci.h | 2 +-
 hw/sd/ssi-sd.c| 2 +-
 include/hw/{ => sd}/sd.h  | 0
 8 files changed, 7 insertions(+), 7 deletions(-)
 rename include/hw/{ => sd}/sd.h (100%)

diff --git a/hw/sd/milkymist-memcard.c b/hw/sd/milkymist-memcard.c
index 2209ef1..b430d56 100644
--- a/hw/sd/milkymist-memcard.c
+++ b/hw/sd/milkymist-memcard.c
@@ -28,7 +28,7 @@
 #include "qemu/error-report.h"
 #include "sysemu/block-backend.h"
 #include "sysemu/blockdev.h"
-#include "hw/sd.h"
+#include "hw/sd/sd.h"
 
 enum {
 ENABLE_CMD_TX   = (1<<0),
diff --git a/hw/sd/omap_mmc.c b/hw/sd/omap_mmc.c
index 35d8033..5bc4719 100644
--- a/hw/sd/omap_mmc.c
+++ b/hw/sd/omap_mmc.c
@@ -18,7 +18,7 @@
  */
 #include "hw/hw.h"
 #include "hw/arm/omap.h"
-#include "hw/sd.h"
+#include "hw/sd/sd.h"
 
 struct omap_mmc_s {
 qemu_irq irq;
diff --git a/hw/sd/pl181.c b/hw/sd/pl181.c
index 11fcd47..ddd9b6f 100644
--- a/hw/sd/pl181.c
+++ b/hw/sd/pl181.c
@@ -10,7 +10,7 @@
 #include "sysemu/block-backend.h"
 #include "sysemu/blockdev.h"
 #include "hw/sysbus.h"
-#include "hw/sd.h"
+#include "hw/sd/sd.h"
 
 //#define DEBUG_PL181 1
 
diff --git a/hw/sd/pxa2xx_mmci.c b/hw/sd/pxa2xx_mmci.c
index d1fe6d5..b217080 100644
--- a/hw/sd/pxa2xx_mmci.c
+++ b/hw/sd/pxa2xx_mmci.c
@@ -12,7 +12,7 @@
 
 #include "hw/hw.h"
 #include "hw/arm/pxa.h"
-#include "hw/sd.h"
+#include "hw/sd/sd.h"
 #include "hw/qdev.h"
 
 struct PXA2xxMMCIState {
diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index a1ff465..0787e33 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -31,7 +31,7 @@
 
 #include "hw/hw.h"
 #include "sysemu/block-backend.h"
-#include "hw/sd.h"
+#include "hw/sd/sd.h"
 #include "qemu/bitmap.h"
 
 //#define DEBUG_SD 1
diff --git a/hw/sd/sdhci.h b/hw/sd/sdhci.h
index 3352d23..a45593f 100644
--- a/hw/sd/sdhci.h
+++ b/hw/sd/sdhci.h
@@ -28,7 +28,7 @@
 #include "qemu-common.h"
 #include "hw/pci/pci.h"
 #include "hw/sysbus.h"
-#include "hw/sd.h"
+#include "hw/sd/sd.h"
 
 /* R/W SDMA System Address register 0x0 */
 #define SDHC_SYSAD 0x00
diff --git a/hw/sd/ssi-sd.c b/hw/sd/ssi-sd.c
index e4b2d4f..c49ff62 100644
--- a/hw/sd/ssi-sd.c
+++ b/hw/sd/ssi-sd.c
@@ -13,7 +13,7 @@
 #include "sysemu/block-backend.h"
 #include "sysemu/blockdev.h"
 #include "hw/ssi.h"
-#include "hw/sd.h"
+#include "hw/sd/sd.h"
 
 //#define DEBUG_SSI_SD 1
 
diff --git a/include/hw/sd.h b/include/hw/sd/sd.h
similarity index 100%
rename from include/hw/sd.h
rename to include/hw/sd/sd.h
-- 
2.1.4

[Qemu-devel] [PATCH V6 2/2] sdhci: Split sdhci.h for public and internal device usage

[Sai Pavan Boddu]

Split sdhci.h into Pubilc version (i.e include/hw/sd/sdhci.h) and
Internal version (i.e hw/sd/sdhci-interna.h) based on register
declarations and object declaration.

Signed-off-by: Sai Pavan Boddu 
---
Changes for V6:
Fix commit message.
Chages for V5:
Rename pubilc header version as sdhci.h and internal version to
sdhci-internal.h
Changes for V4:
Remain the name of internal version of sdchi.h as same. And change
Re-Adding qemu-common.h header.
the one which is in includes/ to sdhci-common.h
Changes for V2:
Create new area in includes for sd. And move sdhci.h to same.
Changes for V3:
Split the headers to public and common.
---
 hw/sd/{sdhci.h => sdhci-internal.h} | 67 +--
 hw/sd/sdhci.c   |  3 +-
 include/hw/sd/sdhci.h   | 92 +
 3 files changed, 95 insertions(+), 67 deletions(-)
 rename hw/sd/{sdhci.h => sdhci-internal.h} (75%)
 create mode 100644 include/hw/sd/sdhci.h

diff --git a/hw/sd/sdhci.h b/hw/sd/sdhci-internal.h
similarity index 75%
rename from hw/sd/sdhci.h
rename to hw/sd/sdhci-internal.h
index a45593f..c40ae2b 100644
--- a/hw/sd/sdhci.h
+++ b/hw/sd/sdhci-internal.h
@@ -21,14 +21,10 @@
  * You should have received a copy of the GNU General Public License along
  * with this program; if not, see .
  */
-
 #ifndef SDHCI_H
 #define SDHCI_H
 
-#include "qemu-common.h"
-#include "hw/pci/pci.h"
-#include "hw/sysbus.h"
-#include "hw/sd/sd.h"
+#include "hw/sd/sdhci.h"
 
 /* R/W SDMA System Address register 0x0 */
 #define SDHC_SYSAD 0x00
@@ -231,65 +227,6 @@ enum {
 sdhc_gap_write  = 2   /* SDHC stopped at block gap during write operation 
*/
 };
 
-/* SD/MMC host controller state */
-typedef struct SDHCIState {
-union {
-PCIDevice pcidev;
-SysBusDevice busdev;
-};
-SDState *card;
-MemoryRegion iomem;
-
-QEMUTimer *insert_timer;   /* timer for 'changing' sd card. */
-QEMUTimer *transfer_timer;
-qemu_irq eject_cb;
-qemu_irq ro_cb;
-qemu_irq irq;
-
-uint32_t sdmasysad;/* SDMA System Address register */
-uint16_t blksize;  /* Host DMA Buff Boundary and Transfer BlkSize Reg 
*/
-uint16_t blkcnt;   /* Blocks count for current transfer */
-uint32_t argument; /* Command Argument Register */
-uint16_t trnmod;   /* Transfer Mode Setting Register */
-uint16_t cmdreg;   /* Command Register */
-uint32_t rspreg[4];/* Response Registers 0-3 */
-uint32_t prnsts;   /* Present State Register */
-uint8_t  hostctl;  /* Host Control Register */
-uint8_t  pwrcon;   /* Power control Register */
-uint8_t  blkgap;   /* Block Gap Control Register */
-uint8_t  wakcon;   /* WakeUp Control Register */
-uint16_t clkcon;   /* Clock control Register */
-uint8_t  timeoutcon;   /* Timeout Control Register */
-uint8_t  admaerr;  /* ADMA Error Status Register */
-uint16_t norintsts;/* Normal Interrupt Status Register */
-uint16_t errintsts;/* Error Interrupt Status Register */
-uint16_t norintstsen;  /* Normal Interrupt Status Enable Register */
-uint16_t errintstsen;  /* Error Interrupt Status Enable Register */
-uint16_t norintsigen;  /* Normal Interrupt Signal Enable Register */
-uint16_t errintsigen;  /* Error Interrupt Signal Enable Register */
-uint16_t acmd12errsts; /* Auto CMD12 error status register */
-uint64_t admasysaddr;  /* ADMA System Address Register */
-
-uint32_t capareg;  /* Capabilities Register */
-uint32_t maxcurr;  /* Maximum Current Capabilities Register */
-uint8_t  *fifo_buffer; /* SD host i/o FIFO buffer */
-uint32_t buf_maxsz;
-uint16_t data_count;   /* current element in FIFO buffer */
-uint8_t  stopped_state;/* Current SDHC state */
-/* Buffer Data Port Register - virtual access point to R and W buffers */
-/* Software Reset Register - always reads as 0 */
-/* Force Event Auto CMD12 Error Interrupt Reg - write only */
-/* Force Event Error Interrupt Register- write only */
-/* RO Host Controller Version Register always reads as 0x2401 */
-} SDHCIState;
-
 extern const VMStateDescription sdhci_vmstate;
 
-#define TYPE_PCI_SDHCI "sdhci-pci"
-#define PCI_SDHCI(obj) OBJECT_CHECK(SDHCIState, (obj), TYPE_PCI_SDHCI)
-
-#define TYPE_SYSBUS_SDHCI "generic-sdhci"
-#define SYSBUS_SDHCI(obj)   \
- OBJECT_CHECK(SDHCIState, (obj), TYPE_SYSBUS_SDHCI)
-
-#endif /* SDHCI_H */
+#endif
diff --git a/hw/sd/sdhci.c b/hw/sd/sdhci.c
index e63367b..34018fd 100644
--- a/hw/sd/sdhci.c
+++ b/hw/sd/sdhci.c
@@ -28,8 +28,7 @@
 #include "sysemu/dma.h"
 #include "qemu/timer.h"
 #include "qemu/bitops.h"
-
-#include "sdhci.h"
+#include "sdhci-internal.h"
 
 /* host controller debug messages */
 #ifndef SDHC_DEBUG
diff --git a/include/hw/sd/sdhci.h 

Re: [Qemu-devel] [Qemu-block] [PATCH] block/nfs: add support for setting debug level

[Peter Lieven]

Am 25.06.2015 um 15:18 schrieb Stefan Hajnoczi:

On Tue, Jun 23, 2015 at 10:12:15AM +0200, Peter Lieven wrote:

upcoming libnfs versions will support logging debug messages. Add
support for it in qemu through an URL parameter.

Signed-off-by: Peter Lieven 
---
  block/nfs.c | 4 
  1 file changed, 4 insertions(+)

diff --git a/block/nfs.c b/block/nfs.c
index ca9e24e..f7388a3 100644
--- a/block/nfs.c
+++ b/block/nfs.c
@@ -329,6 +329,10 @@ static int64_t nfs_client_open(NFSClient *client, const 
char *filename,
  } else if (!strcmp(qp->p[i].name, "readahead")) {
  nfs_set_readahead(client->context, val);
  #endif
+#ifdef LIBNFS_FEATURE_DEBUG
+} else if (!strcmp(qp->p[i].name, "debug")) {
+nfs_set_debug(client->context, val);
+#endif
  } else {
  error_setg(errp, "Unknown NFS parameter name: %s",
 qp->p[i].name);

Untrusted users may be able to set these options since they are encoded
in the URI.  I'm imagining a hosting or cloud scenario like OpenStack.

A verbose debug level spams stderr and could consume a lot of disk
space.

(The uid and gid options are probably okay since the NFS server cannot
trust the uid/gid coming from QEMU anyway.)

I think we can merge this patch for QEMU 2.4 but I'd like to have a
discussion about the security risk of encoding libnfs options in the
URI.

CCed Eric Blake in case libvirt is affected.

Has anyone thought about this and what are the rules?


As I hadn't time to work further on the best way to add options for NFS (and 
other
protocols), would it be feasible to allow passing debug as an URL parameter, but
limit the maximum debug level to limit a possible security impact (flooding 
logs)?

If a higher debug level is needed it can be set via device specific options as 
soon
there is a common scheme for them.

Peter

Re: [Qemu-devel] [PATCH v6 3/4] block: add a 'blockdev-snapshot' QMP command

[Max Reitz]

On 22.09.2015 15:28, Alberto Garcia wrote:
> One of the limitations of the 'blockdev-snapshot-sync' command is that
> it does not allow passing BlockdevOptions to the newly created
> snapshots, so they are always opened using the default values.
> 
> Extending the command to allow passing options is not a practical
> solution because there is overlap between those options and some of
> the existing parameters of the command.
> 
> This patch introduces a new 'blockdev-snapshot' command with a simpler
> interface: it just takes two references to existing block devices that
> will be used as the source and target for the snapshot.
> 
> Since the main difference between the two commands is that one of them
> creates and opens the target image, while the other uses an already
> opened one, the bulk of the implementation is shared.
> 
> Signed-off-by: Alberto Garcia 
> Cc: Eric Blake 
> Cc: Max Reitz 
> ---
>  blockdev.c   | 163 
> ---
>  qapi-schema.json |   2 +
>  qapi/block-core.json |  28 +
>  qmp-commands.hx  |  38 
>  4 files changed, 171 insertions(+), 60 deletions(-)
> 

[snip]

> diff --git a/qmp-commands.hx b/qmp-commands.hx
> index 495670b..e5bd0e0 100644
> --- a/qmp-commands.hx
> +++ b/qmp-commands.hx
> @@ -1454,6 +1454,44 @@ Example:
>  EQMP
>  
>  {
> +.name   = "blockdev-snapshot",
> +.args_type  = "node:s,overlay:s",
> +.mhandler.cmd_new = qmp_marshal_input_blockdev_snapshot,

As of 7fad30f06eb6aa57aaa8f3d264288f24ae7646f0, this needs to be
qmp_marshal_blockdev_snapshot.

> +},
> +
> +SQMP
> +blockdev-snapshot
> +-
> +Since 2.5
> +
> +Create a snapshot, by installing 'node' as the backing image of
> +'overlay'. Additionally, if 'node' is associated with a block
> +device, the block device changes to using 'overlay' as its new active
> +image.
> +
> +Arguments:
> +
> +- "node": device that will have a snapshot created (json-string)
> +- "overlay": device that will have 'node' as its backing image (json-string)
> +
> +Example:
> +
> +-> { "execute": "blockdev-add",
> +"arguments": { "options": { "driver": "qcow2",
> +"node-name": "node1534",
> +"file": { "driver": "file",
> +  "filename": 
> "hd1.qcow2" },
> +"backing": "" } } }
> +
> +<- { "return": {} }
> +
> +-> { "execute": "blockdev-snapshot", "arguments": { "node": "ide-hd0",
> +"overlay": "node1534" } }
> +<- { "return": {} }
> +
> +EQMP
> +
> +{
>  .name   = "blockdev-snapshot-internal-sync",
>  .args_type  = "device:B,name:s",
>  .mhandler.cmd_new = 
> qmp_marshal_input_blockdev_snapshot_internal_sync,

Consequently, this context needs to be fixed up, too.

With that changed:

Reviewed-by: Max Reitz 



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH v2 11/22] target-cris: Mirror gen_opc_pc into insn_start

[Aurelien Jarno]

On 2015-09-17 21:55, Richard Henderson wrote:
> This perhaps isn't ideal in terms of (ab)using the "pc" field
> to encode both pc and ppc + delay branch state, as one has to
> be aware of this when examining opcode dumps.
> 
> But it preserves existing logic, which will be good for bisection,
> and it certainly does save storage space.
> 
> Reviewed-by: Peter Maydell 
> Signed-off-by: Richard Henderson 
> ---
>  target-cris/translate.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)

Reviewed-by: Aurelien Jarno 

-- 
Aurelien Jarno  GPG: 4096R/1DDD8C9B
aurel...@aurel32.net http://www.aurel32.net

Re: [Qemu-devel] [PATCH] docs: describe the QEMU build system structure / design

[John Snow]

Reviewed from an en_US perspective, though I left alone things that are
clearly regional (e.g. 'behaviour' vs 'behavior')

On 09/22/2015 12:35 PM, Daniel P. Berrange wrote:
> Developers who are new to QEMU, or have a background familiarity
> with GNU autotools can have trouble getting their head around the
> home-grown QEMU build system. This document attempts to explain
> the structure / design of the configure script and the various
> Makefile pieces that live across the source tree.
> 
> Signed-off-by: Daniel P. Berrange 
> ---
>  docs/build-system.txt | 493 
> ++
>  1 file changed, 493 insertions(+)
>  create mode 100644 docs/build-system.txt
> 
> diff --git a/docs/build-system.txt b/docs/build-system.txt
> new file mode 100644
> index 000..2209261
> --- /dev/null
> +++ b/docs/build-system.txt
> @@ -0,0 +1,493 @@
> +The QEMU build system architecture
> +==
> +
> +This document aims to help developers understand the architecture of the
> +QEMU build system. As with projects using GNU autotools, the QEMU build
> +system has two stages, first the developer runs the "configure" script
> +to determine the local build environment characteristics, then they run
> +"make" to build the project. There is about where the similarities with
> +GNU autotools end, so try to forget what you know about them.
> +
> +
> +Stage 1: configure
> +==
> +
> +The QEMU configure script is written directly in shell, and should be
> +compatible with any POSIX shell, hence it uses #!/bin/sh. An important
> +implication of this is that it is important to avoid using bash-isms on
> +development platforms where bash is the primary host.
> +
> +In contrast to autoconf scripts, QEMU's configure is expected to be
> +silent while it is checking for features. It will only display output
> +when an error occurrs, or to show the final feature enablement summary

occurs.

> +on completion.
> +
> +Adding new checks to the configure script usually comprises the
> +following tasks
> +

colon for consistency with other lists below

> + - Initialize one or more variables with the default feature state.
> +
> +   Ideally features should auto-detect whether they are present,
> +   so try to avoid hardcoding the initial state to either enabled
> +   or disabled, as that forces the user to pass a --{dis,en}able-XXX
> +   flag on every invokation of configure
> +

invocation?

> + - Add support to the command line arg parser to handle any new
> +   --{dis,en}able-XXX flags required by the feature XXX
> +
> + - Add information to the help output message to report on the new
> +   feature flag.
> +

Remove period, or add to the other list items for consistency. My
personal preference is to use the period for any sentences with proper
grammatical structure, omitting it for simple list items.

> + - Add code to perform the actual feature check. As noted above, try to
> +   be fully dynamic in checking enablement/disablement
> +
> + - Add code to print out the feature status in the configure summary
> +   upon completion
> +
> + - Add any new makefile variables to $config_host_mak on completion
> +
> +
> +Taking (a simplified version of) the probe for gnutls from configure,
> +we have the following pieces:
> +
> +  # Initial variable state
> +  gnutls=""
> +
> +  ..snip..
> +
> +
> +  # Configure flag processing
> +  --disable-gnutls) gnutls="no"
> +  ;;
> +  --enable-gnutls) gnutls="yes"
> +  ;;
> +
> +  ..snip..
> +
> +
> +  # Help output feature message
> +  gnutls  GNUTLS cryptography support
> +
> +  ..snip..
> +
> +
> +  # Test for gnutls
> +  if test "$gnutls" != "no"; then
> + if ! $pkg_config --exists "gnutls"; then
> +gnutls_cflags=`$pkg_config --cflags gnutls`
> +gnutls_libs=`$pkg_config --libs gnutls`
> +libs_softmmu="$gnutls_libs $libs_softmmu"
> +libs_tools="$gnutls_libs $libs_tools"
> +QEMU_CFLAGS="$QEMU_CFLAGS $gnutls_cflags"
> +gnutls="yes"
> + elif test "$gnutls" = "yes"; then
> +feature_not_found "gnutls" "Install gnutls devel"
> + else
> +gnutls="no"
> + fi
> +  fi
> +
> +  ..snip..
> +
> +
> +  # Completion feature summary
> +  echo "GNUTLS support$gnutls"
> +
> +  ..snip..
> +
> +
> +  # Define make variables
> +  if test "$gnutls" = "yes" ; then
> + echo "CONFIG_GNUTLS=y" >> $config_host_mak
> +  fi
> +
> +
> +Helper functions
> +
> +
> +The configure script provides a variety of helper functions to assist
> +developers in checking for system features:
> +
> + - do_cc $ARGS...
> +
> +   Attempt to run the system C compiler passing it $ARGS...
> +
> + - do_cxx $ARGS...
> +
> +   Attempt to run the system C++ compiler passing it $ARGS...
> +
> + - compile_object $CFLAGS
> +
> +   Attempt to compile a test program with the system C compiler using
> +   $CFLAGS. The test program must have been 

Re: [Qemu-devel] [PATCH] docs: describe the QEMU build system structure / design

[Laszlo Ersek]

On 09/22/15 18:35, Daniel P. Berrange wrote:
> Developers who are new to QEMU, or have a background familiarity
> with GNU autotools can have trouble getting their head around the
> home-grown QEMU build system. This document attempts to explain
> the structure / design of the configure script and the various
> Makefile pieces that live across the source tree.
> 
> Signed-off-by: Daniel P. Berrange 
> ---
>  docs/build-system.txt | 493 
> ++
>  1 file changed, 493 insertions(+)
>  create mode 100644 docs/build-system.txt

Wow, this is awesome. Just a few comments (I'm rather a happy student of
this document than an expert on the build system):

> 
> diff --git a/docs/build-system.txt b/docs/build-system.txt
> new file mode 100644
> index 000..2209261
> --- /dev/null
> +++ b/docs/build-system.txt
> @@ -0,0 +1,493 @@
> +The QEMU build system architecture
> +==
> +
> +This document aims to help developers understand the architecture of the
> +QEMU build system. As with projects using GNU autotools, the QEMU build
> +system has two stages, first the developer runs the "configure" script
> +to determine the local build environment characteristics, then they run
> +"make" to build the project. There is about where the similarities with
> +GNU autotools end, so try to forget what you know about them.
> +
> +
> +Stage 1: configure
> +==
> +
> +The QEMU configure script is written directly in shell, and should be
> +compatible with any POSIX shell, hence it uses #!/bin/sh. An important
> +implication of this is that it is important to avoid using bash-isms on
> +development platforms where bash is the primary host.
> +
> +In contrast to autoconf scripts, QEMU's configure is expected to be
> +silent while it is checking for features. It will only display output
> +when an error occurrs, or to show the final feature enablement summary
> +on completion.
> +
> +Adding new checks to the configure script usually comprises the
> +following tasks
> +
> + - Initialize one or more variables with the default feature state.
> +
> +   Ideally features should auto-detect whether they are present,
> +   so try to avoid hardcoding the initial state to either enabled
> +   or disabled, as that forces the user to pass a --{dis,en}able-XXX
> +   flag on every invokation of configure
> +
> + - Add support to the command line arg parser to handle any new
> +   --{dis,en}able-XXX flags required by the feature XXX

Suggest to spell out --disable-XXX and --enable-XXX separately, so that
they be the first hits when someone searches this file for "--enable"
and "--disable".

> +
> + - Add information to the help output message to report on the new
> +   feature flag.
> +
> + - Add code to perform the actual feature check. As noted above, try to
> +   be fully dynamic in checking enablement/disablement
> +
> + - Add code to print out the feature status in the configure summary
> +   upon completion
> +
> + - Add any new makefile variables to $config_host_mak on completion
> +
> +
> +Taking (a simplified version of) the probe for gnutls from configure,
> +we have the following pieces:
> +
> +  # Initial variable state
> +  gnutls=""
> +
> +  ..snip..
> +
> +

suggest to add only one empty line after "..snip..".

> +  # Configure flag processing
> +  --disable-gnutls) gnutls="no"
> +  ;;
> +  --enable-gnutls) gnutls="yes"
> +  ;;
> +
> +  ..snip..
> +
> +
> +  # Help output feature message
> +  gnutls  GNUTLS cryptography support
> +
> +  ..snip..
> +
> +
> +  # Test for gnutls
> +  if test "$gnutls" != "no"; then
> + if ! $pkg_config --exists "gnutls"; then
> +gnutls_cflags=`$pkg_config --cflags gnutls`
> +gnutls_libs=`$pkg_config --libs gnutls`
> +libs_softmmu="$gnutls_libs $libs_softmmu"
> +libs_tools="$gnutls_libs $libs_tools"
> +QEMU_CFLAGS="$QEMU_CFLAGS $gnutls_cflags"
> +gnutls="yes"
> + elif test "$gnutls" = "yes"; then
> +feature_not_found "gnutls" "Install gnutls devel"
> + else
> +gnutls="no"
> + fi
> +  fi
> +
> +  ..snip..
> +
> +
> +  # Completion feature summary
> +  echo "GNUTLS support$gnutls"
> +
> +  ..snip..
> +
> +
> +  # Define make variables
> +  if test "$gnutls" = "yes" ; then
> + echo "CONFIG_GNUTLS=y" >> $config_host_mak
> +  fi
> +
> +
> +Helper functions
> +
> +
> +The configure script provides a variety of helper functions to assist
> +developers in checking for system features:
> +
> + - do_cc $ARGS...
> +
> +   Attempt to run the system C compiler passing it $ARGS...
> +
> + - do_cxx $ARGS...
> +
> +   Attempt to run the system C++ compiler passing it $ARGS...
> +
> + - compile_object $CFLAGS
> +
> +   Attempt to compile a test program with the system C compiler using
> +   $CFLAGS. The test program must have been previously written to a file
> +   called $TMPC.
> +
> + - 

Re: [Qemu-devel] [PATCH v2 REPOST] oslib-win32: only provide localtime_r/gmtime_r if missing

[Stefan Weil]

Hi,

I suggest cleaning some comments, mostly using the "official"
spellings for MinGW and Mingw-w64.

Am 22.09.2015 um 16:13 schrieb Daniel P. Berrange:
> The oslib-win32 file currently provides a localtime_r and
> gmtime_r replacement unconditionally. Some versions of
> Mingw64 would provide crude macros for localtime_r/gmtime_r

MinGW / Mingw-w64

> which QEMU takes care to disable. Latest versions of Mingw64

Mingw-w64

> now provide actual functions for localtime_r/gmtime_r, but
> with a twist that you have to include unistd.h or pthread.h
> before including time.h.  By luck some files in QEMU have
> such an include order, resulting in compile errors:
> 
>   CCutil/osdep.o
> In file included from include/qemu-common.h:48:0,
>  from util/osdep.c:48:
> include/sysemu/os-win32.h:77:12: error: redundant redeclaration of 'gmtime_r' 
> [-Werror=redundant-decls]
>  struct tm *gmtime_r(const time_t *timep, struct tm *result);
> ^
> In file included from include/qemu-common.h:35:0,
>  from util/osdep.c:48:
> /usr/i686-w64-mingw32/sys-root/mingw/include/time.h:272:107: note: previous 
> definition of 'gmtime_r' was here
> In file included from include/qemu-common.h:48:0,
>  from util/osdep.c:48:
> include/sysemu/os-win32.h:79:12: error: redundant redeclaration of 
> 'localtime_r' [-Werror=redundant-decls]
>  struct tm *localtime_r(const time_t *timep, struct tm *result);
> ^
> In file included from include/qemu-common.h:35:0,
>  from util/osdep.c:48:
> /usr/i686-w64-mingw32/sys-root/mingw/include/time.h:269:107: note: previous 
> definition of 'localtime_r' was here
> 
> This change adds a configure test to see if localtime_r
> exits, and only enables the QEMU impl if missing. We also
> re-arrange qemu-common.h try attempt to guarantee that all
> source files get unistd.h before time.h and thus see the
> localtime_r/gmtime_r defs.
> 
> Signed-off-by: Daniel P. Berrange 
> ---
>  configure | 34 ++
>  include/qemu/osdep.h  |  4 +++-
>  include/sysemu/os-win32.h |  2 ++
>  util/oslib-win32.c|  2 ++
>  4 files changed, 41 insertions(+), 1 deletion(-)
> 
> diff --git a/configure b/configure
> index 52f5b79..4654be8 100755
> --- a/configure
> +++ b/configure
> @@ -1737,6 +1737,37 @@ else
>  fi
>  
>  ##
> +# Mingw64 localtime_r/gmtime_r check

MinGW / Mingw-w64

> +
> +if test "$mingw32" = "yes"; then
> +# Some versions of Mingw32/64 lack localtime_r

MinGW / Mingw-w64

> +# and gmtime_r entirely

Missing .

> +#
> +# Some versions of Mingw64 define a macro for

Mingw-w64

> +# localtime_r/gmtime_r/etc

Why etc? Missing .

> +#
> +# Some versions of Ming64 will define functions
> +# for localtime_r/gmtime_r, but only if you have
> +# _POSIX_THREAD_SAFE_FUNCTIONS defined. For fun
> +# though, unistd.h and pthread.h both define
> +# that for you.
> +#
> +# So this #undef localtime_r and #include 
> +# are not in fact redundant

Missing .

> +cat > $TMPC << EOF
> +#include 
> +#include 
> +#undef localtime_r
> +int main(void) { localtime_r(NULL, NULL); return 0; }
> +EOF
> +if compile_prog "" "" ; then
> +localtime_r="yes"
> +else
> +localtime_r="no"
> +fi
> +fi
> +
> +##
>  # pkg-config probe
>  
>  if ! has "$pkg_config_exe"; then
> @@ -5050,6 +5081,9 @@ fi
>  if test "$zero_malloc" = "yes" ; then
>echo "CONFIG_ZERO_MALLOC=y" >> $config_host_mak
>  fi
> +if test "$localtime_r" = "yes" ; then
> +  echo "CONFIG_LOCALTIME_R=y" >> $config_host_mak
> +fi
>  if test "$qom_cast_debug" = "yes" ; then
>echo "CONFIG_QOM_CAST_DEBUG=y" >> $config_host_mak
>  fi
> diff --git a/include/qemu/osdep.h b/include/qemu/osdep.h
> index ab3c876..e490028 100644
> --- a/include/qemu/osdep.h
> +++ b/include/qemu/osdep.h
> @@ -38,10 +38,12 @@
>  #include 
>  #include 
>  #include 
> +/* Put unistd.h before time.h as that triggers localtime_r/gmtime_r
> + * function availability on recentish Mingw64 platforms */

Mingw-w64. I'm not sure how long this hack will work.
A future patch could add -DPOSIX_C_SOURCE to the compiler options
if configure has set $localtime_r.

> +#include 
>  #include 
>  #include 
>  #include 
> -#include 
>  #include 
>  #include 
>  #include 
> diff --git a/include/sysemu/os-win32.h b/include/sysemu/os-win32.h
> index 706d85a..13dcef6 100644
> --- a/include/sysemu/os-win32.h
> +++ b/include/sysemu/os-win32.h
> @@ -73,10 +73,12 @@
>  #define siglongjmp(env, val) longjmp(env, val)
>  
>  /* Missing POSIX functions. Don't use MinGW-w64 macros. */
> +#ifndef CONFIG_LOCALTIME_R
>  #undef gmtime_r
>  struct tm *gmtime_r(const time_t *timep, struct tm *result);
>  #undef localtime_r
>  struct tm *localtime_r(const time_t *timep, struct tm *result);
> +#endif /* CONFIG_LOCALTIME_R */
>  

Re: [Qemu-devel] MAINTAINERS leaves too many files uncovered

[Eric Blake]

On 09/22/2015 03:13 AM, Markus Armbruster wrote:

>> Full list of unmaintained files now:
> [...]
> 

> docs/bitmaps.md
> docs/blkdebug.txt
> docs/blkverify.txt

> docs/live-block-ops.txt

> docs/qcow2-cache.txt
> docs/qdev-device-use.txt

> docs/specs/qcow2.txt
> docs/specs/qed_spec.txt

Kevin, Stefan - should these be added to Block maintainers?

> docs/writing-qmp-commands.txt

Markus: Probably belongs to qapi.

> include/qapi/dealloc-visitor.h
> include/qapi/opts-visitor.h
> include/qapi/qmp-event.h
> include/qapi/qmp-input-visitor.h
> include/qapi/qmp-output-visitor.h
> include/qapi/qmp/dispatch.h
> include/qapi/qmp/json-lexer.h
> include/qapi/qmp/json-parser.h
> include/qapi/qmp/json-streamer.h
> include/qapi/qmp/qbool.h
> include/qapi/qmp/qdict.h
> include/qapi/qmp/qerror.h
> include/qapi/qmp/qfloat.h
> include/qapi/qmp/qint.h
> include/qapi/qmp/qjson.h
> include/qapi/qmp/qlist.h
> include/qapi/qmp/qobject.h
> include/qapi/qmp/qstring.h
> include/qapi/qmp/types.h
> include/qapi/string-input-visitor.h
> include/qapi/string-output-visitor.h
> include/qapi/util.h
> include/qapi/visitor-impl.h
> include/qapi/visitor.h

Definitely belongs to qapi.

> include/qemu/throttle.h

Block?

> include/qjson.h

qapi?


> tests/check-qdict.c
> tests/check-qfloat.c
> tests/check-qint.c
> tests/check-qjson.c
> tests/check-qlist.c

> tests/check-qstring.c

qapi?

> tests/test-opts-visitor.c

> tests/test-qmp-commands.c
> tests/test-qmp-event.c
> tests/test-qmp-input-strict.c
> tests/test-qmp-input-visitor.c
> tests/test-qmp-output-visitor.c

> tests/test-string-input-visitor.c
> tests/test-string-output-visitor.c

qapi

Also, should we have some new group for maintainer files, for things like:

.exrc
.gitignore
.gitmodules
.mailmap
.travis.yml
CODING_STYLE
COPYING
COPYING.LIB
Changelog
HACKING
LICENSE
MAINTAINERS

README
VERSION

scripts/get_maintainer.pl


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



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PULL 2/8] hmp: Allow for error message hints on HMP

[Eric Blake]

On 09/22/2015 09:23 AM, Kevin Wolf wrote:
> Am 18.09.2015 um 15:22 hat Markus Armbruster geschrieben:
>> From: Eric Blake 
>>
>> Commits 7216ae3d and d2828429 disabled some error message hints,
>> all because a change to use modern error reporting meant that the
>> hint would be output prior to the actual error.  Fix this by making
>> hints a first-class member of Error.
>>
>> For example, we are now back to the pleasant:
>>
>>  $ qemu-system-x86_64 --nodefaults -S --vnc :0 --chardev null,id=,
>>  qemu-system-x86_64: --chardev null,id=,: Parameter 'id' expects an 
>> identifier
>>  Identifiers consist of letters, digits, '-', '.', '_', starting with a 
>> letter.
>>
>> Signed-off-by: Eric Blake 
>> Reviewed-by: Paolo Bonzini 
>> Message-Id: <1441901956-21991-1-git-send-email-ebl...@redhat.com>
>> Signed-off-by: Markus Armbruster 
> 
> This broke qemu-iotests 049. Can you please send a patch to update the
> reference output?

Will do (and I guess I should run qemu-iotests more frequently. Does
'make check' suffice, or is there other magic involved?)

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



signature.asc
Description: OpenPGP digital signature

[Qemu-devel] [PULL 45/48] mips: Remove ELF_MACHINE from cpu.h

[Paolo Bonzini]

From: Peter Crosthwaite 

The only generic code relying on this is linux-user, but linux users'
default behaviour of defaulting ELF_MACHINE to ELF_ARCH will handle
this.

The bootloaders can just pass EM_MIPS directly, as that is
architecture specific code.

This removes another architecture specific definition from the global
namespace.

Cc: Aurelien Jarno 
Cc: Leon Alrae 
Reviewed-by: Aurelien Jarno 
Reviewed-by: Richard Henderson 
Acked-By: Riku Voipio 
Signed-off-by: Peter Crosthwaite 
Signed-off-by: Paolo Bonzini 
---
 hw/mips/mips_fulong2e.c | 2 +-
 hw/mips/mips_malta.c| 2 +-
 hw/mips/mips_mipssim.c  | 2 +-
 hw/mips/mips_r4k.c  | 2 +-
 target-mips/cpu.h   | 2 --
 5 files changed, 4 insertions(+), 6 deletions(-)

diff --git a/hw/mips/mips_fulong2e.c b/hw/mips/mips_fulong2e.c
index e44521f..5988a88 100644
--- a/hw/mips/mips_fulong2e.c
+++ b/hw/mips/mips_fulong2e.c
@@ -116,7 +116,7 @@ static int64_t load_kernel (CPUMIPSState *env)
 
 if (load_elf(loaderparams.kernel_filename, cpu_mips_kseg0_to_phys, NULL,
  (uint64_t *)_entry, (uint64_t *)_low,
- (uint64_t *)_high, 0, ELF_MACHINE, 1) < 0) {
+ (uint64_t *)_high, 0, EM_MIPS, 1) < 0) {
 fprintf(stderr, "qemu: could not load kernel '%s'\n",
 loaderparams.kernel_filename);
 exit(1);
diff --git a/hw/mips/mips_malta.c b/hw/mips/mips_malta.c
index bb864fe..c1f570a 100644
--- a/hw/mips/mips_malta.c
+++ b/hw/mips/mips_malta.c
@@ -795,7 +795,7 @@ static int64_t load_kernel (void)
 
 if (load_elf(loaderparams.kernel_filename, cpu_mips_kseg0_to_phys, NULL,
  (uint64_t *)_entry, NULL, (uint64_t *)_high,
- big_endian, ELF_MACHINE, 1) < 0) {
+ big_endian, EM_MIPS, 1) < 0) {
 fprintf(stderr, "qemu: could not load kernel '%s'\n",
 loaderparams.kernel_filename);
 exit(1);
diff --git a/hw/mips/mips_mipssim.c b/hw/mips/mips_mipssim.c
index e651312..23b35be 100644
--- a/hw/mips/mips_mipssim.c
+++ b/hw/mips/mips_mipssim.c
@@ -69,7 +69,7 @@ static int64_t load_kernel(void)
 kernel_size = load_elf(loaderparams.kernel_filename, 
cpu_mips_kseg0_to_phys,
NULL, (uint64_t *), NULL,
(uint64_t *)_high, big_endian,
-   ELF_MACHINE, 1);
+   EM_MIPS, 1);
 if (kernel_size >= 0) {
 if ((entry & ~0x7fffULL) == 0x8000)
 entry = (int32_t)entry;
diff --git a/hw/mips/mips_r4k.c b/hw/mips/mips_r4k.c
index 97628fc..af10da1 100644
--- a/hw/mips/mips_r4k.c
+++ b/hw/mips/mips_r4k.c
@@ -87,7 +87,7 @@ static int64_t load_kernel(void)
 kernel_size = load_elf(loaderparams.kernel_filename, 
cpu_mips_kseg0_to_phys,
NULL, (uint64_t *), NULL,
(uint64_t *)_high, big_endian,
-   ELF_MACHINE, 1);
+   EM_MIPS, 1);
 if (kernel_size >= 0) {
 if ((entry & ~0x7fffULL) == 0x8000)
 entry = (int32_t)entry;
diff --git a/target-mips/cpu.h b/target-mips/cpu.h
index ed7d86d..ec5f991 100644
--- a/target-mips/cpu.h
+++ b/target-mips/cpu.h
@@ -5,8 +5,6 @@
 
 #define ALIGNED_ONLY
 
-#define ELF_MACHINEEM_MIPS
-
 #define CPUArchState struct CPUMIPSState
 
 #include "config.h"
-- 
2.5.0

Re: [Qemu-devel] [PATCH v7 11/14] block/backup: support block job transactions

[Eric Blake]

On 09/21/2015 08:46 PM, Fam Zheng wrote:
> From: Stefan Hajnoczi 
> 
> Join the transaction when the 'transactional-cancel' QMP argument is
> true.
> 
> This ensures that the sync bitmap is not thrown away if another block
> job in the transaction is cancelled or fails.  This is critical so
> incremental backup with multiple disks can be retried in case of
> cancellation/failure.
> 
> Signed-off-by: Stefan Hajnoczi 
> Reviewed-by: Max Reitz 
> Reviewed-by: Eric Blake 
> Signed-off-by: Fam Zheng 
> ---

> +++ b/qapi/block-core.json
> @@ -735,7 +735,6 @@
>  # @on-target-error: #optional the action to take on an error on the target,
>  #   default 'report' (no limitations, since this applies to
>  #   a different block device than @device).
> -#
>  # Note that @on-source-error and @on-target-error only affect background I/O.
>  # If an error occurs during a guest write request, the device's rerror/werror
>  # actions will be used.

Spurious line deletion when addressing review comment. Maintainer can
touch it up for the pull request, so my R-b still stands.

-- 
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 02/16] vmdk: Use BdrvChild instead of BDS for references to extents

[Max Reitz]

On 17.09.2015 15:48, Kevin Wolf wrote:
> Signed-off-by: Kevin Wolf 
> ---
>  block/vmdk.c | 99 
> +++-
>  1 file changed, 51 insertions(+), 48 deletions(-)

Reviewed-by: Max Reitz 



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH v5 03/46] qapi: Test for C member name collisions

[Eric Blake]

On 09/22/2015 09:23 AM, Markus Armbruster wrote:
> Eric Blake  writes:
> 
>> Expose some weaknesses in the generator: we don't always forbid
>> the generation of structs that contain multiple members that map
> 
> Slightly misleading.  args-name-clash is a clash between command
> arguments.  These are a struct internally, but we don't currently
> generate an actual struct for it, only an argument list.

Maybe struct-member-clash?  Renames are easy enough, but only if patch
1/46 is okay to go in first. :)

> 
>> to the same C name.  This has already been marked FIXME in
>> qapi.py, but having more tests will make sure future patches
>> produce desired behavior.
> 
> Point to commit d90675f?

Sure, now that it finally landed.

> 
>> Some of these tests will be deleted later, and a positive test
>> added to qapi-schema-test.json in its place, when the code is
> 
> "in their place"?
> 

Yep. (Perils of editing, I started with one test, then added more later
and merged into one patch)

>> reworked so that the collision no longer occurs.
>>
>> Signed-off-by: Eric Blake 
>> ---

>> +++ b/tests/qapi-schema/flat-union-branch-clash.json
>> @@ -1,4 +1,4 @@
>> -# we check for no duplicate keys between branches and base
>> +# we check for no duplicate keys between branch members and base
>>  { 'enum': 'TestEnum',
>>'data': [ 'value1', 'value2' ] }
>>  { 'struct': 'Base',
> 
> This clashing business is awfully confusing as soon as unions come into
> play.  When I'm confused, I need to think in writing.

No kidding.  We already attempted to detect clashes, and caught some,
but not all, types of clashes.  And there are indeed two types of member
name clashes: those where the generated C struct has duplicate members
(either because 2 user names map to the same C name, or because the
generated code injects a C name for a purpose other than a "key":value
name), and those where the qapi type would specify the same "key":value
name more than once in the same {} object on the wire (even if the names
would not collide in C because one is accessed through a box pointer).
By patch 16/46, we should be catching all cases of member name clashes,
but there's still work to do to catch collisions in 'command' and/or
'event' names.

Also, by the time 16/46 is in, there are cases where we reject "clashes"
where two member names with different spellings would map to the same C
name, but where the corresponding C struct does not have a clash because
the members are boxed behind different pointers.  Technically, we would
not have to reject such cases, but the case is still confusing enough
that rejecting it forces the qapi writer to consider a naming convention
that is less confusing in the first place.

> 
> The basic case is clash between local, non-variant members.  Needs test
> coverage.  args-name-clash.json provides it, because internally the
> arguments are just another object type.

Correct.  The test proves we don't yet catch the clash, and is fixed
when later commits add the check.

> 
> With a base, the members inherited from base get added to the mix.  We
> need to test a clash betwen local, non-variant member and a member
> inherited from base.

True for both structs and flat unions (the two places where we use
'base').  More on this below.

> 
> With unions, things get complicated, because we have multiple kinds of
> clashes.  Best explained with an example.  Let's use UserDefFlatUnion
> from qapi-schema-test.json.
> 
> { 'union': 'UserDefFlatUnion',
>   'base': 'UserDefUnionBase',   # intentional forward reference
>   'discriminator': 'enum1',
>   'data': { 'value1' : 'UserDefA',
> 'value2' : 'UserDefB',
> 'value3' : 'UserDefB' } }
> 
> { 'struct': 'UserDefUnionBase',
>   'base': 'UserDefZero',
>   'data': { 'string': 'str', 'enum1': 'EnumOne' } }
> 
> Generated C looks like this:
> 
> struct UserDefFlatUnion {
> /* Members inherited from UserDefUnionBase: */
> int64_t integer;
> char *string;
> EnumOne enum1;
> /* Own members: */
> // if the schema language supported adding non-variant local
> // members, they'd go right here
> union { /* union tag is @enum1 */
> void *data;
> UserDefA *value1;
> UserDefB *value2;
> UserDefB *value3;
> };
> };
> 
> Thus, what can clash in C is the tag values value1, value2, value3 with
> the non-variant members integer, string, enum1.

That is, the tag values now appear as C member names, even though they
did not correspond to QMP "key":value names.  Likewise, the 'data' C
member name can cause a clash.

Was even worse before commit 0f61af3e, where we were also burning the C
name 'kind'.

Commit 1e6c1616 was where we quit burning the C member name 'base'.
Prior to that time, members of base classes did not clash with variant
names because of the C boxing.

If we run 

[Qemu-devel] [PATCH] docs: describe the QEMU build system structure / design

[Daniel P. Berrange]

Developers who are new to QEMU, or have a background familiarity
with GNU autotools can have trouble getting their head around the
home-grown QEMU build system. This document attempts to explain
the structure / design of the configure script and the various
Makefile pieces that live across the source tree.

Signed-off-by: Daniel P. Berrange 
---
 docs/build-system.txt | 493 ++
 1 file changed, 493 insertions(+)
 create mode 100644 docs/build-system.txt

diff --git a/docs/build-system.txt b/docs/build-system.txt
new file mode 100644
index 000..2209261
--- /dev/null
+++ b/docs/build-system.txt
@@ -0,0 +1,493 @@
+The QEMU build system architecture
+==
+
+This document aims to help developers understand the architecture of the
+QEMU build system. As with projects using GNU autotools, the QEMU build
+system has two stages, first the developer runs the "configure" script
+to determine the local build environment characteristics, then they run
+"make" to build the project. There is about where the similarities with
+GNU autotools end, so try to forget what you know about them.
+
+
+Stage 1: configure
+==
+
+The QEMU configure script is written directly in shell, and should be
+compatible with any POSIX shell, hence it uses #!/bin/sh. An important
+implication of this is that it is important to avoid using bash-isms on
+development platforms where bash is the primary host.
+
+In contrast to autoconf scripts, QEMU's configure is expected to be
+silent while it is checking for features. It will only display output
+when an error occurrs, or to show the final feature enablement summary
+on completion.
+
+Adding new checks to the configure script usually comprises the
+following tasks
+
+ - Initialize one or more variables with the default feature state.
+
+   Ideally features should auto-detect whether they are present,
+   so try to avoid hardcoding the initial state to either enabled
+   or disabled, as that forces the user to pass a --{dis,en}able-XXX
+   flag on every invokation of configure
+
+ - Add support to the command line arg parser to handle any new
+   --{dis,en}able-XXX flags required by the feature XXX
+
+ - Add information to the help output message to report on the new
+   feature flag.
+
+ - Add code to perform the actual feature check. As noted above, try to
+   be fully dynamic in checking enablement/disablement
+
+ - Add code to print out the feature status in the configure summary
+   upon completion
+
+ - Add any new makefile variables to $config_host_mak on completion
+
+
+Taking (a simplified version of) the probe for gnutls from configure,
+we have the following pieces:
+
+  # Initial variable state
+  gnutls=""
+
+  ..snip..
+
+
+  # Configure flag processing
+  --disable-gnutls) gnutls="no"
+  ;;
+  --enable-gnutls) gnutls="yes"
+  ;;
+
+  ..snip..
+
+
+  # Help output feature message
+  gnutls  GNUTLS cryptography support
+
+  ..snip..
+
+
+  # Test for gnutls
+  if test "$gnutls" != "no"; then
+ if ! $pkg_config --exists "gnutls"; then
+gnutls_cflags=`$pkg_config --cflags gnutls`
+gnutls_libs=`$pkg_config --libs gnutls`
+libs_softmmu="$gnutls_libs $libs_softmmu"
+libs_tools="$gnutls_libs $libs_tools"
+QEMU_CFLAGS="$QEMU_CFLAGS $gnutls_cflags"
+gnutls="yes"
+ elif test "$gnutls" = "yes"; then
+feature_not_found "gnutls" "Install gnutls devel"
+ else
+gnutls="no"
+ fi
+  fi
+
+  ..snip..
+
+
+  # Completion feature summary
+  echo "GNUTLS support$gnutls"
+
+  ..snip..
+
+
+  # Define make variables
+  if test "$gnutls" = "yes" ; then
+ echo "CONFIG_GNUTLS=y" >> $config_host_mak
+  fi
+
+
+Helper functions
+
+
+The configure script provides a variety of helper functions to assist
+developers in checking for system features:
+
+ - do_cc $ARGS...
+
+   Attempt to run the system C compiler passing it $ARGS...
+
+ - do_cxx $ARGS...
+
+   Attempt to run the system C++ compiler passing it $ARGS...
+
+ - compile_object $CFLAGS
+
+   Attempt to compile a test program with the system C compiler using
+   $CFLAGS. The test program must have been previously written to a file
+   called $TMPC.
+
+ - compile_prog $CFLAGS $LDFLAGS
+
+   Attempt to compile a test program with the system C compiler using
+   $CFLAGS and link it with the system linker using $LDFLAGS. The test
+   program must have been previously written to a file called $TMPC.
+
+ - has $COMMAND
+
+   Determine if $COMMAND exists in the current environment, either as a
+   shell builtin, or executable binary, returning 0 on success.
+
+ - path_of $COMMAND
+
+   Return the fully qualified path of $COMMAND, printing it to stdout,
+   and returning 0 on success.
+
+ - check_define $NAME
+
+   Determine if the macro $NAME is defined by the system C compiler
+
+ - check_include $NAME
+
+   Determine if the 

Re: [Qemu-devel] [PATCH v6 4/4] block: add tests for the 'blockdev-snapshot' command

[Max Reitz]

On 22.09.2015 15:28, Alberto Garcia wrote:
> Signed-off-by: Alberto Garcia 
> Cc: Max Reitz 
> ---
>  tests/qemu-iotests/085 | 102 
> ++---
>  tests/qemu-iotests/085.out |  34 ++-
>  2 files changed, 128 insertions(+), 8 deletions(-)

Reviewed-by: Max Reitz 



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH 01/16] block: Introduce BDS.file_child

[Max Reitz]

On 17.09.2015 15:48, Kevin Wolf wrote:
> Store the BdrvChild for bs->file. At this point, bs->file_child->bs just
> duplicates the bs->file pointer. Later, it will completely replace it.
> 
> Signed-off-by: Kevin Wolf 
> ---
>  block.c   | 12 +---
>  include/block/block_int.h |  1 +
>  2 files changed, 10 insertions(+), 3 deletions(-)

Reviewed-by: Max Reitz 

Although I have a small comment below:

> 
> diff --git a/block.c b/block.c
> index 6268e37..2e9e5e2 100644
> --- a/block.c
> +++ b/block.c
> @@ -1487,11 +1487,17 @@ static int bdrv_open_inherit(BlockDriverState **pbs, 
> const char *filename,
>  
>  assert(file == NULL);

This looks strange now, without the direct connection to
bdrv_open_image(). I'd drop this...

>  bs->open_flags = flags;
> -ret = bdrv_open_image(, filename, options, "file",
> -  bs, _file, true, _err);
> -if (ret < 0) {
> +
> +bs->file_child = bdrv_open_child(filename, options, "file", bs,
> + _file, true, _err);
> +if (local_err) {
> +ret = -EINVAL;
>  goto fail;
>  }
> +
> +if (bs->file_child) {
> +file = bs->file_child->bs;
> +}

And make this file = bs->file_child ? bs->file_child->bs : NULL (or the
respective long form, i.e.
if (bs->file_child) { ... } else { file = NULL; }).

We could even put this after this if block and drop the NULL
initialization of file, but that might go overboard.

Max

>  }
>  
>  /* Image format probing */
> diff --git a/include/block/block_int.h b/include/block/block_int.h
> index 14ad4c3..d0dd93e 100644
> --- a/include/block/block_int.h
> +++ b/include/block/block_int.h
> @@ -381,6 +381,7 @@ struct BlockDriverState {
>  BlockDriverState *backing_hd;
>  BdrvChild *backing_child;
>  BlockDriverState *file;
> +BdrvChild *file_child;
>  
>  NotifierList close_notifiers;
>  
> 




signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH v2 2/2] PCI-e device multi-function hot-add support

[Alex Williamson]

On Tue, 2015-09-22 at 18:08 +0800, Cao jin wrote:
> Hi Alex
> 
> On 09/22/2015 02:00 AM, Alex Williamson wrote:
> >
> > Please use different subjects that uniquely identify what each patch
> > does, don't simply re-use the subject for the cover patch on each.
> 
> OK, will change it in next version.
> >
> > On Wed, 2015-09-16 at 10:02 +0800, Cao jin wrote:
> >> In case user regret when hot-add multi-function, we should roll back,
> >> device_del the function added but still not worked.
> >>
> >> Signed-off-by: Cao jin 
> >> ---
> >>   hw/pci/pcie.c | 18 ++
> >>   1 file changed, 18 insertions(+)
> >>
> >> diff --git a/hw/pci/pcie.c b/hw/pci/pcie.c
> >> index 89bf61b..497f390 100644
> >> --- a/hw/pci/pcie.c
> >> +++ b/hw/pci/pcie.c
> >> @@ -265,9 +265,27 @@ void 
> >> pcie_cap_slot_hot_unplug_request_cb(HotplugHandler *hotplug_dev,
> >>DeviceState *dev, Error **errp)
> >>   {
> >>   uint8_t *exp_cap;
> >> +PCIDevice *pci_dev = PCI_DEVICE(dev);
> >> +PCIBus *bus = pci_dev->bus;
> >>
> >>   pcie_cap_slot_hotplug_common(PCI_DEVICE(hotplug_dev), dev, _cap, 
> >> errp);
> >>
> >> +/* handle the condition: user hot-add multi function, but regret 
> >> before
> >> + * finish it, and want to delete the added but not worked function. 
> >> Fake
> >> + * the condition: the slot is polulated, power indicator is off and 
> >> power
> >> + * controller is off, so device can be detached when OS write config 
> >> space.
> >> + */
> >> +if (PCI_FUNC(pci_dev->devfn) > 0 &&
> >> +bus->devices[PCI_DEVFN(0, 0)] == NULL) {
> >> +pci_word_test_and_set_mask(exp_cap + PCI_EXP_SLTSTA,
> >> +PCI_EXP_SLTSTA_PDS);
> >
> > AFAICT, we're only setting this to make pcie_cap_slot_write_config()
> > consider this device for being unplugged.  Would it not be cleaner to
> > flag the device as unexposed to the guest and also use that flag to
> > prevent config reads and writes to the device until function 0 is
> > populated, so we know that the guest hasn't interacted with the device?
> >
> Yes, set PDS bit here, for the purpose that fake the unplug condition in 
> pcie_cap_slot_write_config(), which means, let guest decide when to 
> unplug device. So I think setting PDS bit here is necessary, am I right?

I would consider it a hack.  You're setting up the device a certain way
to make it appear as if the guest has configured it that way, then
effectively sending the guest a spurious hotplug request for a device
that it theoretically doesn't know about.  If we were to prevent access
to the device, couldn't we remove it directly?

> I am not quite clear about "flag device as unexposed", does the flag 
> means PCI_EXP_SLTSTA_PDS bit, or anything else? Could you give more 
> hints about it?

If function 0 doesn't exist in the slot, should the guest be able to
perform PCI config accesses to the device?  If the guest cannot do
config cycle accesses to the device, then we know the device is unused
and we don't need to involve the guest in removing it.

> >> +
> >> +pcie_cap_slot_event(PCI_DEVICE(hotplug_dev),
> >> +PCI_EXP_HP_EV_PDC | PCI_EXP_HP_EV_ABP);
> >
> > Why do we need to test both vs just ABP, which is signaled in the
> > existing patch below?
> >
> 
> Test the two hotplug event, yes, ABP is enough for device_del. will 
> remove PDC in next version.
> 
> >> +
> >> +return;
> >> +}
> >> +
> >>   pcie_cap_slot_push_attention_button(PCI_DEVICE(hotplug_dev));
> >>   }
> >>
> >
> >
> > .
> >
> 

Re: [Qemu-devel] [PATCH 04/16] quorum: Convert to BdrvChild

[Max Reitz]

On 17.09.2015 15:48, Kevin Wolf wrote:
> Signed-off-by: Kevin Wolf 
> ---
>  block/quorum.c | 63 
> ++
>  1 file changed, 33 insertions(+), 30 deletions(-)

Reviewed-by: Max Reitz 



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH] README: fill out some useful quickstart information

[Paolo Bonzini]

On 22/09/2015 10:16, Markus Armbruster wrote:
>> "To build QEMU, you'd generally [some basic steps that have generally
>> worked for a long time], but for up-to-date authoritative information,
>> please do:
>>
>> mkdir path_to/build_dir
>> cd path_to/build_dir
>> ../../configure
>> make qemu-doc.html"
>>
>> We wind up documenting how to almost do a build anyway.

I don't think this is necessary.  We should remove duplicate information
from qemu-doc.texi, developer documentation doesn't belong in manuals.

Paolo

[Qemu-devel] [Bug 1498144] Re: Failure booting hurd with qemu-system-i386 on ARM

[PeteVine]

All right, I'll rebuild and try qemu-gdb.py if still necessary.
Do any fancy CFLAGS make a difference in performance or should they be avoided 
altogether on ARM?

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

Title:
   Failure booting hurd with qemu-system-i386 on ARM

Status in QEMU:
  New

Bug description:
  Trying to boot debian-hurd-20150320.img ends with:

  qemu-system-i386: qemu-coroutine-lock.c:91: qemu_co_queue_restart_all:
  Assertion `qemu_in_coroutine()' failed.

  Program received signal SIGABRT, Aborted.
  __libc_do_syscall ()
  at ../ports/sysdeps/unix/sysv/linux/arm/libc-do-syscall.S:44
  44  ../ports/sysdeps/unix/sysv/linux/arm/libc-do-syscall.S: No such file 
or directory.
  (gdb) bt
  #0  __libc_do_syscall ()
  at ../ports/sysdeps/unix/sysv/linux/arm/libc-do-syscall.S:44
  #1  0xb6ef8f0e in __GI_raise (sig=sig@entry=6)
  at ../nptl/sysdeps/unix/sysv/linux/raise.c:56
  #2  0xb6efb766 in __GI_abort () at abort.c:89
  #3  0xb6ef4150 in __assert_fail_base (
  fmt=0x1 ,
  assertion=0x7f89a234 "qemu_in_coroutine()", assertion@entry=0x0,
  file=0x7f89da58 "qemu-coroutine-lock.c", file@entry=0xb566 "\001",
  line=91, line@entry=3069931692,
  function=function@entry=0x7f89ab78 "qemu_co_queue_restart_all")
  at assert.c:92
  #4  0xb6ef41e6 in __GI___assert_fail (assertion=0x0, file=0xb566 "\001",
  line=3069931692, function=0x7f89ab78 "qemu_co_queue_restart_all")
  at assert.c:101
  #5  0x7f59a6b4 in ?? ()

  I was using the same setup as in Bug 893208 (i.e git checkout from
  2015-09-15, armv7 Odroid C1)

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

[Qemu-devel] [PATCH v3 03/25] target-*: Increment num_insns immediately after tcg_gen_insn_start

[Richard Henderson]

This does tidy the icount test common to all targets.

Reviewed-by: Aurelien Jarno 
Reviewed-by: Peter Maydell 
Signed-off-by: Richard Henderson 
---
 target-alpha/translate.c  | 4 ++--
 target-arm/translate-a64.c| 6 +++---
 target-arm/translate.c| 7 ---
 target-cris/translate.c   | 4 ++--
 target-i386/translate.c   | 5 +++--
 target-lm32/translate.c   | 5 ++---
 target-m68k/translate.c   | 4 ++--
 target-microblaze/translate.c | 5 +++--
 target-mips/translate.c   | 5 ++---
 target-moxie/translate.c  | 2 +-
 target-openrisc/translate.c   | 4 ++--
 target-ppc/translate.c| 4 ++--
 target-s390x/translate.c  | 3 ++-
 target-sh4/translate.c| 4 ++--
 target-sparc/translate.c  | 4 ++--
 target-tilegx/translate.c | 3 ++-
 target-tricore/translate.c| 3 +--
 target-unicore32/translate.c  | 4 ++--
 target-xtensa/translate.c | 4 ++--
 19 files changed, 41 insertions(+), 39 deletions(-)

diff --git a/target-alpha/translate.c b/target-alpha/translate.c
index 60370d6..fa0ac2d 100644
--- a/target-alpha/translate.c
+++ b/target-alpha/translate.c
@@ -2934,12 +2934,12 @@ static inline void 
gen_intermediate_code_internal(AlphaCPU *cpu,
 tcg_ctx.gen_opc_icount[lj] = num_insns;
 }
 tcg_gen_insn_start(ctx.pc);
+num_insns++;
 
-if (num_insns + 1 == max_insns && (tb->cflags & CF_LAST_IO)) {
+if (num_insns == max_insns && (tb->cflags & CF_LAST_IO)) {
 gen_io_start();
 }
 insn = cpu_ldl_code(env, ctx.pc);
-num_insns++;
 
 TCGV_UNUSED_I64(ctx.zero);
 TCGV_UNUSED_I64(ctx.sink);
diff --git a/target-arm/translate-a64.c b/target-arm/translate-a64.c
index 6a66ac0..4670941 100644
--- a/target-arm/translate-a64.c
+++ b/target-arm/translate-a64.c
@@ -11104,8 +11104,9 @@ void gen_intermediate_code_internal_a64(ARMCPU *cpu,
 tcg_ctx.gen_opc_icount[lj] = num_insns;
 }
 tcg_gen_insn_start(dc->pc);
+num_insns++;
 
-if (num_insns + 1 == max_insns && (tb->cflags & CF_LAST_IO)) {
+if (num_insns == max_insns && (tb->cflags & CF_LAST_IO)) {
 gen_io_start();
 }
 
@@ -11120,7 +11121,7 @@ void gen_intermediate_code_internal_a64(ARMCPU *cpu,
  * "did not step an insn" case, and so the syndrome ISV and EX
  * bits should be zero.
  */
-assert(num_insns == 0);
+assert(num_insns == 1);
 gen_exception(EXCP_UDEF, syn_swstep(dc->ss_same_el, 0, 0),
   default_exception_el(dc));
 dc->is_jmp = DISAS_EXC;
@@ -11139,7 +11140,6 @@ void gen_intermediate_code_internal_a64(ARMCPU *cpu,
  * Also stop translation when a page boundary is reached.  This
  * ensures prefetch aborts occur at the right place.
  */
-num_insns++;
 } while (!dc->is_jmp && !tcg_op_buf_full() &&
  !cs->singlestep_enabled &&
  !singlestep &&
diff --git a/target-arm/translate.c b/target-arm/translate.c
index 8348848..cd88997 100644
--- a/target-arm/translate.c
+++ b/target-arm/translate.c
@@ -11349,9 +11349,11 @@ static inline void 
gen_intermediate_code_internal(ARMCPU *cpu,
 tcg_ctx.gen_opc_icount[lj] = num_insns;
 }
 tcg_gen_insn_start(dc->pc);
+num_insns++;
 
-if (num_insns + 1 == max_insns && (tb->cflags & CF_LAST_IO))
+if (num_insns == max_insns && (tb->cflags & CF_LAST_IO)) {
 gen_io_start();
+}
 
 if (dc->ss_active && !dc->pstate_ss) {
 /* Singlestep state is Active-pending.
@@ -11364,7 +11366,7 @@ static inline void 
gen_intermediate_code_internal(ARMCPU *cpu,
  * "did not step an insn" case, and so the syndrome ISV and EX
  * bits should be zero.
  */
-assert(num_insns == 0);
+assert(num_insns == 1);
 gen_exception(EXCP_UDEF, syn_swstep(dc->ss_same_el, 0, 0),
   default_exception_el(dc));
 goto done_generating;
@@ -11400,7 +11402,6 @@ static inline void 
gen_intermediate_code_internal(ARMCPU *cpu,
  * Otherwise the subsequent code could get translated several times.
  * Also stop translation when a page boundary is reached.  This
  * ensures prefetch aborts occur at the right place.  */
-num_insns ++;
 } while (!dc->is_jmp && !tcg_op_buf_full() &&
  !cs->singlestep_enabled &&
  !singlestep &&
diff --git a/target-cris/translate.c b/target-cris/translate.c
index 0a4b363..bba7217 100644
--- a/target-cris/translate.c
+++ b/target-cris/translate.c
@@ -3194,11 +3194,12 @@ gen_intermediate_code_internal(CRISCPU *cpu, 
TranslationBlock *tb,
 tcg_ctx.gen_opc_icount[lj] = num_insns;
 }
 tcg_gen_insn_start(dc->pc);
+num_insns++;

[Qemu-devel] [PATCH v3 13/25] target-sparc: Split out gen_branch_n

[Richard Henderson]

Unify three copies of this code from different
branch types.  Fix the case when npc == DYNAMIC_PC,
i.e. a branch within a delay slot.

Reviewed-by: Peter Maydell 
Signed-off-by: Richard Henderson 
---
 target-sparc/translate.c | 55 
 1 file changed, 28 insertions(+), 27 deletions(-)

diff --git a/target-sparc/translate.c b/target-sparc/translate.c
index cbc90d8..c6a8d86 100644
--- a/target-sparc/translate.c
+++ b/target-sparc/translate.c
@@ -970,6 +970,31 @@ static void gen_branch_a(DisasContext *dc, target_ulong 
pc1)
 dc->is_br = 1;
 }
 
+static void gen_branch_n(DisasContext *dc, target_ulong pc1)
+{
+target_ulong npc = dc->npc;
+
+if (likely(npc != DYNAMIC_PC)) {
+dc->pc = npc;
+dc->jump_pc[0] = pc1;
+dc->jump_pc[1] = npc + 4;
+dc->npc = JUMP_PC;
+} else {
+TCGv t, z;
+
+tcg_gen_mov_tl(cpu_pc, cpu_npc);
+
+tcg_gen_addi_tl(cpu_npc, cpu_npc, 4);
+t = tcg_const_tl(pc1);
+z = tcg_const_tl(0);
+tcg_gen_movcond_tl(TCG_COND_NE, cpu_npc, cpu_cond, z, t, cpu_npc);
+tcg_temp_free(t);
+tcg_temp_free(z);
+
+dc->pc = DYNAMIC_PC;
+}
+}
+
 static inline void gen_generic_branch(DisasContext *dc)
 {
 TCGv npc0 = tcg_const_tl(dc->jump_pc[0]);
@@ -1402,15 +1427,7 @@ static void do_branch(DisasContext *dc, int32_t offset, 
uint32_t insn, int cc)
 if (a) {
 gen_branch_a(dc, target);
 } else {
-dc->pc = dc->npc;
-dc->jump_pc[0] = target;
-if (unlikely(dc->npc == DYNAMIC_PC)) {
-dc->jump_pc[1] = DYNAMIC_PC;
-tcg_gen_addi_tl(cpu_pc, cpu_npc, 4);
-} else {
-dc->jump_pc[1] = dc->npc + 4;
-dc->npc = JUMP_PC;
-}
+gen_branch_n(dc, target);
 }
 }
 }
@@ -1450,15 +1467,7 @@ static void do_fbranch(DisasContext *dc, int32_t offset, 
uint32_t insn, int cc)
 if (a) {
 gen_branch_a(dc, target);
 } else {
-dc->pc = dc->npc;
-dc->jump_pc[0] = target;
-if (unlikely(dc->npc == DYNAMIC_PC)) {
-dc->jump_pc[1] = DYNAMIC_PC;
-tcg_gen_addi_tl(cpu_pc, cpu_npc, 4);
-} else {
-dc->jump_pc[1] = dc->npc + 4;
-dc->npc = JUMP_PC;
-}
+gen_branch_n(dc, target);
 }
 }
 }
@@ -1478,15 +1487,7 @@ static void do_branch_reg(DisasContext *dc, int32_t 
offset, uint32_t insn,
 if (a) {
 gen_branch_a(dc, target);
 } else {
-dc->pc = dc->npc;
-dc->jump_pc[0] = target;
-if (unlikely(dc->npc == DYNAMIC_PC)) {
-dc->jump_pc[1] = DYNAMIC_PC;
-tcg_gen_addi_tl(cpu_pc, cpu_npc, 4);
-} else {
-dc->jump_pc[1] = dc->npc + 4;
-dc->npc = JUMP_PC;
-}
+gen_branch_n(dc, target);
 }
 }
 
-- 
2.4.3

[Qemu-devel] [PATCH v3 05/25] tcg: Allow extra data to be attached to insn_start

[Richard Henderson]

With an eye toward having this data replace the gen_opc_* arrays
that each target collects in order to enable restore_state_from_tb.

Reviewed-by: Aurelien Jarno 
Reviewed-by: Peter Maydell 
Signed-off-by: Richard Henderson 
---
 tcg/tcg-op.h  | 52 
 tcg/tcg-opc.h |  4 ++--
 tcg/tcg.c | 13 +++--
 tcg/tcg.h |  6 ++
 4 files changed, 59 insertions(+), 16 deletions(-)

diff --git a/tcg/tcg-op.h b/tcg/tcg-op.h
index 6409db8..4e20dc1 100644
--- a/tcg/tcg-op.h
+++ b/tcg/tcg-op.h
@@ -700,17 +700,53 @@ static inline void tcg_gen_concat32_i64(TCGv_i64 ret, 
TCGv_i64 lo, TCGv_i64 hi)
 #error must include QEMU headers
 #endif
 
-/* debug info: write the PC of the corresponding QEMU CPU instruction */
-static inline void tcg_gen_insn_start(uint64_t pc)
+#if TARGET_INSN_START_WORDS == 1
+# if TARGET_LONG_BITS <= TCG_TARGET_REG_BITS
+static inline void tcg_gen_insn_start(target_ulong pc)
 {
-/* XXX: must really use a 32 bit size for TCGArg in all cases */
-#if TARGET_LONG_BITS > TCG_TARGET_REG_BITS
-tcg_gen_op2ii(INDEX_op_insn_start,
-  (uint32_t)(pc), (uint32_t)(pc >> 32));
+tcg_gen_op1(_ctx, INDEX_op_insn_start, pc);
+}
+# else
+static inline void tcg_gen_insn_start(target_ulong pc)
+{
+tcg_gen_op2(_ctx, INDEX_op_insn_start,
+(uint32_t)pc, (uint32_t)(pc >> 32));
+}
+# endif
+#elif TARGET_INSN_START_WORDS == 2
+# if TARGET_LONG_BITS <= TCG_TARGET_REG_BITS
+static inline void tcg_gen_insn_start(target_ulong pc, target_ulong a1)
+{
+tcg_gen_op2(_ctx, INDEX_op_insn_start, pc, a1);
+}
+# else
+static inline void tcg_gen_insn_start(target_ulong pc, target_ulong a1)
+{
+tcg_gen_op4(_ctx, INDEX_op_insn_start,
+(uint32_t)pc, (uint32_t)(pc >> 32),
+(uint32_t)a1, (uint32_t)(a1 >> 32));
+}
+# endif
+#elif TARGET_INSN_START_WORDS == 3
+# if TARGET_LONG_BITS <= TCG_TARGET_REG_BITS
+static inline void tcg_gen_insn_start(target_ulong pc, target_ulong a1,
+  target_ulong a2)
+{
+tcg_gen_op3(_ctx, INDEX_op_insn_start, pc, a1, a2);
+}
+# else
+static inline void tcg_gen_insn_start(target_ulong pc, target_ulong a1,
+  target_ulong a2)
+{
+tcg_gen_op6(_ctx, INDEX_op_insn_start,
+(uint32_t)pc, (uint32_t)(pc >> 32),
+(uint32_t)a1, (uint32_t)(a1 >> 32),
+(uint32_t)a2, (uint32_t)(a2 >> 32));
+}
+# endif
 #else
-tcg_gen_op1i(INDEX_op_insn_start, pc);
+# error "Unhandled number of operands to insn_start"
 #endif
-}
 
 static inline void tcg_gen_exit_tb(uintptr_t val)
 {
diff --git a/tcg/tcg-opc.h b/tcg/tcg-opc.h
index f60d3c2..c6f9570 100644
--- a/tcg/tcg-opc.h
+++ b/tcg/tcg-opc.h
@@ -175,9 +175,9 @@ DEF(mulsh_i64, 1, 2, 0, IMPL(TCG_TARGET_HAS_mulsh_i64))
 
 /* QEMU specific */
 #if TARGET_LONG_BITS > TCG_TARGET_REG_BITS
-DEF(insn_start, 0, 0, 2, TCG_OPF_NOT_PRESENT)
+DEF(insn_start, 0, 0, 2 * TARGET_INSN_START_WORDS, TCG_OPF_NOT_PRESENT)
 #else
-DEF(insn_start, 0, 0, 1, TCG_OPF_NOT_PRESENT)
+DEF(insn_start, 0, 0, TARGET_INSN_START_WORDS, TCG_OPF_NOT_PRESENT)
 #endif
 DEF(exit_tb, 0, 0, 1, TCG_OPF_BB_END)
 DEF(goto_tb, 0, 0, 1, TCG_OPF_BB_END)
diff --git a/tcg/tcg.c b/tcg/tcg.c
index df8788b..3308d68 100644
--- a/tcg/tcg.c
+++ b/tcg/tcg.c
@@ -991,16 +991,17 @@ void tcg_dump_ops(TCGContext *s)
 args = >gen_opparam_buf[op->args];
 
 if (c == INDEX_op_insn_start) {
-uint64_t pc;
+qemu_log("%s ", oi != s->gen_first_op_idx ? "\n" : "");
+
+for (i = 0; i < TARGET_INSN_START_WORDS; ++i) {
+target_ulong a;
 #if TARGET_LONG_BITS > TCG_TARGET_REG_BITS
-pc = ((uint64_t)args[1] << 32) | args[0];
+a = ((target_ulong)args[i * 2 + 1] << 32) | args[i * 2];
 #else
-pc = args[0];
+a = args[i];
 #endif
-if (oi != s->gen_first_op_idx) {
-qemu_log("\n");
+qemu_log(" " TARGET_FMT_lx, a);
 }
-qemu_log("  0x%" PRIx64, pc);
 } else if (c == INDEX_op_call) {
 /* variable number of arguments */
 nb_oargs = op->callo;
diff --git a/tcg/tcg.h b/tcg/tcg.h
index 879a665..c975076 100644
--- a/tcg/tcg.h
+++ b/tcg/tcg.h
@@ -129,6 +129,12 @@ typedef uint64_t TCGRegSet;
 # error "Missing unsigned widening multiply"
 #endif
 
+#ifndef TARGET_INSN_START_EXTRA_WORDS
+# define TARGET_INSN_START_WORDS 1
+#else
+# define TARGET_INSN_START_WORDS (1 + TARGET_INSN_START_EXTRA_WORDS)
+#endif
+
 typedef enum TCGOpcode {
 #define DEF(name, oargs, iargs, cargs, flags) INDEX_op_ ## name,
 #include "tcg-opc.h"
-- 
2.4.3

[Qemu-devel] [PATCH v3 07/25] target-i386: Add cc_op state to insn_start

[Richard Henderson]

Reviewed-by: Aurelien Jarno 
Reviewed-by: Peter Maydell 
Signed-off-by: Richard Henderson 
---
 target-i386/cpu.h   | 1 +
 target-i386/translate.c | 2 +-
 2 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/target-i386/cpu.h b/target-i386/cpu.h
index 5231e8c..717d558 100644
--- a/target-i386/cpu.h
+++ b/target-i386/cpu.h
@@ -794,6 +794,7 @@ typedef struct {
 #define MAX_GP_COUNTERS(MSR_IA32_PERF_STATUS - MSR_P6_EVNTSEL0)
 
 #define NB_MMU_MODES 3
+#define TARGET_INSN_START_EXTRA_WORDS 1
 
 #define NB_OPMASK_REGS 8
 
diff --git a/target-i386/translate.c b/target-i386/translate.c
index 9ec9c4c..7501b91 100644
--- a/target-i386/translate.c
+++ b/target-i386/translate.c
@@ -7949,7 +7949,7 @@ static inline void gen_intermediate_code_internal(X86CPU 
*cpu,
 tcg_ctx.gen_opc_instr_start[lj] = 1;
 tcg_ctx.gen_opc_icount[lj] = num_insns;
 }
-tcg_gen_insn_start(pc_ptr);
+tcg_gen_insn_start(pc_ptr, dc->cc_op);
 num_insns++;
 
 /* If RF is set, suppress an internally generated breakpoint.  */
-- 
2.4.3

[Qemu-devel] [PATCH v3 18/25] tcg: Add TCG_MAX_INSNS

[Richard Henderson]

Adjust all translators to respect it.

Reviewed-by: Peter Maydell 
Signed-off-by: Richard Henderson 
---
 target-alpha/translate.c  |  3 +++
 target-arm/translate-a64.c|  3 +++
 target-arm/translate.c|  6 +-
 target-cris/translate.c   |  3 +++
 target-i386/translate.c   |  6 +-
 target-lm32/translate.c   |  3 +++
 target-m68k/translate.c   |  6 +-
 target-microblaze/translate.c |  6 +-
 target-mips/translate.c   |  7 ++-
 target-moxie/translate.c  | 13 +++--
 target-openrisc/translate.c   |  3 +++
 target-ppc/translate.c|  6 +-
 target-s390x/translate.c  |  3 +++
 target-sh4/translate.c|  7 ++-
 target-sparc/translate.c  |  7 ++-
 target-tilegx/translate.c |  3 +++
 target-tricore/translate.c| 20 +---
 target-unicore32/translate.c  |  3 +++
 target-xtensa/translate.c |  3 +++
 tcg/tcg.h |  1 +
 20 files changed, 95 insertions(+), 17 deletions(-)

diff --git a/target-alpha/translate.c b/target-alpha/translate.c
index c10193e..538e202 100644
--- a/target-alpha/translate.c
+++ b/target-alpha/translate.c
@@ -2903,6 +2903,9 @@ static inline void 
gen_intermediate_code_internal(AlphaCPU *cpu,
 if (max_insns == 0) {
 max_insns = CF_COUNT_MASK;
 }
+if (max_insns > TCG_MAX_INSNS) {
+max_insns = TCG_MAX_INSNS;
+}
 
 if (in_superpage(, pc_start)) {
 pc_mask = (1ULL << 41) - 1;
diff --git a/target-arm/translate-a64.c b/target-arm/translate-a64.c
index 654a586..5022fc3 100644
--- a/target-arm/translate-a64.c
+++ b/target-arm/translate-a64.c
@@ -11072,6 +11072,9 @@ void gen_intermediate_code_internal_a64(ARMCPU *cpu,
 if (max_insns == 0) {
 max_insns = CF_COUNT_MASK;
 }
+if (max_insns > TCG_MAX_INSNS) {
+max_insns = TCG_MAX_INSNS;
+}
 
 gen_tb_start(tb);
 
diff --git a/target-arm/translate.c b/target-arm/translate.c
index fb69ecb..fedb781 100644
--- a/target-arm/translate.c
+++ b/target-arm/translate.c
@@ -11258,8 +11258,12 @@ static inline void 
gen_intermediate_code_internal(ARMCPU *cpu,
 lj = -1;
 num_insns = 0;
 max_insns = tb->cflags & CF_COUNT_MASK;
-if (max_insns == 0)
+if (max_insns == 0) {
 max_insns = CF_COUNT_MASK;
+}
+if (max_insns > TCG_MAX_INSNS) {
+max_insns = TCG_MAX_INSNS;
+}
 
 gen_tb_start(tb);
 
diff --git a/target-cris/translate.c b/target-cris/translate.c
index 3d55a6a..d038bdb 100644
--- a/target-cris/translate.c
+++ b/target-cris/translate.c
@@ -3155,6 +3155,9 @@ gen_intermediate_code_internal(CRISCPU *cpu, 
TranslationBlock *tb,
 if (max_insns == 0) {
 max_insns = CF_COUNT_MASK;
 }
+if (max_insns > TCG_MAX_INSNS) {
+max_insns = TCG_MAX_INSNS;
+}
 
 gen_tb_start(tb);
 do {
diff --git a/target-i386/translate.c b/target-i386/translate.c
index 7501b91..d3282e8 100644
--- a/target-i386/translate.c
+++ b/target-i386/translate.c
@@ -7932,8 +7932,12 @@ static inline void gen_intermediate_code_internal(X86CPU 
*cpu,
 lj = -1;
 num_insns = 0;
 max_insns = tb->cflags & CF_COUNT_MASK;
-if (max_insns == 0)
+if (max_insns == 0) {
 max_insns = CF_COUNT_MASK;
+}
+if (max_insns > TCG_MAX_INSNS) {
+max_insns = TCG_MAX_INSNS;
+}
 
 gen_tb_start(tb);
 for(;;) {
diff --git a/target-lm32/translate.c b/target-lm32/translate.c
index 8ea7929..e16c31a 100644
--- a/target-lm32/translate.c
+++ b/target-lm32/translate.c
@@ -1069,6 +1069,9 @@ void gen_intermediate_code_internal(LM32CPU *cpu,
 if (max_insns == 0) {
 max_insns = CF_COUNT_MASK;
 }
+if (max_insns > TCG_MAX_INSNS) {
+max_insns = TCG_MAX_INSNS;
+}
 
 gen_tb_start(tb);
 do {
diff --git a/target-m68k/translate.c b/target-m68k/translate.c
index afef37f..185c565 100644
--- a/target-m68k/translate.c
+++ b/target-m68k/translate.c
@@ -2991,8 +2991,12 @@ gen_intermediate_code_internal(M68kCPU *cpu, 
TranslationBlock *tb,
 lj = -1;
 num_insns = 0;
 max_insns = tb->cflags & CF_COUNT_MASK;
-if (max_insns == 0)
+if (max_insns == 0) {
 max_insns = CF_COUNT_MASK;
+}
+if (max_insns > TCG_MAX_INSNS) {
+max_insns = TCG_MAX_INSNS;
+}
 
 gen_tb_start(tb);
 do {
diff --git a/target-microblaze/translate.c b/target-microblaze/translate.c
index 1224456..58b27ca 100644
--- a/target-microblaze/translate.c
+++ b/target-microblaze/translate.c
@@ -1674,8 +1674,12 @@ gen_intermediate_code_internal(MicroBlazeCPU *cpu, 
TranslationBlock *tb,
 lj = -1;
 num_insns = 0;
 max_insns = tb->cflags & CF_COUNT_MASK;
-if (max_insns == 0)
+if (max_insns == 0) {
 max_insns = CF_COUNT_MASK;
+}
+if (max_insns > TCG_MAX_INSNS) {
+max_insns = TCG_MAX_INSNS;
+}
 
 gen_tb_start(tb);
 do
diff --git a/target-mips/translate.c 

[Qemu-devel] [PATCH v3 15/25] target-sparc: Add npc state to insn_start

[Richard Henderson]

Reviewed-by: Peter Maydell 
Signed-off-by: Richard Henderson 
---
 target-sparc/cpu.h   | 1 +
 target-sparc/translate.c | 7 ++-
 2 files changed, 7 insertions(+), 1 deletion(-)

diff --git a/target-sparc/cpu.h b/target-sparc/cpu.h
index 72ea171..ac8f383 100644
--- a/target-sparc/cpu.h
+++ b/target-sparc/cpu.h
@@ -236,6 +236,7 @@ typedef struct trap_state {
 uint32_t tt;
 } trap_state;
 #endif
+#define TARGET_INSN_START_EXTRA_WORDS 1
 
 typedef struct sparc_def_t {
 const char *name;
diff --git a/target-sparc/translate.c b/target-sparc/translate.c
index 25b5bc0..6e5b82d 100644
--- a/target-sparc/translate.c
+++ b/target-sparc/translate.c
@@ -5257,7 +5257,12 @@ static inline void 
gen_intermediate_code_internal(SPARCCPU *cpu,
 tcg_ctx.gen_opc_icount[lj] = num_insns;
 }
 }
-tcg_gen_insn_start(dc->pc);
+if (dc->npc & JUMP_PC) {
+assert(dc->jump_pc[1] == dc->pc + 4);
+tcg_gen_insn_start(dc->pc, dc->jump_pc[0] | JUMP_PC);
+} else {
+tcg_gen_insn_start(dc->pc, dc->npc);
+}
 num_insns++;
 
 if (unlikely(cpu_breakpoint_test(cs, dc->pc, BP_ANY))) {
-- 
2.4.3

[Qemu-devel] [PATCH v3 20/25] tcg: Save insn data and use it in cpu_restore_state_from_tb

[Richard Henderson]

We can now restore state without retranslation.

Signed-off-by: Richard Henderson 
---
 include/exec/exec-all.h |   1 +
 tcg/tcg.c   |  40 -
 tcg/tcg.h   |   4 +-
 translate-all.c | 149 +++-
 4 files changed, 139 insertions(+), 55 deletions(-)

diff --git a/include/exec/exec-all.h b/include/exec/exec-all.h
index 6a69802..402dd87 100644
--- a/include/exec/exec-all.h
+++ b/include/exec/exec-all.h
@@ -199,6 +199,7 @@ struct TranslationBlock {
 #define CF_USE_ICOUNT  0x2
 
 void *tc_ptr;/* pointer to the translated code */
+uint8_t *tc_search;  /* pointer to search data */
 /* next matching tb for physical address. */
 struct TranslationBlock *phys_hash_next;
 /* original tb when cflags has CF_NOCACHE */
diff --git a/tcg/tcg.c b/tcg/tcg.c
index bdb83d9..a0fce5b 100644
--- a/tcg/tcg.c
+++ b/tcg/tcg.c
@@ -2294,7 +2294,7 @@ static inline int tcg_gen_code_common(TCGContext *s,
   tcg_insn_unit *gen_code_buf,
   long search_pc)
 {
-int i, oi, oi_next;
+int i, oi, oi_next, num_insns;
 
 #ifdef DEBUG_DISAS
 if (unlikely(qemu_loglevel_mask(CPU_LOG_TB_OP))) {
@@ -2338,6 +2338,7 @@ static inline int tcg_gen_code_common(TCGContext *s,
 
 tcg_out_tb_init(s);
 
+num_insns = -1;
 for (oi = s->gen_first_op_idx; oi >= 0; oi = oi_next) {
 TCGOp * const op = >gen_op_buf[oi];
 TCGArg * const args = >gen_opparam_buf[op->args];
@@ -2361,6 +2362,10 @@ static inline int tcg_gen_code_common(TCGContext *s,
 tcg_reg_alloc_movi(s, args, dead_args, sync_args);
 break;
 case INDEX_op_insn_start:
+if (num_insns >= 0) {
+s->gen_insn_end_off[num_insns] = tcg_current_code_size(s);
+}
+num_insns++;
 for (i = 0; i < TARGET_INSN_START_WORDS; ++i) {
 target_ulong a;
 #if TARGET_LONG_BITS > TCG_TARGET_REG_BITS
@@ -2368,7 +2373,7 @@ static inline int tcg_gen_code_common(TCGContext *s,
 #else
 a = args[i];
 #endif
-s->gen_opc_data[i] = a;
+s->gen_insn_data[num_insns][i] = a;
 }
 break;
 case INDEX_op_discard:
@@ -2400,6 +2405,8 @@ static inline int tcg_gen_code_common(TCGContext *s,
 check_regs(s);
 #endif
 }
+tcg_debug_assert(num_insns >= 0);
+s->gen_insn_end_off[num_insns] = tcg_current_code_size(s);
 
 /* Generate TB finalization at the end of block */
 tcg_out_tb_finalize(s);
@@ -2448,24 +2455,26 @@ int tcg_gen_code_search_pc(TCGContext *s, tcg_insn_unit 
*gen_code_buf,
 void tcg_dump_info(FILE *f, fprintf_function cpu_fprintf)
 {
 TCGContext *s = _ctx;
-int64_t tot;
+int64_t tb_count = s->tb_count;
+int64_t tb_div_count = tb_count ? tb_count : 1;
+int64_t tot = s->interm_time + s->code_time;
 
-tot = s->interm_time + s->code_time;
 cpu_fprintf(f, "JIT cycles  %" PRId64 " (%0.3f s at 2.4 GHz)\n",
 tot, tot / 2.4e9);
 cpu_fprintf(f, "translated TBs  %" PRId64 " (aborted=%" PRId64 " 
%0.1f%%)\n", 
-s->tb_count, 
-s->tb_count1 - s->tb_count,
-s->tb_count1 ? (double)(s->tb_count1 - s->tb_count) / 
s->tb_count1 * 100.0 : 0);
+tb_count, s->tb_count1 - tb_count,
+(double)(s->tb_count1 - s->tb_count)
+/ (s->tb_count1 ? s->tb_count1 : 1) * 100.0);
 cpu_fprintf(f, "avg ops/TB  %0.1f max=%d\n", 
-s->tb_count ? (double)s->op_count / s->tb_count : 0, 
s->op_count_max);
+(double)s->op_count / tb_div_count, s->op_count_max);
 cpu_fprintf(f, "deleted ops/TB  %0.2f\n",
-s->tb_count ? 
-(double)s->del_op_count / s->tb_count : 0);
+(double)s->del_op_count / tb_div_count);
 cpu_fprintf(f, "avg temps/TB%0.2f max=%d\n",
-s->tb_count ? 
-(double)s->temp_count / s->tb_count : 0,
-s->temp_count_max);
+(double)s->temp_count / tb_div_count, s->temp_count_max);
+cpu_fprintf(f, "avg host code/TB%0.1f\n",
+(double)s->code_out_len / tb_div_count);
+cpu_fprintf(f, "avg search data/TB  %0.1f\n",
+(double)s->search_out_len / tb_div_count);
 
 cpu_fprintf(f, "cycles/op   %0.1f\n", 
 s->op_count ? (double)tot / s->op_count : 0);
@@ -2473,8 +2482,11 @@ void tcg_dump_info(FILE *f, fprintf_function cpu_fprintf)
 s->code_in_len ? (double)tot / s->code_in_len : 0);
 cpu_fprintf(f, "cycles/out byte %0.1f\n", 
 s->code_out_len ? (double)tot / s->code_out_len : 0);
-if (tot == 0)
+cpu_fprintf(f, "cycles/search byte %0.1f\n", 
+s->search_out_len ? (double)tot / 

Re: [Qemu-devel] [PATCH v7 11/14] block/backup: support block job transactions

[John Snow]

Eric, Markus: I've CC'd you for some crazy what-if QAPI questions after
my R-B on this patch.

On 09/21/2015 10:46 PM, Fam Zheng wrote:
> From: Stefan Hajnoczi 
> 
> Join the transaction when the 'transactional-cancel' QMP argument is
> true.
> 
> This ensures that the sync bitmap is not thrown away if another block
> job in the transaction is cancelled or fails.  This is critical so
> incremental backup with multiple disks can be retried in case of
> cancellation/failure.
> 
> Signed-off-by: Stefan Hajnoczi 
> Reviewed-by: Max Reitz 
> Reviewed-by: Eric Blake 
> Signed-off-by: Fam Zheng 
> ---
>  block/backup.c|  25 +++--
>  blockdev.c| 132 
> +++---
>  include/block/block_int.h |   3 +-
>  qapi-schema.json  |   4 +-
>  qapi/block-core.json  |  27 +-
>  5 files changed, 152 insertions(+), 39 deletions(-)
> 
> diff --git a/block/backup.c b/block/backup.c
> index 609b199..5880116 100644
> --- a/block/backup.c
> +++ b/block/backup.c
> @@ -227,11 +227,29 @@ static void backup_cleanup_sync_bitmap(BackupBlockJob 
> *job, int ret)
>  }
>  }
>  
> +static void backup_commit(BlockJob *job)
> +{
> +BackupBlockJob *s = container_of(job, BackupBlockJob, common);
> +if (s->sync_bitmap) {
> +backup_cleanup_sync_bitmap(s, 0);
> +}
> +}
> +
> +static void backup_abort(BlockJob *job)
> +{
> +BackupBlockJob *s = container_of(job, BackupBlockJob, common);
> +if (s->sync_bitmap) {
> +backup_cleanup_sync_bitmap(s, -1);
> +}
> +}
> +
>  static const BlockJobDriver backup_job_driver = {
>  .instance_size  = sizeof(BackupBlockJob),
>  .job_type   = BLOCK_JOB_TYPE_BACKUP,
>  .set_speed  = backup_set_speed,
>  .iostatus_reset = backup_iostatus_reset,
> +.commit = backup_commit,
> +.abort  = backup_abort,
>  };
>  
>  static BlockErrorAction backup_error_action(BackupBlockJob *job,
> @@ -444,10 +462,6 @@ static void coroutine_fn backup_run(void *opaque)
>  /* wait until pending backup_do_cow() calls have completed */
>  qemu_co_rwlock_wrlock(>flush_rwlock);
>  qemu_co_rwlock_unlock(>flush_rwlock);
> -
> -if (job->sync_bitmap) {
> -backup_cleanup_sync_bitmap(job, ret);
> -}
>  hbitmap_free(job->bitmap);
>  
>  bdrv_iostatus_disable(target);
> @@ -464,7 +478,7 @@ void backup_start(BlockDriverState *bs, BlockDriverState 
> *target,
>BlockdevOnError on_source_error,
>BlockdevOnError on_target_error,
>BlockCompletionFunc *cb, void *opaque,
> -  Error **errp)
> +  BlockJobTxn *txn, Error **errp)
>  {
>  int64_t len;
>  
> @@ -546,6 +560,7 @@ void backup_start(BlockDriverState *bs, BlockDriverState 
> *target,
> sync_bitmap : NULL;
>  job->common.len = len;
>  job->common.co = qemu_coroutine_create(backup_run);
> +block_job_txn_add_job(txn, >common);
>  qemu_coroutine_enter(job->common.co, job);
>  return;
>  

So a backup job will join a transaction unconditionally if it is given one.

> diff --git a/blockdev.c b/blockdev.c
> index ed50cb4..45a9fe7 100644
> --- a/blockdev.c
> +++ b/blockdev.c
> @@ -1586,16 +1586,31 @@ typedef struct DriveBackupState {
>  BlockJob *job;
>  } DriveBackupState;
>  
> +static void do_drive_backup(const char *device, const char *target,
> +bool has_format, const char *format,
> +enum MirrorSyncMode sync,
> +bool has_mode, enum NewImageMode mode,
> +bool has_speed, int64_t speed,
> +bool has_bitmap, const char *bitmap,
> +bool has_on_source_error,
> +BlockdevOnError on_source_error,
> +bool has_on_target_error,
> +BlockdevOnError on_target_error,
> +BlockJobTxn *txn, Error **errp);
> +
>  static void drive_backup_prepare(BlkActionState *common, Error **errp)
>  {
>  DriveBackupState *state = DO_UPCAST(DriveBackupState, common, common);
>  BlockDriverState *bs;
>  BlockBackend *blk;
> +DriveBackupTxn *backup_txn;
>  DriveBackup *backup;
> +BlockJobTxn *txn = NULL;
>  Error *local_err = NULL;
>  
>  assert(common->action->kind == TRANSACTION_ACTION_KIND_DRIVE_BACKUP);
> -backup = common->action->drive_backup;
> +backup_txn = common->action->drive_backup;
> +backup = backup_txn->base;
>  
>  blk = blk_by_name(backup->device);
>  if (!blk) {
> @@ -1609,15 +1624,20 @@ static void drive_backup_prepare(BlkActionState 
> *common, Error **errp)
>  state->aio_context = bdrv_get_aio_context(bs);
>  

Re: [Qemu-devel] [PATCH 2/2] target-ppc: fix xscmpodp and xscmpudp decoding

[Aurelien Jarno]

On 2015-09-22 12:26, Thomas Huth wrote:
> On 13/09/15 23:03, Aurelien Jarno wrote:
> > The xscmpodp and xscmpudp instructions only have the AX, BX bits in
> > there encoding, the lowest bit (usually TX) is marked as an invalid
> > bit. We therefore can't decode them with GEN_XX2FORM, which decodes
> > the two lowest bit.
> > 
> > Introduce a new form GEN_XX2FORM, which decodes AX and BX and mark
> > the lowest bit as invalid.
> > 
> > Cc: Tom Musta 
> > Cc: Alexander Graf 
> > Cc: qemu-sta...@nongnu.org
> > Signed-off-by: Aurelien Jarno 
> > ---
> >  target-ppc/translate.c | 11 +--
> >  1 file changed, 9 insertions(+), 2 deletions(-)
> > 
> > diff --git a/target-ppc/translate.c b/target-ppc/translate.c
> > index 84c5cea..c0eed13 100644
> > --- a/target-ppc/translate.c
> > +++ b/target-ppc/translate.c
> > @@ -10670,6 +10670,13 @@ GEN_HANDLER2_E(name, #name, 0x3C, opc2 | 1, opc3, 
> > 0, PPC_NONE, fl2), \
> >  GEN_HANDLER2_E(name, #name, 0x3C, opc2 | 2, opc3, 0, PPC_NONE, fl2), \
> >  GEN_HANDLER2_E(name, #name, 0x3C, opc2 | 3, opc3, 0, PPC_NONE, fl2)
> >  
> > +#undef GEN_XX2IFORM
> > +#define GEN_XX2IFORM(name, opc2, opc3, fl2)   \
> > +GEN_HANDLER2_E(name, #name, 0x3C, opc2 | 0, opc3, 1, PPC_NONE, fl2), \
> > +GEN_HANDLER2_E(name, #name, 0x3C, opc2 | 1, opc3, 1, PPC_NONE, fl2), \
> > +GEN_HANDLER2_E(name, #name, 0x3C, opc2 | 2, opc3, 1, PPC_NONE, fl2), \
> > +GEN_HANDLER2_E(name, #name, 0x3C, opc2 | 3, opc3, 1, PPC_NONE, fl2)
> > +
> >  #undef GEN_XX3_RC_FORM
> >  #define GEN_XX3_RC_FORM(name, opc2, opc3, fl2)  \
> >  GEN_HANDLER2_E(name, #name, 0x3C, opc2 | 0x00, opc3 | 0x00, 0, PPC_NONE, 
> > fl2), \
> > @@ -10731,8 +10738,8 @@ GEN_XX3FORM(xsnmaddadp, 0x04, 0x14, PPC2_VSX),
> >  GEN_XX3FORM(xsnmaddmdp, 0x04, 0x15, PPC2_VSX),
> >  GEN_XX3FORM(xsnmsubadp, 0x04, 0x16, PPC2_VSX),
> >  GEN_XX3FORM(xsnmsubmdp, 0x04, 0x17, PPC2_VSX),
> > -GEN_XX2FORM(xscmpodp,  0x0C, 0x05, PPC2_VSX),
> > -GEN_XX2FORM(xscmpudp,  0x0C, 0x04, PPC2_VSX),
> > +GEN_XX2IFORM(xscmpodp,  0x0C, 0x05, PPC2_VSX),
> > +GEN_XX2IFORM(xscmpudp,  0x0C, 0x04, PPC2_VSX),
> 
> According to PowerISA 2.07, xscmpodp and xscmpudp are of type XX3, not
> of type XX2 ... so should this macro maybe rather be named XX3IFORM instead?

Indeed, I have chosen the name without actually realizing the manual
also give names. Then I do wonder if the lower bit is really decoded as
invalid, I wouldn't be surprised it is actually just ignored.

I'll try to do some tests on real hardware and come up with a fixup
patch.

Aurelien

-- 
Aurelien Jarno  GPG: 4096R/1DDD8C9B
aurel...@aurel32.net http://www.aurel32.net

Re: [Qemu-devel] [PATCH v5 0/4] qapi: child add/delete support

[Wen Congyang]

On 09/22/2015 07:15 PM, Dr. David Alan Gilbert wrote:
> * Wen Congyang (we...@cn.fujitsu.com) wrote:
>> If quorum's child is broken, we can use mirror job to replace it.
>> But sometimes, the user only need to remove the broken child, and
>> add it later when the problem is fixed.
>>
> 
> Hi,
>   Two questions:
> 1) Do you have an example of a pair of add/remove commands that work
>   together? (I'm not quite sure I understand where the ID for the remove
>   comes from).

The command line:
-drive 
if=virtio,id=disk1,driver=quorum,read-pattern=fifo,vote-threshold=1,children.0.file.filename=/data/images/kvm/suse/suse11_3.img,children.0.driver=raw

And the QMP monitor command:
{'execute':'blockdev-add', 'arguments':{'options':{'driver': 'raw', 
'node-name': 'test1', 'file': {'driver': 'file', 'filename': '/dev/null'}, 
'id': 'test11' }  } }
{'execute': 'human-monitor-command', 'arguments': {'command-line': 'drive_add 
buddy 
driver=nbd,host=192.168.3.1,port=8889,export=colo-disk1,node-name=test2,if=none'}}
{'execute':'x-blockdev-child-add', 'arguments':{'parent': 'disk1', 'child': 
'test1' } }
{'execute':'x-blockdev-child-add', 'arguments':{'parent': 'disk1', 'child': 
'test2' } }
{'execute': 'x-blockdev-child-del', 'arguments': {'parent': 'disk1', 'child': 
'test1' } }
{'execute': 'x-blockdev-child-del', 'arguments': {'parent': 'disk1', 'child': 
'test2' } }

Note: the qmp monitor command doesn't support nbd now, and I use the hmp 
command to add a BDS.

> 
> 2) If the child has failed and is not responding to block operations
>at all (e.g a networking failure to an nbd device which may take 
> minutes
>to time out); how do you recover - flush or drain on the devices
>hang at that point.

If the network fails, the kernel doesn't notify the application...

> 
> (I was trying to test recovery from a failed secondary using the July COLO
> release; but the primary gets stuck in bdrv_drain or bdrv_flush if I kill
> the secondary in the right way).

IIRC, if the qemu is killed, the connection is closed at the same time. 
bdrv_drain()
or bdrv_flush() should not get stuck.

Thanks
Wen Congyang

> 
> Dave
> 
> 
>> It is based on the following patch:
>> http://lists.nongnu.org/archive/html/qemu-devel/2015-09/msg04579.html
>>
>> ChangLog:
>> v5:
>> 1. Address Eric Blake's comments
>> v4:
>> 1. drop nbd driver's implementation. We can use human-monitor-command
>>to do it.
>> 2. Rename the command name.
>> v3:
>> 1. Don't open BDS in bdrv_add_child(). Use the existing BDS which is
>>created by the QMP command blockdev-add.
>> 2. The driver NBD can support filename, path, host:port now.
>> v2:
>> 1. Use bdrv_get_device_or_node_name() instead of new function
>>bdrv_get_id_or_node_name()
>> 2. Update the error message
>> 3. Update the documents in block-core.json
>>
>> Wen Congyang (4):
>>   Add new block driver interface to add/delete a BDS's child
>>   quorum: implement bdrv_add_child() and bdrv_del_child()
>>   qmp: add monitor command to add/remove a child
>>   hmp: add monitor command to add/remove a child
>>
>>  block.c   | 56 ++--
>>  block/quorum.c| 72 
>> +--
>>  blockdev.c| 48 +++
>>  hmp-commands.hx   | 28 ++
>>  hmp.c | 20 +
>>  hmp.h |  2 ++
>>  include/block/block.h |  8 ++
>>  include/block/block_int.h |  5 
>>  qapi/block-core.json  | 34 ++
>>  qmp-commands.hx   | 61 +++
>>  10 files changed, 329 insertions(+), 5 deletions(-)
>>
>> -- 
>> 2.4.3
>>
> --
> Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK
> .
> 

Re: [Qemu-devel] [PATCH v10 6/7] vhost-user: add multiple queue support

[Yuanhan Liu]

On Tue, Sep 22, 2015 at 03:52:49PM -0300, Eduardo Habkost wrote:
> On Fri, Sep 18, 2015 at 10:58:43PM +0800, Yuanhan Liu wrote:
> [...]
> > diff --git a/net/vhost-user.c b/net/vhost-user.c
> > index 93dcecd..4fa3d64 100644
> > --- a/net/vhost-user.c
> > +++ b/net/vhost-user.c
> [...]
> > +if (i == 0) {
> > +max_queues = vhost_net_get_max_queues(s->vhost_net);
> 
> This breaks arm-softmmu (and any target that doesn't have CONFIG_VHOST_NET):

Thanks for testing. Will fix it next version.

--yliu
> 
>   $ make subdir-arm-softmmu
> CCarm-softmmu/hw/net/vhost_net.o
> LINK  arm-softmmu/qemu-system-arm
>   ../net/vhost-user.o: In function `vhost_user_start':
>   /home/ehabkost/rh/proj/virt/qemu/net/vhost-user.c:88: undefined reference 
> to `vhost_net_get_max_queues'
>   collect2: error: ld returned 1 exit status
>   Makefile:193: recipe for target 'qemu-system-arm' failed
>   make[1]: *** [qemu-system-arm] Error 1
>   Makefile:184: recipe for target 'subdir-arm-softmmu' failed
>   make: *** [subdir-arm-softmmu] Error 2
> 
> -- 
> Eduardo

[Qemu-devel] [PULL 03/36] spapr: Add /ibm,partition-name

[David Gibson]

From: Sam Bobroff 

QEMU has a notion of the guest name, so if it's present we might as
well put that into the device tree as /ibm,partition-name.

This is specificed by PAPR.

Signed-off-by: Sam Bobroff 
Reviewed-by: David Gibson 
Signed-off-by: David Gibson 
---
 hw/ppc/spapr.c | 5 +
 1 file changed, 5 insertions(+)

diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index df76cb7..09962c4 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -375,6 +375,11 @@ static void *spapr_create_fdt_skel(hwaddr initrd_base,
 _FDT((fdt_property_string(fdt, "vm,uuid", buf)));
 g_free(buf);
 
+if (qemu_get_vm_name()) {
+_FDT((fdt_property_string(fdt, "ibm,partition-name",
+  qemu_get_vm_name(;
+}
+
 _FDT((fdt_property_cell(fdt, "#address-cells", 0x2)));
 _FDT((fdt_property_cell(fdt, "#size-cells", 0x2)));
 
-- 
2.4.3

[Qemu-devel] [PULL 00/36] spapr-next queue 20150923

[David Gibson]

The following changes since commit 27c7275a56948f48f536e2d1599b22355f5714ac:

  Merge remote-tracking branch 'remotes/kraxel/tags/pull-ipxe-20150903-1' into 
staging (2015-09-22 19:22:23 +0100)

are available in the git repository at:

  git://github.com/dgibson/qemu.git tags/spapr-next-20150923

for you to fetch changes up to d76548a98f4e18d3c65a3d921bbb70caf9be6138:

  sPAPR: Enable EEH on VFIO PCI device only (2015-09-23 10:51:11 +1000)

Apologies for the breakage in the previous pull request, this should fix it.


sPAPR Patch Queue: 2015-09-23

Highlights:
* pseries-2.5 machine type
* Memory hotplug for "pseries" guests
* Fixes to the PAPR Dynamic Reconfiguration hotplug code
* Several PAPR compliance fixes
* New SLOF with:
* GPT support
* Much faster VGA handling


Alexey Kardashevskiy (3):
  pseries: Update SLOF firmware image to qemu-slof-20150813
  spapr: Enable in-kernel H_SET_MODE handling
  spapr: Use QEMU limit for maximum CPUs number

Andrew Jones (1):
  spapr: add dumpdtb support

Bharata B Rao (12):
  spapr: Provide an error message when migration fails due to htab_shift 
mismatch
  spapr_rtas: Prevent QEMU crash during hotplug without a prior device_add
  spapr: Initialize hotplug memory address space
  spapr: Support ibm,dynamic-reconfiguration-memory
  spapr: Make hash table size a factor of maxram_size
  spapr: Memory hotplug support
  spapr: Don't allow memory hotplug to memory less nodes
  spapr: Provide better error message when slots exceed max allowed
  spapr: Populate ibm,associativity-lookup-arrays correctly for non-NUMA
  spapr: Revert to memory@ representation for non-hotplugged memory
  spapr: Support hotplug by specifying DRC count
  spapr: Move memory hotplug to RTAS_LOG_V6_HP_ID_DRC_COUNT type

David Gibson (6):
  spapr: Create pseries-2.5 machine
  spapr_drc: Fix potential undefined behaviour
  pseries: Fix incorrect calculation of threads per socket for chip-id
  spapr: Don't use QOM [*] syntax for DR connectors.
  spapr: Add LMB DR connectors
  spapr: Fix default NUMA node allocation for threads

Gavin Shan (3):
  sPAPR: Introduce rtas_ldq()
  sPAPR: Revert don't enable EEH on emulated PCI devices
  sPAPR: Enable EEH on VFIO PCI device only

Laurent Vivier (1):
  pseries: define coldplugged devices as "configured"

Michael Roth (3):
  spapr_pci: fix device tree props for MSI/MSI-X
  spapr_drc: don't allow 'empty' DRCs to be unisolated or allocated
  spapr_drc: use RTAS return codes for methods called by RTAS

Sam Bobroff (4):
  spapr: Add /ibm,partition-name
  spapr: Add /rtas/ibm,change-msix-capable
  spapr: Make ibm, change-msi respect 3 return values
  spapr: SPLPAR Characteristics

Thomas Huth (3):
  ppc/spapr: Use qemu_log_mask() for hcall_dprintf()
  ppc/spapr: Fix buffer overflow in spapr_populate_drconf_memory()
  ppc/spapr: Implement H_RANDOM hypercall in QEMU

 default-configs/ppc64-softmmu.mak |   1 +
 docs/specs/ppc-spapr-hotplug.txt  |  48 
 hw/ppc/Makefile.objs  |   2 +-
 hw/ppc/spapr.c| 483 ++
 hw/ppc/spapr_drc.c|  67 --
 hw/ppc/spapr_events.c |  51 +++-
 hw/ppc/spapr_hcall.c  |  54 -
 hw/ppc/spapr_pci.c|  47 ++--
 hw/ppc/spapr_pci_vfio.c   |   2 +-
 hw/ppc/spapr_rng.c| 186 +++
 hw/ppc/spapr_rtas.c   |  57 +++--
 include/hw/ppc/spapr.h|  57 -
 include/hw/ppc/spapr_drc.h|  29 +--
 pc-bios/README|   2 +-
 pc-bios/slof.bin  | Bin 912720 -> 915584 bytes
 roms/SLOF |   2 +-
 target-ppc/kvm.c  |  14 ++
 target-ppc/kvm_ppc.h  |  10 +
 18 files changed, 955 insertions(+), 157 deletions(-)
 create mode 100644 hw/ppc/spapr_rng.c

[Qemu-devel] [PULL 01/36] spapr: Provide an error message when migration fails due to htab_shift mismatch

[David Gibson]

From: Bharata B Rao 

Include an error message when migration fails due to mismatch in
htab_shift values at source and target. This should provide a bit more
verbose message in addition to the current migration failure message
that reads like:

qemu-system-ppc64: error while loading state for instance 0x0 of device 
'spapr/htab'

After this patch, the failure message will look like this:

qemu-system-ppc64: htab_shift mismatch: source 29 target 24
qemu-system-ppc64: error while loading state for instance 0x0 of device 
'spapr/htab'

Signed-off-by: Bharata B Rao 
Signed-off-by: David Gibson 
---
 hw/ppc/spapr.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index 05926a3..dd58eb4 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -1329,6 +1329,8 @@ static int htab_load(QEMUFile *f, void *opaque, int 
version_id)
 if (section_hdr) {
 /* First section, just the hash shift */
 if (spapr->htab_shift != section_hdr) {
+error_report("htab_shift mismatch: source %d target %d",
+ section_hdr, spapr->htab_shift);
 return -EINVAL;
 }
 return 0;
-- 
2.4.3

[Qemu-devel] [PULL 09/36] ppc/spapr: Use qemu_log_mask() for hcall_dprintf()

[David Gibson]

From: Thomas Huth 

To see the output of the hcall_dprintf statements, you currently have
to enable the DEBUG_SPAPR_HCALLS macro in include/hw/ppc/spapr.h.
This is ugly because a) not every user who wants to debug guest
problems can or wants to recompile QEMU to be able to see such issues,
and b) since this macro is disabled by default, the code in the
hcall_dprintf() brackets tends to bitrot until somebody temporarily
enables that macro again.
Since the hcall_dprintf statements except one indicate guest
problems, let's always use qemu_log_mask(LOG_GUEST_ERROR, ...) for
this macro instead. One spot indicated an unimplemented host feature,
so this is changed into qemu_log_mask(LOG_UNIMP, ...) instead. Now
it's possible to see all those messages by simply adding the CLI
parameter "-d guest_errors,unimp", without the need to re-compile
the binary.

Signed-off-by: Thomas Huth 
Reviewed-by: David Gibson 
Signed-off-by: David Gibson 
---
 hw/ppc/spapr_hcall.c   |  3 ++-
 include/hw/ppc/spapr.h | 11 +++
 2 files changed, 5 insertions(+), 9 deletions(-)

diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c
index 652ddf6..71fc9f2 100644
--- a/hw/ppc/spapr_hcall.c
+++ b/hw/ppc/spapr_hcall.c
@@ -971,7 +971,8 @@ target_ulong spapr_hypercall(PowerPCCPU *cpu, target_ulong 
opcode,
 }
 }
 
-hcall_dprintf("Unimplemented hcall 0x" TARGET_FMT_lx "\n", opcode);
+qemu_log_mask(LOG_UNIMP, "Unimplemented SPAPR hcall 0x" TARGET_FMT_lx "\n",
+  opcode);
 return H_FUNCTION;
 }
 
diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h
index 91a61ab..d250967 100644
--- a/include/hw/ppc/spapr.h
+++ b/include/hw/ppc/spapr.h
@@ -353,15 +353,10 @@ typedef struct sPAPRDeviceTreeUpdateHeader {
 uint32_t version_id;
 } sPAPRDeviceTreeUpdateHeader;
 
-/*#define DEBUG_SPAPR_HCALLS*/
-
-#ifdef DEBUG_SPAPR_HCALLS
-#define hcall_dprintf(fmt, ...) \
-do { fprintf(stderr, "%s: " fmt, __func__, ## __VA_ARGS__); } while (0)
-#else
 #define hcall_dprintf(fmt, ...) \
-do { } while (0)
-#endif
+do { \
+qemu_log_mask(LOG_GUEST_ERROR, "%s: " fmt, __func__, ## __VA_ARGS__); \
+} while (0)
 
 typedef target_ulong (*spapr_hcall_fn)(PowerPCCPU *cpu, sPAPRMachineState *sm,
target_ulong opcode,
-- 
2.4.3

[Qemu-devel] [PULL 10/36] spapr_rtas: Prevent QEMU crash during hotplug without a prior device_add

[David Gibson]

From: Bharata B Rao 

If drmgr is used in the guest to hotplug a device before a device_add
has been issued via the QEMU monitor, QEMU segfaults in configure_connector
call. This occurs due to accessing of NULL FDT which otherwise would have
been created and associated with the DRC during device_add command.

Check for NULL FDT and return failure from configure_connector call.
As per PAPR+, an error value of -9003 seems appropriate for this failure.

Signed-off-by: Bharata B Rao 
Cc: Michael Roth 
Reviewed-by: David Gibson 
Signed-off-by: David Gibson 
---
 hw/ppc/spapr_rtas.c|  6 ++
 include/hw/ppc/spapr_drc.h | 15 ---
 2 files changed, 14 insertions(+), 7 deletions(-)

diff --git a/hw/ppc/spapr_rtas.c b/hw/ppc/spapr_rtas.c
index 5cbf9a0..2f8e25c 100644
--- a/hw/ppc/spapr_rtas.c
+++ b/hw/ppc/spapr_rtas.c
@@ -522,6 +522,12 @@ static void rtas_ibm_configure_connector(PowerPCCPU *cpu,
 
 drck = SPAPR_DR_CONNECTOR_GET_CLASS(drc);
 fdt = drck->get_fdt(drc, NULL);
+if (!fdt) {
+DPRINTF("rtas_ibm_configure_connector: Missing FDT for DRC index: 
%xh\n",
+drc_index);
+rc = SPAPR_DR_CC_RESPONSE_NOT_CONFIGURABLE;
+goto out;
+}
 
 ccs = spapr_ccs_find(spapr, drc_index);
 if (!ccs) {
diff --git a/include/hw/ppc/spapr_drc.h b/include/hw/ppc/spapr_drc.h
index 60cda35..28ffeae 100644
--- a/include/hw/ppc/spapr_drc.h
+++ b/include/hw/ppc/spapr_drc.h
@@ -119,13 +119,14 @@ typedef enum {
 } sPAPRDREntitySense;
 
 typedef enum {
-SPAPR_DR_CC_RESPONSE_NEXT_SIB   = 1, /* currently unused */
-SPAPR_DR_CC_RESPONSE_NEXT_CHILD = 2,
-SPAPR_DR_CC_RESPONSE_NEXT_PROPERTY  = 3,
-SPAPR_DR_CC_RESPONSE_PREV_PARENT= 4,
-SPAPR_DR_CC_RESPONSE_SUCCESS= 0,
-SPAPR_DR_CC_RESPONSE_ERROR  = -1,
-SPAPR_DR_CC_RESPONSE_CONTINUE   = -2,
+SPAPR_DR_CC_RESPONSE_NEXT_SIB = 1, /* currently unused */
+SPAPR_DR_CC_RESPONSE_NEXT_CHILD   = 2,
+SPAPR_DR_CC_RESPONSE_NEXT_PROPERTY= 3,
+SPAPR_DR_CC_RESPONSE_PREV_PARENT  = 4,
+SPAPR_DR_CC_RESPONSE_SUCCESS  = 0,
+SPAPR_DR_CC_RESPONSE_ERROR= -1,
+SPAPR_DR_CC_RESPONSE_CONTINUE = -2,
+SPAPR_DR_CC_RESPONSE_NOT_CONFIGURABLE = -9003,
 } sPAPRDRCCResponse;
 
 typedef void (spapr_drc_detach_cb)(DeviceState *d, void *opaque);
-- 
2.4.3

[Qemu-devel] [PULL 02/36] spapr: Create pseries-2.5 machine

[David Gibson]

Add pseries-2.5 machine version.

Signed-off-by: Bharata B Rao 
[Altered to merge before memory hotplug -- dwg]
[Altered to work with b9f072d01 -- dwg]
Signed-off-by: David Gibson 
---
 hw/ppc/spapr.c | 19 ++-
 1 file changed, 18 insertions(+), 1 deletion(-)

diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index dd58eb4..df76cb7 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -1976,7 +1976,7 @@ static void spapr_machine_2_4_class_init(ObjectClass *oc, 
void *data)
 
 mc->desc = "pSeries Logical Partition (PAPR compliant) v2.4";
 mc->alias = "pseries";
-mc->is_default = 1;
+mc->is_default = 0;
 }
 
 static const TypeInfo spapr_machine_2_4_info = {
@@ -1985,6 +1985,22 @@ static const TypeInfo spapr_machine_2_4_info = {
 .class_init= spapr_machine_2_4_class_init,
 };
 
+static void spapr_machine_2_5_class_init(ObjectClass *oc, void *data)
+{
+MachineClass *mc = MACHINE_CLASS(oc);
+
+mc->name = "pseries-2.5";
+mc->desc = "pSeries Logical Partition (PAPR compliant) v2.5";
+mc->alias = "pseries";
+mc->is_default = 1;
+}
+
+static const TypeInfo spapr_machine_2_5_info = {
+.name  = MACHINE_TYPE_NAME("pseries-2.5"),
+.parent= TYPE_SPAPR_MACHINE,
+.class_init= spapr_machine_2_5_class_init,
+};
+
 static void spapr_machine_register_types(void)
 {
 type_register_static(_machine_info);
@@ -1992,6 +2008,7 @@ static void spapr_machine_register_types(void)
 type_register_static(_machine_2_2_info);
 type_register_static(_machine_2_3_info);
 type_register_static(_machine_2_4_info);
+type_register_static(_machine_2_5_info);
 }
 
 type_init(spapr_machine_register_types)
-- 
2.4.3

[Qemu-devel] [PULL 07/36] spapr: add dumpdtb support

[David Gibson]

From: Andrew Jones 

dumpdtb (-machine dumpdtb=) allows one to inspect the generated
device tree of machine types that generate device trees. This is
useful for a) seeing what's there b) debugging/testing device tree
generator patches. It can be used as follows

$QEMU_CMDLINE -machine dumpdtb=dtb
dtc -I dtb -O dts dtb

Signed-off-by: Andrew Jones 
Reviewed-by: David Gibson 
Signed-off-by: David Gibson 
---
 hw/ppc/spapr.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index ed0abd8..a69d7e4 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -30,6 +30,7 @@
 #include "hw/fw-path-provider.h"
 #include "elf.h"
 #include "net/net.h"
+#include "sysemu/device_tree.h"
 #include "sysemu/block-backend.h"
 #include "sysemu/cpus.h"
 #include "sysemu/kvm.h"
@@ -831,6 +832,7 @@ static void spapr_finalize_fdt(sPAPRMachineState *spapr,
 exit(1);
 }
 
+qemu_fdt_dumpdtb(fdt, fdt_totalsize(fdt));
 cpu_physical_memory_write(fdt_addr, fdt, fdt_totalsize(fdt));
 
 g_free(bootlist);
-- 
2.4.3

[Qemu-devel] [PULL 19/36] spapr_drc: use RTAS return codes for methods called by RTAS

[David Gibson]

From: Michael Roth 

Certain methods in sPAPRDRConnector objects are only ever called by
RTAS and in many cases are responsible for the logic that determines
the RTAS return codes.

Rather than having a level of indirection requiring RTAS code to
re-interpret return values from such methods to determine the
appropriate return code, just pass them through directly.

This requires changing method return types to uint32_t to match the
type of values currently passed to RTAS helpers.

In the case of read accesses like drc->entity_sense() where we weren't
previously reporting any errors, just the read value, we modify the
function to return RTAS return code, and pass the read value back via
reference.

Suggested-by: Bharata B Rao 
Suggested-by: David Gibson 
Cc: Bharata B Rao 
Signed-off-by: Michael Roth 
Reviewed-by: David Gibson 
Signed-off-by: David Gibson 
---
 hw/ppc/spapr_drc.c | 37 +++--
 hw/ppc/spapr_rtas.c| 40 ++--
 include/hw/ppc/spapr_drc.h | 14 +++---
 3 files changed, 48 insertions(+), 43 deletions(-)

diff --git a/hw/ppc/spapr_drc.c b/hw/ppc/spapr_drc.c
index faf8760..b7b9891 100644
--- a/hw/ppc/spapr_drc.c
+++ b/hw/ppc/spapr_drc.c
@@ -15,6 +15,7 @@
 #include "hw/qdev.h"
 #include "qapi/visitor.h"
 #include "qemu/error-report.h"
+#include "hw/ppc/spapr.h" /* for RTAS return codes */
 
 /* #define DEBUG_SPAPR_DRC */
 
@@ -59,8 +60,8 @@ static uint32_t get_index(sPAPRDRConnector *drc)
 (drc->id & DRC_INDEX_ID_MASK);
 }
 
-static int set_isolation_state(sPAPRDRConnector *drc,
-   sPAPRDRIsolationState state)
+static uint32_t set_isolation_state(sPAPRDRConnector *drc,
+sPAPRDRIsolationState state)
 {
 sPAPRDRConnectorClass *drck = SPAPR_DR_CONNECTOR_GET_CLASS(drc);
 
@@ -99,19 +100,19 @@ static int set_isolation_state(sPAPRDRConnector *drc,
 drc->configured = false;
 }
 
-return 0;
+return RTAS_OUT_SUCCESS;
 }
 
-static int set_indicator_state(sPAPRDRConnector *drc,
-   sPAPRDRIndicatorState state)
+static uint32_t set_indicator_state(sPAPRDRConnector *drc,
+sPAPRDRIndicatorState state)
 {
 DPRINTFN("drc: %x, set_indicator_state: %x", get_index(drc), state);
 drc->indicator_state = state;
-return 0;
+return RTAS_OUT_SUCCESS;
 }
 
-static int set_allocation_state(sPAPRDRConnector *drc,
-sPAPRDRAllocationState state)
+static uint32_t set_allocation_state(sPAPRDRConnector *drc,
+ sPAPRDRAllocationState state)
 {
 sPAPRDRConnectorClass *drck = SPAPR_DR_CONNECTOR_GET_CLASS(drc);
 
@@ -137,7 +138,7 @@ static int set_allocation_state(sPAPRDRConnector *drc,
  drc->detach_cb_opaque, NULL);
 }
 }
-return 0;
+return RTAS_OUT_SUCCESS;
 }
 
 static uint32_t get_type(sPAPRDRConnector *drc)
@@ -178,10 +179,8 @@ static void set_configured(sPAPRDRConnector *drc)
  * based on the current allocation/indicator/power states
  * for the DR connector.
  */
-static sPAPRDREntitySense entity_sense(sPAPRDRConnector *drc)
+static uint32_t entity_sense(sPAPRDRConnector *drc, sPAPRDREntitySense *state)
 {
-sPAPRDREntitySense state;
-
 if (drc->dev) {
 if (drc->type != SPAPR_DR_CONNECTOR_TYPE_PCI &&
 drc->allocation_state == SPAPR_DR_ALLOCATION_STATE_UNUSABLE) {
@@ -190,7 +189,7 @@ static sPAPRDREntitySense entity_sense(sPAPRDRConnector 
*drc)
  * Otherwise, report the state as USABLE/PRESENT,
  * as we would for PCI.
  */
-state = SPAPR_DR_ENTITY_SENSE_UNUSABLE;
+*state = SPAPR_DR_ENTITY_SENSE_UNUSABLE;
 } else {
 /* this assumes all PCI devices are assigned to
  * a 'live insertion' power domain, where QEMU
@@ -198,21 +197,21 @@ static sPAPRDREntitySense entity_sense(sPAPRDRConnector 
*drc)
  * to the guest. present, non-PCI resources are
  * unaffected by power state.
  */
-state = SPAPR_DR_ENTITY_SENSE_PRESENT;
+*state = SPAPR_DR_ENTITY_SENSE_PRESENT;
 }
 } else {
 if (drc->type == SPAPR_DR_CONNECTOR_TYPE_PCI) {
 /* PCI devices, and only PCI devices, use EMPTY
  * in cases where we'd otherwise use UNUSABLE
  */
-state = SPAPR_DR_ENTITY_SENSE_EMPTY;
+*state = SPAPR_DR_ENTITY_SENSE_EMPTY;
 } else {
-state = SPAPR_DR_ENTITY_SENSE_UNUSABLE;
+*state = SPAPR_DR_ENTITY_SENSE_UNUSABLE;
 }
 }
 
 DPRINTFN("drc: %x, entity_sense: %x", get_index(drc), state);
-  

[Qemu-devel] [PULL 18/36] spapr: Initialize hotplug memory address space

[David Gibson]

From: Bharata B Rao 

Initialize a hotplug memory region under which all the hotplugged
memory is accommodated. Also enable memory hotplug by setting
CONFIG_MEM_HOTPLUG.

Modelled on i386 memory hotplug.

Signed-off-by: Bharata B Rao 
Reviewed-by: David Gibson 
Signed-off-by: David Gibson 
---
 default-configs/ppc64-softmmu.mak |  1 +
 hw/ppc/spapr.c| 18 ++
 include/hw/ppc/spapr.h| 12 
 3 files changed, 31 insertions(+)

diff --git a/default-configs/ppc64-softmmu.mak 
b/default-configs/ppc64-softmmu.mak
index ab62cc7..e77cb1a 100644
--- a/default-configs/ppc64-softmmu.mak
+++ b/default-configs/ppc64-softmmu.mak
@@ -52,3 +52,4 @@ CONFIG_XICS_KVM=$(and $(CONFIG_PSERIES),$(CONFIG_KVM))
 # For PReP
 CONFIG_MC146818RTC=y
 CONFIG_ISA_TESTDEV=y
+CONFIG_MEM_HOTPLUG=y
diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index d49f322..2fb5e36 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -1562,6 +1562,24 @@ static void ppc_spapr_init(MachineState *machine)
 memory_region_add_subregion(sysmem, 0, rma_region);
 }
 
+/* initialize hotplug memory address space */
+if (machine->ram_size < machine->maxram_size) {
+ram_addr_t hotplug_mem_size = machine->maxram_size - machine->ram_size;
+
+if (machine->ram_slots > SPAPR_MAX_RAM_SLOTS) {
+error_report("unsupported amount of memory slots: %"PRIu64,
+  machine->ram_slots);
+exit(EXIT_FAILURE);
+}
+
+spapr->hotplug_memory.base = ROUND_UP(machine->ram_size,
+  SPAPR_HOTPLUG_MEM_ALIGN);
+memory_region_init(>hotplug_memory.mr, OBJECT(spapr),
+   "hotplug-memory", hotplug_mem_size);
+memory_region_add_subregion(sysmem, spapr->hotplug_memory.base,
+>hotplug_memory.mr);
+}
+
 filename = qemu_find_file(QEMU_FILE_TYPE_BIOS, "spapr-rtas.bin");
 if (!filename) {
 error_report("Could not find LPAR rtas '%s'", "spapr-rtas.bin");
diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h
index 8a0a74d..e882770 100644
--- a/include/hw/ppc/spapr.h
+++ b/include/hw/ppc/spapr.h
@@ -5,6 +5,7 @@
 #include "hw/boards.h"
 #include "hw/ppc/xics.h"
 #include "hw/ppc/spapr_drc.h"
+#include "hw/mem/pc-dimm.h"
 
 struct VIOsPAPRBus;
 struct sPAPRPHBState;
@@ -76,6 +77,7 @@ struct sPAPRMachineState {
 
 /*< public >*/
 char *kvm_type;
+MemoryHotplugState hotplug_memory;
 };
 
 #define H_SUCCESS 0
@@ -610,4 +612,14 @@ int spapr_rtc_import_offset(DeviceState *dev, int64_t 
legacy_offset);
 
 #define SPAPR_MEMORY_BLOCK_SIZE (1 << 28) /* 256MB */
 
+/*
+ * This defines the maximum number of DIMM slots we can have for sPAPR
+ * guest. This is not defined by sPAPR but we are defining it to 32 slots
+ * based on default number of slots provided by PowerPC kernel.
+ */
+#define SPAPR_MAX_RAM_SLOTS 32
+
+/* 1GB alignment for hotplug memory region */
+#define SPAPR_HOTPLUG_MEM_ALIGN (1ULL << 30)
+
 #endif /* !defined (__HW_SPAPR_H__) */
-- 
2.4.3

[Qemu-devel] [PULL 23/36] spapr: Support ibm, dynamic-reconfiguration-memory

[David Gibson]

From: Bharata B Rao 

Parse ibm,architecture.vec table obtained from the guest and enable
memory node configuration via ibm,dynamic-reconfiguration-memory if guest
supports it. This is in preparation to support memory hotplug for
sPAPR guests.

This changes the way memory node configuration is done. Currently all
memory nodes are built upfront. But after this patch, only memory@0 node
for RMA is built upfront. Guest kernel boots with just that and rest of
the memory nodes (via memory@XXX or ibm,dynamic-reconfiguration-memory)
are built when guest does ibm,client-architecture-support call.

Note: This patch needs a SLOF enhancement which is already part of
SLOF binary in QEMU.

Signed-off-by: Bharata B Rao 
Reviewed-by: David Gibson 
Signed-off-by: David Gibson 
---
 docs/specs/ppc-spapr-hotplug.txt |  48 +
 hw/ppc/spapr.c   | 210 +++
 hw/ppc/spapr_hcall.c |  51 --
 include/hw/ppc/spapr.h   |  15 ++-
 4 files changed, 274 insertions(+), 50 deletions(-)

diff --git a/docs/specs/ppc-spapr-hotplug.txt b/docs/specs/ppc-spapr-hotplug.txt
index 46e0719..631b0ca 100644
--- a/docs/specs/ppc-spapr-hotplug.txt
+++ b/docs/specs/ppc-spapr-hotplug.txt
@@ -302,4 +302,52 @@ consisting of ,  and .
 pseries guests use this property to note the maximum allowed CPUs for the
 guest.
 
+== ibm,dynamic-reconfiguration-memory ==
+
+ibm,dynamic-reconfiguration-memory is a device tree node that represents
+dynamically reconfigurable logical memory blocks (LMB). This node
+is generated only when the guest advertises the support for it via
+ibm,client-architecture-support call. Memory that is not dynamically
+reconfigurable is represented by /memory nodes. The properties of this
+node that are of interest to the sPAPR memory hotplug implementation
+in QEMU are described here.
+
+ibm,lmb-size
+
+This 64bit integer defines the size of each dynamically reconfigurable LMB.
+
+ibm,associativity-lookup-arrays
+
+This property defines a lookup array in which the NUMA associativity
+information for each LMB can be found. It is a property encoded array
+that begins with an integer M, the number of associativity lists followed
+by an integer N, the number of entries per associativity list and terminated
+by M associativity lists each of length N integers.
+
+This property provides the same information as given by ibm,associativity
+property in a /memory node. Each assigned LMB has an index value between
+0 and M-1 which is used as an index into this table to select which
+associativity list to use for the LMB. This index value for each LMB
+is defined in ibm,dynamic-memory property.
+
+ibm,dynamic-memory
+
+This property describes the dynamically reconfigurable memory. It is a
+property encoded array that has an integer N, the number of LMBs followed
+by N LMB list entires.
+
+Each LMB list entry consists of the following elements:
+
+- Logical address of the start of the LMB encoded as a 64bit integer. This
+  corresponds to reg property in /memory node.
+- DRC index of the LMB that corresponds to ibm,my-drc-index property
+  in a /memory node.
+- Four bytes reserved for expansion.
+- Associativity list index for the LMB that is used as an index into
+  ibm,associativity-lookup-arrays property described earlier. This
+  is used to retrieve the right associativity list to be used for this
+  LMB.
+- A 32bit flags word. The bit at bit position 0x0008 defines whether
+  the LMB is assigned to the the partition as of boot time.
+
 [1] http://thread.gmane.org/gmane.linux.ports.ppc.embedded/75350/focus=106867
diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index 940a82f..2f49f97 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -504,44 +504,7 @@ static void *spapr_create_fdt_skel(hwaddr initrd_base,
 return fdt;
 }
 
-int spapr_h_cas_compose_response(sPAPRMachineState *spapr,
- target_ulong addr, target_ulong size)
-{
-void *fdt, *fdt_skel;
-sPAPRDeviceTreeUpdateHeader hdr = { .version_id = 1 };
-
-size -= sizeof(hdr);
-
-/* Create sceleton */
-fdt_skel = g_malloc0(size);
-_FDT((fdt_create(fdt_skel, size)));
-_FDT((fdt_begin_node(fdt_skel, "")));
-_FDT((fdt_end_node(fdt_skel)));
-_FDT((fdt_finish(fdt_skel)));
-fdt = g_malloc0(size);
-_FDT((fdt_open_into(fdt_skel, fdt, size)));
-g_free(fdt_skel);
-
-/* Fix skeleton up */
-_FDT((spapr_fixup_cpu_dt(fdt, spapr)));
-
-/* Pack resulting tree */
-_FDT((fdt_pack(fdt)));
-
-if (fdt_totalsize(fdt) + sizeof(hdr) > size) {
-trace_spapr_cas_failed(size);
-return -1;
-}
-
-cpu_physical_memory_write(addr, , sizeof(hdr));
-cpu_physical_memory_write(addr + sizeof(hdr), fdt, fdt_totalsize(fdt));
-trace_spapr_cas_continue(fdt_totalsize(fdt) + sizeof(hdr));
-g_free(fdt);
-
-

[Qemu-devel] [PULL 26/36] spapr: Don't allow memory hotplug to memory less nodes

[David Gibson]

From: Bharata B Rao 

Currently PowerPC kernel doesn't allow hot-adding memory to memory-less
node, but instead will silently add the memory to the first node that has
some memory. This causes two unexpected behaviours for the user.

- Memory gets hotplugged to a different node than what the user specified.
- Since pc-dimm subsystem in QEMU still thinks that memory belongs to
  memory-less node, a reboot will set things accordingly and the previously
  hotplugged memory now ends in the right node. This appears as if some
  memory moved from one node to another.

So until kernel starts supporting memory hotplug to memory-less
nodes, just prevent such attempts upfront in QEMU.

Signed-off-by: Bharata B Rao 
Reviewed-by: David Gibson 
Signed-off-by: David Gibson 
---
 hw/ppc/spapr.c | 24 +++-
 1 file changed, 23 insertions(+), 1 deletion(-)

diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index 5b46229..1170028 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -2136,7 +2136,7 @@ static void spapr_machine_device_plug(HotplugHandler 
*hotplug_dev,
 sPAPRMachineClass *smc = SPAPR_MACHINE_GET_CLASS(qdev_get_machine());
 
 if (object_dynamic_cast(OBJECT(dev), TYPE_PC_DIMM)) {
-uint32_t node;
+int node;
 
 if (!smc->dr_lmb_enabled) {
 error_setg(errp, "Memory hotplug not supported for this machine");
@@ -2147,6 +2147,28 @@ static void spapr_machine_device_plug(HotplugHandler 
*hotplug_dev,
 return;
 }
 
+/*
+ * Currently PowerPC kernel doesn't allow hot-adding memory to
+ * memory-less node, but instead will silently add the memory
+ * to the first node that has some memory. This causes two
+ * unexpected behaviours for the user.
+ *
+ * - Memory gets hotplugged to a different node than what the user
+ *   specified.
+ * - Since pc-dimm subsystem in QEMU still thinks that memory belongs
+ *   to memory-less node, a reboot will set things accordingly
+ *   and the previously hotplugged memory now ends in the right node.
+ *   This appears as if some memory moved from one node to another.
+ *
+ * So until kernel starts supporting memory hotplug to memory-less
+ * nodes, just prevent such attempts upfront in QEMU.
+ */
+if (nb_numa_nodes && !numa_info[node].node_mem) {
+error_setg(errp, "Can't hotplug memory to memory-less node %d",
+   node);
+return;
+}
+
 spapr_memory_plug(hotplug_dev, dev, node, errp);
 }
 }
-- 
2.4.3

[Qemu-devel] [PULL 24/36] spapr: Make hash table size a factor of maxram_size

[David Gibson]

From: Bharata B Rao 

The hash table size is dependent on ram_size, but since with hotplug
the memory can grow till maxram_size. Hence make hash table size dependent
on maxram_size.

This allows to hotplug huge amounts of memory to the guest.

Signed-off-by: Bharata B Rao 
Reviewed-by: David Gibson 
Signed-off-by: David Gibson 
---
 hw/ppc/spapr.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index 2f49f97..d21e95c 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -1715,7 +1715,7 @@ static void ppc_spapr_init(MachineState *machine)
  * more than needed for the Linux guests we support. */
 spapr->htab_shift = 18; /* Minimum architected size */
 while (spapr->htab_shift <= 46) {
-if ((1ULL << (spapr->htab_shift + 7)) >= machine->ram_size) {
+if ((1ULL << (spapr->htab_shift + 7)) >= machine->maxram_size) {
 break;
 }
 spapr->htab_shift++;
-- 
2.4.3

Re: [Qemu-devel] [PATCH v10 4/7] vhost-user: add VHOST_USER_GET_QUEUE_NUM message

[Yuanhan Liu]

On Tue, Sep 22, 2015 at 05:56:16PM +0800, Jason Wang wrote:
> 
> 
> On 09/18/2015 10:58 PM, Yuanhan Liu wrote:
> > This is for querying how many queues the backend supports if it has mq
> > support(when VHOST_USER_PROTOCOL_F_MQ flag is set from the quried
> > protocol features).
> >
> > vhost_net_get_max_queues() is the interface to export that value, and
> > to tell if the backend supports # of queues user requested, which is
> > done in the following patch.
> >
> > Signed-off-by: Yuanhan Liu 
> > ---
> >  docs/specs/vhost-user.txt | 11 +++
> >  hw/net/vhost_net.c|  7 +++
> >  hw/virtio/vhost-user.c| 15 ++-
> >  include/hw/virtio/vhost.h |  1 +
> >  include/net/vhost_net.h   |  1 +
> >  5 files changed, 34 insertions(+), 1 deletion(-)
> >
> > diff --git a/docs/specs/vhost-user.txt b/docs/specs/vhost-user.txt
> > index ccbbcbb..43db9b4 100644
> > --- a/docs/specs/vhost-user.txt
> > +++ b/docs/specs/vhost-user.txt
> > @@ -301,3 +301,14 @@ Message types
> >Bits (0-7) of the payload contain the vring index. Bit 8 is the
> >invalid FD flag. This flag is set when there is no file descriptor
> >in the ancillary data.
> > +
> > + * VHOST_USER_GET_QUEUE_NUM
> > +
> > +  Id: 17
> > +  Equivalent ioctl: N/A
> > +  Master payload: N/A
> > +  Slave payload: u64
> > +
> > +  Query how many queues the backend supports. This request should be
> > +  sent only when VHOST_USER_PROTOCOL_F_MQ is set in quried protocol
> > +  features by VHOST_USER_GET_PROTOCOL_FEATURES.
> > diff --git a/hw/net/vhost_net.c b/hw/net/vhost_net.c
> > index b7d29b7..bc88006 100644
> > --- a/hw/net/vhost_net.c
> > +++ b/hw/net/vhost_net.c
> > @@ -122,6 +122,11 @@ void vhost_net_ack_features(struct vhost_net *net, 
> > uint64_t features)
> >  vhost_ack_features(>dev, vhost_net_get_feature_bits(net), 
> > features);
> >  }
> >  
> > +uint64_t vhost_net_get_max_queues(VHostNetState *net)
> > +{
> > +return net->dev.max_queues;
> > +}
> 
> Looks like need a dummy function for !CONFIG_VHOST_NET.

Yes, it is. A such build error report has been already reported by some
one.

--yliu
> 
> > +
> >  static int vhost_net_get_fd(NetClientState *backend)
> >  {
> >  switch (backend->info->type) {
> > @@ -144,6 +149,8 @@ struct vhost_net *vhost_net_init(VhostNetOptions 
> > *options)
> >  goto fail;
> >  }
> >  
> > +net->dev.max_queues = 1;
> > +
> >  if (backend_kernel) {
> >  r = vhost_net_get_fd(options->net_backend);
> >  if (r < 0) {
> > diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c
> > index 9cb2f52..694fde5 100644
> > --- a/hw/virtio/vhost-user.c
> > +++ b/hw/virtio/vhost-user.c
> > @@ -25,7 +25,9 @@
> >  
> >  #define VHOST_MEMORY_MAX_NREGIONS8
> >  #define VHOST_USER_F_PROTOCOL_FEATURES 30
> > -#define VHOST_USER_PROTOCOL_FEATURE_MASK 0x0ULL
> > +#define VHOST_USER_PROTOCOL_FEATURE_MASK 0x1ULL
> > +
> > +#define VHOST_USER_PROTOCOL_F_MQ0
> >  
> >  typedef enum VhostUserRequest {
> >  VHOST_USER_NONE = 0,
> > @@ -45,6 +47,7 @@ typedef enum VhostUserRequest {
> >  VHOST_USER_SET_VRING_ERR = 14,
> >  VHOST_USER_GET_PROTOCOL_FEATURES = 15,
> >  VHOST_USER_SET_PROTOCOL_FEATURES = 16,
> > +VHOST_USER_GET_QUEUE_NUM = 17,
> >  VHOST_USER_MAX
> >  } VhostUserRequest;
> >  
> > @@ -211,6 +214,7 @@ static int vhost_user_call(struct vhost_dev *dev, 
> > unsigned long int request,
> >  switch (msg_request) {
> >  case VHOST_USER_GET_FEATURES:
> >  case VHOST_USER_GET_PROTOCOL_FEATURES:
> > +case VHOST_USER_GET_QUEUE_NUM:
> >  need_reply = 1;
> >  break;
> >  
> > @@ -315,6 +319,7 @@ static int vhost_user_call(struct vhost_dev *dev, 
> > unsigned long int request,
> >  switch (msg_request) {
> >  case VHOST_USER_GET_FEATURES:
> >  case VHOST_USER_GET_PROTOCOL_FEATURES:
> > +case VHOST_USER_GET_QUEUE_NUM:
> >  if (msg.size != sizeof(m.u64)) {
> >  error_report("Received bad msg size.");
> >  return -1;
> > @@ -366,6 +371,14 @@ static int vhost_user_init(struct vhost_dev *dev, void 
> > *opaque)
> >  if (err < 0) {
> >  return err;
> >  }
> > +
> > +/* query the max queues we support if backend supports Multiple 
> > Queue */
> > +if (dev->protocol_features & (1ULL << VHOST_USER_PROTOCOL_F_MQ)) {
> > +err = vhost_user_call(dev, VHOST_USER_GET_QUEUE_NUM, 
> > >max_queues);
> > +if (err < 0) {
> > +return err;
> > +}
> > +}
> >  }
> >  
> >  return 0;
> > diff --git a/include/hw/virtio/vhost.h b/include/hw/virtio/vhost.h
> > index 6467c73..c3758f3 100644
> > --- a/include/hw/virtio/vhost.h
> > +++ b/include/hw/virtio/vhost.h
> > @@ -48,6 +48,7 @@ struct vhost_dev {
> >  unsigned long long acked_features;
> >  unsigned long long 

[Qemu-devel] [PULL 28/36] spapr: Populate ibm, associativity-lookup-arrays correctly for non-NUMA

[David Gibson]

From: Bharata B Rao 

When NUMA isn't configured explicitly, assume node 0 is present for
the purpose of creating ibm,associativity-lookup-arrays property
under ibm,dynamic-reconfiguration-memory DT node. This ensures that
the associativity index property is correctly updated in ibm,dynamic-memory
for the LMB that is hotplugged.

Signed-off-by: Bharata B Rao 
Reviewed-by: David Gibson 
Signed-off-by: David Gibson 
---
 hw/ppc/spapr.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index 5c81e19..64d07cd 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -724,6 +724,7 @@ static int spapr_populate_drconf_memory(sPAPRMachineState 
*spapr, void *fdt)
 uint32_t nr_lmbs = machine->maxram_size/lmb_size - nr_rma_lmbs;
 uint32_t nr_assigned_lmbs = machine->ram_size/lmb_size - nr_rma_lmbs;
 uint32_t *int_buf, *cur_index, buf_len;
+int nr_nodes = nb_numa_nodes ? nb_numa_nodes : 1;
 
 /* Allocate enough buffer size to fit in ibm,dynamic-memory */
 buf_len = nr_lmbs * SPAPR_DR_LMB_LIST_ENTRY_SIZE * sizeof(uint32_t) +
@@ -789,10 +790,10 @@ static int spapr_populate_drconf_memory(sPAPRMachineState 
*spapr, void *fdt)
 
 /* ibm,associativity-lookup-arrays */
 cur_index = int_buf;
-int_buf[0] = cpu_to_be32(nb_numa_nodes);
+int_buf[0] = cpu_to_be32(nr_nodes);
 int_buf[1] = cpu_to_be32(4); /* Number of entries per associativity list */
 cur_index += 2;
-for (i = 0; i < nb_numa_nodes; i++) {
+for (i = 0; i < nr_nodes; i++) {
 uint32_t associativity[] = {
 cpu_to_be32(0x0),
 cpu_to_be32(0x0),
-- 
2.4.3

Re: [Qemu-devel] [PATCH v10 6/7] vhost-user: add multiple queue support

[Yuanhan Liu]

On Wed, Sep 23, 2015 at 10:12:10AM +0800, Jason Wang wrote:
> 
> 
> On 09/23/2015 09:57 AM, Yuanhan Liu wrote:
> > On Tue, Sep 22, 2015 at 06:14:14PM +0800, Jason Wang wrote:
> >>
> > [...]
> >>> -static void net_vhost_link_down(VhostUserState *s, bool link_down)
> >>> +static void net_vhost_link_down(int queues, NetClientState *ncs[],
> >>> +bool link_down)
> >>>  {
> >>> -s->nc.link_down = link_down;
> >>> +NetClientState *nc;
> >>> +int i;
> >>>  
> >>> -if (s->nc.peer) {
> >>> -s->nc.peer->link_down = link_down;
> >>> -}
> >>> +for (i = 0; i < queues; i++) {
> >>> +nc = ncs[i];
> >>>  
> >>> -if (s->nc.info->link_status_changed) {
> >>> -s->nc.info->link_status_changed(>nc);
> >>> -}
> >>> +nc->link_down = link_down;
> >>> +
> >>> +if (nc->peer) {
> >>> +nc->peer->link_down = link_down;
> >>> +}
> >>> +
> >>> +if (nc->info->link_status_changed) {
> >>> +nc->info->link_status_changed(nc);
> >>> +}
> >>>  
> >>> -if (s->nc.peer && s->nc.peer->info->link_status_changed) {
> >>> -s->nc.peer->info->link_status_changed(s->nc.peer);
> >>> +if (nc->peer && nc->peer->info->link_status_changed) {
> >>> +nc->peer->info->link_status_changed(nc->peer);
> >>> +}
> >>>  }
> >>>  }
> >> The semantics is a little bit difference with qmp_set_link. What's the
> >> reason for this? If there's no specific reason, probably, we could just
> >> reuse qmp_set_link() (or part of) here?
> > I did this just because I refactored the vhost_user init code, making
> > the char dev handler to initiate all ncs. Hence I turned 
> > net_vhost_link_down()
> > to handle all ncs, just like what I did for vhost_user_start().
> >
> > And TBH, I'm not aware of qmp_set_link() before, and after reading the
> > code, I do think we can simply invoke it instead. And thanks for the
> > info.
> >
> >> Other looks good to me.
> > Thanks for the review. May I add your reviewed-by if I fix above issue?
> >
> > --yliu
> 
> I prefer to add it myself :)

Fair enough; I meant to save your effort :-)

--yliu

Re: [Qemu-devel] [PULL 00/33] spapr-next queue 2015-09-16

[David Gibson]

On Mon, Sep 21, 2015 at 02:33:12PM -0700, Peter Maydell wrote:
> On 20 September 2015 at 18:05, David Gibson  
> wrote:
> > The following changes since commit 18640989a9f5e4d2e84b566c52ff1fccfa0dbf4a:
> >
> >   Merge remote-tracking branch 
> > 'remotes/afaerber/tags/qom-devices-for-peter' into staging (2015-09-19 
> > 15:59:52 +0100)
> >
> > are available in the git repository at:
> >
> >   git://github.com/dgibson/qemu.git spapr-next-20150921
> >
> > for you to fetch changes up to 5576a6644cb302ac5b715205e7b4f157f116b9de:
> >
> >   ppc/spapr: Fix buffer overflow in spapr_populate_drconf_memory() 
> > (2015-09-21 10:35:42 +1000)
> >
> > 
> > sPAPR Patch Queue: 2015-09-21
> >
> > Highlights:
> > * Memory hotplug for "pseries" guests
> > * Fixes to the PAPR Dynamic Reconfiguration hotplug code
> > * Several PAPR compliance fixes
> > * New SLOF with:
> > * GPT support
> > * Much faster VGA handling
> 
> Hi. I'm afraid this fails 'make check':
> 
> qemu-system-ppc64:
> /home/petmay01/linaro/qemu-for-merges/hw/core/machine.c:324:
> machine_class_base_init: Assertion `g_str_has_suffix(cname,
> "-machine")' failed.

Sod, sorry.  Right after I sent it I realised that I hadn't updated
the 2.5 machine type (added in that pull) for the stuff requiring
"-machine" in the type names.

I hadn't realised it broke things so badly though, because I thought
Travis CI would catch a make check failure for me.  Seems not though
:(.

I'll fix this up and send a new pull shortly.

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


pgpIhDhd5WHKT.pgp
Description: PGP signature

[Qemu-devel] [PULL 11/36] sPAPR: Introduce rtas_ldq()

[David Gibson]

From: Gavin Shan 

This introduces rtas_ldq() to load 64-bits parameter from continuous
two 4-bytes memory chunk of RTAS parameter buffer, to simplify the
code.

Signed-off-by: Gavin Shan 
Reviewed-by: Thomas Huth 
Reviewed-by: David Gibson 
Signed-off-by: David Gibson 
---
 hw/ppc/spapr_pci.c | 20 ++--
 include/hw/ppc/spapr.h |  5 +
 2 files changed, 15 insertions(+), 10 deletions(-)

diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c
index 6782fd0..54292c9 100644
--- a/hw/ppc/spapr_pci.c
+++ b/hw/ppc/spapr_pci.c
@@ -140,7 +140,7 @@ static void rtas_ibm_read_pci_config(PowerPCCPU *cpu, 
sPAPRMachineState *spapr,
 return;
 }
 
-buid = ((uint64_t)rtas_ld(args, 1) << 32) | rtas_ld(args, 2);
+buid = rtas_ldq(args, 1);
 size = rtas_ld(args, 3);
 addr = rtas_ld(args, 0);
 
@@ -206,7 +206,7 @@ static void rtas_ibm_write_pci_config(PowerPCCPU *cpu, 
sPAPRMachineState *spapr,
 return;
 }
 
-buid = ((uint64_t)rtas_ld(args, 1) << 32) | rtas_ld(args, 2);
+buid = rtas_ldq(args, 1);
 val = rtas_ld(args, 4);
 size = rtas_ld(args, 3);
 addr = rtas_ld(args, 0);
@@ -269,7 +269,7 @@ static void rtas_ibm_change_msi(PowerPCCPU *cpu, 
sPAPRMachineState *spapr,
 target_ulong rets)
 {
 uint32_t config_addr = rtas_ld(args, 0);
-uint64_t buid = ((uint64_t)rtas_ld(args, 1) << 32) | rtas_ld(args, 2);
+uint64_t buid = rtas_ldq(args, 1);
 unsigned int func = rtas_ld(args, 3);
 unsigned int req_num = rtas_ld(args, 4); /* 0 == remove all */
 unsigned int seq_num = rtas_ld(args, 5);
@@ -391,7 +391,7 @@ static void 
rtas_ibm_query_interrupt_source_number(PowerPCCPU *cpu,
target_ulong rets)
 {
 uint32_t config_addr = rtas_ld(args, 0);
-uint64_t buid = ((uint64_t)rtas_ld(args, 1) << 32) | rtas_ld(args, 2);
+uint64_t buid = rtas_ldq(args, 1);
 unsigned int intr_src_num = -1, ioa_intr_num = rtas_ld(args, 3);
 sPAPRPHBState *phb = NULL;
 PCIDevice *pdev = NULL;
@@ -440,7 +440,7 @@ static void rtas_ibm_set_eeh_option(PowerPCCPU *cpu,
 goto param_error_exit;
 }
 
-buid = ((uint64_t)rtas_ld(args, 1) << 32) | rtas_ld(args, 2);
+buid = rtas_ldq(args, 1);
 addr = rtas_ld(args, 0);
 option = rtas_ld(args, 3);
 
@@ -484,7 +484,7 @@ static void rtas_ibm_get_config_addr_info2(PowerPCCPU *cpu,
 goto param_error_exit;
 }
 
-buid = ((uint64_t)rtas_ld(args, 1) << 32) | rtas_ld(args, 2);
+buid = rtas_ldq(args, 1);
 sphb = spapr_pci_find_phb(spapr, buid);
 if (!sphb) {
 goto param_error_exit;
@@ -539,7 +539,7 @@ static void rtas_ibm_read_slot_reset_state2(PowerPCCPU *cpu,
 goto param_error_exit;
 }
 
-buid = ((uint64_t)rtas_ld(args, 1) << 32) | rtas_ld(args, 2);
+buid = rtas_ldq(args, 1);
 sphb = spapr_pci_find_phb(spapr, buid);
 if (!sphb) {
 goto param_error_exit;
@@ -584,7 +584,7 @@ static void rtas_ibm_set_slot_reset(PowerPCCPU *cpu,
 goto param_error_exit;
 }
 
-buid = ((uint64_t)rtas_ld(args, 1) << 32) | rtas_ld(args, 2);
+buid = rtas_ldq(args, 1);
 option = rtas_ld(args, 3);
 sphb = spapr_pci_find_phb(spapr, buid);
 if (!sphb) {
@@ -619,7 +619,7 @@ static void rtas_ibm_configure_pe(PowerPCCPU *cpu,
 goto param_error_exit;
 }
 
-buid = ((uint64_t)rtas_ld(args, 1) << 32) | rtas_ld(args, 2);
+buid = rtas_ldq(args, 1);
 sphb = spapr_pci_find_phb(spapr, buid);
 if (!sphb) {
 goto param_error_exit;
@@ -654,7 +654,7 @@ static void rtas_ibm_slot_error_detail(PowerPCCPU *cpu,
 goto param_error_exit;
 }
 
-buid = ((uint64_t)rtas_ld(args, 1) << 32) | rtas_ld(args, 2);
+buid = rtas_ldq(args, 1);
 sphb = spapr_pci_find_phb(spapr, buid);
 if (!sphb) {
 goto param_error_exit;
diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h
index d250967..cbe3463 100644
--- a/include/hw/ppc/spapr.h
+++ b/include/hw/ppc/spapr.h
@@ -489,6 +489,11 @@ static inline uint32_t rtas_ld(target_ulong phys, int n)
 return ldl_be_phys(_space_memory, ppc64_phys_to_real(phys + 4*n));
 }
 
+static inline uint64_t rtas_ldq(target_ulong phys, int n)
+{
+return (uint64_t)rtas_ld(phys, n) << 32 | rtas_ld(phys, n + 1);
+}
+
 static inline void rtas_st(target_ulong phys, int n, uint32_t val)
 {
 stl_be_phys(_space_memory, ppc64_phys_to_real(phys + 4*n), val);
-- 
2.4.3

[Qemu-devel] [PULL 06/36] spapr: SPLPAR Characteristics

[David Gibson]

From: Sam Bobroff 

Improve the SPLPAR Characteristics information:

Add MaxPlatProcs: set to max_cpus, the maximum CPUs that could be
addded to the system.
Add DesMem: set to the initial memory of the system.
Add DesProcs: set to smp_cpus, the inital number of CPUs in the
system.

These tokens and values are specified by PAPR.

Signed-off-by: Sam Bobroff 
Reviewed-by: David Gibson 
Signed-off-by: David Gibson 
---
 hw/ppc/spapr_rtas.c | 11 +--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/hw/ppc/spapr_rtas.c b/hw/ppc/spapr_rtas.c
index 9869bc9..5cbf9a0 100644
--- a/hw/ppc/spapr_rtas.c
+++ b/hw/ppc/spapr_rtas.c
@@ -34,6 +34,7 @@
 #include "hw/ppc/spapr.h"
 #include "hw/ppc/spapr_vio.h"
 #include "qapi-event.h"
+#include "hw/boards.h"
 
 #include 
 #include "hw/ppc/spapr_drc.h"
@@ -240,8 +241,14 @@ static void rtas_ibm_get_system_parameter(PowerPCCPU *cpu,
 
 switch (parameter) {
 case RTAS_SYSPARM_SPLPAR_CHARACTERISTICS: {
-char *param_val = g_strdup_printf("MaxEntCap=%d,MaxPlatProcs=%d",
-  max_cpus, smp_cpus);
+char *param_val = g_strdup_printf("MaxEntCap=%d,"
+  "DesMem=%llu,"
+  "DesProcs=%d,"
+  "MaxPlatProcs=%d",
+  max_cpus,
+  current_machine->ram_size / M_BYTE,
+  smp_cpus,
+  max_cpus);
 rtas_st_buffer(buffer, length, (uint8_t *)param_val, 
strlen(param_val));
 g_free(param_val);
 break;
-- 
2.4.3

[Qemu-devel] [PULL 20/36] spapr: Don't use QOM [*] syntax for DR connectors.

[David Gibson]

The dynamic reconfiguration (hotplug) code for the pseries machine type
uses a "DR connector" QOM object for each resource it will be possible
to hotplug.  Each of these is added to its owner using
object_property_add_child(owner, "dr-connector[*], ...);

That works ok, mostly, but it means that the property indices are
arbitrary, depending on the order in which the connectors are constructed.
That might line up to something useful, but it doesn't have to.

It will get worse once we add hotplug RAM support.  That will add a DR
connector object for every 256MB of potential memory.  So if maxmem=2T,
for example, there are 8192 objects under the same parent.

The QOM interfaces aren't really designed for this.  In particular
object_property_add() with [*] has O(n^2) time complexity (in the number of
existing children): first it has a linear search through array indices to
find a free slot, each of which is attempted to a recursive call to
object_property_add() with a specific [N].  Those calls are O(n) because
there's a linear search through all properties to check for duplicates.

By using a meaningful index value, which we already know is unique we can
avoid the [*] special behaviour.  That lets us reduce the total time for
creating the DR objects from O(n^3) to O(n^2).

O(n^2) is still kind of crappy, but it's enough to reduce the startup time
of qemu (with in-progress memory hotplug support) with maxmem=2T from ~20
minutes to ~4 seconds.

Signed-off-by: David Gibson 
Cc: Bharata B Rao 
Tested-by: Bharata B Rao 
Reviewed-by: Alexey Kardashevskiy 
---
 hw/ppc/spapr_drc.c | 5 -
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/hw/ppc/spapr_drc.c b/hw/ppc/spapr_drc.c
index b7b9891..5d6ea7c 100644
--- a/hw/ppc/spapr_drc.c
+++ b/hw/ppc/spapr_drc.c
@@ -473,14 +473,17 @@ sPAPRDRConnector *spapr_dr_connector_new(Object *owner,
 {
 sPAPRDRConnector *drc =
 SPAPR_DR_CONNECTOR(object_new(TYPE_SPAPR_DR_CONNECTOR));
+char *prop_name;
 
 g_assert(type);
 
 drc->type = type;
 drc->id = id;
 drc->owner = owner;
-object_property_add_child(owner, "dr-connector[*]", OBJECT(drc), NULL);
+prop_name = g_strdup_printf("dr-connector[%"PRIu32"]", get_index(drc));
+object_property_add_child(owner, prop_name, OBJECT(drc), NULL);
 object_property_set_bool(OBJECT(drc), true, "realized", NULL);
+g_free(prop_name);
 
 /* human-readable name for a DRC to encode into the DT
  * description. this is mainly only used within a guest in place
-- 
2.4.3

[Qemu-devel] [PULL 12/36] pseries: define coldplugged devices as "configured"

[David Gibson]

From: Laurent Vivier 

When a device is hotplugged, attach() sets "configured" to
false, waiting an action from the OS to configure it and then
to call ibm,configure-connector. On ibm,configure-connector,
the hypervisor sets "configured" to true.

In case of coldplugged device, attach() sets "configured" to
false, but firmware and OS never call the ibm,configure-connector
in this case, so it remains set to false.

It could be harmless, but when we unplug a device, hypervisor
waits the device becomes configured because for it, a not configured
device is a device being configured, so it waits the end of configuration
to unplug it... and it never happens, so it is never unplugged.

This patch set by default coldplugged device to "configured=true",
hotplugged device to "configured=false".

Signed-off-by: Laurent Vivier 
Reviewed-by: David Gibson 
Signed-off-by: David Gibson 
---
 hw/ppc/spapr_drc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/ppc/spapr_drc.c b/hw/ppc/spapr_drc.c
index 8cbcf4d..9ce844a 100644
--- a/hw/ppc/spapr_drc.c
+++ b/hw/ppc/spapr_drc.c
@@ -310,7 +310,7 @@ static void attach(sPAPRDRConnector *drc, DeviceState *d, 
void *fdt,
 drc->dev = d;
 drc->fdt = fdt;
 drc->fdt_start_offset = fdt_start_offset;
-drc->configured = false;
+drc->configured = coldplug;
 
 object_property_add_link(OBJECT(drc), "device",
  object_get_typename(OBJECT(drc->dev)),
-- 
2.4.3

[Qemu-devel] [PULL 22/36] spapr: Add LMB DR connectors

[David Gibson]

Enable memory hotplug for pseries 2.4 and add LMB DR connectors.
With memory hotplug, enforce RAM size, NUMA node memory size and maxmem
to be a multiple of SPAPR_MEMORY_BLOCK_SIZE (256M) since that's the
granularity in which LMBs are represented and hot-added.

LMB DR connectors will be used by the memory hotplug code.

Signed-off-by: Bharata B Rao 
Signed-off-by: Michael Roth 
   [spapr_drc_reset implementation]
[since this missed the 2.4 cutoff, changing to only enable for 2.5]
Signed-off-by: David Gibson 
---
 hw/ppc/spapr.c | 87 ++
 include/hw/ppc/spapr.h |  1 +
 2 files changed, 88 insertions(+)

diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index 33e22ce..940a82f 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -61,6 +61,7 @@
 #include "hw/nmi.h"
 
 #include "hw/compat.h"
+#include "qemu-common.h"
 
 #include 
 
@@ -1446,10 +1447,84 @@ static void spapr_cpu_init(sPAPRMachineState *spapr, 
PowerPCCPU *cpu)
 qemu_register_reset(spapr_cpu_reset, cpu);
 }
 
+/*
+ * Reset routine for LMB DR devices.
+ *
+ * Unlike PCI DR devices, LMB DR devices explicitly register this reset
+ * routine. Reset for PCI DR devices will be handled by PHB reset routine
+ * when it walks all its children devices. LMB devices reset occurs
+ * as part of spapr_ppc_reset().
+ */
+static void spapr_drc_reset(void *opaque)
+{
+sPAPRDRConnector *drc = opaque;
+DeviceState *d = DEVICE(drc);
+
+if (d) {
+device_reset(d);
+}
+}
+
+static void spapr_create_lmb_dr_connectors(sPAPRMachineState *spapr)
+{
+MachineState *machine = MACHINE(spapr);
+uint64_t lmb_size = SPAPR_MEMORY_BLOCK_SIZE;
+uint32_t nr_rma_lmbs = spapr->rma_size/lmb_size;
+uint32_t nr_lmbs = machine->maxram_size/lmb_size - nr_rma_lmbs;
+uint32_t nr_assigned_lmbs = machine->ram_size/lmb_size - nr_rma_lmbs;
+int i;
+
+for (i = 0; i < nr_lmbs; i++) {
+sPAPRDRConnector *drc;
+uint64_t addr;
+
+if (i < nr_assigned_lmbs) {
+addr = (i + nr_rma_lmbs) * lmb_size;
+} else {
+addr = (i - nr_assigned_lmbs) * lmb_size +
+spapr->hotplug_memory.base;
+}
+drc = spapr_dr_connector_new(OBJECT(spapr), 
SPAPR_DR_CONNECTOR_TYPE_LMB,
+ addr/lmb_size);
+qemu_register_reset(spapr_drc_reset, drc);
+}
+}
+
+/*
+ * If RAM size, maxmem size and individual node mem sizes aren't aligned
+ * to SPAPR_MEMORY_BLOCK_SIZE(256MB), then refuse to start the guest
+ * since we can't support such unaligned sizes with DRCONF_MEMORY.
+ */
+static void spapr_validate_node_memory(MachineState *machine)
+{
+int i;
+
+if (machine->maxram_size % SPAPR_MEMORY_BLOCK_SIZE ||
+machine->ram_size % SPAPR_MEMORY_BLOCK_SIZE) {
+error_report("Can't support memory configuration where RAM size "
+ "0x" RAM_ADDR_FMT " or maxmem size "
+ "0x" RAM_ADDR_FMT " isn't aligned to %llu MB",
+ machine->ram_size, machine->maxram_size,
+ SPAPR_MEMORY_BLOCK_SIZE/M_BYTE);
+exit(EXIT_FAILURE);
+}
+
+for (i = 0; i < nb_numa_nodes; i++) {
+if (numa_info[i].node_mem % SPAPR_MEMORY_BLOCK_SIZE) {
+error_report("Can't support memory configuration where memory size"
+ " %" PRIx64 " of node %d isn't aligned to %llu MB",
+ numa_info[i].node_mem, i,
+ SPAPR_MEMORY_BLOCK_SIZE/M_BYTE);
+exit(EXIT_FAILURE);
+}
+}
+}
+
 /* pSeries LPAR / sPAPR hardware init */
 static void ppc_spapr_init(MachineState *machine)
 {
 sPAPRMachineState *spapr = SPAPR_MACHINE(machine);
+sPAPRMachineClass *smc = SPAPR_MACHINE_GET_CLASS(machine);
 const char *kernel_filename = machine->kernel_filename;
 const char *kernel_cmdline = machine->kernel_cmdline;
 const char *initrd_filename = machine->initrd_filename;
@@ -1528,6 +1603,10 @@ static void ppc_spapr_init(MachineState *machine)
smp_threads),
   XICS_IRQS);
 
+if (smc->dr_lmb_enabled) {
+spapr_validate_node_memory(machine);
+}
+
 /* init CPUs */
 if (machine->cpu_model == NULL) {
 machine->cpu_model = kvm_enabled() ? "host" : "POWER7";
@@ -1578,6 +1657,10 @@ static void ppc_spapr_init(MachineState *machine)
 >hotplug_memory.mr);
 }
 
+if (smc->dr_lmb_enabled) {
+spapr_create_lmb_dr_connectors(spapr);
+}
+
 filename = qemu_find_file(QEMU_FILE_TYPE_BIOS, "spapr-rtas.bin");
 if (!filename) {
 error_report("Could not find LPAR rtas '%s'", "spapr-rtas.bin");
@@ -1851,6 +1934,7 @@ static void spapr_nmi(NMIState *n, int cpu_index, Error 
**errp)
 static 

[Qemu-devel] [PULL 05/36] spapr: Make ibm, change-msi respect 3 return values

[David Gibson]

From: Sam Bobroff 

Currently, rtas_ibm_change_msi() always returns four values even if
less are specified.

Correct this by only returning the fourth parameter if it was
requested.

This is specified by PAPR.

Signed-off-by: Sam Bobroff 
Reviewed-by: David Gibson 
Signed-off-by: David Gibson 
---
 hw/ppc/spapr_pci.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c
index a2feb4c..6782fd0 100644
--- a/hw/ppc/spapr_pci.c
+++ b/hw/ppc/spapr_pci.c
@@ -375,7 +375,9 @@ out:
 rtas_st(rets, 0, RTAS_OUT_SUCCESS);
 rtas_st(rets, 1, req_num);
 rtas_st(rets, 2, ++seq_num);
-rtas_st(rets, 3, ret_intr_type);
+if (nret > 3) {
+rtas_st(rets, 3, ret_intr_type);
+}
 
 trace_spapr_pci_rtas_ibm_change_msi(config_addr, func, req_num, irq);
 }
-- 
2.4.3

[Qemu-devel] [PULL 08/36] spapr_drc: Fix potential undefined behaviour

[David Gibson]

The DRC_INDEX_ID_MASK macro does a left shift on ~0, which is a signed
quantity, and therefore undefined behaviour according to the C spec.  In
particular this causes warnings from the clang sanitizer.

This fixes it by calculating the same mask without using ~0 (I think the
new method is a more common idiom for generating masks anyway).  For good
measure I also use 1ULL to force the expression's type to unsigned long
long, which should be good for assigning to anything we're going to want
to.

Reported-by: Peter Maydell 
Signed-off-by: David Gibson 
Reviewed-by: Alexey Kardashevskiy 
---
 hw/ppc/spapr_drc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/ppc/spapr_drc.c b/hw/ppc/spapr_drc.c
index ee87432..8cbcf4d 100644
--- a/hw/ppc/spapr_drc.c
+++ b/hw/ppc/spapr_drc.c
@@ -32,7 +32,7 @@
 
 #define DRC_CONTAINER_PATH "/dr-connector"
 #define DRC_INDEX_TYPE_SHIFT 28
-#define DRC_INDEX_ID_MASK (~(~0 << DRC_INDEX_TYPE_SHIFT))
+#define DRC_INDEX_ID_MASK ((1ULL << DRC_INDEX_TYPE_SHIFT) - 1)
 
 static sPAPRDRConnectorTypeShift get_type_shift(sPAPRDRConnectorType type)
 {
-- 
2.4.3

[Qemu-devel] [PULL 29/36] spapr: Revert to memory@XXXX representation for non-hotplugged memory

[David Gibson]

From: Bharata B Rao 

Don't represent non-hotluggable memory under drconf node. With this
we don't have to create DRC objects for them.

The effect of this patch is that we revert back to memory@ representation
for all the memory specified with -m option and represent the cold
plugged memory and hot-pluggable memory under
ibm,dynamic-reconfiguration-memory.

Signed-off-by: Bharata B Rao 
Reviewed-by: David Gibson 
Signed-off-by: David Gibson 
---
 hw/ppc/spapr.c | 47 +--
 1 file changed, 9 insertions(+), 38 deletions(-)

diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index 64d07cd..402c00d 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -560,6 +560,7 @@ static int spapr_populate_memory(sPAPRMachineState *spapr, 
void *fdt)
 }
 if (!mem_start) {
 /* ppc_spapr_init() checks for rma_size <= node0_size already */
+spapr_populate_memory_node(fdt, i, 0, spapr->rma_size);
 mem_start += spapr->rma_size;
 node_size -= spapr->rma_size;
 }
@@ -720,9 +721,7 @@ static int spapr_populate_drconf_memory(sPAPRMachineState 
*spapr, void *fdt)
 int ret, i, offset;
 uint64_t lmb_size = SPAPR_MEMORY_BLOCK_SIZE;
 uint32_t prop_lmb_size[] = {0, cpu_to_be32(lmb_size)};
-uint32_t nr_rma_lmbs = spapr->rma_size/lmb_size;
-uint32_t nr_lmbs = machine->maxram_size/lmb_size - nr_rma_lmbs;
-uint32_t nr_assigned_lmbs = machine->ram_size/lmb_size - nr_rma_lmbs;
+uint32_t nr_lmbs = (machine->maxram_size - machine->ram_size)/lmb_size;
 uint32_t *int_buf, *cur_index, buf_len;
 int nr_nodes = nb_numa_nodes ? nb_numa_nodes : 1;
 
@@ -755,15 +754,9 @@ static int spapr_populate_drconf_memory(sPAPRMachineState 
*spapr, void *fdt)
 for (i = 0; i < nr_lmbs; i++) {
 sPAPRDRConnector *drc;
 sPAPRDRConnectorClass *drck;
-uint64_t addr;
+uint64_t addr = i * lmb_size + spapr->hotplug_memory.base;;
 uint32_t *dynamic_memory = cur_index;
 
-if (i < nr_assigned_lmbs) {
-addr = (i + nr_rma_lmbs) * lmb_size;
-} else {
-addr = (i - nr_assigned_lmbs) * lmb_size +
-spapr->hotplug_memory.base;
-}
 drc = spapr_dr_connector_by_id(SPAPR_DR_CONNECTOR_TYPE_LMB,
addr/lmb_size);
 g_assert(drc);
@@ -838,8 +831,6 @@ int spapr_h_cas_compose_response(sPAPRMachineState *spapr,
 /* Generate memory nodes or ibm,dynamic-reconfiguration-memory node */
 if (memory_update && smc->dr_lmb_enabled) {
 _FDT((spapr_populate_drconf_memory(spapr, fdt)));
-} else {
-_FDT((spapr_populate_memory(spapr, fdt)));
 }
 
 /* Pack resulting tree */
@@ -877,23 +868,10 @@ static void spapr_finalize_fdt(sPAPRMachineState *spapr,
 /* open out the base tree into a temp buffer for the final tweaks */
 _FDT((fdt_open_into(spapr->fdt_skel, fdt, FDT_MAX_SIZE)));
 
-/*
- * Add memory@0 node to represent RMA. Rest of the memory is either
- * represented by memory nodes or ibm,dynamic-reconfiguration-memory
- * node later during ibm,client-architecture-support call.
- *
- * If NUMA is configured, ensure that memory@0 ends up in the
- * first memory-less node.
- */
-if (nb_numa_nodes) {
-for (i = 0; i < nb_numa_nodes; ++i) {
-if (numa_info[i].node_mem) {
-spapr_populate_memory_node(fdt, i, 0, spapr->rma_size);
-break;
-}
-}
-} else {
-spapr_populate_memory_node(fdt, 0, 0, spapr->rma_size);
+ret = spapr_populate_memory(spapr, fdt);
+if (ret < 0) {
+fprintf(stderr, "couldn't setup memory nodes in fdt\n");
+exit(1);
 }
 
 ret = spapr_populate_vdevice(spapr->vio_bus, fdt);
@@ -1600,21 +1578,14 @@ static void 
spapr_create_lmb_dr_connectors(sPAPRMachineState *spapr)
 {
 MachineState *machine = MACHINE(spapr);
 uint64_t lmb_size = SPAPR_MEMORY_BLOCK_SIZE;
-uint32_t nr_rma_lmbs = spapr->rma_size/lmb_size;
-uint32_t nr_lmbs = machine->maxram_size/lmb_size - nr_rma_lmbs;
-uint32_t nr_assigned_lmbs = machine->ram_size/lmb_size - nr_rma_lmbs;
+uint32_t nr_lmbs = (machine->maxram_size - machine->ram_size)/lmb_size;
 int i;
 
 for (i = 0; i < nr_lmbs; i++) {
 sPAPRDRConnector *drc;
 uint64_t addr;
 
-if (i < nr_assigned_lmbs) {
-addr = (i + nr_rma_lmbs) * lmb_size;
-} else {
-addr = (i - nr_assigned_lmbs) * lmb_size +
-spapr->hotplug_memory.base;
-}
+addr = i * lmb_size + spapr->hotplug_memory.base;
 drc = spapr_dr_connector_new(OBJECT(spapr), 
SPAPR_DR_CONNECTOR_TYPE_LMB,
  addr/lmb_size);
 

[Qemu-devel] [PULL 16/36] spapr_pci: fix device tree props for MSI/MSI-X

[David Gibson]

From: Michael Roth 

PAPR requires ibm,req#msi and ibm,req#msi-x to be present in the
device node to define the number of msi/msi-x interrupts the device
supports, respectively.

Currently we have ibm,req#msi-x hardcoded to a non-sensical constant
that happens to be 2, and are missing ibm,req#msi entirely. The result
of that is that msi-x capable devices get limited to 2 msi-x
interrupts (which can impact performance), and msi-only devices likely
wouldn't work at all. Additionally, if devices expect a minimum that
exceeds 2, the guest driver may fail to load entirely.

SLOF still owns the generation of these properties at boot-time
(although other device properties have since been offloaded to QEMU),
but for hotplugged devices we rely on the values generated by QEMU
and thus hit the limitations above.

Fix this by generating these properties in QEMU as expected by guests.

In the future it may make sense to modify SLOF to pass through these
values directly as we do with other props since we're duplicating SLOF
code.

Cc: qemu-...@nongnu.org
Cc: qemu-sta...@nongnu.org
Cc: David Gibson 
Cc: Nikunj A Dadhania 
Signed-off-by: Michael Roth 
Reviewed-by: David Gibson 
Signed-off-by: David Gibson 
---
 hw/ppc/spapr_pci.c | 12 ++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c
index 54292c9..d2188c8 100644
--- a/hw/ppc/spapr_pci.c
+++ b/hw/ppc/spapr_pci.c
@@ -957,6 +957,7 @@ static int spapr_populate_pci_child_dt(PCIDevice *dev, void 
*fdt, int offset,
 int pci_status, err;
 char *buf = NULL;
 uint32_t drc_index = spapr_phb_get_pci_drc_index(sphb, dev);
+uint32_t max_msi, max_msix;
 
 if (pci_default_read_config(dev, PCI_HEADER_TYPE, 1) ==
 PCI_HEADER_TYPE_BRIDGE) {
@@ -1037,8 +1038,15 @@ static int spapr_populate_pci_child_dt(PCIDevice *dev, 
void *fdt, int offset,
   RESOURCE_CELLS_ADDRESS));
 _FDT(fdt_setprop_cell(fdt, offset, "#size-cells",
   RESOURCE_CELLS_SIZE));
-_FDT(fdt_setprop_cell(fdt, offset, "ibm,req#msi-x",
-  RESOURCE_CELLS_SIZE));
+
+max_msi = msi_nr_vectors_allocated(dev);
+if (max_msi) {
+_FDT(fdt_setprop_cell(fdt, offset, "ibm,req#msi", max_msi));
+}
+max_msix = dev->msix_entries_nr;
+if (max_msix) {
+_FDT(fdt_setprop_cell(fdt, offset, "ibm,req#msi-x", max_msix));
+}
 
 populate_resource_props(dev, );
 _FDT(fdt_setprop(fdt, offset, "reg", (uint8_t *)rp.reg, rp.reg_len));
-- 
2.4.3

[Qemu-devel] [PULL 35/36] sPAPR: Revert don't enable EEH on emulated PCI devices

[David Gibson]

From: Gavin Shan 

This reverts commit 7cb18007 ("sPAPR: Don't enable EEH on emulated
PCI devices") as rtas_ibm_set_eeh_option() isn't the right place
to check if there has the corresponding PCI device for the input
address, which can be PE address, not PCI device address.

Signed-off-by: Gavin Shan 
Signed-off-by: David Gibson 
---
 hw/ppc/spapr_pci.c | 7 ---
 1 file changed, 7 deletions(-)

diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c
index b088491..617b7f3 100644
--- a/hw/ppc/spapr_pci.c
+++ b/hw/ppc/spapr_pci.c
@@ -431,7 +431,6 @@ static void rtas_ibm_set_eeh_option(PowerPCCPU *cpu,
 {
 sPAPRPHBState *sphb;
 sPAPRPHBClass *spc;
-PCIDevice *pdev;
 uint32_t addr, option;
 uint64_t buid;
 int ret;
@@ -449,12 +448,6 @@ static void rtas_ibm_set_eeh_option(PowerPCCPU *cpu,
 goto param_error_exit;
 }
 
-pdev = pci_find_device(PCI_HOST_BRIDGE(sphb)->bus,
-   (addr >> 16) & 0xFF, (addr >> 8) & 0xFF);
-if (!pdev || !object_dynamic_cast(OBJECT(pdev), "vfio-pci")) {
-goto param_error_exit;
-}
-
 spc = SPAPR_PCI_HOST_BRIDGE_GET_CLASS(sphb);
 if (!spc->eeh_set_option) {
 goto param_error_exit;
-- 
2.4.3

Re: [Qemu-devel] [PATCH v10 7/7] vhost-user: add a new message to disable/enable a specific virt queue.

[Eric Blake]

On 09/22/2015 08:05 PM, Yuanhan Liu wrote:

>>> + * VHOST_USER_SET_VRING_ENABLE
>>> +
>>> +  Id: 18
>>> +  Equivalent ioctl: N/A
>>> +  Master payload: vring state description
>>> +
>>> +  Signal slave to enable or disable corresponding vring.
>>
>> Does there need to be any QMP control to manually change a given queue,
>> or is it all used under the hood with no need for management apps to
>> care other than their initial request of max queues?
> 
> TBH, I don't know. As far as I know, there is only one queue pair will
> be enabled by default, and it's user's job to enable (or disable) more
> queue pairs, say, by ethtool:
> 
> # ethtool -L eth0 combined 
> 
> Which ends up sending the VHOST_USER_SET_VRING_ENABLE to actually enable
> (or disable) a specific queue pairs.

So if I'm understanding, it is ethtool, not qemu, that is used to turn
on additional queues, and therefore we don't need a QMP command in qemu
to control things.

> 
> Does that answer your question?

I think so, but I'll let other reviewers more familiar with this area of
code give a final say.

-- 
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 v10 7/7] vhost-user: add a new message to disable/enable a specific virt queue.

[Yuanhan Liu]

On Tue, Sep 22, 2015 at 08:47:04AM -0600, Eric Blake wrote:
> On 09/18/2015 08:58 AM, Yuanhan Liu wrote:
> > From: Changchun Ouyang 
> > 
> > Add a new message, VHOST_USER_SET_VRING_ENABLE, to enable or disable
> > a specific virt queue, which is similar to attach/detach queue for
> > tap device.
> > 
> > virtio driver on guest doesn't have to use max virt queue pair, it
> > could enable any number of virt queue ranging from 1 to max virt
> > queue pair.
> > 
> > Signed-off-by: Changchun Ouyang 
> > Signed-off-by: Yuanhan Liu 
> > ---
> >  docs/specs/vhost-user.txt | 12 +++-
> >  hw/net/vhost_net.c| 18 ++
> >  hw/net/virtio-net.c   |  8 
> >  hw/virtio/vhost-user.c| 19 +++
> >  include/hw/virtio/vhost-backend.h |  2 ++
> >  include/net/vhost_net.h   |  2 ++
> >  6 files changed, 60 insertions(+), 1 deletion(-)
> > 
> > diff --git a/docs/specs/vhost-user.txt b/docs/specs/vhost-user.txt
> > index cfc9d41..a5f1c31 100644
> > --- a/docs/specs/vhost-user.txt
> > +++ b/docs/specs/vhost-user.txt
> > @@ -148,7 +148,9 @@ VHOST_USER_GET_PROTOCOL_FEATURES. Master should stop 
> > when the number of
> >  requested queues is bigger than that.
> >  
> >  As all queues share one connection, the master uses a unique index for each
> > -queue in the sent message to identify a specified queue.
> > +queue in the sent message to identify a specified queue. One queue pairs
> 
> s/pairs/pair/

Thanks.

> 
> > +is enabled initially. More queues are enabled dynamically, by sending
> > +message VHOST_USER_SET_VRING_ENABLE.
> >  
> >  Message types
> >  -
> > @@ -327,3 +329,11 @@ Message types
> >Query how many queues the backend supports. This request should be
> >sent only when VHOST_USER_PROTOCOL_F_MQ is set in quried protocol
> >features by VHOST_USER_GET_PROTOCOL_FEATURES.
> > +
> > + * VHOST_USER_SET_VRING_ENABLE
> > +
> > +  Id: 18
> > +  Equivalent ioctl: N/A
> > +  Master payload: vring state description
> > +
> > +  Signal slave to enable or disable corresponding vring.
> 
> Does there need to be any QMP control to manually change a given queue,
> or is it all used under the hood with no need for management apps to
> care other than their initial request of max queues?

TBH, I don't know. As far as I know, there is only one queue pair will
be enabled by default, and it's user's job to enable (or disable) more
queue pairs, say, by ethtool:

# ethtool -L eth0 combined 

Which ends up sending the VHOST_USER_SET_VRING_ENABLE to actually enable
(or disable) a specific queue pairs.

Does that answer your question?

--yliu

[Qemu-devel] [PULL 30/36] spapr: Support hotplug by specifying DRC count

[David Gibson]

From: Bharata B Rao 

Support hotplug identifier type RTAS_LOG_V6_HP_ID_DRC_COUNT that allows
hotplugging of DRCs by specifying the DRC count.

While we are here, rename

spapr_hotplug_req_add_event() to spapr_hotplug_req_add_by_index()
spapr_hotplug_req_remove_event() to spapr_hotplug_req_remove_by_index()

so that they match with spapr_hotplug_req_add_by_count().

Signed-off-by: Bharata B Rao 
Reviewed-by: David Gibson 
Signed-off-by: David Gibson 
---
 hw/ppc/spapr.c |  2 +-
 hw/ppc/spapr_events.c  | 47 ++-
 hw/ppc/spapr_pci.c |  4 ++--
 include/hw/ppc/spapr.h |  8 ++--
 4 files changed, 47 insertions(+), 14 deletions(-)

diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index 402c00d..d00513f 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -2062,7 +2062,7 @@ static void spapr_add_lmbs(DeviceState *dev, uint64_t 
addr, uint64_t size,
 
 drck = SPAPR_DR_CONNECTOR_GET_CLASS(drc);
 drck->attach(drc, dev, fdt, fdt_offset, !dev->hotplugged, errp);
-spapr_hotplug_req_add_event(drc);
+spapr_hotplug_req_add_by_index(drc);
 addr += SPAPR_MEMORY_BLOCK_SIZE;
 }
 }
diff --git a/hw/ppc/spapr_events.c b/hw/ppc/spapr_events.c
index 98bf7ae..744ea62 100644
--- a/hw/ppc/spapr_events.c
+++ b/hw/ppc/spapr_events.c
@@ -386,7 +386,9 @@ static void spapr_powerdown_req(Notifier *n, void *opaque)
 qemu_irq_pulse(xics_get_qirq(spapr->icp, spapr->check_exception_irq));
 }
 
-static void spapr_hotplug_req_event(sPAPRDRConnector *drc, uint8_t hp_action)
+static void spapr_hotplug_req_event(uint8_t hp_id, uint8_t hp_action,
+sPAPRDRConnectorType drc_type,
+uint32_t drc)
 {
 sPAPRMachineState *spapr = SPAPR_MACHINE(qdev_get_machine());
 struct hp_log_full *new_hp;
@@ -395,8 +397,6 @@ static void spapr_hotplug_req_event(sPAPRDRConnector *drc, 
uint8_t hp_action)
 struct rtas_event_log_v6_maina *maina;
 struct rtas_event_log_v6_mainb *mainb;
 struct rtas_event_log_v6_hp *hp;
-sPAPRDRConnectorClass *drck = SPAPR_DR_CONNECTOR_GET_CLASS(drc);
-sPAPRDRConnectorType drc_type = drck->get_type(drc);
 
 new_hp = g_malloc0(sizeof(struct hp_log_full));
 hdr = _hp->hdr;
@@ -427,8 +427,7 @@ static void spapr_hotplug_req_event(sPAPRDRConnector *drc, 
uint8_t hp_action)
 hp->hdr.section_length = cpu_to_be16(sizeof(*hp));
 hp->hdr.section_version = 1; /* includes extended modifier */
 hp->hotplug_action = hp_action;
-hp->drc.index = cpu_to_be32(drck->get_index(drc));
-hp->hotplug_identifier = RTAS_LOG_V6_HP_ID_DRC_INDEX;
+hp->hotplug_identifier = hp_id;
 
 switch (drc_type) {
 case SPAPR_DR_CONNECTOR_TYPE_PCI:
@@ -445,19 +444,49 @@ static void spapr_hotplug_req_event(sPAPRDRConnector 
*drc, uint8_t hp_action)
 return;
 }
 
+if (hp_id == RTAS_LOG_V6_HP_ID_DRC_COUNT) {
+hp->drc.count = cpu_to_be32(drc);
+} else if (hp_id == RTAS_LOG_V6_HP_ID_DRC_INDEX) {
+hp->drc.index = cpu_to_be32(drc);
+}
+
 rtas_event_log_queue(RTAS_LOG_TYPE_HOTPLUG, new_hp, true);
 
 qemu_irq_pulse(xics_get_qirq(spapr->icp, spapr->check_exception_irq));
 }
 
-void spapr_hotplug_req_add_event(sPAPRDRConnector *drc)
+void spapr_hotplug_req_add_by_index(sPAPRDRConnector *drc)
+{
+sPAPRDRConnectorClass *drck = SPAPR_DR_CONNECTOR_GET_CLASS(drc);
+sPAPRDRConnectorType drc_type = drck->get_type(drc);
+uint32 index = drck->get_index(drc);
+
+spapr_hotplug_req_event(RTAS_LOG_V6_HP_ID_DRC_INDEX,
+RTAS_LOG_V6_HP_ACTION_ADD, drc_type, index);
+}
+
+void spapr_hotplug_req_remove_by_index(sPAPRDRConnector *drc)
+{
+sPAPRDRConnectorClass *drck = SPAPR_DR_CONNECTOR_GET_CLASS(drc);
+sPAPRDRConnectorType drc_type = drck->get_type(drc);
+uint32 index = drck->get_index(drc);
+
+spapr_hotplug_req_event(RTAS_LOG_V6_HP_ID_DRC_INDEX,
+RTAS_LOG_V6_HP_ACTION_REMOVE, drc_type, index);
+}
+
+void spapr_hotplug_req_add_by_count(sPAPRDRConnectorType drc_type,
+   uint32_t count)
 {
-spapr_hotplug_req_event(drc, RTAS_LOG_V6_HP_ACTION_ADD);
+spapr_hotplug_req_event(RTAS_LOG_V6_HP_ID_DRC_COUNT,
+RTAS_LOG_V6_HP_ACTION_ADD, drc_type, count);
 }
 
-void spapr_hotplug_req_remove_event(sPAPRDRConnector *drc)
+void spapr_hotplug_req_remove_by_count(sPAPRDRConnectorType drc_type,
+  uint32_t count)
 {
-spapr_hotplug_req_event(drc, RTAS_LOG_V6_HP_ACTION_REMOVE);
+spapr_hotplug_req_event(RTAS_LOG_V6_HP_ID_DRC_COUNT,
+RTAS_LOG_V6_HP_ACTION_REMOVE, drc_type, count);
 }
 
 static void check_exception(PowerPCCPU *cpu, sPAPRMachineState *spapr,
diff --git a/hw/ppc/spapr_pci.c 

[Qemu-devel] [PULL 31/36] spapr: Move memory hotplug to RTAS_LOG_V6_HP_ID_DRC_COUNT type

[David Gibson]

From: Bharata B Rao 

Till now memory hotplug used RTAS_LOG_V6_HP_ID_DRC_INDEX hotplug type
which meant that we generated one hotplug type of EPOW event for every
256MB (SPAPR_MEMORY_BLOCK_SIZE). This quickly overruns the kernel
rtas log buffer thus resulting in loss of memory hotplug events. Switch
to RTAS_LOG_V6_HP_ID_DRC_COUNT hotplug type for memory so that we
generate only one event per hotplug request.

Signed-off-by: Bharata B Rao 
Reviewed-by: Michael Roth 
Reviewed-by: David Gibson 
Signed-off-by: David Gibson 
---
 hw/ppc/spapr.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index d00513f..533c7ad 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -2062,9 +2062,9 @@ static void spapr_add_lmbs(DeviceState *dev, uint64_t 
addr, uint64_t size,
 
 drck = SPAPR_DR_CONNECTOR_GET_CLASS(drc);
 drck->attach(drc, dev, fdt, fdt_offset, !dev->hotplugged, errp);
-spapr_hotplug_req_add_by_index(drc);
 addr += SPAPR_MEMORY_BLOCK_SIZE;
 }
+spapr_hotplug_req_add_by_count(SPAPR_DR_CONNECTOR_TYPE_LMB, nr_lmbs);
 }
 
 static void spapr_memory_plug(HotplugHandler *hotplug_dev, DeviceState *dev,
-- 
2.4.3

Re: [Qemu-devel] [PATCH v10 6/7] vhost-user: add multiple queue support

[Yuanhan Liu]

On Tue, Sep 22, 2015 at 06:14:14PM +0800, Jason Wang wrote:
> 
> 
[...]
> > -static void net_vhost_link_down(VhostUserState *s, bool link_down)
> > +static void net_vhost_link_down(int queues, NetClientState *ncs[],
> > +bool link_down)
> >  {
> > -s->nc.link_down = link_down;
> > +NetClientState *nc;
> > +int i;
> >  
> > -if (s->nc.peer) {
> > -s->nc.peer->link_down = link_down;
> > -}
> > +for (i = 0; i < queues; i++) {
> > +nc = ncs[i];
> >  
> > -if (s->nc.info->link_status_changed) {
> > -s->nc.info->link_status_changed(>nc);
> > -}
> > +nc->link_down = link_down;
> > +
> > +if (nc->peer) {
> > +nc->peer->link_down = link_down;
> > +}
> > +
> > +if (nc->info->link_status_changed) {
> > +nc->info->link_status_changed(nc);
> > +}
> >  
> > -if (s->nc.peer && s->nc.peer->info->link_status_changed) {
> > -s->nc.peer->info->link_status_changed(s->nc.peer);
> > +if (nc->peer && nc->peer->info->link_status_changed) {
> > +nc->peer->info->link_status_changed(nc->peer);
> > +}
> >  }
> >  }
> 
> The semantics is a little bit difference with qmp_set_link. What's the
> reason for this? If there's no specific reason, probably, we could just
> reuse qmp_set_link() (or part of) here?

I did this just because I refactored the vhost_user init code, making
the char dev handler to initiate all ncs. Hence I turned net_vhost_link_down()
to handle all ncs, just like what I did for vhost_user_start().

And TBH, I'm not aware of qmp_set_link() before, and after reading the
code, I do think we can simply invoke it instead. And thanks for the
info.

> 
> Other looks good to me.

Thanks for the review. May I add your reviewed-by if I fix above issue?

--yliu

[Qemu-devel] [PULL 14/36] pseries: Fix incorrect calculation of threads per socket for chip-id

[David Gibson]

The device tree presented to pseries machine type guests includes an
ibm,chip-id property which gives essentially the socket number of each
vcpu core (individual vcpu threads don't get a node in the device
tree).

To calculate this, it uses a vcpus_per_socket variable computed as
(smp_cpus / #sockets).  This is correct for the usual case where
smp_cpus == smp_threads * smp_cores * #sockets.

However, you can start QEMU with the number of cores and threads
mismatching the total number of vcpus (whether that _should_ be
permitted is a topic for another day).  It's a bit hard to say what
the "real" number of vcpus per socket here is, but for most purposes
(smp_threads * smp_cores) will more meaningfully match how QEMU
behaves with respect to socket boundaries.

Signed-off-by: David Gibson 
Reviewed-by: Alexey Kardashevskiy 
---
 hw/ppc/spapr.c | 6 ++
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index a69d7e4..ac0d2fe 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -630,9 +630,7 @@ static void spapr_populate_cpu_dt(CPUState *cs, void *fdt, 
int offset,
 uint32_t cpufreq = kvm_enabled() ? kvmppc_get_clockfreq() : 10;
 uint32_t page_sizes_prop[64];
 size_t page_sizes_prop_size;
-QemuOpts *opts = qemu_opts_find(qemu_find_opts("smp-opts"), NULL);
-unsigned sockets = opts ? qemu_opt_get_number(opts, "sockets", 0) : 0;
-uint32_t cpus_per_socket = sockets ? (smp_cpus / sockets) : 1;
+uint32_t vcpus_per_socket = smp_threads * smp_cores;
 uint32_t pft_size_prop[] = {0, cpu_to_be32(spapr->htab_shift)};
 
 _FDT((fdt_setprop_cell(fdt, offset, "reg", index)));
@@ -701,7 +699,7 @@ static void spapr_populate_cpu_dt(CPUState *cs, void *fdt, 
int offset,
 }
 
 _FDT((fdt_setprop_cell(fdt, offset, "ibm,chip-id",
-   cs->cpu_index / cpus_per_socket)));
+   cs->cpu_index / vcpus_per_socket)));
 
 _FDT((fdt_setprop(fdt, offset, "ibm,pft-size",
   pft_size_prop, sizeof(pft_size_prop;
-- 
2.4.3

[Qemu-devel] [PULL 04/36] spapr: Add /rtas/ibm, change-msix-capable

[David Gibson]

From: Sam Bobroff 

QEMU is MSI-X capable and makes it available via ibm,change-msi, so
we should indicate this by adding /rtas/ibm,change-msix-capable to the
device tree.

This is specificed by PAPR.

Signed-off-by: Sam Bobroff 
Reviewed-by: David Gibson 
Signed-off-by: David Gibson 
---
 hw/ppc/spapr.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index 09962c4..ed0abd8 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -432,6 +432,10 @@ static void *spapr_create_fdt_skel(hwaddr initrd_base,
 _FDT((fdt_property_cell(fdt, "rtas-event-scan-rate",
 RTAS_EVENT_SCAN_RATE)));
 
+if (msi_supported) {
+_FDT((fdt_property(fdt, "ibm,change-msix-capable", NULL, 0)));
+}
+
 /*
  * According to PAPR, rtas ibm,os-term does not guarantee a return
  * back to the guest cpu.
-- 
2.4.3

[Qemu-devel] [PULL 15/36] spapr: Enable in-kernel H_SET_MODE handling

[David Gibson]

From: Alexey Kardashevskiy 

For setting debug watchpoints, sPAPR guests use H_SET_MODE hypercall.
The existing QEMU H_SET_MODE handler does not support this but
the KVM handler in HV KVM does. However it is not enabled.

This enables the in-kernel H_SET_MODE handler which handles:
- Completed Instruction Address Breakpoint Register
- Watch point 0 registers.

The rest is still handled in QEMU.

Reported-by: Anton Blanchard 
Signed-off-by: Alexey Kardashevskiy 
Reviewed-by: David Gibson 
Signed-off-by: David Gibson 
---
 hw/ppc/spapr.c   | 1 +
 target-ppc/kvm.c | 5 +
 target-ppc/kvm_ppc.h | 5 +
 3 files changed, 11 insertions(+)

diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index ac0d2fe..d49f322 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -1546,6 +1546,7 @@ static void ppc_spapr_init(MachineState *machine)
 if (kvm_enabled()) {
 /* Enable H_LOGICAL_CI_* so SLOF can talk to in-kernel devices */
 kvmppc_enable_logical_ci_hcalls();
+kvmppc_enable_set_mode_hcall();
 }
 
 /* allocate RAM */
diff --git a/target-ppc/kvm.c b/target-ppc/kvm.c
index 110436d..9cf5308 100644
--- a/target-ppc/kvm.c
+++ b/target-ppc/kvm.c
@@ -1953,6 +1953,11 @@ void kvmppc_enable_logical_ci_hcalls(void)
 kvmppc_enable_hcall(kvm_state, H_LOGICAL_CI_STORE);
 }
 
+void kvmppc_enable_set_mode_hcall(void)
+{
+kvmppc_enable_hcall(kvm_state, H_SET_MODE);
+}
+
 void kvmppc_set_papr(PowerPCCPU *cpu)
 {
 CPUState *cs = CPU(cpu);
diff --git a/target-ppc/kvm_ppc.h b/target-ppc/kvm_ppc.h
index 5c1d334..f790d50 100644
--- a/target-ppc/kvm_ppc.h
+++ b/target-ppc/kvm_ppc.h
@@ -23,6 +23,7 @@ int kvmppc_get_hasidle(CPUPPCState *env);
 int kvmppc_get_hypercall(CPUPPCState *env, uint8_t *buf, int buf_len);
 int kvmppc_set_interrupt(PowerPCCPU *cpu, int irq, int level);
 void kvmppc_enable_logical_ci_hcalls(void);
+void kvmppc_enable_set_mode_hcall(void);
 void kvmppc_set_papr(PowerPCCPU *cpu);
 int kvmppc_set_compat(PowerPCCPU *cpu, uint32_t cpu_version);
 void kvmppc_set_mpic_proxy(PowerPCCPU *cpu, int mpic_proxy);
@@ -110,6 +111,10 @@ static inline void kvmppc_enable_logical_ci_hcalls(void)
 {
 }
 
+static inline void kvmppc_enable_set_mode_hcall(void)
+{
+}
+
 static inline void kvmppc_set_papr(PowerPCCPU *cpu)
 {
 }
-- 
2.4.3

[Qemu-devel] [PULL 25/36] spapr: Memory hotplug support

[David Gibson]

From: Bharata B Rao 

Make use of pc-dimm infrastructure to support memory hotplug
for PowerPC.

Signed-off-by: Bharata B Rao 
Reviewed-by: David Gibson 
Signed-off-by: David Gibson 
---
 hw/ppc/spapr.c| 118 ++
 hw/ppc/spapr_events.c |   8 ++--
 2 files changed, 123 insertions(+), 3 deletions(-)

diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index d21e95c..5b46229 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -34,6 +34,7 @@
 #include "sysemu/block-backend.h"
 #include "sysemu/cpus.h"
 #include "sysemu/kvm.h"
+#include "sysemu/device_tree.h"
 #include "kvm_ppc.h"
 #include "migration/migration.h"
 #include "mmu-hash64.h"
@@ -862,6 +863,7 @@ static void spapr_finalize_fdt(sPAPRMachineState *spapr,
hwaddr rtas_size)
 {
 MachineState *machine = MACHINE(qdev_get_machine());
+sPAPRMachineClass *smc = SPAPR_MACHINE_GET_CLASS(machine);
 const char *boot_device = machine->boot_order;
 int ret, i;
 size_t cb = 0;
@@ -945,6 +947,10 @@ static void spapr_finalize_fdt(sPAPRMachineState *spapr,
 spapr_populate_chosen_stdout(fdt, spapr->vio_bus);
 }
 
+if (smc->dr_lmb_enabled) {
+_FDT(spapr_drc_populate_dt(fdt, 0, NULL, SPAPR_DR_CONNECTOR_TYPE_LMB));
+}
+
 _FDT((fdt_pack(fdt)));
 
 if (fdt_totalsize(fdt) > FDT_MAX_SIZE) {
@@ -2055,12 +2061,120 @@ static void spapr_nmi(NMIState *n, int cpu_index, 
Error **errp)
 }
 }
 
+static void spapr_add_lmbs(DeviceState *dev, uint64_t addr, uint64_t size,
+   uint32_t node, Error **errp)
+{
+sPAPRDRConnector *drc;
+sPAPRDRConnectorClass *drck;
+uint32_t nr_lmbs = size/SPAPR_MEMORY_BLOCK_SIZE;
+int i, fdt_offset, fdt_size;
+void *fdt;
+
+/*
+ * Check for DRC connectors and send hotplug notification to the
+ * guest only in case of hotplugged memory. This allows cold plugged
+ * memory to be specified at boot time.
+ */
+if (!dev->hotplugged) {
+return;
+}
+
+for (i = 0; i < nr_lmbs; i++) {
+drc = spapr_dr_connector_by_id(SPAPR_DR_CONNECTOR_TYPE_LMB,
+addr/SPAPR_MEMORY_BLOCK_SIZE);
+g_assert(drc);
+
+fdt = create_device_tree(_size);
+fdt_offset = spapr_populate_memory_node(fdt, node, addr,
+SPAPR_MEMORY_BLOCK_SIZE);
+
+drck = SPAPR_DR_CONNECTOR_GET_CLASS(drc);
+drck->attach(drc, dev, fdt, fdt_offset, !dev->hotplugged, errp);
+spapr_hotplug_req_add_event(drc);
+addr += SPAPR_MEMORY_BLOCK_SIZE;
+}
+}
+
+static void spapr_memory_plug(HotplugHandler *hotplug_dev, DeviceState *dev,
+  uint32_t node, Error **errp)
+{
+Error *local_err = NULL;
+sPAPRMachineState *ms = SPAPR_MACHINE(hotplug_dev);
+PCDIMMDevice *dimm = PC_DIMM(dev);
+PCDIMMDeviceClass *ddc = PC_DIMM_GET_CLASS(dimm);
+MemoryRegion *mr = ddc->get_memory_region(dimm);
+uint64_t align = memory_region_get_alignment(mr);
+uint64_t size = memory_region_size(mr);
+uint64_t addr;
+
+if (size % SPAPR_MEMORY_BLOCK_SIZE) {
+error_setg(_err, "Hotplugged memory size must be a multiple of "
+  "%lld MB", SPAPR_MEMORY_BLOCK_SIZE/M_BYTE);
+goto out;
+}
+
+pc_dimm_memory_plug(dev, >hotplug_memory, mr, align, _err);
+if (local_err) {
+goto out;
+}
+
+addr = object_property_get_int(OBJECT(dimm), PC_DIMM_ADDR_PROP, 
_err);
+if (local_err) {
+pc_dimm_memory_unplug(dev, >hotplug_memory, mr);
+goto out;
+}
+
+spapr_add_lmbs(dev, addr, size, node, _abort);
+
+out:
+error_propagate(errp, local_err);
+}
+
+static void spapr_machine_device_plug(HotplugHandler *hotplug_dev,
+  DeviceState *dev, Error **errp)
+{
+sPAPRMachineClass *smc = SPAPR_MACHINE_GET_CLASS(qdev_get_machine());
+
+if (object_dynamic_cast(OBJECT(dev), TYPE_PC_DIMM)) {
+uint32_t node;
+
+if (!smc->dr_lmb_enabled) {
+error_setg(errp, "Memory hotplug not supported for this machine");
+return;
+}
+node = object_property_get_int(OBJECT(dev), PC_DIMM_NODE_PROP, errp);
+if (*errp) {
+return;
+}
+
+spapr_memory_plug(hotplug_dev, dev, node, errp);
+}
+}
+
+static void spapr_machine_device_unplug(HotplugHandler *hotplug_dev,
+  DeviceState *dev, Error **errp)
+{
+if (object_dynamic_cast(OBJECT(dev), TYPE_PC_DIMM)) {
+error_setg(errp, "Memory hot unplug not supported by sPAPR");
+}
+}
+
+static HotplugHandler *spapr_get_hotpug_handler(MachineState *machine,
+ DeviceState *dev)
+{
+if (object_dynamic_cast(OBJECT(dev), TYPE_PC_DIMM)) 

[Qemu-devel] [PULL 17/36] spapr_drc: don't allow 'empty' DRCs to be unisolated or allocated

[David Gibson]

From: Michael Roth 

Logical resources start with allocation-state:UNUSABLE /
isolation-state:ISOLATED. During hotplug, guests will transition
them to allocation-state:USABLE, and then to
isolation-state:UNISOLATED.

For cases where we cannot transition to allocation-state:USABLE,
in this case due to no device/resource being association with
the logical DRC, we should return an error -3.

For physical DRCs, we default to allocation-state:USABLE and stay
there, so in this case we should report an error -3 when the guest
attempts to make the isolation-state:ISOLATED transition for a DRC
with no device associated.

These are as documented in PAPR 2.7, 13.5.3.4.

We also ensure allocation-state:USABLE when the guest attempts
transition to isolation-state:UNISOLATED to deal with misbehaving
guests attempting to bring online an unallocated logical resource.

This is as documented in PAPR 2.7, 13.7.

Currently we implement no such error logic. Fix this by handling
these error cases as PAPR defines.

Cc: Bharata B Rao 
Signed-off-by: Michael Roth 
Reviewed-by: David Gibson 
Signed-off-by: David Gibson 
---
 hw/ppc/spapr_drc.c | 21 +
 include/hw/ppc/spapr.h |  1 +
 2 files changed, 22 insertions(+)

diff --git a/hw/ppc/spapr_drc.c b/hw/ppc/spapr_drc.c
index 9ce844a..faf8760 100644
--- a/hw/ppc/spapr_drc.c
+++ b/hw/ppc/spapr_drc.c
@@ -66,6 +66,16 @@ static int set_isolation_state(sPAPRDRConnector *drc,
 
 DPRINTFN("drc: %x, set_isolation_state: %x", get_index(drc), state);
 
+if (state == SPAPR_DR_ISOLATION_STATE_UNISOLATED) {
+/* cannot unisolate a non-existant resource, and, or resources
+ * which are in an 'UNUSABLE' allocation state. (PAPR 2.7, 13.5.3.5)
+ */
+if (!drc->dev ||
+drc->allocation_state == SPAPR_DR_ALLOCATION_STATE_UNUSABLE) {
+return RTAS_OUT_NO_SUCH_INDICATOR;
+}
+}
+
 drc->isolation_state = state;
 
 if (drc->isolation_state == SPAPR_DR_ISOLATION_STATE_ISOLATED) {
@@ -107,6 +117,17 @@ static int set_allocation_state(sPAPRDRConnector *drc,
 
 DPRINTFN("drc: %x, set_allocation_state: %x", get_index(drc), state);
 
+if (state == SPAPR_DR_ALLOCATION_STATE_USABLE) {
+/* if there's no resource/device associated with the DRC, there's
+ * no way for us to put it in an allocation state consistent with
+ * being 'USABLE'. PAPR 2.7, 13.5.3.4 documents that this should
+ * result in an RTAS return code of -3 / "no such indicator"
+ */
+if (!drc->dev) {
+return RTAS_OUT_NO_SUCH_INDICATOR;
+}
+}
+
 if (drc->type != SPAPR_DR_CONNECTOR_TYPE_PCI) {
 drc->allocation_state = state;
 if (drc->awaiting_release &&
diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h
index cbe3463..8a0a74d 100644
--- a/include/hw/ppc/spapr.h
+++ b/include/hw/ppc/spapr.h
@@ -409,6 +409,7 @@ int spapr_allocate_irq_block(int num, bool lsi, bool msi);
 #define RTAS_OUT_BUSY   -2
 #define RTAS_OUT_PARAM_ERROR-3
 #define RTAS_OUT_NOT_SUPPORTED  -3
+#define RTAS_OUT_NO_SUCH_INDICATOR  -3
 #define RTAS_OUT_NOT_AUTHORIZED -9002
 
 /* RTAS tokens */
-- 
2.4.3

[Qemu-devel] [PULL 27/36] spapr: Provide better error message when slots exceed max allowed

[David Gibson]

From: Bharata B Rao 

Currently when user specifies more slots than allowed max of
SPAPR_MAX_RAM_SLOTS (32), we error out like this:

qemu-system-ppc64: unsupported amount of memory slots: 64

Let the user know about the max allowed slots like this:

qemu-system-ppc64: Specified number of memory slots 64 exceeds max supported 32

Signed-off-by: Bharata B Rao 
Reviewed-by: David Gibson 
Signed-off-by: David Gibson 
---
 hw/ppc/spapr.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index 1170028..5c81e19 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -1774,8 +1774,8 @@ static void ppc_spapr_init(MachineState *machine)
 ram_addr_t hotplug_mem_size = machine->maxram_size - machine->ram_size;
 
 if (machine->ram_slots > SPAPR_MAX_RAM_SLOTS) {
-error_report("unsupported amount of memory slots: %"PRIu64,
-  machine->ram_slots);
+error_report("Specified number of memory slots %"PRIu64" exceeds 
max supported %d\n",
+ machine->ram_slots, SPAPR_MAX_RAM_SLOTS);
 exit(EXIT_FAILURE);
 }
 
-- 
2.4.3

[Qemu-devel] [PULL 21/36] spapr: Use QEMU limit for maximum CPUs number

[David Gibson]

From: Alexey Kardashevskiy 

sPAPR uses hard coded limit of maximum 255 supported CPUs which is
exactly the same as QEMU-wide limit which is MAX_CPUMASK_BITS and also
defined as 255.

This makes use of a global CPU number limit for the "pseries" machine.

In order to anticipate future increase of the MAX_CPUMASK_BITS
(or to help debugging large systems), this also bumps the FDT_MAX_SIZE
limit from 256K to 1M assuming that 1 CPU core needs roughly 512 bytes
in the device tree so the new limit can cover up to 2048 CPU cores.

Signed-off-by: Alexey Kardashevskiy 
Reviewed-by: David Gibson 
Signed-off-by: David Gibson 
---
 hw/ppc/spapr.c | 6 ++
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index 2fb5e36..33e22ce 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -74,7 +74,7 @@
  *
  * We load our kernel at 4M, leaving space for SLOF initial image
  */
-#define FDT_MAX_SIZE0x4
+#define FDT_MAX_SIZE0x10
 #define RTAS_MAX_SIZE   0x1
 #define RTAS_MAX_ADDR   0x8000 /* RTAS must stay below that */
 #define FW_MAX_SIZE 0x40
@@ -86,8 +86,6 @@
 
 #define TIMEBASE_FREQ   51200ULL
 
-#define MAX_CPUS255
-
 #define PHANDLE_XICP0x
 
 #define HTAB_SIZE(spapr)(1ULL << ((spapr)->htab_shift))
@@ -1859,7 +1857,7 @@ static void spapr_machine_class_init(ObjectClass *oc, 
void *data)
 mc->init = ppc_spapr_init;
 mc->reset = ppc_spapr_reset;
 mc->block_default_type = IF_SCSI;
-mc->max_cpus = MAX_CPUS;
+mc->max_cpus = MAX_CPUMASK_BITS;
 mc->no_parallel = 1;
 mc->default_boot_order = "";
 mc->default_ram_size = 512 * M_BYTE;
-- 
2.4.3

[Qemu-devel] [PULL 34/36] ppc/spapr: Implement H_RANDOM hypercall in QEMU

[David Gibson]

From: Thomas Huth 

The PAPR interface defines a hypercall to pass high-quality
hardware generated random numbers to guests. Recent kernels can
already provide this hypercall to the guest if the right hardware
random number generator is available. But in case the user wants
to use another source like EGD, or QEMU is running with an older
kernel, we should also have this call in QEMU, so that guests that
do not support virtio-rng yet can get good random numbers, too.

This patch now adds a new pseudo-device to QEMU that either
directly provides this hypercall to the guest or is able to
enable the in-kernel hypercall if available. The in-kernel
hypercall can be enabled with the use-kvm property, e.g.:

 qemu-system-ppc64 -device spapr-rng,use-kvm=true

For handling the hypercall in QEMU instead, a "RngBackend" is
required since the hypercall should provide "good" random data
instead of pseudo-random (like from a "simple" library function
like rand() or g_random_int()). Since there are multiple RngBackends
available, the user must select an appropriate back-end via the
"rng" property of the device, e.g.:

 qemu-system-ppc64 -object rng-random,filename=/dev/hwrng,id=gid0 \
   -device spapr-rng,rng=gid0 ...

See http://wiki.qemu-project.org/Features-Done/VirtIORNG for
other example of specifying RngBackends.

Signed-off-by: Thomas Huth 
Signed-off-by: David Gibson 
---
 hw/ppc/Makefile.objs   |   2 +-
 hw/ppc/spapr.c |   8 +++
 hw/ppc/spapr_rng.c | 186 +
 include/hw/ppc/spapr.h |   4 ++
 target-ppc/kvm.c   |   9 +++
 target-ppc/kvm_ppc.h   |   5 ++
 6 files changed, 213 insertions(+), 1 deletion(-)
 create mode 100644 hw/ppc/spapr_rng.c

diff --git a/hw/ppc/Makefile.objs b/hw/ppc/Makefile.objs
index c8ab06e..c1ffc77 100644
--- a/hw/ppc/Makefile.objs
+++ b/hw/ppc/Makefile.objs
@@ -3,7 +3,7 @@ obj-y += ppc.o ppc_booke.o
 # IBM pSeries (sPAPR)
 obj-$(CONFIG_PSERIES) += spapr.o spapr_vio.o spapr_events.o
 obj-$(CONFIG_PSERIES) += spapr_hcall.o spapr_iommu.o spapr_rtas.o
-obj-$(CONFIG_PSERIES) += spapr_pci.o spapr_rtc.o spapr_drc.o
+obj-$(CONFIG_PSERIES) += spapr_pci.o spapr_rtc.o spapr_drc.o spapr_rng.o
 ifeq ($(CONFIG_PCI)$(CONFIG_PSERIES)$(CONFIG_LINUX), yyy)
 obj-y += spapr_pci_vfio.o
 endif
diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index 175..7f4f196 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -883,6 +883,14 @@ static void spapr_finalize_fdt(sPAPRMachineState *spapr,
 exit(1);
 }
 
+if (object_resolve_path_type("", TYPE_SPAPR_RNG, NULL)) {
+ret = spapr_rng_populate_dt(fdt);
+if (ret < 0) {
+fprintf(stderr, "could not set up rng device in the fdt\n");
+exit(1);
+}
+}
+
 QLIST_FOREACH(phb, >phbs, list) {
 ret = spapr_populate_pci_dt(phb, PHANDLE_XICP, fdt);
 }
diff --git a/hw/ppc/spapr_rng.c b/hw/ppc/spapr_rng.c
new file mode 100644
index 000..ed43d5e
--- /dev/null
+++ b/hw/ppc/spapr_rng.c
@@ -0,0 +1,186 @@
+/*
+ * QEMU sPAPR random number generator "device" for H_RANDOM hypercall
+ *
+ * Copyright 2015 Thomas Huth, Red Hat Inc.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License,
+ * or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, see .
+ */
+
+#include "qemu/error-report.h"
+#include "sysemu/sysemu.h"
+#include "sysemu/device_tree.h"
+#include "sysemu/rng.h"
+#include "hw/ppc/spapr.h"
+#include "kvm_ppc.h"
+
+#define SPAPR_RNG(obj) \
+OBJECT_CHECK(sPAPRRngState, (obj), TYPE_SPAPR_RNG)
+
+struct sPAPRRngState {
+/*< private >*/
+DeviceState ds;
+RngBackend *backend;
+bool use_kvm;
+};
+typedef struct sPAPRRngState sPAPRRngState;
+
+struct HRandomData {
+QemuSemaphore sem;
+union {
+uint64_t v64;
+uint8_t v8[8];
+} val;
+int received;
+};
+typedef struct HRandomData HRandomData;
+
+/* Callback function for the RngBackend */
+static void random_recv(void *dest, const void *src, size_t size)
+{
+HRandomData *hrdp = dest;
+
+if (src && size > 0) {
+assert(size + hrdp->received <= sizeof(hrdp->val.v8));
+memcpy(>val.v8[hrdp->received], src, size);
+hrdp->received += size;
+}
+
+qemu_sem_post(>sem);
+}
+
+/* Handler for the H_RANDOM hypercall */
+static target_ulong h_random(PowerPCCPU *cpu, sPAPRMachineState *spapr,
+   

[Qemu-devel] [PULL 36/36] sPAPR: Enable EEH on VFIO PCI device only

[David Gibson]

From: Gavin Shan 

This checks if the PCI device retrieved from the PCI device address
is VFIO PCI device when enabling EEH functionality. If it's not
VFIO PCI device, the EEH functonality isn't enabled.

Signed-off-by: Gavin Shan 
Signed-off-by: David Gibson 
---
 hw/ppc/spapr_pci_vfio.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/ppc/spapr_pci_vfio.c b/hw/ppc/spapr_pci_vfio.c
index cca45ed..a61b418 100644
--- a/hw/ppc/spapr_pci_vfio.c
+++ b/hw/ppc/spapr_pci_vfio.c
@@ -117,7 +117,7 @@ static int spapr_phb_vfio_eeh_set_option(sPAPRPHBState 
*sphb,
 phb = PCI_HOST_BRIDGE(sphb);
 pdev = pci_find_device(phb->bus,
(addr >> 16) & 0xFF, (addr >> 8) & 0xFF);
-if (!pdev) {
+if (!pdev || !object_dynamic_cast(OBJECT(pdev), "vfio-pci")) {
 return RTAS_OUT_PARAM_ERROR;
 }
 
-- 
2.4.3

Re: [Qemu-devel] [PATCH v10 6/7] vhost-user: add multiple queue support

[Jason Wang]

On 09/23/2015 09:57 AM, Yuanhan Liu wrote:
> On Tue, Sep 22, 2015 at 06:14:14PM +0800, Jason Wang wrote:
>>
> [...]
>>> -static void net_vhost_link_down(VhostUserState *s, bool link_down)
>>> +static void net_vhost_link_down(int queues, NetClientState *ncs[],
>>> +bool link_down)
>>>  {
>>> -s->nc.link_down = link_down;
>>> +NetClientState *nc;
>>> +int i;
>>>  
>>> -if (s->nc.peer) {
>>> -s->nc.peer->link_down = link_down;
>>> -}
>>> +for (i = 0; i < queues; i++) {
>>> +nc = ncs[i];
>>>  
>>> -if (s->nc.info->link_status_changed) {
>>> -s->nc.info->link_status_changed(>nc);
>>> -}
>>> +nc->link_down = link_down;
>>> +
>>> +if (nc->peer) {
>>> +nc->peer->link_down = link_down;
>>> +}
>>> +
>>> +if (nc->info->link_status_changed) {
>>> +nc->info->link_status_changed(nc);
>>> +}
>>>  
>>> -if (s->nc.peer && s->nc.peer->info->link_status_changed) {
>>> -s->nc.peer->info->link_status_changed(s->nc.peer);
>>> +if (nc->peer && nc->peer->info->link_status_changed) {
>>> +nc->peer->info->link_status_changed(nc->peer);
>>> +}
>>>  }
>>>  }
>> The semantics is a little bit difference with qmp_set_link. What's the
>> reason for this? If there's no specific reason, probably, we could just
>> reuse qmp_set_link() (or part of) here?
> I did this just because I refactored the vhost_user init code, making
> the char dev handler to initiate all ncs. Hence I turned net_vhost_link_down()
> to handle all ncs, just like what I did for vhost_user_start().
>
> And TBH, I'm not aware of qmp_set_link() before, and after reading the
> code, I do think we can simply invoke it instead. And thanks for the
> info.
>
>> Other looks good to me.
> Thanks for the review. May I add your reviewed-by if I fix above issue?
>
>   --yliu

I prefer to add it myself :)

Thanks.

[Qemu-devel] [PULL 32/36] spapr: Fix default NUMA node allocation for threads

[David Gibson]

At present, if guest numa nodes are requested, but the cpus in each node
are not specified, spapr just uses the default behaviour or assigning each
vcpu round-robin to nodes.

If smp_threads != 1, that will assign adjacent threads in a core to
different NUMA nodes.  As well as being just weird, that's a configuration
that can't be represented in the device tree we give to the guest, which
means the guest and qemu end up with different ideas of the NUMA topology.

This patch implements mc->cpu_index_to_socket_id in the spapr code to
make sure vcpus get assigned to nodes only at the socket granularity.

Signed-off-by: David Gibson 
Reviewed-by: Alexey Kardashevskiy 
---
 hw/ppc/spapr.c | 8 
 1 file changed, 8 insertions(+)

diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index 533c7ad..59fc814 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -2162,6 +2162,13 @@ static HotplugHandler 
*spapr_get_hotpug_handler(MachineState *machine,
 return NULL;
 }
 
+static unsigned spapr_cpu_index_to_socket_id(unsigned cpu_index)
+{
+/* Allocate to NUMA nodes on a "socket" basis (not that concept of
+ * socket means much for the paravirtualized PAPR platform) */
+return cpu_index / smp_threads / smp_cores;
+}
+
 static void spapr_machine_class_init(ObjectClass *oc, void *data)
 {
 MachineClass *mc = MACHINE_CLASS(oc);
@@ -2183,6 +2190,7 @@ static void spapr_machine_class_init(ObjectClass *oc, 
void *data)
 mc->get_hotplug_handler = spapr_get_hotpug_handler;
 hc->plug = spapr_machine_device_plug;
 hc->unplug = spapr_machine_device_unplug;
+mc->cpu_index_to_socket_id = spapr_cpu_index_to_socket_id;
 
 smc->dr_lmb_enabled = false;
 fwc->get_dev_path = spapr_get_fw_dev_path;
-- 
2.4.3

[Qemu-devel] [PULL 33/36] ppc/spapr: Fix buffer overflow in spapr_populate_drconf_memory()

[David Gibson]

From: Thomas Huth 

The buffer that is allocated in spapr_populate_drconf_memory()
is used for setting both, the "ibm,dynamic-memory" and the
"ibm,associativity-lookup-arrays" property. However, only the
size of the first one is taken into account when allocating the
memory. So if the length of the second property is larger than
the length of the first one, we run into a buffer overflow here!
Fix it by taking the length of the second property into account,
too.

Fixes: "spapr: Support ibm,dynamic-reconfiguration-memory" patch
Signed-off-by: Thomas Huth 
Reviewed-by: David Gibson 
Signed-off-by: David Gibson 
---
 hw/ppc/spapr.c | 9 ++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index 59fc814..175 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -725,9 +725,12 @@ static int spapr_populate_drconf_memory(sPAPRMachineState 
*spapr, void *fdt)
 uint32_t *int_buf, *cur_index, buf_len;
 int nr_nodes = nb_numa_nodes ? nb_numa_nodes : 1;
 
-/* Allocate enough buffer size to fit in ibm,dynamic-memory */
-buf_len = nr_lmbs * SPAPR_DR_LMB_LIST_ENTRY_SIZE * sizeof(uint32_t) +
-sizeof(uint32_t);
+/*
+ * Allocate enough buffer size to fit in ibm,dynamic-memory
+ * or ibm,associativity-lookup-arrays
+ */
+buf_len = MAX(nr_lmbs * SPAPR_DR_LMB_LIST_ENTRY_SIZE + 1, nr_nodes * 4 + 2)
+  * sizeof(uint32_t);
 cur_index = int_buf = g_malloc0(buf_len);
 
 offset = fdt_add_subnode(fdt, 0, "ibm,dynamic-reconfiguration-memory");
-- 
2.4.3

Re: [Qemu-devel] [PATCH v10 7/7] vhost-user: add a new message to disable/enable a specific virt queue.

[Yuanhan Liu]

On Tue, Sep 22, 2015 at 08:06:58PM -0600, Eric Blake wrote:
> On 09/22/2015 08:05 PM, Yuanhan Liu wrote:
> 
> >>> + * VHOST_USER_SET_VRING_ENABLE
> >>> +
> >>> +  Id: 18
> >>> +  Equivalent ioctl: N/A
> >>> +  Master payload: vring state description
> >>> +
> >>> +  Signal slave to enable or disable corresponding vring.
> >>
> >> Does there need to be any QMP control to manually change a given queue,
> >> or is it all used under the hood with no need for management apps to
> >> care other than their initial request of max queues?
> > 
> > TBH, I don't know. As far as I know, there is only one queue pair will
> > be enabled by default, and it's user's job to enable (or disable) more
> > queue pairs, say, by ethtool:
> > 
> > # ethtool -L eth0 combined 
> > 
> > Which ends up sending the VHOST_USER_SET_VRING_ENABLE to actually enable
> > (or disable) a specific queue pairs.
> 
> So if I'm understanding, it is ethtool, not qemu, that is used to turn
> on additional queues, and therefore we don't need a QMP command in qemu
> to control things.

I guess so, and that's what Michael told me before.

> > Does that answer your question?
> 
> I think so, but I'll let other reviewers more familiar with this area of
> code give a final say.

Michael?

--yliu