[Qemu-devel] [PATCH v1 3/4] qemu-iotests: s390x: fix test 068

[Bo Tu]

Now, s390-virtio-ccw is default machine and s390-ccw.img is default boot
loader. If the s390-virtio-ccw machine finds no device to load from and
errors out, then emits a panic and exits the vm. This breaks test cases
068 for s390x.
Adding the parameter of "-no-shutdown" for s390-ccw-virtio will pause VM
before shutdown.

Reviewed-by: Sascha Silbe 
Signed-off-by: Bo Tu 
---
 tests/qemu-iotests/068 | 14 --
 1 file changed, 12 insertions(+), 2 deletions(-)

diff --git a/tests/qemu-iotests/068 b/tests/qemu-iotests/068
index b72e555..0d58482 100755
--- a/tests/qemu-iotests/068
+++ b/tests/qemu-iotests/068
@@ -50,13 +50,23 @@ echo
 echo "=== Saving and reloading a VM state to/from a qcow2 image ==="
 echo
 _make_test_img $IMG_SIZE
+
+case "$QEMU_DEFAULT_MACHINE" in
+  s390-ccw-virtio)
+  platform_parm="-no-shutdown -machine accel=kvm"
+  ;;
+  *)
+  platform_parm=""
+  ;;
+esac
+
 # Give qemu some time to boot before saving the VM state
 bash -c 'sleep 1; echo -e "savevm 0\nquit"' |\
-$QEMU -nographic -monitor stdio -serial none -hda "$TEST_IMG" |\
+$QEMU $platform_parm -nographic -monitor stdio -serial none -hda 
"$TEST_IMG" |\
 _filter_qemu
 # Now try to continue from that VM state (this should just work)
 echo quit |\
-$QEMU -nographic -monitor stdio -serial none -hda "$TEST_IMG" -loadvm 0 |\
+$QEMU $platform_parm -nographic -monitor stdio -serial none -hda 
"$TEST_IMG" -loadvm 0 |\
 _filter_qemu
 
 # success, all done
-- 
2.3.0

[Qemu-devel] [PATCH v1 4/4] qemu-iotests: disable VNC server for test 120

[Bo Tu]

Ever since qemu-iotest 120 was introduced, its expected output didn't
include the output from the built-in VNC server:

 QA output created by 120
 Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864
 QMP_VERSION
