Re: [Qemu-devel] [PATCH 1/4] error: Strip trailing '\n' from error string arguments (again)

[Markus Armbruster]

Eric Blake  writes:

> On 08/03/2016 05:37 AM, Markus Armbruster wrote:
>> Commit 9af9e0f, 6daf194d, be62a2eb and 312fd5f got rid of a bunch, but
>> they keep coming back.  checkpatch.pl tries to flag them since commit
>> 5d596c2, but it's not very good at it.  Offenders tracked down with
>> Coccinelle script scripts/coccinelle/err-bad-newline.cocci, an updated
>> version of the script from commit 312fd5f.
>> 
>> Signed-off-by: Markus Armbruster 
>> ---
>
>> +++ b/scripts/coccinelle/err-bad-newline.cocci
>> @@ -0,0 +1,29 @@
>> +// Error messages should not contain newlines.  This script finds
>> +// messages that do.  Fixing them is manual.
>> +@r@
>> +expression errp, eno, cls, fmt;
>> +position p;
>> +@@
>> +(
>> +error_report(fmt, ...)@p
>
> So you use Coccinelle to find error messages...
>
>> +@script:python@
>> +fmt << r.fmt;
>> +p << r.p;
>> +@@
>> +if "\\n" in str(fmt):
>> +print "%s:%s:%s:%s" % (p[0].file, p[0].line, p[0].column, fmt)
>
> ...and python to then parse the format string and flag violations.
> Slick.  Does it still work across formats like "foo %" PRIdMAX "\n"?

Testing... yes, it does.

> Reviewed-by: Eric Blake 

Thanks!

Re: [Qemu-devel] fix incorrect identify implementation in nvme

[Markus Armbruster]

Keith Busch  writes:

> On Thu, Aug 04, 2016 at 09:42:13PM +0200, Christoph Hellwig wrote:
>> Third resent of this series after this didn't get picked up the
>> previous times.  The Qemu NVMe implementation mistakes the cns
>> field in the Identify command as a boolean.  This was never
>> true, and is actively harmful since NVMe1.1 (which the Qemu
>> device claims to support) supports more than two Identify variants.
>> 
>> We had to add a quirk in Linux to work around this behavior.
>
> Yes, these are great. Do we need to ping a maintainer to go through
> their tree, or can this be applied immediately? If need be, I can apply
> and send a pull request.

$ scripts/get_maintainer.pl -f hw/block/nvme.c 
Keith Busch  (supporter:nvme)
Kevin Wolf  (supporter:Block layer core)
Max Reitz  (supporter:Block layer core)
qemu-bl...@nongnu.org (open list:nvme)
qemu-devel@nongnu.org (open list:All patches CC here)

Send a pull request (assuming you have a properly signed PGP key).

Re: [Qemu-devel] qtest protocol: should memset/read/write etc of a size of 0 bytes be permitted?

[Markus Armbruster]

Eric Blake  writes:

> On 08/04/2016 12:46 PM, Peter Maydell wrote:
>> I've upgraded to a more recent version of clang, which now produces
>> undefined-behaviour warnings for passing NULL pointers to some library
>> functions. One of the things it has shown up is that some of the
>> qtest tests ask for "memset" with size zero. In our current implementation
>> this results in qtest.c calling g_malloc(0), which returns NULL, and
>
> I never understood why glib made that choice on g_malloc(0). I would
> much prefer it to ALWAYS return something, just as glibc malloc(0) does.

I guess it made the choice because unlike malloc(0), it can't cause
confusion.  malloc() returning null can mean either error or nothing.
The caller should pick nothing when it passed zero.  g_malloc()
returning null can only mean nothing.

>> then calling memset(NULL, chr, 0), which is UB.
>
> Indeed, although I really wish POSIX could be loosened to say that the
> source pointer is untouched if the length is 0 (I've debated about
> filing a POSIX bug report to that effect, but have not done so yet), so
> that the UB only happens when passing NULL with a non-zero size.

For me, this is one of those pointless UBs, whose only effect on the
real world is wasting programmer time.  Seriously, what else could a
sane memset(NULL, 0, 0) do?  What could anyone gain from an insane
memset() other than the glee of telling people "your program is wrong,
and you're !".

The mission of this standard is to codify existing practice.  Show me a
real-world implementation of memset() that does something other than
nothing when passed a zero size, and whose maintainers don't consider
that a bug in need of fixing.

>> So should we:
>> (1) declare the qtest protocol commands 'memset', 'read', 'write'
>> etc which operate on a lump of guest memory of specified size to
>> support size == 0 as meaning "do nothing"
>
> My preference - even if we have to special case things to avoid UB at
> the lower level, presenting well-defined behavior at the upper level is
> easier to think about.
>
>> (2) declare that size == 0 is not valid and make it return a failure
>> code back down the qtest pipe (and fix the offending tests)
>
> Doable, but not as fun to audit, and not my preference.

Concur.  We shouldn't slavishly replicate an underlying interface's
complexities when we can just as well provide a more regular interface
instead.

Re: [Qemu-devel] [PULL 1/2] spapr: Error out when CPU hotplug is attempted on older pseries machines

[David Gibson]

On Thu, Aug 04, 2016 at 04:14:53PM +0530, Bharata B Rao wrote:
> On Wed, Aug 03, 2016 at 03:25:50PM +1000, David Gibson wrote:
> > From: Bharata B Rao 
> > 
> > CPU hotplug and coldplug aren't supported prior to pseries-2.7.  Further,
> > earlier machine types don't use CPU core objects at all.  These mean that
> > query-hotpluggable-cpus and coldplug on older pseries machines will crash
> > QEMU.  It also means that hotpluggable_cpus flag in query-machines will
> > be incorrectly set to true for pseries < 2.7, since it is based on the
> > presence of the query_hotpluggable_cpus hook.
> > 
> > - Don't assign the query_hotpluggable_cpus hook for pseries < 2.7
> > - query_hotpluggable_cpus should therefore never be called on pseries <
> >   2.7, so add an assert
> > - spapr_core_pre_plug() should fail hot/cold plug attempts for pseries <
> >   2.7, since core objects are never used there
> > - spapr_core_plug() should therefore never be called for pseries < 2.7, so
> >   add an assert.
> > 
> > Signed-off-by: Bharata B Rao 
> > [dwg: Change from query_hotpluggable_cpus returning NULL for pseries < 2.7
> >  to not being called at all, reword commit message for accuracy]
> > Signed-off-by: David Gibson 
> > ---
> >  hw/ppc/spapr.c  |  7 ++-
> >  hw/ppc/spapr_cpu_core.c | 19 ++-
> >  2 files changed, 12 insertions(+), 14 deletions(-)
> > 
> > diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> > index fbbd051..bce2371 100644
> > --- a/hw/ppc/spapr.c
> > +++ b/hw/ppc/spapr.c
> > @@ -2376,8 +2376,11 @@ static HotpluggableCPUList 
> > *spapr_query_hotpluggable_cpus(MachineState *machine)
> >  int i;
> >  HotpluggableCPUList *head = NULL;
> >  sPAPRMachineState *spapr = SPAPR_MACHINE(machine);
> > +sPAPRMachineClass *smc = SPAPR_MACHINE_GET_CLASS(machine);
> >  int spapr_max_cores = max_cpus / smp_threads;
> > 
> > +g_assert(smc->dr_cpu_enabled);
> > +
> >  for (i = 0; i < spapr_max_cores; i++) {
> >  HotpluggableCPUList *list_item = g_new0(typeof(*list_item), 1);
> >  HotpluggableCPU *cpu_item = g_new0(typeof(*cpu_item), 1);
> > @@ -2432,7 +2435,9 @@ static void spapr_machine_class_init(ObjectClass *oc, 
> > void *data)
> >  hc->plug = spapr_machine_device_plug;
> >  hc->unplug = spapr_machine_device_unplug;
> >  mc->cpu_index_to_socket_id = spapr_cpu_index_to_socket_id;
> > -mc->query_hotpluggable_cpus = spapr_query_hotpluggable_cpus;
> > +if (smc->dr_cpu_enabled) {
> > +mc->query_hotpluggable_cpus = spapr_query_hotpluggable_cpus;
> > +}
> > 
> >  smc->dr_lmb_enabled = true;
> >  smc->dr_cpu_enabled = true;
> 
> Unfortunately smc->dr_cpu_enabled is always false when you set
> mc->query_hotpluggable_cpus and will be set to true immediately
> afterwards as seen in above hunk.
> 
> This leads to query-hotpluggable-cpus being unavailable for all
> machine type versions. Your first version of setting
> mc->query_hotpluggable_cpus to NULL explicitly for 2.6 and downwards
> was correct.

*facepalm* I can't believe I forgot to check that.


In fact.. we could just replace dr_lmb_enabled with the NULL or
not-NULL status of query_hotpluggable_cpus.

I'll send a fix shortly.

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


signature.asc
Description: PGP signature

Re: [Qemu-devel] [PATCH 3/7] vdi: Use QEMU UUID API

[Markus Armbruster]

Eric Blake  writes:

> On 08/04/2016 12:58 PM, Stefan Weil wrote:
>> Hi,
>> 
>> On 08/02/16 11:18, Fam Zheng wrote:
>>> The QEMU UUID api, including the data structure (qemu_uuid_t), is fully
>>> compatible with libuuid.  Use it, and remove the unused code.
>>>
>>> Signed-off-by: Fam Zheng 
>>> ---
>>>  block/vdi.c | 49 ++---
>>>  1 file changed, 10 insertions(+), 39 deletions(-)
>>>
>
>>> @@ -182,10 +153,10 @@ typedef struct {
>>>  uint32_t block_extra;   /* unused here */
>>>  uint32_t blocks_in_image;
>>>  uint32_t blocks_allocated;
>>> -uuid_t uuid_image;
>>> -uuid_t uuid_last_snap;
>>> -uuid_t uuid_link;
>>> -uuid_t uuid_parent;
>>> +qemu_uuid_t uuid_image;
>>> +qemu_uuid_t uuid_last_snap;
>>> +qemu_uuid_t uuid_link;
>>> +qemu_uuid_t uuid_parent;
>> 
>> As far as I remember, _t should be avoided because that postfix is
>> reserved by POSIX. Should we use qemu_uuid, or can we ignore POSIX
>> because the type name uses the qemu_ prefix?
>
> Correct that POSIX reserved all _t (which is a bit broad, oh well), and
> also correct that we can take the risk of using it anyways (but if POSIX
> ever standardizes something, we get to keep both of our broken pieces).
>
>
>> Either with qemu_uuid_t or with qemu_uuid:
>
> I thought our coding standard preferred QemuUUID or something similar in
> camelcase, rather than lower case.

Correct.  It's ugly (in my opinion), but we should stick to it, so it's
at least consistently ugly.

> But now we are just painting a
> bikeshed, so I won't strongly object.

I'd prefer QemuUUID.

Re: [Qemu-devel] [PATCH v6 1/4] vfio: Mediated device Core driver

[Kirti Wankhede]

On 8/4/2016 12:51 PM, Tian, Kevin wrote:
>> From: Kirti Wankhede [mailto:kwankh...@nvidia.com]
>> Sent: Thursday, August 04, 2016 3:04 AM
>>
>>
>> 2. Physical device driver interface
>> This interface provides vendor driver the set APIs to manage physical
>> device related work in their own driver. APIs are :
>> - supported_config: provide supported configuration list by the vendor
>>  driver
>> - create: to allocate basic resources in vendor driver for a mediated
>>device.
>> - destroy: to free resources in vendor driver when mediated device is
>> destroyed.
>> - reset: to free and reallocate resources in vendor driver during reboot
> 
> Currently I saw 'reset' callback only invoked from VFIO ioctl path. Do 
> you think whether it makes sense to expose a sysfs 'reset' node too,
> similar to what people see under a PCI device node?
> 

All vendor drivers might not support reset of mdev from sysfs. But those
who want to support can expose 'reset' node using 'mdev_attr_groups' of
'struct parent_ops'.


>> - start: to initiate mediated device initialization process from vendor
>>   driver
>> - shutdown: to teardown mediated device resources during teardown.
> 
> I think 'shutdown' should be 'stop' based on actual code.
>

Thanks for catching that, yes I missed to updated here.

Thanks,
Kirti

> Thanks
> Kevin
> 

[Qemu-devel] [RFC PATCH 4/4] Enable aarch64 mttcg litmus tests in travis CI

[Pranith Kumar]

Signed-off-by: Pranith Kumar 
---
 .travis.yml | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/.travis.yml b/.travis.yml
index 881ba9d..f84f800 100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -33,6 +33,7 @@ addons:
   - libvte-2.90-dev
   - sparse
   - uuid-dev
+  - gcc-aarch64-linux-gnu
 
 # The channel name "irc.oftc.net#qemu" is encrypted against qemu/qemu
 # to prevent IRC notifications from forks. This was created using:
@@ -102,3 +103,5 @@ matrix:
 after_success:
   - cd tests/tcg/mttcg/x86
   - make tests
+  - cd ../aarch64
+  - make tests
-- 
2.9.2

[Qemu-devel] [RFC PATCH 1/4] tests/tcg: Add x86 mttcg litmus test

[Pranith Kumar]

This adds the x86 store-after-load re-ordering litmus test.

Most of the supporting files are mostly unmodified and generated by
the litmus tool.

Signed-off-by: Pranith Kumar 
---
 tests/tcg/mttcg/x86/Makefile  |   42 ++
 tests/tcg/mttcg/x86/README.txt|   22 +
 tests/tcg/mttcg/x86/SAL.c |  491 
 tests/tcg/mttcg/x86/affinity.c|  159 +
 tests/tcg/mttcg/x86/affinity.h|   34 ++
 tests/tcg/mttcg/x86/comp.sh   |   10 +
 tests/tcg/mttcg/x86/litmus_rand.c |   64 +++
 tests/tcg/mttcg/x86/litmus_rand.h |   29 +
 tests/tcg/mttcg/x86/outs.c|  148 +
 tests/tcg/mttcg/x86/outs.h|   49 ++
 tests/tcg/mttcg/x86/run.sh|   56 ++
 tests/tcg/mttcg/x86/show.awk  |2 +
 tests/tcg/mttcg/x86/utils.c   | 1148 +
 tests/tcg/mttcg/x86/utils.h   |  275 +
 14 files changed, 2529 insertions(+)
 create mode 100644 tests/tcg/mttcg/x86/Makefile
 create mode 100644 tests/tcg/mttcg/x86/README.txt
 create mode 100644 tests/tcg/mttcg/x86/SAL.c
 create mode 100644 tests/tcg/mttcg/x86/affinity.c
 create mode 100644 tests/tcg/mttcg/x86/affinity.h
 create mode 100644 tests/tcg/mttcg/x86/comp.sh
 create mode 100644 tests/tcg/mttcg/x86/litmus_rand.c
 create mode 100644 tests/tcg/mttcg/x86/litmus_rand.h
 create mode 100644 tests/tcg/mttcg/x86/outs.c
 create mode 100644 tests/tcg/mttcg/x86/outs.h
 create mode 100755 tests/tcg/mttcg/x86/run.sh
 create mode 100644 tests/tcg/mttcg/x86/show.awk
 create mode 100644 tests/tcg/mttcg/x86/utils.c
 create mode 100644 tests/tcg/mttcg/x86/utils.h

diff --git a/tests/tcg/mttcg/x86/Makefile b/tests/tcg/mttcg/x86/Makefile
new file mode 100644
index 000..6b8fa37
--- /dev/null
+++ b/tests/tcg/mttcg/x86/Makefile
@@ -0,0 +1,42 @@
+GCC=gcc
+GCCOPTS=-D_GNU_SOURCE -DFORCE_AFFINITY -Wall -std=gnu99 -fomit-frame-pointer 
-O2 -pthread
+LINKOPTS=
+SRC=\
+ SAL.c\
+
+EXE=$(SRC:.c=.exe)
+T=$(SRC:.c=.t)
+
+all: $(EXE) $(T)
+
+clean:
+   /bin/rm -f *.o *.s *.t *.exe *~
+
+cleansource:
+   /bin/rm -f *.o *.c *.h *.s *~
+
+affinity.o: affinity.c
+   $(GCC) $(GCCOPTS) -O2 -c affinity.c
+
+outs.o: outs.c
+   $(GCC) $(GCCOPTS) -O2 -c outs.c
+
+utils.o: utils.c
+   $(GCC) $(GCCOPTS) -O2 -c utils.c
+
+litmus_rand.o: litmus_rand.c
+   $(GCC) $(GCCOPTS) -O2 -c litmus_rand.c
+
+UTILS=affinity.o outs.o utils.o litmus_rand.o
+
+%.exe:%.s $(UTILS)
+   $(GCC) $(GCCOPTS) $(LINKOPTS) -o $@ $(UTILS) $<
+
+%.s:%.c
+   $(GCC) $(GCCOPTS) -S $<
+
+%.t:%.s
+   awk -f show.awk $< > $@
+
+tests: all
+   ./run.sh
diff --git a/tests/tcg/mttcg/x86/README.txt b/tests/tcg/mttcg/x86/README.txt
new file mode 100644
index 000..98ce238
--- /dev/null
+++ b/tests/tcg/mttcg/x86/README.txt
@@ -0,0 +1,22 @@
+Tests produced by litmus for architecture X86 on linux 
+
+COMPILING
+  with command 'make [-j N]' or 'sh comp.sh'
+
+RUNNING ALL TESTS
+  with command 'sh run.sh'. Test result on standard output.
+
+RUNNING ONE TEST
+  Tests are .exe files, for instance SAL.exe, run it by './SAL.exe'
+
+RUNNING OPTIONS
+  Main options to the run.sh script and to .exe files:
+  -v be verbose (can be repeated).
+  -a  number of (logical) processors available, default 0.
+  The default value of 0 means that .exe files attempt
+  to infer the actual number of logical threads.
+  -s  one run operates on arrays of size , default 10.
+  -r  number of runs, default 10.
+
+  For more options see for instance './SAL.exe -help' and litmus documentation
+  
diff --git a/tests/tcg/mttcg/x86/SAL.c b/tests/tcg/mttcg/x86/SAL.c
new file mode 100644
index 000..1b66508
--- /dev/null
+++ b/tests/tcg/mttcg/x86/SAL.c
@@ -0,0 +1,491 @@
+//
+/*   the diy toolsuite  */
+/*  */
+/* Jade Alglave, University College London, UK. */
+/* Luc Maranget, INRIA Paris-Rocquencourt, France.  */
+/*  */
+/* This C source is a product of litmus7 and includes source that is*/
+/* governed by the CeCILL-B license.*/
+//
+/* Parameters */
+#define SIZE_OF_TEST 10
+#define NUMBER_OF_RUN 10
+#define AVAIL 0
+#define STRIDE 1
+#define MAX_LOOP 0
+#define N 2
+#define AFF_INCR (0)
+/* Includes */
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include "utils.h"
+#include "outs.h"
+#include "affinity.h"
+
+/* params */
+typedef struct {
+  int verbose;
+  int size_of_test,max_run;
+  int stride;
+  aff_mode_t aff_mode;
+  int ncpus, ncpus_used;
+  int do_change;
+} param_t;
+
+
+/* Full memory barrier */
+inline static

[Qemu-devel] [RFC PATCH 0/4] Add MTTCG litmus tests

[Pranith Kumar]

This patch series adds litmus tests to test that MTTCG is properly
following the consistency semantics. This series has the one possible
re-ordering test on x86 and some subset of tests for ARM64.

We run these tests using the qemu-user binary of the corresponding
architecture(set the QEMU env variable in run.sh).

These tests were generated using the litmus tool available here:
http://diy.inria.fr/sources/index.html

The tests can be run locally by running 'make tests' in the test
directory. This has been added to travis-ci so that the test is run
automatically.

The tests have been verified to pass with the fence generation patch
series. The tests will fail otherwise.

Pranith Kumar (4):
  tests/tcg: Add x86 mttcg litmus test
  Enable x86 mttcg litmus tests in travis CI
  tests/tcg: Add mttcg ARM64 litmus tests
  Enable aarch64 mttcg litmus tests in travis CI

 .travis.yml  |6 +
 tests/tcg/mttcg/aarch64/ARMARM00.c   |  501 +
 tests/tcg/mttcg/aarch64/ARMARM01.c   |  504 +
 tests/tcg/mttcg/aarch64/ARMARM02.c   |  571 +++
 tests/tcg/mttcg/aarch64/ARMARM03.c   |  498 +
 tests/tcg/mttcg/aarch64/ARMARM04+BIS.c   |  556 +++
 tests/tcg/mttcg/aarch64/ARMARM04+TER.c   |  538 ++
 tests/tcg/mttcg/aarch64/ARMARM04.c   |  556 +++
 tests/tcg/mttcg/aarch64/ARMARM05.c   |  553 ++
 tests/tcg/mttcg/aarch64/ARMARM06+AP+AA.c |  581 +++
 tests/tcg/mttcg/aarch64/ARMARM06+AP+AP.c |  581 +++
 tests/tcg/mttcg/aarch64/ARMARM06.c   |  581 +++
 tests/tcg/mttcg/aarch64/Makefile |   52 ++
 tests/tcg/mttcg/aarch64/README.txt   |   22 +
 tests/tcg/mttcg/aarch64/affinity.c   |  159 +
 tests/tcg/mttcg/aarch64/affinity.h   |   34 +
 tests/tcg/mttcg/aarch64/comp.sh  |   30 +
 tests/tcg/mttcg/aarch64/litmus_rand.c|   64 ++
 tests/tcg/mttcg/aarch64/litmus_rand.h|   29 +
 tests/tcg/mttcg/aarch64/outs.c   |  148 
 tests/tcg/mttcg/aarch64/outs.h   |   49 ++
 tests/tcg/mttcg/aarch64/run.sh   |  351 +
 tests/tcg/mttcg/aarch64/show.awk |2 +
 tests/tcg/mttcg/aarch64/utils.c  | 1148 ++
 tests/tcg/mttcg/aarch64/utils.h  |  275 +++
 tests/tcg/mttcg/x86/Makefile |   42 ++
 tests/tcg/mttcg/x86/README.txt   |   22 +
 tests/tcg/mttcg/x86/SAL.c|  491 +
 tests/tcg/mttcg/x86/affinity.c   |  159 +
 tests/tcg/mttcg/x86/affinity.h   |   34 +
 tests/tcg/mttcg/x86/comp.sh  |   10 +
 tests/tcg/mttcg/x86/litmus_rand.c|   64 ++
 tests/tcg/mttcg/x86/litmus_rand.h|   29 +
 tests/tcg/mttcg/x86/outs.c   |  148 
 tests/tcg/mttcg/x86/outs.h   |   49 ++
 tests/tcg/mttcg/x86/run.sh   |   56 ++
 tests/tcg/mttcg/x86/show.awk |2 +
 tests/tcg/mttcg/x86/utils.c  | 1148 ++
 tests/tcg/mttcg/x86/utils.h  |  275 +++
 39 files changed, 10918 insertions(+)
 create mode 100644 tests/tcg/mttcg/aarch64/ARMARM00.c
 create mode 100644 tests/tcg/mttcg/aarch64/ARMARM01.c
 create mode 100644 tests/tcg/mttcg/aarch64/ARMARM02.c
 create mode 100644 tests/tcg/mttcg/aarch64/ARMARM03.c
 create mode 100644 tests/tcg/mttcg/aarch64/ARMARM04+BIS.c
 create mode 100644 tests/tcg/mttcg/aarch64/ARMARM04+TER.c
 create mode 100644 tests/tcg/mttcg/aarch64/ARMARM04.c
 create mode 100644 tests/tcg/mttcg/aarch64/ARMARM05.c
 create mode 100644 tests/tcg/mttcg/aarch64/ARMARM06+AP+AA.c
 create mode 100644 tests/tcg/mttcg/aarch64/ARMARM06+AP+AP.c
 create mode 100644 tests/tcg/mttcg/aarch64/ARMARM06.c
 create mode 100644 tests/tcg/mttcg/aarch64/Makefile
 create mode 100644 tests/tcg/mttcg/aarch64/README.txt
 create mode 100644 tests/tcg/mttcg/aarch64/affinity.c
 create mode 100644 tests/tcg/mttcg/aarch64/affinity.h
 create mode 100644 tests/tcg/mttcg/aarch64/comp.sh
 create mode 100644 tests/tcg/mttcg/aarch64/litmus_rand.c
 create mode 100644 tests/tcg/mttcg/aarch64/litmus_rand.h
 create mode 100644 tests/tcg/mttcg/aarch64/outs.c
 create mode 100644 tests/tcg/mttcg/aarch64/outs.h
 create mode 100755 tests/tcg/mttcg/aarch64/run.sh
 create mode 100644 tests/tcg/mttcg/aarch64/show.awk
 create mode 100644 tests/tcg/mttcg/aarch64/utils.c
 create mode 100644 tests/tcg/mttcg/aarch64/utils.h
 create mode 100644 tests/tcg/mttcg/x86/Makefile
 create mode 100644 tests/tcg/mttcg/x86/README.txt
 create mode 100644 tests/tcg/mttcg/x86/SAL.c
 create mode 100644 tests/tcg/mttcg/x86/affinity.c
 create mode 100644 tests/tcg/mttcg/x86/affinity.h
 create mode 100644 tests/tcg/mttcg/x86/comp.sh
 create mode 100644 tests/tcg/mttcg/x86/litmus_rand.c
 create mode 100644 tests/tcg/mttcg/x86/litmus_rand.h
 create mode 100644 tests/tcg/mttcg/x86/outs.c
 create mode 100644 tests/tcg/mttcg/x86/outs.h
 create mode 100755 

[Qemu-devel] [RFC PATCH 2/4] Enable x86 mttcg litmus tests in travis CI

[Pranith Kumar]

Enable the store-after-load litmus test to be run on travis-CI.

Signed-off-by: Pranith Kumar 
---
 .travis.yml | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/.travis.yml b/.travis.yml
index f30b10e..881ba9d 100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -99,3 +99,6 @@ matrix:
 - sudo apt-get build-dep -qq qemu
 - wget -O - 
http://people.linaro.org/~alex.bennee/qemu-submodule-git-seed.tar.xz | tar -xvJ
 - git submodule update --init --recursive
+after_success:
+  - cd tests/tcg/mttcg/x86
+  - make tests
-- 
2.9.2

[Qemu-devel] something broken in "v2.7.0-rc1" or just a behavior change?

[Dennis Luehring]

my scenario works with before "v2.7.0-rc1"
git "v2.5.0" OK
git "v2.6.0" OK
git "v2.7.0-rc0" OK
git "v2.7.0-rc1" FAILS

[qemu-git]
build qemu from git at ~2016/08/04

git rev-parse HEAD
09704e6ded83fa0bec14baf32f800f6512156ca0

[scenario]

qemu/sparc64-softmmu/qemu-system-sparc64 --version
QEMU emulator version 2.6.91 (v2.7.0-rc1-46-g09704e6-dirty), Copyright 
(c) 2003-2008 Fabrice Bellard


if i start qemu with (the other platforms are equaly started)

qemu/sparc64-softmmu/qemu-system-sparc64 -m 1GB -nographic -monitor 
telnet::4440,server,nowait -serial telnet::3000,server -kernel 
clfskernel-4.5.0 -initrd big_initrd.cpio


qemu prints at first:
QEMU waiting for connection on: disconnected:telnet::3000,server

then i open up a second console and start:
telnet localhost 3000
qemu prints
Trying 127.0.0.1...
Connected to localhost.

on the console where i started qemu it prints another (v2.7.0-rc0 does 
not print this line again):

QEMU waiting for connection on: disconnected:telnet::3000,server

and thats it - nothing more

same happens for mips64-softmmu/qemu-system-mips64, 
ppc64-softmmu/qemu-system-ppc64 and alpha-softmmu/qemu-system-alpha

something broken or a behavior change?

Re: [Qemu-devel] [QUESTION]stuck in SeaBIOS because of losing a SMI

[Xulei (Stone)]

>
>
>On 03/08/2016 11:43, Xulei (Stone) wrote:
>> Hi, all:
>> Recently I use a shell script to continuously reset a vm to see what may 
>> happen.
>> After one day, the vm is stuck. Looking from the following seabios log and
>> kvm trace log, it seems like losing a SMI or SeaBIOS can not handle a SMI.
>> This problem is reproducible on my machine (SeaBIOS 1.9.1, Qemu 2.6.0,
>> Kmod 4.4.11).
>
>Thanks for the report!
>
>The more interesting part of the trace would be around the pio_write at
>0xb2.  Also please try kernel commit c43203cab1e, which just went into 4.8.
>

kernel commit c43203cab1e can not solve my problem, but seems help me to
find a way to easy reproduce this problem.

I add a sleep mechnism in seabios before raising an SMI interrupt so as to
waiting for me to press keyboard quickly and repeatedly during the booting 
procedure. My vm will be stuck after several try times.

diff --git a/roms/seabios/src/fw/smm.c b/roms/seabios/src/fw/smm.c
--- a/roms/seabios/src/fw/smm.c
+++ b/roms/seabios/src/fw/smm.c
@@ -145,16 +146,26 @@ smm_save_and_copy(void)
 static void
 smm_relocate_and_restore(void)
 {
+ long long i = 0;
+ long long j =10;
 /* init APM status port */
 outb(0x01, PORT_SMI_STATUS);
 
+while(i++ < 10)
+{
+   if((i-j) == 0) 
+  {dprintf(1, ".");
+   j +=10;
+ }
+}
 /* raise an SMI interrupt */
 outb(0x00, PORT_SMI_CMD);

 /* wait until SMM code executed */
 while (inb(PORT_SMI_STATUS) != 0x00);

It seems kvm has something wrong in injecting SMI with common IRQ.
>Paolo
>
>> ==my shell script===
>> #!/bin/bash
>> while((1))
>> do
>> virsh reset VMNAME
>> sleep 1
>> done
>> 
>> ==kvm trace log===
>> CPU 0/KVM-13843 [020] d... 1025056.813494: kvm_entry: vcpu 0
>> CPU 0/KVM-13843 [020]  1025056.813495: kvm_exit: reason IO_INSTRUCTION 
>> rip 0xef10e info b30048 0
>> CPU 0/KVM-13843 [020]  1025056.813495: kvm_emulate_insn: 0:ef10e:e4 b3 
>> (prot32)
>> CPU 0/KVM-13843 [020]  1025056.813496: kvm_userspace_exit: reason 
>> KVM_EXIT_IO (2)
>> CPU 0/KVM-13843 [020]  1025056.813496: kvm_fpu: unload
>> CPU 0/KVM-13843 [020]  1025056.813497: kvm_pio: pio_read at 0xb3 size 1 
>> count 1 val 0x1
>> CPU 0/KVM-13843 [020]  1025056.813497: kvm_fpu: load
>> CPU 0/KVM-13843 [020] d... 1025056.813497: kvm_entry: vcpu 0
>> CPU 0/KVM-13843 [020]  1025056.813498: kvm_exit: reason IO_INSTRUCTION 
>> rip 0xef10e info b30048 0
>> CPU 0/KVM-13843 [020]  1025056.813498: kvm_emulate_insn: 0:ef10e:e4 b3 
>> (prot32)
>> CPU 0/KVM-13843 [020]  1025056.813499: kvm_userspace_exit: reason 
>> KVM_EXIT_IO (2)
>> CPU 0/KVM-13843 [020]  1025056.813499: kvm_fpu: unload
>> CPU 0/KVM-13843 [020]  1025056.813500: kvm_pio: pio_read at 0xb3 size 1 
>> count 1 val 0x1
>> CPU 0/KVM-13843 [020]  1025056.813500: kvm_fpu: load
>> 
>> ==my seabios log===
>> --- a/roms/seabios/src/fw/smm.c
>> +++ b/roms/seabios/src/fw/smm.c
>> @@ -65,7 +65,8 @@ handle_smi(u16 cs)
>>  u8 cmd = inb(PORT_SMI_CMD);
>>  struct smm_layout *smm = MAKE_FLATPTR(cs, 0);
>>  u32 rev = smm->cpu.i32.smm_rev & SMM_REV_MASK;
>> -dprintf(DEBUG_HDL_smi, "handle_smi cmd=%x smbase=%p\n", cmd, smm);
>> +if(cmd == 0x00) {
>> +dprintf(1, "handle_smi cmd=%x smbase=%p\n", cmd, smm);
>> +}
>>  
>>  if (smm == (void*)BUILD_SMM_INIT_ADDR) {
>>  // relocate SMBASE to 0xa
>> @@ -147,14 +148,14 @@ smm_relocate_and_restore(void)
>>  {
>>  /* init APM status port */
>>  outb(0x01, PORT_SMI_STATUS);
>> +   dprintf(1,"before SMI\n");
>> 
>>  /* raise an SMI interrupt */
>>  outb(0x00, PORT_SMI_CMD);
>> +dprintf(1,"after SMI=\n");
>> 
>>  /* wait until SMM code executed */
>>  while (inb(PORT_SMI_STATUS) != 0x00)
>>  ;
>> +   dprintf(1,"smm code executes complete\n");
>> 
>> And the log output like this:
>> 2016-08-03 16:23:15PCI: Using 00:02.0 for primary VGA
>> 2016-08-03 16:23:15smm_device_setup start
>> 2016-08-03 16:23:15init smm
>> 2016-08-03 16:23:15before SMI
>> 2016-08-03 16:23:15after SMI= > destroy it
>> 
>> As above, it is obviously that if bios doesn't handle_smi, PORT_SMI_STATUS is
>> always 0x01. smm_relocate_and_restore() will always in the while loop.
>> 
>> Why does bios handle_smi at this point, is this a kvm bug? or SeaBIOS bug?
>> 
>> --
>> Xulei (Stone)
>> 

[Qemu-devel] [Help]: Does qemu-system-aarch64 support virtio-9p? I got a problem when remap host file to guest in AArch64.

[Kevin Zhao]

Hi All,
 I have a problem may about Qemu and kindly need your help. Does
qemu-system-aarch64 support virtio-9p ?
 Recently I have tried to use qemu remapped the file from host to
guest. As I know, Qemu has supported this so long as guest kernel has
support 9p(virtfs). Reference to this link:
http://wiki.qemu.org/Documentation/9psetup
Fedora 24 AArch64 kernel has supported this:
[root@sha-win-225 ~]# lsmod | grep 9p
9p 56273  0
fscache87449  1 9p
9pnet_virtio9122  0
9pnet  83564  2 9p,9pnet_virtio
virtio_ring13866  5 virtio_net,virtio_pci,9pnet_
virtio,virtio_mmio,virtio_scsi
virtio  9467  5 virtio_net,virtio_pci,9pnet_
virtio,virtio_mmio,virtio_scsi

Now I use virsh to launch the VM, and the corresponding qemu command I
have pasted here:
http://paste.openstack.org/show/549225/.
You can see that:
* -fsdev
local,security_model=mapped,id=fsdev-fs0,path=/var/lib/libvirt/images/coreos
-device
virtio-9p-pci,id=fs0,fsdev=fsdev-fs0,mount_tag=share,bus=pci.2,addr=0x1*
 Here is the command that remapped the directory from host to guest.
After VM launched, I use the command to mount:
* mount -t 9p -o trans=virtio share /tmp/shared/
-oversion=9p2000.L,posixacl,cache=loose*
But mount command will be blocked and output nothing.
 The Qemu version is QEMU emulator version 2.5.0 (Debian
1:2.5+dfsg-5ubuntu10.2). Besides test fedora24 guest, I have got the same
problem in Debian jessie.
  Kindly need your help~You will be really appreciated.

Best Regards,
Kevin Zhao

Re: [Qemu-devel] [PATCH] MAINTAINERS: Add Fam and Jsnow for Bitmap support

[Fam Zheng]

On Thu, 08/04 14:18, John Snow wrote:
> These files are currently unmaintained.
> 
> I'm proposing that Fam and I co-maintain them; under the model that
> whomever between us isn't authoring a given series will be responsible
> for reviewing it.
> 
> Signed-off-by: John Snow 
> ---
>  MAINTAINERS | 14 ++
>  1 file changed, 14 insertions(+)
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index b6fb84e..0086f0e 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -1041,6 +1041,20 @@ F: block/qapi.c
>  F: qapi/block*.json
>  T: git git://repo.or.cz/qemu/armbru.git block-next
>  
> +Dirty Bitmaps
> +M: Fam Zheng 
> +M: John Snow 
> +L: qemu-bl...@nongnu.org
> +S: Supported
> +F: util/hbitmap.c
> +F: block/dirty-bitmap.c
> +F: include/qemu/hbitmap.h
> +F: include/block/dirty-bitmap.h
> +F: tests/test-hbitmap.c
> +F: docs/bitmaps.md
> +T: git git://github.com/famz/qemu.git bitmaps
> +T: git git://github.com/jnsnow/qemu.git bitmaps
> +
>  Character device backends
>  M: Paolo Bonzini 
>  S: Maintained
> -- 
> 2.7.4
> 

Acked-by: Fam Zheng 

Re: [Qemu-devel] [RFC PATCH 1/3] hw/arm/virt: add hotplug memory support

[kwangwoo....@sk.com]

Hi Igor,

Thank you for your guide to split the hotplug patch.
Currently I'm looking at the Linux kernel codes related with huge page size.

> -Original Message-
> From: Igor Mammedov [mailto:imamm...@redhat.com]
> Sent: Tuesday, August 02, 2016 9:18 PM
> To: Peter Maydell
> Cc: Xiao Guangrong; Eduardo Habkost; Michael S. Tsirkin; QEMU Developers; 
> 정우석(CHUNG WOO SUK) MS SW;
> qemu-arm; Shannon Zhao; Shannon Zhao; Paolo Bonzini; 김현철(KIM HYUNCHUL) MS SW; 
> 이광우(LEE KWANGWOO) MS
> SW; Richard Henderson
> Subject: Re: [Qemu-devel] [RFC PATCH 1/3] hw/arm/virt: add hotplug memory 
> support
> 
> On Tue, 2 Aug 2016 08:59:46 +0100
> Peter Maydell  wrote:
> 
> > On 1 August 2016 at 10:14, Igor Mammedov  wrote:
> > > On Mon, 1 Aug 2016 09:13:34 +0100
> > > Peter Maydell  wrote:
> > >> On 1 August 2016 at 08:46, Igor Mammedov  wrote:
> > >> > Base alignment comes from max supported hugepage size,
> > >>
> > >> Max hugepage size for any host? (if so, should be defined
> > >> in a common header somewhere)
> > >> Max hugepage size for ARM hosts? (if so, why is TCG
> > >> different from KVM?, and should still be in a common
> > >> header somewhere)
> > > It's the same for TCG but it probably doesn't matter much there,
> > > main usage is to provide better performance with KVM.
> > >
> > > So I'd say it's host depended (for x86 it's 1Gb),
> > > probably other values for ARM and PPC
> >
> > We probably don't want to make the memory layout depend
> > on the host architecture, though :-(
> Important here is not change it dynamically so it won't
> break migration.
> Otherwise it could be a value that makes a sense for
> the use-case where performance matters most, i.e. KVM
> which makes us to derive value from max supported page
> size for a KVM host (meaning guest is the same arch)
> 
> In case of x86 value is constant hardcoded in target
> specific code which applies to both KVM and TCG.
> 
> Perhaps there is a better way to handle it which I just
> don't see.
> 
> >
> > >>
> > >> > while
> > >> > size alignment should depend on backend's page size
> > >>
> > >> Which page size do you have in mind here? TARGET_PAGE_SIZE
> > >> is often not the right answer, since it doesn't
> > >> correspond either to the actual page size being used
> > >> by the host kernel or to the actual page size used
> > >> by the guest kernel...
> > > alignment comes from here: memory_region_get_alignment()
> > >
> > > exec:c
> > >MAX(page_size, QEMU_VMALLOC_ALIGN)
> > > so it's either backend's page size or a min chunk QEMU
> > > allocates memory to make KVM/valgrind/whatnot happy.
> >
> > Since that's always larger than TARGET_PAGE_SIZE
> > why are we checking for an alignment here that's
> > not actually sufficient to make things work?
> You mean following hunk:
> 
> +if (QEMU_ALIGN_UP(machine->maxram_size,
> + TARGET_PAGE_SIZE) != machine->maxram_size) {
> 
> It's a bit late to fix for x86 without breaking CLI,
> side effect would be inability to hotplug upto maxmem if maxmem
> is misaligned wrt used backend page size
> 
> ARCH_VIRT_HOTPLUG_MEM_ALIGN should be used instead of TARGET_PAGE_SIZE
> 
> PS:
> I haven't reviewed series yet, but I'd split this patch in 3
> to make review easier
>   1st - introduce machine level hotplug hooks
>   2nd - add MemoryHotplugState to VirtMachineState and initialize it
>   3rd - add virt_dimm_plug() handler

OK. I'll try to split it.

> >
> > thanks
> > -- PMM
> >

Best Regards,
Kwangwoo Lee

[Qemu-devel] [Bug 1490611]

[Nish Aravamudan]

** Attachment added: "attachment"
   https://bugs.launchpad.net/bugs/1490611/+attachment/4714298/+files/attachment

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

Title:
  Using qemu >=2.2.1 to convert raw->VHD (fixed) adds extra padding to
  the result file, which Microsoft Azure rejects as invalid

Status in QEMU:
  Fix Released
Status in qemu package in Ubuntu:
  Fix Released
Status in qemu source package in Xenial:
  Triaged

Bug description:
  [Impact]

   * Starting with a raw disk image, using "qemu-img convert" to convert
  from raw to VHD results in the output VHD file's virtual size being
  aligned to the nearest 516096 bytes (16 heads x 63 sectors per head x
  512 bytes per sector), instead of preserving the input file's size as
  the output VHD's virtual disk size.

   * Microsoft Azure requires that disk images (VHDs) submitted for
  upload have virtual sizes aligned to a megabyte boundary. (Ex. 4096MB,
  4097MB, 4098MB, etc. are OK, 4096.5MB is rejected with an error.) This
  is reflected in Microsoft's documentation: https://azure.microsoft.com
  /en-us/documentation/articles/virtual-machines-linux-create-upload-
  vhd-generic/

   * The fix for this bug is a backport from upstream.
  http://git.qemu.org/?p=qemu.git;a=commitdiff;h=fb9245c2610932d33ce14

  [Test Case]

   * This is reproducible with the following set of commands (including
  the Azure command line tools from https://github.com/Azure/azure-
  xplat-cli). For the following example, I used qemu version 2.2.1:

  $ dd if=/dev/zero of=source-disk.img bs=1M count=4096

  $ stat source-disk.img
    File: ‘source-disk.img’
    Size: 4294967296  Blocks: 798656 IO Block: 4096   regular file
  Device: fc01h/64513dInode: 13247963Links: 1
  Access: (0644/-rw-r--r--)  Uid: ( 1000/  smkent)   Gid: ( 1000/  smkent)
  Access: 2015-08-18 09:48:02.613988480 -0700
  Modify: 2015-08-18 09:48:02.825985646 -0700
  Change: 2015-08-18 09:48:02.825985646 -0700
   Birth: -

  $ qemu-img convert -f raw -o subformat=fixed -O vpc source-disk.img
  dest-disk.vhd

  $ stat dest-disk.vhd
    File: ‘dest-disk.vhd’
    Size: 4296499712  Blocks: 535216 IO Block: 4096   regular file
  Device: fc01h/64513dInode: 13247964Links: 1
  Access: (0644/-rw-r--r--)  Uid: ( 1000/  smkent)   Gid: ( 1000/  smkent)
  Access: 2015-08-18 09:50:22.252077624 -0700
  Modify: 2015-08-18 09:49:24.424868868 -0700
  Change: 2015-08-18 09:49:24.424868868 -0700
   Birth: -

  $ azure vm image create testimage1 dest-disk.vhd -o linux -l "West US"
  info:Executing command vm image create
  + Retrieving storage accounts
  info:VHD size : 4097 MB
  info:Uploading 4195800.5 KB
  Requested:100.0% Completed:100.0% Running:   0 Time: 1m 0s Speed:  6744 KB/s
  info:https://[redacted].blob.core.windows.net/vm-images/dest-disk.vhd was 
uploaded successfully
  error:   The VHD 
https://[redacted].blob.core.windows.net/vm-images/dest-disk.vhd has an 
unsupported virtual size of 4296499200 bytes.  The size must be a whole number 
(in MBs).
  info:Error information has been recorded to /home/smkent/.azure/azure.err
  error:   vm image create command failed

   * A fixed qemu-img will not result in an error during azure image
  creation. It will require passing -o force_size, which will leverage
  the backported functionality.

  [Regression Potential]

   * The upstream fix introduces a qemu-img option (-o force_size) which
  is unset by default. The regression potential is very low, as a
  result.

  ...

  I also ran the above commands using qemu 2.4.0, which resulted in the
  same error as the conversion behavior is the same.

  However, qemu 2.1.1 and earlier (including qemu 2.0.0 installed by
  Ubuntu 14.04) does not pad the virtual disk size during conversion.
  Using qemu-img convert from qemu versions <=2.1.1 results in a VHD
  that is exactly the size of the raw input file plus 512 bytes (for the
  VHD footer). Those qemu versions do not attempt to realign the disk.
  As a result, Azure accepts VHD files created using those versions of
  qemu-img convert for upload.

  Is there a reason why newer qemu realigns the converted VHD file? It
  would be useful if an option were added to disable this feature, as
  current versions of qemu cannot be used to create VHD files for Azure
  using Microsoft's official instructions.

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

[Qemu-devel] [Bug 1490611] Re: Using qemu >=2.2.1 to convert raw->VHD (fixed) adds extra padding to the result file, which Microsoft Azure rejects as invalid

[Robie Basak]

Rejected:

Rejected by Brian Murray: There was a security update to qemu today, so
the SRU will need to be redone on top of that.

** Changed in: qemu (Ubuntu Xenial)
   Status: In Progress => Triaged

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

Title:
  Using qemu >=2.2.1 to convert raw->VHD (fixed) adds extra padding to
  the result file, which Microsoft Azure rejects as invalid

Status in QEMU:
  Fix Released
Status in qemu package in Ubuntu:
  Fix Released
Status in qemu source package in Xenial:
  Triaged

Bug description:
  [Impact]

   * Starting with a raw disk image, using "qemu-img convert" to convert
  from raw to VHD results in the output VHD file's virtual size being
  aligned to the nearest 516096 bytes (16 heads x 63 sectors per head x
  512 bytes per sector), instead of preserving the input file's size as
  the output VHD's virtual disk size.

   * Microsoft Azure requires that disk images (VHDs) submitted for
  upload have virtual sizes aligned to a megabyte boundary. (Ex. 4096MB,
  4097MB, 4098MB, etc. are OK, 4096.5MB is rejected with an error.) This
  is reflected in Microsoft's documentation: https://azure.microsoft.com
  /en-us/documentation/articles/virtual-machines-linux-create-upload-
  vhd-generic/

   * The fix for this bug is a backport from upstream.
  http://git.qemu.org/?p=qemu.git;a=commitdiff;h=fb9245c2610932d33ce14

  [Test Case]

   * This is reproducible with the following set of commands (including
  the Azure command line tools from https://github.com/Azure/azure-
  xplat-cli). For the following example, I used qemu version 2.2.1:

  $ dd if=/dev/zero of=source-disk.img bs=1M count=4096

  $ stat source-disk.img
    File: ‘source-disk.img’
    Size: 4294967296  Blocks: 798656 IO Block: 4096   regular file
  Device: fc01h/64513dInode: 13247963Links: 1
  Access: (0644/-rw-r--r--)  Uid: ( 1000/  smkent)   Gid: ( 1000/  smkent)
  Access: 2015-08-18 09:48:02.613988480 -0700
  Modify: 2015-08-18 09:48:02.825985646 -0700
  Change: 2015-08-18 09:48:02.825985646 -0700
   Birth: -

  $ qemu-img convert -f raw -o subformat=fixed -O vpc source-disk.img
  dest-disk.vhd

  $ stat dest-disk.vhd
    File: ‘dest-disk.vhd’
    Size: 4296499712  Blocks: 535216 IO Block: 4096   regular file
  Device: fc01h/64513dInode: 13247964Links: 1
  Access: (0644/-rw-r--r--)  Uid: ( 1000/  smkent)   Gid: ( 1000/  smkent)
  Access: 2015-08-18 09:50:22.252077624 -0700
  Modify: 2015-08-18 09:49:24.424868868 -0700
  Change: 2015-08-18 09:49:24.424868868 -0700
   Birth: -

  $ azure vm image create testimage1 dest-disk.vhd -o linux -l "West US"
  info:Executing command vm image create
  + Retrieving storage accounts
  info:VHD size : 4097 MB
  info:Uploading 4195800.5 KB
  Requested:100.0% Completed:100.0% Running:   0 Time: 1m 0s Speed:  6744 KB/s
  info:https://[redacted].blob.core.windows.net/vm-images/dest-disk.vhd was 
uploaded successfully
  error:   The VHD 
https://[redacted].blob.core.windows.net/vm-images/dest-disk.vhd has an 
unsupported virtual size of 4296499200 bytes.  The size must be a whole number 
(in MBs).
  info:Error information has been recorded to /home/smkent/.azure/azure.err
  error:   vm image create command failed

   * A fixed qemu-img will not result in an error during azure image
  creation. It will require passing -o force_size, which will leverage
  the backported functionality.

  [Regression Potential]

   * The upstream fix introduces a qemu-img option (-o force_size) which
  is unset by default. The regression potential is very low, as a
  result.

  ...

  I also ran the above commands using qemu 2.4.0, which resulted in the
  same error as the conversion behavior is the same.

  However, qemu 2.1.1 and earlier (including qemu 2.0.0 installed by
  Ubuntu 14.04) does not pad the virtual disk size during conversion.
  Using qemu-img convert from qemu versions <=2.1.1 results in a VHD
  that is exactly the size of the raw input file plus 512 bytes (for the
  VHD footer). Those qemu versions do not attempt to realign the disk.
  As a result, Azure accepts VHD files created using those versions of
  qemu-img convert for upload.

  Is there a reason why newer qemu realigns the converted VHD file? It
  would be useful if an option were added to disable this feature, as
  current versions of qemu cannot be used to create VHD files for Azure
  using Microsoft's official instructions.

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

Re: [Qemu-devel] [PATCH for-2.7 v3 35/36] ahci: fix sglist leak on retry

[John Snow]

On 08/04/2016 10:46 AM, Markus Armbruster wrote:

John, can you review this one?



I'm very sorry for the delay.


marcandre.lur...@redhat.com writes:


From: Marc-André Lureau 

ahci-test /x86_64/ahci/io/dma/lba28/retry triggers the following leak:

Direct leak of 16 byte(s) in 1 object(s) allocated from:
#0 0x7fc4b2a25e20 in malloc (/lib64/libasan.so.3+0xc6e20)
#1 0x7fc4993bce58 in g_malloc (/lib64/libglib-2.0.so.0+0x4ee58)
#2 0x556a187d4b34 in ahci_populate_sglist hw/ide/ahci.c:896
#3 0x556a187d8237 in ahci_dma_prepare_buf hw/ide/ahci.c:1367
#4 0x556a187b5a1a in ide_dma_cb hw/ide/core.c:844
#5 0x556a187d7eec in ahci_start_dma hw/ide/ahci.c:1333
#6 0x556a187b650b in ide_start_dma hw/ide/core.c:921
#7 0x556a187b61e6 in ide_sector_start_dma hw/ide/core.c:911
#8 0x556a187b9e26 in cmd_write_dma hw/ide/core.c:1486
#9 0x556a187bd519 in ide_exec_cmd hw/ide/core.c:2027
#10 0x556a187d71c5 in handle_reg_h2d_fis hw/ide/ahci.c:1204
#11 0x556a187d7681 in handle_cmd hw/ide/ahci.c:1254
#12 0x556a187d168a in check_cmd hw/ide/ahci.c:510
#13 0x556a187d0afc in ahci_port_write hw/ide/ahci.c:314
#14 0x556a187d105d in ahci_mem_write hw/ide/ahci.c:435
#15 0x556a1831d959 in memory_region_write_accessor 
/home/elmarco/src/qemu/memory.c:525
#16 0x556a1831dc35 in access_with_adjusted_size 
/home/elmarco/src/qemu/memory.c:591
#17 0x556a18323ce3 in memory_region_dispatch_write 
/home/elmarco/src/qemu/memory.c:1262
#18 0x556a1828cf67 in address_space_write_continue 
/home/elmarco/src/qemu/exec.c:2578
#19 0x556a1828d20b in address_space_write /home/elmarco/src/qemu/exec.c:2635
#20 0x556a1828d92b in address_space_rw /home/elmarco/src/qemu/exec.c:2737
#21 0x556a1828daf7 in cpu_physical_memory_rw 
/home/elmarco/src/qemu/exec.c:2746
#22 0x556a183068d3 in cpu_physical_memory_write 
/home/elmarco/src/qemu/include/exec/cpu-common.h:72
#23 0x556a18308194 in qtest_process_command 
/home/elmarco/src/qemu/qtest.c:382
#24 0x556a1830 in qtest_process_inbuf /home/elmarco/src/qemu/qtest.c:573
#25 0x556a18309a4a in qtest_read /home/elmarco/src/qemu/qtest.c:585
#26 0x556a18598b85 in qemu_chr_be_write_impl 
/home/elmarco/src/qemu/qemu-char.c:387
#27 0x556a18598c52 in qemu_chr_be_write 
/home/elmarco/src/qemu/qemu-char.c:399
#28 0x556a185a2afa in tcp_chr_read /home/elmarco/src/qemu/qemu-char.c:2902
#29 0x556a18cbaf52 in qio_channel_fd_source_dispatch io/channel-watch.c:84

Follow John Snow recommendation:
  Everywhere else ncq_err is used, it is accompanied by a list cleanup
  except for ncq_cb, which is the case you are fixing here.

  Move the sglist destruction inside of ncq_err and then delete it from
  the other two locations to keep it tidy.

  Call dma_buf_commit in ide_dma_cb after the early return. Though, this
  is also a little wonky because this routine does more than clear the
  list, but it is at the moment the centralized "we're done with the
  sglist" function and none of the other side effects that occur in
  dma_buf_commit will interfere with the reset that occurs from
  ide_restart_bh, I think

Signed-off-by: Marc-André Lureau 
---
 hw/ide/ahci.c | 3 +--
 hw/ide/core.c | 1 +
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/hw/ide/ahci.c b/hw/ide/ahci.c
index 6defeed..f3438ad 100644
--- a/hw/ide/ahci.c
+++ b/hw/ide/ahci.c
@@ -919,6 +919,7 @@ static void ncq_err(NCQTransferState *ncq_tfs)
 ide_state->error = ABRT_ERR;
 ide_state->status = READY_STAT | ERR_STAT;
 ncq_tfs->drive->port_regs.scr_err |= (1 << ncq_tfs->tag);
+qemu_sglist_destroy(&ncq_tfs->sglist);
 ncq_tfs->used = 0;
 }

@@ -1025,7 +1026,6 @@ static void execute_ncq_command(NCQTransferState *ncq_tfs)
 default:
 DPRINTF(port, "error: unsupported NCQ command (0x%02x) received\n",
 ncq_tfs->cmd);
-qemu_sglist_destroy(&ncq_tfs->sglist);
 ncq_err(ncq_tfs);
 }
 }
@@ -1092,7 +1092,6 @@ static void process_ncq_command(AHCIState *s, int port, 
uint8_t *cmd_fis,
 error_report("ahci: PRDT length for NCQ command (0x%zx) "
  "is smaller than the requested size (0x%zx)",
  ncq_tfs->sglist.size, size);
-qemu_sglist_destroy(&ncq_tfs->sglist);
 ncq_err(ncq_tfs);
 ahci_trigger_irq(ad->hba, ad, PORT_IRQ_OVERFLOW);
 return;
diff --git a/hw/ide/core.c b/hw/ide/core.c
index f9c8162..82d7f2a 100644
--- a/hw/ide/core.c
+++ b/hw/ide/core.c
@@ -822,6 +822,7 @@ static void ide_dma_cb(void *opaque, int ret)
 return;
 }
 if (ret < 0) {
+dma_buf_commit(s, 0);


You may flog me for not replying to your V2, but I believe this cleanup 
needs to be in the early return case. Otherwise, we will perform cleanup 
twice in the 'IGNORE' case.


I thiiink we need to shimmy this down to before the return.


 if (ide_handle_rw_error(s, -ret, ide_dma_cmd_to_retry(s->dma_cmd))) {
 s->bus->dm

Re: [Qemu-devel] [PATCH 4/4] error: Fix error_printf() calls lacking newlines

[Eric Blake]

On 08/03/2016 05:37 AM, Markus Armbruster wrote:
> Signed-off-by: Markus Armbruster 
> ---
>  hw/i386/pc.c | 2 +-
>  kvm-all.c| 2 +-
>  ui/vnc.c | 2 +-
>  3 files changed, 3 insertions(+), 3 deletions(-)
> 

I'm guessing these were found with a slight tweak to the Coccinelle
script in 1/4? Worth checking that in, or at least leaving a comment
bread-crumb?

Reviewed-by: Eric Blake 

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



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH 3/4] vfio: Use error_report() instead of error_printf() for errors

[Eric Blake]

On 08/03/2016 05:37 AM, Markus Armbruster wrote:
> Cc: Alex Williamson 
> Signed-off-by: Markus Armbruster 
> ---
>  hw/vfio/platform.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 

Reviewed-by: Eric Blake 

> diff --git a/hw/vfio/platform.c b/hw/vfio/platform.c
> index 1798a00..a559e7b 100644
> --- a/hw/vfio/platform.c
> +++ b/hw/vfio/platform.c
> @@ -496,7 +496,7 @@ static int vfio_populate_device(VFIODevice *vbasedev)
>  irq.index = i;
>  ret = ioctl(vbasedev->fd, VFIO_DEVICE_GET_IRQ_INFO, &irq);
>  if (ret) {
> -error_printf("vfio: error getting device %s irq info",
> +error_report("vfio: error getting device %s irq info",
>   vbasedev->name);
>  goto irq_err;
>  } else {
> 

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



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH 2/4] checkpatch: Fix newline detection in error_setg() & friends

[Eric Blake]

On 08/03/2016 05:37 AM, Markus Armbruster wrote:
> Commit 5d596c2's regexp assumes the error message string is the first
> argument.  Correct for error_report(), wrong for all the others.
> Relax the regexp to match newline in anywhere.  This might cause
> additional false positives.

Might, but those are still rare enough that we don't have to worry about
it unless someone actually hits such a false positive.

> 
> While there, update the list of error_reporting functions.
> 
> Cc: Jason J. Herne 
> Signed-off-by: Markus Armbruster 
> ---
>  scripts/checkpatch.pl | 5 -
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 

Reviewed-by: Eric Blake 

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



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH 1/4] error: Strip trailing '\n' from error string arguments (again)

[Eric Blake]

On 08/03/2016 05:37 AM, Markus Armbruster wrote:
> Commit 9af9e0f, 6daf194d, be62a2eb and 312fd5f got rid of a bunch, but
> they keep coming back.  checkpatch.pl tries to flag them since commit
> 5d596c2, but it's not very good at it.  Offenders tracked down with
> Coccinelle script scripts/coccinelle/err-bad-newline.cocci, an updated
> version of the script from commit 312fd5f.
> 
> Signed-off-by: Markus Armbruster 
> ---

> +++ b/scripts/coccinelle/err-bad-newline.cocci
> @@ -0,0 +1,29 @@
> +// Error messages should not contain newlines.  This script finds
> +// messages that do.  Fixing them is manual.
> +@r@
> +expression errp, eno, cls, fmt;
> +position p;
> +@@
> +(
> +error_report(fmt, ...)@p

So you use Coccinelle to find error messages...

> +@script:python@
> +fmt << r.fmt;
> +p << r.p;
> +@@
> +if "\\n" in str(fmt):
> +print "%s:%s:%s:%s" % (p[0].file, p[0].line, p[0].column, fmt)

...and python to then parse the format string and flag violations.
Slick.  Does it still work across formats like "foo %" PRIdMAX "\n"?

Reviewed-by: Eric Blake 

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



signature.asc
Description: OpenPGP digital signature

[Qemu-devel] [Bug 1609968] [NEW] "cannot set up guest memory" b/c no automatic clearing of Linux' cache

[Celmor]

Public bug reported:

Version: qemu-2.6.0-1
Kernel: 4.4.13-1-MANJARO
Full script (shouldn't matter though): https://pastebin.com/Hp24PWNE

Problem:
When host has been up and used for a while cache has been filled as much that 
guest can't be started without droping caches.

Expected behavior:
Qemu should be able to request as much Memory as it needs and cause Linux to 
drop cache pages if needed. A user shouldn't be required to have to come to 
this conclusion and having to drop caches to start Qemu with the required 
amount of memory.

My fix:
Following command (as root) required before qemu start:
# sync && echo 3 > /proc/sys/vm/drop_caches

Example:
$ sudo qemu.sh -m 10240 && echo success || echo failed
qemu-system-x86_64: cannot set up guest memory 'pc.ram': Cannot allocate memory
failed
$ free
  totalusedfree  shared  buff/cache   available
Mem:   16379476 9126884 3462688  148480 3789904 5123572
Swap: 0   0   0
$ sudo sh -c 'sync && echo 3 > /proc/sys/vm/drop_caches'
$ free
  totalusedfree  shared  buff/cache   available
Mem:   16379476 169452814106552  149772  57839614256428
Swap: 0   0   0
$ sudo qemu.sh -m 10240  && echo success || echo failed
success

** Affects: qemu
 Importance: Undecided
 Status: New


** Tags: cache memory pc.ram ram

** Summary changed:

- "cannot set up guest memory" and can't clear Linux' Cache
+ "cannot set up guest memory" b/c no automatic clearing of Linux' cache

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

Title:
  "cannot set up guest memory" b/c no automatic clearing of Linux' cache

Status in QEMU:
  New

Bug description:
  Version: qemu-2.6.0-1
  Kernel: 4.4.13-1-MANJARO
  Full script (shouldn't matter though): https://pastebin.com/Hp24PWNE

  Problem:
  When host has been up and used for a while cache has been filled as much that 
guest can't be started without droping caches.

  Expected behavior:
  Qemu should be able to request as much Memory as it needs and cause Linux to 
drop cache pages if needed. A user shouldn't be required to have to come to 
this conclusion and having to drop caches to start Qemu with the required 
amount of memory.

  My fix:
  Following command (as root) required before qemu start:
  # sync && echo 3 > /proc/sys/vm/drop_caches

  Example:
  $ sudo qemu.sh -m 10240 && echo success || echo failed
  qemu-system-x86_64: cannot set up guest memory 'pc.ram': Cannot allocate 
memory
  failed
  $ free
totalusedfree  shared  buff/cache   
available
  Mem:   16379476 9126884 3462688  148480 3789904 
5123572
  Swap: 0   0   0
  $ sudo sh -c 'sync && echo 3 > /proc/sys/vm/drop_caches'
  $ free
totalusedfree  shared  buff/cache   
available
  Mem:   16379476 169452814106552  149772  578396
14256428
  Swap: 0   0   0
  $ sudo qemu.sh -m 10240  && echo success || echo failed
  success

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

Re: [Qemu-devel] qtest protocol: should memset/read/write etc of a size of 0 bytes be permitted?

[Eric Blake]

On 08/04/2016 12:46 PM, Peter Maydell wrote:
> I've upgraded to a more recent version of clang, which now produces
> undefined-behaviour warnings for passing NULL pointers to some library
> functions. One of the things it has shown up is that some of the
> qtest tests ask for "memset" with size zero. In our current implementation
> this results in qtest.c calling g_malloc(0), which returns NULL, and

I never understood why glib made that choice on g_malloc(0). I would
much prefer it to ALWAYS return something, just as glibc malloc(0) does.

> then calling memset(NULL, chr, 0), which is UB.

Indeed, although I really wish POSIX could be loosened to say that the
source pointer is untouched if the length is 0 (I've debated about
filing a POSIX bug report to that effect, but have not done so yet), so
that the UB only happens when passing NULL with a non-zero size.

> 
> So should we:
> (1) declare the qtest protocol commands 'memset', 'read', 'write'
> etc which operate on a lump of guest memory of specified size to
> support size == 0 as meaning "do nothing"

My preference - even if we have to special case things to avoid UB at
the lower level, presenting well-defined behavior at the upper level is
easier to think about.

> (2) declare that size == 0 is not valid and make it return a failure
> code back down the qtest pipe (and fix the offending tests)

Doable, but not as fun to audit, and not my preference.

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



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH 3/7] vdi: Use QEMU UUID API

[Eric Blake]

On 08/04/2016 12:58 PM, Stefan Weil wrote:
> Hi,
> 
> On 08/02/16 11:18, Fam Zheng wrote:
>> The QEMU UUID api, including the data structure (qemu_uuid_t), is fully
>> compatible with libuuid.  Use it, and remove the unused code.
>>
>> Signed-off-by: Fam Zheng 
>> ---
>>  block/vdi.c | 49 ++---
>>  1 file changed, 10 insertions(+), 39 deletions(-)
>>

>> @@ -182,10 +153,10 @@ typedef struct {
>>  uint32_t block_extra;   /* unused here */
>>  uint32_t blocks_in_image;
>>  uint32_t blocks_allocated;
>> -uuid_t uuid_image;
>> -uuid_t uuid_last_snap;
>> -uuid_t uuid_link;
>> -uuid_t uuid_parent;
>> +qemu_uuid_t uuid_image;
>> +qemu_uuid_t uuid_last_snap;
>> +qemu_uuid_t uuid_link;
>> +qemu_uuid_t uuid_parent;
> 
> As far as I remember, _t should be avoided because that postfix is
> reserved by POSIX. Should we use qemu_uuid, or can we ignore POSIX
> because the type name uses the qemu_ prefix?

Correct that POSIX reserved all _t (which is a bit broad, oh well), and
also correct that we can take the risk of using it anyways (but if POSIX
ever standardizes something, we get to keep both of our broken pieces).


> Either with qemu_uuid_t or with qemu_uuid:

I thought our coding standard preferred QemuUUID or something similar in
camelcase, rather than lower case.  But now we are just painting a
bikeshed, so I won't strongly object.

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



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] qtest protocol: should memset/read/write etc of a size of 0 bytes be permitted?

[Peter Maydell]

On 4 August 2016 at 19:49, John Snow  wrote:
>
>
> On 08/04/2016 02:46 PM, Peter Maydell wrote:
>> (2) declare that size == 0 is not valid and make it return a failure
>> code back down the qtest pipe (and fix the offending tests)
>>
>
> This is probably the nicer thing to do -- if memset of length 0 is
> undefined, probably qmemset and friends should also be undefined by
> extension.

That was my thought.

> I reserve the right to change my mind depending on how gnarly it is to
> untangle.
>
> I assume you're hoping for 2.7.

I dunno. If the patch turns out easy we can put it in 2.7.
But it's only test code and it's been working fine for
a long time til we noticed it, so I don't think it matters
if we let it slide til 2.8.

thanks
-- PMM

Re: [Qemu-devel] fix incorrect identify implementation in nvme

[Keith Busch]

On Thu, Aug 04, 2016 at 09:42:13PM +0200, Christoph Hellwig wrote:
> Third resent of this series after this didn't get picked up the
> previous times.  The Qemu NVMe implementation mistakes the cns
> field in the Identify command as a boolean.  This was never
> true, and is actively harmful since NVMe1.1 (which the Qemu
> device claims to support) supports more than two Identify variants.
> 
> We had to add a quirk in Linux to work around this behavior.

Yes, these are great. Do we need to ping a maintainer to go through
their tree, or can this be applied immediately? If need be, I can apply
and send a pull request.

Re: [Qemu-devel] [PATCH v6 1/2] virtio-crypto: Add virtio crypto device specification

[Michael S. Tsirkin]

On Thu, Aug 04, 2016 at 11:24:26AM +, Gonglei (Arei) wrote:
> > > +The first driver-read-only field, \field{version} specifies the virtio 
> > > crypto's
> > > +version, which is reserved for back-compatibility in future.It's 
> > > currently
> > > +defined for the version field:
> > > +
> > > +\begin{lstlisting}
> > > +#define VIRTIO_CRYPTO_VERSION_1  (1)
> > 
> > Suggest to remove this macro,
> > Do you think a version which is composed of major version and
> > minor version is better?
> > 
> 
> I think we should tell the developer how to set the value of version field,
> but I have no idea about which value or form is better, so I used 1 as the
> first version. What's your opinion?

My opinion is that you should drop this completely. We do feature bits,
not version numbers in virtio. We do not want each device doing its own
thing for compatibility.

-- 
MST

Re: [Qemu-devel] [PATCH] virtio-net: allow increasing rx queue size

[Michael S. Tsirkin]

On Thu, Aug 04, 2016 at 09:35:15AM +0200, Cornelia Huck wrote:
> On Thu, 4 Aug 2016 02:16:14 +0300
> "Michael S. Tsirkin"  wrote:
> 
> > This allows increasing the rx queue size up to 1024: unlike with tx,
> > guests don't put in huge S/G lists into RX so the risk of running into
> > the max 1024 limitation due to some off-by-one seems small.
> > 
> > It's helpful for users like OVS-DPDK which don't do any buffering on the
> > host - 1K roughly matches 500 entries in tun + 256 in the current rx
> > queue, which seems to work reasonably well. We could probably make do
> > with ~750 entries but virtio spec limits us to powers of two.
> > It might be a good idea to specify an s/g size limit in a future
> > version.
> > 
> > It also might be possible to make the queue size smaller down the road, 64
> > seems like the minimal value which will still work (as guests seem to
> > assume a queue full of 1.5K buffers is enough to process the largest
> > incoming packet, which is ~64K).  No one actually asked for this, and
> > with virtio 1 guests can reduce ring size without need for host
> > configuration, so don't bother with this for now.
> 
> Do we need some kind of sanity check that the guest did not resize
> below a reasonable limit?

Unfortunately the spec does not have an interface for that.
Guests expect they can get away with any size.

> > 
> > Signed-off-by: Michael S. Tsirkin 
> > ---
> >  include/hw/virtio/virtio-net.h |  1 +
> >  hw/net/virtio-net.c| 22 +-
> >  2 files changed, 22 insertions(+), 1 deletion(-)
> > 
> 
> 
> > @@ -1716,10 +1717,28 @@ static void virtio_net_device_realize(DeviceState 
> > *dev, Error **errp)
> >  VirtIONet *n = VIRTIO_NET(dev);
> >  NetClientState *nc;
> >  int i;
> > +int min_rx_queue_size;
> > 
> >  virtio_net_set_config_size(n, n->host_features);
> >  virtio_init(vdev, "virtio-net", VIRTIO_ID_NET, n->config_size);
> > 
> > +/*
> > + * We set a lower limit on RX queue size to what it always was.
> > + * Guests that want a smaller ring can always resize it without
> > + * help from us (using virtio 1 and up).
> > + */
> > +min_rx_queue_size = 256;
> 
> I'd find it more readable to introduce a #define with the old queue
> size as the minimum size...
> 
> > +if (n->net_conf.rx_queue_size < min_rx_queue_size ||
> > +n->net_conf.rx_queue_size > VIRTQUEUE_MAX_SIZE ||
> > +(n->net_conf.rx_queue_size & (n->net_conf.rx_queue_size - 1))) {
> > +error_setg(errp, "Invalid rx_queue_size (= %" PRIu16 "), "
> > +   "must be a power of 2 between %d and %d.",
> > +   n->net_conf.rx_queue_size, min_rx_queue_size,
> > +   VIRTQUEUE_MAX_SIZE);
> > +virtio_cleanup(vdev);
> > +return;
> > +}
> > +
> >  n->max_queues = MAX(n->nic_conf.peers.queues, 1);
> >  if (n->max_queues * 2 + 1 > VIRTIO_QUEUE_MAX) {
> >  error_setg(errp, "Invalid number of queues (= %" PRIu32 "), "
> > @@ -1880,6 +1899,7 @@ static Property virtio_net_properties[] = {
> > TX_TIMER_INTERVAL),
> >  DEFINE_PROP_INT32("x-txburst", VirtIONet, net_conf.txburst, TX_BURST),
> >  DEFINE_PROP_STRING("tx", VirtIONet, net_conf.tx),
> > +DEFINE_PROP_UINT16("rx_queue_size", VirtIONet, net_conf.rx_queue_size, 
> > 256),
> 
> ...and defaulting to that #define (or one derived from the #define
> above) here.

These happen to be the same, but they are in fact
unrelated: one is the default, the other is the
min value.


> >  DEFINE_PROP_END_OF_LIST(),
> >  };
> > 
> 
> Do we need compat handling for the new property?

No since we did not change the default :)

Re: [Qemu-devel] [PATCH v6 0/2] virtio-crypto: virtio crypto device specification

[Michael S. Tsirkin]

On Wed, Aug 03, 2016 at 07:23:36PM +0200, Cornelia Huck wrote:
> On Wed, 3 Aug 2016 16:53:36 +0200
> Cornelia Huck  wrote:
> 
> > On Wed, 3 Aug 2016 17:40:07 +0300
> > "Michael S. Tsirkin"  wrote:
> > 
> > > On Wed, Aug 03, 2016 at 04:33:28PM +0200, Cornelia Huck wrote:
> > > > On Wed, 3 Aug 2016 17:30:22 +0300
> > > > "Michael S. Tsirkin"  wrote:
> > > > 
> > > > > On Mon, Aug 01, 2016 at 05:20:19PM +0800, Gonglei wrote:
> > > > > > This is the specification (version 6) about a new virtio crypto 
> > > > > > device.
> > > > > > After a big reconstruction, the spec (symmetric algos) is near to 
> > > > > > stabilize.
> > > > > > This version fix some problems of formating and return value, etc.
> > > > > > 
> > > > > > If you have any comments, please let me know, thanks :)
> > > > > 
> > > > > You might want to open a jira tracker in oasis jira to add this.
> > > > 
> > > > It may make sense to open one/many jira trackers to reserve the device
> > > > ids (for the various device types that have accumulated) at least.
> > > 
> > > Yes! Can you do this please?
> > 
> > Sure, will do.
> 
> Done (issues 147-150).
> 
> I'm just unsure what to put as target version :/

So please use 1.1 - I will rename it if we decide
on a different version.

-- 
MST

[Qemu-devel] fix incorrect identify implementation in nvme

[Christoph Hellwig]

Third resent of this series after this didn't get picked up the
previous times.  The Qemu NVMe implementation mistakes the cns
field in the Identify command as a boolean.  This was never
true, and is actively harmful since NVMe1.1 (which the Qemu
device claims to support) supports more than two Identify variants.

We had to add a quirk in Linux to work around this behavior.

Re: [Qemu-devel] [PATCH v2] balloon: Fix failure of updating guest memory status

[Ladi Prosek]

On Thu, Aug 4, 2016 at 6:01 PM, Michael S. Tsirkin  wrote:
> On Thu, Aug 04, 2016 at 05:14:14PM +0200, Ladi Prosek wrote:
>> On Wed, Aug 3, 2016 at 9:25 AM, Ladi Prosek  wrote:
>> > On Tue, Aug 2, 2016 at 2:11 AM, Michael S. Tsirkin  wrote:
>> >> On Mon, Aug 01, 2016 at 11:59:31PM +, Li, Liang Z wrote:
>> >>> > On Wed, Jul 06, 2016 at 12:49:06PM +, Li, Liang Z wrote:
>> >>> > > > > > > After live migration, 'guest-stats' can't get the expected
>> >>> > > > > > > memory status in the guest. This issue is caused by commit
>> >>> > 4eae2a657d.
>> >>> > > > > > > The value of 's->stats_vq_elem' will be NULL after live
>> >>> > > > > > > migration, and the check in the function
>> >>> > > > > > > 'balloon_stats_poll_cb()' will prevent the 'virtio_notify()'
>> >>> > > > > > > from executing. So guest will not update the memory status.
>> >>> > > > > > >
>> >>> > > > > > > Commit 4eae2a657d is doing the right thing, but 
>> >>> > > > > > > 's->stats_vq_elem'
>> >>> > > > > > > should be treated as part of balloon device state and 
>> >>> > > > > > > migrated
>> >>> > > > > > > to destination if it's not NULL to make everything works 
>> >>> > > > > > > well.
>> >>> > > > > > >
>> >>> > > > > > > Signed-off-by: Liang Li 
>> >>> > > > > > > Suggested-by: Paolo Bonzini 
>> >>> > > > > > > Cc: Michael S. Tsirkin 
>> >>> > > > > > > Cc: Ladi Prosek 
>> >>> > > > > > > Cc: Paolo Bonzini 
>> >>> > > > > >
>> >>> > > > > > I agree there's an issue but we don't change versions anymore.
>> >>> > > > > > Breaking migrations for everyone is also not nice.
>> >>> > > > > >
>> >>> > > > > > How about queueing virtio_balloon_receive_stats so it will get
>> >>> > > > > > invoked when vm starts?
>> >>> > > > > >
>> >>> > > > >
>> >>> > > > > Could you give more explanation about how it works?  I can't 
>> >>> > > > > catch you.
>> >>> > > > >
>> >>> > > > > Thanks!
>> >>> > > > > Liang
>> >>> > > >
>> >>> > > > virtqueue_discard before migration
>> >>> > > >
>> >>> > > > virtio_balloon_receive_stats after migration
>> >>> > > >
>> >>> > >
>> >>> > > Sorry, I still can't catch you. Maybe it's easier for you to submit a
>> >>> > > patch than writing a lot a words to make me understand your idea.
>> >>> >
>> >>> > I'm rather busy now.  I might look into it towards end of the month.
>> >>> >
>> >>> > > I just don't understand why not to use the version to make things
>> >>> > > easier, is that not the original intent of version id?
>> >>> >
>> >>> > This was the original idea but we stopped using version ids since they 
>> >>> > have
>> >>> > many shortcomings.
>> >>> >
>> >>> > > If we want to extend the device and more states are needed, the idea
>> >>> > > you suggest can be used as a common solution?
>> >>> > >
>> >>> > > Thanks!
>> >>> > > Liang
>> >>> >
>> >>> > The idea is to try to avoid adding more state. that's not always 
>> >>> > possible but in
>> >>> > this case element was seen but not consumed yet, so it should be 
>> >>> > possible
>> >>> > for destination to simply get it from the VQ again.
>> >>> >
>> >>> > > > --
>> >>> > > > MST
>> >>>
>> >>> Hi Michel,
>> >>>
>> >>> Do you have time for this issue recently?
>> >>>
>> >>> Thanks!
>> >>> Liang
>> >
>> > Hi Liang,
>> >
>> > I should be able to look into it this week if you help me with testing.
>> >
>> > Thanks,
>> > Ladi
>>
>> Please try the attached patch. I have tested it with a simple
>> 'migrate' to save the state and then '-incoming' to load it back.
>>
>> One question for you: is it expected that stats_poll_interval is not
>> preserved by save/load? I had to explicitly set
>> guest-stats-polling-interval on the receiving VM to start getting
>> stats again. It's also the reason why the new
>> virtio_balloon_receive_stats call is not under if
>> (balloon_stats_enabled(s)) because this condition always evaluates to
>> false for me.
>>
>> Thanks!
>> Ladi
>>
>> >> Sorry, doesn't look like I will.
>> >> Idea is to make sure balloon_stats_poll_cb runs
>> >> on source. This will set stats_vq_elem to NULL.
>> >>
>> >>
>> >> --
>> >> MST
>
>> From f2f779e12f4aa4d3469d1b44e54484e66f82a2d7 Mon Sep 17 00:00:00 2001
>> From: Ladi Prosek 
>> Date: Thu, 4 Aug 2016 15:22:05 +0200
>> Subject: [PATCH] balloon: preserve stats virtqueue state across migrations
>>
>> Signed-off-by: Ladi Prosek 
>> ---
>>  hw/virtio/virtio-balloon.c | 18 +-
>>  1 file changed, 17 insertions(+), 1 deletion(-)
>>
>> diff --git a/hw/virtio/virtio-balloon.c b/hw/virtio/virtio-balloon.c
>> index 5af429a..1293be0 100644
>> --- a/hw/virtio/virtio-balloon.c
>> +++ b/hw/virtio/virtio-balloon.c
>> @@ -396,6 +396,19 @@ static void virtio_balloon_to_target(void *opaque, 
>> ram_addr_t target)
>>  trace_virtio_balloon_to_target(target, dev->num_pages);
>>  }
>>
>> +static void virtio_balloon_save(QEMUFile *f, void *opaque, size_t size)
>> +{
>> +VirtIOBalloon *s = VIRTIO_BALLOON(opaque);
>> +
>> +if (s->stats_vq_elem != NULL) {
>> +virtqueue_discard(s->svq, s->stats_

[Qemu-devel] [PATCH 1/2] nvme: fix identify to be NVMe 1.1 compliant

[Christoph Hellwig]

NVMe 1.1 requires devices to implement a Namespace List subcommand of
the identify command.  Qemu not only not implements this features, but
also misinterprets it as an Identify Controller request.  Due to this
any OS trying to use the Namespace List will fail the probe.

Signed-off-by: Christoph Hellwig 
---
 hw/block/nvme.c | 59 ++---
 1 file changed, 52 insertions(+), 7 deletions(-)

diff --git a/hw/block/nvme.c b/hw/block/nvme.c
index 2ded247..a0655a3 100644
--- a/hw/block/nvme.c
+++ b/hw/block/nvme.c
@@ -469,19 +469,22 @@ static uint16_t nvme_create_cq(NvmeCtrl *n, NvmeCmd *cmd)
 return NVME_SUCCESS;
 }
 
-static uint16_t nvme_identify(NvmeCtrl *n, NvmeCmd *cmd)
+static uint16_t nvme_identify_ctrl(NvmeCtrl *n, NvmeIdentify *c)
+{
+uint64_t prp1 = le64_to_cpu(c->prp1);
+uint64_t prp2 = le64_to_cpu(c->prp2);
+
+return nvme_dma_read_prp(n, (uint8_t *)&n->id_ctrl, sizeof(n->id_ctrl),
+prp1, prp2);
+}
+
+static uint16_t nvme_identify_ns(NvmeCtrl *n, NvmeIdentify *c)
 {
 NvmeNamespace *ns;
-NvmeIdentify *c = (NvmeIdentify *)cmd;
-uint32_t cns  = le32_to_cpu(c->cns);
 uint32_t nsid = le32_to_cpu(c->nsid);
 uint64_t prp1 = le64_to_cpu(c->prp1);
 uint64_t prp2 = le64_to_cpu(c->prp2);
 
-if (cns) {
-return nvme_dma_read_prp(n, (uint8_t *)&n->id_ctrl, sizeof(n->id_ctrl),
-prp1, prp2);
-}
 if (nsid == 0 || nsid > n->num_namespaces) {
 return NVME_INVALID_NSID | NVME_DNR;
 }
@@ -491,6 +494,48 @@ static uint16_t nvme_identify(NvmeCtrl *n, NvmeCmd *cmd)
 prp1, prp2);
 }
 
+static uint16_t nvme_identify_nslist(NvmeCtrl *n, NvmeIdentify *c)
+{
+static const int data_len = 4096;
+uint32_t min_nsid = le32_to_cpu(c->nsid);
+uint64_t prp1 = le64_to_cpu(c->prp1);
+uint64_t prp2 = le64_to_cpu(c->prp2);
+uint32_t *list;
+uint16_t ret;
+int i, j = 0;
+
+list = g_malloc0(data_len);
+for (i = 0; i < n->num_namespaces; i++) {
+if (i < min_nsid) {
+continue;
+}
+list[j++] = cpu_to_le32(i + 1);
+if (j == data_len / sizeof(uint32_t)) {
+break;
+}
+}
+ret = nvme_dma_read_prp(n, (uint8_t *)list, data_len, prp1, prp2);
+g_free(list);
+return ret;
+}
+
+
+static uint16_t nvme_identify(NvmeCtrl *n, NvmeCmd *cmd)
+{
+NvmeIdentify *c = (NvmeIdentify *)cmd;
+
+switch (le32_to_cpu(c->cns)) {
+case 0x00:
+return nvme_identify_ns(n, c);
+case 0x01:
+return nvme_identify_ctrl(n, c);
+case 0x02:
+return nvme_identify_nslist(n, c);
+default:
+return NVME_INVALID_FIELD | NVME_DNR;
+}
+}
+
 static uint16_t nvme_get_feature(NvmeCtrl *n, NvmeCmd *cmd, NvmeRequest *req)
 {
 uint32_t dw10 = le32_to_cpu(cmd->cdw10);
-- 
2.1.4

[Qemu-devel] [PATCH 2/2] nvme: bump PCI revision

[Christoph Hellwig]

The broken Identify implementation in earlier Qemu versions means we
need to blacklist it from issueing the NVMe 1.1 Identify Namespace List
command.  As we want to be able to use it in newer Qemu versions we need
a way to identify those.  Bump the PCI revision as a guest visible
indicator of this bug fix.

Signed-off-by: Christoph Hellwig 
---
 hw/block/nvme.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/block/nvme.c b/hw/block/nvme.c
index a0655a3..cef3bb4 100644
--- a/hw/block/nvme.c
+++ b/hw/block/nvme.c
@@ -954,7 +954,7 @@ static void nvme_class_init(ObjectClass *oc, void *data)
 pc->class_id = PCI_CLASS_STORAGE_EXPRESS;
 pc->vendor_id = PCI_VENDOR_ID_INTEL;
 pc->device_id = 0x5845;
-pc->revision = 1;
+pc->revision = 2;
 pc->is_express = 1;
 
 set_bit(DEVICE_CATEGORY_STORAGE, dc->categories);
-- 
2.1.4

Re: [Qemu-devel] [PATCH 3/7] vdi: Use QEMU UUID API

[Stefan Weil]

Hi,

On 08/02/16 11:18, Fam Zheng wrote:
> The QEMU UUID api, including the data structure (qemu_uuid_t), is fully
> compatible with libuuid.  Use it, and remove the unused code.
> 
> Signed-off-by: Fam Zheng 
> ---
>  block/vdi.c | 49 ++---
>  1 file changed, 10 insertions(+), 39 deletions(-)
> 
> diff --git a/block/vdi.c b/block/vdi.c
> index 8a1cf97..ed63184 100644
> --- a/block/vdi.c
> +++ b/block/vdi.c
> @@ -58,14 +58,7 @@
>  #include "migration/migration.h"
>  #include "qemu/coroutine.h"
>  #include "qemu/cutils.h"
> -
> -#if defined(CONFIG_UUID)
> -#include 
> -#else
> -/* TODO: move uuid emulation to some central place in QEMU. */
> -#include "sysemu/sysemu.h" /* UUID_FMT */
> -typedef unsigned char uuid_t[16];
> -#endif
> +#include "qemu/uuid.h"
>  
>  /* Code configuration options. */
>  
> @@ -140,28 +133,6 @@ typedef unsigned char uuid_t[16];
>  #define VDI_DISK_SIZE_MAX((uint64_t)VDI_BLOCKS_IN_IMAGE_MAX * \
>(uint64_t)DEFAULT_CLUSTER_SIZE)
>  
> -#if !defined(CONFIG_UUID)
> -static inline void uuid_generate(uuid_t out)
> -{
> -memset(out, 0, sizeof(uuid_t));
> -}
> -
> -static inline int uuid_is_null(const uuid_t uu)
> -{
> -uuid_t null_uuid = { 0 };
> -return memcmp(uu, null_uuid, sizeof(uuid_t)) == 0;
> -}
> -
> -# if defined(CONFIG_VDI_DEBUG)
> -static inline void uuid_unparse(const uuid_t uu, char *out)
> -{
> -snprintf(out, 37, UUID_FMT,
> -uu[0], uu[1], uu[2], uu[3], uu[4], uu[5], uu[6], uu[7],
> -uu[8], uu[9], uu[10], uu[11], uu[12], uu[13], uu[14], uu[15]);
> -}
> -# endif
> -#endif
> -
>  typedef struct {
>  char text[0x40];
>  uint32_t signature;
> @@ -182,10 +153,10 @@ typedef struct {
>  uint32_t block_extra;   /* unused here */
>  uint32_t blocks_in_image;
>  uint32_t blocks_allocated;
> -uuid_t uuid_image;
> -uuid_t uuid_last_snap;
> -uuid_t uuid_link;
> -uuid_t uuid_parent;
> +qemu_uuid_t uuid_image;
> +qemu_uuid_t uuid_last_snap;
> +qemu_uuid_t uuid_link;
> +qemu_uuid_t uuid_parent;

As far as I remember, _t should be avoided because that postfix is
reserved by POSIX. Should we use qemu_uuid, or can we ignore POSIX
because the type name uses the qemu_ prefix?


>  uint64_t unused2[7];
>  } QEMU_PACKED VdiHeader;
>  
> @@ -209,7 +180,7 @@ typedef struct {
>  /* Change UUID from little endian (IPRT = VirtualBox format) to big endian
>   * format (network byte order, standard, see RFC 4122) and vice versa.
>   */
> -static void uuid_convert(uuid_t uuid)
> +static void uuid_convert(qemu_uuid_t uuid)
>  {
>  bswap32s((uint32_t *)&uuid[0]);
>  bswap16s((uint16_t *)&uuid[4]);
> @@ -469,11 +440,11 @@ static int vdi_open(BlockDriverState *bs, QDict 
> *options, int flags,
> (uint64_t)header.blocks_in_image * header.block_size);
>  ret = -ENOTSUP;
>  goto fail;
> -} else if (!uuid_is_null(header.uuid_link)) {
> +} else if (!qemu_uuid_is_null(header.uuid_link)) {
>  error_setg(errp, "unsupported VDI image (non-NULL link UUID)");
>  ret = -ENOTSUP;
>  goto fail;
> -} else if (!uuid_is_null(header.uuid_parent)) {
> +} else if (!qemu_uuid_is_null(header.uuid_parent)) {
>  error_setg(errp, "unsupported VDI image (non-NULL parent UUID)");
>  ret = -ENOTSUP;
>  goto fail;
> @@ -821,8 +792,8 @@ static int vdi_create(const char *filename, QemuOpts 
> *opts, Error **errp)
>  if (image_type == VDI_TYPE_STATIC) {
>  header.blocks_allocated = blocks;
>  }
> -uuid_generate(header.uuid_image);
> -uuid_generate(header.uuid_last_snap);
> +qemu_uuid_generate(header.uuid_image);
> +qemu_uuid_generate(header.uuid_last_snap);
>  /* There is no need to set header.uuid_link or header.uuid_parent here. 
> */
>  #if defined(CONFIG_VDI_DEBUG)
>  vdi_header_print(&header);
> 

Either with qemu_uuid_t or with qemu_uuid:

Reviewed-by: Stefan Weil 




signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] qtest protocol: should memset/read/write etc of a size of 0 bytes be permitted?

[John Snow]

On 08/04/2016 02:46 PM, Peter Maydell wrote:

I've upgraded to a more recent version of clang, which now produces
undefined-behaviour warnings for passing NULL pointers to some library
functions. One of the things it has shown up is that some of the
qtest tests ask for "memset" with size zero. In our current implementation
this results in qtest.c calling g_malloc(0), which returns NULL, and
then calling memset(NULL, chr, 0), which is UB.

So should we:
(1) declare the qtest protocol commands 'memset', 'read', 'write'
etc which operate on a lump of guest memory of specified size to
support size == 0 as meaning "do nothing"


This would be easy to do.


(2) declare that size == 0 is not valid and make it return a failure
code back down the qtest pipe (and fix the offending tests)



This is probably the nicer thing to do -- if memset of length 0 is 
undefined, probably qmemset and friends should also be undefined by 
extension.


I reserve the right to change my mind depending on how gnarly it is to 
untangle.


I assume you're hoping for 2.7.


?

The offending tests are i386/ahci/flush/simple and i386/ahci/max
(because ahci_io() calls qmemset() with a zero size.)

thanks
-- PMM



--js

[Qemu-devel] qtest protocol: should memset/read/write etc of a size of 0 bytes be permitted?

[Peter Maydell]

I've upgraded to a more recent version of clang, which now produces
undefined-behaviour warnings for passing NULL pointers to some library
functions. One of the things it has shown up is that some of the
qtest tests ask for "memset" with size zero. In our current implementation
this results in qtest.c calling g_malloc(0), which returns NULL, and
then calling memset(NULL, chr, 0), which is UB.

So should we:
(1) declare the qtest protocol commands 'memset', 'read', 'write'
etc which operate on a lump of guest memory of specified size to
support size == 0 as meaning "do nothing"
(2) declare that size == 0 is not valid and make it return a failure
code back down the qtest pipe (and fix the offending tests)

?

The offending tests are i386/ahci/flush/simple and i386/ahci/max
(because ahci_io() calls qmemset() with a zero size.)

thanks
-- PMM

Re: [Qemu-devel] [PATCH] ide: fix DMA register transitions

[John Snow]

On 08/02/2016 06:05 PM, John Snow wrote:

ATA8-APT defines the state transitions for both a host controller and
for the hardware device during the lifecycle of a DMA transfer, in
section 9.7 "DMA command protocol."

One of the interesting tidbits here is that when a device transitions
from DDMA0 ("Prepare state") to DDMA1 ("Data_Transfer State"), it can
choose to set either BSY or DRQ to signal this transition, but not both.

as ide_sector_dma_start is the last point in our preparation process
before we begin the real data transfer process (for either AHCI or BMDMA),
this is the correct transition point for DDMA0 to DDMA1.

I have chosen !BSY && DRQ for QEMU to make the transition from DDMA0 the
most obvious.

Reported-by: Benjamin David Lunt 
Signed-off-by: John Snow 
---
 hw/ide/core.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/ide/core.c b/hw/ide/core.c
index d117b7c..e961d42 100644
--- a/hw/ide/core.c
+++ b/hw/ide/core.c
@@ -907,7 +907,7 @@ eot:

 static void ide_sector_start_dma(IDEState *s, enum ide_dma_cmd dma_cmd)
 {
-s->status = READY_STAT | SEEK_STAT | DRQ_STAT | BUSY_STAT;
+s->status = READY_STAT | SEEK_STAT | DRQ_STAT;
 s->io_buffer_size = 0;
 s->dma_cmd = dma_cmd;




Thanks, applied to my IDE-Next tree:

https://github.com/jnsnow/qemu/commits/ide-next
https://github.com/jnsnow/qemu.git

--js

Re: [Qemu-devel] [PATCH for-2.7] atapi: fix halted DMA reset

[John Snow]

On 08/02/2016 02:55 PM, John Snow wrote:

Followup to 87ac25fd, this time for ATAPI DMA.

Reported-by: Paolo Bonzini 
Signed-off-by: John Snow 
---
 hw/ide/atapi.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/hw/ide/atapi.c b/hw/ide/atapi.c
index 95056d9..6189675 100644
--- a/hw/ide/atapi.c
+++ b/hw/ide/atapi.c
@@ -386,6 +386,7 @@ static void ide_atapi_cmd_read_dma_cb(void *opaque, int ret)
 if (ret < 0) {
 if (ide_handle_rw_error(s, -ret, ide_dma_cmd_to_retry(s->dma_cmd))) {
 if (s->bus->error_status) {
+s->bus->dma->aiocb = NULL;
 return;
 }
 goto eot;



I extracted an ACK out of Paolo offline, so I have staged this patch.

Thanks, applied to my IDE tree:

https://github.com/jnsnow/qemu/commits/ide
https://github.com/jnsnow/qemu.git

--js

[Qemu-devel] [PATCH] MAINTAINERS: Add Fam and Jsnow for Bitmap support

[John Snow]

These files are currently unmaintained.

I'm proposing that Fam and I co-maintain them; under the model that
whomever between us isn't authoring a given series will be responsible
for reviewing it.

Signed-off-by: John Snow 
---
 MAINTAINERS | 14 ++
 1 file changed, 14 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index b6fb84e..0086f0e 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1041,6 +1041,20 @@ F: block/qapi.c
 F: qapi/block*.json
 T: git git://repo.or.cz/qemu/armbru.git block-next
 
+Dirty Bitmaps
+M: Fam Zheng 
+M: John Snow 
+L: qemu-bl...@nongnu.org
+S: Supported
+F: util/hbitmap.c
+F: block/dirty-bitmap.c
+F: include/qemu/hbitmap.h
+F: include/block/dirty-bitmap.h
+F: tests/test-hbitmap.c
+F: docs/bitmaps.md
+T: git git://github.com/famz/qemu.git bitmaps
+T: git git://github.com/jnsnow/qemu.git bitmaps
+
 Character device backends
 M: Paolo Bonzini 
 S: Maintained
-- 
2.7.4

[Qemu-devel] [PULL 1/1] Xen PCI passthrough: fix passthrough failure when no interrupt pin

[Stefano Stabellini]

From: Bruce Rogers 

Commit 5a11d0f7 mistakenly converted a log message into an error
condition when no pin interrupt is found for the pci device being
passed through. Revert that part of the commit.

Signed-off-by: Bruce Rogers 
Signed-off-by: Stefano Stabellini 
Acked-by: Anthony PERARD 
---
 hw/xen/xen_pt.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/xen/xen_pt.c b/hw/xen/xen_pt.c
index f593b04..b6d71bb 100644
--- a/hw/xen/xen_pt.c
+++ b/hw/xen/xen_pt.c
@@ -842,7 +842,7 @@ static void xen_pt_realize(PCIDevice *d, Error **errp)
 goto err_out;
 }
 if (!scratch) {
-error_setg(errp, "no pin interrupt");
+XEN_PT_LOG(d, "no pin interrupt\n");
 goto out;
 }
 
-- 
1.9.1

[Qemu-devel] xen-20160804-tag

[Stefano Stabellini]

The following changes since commit 09704e6ded83fa0bec14baf32f800f6512156ca0:

  Merge remote-tracking branch 'remotes/bonzini/tags/for-upstream' into staging 
(2016-08-04 10:24:27 +0100)

are available in the git repository at:


  git://xenbits.xen.org/people/sstabellini/qemu-dm.git tags/xen-20160804-tag

for you to fetch changes up to 0968c91ce00f42487fb11de5da38e53b5dc6bc7f:

  Xen PCI passthrough: fix passthrough failure when no interrupt pin 
(2016-08-04 10:42:48 -0700)


Xen 2016/08/04


Bruce Rogers (1):
  Xen PCI passthrough: fix passthrough failure when no interrupt pin

 hw/xen/xen_pt.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Re: [Qemu-devel] [PATCH v4 for-2.7 0/7] Fixing i686 host / sparc64 guest crash

[Richard Henderson]

On 08/04/2016 10:07 PM, Peter Maydell wrote:

On 4 August 2016 at 17:26, Richard Henderson  wrote:

This is a revision of my "third" attempt, tweaked a bit for Aurelien's
review.  I've sort of lost track of where we are with the release process,
so I'll understand if we've gone too far now.



 include/exec/gen-icount.h |   2 +-
 include/qemu/log.h|   3 +-
 tcg/optimize.c|  37 +--
 tcg/tcg-op.c  |   2 +-
 tcg/tcg.c | 588 ++
 tcg/tcg.h |  52 ++--
 util/log.c|  24 +-
 7 files changed, 441 insertions(+), 267 deletions(-)


We're probably going to tag rc2 tomorrow or maybe Monday, with a theoretical
final release date of the 16th (about a fortnight or so). So this
is a pretty big diffstat to be going in, given it's in generic code
rather than anything specific to only (say) sparc targets or i386
hosts. On the other hand it is a bugfix...


Yeah.  Review delay and travel delay have conspired against in this case.


r~

Re: [Qemu-devel] [PATCH v1 5/5] target-ppc: add vector permute right indexed instruction

[Richard Henderson]

On 08/04/2016 06:33 PM, Rajalakshmi Srinivasaraghavan wrote:

+if (s & 0x10) {
+result.u8[i] = a->u8[15 - index];
+} else {
+result.u8[i] = b->u8[15 - index];
+}


Fold these "15 -" into the computation of index above?


r~

Re: [Qemu-devel] [PATCH v1 3/5] target-ppc: add vector count trailing zeros instructions

[Richard Henderson]

On 08/04/2016 06:33 PM, Rajalakshmi Srinivasaraghavan wrote:

+#define ctzb(v) ((v) ? ctz32((uint32_t)(v)) : 8)
+#define ctzh(v) ((v) ? ctz32((uint32_t)(v)) : 16)


You don't need these casts.  Otherwise,

Reviewed-by: Richard Henderson  


r~

Re: [Qemu-devel] [PATCH v1 4/5] target-ppc: add vector bit permute doubleword instruction

[Richard Henderson]

On 08/04/2016 06:33 PM, Rajalakshmi Srinivasaraghavan wrote:

+uint64_t perm = 0;


Move the variable inside the loop.


+VECTOR_FOR_INORDER_I(i, u64) {
+perm = 0;
+VECTOR_FOR_INORDER_I(j, u16) {


Surely this is more clearly written as

  for (j = 0; j < 8; ++j)

since u16 really has nothing to do with this.


+int index = VBPERMQ_INDEX(b, (i * 8) + j);
+if (index < 64) {
+uint64_t mask = (1ull << (63 - (index & 0x3F)));
+if (a->u64[VBPERMQ_DW(index)] & mask) {
+perm |= (0x80 >> j);
+}
+}
+r->u64[i] = perm;
+}
+}


The final assignment of perm is done in the wrong loop.


r~

Re: [Qemu-devel] [PATCH v4 for-2.7 0/7] Fixing i686 host / sparc64 guest crash

[Peter Maydell]

On 4 August 2016 at 17:26, Richard Henderson  wrote:
> This is a revision of my "third" attempt, tweaked a bit for Aurelien's
> review.  I've sort of lost track of where we are with the release process,
> so I'll understand if we've gone too far now.

>  include/exec/gen-icount.h |   2 +-
>  include/qemu/log.h|   3 +-
>  tcg/optimize.c|  37 +--
>  tcg/tcg-op.c  |   2 +-
>  tcg/tcg.c | 588 
> ++
>  tcg/tcg.h |  52 ++--
>  util/log.c|  24 +-
>  7 files changed, 441 insertions(+), 267 deletions(-)

We're probably going to tag rc2 tomorrow or maybe Monday, with a theoretical
final release date of the 16th (about a fortnight or so). So this
is a pretty big diffstat to be going in, given it's in generic code
rather than anything specific to only (say) sparc targets or i386
hosts. On the other hand it is a bugfix...

thanks
-- PMM

Re: [Qemu-devel] [PATCH v1 2/5] target-ppc: add vector extract instructions

[Richard Henderson]

On 08/04/2016 06:33 PM, Rajalakshmi Srinivasaraghavan wrote:

+#define VEXTRACT(suffix, element, index) \
+void helper_vextract##suffix(ppc_avr_t *r, ppc_avr_t *b, uint32_t splat) \
+{\
+int i;   \
+ \
+for (i = 0; i < ARRAY_SIZE(r->element); i++) {   \
+r->element[i] = 0;   \
+}\
+memcpy(&r->element[index], &b->u8[SPLAT_ELEMENT(u8)],\
+   sizeof(r->element[0]));   \
+}


You're not caring for R == B.


r~

Re: [Qemu-devel] [PATCH v1 1/5] target-ppc: add vector insert instructions

[Richard Henderson]

On 08/04/2016 06:33 PM, Rajalakshmi Srinivasaraghavan wrote:

+#if defined(HOST_WORDS_BIGENDIAN)
+#define VINSERT(suffix, element, index) \
+void helper_vinsert##suffix(ppc_avr_t *r, ppc_avr_t *b, uint32_t splat) \
+{   \
+memcpy(&r->u8[SPLAT_ELEMENT(u8)], &b->element[index],   \
+   sizeof(r->element[0]));  \
+}
+#else
+#define VINSERT(suffix, element, index) \
+void helper_vinsert##suffix(ppc_avr_t *r, ppc_avr_t *b, uint32_t splat) \
+{   \
+memcpy(&r->u8[(16 - splat) - sizeof(r->element[0])],\
+   &b->element[(ARRAY_SIZE(r->element) - index) - 1],   \
+   sizeof(r->element[0]));  \
+}


Something somewhere needs to check for out of bounds SPLAT, for evil guests.

The spec says it's undefined; I don't recall if that gives you the latitude to 
generate an illegal instruction trap during translate.



r~

[Qemu-devel] [PATCH v4 for-2.7 7/7] tcg: Lower indirect registers in a separate pass

[Richard Henderson]

Rather than rely on recursion during the middle of register allocation,
lower indirect registers to loads and stores off the indirect base into
plain temps.

For an x86_64 host, with sufficient registers, this results in identical
code, modulo the actual register assignments.

For an i686 host, with insufficient registers, this means that temps can
be (temporarily) spilled to the stack in order to satisfy an allocation.
This as opposed to the possibility of not being able to spill, to allocate
a register for the indirect base, in order to perform a spill.

Reviewed-by: Aurelien Jarno 
Signed-off-by: Richard Henderson 
---
 include/qemu/log.h |   1 +
 tcg/optimize.c |  31 +-
 tcg/tcg.c  | 306 +++--
 tcg/tcg.h  |   4 +
 util/log.c |   5 +-
 5 files changed, 263 insertions(+), 84 deletions(-)

diff --git a/include/qemu/log.h b/include/qemu/log.h
index 9ab8f51..00bf37f 100644
--- a/include/qemu/log.h
+++ b/include/qemu/log.h
@@ -42,6 +42,7 @@ static inline bool qemu_log_separate(void)
 #define CPU_LOG_TB_NOCHAIN (1 << 13)
 #define CPU_LOG_PAGE   (1 << 14)
 #define LOG_TRACE  (1 << 15)
+#define CPU_LOG_TB_OP_IND  (1 << 16)
 
 /* Returns true if a bit is set in the current loglevel mask
  */
diff --git a/tcg/optimize.c b/tcg/optimize.c
index 8df7fc7..cffe89b 100644
--- a/tcg/optimize.c
+++ b/tcg/optimize.c
@@ -82,33 +82,6 @@ static void init_temp_info(TCGArg temp)
 }
 }
 
-static TCGOp *insert_op_before(TCGContext *s, TCGOp *old_op,
-TCGOpcode opc, int nargs)
-{
-int oi = s->gen_next_op_idx;
-int pi = s->gen_next_parm_idx;
-int prev = old_op->prev;
-int next = old_op - s->gen_op_buf;
-TCGOp *new_op;
-
-tcg_debug_assert(oi < OPC_BUF_SIZE);
-tcg_debug_assert(pi + nargs <= OPPARAM_BUF_SIZE);
-s->gen_next_op_idx = oi + 1;
-s->gen_next_parm_idx = pi + nargs;
-
-new_op = &s->gen_op_buf[oi];
-*new_op = (TCGOp){
-.opc = opc,
-.args = pi,
-.prev = prev,
-.next = next
-};
-s->gen_op_buf[prev].next = oi;
-old_op->prev = oi;
-
-return new_op;
-}
-
 static int op_bits(TCGOpcode op)
 {
 const TCGOpDef *def = &tcg_op_defs[op];
@@ -1116,7 +1089,7 @@ void tcg_optimize(TCGContext *s)
 uint64_t a = ((uint64_t)ah << 32) | al;
 uint64_t b = ((uint64_t)bh << 32) | bl;
 TCGArg rl, rh;
-TCGOp *op2 = insert_op_before(s, op, INDEX_op_movi_i32, 2);
+TCGOp *op2 = tcg_op_insert_before(s, op, INDEX_op_movi_i32, 2);
 TCGArg *args2 = &s->gen_opparam_buf[op2->args];
 
 if (opc == INDEX_op_add2_i32) {
@@ -1142,7 +1115,7 @@ void tcg_optimize(TCGContext *s)
 uint32_t b = temps[args[3]].val;
 uint64_t r = (uint64_t)a * b;
 TCGArg rl, rh;
-TCGOp *op2 = insert_op_before(s, op, INDEX_op_movi_i32, 2);
+TCGOp *op2 = tcg_op_insert_before(s, op, INDEX_op_movi_i32, 2);
 TCGArg *args2 = &s->gen_opparam_buf[op2->args];
 
 rl = args[0];
diff --git a/tcg/tcg.c b/tcg/tcg.c
index 3c1f526..42417bd 100644
--- a/tcg/tcg.c
+++ b/tcg/tcg.c
@@ -531,8 +531,12 @@ int tcg_global_mem_new_internal(TCGType type, TCGv_ptr 
base,
 #endif
 
 if (!base_ts->fixed_reg) {
-indirect_reg = 1;
+/* We do not support double-indirect registers.  */
+tcg_debug_assert(!base_ts->indirect_reg);
 base_ts->indirect_base = 1;
+s->nb_indirects += (TCG_TARGET_REG_BITS == 32 && type == TCG_TYPE_I64
+? 2 : 1);
+indirect_reg = 1;
 }
 
 if (TCG_TARGET_REG_BITS == 32 && type == TCG_TYPE_I64) {
@@ -1336,9 +1340,66 @@ void tcg_op_remove(TCGContext *s, TCGOp *op)
 #endif
 }
 
+TCGOp *tcg_op_insert_before(TCGContext *s, TCGOp *old_op,
+TCGOpcode opc, int nargs)
+{
+int oi = s->gen_next_op_idx;
+int pi = s->gen_next_parm_idx;
+int prev = old_op->prev;
+int next = old_op - s->gen_op_buf;
+TCGOp *new_op;
+
+tcg_debug_assert(oi < OPC_BUF_SIZE);
+tcg_debug_assert(pi + nargs <= OPPARAM_BUF_SIZE);
+s->gen_next_op_idx = oi + 1;
+s->gen_next_parm_idx = pi + nargs;
+
+new_op = &s->gen_op_buf[oi];
+*new_op = (TCGOp){
+.opc = opc,
+.args = pi,
+.prev = prev,
+.next = next
+};
+s->gen_op_buf[prev].next = oi;
+old_op->prev = oi;
+
+return new_op;
+}
+
+TCGOp *tcg_op_insert_after(TCGContext *s, TCGOp *old_op,
+   TCGOpcode opc, int nargs)
+{
+int oi = s->gen_next_op_idx;
+int pi = s->gen_next_parm_idx;
+int prev = old_op - s->gen_op_buf;
+int next = old_op->next;
+TCGOp *new_op;
+
+tcg_debug_assert(oi < OPC_BUF_SIZE);
+tcg_debug_assert(pi + nargs <= OPPARAM_BUF_SIZE);
+s->gen_next_op_idx = oi + 1;
+s->gen_next_parm_i

[Qemu-devel] [PATCH v4 for-2.7 4/7] tcg: Compress dead_temps and mem_temps into a single array

[Richard Henderson]

We only need two bits per temporary.  Fold the two bytes into one,
and reduce the memory and cachelines required during compilation.

Reviewed-by: Aurelien Jarno 
Signed-off-by: Richard Henderson 
---
 tcg/tcg.c | 119 +++---
 1 file changed, 60 insertions(+), 59 deletions(-)

diff --git a/tcg/tcg.c b/tcg/tcg.c
index 6bcf6e5..27bbb4d 100644
--- a/tcg/tcg.c
+++ b/tcg/tcg.c
@@ -333,7 +333,7 @@ void tcg_context_init(TCGContext *s)
 
 memset(s, 0, sizeof(*s));
 s->nb_globals = 0;
-
+
 /* Count total number of arguments and allocate the corresponding
space */
 total_args = 0;
@@ -825,16 +825,16 @@ void tcg_gen_callN(TCGContext *s, void *func, TCGArg ret,
 real_args++;
 }
 #endif
-   /* If stack grows up, then we will be placing successive
-  arguments at lower addresses, which means we need to
-  reverse the order compared to how we would normally
-  treat either big or little-endian.  For those arguments
-  that will wind up in registers, this still works for
-  HPPA (the only current STACK_GROWSUP target) since the
-  argument registers are *also* allocated in decreasing
-  order.  If another such target is added, this logic may
-  have to get more complicated to differentiate between
-  stack arguments and register arguments.  */
+   /* If stack grows up, then we will be placing successive
+  arguments at lower addresses, which means we need to
+  reverse the order compared to how we would normally
+  treat either big or little-endian.  For those arguments
+  that will wind up in registers, this still works for
+  HPPA (the only current STACK_GROWSUP target) since the
+  argument registers are *also* allocated in decreasing
+  order.  If another such target is added, this logic may
+  have to get more complicated to differentiate between
+  stack arguments and register arguments.  */
 #if defined(HOST_WORDS_BIGENDIAN) != defined(TCG_TARGET_STACK_GROWSUP)
 s->gen_opparam_buf[pi++] = args[i] + 1;
 s->gen_opparam_buf[pi++] = args[i];
@@ -1312,27 +1312,29 @@ void tcg_op_remove(TCGContext *s, TCGOp *op)
 }
 
 #ifdef USE_LIVENESS_ANALYSIS
+
+#define TS_DEAD  1
+#define TS_MEM   2
+
 /* liveness analysis: end of function: all temps are dead, and globals
should be in memory. */
-static inline void tcg_la_func_end(TCGContext *s, uint8_t *dead_temps,
-   uint8_t *mem_temps)
+static inline void tcg_la_func_end(TCGContext *s, uint8_t *temp_state)
 {
-memset(dead_temps, 1, s->nb_temps);
-memset(mem_temps, 1, s->nb_globals);
-memset(mem_temps + s->nb_globals, 0, s->nb_temps - s->nb_globals);
+memset(temp_state, TS_DEAD | TS_MEM, s->nb_globals);
+memset(temp_state + s->nb_globals, TS_DEAD, s->nb_temps - s->nb_globals);
 }
 
 /* liveness analysis: end of basic block: all temps are dead, globals
and local temps should be in memory. */
-static inline void tcg_la_bb_end(TCGContext *s, uint8_t *dead_temps,
- uint8_t *mem_temps)
+static inline void tcg_la_bb_end(TCGContext *s, uint8_t *temp_state)
 {
-int i;
+int i, n;
 
-memset(dead_temps, 1, s->nb_temps);
-memset(mem_temps, 1, s->nb_globals);
-for(i = s->nb_globals; i < s->nb_temps; i++) {
-mem_temps[i] = s->temps[i].temp_local;
+tcg_la_func_end(s, temp_state);
+for (i = s->nb_globals, n = s->nb_temps; i < n; i++) {
+if (s->temps[i].temp_local) {
+temp_state[i] |= TS_MEM;
+}
 }
 }
 
@@ -1341,12 +1343,12 @@ static inline void tcg_la_bb_end(TCGContext *s, uint8_t 
*dead_temps,
temporaries are removed. */
 static void tcg_liveness_analysis(TCGContext *s)
 {
-uint8_t *dead_temps, *mem_temps;
+uint8_t *temp_state;
 int oi, oi_prev;
+int nb_globals = s->nb_globals;
 
-dead_temps = tcg_malloc(s->nb_temps);
-mem_temps = tcg_malloc(s->nb_temps);
-tcg_la_func_end(s, dead_temps, mem_temps);
+temp_state = tcg_malloc(s->nb_temps);
+tcg_la_func_end(s, temp_state);
 
 for (oi = s->gen_op_buf[0].prev; oi != 0; oi = oi_prev) {
 int i, nb_iargs, nb_oargs;
@@ -1375,7 +1377,7 @@ static void tcg_liveness_analysis(TCGContext *s)
 if (call_flags & TCG_CALL_NO_SIDE_EFFECTS) {
 for (i = 0; i < nb_oargs; i++) {
 arg = args[i];
-if (!dead_temps[arg] || mem_temps[arg]) {
+if (temp_state[arg] != TS_DEAD) {
 goto do_not_remove_call;
 }
 }
@@ -1386,39 +1388,41 @@ static void tcg_liveness_analysis(TCGContext *s)
 /* output args are dead */

[Qemu-devel] [PATCH v4 for-2.7 5/7] tcg: Include liveness info in the dumps

[Richard Henderson]

Reviewed-by: Aurelien Jarno 
Signed-off-by: Richard Henderson 
---
 include/qemu/log.h |  2 +-
 tcg/tcg.c  | 68 +-
 util/log.c | 19 ++-
 3 files changed, 61 insertions(+), 28 deletions(-)

diff --git a/include/qemu/log.h b/include/qemu/log.h
index 8bec6b4..9ab8f51 100644
--- a/include/qemu/log.h
+++ b/include/qemu/log.h
@@ -54,7 +54,7 @@ static inline bool qemu_loglevel_mask(int mask)
 
 /* main logging function
  */
-void GCC_FMT_ATTR(1, 2) qemu_log(const char *fmt, ...);
+int GCC_FMT_ATTR(1, 2) qemu_log(const char *fmt, ...);
 
 /* vfprintf-like logging function
  */
diff --git a/tcg/tcg.c b/tcg/tcg.c
index 27bbb4d..b0a88ba 100644
--- a/tcg/tcg.c
+++ b/tcg/tcg.c
@@ -1026,6 +1026,7 @@ void tcg_dump_ops(TCGContext *s)
 const TCGOpDef *def;
 const TCGArg *args;
 TCGOpcode c;
+int col = 0;
 
 op = &s->gen_op_buf[oi];
 c = op->opc;
@@ -1033,7 +1034,7 @@ void tcg_dump_ops(TCGContext *s)
 args = &s->gen_opparam_buf[op->args];
 
 if (c == INDEX_op_insn_start) {
-qemu_log("%s ", oi != s->gen_op_buf[0].next ? "\n" : "");
+col += qemu_log("%s ", oi != s->gen_op_buf[0].next ? "\n" : 
"");
 
 for (i = 0; i < TARGET_INSN_START_WORDS; ++i) {
 target_ulong a;
@@ -1042,7 +1043,7 @@ void tcg_dump_ops(TCGContext *s)
 #else
 a = args[i];
 #endif
-qemu_log(" " TARGET_FMT_lx, a);
+col += qemu_log(" " TARGET_FMT_lx, a);
 }
 } else if (c == INDEX_op_call) {
 /* variable number of arguments */
@@ -1051,12 +1052,12 @@ void tcg_dump_ops(TCGContext *s)
 nb_cargs = def->nb_cargs;
 
 /* function name, flags, out args */
-qemu_log(" %s %s,$0x%" TCG_PRIlx ",$%d", def->name,
- tcg_find_helper(s, args[nb_oargs + nb_iargs]),
- args[nb_oargs + nb_iargs + 1], nb_oargs);
+col += qemu_log(" %s %s,$0x%" TCG_PRIlx ",$%d", def->name,
+tcg_find_helper(s, args[nb_oargs + nb_iargs]),
+args[nb_oargs + nb_iargs + 1], nb_oargs);
 for (i = 0; i < nb_oargs; i++) {
-qemu_log(",%s", tcg_get_arg_str_idx(s, buf, sizeof(buf),
-   args[i]));
+col += qemu_log(",%s", tcg_get_arg_str_idx(s, buf, sizeof(buf),
+   args[i]));
 }
 for (i = 0; i < nb_iargs; i++) {
 TCGArg arg = args[nb_oargs + i];
@@ -1064,10 +1065,10 @@ void tcg_dump_ops(TCGContext *s)
 if (arg != TCG_CALL_DUMMY_ARG) {
 t = tcg_get_arg_str_idx(s, buf, sizeof(buf), arg);
 }
-qemu_log(",%s", t);
+col += qemu_log(",%s", t);
 }
 } else {
-qemu_log(" %s ", def->name);
+col += qemu_log(" %s ", def->name);
 
 nb_oargs = def->nb_oargs;
 nb_iargs = def->nb_iargs;
@@ -1076,17 +1077,17 @@ void tcg_dump_ops(TCGContext *s)
 k = 0;
 for (i = 0; i < nb_oargs; i++) {
 if (k != 0) {
-qemu_log(",");
+col += qemu_log(",");
 }
-qemu_log("%s", tcg_get_arg_str_idx(s, buf, sizeof(buf),
-   args[k++]));
+col += qemu_log("%s", tcg_get_arg_str_idx(s, buf, sizeof(buf),
+  args[k++]));
 }
 for (i = 0; i < nb_iargs; i++) {
 if (k != 0) {
-qemu_log(",");
+col += qemu_log(",");
 }
-qemu_log("%s", tcg_get_arg_str_idx(s, buf, sizeof(buf),
-   args[k++]));
+col += qemu_log("%s", tcg_get_arg_str_idx(s, buf, sizeof(buf),
+  args[k++]));
 }
 switch (c) {
 case INDEX_op_brcond_i32:
@@ -1098,9 +1099,9 @@ void tcg_dump_ops(TCGContext *s)
 case INDEX_op_setcond_i64:
 case INDEX_op_movcond_i64:
 if (args[k] < ARRAY_SIZE(cond_name) && cond_name[args[k]]) {
-qemu_log(",%s", cond_name[args[k++]]);
+col += qemu_log(",%s", cond_name[args[k++]]);
 } else {
-qemu_log(",$0x%" TCG_PRIlx, args[k++]);
+col += qemu_log(",$0x%" TCG_PRIlx, args[k++]);
 }
 i = 1;
 break;
@@ -1114,12 +1115,12 @@ void tcg_dump_ops(TCGContext *s)
 unsigned ix = get_mmuidx(oi);
 
 if (op & ~(MO_AMASK | MO_BSWAP |

[Qemu-devel] [PATCH v4 for-2.7 6/7] tcg: Require liveness analysis

[Richard Henderson]

Reviewed-by: Aurelien Jarno 
Signed-off-by: Richard Henderson 
---
 tcg/tcg.c | 21 -
 1 file changed, 21 deletions(-)

diff --git a/tcg/tcg.c b/tcg/tcg.c
index b0a88ba..3c1f526 100644
--- a/tcg/tcg.c
+++ b/tcg/tcg.c
@@ -23,7 +23,6 @@
  */
 
 /* define it to use liveness analysis (better code) */
-#define USE_LIVENESS_ANALYSIS
 #define USE_TCG_OPTIMIZATIONS
 
 #include "qemu/osdep.h"
@@ -1337,8 +1336,6 @@ void tcg_op_remove(TCGContext *s, TCGOp *op)
 #endif
 }
 
-#ifdef USE_LIVENESS_ANALYSIS
-
 #define TS_DEAD  1
 #define TS_MEM   2
 
@@ -1595,18 +1592,6 @@ static void tcg_liveness_analysis(TCGContext *s)
 op->life = arg_life;
 }
 }
-#else
-/* dummy liveness analysis */
-static void tcg_liveness_analysis(TCGContext *s)
-{
-int nb_ops = s->gen_next_op_idx;
-
-s->op_dead_args = tcg_malloc(nb_ops * sizeof(uint16_t));
-memset(s->op_dead_args, 0, nb_ops * sizeof(uint16_t));
-s->op_sync_args = tcg_malloc(nb_ops * sizeof(uint8_t));
-memset(s->op_sync_args, 0, nb_ops * sizeof(uint8_t));
-}
-#endif
 
 #ifdef CONFIG_DEBUG_TCG
 static void dump_regs(TCGContext *s)
@@ -1858,7 +1843,6 @@ static void temp_load(TCGContext *s, TCGTemp *ts, 
TCGRegSet desired_regs,
temporary registers needs to be allocated to store a constant.  */
 static void temp_save(TCGContext *s, TCGTemp *ts, TCGRegSet allocated_regs)
 {
-#ifdef USE_LIVENESS_ANALYSIS
 /* ??? Liveness does not yet incorporate indirect bases.  */
 if (!ts->indirect_base) {
 /* The liveness analysis already ensures that globals are back
@@ -1866,7 +1850,6 @@ static void temp_save(TCGContext *s, TCGTemp *ts, 
TCGRegSet allocated_regs)
 tcg_debug_assert(ts->val_type == TEMP_VAL_MEM || ts->fixed_reg);
 return;
 }
-#endif
 temp_sync(s, ts, allocated_regs, 1);
 }
 
@@ -1891,7 +1874,6 @@ static void sync_globals(TCGContext *s, TCGRegSet 
allocated_regs)
 
 for (i = 0; i < s->nb_globals; i++) {
 TCGTemp *ts = &s->temps[i];
-#ifdef USE_LIVENESS_ANALYSIS
 /* ??? Liveness does not yet incorporate indirect bases.  */
 if (!ts->indirect_base) {
 tcg_debug_assert(ts->val_type != TEMP_VAL_REG
@@ -1899,7 +1881,6 @@ static void sync_globals(TCGContext *s, TCGRegSet 
allocated_regs)
  || ts->mem_coherent);
 continue;
 }
-#endif
 temp_sync(s, ts, allocated_regs, 0);
 }
 }
@@ -1915,7 +1896,6 @@ static void tcg_reg_alloc_bb_end(TCGContext *s, TCGRegSet 
allocated_regs)
 if (ts->temp_local) {
 temp_save(s, ts, allocated_regs);
 } else {
-#ifdef USE_LIVENESS_ANALYSIS
 /* ??? Liveness does not yet incorporate indirect bases.  */
 if (!ts->indirect_base) {
 /* The liveness analysis already ensures that temps are dead.
@@ -1923,7 +1903,6 @@ static void tcg_reg_alloc_bb_end(TCGContext *s, TCGRegSet 
allocated_regs)
 tcg_debug_assert(ts->val_type == TEMP_VAL_DEAD);
 continue;
 }
-#endif
 temp_dead(s, ts);
 }
 }
-- 
2.7.4

[Qemu-devel] [PATCH v4 for-2.7 3/7] tcg: Fold life data into TCGOp

[Richard Henderson]

Reduce the size of other bitfields to make room.
This reduces the cache footprint of compilation.

Reviewed-by: Aurelien Jarno 
Signed-off-by: Richard Henderson 
---
 tcg/tcg.c |  9 +++--
 tcg/tcg.h | 26 ++
 2 files changed, 17 insertions(+), 18 deletions(-)

diff --git a/tcg/tcg.c b/tcg/tcg.c
index cd76e42..6bcf6e5 100644
--- a/tcg/tcg.c
+++ b/tcg/tcg.c
@@ -1342,10 +1342,7 @@ static inline void tcg_la_bb_end(TCGContext *s, uint8_t 
*dead_temps,
 static void tcg_liveness_analysis(TCGContext *s)
 {
 uint8_t *dead_temps, *mem_temps;
-int oi, oi_prev, nb_ops;
-
-nb_ops = s->gen_next_op_idx;
-s->op_arg_life = tcg_malloc(nb_ops * sizeof(TCGLifeData));
+int oi, oi_prev;
 
 dead_temps = tcg_malloc(s->nb_temps);
 mem_temps = tcg_malloc(s->nb_temps);
@@ -1568,7 +1565,7 @@ static void tcg_liveness_analysis(TCGContext *s)
 }
 break;
 }
-s->op_arg_life[oi] = arg_life;
+op->life = arg_life;
 }
 }
 #else
@@ -2410,7 +2407,7 @@ int tcg_gen_code(TCGContext *s, TranslationBlock *tb)
 TCGArg * const args = &s->gen_opparam_buf[op->args];
 TCGOpcode opc = op->opc;
 const TCGOpDef *def = &tcg_op_defs[opc];
-TCGLifeData arg_life = s->op_arg_life[oi];
+TCGLifeData arg_life = op->life;
 
 oi_next = op->next;
 #ifdef CONFIG_PROFILER
diff --git a/tcg/tcg.h b/tcg/tcg.h
index 007d7bc..ebf6867 100644
--- a/tcg/tcg.h
+++ b/tcg/tcg.h
@@ -583,25 +583,30 @@ typedef struct TCGTempSet {
 #define SYNC_ARG  1
 typedef uint16_t TCGLifeData;
 
+/* The layout here is designed to avoid crossing of a 32-bit boundary.
+   If we do so, gcc adds padding, expanding the size to 12.  */
 typedef struct TCGOp {
-TCGOpcode opc   : 8;
+TCGOpcode opc   : 8;/*  8 */
+
+/* Index of the prev/next op, or 0 for the end of the list.  */
+unsigned prev   : 10;   /* 18 */
+unsigned next   : 10;   /* 28 */
 
 /* The number of out and in parameter for a call.  */
-unsigned callo  : 2;
-unsigned calli  : 6;
+unsigned calli  : 4;/* 32 */
+unsigned callo  : 2;/* 34 */
 
 /* Index of the arguments for this op, or 0 for zero-operand ops.  */
-unsigned args   : 16;
+unsigned args   : 14;   /* 48 */
 
-/* Index of the prev/next op, or 0 for the end of the list.  */
-unsigned prev   : 16;
-unsigned next   : 16;
+/* Lifetime data of the operands.  */
+unsigned life   : 16;   /* 64 */
 } TCGOp;
 
 /* Make sure operands fit in the bitfields above.  */
 QEMU_BUILD_BUG_ON(NB_OPS > (1 << 8));
-QEMU_BUILD_BUG_ON(OPC_BUF_SIZE > (1 << 16));
-QEMU_BUILD_BUG_ON(OPPARAM_BUF_SIZE > (1 << 16));
+QEMU_BUILD_BUG_ON(OPC_BUF_SIZE > (1 << 10));
+QEMU_BUILD_BUG_ON(OPPARAM_BUF_SIZE > (1 << 14));
 
 /* Make sure that we don't overflow 64 bits without noticing.  */
 QEMU_BUILD_BUG_ON(sizeof(TCGOp) > 8);
@@ -619,9 +624,6 @@ struct TCGContext {
 uint16_t *tb_jmp_insn_offset; /* tb->jmp_insn_offset if USE_DIRECT_JUMP */
 uintptr_t *tb_jmp_target_addr; /* tb->jmp_target_addr if !USE_DIRECT_JUMP 
*/
 
-/* liveness analysis */
-TCGLifeData *op_arg_life;
-
 TCGRegSet reserved_regs;
 intptr_t current_frame_offset;
 intptr_t frame_start;
-- 
2.7.4

[Qemu-devel] [PATCH v4 for-2.7 2/7] tcg: Reorg TCGOp chaining

[Richard Henderson]

Instead of using -1 as end of chain, use 0, and link through the 0
entry as a fully circular double-linked list.

Signed-off-by: Richard Henderson 
---
 include/exec/gen-icount.h |  2 +-
 tcg/optimize.c|  8 ++--
 tcg/tcg-op.c  |  2 +-
 tcg/tcg.c | 35 +++
 tcg/tcg.h | 22 --
 5 files changed, 31 insertions(+), 38 deletions(-)

diff --git a/include/exec/gen-icount.h b/include/exec/gen-icount.h
index 1af03d8..050de59 100644
--- a/include/exec/gen-icount.h
+++ b/include/exec/gen-icount.h
@@ -59,7 +59,7 @@ static void gen_tb_end(TranslationBlock *tb, int num_insns)
 }
 
 /* Terminate the linked list.  */
-tcg_ctx.gen_op_buf[tcg_ctx.gen_last_op_idx].next = -1;
+tcg_ctx.gen_op_buf[tcg_ctx.gen_op_buf[0].prev].next = 0;
 }
 
 static inline void gen_io_start(void)
diff --git a/tcg/optimize.c b/tcg/optimize.c
index c0d975b..8df7fc7 100644
--- a/tcg/optimize.c
+++ b/tcg/optimize.c
@@ -103,11 +103,7 @@ static TCGOp *insert_op_before(TCGContext *s, TCGOp 
*old_op,
 .prev = prev,
 .next = next
 };
-if (prev >= 0) {
-s->gen_op_buf[prev].next = oi;
-} else {
-s->gen_first_op_idx = oi;
-}
+s->gen_op_buf[prev].next = oi;
 old_op->prev = oi;
 
 return new_op;
@@ -583,7 +579,7 @@ void tcg_optimize(TCGContext *s)
 nb_globals = s->nb_globals;
 reset_all_temps(nb_temps);
 
-for (oi = s->gen_first_op_idx; oi >= 0; oi = oi_next) {
+for (oi = s->gen_op_buf[0].next; oi != 0; oi = oi_next) {
 tcg_target_ulong mask, partmask, affected;
 int nb_oargs, nb_iargs, i;
 TCGArg tmp;
diff --git a/tcg/tcg-op.c b/tcg/tcg-op.c
index 293b854..0243c99 100644
--- a/tcg/tcg-op.c
+++ b/tcg/tcg-op.c
@@ -52,7 +52,7 @@ static void tcg_emit_op(TCGContext *ctx, TCGOpcode opc, int 
args)
 int pi = oi - 1;
 
 tcg_debug_assert(oi < OPC_BUF_SIZE);
-ctx->gen_last_op_idx = oi;
+ctx->gen_op_buf[0].prev = oi;
 ctx->gen_next_op_idx = ni;
 
 ctx->gen_op_buf[oi] = (TCGOp){
diff --git a/tcg/tcg.c b/tcg/tcg.c
index 4aa1933..cd76e42 100644
--- a/tcg/tcg.c
+++ b/tcg/tcg.c
@@ -438,9 +438,9 @@ void tcg_func_start(TCGContext *s)
 s->goto_tb_issue_mask = 0;
 #endif
 
-s->gen_first_op_idx = 0;
-s->gen_last_op_idx = -1;
-s->gen_next_op_idx = 0;
+s->gen_op_buf[0].next = 1;
+s->gen_op_buf[0].prev = 0;
+s->gen_next_op_idx = 1;
 s->gen_next_parm_idx = 0;
 
 s->be = tcg_malloc(sizeof(TCGBackendData));
@@ -869,7 +869,7 @@ void tcg_gen_callN(TCGContext *s, void *func, TCGArg ret,
 /* Make sure the calli field didn't overflow.  */
 tcg_debug_assert(s->gen_op_buf[i].calli == real_args);
 
-s->gen_last_op_idx = i;
+s->gen_op_buf[0].prev = i;
 s->gen_next_op_idx = i + 1;
 s->gen_next_parm_idx = pi;
 
@@ -1021,7 +1021,7 @@ void tcg_dump_ops(TCGContext *s)
 TCGOp *op;
 int oi;
 
-for (oi = s->gen_first_op_idx; oi >= 0; oi = op->next) {
+for (oi = s->gen_op_buf[0].next; oi != 0; oi = op->next) {
 int i, k, nb_oargs, nb_iargs, nb_cargs;
 const TCGOpDef *def;
 const TCGArg *args;
@@ -1033,7 +1033,7 @@ void tcg_dump_ops(TCGContext *s)
 args = &s->gen_opparam_buf[op->args];
 
 if (c == INDEX_op_insn_start) {
-qemu_log("%s ", oi != s->gen_first_op_idx ? "\n" : "");
+qemu_log("%s ", oi != s->gen_op_buf[0].next ? "\n" : "");
 
 for (i = 0; i < TARGET_INSN_START_WORDS; ++i) {
 target_ulong a;
@@ -1298,18 +1298,13 @@ void tcg_op_remove(TCGContext *s, TCGOp *op)
 int next = op->next;
 int prev = op->prev;
 
-if (next >= 0) {
-s->gen_op_buf[next].prev = prev;
-} else {
-s->gen_last_op_idx = prev;
-}
-if (prev >= 0) {
-s->gen_op_buf[prev].next = next;
-} else {
-s->gen_first_op_idx = next;
-}
+/* We should never attempt to remove the list terminator.  */
+tcg_debug_assert(op != &s->gen_op_buf[0]);
+
+s->gen_op_buf[next].prev = prev;
+s->gen_op_buf[prev].next = next;
 
-memset(op, -1, sizeof(*op));
+memset(op, 0, sizeof(*op));
 
 #ifdef CONFIG_PROFILER
 s->del_op_count++;
@@ -1356,7 +1351,7 @@ static void tcg_liveness_analysis(TCGContext *s)
 mem_temps = tcg_malloc(s->nb_temps);
 tcg_la_func_end(s, dead_temps, mem_temps);
 
-for (oi = s->gen_last_op_idx; oi >= 0; oi = oi_prev) {
+for (oi = s->gen_op_buf[0].prev; oi != 0; oi = oi_prev) {
 int i, nb_iargs, nb_oargs;
 TCGOpcode opc_new, opc_new2;
 bool have_opc_new2;
@@ -2351,7 +2346,7 @@ int tcg_gen_code(TCGContext *s, TranslationBlock *tb)
 {
 int n;
 
-n = s->gen_last_op_idx + 1;
+n = s->gen_op_buf[0].prev + 1;
 s->op_count += n;
 if (n > s->op_count_max) {
 s->op_count_max = n;
@@ -2410,7 +2405,7 @@ int tcg_gen_code(TCGContext *s, TranslationBlock *

[Qemu-devel] [PATCH v4 for-2.7 1/7] tcg: Compress liveness data to 16 bits

[Richard Henderson]

This reduces both memory usage and per-insn cacheline usage
during code generation.

Reviewed-by: Aurelien Jarno 
Signed-off-by: Richard Henderson 
---
 tcg/tcg.c | 58 ++
 tcg/tcg.h | 16 ++--
 2 files changed, 32 insertions(+), 42 deletions(-)

diff --git a/tcg/tcg.c b/tcg/tcg.c
index 0c46c43..4aa1933 100644
--- a/tcg/tcg.c
+++ b/tcg/tcg.c
@@ -1341,7 +1341,7 @@ static inline void tcg_la_bb_end(TCGContext *s, uint8_t 
*dead_temps,
 }
 }
 
-/* Liveness analysis : update the opc_dead_args array to tell if a
+/* Liveness analysis : update the opc_arg_life array to tell if a
given input arguments is dead. Instructions updating dead
temporaries are removed. */
 static void tcg_liveness_analysis(TCGContext *s)
@@ -1350,9 +1350,8 @@ static void tcg_liveness_analysis(TCGContext *s)
 int oi, oi_prev, nb_ops;
 
 nb_ops = s->gen_next_op_idx;
-s->op_dead_args = tcg_malloc(nb_ops * sizeof(uint16_t));
-s->op_sync_args = tcg_malloc(nb_ops * sizeof(uint8_t));
-
+s->op_arg_life = tcg_malloc(nb_ops * sizeof(TCGLifeData));
+
 dead_temps = tcg_malloc(s->nb_temps);
 mem_temps = tcg_malloc(s->nb_temps);
 tcg_la_func_end(s, dead_temps, mem_temps);
@@ -1361,8 +1360,7 @@ static void tcg_liveness_analysis(TCGContext *s)
 int i, nb_iargs, nb_oargs;
 TCGOpcode opc_new, opc_new2;
 bool have_opc_new2;
-uint16_t dead_args;
-uint8_t sync_args;
+TCGLifeData arg_life = 0;
 TCGArg arg;
 
 TCGOp * const op = &s->gen_op_buf[oi];
@@ -1394,15 +1392,13 @@ static void tcg_liveness_analysis(TCGContext *s)
 do_not_remove_call:
 
 /* output args are dead */
-dead_args = 0;
-sync_args = 0;
 for (i = 0; i < nb_oargs; i++) {
 arg = args[i];
 if (dead_temps[arg]) {
-dead_args |= (1 << i);
+arg_life |= DEAD_ARG << i;
 }
 if (mem_temps[arg]) {
-sync_args |= (1 << i);
+arg_life |= SYNC_ARG << i;
 }
 dead_temps[arg] = 1;
 mem_temps[arg] = 0;
@@ -1423,7 +1419,7 @@ static void tcg_liveness_analysis(TCGContext *s)
 arg = args[i];
 if (arg != TCG_CALL_DUMMY_ARG) {
 if (dead_temps[arg]) {
-dead_args |= (1 << i);
+arg_life |= DEAD_ARG << i;
 }
 }
 }
@@ -1432,8 +1428,6 @@ static void tcg_liveness_analysis(TCGContext *s)
 arg = args[i];
 dead_temps[arg] = 0;
 }
-s->op_dead_args[oi] = dead_args;
-s->op_sync_args[oi] = sync_args;
 }
 }
 break;
@@ -1544,15 +1538,13 @@ static void tcg_liveness_analysis(TCGContext *s)
 } else {
 do_not_remove:
 /* output args are dead */
-dead_args = 0;
-sync_args = 0;
 for (i = 0; i < nb_oargs; i++) {
 arg = args[i];
 if (dead_temps[arg]) {
-dead_args |= (1 << i);
+arg_life |= DEAD_ARG << i;
 }
 if (mem_temps[arg]) {
-sync_args |= (1 << i);
+arg_life |= SYNC_ARG << i;
 }
 dead_temps[arg] = 1;
 mem_temps[arg] = 0;
@@ -1570,7 +1562,7 @@ static void tcg_liveness_analysis(TCGContext *s)
 for (i = nb_oargs; i < nb_oargs + nb_iargs; i++) {
 arg = args[i];
 if (dead_temps[arg]) {
-dead_args |= (1 << i);
+arg_life |= DEAD_ARG << i;
 }
 }
 /* input arguments are live for preceding opcodes */
@@ -1578,11 +1570,10 @@ static void tcg_liveness_analysis(TCGContext *s)
 arg = args[i];
 dead_temps[arg] = 0;
 }
-s->op_dead_args[oi] = dead_args;
-s->op_sync_args[oi] = sync_args;
 }
 break;
 }
+s->op_arg_life[oi] = arg_life;
 }
 }
 #else
@@ -1921,11 +1912,11 @@ static void tcg_reg_alloc_bb_end(TCGContext *s, 
TCGRegSet allocated_regs)
 save_globals(s, allocated_regs);
 }
 
-#define IS_DEAD_ARG(n) ((dead_args >> (n)) & 1)
-#define NEED_SYNC_ARG(n) ((sync_args >> (n)) & 1)
+#define IS_DEAD_ARG(n)   (arg_life & (DEAD_ARG << (n)))
+#define NEED_SYNC_ARG(n) 

[Qemu-devel] [PATCH v4 for-2.7 0/7] Fixing i686 host / sparc64 guest crash

[Richard Henderson]

This is a revision of my "third" attempt, tweaked a bit for Aurelien's
review.  I've sort of lost track of where we are with the release process,
so I'll understand if we've gone too far now.


r~


Richard Henderson (7):
  tcg: Compress liveness data to 16 bits
  tcg: Reorg TCGOp chaining
  tcg: Fold life data into TCGOp
  tcg: Compress dead_temps and mem_temps into a single array
  tcg: Include liveness info in the dumps
  tcg: Require liveness analysis
  tcg: Lower indirect registers in a separate pass

 include/exec/gen-icount.h |   2 +-
 include/qemu/log.h|   3 +-
 tcg/optimize.c|  37 +--
 tcg/tcg-op.c  |   2 +-
 tcg/tcg.c | 588 ++
 tcg/tcg.h |  52 ++--
 util/log.c|  24 +-
 7 files changed, 441 insertions(+), 267 deletions(-)

-- 
2.7.4

Re: [Qemu-devel] [PATCH v2] balloon: Fix failure of updating guest memory status

[Michael S. Tsirkin]

On Thu, Aug 04, 2016 at 05:14:14PM +0200, Ladi Prosek wrote:
> On Wed, Aug 3, 2016 at 9:25 AM, Ladi Prosek  wrote:
> > On Tue, Aug 2, 2016 at 2:11 AM, Michael S. Tsirkin  wrote:
> >> On Mon, Aug 01, 2016 at 11:59:31PM +, Li, Liang Z wrote:
> >>> > On Wed, Jul 06, 2016 at 12:49:06PM +, Li, Liang Z wrote:
> >>> > > > > > > After live migration, 'guest-stats' can't get the expected
> >>> > > > > > > memory status in the guest. This issue is caused by commit
> >>> > 4eae2a657d.
> >>> > > > > > > The value of 's->stats_vq_elem' will be NULL after live
> >>> > > > > > > migration, and the check in the function
> >>> > > > > > > 'balloon_stats_poll_cb()' will prevent the 'virtio_notify()'
> >>> > > > > > > from executing. So guest will not update the memory status.
> >>> > > > > > >
> >>> > > > > > > Commit 4eae2a657d is doing the right thing, but 
> >>> > > > > > > 's->stats_vq_elem'
> >>> > > > > > > should be treated as part of balloon device state and migrated
> >>> > > > > > > to destination if it's not NULL to make everything works well.
> >>> > > > > > >
> >>> > > > > > > Signed-off-by: Liang Li 
> >>> > > > > > > Suggested-by: Paolo Bonzini 
> >>> > > > > > > Cc: Michael S. Tsirkin 
> >>> > > > > > > Cc: Ladi Prosek 
> >>> > > > > > > Cc: Paolo Bonzini 
> >>> > > > > >
> >>> > > > > > I agree there's an issue but we don't change versions anymore.
> >>> > > > > > Breaking migrations for everyone is also not nice.
> >>> > > > > >
> >>> > > > > > How about queueing virtio_balloon_receive_stats so it will get
> >>> > > > > > invoked when vm starts?
> >>> > > > > >
> >>> > > > >
> >>> > > > > Could you give more explanation about how it works?  I can't 
> >>> > > > > catch you.
> >>> > > > >
> >>> > > > > Thanks!
> >>> > > > > Liang
> >>> > > >
> >>> > > > virtqueue_discard before migration
> >>> > > >
> >>> > > > virtio_balloon_receive_stats after migration
> >>> > > >
> >>> > >
> >>> > > Sorry, I still can't catch you. Maybe it's easier for you to submit a
> >>> > > patch than writing a lot a words to make me understand your idea.
> >>> >
> >>> > I'm rather busy now.  I might look into it towards end of the month.
> >>> >
> >>> > > I just don't understand why not to use the version to make things
> >>> > > easier, is that not the original intent of version id?
> >>> >
> >>> > This was the original idea but we stopped using version ids since they 
> >>> > have
> >>> > many shortcomings.
> >>> >
> >>> > > If we want to extend the device and more states are needed, the idea
> >>> > > you suggest can be used as a common solution?
> >>> > >
> >>> > > Thanks!
> >>> > > Liang
> >>> >
> >>> > The idea is to try to avoid adding more state. that's not always 
> >>> > possible but in
> >>> > this case element was seen but not consumed yet, so it should be 
> >>> > possible
> >>> > for destination to simply get it from the VQ again.
> >>> >
> >>> > > > --
> >>> > > > MST
> >>>
> >>> Hi Michel,
> >>>
> >>> Do you have time for this issue recently?
> >>>
> >>> Thanks!
> >>> Liang
> >
> > Hi Liang,
> >
> > I should be able to look into it this week if you help me with testing.
> >
> > Thanks,
> > Ladi
> 
> Please try the attached patch. I have tested it with a simple
> 'migrate' to save the state and then '-incoming' to load it back.
> 
> One question for you: is it expected that stats_poll_interval is not
> preserved by save/load? I had to explicitly set
> guest-stats-polling-interval on the receiving VM to start getting
> stats again. It's also the reason why the new
> virtio_balloon_receive_stats call is not under if
> (balloon_stats_enabled(s)) because this condition always evaluates to
> false for me.
> 
> Thanks!
> Ladi
> 
> >> Sorry, doesn't look like I will.
> >> Idea is to make sure balloon_stats_poll_cb runs
> >> on source. This will set stats_vq_elem to NULL.
> >>
> >>
> >> --
> >> MST

> From f2f779e12f4aa4d3469d1b44e54484e66f82a2d7 Mon Sep 17 00:00:00 2001
> From: Ladi Prosek 
> Date: Thu, 4 Aug 2016 15:22:05 +0200
> Subject: [PATCH] balloon: preserve stats virtqueue state across migrations
> 
> Signed-off-by: Ladi Prosek 
> ---
>  hw/virtio/virtio-balloon.c | 18 +-
>  1 file changed, 17 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/virtio/virtio-balloon.c b/hw/virtio/virtio-balloon.c
> index 5af429a..1293be0 100644
> --- a/hw/virtio/virtio-balloon.c
> +++ b/hw/virtio/virtio-balloon.c
> @@ -396,6 +396,19 @@ static void virtio_balloon_to_target(void *opaque, 
> ram_addr_t target)
>  trace_virtio_balloon_to_target(target, dev->num_pages);
>  }
>  
> +static void virtio_balloon_save(QEMUFile *f, void *opaque, size_t size)
> +{
> +VirtIOBalloon *s = VIRTIO_BALLOON(opaque);
> +
> +if (s->stats_vq_elem != NULL) {
> +virtqueue_discard(s->svq, s->stats_vq_elem, s->stats_vq_offset);
> +g_free(s->stats_vq_elem);
> +s->stats_vq_elem = NULL;
> +}
> +
> +virtio_save(VIRTIO_DEVICE(opaque), f);
> +}
> +
>  static void virtio_balloon_save_device(VirtIODev

Re: [Qemu-devel] [PATCH 1/2] ide: ide-cd without drive property for empty drive

[Eric Blake]

On 08/01/2016 01:47 PM, Kevin Wolf wrote:
> Am 14.07.2016 um 21:48 hat Eric Blake geschrieben:
>> On 07/14/2016 07:49 AM, Kevin Wolf wrote:
>>> This allows to create an empty ide-cd device without manually creating a
>>> BlockBackend.
>>>
>>> Signed-off-by: Kevin Wolf 
>>> ---
>>>  hw/ide/qdev.c | 20 +++-
>>>  1 file changed, 15 insertions(+), 5 deletions(-)
>>
>>> @@ -158,6 +154,16 @@ static int ide_dev_initfn(IDEDevice *dev, IDEDriveKind 
>>> kind)
>>>  IDEState *s = bus->ifs + dev->unit;
>>>  Error *err = NULL;
>>>  
>>> +if (!dev->conf.blk) {
>>> +if (kind != IDE_CD) {
>>> +error_report("No drive specified");
>>> +return -1;
>>> +} else {
>>> +/* Anonymous BlockBackend for an empty drive */
>>> +dev->conf.blk = blk_new();
>>
>> So we either fail or dev->conf.blk is set...
>>
>>> +}
>>> +}
>>> +
>>>  if (dev->conf.discard_granularity == -1) {
>>>  dev->conf.discard_granularity = 512;
>>>  } else if (dev->conf.discard_granularity &&
>>> @@ -257,7 +263,11 @@ static int ide_cd_initfn(IDEDevice *dev)
>>>  
>>>  static int ide_drive_initfn(IDEDevice *dev)
>>>  {
>>> -DriveInfo *dinfo = blk_legacy_dinfo(dev->conf.blk);
>>> +DriveInfo *dinfo = NULL;
>>> +
>>> +if (dev->conf.blk) {
>>> +dinfo = blk_legacy_dinfo(dev->conf.blk);
>>> +}
>>>  
>>>  return ide_dev_initfn(dev, dinfo && dinfo->media_cd ? IDE_CD : IDE_HD);
>>
>> ...yet, this claims dev->conf.blk can be NULL.  What am I missing?
> 
> That ide_drive_initfn() is the outer function and runs first, before we
> handle the case in ide_dev_initfn()?

Ah, I think I glazed over the difference between 'dev' and 'drive', and
was thinking 'both of these are initfn, what's different?'.  I think you
are okay, after all.

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



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH 1/7] util: Add UUID API

[Daniel P. Berrange]

On Tue, Aug 02, 2016 at 05:18:32PM +0800, Fam Zheng wrote:
> A number of different places across the code base use CONFIG_UUID. Some
> of them are soft dependency, some are not built if libuuid is not
> available, some come with dummy fallback, some throws runtime error.
> 
> It is hard to maintain, and hard to reason for users.
> 
> Since UUID is a simple standard with only a small number of operations,
> it is cleaner to have a central support in libqemuutil. This patch adds
> qemu_uuid_* the functions so that all uuid users in the code base can
> rely on. Except for qemu_uuid_generate which is new code, all other
> functions are just copy from existing fallbacks from other files.
> 
> Signed-off-by: Fam Zheng 
> ---
>  arch_init.c | 19 ---
>  block/iscsi.c   |  2 +-
>  hw/smbios/smbios.c  |  1 +
>  include/qemu/uuid.h | 37 +
>  include/sysemu/sysemu.h |  4 
>  qmp.c   |  1 +
>  stubs/uuid.c|  2 +-
>  util/Makefile.objs  |  1 +
>  util/uuid.c | 63 
> +
>  vl.c|  1 +
>  10 files changed, 106 insertions(+), 25 deletions(-)
>  create mode 100644 include/qemu/uuid.h
>  create mode 100644 util/uuid.c

It would be nice to see you add a tests/test-uuid.c unit test to
exercise all the new utility APIs you're adding & check their
corner cases.


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.7 v2] block/qdev: Let 'drive' property fall back to node name

[Eric Blake]

On 08/04/2016 06:09 AM, Kevin Wolf wrote:
> If a qdev block device is created with an anonymous BlockBackend (i.e.
> a node name rather than a BB name was given for the drive property),
> qdev used to return an empty string when the property was read. This
> patch fixes it to return the node name instead.
> 
> Signed-off-by: Kevin Wolf 
> ---
> v2:
> - Check whether the BB has even a BDS inserted. Fixes a segfault with
>   empty anonymous BlockBackends.
> 

Reviewed-by: Eric Blake 

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



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [Qemu-block] [PATCH 1/7] util: Add UUID API

[Jeff Cody]

On Tue, Aug 02, 2016 at 05:18:32PM +0800, Fam Zheng wrote:
> A number of different places across the code base use CONFIG_UUID. Some
> of them are soft dependency, some are not built if libuuid is not
> available, some come with dummy fallback, some throws runtime error.
> 
> It is hard to maintain, and hard to reason for users.
> 
> Since UUID is a simple standard with only a small number of operations,
> it is cleaner to have a central support in libqemuutil. This patch adds
> qemu_uuid_* the functions so that all uuid users in the code base can
> rely on. Except for qemu_uuid_generate which is new code, all other
> functions are just copy from existing fallbacks from other files.
> 
> Signed-off-by: Fam Zheng 
> ---
>  arch_init.c | 19 ---
>  block/iscsi.c   |  2 +-
>  hw/smbios/smbios.c  |  1 +
>  include/qemu/uuid.h | 37 +
>  include/sysemu/sysemu.h |  4 
>  qmp.c   |  1 +
>  stubs/uuid.c|  2 +-
>  util/Makefile.objs  |  1 +
>  util/uuid.c | 63 
> +
>  vl.c|  1 +
>  10 files changed, 106 insertions(+), 25 deletions(-)
>  create mode 100644 include/qemu/uuid.h
>  create mode 100644 util/uuid.c
> 
> diff --git a/arch_init.c b/arch_init.c
> index fa05973..5cc58b2 100644
> --- a/arch_init.c
> +++ b/arch_init.c
> @@ -235,25 +235,6 @@ void audio_init(void)
>  }
>  }
>  
> -int qemu_uuid_parse(const char *str, uint8_t *uuid)
> -{
> -int ret;
> -
> -if (strlen(str) != 36) {
> -return -1;
> -}
> -
> -ret = sscanf(str, UUID_FMT, &uuid[0], &uuid[1], &uuid[2], &uuid[3],
> - &uuid[4], &uuid[5], &uuid[6], &uuid[7], &uuid[8], &uuid[9],
> - &uuid[10], &uuid[11], &uuid[12], &uuid[13], &uuid[14],
> - &uuid[15]);
> -
> -if (ret != 16) {
> -return -1;
> -}
> -return 0;
> -}
> -
>  void do_acpitable_option(const QemuOpts *opts)
>  {
>  #ifdef TARGET_I386
> diff --git a/block/iscsi.c b/block/iscsi.c
> index 95ce9e1..961ac76 100644
> --- a/block/iscsi.c
> +++ b/block/iscsi.c
> @@ -36,7 +36,7 @@
>  #include "block/block_int.h"
>  #include "block/scsi.h"
>  #include "qemu/iov.h"
> -#include "sysemu/sysemu.h"
> +#include "qemu/uuid.h"
>  #include "qmp-commands.h"
>  #include "qapi/qmp/qstring.h"
>  #include "crypto/secret.h"
> diff --git a/hw/smbios/smbios.c b/hw/smbios/smbios.c
> index 74c7102..0705eb1 100644
> --- a/hw/smbios/smbios.c
> +++ b/hw/smbios/smbios.c
> @@ -20,6 +20,7 @@
>  #include "qemu/config-file.h"
>  #include "qemu/error-report.h"
>  #include "sysemu/sysemu.h"
> +#include "qemu/uuid.h"
>  #include "sysemu/cpus.h"
>  #include "hw/smbios/smbios.h"
>  #include "hw/loader.h"
> diff --git a/include/qemu/uuid.h b/include/qemu/uuid.h
> new file mode 100644
> index 000..53d572f
> --- /dev/null
> +++ b/include/qemu/uuid.h
> @@ -0,0 +1,37 @@
> +/*
> + *  QEMU UUID functions
> + *
> + *  Copyright 2016 Red Hat, Inc.,
> + *
> + *  Authors:
> + *   Fam Zheng 
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms of the GNU General Public License as published by the Free
> + * Software Foundation; either version 2 of the License, or (at your option)
> + * any later version.
> + *
> + */
> +
> +#ifndef QEMU_UUID_H
> +#define QEMU_UUID_H
> +
> +#include "qemu-common.h"
> +
> +typedef unsigned char qemu_uuid_t[16];
> +
> +#define UUID_FMT "%02hhx%02hhx%02hhx%02hhx-" \
> + "%02hhx%02hhx-%02hhx%02hhx-" \
> + "%02hhx%02hhx-" \
> + "%02hhx%02hhx%02hhx%02hhx%02hhx%02hhx"
> +#define UUID_NONE "----"
> +
> +void qemu_uuid_generate(qemu_uuid_t out);
> +
> +int qemu_uuid_is_null(const qemu_uuid_t uu);
> +
> +void qemu_uuid_unparse(const qemu_uuid_t uu, char *out);
> +
> +int qemu_uuid_parse(const char *str, uint8_t *uuid);
> +
> +#endif
> diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h
> index ee7c760..6111950 100644
> --- a/include/sysemu/sysemu.h
> +++ b/include/sysemu/sysemu.h
> @@ -18,10 +18,6 @@ extern const char *bios_name;
>  extern const char *qemu_name;
>  extern uint8_t qemu_uuid[];
>  extern bool qemu_uuid_set;
> -int qemu_uuid_parse(const char *str, uint8_t *uuid);
> -
> -#define UUID_FMT 
> "%02hhx%02hhx%02hhx%02hhx-%02hhx%02hhx-%02hhx%02hhx-%02hhx%02hhx-%02hhx%02hhx%02hhx%02hhx%02hhx%02hhx"
> -#define UUID_NONE "----"
>  
>  bool runstate_check(RunState state);
>  void runstate_set(RunState new_state);
> diff --git a/qmp.c b/qmp.c
> index b6d531e..7fbde29 100644
> --- a/qmp.c
> +++ b/qmp.c
> @@ -35,6 +35,7 @@
>  #include "qom/object_interfaces.h"
>  #include "hw/mem/pc-dimm.h"
>  #include "hw/acpi/acpi_dev_interface.h"
> +#include "qemu/uuid.h"
>  
>  NameInfo *qmp_query_name(Error **errp)
>  {
> diff --git a/stubs/uuid.c b/stubs/uuid.c
> index 92ad717..a880de8 10064

Re: [Qemu-devel] [PATCH v2 for-2.7] Update ancient copyright string in -version output

[Eric Blake]

On 08/04/2016 05:14 AM, Peter Maydell wrote:
> Currently the -version command line argument prints a string ending
> with "Copyright (c) 2003-2008 Fabrice Bellard".  This is now some
> eight years out of date; abstract it out of the several places that
> print the string and update it to:
> 
> Copyright (c) 2003-2016 Fabrice Bellard and the QEMU Project developers
> 
> to reflect the work by all the QEMU Project contributors over the
> last decade.
> 
> Signed-off-by: Peter Maydell 
> Acked-by: Stefan Hajnoczi 
> ---

Reviewed-by: Eric Blake 

> The aim here is to (1) update the dates and (2) acknowledge
> the work of all our contributors. I'm open to bikeshedding
> on the exact wording (or on which header file we should
> put the #define in...)
> 
> I only pulled out the copyright string proper into the #define
> because a GUI About box is going to want just that, with no
> leading ',' or trailing newline.
> 
> Fabrice: I have cc'd you since this is proposing an update
> to your copyright info.
> 
> v1->v2 changes: add the qemu-img.c string.
> ---
>  bsd-user/main.c   | 3 ++-
>  include/qemu-common.h | 4 
>  linux-user/main.c | 2 +-
>  qemu-img.c| 2 +-
>  vl.c  | 3 ++-
>  5 files changed, 10 insertions(+), 4 deletions(-)

qemu-io has a --version option that probably ought to output copyright
information, but as it currently does not do so, this patch need not be
the one that tweaks it.

qemu-nbd has a --version option, where the copyright info is currently:
Copyright (C) 2006 Anthony Liguori .

I don't know if we want to rework that one in light of this patch, or if
we can just blindly use this code.  Anthony?

> +/* Copyright string for -version arguments, About dialogs, etc */
> +#define QEMU_COPYRIGHT "Copyright (c) 2003-2016 " \
> +"Fabrice Bellard and the QEMU Project developers"
> +

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



signature.asc
Description: OpenPGP digital signature

[Qemu-devel] [Bug 1490611] Re: Using qemu >=2.2.1 to convert raw->VHD (fixed) adds extra padding to the result file, which Microsoft Azure rejects as invalid

[Nish Aravamudan]

On 04.08.2016 [10:49:47 -], Robie Basak wrote:
> Uploaded to Xenial, thanks. Am I right in thinking that the new option
> force_size is required to be used with the patch to fix the problem?
> It's probably worth mentioning this in the Test Case in the bug
> description; otherwise it will appear to testers that the fix doesn't
> work.

Agreed, updated.


** Description changed:

  [Impact]
  
-  * Starting with a raw disk image, using "qemu-img convert" to convert
+  * Starting with a raw disk image, using "qemu-img convert" to convert
  from raw to VHD results in the output VHD file's virtual size being
  aligned to the nearest 516096 bytes (16 heads x 63 sectors per head x
  512 bytes per sector), instead of preserving the input file's size as
  the output VHD's virtual disk size.
  
-  * Microsoft Azure requires that disk images (VHDs) submitted for upload
+  * Microsoft Azure requires that disk images (VHDs) submitted for upload
  have virtual sizes aligned to a megabyte boundary. (Ex. 4096MB, 4097MB,
  4098MB, etc. are OK, 4096.5MB is rejected with an error.) This is
  reflected in Microsoft's documentation: https://azure.microsoft.com/en-
  us/documentation/articles/virtual-machines-linux-create-upload-vhd-
  generic/
  
-  * The fix for this bug is a backport from upstream.
+  * The fix for this bug is a backport from upstream.
  http://git.qemu.org/?p=qemu.git;a=commitdiff;h=fb9245c2610932d33ce14
  
  [Test Case]
  
-  * This is reproducible with the following set of commands (including
+  * This is reproducible with the following set of commands (including
  the Azure command line tools from https://github.com/Azure/azure-xplat-
  cli). For the following example, I used qemu version 2.2.1:
  
  $ dd if=/dev/zero of=source-disk.img bs=1M count=4096
  
  $ stat source-disk.img
    File: ‘source-disk.img’
    Size: 4294967296  Blocks: 798656 IO Block: 4096   regular file
  Device: fc01h/64513dInode: 13247963Links: 1
  Access: (0644/-rw-r--r--)  Uid: ( 1000/  smkent)   Gid: ( 1000/  smkent)
  Access: 2015-08-18 09:48:02.613988480 -0700
  Modify: 2015-08-18 09:48:02.825985646 -0700
  Change: 2015-08-18 09:48:02.825985646 -0700
   Birth: -
  
  $ qemu-img convert -f raw -o subformat=fixed -O vpc source-disk.img
  dest-disk.vhd
  
  $ stat dest-disk.vhd
    File: ‘dest-disk.vhd’
    Size: 4296499712  Blocks: 535216 IO Block: 4096   regular file
  Device: fc01h/64513dInode: 13247964Links: 1
  Access: (0644/-rw-r--r--)  Uid: ( 1000/  smkent)   Gid: ( 1000/  smkent)
  Access: 2015-08-18 09:50:22.252077624 -0700
  Modify: 2015-08-18 09:49:24.424868868 -0700
  Change: 2015-08-18 09:49:24.424868868 -0700
   Birth: -
  
  $ azure vm image create testimage1 dest-disk.vhd -o linux -l "West US"
  info:Executing command vm image create
  + Retrieving storage accounts
  info:VHD size : 4097 MB
  info:Uploading 4195800.5 KB
  Requested:100.0% Completed:100.0% Running:   0 Time: 1m 0s Speed:  6744 KB/s
  info:https://[redacted].blob.core.windows.net/vm-images/dest-disk.vhd was 
uploaded successfully
  error:   The VHD 
https://[redacted].blob.core.windows.net/vm-images/dest-disk.vhd has an 
unsupported virtual size of 4296499200 bytes.  The size must be a whole number 
(in MBs).
  info:Error information has been recorded to /home/smkent/.azure/azure.err
  error:   vm image create command failed
  
-  * A fixed qemu-img will not result in an error during azure image
- creation.
+  * A fixed qemu-img will not result in an error during azure image
+ creation. It will require passing -o force_size, which will leverage the
+ backported functionality.
  
  [Regression Potential]
  
-  * The upstream fix introduces a qemu-img option (-o force_size) which
+  * The upstream fix introduces a qemu-img option (-o force_size) which
  is unset by default. The regression potential is very low, as a result.
  
  ...
  
  I also ran the above commands using qemu 2.4.0, which resulted in the
  same error as the conversion behavior is the same.
  
  However, qemu 2.1.1 and earlier (including qemu 2.0.0 installed by
  Ubuntu 14.04) does not pad the virtual disk size during conversion.
  Using qemu-img convert from qemu versions <=2.1.1 results in a VHD that
  is exactly the size of the raw input file plus 512 bytes (for the VHD
  footer). Those qemu versions do not attempt to realign the disk. As a
  result, Azure accepts VHD files created using those versions of qemu-img
  convert for upload.
  
  Is there a reason why newer qemu realigns the converted VHD file? It
  would be useful if an option were added to disable this feature, as
  current versions of qemu cannot be used to create VHD files for Azure
  using Microsoft's official instructions.

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

Title:
  Using qemu >=2.2.1 to convert raw->VHD (fixed) adds extra padding to
  the r

Re: [Qemu-devel] [PATCH for-2.7 v3 17/36] machine: use class base init generated name

[Markus Armbruster]

Marc-André Lureau  writes:

> Hi
>
> On Thu, Aug 4, 2016 at 5:58 PM Markus Armbruster  wrote:
>
>> marcandre.lur...@redhat.com writes:
>>
>> > From: Marc-André Lureau 
>> >
>> > Remove machine class name initialization from DEFINE_PC_MACHINE, rely on
>> > class base init name generation instead. Get rid of some leaks that way.
>> >
>> > Signed-off-by: Marc-André Lureau 
>> > ---
>> >  hw/core/machine.c| 1 +
>> >  include/hw/boards.h  | 2 +-
>> >  include/hw/i386/pc.h | 1 -
>> >  3 files changed, 2 insertions(+), 2 deletions(-)
>> >
>> > diff --git a/hw/core/machine.c b/hw/core/machine.c
>> > index e5a456f..00fbe3e 100644
>> > --- a/hw/core/machine.c
>> > +++ b/hw/core/machine.c
>> > @@ -561,6 +561,7 @@ static void machine_class_finalize(ObjectClass *klass, 
>> > void *data)
>> >  if (mc->compat_props) {
>> >  g_array_free(mc->compat_props, true);
>> >  }
>> > +g_free(mc->name);
>> >  }
>> >
>> >  void machine_register_compat_props(MachineState *machine)
>> > diff --git a/include/hw/boards.h b/include/hw/boards.h
>> > index 3e69eca..e46a744 100644
>> > --- a/include/hw/boards.h
>> > +++ b/include/hw/boards.h
>> > @@ -93,7 +93,7 @@ struct MachineClass {
>> >  /*< public >*/
>> >
>> >  const char *family; /* NULL iff @name identifies a standalone 
>> > machtype */
>> > -const char *name;
>> > +char *name;
>> >  const char *alias;
>> >  const char *desc;
>> >
>> > diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
>> > index eb1d414..afd025a 100644
>> > --- a/include/hw/i386/pc.h
>> > +++ b/include/hw/i386/pc.h
>> > @@ -903,7 +903,6 @@ bool e820_get_entry(int, uint32_t, uint64_t *, 
>> > uint64_t *);
>> >  { \
>> >  MachineClass *mc = MACHINE_CLASS(oc); \
>> >  optsfn(mc); \
>> > -mc->name = namestr; \
>> >  mc->init = initfn; \
>> >  } \
>> >  static const TypeInfo pc_machine_type_##suffix = { \
>>
>> I guess there are is at least one assignment to mc->name not visible in
>> this patch that assigns an allocated string, which leaks before the
>> patch.  The commit message seems to provide a clue "class base init name
>> generation".  I could probably find it with some effort, but patches
>> that take that much work to understand make me grumpy.  Please provide
>> another clue :)
>>
>
> Sorry, thanks for reminding me to write better commit messages.

Good commit messages are hard.

> git grep 'mc->name ='
> hw/core/machine.c:mc->name = g_strndup(cname,

Aha: the concrete machine type's init function overwrites the strdup()ed
value set by machine_class_base_init(), leaking it.  Your fix removes
the overwrites and adds a free.

As far as I can see, you got all such overwrites.

> Is that better:
>
> Remove machine class name initialization from DEFINE_PC_MACHINE, rely on
> name generation from machine_class_base_init() instead, and free the
> corresponding allocation in machine_class_finalize().

Works for me.  Alternatively:

machine_class_base_init() member name is allocated by
machine_class_base_init(), but not freed by machine_class_finalize().
Simply freeing there doesn't work, because DEFINE_PC_MACHINE()
overwrites it with a literal string.

Fix DEFINE_PC_MACHINE() not to overwrite it, and add the missing
free to machine_class_finalize().

Use the one you like better, or mix them up to taste.

Reviewed-by: Markus Armbruster 

Re: [Qemu-devel] [PATCH v2] balloon: Fix failure of updating guest memory status

[Ladi Prosek]

On Wed, Aug 3, 2016 at 9:25 AM, Ladi Prosek  wrote:
> On Tue, Aug 2, 2016 at 2:11 AM, Michael S. Tsirkin  wrote:
>> On Mon, Aug 01, 2016 at 11:59:31PM +, Li, Liang Z wrote:
>>> > On Wed, Jul 06, 2016 at 12:49:06PM +, Li, Liang Z wrote:
>>> > > > > > > After live migration, 'guest-stats' can't get the expected
>>> > > > > > > memory status in the guest. This issue is caused by commit
>>> > 4eae2a657d.
>>> > > > > > > The value of 's->stats_vq_elem' will be NULL after live
>>> > > > > > > migration, and the check in the function
>>> > > > > > > 'balloon_stats_poll_cb()' will prevent the 'virtio_notify()'
>>> > > > > > > from executing. So guest will not update the memory status.
>>> > > > > > >
>>> > > > > > > Commit 4eae2a657d is doing the right thing, but 
>>> > > > > > > 's->stats_vq_elem'
>>> > > > > > > should be treated as part of balloon device state and migrated
>>> > > > > > > to destination if it's not NULL to make everything works well.
>>> > > > > > >
>>> > > > > > > Signed-off-by: Liang Li 
>>> > > > > > > Suggested-by: Paolo Bonzini 
>>> > > > > > > Cc: Michael S. Tsirkin 
>>> > > > > > > Cc: Ladi Prosek 
>>> > > > > > > Cc: Paolo Bonzini 
>>> > > > > >
>>> > > > > > I agree there's an issue but we don't change versions anymore.
>>> > > > > > Breaking migrations for everyone is also not nice.
>>> > > > > >
>>> > > > > > How about queueing virtio_balloon_receive_stats so it will get
>>> > > > > > invoked when vm starts?
>>> > > > > >
>>> > > > >
>>> > > > > Could you give more explanation about how it works?  I can't catch 
>>> > > > > you.
>>> > > > >
>>> > > > > Thanks!
>>> > > > > Liang
>>> > > >
>>> > > > virtqueue_discard before migration
>>> > > >
>>> > > > virtio_balloon_receive_stats after migration
>>> > > >
>>> > >
>>> > > Sorry, I still can't catch you. Maybe it's easier for you to submit a
>>> > > patch than writing a lot a words to make me understand your idea.
>>> >
>>> > I'm rather busy now.  I might look into it towards end of the month.
>>> >
>>> > > I just don't understand why not to use the version to make things
>>> > > easier, is that not the original intent of version id?
>>> >
>>> > This was the original idea but we stopped using version ids since they 
>>> > have
>>> > many shortcomings.
>>> >
>>> > > If we want to extend the device and more states are needed, the idea
>>> > > you suggest can be used as a common solution?
>>> > >
>>> > > Thanks!
>>> > > Liang
>>> >
>>> > The idea is to try to avoid adding more state. that's not always possible 
>>> > but in
>>> > this case element was seen but not consumed yet, so it should be possible
>>> > for destination to simply get it from the VQ again.
>>> >
>>> > > > --
>>> > > > MST
>>>
>>> Hi Michel,
>>>
>>> Do you have time for this issue recently?
>>>
>>> Thanks!
>>> Liang
>
> Hi Liang,
>
> I should be able to look into it this week if you help me with testing.
>
> Thanks,
> Ladi

Please try the attached patch. I have tested it with a simple
'migrate' to save the state and then '-incoming' to load it back.

One question for you: is it expected that stats_poll_interval is not
preserved by save/load? I had to explicitly set
guest-stats-polling-interval on the receiving VM to start getting
stats again. It's also the reason why the new
virtio_balloon_receive_stats call is not under if
(balloon_stats_enabled(s)) because this condition always evaluates to
false for me.

Thanks!
Ladi

>> Sorry, doesn't look like I will.
>> Idea is to make sure balloon_stats_poll_cb runs
>> on source. This will set stats_vq_elem to NULL.
>>
>>
>> --
>> MST
From f2f779e12f4aa4d3469d1b44e54484e66f82a2d7 Mon Sep 17 00:00:00 2001
From: Ladi Prosek 
Date: Thu, 4 Aug 2016 15:22:05 +0200
Subject: [PATCH] balloon: preserve stats virtqueue state across migrations

Signed-off-by: Ladi Prosek 
---
 hw/virtio/virtio-balloon.c | 18 +-
 1 file changed, 17 insertions(+), 1 deletion(-)

diff --git a/hw/virtio/virtio-balloon.c b/hw/virtio/virtio-balloon.c
index 5af429a..1293be0 100644
--- a/hw/virtio/virtio-balloon.c
+++ b/hw/virtio/virtio-balloon.c
@@ -396,6 +396,19 @@ static void virtio_balloon_to_target(void *opaque, ram_addr_t target)
 trace_virtio_balloon_to_target(target, dev->num_pages);
 }
 
+static void virtio_balloon_save(QEMUFile *f, void *opaque, size_t size)
+{
+VirtIOBalloon *s = VIRTIO_BALLOON(opaque);
+
+if (s->stats_vq_elem != NULL) {
+virtqueue_discard(s->svq, s->stats_vq_elem, s->stats_vq_offset);
+g_free(s->stats_vq_elem);
+s->stats_vq_elem = NULL;
+}
+
+virtio_save(VIRTIO_DEVICE(opaque), f);
+}
+
 static void virtio_balloon_save_device(VirtIODevice *vdev, QEMUFile *f)
 {
 VirtIOBalloon *s = VIRTIO_BALLOON(vdev);
@@ -417,6 +430,9 @@ static int virtio_balloon_load_device(VirtIODevice *vdev, QEMUFile *f,
 s->num_pages = qemu_get_be32(f);
 s->actual = qemu_get_be32(f);
 
+/* poll the queue for the element we may have discarded on save */
+virtio_balloon_receive_sta

Re: [Qemu-devel] [PATCH for-2.7 v3 32/36] tests: add qtest_add_data_func_full

[Marc-André Lureau]

Hi

- Original Message -
> marcandre.lur...@redhat.com writes:
> 
> > From: Marc-André Lureau 
> >
> > Allows one to specify a destroy function for the test data.
> >
> > Signed-off-by: Marc-André Lureau 
> > ---
> >  tests/libqtest.c | 15 +++
> >  tests/libqtest.h |  7 ++-
> >  2 files changed, 21 insertions(+), 1 deletion(-)
> >
> > diff --git a/tests/libqtest.c b/tests/libqtest.c
> > index eb00f13..9e2d0cd 100644
> > --- a/tests/libqtest.c
> > +++ b/tests/libqtest.c
> > @@ -758,6 +758,21 @@ void qtest_add_func(const char *str, void (*fn)(void))
> >  g_free(path);
> >  }
> >  
> > +void qtest_add_data_func_full(const char *str, void *data,
> > +  void (*fn)(const void *),
> > +  GDestroyNotify data_free_func)
> > +{
> > +gchar *path = g_strdup_printf("/%s/%s", qtest_get_arch(), str);
> > +#if GLIB_CHECK_VERSION(2, 34, 0)
> > +g_test_add_data_func_full(path, data, fn, data_free_func);
> > +#else
> > +/* back-compat casts, remove this once we can require new-enough glib
> > */
> > +g_test_add_vtable(path, 0, data, NULL,
> > +  (GTestFixtureFunc) fn, (GTestFixtureFunc)
> > data_free_func);
> 
> Umm, are these function casts kosher?
> 
> I can't find documentation for g_test_add_vtable().  Got a pointer for
> me?  Sure GLib 2.22 provides it?

Yes, https://git.gnome.org/browse/glib/tree/glib/gtestutils.h?h=2.22.0#n214

See also: https://lists.gnu.org/archive/html/qemu-devel/2016-08/msg00073.html


> 
> > +#endif
> > +g_free(path);
> > +}
> > +
> >  void qtest_add_data_func(const char *str, const void *data,
> >   void (*fn)(const void *))
> >  {
> > diff --git a/tests/libqtest.h b/tests/libqtest.h
> > index 37f37ad..e4c9c39 100644
> > --- a/tests/libqtest.h
> > +++ b/tests/libqtest.h
> > @@ -413,15 +413,20 @@ const char *qtest_get_arch(void);
> >  void qtest_add_func(const char *str, void (*fn)(void));
> >  
> >  /**
> > - * qtest_add_data_func:
> > + * qtest_add_data_func_full:
> >   * @str: Test case path.
> >   * @data: Test case data
> >   * @fn: Test case function
> > + * @data_free_func: GDestroyNotify for data
> >   *
> >   * Add a GTester testcase with the given name, data and function.
> >   * The path is prefixed with the architecture under test, as
> >   * returned by qtest_get_arch().
> >   */
> > +void qtest_add_data_func_full(const char *str, void *data,
> > +  void (*fn)(const void *),
> > +  GDestroyNotify data_free_func);
> > +
> >  void qtest_add_data_func(const char *str, const void *data,
> >   void (*fn)(const void *));
> 
> Please keep qtest_add_data_func() documented.

Ok (I thought it was quite enough based on the _full description)

Re: [Qemu-devel] [PATCH for-2.7 v3 03/36] qga: free the whole blacklist

[Marc-André Lureau]

Hi

- Original Message -
> Marc-André Lureau  writes:
> 
> > Hi
> >
> > - Original Message -
> >> Copying the QGA maintainer.
> >> 
> >> marcandre.lur...@redhat.com writes:
> >> 
> >> > From: Marc-André Lureau 
> >> >
> >> > Free the list, not just the elements.
> >> >
> >> > Signed-off-by: Marc-André Lureau 
> >> > ---
> >> >  include/glib-compat.h | 9 +
> >> >  qga/main.c| 8 ++--
> >> >  2 files changed, 11 insertions(+), 6 deletions(-)
> >> >
> >> > diff --git a/include/glib-compat.h b/include/glib-compat.h
> >> > index 01aa7b3..6d643b2 100644
> >> > --- a/include/glib-compat.h
> >> > +++ b/include/glib-compat.h
> >> > @@ -260,4 +260,13 @@ static inline void g_hash_table_add(GHashTable
> >> > *hash_table, gpointer key)
> >> >  } while (0)
> >> >  #endif
> >> >  
> >> > +/*
> >> > + * A GFunc function helper freeing the first argument (not part of
> >> > glib)
> >> 
> >> Well, it's not part of GLib, because non-obsolete GLib has no use for
> >> it!  You'd simply pass g_free directly to a _free_full() function
> >> instead of passing a silly wrapper to a _foreach() function.
> >> 
> >> The commit does a bit more than just plug a leak, it also provides a new
> >> helper for general use.  Mention in the commit message?
> >> 
> >> I wonder how many more of these silly g_free() wrappers we have.  A
> >> quick grep reports hits in string-input-visitor.c and
> >> qobject/json-streamer.c.  If you add one to a header, you get to hunt
> >> them down :)
> >> 
> >> > + */
> >> > +static inline void qemu_g_func_free(gpointer data,
> >> > +gpointer user_data)
> >> > +{
> >> > +g_free(data);
> >> > +}
> >> > +
> >> >  #endif
> >> > diff --git a/qga/main.c b/qga/main.c
> >> > index 4c3b2c7..868508b 100644
> >> > --- a/qga/main.c
> >> > +++ b/qga/main.c
> >> > @@ -1175,6 +1175,8 @@ static void config_free(GAConfig *config)
> >> >  #ifdef CONFIG_FSFREEZE
> >> >  g_free(config->fsfreeze_hook);
> >> >  #endif
> >> > +g_list_foreach(config->blacklist, qemu_g_func_free, NULL);
> >> > +g_list_free(config->blacklist);
> >> 
> >> Modern GLib code doesn't need silly wrappers:
> >> 
> >> g_list_free_full(config->blacklist, g_free);
> >> 
> >> Unfortunately, this requires 2.28, and we're stull stuck at 2.22.
> >> Please explain this in the commit message.
> >> 
> >> Even better, provide a replacement in glib-compat.h #if
> >> !GLIB_CHECK_VERSION(2, 28, 0).  Will facilitate ditching the work-around
> >> when we upgrade to 2.28.
> >
> > ok
> >
> >> 
> >> >  g_free(config);
> >> >  }
> >> >  
> >> > @@ -1310,11 +1312,6 @@ static int run_agent(GAState *s, GAConfig
> >> > *config)
> >> >  return EXIT_SUCCESS;
> >> >  }
> >> >  
> >> > -static void free_blacklist_entry(gpointer entry, gpointer unused)
> >> > -{
> >> > -g_free(entry);
> >> > -}
> >> > -
> >> >  int main(int argc, char **argv)
> >> >  {
> >> >  int ret = EXIT_SUCCESS;
> >> > @@ -1379,7 +1376,6 @@ end:
> >> >  if (s->channel) {
> >> >  ga_channel_free(s->channel);
> >> >  }
> >> > -g_list_foreach(config->blacklist, free_blacklist_entry, NULL);
> >> >  g_free(s->pstate_filepath);
> >> >  g_free(s->state_filepath_isfrozen);
> >> 
> >> If you at least explain why not g_list_free_full() in the commit
> >> message, you can add
> >> Reviewed-by: Markus Armbruster 
> >> 
> >> But I'd like to encourage you to explore providing a replacement for
> >> g_list_free_full().
> >
> > Such replacement would make use of a (GFunc) cast, glib implementation is
> > calling g_list_foreach (list, (GFunc) free_func, NULL), but Eric didn't
> > want to have such cast in qemu code. He suggested to have the static
> > inline helper in
> > https://lists.gnu.org/archive/html/qemu-devel/2016-07/msg06561.html
> 
> Pointlessly dirty:
> 
> g_list_foreach(list, (GFunc)g_free, NULL)
> 
> Trade dirty for cumbersome:
> 
> void free_wrapper(gpointer data, gpointer unused)
> {
> g_free(data)
> }
> 
> g_list_foreach(list, free_wrapper, NULL);
> 
> But this is C.  In C, simple things are best done the simple way.  Even
> when that means we don't get to show off how amazingly well we've been
> educated on higher order functions:
> 
> for (node = list; node; node = next) {
> next = node->next;
> g_free(node);
> }
> 
> Simple, stupid, and not dirty.

If only we were paid to write more lines of code ;) Anyway, that's fine by me 
(it's work after all, I'll write elegant code for fun time ;)

> Quote: "Note that it is considered perfectly acceptable to access
> list->next directly."

Funny that quote is only in GList, but GSList also documents and exposes the 
struct.

Re: [Qemu-devel] [PATCH for-2.7 v3 32/36] tests: add qtest_add_data_func_full

[Markus Armbruster]

marcandre.lur...@redhat.com writes:

> From: Marc-André Lureau 
>
> Allows one to specify a destroy function for the test data.
>
> Signed-off-by: Marc-André Lureau 
> ---
>  tests/libqtest.c | 15 +++
>  tests/libqtest.h |  7 ++-
>  2 files changed, 21 insertions(+), 1 deletion(-)
>
> diff --git a/tests/libqtest.c b/tests/libqtest.c
> index eb00f13..9e2d0cd 100644
> --- a/tests/libqtest.c
> +++ b/tests/libqtest.c
> @@ -758,6 +758,21 @@ void qtest_add_func(const char *str, void (*fn)(void))
>  g_free(path);
>  }
>  
> +void qtest_add_data_func_full(const char *str, void *data,
> +  void (*fn)(const void *),
> +  GDestroyNotify data_free_func)
> +{
> +gchar *path = g_strdup_printf("/%s/%s", qtest_get_arch(), str);
> +#if GLIB_CHECK_VERSION(2, 34, 0)
> +g_test_add_data_func_full(path, data, fn, data_free_func);
> +#else
> +/* back-compat casts, remove this once we can require new-enough glib */
> +g_test_add_vtable(path, 0, data, NULL,
> +  (GTestFixtureFunc) fn, (GTestFixtureFunc) 
> data_free_func);

Umm, are these function casts kosher?

I can't find documentation for g_test_add_vtable().  Got a pointer for
me?  Sure GLib 2.22 provides it?

> +#endif
> +g_free(path);
> +}
> +
>  void qtest_add_data_func(const char *str, const void *data,
>   void (*fn)(const void *))
>  {
> diff --git a/tests/libqtest.h b/tests/libqtest.h
> index 37f37ad..e4c9c39 100644
> --- a/tests/libqtest.h
> +++ b/tests/libqtest.h
> @@ -413,15 +413,20 @@ const char *qtest_get_arch(void);
>  void qtest_add_func(const char *str, void (*fn)(void));
>  
>  /**
> - * qtest_add_data_func:
> + * qtest_add_data_func_full:
>   * @str: Test case path.
>   * @data: Test case data
>   * @fn: Test case function
> + * @data_free_func: GDestroyNotify for data
>   *
>   * Add a GTester testcase with the given name, data and function.
>   * The path is prefixed with the architecture under test, as
>   * returned by qtest_get_arch().
>   */
> +void qtest_add_data_func_full(const char *str, void *data,
> +  void (*fn)(const void *),
> +  GDestroyNotify data_free_func);
> +
>  void qtest_add_data_func(const char *str, const void *data,
>   void (*fn)(const void *));

Please keep qtest_add_data_func() documented.

Re: [Qemu-devel] [PATCH for-2.7 v3 35/36] ahci: fix sglist leak on retry

[Markus Armbruster]

John, can you review this one?

marcandre.lur...@redhat.com writes:

> From: Marc-André Lureau 
>
> ahci-test /x86_64/ahci/io/dma/lba28/retry triggers the following leak:
>
> Direct leak of 16 byte(s) in 1 object(s) allocated from:
> #0 0x7fc4b2a25e20 in malloc (/lib64/libasan.so.3+0xc6e20)
> #1 0x7fc4993bce58 in g_malloc (/lib64/libglib-2.0.so.0+0x4ee58)
> #2 0x556a187d4b34 in ahci_populate_sglist hw/ide/ahci.c:896
> #3 0x556a187d8237 in ahci_dma_prepare_buf hw/ide/ahci.c:1367
> #4 0x556a187b5a1a in ide_dma_cb hw/ide/core.c:844
> #5 0x556a187d7eec in ahci_start_dma hw/ide/ahci.c:1333
> #6 0x556a187b650b in ide_start_dma hw/ide/core.c:921
> #7 0x556a187b61e6 in ide_sector_start_dma hw/ide/core.c:911
> #8 0x556a187b9e26 in cmd_write_dma hw/ide/core.c:1486
> #9 0x556a187bd519 in ide_exec_cmd hw/ide/core.c:2027
> #10 0x556a187d71c5 in handle_reg_h2d_fis hw/ide/ahci.c:1204
> #11 0x556a187d7681 in handle_cmd hw/ide/ahci.c:1254
> #12 0x556a187d168a in check_cmd hw/ide/ahci.c:510
> #13 0x556a187d0afc in ahci_port_write hw/ide/ahci.c:314
> #14 0x556a187d105d in ahci_mem_write hw/ide/ahci.c:435
> #15 0x556a1831d959 in memory_region_write_accessor 
> /home/elmarco/src/qemu/memory.c:525
> #16 0x556a1831dc35 in access_with_adjusted_size 
> /home/elmarco/src/qemu/memory.c:591
> #17 0x556a18323ce3 in memory_region_dispatch_write 
> /home/elmarco/src/qemu/memory.c:1262
> #18 0x556a1828cf67 in address_space_write_continue 
> /home/elmarco/src/qemu/exec.c:2578
> #19 0x556a1828d20b in address_space_write 
> /home/elmarco/src/qemu/exec.c:2635
> #20 0x556a1828d92b in address_space_rw /home/elmarco/src/qemu/exec.c:2737
> #21 0x556a1828daf7 in cpu_physical_memory_rw 
> /home/elmarco/src/qemu/exec.c:2746
> #22 0x556a183068d3 in cpu_physical_memory_write 
> /home/elmarco/src/qemu/include/exec/cpu-common.h:72
> #23 0x556a18308194 in qtest_process_command 
> /home/elmarco/src/qemu/qtest.c:382
> #24 0x556a1830 in qtest_process_inbuf 
> /home/elmarco/src/qemu/qtest.c:573
> #25 0x556a18309a4a in qtest_read /home/elmarco/src/qemu/qtest.c:585
> #26 0x556a18598b85 in qemu_chr_be_write_impl 
> /home/elmarco/src/qemu/qemu-char.c:387
> #27 0x556a18598c52 in qemu_chr_be_write 
> /home/elmarco/src/qemu/qemu-char.c:399
> #28 0x556a185a2afa in tcp_chr_read /home/elmarco/src/qemu/qemu-char.c:2902
> #29 0x556a18cbaf52 in qio_channel_fd_source_dispatch io/channel-watch.c:84
>
> Follow John Snow recommendation:
>   Everywhere else ncq_err is used, it is accompanied by a list cleanup
>   except for ncq_cb, which is the case you are fixing here.
>
>   Move the sglist destruction inside of ncq_err and then delete it from
>   the other two locations to keep it tidy.
>
>   Call dma_buf_commit in ide_dma_cb after the early return. Though, this
>   is also a little wonky because this routine does more than clear the
>   list, but it is at the moment the centralized "we're done with the
>   sglist" function and none of the other side effects that occur in
>   dma_buf_commit will interfere with the reset that occurs from
>   ide_restart_bh, I think
>
> Signed-off-by: Marc-André Lureau 
> ---
>  hw/ide/ahci.c | 3 +--
>  hw/ide/core.c | 1 +
>  2 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/hw/ide/ahci.c b/hw/ide/ahci.c
> index 6defeed..f3438ad 100644
> --- a/hw/ide/ahci.c
> +++ b/hw/ide/ahci.c
> @@ -919,6 +919,7 @@ static void ncq_err(NCQTransferState *ncq_tfs)
>  ide_state->error = ABRT_ERR;
>  ide_state->status = READY_STAT | ERR_STAT;
>  ncq_tfs->drive->port_regs.scr_err |= (1 << ncq_tfs->tag);
> +qemu_sglist_destroy(&ncq_tfs->sglist);
>  ncq_tfs->used = 0;
>  }
>  
> @@ -1025,7 +1026,6 @@ static void execute_ncq_command(NCQTransferState 
> *ncq_tfs)
>  default:
>  DPRINTF(port, "error: unsupported NCQ command (0x%02x) received\n",
>  ncq_tfs->cmd);
> -qemu_sglist_destroy(&ncq_tfs->sglist);
>  ncq_err(ncq_tfs);
>  }
>  }
> @@ -1092,7 +1092,6 @@ static void process_ncq_command(AHCIState *s, int port, 
> uint8_t *cmd_fis,
>  error_report("ahci: PRDT length for NCQ command (0x%zx) "
>   "is smaller than the requested size (0x%zx)",
>   ncq_tfs->sglist.size, size);
> -qemu_sglist_destroy(&ncq_tfs->sglist);
>  ncq_err(ncq_tfs);
>  ahci_trigger_irq(ad->hba, ad, PORT_IRQ_OVERFLOW);
>  return;
> diff --git a/hw/ide/core.c b/hw/ide/core.c
> index f9c8162..82d7f2a 100644
> --- a/hw/ide/core.c
> +++ b/hw/ide/core.c
> @@ -822,6 +822,7 @@ static void ide_dma_cb(void *opaque, int ret)
>  return;
>  }
>  if (ret < 0) {
> +dma_buf_commit(s, 0);
>  if (ide_handle_rw_error(s, -ret, ide_dma_cmd_to_retry(s->dma_cmd))) {
>  s->bus->dma->aiocb = NULL;
>  return;

Re: [Qemu-devel] [PATCH for-2.7 v3 23/36] pc: keep gsi reference

[Markus Armbruster]

marcandre.lur...@redhat.com writes:

> From: Marc-André Lureau 
>
> Further cleanup would need to call qemu_free_irq() at the appropriate
> time, but for now this silences ASAN about direct leaks.
>
> Signed-off-by: Marc-André Lureau 

I looked into cleaning up the qemu_irq allocation mess long ago, but
gave up after the work-in-progress had ballooned to a dozen commits or
so.

> ---
>  hw/i386/pc_piix.c| 17 -
>  hw/i386/pc_q35.c | 13 ++---
>  include/hw/i386/pc.h |  1 +
>  3 files changed, 15 insertions(+), 16 deletions(-)
>
> diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
> index a07dc81..2af 100644
> --- a/hw/i386/pc_piix.c
> +++ b/hw/i386/pc_piix.c
> @@ -74,7 +74,6 @@ static void pc_init1(MachineState *machine,
>  ISABus *isa_bus;
>  PCII440FXState *i440fx_state;
>  int piix3_devfn = -1;
> -qemu_irq *gsi;
>  qemu_irq *i8259;
>  qemu_irq smi_irq;
>  GSIState *gsi_state;
> @@ -185,16 +184,16 @@ static void pc_init1(MachineState *machine,
>  gsi_state = g_malloc0(sizeof(*gsi_state));
>  if (kvm_ioapic_in_kernel()) {
>  kvm_pc_setup_irq_routing(pcmc->pci_enabled);
> -gsi = qemu_allocate_irqs(kvm_pc_gsi_handler, gsi_state,
> - GSI_NUM_PINS);
> +pcms->gsi = qemu_allocate_irqs(kvm_pc_gsi_handler, gsi_state,
> +   GSI_NUM_PINS);
>  } else {
> -gsi = qemu_allocate_irqs(gsi_handler, gsi_state, GSI_NUM_PINS);
> +pcms->gsi = qemu_allocate_irqs(gsi_handler, gsi_state, GSI_NUM_PINS);
>  }
>  
>  if (pcmc->pci_enabled) {
>  pci_bus = i440fx_init(host_type,
>pci_type,
> -  &i440fx_state, &piix3_devfn, &isa_bus, gsi,
> +  &i440fx_state, &piix3_devfn, &isa_bus, 
> pcms->gsi,
>system_memory, system_io, machine->ram_size,
>pcms->below_4g_mem_size,
>pcms->above_4g_mem_size,
> @@ -207,7 +206,7 @@ static void pc_init1(MachineState *machine,
>&error_abort);
>  no_hpet = 1;
>  }
> -isa_bus_irqs(isa_bus, gsi);
> +isa_bus_irqs(isa_bus, pcms->gsi);
>  
>  if (kvm_pic_in_kernel()) {
>  i8259 = kvm_i8259_init(isa_bus);
> @@ -225,7 +224,7 @@ static void pc_init1(MachineState *machine,
>  ioapic_init_gsi(gsi_state, "i440fx");
>  }
>  
> -pc_register_ferr_irq(gsi[13]);
> +pc_register_ferr_irq(pcms->gsi[13]);
>  
>  pc_vga_init(isa_bus, pcmc->pci_enabled ? pci_bus : NULL);
>  
> @@ -235,7 +234,7 @@ static void pc_init1(MachineState *machine,
>  }
>  
>  /* init basic PC hardware */
> -pc_basic_device_init(isa_bus, gsi, &rtc_state, true,
> +pc_basic_device_init(isa_bus, pcms->gsi, &rtc_state, true,
>   (pcms->vmport != ON_OFF_AUTO_ON), 0x4);
>  
>  pc_nic_init(isa_bus, pci_bus);
> @@ -279,7 +278,7 @@ static void pc_init1(MachineState *machine,
>  smi_irq = qemu_allocate_irq(pc_acpi_smi_interrupt, first_cpu, 0);
>  /* TODO: Populate SPD eeprom data.  */
>  smbus = piix4_pm_init(pci_bus, piix3_devfn + 3, 0xb100,
> -  gsi[9], smi_irq,
> +  pcms->gsi[9], smi_irq,
>pc_machine_is_smm_enabled(pcms),
>&piix4_pm);
>  smbus_eeprom_init(smbus, 8, NULL, 0);
> diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
> index c5e8367..3cbcbb0 100644
> --- a/hw/i386/pc_q35.c
> +++ b/hw/i386/pc_q35.c
> @@ -69,7 +69,6 @@ static void pc_q35_init(MachineState *machine)
>  MemoryRegion *ram_memory;
>  GSIState *gsi_state;
>  ISABus *isa_bus;
> -qemu_irq *gsi;
>  qemu_irq *i8259;
>  int i;
>  ICH9LPCState *ich9_lpc;
> @@ -153,10 +152,10 @@ static void pc_q35_init(MachineState *machine)
>  gsi_state = g_malloc0(sizeof(*gsi_state));
>  if (kvm_ioapic_in_kernel()) {
>  kvm_pc_setup_irq_routing(pcmc->pci_enabled);
> -gsi = qemu_allocate_irqs(kvm_pc_gsi_handler, gsi_state,
> - GSI_NUM_PINS);
> +pcms->gsi = qemu_allocate_irqs(kvm_pc_gsi_handler, gsi_state,
> +   GSI_NUM_PINS);
>  } else {
> -gsi = qemu_allocate_irqs(gsi_handler, gsi_state, GSI_NUM_PINS);
> +pcms->gsi = qemu_allocate_irqs(gsi_handler, gsi_state, GSI_NUM_PINS);
>  }
>  
>  /* create pci host bus */
> @@ -195,7 +194,7 @@ static void pc_q35_init(MachineState *machine)
>  ich9_lpc = ICH9_LPC_DEVICE(lpc);
>  lpc_dev = DEVICE(lpc);
>  for (i = 0; i < GSI_NUM_PINS; i++) {
> -qdev_connect_gpio_out_named(lpc_dev, ICH9_GPIO_GSI, i, gsi[i]);
> +qdev_connect_gpio_out_named(lpc_dev, ICH9_GPIO_GSI, i, pcms->gsi[i]);
>  }
>  pci_bus_irqs(host_bus, ich9_lpc_set_irq, ich9_lpc

Re: [Qemu-devel] [PATCH for-2.7 v3 03/36] qga: free the whole blacklist

[Markus Armbruster]

Marc-André Lureau  writes:

> Hi
>
> - Original Message -
>> Copying the QGA maintainer.
>> 
>> marcandre.lur...@redhat.com writes:
>> 
>> > From: Marc-André Lureau 
>> >
>> > Free the list, not just the elements.
>> >
>> > Signed-off-by: Marc-André Lureau 
>> > ---
>> >  include/glib-compat.h | 9 +
>> >  qga/main.c| 8 ++--
>> >  2 files changed, 11 insertions(+), 6 deletions(-)
>> >
>> > diff --git a/include/glib-compat.h b/include/glib-compat.h
>> > index 01aa7b3..6d643b2 100644
>> > --- a/include/glib-compat.h
>> > +++ b/include/glib-compat.h
>> > @@ -260,4 +260,13 @@ static inline void g_hash_table_add(GHashTable
>> > *hash_table, gpointer key)
>> >  } while (0)
>> >  #endif
>> >  
>> > +/*
>> > + * A GFunc function helper freeing the first argument (not part of glib)
>> 
>> Well, it's not part of GLib, because non-obsolete GLib has no use for
>> it!  You'd simply pass g_free directly to a _free_full() function
>> instead of passing a silly wrapper to a _foreach() function.
>> 
>> The commit does a bit more than just plug a leak, it also provides a new
>> helper for general use.  Mention in the commit message?
>> 
>> I wonder how many more of these silly g_free() wrappers we have.  A
>> quick grep reports hits in string-input-visitor.c and
>> qobject/json-streamer.c.  If you add one to a header, you get to hunt
>> them down :)
>> 
>> > + */
>> > +static inline void qemu_g_func_free(gpointer data,
>> > +gpointer user_data)
>> > +{
>> > +g_free(data);
>> > +}
>> > +
>> >  #endif
>> > diff --git a/qga/main.c b/qga/main.c
>> > index 4c3b2c7..868508b 100644
>> > --- a/qga/main.c
>> > +++ b/qga/main.c
>> > @@ -1175,6 +1175,8 @@ static void config_free(GAConfig *config)
>> >  #ifdef CONFIG_FSFREEZE
>> >  g_free(config->fsfreeze_hook);
>> >  #endif
>> > +g_list_foreach(config->blacklist, qemu_g_func_free, NULL);
>> > +g_list_free(config->blacklist);
>> 
>> Modern GLib code doesn't need silly wrappers:
>> 
>> g_list_free_full(config->blacklist, g_free);
>> 
>> Unfortunately, this requires 2.28, and we're stull stuck at 2.22.
>> Please explain this in the commit message.
>> 
>> Even better, provide a replacement in glib-compat.h #if
>> !GLIB_CHECK_VERSION(2, 28, 0).  Will facilitate ditching the work-around
>> when we upgrade to 2.28.
>
> ok
>
>> 
>> >  g_free(config);
>> >  }
>> >  
>> > @@ -1310,11 +1312,6 @@ static int run_agent(GAState *s, GAConfig *config)
>> >  return EXIT_SUCCESS;
>> >  }
>> >  
>> > -static void free_blacklist_entry(gpointer entry, gpointer unused)
>> > -{
>> > -g_free(entry);
>> > -}
>> > -
>> >  int main(int argc, char **argv)
>> >  {
>> >  int ret = EXIT_SUCCESS;
>> > @@ -1379,7 +1376,6 @@ end:
>> >  if (s->channel) {
>> >  ga_channel_free(s->channel);
>> >  }
>> > -g_list_foreach(config->blacklist, free_blacklist_entry, NULL);
>> >  g_free(s->pstate_filepath);
>> >  g_free(s->state_filepath_isfrozen);
>> 
>> If you at least explain why not g_list_free_full() in the commit
>> message, you can add
>> Reviewed-by: Markus Armbruster 
>> 
>> But I'd like to encourage you to explore providing a replacement for
>> g_list_free_full().
>
> Such replacement would make use of a (GFunc) cast, glib implementation is 
> calling g_list_foreach (list, (GFunc) free_func, NULL), but Eric didn't want 
> to have such cast in qemu code. He suggested to have the static inline helper 
> in https://lists.gnu.org/archive/html/qemu-devel/2016-07/msg06561.html

Pointlessly dirty:

g_list_foreach(list, (GFunc)g_free, NULL)

Trade dirty for cumbersome:

void free_wrapper(gpointer data, gpointer unused)
{
g_free(data)
}

g_list_foreach(list, free_wrapper, NULL);

But this is C.  In C, simple things are best done the simple way.  Even
when that means we don't get to show off how amazingly well we've been
educated on higher order functions:

for (node = list; node; node = next) {
next = node->next;
g_free(node);
}

Simple, stupid, and not dirty.

Quote: "Note that it is considered perfectly acceptable to access
list->next directly."

Re: [Qemu-devel] [Patch v1 25/29] qmp: add QMP interface "query-cpu-model-comparison"

[Eduardo Habkost]

On Thu, Aug 04, 2016 at 09:34:16AM +0200, David Hildenbrand wrote:
> ##
>   
> # @CpuModelCompareResult: 
>   
> # 
>   
> # An enumeration of CPU model comparation results. The result is usually  
>   
> # calcualted using e.g. at CPU features or CPU generations.   
>   
> # 
>   
> # @incompatible: If model A is incompatible to model B, model A is not
>   
> #guaranteed to run where model B runs and the other way 
> around. 
> # 
>   
> # @identical: If model A is identical to model B, model A is guaranteed to 
> run  
> # where model B runs and the other way around.
>   
> # 
>   
> # @superset: If model A is a superset of model B, model B is guaranteed to 
> run  
> #where model A runs. There are no guarantees about the other way. 
>   
> # 
>   
> # @subset: If model A is a subset of model B, model A is guaranteed to run
>   
> #  where model B runs. There are no guarantees about the other way.   
>   
> # 
>   
> # Since: 2.8.0
>   
> ##
> 
> Think it's all about guarantees.
> 
[...]
> This comment is now:
> 
> ##
>   
> # @query-cpu-model-comparison:
>   
> # 
>   
> # Compares two CPU models, returning how they compare in a specific   
>   
> # configuration. The results indicates how both models compare regarding  
>   
> # runnability. This result can be used by tooling to make decisions if a  
>   
> # certain CPU model will run in a certain configuration or if a compatible
>   
> # CPU model has to be created by baselining.  
>   
> # 
>   
> # Usually, a CPU model is compared against the maximum possible CPU model 
>   
> # of a ceratin configuration (e.g. the "host" model for KVM). If that CPU 
>   
> # model is identical or a subset, it will run in that configuration.  
>   
> # 
>   
> # The result returned by this command may be affected by: 
>   
> # 
>   
> # * QEMU version: CPU models may look different depending on the QEMU 
> version.  
> #   (Except for CPU models reported as "static" in query-cpu-definitions.)
>   
> # * machine-type: CPU model  may look different depending on the 
> machine-type.  
> #   (Except for CPU models reported as "static" in query-cpu-definitions.)
>   
> # * machine options (including accelerator): in some architectures, CPU 
> models  
> #   may look different depending on machine and accelerator options. (Except 
> for
> #   CPU models reported as "static" in query-cpu-definitions.)
>   
> # * "-cpu" arguments and global properties: arguments to the -cpu option and  
>   
> #   global properties may affect expansion of CPU models. Using   
>   
> #   query-cpu-model-expansion while using these is not advised.   
>   
> # 
>   
> # Some architectures may not support comparing CPU models. s390x supports 
>   
> # comparing CPU models.   
>   
> # 
>   
> # Returns: a CpuModelBaselineInfo. Returns an error if comparing CPU models 
> is  
> #  not supported, if a model cannot be used, if a model contains  
>   
> #  an unknown cpu definition name, unknown properties or properties   
>   
> #  with wrong types.  
>   
> # 
>   
> # Since: 2.8.0
>   
> ##  
> 
> (excluding the remark about s390x in this patch)

Both look very clear to me. Thanks!

-- 
Eduardo

Re: [Qemu-devel] [Patch v1 26/29] qmp: add QMP interface "query-cpu-model-baseline"

[Eduardo Habkost]

On Thu, Aug 04, 2016 at 09:32:22AM +0200, David Hildenbrand wrote:
> ##
>   
> # @query-cpu-model-baseline:  
>   
> # 
>   
> # Baseline two CPU models, creating a compatible third model. The created 
>   
> # model will always be a static, migration-safe CPU model (see "static"   
>   
> # CPU model expansion for details).   
>   
> # 
>   
> # This interface can be used by tooling to create a compatible CPU model out  
>   
> # two CPU models. The created CPU model will be identical to or a subset of   
>   
> # both CPU models when comparing them. Therefore, the created CPU model is
>   
> # guaranteed to run where the given CPU models run.   
>   
> # 
>   
> # The result returned by this command may be affected by: 
>   
> # 
>   
> # * QEMU version: CPU models may look different depending on the QEMU 
> version.  
> #   (Except for CPU models reported as "static" in query-cpu-definitions.)
>   
> # * machine-type: CPU model  may look different depending on the 
> machine-type.  
> #   (Except for CPU models reported as "static" in query-cpu-definitions.)
>   
> # * machine options (including accelerator): in some architectures, CPU 
> models  
> #   may look different depending on machine and accelerator options. (Except 
> for
> #   CPU models reported as "static" in query-cpu-definitions.)
>   
> # * "-cpu" arguments and global properties: arguments to the -cpu option and  
>   
> #   global properties may affect expansion of CPU models. Using   
>   
> #   query-cpu-model-expansion while using these is not advised.   
>   
> # 
>   
> # Some architectures may not support baselining CPU models. s390x supports
>   
> # baselining CPU models.  
>   
> # 
>   
> # Returns: a CpuModelBaselineInfo. Returns an error if baselining CPU models 
> is 
> #  not supported, if a model cannot be used, if a model contains  
>   
> #  an unknown cpu definition name, unknown properties or properties   
>   
> #  with wrong types.  
>   
> # 
>   
> # Since: 2.8.0
>   
> ##  

Looks very clear to me, now. Thanks!

-- 
Eduardo

Re: [Qemu-devel] [PATCH for-2.7 v3 17/36] machine: use class base init generated name

[Marc-André Lureau]

Hi

On Thu, Aug 4, 2016 at 5:58 PM Markus Armbruster  wrote:

> marcandre.lur...@redhat.com writes:
>
> > From: Marc-André Lureau 
> >
> > Remove machine class name initialization from DEFINE_PC_MACHINE, rely on
> > class base init name generation instead. Get rid of some leaks that way.
> >
> > Signed-off-by: Marc-André Lureau 
> > ---
> >  hw/core/machine.c| 1 +
> >  include/hw/boards.h  | 2 +-
> >  include/hw/i386/pc.h | 1 -
> >  3 files changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/hw/core/machine.c b/hw/core/machine.c
> > index e5a456f..00fbe3e 100644
> > --- a/hw/core/machine.c
> > +++ b/hw/core/machine.c
> > @@ -561,6 +561,7 @@ static void machine_class_finalize(ObjectClass
> *klass, void *data)
> >  if (mc->compat_props) {
> >  g_array_free(mc->compat_props, true);
> >  }
> > +g_free(mc->name);
> >  }
> >
> >  void machine_register_compat_props(MachineState *machine)
> > diff --git a/include/hw/boards.h b/include/hw/boards.h
> > index 3e69eca..e46a744 100644
> > --- a/include/hw/boards.h
> > +++ b/include/hw/boards.h
> > @@ -93,7 +93,7 @@ struct MachineClass {
> >  /*< public >*/
> >
> >  const char *family; /* NULL iff @name identifies a standalone
> machtype */
> > -const char *name;
> > +char *name;
> >  const char *alias;
> >  const char *desc;
> >
> > diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
> > index eb1d414..afd025a 100644
> > --- a/include/hw/i386/pc.h
> > +++ b/include/hw/i386/pc.h
> > @@ -903,7 +903,6 @@ bool e820_get_entry(int, uint32_t, uint64_t *,
> uint64_t *);
> >  { \
> >  MachineClass *mc = MACHINE_CLASS(oc); \
> >  optsfn(mc); \
> > -mc->name = namestr; \
> >  mc->init = initfn; \
> >  } \
> >  static const TypeInfo pc_machine_type_##suffix = { \
>
> I guess there are is at least one assignment to mc->name not visible in
> this patch that assigns an allocated string, which leaks before the
> patch.  The commit message seems to provide a clue "class base init name
> generation".  I could probably find it with some effort, but patches
> that take that much work to understand make me grumpy.  Please provide
> another clue :)
>

Sorry, thanks for reminding me to write better commit messages.

git grep 'mc->name ='
hw/core/machine.c:mc->name = g_strndup(cname,

Is that better:

Remove machine class name initialization from DEFINE_PC_MACHINE, rely on
name generation from machine_class_base_init() instead, and free the
corresponding allocation in machine_class_finalize().
-- 
Marc-André Lureau

[Qemu-devel] [PULL 2/5] linux-user: Fix memchr() argument in open_self_cmdline()

[riku . voipio]

From: Peter Maydell 

In open_self_cmdline() we look for a 0 in the buffer we read
from /prc/self/cmdline. We were incorrectly passing the length
of our buf[] array to memchr() as the length to search, rather
than the number of bytes we actually read into it, which could
be shorter. This was spotted by Coverity (because it could
result in our trying to pass a negative length argument to
write()).

Signed-off-by: Peter Maydell 
Signed-off-by: Riku Voipio 
---
 linux-user/syscall.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index ca6a2b4..092ff4e 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -6856,7 +6856,7 @@ static int open_self_cmdline(void *cpu_env, int fd)
 if (!word_skipped) {
 /* Skip the first string, which is the path to qemu-*-static
instead of the actual command. */
-cp_buf = memchr(buf, 0, sizeof(buf));
+cp_buf = memchr(buf, 0, nb_read);
 if (cp_buf) {
 /* Null byte found, skip one string */
 cp_buf++;
-- 
2.1.4

[Qemu-devel] [PULL 4/5] linux-user: Fix target_semid_ds structure definition

[riku . voipio]

From: Peter Maydell 

The target_semid_ds structure is not correct for all
architectures: the padding fields should only exist for:
 * 32-bit ABIs
 * x86

It is also misnamed, since it is following the kernel
semid64_ds structure (QEMU doesn't support the legacy
semid_ds structure at all). Rename the struct, provide
a correct generic definition and allow the oddball x86
architecture to provide its own version.

This fixes broken SYSV semaphores for all our 64-bit
architectures except x86 and ppc.

Signed-off-by: Peter Maydell 
Signed-off-by: Riku Voipio 
---
 linux-user/syscall.c   | 17 ++---
 linux-user/x86_64/target_structs.h | 15 +++
 2 files changed, 25 insertions(+), 7 deletions(-)

diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index 5bc42c0..df6f2a9 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -3754,27 +3754,30 @@ static struct shm_region {
 bool in_use;
 } shm_regions[N_SHM_REGIONS];
 
-struct target_semid_ds
+#ifndef TARGET_SEMID64_DS
+/* asm-generic version of this struct */
+struct target_semid64_ds
 {
   struct target_ipc_perm sem_perm;
   abi_ulong sem_otime;
-#if !defined(TARGET_PPC64)
+#if TARGET_ABI_BITS == 32
   abi_ulong __unused1;
 #endif
   abi_ulong sem_ctime;
-#if !defined(TARGET_PPC64)
+#if TARGET_ABI_BITS == 32
   abi_ulong __unused2;
 #endif
   abi_ulong sem_nsems;
   abi_ulong __unused3;
   abi_ulong __unused4;
 };
+#endif
 
 static inline abi_long target_to_host_ipc_perm(struct ipc_perm *host_ip,
abi_ulong target_addr)
 {
 struct target_ipc_perm *target_ip;
-struct target_semid_ds *target_sd;
+struct target_semid64_ds *target_sd;
 
 if (!lock_user_struct(VERIFY_READ, target_sd, target_addr, 1))
 return -TARGET_EFAULT;
@@ -3802,7 +3805,7 @@ static inline abi_long host_to_target_ipc_perm(abi_ulong 
target_addr,
struct ipc_perm *host_ip)
 {
 struct target_ipc_perm *target_ip;
-struct target_semid_ds *target_sd;
+struct target_semid64_ds *target_sd;
 
 if (!lock_user_struct(VERIFY_WRITE, target_sd, target_addr, 0))
 return -TARGET_EFAULT;
@@ -3829,7 +3832,7 @@ static inline abi_long host_to_target_ipc_perm(abi_ulong 
target_addr,
 static inline abi_long target_to_host_semid_ds(struct semid_ds *host_sd,
abi_ulong target_addr)
 {
-struct target_semid_ds *target_sd;
+struct target_semid64_ds *target_sd;
 
 if (!lock_user_struct(VERIFY_READ, target_sd, target_addr, 1))
 return -TARGET_EFAULT;
@@ -3845,7 +3848,7 @@ static inline abi_long target_to_host_semid_ds(struct 
semid_ds *host_sd,
 static inline abi_long host_to_target_semid_ds(abi_ulong target_addr,
struct semid_ds *host_sd)
 {
-struct target_semid_ds *target_sd;
+struct target_semid64_ds *target_sd;
 
 if (!lock_user_struct(VERIFY_WRITE, target_sd, target_addr, 0))
 return -TARGET_EFAULT;
diff --git a/linux-user/x86_64/target_structs.h 
b/linux-user/x86_64/target_structs.h
index 3489827..b6e82a8 100644
--- a/linux-user/x86_64/target_structs.h
+++ b/linux-user/x86_64/target_structs.h
@@ -55,4 +55,19 @@ struct target_shmid_ds {
 abi_ulong __unused5;
 };
 
+/* The x86 definition differs from the generic one in that the
+ * two padding fields exist whether the ABI is 32 bits or 64 bits.
+ */
+#define TARGET_SEMID64_DS
+struct target_semid64_ds {
+struct target_ipc_perm sem_perm;
+abi_ulong sem_otime;
+abi_ulong __unused1;
+abi_ulong sem_ctime;
+abi_ulong __unused2;
+abi_ulong sem_nsems;
+abi_ulong __unused3;
+abi_ulong __unused4;
+};
+
 #endif
-- 
2.1.4

[Qemu-devel] [PULL 0/5] linux-user fixes for 2.7

[riku . voipio]

From: Riku Voipio 

The following changes since commit 09704e6ded83fa0bec14baf32f800f6512156ca0:

  Merge remote-tracking branch 'remotes/bonzini/tags/for-upstream' into staging 
(2016-08-04 10:24:27 +0100)

are available in the git repository at:

  git://git.linaro.org/people/riku.voipio/qemu.git tags/pull-linux-user-20160804

for you to fetch changes up to ef4330c23bb47b97a859dbdbae1c784fd2ca402f:

  linux-user: Handle brk() attempts with very large sizes (2016-08-04 16:38:17 
+0300)


linux-user important fixes for 2.7



Peter Maydell (5):
  linux-user: Use correct alignment for long long on i386 guests
  linux-user: Fix memchr() argument in open_self_cmdline()
  linux-user: Don't write off end of new_utsname buffer
  linux-user: Fix target_semid_ds structure definition
  linux-user: Handle brk() attempts with very large sizes

 include/exec/user/abitypes.h   |  4 
 linux-user/syscall.c   | 29 +
 linux-user/x86_64/target_structs.h | 15 +++
 3 files changed, 36 insertions(+), 12 deletions(-)

-- 
2.1.4

[Qemu-devel] [PULL 1/5] linux-user: Use correct alignment for long long on i386 guests

[riku . voipio]

From: Peter Maydell 

For i386, the ABI specifies that 'long long' (8 byte values)
need only be 4 aligned, but we were requiring them to be
8-aligned. This meant we were laying out the target_epoll_event
structure wrongly. Add a suitable ifdef to abitypes.h to
specify the i386-specific alignment requirement.

Reported-by: Icenowy Zheng 
Signed-off-by: Peter Maydell 
Reviewed-by: Laurent Vivier 
Signed-off-by: Riku Voipio 
---
 include/exec/user/abitypes.h | 4 
 1 file changed, 4 insertions(+)

diff --git a/include/exec/user/abitypes.h b/include/exec/user/abitypes.h
index a09d6c6..ba18860 100644
--- a/include/exec/user/abitypes.h
+++ b/include/exec/user/abitypes.h
@@ -15,6 +15,10 @@
 #define ABI_LLONG_ALIGNMENT 2
 #endif
 
+#if defined(TARGET_I386) && !defined(TARGET_X86_64)
+#define ABI_LLONG_ALIGNMENT 4
+#endif
+
 #ifndef ABI_SHORT_ALIGNMENT
 #define ABI_SHORT_ALIGNMENT 2
 #endif
-- 
2.1.4

[Qemu-devel] [PULL 5/5] linux-user: Handle brk() attempts with very large sizes

[riku . voipio]

From: Peter Maydell 

In do_brk(), we were inadvertently truncating the size
of a requested brk() from the guest by putting it into an
'int' variable. This meant that we would incorrectly report
success back to the guest rather than a failed allocation,
typically resulting in the guest then segfaulting. Use
abi_ulong instead.

This fixes a crash in the '31370.cc' test in the gcc libstdc++ test
suite (the test case starts by trying to allocate a very large
size and reduces the size until the allocation succeeds).

Signed-off-by: Peter Maydell 
Signed-off-by: Riku Voipio 
---
 linux-user/syscall.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index df6f2a9..833f853 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -839,7 +839,7 @@ void target_set_brk(abi_ulong new_brk)
 abi_long do_brk(abi_ulong new_brk)
 {
 abi_long mapped_addr;
-intnew_alloc_size;
+abi_ulong new_alloc_size;
 
 DEBUGF_BRK("do_brk(" TARGET_ABI_FMT_lx ") -> ", new_brk);
 
-- 
2.1.4

[Qemu-devel] [PULL 3/5] linux-user: Don't write off end of new_utsname buffer

[riku . voipio]

From: Peter Maydell 

Use g_strlcpy() rather than strcpy() to copy the uname string
into the structure we return to the guest for the uname syscall.
This avoids overrunning the buffer if the user passed us an
overlong string via the QEMU command line.

We fix a comment typo while we're in the neighbourhood.

Signed-off-by: Peter Maydell 
Signed-off-by: Riku Voipio 
---
 linux-user/syscall.c | 8 +---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index 092ff4e..5bc42c0 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -9237,12 +9237,14 @@ abi_long do_syscall(void *cpu_env, int num, abi_long 
arg1,
 goto efault;
 ret = get_errno(sys_uname(buf));
 if (!is_error(ret)) {
-/* Overrite the native machine name with whatever is being
+/* Overwrite the native machine name with whatever is being
emulated. */
 strcpy (buf->machine, cpu_to_uname_machine(cpu_env));
 /* Allow the user to override the reported release.  */
-if (qemu_uname_release && *qemu_uname_release)
-  strcpy (buf->release, qemu_uname_release);
+if (qemu_uname_release && *qemu_uname_release) {
+g_strlcpy(buf->release, qemu_uname_release,
+  sizeof(buf->release));
+}
 }
 unlock_user_struct(buf, arg1, 1);
 }
-- 
2.1.4

Re: [Qemu-devel] [PATCH for-2.7] Xen PCI passthrough: fix passthrough failure when no interrupt pin

[Anthony PERARD]

On Wed, Jul 27, 2016 at 09:30:48AM -0600, Bruce Rogers wrote:
> Commit 5a11d0f7 mistakenly converted a log message into an error
> condition when no pin interrupt is found for the pci device being
> passed through. Revert that part of the commit.
> 
> Signed-off-by: Bruce Rogers 

Acked-by: Anthony PERARD 

> ---
>  hw/xen/xen_pt.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/hw/xen/xen_pt.c b/hw/xen/xen_pt.c
> index f593b04..b6d71bb 100644
> --- a/hw/xen/xen_pt.c
> +++ b/hw/xen/xen_pt.c
> @@ -842,7 +842,7 @@ static void xen_pt_realize(PCIDevice *d, Error **errp)
>  goto err_out;
>  }
>  if (!scratch) {
> -error_setg(errp, "no pin interrupt");
> +XEN_PT_LOG(d, "no pin interrupt\n");
>  goto out;
>  }
>  
> -- 
> 1.9.0
> 

-- 
Anthony PERARD

[Qemu-devel] [PATCH v2] trace: add syslog tracing backend

[Paul Durrant]

This patch adds a tracing backend which sends output using syslog().
The syslog backend is limited to POSIX compliant systems.

openlog() is called with facility set to LOG_DAEMON, with the LOG_PID
option. Trace events are logged at level LOG_INFO.

Signed-off-by: Paul Durrant 
Cc: Stefan Hajnoczi 
---

v2:
 - Added a section to docs/tracing.txt
 - Re-ordered header inclusion in generated code
---
 configure   | 19 
 docs/tracing.txt| 12 ++
 scripts/tracetool/backend/syslog.py | 45 +
 trace/control.c |  7 ++
 4 files changed, 83 insertions(+)
 create mode 100644 scripts/tracetool/backend/syslog.py

diff --git a/configure b/configure
index 879324b..fce00b8 100755
--- a/configure
+++ b/configure
@@ -4189,6 +4189,18 @@ if compile_prog "" "" ; then
 fi
 
 ##
+# check if we have posix_syslog
+
+posix_syslog=no
+cat > $TMPC << EOF
+#include 
+int main(void) { openlog("qemu", LOG_PID, LOG_DAEMON); syslog(LOG_INFO, 
"configure"); return 0; }
+EOF
+if compile_prog "" "" ; then
+posix_syslog=yes
+fi
+
+##
 # check if trace backend exists
 
 $python "$source_path/scripts/tracetool.py" "--backends=$trace_backends" 
--check-backends  > /dev/null 2> /dev/null
@@ -5456,6 +5468,13 @@ if have_backend "ftrace"; then
 feature_not_found "ftrace(trace backend)" "ftrace requires Linux"
   fi
 fi
+if have_backend "syslog"; then
+  if test "$posix_syslog" = "yes" ; then
+echo "CONFIG_TRACE_SYSLOG=y" >> $config_host_mak
+  else
+feature_not_found "syslog(trace backend)" "syslog not available"
+  fi
+fi
 echo "CONFIG_TRACE_FILE=$trace_file" >> $config_host_mak
 
 if test "$rdma" = "yes" ; then
diff --git a/docs/tracing.txt b/docs/tracing.txt
index 29f2f9a..e62444c 100644
--- a/docs/tracing.txt
+++ b/docs/tracing.txt
@@ -192,6 +192,18 @@ After running qemu by root user, you can get the trace:
 
 Restriction: "ftrace" backend is restricted to Linux only.
 
+=== Syslog ===
+
+The "syslog" backend sends trace events using the POSIX syslog API. The log
+is opened specifying the LOG_DAEMON facility and LOG_PID option (so events
+are tagged with the pid of the particular QEMU process that generated
+them). All events are logged at LOG_INFO level.
+
+NOTE: syslog may squash duplicate consecutive trace events and apply rate
+  limiting.
+
+Restriction: "syslog" backend is restricted to POSIX compliant OS.
+
  Monitor commands 
 
 * trace-file on|off|flush|set 
diff --git a/scripts/tracetool/backend/syslog.py 
b/scripts/tracetool/backend/syslog.py
new file mode 100644
index 000..89019bc
--- /dev/null
+++ b/scripts/tracetool/backend/syslog.py
@@ -0,0 +1,45 @@
+#!/usr/bin/env python
+# -*- coding: utf-8 -*-
+
+"""
+Syslog built-in backend.
+"""
+
+__author__ = "Paul Durrant "
+__copyright__  = "Copyright 2016, Citrix Systems Inc."
+__license__= "GPL version 2 or (at your option) any later version"
+
+__maintainer__ = "Stefan Hajnoczi"
+__email__  = "stefa...@redhat.com"
+
+
+from tracetool import out
+
+
+PUBLIC = True
+
+
+def generate_h_begin(events):
+out('#include ',
+'#include "trace/control.h"',
+'')
+
+
+def generate_h(event):
+argnames = ", ".join(event.args.names())
+if len(event.args) > 0:
+argnames = ", " + argnames
+
+if "vcpu" in event.properties:
+# already checked on the generic format code
+cond = "true"
+else:
+cond = "trace_event_get_state(%s)" % ("TRACE_" + event.name.upper())
+
+out('if (%(cond)s) {',
+'syslog(LOG_INFO, "%(name)s " %(fmt)s %(argnames)s);',
+'}',
+cond=cond,
+name=event.name,
+fmt=event.fmt.rstrip("\n"),
+argnames=argnames)
diff --git a/trace/control.c b/trace/control.c
index d173c09..b179cde 100644
--- a/trace/control.c
+++ b/trace/control.c
@@ -19,6 +19,9 @@
 #ifdef CONFIG_TRACE_LOG
 #include "qemu/log.h"
 #endif
+#ifdef CONFIG_TRACE_SYSLOG
+#include 
+#endif
 #include "qapi/error.h"
 #include "qemu/error-report.h"
 #include "qemu/config-file.h"
@@ -250,6 +253,10 @@ bool trace_init_backends(void)
 }
 #endif
 
+#ifdef CONFIG_TRACE_SYSLOG
+openlog(NULL, LOG_PID, LOG_DAEMON);
+#endif
+
 return true;
 }
 
-- 
2.1.4

Re: [Qemu-devel] [PATCH for-2.7 v3 17/36] machine: use class base init generated name

[Markus Armbruster]

marcandre.lur...@redhat.com writes:

> From: Marc-André Lureau 
>
> Remove machine class name initialization from DEFINE_PC_MACHINE, rely on
> class base init name generation instead. Get rid of some leaks that way.
>
> Signed-off-by: Marc-André Lureau 
> ---
>  hw/core/machine.c| 1 +
>  include/hw/boards.h  | 2 +-
>  include/hw/i386/pc.h | 1 -
>  3 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/hw/core/machine.c b/hw/core/machine.c
> index e5a456f..00fbe3e 100644
> --- a/hw/core/machine.c
> +++ b/hw/core/machine.c
> @@ -561,6 +561,7 @@ static void machine_class_finalize(ObjectClass *klass, 
> void *data)
>  if (mc->compat_props) {
>  g_array_free(mc->compat_props, true);
>  }
> +g_free(mc->name);
>  }
>  
>  void machine_register_compat_props(MachineState *machine)
> diff --git a/include/hw/boards.h b/include/hw/boards.h
> index 3e69eca..e46a744 100644
> --- a/include/hw/boards.h
> +++ b/include/hw/boards.h
> @@ -93,7 +93,7 @@ struct MachineClass {
>  /*< public >*/
>  
>  const char *family; /* NULL iff @name identifies a standalone machtype */
> -const char *name;
> +char *name;
>  const char *alias;
>  const char *desc;
>  
> diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
> index eb1d414..afd025a 100644
> --- a/include/hw/i386/pc.h
> +++ b/include/hw/i386/pc.h
> @@ -903,7 +903,6 @@ bool e820_get_entry(int, uint32_t, uint64_t *, uint64_t 
> *);
>  { \
>  MachineClass *mc = MACHINE_CLASS(oc); \
>  optsfn(mc); \
> -mc->name = namestr; \
>  mc->init = initfn; \
>  } \
>  static const TypeInfo pc_machine_type_##suffix = { \

I guess there are is at least one assignment to mc->name not visible in
this patch that assigns an allocated string, which leaks before the
patch.  The commit message seems to provide a clue "class base init name
generation".  I could probably find it with some effort, but patches
that take that much work to understand make me grumpy.  Please provide
another clue :)

Re: [Qemu-devel] [PATCH for-2.7 v3 03/36] qga: free the whole blacklist

[Marc-André Lureau]

Hi

- Original Message -
> Copying the QGA maintainer.
> 
> marcandre.lur...@redhat.com writes:
> 
> > From: Marc-André Lureau 
> >
> > Free the list, not just the elements.
> >
> > Signed-off-by: Marc-André Lureau 
> > ---
> >  include/glib-compat.h | 9 +
> >  qga/main.c| 8 ++--
> >  2 files changed, 11 insertions(+), 6 deletions(-)
> >
> > diff --git a/include/glib-compat.h b/include/glib-compat.h
> > index 01aa7b3..6d643b2 100644
> > --- a/include/glib-compat.h
> > +++ b/include/glib-compat.h
> > @@ -260,4 +260,13 @@ static inline void g_hash_table_add(GHashTable
> > *hash_table, gpointer key)
> >  } while (0)
> >  #endif
> >  
> > +/*
> > + * A GFunc function helper freeing the first argument (not part of glib)
> 
> Well, it's not part of GLib, because non-obsolete GLib has no use for
> it!  You'd simply pass g_free directly to a _free_full() function
> instead of passing a silly wrapper to a _foreach() function.
> 
> The commit does a bit more than just plug a leak, it also provides a new
> helper for general use.  Mention in the commit message?
> 
> I wonder how many more of these silly g_free() wrappers we have.  A
> quick grep reports hits in string-input-visitor.c and
> qobject/json-streamer.c.  If you add one to a header, you get to hunt
> them down :)
> 
> > + */
> > +static inline void qemu_g_func_free(gpointer data,
> > +gpointer user_data)
> > +{
> > +g_free(data);
> > +}
> > +
> >  #endif
> > diff --git a/qga/main.c b/qga/main.c
> > index 4c3b2c7..868508b 100644
> > --- a/qga/main.c
> > +++ b/qga/main.c
> > @@ -1175,6 +1175,8 @@ static void config_free(GAConfig *config)
> >  #ifdef CONFIG_FSFREEZE
> >  g_free(config->fsfreeze_hook);
> >  #endif
> > +g_list_foreach(config->blacklist, qemu_g_func_free, NULL);
> > +g_list_free(config->blacklist);
> 
> Modern GLib code doesn't need silly wrappers:
> 
> g_list_free_full(config->blacklist, g_free);
> 
> Unfortunately, this requires 2.28, and we're stull stuck at 2.22.
> Please explain this in the commit message.
> 
> Even better, provide a replacement in glib-compat.h #if
> !GLIB_CHECK_VERSION(2, 28, 0).  Will facilitate ditching the work-around
> when we upgrade to 2.28.

ok

> 
> >  g_free(config);
> >  }
> >  
> > @@ -1310,11 +1312,6 @@ static int run_agent(GAState *s, GAConfig *config)
> >  return EXIT_SUCCESS;
> >  }
> >  
> > -static void free_blacklist_entry(gpointer entry, gpointer unused)
> > -{
> > -g_free(entry);
> > -}
> > -
> >  int main(int argc, char **argv)
> >  {
> >  int ret = EXIT_SUCCESS;
> > @@ -1379,7 +1376,6 @@ end:
> >  if (s->channel) {
> >  ga_channel_free(s->channel);
> >  }
> > -g_list_foreach(config->blacklist, free_blacklist_entry, NULL);
> >  g_free(s->pstate_filepath);
> >  g_free(s->state_filepath_isfrozen);
> 
> If you at least explain why not g_list_free_full() in the commit
> message, you can add
> Reviewed-by: Markus Armbruster 
> 
> But I'd like to encourage you to explore providing a replacement for
> g_list_free_full().

Such replacement would make use of a (GFunc) cast, glib implementation is 
calling g_list_foreach (list, (GFunc) free_func, NULL), but Eric didn't want to 
have such cast in qemu code. He suggested to have the static inline helper in 
https://lists.gnu.org/archive/html/qemu-devel/2016-07/msg06561.html

Re: [Qemu-devel] [PATCH for-2.7 v3 04/36] qga: free remaining leaking state

[Markus Armbruster]

Copying the QGA maintainer.

marcandre.lur...@redhat.com writes:

> From: Marc-André Lureau 
>
> Signed-off-by: Marc-André Lureau 
> ---
>  qga/guest-agent-command-state.c | 7 +++
>  qga/guest-agent-core.h  | 1 +
>  qga/main.c  | 6 ++
>  3 files changed, 14 insertions(+)
>
> diff --git a/qga/guest-agent-command-state.c b/qga/guest-agent-command-state.c
> index 4de229c..31c6368 100644
> --- a/qga/guest-agent-command-state.c
> +++ b/qga/guest-agent-command-state.c
> @@ -71,3 +71,10 @@ GACommandState *ga_command_state_new(void)
>  cs->groups = NULL;
>  return cs;
>  }
> +
> +void ga_command_state_free(GACommandState *cs)
> +{
> +g_slist_foreach(cs->groups, qemu_g_func_free, NULL);
> +g_slist_free(cs->groups);

Remarks on g_list_free_full() in PATCH 03 apply to g_slist_free_full()
here.

> +g_free(cs);
> +}
> diff --git a/qga/guest-agent-core.h b/qga/guest-agent-core.h
> index 0a49516..63e9d39 100644
> --- a/qga/guest-agent-core.h
> +++ b/qga/guest-agent-core.h
> @@ -28,6 +28,7 @@ void ga_command_state_add(GACommandState *cs,
>  void ga_command_state_init_all(GACommandState *cs);
>  void ga_command_state_cleanup_all(GACommandState *cs);
>  GACommandState *ga_command_state_new(void);
> +void ga_command_state_free(GACommandState *cs);
>  bool ga_logging_enabled(GAState *s);
>  void ga_disable_logging(GAState *s);
>  void ga_enable_logging(GAState *s);
> diff --git a/qga/main.c b/qga/main.c
> index 868508b..0038702 100644
> --- a/qga/main.c
> +++ b/qga/main.c
> @@ -1372,6 +1372,8 @@ int main(int argc, char **argv)
>  end:
>  if (s->command_state) {
>  ga_command_state_cleanup_all(s->command_state);
> +ga_command_state_free(s->command_state);
> +json_message_parser_destroy(&s->parser);
>  }
>  if (s->channel) {
>  ga_channel_free(s->channel);
> @@ -1384,6 +1386,10 @@ end:
>  }
>  
>  config_free(config);
> +if (s->main_loop) {
> +g_main_loop_unref(s->main_loop);
> +}
> +g_free(s);
>  
>  return ret;
>  }

Not bothering to free memory immediately before terminating the process
is not a leak.  The commit message shouldn't claim it is.

Personally, I wouldn't bother with freeing memory there.  Applies to
PATCH 03, too.  But it looks like the maintainer likes having it done.
If that's true, then doing it completely is probably better.

Leaving actual review to the maintainer.

Re: [Qemu-devel] [PATCH for-2.7 v3 03/36] qga: free the whole blacklist

[Markus Armbruster]

Copying the QGA maintainer.

marcandre.lur...@redhat.com writes:

> From: Marc-André Lureau 
>
> Free the list, not just the elements.
>
> Signed-off-by: Marc-André Lureau 
> ---
>  include/glib-compat.h | 9 +
>  qga/main.c| 8 ++--
>  2 files changed, 11 insertions(+), 6 deletions(-)
>
> diff --git a/include/glib-compat.h b/include/glib-compat.h
> index 01aa7b3..6d643b2 100644
> --- a/include/glib-compat.h
> +++ b/include/glib-compat.h
> @@ -260,4 +260,13 @@ static inline void g_hash_table_add(GHashTable 
> *hash_table, gpointer key)
>  } while (0)
>  #endif
>  
> +/*
> + * A GFunc function helper freeing the first argument (not part of glib)

Well, it's not part of GLib, because non-obsolete GLib has no use for
it!  You'd simply pass g_free directly to a _free_full() function
instead of passing a silly wrapper to a _foreach() function.

The commit does a bit more than just plug a leak, it also provides a new
helper for general use.  Mention in the commit message?

I wonder how many more of these silly g_free() wrappers we have.  A
quick grep reports hits in string-input-visitor.c and
qobject/json-streamer.c.  If you add one to a header, you get to hunt
them down :)

> + */
> +static inline void qemu_g_func_free(gpointer data,
> +gpointer user_data)
> +{
> +g_free(data);
> +}
> +
>  #endif
> diff --git a/qga/main.c b/qga/main.c
> index 4c3b2c7..868508b 100644
> --- a/qga/main.c
> +++ b/qga/main.c
> @@ -1175,6 +1175,8 @@ static void config_free(GAConfig *config)
>  #ifdef CONFIG_FSFREEZE
>  g_free(config->fsfreeze_hook);
>  #endif
> +g_list_foreach(config->blacklist, qemu_g_func_free, NULL);
> +g_list_free(config->blacklist);

Modern GLib code doesn't need silly wrappers:

g_list_free_full(config->blacklist, g_free);

Unfortunately, this requires 2.28, and we're stull stuck at 2.22.
Please explain this in the commit message.

Even better, provide a replacement in glib-compat.h #if
!GLIB_CHECK_VERSION(2, 28, 0).  Will facilitate ditching the work-around
when we upgrade to 2.28.

>  g_free(config);
>  }
>  
> @@ -1310,11 +1312,6 @@ static int run_agent(GAState *s, GAConfig *config)
>  return EXIT_SUCCESS;
>  }
>  
> -static void free_blacklist_entry(gpointer entry, gpointer unused)
> -{
> -g_free(entry);
> -}
> -
>  int main(int argc, char **argv)
>  {
>  int ret = EXIT_SUCCESS;
> @@ -1379,7 +1376,6 @@ end:
>  if (s->channel) {
>  ga_channel_free(s->channel);
>  }
> -g_list_foreach(config->blacklist, free_blacklist_entry, NULL);
>  g_free(s->pstate_filepath);
>  g_free(s->state_filepath_isfrozen);

If you at least explain why not g_list_free_full() in the commit
message, you can add
Reviewed-by: Markus Armbruster 

But I'd like to encourage you to explore providing a replacement for
g_list_free_full().

[Qemu-devel] [PATCH v1 3/5] target-ppc: add vector count trailing zeros instructions

[Rajalakshmi Srinivasaraghavan]

The following vector count trailing zeros instructions are
added from ISA 3.0.

vctzb - Vector Count Trailing Zeros Byte
vctzh - Vector Count Trailing Zeros Halfword
vctzw - Vector Count Trailing Zeros Word
vctzd - Vector Count Trailing Zeros Doubleword

Signed-off-by: Rajalakshmi Srinivasaraghavan 
---
 target-ppc/helper.h |4 
 target-ppc/int_helper.c |   15 +++
 target-ppc/translate/vmx-impl.c |   19 +++
 target-ppc/translate/vmx-ops.c  |8 
 4 files changed, 46 insertions(+), 0 deletions(-)

diff --git a/target-ppc/helper.h b/target-ppc/helper.h
index 59e7b88..6e6e7b3 100644
--- a/target-ppc/helper.h
+++ b/target-ppc/helper.h
@@ -327,6 +327,10 @@ DEF_HELPER_2(vclzb, void, avr, avr)
 DEF_HELPER_2(vclzh, void, avr, avr)
 DEF_HELPER_2(vclzw, void, avr, avr)
 DEF_HELPER_2(vclzd, void, avr, avr)
+DEF_HELPER_2(vctzb, void, avr, avr)
+DEF_HELPER_2(vctzh, void, avr, avr)
+DEF_HELPER_2(vctzw, void, avr, avr)
+DEF_HELPER_2(vctzd, void, avr, avr)
 DEF_HELPER_2(vpopcntb, void, avr, avr)
 DEF_HELPER_2(vpopcnth, void, avr, avr)
 DEF_HELPER_2(vpopcntw, void, avr, avr)
diff --git a/target-ppc/int_helper.c b/target-ppc/int_helper.c
index d545ec6..09f02f8 100644
--- a/target-ppc/int_helper.c
+++ b/target-ppc/int_helper.c
@@ -2090,6 +2090,21 @@ VGENERIC_DO(clzd, u64)
 #undef clzw
 #undef clzd
 
+#define ctzb(v) ((v) ? ctz32((uint32_t)(v)) : 8)
+#define ctzh(v) ((v) ? ctz32((uint32_t)(v)) : 16)
+#define ctzw(v) ctz32((v))
+#define ctzd(v) ctz64((v))
+
+VGENERIC_DO(ctzb, u8)
+VGENERIC_DO(ctzh, u16)
+VGENERIC_DO(ctzw, u32)
+VGENERIC_DO(ctzd, u64)
+
+#undef ctzb
+#undef ctzh
+#undef ctzw
+#undef ctzd
+
 #define popcntb(v) ctpop8(v)
 #define popcnth(v) ctpop16(v)
 #define popcntw(v) ctpop32(v)
diff --git a/target-ppc/translate/vmx-impl.c b/target-ppc/translate/vmx-impl.c
index 8bd48f3..2cf8c8f 100644
--- a/target-ppc/translate/vmx-impl.c
+++ b/target-ppc/translate/vmx-impl.c
@@ -553,6 +553,21 @@ static void glue(gen_, name)(DisasContext *ctx)
 \
 tcg_temp_free_ptr(rd);  \
 }
 
+#define GEN_VXFORM_NOA_2(name, opc2, opc3, opc4)\
+static void glue(gen_, name)(DisasContext *ctx) \
+{   \
+TCGv_ptr rb, rd;\
+if (unlikely(!ctx->altivec_enabled)) {  \
+gen_exception(ctx, POWERPC_EXCP_VPU);   \
+return; \
+}   \
+rb = gen_avr_ptr(rB(ctx->opcode));  \
+rd = gen_avr_ptr(rD(ctx->opcode));  \
+gen_helper_##name(rd, rb);  \
+tcg_temp_free_ptr(rb);  \
+tcg_temp_free_ptr(rd);  \
+}
+
 GEN_VXFORM_NOA(vupkhsb, 7, 8);
 GEN_VXFORM_NOA(vupkhsh, 7, 9);
 GEN_VXFORM_NOA(vupkhsw, 7, 25);
@@ -723,6 +738,10 @@ GEN_VXFORM_NOA(vclzb, 1, 28)
 GEN_VXFORM_NOA(vclzh, 1, 29)
 GEN_VXFORM_NOA(vclzw, 1, 30)
 GEN_VXFORM_NOA(vclzd, 1, 31)
+GEN_VXFORM_NOA_2(vctzb, 1, 24, 28)
+GEN_VXFORM_NOA_2(vctzh, 1, 24, 29)
+GEN_VXFORM_NOA_2(vctzw, 1, 24, 30)
+GEN_VXFORM_NOA_2(vctzd, 1, 24, 31)
 GEN_VXFORM_NOA(vpopcntb, 1, 28)
 GEN_VXFORM_NOA(vpopcnth, 1, 29)
 GEN_VXFORM_NOA(vpopcntw, 1, 30)
diff --git a/target-ppc/translate/vmx-ops.c b/target-ppc/translate/vmx-ops.c
index aafe70b..5b2826e 100644
--- a/target-ppc/translate/vmx-ops.c
+++ b/target-ppc/translate/vmx-ops.c
@@ -44,6 +44,10 @@ GEN_HANDLER_E(name, 0x04, opc2, opc3, 0x, PPC_NONE, 
PPC2_ISA300)
 #define GEN_VXFORM_300_EXT(name, opc2, opc3, inval) \
 GEN_HANDLER_E(name, 0x04, opc2, opc3, inval, PPC_NONE, PPC2_ISA300)
 
+#define GEN_VXFORM_300_EO(name, opc2, opc3, opc4) \
+GEN_HANDLER_E_2(name, 0x04, opc2, opc3, opc4, 0x, PPC_NONE, \
+   PPC2_ISA300)
+
 #define GEN_VXFORM_DUAL(name0, name1, opc2, opc3, type0, type1) \
 GEN_HANDLER_E(name0##_##name1, 0x4, opc2, opc3, 0x, type0, type1)
 
@@ -211,6 +215,10 @@ GEN_VXFORM_DUAL_INV(vspltish, vinserth, 6, 13, 0x, 
0x10,
 GEN_VXFORM_DUAL_INV(vspltisw, vinsertw, 6, 14, 0x, 0x10,
PPC2_ALTIVEC_207),
 GEN_VXFORM_300_EXT(vinsertd, 6, 15, 0x10),
+GEN_VXFORM_300_EO(vctzb, 0x01, 0x18, 0x1C),
+GEN_VXFORM_300_EO(vctzh, 0x01, 0x18, 0x1D),
+GEN_VXFORM_300_EO(vctzw, 0x01, 0x18, 0x1E),
+GEN_VXFORM_300_EO(vctzd, 0x01, 0x18, 0x1F),
 
 #define GEN_VXFORM_NOA(name, opc2, opc3)\
 GEN_HANDLER(name, 0x04, opc2, opc3, 0x001f, PPC_ALTIVEC)
-- 
1.7.1

[Qemu-devel] [PATCH v1 0/5] POWER9 TCG enablement - part3

[Rajalakshmi Srinivasaraghavan]

This series contains 14 new instructions for POWER9 described in ISA3.0.

Patches:
01: Adds vector insert instructions.
vinsertb - Vector Insert Byte
vinserth - Vector Insert Halfword
vinsertw - Vector Insert Word
vinsertd - Vector Insert Doubleword
02: Adds vector extract instructions.
vextractub - Vector Extract Unsigned Byte
vextractuh - Vector Extract Unsigned Halfword
vextractuw - Vector Extract Unsigned Word
vextractd - Vector Extract Unsigned Doubleword
03: Adds vector count trailing zeros instructions.
vctzb - Vector Count Trailing Zeros Byte
vctzh - Vector Count Trailing Zeros Halfword
vctzw - Vector Count Trailing Zeros Word
vctzd - Vector Count Trailing Zeros Doubleword
04: Adds vbpermd-vector bit permute doubleword instruction.
05: Adds vpermr-vector permute right indexed instruction.

Changelog:
v0:
* Rename GEN_VXFORM_300_EXT1 to GEN_VXFORM_300_EO.
* Rename GEN_VXFORM_DUAL1 to GEN_VXFORM_DUAL_INV.
* Remove undef GEN_VXFORM_DUAL1.

 target-ppc/helper.h |   14 +
 target-ppc/int_helper.c |  110 +++
 target-ppc/translate/vmx-impl.c |   58 
 target-ppc/translate/vmx-ops.c  |   38 +++---
 4 files changed, 212 insertions(+), 8 deletions(-)

[Qemu-devel] [PATCH v1 2/5] target-ppc: add vector extract instructions

[Rajalakshmi Srinivasaraghavan]

The following vector extract instructions are added from ISA 3.0.

vextractub - Vector Extract Unsigned Byte
vextractuh - Vector Extract Unsigned Halfword
vextractuw - Vector Extract Unsigned Word
vextractd - Vector Extract Unsigned Doubleword

Signed-off-by: Rajalakshmi Srinivasaraghavan 
---
 target-ppc/helper.h |4 
 target-ppc/int_helper.c |   31 +++
 target-ppc/translate/vmx-impl.c |   10 ++
 target-ppc/translate/vmx-ops.c  |   10 +++---
 4 files changed, 52 insertions(+), 3 deletions(-)

diff --git a/target-ppc/helper.h b/target-ppc/helper.h
index 0923779..59e7b88 100644
--- a/target-ppc/helper.h
+++ b/target-ppc/helper.h
@@ -250,6 +250,10 @@ DEF_HELPER_2(vspltisw, void, avr, i32)
 DEF_HELPER_3(vspltb, void, avr, avr, i32)
 DEF_HELPER_3(vsplth, void, avr, avr, i32)
 DEF_HELPER_3(vspltw, void, avr, avr, i32)
+DEF_HELPER_3(vextractub, void, avr, avr, i32)
+DEF_HELPER_3(vextractuh, void, avr, avr, i32)
+DEF_HELPER_3(vextractuw, void, avr, avr, i32)
+DEF_HELPER_3(vextractd, void, avr, avr, i32)
 DEF_HELPER_3(vinsertb, void, avr, avr, i32)
 DEF_HELPER_3(vinserth, void, avr, avr, i32)
 DEF_HELPER_3(vinsertw, void, avr, avr, i32)
diff --git a/target-ppc/int_helper.c b/target-ppc/int_helper.c
index 637f0b1..d545ec6 100644
--- a/target-ppc/int_helper.c
+++ b/target-ppc/int_helper.c
@@ -1811,6 +1811,37 @@ VINSERT(h, u16, 3)
 VINSERT(w, u32, 1)
 VINSERT(d, u64, 0)
 #undef VINSERT
+#if defined(HOST_WORDS_BIGENDIAN)
+#define VEXTRACT(suffix, element, index) \
+void helper_vextract##suffix(ppc_avr_t *r, ppc_avr_t *b, uint32_t splat) \
+{\
+int i;   \
+ \
+for (i = 0; i < ARRAY_SIZE(r->element); i++) {   \
+r->element[i] = 0;   \
+}\
+memcpy(&r->element[index], &b->u8[SPLAT_ELEMENT(u8)],\
+   sizeof(r->element[0]));   \
+}
+#else
+#define VEXTRACT(suffix, element, index) \
+void helper_vextract##suffix(ppc_avr_t *r, ppc_avr_t *b, uint32_t splat) \
+{\
+int i;   \
+ \
+for (i = 0; i < ARRAY_SIZE(r->element); i++) {   \
+r->element[i] = 0;   \
+}\
+memcpy(&r->element[(ARRAY_SIZE(r->element) - index) - 1],\
+   &b->u8[(16 - splat) - sizeof(r->element[0])], \
+   sizeof(r->element[0]));   \
+}
+#endif
+VEXTRACT(ub, u8, 7)
+VEXTRACT(uh, u16, 3)
+VEXTRACT(uw, u32, 1)
+VEXTRACT(d, u64, 0)
+#undef VEXTRACT
 #undef SPLAT_ELEMENT
 #undef _SPLAT_MASKED
 
diff --git a/target-ppc/translate/vmx-impl.c b/target-ppc/translate/vmx-impl.c
index 4940ae3..8bd48f3 100644
--- a/target-ppc/translate/vmx-impl.c
+++ b/target-ppc/translate/vmx-impl.c
@@ -626,6 +626,10 @@ static void glue(gen_, name)(DisasContext *ctx)
 \
 GEN_VXFORM_UIMM(vspltb, 6, 8);
 GEN_VXFORM_UIMM(vsplth, 6, 9);
 GEN_VXFORM_UIMM(vspltw, 6, 10);
+GEN_VXFORM_UIMM(vextractub, 6, 8);
+GEN_VXFORM_UIMM(vextractuh, 6, 9);
+GEN_VXFORM_UIMM(vextractuw, 6, 10);
+GEN_VXFORM_UIMM(vextractd, 6, 11);
 GEN_VXFORM_UIMM(vinsertb, 6, 12);
 GEN_VXFORM_UIMM(vinserth, 6, 13);
 GEN_VXFORM_UIMM(vinsertw, 6, 14);
@@ -634,6 +638,12 @@ GEN_VXFORM_UIMM_ENV(vcfux, 5, 12);
 GEN_VXFORM_UIMM_ENV(vcfsx, 5, 13);
 GEN_VXFORM_UIMM_ENV(vctuxs, 5, 14);
 GEN_VXFORM_UIMM_ENV(vctsxs, 5, 15);
+GEN_VXFORM_DUAL(vspltb, PPC_NONE, PPC2_ALTIVEC_207,
+  vextractub, PPC_NONE, PPC2_ISA300);
+GEN_VXFORM_DUAL(vsplth, PPC_NONE, PPC2_ALTIVEC_207,
+  vextractuh, PPC_NONE, PPC2_ISA300);
+GEN_VXFORM_DUAL(vspltw, PPC_NONE, PPC2_ALTIVEC_207,
+  vextractuw, PPC_NONE, PPC2_ISA300);
 GEN_VXFORM_DUAL(vspltisb, PPC_NONE, PPC2_ALTIVEC_207,
   vinsertb, PPC_NONE, PPC2_ISA300);
 GEN_VXFORM_DUAL(vspltish, PPC_NONE, PPC2_ALTIVEC_207,
diff --git a/target-ppc/translate/vmx-ops.c b/target-ppc/translate/vmx-ops.c
index ca69e56..aafe70b 100644
--- a/target-ppc/translate/vmx-ops.c
+++ b/target-ppc/translate/vmx-ops.c
@@ -197,6 +197,13 @@ GEN_VXRFORM_DUAL(vcmpbfp, vcmpgtsd, 3, 15, PPC_ALTIVEC, 
PPC_NONE)
 #define GEN_VXFORM_DUAL_INV(name0, name1, opc2, opc3, inval0, inval1, type

[Qemu-devel] [PATCH v1 1/5] target-ppc: add vector insert instructions

[Rajalakshmi Srinivasaraghavan]

The following vector insert instructions are added from ISA 3.0.

vinsertb - Vector Insert Byte
vinserth - Vector Insert Halfword
vinsertw - Vector Insert Word
vinsertd - Vector Insert Doubleword

Signed-off-by: Rajalakshmi Srinivasaraghavan 
---
 target-ppc/helper.h |4 
 target-ppc/int_helper.c |   21 +
 target-ppc/translate/vmx-impl.c |   10 ++
 target-ppc/translate/vmx-ops.c  |   18 +-
 4 files changed, 48 insertions(+), 5 deletions(-)

diff --git a/target-ppc/helper.h b/target-ppc/helper.h
index 93ac9e1..0923779 100644
--- a/target-ppc/helper.h
+++ b/target-ppc/helper.h
@@ -250,6 +250,10 @@ DEF_HELPER_2(vspltisw, void, avr, i32)
 DEF_HELPER_3(vspltb, void, avr, avr, i32)
 DEF_HELPER_3(vsplth, void, avr, avr, i32)
 DEF_HELPER_3(vspltw, void, avr, avr, i32)
+DEF_HELPER_3(vinsertb, void, avr, avr, i32)
+DEF_HELPER_3(vinserth, void, avr, avr, i32)
+DEF_HELPER_3(vinsertw, void, avr, avr, i32)
+DEF_HELPER_3(vinsertd, void, avr, avr, i32)
 DEF_HELPER_2(vupkhpx, void, avr, avr)
 DEF_HELPER_2(vupklpx, void, avr, avr)
 DEF_HELPER_2(vupkhsb, void, avr, avr)
diff --git a/target-ppc/int_helper.c b/target-ppc/int_helper.c
index 552b2e0..637f0b1 100644
--- a/target-ppc/int_helper.c
+++ b/target-ppc/int_helper.c
@@ -1790,6 +1790,27 @@ VSPLT(b, u8)
 VSPLT(h, u16)
 VSPLT(w, u32)
 #undef VSPLT
+#if defined(HOST_WORDS_BIGENDIAN)
+#define VINSERT(suffix, element, index) \
+void helper_vinsert##suffix(ppc_avr_t *r, ppc_avr_t *b, uint32_t splat) \
+{   \
+memcpy(&r->u8[SPLAT_ELEMENT(u8)], &b->element[index],   \
+   sizeof(r->element[0]));  \
+}
+#else
+#define VINSERT(suffix, element, index) \
+void helper_vinsert##suffix(ppc_avr_t *r, ppc_avr_t *b, uint32_t splat) \
+{   \
+memcpy(&r->u8[(16 - splat) - sizeof(r->element[0])],\
+   &b->element[(ARRAY_SIZE(r->element) - index) - 1],   \
+   sizeof(r->element[0]));  \
+}
+#endif
+VINSERT(b, u8, 7)
+VINSERT(h, u16, 3)
+VINSERT(w, u32, 1)
+VINSERT(d, u64, 0)
+#undef VINSERT
 #undef SPLAT_ELEMENT
 #undef _SPLAT_MASKED
 
diff --git a/target-ppc/translate/vmx-impl.c b/target-ppc/translate/vmx-impl.c
index ac78caf..4940ae3 100644
--- a/target-ppc/translate/vmx-impl.c
+++ b/target-ppc/translate/vmx-impl.c
@@ -626,10 +626,20 @@ static void glue(gen_, name)(DisasContext *ctx)   
  \
 GEN_VXFORM_UIMM(vspltb, 6, 8);
 GEN_VXFORM_UIMM(vsplth, 6, 9);
 GEN_VXFORM_UIMM(vspltw, 6, 10);
+GEN_VXFORM_UIMM(vinsertb, 6, 12);
+GEN_VXFORM_UIMM(vinserth, 6, 13);
+GEN_VXFORM_UIMM(vinsertw, 6, 14);
+GEN_VXFORM_UIMM(vinsertd, 6, 15);
 GEN_VXFORM_UIMM_ENV(vcfux, 5, 12);
 GEN_VXFORM_UIMM_ENV(vcfsx, 5, 13);
 GEN_VXFORM_UIMM_ENV(vctuxs, 5, 14);
 GEN_VXFORM_UIMM_ENV(vctsxs, 5, 15);
+GEN_VXFORM_DUAL(vspltisb, PPC_NONE, PPC2_ALTIVEC_207,
+  vinsertb, PPC_NONE, PPC2_ISA300);
+GEN_VXFORM_DUAL(vspltish, PPC_NONE, PPC2_ALTIVEC_207,
+  vinserth, PPC_NONE, PPC2_ISA300);
+GEN_VXFORM_DUAL(vspltisw, PPC_NONE, PPC2_ALTIVEC_207,
+  vinsertw, PPC_NONE, PPC2_ISA300);
 
 static void gen_vsldoi(DisasContext *ctx)
 {
diff --git a/target-ppc/translate/vmx-ops.c b/target-ppc/translate/vmx-ops.c
index 7449396..ca69e56 100644
--- a/target-ppc/translate/vmx-ops.c
+++ b/target-ppc/translate/vmx-ops.c
@@ -41,6 +41,9 @@ GEN_HANDLER_E(name, 0x04, opc2, opc3, 0x, PPC_NONE, 
PPC2_ALTIVEC_207)
 #define GEN_VXFORM_300(name, opc2, opc3)\
 GEN_HANDLER_E(name, 0x04, opc2, opc3, 0x, PPC_NONE, PPC2_ISA300)
 
+#define GEN_VXFORM_300_EXT(name, opc2, opc3, inval) \
+GEN_HANDLER_E(name, 0x04, opc2, opc3, inval, PPC_NONE, PPC2_ISA300)
+
 #define GEN_VXFORM_DUAL(name0, name1, opc2, opc3, type0, type1) \
 GEN_HANDLER_E(name0##_##name1, 0x4, opc2, opc3, 0x, type0, type1)
 
@@ -191,11 +194,16 @@ GEN_VXRFORM(vcmpgefp, 3, 7)
 GEN_VXRFORM_DUAL(vcmpgtfp, vcmpgtud, 3, 11, PPC_ALTIVEC, PPC_NONE)
 GEN_VXRFORM_DUAL(vcmpbfp, vcmpgtsd, 3, 15, PPC_ALTIVEC, PPC_NONE)
 
-#define GEN_VXFORM_SIMM(name, opc2, opc3)   \
-GEN_HANDLER(name, 0x04, opc2, opc3, 0x, PPC_ALTIVEC)
-GEN_VXFORM_SIMM(vspltisb, 6, 12),
-GEN_VXFORM_SIMM(vspltish, 6, 13),
-GEN_VXFORM_SIMM(vspltisw, 6, 14),
+#define GEN_VXFORM_DUAL_INV(name0, name1, opc2, opc3, inval0, inval1, type) \
+GEN_OPCODE_DUAL(name0##_##name1, 0x04, opc2, opc3, inval0, inval1, type, \
+   PPC_NONE)
+GEN_VXFORM_DUAL_INV(vspltisb, vinsertb, 6, 12, 0x, 0x10,
+   PPC2_

[Qemu-devel] [PATCH v1 5/5] target-ppc: add vector permute right indexed instruction

[Rajalakshmi Srinivasaraghavan]

Add vpermr instruction from ISA 3.0.

Signed-off-by: Rajalakshmi Srinivasaraghavan 
---
 target-ppc/helper.h |1 +
 target-ppc/int_helper.c |   23 +++
 target-ppc/translate/vmx-impl.c |   18 ++
 target-ppc/translate/vmx-ops.c  |1 +
 4 files changed, 43 insertions(+), 0 deletions(-)

diff --git a/target-ppc/helper.h b/target-ppc/helper.h
index d1d9418..3c476c9 100644
--- a/target-ppc/helper.h
+++ b/target-ppc/helper.h
@@ -270,6 +270,7 @@ DEF_HELPER_5(vmsumubm, void, env, avr, avr, avr, avr)
 DEF_HELPER_5(vmsummbm, void, env, avr, avr, avr, avr)
 DEF_HELPER_5(vsel, void, env, avr, avr, avr, avr)
 DEF_HELPER_5(vperm, void, env, avr, avr, avr, avr)
+DEF_HELPER_5(vpermr, void, env, avr, avr, avr, avr)
 DEF_HELPER_4(vpkshss, void, env, avr, avr, avr)
 DEF_HELPER_4(vpkshus, void, env, avr, avr, avr)
 DEF_HELPER_4(vpkswss, void, env, avr, avr, avr)
diff --git a/target-ppc/int_helper.c b/target-ppc/int_helper.c
index d8ad56f..79e4273 100644
--- a/target-ppc/int_helper.c
+++ b/target-ppc/int_helper.c
@@ -1126,6 +1126,29 @@ void helper_vperm(CPUPPCState *env, ppc_avr_t *r, 
ppc_avr_t *a, ppc_avr_t *b,
 *r = result;
 }
 
+void helper_vpermr(CPUPPCState *env, ppc_avr_t *r, ppc_avr_t *a, ppc_avr_t *b,
+  ppc_avr_t *c)
+{
+ppc_avr_t result;
+int i;
+
+VECTOR_FOR_INORDER_I(i, u8) {
+int s = c->u8[i] & 0x1f;
+#if defined(HOST_WORDS_BIGENDIAN)
+int index = s & 0xf;
+#else
+int index = 15 - (s & 0xf);
+#endif
+
+if (s & 0x10) {
+result.u8[i] = a->u8[15 - index];
+} else {
+result.u8[i] = b->u8[15 - index];
+}
+}
+*r = result;
+}
+
 #if defined(HOST_WORDS_BIGENDIAN)
 #define VBPERMQ_INDEX(avr, i) ((avr)->u8[(i)])
 #define VBPERMQ_DW(index) (((index) & 0x40) != 0)
diff --git a/target-ppc/translate/vmx-impl.c b/target-ppc/translate/vmx-impl.c
index 5ddff58..d13640f 100644
--- a/target-ppc/translate/vmx-impl.c
+++ b/target-ppc/translate/vmx-impl.c
@@ -728,6 +728,24 @@ static void gen_vmladduhm(DisasContext *ctx)
 tcg_temp_free_ptr(rd);
 }
 
+static void gen_vpermr(DisasContext *ctx)
+{
+TCGv_ptr ra, rb, rc, rd;
+if (unlikely(!ctx->altivec_enabled)) {
+gen_exception(ctx, POWERPC_EXCP_VPU);
+return;
+}
+ra = gen_avr_ptr(rA(ctx->opcode));
+rb = gen_avr_ptr(rB(ctx->opcode));
+rc = gen_avr_ptr(rC(ctx->opcode));
+rd = gen_avr_ptr(rD(ctx->opcode));
+gen_helper_vpermr(cpu_env, rd, ra, rb, rc);
+tcg_temp_free_ptr(ra);
+tcg_temp_free_ptr(rb);
+tcg_temp_free_ptr(rc);
+tcg_temp_free_ptr(rd);
+}
+
 GEN_VAFORM_PAIRED(vmsumubm, vmsummbm, 18)
 GEN_VAFORM_PAIRED(vmsumuhm, vmsumuhs, 19)
 GEN_VAFORM_PAIRED(vmsumshm, vmsumshs, 20)
diff --git a/target-ppc/translate/vmx-ops.c b/target-ppc/translate/vmx-ops.c
index 32bd533..ad72db5 100644
--- a/target-ppc/translate/vmx-ops.c
+++ b/target-ppc/translate/vmx-ops.c
@@ -219,6 +219,7 @@ GEN_VXFORM_300_EO(vctzb, 0x01, 0x18, 0x1C),
 GEN_VXFORM_300_EO(vctzh, 0x01, 0x18, 0x1D),
 GEN_VXFORM_300_EO(vctzw, 0x01, 0x18, 0x1E),
 GEN_VXFORM_300_EO(vctzd, 0x01, 0x18, 0x1F),
+GEN_VXFORM_300(vpermr, 0x1D, 0xFF),
 
 #define GEN_VXFORM_NOA(name, opc2, opc3)\
 GEN_HANDLER(name, 0x04, opc2, opc3, 0x001f, PPC_ALTIVEC)
-- 
1.7.1

[Qemu-devel] [PATCH v1 4/5] target-ppc: add vector bit permute doubleword instruction

[Rajalakshmi Srinivasaraghavan]

Add vbpermd instruction from ISA 3.0.

Signed-off-by: Rajalakshmi Srinivasaraghavan 
---
 target-ppc/helper.h |1 +
 target-ppc/int_helper.c |   20 
 target-ppc/translate/vmx-impl.c |1 +
 target-ppc/translate/vmx-ops.c  |1 +
 4 files changed, 23 insertions(+), 0 deletions(-)

diff --git a/target-ppc/helper.h b/target-ppc/helper.h
index 6e6e7b3..d1d9418 100644
--- a/target-ppc/helper.h
+++ b/target-ppc/helper.h
@@ -335,6 +335,7 @@ DEF_HELPER_2(vpopcntb, void, avr, avr)
 DEF_HELPER_2(vpopcnth, void, avr, avr)
 DEF_HELPER_2(vpopcntw, void, avr, avr)
 DEF_HELPER_2(vpopcntd, void, avr, avr)
+DEF_HELPER_3(vbpermd, void, avr, avr, avr)
 DEF_HELPER_3(vbpermq, void, avr, avr, avr)
 DEF_HELPER_2(vgbbd, void, avr, avr)
 DEF_HELPER_3(vpmsumb, void, avr, avr, avr)
diff --git a/target-ppc/int_helper.c b/target-ppc/int_helper.c
index 09f02f8..d8ad56f 100644
--- a/target-ppc/int_helper.c
+++ b/target-ppc/int_helper.c
@@ -1134,6 +1134,26 @@ void helper_vperm(CPUPPCState *env, ppc_avr_t *r, 
ppc_avr_t *a, ppc_avr_t *b,
 #define VBPERMQ_DW(index) (((index) & 0x40) == 0)
 #endif
 
+void helper_vbpermd(ppc_avr_t *r, ppc_avr_t *a, ppc_avr_t *b)
+{
+int i, j;
+uint64_t perm = 0;
+
+VECTOR_FOR_INORDER_I(i, u64) {
+perm = 0;
+VECTOR_FOR_INORDER_I(j, u16) {
+int index = VBPERMQ_INDEX(b, (i * 8) + j);
+if (index < 64) {
+uint64_t mask = (1ull << (63 - (index & 0x3F)));
+if (a->u64[VBPERMQ_DW(index)] & mask) {
+perm |= (0x80 >> j);
+}
+}
+r->u64[i] = perm;
+}
+}
+}
+
 void helper_vbpermq(ppc_avr_t *r, ppc_avr_t *a, ppc_avr_t *b)
 {
 int i;
diff --git a/target-ppc/translate/vmx-impl.c b/target-ppc/translate/vmx-impl.c
index 2cf8c8f..5ddff58 100644
--- a/target-ppc/translate/vmx-impl.c
+++ b/target-ppc/translate/vmx-impl.c
@@ -754,6 +754,7 @@ GEN_VXFORM_DUAL(vclzw, PPC_NONE, PPC2_ALTIVEC_207, \
 vpopcntw, PPC_NONE, PPC2_ALTIVEC_207)
 GEN_VXFORM_DUAL(vclzd, PPC_NONE, PPC2_ALTIVEC_207, \
 vpopcntd, PPC_NONE, PPC2_ALTIVEC_207)
+GEN_VXFORM(vbpermd, 6, 23);
 GEN_VXFORM(vbpermq, 6, 21);
 GEN_VXFORM_NOA(vgbbd, 6, 20);
 GEN_VXFORM(vpmsumb, 4, 16)
diff --git a/target-ppc/translate/vmx-ops.c b/target-ppc/translate/vmx-ops.c
index 5b2826e..32bd533 100644
--- a/target-ppc/translate/vmx-ops.c
+++ b/target-ppc/translate/vmx-ops.c
@@ -261,6 +261,7 @@ GEN_VXFORM_DUAL(vclzh, vpopcnth, 1, 29, PPC_NONE, 
PPC2_ALTIVEC_207),
 GEN_VXFORM_DUAL(vclzw, vpopcntw, 1, 30, PPC_NONE, PPC2_ALTIVEC_207),
 GEN_VXFORM_DUAL(vclzd, vpopcntd, 1, 31, PPC_NONE, PPC2_ALTIVEC_207),
 
+GEN_VXFORM_300(vbpermd, 6, 23),
 GEN_VXFORM_207(vbpermq, 6, 21),
 GEN_VXFORM_207(vgbbd, 6, 20),
 GEN_VXFORM_207(vpmsumb, 4, 16),
-- 
1.7.1

[Qemu-devel] [RESEND PATCH V5 2/6] coroutine: add a macro for the coroutine stack size

[Peter Lieven]

Signed-off-by: Peter Lieven 
Reviewed-by: Paolo Bonzini 
Reviewed-by: Richard Henderson 
---
 include/qemu/coroutine_int.h | 2 ++
 util/coroutine-sigaltstack.c | 2 +-
 util/coroutine-ucontext.c| 2 +-
 util/coroutine-win32.c   | 2 +-
 4 files changed, 5 insertions(+), 3 deletions(-)

diff --git a/include/qemu/coroutine_int.h b/include/qemu/coroutine_int.h
index 42d6838..eac323a 100644
--- a/include/qemu/coroutine_int.h
+++ b/include/qemu/coroutine_int.h
@@ -28,6 +28,8 @@
 #include "qemu/queue.h"
 #include "qemu/coroutine.h"
 
+#define COROUTINE_STACK_SIZE (1 << 20)
+
 typedef enum {
 COROUTINE_YIELD = 1,
 COROUTINE_TERMINATE = 2,
diff --git a/util/coroutine-sigaltstack.c b/util/coroutine-sigaltstack.c
index a7c3366..9c2854c 100644
--- a/util/coroutine-sigaltstack.c
+++ b/util/coroutine-sigaltstack.c
@@ -143,7 +143,7 @@ static void coroutine_trampoline(int signal)
 
 Coroutine *qemu_coroutine_new(void)
 {
-const size_t stack_size = 1 << 20;
+const size_t stack_size = COROUTINE_STACK_SIZE;
 CoroutineUContext *co;
 CoroutineThreadState *coTS;
 struct sigaction sa;
diff --git a/util/coroutine-ucontext.c b/util/coroutine-ucontext.c
index 2bb7e10..31254ab 100644
--- a/util/coroutine-ucontext.c
+++ b/util/coroutine-ucontext.c
@@ -82,7 +82,7 @@ static void coroutine_trampoline(int i0, int i1)
 
 Coroutine *qemu_coroutine_new(void)
 {
-const size_t stack_size = 1 << 20;
+const size_t stack_size = COROUTINE_STACK_SIZE;
 CoroutineUContext *co;
 ucontext_t old_uc, uc;
 sigjmp_buf old_env;
diff --git a/util/coroutine-win32.c b/util/coroutine-win32.c
index 02e28e8..de6bd4f 100644
--- a/util/coroutine-win32.c
+++ b/util/coroutine-win32.c
@@ -71,7 +71,7 @@ static void CALLBACK coroutine_trampoline(void *co_)
 
 Coroutine *qemu_coroutine_new(void)
 {
-const size_t stack_size = 1 << 20;
+const size_t stack_size = COROUTINE_STACK_SIZE;
 CoroutineWin32 *co;
 
 co = g_malloc0(sizeof(*co));
-- 
1.9.1

Re: [Qemu-devel] [PATCH 1/7] util: Add UUID API

[Marc-André Lureau]

Hi

On Thu, Aug 4, 2016 at 4:44 PM Paolo Bonzini  wrote:

>
>
> On 03/08/2016 04:36, Fam Zheng wrote:
> > On Tue, 08/02 15:45, Paolo Bonzini wrote:
> >>
> >>
> >> - Original Message -
> >>> From: "Fam Zheng" 
> >>> To: qemu-devel@nongnu.org
> >>> Cc: f...@redhat.com, berra...@redhat.com, pbonz...@redhat.com,
> kw...@redhat.com, mre...@redhat.com,
> >>> mdr...@linux.vnet.ibm.com, arm...@redhat.com, s...@weilnetz.de,
> qemu-bl...@nongnu.org
> >>> Sent: Tuesday, August 2, 2016 11:18:32 AM
> >>> Subject: [PATCH 1/7] util: Add UUID API
> >>>
> >>> A number of different places across the code base use CONFIG_UUID. Some
> >>> of them are soft dependency, some are not built if libuuid is not
> >>> available, some come with dummy fallback, some throws runtime error.
>

For info, there has been glib UUID proposal for a while:

https://bugzilla.gnome.org/show_bug.cgi?id=639078

Although quite ready, the discussion stopped because some dev believe
g_uuid_string_random() is all you need. Anyway, if ever accepted, it would
take another 5y or so to be acceptable for qemu :)

-- 
Marc-André Lureau

[Qemu-devel] [RESEND PATCH V5 5/6] oslib-posix: add a configure switch to debug stack usage

[Peter Lieven]

this adds a knob to track the maximum stack usage of stacks
created by qemu_alloc_stack.

Signed-off-by: Peter Lieven 
Reviewed-by: Paolo Bonzini 
---
 configure  | 19 +++
 util/oslib-posix.c | 37 +
 2 files changed, 56 insertions(+)

diff --git a/configure b/configure
index 5ada56d..a7ee2f3 100755
--- a/configure
+++ b/configure
@@ -296,6 +296,7 @@ libiscsi=""
 libnfs=""
 coroutine=""
 coroutine_pool=""
+debug_stack_usage="no"
 seccomp=""
 glusterfs=""
 glusterfs_xlator_opt="no"
@@ -1005,6 +1006,8 @@ for opt do
   ;;
   --enable-coroutine-pool) coroutine_pool="yes"
   ;;
+  --enable-debug-stack-usage) debug_stack_usage="yes"
+  ;;
   --disable-docs) docs="no"
   ;;
   --enable-docs) docs="yes"
@@ -4302,6 +4305,17 @@ if test "$coroutine" = "gthread" -a "$coroutine_pool" = 
"yes"; then
   error_exit "'gthread' coroutine backend does not support pool (use 
--disable-coroutine-pool)"
 fi
 
+if test "$debug_stack_usage" = "yes"; then
+  if test "$cpu" = "ia64" -o "$cpu" = "hppa"; then
+error_exit "stack usage debugging is not supported for $cpu"
+  fi
+  if test "$coroutine_pool" = "yes"; then
+echo "WARN: disabling coroutine pool for stack usage debugging"
+coroutine_pool=no
+  fi
+fi
+
+
 ##
 # check if we have open_by_handle_at
 
@@ -4879,6 +4893,7 @@ echo "QGA MSI support   $guest_agent_msi"
 echo "seccomp support   $seccomp"
 echo "coroutine backend $coroutine"
 echo "coroutine pool$coroutine_pool"
+echo "debug stack usage $debug_stack_usage"
 echo "GlusterFS support $glusterfs"
 echo "Archipelago support $archipelago"
 echo "gcov  $gcov_tool"
@@ -5347,6 +5362,10 @@ else
   echo "CONFIG_COROUTINE_POOL=0" >> $config_host_mak
 fi
 
+if test "$debug_stack_usage" = "yes" ; then
+  echo "CONFIG_DEBUG_STACK_USAGE=y" >> $config_host_mak
+fi
+
 if test "$open_by_handle_at" = "yes" ; then
   echo "CONFIG_OPEN_BY_HANDLE=y" >> $config_host_mak
 fi
diff --git a/util/oslib-posix.c b/util/oslib-posix.c
index 2303ca6..e818d38 100644
--- a/util/oslib-posix.c
+++ b/util/oslib-posix.c
@@ -50,6 +50,10 @@
 
 #include 
 
+#ifdef CONFIG_DEBUG_STACK_USAGE
+#include "qemu/error-report.h"
+#endif
+
 int qemu_get_thread_id(void)
 {
 #if defined(__linux__)
@@ -512,6 +516,9 @@ static size_t adjust_stack_size(size_t sz)
 void *qemu_alloc_stack(size_t sz)
 {
 void *ptr, *guardpage;
+#ifdef CONFIG_DEBUG_STACK_USAGE
+void *ptr2;
+#endif
 size_t pagesz = getpagesize();
 sz = adjust_stack_size(sz);
 
@@ -535,11 +542,41 @@ void *qemu_alloc_stack(size_t sz)
 abort();
 }
 
+#ifdef CONFIG_DEBUG_STACK_USAGE
+for (ptr2 = ptr + pagesz; ptr2 < ptr + sz; ptr2 += sizeof(uint32_t)) {
+*(uint32_t *)ptr2 = 0xdeadbeaf;
+}
+#endif
+
 return ptr;
 }
 
+#ifdef CONFIG_DEBUG_STACK_USAGE
+static __thread unsigned int max_stack_usage;
+#endif
+
 void qemu_free_stack(void *stack, size_t sz)
 {
+#ifdef CONFIG_DEBUG_STACK_USAGE
+unsigned int usage;
+void *ptr;
+#endif
 sz = adjust_stack_size(sz);
+
+#ifdef CONFIG_DEBUG_STACK_USAGE
+for (ptr = stack + getpagesize(); ptr < stack + sz;
+ ptr += sizeof(uint32_t)) {
+if (*(uint32_t *)ptr != 0xdeadbeaf) {
+break;
+}
+}
+usage = sz - (uintptr_t) (ptr - stack);
+if (usage > max_stack_usage) {
+error_report("thread %d max stack usage increased from %u to %u",
+ qemu_get_thread_id(), max_stack_usage, usage);
+max_stack_usage = usage;
+}
+#endif
+
 munmap(stack, sz);
 }
-- 
1.9.1

[Qemu-devel] [RESEND PATCH V5 1/6] oslib-posix: add helpers for stack alloc and free

[Peter Lieven]

the allocated stack will be adjusted to the minimum supported stack size
by the OS and rounded up to be a multiple of the system pagesize.
Additionally an architecture dependent guard page is added to the stack
to catch stack overflows.

Signed-off-by: Peter Lieven 
Reviewed-by: Eric Blake 
---
 include/sysemu/os-posix.h | 23 +++
 util/oslib-posix.c| 46 ++
 2 files changed, 69 insertions(+)

diff --git a/include/sysemu/os-posix.h b/include/sysemu/os-posix.h
index 9c7dfdf..7630665 100644
--- a/include/sysemu/os-posix.h
+++ b/include/sysemu/os-posix.h
@@ -60,4 +60,27 @@ int qemu_utimens(const char *path, const qemu_timespec 
*times);
 
 bool is_daemonized(void);
 
+/**
+ * qemu_alloc_stack:
+ * @sz: size of required stack in bytes
+ *
+ * Allocate memory that can be used as a stack, for instance for
+ * coroutines. If the memory cannot be allocated, this function
+ * will abort (like g_malloc()).
+ *
+ * The allocated stack must be freed with qemu_free_stack().
+ *
+ * Returns: pointer to (the lowest address of) the stack memory.
+ */
+void *qemu_alloc_stack(size_t sz);
+
+/**
+ * qemu_free_stack:
+ * @stack: stack to free
+ * @sz: size of stack in bytes
+ *
+ * Free a stack allocated via qemu_alloc_stack().
+ */
+void qemu_free_stack(void *stack, size_t sz);
+
 #endif
diff --git a/util/oslib-posix.c b/util/oslib-posix.c
index e2e1d4d..2303ca6 100644
--- a/util/oslib-posix.c
+++ b/util/oslib-posix.c
@@ -497,3 +497,49 @@ pid_t qemu_fork(Error **errp)
 }
 return pid;
 }
+
+static size_t adjust_stack_size(size_t sz)
+{
+#ifdef _SC_THREAD_STACK_MIN
+/* avoid stacks smaller than _SC_THREAD_STACK_MIN */
+sz = MAX(MAX(sysconf(_SC_THREAD_STACK_MIN), 0), sz);
+#endif
+/* adjust stack size to a multiple of the page size */
+sz = ROUND_UP(sz, getpagesize());
+return sz;
+}
+
+void *qemu_alloc_stack(size_t sz)
+{
+void *ptr, *guardpage;
+size_t pagesz = getpagesize();
+sz = adjust_stack_size(sz);
+
+ptr = mmap(NULL, sz, PROT_READ | PROT_WRITE,
+   MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
+if (ptr == MAP_FAILED) {
+abort();
+}
+
+#if defined(HOST_IA64)
+/* separate register stack */
+guardpage = ptr + (((sz - pagesz) / 2) & ~pagesz);
+#elif defined(HOST_HPPA)
+/* stack grows up */
+guardpage = ptr + sz - pagesz;
+#else
+/* stack grows down */
+guardpage = ptr;
+#endif
+if (mprotect(guardpage, pagesz, PROT_NONE) != 0) {
+abort();
+}
+
+return ptr;
+}
+
+void qemu_free_stack(void *stack, size_t sz)
+{
+sz = adjust_stack_size(sz);
+munmap(stack, sz);
+}
-- 
1.9.1

[Qemu-devel] [RESEND PATCH V5 3/6] coroutine-ucontext: use helper for allocating stack memory

[Peter Lieven]

Signed-off-by: Peter Lieven 
Reviewed-by: Paolo Bonzini 
Reviewed-by: Richard Henderson 
---
 util/coroutine-ucontext.c | 9 -
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/util/coroutine-ucontext.c b/util/coroutine-ucontext.c
index 31254ab..b7dea8c 100644
--- a/util/coroutine-ucontext.c
+++ b/util/coroutine-ucontext.c
@@ -82,7 +82,6 @@ static void coroutine_trampoline(int i0, int i1)
 
 Coroutine *qemu_coroutine_new(void)
 {
-const size_t stack_size = COROUTINE_STACK_SIZE;
 CoroutineUContext *co;
 ucontext_t old_uc, uc;
 sigjmp_buf old_env;
@@ -101,17 +100,17 @@ Coroutine *qemu_coroutine_new(void)
 }
 
 co = g_malloc0(sizeof(*co));
-co->stack = g_malloc(stack_size);
+co->stack = qemu_alloc_stack(COROUTINE_STACK_SIZE);
 co->base.entry_arg = &old_env; /* stash away our jmp_buf */
 
 uc.uc_link = &old_uc;
 uc.uc_stack.ss_sp = co->stack;
-uc.uc_stack.ss_size = stack_size;
+uc.uc_stack.ss_size = COROUTINE_STACK_SIZE;
 uc.uc_stack.ss_flags = 0;
 
 #ifdef CONFIG_VALGRIND_H
 co->valgrind_stack_id =
-VALGRIND_STACK_REGISTER(co->stack, co->stack + stack_size);
+VALGRIND_STACK_REGISTER(co->stack, co->stack + COROUTINE_STACK_SIZE);
 #endif
 
 arg.p = co;
@@ -149,7 +148,7 @@ void qemu_coroutine_delete(Coroutine *co_)
 valgrind_stack_deregister(co);
 #endif
 
-g_free(co->stack);
+qemu_free_stack(co->stack, COROUTINE_STACK_SIZE);
 g_free(co);
 }
 
-- 
1.9.1

[Qemu-devel] [RESEND PATCH V5 4/6] coroutine-sigaltstack: use helper for allocating stack memory

[Peter Lieven]

Signed-off-by: Peter Lieven 
Reviewed-by: Paolo Bonzini 
Reviewed-by: Richard Henderson 
---
 util/coroutine-sigaltstack.c | 7 +++
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/util/coroutine-sigaltstack.c b/util/coroutine-sigaltstack.c
index 9c2854c..ccf4861 100644
--- a/util/coroutine-sigaltstack.c
+++ b/util/coroutine-sigaltstack.c
@@ -143,7 +143,6 @@ static void coroutine_trampoline(int signal)
 
 Coroutine *qemu_coroutine_new(void)
 {
-const size_t stack_size = COROUTINE_STACK_SIZE;
 CoroutineUContext *co;
 CoroutineThreadState *coTS;
 struct sigaction sa;
@@ -164,7 +163,7 @@ Coroutine *qemu_coroutine_new(void)
  */
 
 co = g_malloc0(sizeof(*co));
-co->stack = g_malloc(stack_size);
+co->stack = qemu_alloc_stack(COROUTINE_STACK_SIZE);
 co->base.entry_arg = &old_env; /* stash away our jmp_buf */
 
 coTS = coroutine_get_thread_state();
@@ -189,7 +188,7 @@ Coroutine *qemu_coroutine_new(void)
  * Set the new stack.
  */
 ss.ss_sp = co->stack;
-ss.ss_size = stack_size;
+ss.ss_size = COROUTINE_STACK_SIZE;
 ss.ss_flags = 0;
 if (sigaltstack(&ss, &oss) < 0) {
 abort();
@@ -253,7 +252,7 @@ void qemu_coroutine_delete(Coroutine *co_)
 {
 CoroutineUContext *co = DO_UPCAST(CoroutineUContext, base, co_);
 
-g_free(co->stack);
+qemu_free_stack(co->stack, COROUTINE_STACK_SIZE);
 g_free(co);
 }
 
-- 
1.9.1

[Qemu-devel] [RESEND PATCH V5 6/6] coroutine: reduce stack size to 64kB

[Peter Lieven]

evaluation with the recently introduced maximum stack usage monitoring revealed
that the actual used stack size was never above 4kB so allocating 1MB stack
for each coroutine is a lot of wasted memory. So reduce the stack size to
64kB which should still give enough head room. The guard page added
in qemu_alloc_stack will catch a potential stack overflow introduced
by this commit.

Signed-off-by: Peter Lieven 
Reviewed-by: Eric Blake 
---
 include/qemu/coroutine_int.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/qemu/coroutine_int.h b/include/qemu/coroutine_int.h
index eac323a..f84d777 100644
--- a/include/qemu/coroutine_int.h
+++ b/include/qemu/coroutine_int.h
@@ -28,7 +28,7 @@
 #include "qemu/queue.h"
 #include "qemu/coroutine.h"
 
-#define COROUTINE_STACK_SIZE (1 << 20)
+#define COROUTINE_STACK_SIZE (1 << 16)
 
 typedef enum {
 COROUTINE_YIELD = 1,
-- 
1.9.1