+VNC server running on `::1:5900'
 {"return": {}}
 wrote 65536/65536 bytes at offset 0
 64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)

Since some architectures do not even have a graphical console, we just
pass -nographic to avoid the output.

Fixes: a68197ff5b11 ("iotests: Add tests for overriding
BDRV_O_PROTOCOL")

Reviewed-by: Sascha Silbe 
Signed-off-by: Bo Tu 
---
 tests/qemu-iotests/120 | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/qemu-iotests/120 b/tests/qemu-iotests/120
index 9f13078..fb69efd 100755
--- a/tests/qemu-iotests/120
+++ b/tests/qemu-iotests/120
@@ -49,7 +49,7 @@ echo "{'execute': 'qmp_capabilities'}
   {'execute': 'human-monitor-command',
'arguments': {'command-line': 'qemu-io drv \"write -P 42 0 64k\"'}}
   {'execute': 'quit'}" \
-| $QEMU -qmp stdio -nodefaults \
+| $QEMU -qmp stdio -nodefaults -nographic \
 -drive 
id=drv,if=none,file="$TEST_IMG",driver=raw,file.driver=$IMGFMT \
 | _filter_qmp | _filter_qemu_io
 $QEMU_IO -c 'read -P 42 0 64k' "$TEST_IMG" | _filter_qemu_io
-- 
2.3.0

[Qemu-devel] [PATCH v1 2/4] qemu-iotests: s390x: fix test 051

[Bo Tu]

The tests for device type "ide_cd" should only be tested for the pc
platform.
The default device id of hard disk on the s390 platform differs to that
of the x86 platform. A new variable device_id is defined and "virtio0"
set for the s390 platform. A x86 platform specific output file is also
needed.
Warning message expected for s390x when drive without device.

Reviewed-by: Sascha Silbe 
Signed-off-by: Bo Tu 
---
 tests/qemu-iotests/051|  99 ++
 tests/qemu-iotests/051.out| 143 +++---
 tests/qemu-iotests/051.pc.out | 422 ++
 3 files changed, 515 insertions(+), 149 deletions(-)
 create mode 100644 tests/qemu-iotests/051.pc.out

diff --git a/tests/qemu-iotests/051 b/tests/qemu-iotests/051
index 17dbf04..2bc2193 100755
--- a/tests/qemu-iotests/051
+++ b/tests/qemu-iotests/051
@@ -148,33 +148,49 @@ run_qemu -drive if=ide
 run_qemu -drive if=virtio
 run_qemu -drive if=scsi
 
-run_qemu -drive if=none,id=disk -device ide-cd,drive=disk
-run_qemu -drive if=none,id=disk -device lsi53c895a -device scsi-cd,drive=disk
-
-run_qemu -drive if=none,id=disk -device ide-drive,drive=disk
-run_qemu -drive if=none,id=disk -device ide-hd,drive=disk
-run_qemu -drive if=none,id=disk -device lsi53c895a -device scsi-disk,drive=disk
-run_qemu -drive if=none,id=disk -device lsi53c895a -device scsi-hd,drive=disk
+case "$QEMU_DEFAULT_MACHINE" in
+pc)
+run_qemu -drive if=none,id=disk -device ide-cd,drive=disk
+run_qemu -drive if=none,id=disk -device lsi53c895a -device 
scsi-cd,drive=disk
+run_qemu -drive if=none,id=disk -device ide-drive,drive=disk
+run_qemu -drive if=none,id=disk -device ide-hd,drive=disk
+run_qemu -drive if=none,id=disk -device lsi53c895a -device 
scsi-disk,drive=disk
+run_qemu -drive if=none,id=disk -device lsi53c895a -device 
scsi-hd,drive=disk
+;;
+ *)
+;;
+esac
 
 echo
 echo === Read-only ===
 echo
 
-run_qemu -drive file="$TEST_IMG",if=floppy,readonly=on
-run_qemu -drive file="$TEST_IMG",if=ide,media=cdrom,readonly=on
-run_qemu -drive file="$TEST_IMG",if=scsi,media=cdrom,readonly=on
+case "$QEMU_DEFAULT_MACHINE" in
+pc)
+run_qemu -drive file="$TEST_IMG",if=floppy,readonly=on
+run_qemu -drive file="$TEST_IMG",if=ide,media=cdrom,readonly=on
+run_qemu -drive file="$TEST_IMG",if=scsi,media=cdrom,readonly=on
+run_qemu -drive file="$TEST_IMG",if=ide,readonly=on
+;;
+ *)
+;;
+esac
 
-run_qemu -drive file="$TEST_IMG",if=ide,readonly=on
 run_qemu -drive file="$TEST_IMG",if=virtio,readonly=on
 run_qemu -drive file="$TEST_IMG",if=scsi,readonly=on
 
-run_qemu -drive file="$TEST_IMG",if=none,id=disk,readonly=on -device 
ide-cd,drive=disk
-run_qemu -drive file="$TEST_IMG",if=none,id=disk,readonly=on -device 
lsi53c895a -device scsi-cd,drive=disk
-
-run_qemu -drive file="$TEST_IMG",if=none,id=disk,readonly=on -device 
ide-drive,drive=disk
-run_qemu -drive file="$TEST_IMG",if=none,id=disk,readonly=on -device 
ide-hd,drive=disk
-run_qemu -drive file="$TEST_IMG",if=none,id=disk,readonly=on -device 
lsi53c895a -device scsi-disk,drive=disk
-run_qemu -drive file="$TEST_IMG",if=none,id=disk,readonly=on -device 
lsi53c895a -device scsi-hd,drive=disk
+case "$QEMU_DEFAULT_MACHINE" in
+pc)
+run_qemu -drive file="$TEST_IMG",if=none,id=disk,readonly=on -device 
ide-cd,drive=disk
+run_qemu -drive file="$TEST_IMG",if=none,id=disk,readonly=on -device 
lsi53c895a -device scsi-cd,drive=disk
+run_qemu -drive file="$TEST_IMG",if=none,id=disk,readonly=on -device 
ide-drive,drive=disk
+run_qemu -drive file="$TEST_IMG",if=none,id=disk,readonly=on -device 
ide-hd,drive=disk
+run_qemu -drive file="$TEST_IMG",if=none,id=disk,readonly=on -device 
lsi53c895a -device scsi-disk,drive=disk
+run_qemu -drive file="$TEST_IMG",if=none,id=disk,readonly=on -device 
lsi53c895a -device scsi-hd,drive=disk
+;;
+ *)
+;;
+esac
 
 echo
 echo === Cache modes ===
@@ -183,12 +199,18 @@ echo
 # Cannot use the test image because cache=none might not work on the host FS
 # Use cdrom so that we won't get errors about missing media
 
-run_qemu -drive media=cdrom,cache=none
-run_qemu -drive media=cdrom,cache=directsync
-run_qemu -drive media=cdrom,cache=writeback
-run_qemu -drive media=cdrom,cache=writethrough
-run_qemu -drive media=cdrom,cache=unsafe
-run_qemu -drive media=cdrom,cache=invalid_value
+case "$QEMU_DEFAULT_MACHINE" in
+pc)
+run_qemu -drive media=cdrom,cache=none
+run_qemu -drive media=cdrom,cache=directsync
+run_qemu -drive media=cdrom,cache=writeback
+run_qemu -drive media=cdrom,cache=writethrough
+run_qemu -drive media=cdrom,cache=unsafe
+run_qemu -drive media=cdrom,cache=invalid_value
+;;
+ *)
+;;
+esac
 
 echo
 echo === Specifying the protocol layer ===
@@ -251,28 +273,37 @@ echo
 echo === Snapshot mode ===
 echo
 
+case "$QEMU_DEF

[Qemu-devel] [PATCH v1 1/4] qemu-iotests: refine common.config

[Bo Tu]

Be easier to read, and be slightly shorter.

Suggested-By: Sascha Silbe 
Reviewed-by: Sascha Silbe 
Signed-off-by: Bo Tu 
---
 tests/qemu-iotests/common.config | 7 +++
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/tests/qemu-iotests/common.config b/tests/qemu-iotests/common.config
index 596bb2b..0a165e8 100644
--- a/tests/qemu-iotests/common.config
+++ b/tests/qemu-iotests/common.config
@@ -129,10 +129,9 @@ export QEMU_IO=_qemu_io_wrapper
 export QEMU_NBD=_qemu_nbd_wrapper
 
 default_machine=$($QEMU -machine \? | awk '/(default)/{print $1}')
-default_alias_machine=$($QEMU -machine \? |\
-awk -v var_default_machine="$default_machine"\)\
-'{if ($(NF-2)=="(alias"&&$(NF-1)=="of"&&$(NF)==var_default_machine){print 
$1}}')
-if [ ! -z "$default_alias_machine" ]; then
+default_alias_machine=$($QEMU -machine \? \
+| grep -F "(alias of ${default_machine})" |cut -d ' ' -f 1 |head -n 1)
+if [ -n "$default_alias_machine" ]; then
 default_machine="$default_alias_machine"
 fi
 
-- 
2.3.0

Re: [Qemu-devel] [PATCH] monitor: Plug memory leak on QMP error

[Markus Armbruster]

Luiz Capitulino  writes:

> On Thu, 29 Oct 2015 17:23:43 +0100
> Markus Armbruster  wrote:
>
>> Luiz Capitulino  writes:
>> 
>> > On Thu, 29 Oct 2015 12:15:09 +0100
>> > Markus Armbruster  wrote:
>> >
>> >> Leak introduced in commit 8a4f501..710aec9, v2.4.0.
>> >> 
>> >> Signed-off-by: Markus Armbruster 
>> >
>> > Reviewed-by: Luiz Capitulino 
>> 
>> Thanks!
>> 
>> > I think this can go through your tree?
>> 
>> Yes, along with the json-streamer work.
>
> Do you need my review for that one? It's been ages that I don't
> look at that code, I'm not sure it's going to be that worthwhile.

Review is always welcome, but "need" is too strong.  qobject/json* isn't
your code anyway, and I got Eric's review already.

Re: [Qemu-devel] [PATCH 4/4] json-streamer: Limit number of tokens in addition to total size

[Markus Armbruster]

Eric Blake  writes:

> On 10/29/2015 12:27 PM, Markus Armbruster wrote:
>>> Sounds like we have some quadratic (or worse) scaling in the parser.
>>> Worth fixing some day, but I also agree that we don't have to tackle it
>>> in this series.
>> 
>> I believe it's linear with a criminally negligent constant (several KiB
>> per token).  The first hog is actually fairly obvious: we use on QDict
>> per token.
>
> Wow.  I just read through the code, and you're right.  We are passing
> around QDicts right and left (one per token, with 4 keys, which is
> several mallocs per token), and then creating a QList of those tokens.

Disgusting, isn't it?

> Prior to commit 65c0f1e9, when we were really incrementing the refcount
> of each token on each level of nesting (as part of copying context, for
> definite quadratic behavior), the refcounts let us ensure tokens would
> be cleaned up at the end.  But I'm hard pressed to see the refcount of
> tokens going above one in the current code, which means we aren't
> gaining anything by using QDict for reference counting.  For that
> matter, JSON is quite linear - the code talks about needing to backtrack
> in different contexts, but JSON doesn't have ambiguities that need
> backtracking to try multiple different rules.  It seems like the code is
> overengineered because it is copied from another language where
> backtracking to try several alternate parses actually makes sense.

Yes, starting with a copy of an inappropriate solution is a possible
explanation for this mess.

> I suspect that using a C struct per token, and a C array of those
> structs, would go a long way towards alleviating memory abuse per token.

Yes.

> Are you tackling that, or would you like me to take a stab at it while
> you work on flushing my pending qapi patches?

I think I could use a bit of hacking to clear my mind between bouts of
patch review.  This job should be about the right size.  Plus, I better
become more familiar with qobject/json-* --- Luiz is dying to get rid of
it ;)

>>> I'm assuming you temporarily patched check-qjson to use larger constants
>>> when you hit your ~100K token testing?  Because I am definitely seeing a
>>> lot of execution time spent on large_dict when running tests/check-qjson
>>> by hand, in relation to all the other tests of that file, but not
>>> minutes worth.  Care to post the diff you played with?
>> 
>> I tested on a slow machine.
>
> I guess it was all the malloc pressure on a low-memory system that would
> make it so much slower than what I'm seeing, if you stuck with the
> default gen_test_json(gstr, 10, 100).

Something must have been wrong with this machine yesterday, because
today it runs the exact same test much, much faster.  Computers...

I'll update the commit message.  Thanks for your sanity check!

Re: [Qemu-devel] [PATCH v1 00/15] data-driven device registers

[Peter Maydell]

On 30 October 2015 at 06:52, Peter Crosthwaite
 wrote:
> Ping^3
>
> This has been on list for a very long time without 3rd party review.
> Can I send a PULL?

I would prefer not to take a new unreviewed feature
in softfreeze for 2.5...

thanks
-- PMM

[Qemu-devel] [PULL v4 00/14] QMP and QObject patches

[Markus Armbruster]

The following changes since commit 7bc8e0c967a4ef77657174d28af775691e18b4ce:

  Merge remote-tracking branch 'remotes/mst/tags/for_upstream' into staging 
(2015-10-29 09:49:52 +)

are available in the git repository at:

  git://repo.or.cz/qemu/armbru.git tags/pull-monitor-2015-10-30

for you to fetch changes up to 7f1e7b23d57408c86d350b3544673fdcd6be55c0:

  docs: Document QMP event rate limiting (2015-10-30 09:05:38 +0100)


QMP and QObject patches


Markus Armbruster (14):
  qobject: Drop QObject_HEAD
  qbool: Make conversion from QObject * accept null
  qdict: Make conversion from QObject * accept null
  qfloat qint: Make conversion from QObject * accept null
  qlist: Make conversion from QObject * accept null
  qstring: Make conversion from QObject * accept null
  monitor: Reduce casting of QAPI event QDict
  monitor: Simplify event throttling
  monitor: Switch from timer_new() to timer_new_ns()
  monitor: Split MonitorQAPIEventConf off MonitorQAPIEventState
  glib: add compatibility interface for g_hash_table_add()
  monitor: Turn monitor_qapi_event_state[] into a hash table
  monitor: Throttle event VSERPORT_CHANGE separately by "id"
  docs: Document QMP event rate limiting

 docs/qmp-events.txt|  12 +++
 docs/qmp-spec.txt  |   5 ++
 include/glib-compat.h  |   8 ++
 include/qapi/qmp/qbool.h   |   2 +-
 include/qapi/qmp/qdict.h   |   2 +-
 include/qapi/qmp/qfloat.h  |   2 +-
 include/qapi/qmp/qint.h|   2 +-
 include/qapi/qmp/qlist.h   |   2 +-
 include/qapi/qmp/qobject.h |   4 -
 include/qapi/qmp/qstring.h |   2 +-
 monitor.c  | 192 ++---
 qapi/qmp-input-visitor.c   |  40 +-
 qga/main.c |  11 +--
 qobject/qbool.c|   4 +-
 qobject/qdict.c|  39 +++--
 qobject/qfloat.c   |   4 +-
 qobject/qint.c |   4 +-
 qobject/qlist.c|   3 +-
 qobject/qstring.c  |   4 +-
 trace-events   |   4 +-
 20 files changed, 191 insertions(+), 155 deletions(-)

-- 
2.4.3

Re: [Qemu-devel] [PULL v3 00/14] QMP and QObject patches

[Markus Armbruster]

Peter Maydell  writes:

> On 29 October 2015 at 14:32, Markus Armbruster  wrote:
>> The following changes since commit 7bc8e0c967a4ef77657174d28af775691e18b4ce:
>>
>>   Merge remote-tracking branch 'remotes/mst/tags/for_upstream' into
>> staging (2015-10-29 09:49:52 +)
>>
>> are available in the git repository at:
>>
>>   git://repo.or.cz/qemu/armbru.git tags/pull-monitor-2015-10-29
>>
>> for you to fetch changes up to f6724b5b1b8fb1ca76b3df10d80415c206e2c4b9:
>>
>>   docs: Document QMP event rate limiting (2015-10-29 14:37:34 +0100)
>>
>> 
>> QMP and QObject patches
>>
>
> Still doesn't build with old glib:
>
> In file included from /Users/pm215/src/qemu-for-merges/qga/commands.c:14:
> In file included from
> /Users/pm215/src/qemu-for-merges/qga/guest-agent-core.h:14:
> In file included from 
> /Users/pm215/src/qemu-for-merges/include/qemu-common.h:25:
> /Users/pm215/src/qemu-for-merges/include/glib-compat.h:172:47: error:
> expected ';' after expression
> g_hash_table_replace(hash_table, key, key)
>   ^
>   ;
>
> I did a trial build with the typo fixed, and that is OK, so this
> should be the last fix required, I think.

I fixed up this line, pushed, and sent a PULL v4 cover letter.  Sorry
for the extra iterations.

Re: [Qemu-devel] [PATCH for-2.5] MAINTAINERS: Add new qemu-arm mailing list to ARM related entries

[Shannon Zhao]

On 2015/10/29 22:41, Peter Maydell wrote:
> We now have a qemu-arm mailing list for ARM patches and discussion,
> so add an L: entry for it to the various ARM related entries in
> MAINTAINERS.
> 
> Signed-off-by: Peter Maydell 

Reviewed-by: Shannon Zhao 

> ---
> I basically just added an L: entry for everything that was vaguely
> ARM-ish...it's a shame the MAINTAINERS format doesn't let you say
> "and cc this list for any of the sub-entries under this subsection".
> 
>  MAINTAINERS | 23 +++
>  1 file changed, 23 insertions(+)
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 3144113..fc8abe8 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -81,6 +81,7 @@ F: disas/alpha.c
>  
>  ARM
>  M: Peter Maydell 
> +L: qemu-...@nongnu.org
>  S: Maintained
>  F: target-arm/
>  F: hw/arm/
> @@ -216,6 +217,7 @@ F: */kvm.*
>  
>  ARM
>  M: Peter Maydell 
> +L: qemu-...@nongnu.org
>  S: Maintained
>  F: target-arm/kvm.c
>  
> @@ -287,6 +289,7 @@ ARM Machines
>  
>  Allwinner-a10
>  M: Beniamino Galvani 
> +L: qemu-...@nongnu.org
>  S: Maintained
>  F: hw/*/allwinner*
>  F: include/hw/*/allwinner*
> @@ -294,6 +297,7 @@ F: hw/arm/cubieboard.c
>  
>  ARM PrimeCell
>  M: Peter Maydell 
> +L: qemu-...@nongnu.org
>  S: Maintained
>  F: hw/char/pl011.c
>  F: hw/display/pl110*
> @@ -308,6 +312,7 @@ F: include/hw/arm/primecell.h
>  
>  ARM cores
>  M: Peter Maydell 
> +L: qemu-...@nongnu.org
>  S: Maintained
>  F: hw/intc/arm*
>  F: hw/intc/gic_internal.h
> @@ -327,54 +332,64 @@ M: Evgeny Voevodin 
>  M: Maksim Kozlov 
>  M: Igor Mitsyanko 
>  M: Dmitry Solodkiy 
> +L: qemu-...@nongnu.org
>  S: Maintained
>  F: hw/*/exynos*
>  
>  Calxeda Highbank
>  M: Rob Herring 
> +L: qemu-...@nongnu.org
>  S: Maintained
>  F: hw/arm/highbank.c
>  F: hw/net/xgmac.c
>  
>  Canon DIGIC
>  M: Antony Pavlov 
> +L: qemu-...@nongnu.org
>  S: Maintained
>  F: include/hw/arm/digic.h
>  F: hw/*/digic*
>  
>  Gumstix
>  L: qemu-devel@nongnu.org
> +L: qemu-...@nongnu.org
>  S: Orphan
>  F: hw/arm/gumstix.c
>  
>  i.MX31
>  M: Peter Chubb 
> +L: qemu-...@nongnu.org
>  S: Odd fixes
>  F: hw/*/imx*
>  F: hw/arm/kzm.c
>  
>  Integrator CP
>  M: Peter Maydell 
> +L: qemu-...@nongnu.org
>  S: Maintained
>  F: hw/arm/integratorcp.c
>  
>  Musicpal
>  M: Jan Kiszka 
> +L: qemu-...@nongnu.org
>  S: Maintained
>  F: hw/arm/musicpal.c
>  
>  nSeries
>  M: Andrzej Zaborowski 
> +L: qemu-...@nongnu.org
>  S: Maintained
>  F: hw/arm/nseries.c
>  
>  Palm
>  M: Andrzej Zaborowski 
> +L: qemu-...@nongnu.org
>  S: Maintained
>  F: hw/arm/palm.c
>  
>  Real View
>  M: Peter Maydell 
> +L: qemu-...@nongnu.org
>  S: Maintained
>  F: hw/arm/realview*
>  F: hw/intc/realview_gic.c
> @@ -382,6 +397,7 @@ F: include/hw/intc/realview_gic.h
>  
>  PXA2XX
>  M: Andrzej Zaborowski 
> +L: qemu-...@nongnu.org
>  S: Maintained
>  F: hw/arm/mainstone.c
>  F: hw/arm/spitz.c
> @@ -391,17 +407,20 @@ F: hw/*/pxa2xx*
>  
>  Stellaris
>  M: Peter Maydell 
> +L: qemu-...@nongnu.org
>  S: Maintained
>  F: hw/*/stellaris*
>  
>  Versatile PB
>  M: Peter Maydell 
> +L: qemu-...@nongnu.org
>  S: Maintained
>  F: hw/*/versatile*
>  
>  Xilinx Zynq
>  M: Alistair Francis 
>  M: Peter Crosthwaite 
> +L: qemu-...@nongnu.org
>  S: Maintained
>  F: hw/arm/xilinx_zynq.c
>  F: hw/misc/zynq_slcr.c
> @@ -411,6 +430,7 @@ F: hw/ssi/xilinx_spips.c
>  Xilinx ZynqMP
>  M: Alistair Francis 
>  M: Peter Crosthwaite 
> +L: qemu-...@nongnu.org
>  S: Maintained
>  F: hw/arm/xlnx-zynqmp.c
>  F: hw/arm/xlnx-ep108.c
> @@ -419,6 +439,7 @@ F: include/hw/arm/xlnx-zynqmp.h
>  ARM ACPI Subsystem
>  M: Shannon Zhao 
>  M: Shannon Zhao 
> +L: qemu-...@nongnu.org
>  S: Maintained
>  F: hw/arm/virt-acpi-build.c
>  F: include/hw/arm/virt-acpi-build.h
> @@ -1235,6 +1256,7 @@ AArch64 target
>  M: Claudio Fontana 
>  M: Claudio Fontana 
>  S: Maintained
> +L: qemu-...@nongnu.org
>  F: tcg/aarch64/
>  F: disas/arm-a64.cc
>  F: disas/libvixl/
> @@ -1242,6 +1264,7 @@ F: disas/libvixl/
>  ARM target
>  M: Andrzej Zaborowski 
>  S: Maintained
> +L: qemu-...@nongnu.org
>  F: tcg/arm/
>  F: disas/arm.c
>  
> 

-- 
Shannon

Re: [Qemu-devel] [PULL 00/12] Block patches

[Peter Maydell]

On 29 October 2015 at 18:09, Stefan Hajnoczi  wrote:
> The following changes since commit 7bc8e0c967a4ef77657174d28af775691e18b4ce:
>
>   Merge remote-tracking branch 'remotes/mst/tags/for_upstream' into staging 
> (2015-10-29 09:49:52 +)
>
> are available in the git repository at:
>
>   git://github.com/stefanha/qemu.git tags/block-pull-request
>
> for you to fetch changes up to 37a639a7fbc5c6b065c80e7e2de78d22af735496:
>
>   block: Consider all child nodes in bdrv_requests_pending() (2015-10-29 
> 17:59:27 +)
>
> 
>
> 

I get an error on 64-bit ARM running the ivshmem tests:

TEST: tests/ivshmem-test... (pid=22948)
  /i386/ivshmem/single:OK
  /i386/ivshmem/pair:  OK
  /i386/ivshmem/server:**
ERROR:/home/petmay01/qemu/tests/ivshmem-test.c:345:test_ivshmem_server:
assertion failed (ret != 0): (0 != 0)
FAIL
GTester: last random seed: R02S51e68a84790014e86af5b8b7264d3e39
(pid=23709)
  /i386/ivshmem/hotplug:   OK
  /i386/ivshmem/memdev:OK
FAIL: tests/ivshmem-test

Nothing obviously related in this patchset that would cause that,
though...

thanks
-- PMM

Re: [Qemu-devel] [PATCH v4] target-mips: fix updating XContext on mmu exception

[Leon Alrae]

On 29/10/15 17:17, Yongbok Kim wrote:
> Correct updating XContext.Region field on mmu exceptions.
> If Config3.CTXTC = 0 then the R field of XContext has to be updated
> with the value of bits 63..62 of the virtual address upon a TLB
> exception.
> Also fixed the below line which overs 80 characters.
> 
> Signed-off-by: Yongbok Kim 
> ---
>  target-mips/helper.c |7 ---
>  1 files changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/target-mips/helper.c b/target-mips/helper.c
> index 2d86323..b3fe816 100644
> --- a/target-mips/helper.c
> +++ b/target-mips/helper.c
> @@ -293,9 +293,10 @@ static void raise_mmu_exception(CPUMIPSState *env, 
> target_ulong address,
>  (env->CP0_EntryHi & 0xFF) | (address & (TARGET_PAGE_MASK << 1));
>  #if defined(TARGET_MIPS64)
>  env->CP0_EntryHi &= env->SEGMask;
> -env->CP0_XContext = (env->CP0_XContext & ((~0ULL) << (env->SEGBITS - 
> 7))) |
> -((address & 0xC000ULL) >> (55 - 
> env->SEGBITS)) |
> -((address & ((1ULL << env->SEGBITS) - 1) & 
> 0xE000ULL) >> 9);
> +env->CP0_XContext =
> +/* PTEBase */   (env->CP0_XContext & ((~0ULL) << (env->SEGBITS - 
> 7))) |
> +/* R */ (extract64(address, 62, 2) << (env->SEGBITS - 9)) |
> +/* BadVPN2 */   (extract64(address, 13, env->SEGBITS - 13) << 4);
>  #endif
>  cs->exception_index = exception;
>  env->error_code = error_code;
> 

Thanks for cleaning up the XContext calculation. I applied this one as
well as SIGRIE and RDHWR patches to the target-mips queue.

Regards,
Leon

Re: [Qemu-devel] [PATCH 2/2] trace: add make dependencies on tracetool source

[Stefan Hajnoczi]

On Thu, Oct 29, 2015 at 04:28:49PM +0100, Lluís Vilanova wrote:
> Stefan Hajnoczi writes:
> >  ##
> > +# tracetool source files
> > +# Every rule that invokes tracetool must depend on this so code is 
> > regenerated
> > +# if tracetool itself changes.
> > +
> > +tracetool-y = $(SRC_PATH)/scripts/tracetool.py
> > +tracetool-y += $(SRC_PATH)/scripts/tracetool/*.py
> > +tracetool-y += $(SRC_PATH)/scripts/tracetool/backend/*.py
> > +tracetool-y += $(SRC_PATH)/scripts/tracetool/format/*.py
> 
> If 'find' is an acceptable build dependency, I'd rather use this to avoid
> missing future sub-directories:
> 
>   tracetool-y = $(SRC_PATH)/scripts/tracetool.py
>   tracetool-y += $(shell find $(SRC_PATH)/scripts/tracetool -path "*.py")

find is already used in Makefile so it should be safe.  Thanks!


signature.asc
Description: PGP signature

[Qemu-devel] [PATCH v2 0/2] trace: fix Makefile dependencies

[Stefan Hajnoczi]

v2:
 * Use find(1) to catch all tracetool *.py files [Lluís]

   Note: I used find -name instead of find -path for better POSIX compliance

Issues with trace/Makefile.objs:

1. Generated code is not recreated when patches modify scripts/tracetool/*.py.
   Typically such patches also modify trace/*.[ch] and the result is build
   failures when new C code compiles against stale generated code.

2. The timestamp mechanism used to avoid unnecessary rebuilding is broken, it
   currently requires two make invocations for a full build.

Stefan Hajnoczi (2):
  trace: fix make foo-timestamp rules
  trace: add make dependencies on tracetool source

 trace/Makefile.objs | 48 
 1 file changed, 28 insertions(+), 20 deletions(-)

-- 
2.4.3

[Qemu-devel] [PATCH v2 1/2] trace: fix make foo-timestamp rules

[Stefan Hajnoczi]

The Makefile uses intermediate timestamp files to avoid rebuilding if
tracetool output is unchanged.

Timestamps are implemented incorrectly.  This was fixed for rules.mak in
commit 4b25966ab976f3a7fd9008193b2defcc82f8f04d ("rules.mak: cleanup
config generation rules") but never fixed in trace/Makefile.objs.

The problem with the old timestamp implementation was that make doesn't
notice the updated file modification time until the next time it is run.
It was necessary to run make twice in a row to achieve a full rebuild.

Signed-off-by: Stefan Hajnoczi 
---
 trace/Makefile.objs | 18 +-
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/trace/Makefile.objs b/trace/Makefile.objs
index 32f7a32..73bec38 100644
--- a/trace/Makefile.objs
+++ b/trace/Makefile.objs
@@ -5,20 +5,20 @@
 
 ifeq ($(findstring ust,$(TRACE_BACKENDS)),ust)
 $(obj)/generated-ust-provider.h: $(obj)/generated-ust-provider.h-timestamp
+   @cmp $< $@ >/dev/null 2>&1 || cp $< $@
 $(obj)/generated-ust-provider.h-timestamp: $(SRC_PATH)/trace-events
$(call quiet-command,$(TRACETOOL) \
--format=ust-events-h \
--backends=$(TRACE_BACKENDS) \
< $< > $@,"  GEN   $(patsubst %-timestamp,%,$@)")
-   @cmp -s $@ $(patsubst %-timestamp,%,$@) || cp $@ $(patsubst 
%-timestamp,%,$@)
 
 $(obj)/generated-ust.c: $(obj)/generated-ust.c-timestamp 
$(BUILD_DIR)/config-host.mak
+   @cmp $< $@ >/dev/null 2>&1 || cp $< $@
 $(obj)/generated-ust.c-timestamp: $(SRC_PATH)/trace-events
$(call quiet-command,$(TRACETOOL) \
--format=ust-events-c \
--backends=$(TRACE_BACKENDS) \
< $< > $@,"  GEN   $(patsubst %-timestamp,%,$@)")
-   @cmp -s $@ $(patsubst %-timestamp,%,$@) || cp $@ $(patsubst 
%-timestamp,%,$@)
 
 $(obj)/generated-events.h: $(obj)/generated-ust-provider.h
 $(obj)/generated-events.c: $(obj)/generated-ust.c
@@ -28,20 +28,20 @@ endif
 # Auto-generated event descriptions
 
 $(obj)/generated-events.h: $(obj)/generated-events.h-timestamp
+   @cmp $< $@ >/dev/null 2>&1 || cp $< $@
 $(obj)/generated-events.h-timestamp: $(SRC_PATH)/trace-events
$(call quiet-command,$(TRACETOOL) \
--format=events-h \
--backends=$(TRACE_BACKENDS) \
< $< > $@,"  GEN   $(patsubst %-timestamp,%,$@)")
-   @cmp -s $@ $(patsubst %-timestamp,%,$@) || cp $@ $(patsubst 
%-timestamp,%,$@)
 
 $(obj)/generated-events.c: $(obj)/generated-events.c-timestamp 
$(BUILD_DIR)/config-host.mak
+   @cmp $< $@ >/dev/null 2>&1 || cp $< $@
 $(obj)/generated-events.c-timestamp: $(SRC_PATH)/trace-events
$(call quiet-command,$(TRACETOOL) \
--format=events-c \
--backends=$(TRACE_BACKENDS) \
< $< > $@,"  GEN   $(patsubst %-timestamp,%,$@)")
-   @cmp -s $@ $(patsubst %-timestamp,%,$@) || cp $@ $(patsubst 
%-timestamp,%,$@)
 
 util-obj-y += generated-events.o
 
@@ -81,12 +81,12 @@ $(obj)/generated-tracers.o: $(obj)/generated-tracers.c 
$(obj)/generated-tracers.
 # rule file. So we use '.dtrace' instead
 ifeq ($(findstring dtrace,$(TRACE_BACKENDS)),dtrace)
 $(obj)/generated-tracers-dtrace.dtrace: 
$(obj)/generated-tracers-dtrace.dtrace-timestamp
+   @cmp $< $@ >/dev/null 2>&1 || cp $< $@
 $(obj)/generated-tracers-dtrace.dtrace-timestamp: $(SRC_PATH)/trace-events 
$(BUILD_DIR)/config-host.mak
$(call quiet-command,$(TRACETOOL) \
--format=d \
--backends=$(TRACE_BACKENDS) \
< $< > $@,"  GEN   $(patsubst %-timestamp,%,$@)")
-   @cmp -s $@ $(patsubst %-timestamp,%,$@) || cp $@ $(patsubst 
%-timestamp,%,$@)
 
 $(obj)/generated-tracers-dtrace.h: $(obj)/generated-tracers-dtrace.dtrace
$(call quiet-command,dtrace -o $@ -h -s $<, "  GEN   $@")
@@ -100,28 +100,28 @@ endif
 # Translation level
 
 $(obj)/generated-helpers-wrappers.h: 
$(obj)/generated-helpers-wrappers.h-timestamp
+   @cmp $< $@ >/dev/null 2>&1 || cp $< $@
 $(obj)/generated-helpers-wrappers.h-timestamp: $(SRC_PATH)/trace-events 
$(BUILD_DIR)/config-host.mak
$(call quiet-command,$(TRACETOOL) \
--format=tcg-helper-wrapper-h \
--backend=$(TRACE_BACKENDS) \
< $< > $@,"  GEN   $(patsubst %-timestamp,%,$@)")
-   @cmp -s $@ $(patsubst %-timestamp,%,$@) || cp $@ $(patsubst 
%-timestamp,%,$@)
 
 $(obj)/generated-helpers.h: $(obj)/generated-helpers.h-timestamp
+   @cmp $< $@ >/dev/null 2>&1 || cp $< $@
 $(obj)/generated-helpers.h-timestamp: $(SRC_PATH)/trace-events 
$(BUILD_DIR)/config-host.mak
$(call quiet-command,$(TRACETOOL) \
--format=tcg-helper-h \
--backend=$(TRACE_BACKENDS) \
< $< > $@,"  GEN   $(patsubst %-timestamp,%,$@)")
-   @cmp -s $@ $(patsubst %-timestamp,%,$@) || cp $@ $(patsubst 
%-timestamp,%,$@)
 
 $(obj)/generated-helpers.c: $(obj)/generated-helpers.c-timestamp
+   @cmp $< $@ >/dev/nu

[Qemu-devel] [PATCH v2 2/2] trace: add make dependencies on tracetool source

[Stefan Hajnoczi]

Patches that change tracetool can break the build if old build output
files are lying around.

This happens because the Makefile does not specify dependencies on
tracetool.  The build will use old object files that do not match the
current source code.

Signed-off-by: Stefan Hajnoczi 
---
 trace/Makefile.objs | 30 +++---
 1 file changed, 19 insertions(+), 11 deletions(-)

diff --git a/trace/Makefile.objs b/trace/Makefile.objs
index 73bec38..5145b34 100644
--- a/trace/Makefile.objs
+++ b/trace/Makefile.objs
@@ -1,12 +1,20 @@
 # -*- mode: makefile -*-
 
 ##
+# tracetool source files
+# Every rule that invokes tracetool must depend on this so code is regenerated
+# if tracetool itself changes.
+
+tracetool-y = $(SRC_PATH)/scripts/tracetool.py
+tracetool-y += $(shell find $(SRC_PATH)/scripts/tracetool -name "*.py")
+
+##
 # Auto-generated event descriptions for LTTng ust code
 
 ifeq ($(findstring ust,$(TRACE_BACKENDS)),ust)
 $(obj)/generated-ust-provider.h: $(obj)/generated-ust-provider.h-timestamp
@cmp $< $@ >/dev/null 2>&1 || cp $< $@
-$(obj)/generated-ust-provider.h-timestamp: $(SRC_PATH)/trace-events
+$(obj)/generated-ust-provider.h-timestamp: $(SRC_PATH)/trace-events 
$(tracetool-y)
$(call quiet-command,$(TRACETOOL) \
--format=ust-events-h \
--backends=$(TRACE_BACKENDS) \
@@ -14,7 +22,7 @@ $(obj)/generated-ust-provider.h-timestamp: 
$(SRC_PATH)/trace-events
 
 $(obj)/generated-ust.c: $(obj)/generated-ust.c-timestamp 
$(BUILD_DIR)/config-host.mak
@cmp $< $@ >/dev/null 2>&1 || cp $< $@
-$(obj)/generated-ust.c-timestamp: $(SRC_PATH)/trace-events
+$(obj)/generated-ust.c-timestamp: $(SRC_PATH)/trace-events $(tracetool-y)
$(call quiet-command,$(TRACETOOL) \
--format=ust-events-c \
--backends=$(TRACE_BACKENDS) \
@@ -29,7 +37,7 @@ endif
 
 $(obj)/generated-events.h: $(obj)/generated-events.h-timestamp
@cmp $< $@ >/dev/null 2>&1 || cp $< $@
-$(obj)/generated-events.h-timestamp: $(SRC_PATH)/trace-events
+$(obj)/generated-events.h-timestamp: $(SRC_PATH)/trace-events $(tracetool-y)
$(call quiet-command,$(TRACETOOL) \
--format=events-h \
--backends=$(TRACE_BACKENDS) \
@@ -37,7 +45,7 @@ $(obj)/generated-events.h-timestamp: $(SRC_PATH)/trace-events
 
 $(obj)/generated-events.c: $(obj)/generated-events.c-timestamp 
$(BUILD_DIR)/config-host.mak
@cmp $< $@ >/dev/null 2>&1 || cp $< $@
-$(obj)/generated-events.c-timestamp: $(SRC_PATH)/trace-events
+$(obj)/generated-events.c-timestamp: $(SRC_PATH)/trace-events $(tracetool-y)
$(call quiet-command,$(TRACETOOL) \
--format=events-c \
--backends=$(TRACE_BACKENDS) \
@@ -54,7 +62,7 @@ util-obj-y += generated-events.o
 
 $(obj)/generated-tracers.h: $(obj)/generated-tracers.h-timestamp
@cmp -s $< $@ || cp $< $@
-$(obj)/generated-tracers.h-timestamp: $(SRC_PATH)/trace-events 
$(BUILD_DIR)/config-host.mak
+$(obj)/generated-tracers.h-timestamp: $(SRC_PATH)/trace-events 
$(BUILD_DIR)/config-host.mak $(tracetool-y)
$(call quiet-command,$(TRACETOOL) \
--format=h \
--backends=$(TRACE_BACKENDS) \
@@ -65,7 +73,7 @@ $(obj)/generated-tracers.h-timestamp: 
$(SRC_PATH)/trace-events $(BUILD_DIR)/conf
 
 $(obj)/generated-tracers.c: $(obj)/generated-tracers.c-timestamp
@cmp -s $< $@ || cp $< $@
-$(obj)/generated-tracers.c-timestamp: $(SRC_PATH)/trace-events 
$(BUILD_DIR)/config-host.mak
+$(obj)/generated-tracers.c-timestamp: $(SRC_PATH)/trace-events 
$(BUILD_DIR)/config-host.mak $(tracetool-y)
$(call quiet-command,$(TRACETOOL) \
--format=c \
--backends=$(TRACE_BACKENDS) \
@@ -82,7 +90,7 @@ $(obj)/generated-tracers.o: $(obj)/generated-tracers.c 
$(obj)/generated-tracers.
 ifeq ($(findstring dtrace,$(TRACE_BACKENDS)),dtrace)
 $(obj)/generated-tracers-dtrace.dtrace: 
$(obj)/generated-tracers-dtrace.dtrace-timestamp
@cmp $< $@ >/dev/null 2>&1 || cp $< $@
-$(obj)/generated-tracers-dtrace.dtrace-timestamp: $(SRC_PATH)/trace-events 
$(BUILD_DIR)/config-host.mak
+$(obj)/generated-tracers-dtrace.dtrace-timestamp: $(SRC_PATH)/trace-events 
$(BUILD_DIR)/config-host.mak $(tracetool-y)
$(call quiet-command,$(TRACETOOL) \
--format=d \
--backends=$(TRACE_BACKENDS) \
@@ -101,7 +109,7 @@ endif
 
 $(obj)/generated-helpers-wrappers.h: 
$(obj)/generated-helpers-wrappers.h-timestamp
@cmp $< $@ >/dev/null 2>&1 || cp $< $@
-$(obj)/generated-helpers-wrappers.h-timestamp: $(SRC_PATH)/trace-events 
$(BUILD_DIR)/config-host.mak
+$(obj)/generated-helpers-wrappers.h-timestamp: $(SRC_PATH)/trace-events 
$(BUILD_DIR)/config-host.mak $(tracetool-y)
$(call quiet-command,$(TRACETOOL) \
--format=tcg-helper-wrapper-h

Re: [Qemu-devel] [PATCH v4 3/3] aio: Introduce aio-epoll.c

[Stefan Hajnoczi]

On Fri, Oct 30, 2015 at 12:06:29PM +0800, Fam Zheng wrote:
> To comply with aio_{disable,enable}_external, we always use ppoll when
> aio_external_disabled() is true.

All file descriptors are added to the epoll fd.  Does that mean epoll
will report the same fds again after we come out of
ppoll()/aio_external_disabled()?

The two constraints to think about:
1. Ideally there should be no duplicated events.
2. There absolutely cannot be any missed events.

Stefan


signature.asc
Description: PGP signature

Re: [Qemu-devel] [PATCH v6 27/33] nvdimm acpi: support function 0

[Stefan Hajnoczi]

On Fri, Oct 30, 2015 at 01:56:21PM +0800, Xiao Guangrong wrote:
>  static uint64_t
>  nvdimm_dsm_read(void *opaque, hwaddr addr, unsigned size)
>  {
> -return 0;
> +AcpiNVDIMMState *state = opaque;
> +MemoryRegion *dsm_ram_mr = &state->ram_mr;
> +NvdimmDsmIn *in;
> +GArray *out;
> +void *dsm_ram_addr;
> +uint32_t buf_size;
> +
> +assert(memory_region_size(dsm_ram_mr) >= sizeof(NvdimmDsmIn));
> +dsm_ram_addr = memory_region_get_ram_ptr(dsm_ram_mr);
> +
> +/*
> + * The DSM memory is mapped to guest address space so an evil guest
> + * can change its content while we are doing DSM emulation. Avoid
> + * this by copying DSM memory to QEMU local memory.
> + */
> +in = g_malloc(memory_region_size(dsm_ram_mr));
> +memcpy(in, dsm_ram_addr, memory_region_size(dsm_ram_mr));
> +
> +le32_to_cpus(&in->revision);
> +le32_to_cpus(&in->function);
> +le32_to_cpus(&in->handle);
> +
> +nvdimm_debug("Revision %#x Handler %#x Function %#x.\n", in->revision,
> + in->handle, in->function);
> +
> +out = g_array_new(false, true /* clear */, 1);
> +
> +if (in->revision != 0x1 /* Current we support DSM Spec Rev1. */) {
> +nvdimm_debug("Revision %#x is not supported, expect %#x.\n",
> +  in->revision, 0x1);
> +nvdimm_dsm_write_status(out, NVDIMM_DSM_STATUS_NOT_SUPPORTED);
> +goto exit;
> +}
> +
> +/* Handle 0 is reserved for NVDIMM Root Device. */
> +if (!in->handle) {
> +nvdimm_dsm_root(in, out);
> +goto exit;
> +}
> +
> +nvdimm_dsm_device(in, out);
> +
> +exit:
> +/* Write output result to dsm memory. */
> +memcpy(dsm_ram_addr, out->data, out->len);
> +memory_region_set_dirty(dsm_ram_mr, 0, out->len);

If you respin this series, please add this before the memcpy out:

  assert(out->len <= memory_region_size(dsm_ram_mr))

That way we can catch situations where too much output data was
generated by mistake.


signature.asc
Description: PGP signature

Re: [Qemu-devel] [PATCH v6 00/33] implement vNVDIMM

[Stefan Hajnoczi]

On Fri, Oct 30, 2015 at 01:55:54PM +0800, Xiao Guangrong wrote:
> This patchset can be found at:
>   https://github.com/xiaogr/qemu.git nvdimm-v6
> 
> It is based on pci branch on Michael's tree and the top commit is:
> commit 6f96a31a06c2a1 (tests: re-enable vhost-user-test).
> 
> Changelog in v6:
> - changes from Stefan's comments:
>   1) fix code style of struct naming by CamelCase way
>   2) fix offset + length overflow when read/write label data
>   3) compile hw/acpi/nvdimm.c for per target so that TARGET_PAGE_SIZE can
>  be used to replace getpagesize()
> 
> Changelog in v5:
> - changes from Michael's comments:
>   1) prefix nvdimm_ to everything in NVDIMM source files
>   2) make parsing _DSM Arg3 more clear
>   3) comment style fix
>   5) drop single used definition
>   6) fix dirty dsm buffer lost due to memory write happened on host
>   7) check dsm buffer if it is big enough to contain input data
>   8) use build_append_int_noprefix to store single value to GArray
> 
> - changes from Michael's and Igor's comments:
>   1) introduce 'nvdimm-support' parameter to control nvdimm
>  enablement and it is disabled for 2.4 and its earlier versions
>  to make live migration compatible
>   2) only reserve 1 RAM page and 4 bytes IO Port for NVDIMM ACPI
>  virtualization
> 
> - changes from Stefan's comments:
>   1) do endian adjustment for the buffer length
> 
> - changes from Bharata B Rao's comments:
>   1) fix compile on ppc
> 
> - others:
>   1) the buffer length is directly got from IO read rather than got
>  from dsm memory
>   2) fix dirty label data lost due to memory write happened on host
> 
> Changelog in v4:
> - changes from Michael's comments:
>   1) show the message, "Memory is not allocated from HugeTlbfs", if file
>  based memory is not allocated from hugetlbfs.
>   2) introduce function, acpi_get_nvdimm_state(), to get NVDIMMState
>  from Machine.
>   3) statically define UUID and make its operation more clear
>   4) use GArray to build device structures to avoid potential buffer
>  overflow
>   4) improve comments in the code
>   5) improve code style
> 
> - changes from Igor's comments:
>   1) add NVDIMM ACPI spec document
>   2) use serialized method to avoid Mutex
>   3) move NVDIMM ACPI's code to hw/acpi/nvdimm.c
>   4) introduce a common ASL method used by _DSM for all devices to reduce
>  ACPI size
>   5) handle UUID in ACPI AML code. BTW, i'd keep handling revision in QEMU
>  it's better to upgrade QEMU to support Rev2 in the future
> 
> - changes from Stefan's comments:
>   1) copy input data from DSM memory to local buffer to avoid potential
>  issues as DSM memory is visible to guest. Output data is handled
>  in a similar way
> 
> - changes from Dan's comments:
>   1) drop static namespace as Linux has already supported label-less
>  nvdimm devices
> 
> - changes from Vladimir's comments:
>   1) print better message, "failed to get file size for %s, can't create
>  backend on it", if any file operation filed to obtain file size
> 
> - others:
>   create a git repo on github.com for better review/test
> 
> Also, thanks for Eric Blake's review on QAPI's side.
> 
> Thank all of you to review this patchset.
> 
> Changelog in v3:
> There is huge change in this version, thank Igor, Stefan, Paolo, Eduardo,
> Michael for their valuable comments, the patchset finally gets better shape.
> - changes from Igor's comments:
>   1) abstract dimm device type from pc-dimm and create nvdimm device based on
>  dimm, then it uses memory backend device as nvdimm's memory and NUMA has
>  easily been implemented.
>   2) let file-backend device support any kind of filesystem not only for
>  hugetlbfs and let it work on file not only for directory which is
>  achieved by extending 'mem-path' - if it's a directory then it works as
>  current behavior, otherwise if it's file then directly allocates memory
>  from it.
>   3) we figure out a unused memory hole below 4G that is 0xFF0 ~ 
>  0xFFF0, this range is large enough for NVDIMM ACPI as build 64-bit
>  ACPI SSDT/DSDT table will break windows XP.
>  BTW, only make SSDT.rev = 2 can not work since the width is only depended
>  on DSDT.rev based on 19.6.28 DefinitionBlock (Declare Definition Block)
>  in ACPI spec:
> | Note: For compatibility with ACPI versions before ACPI 2.0, the bit 
> | width of Integer objects is dependent on the ComplianceRevision of the DSDT.
> | If the ComplianceRevision is less than 2, all integers are restricted to 32 
> | bits. Otherwise, full 64-bit integers are used. The version of the DSDT 
> sets 
> | the global integer width for all integers, including integers in SSDTs.
>   4) use the lowest ACPI spec version to document AML terms.
>   5) use "nvdimm" as nvdimm device name instead of "pc-nvdimm"
> 
> - changes from Stefan's comments:
>   1) do not do endian adjustment in-place since _DSM memory is visible to 
>

Re: [Qemu-devel] [PULL v4 00/14] QMP and QObject patches

[Peter Maydell]

On 30 October 2015 at 08:10, Markus Armbruster  wrote:
> The following changes since commit 7bc8e0c967a4ef77657174d28af775691e18b4ce:
>
>   Merge remote-tracking branch 'remotes/mst/tags/for_upstream' into staging 
> (2015-10-29 09:49:52 +)
>
> are available in the git repository at:
>
>   git://repo.or.cz/qemu/armbru.git tags/pull-monitor-2015-10-30
>
> for you to fetch changes up to 7f1e7b23d57408c86d350b3544673fdcd6be55c0:
>
>   docs: Document QMP event rate limiting (2015-10-30 09:05:38 +0100)
>
> 
> QMP and QObject patches
>
> 
> Markus Armbruster (14):
>   qobject: Drop QObject_HEAD
>   qbool: Make conversion from QObject * accept null
>   qdict: Make conversion from QObject * accept null
>   qfloat qint: Make conversion from QObject * accept null
>   qlist: Make conversion from QObject * accept null
>   qstring: Make conversion from QObject * accept null
>   monitor: Reduce casting of QAPI event QDict
>   monitor: Simplify event throttling
>   monitor: Switch from timer_new() to timer_new_ns()
>   monitor: Split MonitorQAPIEventConf off MonitorQAPIEventState
>   glib: add compatibility interface for g_hash_table_add()
>   monitor: Turn monitor_qapi_event_state[] into a hash table
>   monitor: Throttle event VSERPORT_CHANGE separately by "id"
>   docs: Document QMP event rate limiting

Applied, thanks.

-- PMM

Re: [Qemu-devel] [PATCH] hw/usb/dev-audio.c: make USB audio card sound perfect

[Gerd Hoffmann]

  Hi,

[ context for spice folks: patch was added to qemu increasing usb-audio 
  default buffer size ]

> > What bothers me is that you have no qualms about making latency on
> > everyone's system worse.
> 
> How do you know it makes sound on other people's systems worse? If you have
> actually done any testing, I would like to see the results. 

It's real.  With that change we *do* actually trade latency for better
sound quality.

You probably wouldn't notice with pure music playback.

Higher chance is with video playback.  lip sync issues might show up,
although you probably still have to watch carefully to actually notice.

Anything sending audio both ways and expecting it to run with low
latency (VoIP phone, music jam as mentioned by stefan) is affected even
more.

And we *do* actually just paper over the root cause.  Problem is the
real root cause can is very hard to track down.  It can be pretty much
anywhere in qemu, and even outside qemu.

One known issue actually is in spice-server (added spice-devel because
of that).  It does audio processing in the qemu iothread (instead of a
separate thread like it is done for the display channel).  If you turn
off audio compression in spice sound quality suddenly becomes better. I
think this this happens because the latency spikes caused by audio
compression go away.  Happens with intel-hda and windows guests (which
use very small audio buffers).  Not (yet) investigated in detail though.

cheers,
  Gerd

[Qemu-devel] [PATCH 02/19] buffer: add buffer_init

[Gerd Hoffmann]

Signed-off-by: Gerd Hoffmann 
Reviewed-by: Peter Lieven 
---
 include/qemu/buffer.h | 12 
 util/buffer.c | 11 +++
 2 files changed, 23 insertions(+)

diff --git a/include/qemu/buffer.h b/include/qemu/buffer.h
index b380cec..0710e16 100644
--- a/include/qemu/buffer.h
+++ b/include/qemu/buffer.h
@@ -34,12 +34,24 @@ typedef struct Buffer Buffer;
  */
 
 struct Buffer {
+char *name;
 size_t capacity;
 size_t offset;
 uint8_t *buffer;
 };
 
 /**
+ * buffer_init:
+ * @buffer: the buffer object
+ * @name: buffer name
+ *
+ * Optionally attach a name to the buffer, to make it easier
+ * to identify in debug traces.
+ */
+void buffer_init(Buffer *buffer, const char *name, ...)
+GCC_FMT_ATTR(2, 3);
+
+/**
  * buffer_reserve:
  * @buffer: the buffer object
  * @len: the minimum required free space
diff --git a/util/buffer.c b/util/buffer.c
index 7ddd693..12bf2d7 100644
--- a/util/buffer.c
+++ b/util/buffer.c
@@ -22,6 +22,15 @@
 
 #define BUFFER_MIN_INIT_SIZE 4096
 
+void buffer_init(Buffer *buffer, const char *name, ...)
+{
+va_list ap;
+
+va_start(ap, name);
+buffer->name = g_strdup_vprintf(name, ap);
+va_end(ap);
+}
+
 void buffer_reserve(Buffer *buffer, size_t len)
 {
 if ((buffer->capacity - buffer->offset) < len) {
@@ -49,9 +58,11 @@ void buffer_reset(Buffer *buffer)
 void buffer_free(Buffer *buffer)
 {
 g_free(buffer->buffer);
+g_free(buffer->name);
 buffer->offset = 0;
 buffer->capacity = 0;
 buffer->buffer = NULL;
+buffer->name = NULL;
 }
 
 void buffer_append(Buffer *buffer, const void *data, size_t len)
-- 
1.8.3.1

[Qemu-devel] [PATCH 04/19] buffer: add buffer_move

[Gerd Hoffmann]

Signed-off-by: Gerd Hoffmann 
Reviewed-by: Peter Lieven 
---
 include/qemu/buffer.h | 10 ++
 util/buffer.c | 16 
 2 files changed, 26 insertions(+)

diff --git a/include/qemu/buffer.h b/include/qemu/buffer.h
index f53ee9e..1358df1 100644
--- a/include/qemu/buffer.h
+++ b/include/qemu/buffer.h
@@ -137,4 +137,14 @@ gboolean buffer_empty(Buffer *buffer);
  */
 void buffer_move_empty(Buffer *to, Buffer *from);
 
+/**
+ * buffer_move:
+ * @to: destination buffer object
+ * @from: source buffer object
+ *
+ * Moves buffer, copying data (unless 'to' buffer happens to be empty).
+ * 'from' buffer is empty and zero-sized on return.
+ */
+void buffer_move(Buffer *to, Buffer *from);
+
 #endif /* QEMU_BUFFER_H__ */
diff --git a/util/buffer.c b/util/buffer.c
index c7a39ec..e8f798e 100644
--- a/util/buffer.c
+++ b/util/buffer.c
@@ -91,3 +91,19 @@ void buffer_move_empty(Buffer *to, Buffer *from)
 from->capacity = 0;
 from->buffer = NULL;
 }
+
+void buffer_move(Buffer *to, Buffer *from)
+{
+if (to->offset == 0) {
+buffer_move_empty(to, from);
+return;
+}
+
+buffer_reserve(to, from->offset);
+buffer_append(to, from->buffer, from->offset);
+
+g_free(from->buffer);
+from->offset = 0;
+from->capacity = 0;
+from->buffer = NULL;
+}
-- 
1.8.3.1

[Qemu-devel] [PATCH 08/19] vnc: kill jobs queue buffer

[Gerd Hoffmann]

Signed-off-by: Gerd Hoffmann 
Reviewed-by: Peter Lieven 
---
 ui/vnc-jobs.c | 6 --
 1 file changed, 6 deletions(-)

diff --git a/ui/vnc-jobs.c b/ui/vnc-jobs.c
index 2e6c15f..329d13e 100644
--- a/ui/vnc-jobs.c
+++ b/ui/vnc-jobs.c
@@ -54,7 +54,6 @@ struct VncJobQueue {
 QemuCond cond;
 QemuMutex mutex;
 QemuThread thread;
-Buffer buffer;
 bool exit;
 QTAILQ_HEAD(, VncJob) jobs;
 };
@@ -193,7 +192,6 @@ static void vnc_async_encoding_start(VncState *orig, 
VncState *local)
 local->zlib = orig->zlib;
 local->hextile = orig->hextile;
 local->zrle = orig->zrle;
-local->output =  queue->buffer;
 local->csock = -1; /* Don't do any network work on this thread */
 
 buffer_reset(&local->output);
@@ -206,8 +204,6 @@ static void vnc_async_encoding_end(VncState *orig, VncState 
*local)
 orig->hextile = local->hextile;
 orig->zrle = local->zrle;
 orig->lossy_rect = local->lossy_rect;
-
-queue->buffer = local->output;
 }
 
 static int vnc_worker_thread_loop(VncJobQueue *queue)
@@ -304,7 +300,6 @@ static VncJobQueue *vnc_queue_init(void)
 
 qemu_cond_init(&queue->cond);
 qemu_mutex_init(&queue->mutex);
-buffer_init(&queue->buffer, "vnc-job-queue");
 QTAILQ_INIT(&queue->jobs);
 return queue;
 }
@@ -313,7 +308,6 @@ static void vnc_queue_clear(VncJobQueue *q)
 {
 qemu_cond_destroy(&queue->cond);
 qemu_mutex_destroy(&queue->mutex);
-buffer_free(&queue->buffer);
 g_free(q);
 queue = NULL; /* Unset global queue */
 }
-- 
1.8.3.1

[Qemu-devel] [PATCH 05/19] buffer: add buffer_shrink

[Gerd Hoffmann]

Signed-off-by: Gerd Hoffmann 
---
 include/qemu/buffer.h | 10 ++
 util/buffer.c | 20 +++-
 2 files changed, 29 insertions(+), 1 deletion(-)

diff --git a/include/qemu/buffer.h b/include/qemu/buffer.h
index 1358df1..0a69b3a 100644
--- a/include/qemu/buffer.h
+++ b/include/qemu/buffer.h
@@ -52,6 +52,16 @@ void buffer_init(Buffer *buffer, const char *name, ...)
 GCC_FMT_ATTR(2, 3);
 
 /**
+ * buffer_shrink:
+ * @buffer: the buffer object
+ *
+ * Try to shrink the buffer.  Checks current buffer capacity and size
+ * and reduces capacity in case only a fraction of the buffer is
+ * actually used.
+ */
+void buffer_shrink(Buffer *buffer);
+
+/**
  * buffer_reserve:
  * @buffer: the buffer object
  * @len: the minimum required free space
diff --git a/util/buffer.c b/util/buffer.c
index e8f798e..234e33d 100644
--- a/util/buffer.c
+++ b/util/buffer.c
@@ -20,7 +20,8 @@
 
 #include "qemu/buffer.h"
 
-#define BUFFER_MIN_INIT_SIZE 4096
+#define BUFFER_MIN_INIT_SIZE 4096
+#define BUFFER_MIN_SHRINK_SIZE  65536
 
 void buffer_init(Buffer *buffer, const char *name, ...)
 {
@@ -31,6 +32,23 @@ void buffer_init(Buffer *buffer, const char *name, ...)
 va_end(ap);
 }
 
+void buffer_shrink(Buffer *buffer)
+{
+/*
+ * Only shrink in case the used size is *much* smaller than the
+ * capacity, to avoid bumping up & down the buffers all the time.
+ * realloc() isn't exactly cheap ...
+ */
+if (buffer->offset < (buffer->capacity >> 3) &&
+buffer->capacity > BUFFER_MIN_SHRINK_SIZE) {
+return;
+}
+
+buffer->capacity = pow2ceil(buffer->offset);
+buffer->capacity = MAX(buffer->capacity, BUFFER_MIN_SHRINK_SIZE);
+buffer->buffer = g_realloc(buffer->buffer, buffer->capacity);
+}
+
 void buffer_reserve(Buffer *buffer, size_t len)
 {
 if ((buffer->capacity - buffer->offset) < len) {
-- 
1.8.3.1

[Qemu-devel] [PATCH 03/19] buffer: add buffer_move_empty

[Gerd Hoffmann]

Signed-off-by: Gerd Hoffmann 
Reviewed-by: Peter Lieven 
---
 include/qemu/buffer.h | 10 ++
 util/buffer.c | 14 ++
 2 files changed, 24 insertions(+)

diff --git a/include/qemu/buffer.h b/include/qemu/buffer.h
index 0710e16..f53ee9e 100644
--- a/include/qemu/buffer.h
+++ b/include/qemu/buffer.h
@@ -127,4 +127,14 @@ uint8_t *buffer_end(Buffer *buffer);
  */
 gboolean buffer_empty(Buffer *buffer);
 
+/**
+ * buffer_move_empty:
+ * @to: destination buffer object
+ * @from: source buffer object
+ *
+ * Moves buffer, without copying data.  'to' buffer must be empty.
+ * 'from' buffer is empty and zero-sized on return.
+ */
+void buffer_move_empty(Buffer *to, Buffer *from);
+
 #endif /* QEMU_BUFFER_H__ */
diff --git a/util/buffer.c b/util/buffer.c
index 12bf2d7..c7a39ec 100644
--- a/util/buffer.c
+++ b/util/buffer.c
@@ -77,3 +77,17 @@ void buffer_advance(Buffer *buffer, size_t len)
 (buffer->offset - len));
 buffer->offset -= len;
 }
+
+void buffer_move_empty(Buffer *to, Buffer *from)
+{
+assert(to->offset == 0);
+
+g_free(to->buffer);
+to->offset = from->offset;
+to->capacity = from->capacity;
+to->buffer = from->buffer;
+
+from->offset = 0;
+from->capacity = 0;
+from->buffer = NULL;
+}
-- 
1.8.3.1

[Qemu-devel] [PATCH 01/19] buffer: make the Buffer capacity increase in powers of two

[Gerd Hoffmann]

From: Peter Lieven 

This makes sure the number of reallocs is in O(log N).

Signed-off-by: Peter Lieven 

[ rebased to util/buffer.c ]

Signed-off-by: Gerd Hoffmann 
---
 util/buffer.c | 5 -
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/util/buffer.c b/util/buffer.c
index cedd055..7ddd693 100644
--- a/util/buffer.c
+++ b/util/buffer.c
@@ -20,10 +20,13 @@
 
 #include "qemu/buffer.h"
 
+#define BUFFER_MIN_INIT_SIZE 4096
+
 void buffer_reserve(Buffer *buffer, size_t len)
 {
 if ((buffer->capacity - buffer->offset) < len) {
-buffer->capacity += (len + 1024);
+buffer->capacity = pow2ceil(buffer->offset + len);
+buffer->capacity = MAX(buffer->capacity, BUFFER_MIN_INIT_SIZE);
 buffer->buffer = g_realloc(buffer->buffer, buffer->capacity);
 }
 }
-- 
1.8.3.1

[Qemu-devel] [PATCH 14/19] vnc: only alloc server surface with clients connected

[Gerd Hoffmann]

Signed-off-by: Gerd Hoffmann 
---
 ui/vnc.c | 12 
 1 file changed, 12 insertions(+)

diff --git a/ui/vnc.c b/ui/vnc.c
index 8ee1266..c5bef47 100644
--- a/ui/vnc.c
+++ b/ui/vnc.c
@@ -721,6 +721,10 @@ static void vnc_update_server_surface(VncDisplay *vd)
 qemu_pixman_image_unref(vd->server);
 vd->server = NULL;
 
+if (QTAILQ_EMPTY(&vd->clients)) {
+return;
+}
+
 vd->server = pixman_image_create_bits(VNC_SERVER_FB_FORMAT,
   vnc_width(vd),
   vnc_height(vd),
@@ -1231,6 +1235,10 @@ void vnc_disconnect_finish(VncState *vs)
 if (vs->initialized) {
 QTAILQ_REMOVE(&vs->vd->clients, vs, next);
 qemu_remove_mouse_mode_change_notifier(&vs->mouse_mode_notifier);
+if (QTAILQ_EMPTY(&vs->vd->clients)) {
+/* last client gone */
+vnc_update_server_surface(vs->vd);
+}
 }
 
 if (vs->vd->lock_key_sync)
@@ -3069,6 +3077,7 @@ void vnc_init_state(VncState *vs)
 {
 vs->initialized = true;
 VncDisplay *vd = vs->vd;
+bool first_client = QTAILQ_EMPTY(&vd->clients);
 
 vs->last_x = -1;
 vs->last_y = -1;
@@ -3082,6 +3091,9 @@ void vnc_init_state(VncState *vs)
 vs->bh = qemu_bh_new(vnc_jobs_bh, vs);
 
 QTAILQ_INSERT_TAIL(&vd->clients, vs, next);
+if (first_client) {
+vnc_update_server_surface(vd);
+}
 
 graphic_hw_update(vd->dcl.con);
 
-- 
1.8.3.1

[Qemu-devel] [PATCH 07/19] vnc: attach names to buffers

[Gerd Hoffmann]

Signed-off-by: Gerd Hoffmann 
---
 ui/vnc-jobs.c |  3 +++
 ui/vnc.c  | 20 
 2 files changed, 23 insertions(+)

diff --git a/ui/vnc-jobs.c b/ui/vnc-jobs.c
index 22c9abc..2e6c15f 100644
--- a/ui/vnc-jobs.c
+++ b/ui/vnc-jobs.c
@@ -218,6 +218,8 @@ static int vnc_worker_thread_loop(VncJobQueue *queue)
 int n_rectangles;
 int saved_offset;
 
+buffer_init(&vs.output, "vnc-worker-output");
+
 vnc_lock_queue(queue);
 while (QTAILQ_EMPTY(&queue->jobs) && !queue->exit) {
 qemu_cond_wait(&queue->cond, &queue->mutex);
@@ -302,6 +304,7 @@ static VncJobQueue *vnc_queue_init(void)
 
 qemu_cond_init(&queue->cond);
 qemu_mutex_init(&queue->mutex);
+buffer_init(&queue->buffer, "vnc-job-queue");
 QTAILQ_INIT(&queue->jobs);
 return queue;
 }
diff --git a/ui/vnc.c b/ui/vnc.c
index dd4f581..49e8f3a 100644
--- a/ui/vnc.c
+++ b/ui/vnc.c
@@ -2978,6 +2978,26 @@ static void vnc_connect(VncDisplay *vd, int csock,
 vs->csock = csock;
 vs->vd = vd;
 
+buffer_init(&vs->input,  "vnc-input/%d", csock);
+buffer_init(&vs->output, "vnc-output/%d", csock);
+buffer_init(&vs->ws_input,   "vnc-ws_input/%d", csock);
+buffer_init(&vs->ws_output,  "vnc-ws_output/%d", csock);
+buffer_init(&vs->jobs_buffer,"vnc-jobs_buffer/%d", csock);
+
+buffer_init(&vs->tight.tight,"vnc-tight/%d", csock);
+buffer_init(&vs->tight.zlib, "vnc-tight-zlib/%d", csock);
+buffer_init(&vs->tight.gradient, "vnc-tight-gradient/%d", csock);
+#ifdef CONFIG_VNC_JPEG
+buffer_init(&vs->tight.jpeg, "vnc-tight-jpeg/%d", csock);
+#endif
+#ifdef CONFIG_VNC_PNG
+buffer_init(&vs->tight.png,  "vnc-tight-png/%d", csock);
+#endif
+buffer_init(&vs->zlib.zlib,  "vnc-zlib/%d", csock);
+buffer_init(&vs->zrle.zrle,  "vnc-zrle/%d", csock);
+buffer_init(&vs->zrle.fb,"vnc-zrle-fb/%d", csock);
+buffer_init(&vs->zrle.zlib,  "vnc-zrle-zlib/%d", csock);
+
 if (skipauth) {
vs->auth = VNC_AUTH_NONE;
vs->subauth = VNC_AUTH_INVALID;
-- 
1.8.3.1

[Qemu-devel] [PATCH 13/19] vnc: use vnc_{width, height} in vnc_set_area_dirty

[Gerd Hoffmann]

Signed-off-by: Gerd Hoffmann 
---
 ui/vnc.c | 21 ++---
 1 file changed, 10 insertions(+), 11 deletions(-)

diff --git a/ui/vnc.c b/ui/vnc.c
index 61a8f2c..8ee1266 100644
--- a/ui/vnc.c
+++ b/ui/vnc.c
@@ -618,8 +618,12 @@ static int vnc_height(VncDisplay *vd)
 
 static void vnc_set_area_dirty(DECLARE_BITMAP(dirty[VNC_MAX_HEIGHT],
VNC_MAX_WIDTH / VNC_DIRTY_PIXELS_PER_BIT),
-   int width, int height,
-   int x, int y, int w, int h) {
+   VncDisplay *vd,
+   int x, int y, int w, int h)
+{
+int width = vnc_width(vd);
+int height = vnc_height(vd);
+
 /* this is needed this to ensure we updated all affected
  * blocks if x % VNC_DIRTY_PIXELS_PER_BIT != 0 */
 w += (x % VNC_DIRTY_PIXELS_PER_BIT);
@@ -641,10 +645,8 @@ static void vnc_dpy_update(DisplayChangeListener *dcl,
 {
 VncDisplay *vd = container_of(dcl, VncDisplay, dcl);
 struct VncSurface *s = &vd->guest;
-int width = pixman_image_get_width(vd->server);
-int height = pixman_image_get_height(vd->server);
 
-vnc_set_area_dirty(s->dirty, width, height, x, y, w, h);
+vnc_set_area_dirty(s->dirty, vd, x, y, w, h);
 }
 
 void vnc_framebuffer_update(VncState *vs, int x, int y, int w, int h,
@@ -745,7 +747,7 @@ static void vnc_dpy_switch(DisplayChangeListener *dcl,
 width = vnc_width(vd);
 height = vnc_height(vd);
 memset(vd->guest.dirty, 0x00, sizeof(vd->guest.dirty));
-vnc_set_area_dirty(vd->guest.dirty, width, height, 0, 0,
+vnc_set_area_dirty(vd->guest.dirty, vd, 0, 0,
width, height);
 
 QTAILQ_FOREACH(vs, &vd->clients, next) {
@@ -755,7 +757,7 @@ static void vnc_dpy_switch(DisplayChangeListener *dcl,
 vnc_cursor_define(vs);
 }
 memset(vs->dirty, 0x00, sizeof(vs->dirty));
-vnc_set_area_dirty(vs->dirty, width, height, 0, 0,
+vnc_set_area_dirty(vs->dirty, vd, 0, 0,
width, height);
 }
 }
@@ -2011,9 +2013,6 @@ static void ext_key_event(VncState *vs, int down,
 static void framebuffer_update_request(VncState *vs, int incremental,
int x, int y, int w, int h)
 {
-int width = pixman_image_get_width(vs->vd->server);
-int height = pixman_image_get_height(vs->vd->server);
-
 vs->need_update = 1;
 
 if (incremental) {
@@ -2021,7 +2020,7 @@ static void framebuffer_update_request(VncState *vs, int 
incremental,
 }
 
 vs->force_update = 1;
-vnc_set_area_dirty(vs->dirty, width, height, x, y, w, h);
+vnc_set_area_dirty(vs->dirty, vs->vd, x, y, w, h);
 }
 
 static void send_ext_key_event_ack(VncState *vs)
-- 
1.8.3.1

[Qemu-devel] [PATCH 15/19] vnc: fix local state init

[Gerd Hoffmann]

Signed-off-by: Gerd Hoffmann 
---
 ui/vnc-jobs.c | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/ui/vnc-jobs.c b/ui/vnc-jobs.c
index fd9ed39..12389cc 100644
--- a/ui/vnc-jobs.c
+++ b/ui/vnc-jobs.c
@@ -185,6 +185,9 @@ void vnc_jobs_consume_buffer(VncState *vs)
  */
 static void vnc_async_encoding_start(VncState *orig, VncState *local)
 {
+buffer_init(&local->output, "vnc-worker-output");
+local->csock = -1; /* Don't do any network work on this thread */
+
 local->vnc_encoding = orig->vnc_encoding;
 local->features = orig->features;
 local->vd = orig->vd;
@@ -196,7 +199,6 @@ static void vnc_async_encoding_start(VncState *orig, 
VncState *local)
 local->zlib = orig->zlib;
 local->hextile = orig->hextile;
 local->zrle = orig->zrle;
-local->csock = -1; /* Don't do any network work on this thread */
 }
 
 static void vnc_async_encoding_end(VncState *orig, VncState *local)
@@ -212,12 +214,10 @@ static int vnc_worker_thread_loop(VncJobQueue *queue)
 {
 VncJob *job;
 VncRectEntry *entry, *tmp;
-VncState vs;
+VncState vs = {};
 int n_rectangles;
 int saved_offset;
 
-buffer_init(&vs.output, "vnc-worker-output");
-
 vnc_lock_queue(queue);
 while (QTAILQ_EMPTY(&queue->jobs) && !queue->exit) {
 qemu_cond_wait(&queue->cond, &queue->mutex);
-- 
1.8.3.1

[Qemu-devel] [PATCH 10/19] vnc: zap dead code

[Gerd Hoffmann]

Signed-off-by: Gerd Hoffmann 
---
 ui/vnc.c | 4 
 1 file changed, 4 deletions(-)

diff --git a/ui/vnc.c b/ui/vnc.c
index 49e8f3a..cb1954c 100644
--- a/ui/vnc.c
+++ b/ui/vnc.c
@@ -722,10 +722,6 @@ static void vnc_dpy_switch(DisplayChangeListener *dcl,
   width, height, NULL, 0);
 
 /* guest surface */
-#if 0 /* FIXME */
-if (ds_get_bytes_per_pixel(ds) != vd->guest.ds->pf.bytes_per_pixel)
-console_color_init(ds);
-#endif
 qemu_pixman_image_unref(vd->guest.fb);
 vd->guest.fb = pixman_image_ref(surface->image);
 vd->guest.format = surface->format;
-- 
1.8.3.1

[Qemu-devel] [PATCH 17/19] buffer: factor out buffer_req_size

[Gerd Hoffmann]

From: Peter Lieven 

Signed-off-by: Peter Lieven 
Signed-off-by: Gerd Hoffmann 
---
 util/buffer.c | 10 --
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/util/buffer.c b/util/buffer.c
index ae2907e..31f1d9f 100644
--- a/util/buffer.c
+++ b/util/buffer.c
@@ -24,6 +24,13 @@
 #define BUFFER_MIN_INIT_SIZE 4096
 #define BUFFER_MIN_SHRINK_SIZE  65536
 
+static size_t buffer_req_size(Buffer *buffer, size_t len)
+{
+return MAX(BUFFER_MIN_INIT_SIZE,
+   pow2ceil(buffer->offset + len));
+}
+
+
 void buffer_init(Buffer *buffer, const char *name, ...)
 {
 va_list ap;
@@ -61,8 +68,7 @@ void buffer_reserve(Buffer *buffer, size_t len)
 
 if ((buffer->capacity - buffer->offset) < len) {
 old = buffer->capacity;
-buffer->capacity = pow2ceil(buffer->offset + len);
-buffer->capacity = MAX(buffer->capacity, BUFFER_MIN_INIT_SIZE);
+buffer->capacity = buffer_req_size(buffer, len);
 buffer->buffer = g_realloc(buffer->buffer, buffer->capacity);
 trace_buffer_resize(buffer->name ?: "unnamed",
 old, buffer->capacity);
-- 
1.8.3.1

[Qemu-devel] [PATCH 06/19] buffer: add tracing

[Gerd Hoffmann]

Signed-off-by: Gerd Hoffmann 
---
 trace-events  |  6 ++
 util/buffer.c | 18 ++
 2 files changed, 24 insertions(+)

diff --git a/trace-events b/trace-events
index bdfe79f..050d45b 100644
--- a/trace-events
+++ b/trace-events
@@ -1387,6 +1387,12 @@ ppc_tb_adjust(uint64_t offs1, uint64_t offs2, int64_t 
diff, int64_t seconds) "ad
 prep_io_800_writeb(uint32_t addr, uint32_t val) "0x%08" PRIx32 " => 0x%02" 
PRIx32
 prep_io_800_readb(uint32_t addr, uint32_t retval) "0x%08" PRIx32 " <= 0x%02" 
PRIx32
 
+# io/buffer.c
+buffer_resize(const char *buf, size_t olen, size_t len) "%s: old %zd, new %zd"
+buffer_move_empty(const char *buf, size_t len, const char *from) "%s: %zd 
bytes from %s"
+buffer_move(const char *buf, size_t len, const char *from) "%s: %zd bytes from 
%s"
+buffer_free(const char *buf, size_t len) "%s: capacity %zd"
+
 # util/hbitmap.c
 hbitmap_iter_skip_words(const void *hb, void *hbi, uint64_t pos, unsigned long 
cur) "hb %p hbi %p pos %"PRId64" cur 0x%lx"
 hbitmap_reset(void *hb, uint64_t start, uint64_t count, uint64_t sbit, 
uint64_t ebit) "hb %p items %"PRIu64",%"PRIu64" bits %"PRIu64"..%"PRIu64
diff --git a/util/buffer.c b/util/buffer.c
index 234e33d..ae2907e 100644
--- a/util/buffer.c
+++ b/util/buffer.c
@@ -19,6 +19,7 @@
  */
 
 #include "qemu/buffer.h"
+#include "trace.h"
 
 #define BUFFER_MIN_INIT_SIZE 4096
 #define BUFFER_MIN_SHRINK_SIZE  65536
@@ -34,6 +35,8 @@ void buffer_init(Buffer *buffer, const char *name, ...)
 
 void buffer_shrink(Buffer *buffer)
 {
+size_t old;
+
 /*
  * Only shrink in case the used size is *much* smaller than the
  * capacity, to avoid bumping up & down the buffers all the time.
@@ -44,17 +47,25 @@ void buffer_shrink(Buffer *buffer)
 return;
 }
 
+old = buffer->capacity;
 buffer->capacity = pow2ceil(buffer->offset);
 buffer->capacity = MAX(buffer->capacity, BUFFER_MIN_SHRINK_SIZE);
 buffer->buffer = g_realloc(buffer->buffer, buffer->capacity);
+trace_buffer_resize(buffer->name ?: "unnamed",
+old, buffer->capacity);
 }
 
 void buffer_reserve(Buffer *buffer, size_t len)
 {
+size_t old;
+
 if ((buffer->capacity - buffer->offset) < len) {
+old = buffer->capacity;
 buffer->capacity = pow2ceil(buffer->offset + len);
 buffer->capacity = MAX(buffer->capacity, BUFFER_MIN_INIT_SIZE);
 buffer->buffer = g_realloc(buffer->buffer, buffer->capacity);
+trace_buffer_resize(buffer->name ?: "unnamed",
+old, buffer->capacity);
 }
 }
 
@@ -75,6 +86,7 @@ void buffer_reset(Buffer *buffer)
 
 void buffer_free(Buffer *buffer)
 {
+trace_buffer_free(buffer->name ?: "unnamed", buffer->capacity);
 g_free(buffer->buffer);
 g_free(buffer->name);
 buffer->offset = 0;
@@ -98,6 +110,9 @@ void buffer_advance(Buffer *buffer, size_t len)
 
 void buffer_move_empty(Buffer *to, Buffer *from)
 {
+trace_buffer_move_empty(to->name ?: "unnamed",
+from->offset,
+from->name ?: "unnamed");
 assert(to->offset == 0);
 
 g_free(to->buffer);
@@ -117,6 +132,9 @@ void buffer_move(Buffer *to, Buffer *from)
 return;
 }
 
+trace_buffer_move(to->name ?: "unnamed",
+  from->offset,
+  from->name ?: "unnamed");
 buffer_reserve(to, from->offset);
 buffer_append(to, from->buffer, from->offset);
 
-- 
1.8.3.1

[Qemu-devel] [PATCH 18/19] buffer: factor out buffer_adj_size

[Gerd Hoffmann]

From: Peter Lieven 

Signed-off-by: Peter Lieven 
Signed-off-by: Gerd Hoffmann 
---
 util/buffer.c | 25 ++---
 1 file changed, 10 insertions(+), 15 deletions(-)

diff --git a/util/buffer.c b/util/buffer.c
index 31f1d9f..fe5a44e 100644
--- a/util/buffer.c
+++ b/util/buffer.c
@@ -30,6 +30,14 @@ static size_t buffer_req_size(Buffer *buffer, size_t len)
pow2ceil(buffer->offset + len));
 }
 
+static void buffer_adj_size(Buffer *buffer, size_t len)
+{
+size_t old = buffer->capacity;
+buffer->capacity = buffer_req_size(buffer, len);
+buffer->buffer = g_realloc(buffer->buffer, buffer->capacity);
+trace_buffer_resize(buffer->name ?: "unnamed",
+old, buffer->capacity);
+}
 
 void buffer_init(Buffer *buffer, const char *name, ...)
 {
@@ -42,8 +50,6 @@ void buffer_init(Buffer *buffer, const char *name, ...)
 
 void buffer_shrink(Buffer *buffer)
 {
-size_t old;
-
 /*
  * Only shrink in case the used size is *much* smaller than the
  * capacity, to avoid bumping up & down the buffers all the time.
@@ -54,24 +60,13 @@ void buffer_shrink(Buffer *buffer)
 return;
 }
 
-old = buffer->capacity;
-buffer->capacity = pow2ceil(buffer->offset);
-buffer->capacity = MAX(buffer->capacity, BUFFER_MIN_SHRINK_SIZE);
-buffer->buffer = g_realloc(buffer->buffer, buffer->capacity);
-trace_buffer_resize(buffer->name ?: "unnamed",
-old, buffer->capacity);
+buffer_adj_size(buffer, 0);
 }
 
 void buffer_reserve(Buffer *buffer, size_t len)
 {
-size_t old;
-
 if ((buffer->capacity - buffer->offset) < len) {
-old = buffer->capacity;
-buffer->capacity = buffer_req_size(buffer, len);
-buffer->buffer = g_realloc(buffer->buffer, buffer->capacity);
-trace_buffer_resize(buffer->name ?: "unnamed",
-old, buffer->capacity);
+buffer_adj_size(buffer, len);
 }
 }
 
-- 
1.8.3.1

[Qemu-devel] [PATCH 16/19] vnc: recycle empty vs->output buffer

[Gerd Hoffmann]

From: Peter Lieven 

If the vs->output buffer is empty it will be dropped
by the next qio_buffer_move_empty in vnc_jobs_consume_buffer
anyway. So reuse the allocated buffer from this buffer
in the worker thread where we otherwise would start with
an empty (unallocated buffer).

Signed-off-by: Peter Lieven 

[ added a comment describing the non-obvious optimization ]

Signed-off-by: Gerd Hoffmann 
---
 ui/vnc-jobs.c | 8 
 1 file changed, 8 insertions(+)

diff --git a/ui/vnc-jobs.c b/ui/vnc-jobs.c
index 12389cc..08f0163 100644
--- a/ui/vnc-jobs.c
+++ b/ui/vnc-jobs.c
@@ -235,6 +235,14 @@ static int vnc_worker_thread_loop(VncJobQueue *queue)
 vnc_unlock_output(job->vs);
 goto disconnected;
 }
+if (buffer_empty(&job->vs->output)) {
+/*
+ * Looks like a NOP as it obviously moves no data.  But it
+ * moves the empty buffer, so we don't have to malloc a new
+ * one for vs.output
+ */
+buffer_move_empty(&vs.output, &job->vs->output);
+}
 vnc_unlock_output(job->vs);
 
 /* Make a local copy of vs and switch output buffers */
-- 
1.8.3.1

[Qemu-devel] [PATCH 12/19] vnc: factor out vnc_update_server_surface

[Gerd Hoffmann]

Signed-off-by: Gerd Hoffmann 
---
 ui/vnc.c | 23 ---
 1 file changed, 16 insertions(+), 7 deletions(-)

diff --git a/ui/vnc.c b/ui/vnc.c
index 283b93c..61a8f2c 100644
--- a/ui/vnc.c
+++ b/ui/vnc.c
@@ -714,6 +714,17 @@ void *vnc_server_fb_ptr(VncDisplay *vd, int x, int y)
 return ptr;
 }
 
+static void vnc_update_server_surface(VncDisplay *vd)
+{
+qemu_pixman_image_unref(vd->server);
+vd->server = NULL;
+
+vd->server = pixman_image_create_bits(VNC_SERVER_FB_FORMAT,
+  vnc_width(vd),
+  vnc_height(vd),
+  NULL, 0);
+}
+
 static void vnc_dpy_switch(DisplayChangeListener *dcl,
DisplaySurface *surface)
 {
@@ -722,19 +733,17 @@ static void vnc_dpy_switch(DisplayChangeListener *dcl,
 int width, height;
 
 vnc_abort_display_jobs(vd);
-
-/* server surface */
-qemu_pixman_image_unref(vd->server);
 vd->ds = surface;
-width = vnc_width(vd);
-height = vnc_height(vd);
-vd->server = pixman_image_create_bits(VNC_SERVER_FB_FORMAT,
-  width, height, NULL, 0);
+
+/* server surface */
+vnc_update_server_surface(vd);
 
 /* guest surface */
 qemu_pixman_image_unref(vd->guest.fb);
 vd->guest.fb = pixman_image_ref(surface->image);
 vd->guest.format = surface->format;
+width = vnc_width(vd);
+height = vnc_height(vd);
 memset(vd->guest.dirty, 0x00, sizeof(vd->guest.dirty));
 vnc_set_area_dirty(vd->guest.dirty, width, height, 0, 0,
width, height);
-- 
1.8.3.1

[Qemu-devel] [PATCH 00/19] buffer/vnc: improve vnc buffer hsndling

[Gerd Hoffmann]

  Hi,

This series has a bunch of improvements in the vnc buffer handling,
to reduce the memory footprint.  Some of the changes are applied to
the buffer helper functions which Daniel separated out of the vnc code
recently.

Most patches have been on the list before, based on a older version of
Daniel's "separate out buffer code" patches.  Now I finally managed to
rebase and adapt the changes to the latest version which landed in
master meanwhile.  I don't expect major issues showing up here and plan
to have a pull request with this in time for 2.5-rc0.

Peter, if you have anything pending not yet in here please rebase and
resend.

please review,
  Gerd

Gerd Hoffmann (14):
  buffer: add buffer_init
  buffer: add buffer_move_empty
  buffer: add buffer_move
  buffer: add buffer_shrink
  buffer: add tracing
  vnc: attach names to buffers
  vnc: kill jobs queue buffer
  vnc-jobs: move buffer reset, use new buffer move
  vnc: zap dead code
  vnc: add vnc_width+vnc_height helpers
  vnc: factor out vnc_update_server_surface
  vnc: use vnc_{width,height} in vnc_set_area_dirty
  vnc: only alloc server surface with clients connected
  vnc: fix local state init

Peter Lieven (5):
  buffer: make the Buffer capacity increase in powers of two
  vnc: recycle empty vs->output buffer
  buffer: factor out buffer_req_size
  buffer: factor out buffer_adj_size
  buffer: allow a buffer to shrink gracefully

 include/qemu/buffer.h |  43 
 trace-events  |   6 +++
 ui/vnc-jobs.c |  34 +---
 ui/vnc.c  |  92 ---
 util/buffer.c | 107 +-
 5 files changed, 243 insertions(+), 39 deletions(-)

-- 
1.8.3.1

[Qemu-devel] [PATCH 11/19] vnc: add vnc_width+vnc_height helpers

[Gerd Hoffmann]

Signed-off-by: Gerd Hoffmann 
---
 ui/vnc.c | 16 +---
 1 file changed, 13 insertions(+), 3 deletions(-)

diff --git a/ui/vnc.c b/ui/vnc.c
index cb1954c..283b93c 100644
--- a/ui/vnc.c
+++ b/ui/vnc.c
@@ -605,6 +605,17 @@ static void framebuffer_update_request(VncState *vs, int 
incremental,
 static void vnc_refresh(DisplayChangeListener *dcl);
 static int vnc_refresh_server_surface(VncDisplay *vd);
 
+static int vnc_width(VncDisplay *vd)
+{
+return MIN(VNC_MAX_WIDTH, ROUND_UP(surface_width(vd->ds),
+   VNC_DIRTY_PIXELS_PER_BIT));
+}
+
+static int vnc_height(VncDisplay *vd)
+{
+return MIN(VNC_MAX_HEIGHT, surface_height(vd->ds));
+}
+
 static void vnc_set_area_dirty(DECLARE_BITMAP(dirty[VNC_MAX_HEIGHT],
VNC_MAX_WIDTH / VNC_DIRTY_PIXELS_PER_BIT),
int width, int height,
@@ -715,9 +726,8 @@ static void vnc_dpy_switch(DisplayChangeListener *dcl,
 /* server surface */
 qemu_pixman_image_unref(vd->server);
 vd->ds = surface;
-width = MIN(VNC_MAX_WIDTH, ROUND_UP(surface_width(vd->ds),
-VNC_DIRTY_PIXELS_PER_BIT));
-height = MIN(VNC_MAX_HEIGHT, surface_height(vd->ds));
+width = vnc_width(vd);
+height = vnc_height(vd);
 vd->server = pixman_image_create_bits(VNC_SERVER_FB_FORMAT,
   width, height, NULL, 0);
 
-- 
1.8.3.1

[Qemu-devel] [PATCH 09/19] vnc-jobs: move buffer reset, use new buffer move

[Gerd Hoffmann]

Signed-off-by: Gerd Hoffmann 
---
 ui/vnc-jobs.c | 15 ---
 1 file changed, 8 insertions(+), 7 deletions(-)

diff --git a/ui/vnc-jobs.c b/ui/vnc-jobs.c
index 329d13e..fd9ed39 100644
--- a/ui/vnc-jobs.c
+++ b/ui/vnc-jobs.c
@@ -29,6 +29,7 @@
 #include "vnc.h"
 #include "vnc-jobs.h"
 #include "qemu/sockets.h"
+#include "qemu/main-loop.h"
 #include "block/aio.h"
 
 /*
@@ -165,8 +166,11 @@ void vnc_jobs_consume_buffer(VncState *vs)
 
 vnc_lock_output(vs);
 if (vs->jobs_buffer.offset) {
-vnc_write(vs, vs->jobs_buffer.buffer, vs->jobs_buffer.offset);
-buffer_reset(&vs->jobs_buffer);
+if (vs->csock != -1 && buffer_empty(&vs->output)) {
+qemu_set_fd_handler(vs->csock, vnc_client_read,
+vnc_client_write, vs);
+}
+buffer_move(&vs->output, &vs->jobs_buffer);
 }
 flush = vs->csock != -1 && vs->abort != true;
 vnc_unlock_output(vs);
@@ -193,8 +197,6 @@ static void vnc_async_encoding_start(VncState *orig, 
VncState *local)
 local->hextile = orig->hextile;
 local->zrle = orig->zrle;
 local->csock = -1; /* Don't do any network work on this thread */
-
-buffer_reset(&local->output);
 }
 
 static void vnc_async_encoding_end(VncState *orig, VncState *local)
@@ -272,14 +274,13 @@ static int vnc_worker_thread_loop(VncJobQueue *queue)
 
 vnc_lock_output(job->vs);
 if (job->vs->csock != -1) {
-buffer_reserve(&job->vs->jobs_buffer, vs.output.offset);
-buffer_append(&job->vs->jobs_buffer, vs.output.buffer,
-  vs.output.offset);
+buffer_move(&job->vs->jobs_buffer, &vs.output);
 /* Copy persistent encoding data */
 vnc_async_encoding_end(job->vs, &vs);
 
qemu_bh_schedule(job->vs->bh);
 }  else {
+buffer_reset(&vs.output);
 /* Copy persistent encoding data */
 vnc_async_encoding_end(job->vs, &vs);
 }
-- 
1.8.3.1

[Qemu-devel] [PATCH 19/19] buffer: allow a buffer to shrink gracefully

[Gerd Hoffmann]

From: Peter Lieven 

the idea behind this patch is to allow the buffer to shrink, but
make this a seldom operation. The buffers average size is measured
exponentionally smoothed with am alpha of 1/128.

Signed-off-by: Peter Lieven 
Signed-off-by: Gerd Hoffmann 
---
 include/qemu/buffer.h |  1 +
 util/buffer.c | 34 --
 2 files changed, 29 insertions(+), 6 deletions(-)

diff --git a/include/qemu/buffer.h b/include/qemu/buffer.h
index 0a69b3a..dead9b7 100644
--- a/include/qemu/buffer.h
+++ b/include/qemu/buffer.h
@@ -37,6 +37,7 @@ struct Buffer {
 char *name;
 size_t capacity;
 size_t offset;
+uint64_t avg_size;
 uint8_t *buffer;
 };
 
diff --git a/util/buffer.c b/util/buffer.c
index fe5a44e..5461f86 100644
--- a/util/buffer.c
+++ b/util/buffer.c
@@ -23,6 +23,7 @@
 
 #define BUFFER_MIN_INIT_SIZE 4096
 #define BUFFER_MIN_SHRINK_SIZE  65536
+#define BUFFER_AVG_SIZE_SHIFT   7
 
 static size_t buffer_req_size(Buffer *buffer, size_t len)
 {
@@ -37,6 +38,11 @@ static void buffer_adj_size(Buffer *buffer, size_t len)
 buffer->buffer = g_realloc(buffer->buffer, buffer->capacity);
 trace_buffer_resize(buffer->name ?: "unnamed",
 old, buffer->capacity);
+
+/* make it even harder for the buffer to shrink, reset average size
+ * to currenty capacity if it is larger than the average. */
+buffer->avg_size = MAX(buffer->avg_size,
+   buffer->capacity << BUFFER_AVG_SIZE_SHIFT);
 }
 
 void buffer_init(Buffer *buffer, const char *name, ...)
@@ -48,16 +54,30 @@ void buffer_init(Buffer *buffer, const char *name, ...)
 va_end(ap);
 }
 
+static uint64_t buffer_get_avg_size(Buffer *buffer)
+{
+return buffer->avg_size >> BUFFER_AVG_SIZE_SHIFT;
+}
+
 void buffer_shrink(Buffer *buffer)
 {
-/*
- * Only shrink in case the used size is *much* smaller than the
- * capacity, to avoid bumping up & down the buffers all the time.
+size_t new;
+
+/* Calculate the average size of the buffer as
+ * avg_size = avg_size * ( 1 - a ) + required_size * a
+ * where a is 1 / 2 ^ QIO_BUFFER_AVG_SIZE_SHIFT. */
+buffer->avg_size *= (1 << BUFFER_AVG_SIZE_SHIFT) - 1;
+buffer->avg_size >>= BUFFER_AVG_SIZE_SHIFT;
+buffer->avg_size += buffer_req_size(buffer, 0);
+
+/* And then only shrink if the average size of the buffer is much
+ * too big, to avoid bumping up & down the buffers all the time.
  * realloc() isn't exactly cheap ...
  */
-if (buffer->offset < (buffer->capacity >> 3) &&
-buffer->capacity > BUFFER_MIN_SHRINK_SIZE) {
-return;
+new = buffer_req_size(buffer, buffer_get_avg_size(buffer));
+if (new < buffer->capacity >> 3 &&
+new >= BUFFER_MIN_SHRINK_SIZE) {
+buffer_adj_size(buffer, buffer_get_avg_size(buffer));
 }
 
 buffer_adj_size(buffer, 0);
@@ -83,6 +103,7 @@ uint8_t *buffer_end(Buffer *buffer)
 void buffer_reset(Buffer *buffer)
 {
 buffer->offset = 0;
+buffer_shrink(buffer);
 }
 
 void buffer_free(Buffer *buffer)
@@ -107,6 +128,7 @@ void buffer_advance(Buffer *buffer, size_t len)
 memmove(buffer->buffer, buffer->buffer + len,
 (buffer->offset - len));
 buffer->offset -= len;
+buffer_shrink(buffer);
 }
 
 void buffer_move_empty(Buffer *to, Buffer *from)
-- 
1.8.3.1

Re: [Qemu-devel] [PATCH v2 0/4] ui audio qxl usb: Use g_new() & friends where that makes obvious sense

[Gerd Hoffmann]

On Do, 2015-10-29 at 16:55 +0100, Markus Armbruster wrote:
> v2:
> * Trivially rebased
> 
> Markus Armbruster (4):
>   ui: Use g_new() & friends where that makes obvious sense
>   audio: Use g_new() & friends where that makes obvious sense
>   qxl: Use g_new() & friends where that makes obvious sense
>   usb: Use g_new() & friends where that makes obvious sense

I'd prefer to drop the audio one because there still is the gsoc patch
series pending which touches all this anyway.

Otherwise looks fine to me.

Do you want me pick this up or send a pull request yourself?

On case of the latter:

Reviewed-by: Gerd Hoffmann 

cheers,
  Gerd

Re: [Qemu-devel] [PATCH v8 04/17] qapi-introspect: Guarantee particular sorting

[Markus Armbruster]

For now, only high-level review.

The main cost of sorting is interface complexity: we need to specify
which things are sorted, and what the sorting order is (see your TODO
below).  Once we've done so, we can't go back.

There's also implementation complexity, but your patch shows it's low
enough to be ignored.

Cost needs be justified by benefits.  Let's have a look at the benefits.

Eric Blake  writes:

> Sorting the values of an enum makes it easier to look up whether
> a particular value is present, via binary rather than linear search
> (probably most visible with QKeyCode, which has grown over
> several releases).

Making the interface guarantee "sorted" saves a client that wants it
sorted (say for binary search) the trouble of sorting itself.  We sort
at QEMU compile time instead of client run time.

My qmp-introspect.c has 48 SchemaInfoEnum, ranging from one to 125
members.  Histogram:

 #enums with this #members
  6 1
 11 2
  8 3
  9 4
  2 5
  1 6
  3 7
  1 8
  2 9
  1 15
  1 19
  1 27
  1 43
  1 125

Mean is less than eight.  Median is *three*.

Sorting these member arrays in the client won't have a noticeable
performance impact.  Heck, I suspect even using linear search wouldn't!
Therefore, we can't really claim client performance as a benefit.

We might be able to claim reduced client complexity as a benefit, but
I'm rather skeptical.  When your search space has a dozen members, you
better stick to simple search techniques.  Linear search in array or
list and binary search in array look adequate, hash table already
approaches overkill, and anything fancier definitely is.  Of these, only
binary search profits from a "sorted" guarantee.  But when you got data
in an array already, all it takes to sort it is a call to qsort() and a
comparison function.  Ten straightforward lines of code, tops.  Less in
a language that isn't quite as primitive as C.  Not much of a benefit,
I'm afraid.

> Additionally, QMP clients need not know which
> C value is associated with an enum name, so sorting is an
> effective way to hide that non-ABI aspect of qapi.

Sorting indeed hides the implementation detail of how enumerations are
encoded in the server.  However, I can't see what would tempt clients
into relying on it.  I can see for type names, and that's why we hide
them (commit 1a9a507).

One more potential benefit: when the schema changes in a way that
affects only order in introspection, sorting can hide the changes from
clients.  Can't think of a practical use of it, though.

> While we are at it, it is also easy to sort the members and
> variants of objects, to allow for a similar binary search (although
> less compelling, as any struct with that many members should
> arguably be broken into hierarchical substructs), and equally
> valid since JSON objects have no specific order in which keys must
> appear.

My qmp-introspect.c has 48 SchemaInfoObject, ranging from zero to 27
members.  Histogram:

  #objs with this #members
  3 0
102 2
 58 3
 28 4
 14 5
 17 6
 15 7
  3 8
  2 9
  7 10
  2 11
  3 12
  1 13
  2 14
  1 15
  1 16
  1 27

Mean is 9.5, median is 9.

The variants are also enums, and therefore can't be any worse than
enums.

Again, client performance is hardly a benefit, and client complexity
isn't particularly convincing, either.

>  There is no trivial or obvious way to sort the types of
> an alternate, so that is left unchanged.

In the schema, an alternate's member has a name, a type and nothing
else, so that's what we could use to sort.  The name isn't visible in
introspection, and the type is masked.  Sorting by either hides schema
change from the client, but isn't of much use to the client otherwise.

If you want to let the client use binary search without having to sort
itself, you obviously have to sort by masked type.

My qmp-introspect.c has *two* alternate types, each with two members.

> However, the overall SchemaInfo array remains unsorted.  It might
> make sense to sort with 'meta-type' as a primary key and 'name'
> as a secondary key, but it is not obvious that this will provide
> benefits to end-user clients (we allow mutually recursive types,
> so there is no posible topological sorting where a single pass
> over the array could resolve all types, and while binary searches
> could be made possible by sorting, it would be even more efficient
> for clients to read the array into a hashtable for O(1) rather
> than O(log n) random-access lookups, at which point pre-sorting is
> wasted effort).

My qmp-introspect.c has:

  2 alternate
 55 array
  5 builtin
126 command
 48 enum
 35 event
260 object
-
531 total

Since they all share the same entity name space, I'd use a single hash
table to map from name to SchemaInfo, just like qapi.py's QAPISchema
does.

Since we visit stuff in a defined orde

Re: [Qemu-devel] [PATCH v2 0/4] ui audio qxl usb: Use g_new() & friends where that makes obvious sense

[Gerd Hoffmann]

On Fr, 2015-10-30 at 12:13 +0100, Gerd Hoffmann wrote:
> Do you want me pick this up or send a pull request yourself?

Oh, I see you have trivial cc'ed.  scratch the question then ;)

Re: [Qemu-devel] [PATCH for-2.5 v2 2/4] mips: add Global Config Register block (part)

[James Hogan]

On Fri, Oct 30, 2015 at 12:36:07AM +, James Hogan wrote:
> Hi Yongbok,
> 
> On Tue, Oct 27, 2015 at 05:12:35PM +, Yongbok Kim wrote:
> > Add part of GCR Block which Linux Kernel utilises and it is enough
> > to bring the GIC up.
> > It defines full 32 Kbytes address space allocated for GCR but only few
> > registers are implemented such as config, revision, status, L2 config and
> > local config registers.
> > To support MIPS Coherent Manager, this module would be necessary to be 
> > extended
> 
> s/Coherent/Coherence/
> 
> > further.
> > 
> > Signed-off-by: Yongbok Kim 
> > ---
> >  default-configs/mips-softmmu.mak |1 +
> >  default-configs/mips64-softmmu.mak   |1 +
> >  default-configs/mips64el-softmmu.mak |1 +
> >  default-configs/mipsel-softmmu.mak   |1 +
> >  hw/misc/Makefile.objs|1 +
> >  hw/misc/mips_gcr.c   |  113 
> > ++
> >  include/hw/misc/mips_gcr.h   |   58 +
> 
> would these be better named mips_cmgcr.{h,c}?
> 
> >  7 files changed, 176 insertions(+), 0 deletions(-)
> >  create mode 100644 hw/misc/mips_gcr.c
> >  create mode 100644 include/hw/misc/mips_gcr.h
> > 
> > diff --git a/default-configs/mips-softmmu.mak 
> > b/default-configs/mips-softmmu.mak
> > index 44467c3..a784644 100644
> > --- a/default-configs/mips-softmmu.mak
> > +++ b/default-configs/mips-softmmu.mak
> > @@ -30,3 +30,4 @@ CONFIG_I8259=y
> >  CONFIG_MC146818RTC=y
> >  CONFIG_ISA_TESTDEV=y
> >  CONFIG_EMPTY_SLOT=y
> > +CONFIG_MIPS_GIC=y
> > diff --git a/default-configs/mips64-softmmu.mak 
> > b/default-configs/mips64-softmmu.mak
> > index 66ed5f9..957508d 100644
> > --- a/default-configs/mips64-softmmu.mak
> > +++ b/default-configs/mips64-softmmu.mak
> > @@ -36,3 +36,4 @@ CONFIG_JAZZ_LED=y
> >  CONFIG_MC146818RTC=y
> >  CONFIG_ISA_TESTDEV=y
> >  CONFIG_EMPTY_SLOT=y
> > +CONFIG_MIPS_GIC=y
> > diff --git a/default-configs/mips64el-softmmu.mak 
> > b/default-configs/mips64el-softmmu.mak
> > index bfca2b2..6c1065f 100644
> > --- a/default-configs/mips64el-softmmu.mak
> > +++ b/default-configs/mips64el-softmmu.mak
> > @@ -39,3 +39,4 @@ CONFIG_MC146818RTC=y
> >  CONFIG_VT82C686=y
> >  CONFIG_ISA_TESTDEV=y
> >  CONFIG_EMPTY_SLOT=y
> > +CONFIG_MIPS_GIC=y
> > diff --git a/default-configs/mipsel-softmmu.mak 
> > b/default-configs/mipsel-softmmu.mak
> > index 0162ef0..4633b0b 100644
> > --- a/default-configs/mipsel-softmmu.mak
> > +++ b/default-configs/mipsel-softmmu.mak
> > @@ -30,3 +30,4 @@ CONFIG_I8259=y
> >  CONFIG_MC146818RTC=y
> >  CONFIG_ISA_TESTDEV=y
> >  CONFIG_EMPTY_SLOT=y
> > +CONFIG_MIPS_GIC=y
> > diff --git a/hw/misc/Makefile.objs b/hw/misc/Makefile.objs
> > index 4aa76ff..02ac5bb 100644
> > --- a/hw/misc/Makefile.objs
> > +++ b/hw/misc/Makefile.objs
> > @@ -37,6 +37,7 @@ obj-$(CONFIG_OMAP) += omap_tap.o
> >  obj-$(CONFIG_SLAVIO) += slavio_misc.o
> >  obj-$(CONFIG_ZYNQ) += zynq_slcr.o
> >  obj-$(CONFIG_STM32F2XX_SYSCFG) += stm32f2xx_syscfg.o
> > +obj-$(CONFIG_MIPS_GIC) += mips_gcr.o
> 
> would CONFIG_MIPS_CMGCR be a better name?
> 
> >  
> >  obj-$(CONFIG_PVPANIC) += pvpanic.o
> >  obj-$(CONFIG_EDU) += edu.o
> > diff --git a/hw/misc/mips_gcr.c b/hw/misc/mips_gcr.c
> > new file mode 100644
> > index 000..6db4a9d
> > --- /dev/null
> > +++ b/hw/misc/mips_gcr.c
> > @@ -0,0 +1,113 @@
> > +/*
> > + * This file is subject to the terms and conditions of the GNU General 
> > Public
> > + * License.  See the file "COPYING" in the main directory of this archive
> > + * for more details.
> > + *
> > + * Copyright (C) 2012  MIPS Technologies, Inc.  All rights reserved.
> > + * Authors: Sanjay Lal 
> > + *
> > + * Copyright (C) 2015 Imagination Technologies
> > + */
> > +
> > +#include "hw/hw.h"
> > +#include "hw/sysbus.h"
> > +#include "sysemu/sysemu.h"
> > +#include "hw/misc/mips_gcr.h"
> > +#include "hw/intc/mips_gic.h"
> 
> I don't think this header exists yet at this point in the patchset.
> 
> > +
> > +/* Read GCR registers */
> > +static uint64_t gcr_read(void *opaque, hwaddr addr, unsigned size)
> > +{
> > +MIPSGCRState *gcr = (MIPSGCRState *) opaque;
> > +
> > +switch (addr) {
> > +/* Global Control Block Register */
> > +case GCR_CONFIG_OFS:
> > +/* Set PCORES to 0 */
> > +return 0;
> > +case GCR_BASE_OFS:
> > +return gcr->gcr_base;
> > +case GCR_REV_OFS:
> > +return gcr->gcr_rev;
> > +case GCR_GIC_BASE_OFS:
> > +return gcr->gic_base;

Note also, that this is a read-write register. It starts undefined and
the kernel will write the address it wants it to appear at. The GIC_EN
also resets to 0, and needs setting to 1 by software to enable the GIC
region.

Cheers
James

> > +case GCR_GIC_STATUS_OFS:
> > +return GCR_GIC_STATUS_GICEX_MSK;
> 
> well, it doesn't exist yet, does it?
> 
> > +case GCR_CPC_STATUS_OFS:
> > +return 0;
> > +case GCR_L2_CONFIG_OFS:
> > +/* L2 BYPASS */
> > +return GCR_L2_CONFI

Re: [Qemu-devel] [PATCH 01/19] buffer: make the Buffer capacity increase in powers of two

[Daniel P. Berrange]

On Fri, Oct 30, 2015 at 12:09:56PM +0100, Gerd Hoffmann wrote:
> From: Peter Lieven 
> 
> This makes sure the number of reallocs is in O(log N).
> 
> Signed-off-by: Peter Lieven 
> 
> [ rebased to util/buffer.c ]
> 
> Signed-off-by: Gerd Hoffmann 

Reviewed-by: Daniel P. Berrange 

Regards,
Daniel
-- 
|: http://berrange.com  -o-http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org  -o- http://virt-manager.org :|
|: http://autobuild.org   -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org   -o-   http://live.gnome.org/gtk-vnc :|

Re: [Qemu-devel] [PATCH for-2.5 v2 2/4] mips: add Global Config Register block (part)

[Peter Maydell]

On 30 October 2015 at 11:40, James Hogan  wrote:
> On Fri, Oct 30, 2015 at 12:36:07AM +, James Hogan wrote:
>> Hi Yongbok,
>>
>> > +case GCR_GIC_BASE_OFS:
>> > +return gcr->gic_base;
>
> Note also, that this is a read-write register. It starts undefined and
> the kernel will write the address it wants it to appear at. The GIC_EN
> also resets to 0, and needs setting to 1 by software to enable the GIC
> region.

That rather suggests that you want a 'container' type component
that has this GCR and the GIC and the CPUs in it, so that the GCR can
remap the GIC mmio regions suitably as this register is written.

thanks
-- PMM

Re: [Qemu-devel] [PATCH 02/19] buffer: add buffer_init

[Daniel P. Berrange]

On Fri, Oct 30, 2015 at 12:09:57PM +0100, Gerd Hoffmann wrote:
> Signed-off-by: Gerd Hoffmann 
> Reviewed-by: Peter Lieven 
> ---
>  include/qemu/buffer.h | 12 
>  util/buffer.c | 11 +++
>  2 files changed, 23 insertions(+)

Reviewed-by: Daniel P. Berrange 


Regards,
Daniel
-- 
|: http://berrange.com  -o-http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org  -o- http://virt-manager.org :|
|: http://autobuild.org   -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org   -o-   http://live.gnome.org/gtk-vnc :|

Re: [Qemu-devel] [PATCH 03/19] buffer: add buffer_move_empty

[Daniel P. Berrange]

On Fri, Oct 30, 2015 at 12:09:58PM +0100, Gerd Hoffmann wrote:
> Signed-off-by: Gerd Hoffmann 
> Reviewed-by: Peter Lieven 
> ---
>  include/qemu/buffer.h | 10 ++
>  util/buffer.c | 14 ++
>  2 files changed, 24 insertions(+)
> 
> diff --git a/include/qemu/buffer.h b/include/qemu/buffer.h
> index 0710e16..f53ee9e 100644
> --- a/include/qemu/buffer.h
> +++ b/include/qemu/buffer.h
> @@ -127,4 +127,14 @@ uint8_t *buffer_end(Buffer *buffer);
>   */
>  gboolean buffer_empty(Buffer *buffer);
>  
> +/**
> + * buffer_move_empty:
> + * @to: destination buffer object
> + * @from: source buffer object
> + *
> + * Moves buffer, without copying data.  'to' buffer must be empty.

Hmm, on the one hand the code do 'assert(to->offset) == 0', but on
the other hand it does 'g_free(to->buffer)' as if it expected the
buffer to have existing data.

If you just remove the 'assert', then there's no real requirement
for 'to' to be empty, as we'd do the right thin in free'ing the
data. Then we can just change this API doc to say

"Any existing data in "to" will be freed"

> + * 'from' buffer is empty and zero-sized on return.
> + */
> +void buffer_move_empty(Buffer *to, Buffer *from);
> +
>  #endif /* QEMU_BUFFER_H__ */
> diff --git a/util/buffer.c b/util/buffer.c
> index 12bf2d7..c7a39ec 100644
> --- a/util/buffer.c
> +++ b/util/buffer.c
> @@ -77,3 +77,17 @@ void buffer_advance(Buffer *buffer, size_t len)
>  (buffer->offset - len));
>  buffer->offset -= len;
>  }
> +
> +void buffer_move_empty(Buffer *to, Buffer *from)
> +{
> +assert(to->offset == 0);
> +
> +g_free(to->buffer);
> +to->offset = from->offset;
> +to->capacity = from->capacity;
> +to->buffer = from->buffer;
> +
> +from->offset = 0;
> +from->capacity = 0;
> +from->buffer = NULL;
> +}

Regards,
Daniel
-- 
|: http://berrange.com  -o-http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org  -o- http://virt-manager.org :|
|: http://autobuild.org   -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org   -o-   http://live.gnome.org/gtk-vnc :|

Re: [Qemu-devel] [PATCH 04/19] buffer: add buffer_move

[Daniel P. Berrange]

On Fri, Oct 30, 2015 at 12:09:59PM +0100, Gerd Hoffmann wrote:
> Signed-off-by: Gerd Hoffmann 
> Reviewed-by: Peter Lieven 
> ---
>  include/qemu/buffer.h | 10 ++
>  util/buffer.c | 16 
>  2 files changed, 26 insertions(+)

Reviewed-by: Daniel P. Berrange 


Regards,
Daniel
-- 
|: http://berrange.com  -o-http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org  -o- http://virt-manager.org :|
|: http://autobuild.org   -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org   -o-   http://live.gnome.org/gtk-vnc :|

Re: [Qemu-devel] [PATCH 05/19] buffer: add buffer_shrink

[Daniel P. Berrange]

On Fri, Oct 30, 2015 at 12:10:00PM +0100, Gerd Hoffmann wrote:
> Signed-off-by: Gerd Hoffmann 
> ---
>  include/qemu/buffer.h | 10 ++
>  util/buffer.c | 20 +++-
>  2 files changed, 29 insertions(+), 1 deletion(-)

Reviewed-by: Daniel P. Berrange 

Regards,
Daniel
-- 
|: http://berrange.com  -o-http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org  -o- http://virt-manager.org :|
|: http://autobuild.org   -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org   -o-   http://live.gnome.org/gtk-vnc :|

Re: [Qemu-devel] [PATCH 06/19] buffer: add tracing

[Daniel P. Berrange]

On Fri, Oct 30, 2015 at 12:10:01PM +0100, Gerd Hoffmann wrote:
> Signed-off-by: Gerd Hoffmann 
> ---
>  trace-events  |  6 ++
>  util/buffer.c | 18 ++
>  2 files changed, 24 insertions(+)

Reviewed-by: Daniel P. Berrange 


Regards,
Daniel
-- 
|: http://berrange.com  -o-http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org  -o- http://virt-manager.org :|
|: http://autobuild.org   -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org   -o-   http://live.gnome.org/gtk-vnc :|

Re: [Qemu-devel] [PATCH 07/19] vnc: attach names to buffers

[Daniel P. Berrange]

On Fri, Oct 30, 2015 at 12:10:02PM +0100, Gerd Hoffmann wrote:
> Signed-off-by: Gerd Hoffmann 
> ---
>  ui/vnc-jobs.c |  3 +++
>  ui/vnc.c  | 20 
>  2 files changed, 23 insertions(+)

Reviewed-by: Daniel P. Berrange 

Regards,
Daniel
-- 
|: http://berrange.com  -o-http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org  -o- http://virt-manager.org :|
|: http://autobuild.org   -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org   -o-   http://live.gnome.org/gtk-vnc :|

Re: [Qemu-devel] [PATCH 08/19] vnc: kill jobs queue buffer

[Daniel P. Berrange]

On Fri, Oct 30, 2015 at 12:10:03PM +0100, Gerd Hoffmann wrote:
> Signed-off-by: Gerd Hoffmann 
> Reviewed-by: Peter Lieven 
> ---
>  ui/vnc-jobs.c | 6 --
>  1 file changed, 6 deletions(-)

Reviewed-by: Daniel P. Berrange 

Regards,
Daniel
-- 
|: http://berrange.com  -o-http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org  -o- http://virt-manager.org :|
|: http://autobuild.org   -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org   -o-   http://live.gnome.org/gtk-vnc :|

Re: [Qemu-devel] [PATCH 09/19] vnc-jobs: move buffer reset, use new buffer move

[Daniel P. Berrange]

On Fri, Oct 30, 2015 at 12:10:04PM +0100, Gerd Hoffmann wrote:
> Signed-off-by: Gerd Hoffmann 
> ---
>  ui/vnc-jobs.c | 15 ---
>  1 file changed, 8 insertions(+), 7 deletions(-)

Reviewed-by: Daniel P. Berrange 

Regards,
Daniel
-- 
|: http://berrange.com  -o-http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org  -o- http://virt-manager.org :|
|: http://autobuild.org   -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org   -o-   http://live.gnome.org/gtk-vnc :|

Re: [Qemu-devel] [PATCH 10/19] vnc: zap dead code

[Daniel P. Berrange]

On Fri, Oct 30, 2015 at 12:10:05PM +0100, Gerd Hoffmann wrote:
> Signed-off-by: Gerd Hoffmann 
> ---
>  ui/vnc.c | 4 
>  1 file changed, 4 deletions(-)

Reviewed-by: Daniel P. Berrange 

Regards,
Daniel
-- 
|: http://berrange.com  -o-http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org  -o- http://virt-manager.org :|
|: http://autobuild.org   -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org   -o-   http://live.gnome.org/gtk-vnc :|

Re: [Qemu-devel] [PATCH 11/19] vnc: add vnc_width+vnc_height helpers

[Daniel P. Berrange]

On Fri, Oct 30, 2015 at 12:10:06PM +0100, Gerd Hoffmann wrote:
> Signed-off-by: Gerd Hoffmann 
> ---
>  ui/vnc.c | 16 +---
>  1 file changed, 13 insertions(+), 3 deletions(-)

Reviewed-by: Daniel P. Berrange 


Regards,
Daniel
-- 
|: http://berrange.com  -o-http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org  -o- http://virt-manager.org :|
|: http://autobuild.org   -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org   -o-   http://live.gnome.org/gtk-vnc :|

Re: [Qemu-devel] [PATCH 12/19] vnc: factor out vnc_update_server_surface

[Daniel P. Berrange]

On Fri, Oct 30, 2015 at 12:10:07PM +0100, Gerd Hoffmann wrote:
> Signed-off-by: Gerd Hoffmann 
> ---
>  ui/vnc.c | 23 ---
>  1 file changed, 16 insertions(+), 7 deletions(-)

Reviewed-by: Daniel P. Berrange 

Regards,
Daniel
-- 
|: http://berrange.com  -o-http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org  -o- http://virt-manager.org :|
|: http://autobuild.org   -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org   -o-   http://live.gnome.org/gtk-vnc :|

Re: [Qemu-devel] [PATCH 13/19] vnc: use vnc_{width, height} in vnc_set_area_dirty

[Daniel P. Berrange]

On Fri, Oct 30, 2015 at 12:10:08PM +0100, Gerd Hoffmann wrote:
> Signed-off-by: Gerd Hoffmann 
> ---
>  ui/vnc.c | 21 ++---
>  1 file changed, 10 insertions(+), 11 deletions(-)

Reviewed-by: Daniel P. Berrange 


Regards,
Daniel
-- 
|: http://berrange.com  -o-http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org  -o- http://virt-manager.org :|
|: http://autobuild.org   -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org   -o-   http://live.gnome.org/gtk-vnc :|

Re: [Qemu-devel] [PATCH 14/19] vnc: only alloc server surface with clients connected

[Daniel P. Berrange]

On Fri, Oct 30, 2015 at 12:10:09PM +0100, Gerd Hoffmann wrote:
> Signed-off-by: Gerd Hoffmann 
> ---
>  ui/vnc.c | 12 
>  1 file changed, 12 insertions(+)

Reviewed-by: Daniel P. Berrange 

Regards,
Daniel
-- 
|: http://berrange.com  -o-http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org  -o- http://virt-manager.org :|
|: http://autobuild.org   -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org   -o-   http://live.gnome.org/gtk-vnc :|

Re: [Qemu-devel] [PATCH 15/19] vnc: fix local state init

[Daniel P. Berrange]

On Fri, Oct 30, 2015 at 12:10:10PM +0100, Gerd Hoffmann wrote:

It isn't entirely obvious what bug is being fixed here, perhaps
a little more info in the commit message would help.

> Signed-off-by: Gerd Hoffmann 
> ---
>  ui/vnc-jobs.c | 8 
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/ui/vnc-jobs.c b/ui/vnc-jobs.c
> index fd9ed39..12389cc 100644
> --- a/ui/vnc-jobs.c
> +++ b/ui/vnc-jobs.c
> @@ -185,6 +185,9 @@ void vnc_jobs_consume_buffer(VncState *vs)
>   */
>  static void vnc_async_encoding_start(VncState *orig, VncState *local)
>  {
> +buffer_init(&local->output, "vnc-worker-output");
> +local->csock = -1; /* Don't do any network work on this thread */
> +
>  local->vnc_encoding = orig->vnc_encoding;
>  local->features = orig->features;
>  local->vd = orig->vd;
> @@ -196,7 +199,6 @@ static void vnc_async_encoding_start(VncState *orig, 
> VncState *local)
>  local->zlib = orig->zlib;
>  local->hextile = orig->hextile;
>  local->zrle = orig->zrle;
> -local->csock = -1; /* Don't do any network work on this thread */
>  }
>  
>  static void vnc_async_encoding_end(VncState *orig, VncState *local)
> @@ -212,12 +214,10 @@ static int vnc_worker_thread_loop(VncJobQueue *queue)
>  {
>  VncJob *job;
>  VncRectEntry *entry, *tmp;
> -VncState vs;
> +VncState vs = {};

Ok, so we'd not initialized memory in VncState to all zeros before.

>  int n_rectangles;
>  int saved_offset;
>  
> -buffer_init(&vs.output, "vnc-worker-output");
> -
>  vnc_lock_queue(queue);
>  while (QTAILQ_EMPTY(&queue->jobs) && !queue->exit) {
>  qemu_cond_wait(&queue->cond, &queue->mutex);

Reviewed-by: Daniel P. Berrange 

> -- 
> 1.8.3.1
> 

Regards,
Daniel
-- 
|: http://berrange.com  -o-http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org  -o- http://virt-manager.org :|
|: http://autobuild.org   -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org   -o-   http://live.gnome.org/gtk-vnc :|

Re: [Qemu-devel] [PATCH 16/19] vnc: recycle empty vs->output buffer

[Daniel P. Berrange]

On Fri, Oct 30, 2015 at 12:10:11PM +0100, Gerd Hoffmann wrote:
> From: Peter Lieven 
> 
> If the vs->output buffer is empty it will be dropped
> by the next qio_buffer_move_empty in vnc_jobs_consume_buffer
> anyway. So reuse the allocated buffer from this buffer
> in the worker thread where we otherwise would start with
> an empty (unallocated buffer).
> 
> Signed-off-by: Peter Lieven 
> 
> [ added a comment describing the non-obvious optimization ]
> 
> Signed-off-by: Gerd Hoffmann 
> ---
>  ui/vnc-jobs.c | 8 
>  1 file changed, 8 insertions(+)

Reviewed-by: Daniel P. Berrange 

Regards,
Daniel
-- 
|: http://berrange.com  -o-http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org  -o- http://virt-manager.org :|
|: http://autobuild.org   -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org   -o-   http://live.gnome.org/gtk-vnc :|

Re: [Qemu-devel] [PATCH 17/19] buffer: factor out buffer_req_size

[Daniel P. Berrange]

On Fri, Oct 30, 2015 at 12:10:12PM +0100, Gerd Hoffmann wrote:
> From: Peter Lieven 
> 
> Signed-off-by: Peter Lieven 
> Signed-off-by: Gerd Hoffmann 
> ---
>  util/buffer.c | 10 --
>  1 file changed, 8 insertions(+), 2 deletions(-)

Reviewed-by: Daniel P. Berrange 


Regards,
Daniel
-- 
|: http://berrange.com  -o-http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org  -o- http://virt-manager.org :|
|: http://autobuild.org   -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org   -o-   http://live.gnome.org/gtk-vnc :|

Re: [Qemu-devel] [PATCH 18/19] buffer: factor out buffer_adj_size

[Daniel P. Berrange]

On Fri, Oct 30, 2015 at 12:10:13PM +0100, Gerd Hoffmann wrote:
> From: Peter Lieven 
> 
> Signed-off-by: Peter Lieven 
> Signed-off-by: Gerd Hoffmann 
> ---
>  util/buffer.c | 25 ++---
>  1 file changed, 10 insertions(+), 15 deletions(-)

Reviewed-by: Daniel P. Berrange 

Regards,
Daniel
-- 
|: http://berrange.com  -o-http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org  -o- http://virt-manager.org :|
|: http://autobuild.org   -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org   -o-   http://live.gnome.org/gtk-vnc :|

Re: [Qemu-devel] [PATCH 19/19] buffer: allow a buffer to shrink gracefully

[Daniel P. Berrange]

On Fri, Oct 30, 2015 at 12:10:14PM +0100, Gerd Hoffmann wrote:
> From: Peter Lieven 
> 
> the idea behind this patch is to allow the buffer to shrink, but
> make this a seldom operation. The buffers average size is measured
> exponentionally smoothed with am alpha of 1/128.

s/am/an/

> 
> Signed-off-by: Peter Lieven 
> Signed-off-by: Gerd Hoffmann 
> ---
>  include/qemu/buffer.h |  1 +
>  util/buffer.c | 34 --
>  2 files changed, 29 insertions(+), 6 deletions(-)


> diff --git a/util/buffer.c b/util/buffer.c
> index fe5a44e..5461f86 100644
> --- a/util/buffer.c
> +++ b/util/buffer.c
> @@ -23,6 +23,7 @@
>  
>  #define BUFFER_MIN_INIT_SIZE 4096
>  #define BUFFER_MIN_SHRINK_SIZE  65536
> +#define BUFFER_AVG_SIZE_SHIFT   7
>  
>  static size_t buffer_req_size(Buffer *buffer, size_t len)
>  {
> @@ -37,6 +38,11 @@ static void buffer_adj_size(Buffer *buffer, size_t len)
>  buffer->buffer = g_realloc(buffer->buffer, buffer->capacity);
>  trace_buffer_resize(buffer->name ?: "unnamed",
>  old, buffer->capacity);
> +
> +/* make it even harder for the buffer to shrink, reset average size
> + * to currenty capacity if it is larger than the average. */

s/currenty/current/

Reviewed-by: Daniel P. Berrange 


Regards,
Daniel
-- 
|: http://berrange.com  -o-http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org  -o- http://virt-manager.org :|
|: http://autobuild.org   -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org   -o-   http://live.gnome.org/gtk-vnc :|

Re: [Qemu-devel] [PATCH 03/19] buffer: add buffer_move_empty

[Daniel P. Berrange]

On Fri, Oct 30, 2015 at 09:11:01PM +0900, Daniel P. Berrange wrote:
> On Fri, Oct 30, 2015 at 12:09:58PM +0100, Gerd Hoffmann wrote:
> > Signed-off-by: Gerd Hoffmann 
> > Reviewed-by: Peter Lieven 
> > ---
> >  include/qemu/buffer.h | 10 ++
> >  util/buffer.c | 14 ++
> >  2 files changed, 24 insertions(+)
> > 
> > diff --git a/include/qemu/buffer.h b/include/qemu/buffer.h
> > index 0710e16..f53ee9e 100644
> > --- a/include/qemu/buffer.h
> > +++ b/include/qemu/buffer.h
> > @@ -127,4 +127,14 @@ uint8_t *buffer_end(Buffer *buffer);
> >   */
> >  gboolean buffer_empty(Buffer *buffer);
> >  
> > +/**
> > + * buffer_move_empty:
> > + * @to: destination buffer object
> > + * @from: source buffer object
> > + *
> > + * Moves buffer, without copying data.  'to' buffer must be empty.
> 
> Hmm, on the one hand the code do 'assert(to->offset) == 0', but on
> the other hand it does 'g_free(to->buffer)' as if it expected the
> buffer to have existing data.
> 
> If you just remove the 'assert', then there's no real requirement
> for 'to' to be empty, as we'd do the right thin in free'ing the
> data. Then we can just change this API doc to say
> 
> "Any existing data in "to" will be freed"

Ignore this comment. I forget that we have an optimization to keep
buffer around when buffer_reset(), as distinct from buffer_free().

Reviewed-by: Daniel Berrange 

Regards,
Daniel
-- 
|: http://berrange.com  -o-http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org  -o- http://virt-manager.org :|
|: http://autobuild.org   -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org   -o-   http://live.gnome.org/gtk-vnc :|

Re: [Qemu-devel] [PATCH 00/19] buffer/vnc: improve vnc buffer hsndling

[Daniel P. Berrange]

On Fri, Oct 30, 2015 at 12:09:55PM +0100, Gerd Hoffmann wrote:
>   Hi,
> 
> This series has a bunch of improvements in the vnc buffer handling,
> to reduce the memory footprint.  Some of the changes are applied to
> the buffer helper functions which Daniel separated out of the vnc code
> recently.
> 
> Most patches have been on the list before, based on a older version of
> Daniel's "separate out buffer code" patches.  Now I finally managed to
> rebase and adapt the changes to the latest version which landed in
> master meanwhile.  I don't expect major issues showing up here and plan
> to have a pull request with this in time for 2.5-rc0.

I think this series looks good for a merge in 2.5

Regards,
Daniel
-- 
|: http://berrange.com  -o-http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org  -o- http://virt-manager.org :|
|: http://autobuild.org   -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org   -o-   http://live.gnome.org/gtk-vnc :|

Re: [Qemu-devel] [PATCH v8 04/54] Move configuration section writing

[Dr. David Alan Gilbert]

* Amit Shah (amit.s...@redhat.com) wrote:
> On (Tue) 29 Sep 2015 [09:37:28], Dr. David Alan Gilbert (git) wrote:
> > From: "Dr. David Alan Gilbert" 
> > 
> > The vmstate_configuration is currently written
> > in 'qemu_savevm_state_begin', move it to
> > 'qemu_savevm_state_header' since it's got a hard
> > requirement that it must be the 1st thing after
> > the header.
> > (In postcopy some 'command' sections get sent
> > early before the saving of the main sections
> > and hence before qemu_savevm_state_begin).
> > 
> > Signed-off-by: Dr. David Alan Gilbert 
> 
> Reviewed-by: Amit Shah 
> 
> The function name 'savevm_state_header()' isn't accurate anymore.  Not
> serious for this series.

Well, it does still write the header; but if you have a simple
better name, I'd be happy to change it.

Dave

> 
>   Amit
--
Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK

Re: [Qemu-devel] [PATCH v8 05/17] qapi: Track simple union tag in object.local_members

[Markus Armbruster]

Eric Blake  writes:

> We were previously creating all unions with an empty list for
> local_members.  However, it will make it easier to unify struct
> and union generation if we include the generated tag member in
> local_members.  That way, we can have a common code pattern:
> visit the base (if any), visit the local members (if any), visit
> the variants (if any).  The local_members of a flat union
> remains empty (because the discriminator is already visited as
> part of the base).

Keeping the implicit tag implicit by not including it in local_members
was a conscious design decision, but if including it makes unifying
struct and union into objects easier, go right ahead.

> The various front end entities then map as follows:
> struct: optional base, optional local_members, no variants
> simple union: no base, one-element local_members, variants with tag_member
>   from local_members
> flat union: base, no local_members, variants with tag_member from base
> alternate: no base, no local_members, variants
>
> With the new local members, we require a bit of finesse to
> avoid assertions in the clients.  No change to generated code.
>
> Signed-off-by: Eric Blake 
>
> ---
> v8: new patch
> ---
>  scripts/qapi-types.py  |  4 +++-
>  scripts/qapi-visit.py  |  4 +++-
>  scripts/qapi.py| 14 ++
>  tests/qapi-schema/qapi-schema-test.out |  2 ++
>  tests/qapi-schema/union-clash-data.out |  1 +
>  tests/qapi-schema/union-empty.out  |  1 +
>  6 files changed, 20 insertions(+), 6 deletions(-)
>
> diff --git a/scripts/qapi-types.py b/scripts/qapi-types.py
> index b37900f..a6bf95d 100644
> --- a/scripts/qapi-types.py
> +++ b/scripts/qapi-types.py
> @@ -269,7 +269,9 @@ class QAPISchemaGenTypeVisitor(QAPISchemaVisitor):
>  def visit_object_type(self, name, info, base, members, variants):
>  self._fwdecl += gen_fwd_object_or_array(name)
>  if variants:
> -assert not members  # not implemented
> +if members:
> +assert len(members) == 1
> +assert members[0] == variants.tag_member
>  self.decl += gen_union(name, base, variants)
>  else:
>  self.decl += gen_struct(name, base, members)

The assertion checks that not passing members to gen_union() won't lose
any.  Before the patch, we assert there are none.  After the patch, we
assert there's either none or variants.tag_member.  Before ans after,
gen_union() takes care of variants.tag_member.  Okay.

Let's keep the comment, though.  Perhaps like this:

# Members other than variants.tag_member not implemented
assert len(members) == 1
assert members[0] == variants.tag_member

I hope this the whole conditional will eventually be replaced by

   self.decl += gen_object(name, base, members, variants)

> diff --git a/scripts/qapi-visit.py b/scripts/qapi-visit.py
> index f40c3c7..318b8e6 100644
> --- a/scripts/qapi-visit.py
> +++ b/scripts/qapi-visit.py
> @@ -360,7 +360,9 @@ class QAPISchemaGenVisitVisitor(QAPISchemaVisitor):
>  def visit_object_type(self, name, info, base, members, variants):
>  self.decl += gen_visit_decl(name)
>  if variants:
> -assert not members  # not implemented
> +if members:
> +assert len(members) == 1
> +assert members[0] == variants.tag_member
>  self.defn += gen_visit_union(name, base, variants)
>  else:
>  self.defn += gen_visit_struct(name, base, members)

Likewise, let's keep the comment.

> diff --git a/scripts/qapi.py b/scripts/qapi.py
> index 7c50cc4..84ac151 100644
> --- a/scripts/qapi.py
> +++ b/scripts/qapi.py
> @@ -957,6 +957,9 @@ class QAPISchemaArrayType(QAPISchemaType):
>
>  class QAPISchemaObjectType(QAPISchemaType):
>  def __init__(self, name, info, base, local_members, variants):
> +# struct has local_members, optional base, and no variants
> +# flat union has base, variants, and no local_members
> +# simple union has local_members, variants, and no base
>  QAPISchemaType.__init__(self, name, info)
>  assert base is None or isinstance(base, str)
>  for m in local_members:
> @@ -1048,9 +1051,11 @@ class QAPISchemaObjectTypeVariants(object):
>  self.variants = variants
>
>  def check(self, schema, members, seen):
> -if self.tag_name:
> +if self.tag_name:# flat union
>  self.tag_member = seen[self.tag_name]
> -else:
> +elif seen:   # simple union
> +assert self.tag_member in seen.itervalues()
> +else:# alternate
>  self.tag_member.check(schema, members, seen)
>  assert isinstance(self.tag_member.type, QAPISchemaEnumType)
>  for v in self.variants:

The test for simple union is hackish.

I guess you want to bypass sel

Re: [Qemu-devel] [PATCH v8 42/54] Postcopy: Use helpers to map pages during migration

[Dr. David Alan Gilbert]

* Juan Quintela (quint...@redhat.com) wrote:
> "Dr. David Alan Gilbert (git)"  wrote:
> > From: "Dr. David Alan Gilbert" 
> >
> > In postcopy, the destination guest is running at the same time
> > as it's receiving pages; as we receive new pages we must put
> > them into the guests address space atomically to avoid a running
> > CPU accessing a partially written page.
> >
> > Use the helpers in postcopy-ram.c to map these pages.
> >
> > qemu_get_buffer_in_place is used to avoid a copy out of qemu_file
> > in the case that postcopy is going to do a copy anyway.
> >
> > Signed-off-by: Dr. David Alan Gilbert 
> > ---
> >  migration/ram.c | 128 
> > +---
> >  1 file changed, 103 insertions(+), 25 deletions(-)
> >
> > diff --git a/migration/ram.c b/migration/ram.c
> > index 487e838..6d9cfb5 100644
> > --- a/migration/ram.c
> > +++ b/migration/ram.c
> > @@ -1848,7 +1848,17 @@ static int load_xbzrle(QEMUFile *f, ram_addr_t addr, 
> > void *host)
> >  /* Must be called from within a rcu critical section.
> >   * Returns a pointer from within the RCU-protected ram_list.
> >   */
> > +/*
> > + * Read a RAMBlock ID from the stream f, find the host address of the
> > + * start of that block and add on 'offset'
> > + *
> > + * f: Stream to read from
> > + * mis: MigrationIncomingState
> > + * offset: Offset within the block
> > + * flags: Page flags (mostly to see if it's a continuation of previous 
> > block)
> > + */
> >  static inline void *host_from_stream_offset(QEMUFile *f,
> > +MigrationIncomingState *mis,
> >  ram_addr_t offset,
> >  int flags)
> >  {
> 
> 
> Uh, oh, we change the prototype of host_from_stream_offset() but not the
> function itself?  Strange, no?

Ah, that's a straggler from an old version of the patches that needed mis; gone.



> Hahahaha, just change the if or the variable name.
> 
> having a
> 
> if (!cond) {
>f1();
> } else {
>f2();
> }
> 
> makes no sense, better to have
> 
> if (cond) {
>f2()
> } else {
>f1()
> }
> no?

Done.

Dave

> 
> 
> 
> The patch itself is ok.
> 
> Thanks, Juan.
--
Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK

Re: [Qemu-devel] [kvm-unit-tests PATCHv5 3/3] arm: pmu: Add CPI checking

[Andrew Jones]

On Wed, Oct 28, 2015 at 03:12:55PM -0400, Christopher Covington wrote:
> Calculate the numbers of cycles per instruction (CPI) implied by ARM
> PMU cycle counter values. The code includes a strict checking facility
> intended for the -icount option in TCG mode but it is not yet enabled
> in the configuration file. Enabling it must wait on infrastructure
> improvements which allow for different tests to be run on TCG versus
> KVM.
> 
> Signed-off-by: Christopher Covington 
> ---
>  arm/pmu.c | 103 
> +-
>  1 file changed, 102 insertions(+), 1 deletion(-)
> 
> diff --git a/arm/pmu.c b/arm/pmu.c
> index 4334de4..76a 100644
> --- a/arm/pmu.c
> +++ b/arm/pmu.c
> @@ -43,6 +43,23 @@ static inline unsigned long get_pmccntr(void)
>   asm volatile("mrc p15, 0, %0, c9, c13, 0" : "=r" (cycles));
>   return cycles;
>  }
> +
> +/*
> + * Extra instructions inserted by the compiler would be difficult to 
> compensate
> + * for, so hand assemble everything between, and including, the PMCR accesses
> + * to start and stop counting.
> + */
> +static inline void loop(int i, uint32_t pmcr)
> +{
> + asm volatile(
> + "   mcr p15, 0, %[pmcr], c9, c12, 0\n"
> + "1: subs%[i], %[i], #1\n"
> + "   bgt 1b\n"
> + "   mcr p15, 0, %[z], c9, c12, 0\n"
> + : [i] "+r" (i)
> + : [pmcr] "r" (pmcr), [z] "r" (0)
> + : "cc");
> +}
>  #elif defined(__aarch64__)
>  static inline uint32_t get_pmcr(void)
>  {
> @@ -64,6 +81,23 @@ static inline unsigned long get_pmccntr(void)
>   asm volatile("mrs %0, pmccntr_el0" : "=r" (cycles));
>   return cycles;
>  }
> +
> +/*
> + * Extra instructions inserted by the compiler would be difficult to 
> compensate
> + * for, so hand assemble everything between, and including, the PMCR accesses
> + * to start and stop counting.
> + */
> +static inline void loop(int i, uint32_t pmcr)
> +{
> + asm volatile(
> + "   msr pmcr_el0, %[pmcr]\n"
> + "1: subs%[i], %[i], #1\n"
> + "   b.gt1b\n"
> + "   msr pmcr_el0, xzr\n"
> + : [i] "+r" (i)
> + : [pmcr] "r" (pmcr)
> + : "cc");
> +}
>  #endif
>  
>  struct pmu_data {
> @@ -131,12 +165,79 @@ static bool check_cycles_increase(void)
>   return true;
>  }
>  
> -int main(void)
> +/*
> + * Execute a known number of guest instructions. Only odd instruction counts
> + * greater than or equal to 3 are supported by the in-line assembly code. The
> + * control register (PMCR_EL0) is initialized with the provided value 
> (allowing
> + * for example for the cycle counter or event counters to be reset). At the 
> end
> + * of the exact instruction loop, zero is written to PMCR_EL0 to disable
> + * counting, allowing the cycle counter or event counters to be read at the
> + * leisure of the calling code.
> + */
> +static void measure_instrs(int num, uint32_t pmcr)
> +{
> + int i = (num - 1) / 2;
> +
> + assert(num >= 3 && ((num - 1) % 2 == 0));
> + loop(i, pmcr);
> +}
> +
> +/*
> + * Measure cycle counts for various known instruction counts. Ensure that the
> + * cycle counter progresses (similar to check_cycles_increase() but with more
> + * instructions and using reset and stop controls). If supplied a positive,
> + * nonzero CPI parameter, also strictly check that every measurement matches
> + * it. Strict CPI checking is used to test -icount mode.
> + */
> +static bool check_cpi(int cpi)
> +{
> + struct pmu_data pmu = {0};
> +
> + pmu.cycle_counter_reset = 1;
> + pmu.enable = 1;
> +
> + if (cpi > 0)
> + printf("Checking for CPI=%d.\n", cpi);
> + printf("instrs : cycles0 cycles1 ...\n");
> +
> + for (int i = 3; i < 300; i += 32) {
> + int avg, sum = 0;
> +
> + printf("%d :", i);
> + for (int j = 0; j < NR_SAMPLES; j++) {
> + int cycles;
> +
> + measure_instrs(i, pmu.pmcr_el0);
> + cycles = get_pmccntr();
> + printf(" %d", cycles);
> +
> + if (!cycles || (cpi > 0 && cycles != i * cpi)) {
> + printf("\n");
> + return false;
> + }
> +
> + sum += cycles;
> + }
> + avg = sum / NR_SAMPLES;
> + printf(" sum=%d avg=%d avg_ipc=%d avg_cpi=%d\n",
> + sum, avg, i / avg, avg / i);
> + }
> +
> + return true;
> +}
> +
> +int main(int argc, char *argv[])
>  {
> + int cpi = 0;
> +
> + if (argc >= 1)
> + cpi = atol(argv[0]);
> +
>   report_prefix_push("pmu");
>  
>   report("Control register", check_pmcr());
>   report("Monotonically increasing cycle count", check_cycles_increase());
> + report("Cycle/instruction ratio", check_cpi(cpi));
>  
>   return report_summary();
>  }
> -- 
> Qualcomm Innovation Center, Inc.
> 

Re: [Qemu-devel] [PATCH v8 06/17] qapi-types: Consolidate gen_struct() and gen_union()

[Markus Armbruster]

Eric Blake  writes:

> These two methods are now close enough that we can finally merge
> them, relying on the fact that simple unions now provide a
> reasonable local_members.  Change gen_struct() to gen_object()
> that handles all forms of QAPISchemaObjectType, and rename and
> shrink gen_union() to gen_variants() to handle the portion of
> gen_object() needed when variants are present.
>
> gen_struct_fields() now has a single caller, so it no longer
> needs an optional parameter; however, I did not choose to inline
> it into the caller.
>
> No difference to generated code.
>
> Signed-off-by: Eric Blake 
>
> ---
> v8: new patch
> ---
>  scripts/qapi-types.py | 36 +++-
>  1 file changed, 11 insertions(+), 25 deletions(-)
>
> diff --git a/scripts/qapi-types.py b/scripts/qapi-types.py
> index a6bf95d..403768b 100644
> --- a/scripts/qapi-types.py
> +++ b/scripts/qapi-types.py
> @@ -51,7 +51,7 @@ def gen_struct_field(member):
>  return ret
>
>
> -def gen_struct_fields(local_members, base=None):
> +def gen_struct_fields(local_members, base):
>  ret = ''
>
>  if base:
> @@ -70,7 +70,7 @@ def gen_struct_fields(local_members, base=None):
>  return ret
>
>
> -def gen_struct(name, base, members):
> +def gen_object(name, base, members, variants):
>  ret = mcgen('''
>
>  struct %(c_name)s {
> @@ -79,11 +79,14 @@ struct %(c_name)s {
>
>  ret += gen_struct_fields(members, base)
>
> +if variants:
> +ret += gen_variants(variants)
> +
>  # Make sure that all structs have at least one field; this avoids
>  # potential issues with attempting to malloc space for zero-length
>  # structs in C, and also incompatibility with C++ (where an empty
>  # struct is size 1).
> -if not (base and base.members) and not members:
> +if not (base and base.members) and not members and not variants:
>  ret += mcgen('''
>  char qapi_dummy_field_for_empty_struct;
>  ''')
> @@ -140,17 +143,7 @@ const int %(c_name)s_qtypes[QTYPE_MAX] = {
>  return ret
>
>
> -def gen_union(name, base, variants):
> -ret = mcgen('''
> -
> -struct %(c_name)s {
> -''',
> -c_name=c_name(name))
> -if base:
> -ret += gen_struct_fields([], base)
> -else:
> -ret += gen_struct_field(variants.tag_member)
> -
> +def gen_variants(variants):
>  # FIXME: What purpose does data serve, besides preventing a union that
>  # has a branch named 'data'? We use it in qapi-visit.py to decide
>  # whether to bypass the switch statement if visiting the discriminator
> @@ -159,11 +152,11 @@ struct %(c_name)s {
>  # should not be any data leaks even without a data pointer.  Or, if
>  # 'data' is merely added to guarantee we don't have an empty union,
>  # shouldn't we enforce that at .json parse time?
> -ret += mcgen('''
> +ret = mcgen('''
>  union { /* union tag is @%(c_name)s */
>  void *data;
>  ''',
> - c_name=c_name(variants.tag_member.name))
> +c_name=c_name(variants.tag_member.name))
>
>  for var in variants.variants:
>  # Ugly special case for simple union TODO get rid of it
> @@ -176,7 +169,6 @@ struct %(c_name)s {
>
>  ret += mcgen('''
>  } u;
> -};
>  ''')
>
>  return ret
> @@ -268,13 +260,7 @@ class QAPISchemaGenTypeVisitor(QAPISchemaVisitor):
>
>  def visit_object_type(self, name, info, base, members, variants):
>  self._fwdecl += gen_fwd_object_or_array(name)
> -if variants:
> -if members:
> -assert len(members) == 1
> -assert members[0] == variants.tag_member
> -self.decl += gen_union(name, base, variants)
> -else:
> -self.decl += gen_struct(name, base, members)
> +self.decl += gen_object(name, base, members, variants)
>  if base:
>  self.decl += gen_upcast(name, base)
>  self._gen_type_cleanup(name)
> @@ -282,7 +268,7 @@ class QAPISchemaGenTypeVisitor(QAPISchemaVisitor):
>  def visit_alternate_type(self, name, info, variants):
>  self._fwdecl += gen_fwd_object_or_array(name)
>  self._fwdefn += gen_alternate_qtypes(name, variants)
> -self.decl += gen_union(name, None, variants)
> +self.decl += gen_object(name, None, [variants.tag_member], variants)
>  self.decl += gen_alternate_qtypes_decl(name)
>  self._gen_type_cleanup(name)

Turned out nicely.

We could morph gen_struct_field() back into its original shape in a
separate patch:

def gen_struct_fields(members):
ret = ''

for memb in members:
ret += gen_struct_field(memb.name, memb.type, memb.optional)
return ret

with calling code

ret += mcgen('''
/* Members inherited from %(c_name)s: */
''',
 c_name=base.c_name())
ret += gen_struct_fields(base.members)
ret += mcgen('''
/* Own members: */
''')
ret += gen_struct_fields(

Re: [Qemu-devel] RAM backend and guest ABI (was Re: [PATCH v2] pc: memhp: enforce minimal 128Mb) alignment for pc-dimm

[Igor Mammedov]

On Thu, 29 Oct 2015 16:16:57 -0200
Eduardo Habkost  wrote:

> (CCing Michal and libvir-list, so libvirt team is aware of this
> restriction)
> 
> On Thu, Oct 29, 2015 at 02:36:37PM +0100, Igor Mammedov wrote:
> > On Tue, 27 Oct 2015 14:36:35 -0200
> > Eduardo Habkost  wrote:
> > 
> > > On Tue, Oct 27, 2015 at 10:14:56AM +0100, Igor Mammedov wrote:
> > > > On Tue, 27 Oct 2015 10:53:08 +0200
> > > > "Michael S. Tsirkin"  wrote:
> > > > 
> > > > > On Tue, Oct 27, 2015 at 09:48:37AM +0100, Igor Mammedov wrote:
> > > > > > On Tue, 27 Oct 2015 10:31:21 +0200
> > > > > > "Michael S. Tsirkin"  wrote:
> > > > > > 
> > > > > > > On Mon, Oct 26, 2015 at 02:24:32PM +0100, Igor Mammedov wrote:
> > > > > > > > Yep it's workaround but it works around QEMU's broken virtio
> > > > > > > > implementation in a simple way without need for guest side 
> > > > > > > > changes.
> > > > > > > > 
> > > > > > > > Without foreseeable virtio fix it makes memory hotplug unusable 
> > > > > > > > and even
> > > > > > > > more so if there were a virtio fix it won't fix old guests 
> > > > > > > > since you've
> > > > > > > > said that virtio fix would require changes of both QEMU and 
> > > > > > > > guest sides.
> > > > > > > 
> > > > > > > What makes it not foreseeable?
> > > > > > > Apparently only the fact that we have a work-around in place so 
> > > > > > > no one
> > > > > > > works on it.  I can code it up pretty quickly, but I'm flat out 
> > > > > > > of time
> > > > > > > for testing as I'm going on vacation soon, and hard freeze is 
> > > > > > > pretty
> > > > > > > close.
> > > > > > I can lend a hand for testing part.
> > > > > > 
> > > > > > > 
> > > > > > > GPA space is kind of cheap, but wasting it in chunks of 512M
> > > > > > > seems way too aggressive.
> > > > > > hotplug region is sized with 1Gb alignment reserve per DIMM so we 
> > > > > > aren't
> > > > > > actually wasting anything here.
> > > > > >
> > > > > 
> > > > > If I allocate two 1G DIMMs, what will be the gap size? 512M? 1G?
> > > > > It's too much either way.
> > > > minimum would be 512, and if backend is 1Gb-hugepage gap will be
> > > > backend's natural alignment (i.e. 1Gb).
> > > 
> > > Is backend configuration even allowed to affect the machine ABI? We need
> > > to be able to change backend configuration when migrating the VM to
> > > another host.
> > for now, one has to use the same type of backend on both sides
> > i.e. if source uses 1Gb huge pages backend then target also
> > need to use it.
> > 
> 
> The page size of the backend don't even depend on QEMU arguments, but on
> the kernel command-line or hugetlbfs mount options. So it's possible to
> have exactly the same QEMU command-line on source and destination (with
> an explicit versioned machine-type), and get a VM that can't be
> migrated? That means we are breaking our guarantees about migration and
> guest ABI.
> 
> 
> > We could change this for the next machine type to always force
> > max alignment (1Gb), then it would be possible to change
> > between backends with different alignments.
> 
> I'm not sure what's the best solution here. If always using 1GB is too
> aggressive, we could require management to ask for an explicit alignment
> as a -machine option if they know they will need a specific backend page
> size.
> 
> BTW, are you talking about the behavior introduced by
> aa8580cddf011e8cedcf87f7a0fdea7549fc4704 ("pc: memhp: force gaps between
> DIMM's GPA") only, or the backend page size was already affecting GPA
> allocation before that commit?
backend alignment was there since beginning,
we always over-reserve 1GB per slot since we don't know in advance what
alignment hotplugged backend would require.

Re: [Qemu-devel] [PATCH v6 27/33] nvdimm acpi: support function 0

[Xiao Guangrong]

On 10/30/2015 06:14 PM, Stefan Hajnoczi wrote:

On Fri, Oct 30, 2015 at 01:56:21PM +0800, Xiao Guangrong wrote:

  static uint64_t
  nvdimm_dsm_read(void *opaque, hwaddr addr, unsigned size)
  {
-return 0;
+AcpiNVDIMMState *state = opaque;
+MemoryRegion *dsm_ram_mr = &state->ram_mr;
+NvdimmDsmIn *in;
+GArray *out;
+void *dsm_ram_addr;
+uint32_t buf_size;
+
+assert(memory_region_size(dsm_ram_mr) >= sizeof(NvdimmDsmIn));
+dsm_ram_addr = memory_region_get_ram_ptr(dsm_ram_mr);
+
+/*
+ * The DSM memory is mapped to guest address space so an evil guest
+ * can change its content while we are doing DSM emulation. Avoid
+ * this by copying DSM memory to QEMU local memory.
+ */
+in = g_malloc(memory_region_size(dsm_ram_mr));
+memcpy(in, dsm_ram_addr, memory_region_size(dsm_ram_mr));
+
+le32_to_cpus(&in->revision);
+le32_to_cpus(&in->function);
+le32_to_cpus(&in->handle);
+
+nvdimm_debug("Revision %#x Handler %#x Function %#x.\n", in->revision,
+ in->handle, in->function);
+
+out = g_array_new(false, true /* clear */, 1);
+
+if (in->revision != 0x1 /* Current we support DSM Spec Rev1. */) {
+nvdimm_debug("Revision %#x is not supported, expect %#x.\n",
+  in->revision, 0x1);
+nvdimm_dsm_write_status(out, NVDIMM_DSM_STATUS_NOT_SUPPORTED);
+goto exit;
+}
+
+/* Handle 0 is reserved for NVDIMM Root Device. */
+if (!in->handle) {
+nvdimm_dsm_root(in, out);
+goto exit;
+}
+
+nvdimm_dsm_device(in, out);
+
+exit:
+/* Write output result to dsm memory. */
+memcpy(dsm_ram_addr, out->data, out->len);
+memory_region_set_dirty(dsm_ram_mr, 0, out->len);


If you respin this series, please add this before the memcpy out:

   assert(out->len <= memory_region_size(dsm_ram_mr))

That way we can catch situations where too much output data was
generated by mistake.



Okay. If this patchset is okay to be merged, i will add this in
future development.

Thanks for your continuous and active review, Stefan!

Re: [Qemu-devel] [PATCH v6 07/33] util: introduce qemu_file_get_page_size()

[Vladimir Sementsov-Ogievskiy]

logic is changed:
in old version gethugepagesize on statfs error generates exit(1)
in new it returns getpagesize() in this case (through fd_getpagesize)
(I think, fd_getpagesize should be fixed to handle error)

also, in new version for windows we have getpagesize(), when in old 
version there was no difference (how did it work?). May be it's ok, but 
should be mentioned in commit message



On 30.10.2015 08:56, Xiao Guangrong wrote:

There are three places use the some logic to get the page size on
the file path or file fd

This patch introduces qemu_file_get_page_size() to unify the code

Signed-off-by: Xiao Guangrong 
---
  include/qemu/osdep.h |  1 +
  target-ppc/kvm.c | 21 +++--
  util/oslib-posix.c   | 16 
  util/oslib-win32.c   |  5 +
  4 files changed, 25 insertions(+), 18 deletions(-)

diff --git a/include/qemu/osdep.h b/include/qemu/osdep.h
index b568424..d4dde02 100644
--- a/include/qemu/osdep.h
+++ b/include/qemu/osdep.h
@@ -302,4 +302,5 @@ int qemu_read_password(char *buf, int buf_size);
   */
  pid_t qemu_fork(Error **errp);
  
+size_t qemu_file_get_page_size(const char *mem_path);

  #endif
diff --git a/target-ppc/kvm.c b/target-ppc/kvm.c
index ac70f08..c661f1c 100644
--- a/target-ppc/kvm.c
+++ b/target-ppc/kvm.c
@@ -308,28 +308,13 @@ static void kvm_get_smmu_info(PowerPCCPU *cpu, struct 
kvm_ppc_smmu_info *info)
  
  static long gethugepagesize(const char *mem_path)

  {
-struct statfs fs;
-int ret;
-
-do {
-ret = statfs(mem_path, &fs);
-} while (ret != 0 && errno == EINTR);
+long size = qemu_file_get_page_size(mem_path);
  
-if (ret != 0) {

-fprintf(stderr, "Couldn't statfs() memory path: %s\n",
-strerror(errno));
+if (!size) {
  exit(1);
  }
  
-#define HUGETLBFS_MAGIC   0x958458f6

-
-if (fs.f_type != HUGETLBFS_MAGIC) {
-/* Explicit mempath, but it's ordinary pages */
-return getpagesize();
-}
-
-/* It's hugepage, return the huge page size */
-return fs.f_bsize;
+return size;
  }
  
  static int find_max_supported_pagesize(Object *obj, void *opaque)

diff --git a/util/oslib-posix.c b/util/oslib-posix.c
index 914cef5..ad94c5a 100644
--- a/util/oslib-posix.c
+++ b/util/oslib-posix.c
@@ -360,6 +360,22 @@ static size_t fd_getpagesize(int fd)
  return getpagesize();
  }
  
+size_t qemu_file_get_page_size(const char *path)

+{
+size_t size = 0;
+int fd = qemu_open(path, O_RDONLY);
+
+if (fd < 0) {
+fprintf(stderr, "Could not open %s.\n", path);
+goto exit;
+}
+
+size = fd_getpagesize(fd);
+qemu_close(fd);
+exit:
+return size;
+}
+
  void os_mem_prealloc(int fd, char *area, size_t memory)
  {
  int ret;
diff --git a/util/oslib-win32.c b/util/oslib-win32.c
index 09f9e98..a18aa87 100644
--- a/util/oslib-win32.c
+++ b/util/oslib-win32.c
@@ -462,6 +462,11 @@ size_t getpagesize(void)
  return system_info.dwPageSize;
  }
  
+size_t qemu_file_get_page_size(const char *path)

+{
+return getpagesize();
+}
+
  void os_mem_prealloc(int fd, char *area, size_t memory)
  {
  int i;



--
Best regards,
Vladimir
* now, @virtuozzo.com instead of @parallels.com. Sorry for this inconvenience.

Re: [Qemu-devel] [PATCH v2 07/16] vl.c: Use "%s support disabled" error messages consistently

[Andrew Jones]

On Thu, Oct 29, 2015 at 05:59:13PM +0100, Markus Armbruster wrote:
> Eduardo Habkost  writes:
> 
> > Signed-off-by: Eduardo Habkost 
> > ---
> >  vl.c | 34 --
> >  1 file changed, 16 insertions(+), 18 deletions(-)
> >
> > diff --git a/vl.c b/vl.c
> > index d417dd9..f37e3a9 100644
> > --- a/vl.c
> > +++ b/vl.c
> > @@ -1018,8 +1018,7 @@ static int parse_sandbox(void *opaque, QemuOpts 
> > *opts, Error **errp)
> >  return -1;
> >  }
> >  #else
> > -error_report("sandboxing request but seccomp is not compiled "
> > - "into this build");
> > +error_report("seccomp support disabled");
> >  return -1;
> >  #endif
> >  }
> 
> Yes, please.
> 
> > @@ -2112,7 +2111,7 @@ static DisplayType select_display(const char *p)
> >  opts = nextopt;
> >  }
> >  #else
> > -error_report("SDL support is disabled");
> > +error_report("SDL support disabled");
> >  exit(1);
> >  #endif
> >  } else if (strstart(p, "vnc", &opts)) {
> 
> I'd prefer the old wording.  Drew?

I like the 'is' version better too. I'm OK with both though.

> 
> > @@ -2128,14 +2127,14 @@ static DisplayType select_display(const char *p)
> >  exit(1);
> >  }
> >  #else
> > -error_report("VNC support is disabled");
> > +error_report("VNC support disabled");
> >  exit(1);
> >  #endif
> >  } else if (strstart(p, "curses", &opts)) {
> >  #ifdef CONFIG_CURSES
> >  display = DT_CURSES;
> >  #else
> > -error_report("Curses support is disabled");
> > +error_report("curses support disabled");
> >  exit(1);
> >  #endif
> >  } else if (strstart(p, "gtk", &opts)) {
> > @@ -2170,7 +2169,7 @@ static DisplayType select_display(const char *p)
> >  opts = nextopt;
> >  }
> >  #else
> > -error_report("GTK support is disabled");
> > +error_report("GTK support disabled");
> >  exit(1);
> >  #endif
> >  } else if (strstart(p, "none", &opts)) {
> > @@ -2635,7 +2634,7 @@ static gint machine_class_cmp(gconstpointer a, 
> > gconstpointer b)
> >  return mc;
> >  }
> >  if (name && !is_help_option(name)) {
> > -error_report("Unsupported machine type");
> > +error_report("unsupported machine type");
> >  error_printf("Use -machine help to list supported machines!\n");
> >  } else {
> >  printf("Supported machines are:\n");
> 
> This belongs to PATCH 11.
> 
> > @@ -3011,7 +3010,7 @@ int main(int argc, char **argv, char **envp)
> >  runstate_init();
> >  
> >  if (qcrypto_init(&err) < 0) {
> > -error_report("Cannot initialize crypto: %s", 
> > error_get_pretty(err));
> > +error_report("cannot initialize crypto: %s", 
> > error_get_pretty(err));
> >  exit(1);
> >  }
> >  rtc_clock = QEMU_CLOCK_HOST;
> 
> Likewise.
> 
> > @@ -3212,7 +3211,7 @@ int main(int argc, char **argv, char **envp)
> >  #ifdef CONFIG_CURSES
> >  display_type = DT_CURSES;
> >  #else
> > -error_report("Curses support is disabled");
> > +error_report("curses support disabled");
> >  exit(1);
> >  #endif
> >  break;
> 
> Likewise.
> 
> > @@ -3440,7 +3439,7 @@ int main(int argc, char **argv, char **envp)
> >  case QEMU_OPTION_fsdev:
> >  olist = qemu_find_opts("fsdev");
> >  if (!olist) {
> > -error_report("fsdev is not supported by this qemu 
> > build");
> > +error_report("fsdev support disabled");
> >  exit(1);
> >  }
> >  opts = qemu_opts_parse_noisily(olist, optarg, true);
> > @@ -3455,7 +3454,7 @@ int main(int argc, char **argv, char **envp)
> >  
> >  olist = qemu_find_opts("virtfs");
> >  if (!olist) {
> > -error_report("virtfs is not supported by this qemu 
> > build");
> > +error_report("virtfs support disabled");
> >  exit(1);
> >  }
> >  opts = qemu_opts_parse_noisily(olist, optarg, true);
> > @@ -3593,7 +3592,7 @@ int main(int argc, char **argv, char **envp)
> >  display_type = DT_SDL;
> >  break;
> >  #else
> > -error_report("SDL support is disabled");
> > +error_report("SDL support disabled");
> >  exit(1);
> >  #endif
> >  case QEMU_OPTION_pidfile:
> > @@ -3705,7 +3704,7 @@ int main(int argc, char **argv, char **envp)
> >  exit(1);
> >  }
> >  #else
> > -error_report("VNC support is disabled");
> > +error_report("VNC support disabled");
> >  exit(1);
> >  #endif
> >  break;
> > @@ -3899,7 +3898,7 @@ int main(int argc, char 

Re: [Qemu-devel] [PATCH v2 09/16] vl.c: Reword -no-kvm-pit-reinjection deprecation warning

[Andrew Jones]

On Thu, Oct 29, 2015 at 06:10:17PM +0100, Markus Armbruster wrote:
> Eduardo Habkost  writes:
> 
> > Signed-off-by: Eduardo Habkost 
> > ---
> >  vl.c | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/vl.c b/vl.c
> > index af8d09c..67147e0 100644
> > --- a/vl.c
> > +++ b/vl.c
> > @@ -3667,8 +3667,8 @@ int main(int argc, char **argv, char **envp)
> >  { /* end of list */ }
> >  };
> >  
> > -error_report("warning: option deprecated, use "
> > - "lost_tick_policy property of kvm-pit 
> > instead");
> > +error_report("warning: -no-kvm-pit-reinjection deprecated 
> > by "
> > + "-global kvm-pit.lost_tick_policy=discard");
> >  qdev_prop_register_global_list(kvm_pit_lost_tick_policy);
> >  break;
> >  }
> 
> Repeating the option in the error message rarely improves the message,
> because error_report() already shows it.  It's actually misleading when
> the option comes from a configuration file read with -readconfig.
> 
> Fortunately, -readconfig doesn't support the option, so that's not an
> issue here.
> 
> For the command line, the message changes from
> 
> qemu-system-x86_64: -no-kvm-pit-reinjection: warning: option deprecated, 
> use lost_tick_policy property of kvm-pit instead
> 
> to
> 
> qemu-system-x86_64: -no-kvm-pit-reinjection: warning: 
> -no-kvm-pit-reinjection deprecated by -global kvm-pit.lost_tick_policy=discard
> 
> Perhaps:
> 
> qemu-system-x86_64: -no-kvm-pit-reinjection: warning: deprecated, 
> replaced by -global kvm-pit.lost_tick_policy=discard
> 
> Similar argument for PATCH 08.
> 

Ah, I hadn't looked closely enough at error_report to know that the option
would be output already with my "ignoring" phrase suggestion. I definitely
agree that repeating it should be avoided.

"warning: deprecated, ignoring"

Thanks,
drew

[Qemu-devel] [PULL 01/05] seccomp: add cacheflush to whitelist

[Eduardo Otubo]

From: Andrew Jones 

cacheflush is an arm-specific syscall that qemu built for arm
uses. Add it to the whitelist.

Signed-off-by: Andrew Jones 
Acked-by: Eduardo Otubo 
---
 qemu-seccomp.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/qemu-seccomp.c b/qemu-seccomp.c
index f9de0d3..33644a4 100644
--- a/qemu-seccomp.c
+++ b/qemu-seccomp.c
@@ -237,7 +237,8 @@ static const struct QemuSeccompSyscall seccomp_whitelist[] 
= {
 { SCMP_SYS(fadvise64), 240 },
 { SCMP_SYS(inotify_init1), 240 },
 { SCMP_SYS(inotify_add_watch), 240 },
-{ SCMP_SYS(mbind), 240 }
+{ SCMP_SYS(mbind), 240 },
+{ SCMP_SYS(cacheflush), 240 },
 };
 
 int seccomp_start(void)
-- 
2.1.4

[Qemu-devel] [PULL 00/05] seccomp branch queue

[Eduardo Otubo]

The following changes since commit c49d3411faae8ffaab8f7e5db47405a008411c10:

  Merge remote-tracking branch 'remotes/armbru/tags/pull-qapi-2015-10-12' into 
staging (2015-10-13 10:42:06 +0100)

are available in the git repository at:

  git://github.com/otubo/qemu.git tags/pull-seccomp-20151030

for you to fetch changes up to b1e1f0bbe7268d0bb8f63da220b41803b2e54081:

  seccomp: loosen library version dependency (2015-10-30 14:33:00 +0100)


seccomp branch queue


Andrew Jones (2):
  seccomp: add cacheflush to whitelist
  configure: arm/aarch64: allow enable-seccomp

Namsun Ch'o (2):
  seccomp: add madvise, shmget, and shmctl to whitelist
  seccomp: add setuid, setgid, chroot and setgroups to whitelist

dann frazier (1):
  seccomp: loosen library version dependency

 configure  | 32 ++---
 qemu-seccomp.c | 65 ++
 2 files changed, 86 insertions(+), 11 deletions(-)

-- 
2.1.4

[Qemu-devel] [PULL 05/05] seccomp: loosen library version dependency

[Eduardo Otubo]

From: dann frazier 

Drop the libseccomp required version back to 2.1.0, restoring the ability
to build w/ --enable-seccomp on Ubuntu 14.04.

Commit 4cc47f8b3cc4f32586ba2f7fce1dc267da774a69 tightened the dependency
on libseccomp from version 2.1.0 to 2.1.1. This broke building on Ubuntu
14.04, the current Ubuntu LTS release. The commit message didn't mention
any specific functional need for 2.1.1, just that it was the most recent
stable version at the time. I reviewed the changes between 2.1.0 and 2.1.1,
but it looks like that update just contained minor fixes and cleanups - no
obvious (to me) new interfaces or critical bug fixes.

Signed-off-by: dann frazier 
Acked-by: Eduardo Otubo 
---
 configure | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/configure b/configure
index 7d5aab2..8a9794b 100755
--- a/configure
+++ b/configure
@@ -1878,7 +1878,7 @@ fi
 if test "$seccomp" != "no" ; then
 case "$cpu" in
 i386|x86_64)
-libseccomp_minver="2.1.1"
+libseccomp_minver="2.1.0"
 ;;
 arm|aarch64)
 libseccomp_minver="2.2.3"
-- 
2.1.4

[Qemu-devel] [PULL 03/05] seccomp: add madvise, shmget, and shmctl to whitelist

[Eduardo Otubo]

From: Namsun Ch'o 

This patch introduces madvise, shmget, shmctl and its flags to the
seccomp whitelist. This prevents Qemu to break in case of using -runas
or chroot with sandbox enabled.

Signed-off-by: Namsun Ch'o 
Acked-by: Eduardo Otubo 
---
Changelog:
v1
 - Created argument filters for the madvise, shmget, and shmctl syscalls.
v1 -> v2
 - Added 5 new madvise flags which were present in the source code but not in
   the strace which I generated.
 - Added IP_CREAT|0600 to shmget, which Daniel Berrange pointed out was
   present in GTK2, which QEMU uses but does not call directly.
v2 -> v3
 - Replaced include asm/mman-common.h with sys/mman.h which is more proper.
 - Fixed typo where I had IP_CREAT instead of IPC_CREAT.
 - Removed the comma on the last entry of the madvise_flags array.
 - Removed one madvise flag (MADV_INVALID) which doesn't exist, apparently.

 qemu-seccomp.c | 58 +++---
 1 file changed, 55 insertions(+), 3 deletions(-)

diff --git a/qemu-seccomp.c b/qemu-seccomp.c
index 33644a4..e7a54e8 100644
--- a/qemu-seccomp.c
+++ b/qemu-seccomp.c
@@ -14,6 +14,8 @@
  */
 #include 
 #include 
+#include 
+#include 
 #include "sysemu/seccomp.h"
 
 struct QemuSeccompSyscall {
@@ -105,7 +107,6 @@ static const struct QemuSeccompSyscall seccomp_whitelist[] 
= {
 { SCMP_SYS(rt_sigreturn), 245 },
 { SCMP_SYS(sync), 245 },
 { SCMP_SYS(pread64), 245 },
-{ SCMP_SYS(madvise), 245 },
 { SCMP_SYS(set_robust_list), 245 },
 { SCMP_SYS(lseek), 245 },
 { SCMP_SYS(pselect6), 245 },
@@ -224,11 +225,9 @@ static const struct QemuSeccompSyscall seccomp_whitelist[] 
= {
 { SCMP_SYS(arch_prctl), 240 },
 { SCMP_SYS(mkdir), 240 },
 { SCMP_SYS(fchmod), 240 },
-{ SCMP_SYS(shmget), 240 },
 { SCMP_SYS(shmat), 240 },
 { SCMP_SYS(shmdt), 240 },
 { SCMP_SYS(timerfd_create), 240 },
-{ SCMP_SYS(shmctl), 240 },
 { SCMP_SYS(mlockall), 240 },
 { SCMP_SYS(mlock), 240 },
 { SCMP_SYS(munlock), 240 },
@@ -265,6 +264,59 @@ int seccomp_start(void)
 }
 }
 
+/* madvise */
+static const int madvise_flags[] = {
+MADV_DODUMP,
+MADV_DONTDUMP,
+MADV_UNMERGEABLE,
+MADV_WILLNEED,
+MADV_DONTFORK,
+MADV_DONTNEED,
+MADV_HUGEPAGE,
+MADV_MERGEABLE
+};
+for (i = 0; i < ARRAY_SIZE(madvise_flags); i++) {
+rc = seccomp_rule_add(ctx, SCMP_ACT_ALLOW, SCMP_SYS(madvise), 1,
+SCMP_A2(SCMP_CMP_EQ, madvise_flags[i]));
+if (rc < 0) {
+goto seccomp_return;
+}
+}
+rc = seccomp_syscall_priority(ctx, SCMP_SYS(madvise), 245);
+if (rc < 0) {
+goto seccomp_return;
+}
+
+/* shmget */
+rc = seccomp_rule_add(ctx, SCMP_ACT_ALLOW, SCMP_SYS(shmget), 2,
+SCMP_A0(SCMP_CMP_EQ, IPC_PRIVATE),
+SCMP_A2(SCMP_CMP_EQ, IPC_CREAT|0777));
+if (rc < 0) {
+goto seccomp_return;
+}
+rc = seccomp_rule_add(ctx, SCMP_ACT_ALLOW, SCMP_SYS(shmget), 2,
+SCMP_A0(SCMP_CMP_EQ, IPC_PRIVATE),
+SCMP_A2(SCMP_CMP_EQ, IPC_CREAT|0600));
+if (rc < 0) {
+goto seccomp_return;
+}
+rc = seccomp_syscall_priority(ctx, SCMP_SYS(shmget), 240);
+if (rc < 0) {
+goto seccomp_return;
+}
+
+/* shmctl */
+rc = seccomp_rule_add(ctx, SCMP_ACT_ALLOW, SCMP_SYS(shmctl), 2,
+SCMP_A1(SCMP_CMP_EQ, IPC_RMID),
+SCMP_A2(SCMP_CMP_EQ, 0));
+if (rc < 0) {
+goto seccomp_return;
+}
+rc = seccomp_syscall_priority(ctx, SCMP_SYS(shmctl), 240);
+if (rc < 0) {
+goto seccomp_return;
+}
+
 rc = seccomp_load(ctx);
 
   seccomp_return:
-- 
2.1.4

[Qemu-devel] [PULL 02/05] configure: arm/aarch64: allow enable-seccomp

[Eduardo Otubo]

From: Andrew Jones 

This is a revert of ae6e8ef11e6cb, but with a bit of refactoring,
and also specifically adding arm/aarch64, rather than all
architectures. Currently, libseccomp code appears to also support
mips, ppc, and s390. We could therefore allow qemu to enable
seccomp for those platforms as well, with additional configure
patches, given they're tested and proven to work.

Signed-off-by: Andrew Jones 
Acked-by: Eduardo Otubo 
---
 configure | 32 +---
 1 file changed, 25 insertions(+), 7 deletions(-)

diff --git a/configure b/configure
index f08327e..7d5aab2 100755
--- a/configure
+++ b/configure
@@ -1876,16 +1876,34 @@ fi
 # libseccomp check
 
 if test "$seccomp" != "no" ; then
-if test "$cpu" = "i386" || test "$cpu" = "x86_64" &&
-$pkg_config --atleast-version=2.1.1 libseccomp; then
+case "$cpu" in
+i386|x86_64)
+libseccomp_minver="2.1.1"
+;;
+arm|aarch64)
+libseccomp_minver="2.2.3"
+;;
+*)
+libseccomp_minver=""
+;;
+esac
+
+if test "$libseccomp_minver" != "" &&
+   $pkg_config --atleast-version=$libseccomp_minver libseccomp ; then
 libs_softmmu="$libs_softmmu `$pkg_config --libs libseccomp`"
 QEMU_CFLAGS="$QEMU_CFLAGS `$pkg_config --cflags libseccomp`"
-   seccomp="yes"
+seccomp="yes"
 else
-   if test "$seccomp" = "yes"; then
-feature_not_found "libseccomp" "Install libseccomp devel >= 2.1.1"
-   fi
-   seccomp="no"
+if test "$seccomp" = "yes" ; then
+if test "$libseccomp_minver" != "" ; then
+feature_not_found "libseccomp" \
+"Install libseccomp devel >= $libseccomp_minver"
+else
+feature_not_found "libseccomp" \
+"libseccomp is not supported for host cpu $cpu"
+fi
+fi
+seccomp="no"
 fi
 fi
 ##
-- 
2.1.4

[Qemu-devel] [PULL 04/05] seccomp: add setuid, setgid, chroot and setgroups to whitelist

[Eduardo Otubo]

From: Namsun Ch'o 

The seccomp sandbox doesn't whitelist setuid, setgid, or setgroups, which are
needed for -runas to work. It also doesn't whitelist chroot, which is needed
for the -chroot option. Unfortunately, QEMU enables seccomp before it drops
privileges or chroots, so without these whitelisted, -runas and -chroot cause
QEMU to be killed with -sandbox on. This patch adds those syscalls.

Signed-off-by: Namsun Ch'o 
Acked-by: Eduardo Otubo 
---
 qemu-seccomp.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/qemu-seccomp.c b/qemu-seccomp.c
index e7a54e8..877fd88 100644
--- a/qemu-seccomp.c
+++ b/qemu-seccomp.c
@@ -238,6 +238,10 @@ static const struct QemuSeccompSyscall seccomp_whitelist[] 
= {
 { SCMP_SYS(inotify_add_watch), 240 },
 { SCMP_SYS(mbind), 240 },
 { SCMP_SYS(cacheflush), 240 },
+{ SCMP_SYS(setuid), 240 },
+{ SCMP_SYS(setgid), 240 },
+{ SCMP_SYS(chroot), 240 },
+{ SCMP_SYS(setgroups), 240 },
 };
 
 int seccomp_start(void)
-- 
2.1.4

Re: [Qemu-devel] [PATCH v6 08/33] exec: allow memory to be allocated from any kind of path

[Vladimir Sementsov-Ogievskiy]

On 30.10.2015 08:56, Xiao Guangrong wrote:

Currently file_ram_alloc() is designed for hugetlbfs, however, the memory
of nvdimm can come from either raw pmem device eg, /dev/pmem, or the file
locates at DAX enabled filesystem

So this patch let it work on any kind of path

Signed-off-by: Xiao Guangrong 
---
  exec.c | 56 +---
  1 file changed, 17 insertions(+), 39 deletions(-)

diff --git a/exec.c b/exec.c
index 8af2570..3ca7e50 100644
--- a/exec.c
+++ b/exec.c
@@ -1174,32 +1174,6 @@ void qemu_mutex_unlock_ramlist(void)
  }
  
  #ifdef __linux__

-
-#include 
-
-#define HUGETLBFS_MAGIC   0x958458f6
-
-static long gethugepagesize(const char *path, Error **errp)
-{
-struct statfs fs;
-int ret;
-
-do {
-ret = statfs(path, &fs);
-} while (ret != 0 && errno == EINTR);
-
-if (ret != 0) {
-error_setg_errno(errp, errno, "failed to get page size of file %s",
- path);
-return 0;
-}
-
-if (fs.f_type != HUGETLBFS_MAGIC)
-fprintf(stderr, "Warning: path not on HugeTLBFS: %s\n", path);
-
-return fs.f_bsize;
-}
-
  static void *file_ram_alloc(RAMBlock *block,
  ram_addr_t memory,
  const char *path,
@@ -1210,20 +1184,24 @@ static void *file_ram_alloc(RAMBlock *block,
  char *c;
  void *area;
  int fd;
-uint64_t hpagesize;
-Error *local_err = NULL;
+uint64_t pagesize;
  
-hpagesize = gethugepagesize(path, &local_err);

-if (local_err) {
-error_propagate(errp, local_err);
+pagesize = qemu_file_get_page_size(path);
+if (!pagesize) {
+error_setg(errp, "can't get page size for %s", path);
  goto error;
  }
-block->mr->align = hpagesize;
  
-if (memory < hpagesize) {

+if (pagesize == getpagesize()) {
+fprintf(stderr, "Memory is not allocated from HugeTlbfs.\n");
+}


It is strange to see this warning every time.

Shouldn't the differentiation be done explicitly in command line? May be 
separate option mem-tlb, or separate flag tlbfs=on, or for new feature - 
new option mem-file, or prefixes for paths (tlbfs://, file://).. Or the 
other way to not mix things but split them.



+
+block->mr->align = pagesize;
+
+if (memory < pagesize) {
  error_setg(errp, "memory size 0x" RAM_ADDR_FMT " must be equal to "
-   "or larger than huge page size 0x%" PRIx64,
-   memory, hpagesize);
+   "or larger than page size 0x%" PRIx64,
+   memory, pagesize);
  goto error;
  }
  
@@ -1247,14 +1225,14 @@ static void *file_ram_alloc(RAMBlock *block,

  fd = mkstemp(filename);
  if (fd < 0) {
  error_setg_errno(errp, errno,
- "unable to create backing store for hugepages");
+ "unable to create backing store for path %s", path);
  g_free(filename);
  goto error;
  }
  unlink(filename);
  g_free(filename);
  
-memory = ROUND_UP(memory, hpagesize);

+memory = ROUND_UP(memory, pagesize);
  
  /*

   * ftruncate is not supported by hugetlbfs in older
@@ -1266,10 +1244,10 @@ static void *file_ram_alloc(RAMBlock *block,
  perror("ftruncate");
  }
  
-area = qemu_ram_mmap(fd, memory, hpagesize, block->flags & RAM_SHARED);

+area = qemu_ram_mmap(fd, memory, pagesize, block->flags & RAM_SHARED);
  if (area == MAP_FAILED) {
  error_setg_errno(errp, errno,
- "unable to map backing store for hugepages");
+ "unable to map backing store for path %s", path);
  close(fd);
  goto error;
  }



--
Best regards,
Vladimir
* now, @virtuozzo.com instead of @parallels.com. Sorry for this inconvenience.

[Qemu-devel] [Bug 1511710] [NEW] unknown option --disable-modules

[Guy]

Public bug reported:

MSYS64, Windows 7 x64

$ ./configure --target-list=i386-softmmu --static --prefix=/d/qemu/ 
--disable-system --disable-user --disable-linux-user --disable-bsd-user 
--disable-guest-base --disable-docs --disable-guest-agent 
--disable-guest-agent-msi --disable-pie --disable-modules --disable-debug-tcg 
--disable-debug-info --disable-sparse --disable-seccomp --disable-gnutls 
--disable-sdl--disable-gtk --disable-vte --disable-curses --disable-vnc 
--disable-vnc-tls --disable-vnc-sasl --disable-vnc-jpeg --disable-vnc-png 
--disable-cocoa --disable-virtfs --disable-xen --disable-xen-pci-passthro 
--disable-brlapi --disable-curl --disable-fdt --disable-bluez --disable-kvm 
--disable-rdma --disable-uuid --disable-vde --disable-netmap 
--disable-linux-aio --disable-cap-ng --disable-attr --disable-vhost-net 
--disable-spice --disable-rbd --disable-libiscsi --disable-libnfs 
--disable-smartcard-nss --disable-libusb --disable-usb-redir --disable-lzo 
--disable-snappy --disable-bzip2 --disable-coroutine-pool --disable-glusterfs 
--disable-archipelago --disable-tpm --disable-libssh2 --disable-vhdx 
--disable-numa --disable-tcmalloc
ERROR: unknown option --disable-modules
Try './configure --help' for more information

** Affects: qemu
 Importance: Undecided
 Status: New

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

Title:
  unknown option --disable-modules

Status in QEMU:
  New

Bug description:
  MSYS64, Windows 7 x64

  $ ./configure --target-list=i386-softmmu --static --prefix=/d/qemu/ 
--disable-system --disable-user --disable-linux-user --disable-bsd-user 
--disable-guest-base --disable-docs --disable-guest-agent 
--disable-guest-agent-msi --disable-pie --disable-modules --disable-debug-tcg 
--disable-debug-info --disable-sparse --disable-seccomp --disable-gnutls 
--disable-sdl--disable-gtk --disable-vte --disable-curses --disable-vnc 
--disable-vnc-tls --disable-vnc-sasl --disable-vnc-jpeg --disable-vnc-png 
--disable-cocoa --disable-virtfs --disable-xen --disable-xen-pci-passthro 
--disable-brlapi --disable-curl --disable-fdt --disable-bluez --disable-kvm 
--disable-rdma --disable-uuid --disable-vde --disable-netmap 
--disable-linux-aio --disable-cap-ng --disable-attr --disable-vhost-net 
--disable-spice --disable-rbd --disable-libiscsi --disable-libnfs 
--disable-smartcard-nss --disable-libusb --disable-usb-redir --disable-lzo 
--disable-snappy --disable-bzip2 --disable-coroutine-pool --disable-glusterfs 
--disable-archipelago --disable-tpm --disable-libssh2 --disable-vhdx 
--disable-numa --disable-tcmalloc
  ERROR: unknown option --disable-modules
  Try './configure --help' for more information

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

Re: [Qemu-devel] [PATCH v2 0/4] ui audio qxl usb: Use g_new() & friends where that makes obvious sense

[Markus Armbruster]

Gerd Hoffmann  writes:

> On Do, 2015-10-29 at 16:55 +0100, Markus Armbruster wrote:
>> v2:
>> * Trivially rebased
>> 
>> Markus Armbruster (4):
>>   ui: Use g_new() & friends where that makes obvious sense
>>   audio: Use g_new() & friends where that makes obvious sense
>>   qxl: Use g_new() & friends where that makes obvious sense
>>   usb: Use g_new() & friends where that makes obvious sense
>
> I'd prefer to drop the audio one because there still is the gsoc patch
> series pending which touches all this anyway.

I'm fine with delaying the audio part; running spatch again is easy
enough.  I'd appreciate a reminder when the audio subsystem is ready for
this work.

> Otherwise looks fine to me.
>
> Do you want me pick this up or send a pull request yourself?

I thought either you or -trivial.

> On case of the latter:
>
> Reviewed-by: Gerd Hoffmann 

Thanks!

Re: [Qemu-devel] [PULL 00/12] Block patches

[Markus Armbruster]

Peter Maydell  writes:

> On 29 October 2015 at 18:09, Stefan Hajnoczi  wrote:
>> The following changes since commit 7bc8e0c967a4ef77657174d28af775691e18b4ce:
>>
>>   Merge remote-tracking branch 'remotes/mst/tags/for_upstream' into
>> staging (2015-10-29 09:49:52 +)
>>
>> are available in the git repository at:
>>
>>   git://github.com/stefanha/qemu.git tags/block-pull-request
>>
>> for you to fetch changes up to 37a639a7fbc5c6b065c80e7e2de78d22af735496:
>>
>>   block: Consider all child nodes in bdrv_requests_pending()
>> (2015-10-29 17:59:27 +)
>>
>> 
>>
>> 
>
> I get an error on 64-bit ARM running the ivshmem tests:
>
> TEST: tests/ivshmem-test... (pid=22948)
>   /i386/ivshmem/single:OK
>   /i386/ivshmem/pair:  OK
>   /i386/ivshmem/server:**
> ERROR:/home/petmay01/qemu/tests/ivshmem-test.c:345:test_ivshmem_server:
> assertion failed (ret != 0): (0 != 0)
> FAIL
> GTester: last random seed: R02S51e68a84790014e86af5b8b7264d3e39
> (pid=23709)
>   /i386/ivshmem/hotplug:   OK
>   /i386/ivshmem/memdev:OK
> FAIL: tests/ivshmem-test
>
> Nothing obviously related in this patchset that would cause that,
> though...

I've seen this, too, but throwing away my build tree made it go away, so
I blamed "make choking on stale build tree" syndrome.  Perhaps it's an
intermittent ivshmem bug instead.

Re: [Qemu-devel] [PATCH v6 09/33] exec: allow file_ram_alloc to work on file

[Vladimir Sementsov-Ogievskiy]

On 30.10.2015 08:56, Xiao Guangrong wrote:

Currently, file_ram_alloc() only works on directory - it creates a file
under @path and do mmap on it

This patch tries to allow it to work on file directly, if @path is a
directory it works as before, otherwise it treats @path as the target
file then directly allocate memory from it

Signed-off-by: Xiao Guangrong 
---
  exec.c | 80 ++
  1 file changed, 51 insertions(+), 29 deletions(-)

diff --git a/exec.c b/exec.c
index 3ca7e50..f219010 100644
--- a/exec.c
+++ b/exec.c
@@ -1174,14 +1174,60 @@ void qemu_mutex_unlock_ramlist(void)
  }
  
  #ifdef __linux__

+static bool path_is_dir(const char *path)
+{
+struct stat fs;
+
+return stat(path, &fs) == 0 && S_ISDIR(fs.st_mode);
+}
+
+static int open_file_path(RAMBlock *block, const char *path, size_t size)


I think the name should be more descriptive, as it is very special 
function in common exec.c and it doesn't "just open file path'.. May be 
open_ram_file_path




+{
+char *filename;
+char *sanitized_name;
+char *c;
+int fd;
+
+if (!path_is_dir(path)) {
+int flags = (block->flags & RAM_SHARED) ? O_RDWR : O_RDONLY;
+
+flags |= O_EXCL;
+return open(path, flags);
+}


Was not there old scenarios when path is file? statfs will  success for 
any file withing mounted filesystem.


Also, may be we shouldn't warn about "Memory is not allocated from 
HugeTlbfs.\n" in case of !path_is_dir ?



+
+/* Make name safe to use with mkstemp by replacing '/' with '_'. */
+sanitized_name = g_strdup(memory_region_name(block->mr));
+for (c = sanitized_name; *c != '\0'; c++) {
+if (*c == '/') {
+*c = '_';
+}
+}
+filename = g_strdup_printf("%s/qemu_back_mem.%s.XX", path,
+   sanitized_name);
+g_free(sanitized_name);
+fd = mkstemp(filename);
+if (fd >= 0) {
+unlink(filename);
+/*
+ * ftruncate is not supported by hugetlbfs in older
+ * hosts, so don't bother bailing out on errors.
+ * If anything goes wrong with it under other filesystems,
+ * mmap will fail.
+ */
+if (ftruncate(fd, size)) {
+perror("ftruncate");
+}
+}
+g_free(filename);
+
+return fd;
+}
+
  static void *file_ram_alloc(RAMBlock *block,
  ram_addr_t memory,
  const char *path,
  Error **errp)
  {
-char *filename;
-char *sanitized_name;
-char *c;
  void *area;
  int fd;
  uint64_t pagesize;
@@ -1211,38 +1257,14 @@ static void *file_ram_alloc(RAMBlock *block,
  goto error;
  }
  
-/* Make name safe to use with mkstemp by replacing '/' with '_'. */

-sanitized_name = g_strdup(memory_region_name(block->mr));
-for (c = sanitized_name; *c != '\0'; c++) {
-if (*c == '/')
-*c = '_';
-}
-
-filename = g_strdup_printf("%s/qemu_back_mem.%s.XX", path,
-   sanitized_name);
-g_free(sanitized_name);
+memory = ROUND_UP(memory, pagesize);
  
-fd = mkstemp(filename);

+fd = open_file_path(block, path, memory);
  if (fd < 0) {
  error_setg_errno(errp, errno,
   "unable to create backing store for path %s", path);
-g_free(filename);
  goto error;
  }
-unlink(filename);
-g_free(filename);
-
-memory = ROUND_UP(memory, pagesize);
-
-/*
- * ftruncate is not supported by hugetlbfs in older
- * hosts, so don't bother bailing out on errors.
- * If anything goes wrong with it under other filesystems,
- * mmap will fail.
- */
-if (ftruncate(fd, memory)) {
-perror("ftruncate");
-}
  
  area = qemu_ram_mmap(fd, memory, pagesize, block->flags & RAM_SHARED);

  if (area == MAP_FAILED) {



--
Best regards,
Vladimir
* now, @virtuozzo.com instead of @parallels.com. Sorry for this inconvenience.

Re: [Qemu-devel] [PATCH v6 10/33] hostmem-file: clean up memory allocation

[Vladimir Sementsov-Ogievskiy]

On 30.10.2015 08:56, Xiao Guangrong wrote:

- hostmem-file.c is compiled only if CONFIG_LINUX is enabled so that is
   unnecessary to do the same check in the source file

- the interface, HostMemoryBackendClass->alloc(), is not called many
   times, do not need to check if the memory-region is initialized

Signed-off-by: Xiao Guangrong 
---
  backends/hostmem-file.c | 11 +++
  1 file changed, 3 insertions(+), 8 deletions(-)

diff --git a/backends/hostmem-file.c b/backends/hostmem-file.c
index e9b6d21..9097a57 100644
--- a/backends/hostmem-file.c
+++ b/backends/hostmem-file.c
@@ -46,17 +46,12 @@ file_backend_memory_alloc(HostMemoryBackend *backend, Error 
**errp)
  error_setg(errp, "mem-path property not set");
  return;
  }
-#ifndef CONFIG_LINUX
-error_setg(errp, "-mem-path not supported on this host");
-#else
-if (!memory_region_size(&backend->mr)) {
-backend->force_prealloc = mem_prealloc;
-memory_region_init_ram_from_file(&backend->mr, OBJECT(backend),
+
+backend->force_prealloc = mem_prealloc;
+memory_region_init_ram_from_file(&backend->mr, OBJECT(backend),
   object_get_canonical_path(OBJECT(backend)),
   backend->size, fb->share,
   fb->mem_path, errp);
-}
-#endif
  }
  
  static void


Similar function for memory backend (the only other 
HostMemoryBackendClass) - ram_backend_memory_alloc - has not such check 
too. It's ok..


Reviewed-by: Vladimir Sementsov-Ogievskiy 

--
Best regards,
Vladimir
* now, @virtuozzo.com instead of @parallels.com. Sorry for this inconvenience.

[Qemu-devel] [PULL 0/9] target-mips queue

[Leon Alrae]

Hi,

Here's my current target-mips queue, just fixes and relatively minor
improvements.

Thanks,
Leon

Cc: Peter Maydell 
Cc: Aurelien Jarno 

The following changes since commit 7bc8e0c967a4ef77657174d28af775691e18b4ce:

  Merge remote-tracking branch 'remotes/mst/tags/for_upstream' into staging 
(2015-10-29 09:49:52 +)

are available in the git repository at:

  git://github.com/lalrae/qemu.git tags/mips-20151030

for you to fetch changes up to 60270f85cc93d2d34e45b7679c374b1d771f0eeb:

  target-mips: fix updating XContext on mmu exception (2015-10-30 14:36:19 
+)


MIPS patches 2015-10-30

Changes:
* R6 CPU can be woken up by non-enabled interrupts
* PC fix in KVM
* CP0 XContext calculation fix
* various MIPS R6 updates


James Hogan (1):
  hw/mips_malta: Fix KVM PC initialisation

Leon Alrae (3):
  target-mips: move the test for enabled interrupts to a separate function
  target-mips: implement the CPU wake-up on non-enabled interrupts in R6
  target-mips: update writing to CP0.Status.KX/SX/UX in MIPS Release R6

Yongbok Kim (5):
  target-mips: Add enum for BREAK32
  target-mips: add PC, XNP reg numbers to RDHWR
  target-mips: Set Config5.XNP for R6 cores
  target-mips: add SIGRIE instruction
  target-mips: fix updating XContext on mmu exception

 hw/mips/mips_malta.c |  2 +-
 target-mips/cpu.c|  9 ---
 target-mips/cpu.h| 37 ++---
 target-mips/helper.c | 10 ---
 target-mips/helper.h |  2 ++
 target-mips/op_helper.c  | 64 
 target-mips/translate.c  | 43 +
 target-mips/translate_init.c |  4 +--
 8 files changed, 112 insertions(+), 59 deletions(-)

[Qemu-devel] [PULL 1/9] target-mips: move the test for enabled interrupts to a separate function

[Leon Alrae]

Signed-off-by: Leon Alrae 
---
 target-mips/cpu.c|  4 +++-
 target-mips/cpu.h| 29 +++--
 target-mips/helper.c |  3 ++-
 3 files changed, 20 insertions(+), 16 deletions(-)

diff --git a/target-mips/cpu.c b/target-mips/cpu.c
index 37880d2..bbfee45 100644
--- a/target-mips/cpu.c
+++ b/target-mips/cpu.c
@@ -58,7 +58,9 @@ static bool mips_cpu_has_work(CPUState *cs)
check for interrupts that can be taken. */
 if ((cs->interrupt_request & CPU_INTERRUPT_HARD) &&
 cpu_mips_hw_interrupts_pending(env)) {
-has_work = true;
+if (cpu_mips_hw_interrupts_enabled(env)) {
+has_work = true;
+}
 }
 
 /* MIPS-MT has the ability to halt the CPU.  */
diff --git a/target-mips/cpu.h b/target-mips/cpu.h
index f32a0fd..3799d26 100644
--- a/target-mips/cpu.h
+++ b/target-mips/cpu.h
@@ -637,23 +637,24 @@ static inline int cpu_mmu_index (CPUMIPSState *env, bool 
ifetch)
 return env->hflags & MIPS_HFLAG_KSU;
 }
 
-static inline int cpu_mips_hw_interrupts_pending(CPUMIPSState *env)
+static inline bool cpu_mips_hw_interrupts_enabled(CPUMIPSState *env)
 {
-int32_t pending;
-int32_t status;
-int r;
-
-if (!(env->CP0_Status & (1 << CP0St_IE)) ||
-(env->CP0_Status & (1 << CP0St_EXL)) ||
-(env->CP0_Status & (1 << CP0St_ERL)) ||
+return (env->CP0_Status & (1 << CP0St_IE)) &&
+!(env->CP0_Status & (1 << CP0St_EXL)) &&
+!(env->CP0_Status & (1 << CP0St_ERL)) &&
+!(env->hflags & MIPS_HFLAG_DM) &&
 /* Note that the TCStatus IXMT field is initialized to zero,
and only MT capable cores can set it to one. So we don't
need to check for MT capabilities here.  */
-(env->active_tc.CP0_TCStatus & (1 << CP0TCSt_IXMT)) ||
-(env->hflags & MIPS_HFLAG_DM)) {
-/* Interrupts are disabled */
-return 0;
-}
+!(env->active_tc.CP0_TCStatus & (1 << CP0TCSt_IXMT));
+}
+
+/* Check if there is pending and not masked out interrupt */
+static inline bool cpu_mips_hw_interrupts_pending(CPUMIPSState *env)
+{
+int32_t pending;
+int32_t status;
+bool r;
 
 pending = env->CP0_Cause & CP0Ca_IP_mask;
 status = env->CP0_Status & CP0Ca_IP_mask;
@@ -667,7 +668,7 @@ static inline int 
cpu_mips_hw_interrupts_pending(CPUMIPSState *env)
 /* A MIPS configured with compatibility or VInt (Vectored Interrupts)
treats the pending lines as individual interrupt lines, the status
lines are individual masks.  */
-r = pending & status;
+r = (pending & status) != 0;
 }
 return r;
 }
diff --git a/target-mips/helper.c b/target-mips/helper.c
index 01c4461..2d86323 100644
--- a/target-mips/helper.c
+++ b/target-mips/helper.c
@@ -759,7 +759,8 @@ bool mips_cpu_exec_interrupt(CPUState *cs, int 
interrupt_request)
 MIPSCPU *cpu = MIPS_CPU(cs);
 CPUMIPSState *env = &cpu->env;
 
-if (cpu_mips_hw_interrupts_pending(env)) {
+if (cpu_mips_hw_interrupts_enabled(env) &&
+cpu_mips_hw_interrupts_pending(env)) {
 /* Raise it */
 cs->exception_index = EXCP_EXT_INTERRUPT;
 env->error_code = 0;
-- 
2.1.0

[Qemu-devel] [PULL 9/9] target-mips: fix updating XContext on mmu exception

[Leon Alrae]

From: Yongbok Kim 

Correct updating XContext.Region field on mmu exceptions.
If Config3.CTXTC = 0 then the R field of XContext has to be updated
with the value of bits 63..62 of the virtual address upon a TLB
exception.
Also fixed the below line which overs 80 characters.

Signed-off-by: Yongbok Kim 
Reviewed-by: James Hogan 
Reviewed-by: Leon Alrae 
Signed-off-by: Leon Alrae 
---
 target-mips/helper.c | 7 ---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/target-mips/helper.c b/target-mips/helper.c
index 2d86323..b3fe816 100644
--- a/target-mips/helper.c
+++ b/target-mips/helper.c
@@ -293,9 +293,10 @@ static void raise_mmu_exception(CPUMIPSState *env, 
target_ulong address,
 (env->CP0_EntryHi & 0xFF) | (address & (TARGET_PAGE_MASK << 1));
 #if defined(TARGET_MIPS64)
 env->CP0_EntryHi &= env->SEGMask;
-env->CP0_XContext = (env->CP0_XContext & ((~0ULL) << (env->SEGBITS - 7))) |
-((address & 0xC000ULL) >> (55 - env->SEGBITS)) 
|
-((address & ((1ULL << env->SEGBITS) - 1) & 
0xE000ULL) >> 9);
+env->CP0_XContext =
+/* PTEBase */   (env->CP0_XContext & ((~0ULL) << (env->SEGBITS - 7))) |
+/* R */ (extract64(address, 62, 2) << (env->SEGBITS - 9)) |
+/* BadVPN2 */   (extract64(address, 13, env->SEGBITS - 13) << 4);
 #endif
 cs->exception_index = exception;
 env->error_code = error_code;
-- 
2.1.0

[Qemu-devel] [PULL 2/9] target-mips: implement the CPU wake-up on non-enabled interrupts in R6

[Leon Alrae]

In Release 6, the behaviour of WAIT has been modified to make it a
requirement that a processor that has disabled operation as a result of
executing a WAIT will resume operation on arrival of an interrupt even if
interrupts are not enabled.

Signed-off-by: Leon Alrae 
---
 target-mips/cpu.c | 7 ---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/target-mips/cpu.c b/target-mips/cpu.c
index bbfee45..639a24b 100644
--- a/target-mips/cpu.c
+++ b/target-mips/cpu.c
@@ -53,12 +53,13 @@ static bool mips_cpu_has_work(CPUState *cs)
 CPUMIPSState *env = &cpu->env;
 bool has_work = false;
 
-/* It is implementation dependent if non-enabled interrupts
-   wake-up the CPU, however most of the implementations only
+/* Prior to MIPS Release 6 it is implementation dependent if non-enabled
+   interrupts wake-up the CPU, however most of the implementations only
check for interrupts that can be taken. */
 if ((cs->interrupt_request & CPU_INTERRUPT_HARD) &&
 cpu_mips_hw_interrupts_pending(env)) {
-if (cpu_mips_hw_interrupts_enabled(env)) {
+if (cpu_mips_hw_interrupts_enabled(env) ||
+(env->insn_flags & ISA_MIPS32R6)) {
 has_work = true;
 }
 }
-- 
2.1.0

[Qemu-devel] [PULL 6/9] target-mips: add PC, XNP reg numbers to RDHWR

[Leon Alrae]

From: Yongbok Kim 

Add Performance Counter (4) and XNP (5) register numbers to RDHWR.
Add check_hwrena() to simplify access control checkings.
Add RDHWR support to microMIPS R6.

Signed-off-by: Yongbok Kim 
Reviewed-by: Leon Alrae 
Signed-off-by: Leon Alrae 
---
 target-mips/cpu.h   |  1 +
 target-mips/helper.h|  2 ++
 target-mips/op_helper.c | 64 +++--
 target-mips/translate.c | 28 +++---
 4 files changed, 63 insertions(+), 32 deletions(-)

diff --git a/target-mips/cpu.h b/target-mips/cpu.h
index c68681d..fa919c1 100644
--- a/target-mips/cpu.h
+++ b/target-mips/cpu.h
@@ -469,6 +469,7 @@ struct CPUMIPSState {
 #define CP0C5_CV 29
 #define CP0C5_EVA28
 #define CP0C5_MSAEn  27
+#define CP0C5_XNP13
 #define CP0C5_UFE9
 #define CP0C5_FRE8
 #define CP0C5_SBRI   6
diff --git a/target-mips/helper.h b/target-mips/helper.h
index d8cc766..95b9149 100644
--- a/target-mips/helper.h
+++ b/target-mips/helper.h
@@ -358,6 +358,8 @@ DEF_HELPER_1(rdhwr_cpunum, tl, env)
 DEF_HELPER_1(rdhwr_synci_step, tl, env)
 DEF_HELPER_1(rdhwr_cc, tl, env)
 DEF_HELPER_1(rdhwr_ccres, tl, env)
+DEF_HELPER_1(rdhwr_performance, tl, env)
+DEF_HELPER_1(rdhwr_xnp, tl, env)
 DEF_HELPER_2(pmon, void, env, int)
 DEF_HELPER_1(wait, void, env)
 
diff --git a/target-mips/op_helper.c b/target-mips/op_helper.c
index 6739fff..056d53b 100644
--- a/target-mips/op_helper.c
+++ b/target-mips/op_helper.c
@@ -1357,6 +1357,13 @@ void helper_mtc0_hwrena(CPUMIPSState *env, target_ulong 
arg1)
 {
 uint32_t mask = 0x000F;
 
+if ((env->CP0_Config1 & (1 << CP0C1_PC)) &&
+(env->insn_flags & ISA_MIPS32R6)) {
+mask |= (1 << 4);
+}
+if (env->insn_flags & ISA_MIPS32R6) {
+mask |= (1 << 5);
+}
 if (env->CP0_Config3 & (1 << CP0C3_ULRI)) {
 mask |= (1 << 29);
 
@@ -2185,53 +2192,52 @@ void helper_deret(CPUMIPSState *env)
 }
 #endif /* !CONFIG_USER_ONLY */
 
-target_ulong helper_rdhwr_cpunum(CPUMIPSState *env)
+static inline void check_hwrena(CPUMIPSState *env, int reg)
 {
-if ((env->hflags & MIPS_HFLAG_CP0) ||
-(env->CP0_HWREna & (1 << 0)))
-return env->CP0_EBase & 0x3ff;
-else
-do_raise_exception(env, EXCP_RI, GETPC());
+if ((env->hflags & MIPS_HFLAG_CP0) || (env->CP0_HWREna & (1 << reg))) {
+return;
+}
+do_raise_exception(env, EXCP_RI, GETPC());
+}
 
-return 0;
+target_ulong helper_rdhwr_cpunum(CPUMIPSState *env)
+{
+check_hwrena(env, 0);
+return env->CP0_EBase & 0x3ff;
 }
 
 target_ulong helper_rdhwr_synci_step(CPUMIPSState *env)
 {
-if ((env->hflags & MIPS_HFLAG_CP0) ||
-(env->CP0_HWREna & (1 << 1)))
-return env->SYNCI_Step;
-else
-do_raise_exception(env, EXCP_RI, GETPC());
-
-return 0;
+check_hwrena(env, 1);
+return env->SYNCI_Step;
 }
 
 target_ulong helper_rdhwr_cc(CPUMIPSState *env)
 {
-if ((env->hflags & MIPS_HFLAG_CP0) ||
-(env->CP0_HWREna & (1 << 2))) {
+check_hwrena(env, 2);
 #ifdef CONFIG_USER_ONLY
-return env->CP0_Count;
+return env->CP0_Count;
 #else
-return (int32_t)cpu_mips_get_count(env);
+return (int32_t)cpu_mips_get_count(env);
 #endif
-} else {
-do_raise_exception(env, EXCP_RI, GETPC());
-}
-
-return 0;
 }
 
 target_ulong helper_rdhwr_ccres(CPUMIPSState *env)
 {
-if ((env->hflags & MIPS_HFLAG_CP0) ||
-(env->CP0_HWREna & (1 << 3)))
-return env->CCRes;
-else
-do_raise_exception(env, EXCP_RI, GETPC());
+check_hwrena(env, 3);
+return env->CCRes;
+}
 
-return 0;
+target_ulong helper_rdhwr_performance(CPUMIPSState *env)
+{
+check_hwrena(env, 4);
+return env->CP0_Performance0;
+}
+
+target_ulong helper_rdhwr_xnp(CPUMIPSState *env)
+{
+check_hwrena(env, 5);
+return (env->CP0_Config5 >> CP0C5_XNP) & 1;
 }
 
 void helper_pmon(CPUMIPSState *env, int function)
diff --git a/target-mips/translate.c b/target-mips/translate.c
index 3105c7f..bacac2b 100644
--- a/target-mips/translate.c
+++ b/target-mips/translate.c
@@ -10333,7 +10333,7 @@ static void gen_flt3_arith (DisasContext *ctx, uint32_t 
opc,
 }
 }
 
-static void gen_rdhwr(DisasContext *ctx, int rt, int rd)
+static void gen_rdhwr(DisasContext *ctx, int rt, int rd, int sel)
 {
 TCGv t0;
 
@@ -10361,6 +10361,22 @@ static void gen_rdhwr(DisasContext *ctx, int rt, int 
rd)
 gen_helper_rdhwr_ccres(t0, cpu_env);
 gen_store_gpr(t0, rt);
 break;
+case 4:
+check_insn(ctx, ISA_MIPS32R6);
+if (sel != 0) {
+/* Performance counter registers are not implemented other than
+ * control register 0.
+ */
+generate_exception(ctx, EXCP_RI);
+}
+gen_helper_rdhwr_performance(t0, cpu_env);
+gen_store_gpr(t0, rt);
+break;
+case 5:
+check_insn(ctx, ISA_MIPS32R6);
+gen_helper_rdhwr_xnp(t0, cpu

[Qemu-devel] [PULL 5/9] hw/mips_malta: Fix KVM PC initialisation

[Leon Alrae]

From: James Hogan 

Commit 71c199c81d29 ("mips_malta: provide ememsize env variable to
kernels") changed the meaning of loaderparams.ram_size to be the whole
of RAM rather than just the low part below where the boot code is placed
for KVM, but it didn't update the PC initialisation for KVM to use
ram_low_size. Fix that now.

Fixes: 71c199c81d29 ("mips_malta: provide ememsize env variable to kernels")
Signed-off-by: James Hogan 
Cc: Paul Burton 
Cc: Leon Alrae 
Cc: Aurelien Jarno 
Reviewed-by: Aurelien Jarno 
Reviewed-by: Leon Alrae 
Signed-off-by: Leon Alrae 
---
 hw/mips/mips_malta.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/mips/mips_malta.c b/hw/mips/mips_malta.c
index c1f570a..91c36ba 100644
--- a/hw/mips/mips_malta.c
+++ b/hw/mips/mips_malta.c
@@ -901,7 +901,7 @@ static void main_cpu_reset(void *opaque)
 
 if (kvm_enabled()) {
 /* Start running from the bootloader we wrote to end of RAM */
-env->active_tc.PC = 0x4000 + loaderparams.ram_size;
+env->active_tc.PC = 0x4000 + loaderparams.ram_low_size;
 }
 }
 
-- 
2.1.0

[Qemu-devel] [PULL 7/9] target-mips: Set Config5.XNP for R6 cores

[Leon Alrae]

From: Yongbok Kim 

Set Config5.XNP for R6 cores to indicate the extended LL/SC family
of instructions NOT present.

Signed-off-by: Yongbok Kim 
Reviewed-by: Leon Alrae 
Signed-off-by: Leon Alrae 
---
 target-mips/translate_init.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/target-mips/translate_init.c b/target-mips/translate_init.c
index 1b45884..bb33c7c 100644
--- a/target-mips/translate_init.c
+++ b/target-mips/translate_init.c
@@ -447,7 +447,7 @@ static const mips_def_t mips_defs[] =
(1 << CP0C3_RXI) | (1U << CP0C3_M),
 .CP0_Config4 = MIPS_CONFIG4 | (0xfc << CP0C4_KScrExist) |
(3 << CP0C4_IE) | (1U << CP0C4_M),
-.CP0_Config5 = MIPS_CONFIG5 | (1 << CP0C5_LLB),
+.CP0_Config5 = MIPS_CONFIG5 | (1 << CP0C5_XNP) | (1 << CP0C5_LLB),
 .CP0_Config5_rw_bitmask = (1 << CP0C5_SBRI) | (1 << CP0C5_FRE) |
   (1 << CP0C5_UFE),
 .CP0_LLAddr_rw_bitmask = 0,
@@ -665,7 +665,7 @@ static const mips_def_t mips_defs[] =
(1 << CP0C3_RXI) | (1 << CP0C3_LPA),
 .CP0_Config4 = MIPS_CONFIG4 | (1U << CP0C4_M) | (3 << CP0C4_IE) |
(0xfc << CP0C4_KScrExist),
-.CP0_Config5 = MIPS_CONFIG5 | (1 << CP0C5_LLB),
+.CP0_Config5 = MIPS_CONFIG5 | (1 << CP0C5_XNP) | (1 << CP0C5_LLB),
 .CP0_Config5_rw_bitmask = (1 << CP0C5_MSAEn) | (1 << CP0C5_SBRI) |
   (1 << CP0C5_FRE) | (1 << CP0C5_UFE),
 .CP0_LLAddr_rw_bitmask = 0,
-- 
2.1.0

[Qemu-devel] [PULL 3/9] target-mips: update writing to CP0.Status.KX/SX/UX in MIPS Release R6

[Leon Alrae]

Implement the relationship between CP0.Status.KX, SX and UX. It should not
be possible to set UX bit if SX is 0, the same applies for setting SX if
KX is 0.

Signed-off-by: Leon Alrae 
---
 target-mips/cpu.h | 7 ++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/target-mips/cpu.h b/target-mips/cpu.h
index 3799d26..c68681d 100644
--- a/target-mips/cpu.h
+++ b/target-mips/cpu.h
@@ -1001,7 +1001,12 @@ static inline void cpu_mips_store_status(CPUMIPSState 
*env, target_ulong val)
 
 if (env->insn_flags & ISA_MIPS32R6) {
 bool has_supervisor = extract32(mask, CP0St_KSU, 2) == 0x3;
-
+#if defined(TARGET_MIPS64)
+uint32_t ksux = (1 << CP0St_KX) & val;
+ksux |= (ksux >> 1) & val; /* KX = 0 forces SX to be 0 */
+ksux |= (ksux >> 1) & val; /* SX = 0 forces UX to be 0 */
+val = (val & ~(7 << CP0St_UX)) | ksux;
+#endif
 if (has_supervisor && extract32(val, CP0St_KSU, 2) == 0x3) {
 mask &= ~(3 << CP0St_KSU);
 }
-- 
2.1.0

[Qemu-devel] [PULL 8/9] target-mips: add SIGRIE instruction

[Leon Alrae]

From: Yongbok Kim 

Add SIGRIE (Signal Reserved Instruction Exception) for both MIPS and
microMIPS.
The instruction allows to use the 16-bit code field for software use.
This instruction is introduced by and required as of Release 6.

Signed-off-by: Yongbok Kim 
Reviewed-by: Leon Alrae 
Signed-off-by: Leon Alrae 
---
 target-mips/translate.c | 12 +++-
 1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/target-mips/translate.c b/target-mips/translate.c
index bacac2b..5626647 100644
--- a/target-mips/translate.c
+++ b/target-mips/translate.c
@@ -323,6 +323,7 @@ enum {
 OPC_TLTIU= (0x0B << 16) | OPC_REGIMM,
 OPC_TEQI = (0x0C << 16) | OPC_REGIMM,
 OPC_TNEI = (0x0E << 16) | OPC_REGIMM,
+OPC_SIGRIE   = (0x17 << 16) | OPC_REGIMM,
 OPC_SYNCI= (0x1F << 16) | OPC_REGIMM,
 
 OPC_DAHI = (0x06 << 16) | OPC_REGIMM,
@@ -12031,7 +12032,8 @@ enum {
 LSA = 0x0f,
 ALIGN = 0x1f,
 EXT = 0x2c,
-POOL32AXF = 0x3c
+POOL32AXF = 0x3c,
+SIGRIE = 0x3f
 };
 
 /* POOL32AXF encoding of minor opcode field extension */
@@ -13655,6 +13657,10 @@ static void decode_micromips32_opc(CPUMIPSState *env, 
DisasContext *ctx)
 case BREAK32:
 generate_exception_end(ctx, EXCP_BREAK);
 break;
+case SIGRIE:
+check_insn(ctx, ISA_MIPS32R6);
+generate_exception_end(ctx, EXCP_RI);
+break;
 default:
 pool32a_invalid:
 MIPS_INVAL("pool32a");
@@ -18973,6 +18979,10 @@ static void decode_opc(CPUMIPSState *env, DisasContext 
*ctx)
 check_insn_opc_removed(ctx, ISA_MIPS32R6);
 gen_trap(ctx, op1, rs, -1, imm);
 break;
+case OPC_SIGRIE:
+check_insn(ctx, ISA_MIPS32R6);
+generate_exception_end(ctx, EXCP_RI);
+break;
 case OPC_SYNCI:
 check_insn(ctx, ISA_MIPS32R2);
 /* Break the TB to be able to sync copied instructions
-- 
2.1.0

[Qemu-devel] [PULL 4/9] target-mips: Add enum for BREAK32

[Leon Alrae]

From: Yongbok Kim 

Add enum for BREAK32

Signed-off-by: Yongbok Kim 
Reviewed-by: Leon Alrae 
Reviewed-by: Aurelien Jarno 
Signed-off-by: Leon Alrae 
---
 target-mips/translate.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/target-mips/translate.c b/target-mips/translate.c
index a10bfa3..3105c7f 100644
--- a/target-mips/translate.c
+++ b/target-mips/translate.c
@@ -12009,6 +12009,7 @@ enum {
 MODU = 0x7,
 
 /* The following can be distinguished by their lower 6 bits. */
+BREAK32 = 0x07,
 INS = 0x0c,
 LSA = 0x0f,
 ALIGN = 0x1f,
@@ -13629,7 +13630,7 @@ static void decode_micromips32_opc(CPUMIPSState *env, 
DisasContext *ctx)
 case POOL32AXF:
 gen_pool32axf(env, ctx, rt, rs);
 break;
-case 0x07:
+case BREAK32:
 generate_exception_end(ctx, EXCP_BREAK);
 break;
 default:
-- 
2.1.0

[Qemu-devel] [PATCH v3 00/11] vl.c: Error message rework

[Eduardo Habkost]

Changes v2 -> v3:
* Removed patch: "vl.c: Convert error sentences to simpler phrases"
* Removed patch: "vl.c: Reword -machine help error messages"
* Removed patch: "vl.c: Reword fw_cfg name prefix warning"
* Removed patch: "vl.c: Use US spelling for 'unrecognized'"
* New patch: "vl.c: Change 'fail to parse' error message to 'failed to parse'"
* Squashed "vl.c: trivial: Don't wrap lines unnecessarily"
  into "vl.c: Replace fprintf(stderr) with error_report()"

Changes v1 -> v2:
* Extra patches for many suggestions I got when changing vl.c to use
  error_report()

Eduardo Habkost (11):
  vl.c: Replace fprintf(stderr) with error_report()
  vl.c: Use error_report() when reporting shutdown signal
  vl.c: Remove periods and exclamation points from error messages
  vl.c: Use "warning:" prefix consistently on warnings
  vl.c: Use "cannot" instead of "can not" in error messages
  vl.c: Use 'quotes' instead of `quotes' in messages
  vl.c: Remove unnecessary uppercase in error messages
  vl.c: Change "fail to parse" error message to "failed to parse"
  vl.c: Simplify "ignoring deprecated option" warnings
  vl.c: Reword -no-kvm-pit-reinjection deprecation warning
  vl.c: Use "%s support is disabled" error messages consistently

 vl.c | 256 +--
 1 file changed, 125 insertions(+), 131 deletions(-)

-- 
2.1.0