Re: [Qemu-devel] [PATCH v2 0/1] iotests: fix test case 185

[no-reply]

Hi,

This series failed docker-quick@centos6 build test. Please find the testing 
commands and
their output below. If you have Docker installed, you can probably reproduce it
locally.

Type: series
Message-id: 20180323034356.72130-1-ha...@linux.vnet.ibm.com
Subject: [Qemu-devel] [PATCH v2 0/1] iotests: fix test case 185

=== TEST SCRIPT BEGIN ===
#!/bin/bash
set -e
git submodule update --init dtc
# Let docker tests dump environment info
export SHOW_ENV=1
export J=8
time make docker-test-quick@centos6
=== TEST SCRIPT END ===

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
Switched to a new branch 'test'
808f283fa6 iotests: fix test case 185

=== OUTPUT BEGIN ===
Submodule 'dtc' (git://git.qemu-project.org/dtc.git) registered for path 'dtc'
Cloning into '/var/tmp/patchew-tester-tmp-bbhg23ul/src/dtc'...
Submodule path 'dtc': checked out 'e54388015af1fb4bf04d0bca99caba1074d9cc42'
  BUILD   centos6
make[1]: Entering directory '/var/tmp/patchew-tester-tmp-bbhg23ul/src'
  GEN 
/var/tmp/patchew-tester-tmp-bbhg23ul/src/docker-src.2018-03-23-00.37.47.16115/qemu.tar
Cloning into 
'/var/tmp/patchew-tester-tmp-bbhg23ul/src/docker-src.2018-03-23-00.37.47.16115/qemu.tar.vroot'...
done.
Checking out files:  45% (2739/6057)   
Checking out files:  46% (2787/6057)   
Checking out files:  47% (2847/6057)   
Checking out files:  48% (2908/6057)   
Checking out files:  49% (2968/6057)   
Checking out files:  50% (3029/6057)   
Checking out files:  51% (3090/6057)   
Checking out files:  52% (3150/6057)   
Checking out files:  53% (3211/6057)   
Checking out files:  54% (3271/6057)   
Checking out files:  55% (3332/6057)   
Checking out files:  56% (3392/6057)   
Checking out files:  57% (3453/6057)   
Checking out files:  58% (3514/6057)   
Checking out files:  59% (3574/6057)   
Checking out files:  60% (3635/6057)   
Checking out files:  61% (3695/6057)   
Checking out files:  62% (3756/6057)   
Checking out files:  63% (3816/6057)   
Checking out files:  64% (3877/6057)   
Checking out files:  65% (3938/6057)   
Checking out files:  66% (3998/6057)   
Checking out files:  67% (4059/6057)   
Checking out files:  68% (4119/6057)   
Checking out files:  69% (4180/6057)   
Checking out files:  70% (4240/6057)   
Checking out files:  71% (4301/6057)   
Checking out files:  72% (4362/6057)   
Checking out files:  73% (4422/6057)   
Checking out files:  74% (4483/6057)   
Checking out files:  75% (4543/6057)   
Checking out files:  76% (4604/6057)   
Checking out files:  77% (4664/6057)   
Checking out files:  78% (4725/6057)   
Checking out files:  79% (4786/6057)   
Checking out files:  80% (4846/6057)   
Checking out files:  81% (4907/6057)   
Checking out files:  82% (4967/6057)   
Checking out files:  83% (5028/6057)   
Checking out files:  84% (5088/6057)   
Checking out files:  85% (5149/6057)   
Checking out files:  86% (5210/6057)   
Checking out files:  87% (5270/6057)   
Checking out files:  88% (5331/6057)   
Checking out files:  89% (5391/6057)   
Checking out files:  90% (5452/6057)   
Checking out files:  91% (5512/6057)   
Checking out files:  92% (5573/6057)   
Checking out files:  93% (5634/6057)   
Checking out files:  94% (5694/6057)   
Checking out files:  95% (5755/6057)   
Checking out files:  96% (5815/6057)   
Checking out files:  97% (5876/6057)   
Checking out files:  98% (5936/6057)   
Checking out files:  99% (5997/6057)   
Checking out files: 100% (6057/6057)   
Checking out files: 100% (6057/6057), done.
Your branch is up-to-date with 'origin/test'.
Submodule 'dtc' (git://git.qemu-project.org/dtc.git) registered for path 'dtc'
Cloning into 
'/var/tmp/patchew-tester-tmp-bbhg23ul/src/docker-src.2018-03-23-00.37.47.16115/qemu.tar.vroot/dtc'...
Submodule path 'dtc': checked out 'e54388015af1fb4bf04d0bca99caba1074d9cc42'
Submodule 'ui/keycodemapdb' (git://git.qemu.org/keycodemapdb.git) registered 
for path 'ui/keycodemapdb'
Cloning into 
'/var/tmp/patchew-tester-tmp-bbhg23ul/src/docker-src.2018-03-23-00.37.47.16115/qemu.tar.vroot/ui/keycodemapdb'...
Submodule path 'ui/keycodemapdb': checked out 
'6b3d716e2b6472eb7189d3220552280ef3d832ce'
  COPYRUNNER
RUN test-quick in qemu:centos6 
Packages installed:
SDL-devel-1.2.14-7.el6_7.1.x86_64
bison-2.4.1-5.el6.x86_64
bzip2-devel-1.0.5-7.el6_0.x86_64
ccache-3.1.6-2.el6.x86_64
csnappy-devel-0-6.20150729gitd7bc683.el6.x86_64
flex-2.5.35-9.el6.x86_64
gcc-4.4.7-18.el6.x86_64
gettext-0.17-18.el6.x86_64
git-1.7.1-9.el6_9.x86_64
glib2-devel-2.28.8-9.el6.x86_64
libepoxy-devel-1.2-3.el6.x86_64
libfdt-devel-1.4.0-1.el6.x86_64
librdmacm-devel-1.0.21-0.el6.x86_64
lzo-devel-2.03-3.1.el6_5.1.x86_64
make-3.81-23.el6.x86_64
mesa-libEGL-devel-11.0.7-4.el6.x86_64
mesa-libgbm-devel-11.0.7-4.el6.x86_64
package g++ is not installed
pixman-devel-0.32.8-1.el6.x86_64
spice-glib-devel-0.26-8.el6.x86_64
spice-server-devel-0.12.4-16.el6.x86_64
tar-1.23-15.el6_8.x86_64
vte-devel-0.25.1-9.el6.x86_64
xen-devel-4.6.6-2.el6.x86_64
zlib-devel-1.2.3-29.el6.x86_64

Environment variab

Re: [Qemu-devel] [PATCH v8 19/23] qmp: isolate responses into io thread

[Peter Xu]

On Thu, Mar 22, 2018 at 01:00:19PM +0100, Marc-André Lureau wrote:
> Hi
> 
> On Fri, Mar 9, 2018 at 10:00 AM, Peter Xu  wrote:
> > For those monitors who have enabled IO thread, we'll offload the
> > responding procedure into IO thread.  The main reason is that chardev is
> > not thread safe, and we need to do all the read/write IOs in the same
> > thread.  For use_io_thr=true monitors, that thread is the IO thread.
> 
> Actually, the chr write path is suppose to be somewhat thread safe
> (chr_write_lock).
> 
> Secondly, the function responsible to write monitor data has some
> thread-safety, it's called monitor_flush_locked(), because you need
> the mon->out_lock.
> 
> I think that patch is making things more complicated than they need to
> be. You should be able to call monitor_json_emitter/monitor_puts()
> directly from any thread, it will queue the data, start writing, and
> add a watch if necessary in the appropriate context.
> 
> Am I missing something?

Yes there are the locks either in monitor code and chardev code to
protect write operations.  However I don't know enough to be sure of
its safety.  Considering that, having the whole chardev operations
separated into the other IOThread seems to be the only safe approach
to me for now.

Thanks,

-- 
Peter Xu

Re: [Qemu-devel] [PATCH v8 18/23] qmp: support out-of-band (oob) execution

[Peter Xu]

On Thu, Mar 22, 2018 at 11:22:14AM +0100, Marc-André Lureau wrote:
> Hi
> 
> On Fri, Mar 9, 2018 at 10:00 AM, Peter Xu  wrote:
> > Having "allow-oob" to true for a command does not mean that this command
> > will always be run in out-of-band mode.  The out-of-band quick path will
> > only be executed if we specify the extra "run-oob" flag when sending the
> > QMP request:
> >
> > { "execute":   "command-that-allows-oob",
> >   "arguments": { ... },
> >   "control":   { "run-oob": true } }
> >
> > The "control" key is introduced to store this extra flag.  "control"
> > field is used to store arguments that are shared by all the commands,
> > rather than command specific arguments.  Let "run-oob" be the first.
> >
> > Note that in the patch I exported qmp_dispatch_check_obj() to be used to
> > check the request earlier, and at the same time allowed "id" field to be
> > there since actually we always allow that.
> >
> > Reviewed-by: Stefan Hajnoczi 
> > Signed-off-by: Peter Xu 
> > ---
> >  include/qapi/qmp/dispatch.h |  2 ++
> >  monitor.c   | 84 
> > -
> >  qapi/qmp-dispatch.c | 33 +-
> >  trace-events|  2 ++
> >  4 files changed, 111 insertions(+), 10 deletions(-)
> >
> > diff --git a/include/qapi/qmp/dispatch.h b/include/qapi/qmp/dispatch.h
> > index 26eb13ff41..ffb4652f71 100644
> > --- a/include/qapi/qmp/dispatch.h
> > +++ b/include/qapi/qmp/dispatch.h
> > @@ -48,6 +48,8 @@ bool qmp_command_is_enabled(const QmpCommand *cmd);
> >  const char *qmp_command_name(const QmpCommand *cmd);
> >  bool qmp_has_success_response(const QmpCommand *cmd);
> >  QObject *qmp_build_error_object(Error *err);
> > +QDict *qmp_dispatch_check_obj(const QObject *request, Error **errp);
> > +bool qmp_is_oob(QDict *dict);
> >
> >  typedef void (*qmp_cmd_callback_fn)(QmpCommand *cmd, void *opaque);
> >
> > diff --git a/monitor.c b/monitor.c
> > index 4d57a8d341..5c8afe9f50 100644
> > --- a/monitor.c
> > +++ b/monitor.c
> > @@ -1110,6 +1110,44 @@ static void qmp_caps_apply(Monitor *mon, 
> > QMPCapabilityList *list)
> >  }
> >  }
> >
> > +/*
> > + * Return true if check successful, or false otherwise.  When false is
> > + * returned, detailed error will be in errp if provided.
> > + */
> > +static bool qmp_cmd_oob_check(Monitor *mon, QDict *req, Error **errp)
> > +{
> > +const char *command;
> > +QmpCommand *cmd;
> > +
> > +command = qdict_get_try_str(req, "execute");
> > +if (!command) {
> > +error_setg(errp, "Command field 'execute' missing");
> > +return false;
> > +}
> > +
> > +cmd = qmp_find_command(mon->qmp.commands, command);
> > +if (!cmd) {
> > +error_set(errp, ERROR_CLASS_COMMAND_NOT_FOUND,
> > +  "The command %s has not been found", command);
> > +return false;
> > +}
> > +
> > +if (qmp_is_oob(req)) {
> > +if (!qmp_oob_enabled(mon)) {
> > +error_setg(errp, "Please enable Out-Of-Band first "
> > +   "for the session during capabilities negociation");
> > +return false;
> > +}
> > +if (!(cmd->options & QCO_ALLOW_OOB)) {
> > +error_setg(errp, "The command %s does not support OOB",
> > +   command);
> > +return false;
> > +}
> > +}
> > +
> > +return true;
> > +}
> > +
> >  void qmp_qmp_capabilities(bool has_enable, QMPCapabilityList *enable,
> >Error **errp)
> >  {
> > @@ -4001,6 +4039,7 @@ static void monitor_qmp_bh_dispatcher(void *data)
> >  QMPRequest *req_obj = monitor_qmp_requests_pop_one();
> >
> >  if (req_obj) {
> > +trace_monitor_qmp_cmd_in_band(qobject_get_try_str(req_obj->id) ?: 
> > "");
> >  monitor_qmp_dispatch_one(req_obj);
> >  /* Reschedule instead of looping so the main loop stays responsive 
> > */
> >  qemu_bh_schedule(mon_global.qmp_dispatcher_bh);
> > @@ -4024,17 +4063,31 @@ static void handle_qmp_command(JSONMessageParser 
> > *parser, GQueue *tokens)
> >  error_setg(&err, QERR_JSON_PARSING);
> >  }
> >  if (err) {
> > -monitor_qmp_respond(mon, NULL, err, NULL);
> > -qobject_decref(req);
> > -return;
> > +goto err;
> > +}
> > +
> > +/* Check against the request in general layout */
> > +qdict = qmp_dispatch_check_obj(req, &err);
> > +if (!qdict) {
> > +goto err;
> >  }
> >
> > -qdict = qobject_to_qdict(req);
> > -if (qdict) {
> > -id = qdict_get(qdict, "id");
> > -qobject_incref(id);
> > -qdict_del(qdict, "id");
> > -} /* else will fail qmp_dispatch() */
> > +/* Check against OOB specific */
> > +if (!qmp_cmd_oob_check(mon, qdict, &err)) {
> > +goto err;
> > +}
> 
> This check happens before monitor_qmp_dispatch_one(), where there is
> now dead code for COMMAND_NOT_FOUND error handli

[Qemu-devel] [PATCH] target-arm: Check undefined opcodes for SWP in A32 decoder

[Onur Sahin]

Hi all,

I have noticed that the decoding part in ARM/A32 does not verify the
opcodes for SWP instructions. The opcode field ([23:20]) for SWP
instructions should be 0 or 4, and QEMU does not check against these
values.

Other opcode values less than 8 are Undefined within the encoding
space of sychronization primitives (e.g., SWP, LDREX*). See section
A5.2.10 of ARMv7-A manual for reference. Because of the missing opcode
check, QEMU happily executes these Undefined cases as a SWP instruction.

The following fix adds proper opcode checks before assuming a valid SWP.

Best,
Onur

Signed-off-by: Onur Sahin 
---
 target-arm/translate.c | 5 -
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/target-arm/translate.c b/target-arm/translate.c
index bd5d5cb..fb31c12 100644
--- a/target-arm/translate.c
+++ b/target-arm/translate.c
@@ -8831,7 +8831,7 @@ static void disas_arm_insn(DisasContext *s, unsigned int 
insn)
 }
 }
 tcg_temp_free_i32(addr);
-} else {
+} else if (!(insn & 0x00B0)) {
 /* SWP instruction */
 rm = (insn) & 0xf;
 
@@ -8852,6 +8852,9 @@ static void disas_arm_insn(DisasContext *s, unsigned int 
insn)
 tcg_temp_free_i32(addr);
 store_reg(s, rd, tmp2);
 }
+else {
+goto illegal_op;
+}
 }
 } else {
 int address_offset;
-- 
1.8.3.1

Re: [Qemu-devel] [Bug 1754542] Re: colo: vm crash with segmentation fault

[Li Zhijian]

On 03/21/2018 02:04 PM, Zhang Chen wrote:

Hi Suiheng,

I made a new guest image and retest it, and got the same bug from latest branch.
I found that after the COLO checkpoint begin, the secondary guest always send
reset request to Qemu like someone still push the reset button in the guest.
And this bug occurred in COLO frame related codes. This part of codes wrote
by Li zhijian and Zhang hailiang and currently maintained by Zhang hailiang.
So, I add them to this thread.

CC Zhijian and Hailiang:
Any idea or comments about this bug?


One clue is the memory of SVM not is same with PVM.
we can try to compare the memory after checkpoint, i had a draft patch to do 
this before.


Thanks






If you want to test COLO currently, you can try the old version of COLO:
https://github.com/zhangckid/qemu/tree/qemu-colo-18mar10-legacy


Thanks
Zhang Chen

On Mon, Mar 19, 2018 at 10:08 AM, 李穗恒 <1754...@bugs.launchpad.net 
> wrote:

Hi Zhang Chen，
I follow the https://wiki.qemu.org/Features/COLO 
, And Vm no crash.
But SVM rebooting constantly after print RESET, PVM normal startup.

Secondary:
{"timestamp": {"seconds": 1521421788, "microseconds": 541058}, "event": 
"RESUME"}
{"timestamp": {"seconds": 1521421808, "microseconds": 493484}, "event": 
"STOP"}
{"timestamp": {"seconds": 1521421808, "microseconds": 686466}, "event": 
"RESUME"}
{"timestamp": {"seconds": 1521421808, "microseconds": 696152}, "event": "RESET", 
"data": {"guest": true}}
{"timestamp": {"seconds": 1521421808, "microseconds": 740653}, "event": "RESET", 
"data": {"guest": true}}
{"timestamp": {"seconds": 1521421818, "microseconds": 74}, "event": 
"STOP"}
{"timestamp": {"seconds": 1521421818, "microseconds": 969883}, "event": 
"RESUME"}
{"timestamp": {"seconds": 1521421818, "microseconds": 979986}, "event": "RESET", 
"data": {"guest": true}}
{"timestamp": {"seconds": 1521421819, "microseconds": 22652}, "event": "RESET", 
"data": {"guest": true}}


The command(I run two VM in sample machine):

Primary:
sudo /home/lee/Documents/qemu/x86_64-softmmu/qemu-system-x86_64 -enable-kvm 
-boot c -m 2048 -smp 2 -qmp stdio  -name primary -cpu qemu64,+kvmclock -device 
piix3-usb-uhci -device usb-tablet \
    -netdev 
tap,id=hn0,vhost=off,script=/etc/qemu-ifup,downscript=/etc/qemu-ifdown -device 
rtl8139,id=e0,netdev=hn0 \
    -chardev socket,id=mirror0,host=192.168.0.33,port=9003,server,nowait \
    -chardev socket,id=compare1,host=192.168.0.33,port=9004,server,wait \
    -chardev socket,id=compare0,host=192.168.0.33,port=9001,server,nowait \
    -chardev socket,id=compare0-0,host=192.168.0.33,port=9001 \
    -chardev 
socket,id=compare_out,host=192.168.0.33,port=9005,server,nowait \
    -chardev socket,id=compare_out0,host=192.168.0.33,port=9005 \
    -object filter-mirror,id=m0,netdev=hn0,queue=tx,outdev=mirror0 \
    -object 
filter-redirector,netdev=hn0,id=redire0,queue=rx,indev=compare_out \
    -object 
filter-redirector,netdev=hn0,id=redire1,queue=rx,outdev=compare0 \
    -object iothread,id=iothread1 \
    -object 
colo-compare,id=comp0,primary_in=compare0-0,secondary_in=compare1,outdev=compare_out0,iothread=iothread1
 \
    -drive 
if=ide,id=colo-disk0,driver=quorum,read-pattern=fifo,vote-threshold=1,children.0.file.filename=/var/lib/libvirt/images/1.raw,children.0.driver=raw
 -S

Secondary:
sudo /home/lee/Documents/qemu/x86_64-softmmu/qemu-system-x86_64 -boot c -m 
2048 -smp 2 -qmp stdio  -name secondary -enable-kvm -cpu qemu64,+kvmclock \
    -device piix3-usb-uhci -device usb-tablet \
    -netdev 
tap,id=hn0,vhost=off,script=/etc/qemu-ifup,downscript=/etc/qemu-ifdown \
    -device rtl8139,netdev=hn0 \
    -chardev socket,id=red0,host=192.168.0.33,port=9003,reconnect=1 \
    -chardev socket,id=red1,host=192.168.0.33,port=9004,reconnect=1 \
    -object filter-redirector,id=f1,netdev=hn0,queue=tx,indev=red0 \
    -object filter-redirector,id=f2,netdev=hn0,queue=rx,outdev=red1 \
    -object filter-rewriter,id=rew0,netdev=hn0,queue=all \
    -drive 
if=none,id=colo-disk0,file.filename=/var/lib/libvirt/images/2.raw,driver=raw,node-name=node0
 \
    -drive 
if=ide,id=active-disk0,driver=replication,mode=secondary,file.driver=qcow2,top-id=active-disk0,file.file.filename=/mnt/ramfs/active_disk.img,file.backing.driver=qcow2,file.backing.file.filename=/mnt/ramfs/hidden_disk.img,file.backing.backing=colo-disk0
 \
    -incoming tcp:0:

Secondary:
  {'execute':'qmp_capabilities'}
  { 'execute': 'nbd-server-start',
    'arguments': {'addr': {'type': 'inet', 'data': {'host': '192.168.0.33', 
'port': '8889'} } }
  }
  {'execute': 'nbd-server-add', 'arguments': {'device': 'colo-disk0', 
'writable': true } }
  {'execute': 'trace-event-set-state', 'arguments': {'name': 'colo*', 
'enable': true} }


  

[Qemu-devel] [PATCH v2 0/1] iotests: fix test case 185

[QingFeng Hao]

Hi,
This patch is to fix iotest case 185 and based on the latest commit
d522e0bd18364 --- "gitmodules: Use the QEMU mirror of qemu-palcode".
Thanks!

Change Log (v2):
* Recover the change on the call to bdrv_drain_all but don't resume the
  yielded jobs according to Stefan Hajnoczi's comment.
* Change 185.out accordingly as job's status is changed as well.

Change Log (v1):
* Remove the call to bdrv_drain_all in vm_shutdown to make case 185 passed.

QingFeng Hao (1):
  iotests: fix test case 185

 blockjob.c | 10 +-
 include/block/blockjob.h   |  5 +
 tests/qemu-iotests/185.out | 11 +--
 3 files changed, 23 insertions(+), 3 deletions(-)

-- 
2.13.5

[Qemu-devel] [PATCH v2 1/1] iotests: fix test case 185

[QingFeng Hao]

Test case 185 failed since commit 4486e89c219 --- "vl: introduce vm_shutdown()".
It's because of the newly introduced function vm_shutdown calls bdrv_drain_all,
which is called later by bdrv_close_all. bdrv_drain_all resumes the jobs
that doubles the speed and offset is doubled.
Some jobs' status are changed as well.

The fix is to not resume the jobs that are already yielded and also change
185.out accordingly.

Suggested-by: Stefan Hajnoczi 
Signed-off-by: QingFeng Hao 
---
 blockjob.c | 10 +-
 include/block/blockjob.h   |  5 +
 tests/qemu-iotests/185.out | 11 +--
 3 files changed, 23 insertions(+), 3 deletions(-)

diff --git a/blockjob.c b/blockjob.c
index ef3ed69ff1..fa9838ac97 100644
--- a/blockjob.c
+++ b/blockjob.c
@@ -206,11 +206,16 @@ void block_job_txn_add_job(BlockJobTxn *txn, BlockJob 
*job)
 
 static void block_job_pause(BlockJob *job)
 {
-job->pause_count++;
+if (!job->yielded) {
+job->pause_count++;
+}
 }
 
 static void block_job_resume(BlockJob *job)
 {
+if (job->yielded) {
+return;
+}
 assert(job->pause_count > 0);
 job->pause_count--;
 if (job->pause_count) {
@@ -371,6 +376,7 @@ static void block_job_sleep_timer_cb(void *opaque)
 BlockJob *job = opaque;
 
 block_job_enter(job);
+job->yielded = false;
 }
 
 void block_job_start(BlockJob *job)
@@ -935,6 +941,7 @@ void *block_job_create(const char *job_id, const 
BlockJobDriver *driver,
 job->cb= cb;
 job->opaque= opaque;
 job->busy  = false;
+job->yielded   = false;
 job->paused= true;
 job->pause_count   = 1;
 job->refcnt= 1;
@@ -1034,6 +1041,7 @@ static void block_job_do_yield(BlockJob *job, uint64_t ns)
 timer_mod(&job->sleep_timer, ns);
 }
 job->busy = false;
+job->yielded = true;
 block_job_unlock();
 qemu_coroutine_yield();
 
diff --git a/include/block/blockjob.h b/include/block/blockjob.h
index fc645dac68..f8f208bbcf 100644
--- a/include/block/blockjob.h
+++ b/include/block/blockjob.h
@@ -99,6 +99,11 @@ typedef struct BlockJob {
 bool ready;
 
 /**
+ * Set to true when the job is yielded.
+ */
+bool yielded;
+
+/**
  * Set to true when the job has deferred work to the main loop.
  */
 bool deferred_to_main_loop;
diff --git a/tests/qemu-iotests/185.out b/tests/qemu-iotests/185.out
index 57eaf8d699..798282e196 100644
--- a/tests/qemu-iotests/185.out
+++ b/tests/qemu-iotests/185.out
@@ -7,6 +7,7 @@ Formatting 'TEST_DIR/t.IMGFMT.base', fmt=IMGFMT size=67108864
 
 === Creating backing chain ===
 
+{"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": 
"RESUME"}
 Formatting 'TEST_DIR/t.qcow2.mid', fmt=qcow2 size=67108864 
backing_file=TEST_DIR/t.qcow2.base backing_fmt=qcow2 cluster_size=65536 
lazy_refcounts=off refcount_bits=16
 {"return": {}}
 wrote 4194304/4194304 bytes at offset 0
@@ -25,23 +26,28 @@ Formatting 'TEST_DIR/t.qcow2', fmt=qcow2 size=67108864 
backing_file=TEST_DIR/t.q
 === Start active commit job and exit qemu ===
 
 {"return": {}}
+{"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": 
"RESUME"}
 {"return": {}}
 {"return": {}}
 {"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": 
"SHUTDOWN", "data": {"guest": false}}
-{"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": 
"BLOCK_JOB_CANCELLED", "data": {"device": "disk", "len": 4194304, "offset": 
4194304, "speed": 65536, "type": "commit"}}
+{"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": 
"BLOCK_JOB_READY", "data": {"device": "disk", "len": 4194304, "offset": 
4194304, "speed": 65536, "type": "commit"}}
+{"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": 
"BLOCK_JOB_COMPLETED", "data": {"device": "disk", "len": 4194304, "offset": 
4194304, "speed": 65536, "type": "commit"}}
 
 === Start mirror job and exit qemu ===
 
 {"return": {}}
+{"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": 
"RESUME"}
 Formatting 'TEST_DIR/t.qcow2.copy', fmt=qcow2 size=67108864 cluster_size=65536 
lazy_refcounts=off refcount_bits=16
 {"return": {}}
+{"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": 
"BLOCK_JOB_READY", "data": {"device": "disk", "len": 4194304, "offset": 
4194304, "speed": 65536, "type": "mirror"}}
 {"return": {}}
 {"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": 
"SHUTDOWN", "data": {"guest": false}}
-{"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": 
"BLOCK_JOB_CANCELLED", "data": {"device": "disk", "len": 4194304, "offset": 
4194304, "speed": 65536, "type": "mirror"}}
+{"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": 
"BLOCK_JOB_COMPLETED", "data": {"device": "disk", "len": 4194304, "offset": 
4194304, "speed": 65536, "type": "mirror"}}
 
 === Start backup job and exit qemu =

Re: [Qemu-devel] [PATCH 3/3] target/xtensa/import_core.sh: fix #include

[Philippe Mathieu-Daudé]

On 03/22/2018 03:09 PM, Max Filippov wrote:
> Change #include  to #include "xtensa-isa.h" in imported
> files to make references to local files consistent.
> 
> Signed-off-by: Max Filippov 

Reviewed-by: Philippe Mathieu-Daudé 

> ---
>  target/xtensa/import_core.sh | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/target/xtensa/import_core.sh b/target/xtensa/import_core.sh
> index 58a42987d853..af6c6104792d 100755
> --- a/target/xtensa/import_core.sh
> +++ b/target/xtensa/import_core.sh
> @@ -33,6 +33,7 @@ tar -xf "$OVERLAY" -O binutils/xtensa-modules.c | \
>  -e '/^uint32 \*bypass_entry(int i)/,/}/d' \
>  -e '/^#include "ansidecl.h"/d' \
>  -e '/^Slot_[a-zA-Z0-9_]\+_decode (const xtensa_insnbuf 
> insn)/,/^}/s/^  return 0;$/  return XTENSA_UNDEFINED;/' \
> +-e 's/#include /#include "xtensa-isa.h"/' \
>  > "$TARGET"/xtensa-modules.inc.c
>  
>  cat < "${TARGET}.c"
> 

Re: [Qemu-devel] [PATCH 1/3] target/xtensa: add .inc. to non-top level source file names

[Philippe Mathieu-Daudé]

On 03/22/2018 05:37 PM, Max Filippov wrote:
> On Thu, Mar 22, 2018 at 12:16 PM, Eric Blake  wrote:
>> On 03/22/2018 01:09 PM, Max Filippov wrote:
>>>
>>> Fix definitions of existing cores and core importing script.
>>
>> This mentions the script...
>> ...but only touches .c files.
> 
> Yeah, I initially did it in a single patch, but then split it into
> two. Will merge
> them back as suggested.
> 

squashed with #2 (import_core.sh):
Reviewed-by: Philippe Mathieu-Daudé 

Re: [Qemu-devel] [PATCH 4/4] Remove unnecessary variables for function return value

[David Gibson]

On Thu, Mar 22, 2018 at 05:12:26PM +0100, Laurent Vivier wrote:
> Re-run Coccinelle script scripts/coccinelle/return_directly.cocci
> 
> Signed-off-by: Laurent Vivier 
> ---
>  accel/tcg/translate-all.c  |  5 +-
>  block/quorum.c |  6 +--
>  hw/arm/exynos4210.c|  7 +--
>  hw/block/vhost-user-blk.c  |  5 +-
>  hw/hppa/dino.c |  5 +-
>  hw/misc/mos6522.c  |  6 +--
>  hw/net/ftgmac100.c |  5 +-
>  hw/ppc/pnv_lpc.c   | 14 +-
>  io/net-listener.c  |  6 +--
>  target/i386/hax-darwin.c   | 10 ++--
>  target/mips/dsp_helper.c   | 15 ++
>  target/xtensa/core-dc232b/xtensa-modules.c | 56 
> ++
>  target/xtensa/core-dc233c/xtensa-modules.c | 56 
> ++
>  target/xtensa/core-de212/xtensa-modules.c  | 48 +--
>  target/xtensa/core-fsf/xtensa-modules.c| 32 -
>  .../xtensa/core-sample_controller/xtensa-modules.c | 24 +++---
>  target/xtensa/translate.c  |  5 +-
>  tests/m48t59-test.c|  6 +--
>  tests/test-thread-pool.c   |  6 +--
>  util/uri.c |  5 +-
>  20 files changed, 75 insertions(+), 247 deletions(-)

ppc part

Acked-by: David Gibson 

> 
> diff --git a/accel/tcg/translate-all.c b/accel/tcg/translate-all.c
> index 5ad1b919bc..55d822d410 100644
> --- a/accel/tcg/translate-all.c
> +++ b/accel/tcg/translate-all.c
> @@ -644,11 +644,8 @@ static inline void *alloc_code_gen_buffer(void)
>  static inline void *alloc_code_gen_buffer(void)
>  {
>  size_t size = tcg_ctx->code_gen_buffer_size;
> -void *buf;
> -
> -buf = VirtualAlloc(NULL, size, MEM_RESERVE | MEM_COMMIT,
> +return VirtualAlloc(NULL, size, MEM_RESERVE | MEM_COMMIT,
>  PAGE_EXECUTE_READWRITE);
> -return buf;
>  }
>  #else
>  static inline void *alloc_code_gen_buffer(void)
> diff --git a/block/quorum.c b/block/quorum.c
> index 14333c18aa..304442ef65 100644
> --- a/block/quorum.c
> +++ b/block/quorum.c
> @@ -608,7 +608,7 @@ static void read_quorum_children_entry(void *opaque)
>  static int read_quorum_children(QuorumAIOCB *acb)
>  {
>  BDRVQuorumState *s = acb->bs->opaque;
> -int i, ret;
> +int i;
>  
>  acb->children_read = s->num_children;
>  for (i = 0; i < s->num_children; i++) {
> @@ -643,9 +643,7 @@ static int read_quorum_children(QuorumAIOCB *acb)
>  qemu_coroutine_yield();
>  }
>  
> -ret = acb->vote_ret;
> -
> -return ret;
> +return acb->vote_ret;
>  }
>  
>  static int read_fifo_child(QuorumAIOCB *acb)
> diff --git a/hw/arm/exynos4210.c b/hw/arm/exynos4210.c
> index 06f9d1ffa4..d5ce275b21 100644
> --- a/hw/arm/exynos4210.c
> +++ b/hw/arm/exynos4210.c
> @@ -156,12 +156,7 @@ void exynos4210_write_secondary(ARMCPU *cpu,
>  
>  static uint64_t exynos4210_calc_affinity(int cpu)
>  {
> -uint64_t mp_affinity;
> -
> -/* Exynos4210 has 0x9 as cluster ID */
> -mp_affinity = (0x9 << ARM_AFF1_SHIFT) | cpu;
> -
> -return mp_affinity;
> +return (0x9 << ARM_AFF1_SHIFT) | cpu;
>  }
>  
>  Exynos4210State *exynos4210_init(MemoryRegion *system_mem)
> diff --git a/hw/block/vhost-user-blk.c b/hw/block/vhost-user-blk.c
> index f840f07dfe..3f41ca9e26 100644
> --- a/hw/block/vhost-user-blk.c
> +++ b/hw/block/vhost-user-blk.c
> @@ -196,7 +196,6 @@ static uint64_t vhost_user_blk_get_features(VirtIODevice 
> *vdev,
>  Error **errp)
>  {
>  VHostUserBlk *s = VHOST_USER_BLK(vdev);
> -uint64_t get_features;
>  
>  /* Turn on pre-defined features */
>  virtio_add_feature(&features, VIRTIO_BLK_F_SEG_MAX);
> @@ -215,9 +214,7 @@ static uint64_t vhost_user_blk_get_features(VirtIODevice 
> *vdev,
>  virtio_add_feature(&features, VIRTIO_BLK_F_MQ);
>  }
>  
> -get_features = vhost_get_features(&s->dev, user_feature_bits, features);
> -
> -return get_features;
> +return vhost_get_features(&s->dev, user_feature_bits, features);
>  }
>  
>  static void vhost_user_blk_handle_output(VirtIODevice *vdev, VirtQueue *vq)
> diff --git a/hw/hppa/dino.c b/hw/hppa/dino.c
> index 15aefde09c..c5dcf3104d 100644
> --- a/hw/hppa/dino.c
> +++ b/hw/hppa/dino.c
> @@ -403,13 +403,10 @@ static void dino_set_irq(void *opaque, int irq, int 
> level)
>  static int dino_pci_map_irq(PCIDevice *d, int irq_num)
>  {
>  int slot = d->devfn >> 3;
> -int local_irq;
>  
>  assert(irq_num >= 0 && irq_num <= 3);
>  
> -local_irq = slot & 0x03;
> -
> -return local_irq;
> +return slot & 0x03;
>  }
>  
>  static void dino_set_timer_irq(void *opaque, int irq, int level

Re: [Qemu-devel] [PATCH for-2.12] hw/intc/arm_gicv3: Fix secure-GIC NS ICC_PMR and ICC_RPR accesses

[Philippe Mathieu-Daudé]

On 03/15/2018 10:34 AM, Peter Maydell wrote:
> If the GIC has the security extension support enabled, then a
> non-secure access to ICC_PMR must take account of the non-secure
> view of interrupt priorities, where real priorities 0..0x7f
> are secure-only and not visible to the non-secure guest, and
> priorities 0x80..0xff are shown to the guest as if they were
> 0x00..0xff. We had the logic here wrong:
>  * on reads, the priority is in the secure range if bit 7
>is clear, not if it is set
>  * on writes, we want to set bit 7, not mask everything else
> 
> Our ICC_RPR read code had the same error as ICC_PMR.
> 
> (Compare the GICv3 spec pseudocode functions ICC_RPR_EL1
> and ICC_PMR_EL1.)
> 
> Fixes: https://bugs.launchpad.net/qemu/+bug/1748434
> Signed-off-by: Peter Maydell 

Reviewed-by: Philippe Mathieu-Daudé 

> ---
>  hw/intc/arm_gicv3_cpuif.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/hw/intc/arm_gicv3_cpuif.c b/hw/intc/arm_gicv3_cpuif.c
> index 5cbafaf497..26f5eeda94 100644
> --- a/hw/intc/arm_gicv3_cpuif.c
> +++ b/hw/intc/arm_gicv3_cpuif.c
> @@ -836,7 +836,7 @@ static uint64_t icc_pmr_read(CPUARMState *env, const 
> ARMCPRegInfo *ri)
>  /* NS access and Group 0 is inaccessible to NS: return the
>   * NS view of the current priority
>   */
> -if (value & 0x80) {
> +if ((value & 0x80) == 0) {
>  /* Secure priorities not visible to NS */
>  value = 0;
>  } else if (value != 0xff) {
> @@ -871,7 +871,7 @@ static void icc_pmr_write(CPUARMState *env, const 
> ARMCPRegInfo *ri,
>  /* Current PMR in the secure range, don't allow NS to change it 
> */
>  return;
>  }
> -value = (value >> 1) & 0x80;
> +value = (value >> 1) | 0x80;
>  }
>  cs->icc_pmr_el1 = value;
>  gicv3_cpuif_update(cs);
> @@ -1609,7 +1609,7 @@ static uint64_t icc_rpr_read(CPUARMState *env, const 
> ARMCPRegInfo *ri)
>  if (arm_feature(env, ARM_FEATURE_EL3) &&
>  !arm_is_secure(env) && (env->cp15.scr_el3 & SCR_FIQ)) {
>  /* NS GIC access and Group 0 is inaccessible to NS */
> -if (prio & 0x80) {
> +if ((prio & 0x80) == 0) {
>  /* NS mustn't see priorities in the Secure half of the range */
>  prio = 0;
>  } else if (prio != 0xff) {
> 

Re: [Qemu-devel] [PATCH for 2.13 5/5] linux-user: cleanup main()

[Philippe Mathieu-Daudé]

On 03/22/2018 06:58 PM, Laurent Vivier wrote:
> move all prologue specific parts to
> prologue.inc.c in arch directory
> 
> Signed-off-by: Laurent Vivier 

Reviewed-by: Philippe Mathieu-Daudé 

> ---
>  linux-user/aarch64/prologue.inc.c|  21 ++
>  linux-user/alpha/prologue.inc.c  |   9 +
>  linux-user/arm/prologue.inc.c|  23 ++
>  linux-user/cris/prologue.inc.c   |  19 ++
>  linux-user/hppa/prologue.inc.c   |   8 +
>  linux-user/i386/prologue.inc.c   | 113 ++
>  linux-user/m68k/prologue.inc.c   |  26 +++
>  linux-user/main.c| 400 
> +--
>  linux-user/microblaze/prologue.inc.c |  35 +++
>  linux-user/mips/prologue.inc.c   |  25 +++
>  linux-user/mips64/prologue.inc.c |   1 +
>  linux-user/nios2/prologue.inc.c  |  29 +++
>  linux-user/openrisc/prologue.inc.c   |   9 +
>  linux-user/ppc/prologue.inc.c|  16 ++
>  linux-user/riscv/prologue.inc.c  |   4 +
>  linux-user/s390x/prologue.inc.c  |   8 +
>  linux-user/sh4/prologue.inc.c|   8 +
>  linux-user/sparc/prologue.inc.c  |  10 +
>  linux-user/sparc64/prologue.inc.c|   1 +
>  linux-user/tilegx/prologue.inc.c |  10 +
>  linux-user/x86_64/prologue.inc.c |   1 +
>  linux-user/xtensa/prologue.inc.c |   8 +
>  22 files changed, 385 insertions(+), 399 deletions(-)
>  create mode 100644 linux-user/aarch64/prologue.inc.c
>  create mode 100644 linux-user/alpha/prologue.inc.c
>  create mode 100644 linux-user/arm/prologue.inc.c
>  create mode 100644 linux-user/cris/prologue.inc.c
>  create mode 100644 linux-user/hppa/prologue.inc.c
>  create mode 100644 linux-user/i386/prologue.inc.c
>  create mode 100644 linux-user/m68k/prologue.inc.c
>  create mode 100644 linux-user/microblaze/prologue.inc.c
>  create mode 100644 linux-user/mips/prologue.inc.c
>  create mode 100644 linux-user/mips64/prologue.inc.c
>  create mode 100644 linux-user/nios2/prologue.inc.c
>  create mode 100644 linux-user/openrisc/prologue.inc.c
>  create mode 100644 linux-user/ppc/prologue.inc.c
>  create mode 100644 linux-user/riscv/prologue.inc.c
>  create mode 100644 linux-user/s390x/prologue.inc.c
>  create mode 100644 linux-user/sh4/prologue.inc.c
>  create mode 100644 linux-user/sparc/prologue.inc.c
>  create mode 100644 linux-user/sparc64/prologue.inc.c
>  create mode 100644 linux-user/tilegx/prologue.inc.c
>  create mode 100644 linux-user/x86_64/prologue.inc.c
>  create mode 100644 linux-user/xtensa/prologue.inc.c
> 
> diff --git a/linux-user/aarch64/prologue.inc.c 
> b/linux-user/aarch64/prologue.inc.c
> new file mode 100644
> index 00..5ffb50ae84
> --- /dev/null
> +++ b/linux-user/aarch64/prologue.inc.c
> @@ -0,0 +1,21 @@
> +{
> +int i;
> +
> +if (!(arm_feature(env, ARM_FEATURE_AARCH64))) {
> +fprintf(stderr,
> +"The selected ARM CPU does not support 64 bit mode\n");
> +exit(EXIT_FAILURE);
> +}
> +
> +for (i = 0; i < 31; i++) {
> +env->xregs[i] = regs->regs[i];
> +}
> +env->pc = regs->pc;
> +env->xregs[31] = regs->sp;
> +#ifdef TARGET_WORDS_BIGENDIAN
> +env->cp15.sctlr_el[1] |= SCTLR_E0E;
> +for (i = 1; i < 4; ++i) {
> +env->cp15.sctlr_el[i] |= SCTLR_EE;
> +}
> +#endif
> +}
> diff --git a/linux-user/alpha/prologue.inc.c b/linux-user/alpha/prologue.inc.c
> new file mode 100644
> index 00..e7a34c38cd
> --- /dev/null
> +++ b/linux-user/alpha/prologue.inc.c
> @@ -0,0 +1,9 @@
> +{
> +int i;
> +
> +for(i = 0; i < 28; i++) {
> +env->ir[i] = ((abi_ulong *)regs)[i];
> +}
> +env->ir[IR_SP] = regs->usp;
> +env->pc = regs->pc;
> +}
> diff --git a/linux-user/arm/prologue.inc.c b/linux-user/arm/prologue.inc.c
> new file mode 100644
> index 00..712f34fb4d
> --- /dev/null
> +++ b/linux-user/arm/prologue.inc.c
> @@ -0,0 +1,23 @@
> +{
> +int i;
> +cpsr_write(env, regs->uregs[16], CPSR_USER | CPSR_EXEC,
> +   CPSRWriteByInstr);
> +for(i = 0; i < 16; i++) {
> +env->regs[i] = regs->uregs[i];
> +}
> +#ifdef TARGET_WORDS_BIGENDIAN
> +/* Enable BE8.  */
> +if (EF_ARM_EABI_VERSION(info->elf_flags) >= EF_ARM_EABI_VER4
> +&& (info->elf_flags & EF_ARM_BE8)) {
> +env->uncached_cpsr |= CPSR_E;
> +env->cp15.sctlr_el[1] |= SCTLR_E0E;
> +} else {
> +env->cp15.sctlr_el[1] |= SCTLR_B;
> +}
> +#endif
> +}
> +
> +ts->stack_base = info->start_stack;
> +ts->heap_base = info->brk;
> +/* This will be filled in on the first SYS_HEAPINFO call.  */
> +ts->heap_limit = 0;
> diff --git a/linux-user/cris/prologue.inc.c b/linux-user/cris/prologue.inc.c
> new file mode 100644
> index 00..9bc35d0825
> --- /dev/null
> +++ b/linux-user/cris/prologue.inc.c
> @@ -0,0 +1,19 @@
> +{
> + env->

Re: [Qemu-devel] [Qemu-arm] [PATCH 4/4] Remove unnecessary variables for function return value

[Philippe Mathieu-Daudé]

Hi Laurent,

On 03/22/2018 01:12 PM, Laurent Vivier wrote:
> Re-run Coccinelle script scripts/coccinelle/return_directly.cocci
> 
> Signed-off-by: Laurent Vivier 
> ---
>  accel/tcg/translate-all.c  |  5 +-
>  block/quorum.c |  6 +--
>  hw/arm/exynos4210.c|  7 +--
>  hw/block/vhost-user-blk.c  |  5 +-
>  hw/hppa/dino.c |  5 +-
>  hw/misc/mos6522.c  |  6 +--
>  hw/net/ftgmac100.c |  5 +-
>  hw/ppc/pnv_lpc.c   | 14 +-
>  io/net-listener.c  |  6 +--
>  target/i386/hax-darwin.c   | 10 ++--
>  target/mips/dsp_helper.c   | 15 ++
>  target/xtensa/core-dc232b/xtensa-modules.c | 56 
> ++
>  target/xtensa/core-dc233c/xtensa-modules.c | 56 
> ++
>  target/xtensa/core-de212/xtensa-modules.c  | 48 +--
>  target/xtensa/core-fsf/xtensa-modules.c| 32 -
>  .../xtensa/core-sample_controller/xtensa-modules.c | 24 +++---
>  target/xtensa/translate.c  |  5 +-
>  tests/m48t59-test.c|  6 +--
>  tests/test-thread-pool.c   |  6 +--
>  util/uri.c |  5 +-
>  20 files changed, 75 insertions(+), 247 deletions(-)
> 
> diff --git a/accel/tcg/translate-all.c b/accel/tcg/translate-all.c
> index 5ad1b919bc..55d822d410 100644
> --- a/accel/tcg/translate-all.c
> +++ b/accel/tcg/translate-all.c
> @@ -644,11 +644,8 @@ static inline void *alloc_code_gen_buffer(void)
>  static inline void *alloc_code_gen_buffer(void)
>  {
>  size_t size = tcg_ctx->code_gen_buffer_size;
> -void *buf;
> -
> -buf = VirtualAlloc(NULL, size, MEM_RESERVE | MEM_COMMIT,
> +return VirtualAlloc(NULL, size, MEM_RESERVE | MEM_COMMIT,
>  PAGE_EXECUTE_READWRITE);
> -return buf;
>  }
>  #else
>  static inline void *alloc_code_gen_buffer(void)
> diff --git a/block/quorum.c b/block/quorum.c
> index 14333c18aa..304442ef65 100644
> --- a/block/quorum.c
> +++ b/block/quorum.c
> @@ -608,7 +608,7 @@ static void read_quorum_children_entry(void *opaque)
>  static int read_quorum_children(QuorumAIOCB *acb)
>  {
>  BDRVQuorumState *s = acb->bs->opaque;
> -int i, ret;
> +int i;
>  
>  acb->children_read = s->num_children;
>  for (i = 0; i < s->num_children; i++) {
> @@ -643,9 +643,7 @@ static int read_quorum_children(QuorumAIOCB *acb)
>  qemu_coroutine_yield();
>  }
>  
> -ret = acb->vote_ret;
> -
> -return ret;
> +return acb->vote_ret;
>  }
>  
>  static int read_fifo_child(QuorumAIOCB *acb)
> diff --git a/hw/arm/exynos4210.c b/hw/arm/exynos4210.c
> index 06f9d1ffa4..d5ce275b21 100644
> --- a/hw/arm/exynos4210.c
> +++ b/hw/arm/exynos4210.c
> @@ -156,12 +156,7 @@ void exynos4210_write_secondary(ARMCPU *cpu,
>  
>  static uint64_t exynos4210_calc_affinity(int cpu)
>  {
> -uint64_t mp_affinity;
> -
> -/* Exynos4210 has 0x9 as cluster ID */
> -mp_affinity = (0x9 << ARM_AFF1_SHIFT) | cpu;
> -
> -return mp_affinity;
> +return (0x9 << ARM_AFF1_SHIFT) | cpu;

You should run spatch with --keep-comments.

>  }
>  
>  Exynos4210State *exynos4210_init(MemoryRegion *system_mem)
> diff --git a/hw/block/vhost-user-blk.c b/hw/block/vhost-user-blk.c
> index f840f07dfe..3f41ca9e26 100644
> --- a/hw/block/vhost-user-blk.c
> +++ b/hw/block/vhost-user-blk.c
> @@ -196,7 +196,6 @@ static uint64_t vhost_user_blk_get_features(VirtIODevice 
> *vdev,
>  Error **errp)
>  {
>  VHostUserBlk *s = VHOST_USER_BLK(vdev);
> -uint64_t get_features;
>  
>  /* Turn on pre-defined features */
>  virtio_add_feature(&features, VIRTIO_BLK_F_SEG_MAX);
> @@ -215,9 +214,7 @@ static uint64_t vhost_user_blk_get_features(VirtIODevice 
> *vdev,
>  virtio_add_feature(&features, VIRTIO_BLK_F_MQ);
>  }
>  
> -get_features = vhost_get_features(&s->dev, user_feature_bits, features);
> -
> -return get_features;
> +return vhost_get_features(&s->dev, user_feature_bits, features);
>  }
>  
>  static void vhost_user_blk_handle_output(VirtIODevice *vdev, VirtQueue *vq)
> diff --git a/hw/hppa/dino.c b/hw/hppa/dino.c
> index 15aefde09c..c5dcf3104d 100644
> --- a/hw/hppa/dino.c
> +++ b/hw/hppa/dino.c
> @@ -403,13 +403,10 @@ static void dino_set_irq(void *opaque, int irq, int 
> level)
>  static int dino_pci_map_irq(PCIDevice *d, int irq_num)
>  {
>  int slot = d->devfn >> 3;
> -int local_irq;
>  
>  assert(irq_num >= 0 && irq_num <= 3);
>  
> -local_irq = slot & 0x03;
> -
> -return local_irq;
> +return slot & 0x03;
>  }
>  
>  static void dino_set_timer_irq(void *opaque, int irq, int 

Re: [Qemu-devel] [PATCH for 2.13 0/5] linux-user: move arch specific parts to arch directories

[no-reply]

Hi,

This series seems to have some coding style problems. See output below for
more information:

Type: series
Message-id: 20180322215833.7713-1-laur...@vivier.eu
Subject: [Qemu-devel] [PATCH for 2.13 0/5] linux-user: move arch specific parts 
to arch directories

=== TEST SCRIPT BEGIN ===
#!/bin/bash

BASE=base
n=1
total=$(git log --oneline $BASE.. | wc -l)
failed=0

git config --local diff.renamelimit 0
git config --local diff.renames True
git config --local diff.algorithm histogram

commits="$(git log --format=%H --reverse $BASE..)"
for c in $commits; do
echo "Checking PATCH $n/$total: $(git log -n 1 --format=%s $c)..."
if ! git show $c --format=email | ./scripts/checkpatch.pl --mailback -; then
failed=1
echo
fi
n=$((n+1))
done

exit $failed
=== TEST SCRIPT END ===

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
Switched to a new branch 'test'
586d8b371e linux-user: cleanup main()
4f63afd0b5 linux-user: cleanup cpu_loop()
aa2bebb6ad linux-user: define TARGET_ARCH_HAS_SETUP_FRAME
647d489924 linux-user: remove unneeded #ifdef in signal.c
ea340535fe linux-user: cleanup signal.c

=== OUTPUT BEGIN ===
Checking PATCH 1/5: linux-user: cleanup signal.c...
ERROR: code indent should never use tabs
#873: FILE: linux-user/arm/signal.inc.c:30:
+target_sigset_t  tuc_sigmask;^I/* mask last for extensibility */$

ERROR: code indent should never use tabs
#881: FILE: linux-user/arm/signal.inc.c:38:
+target_sigset_t  tuc_sigmask;^I/* mask last for extensibility */$

ERROR: open brace '{' following struct go on the same line
#921: FILE: linux-user/arm/signal.inc.c:78:
+struct sigframe_v1
+{

ERROR: spaces required around that '-' (ctx:VxV)
#923: FILE: linux-user/arm/signal.inc.c:80:
+abi_ulong extramask[TARGET_NSIG_WORDS-1];
  ^

ERROR: open brace '{' following struct go on the same line
#928: FILE: linux-user/arm/signal.inc.c:85:
+struct sigframe_v2
+{

ERROR: open brace '{' following struct go on the same line
#934: FILE: linux-user/arm/signal.inc.c:91:
+struct rt_sigframe_v1
+{

ERROR: open brace '{' following struct go on the same line
#943: FILE: linux-user/arm/signal.inc.c:100:
+struct rt_sigframe_v2
+{

WARNING: line over 80 characters
#954: FILE: linux-user/arm/signal.inc.c:111:
+#define SWI_SYS_SIGRETURN  (0xef00|(TARGET_NR_sigreturn + 
ARM_SYSCALL_BASE))

ERROR: code indent should never use tabs
#954: FILE: linux-user/arm/signal.inc.c:111:
+#define SWI_SYS_SIGRETURN^I(0xef00|(TARGET_NR_sigreturn + 
ARM_SYSCALL_BASE))$

ERROR: spaces required around that '|' (ctx:VxV)
#954: FILE: linux-user/arm/signal.inc.c:111:
+#define SWI_SYS_SIGRETURN  (0xef00|(TARGET_NR_sigreturn + 
ARM_SYSCALL_BASE))
   ^

WARNING: line over 80 characters
#955: FILE: linux-user/arm/signal.inc.c:112:
+#define SWI_SYS_RT_SIGRETURN   (0xef00|(TARGET_NR_rt_sigreturn + 
ARM_SYSCALL_BASE))

ERROR: code indent should never use tabs
#955: FILE: linux-user/arm/signal.inc.c:112:
+#define SWI_SYS_RT_SIGRETURN^I(0xef00|(TARGET_NR_rt_sigreturn + 
ARM_SYSCALL_BASE))$

ERROR: spaces required around that '|' (ctx:VxV)
#955: FILE: linux-user/arm/signal.inc.c:112:
+#define SWI_SYS_RT_SIGRETURN   (0xef00|(TARGET_NR_rt_sigreturn + 
ARM_SYSCALL_BASE))
   ^

ERROR: code indent should never use tabs
#961: FILE: linux-user/arm/signal.inc.c:118:
+#define SWI_THUMB_SIGRETURN^I(0xdf00 << 16 | 0x2700 | (TARGET_NR_sigreturn))$

WARNING: line over 80 characters
#962: FILE: linux-user/arm/signal.inc.c:119:
+#define SWI_THUMB_RT_SIGRETURN (0xdf00 << 16 | 0x2700 | 
(TARGET_NR_rt_sigreturn))

ERROR: code indent should never use tabs
#962: FILE: linux-user/arm/signal.inc.c:119:
+#define SWI_THUMB_RT_SIGRETURN^I(0xdf00 << 16 | 0x2700 | 
(TARGET_NR_rt_sigreturn))$

ERROR: code indent should never use tabs
#965: FILE: linux-user/arm/signal.inc.c:122:
+^ISWI_SYS_SIGRETURN,^ISWI_THUMB_SIGRETURN,$

ERROR: code indent should never use tabs
#966: FILE: linux-user/arm/signal.inc.c:123:
+^ISWI_SYS_RT_SIGRETURN,^ISWI_THUMB_RT_SIGRETURN$

ERROR: "(foo*)" should be "(foo *)"
#1073: FILE: linux-user/arm/signal.inc.c:230:
+return (abi_ulong*)(vfpframe+1);

ERROR: spaces required around that '+' (ctx:VxV)
#1073: FILE: linux-user/arm/signal.inc.c:230:
+return (abi_ulong*)(vfpframe+1);
 ^

ERROR: "(foo*)" should be "(foo *)"
#1093: FILE: linux-user/arm/signal.inc.c:250:
+return (abi_ulong*)(iwmmxtframe+1);

ERROR: spaces required around that '+' (ctx:VxV)
#1093: FILE: linux-user/arm/signal.inc.c:250:
+return (abi_ulong*)(iwmmxtframe+1);
^

ERROR: space required before the open parenthesis '('
#1125: FILE: linux-user/arm/signal.inc.c:282:
+for(i = 0; i < TARGET_NSIG_WORDS; i++) {

ERROR: space required before the open parenthesis '('
#1145: FILE: linux-user/arm/signal.inc.c:302:
+for(i = 1; i < TARGET_NSIG_W

Re: [Qemu-devel] [PATCH v3] RISC-V: Fix riscv_isa_string memory size bug

[DJ Delorie]

"Richard W.M. Jones"  writes:
> DJ, am I remembering correctly that you tried the test case on the
> HiFive evaluation board and it didn't demonstrate the bug?

I tested it on the vc707 board, without seeing the bug.

I can test other test cases if needed, I've got the board running Fedora
at the moment.

Re: [Qemu-devel] [PATCH v3] RISC-V: Fix riscv_isa_string memory size bug

[Michael Clark]

On Thu, Mar 22, 2018 at 3:17 PM, DJ Delorie  wrote:

> "Richard W.M. Jones"  writes:
> > DJ, am I remembering correctly that you tried the test case on the
> > HiFive evaluation board and it didn't demonstrate the bug?
>
> I tested it on the vc707 board, without seeing the bug.
>
> I can test other test cases if needed, I've got the board running Fedora
> at the moment.
>

Okay, I'll assume its a QEMU RISC-V bug.

Re: [Qemu-devel] [PATCH v3] RISC-V: Fix riscv_isa_string memory size bug

[Richard W.M. Jones]

On Thu, Mar 22, 2018 at 02:06:24PM -0700, Michael Clark wrote:
> On Mon, Mar 19, 2018 at 2:39 PM, Michael Clark  wrote:
> > On Mon, Mar 19, 2018 at 12:42 PM, Richard W.M. Jones 
> > wrote:
> >> Did you see the problem with restoring floating point registers on
> >> context switch?  The test case is quite simple:
> >>
> >>   http://oirase.annexia.org/tmp/sched.c
> >
> > No I didn't. Thanks.
> From my local testing, it appears to only show up with SMP enabled. With 1
> CPU enabled the sched.c test passes.
> I'll could try and revert this and see if it makes any difference...
> however I think the float code has changed substantially with respect to
> handling of flags. We could backport the more conservative approach of
> handling MSTATUS_FS in the float routines. We should also run the test case
> on hardware to rule out linux-kernel MSTATUS_FS bugs...

DJ, am I remembering correctly that you tried the test case on the
HiFive evaluation board and it didn't demonstrate the bug?

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
libguestfs lets you edit virtual machines.  Supports shell scripting,
bindings from many languages.  http://libguestfs.org

[Qemu-devel] [PATCH for 2.13 3/5] linux-user: define TARGET_ARCH_HAS_SETUP_FRAME

[Laurent Vivier]

instead of calling setup_frame() conditionnaly to a list of knowm targets,
define TARGET_ARCH_HAS_SETUP_FRAME if the target provides the function
and call it only if the macro is defined

Signed-off-by: Laurent Vivier 
---
 linux-user/aarch64/signal.inc.c|  1 +
 linux-user/alpha/signal.inc.c  |  1 +
 linux-user/arm/signal.inc.c|  1 +
 linux-user/cris/signal.inc.c   |  1 +
 linux-user/i386/signal.inc.c   |  1 +
 linux-user/m68k/signal.inc.c   |  1 +
 linux-user/microblaze/signal.inc.c |  1 +
 linux-user/mips/signal.inc.c   |  1 +
 linux-user/ppc/signal.inc.c|  1 +
 linux-user/s390x/signal.inc.c  |  1 +
 linux-user/sh4/signal.inc.c|  1 +
 linux-user/signal.c| 11 +++
 linux-user/sparc/signal.inc.c  |  1 +
 13 files changed, 15 insertions(+), 8 deletions(-)

diff --git a/linux-user/aarch64/signal.inc.c b/linux-user/aarch64/signal.inc.c
index 28fa0f2f22..369741eae1 100644
--- a/linux-user/aarch64/signal.inc.c
+++ b/linux-user/aarch64/signal.inc.c
@@ -510,6 +510,7 @@ static void setup_rt_frame(int sig, struct target_sigaction 
*ka,
 target_setup_frame(sig, ka, info, set, env);
 }
 
+#define TARGET_ARCH_HAS_SETUP_FRAME
 static void setup_frame(int sig, struct target_sigaction *ka,
 target_sigset_t *set, CPUARMState *env)
 {
diff --git a/linux-user/alpha/signal.inc.c b/linux-user/alpha/signal.inc.c
index 52e379e214..5910a70ac6 100644
--- a/linux-user/alpha/signal.inc.c
+++ b/linux-user/alpha/signal.inc.c
@@ -103,6 +103,7 @@ static inline abi_ulong get_sigframe(struct 
target_sigaction *sa,
 return (sp - framesize) & -32;
 }
 
+#define TARGET_ARCH_HAS_SETUP_FRAME
 static void setup_frame(int sig, struct target_sigaction *ka,
 target_sigset_t *set, CPUAlphaState *env)
 {
diff --git a/linux-user/arm/signal.inc.c b/linux-user/arm/signal.inc.c
index 1d6f9a1f7f..ecca4f2e7b 100644
--- a/linux-user/arm/signal.inc.c
+++ b/linux-user/arm/signal.inc.c
@@ -334,6 +334,7 @@ sigsegv:
 force_sigsegv(usig);
 }
 
+#define TARGET_ARCH_HAS_SETUP_FRAME
 static void setup_frame(int usig, struct target_sigaction *ka,
 target_sigset_t *set, CPUARMState *regs)
 {
diff --git a/linux-user/cris/signal.inc.c b/linux-user/cris/signal.inc.c
index 3f68793727..d910eb5257 100644
--- a/linux-user/cris/signal.inc.c
+++ b/linux-user/cris/signal.inc.c
@@ -74,6 +74,7 @@ static abi_ulong get_sigframe(CPUCRISState *env, int 
framesize)
 return sp - framesize;
 }
 
+#define TARGET_ARCH_HAS_SETUP_FRAME
 static void setup_frame(int sig, struct target_sigaction *ka,
 target_sigset_t *set, CPUCRISState *env)
 {
diff --git a/linux-user/i386/signal.inc.c b/linux-user/i386/signal.inc.c
index c522c449ae..8695806da9 100644
--- a/linux-user/i386/signal.inc.c
+++ b/linux-user/i386/signal.inc.c
@@ -289,6 +289,7 @@ get_sigframe(struct target_sigaction *ka, CPUX86State *env, 
size_t frame_size)
 
 #ifndef TARGET_X86_64
 /* compare linux/arch/i386/kernel/signal.c:setup_frame() */
+#define TARGET_ARCH_HAS_SETUP_FRAME
 static void setup_frame(int sig, struct target_sigaction *ka,
 target_sigset_t *set, CPUX86State *env)
 {
diff --git a/linux-user/m68k/signal.inc.c b/linux-user/m68k/signal.inc.c
index 07db1cdeda..eabd4849e4 100644
--- a/linux-user/m68k/signal.inc.c
+++ b/linux-user/m68k/signal.inc.c
@@ -106,6 +106,7 @@ get_sigframe(struct target_sigaction *ka, CPUM68KState 
*regs,
 return ((sp - frame_size) & -8UL);
 }
 
+#define TARGET_ARCH_HAS_SETUP_FRAME
 static void setup_frame(int sig, struct target_sigaction *ka,
 target_sigset_t *set, CPUM68KState *env)
 {
diff --git a/linux-user/microblaze/signal.inc.c 
b/linux-user/microblaze/signal.inc.c
index 728bd33273..b0db5ab8c4 100644
--- a/linux-user/microblaze/signal.inc.c
+++ b/linux-user/microblaze/signal.inc.c
@@ -117,6 +117,7 @@ static abi_ulong get_sigframe(struct target_sigaction *ka,
 return ((sp - frame_size) & -8UL);
 }
 
+#define TARGET_ARCH_HAS_SETUP_FRAME
 static void setup_frame(int sig, struct target_sigaction *ka,
 target_sigset_t *set, CPUMBState *env)
 {
diff --git a/linux-user/mips/signal.inc.c b/linux-user/mips/signal.inc.c
index 19e5b3aad4..76e7ad77c2 100644
--- a/linux-user/mips/signal.inc.c
+++ b/linux-user/mips/signal.inc.c
@@ -185,6 +185,7 @@ static void mips_set_hflags_isa_mode_from_pc(CPUMIPSState 
*env)
 
 # if defined(TARGET_ABI_MIPSO32)
 /* compare linux/arch/mips/kernel/signal.c:setup_frame() */
+#define TARGET_ARCH_HAS_SETUP_FRAME
 static void setup_frame(int sig, struct target_sigaction * ka,
 target_sigset_t *set, CPUMIPSState *regs)
 {
diff --git a/linux-user/ppc/signal.inc.c b/linux-user/ppc/signal.inc.c
index ea3ee45202..1aaef0dbd8 100644
--- a/linux-user/ppc/signal.inc.c
+++ b/linux-user/ppc/signal.inc.c
@@ -411,6 +411,7 @@ static void restore_user_regs(CPUPPCState *env,
 }
 
 #if !defined(TARGET

[Qemu-devel] [PATCH for 2.13 5/5] linux-user: cleanup main()

[Laurent Vivier]

move all prologue specific parts to
prologue.inc.c in arch directory

Signed-off-by: Laurent Vivier 
---
 linux-user/aarch64/prologue.inc.c|  21 ++
 linux-user/alpha/prologue.inc.c  |   9 +
 linux-user/arm/prologue.inc.c|  23 ++
 linux-user/cris/prologue.inc.c   |  19 ++
 linux-user/hppa/prologue.inc.c   |   8 +
 linux-user/i386/prologue.inc.c   | 113 ++
 linux-user/m68k/prologue.inc.c   |  26 +++
 linux-user/main.c| 400 +--
 linux-user/microblaze/prologue.inc.c |  35 +++
 linux-user/mips/prologue.inc.c   |  25 +++
 linux-user/mips64/prologue.inc.c |   1 +
 linux-user/nios2/prologue.inc.c  |  29 +++
 linux-user/openrisc/prologue.inc.c   |   9 +
 linux-user/ppc/prologue.inc.c|  16 ++
 linux-user/riscv/prologue.inc.c  |   4 +
 linux-user/s390x/prologue.inc.c  |   8 +
 linux-user/sh4/prologue.inc.c|   8 +
 linux-user/sparc/prologue.inc.c  |  10 +
 linux-user/sparc64/prologue.inc.c|   1 +
 linux-user/tilegx/prologue.inc.c |  10 +
 linux-user/x86_64/prologue.inc.c |   1 +
 linux-user/xtensa/prologue.inc.c |   8 +
 22 files changed, 385 insertions(+), 399 deletions(-)
 create mode 100644 linux-user/aarch64/prologue.inc.c
 create mode 100644 linux-user/alpha/prologue.inc.c
 create mode 100644 linux-user/arm/prologue.inc.c
 create mode 100644 linux-user/cris/prologue.inc.c
 create mode 100644 linux-user/hppa/prologue.inc.c
 create mode 100644 linux-user/i386/prologue.inc.c
 create mode 100644 linux-user/m68k/prologue.inc.c
 create mode 100644 linux-user/microblaze/prologue.inc.c
 create mode 100644 linux-user/mips/prologue.inc.c
 create mode 100644 linux-user/mips64/prologue.inc.c
 create mode 100644 linux-user/nios2/prologue.inc.c
 create mode 100644 linux-user/openrisc/prologue.inc.c
 create mode 100644 linux-user/ppc/prologue.inc.c
 create mode 100644 linux-user/riscv/prologue.inc.c
 create mode 100644 linux-user/s390x/prologue.inc.c
 create mode 100644 linux-user/sh4/prologue.inc.c
 create mode 100644 linux-user/sparc/prologue.inc.c
 create mode 100644 linux-user/sparc64/prologue.inc.c
 create mode 100644 linux-user/tilegx/prologue.inc.c
 create mode 100644 linux-user/x86_64/prologue.inc.c
 create mode 100644 linux-user/xtensa/prologue.inc.c

diff --git a/linux-user/aarch64/prologue.inc.c 
b/linux-user/aarch64/prologue.inc.c
new file mode 100644
index 00..5ffb50ae84
--- /dev/null
+++ b/linux-user/aarch64/prologue.inc.c
@@ -0,0 +1,21 @@
+{
+int i;
+
+if (!(arm_feature(env, ARM_FEATURE_AARCH64))) {
+fprintf(stderr,
+"The selected ARM CPU does not support 64 bit mode\n");
+exit(EXIT_FAILURE);
+}
+
+for (i = 0; i < 31; i++) {
+env->xregs[i] = regs->regs[i];
+}
+env->pc = regs->pc;
+env->xregs[31] = regs->sp;
+#ifdef TARGET_WORDS_BIGENDIAN
+env->cp15.sctlr_el[1] |= SCTLR_E0E;
+for (i = 1; i < 4; ++i) {
+env->cp15.sctlr_el[i] |= SCTLR_EE;
+}
+#endif
+}
diff --git a/linux-user/alpha/prologue.inc.c b/linux-user/alpha/prologue.inc.c
new file mode 100644
index 00..e7a34c38cd
--- /dev/null
+++ b/linux-user/alpha/prologue.inc.c
@@ -0,0 +1,9 @@
+{
+int i;
+
+for(i = 0; i < 28; i++) {
+env->ir[i] = ((abi_ulong *)regs)[i];
+}
+env->ir[IR_SP] = regs->usp;
+env->pc = regs->pc;
+}
diff --git a/linux-user/arm/prologue.inc.c b/linux-user/arm/prologue.inc.c
new file mode 100644
index 00..712f34fb4d
--- /dev/null
+++ b/linux-user/arm/prologue.inc.c
@@ -0,0 +1,23 @@
+{
+int i;
+cpsr_write(env, regs->uregs[16], CPSR_USER | CPSR_EXEC,
+   CPSRWriteByInstr);
+for(i = 0; i < 16; i++) {
+env->regs[i] = regs->uregs[i];
+}
+#ifdef TARGET_WORDS_BIGENDIAN
+/* Enable BE8.  */
+if (EF_ARM_EABI_VERSION(info->elf_flags) >= EF_ARM_EABI_VER4
+&& (info->elf_flags & EF_ARM_BE8)) {
+env->uncached_cpsr |= CPSR_E;
+env->cp15.sctlr_el[1] |= SCTLR_E0E;
+} else {
+env->cp15.sctlr_el[1] |= SCTLR_B;
+}
+#endif
+}
+
+ts->stack_base = info->start_stack;
+ts->heap_base = info->brk;
+/* This will be filled in on the first SYS_HEAPINFO call.  */
+ts->heap_limit = 0;
diff --git a/linux-user/cris/prologue.inc.c b/linux-user/cris/prologue.inc.c
new file mode 100644
index 00..9bc35d0825
--- /dev/null
+++ b/linux-user/cris/prologue.inc.c
@@ -0,0 +1,19 @@
+{
+   env->regs[0] = regs->r0;
+   env->regs[1] = regs->r1;
+   env->regs[2] = regs->r2;
+   env->regs[3] = regs->r3;
+   env->regs[4] = regs->r4;
+   env->regs[5] = regs->r5;
+   env->regs[6] = regs->r6;
+   env->regs[7] = regs->r7;
+   env->regs[8] = regs->r8;
+   env->regs[9] = reg

[Qemu-devel] [PATCH for 2.13 0/5] linux-user: move arch specific parts to arch directories

[Laurent Vivier]

Some files like signal.c are really hard to read
because all architectures are mixed in the same
file.

This series moves from signal.c these parts to
the architecture dedicated directories in linux-user.
Moerover, this allows to compare easier functions
between architectures (it helps to debug problems).
Adding new functions for a new architecture will
be facilitated too.

As we are doing that for signal.c, we can also
do that for main.c, for the cpu loop part
and the cpu loop prologue too.

checkpatch.pl is not happy... but I only want to
move code from a file to another. I don't want
to change the content of the parts I move.

Laurent Vivier (5):
  linux-user: cleanup signal.c
  linux-user: remove unneeded #ifdef in signal.c
  linux-user: define TARGET_ARCH_HAS_SETUP_FRAME
  linux-user: cleanup cpu_loop()
  linux-user: cleanup main()

 linux-user/aarch64/cpu_loop.inc.c|1 +
 linux-user/aarch64/prologue.inc.c|   21 +
 linux-user/aarch64/signal.inc.c  |  557 +++
 linux-user/alpha/cpu_loop.inc.c  |  191 +
 linux-user/alpha/prologue.inc.c  |9 +
 linux-user/alpha/signal.inc.c|  259 ++
 linux-user/arm/cpu_loop.inc.c|  488 +++
 linux-user/arm/prologue.inc.c|   23 +
 linux-user/arm/signal.inc.c  |  750 
 linux-user/cris/cpu_loop.inc.c   |   67 +
 linux-user/cris/prologue.inc.c   |   19 +
 linux-user/cris/signal.inc.c |  167 +
 linux-user/hppa/cpu_loop.inc.c   |  178 +
 linux-user/hppa/prologue.inc.c   |8 +
 linux-user/hppa/signal.inc.c |  188 +
 linux-user/i386/cpu_loop.inc.c   |  229 ++
 linux-user/i386/prologue.inc.c   |  113 +
 linux-user/i386/signal.inc.c |  580 +++
 linux-user/m68k/cpu_loop.inc.c   |  115 +
 linux-user/m68k/prologue.inc.c   |   26 +
 linux-user/m68k/signal.inc.c |  407 +++
 linux-user/main.c| 4318 +-
 linux-user/microblaze/cpu_loop.inc.c |  116 +
 linux-user/microblaze/prologue.inc.c |   35 +
 linux-user/microblaze/signal.inc.c   |  227 ++
 linux-user/mips/cpu_loop.inc.c   |  695 
 linux-user/mips/prologue.inc.c   |   25 +
 linux-user/mips/signal.inc.c |  379 ++
 linux-user/mips64/cpu_loop.inc.c |1 +
 linux-user/mips64/prologue.inc.c |1 +
 linux-user/mips64/signal.inc.c   |1 +
 linux-user/nios2/cpu_loop.inc.c  |   98 +
 linux-user/nios2/prologue.inc.c  |   29 +
 linux-user/nios2/signal.inc.c|  232 ++
 linux-user/openrisc/cpu_loop.inc.c   |   81 +
 linux-user/openrisc/prologue.inc.c   |9 +
 linux-user/openrisc/signal.inc.c |  209 ++
 linux-user/ppc/cpu_loop.inc.c|  538 +++
 linux-user/ppc/prologue.inc.c|   16 +
 linux-user/ppc/signal.inc.c  |  668 
 linux-user/riscv/cpu_loop.inc.c  |   89 +
 linux-user/riscv/prologue.inc.c  |4 +
 linux-user/riscv/signal.inc.c|  196 +
 linux-user/s390x/cpu_loop.inc.c  |  132 +
 linux-user/s390x/prologue.inc.c  |8 +
 linux-user/s390x/signal.inc.c|  306 ++
 linux-user/sh4/cpu_loop.inc.c|   78 +
 linux-user/sh4/prologue.inc.c|8 +
 linux-user/sh4/signal.inc.c  |  328 ++
 linux-user/signal.c  | 6623 +-
 linux-user/sparc/cpu_loop.inc.c  |  271 ++
 linux-user/sparc/prologue.inc.c  |   10 +
 linux-user/sparc/signal.inc.c|  602 +++
 linux-user/sparc64/cpu_loop.inc.c|1 +
 linux-user/sparc64/prologue.inc.c|1 +
 linux-user/sparc64/signal.inc.c  |1 +
 linux-user/tilegx/cpu_loop.inc.c |  251 ++
 linux-user/tilegx/prologue.inc.c |   10 +
 linux-user/tilegx/signal.inc.c   |  163 +
 linux-user/x86_64/cpu_loop.inc.c |1 +
 linux-user/x86_64/prologue.inc.c |1 +
 linux-user/x86_64/signal.inc.c   |1 +
 linux-user/xtensa/cpu_loop.inc.c |  231 ++
 linux-user/xtensa/prologue.inc.c |8 +
 linux-user/xtensa/signal.inc.c   |  253 ++
 65 files changed, 10776 insertions(+), 10875 deletions(-)
 create mode 100644 linux-user/aarch64/cpu_loop.inc.c
 create mode 100644 linux-user/aarch64/prologue.inc.c
 create mode 100644 linux-user/aarch64/signal.inc.c
 create mode 100644 linux-user/alpha/cpu_loop.inc.c
 create mode 100644 linux-user/alpha/prologue.inc.c
 create mode 100644 linux-user/alpha/signal.inc.c
 create mode 100644 linux-user/arm/cpu_loop.inc.c
 create mode 100644 linux-user/arm/prologue.inc.c
 create mode 100644 linux-user/arm/signal.inc.c
 create mode 100644 linux-user/cris/cpu_loop.inc.c
 create mode 100644 linux-user/cris/prologue.inc.c
 create mode 100644 linux-user/cris/signal.inc.c
 create mode 100644 linux-user/hppa/cpu_loop.inc.c
 create mode 100644 linux-user/hppa/prologue.inc.c
 create mode 100644 linux-user/hppa/signal.inc.c
 create mode 100644 linux-user/i386/cpu_loop.inc.c
 create mode 100644 linux-user/i386/prologue.inc.c
 create mode 100644 linux-user/i386/signal.inc.c
 create mode 100644 linux-user/m6

[Qemu-devel] [PATCH for 2.13 2/5] linux-user: remove unneeded #ifdef in signal.c

[Laurent Vivier]

Signed-off-by: Laurent Vivier 
---
 linux-user/signal.c | 9 ++---
 1 file changed, 2 insertions(+), 7 deletions(-)

diff --git a/linux-user/signal.c b/linux-user/signal.c
index 2c08ca14cf..514145b299 100644
--- a/linux-user/signal.c
+++ b/linux-user/signal.c
@@ -253,17 +253,15 @@ int do_sigprocmask(int how, const sigset_t *set, sigset_t 
*oldset)
 return 0;
 }
 
-#if !defined(TARGET_OPENRISC) && !defined(TARGET_NIOS2)
 /* Just set the guest's signal mask to the specified value; the
  * caller is assumed to have called block_signals() already.
  */
-static void set_sigmask(const sigset_t *set)
+static __attribute__((unused)) void set_sigmask(const sigset_t *set)
 {
 TaskState *ts = (TaskState *)thread_cpu->opaque;
 
 ts->signal_mask = *set;
 }
-#endif
 
 /* siginfo conversion */
 
@@ -533,8 +531,7 @@ static void force_sig(int sig)
  * up the signal frame. oldsig is the signal we were trying to handle
  * at the point of failure.
  */
-#if !defined(TARGET_RISCV)
-static void force_sigsegv(int oldsig)
+static __attribute__((unused)) void force_sigsegv(int oldsig)
 {
 if (oldsig == SIGSEGV) {
 /* Make sure we don't try to deliver the signal again; this will
@@ -545,8 +542,6 @@ static void force_sigsegv(int oldsig)
 force_sig(TARGET_SIGSEGV);
 }
 
-#endif
-
 /* abort execution with signal */
 static void QEMU_NORETURN dump_core_and_abort(int target_sig)
 {
-- 
2.14.3

[Qemu-devel] [Bug 1749393] Re: sbrk() not working under qemu-user with a PIE-compiled binary?

[Launchpad Bug Tracker]

Status changed to 'Confirmed' because the bug affects multiple users.

** Changed in: qemu (Ubuntu)
   Status: New => Confirmed

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

Title:
  sbrk() not working under qemu-user with a PIE-compiled binary?

Status in QEMU:
  New
Status in qemu package in Ubuntu:
  Confirmed

Bug description:
  In Debian unstable, we recently switched bash to be a PIE-compiled
  binary (for hardening). Unfortunately this resulted in bash being
  broken when run under qemu-user (for all target architectures, host
  being amd64 for me).

  $ sudo chroot /srv/chroots/sid-i386/ qemu-i386-static /bin/bash
  bash: xmalloc: .././shell.c:1709: cannot allocate 10 bytes (0 bytes allocated)

  bash has its own malloc implementation based on sbrk():
  https://git.savannah.gnu.org/cgit/bash.git/tree/lib/malloc/malloc.c

  When we disable this internal implementation and rely on glibc's
  malloc, then everything is fine. But it might be that glibc has a
  fallback when sbrk() is not working properly and it might hide the
  underlying problem in qemu-user.

  This issue has also been reported to the bash upstream author and he 
suggested that the issue might be in qemu-user so I'm opening a ticket here. 
Here's the discussion with the bash upstream author:
  https://lists.gnu.org/archive/html/bug-bash/2018-02/threads.html#00080

  You can find the problematic bash binary in that .deb file:
  
http://snapshot.debian.org/archive/debian/20180206T154716Z/pool/main/b/bash/bash_4.4.18-1_i386.deb

  The version of qemu I have been using is 2.11 (Debian package qemu-
  user-static version 1:2.11+dfsg-1) but I have had reports that the
  problem is reproducible with older versions (back to 2.8 at least).

  Here are the related Debian bug reports:
  https://bugs.debian.org/889869
  https://bugs.debian.org/865599

  It's worth noting that bash used to have this problem (when compiled as a PIE 
binary) even when run directly but then something got fixed in the kernel and 
now the problem only appears when run under qemu-user:
  https://bugs.launchpad.net/ubuntu/+source/linux/+bug/1518483

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

[Qemu-devel] [PATCH V2 1/1] mach-virt: Set VM's SMBIOS system version to mc->name

[Wei Huang]

Instead of using "1.0" as the system version of SMBIOS, we should use
mc->name for mach-virt machine type to be consistent other architectures.
With this patch, "dmidecode -t 1" (e.g., "-M virt-2.12,accel=kvm") will
show:

Handle 0x0100, DMI type 1, 27 bytes
System Information
Manufacturer: QEMU
Product Name: KVM Virtual Machine
Version: virt-2.12
Serial Number: Not Specified
...

instead of:

Handle 0x0100, DMI type 1, 27 bytes
System Information
Manufacturer: QEMU
Product Name: KVM Virtual Machine
Version: 1.0
Serial Number: Not Specified
...

For backward compatibility, we allow older machine types to keep "1.0"
as the default system version.

Signed-off-by: Wei Huang 
---
 hw/arm/virt.c | 8 +++-
 include/hw/arm/virt.h | 1 +
 2 files changed, 8 insertions(+), 1 deletion(-)

diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index 2c07245047..94dcb125d3 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -1132,6 +1132,8 @@ static void *machvirt_dtb(const struct arm_boot_info 
*binfo, int *fdt_size)
 
 static void virt_build_smbios(VirtMachineState *vms)
 {
+MachineClass *mc = MACHINE_GET_CLASS(vms);
+VirtMachineClass *vmc = VIRT_MACHINE_GET_CLASS(vms);
 uint8_t *smbios_tables, *smbios_anchor;
 size_t smbios_tables_len, smbios_anchor_len;
 const char *product = "QEMU Virtual Machine";
@@ -1145,7 +1147,8 @@ static void virt_build_smbios(VirtMachineState *vms)
 }
 
 smbios_set_defaults("QEMU", product,
-"1.0", false, true, SMBIOS_ENTRY_POINT_30);
+vmc->smbios_old_sys_ver ? "1.0" : mc->name, false,
+true, SMBIOS_ENTRY_POINT_30);
 
 smbios_get_tables(NULL, 0, &smbios_tables, &smbios_tables_len,
   &smbios_anchor, &smbios_anchor_len);
@@ -1646,8 +1649,11 @@ static void virt_2_11_instance_init(Object *obj)
 
 static void virt_machine_2_11_options(MachineClass *mc)
 {
+VirtMachineClass *vmc = VIRT_MACHINE_CLASS(OBJECT_CLASS(mc));
+
 virt_machine_2_12_options(mc);
 SET_MACHINE_COMPAT(mc, VIRT_COMPAT_2_11);
+vmc->smbios_old_sys_ver = true;
 }
 DEFINE_VIRT_MACHINE(2, 11)
 
diff --git a/include/hw/arm/virt.h b/include/hw/arm/virt.h
index 33b0ff3892..ba0c1a4faa 100644
--- a/include/hw/arm/virt.h
+++ b/include/hw/arm/virt.h
@@ -85,6 +85,7 @@ typedef struct {
 bool no_its;
 bool no_pmu;
 bool claim_edge_triggered_timers;
+bool smbios_old_sys_ver;
 } VirtMachineClass;
 
 typedef struct {
-- 
2.14.3

Re: [Qemu-devel] [PATCH for-2.12 2/2] make: switch from -I to -iquote

[Stefan Hajnoczi]

On Wed, Mar 21, 2018 at 05:22:07PM +0200, Michael S. Tsirkin wrote:
> Our rule right now is to use <> for external headers,
> "" for internal ones. The idea was to avoid conflicts
> between e.g. a system file named  and an
> internal one by the same name.
> 
> Unfortunately we use -I compiler flag so it does not
> help: a system file doing #include  will
> still pick up ours first.
> 
> To fix, switch to -iquote which is supported by both
> gcc and clang and only affects #include "" directives.
> 
> As a side effect, this catches any future uses of
>  #include <> for internal headers.
> 
> Suggested-by: Stefan Weil 
> Signed-off-by: Michael S. Tsirkin 
> ---
> 
> I still think we want to switch to a more formal rule such as qemu/
> prefix for all includes down the road, but this will at least catch any
> scheme violations from creeping in meanwhile.
> 
> 
>  configure   | 16 
>  rules.mak   |  2 +-
>  Makefile.target |  4 ++--
>  3 files changed, 11 insertions(+), 11 deletions(-)

Reviewed-by: Stefan Hajnoczi 


signature.asc
Description: PGP signature

Re: [Qemu-devel] [PATCH v3] RISC-V: Fix riscv_isa_string memory size bug

[Michael Clark]

On Mon, Mar 19, 2018 at 2:39 PM, Michael Clark  wrote:

>
>
> On Mon, Mar 19, 2018 at 12:42 PM, Richard W.M. Jones 
> wrote:
>
>> On Mon, Mar 19, 2018 at 11:35:51AM -0700, Michael Clark wrote:
>> > The RISC-V post-merge spec conformance and cleanup series has had a lot
>> of
>> > testing. I've been using it to compile QEMU inside of QEMU using the
>> RISC-V
>> > Fedora Image and its native RISC-V GCC toolchain running inside SMP
>> Linux
>> > 4.16-rc2. It appears to be pretty rock-solid. The rcu lock fix would
>> likely
>> > only affect users who are ballooning memory while a guest is under load.
>> > The page walker changes have also been tested under load (including
>> > performance tests).
>>
>> Did you see the problem with restoring floating point registers on
>> context switch?  The test case is quite simple:
>>
>>   http://oirase.annexia.org/tmp/sched.c
>
>
> No I didn't. Thanks.
>
>
>> Note you must compile it with gcc -O2 to manifest the bug.  (We
>> originally thought it was a problem with gcc's optimization, but it
>> isn't.)
>>
>
> Okay. I must admit most of my testing has been integer heavy workloads.
>
>
>> In Fedora's qemu tree are carrying the following patch which is just a
>> workaround:
>>
>>   https://github.com/rwmjones/fedora-riscv-bootstrap/blob/mast
>> er/stage1-riscv-qemu/force-float-save.patch
>
>
> I will take a look...
>

>From my local testing, it appears to only show up with SMP enabled. With 1
CPU enabled the sched.c test passes.


> There were some changes around v4 of our port upstream patch series to add
> MSTATUS.FS (TB_FLAGS_FP_ENABLE) to to tb->flags to avoid checking the flags
> on all FP ops.
>

I'm curious about the change in v4 of our upstream patch series "Enforce
MSTATUS_FS via TB flags"

-
https://github.com/riscv/riscv-qemu/commit/1b0631573008d19b2cb4bb1ef388bd51d6a1d722

I'll could try and revert this and see if it makes any difference...
however I think the float code has changed substantially with respect to
handling of flags. We could backport the more conservative approach of
handling MSTATUS_FS in the float routines. We should also run the test case
on hardware to rule out linux-kernel MSTATIS_FS bugs...

BTW We didn't have MTTCG enabled until v6 of our patch series so given it
only shows up on SMP, it wouldn't have been visible until v6 or after.

static inline void cpu_get_tb_cpu_state(CPURISCVState *env, target_ulong
> *pc,
> target_ulong *cs_base, uint32_t
> *flags)
> {
> *pc = env->pc;
> *cs_base = 0;
> #ifdef CONFIG_USER_ONLY
> *flags = TB_FLAGS_FP_ENABLE;
> #else
> *flags = cpu_mmu_index(env, 0) | (env->mstatus & MSTATUS_FS);
> #endif
> }
>
> I'll have to get my head around the implication of this or with the
> privilege level and how MSTATUS.FS is currently set...
>

Re: [Qemu-devel] [Qemu-arm] [PATCH for-2.12] hw/intc/arm_gicv3: Fix secure-GIC NS ICC_PMR and ICC_RPR accesses

[Philippe Mathieu-Daudé]

Hi Peter,

On 03/22/2018 03:29 PM, Peter Maydell wrote:
> On 22 March 2018 at 14:23, Andrew Jones  wrote:
>> On Thu, Mar 15, 2018 at 01:34:41PM +, Peter Maydell wrote:
>>> If the GIC has the security extension support enabled, then a
>>> non-secure access to ICC_PMR must take account of the non-secure
>>> view of interrupt priorities, where real priorities 0..0x7f
>>> are secure-only and not visible to the non-secure guest, and
>>> priorities 0x80..0xff are shown to the guest as if they were
>>> 0x00..0xff. We had the logic here wrong:
>>
>> 0x00..0x7f
> 
> I think 0x00..0xff is correct.

I guess Andrew only suggested to correct the hex prefix in your comment:
- ... where real priorities 0..0x7f
+ ... where real priorities 0x00..0x7f

> The conversion from actual
> priority value to the NS-view is
>if (prio & 0x80 == 0) {
>nsview = 0;
>} else {
>nsview = (prio << 1) & 0xff;
>}
> 
> so:
>real priority NS view
> 0x80  0x00
> 0x90  0x20
> 0xa0  0x40
> 0xb0  0x60
> 0xc0  0x80
> 0xd0  0xa0
> 0xe0  0xc0
> 0xf0  0xe0
> 
> the NS view covers the whole 0x00..0xff range, but more sparsely.
> (OK, technically you can't ever read 0xff, only 0xfe.)

Re: [Qemu-devel] [PATCH 1/3] target/xtensa: add .inc. to non-top level source file names

[Max Filippov]

On Thu, Mar 22, 2018 at 12:16 PM, Eric Blake  wrote:
> On 03/22/2018 01:09 PM, Max Filippov wrote:
>>
>> Fix definitions of existing cores and core importing script.
>
> This mentions the script...
> ...but only touches .c files.

Yeah, I initially did it in a single patch, but then split it into
two. Will merge
them back as suggested.

-- 
Thanks.
-- Max

Re: [Qemu-devel] [PATCH v2] qemu: replace "" with <> in headers

[Paolo Bonzini]

On 22/03/2018 20:29, Michael S. Tsirkin wrote:
> On Wed, Mar 21, 2018 at 05:22:03PM +0100, Kevin Wolf wrote:
>>> It's all still very much a non-standard convention and so less robust
>>> than prefixing file name with a project-specifix prefix.
>> I've always had the impression that it's by far the most common
>> convention, to the point that I'd blindly assume it when joining a new
>> project.
> 
> Any examples?

GCC - https://github.com/gcc-mirror/gcc/blob/master/gcc/reload.c
Libvirt - https://github.com/libvirt/libvirt/blob/master/src/util/virprocess.c
SDL - https://github.com/SDL-mirror/SDL/blob/master/src/core/unix/SDL_poll.c

Anything but Linux really.

I find  verbose and unnecessary.  The only advantage
of your proposal is that files included from the source directory would be
clearly noticeable.  That said, it's easy to add a checkpatch.pl rule that
detects when ".../..." is used on a file not under include/.

Paolo

[Qemu-devel] [Bug 1740219] Re: static linux-user ARM emulation has several-second startup time

[LukeShu]

This is now fixed on master, as of
3be2e41b3323169852dca11ffe6ff772c33e5aaa.

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

Title:
  static linux-user ARM emulation has several-second startup time

Status in QEMU:
  New
Status in qemu package in Ubuntu:
  Incomplete

Bug description:
  static linux-user emulation has several-second startup time

  My problem: I'm a Parabola packager, and I'm updating our
  qemu-user-static package from 2.8 to 2.11.  With my new
  statically-linked 2.11, running `qemu-arm /my/arm-chroot/bin/true`
  went from taking 0.006s to 3s!  This does not happen with the normal
  dynamically linked 2.11, or the old static 2.8.

  What happens is it gets stuck in
  `linux-user/elfload.c:init_guest_space()`.  What `init_guest_space`
  does is map 2 parts of the address space: `[base, base+guest_size]`
  and `[base+0x, base+0x+page_size]`; where it must find
  an acceptable `base`.  Its strategy is to `mmap(NULL, guest_size,
  ...)` decide where the first range is, and then check if that
  +0x is also available.  If it isn't, then it starts trying
  `mmap(base, ...)` for the entire address space from low-address to
  high-address.

  "Normally," it finds an accaptable `base` within the first 2 tries.
  With a static 2.11, it's taking thousands of tries.

  

  Now, from my understanding, there are 2 factors working together to
  cause that in static 2.11 but not the other builds:

   - 2.11 increased the default `guest_size` from 0xf700 to 0x
   - PIE (and thus ASLR) is disabled for static builds

  For some reason that I don't understand, with the smaller
  `guest_size` the initial `mmap(NULL, guest_size, ...)` usually
  returns an acceptable address range; but larger `guest_size` makes it
  consistently return a block of memory that butts right up against
  another already mapped chunk of memory.  This isn't just true on the
  older builds, it's true with the 2.11 builds if I use the `-R` flag to
  shrink the `guest_size` back down to 0xf700.  That is with
  linux-hardened 4.13.13 on x86-64.

  So then, it it falls back to crawling the entire address space; so it
  tries base=0x1000.  With ASLR, that probably succeeds.  But with
  ASLR being disabled on static builds, the text segment is at
  0x6000; which is does not leave room for the needed
  0x1000-size block before it.  So then it tries base=0x2000.
  And so on, more than 6000 times until it finally gets to and passes
  the text segment; calling mmap more than 12000 times.

  

  I'm not sure what the fix is.  Perhaps try to mmap a continuous chunk
  of size 0x1000, then munmap it and then mmap the 2 chunks that we
  actually need.  The disadvantage to that is that it does not support
  the sparse address space that the current algorithm supports for
  `guest_size < 0x`.  If `guest_size < 0x` *and* the big
  mmap fails, then it could fall back to a sparse search; though I'm not
  sure the current algorithm is a good choice for it, as we see in this
  bug.  Perhaps it should inspect /proc/self/maps to try to find a
  suitable range before ever calling mmap?

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

Re: [Qemu-devel] [PATCH 1/1] block/file-posix.c: fix not reopened lock file descriptor

[Dion Bosschieter]

Yeah I have a use case, before a last sync on a storage migration we suspend a 
VM -> send the last diffs -> mount the new storage server and after that we 
change a symlink -> call reopen -> check if all file descriptors are changed 
before resuming the VM.

Dion

> Op 22 mrt. 2018 om 18:39 heeft Kevin Wolf  het volgende 
> geschreven:
> 
> [ Cc: qemu-block ]
> 
> Am 22.03.2018 um 18:20 hat Dion Bosschieter geschrieben:
>> In commit 244a5668106297378391b768e7288eb157616f64 another
>> file descriptor to BDRVRawState is added. When we try to issue the
>> reopen command only s->fd is reopened; lock_fd could still hold an old
>> file descriptor "possibly" pointing to another file.
>> 
>> - change raw_reopen_prepare so it checks use_lock from BDRVRawState and
>> tries to reopen lock_fd accordingly
>> - change raw_reopen_commit so it closes the old lock_fd on use_lock
>> 
>> Signed-off-by: Dion Bosschieter 
> 
> bdrv_reopen() is not meant for opening a different file, it is meant to
> change the flags and options of the same file. Do you have a use case
> where you would actually need to switch to a different file?
> 
> As far as I know, lock_fd was specifically introduced _because_ it stays
> the same across reopen, so we don't need a racy release/reacquire pair.
> Fam (CCed) should know more.
> 
> In any case, doesn't your patch drop all the locks without reacquiring
> them on the new lock_fd?
> 
> Kevin
> 
>> block/file-posix.c | 25 +
>> 1 file changed, 25 insertions(+)
>> 
>> diff --git a/block/file-posix.c b/block/file-posix.c
>> index d7fb772c14..16d83fc49e 100644
>> --- a/block/file-posix.c
>> +++ b/block/file-posix.c
>> @@ -167,6 +167,7 @@ typedef struct BDRVRawState {
>> 
>> typedef struct BDRVRawReopenState {
>> int fd;
>> +int lock_fd;
>> int open_flags;
>> } BDRVRawReopenState;
>> 
>> @@ -795,6 +796,7 @@ static int raw_reopen_prepare(BDRVReopenState *state,
>> raw_parse_flags(state->flags, &rs->open_flags);
>> 
>> rs->fd = -1;
>> +rs->lock_fd = -1;
>> 
>> int fcntl_flags = O_APPEND | O_NONBLOCK;
>> #ifdef O_NOATIME
>> @@ -820,6 +822,17 @@ static int raw_reopen_prepare(BDRVReopenState *state,
>> rs->fd = -1;
>> }
>> }
>> +
>> +if (s->use_lock) {
>> +rs->lock_fd = qemu_dup(s->lock_fd);
>> +if (rs->lock_fd >= 0) {
>> +ret = fcntl_setfl(rs->lock_fd, rs->open_flags);
>> +if (ret) {
>> +qemu_close(rs->lock_fd);
>> +rs->lock_fd = -1;
>> +}
>> +}
>> +}
>> }
>> 
>> /* If we cannot use fcntl, or fcntl failed, fall back to qemu_open() */
>> @@ -835,6 +848,14 @@ static int raw_reopen_prepare(BDRVReopenState *state,
>> error_setg_errno(errp, errno, "Could not reopen file");
>> ret = -1;
>> }
>> +
>> +if (s->use_lock) {
>> +rs->lock_fd = qemu_open(normalized_filename, 
>> rs->open_flags);
>> +if (rs->lock_fd == -1) {
>> +error_setg_errno(errp, errno, "Could not reopen file 
>> for locking");
>> +ret = -1;
>> +}
>> +}
>> }
>> }
>> 
>> @@ -861,7 +882,11 @@ static void raw_reopen_commit(BDRVReopenState *state)
>> s->open_flags = rs->open_flags;
>> 
>> qemu_close(s->fd);
>> +if (s->use_lock) {
>> +qemu_close(s->lock_fd);
>> +}
>> s->fd = rs->fd;
>> +s->lock_fd = rs->lock_fd;
>> 
>> g_free(state->opaque);
>> state->opaque = NULL;
>> -- 
>> 2.14.2
>> 

[Qemu-devel] [PATCH 1/1] block/file-posix.c: fix not reopened lock file descriptor

[Dion Bosschieter]

In commit 244a5668106297378391b768e7288eb157616f64 another
file descriptor to BDRVRawState is added. When we try to issue the
reopen command only s->fd is reopened; lock_fd could still hold an old
file descriptor "possibly" pointing to another file.

- change raw_reopen_prepare so it checks use_lock from BDRVRawState and
tries to reopen lock_fd accordingly
- change raw_reopen_commit so it closes the old lock_fd on use_lock

Signed-off-by: Dion Bosschieter 
---
 block/file-posix.c | 25 +
 1 file changed, 25 insertions(+)

diff --git a/block/file-posix.c b/block/file-posix.c
index d7fb772c14..16d83fc49e 100644
--- a/block/file-posix.c
+++ b/block/file-posix.c
@@ -167,6 +167,7 @@ typedef struct BDRVRawState {
 
 typedef struct BDRVRawReopenState {
 int fd;
+int lock_fd;
 int open_flags;
 } BDRVRawReopenState;
 
@@ -795,6 +796,7 @@ static int raw_reopen_prepare(BDRVReopenState *state,
 raw_parse_flags(state->flags, &rs->open_flags);
 
 rs->fd = -1;
+rs->lock_fd = -1;
 
 int fcntl_flags = O_APPEND | O_NONBLOCK;
 #ifdef O_NOATIME
@@ -820,6 +822,17 @@ static int raw_reopen_prepare(BDRVReopenState *state,
 rs->fd = -1;
 }
 }
+
+if (s->use_lock) {
+rs->lock_fd = qemu_dup(s->lock_fd);
+if (rs->lock_fd >= 0) {
+ret = fcntl_setfl(rs->lock_fd, rs->open_flags);
+if (ret) {
+qemu_close(rs->lock_fd);
+rs->lock_fd = -1;
+}
+}
+}
 }
 
 /* If we cannot use fcntl, or fcntl failed, fall back to qemu_open() */
@@ -835,6 +848,14 @@ static int raw_reopen_prepare(BDRVReopenState *state,
 error_setg_errno(errp, errno, "Could not reopen file");
 ret = -1;
 }
+
+if (s->use_lock) {
+rs->lock_fd = qemu_open(normalized_filename, rs->open_flags);
+if (rs->lock_fd == -1) {
+error_setg_errno(errp, errno, "Could not reopen file for 
locking");
+ret = -1;
+}
+}
 }
 }
 
@@ -861,7 +882,11 @@ static void raw_reopen_commit(BDRVReopenState *state)
 s->open_flags = rs->open_flags;
 
 qemu_close(s->fd);
+if (s->use_lock) {
+qemu_close(s->lock_fd);
+}
 s->fd = rs->fd;
+s->lock_fd = rs->lock_fd;
 
 g_free(state->opaque);
 state->opaque = NULL;
-- 
2.14.2

Re: [Qemu-devel] [PATCH v2 1/2] make: move generated headers to qemu-build/

[Michael S. Tsirkin]

On Thu, Mar 22, 2018 at 02:42:55PM -0500, Eric Blake wrote:
> On 03/22/2018 02:27 PM, Michael S. Tsirkin wrote:
> > Make sure all generated files go into qemu-build subdirectory.
> > We can then include them like this:
> >   #include "qemu-build/trace.h"
> > 
> > This serves two purposes:
> > - make it easy to detect which files are in the source
> >directory (a bit more work for writers, easier for readers)
> > - reduce chances of conflicts with possible stale files in source
> >directory (which could be left over from e.g. old patches, etc)
> > 
> > This patch needs to be merged with patch 2  of series updating all
> > files: sending it separately to avoid spamming the list.
> > 
> > Signed-off-by: Michael S. Tsirkin 
> > ---
> 
> > +++ b/Makefile
> > @@ -89,102 +89,102 @@ endif
> >   include $(SRC_PATH)/rules.mak
> > -GENERATED_FILES = qemu-version.h config-host.h qemu-options.def
> > -GENERATED_FILES += qapi/qapi-builtin-types.h qapi/qapi-builtin-types.c
> 
> Uggh - I really need to follow up on my threat to make smarter use of make
> variables and string manipulation to cut down on the boilerplate involved
> here.  Sadly, I'm not convinced that doing so is a 2.12 bugfix priority, so
> it isn't at the top of my work queue.
> 
> Overall, the patch is an interesting idea.  I'm still not 100% sold on it
> (as you say, it's now slightly more work for writers), but I'm not coming up
> with any solid reasons why it should not be applied (at least, for 2.13 -
> doing it during freeze for 2.12 is a bit harder to justify).

It's up to Peter really: it helps reduce conflicts if we apply patches
like this during freeze.  But with enough effort on Pater's part it's
not a huge deal.

> -- 
> Eric Blake, Principal Software Engineer
> Red Hat, Inc.   +1-919-301-3266
> Virtualization:  qemu.org | libvirt.org

Re: [Qemu-devel] [PATCH v1 08/14] hostfloat: support float32/64 addition and subtraction

[Emilio G. Cota]

On Thu, Mar 22, 2018 at 14:41:05 +0800, Richard Henderson wrote:
(snip)
> Another thought re all of the soft_is_normal || soft_is_zero checks that 
> you're
> performing.  I think it would be nice if we could work with
> float*_unpack_canonical so that we don't have to duplicate work.  E.g.
> 
> /* Return true for float_class_normal && float_class_zero.  */
> static inline bool is_finite(FloatClass c) { return c <= float_class_zero; }
> 
> float32 float32_add(float32 a, float32 b, float_status *s)
> {
>   FloatClass a_cls = float32_classify(a);
>   FloatClass b_cls = float32_classify(b);

Just looked at this. It can be done, although it comes at the
price of some performance for fp-bench -o add:
180 Mflops vs. 196 Mflops, i.e. a 8% slowdown. That is with
adequate inlining etc., otherwise perf is worse.

I'm not convinced that we can gain much in simplicity to
justify the perf impact. Yes, we'd simplify canonicalize(),
but we'd probably need a float_class_denormal[*], which
would complicate everything else.

I think it makes sense to keep some inlines that work on
the float32/64's directly.

>   if (is_finite(a_cls) && is_finite(b_cls) && ...) {
>   /* do hardfp thing */
>   }

[*] Taking 0, denormals and normals would be OK from correctness,
but we really don't want to compute ops with denormal inputs on
the host; it is very likely that the output will also be denormal,
and we'll end up deferring to soft-fp anyway to avoid
computing whether the underflow exception has occurred,
which is expensive.

>   pa = float32_unpack(a, ca, s);
>   pb = float32_unpack(b, cb, s);
>   pr = addsub_floats(pa, pb, s, false);
>   return float32_round_pack(pr, s);
> }

It pays off to have two separate functions (add & sub) for the
slow path. With soft_f32_add/sub factored out:

$ taskset -c 0 x86_64-linux-user/qemu-x86_64 tests/fp-bench -o add
197.53 MFlops

With the above four lines (pa...return) as an else branch:
169.16 MFlops

BTW flattening makes things worse (150.63 MFlops).

Note that fp-bench only tests normal numbers. But I think it's fair
to assume that this is the path we want to speed up.

Thanks,

E.

Re: [Qemu-devel] [PATCH v2 1/2] make: move generated headers to qemu-build/

[Eric Blake]

On 03/22/2018 02:27 PM, Michael S. Tsirkin wrote:

Make sure all generated files go into qemu-build subdirectory.
We can then include them like this:
  #include "qemu-build/trace.h"

This serves two purposes:
- make it easy to detect which files are in the source
   directory (a bit more work for writers, easier for readers)
- reduce chances of conflicts with possible stale files in source
   directory (which could be left over from e.g. old patches, etc)

This patch needs to be merged with patch 2  of series updating all
files: sending it separately to avoid spamming the list.

Signed-off-by: Michael S. Tsirkin 
---



+++ b/Makefile
@@ -89,102 +89,102 @@ endif
  
  include $(SRC_PATH)/rules.mak
  
-GENERATED_FILES = qemu-version.h config-host.h qemu-options.def

-GENERATED_FILES += qapi/qapi-builtin-types.h qapi/qapi-builtin-types.c


Uggh - I really need to follow up on my threat to make smarter use of 
make variables and string manipulation to cut down on the boilerplate 
involved here.  Sadly, I'm not convinced that doing so is a 2.12 bugfix 
priority, so it isn't at the top of my work queue.


Overall, the patch is an interesting idea.  I'm still not 100% sold on 
it (as you say, it's now slightly more work for writers), but I'm not 
coming up with any solid reasons why it should not be applied (at least, 
for 2.13 - doing it during freeze for 2.12 is a bit harder to justify).


--
Eric Blake, Principal Software Engineer
Red Hat, Inc.   +1-919-301-3266
Virtualization:  qemu.org | libvirt.org

Re: [Qemu-devel] [PULL 00/25] RISC-V Post-merge spec conformance and cleanup

[Eric Blake]

On 03/22/2018 02:10 PM, Peter Maydell wrote:

[snip lots of good advice]


Code review isn't only about "does this behave as the
secification requires". It can also catch:
  * simple logic bugs
  * places where the code is more complicated than it needs to be
  * style issues
  * places where a QEMU utility routine could have been used
  * misunderstandings about QEMU internal APIs
  * memory leaks or memory corruption bugs


* grammar/typo fixes in comments/documentation/user-visible error 
strings



Review by somebody who is familiar with QEMU is just as useful
as review by somebody familiar with the target ISA.


Indeed.  Not everyone is an expert on every subject, but the more people 
that get involved, each in their own area of expertise, the wider the 
review coverage can be for a better end product.  And I'm very much okay 
with a reviewer stating up front "I'm only commenting on aspect XYZ, but 
will leave the rest of your patch to someone more familiar with ABC" - 
even that admission of a partial review is still helpful.



If your queue of unsubmitted or unreviewed patches is steadily
getting longer then something is going wrong; you should be
aiming for it to be as short as possible.


More on this point - typically, the review process is OFTEN the long 
pole in open source projects, where patches are submitted faster than 
reviewers can keep up.  While the following advice is by no means a 
requirement that everyone should follow, my personal goal is that I 
review at least two patches for every one that I submit, to help 
alleviate the review bottleneck.  And doing so has made ME a better 
programmer, as I get to explore coding paradigms and portions of the 
tree that I was previously unfamiliar with.


Furthermore, providing reviews to other's patches in addition to writing 
your own patches has some nice benefits: you gain some name recognition 
in the community, and it is obvious that you care more about the project 
as a whole than just about getting your stuff merged.  That in turn 
tends to have an interesting effect that your own patches get reviewed 
faster.


--
Eric Blake, Principal Software Engineer
Red Hat, Inc.   +1-919-301-3266
Virtualization:  qemu.org | libvirt.org

Re: [Qemu-devel] [PATCH v2] qemu: replace "" with <> in headers

[Michael S. Tsirkin]

On Wed, Mar 21, 2018 at 05:22:03PM +0100, Kevin Wolf wrote:
> > It's all still very much a non-standard convention and so less robust
> > than prefixing file name with a project-specifix prefix.
> 
> I've always had the impression that it's by far the most common
> convention, to the point that I'd blindly assume it when joining a new
> project.

Any examples?

> > > > As another example of problems, a header by the same name in the source
> > > > directory will always be picked up first - before any headers in
> > > > the include directory.
> > > > 
> > > > Let's change the scheme: make sure all headers that are not
> > > > in the source directory are included through a path
> > > > starting with qemu/ , thus:
> > > > 
> > > >  #include <>
> > > > 
> > > > headers in the same directory as source are included with
> > > > 
> > > >  #include ""
> > > > 
> > > > as per standard.
> > > > 
> > > > This (untested) patch is just to start the discussion and does not
> > > > change all of the codebase. If there's agreement, this will be
> > > > run on all code to converting code to this scheme.
> > > 
> > > Renaming files is always painful. If that's the fix, the cure might be
> > > worse than the disease. As far as I know, the conflict is only
> > > theoretical, so in that case I'd say: If it ain't broke, don't fix it.
> > > 
> > > Kevin
> > 
> > It's broke I think, it's very hard for new people to contribute to QEMU.
> > Look e.g. at rdma which all has messed up includes - and that's from an
> > experienced conributor who just isn't an experienced maintainer.
> 
> I don't think the problem is that the convention is hard to apply (it's
> definitely not). It's knowing about the convention. This problem isn't
> going away by switching to a different, less common convention. We're
> only going to see more offenders then.

Not if we have some automatic tools to catch violators.

> > Amount of time spent on teaching new people trivia about our
> > conventions just isn't funny. They should be self-documenting
> > and violations should cause the build to fail.
> 
> Yes, but your proposal doesn't achieve this. You can still use
> "qemu/foo.h" instead of  and it will build successfully.
> That's something we can't change, as far as I know, because the include
> path for "foo.h" is always a superset of .

If the rule is that "" is only for files in the current directory
then we can easily code up a checkpatch script to catch violators.

> If anything, this means that we should prefer "foo.h" for local headers
> (i.e. the way it currently is) because we can let the compiler enforce
> it:  for "foo.h" can become a build error, and does so with your
> -iquote patch, but the other way round doesn't work.
> 
> Then it's only system headers that you can possibly get wrong, but for
> those everyone should be used to using  anyway.
> 
> Kevin

If my proposal to prefix all include directories with qemu/
is accepted, then we can solve the stale file problem
by prohibiting a directory named qemu everywhere in source.


-- 
MST

[Qemu-devel] [PATCH v2 1/2] make: move generated headers to qemu-build/

[Michael S. Tsirkin]

Make sure all generated files go into qemu-build subdirectory.
We can then include them like this:
 #include "qemu-build/trace.h"

This serves two purposes:
- make it easy to detect which files are in the source
  directory (a bit more work for writers, easier for readers)
- reduce chances of conflicts with possible stale files in source
  directory (which could be left over from e.g. old patches, etc)

This patch needs to be merged with patch 2  of series updating all
files: sending it separately to avoid spamming the list.

Signed-off-by: Michael S. Tsirkin 
---
 configure   |   6 +-
 Makefile| 412 +++-
 rules.mak   |   5 +-
 .gitignore  |   1 +
 Makefile.objs   | 144 +-
 Makefile.target |  21 +--
 trace/Makefile.objs |  15 +-
 7 files changed, 313 insertions(+), 291 deletions(-)

diff --git a/configure b/configure
index 23a4f3b..7b0a183 100755
--- a/configure
+++ b/configure
@@ -6638,6 +6638,8 @@ if test "$gcov" = "yes" ; then
   echo "GCOV=$gcov_tool" >> $config_host_mak
 fi
 
+mkdir -p qemu-build
+
 # use included Linux headers
 if test "$linux" = "yes" ; then
   mkdir -p linux-headers
@@ -7046,10 +7048,10 @@ echo "QEMU_CFLAGS+=$cflags" >> $config_target_mak
 done # for target in $targets
 
 if [ "$dtc_internal" = "yes" ]; then
-  echo "config-host.h: subdir-dtc" >> $config_host_mak
+  echo "qemu-build/config-host.h: subdir-dtc" >> $config_host_mak
 fi
 if [ "$capstone" = "git" -o "$capstone" = "internal" ]; then
-  echo "config-host.h: subdir-capstone" >> $config_host_mak
+  echo "qemu-build/config-host.h: subdir-capstone" >> $config_host_mak
 fi
 if test -n "$LIBCAPSTONE"; then
   echo "LIBCAPSTONE=$LIBCAPSTONE" >> $config_host_mak
diff --git a/Makefile b/Makefile
index f799390..6fd90a8 100644
--- a/Makefile
+++ b/Makefile
@@ -89,102 +89,102 @@ endif
 
 include $(SRC_PATH)/rules.mak
 
-GENERATED_FILES = qemu-version.h config-host.h qemu-options.def
-GENERATED_FILES += qapi/qapi-builtin-types.h qapi/qapi-builtin-types.c
-GENERATED_FILES += qapi/qapi-types.h qapi/qapi-types.c
-GENERATED_FILES += qapi/qapi-types-block-core.h qapi/qapi-types-block-core.c
-GENERATED_FILES += qapi/qapi-types-block.h qapi/qapi-types-block.c
-GENERATED_FILES += qapi/qapi-types-char.h qapi/qapi-types-char.c
-GENERATED_FILES += qapi/qapi-types-common.h qapi/qapi-types-common.c
-GENERATED_FILES += qapi/qapi-types-crypto.h qapi/qapi-types-crypto.c
-GENERATED_FILES += qapi/qapi-types-introspect.h qapi/qapi-types-introspect.c
-GENERATED_FILES += qapi/qapi-types-migration.h qapi/qapi-types-migration.c
-GENERATED_FILES += qapi/qapi-types-misc.h qapi/qapi-types-misc.c
-GENERATED_FILES += qapi/qapi-types-net.h qapi/qapi-types-net.c
-GENERATED_FILES += qapi/qapi-types-rocker.h qapi/qapi-types-rocker.c
-GENERATED_FILES += qapi/qapi-types-run-state.h qapi/qapi-types-run-state.c
-GENERATED_FILES += qapi/qapi-types-sockets.h qapi/qapi-types-sockets.c
-GENERATED_FILES += qapi/qapi-types-tpm.h qapi/qapi-types-tpm.c
-GENERATED_FILES += qapi/qapi-types-trace.h qapi/qapi-types-trace.c
-GENERATED_FILES += qapi/qapi-types-transaction.h qapi/qapi-types-transaction.c
-GENERATED_FILES += qapi/qapi-types-ui.h qapi/qapi-types-ui.c
-GENERATED_FILES += qapi/qapi-builtin-visit.h qapi/qapi-builtin-visit.c
-GENERATED_FILES += qapi/qapi-visit.h qapi/qapi-visit.c
-GENERATED_FILES += qapi/qapi-visit-block-core.h qapi/qapi-visit-block-core.c
-GENERATED_FILES += qapi/qapi-visit-block.h qapi/qapi-visit-block.c
-GENERATED_FILES += qapi/qapi-visit-char.h qapi/qapi-visit-char.c
-GENERATED_FILES += qapi/qapi-visit-common.h qapi/qapi-visit-common.c
-GENERATED_FILES += qapi/qapi-visit-crypto.h qapi/qapi-visit-crypto.c
-GENERATED_FILES += qapi/qapi-visit-introspect.h qapi/qapi-visit-introspect.c
-GENERATED_FILES += qapi/qapi-visit-migration.h qapi/qapi-visit-migration.c
-GENERATED_FILES += qapi/qapi-visit-misc.h qapi/qapi-visit-misc.c
-GENERATED_FILES += qapi/qapi-visit-net.h qapi/qapi-visit-net.c
-GENERATED_FILES += qapi/qapi-visit-rocker.h qapi/qapi-visit-rocker.c
-GENERATED_FILES += qapi/qapi-visit-run-state.h qapi/qapi-visit-run-state.c
-GENERATED_FILES += qapi/qapi-visit-sockets.h qapi/qapi-visit-sockets.c
-GENERATED_FILES += qapi/qapi-visit-tpm.h qapi/qapi-visit-tpm.c
-GENERATED_FILES += qapi/qapi-visit-trace.h qapi/qapi-visit-trace.c
-GENERATED_FILES += qapi/qapi-visit-transaction.h qapi/qapi-visit-transaction.c
-GENERATED_FILES += qapi/qapi-visit-ui.h qapi/qapi-visit-ui.c
-GENERATED_FILES += qapi/qapi-commands.h qapi/qapi-commands.c
-GENERATED_FILES += qapi/qapi-commands-block-core.h 
qapi/qapi-commands-block-core.c
-GENERATED_FILES += qapi/qapi-commands-block.h qapi/qapi-commands-block.c
-GENERATED_FILES += qapi/qapi-commands-char.h qapi/qapi-commands-char.c
-GENERATED_FILES += qapi/qapi-commands-common.h qapi/qapi-commands-common.c
-GENERATED_FILES += qapi/qapi-commands-crypto.h qapi/qapi-commands-crypto.c
-GENERATED_FILES += qapi/qapi-commands-introsp

Re: [Qemu-devel] [PATCH for-2.12] gitmodules: Use the QEMU mirror of qemu-palcode

[Peter Maydell]

On 19 March 2018 at 16:25, Stefan Hajnoczi  wrote:
> On Mon, Mar 19, 2018 at 01:17:43PM +, Peter Maydell wrote:
>> We have a mirror of the qemu-palcode repository on
>> git.qemu.org; use that instead of the upstream github,
>> in line with our general policy of keeping and using
>> a mirror for submodules.
>>
>> Signed-off-by: Peter Maydell 
>> ---
>> We also currently have two submodules we don't have mirroring
>> for: seabios-hppa and u-boot-sam460ex.
>>
>>  .gitmodules | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> The git.qemu.org mirror appears up-to-date:
>
> Reviewed-by: Stefan Hajnoczi 

Thanks; applied to master.

-- PMM

Re: [Qemu-devel] [PATCH 0/4] coccinelle: re-run scripts from scripts/coccinelle

[Peter Maydell]

On 22 March 2018 at 19:12, Eric Blake  wrote:
> Or if we don't patch the false negative, you can bypass checkpatch with an
> ugly hack:
>
> return 0 + (...) | (...);
>
> (I'm NOT going to do that bypass - it's too ugly for my taste)

Yeah, that's definitely not something we should be doing.
checkpatch has plenty of false positives anyway, ignoring one
more is no big deal.

thanks
-- PMM

Re: [Qemu-devel] [PATCH 0/3] target/xtensa: improvements for core-specific files

[Eric Blake]

On 03/22/2018 01:09 PM, Max Filippov wrote:

Hello,

this series adds .inc. to the names of non-top level xtensa core-specific
files and fixes script import_core.sh so that it does it automatically.
It also adds a fixup to the script that changes #include 
to #include "xtensa-isa.h".

Max Filippov (3):
   target/xtensa: add .inc. to non-top level source file names
   target/xtensa/import_core.sh: fix names of non-top level files
   target/xtensa/import_core.sh: fix #include 


Simple enough.  I don't know if it qualifies as a bug fix, so up to you 
as maintainer whether you feel comfortable enough getting it into 2.12. 
See my per-patch comments about possibly merging 1 and 2; but for the 
series,

Reviewed-by: Eric Blake 

--
Eric Blake, Principal Software Engineer
Red Hat, Inc.   +1-919-301-3266
Virtualization:  qemu.org | libvirt.org

Re: [Qemu-devel] [PATCH 1/3] target/xtensa: add .inc. to non-top level source file names

[Eric Blake]

On 03/22/2018 01:09 PM, Max Filippov wrote:

Fix definitions of existing cores and core importing script.


This mentions the script...



Signed-off-by: Max Filippov 
---
  target/xtensa/core-dc232b.c   | 4 ++--
  target/xtensa/core-dc232b/{gdb-config.c => gdb-config.inc.c}  | 0
  target/xtensa/core-dc232b/{xtensa-modules.c => xtensa-modules.inc.c}  | 0
  target/xtensa/core-dc233c.c   | 4 ++--
  target/xtensa/core-dc233c/{gdb-config.c => gdb-config.inc.c}  | 0
  target/xtensa/core-dc233c/{xtensa-modules.c => xtensa-modules.inc.c}  | 0
  target/xtensa/core-de212.c| 4 ++--
  target/xtensa/core-de212/{gdb-config.c => gdb-config.inc.c}   | 0
  target/xtensa/core-de212/{xtensa-modules.c => xtensa-modules.inc.c}   | 0
  target/xtensa/core-fsf.c  | 2 +-
  target/xtensa/core-fsf/{xtensa-modules.c => xtensa-modules.inc.c} | 0
  target/xtensa/core-sample_controller.c| 4 ++--
  .../xtensa/core-sample_controller/{gdb-config.c => gdb-config.inc.c}  | 0
  .../core-sample_controller/{xtensa-modules.c => xtensa-modules.inc.c} | 0
  14 files changed, 9 insertions(+), 9 deletions(-)


...but only touches .c files.


--
Eric Blake, Principal Software Engineer
Red Hat, Inc.   +1-919-301-3266
Virtualization:  qemu.org | libvirt.org

Re: [Qemu-devel] [PATCH 2/3] target/xtensa/import_core.sh: fix names of non-top level files

[Eric Blake]

On 03/22/2018 01:09 PM, Max Filippov wrote:

Add .inc. to the names of files imported from configuration overlay in
the import_core.sh script to follow the rule of naming non-top level
source files.

Signed-off-by: Max Filippov 
---
  target/xtensa/import_core.sh | 8 
  1 file changed, 4 insertions(+), 4 deletions(-)


I'd squash this with 1/3 (otherwise, you have a one patch gap where the 
generator doesn't match the current tree contents, which may confuse 
someone doing a bisect).


--
Eric Blake, Principal Software Engineer
Red Hat, Inc.   +1-919-301-3266
Virtualization:  qemu.org | libvirt.org

Re: [Qemu-devel] [PATCH 0/4] coccinelle: re-run scripts from scripts/coccinelle

[Eric Blake]

On 03/22/2018 01:00 PM, Peter Maydell wrote:

On 22 March 2018 at 17:57,   wrote:

Checking PATCH 4/4: Remove unnecessary variables for function return value...
ERROR: return is not a function, parentheses are not required
#251: FILE: target/mips/dsp_helper.c:3281:
+return (temp[1] << 63) | (temp[0] >> 1);


This looks like a bug in checkpatch. I guess to fix it you'd need
to make checkpatch count opening and closing parens in the line
to see if it goes to 0 somewhere other than just before the ';'...


Or if we don't patch the false negative, you can bypass checkpatch with 
an ugly hack:


return 0 + (...) | (...);

(I'm NOT going to do that bypass - it's too ugly for my taste)

--
Eric Blake, Principal Software Engineer
Red Hat, Inc.   +1-919-301-3266
Virtualization:  qemu.org | libvirt.org

Re: [Qemu-devel] [PULL 00/25] RISC-V Post-merge spec conformance and cleanup

[Peter Maydell]

On 22 March 2018 at 18:26, Michael Clark  wrote:
> Besides some trivial cleanups (erroneous comments, dead-code), and the cpu
> init work we were asked to work on by Peter Maydell, the focus of the
> changes are specification conformance. e.g. cases where we were trapping on
> CSR accesses when we should return 0 or cases where the page walker didn't
> comply with the specification. To review these, it will require a thorough
> reading of the RISC-V Privileged ISA Specification:
>
> - https://riscv.org/specifications/privileged-isa/
>
> Given i'm the primary active RISC-V QEMU maintainer then its mostly my
> responsibility that we conform with the RISC-V specifications.

Right, but it is also your responsibility to make the best effort
you can to make your code available for review. Sometimes there are
patches that go out on the list and just don't get any review, and
you have to make a judgement call about whether you think they're
solid enough to go in anyway. But the ideal is that those are
not very common, and you should allow plenty of time and several
"pings for review" before doing that.

I am at the moment allowing you some extra slack because risc-v
support is new to QEMU and delay at this point would risk these
things not getting into 2.12. If we were not currently in freeze
I would take the approach of asking you to submit just the reviewed
patches and let the others have more time on-list for review.

> In time we
> will have tests for these corner cases in page walker, and control and
> status registers on processor variants that implement different subsets of
> the specification (such as SiFive's E series that has no MMU and no S mode,
> versus the SiFive U series which implements S mode and has an MMU), but at
> this stage it requires a very careful reading of the RISC-V ISA Privileged
> ISA Specification.
>
> We will more changes and the patch queue will only grow longer. Soon we will
> be trapping on s* CSRs if misa.S is not set and we'll add
> RISCV_FEATURE_MMU_HW_AD so that we can distinguish processors that handle
> PTE AD bits in hardware vs processors that defer PTE AD bit handling to
> software. These can only be reviewed by someone who is familar with the
> RISC-V Privileged ISA specification.

Pretty much all of our target frontends require at least some
familiarity with the relevant ISA specifications, yes. (You can
get a long way with knowing ISAs in general and having the manual,
though. We have several developers who have reviewed code for
multiple different supported guest architectures.)
Code review isn't only about "does this behave as the
secification requires". It can also catch:
 * simple logic bugs
 * places where the code is more complicated than it needs to be
 * style issues
 * places where a QEMU utility routine could have been used
 * misunderstandings about QEMU internal APIs
 * memory leaks or memory corruption bugs
Review by somebody who is familiar with QEMU is just as useful
as review by somebody familiar with the target ISA.

>> In your 25 matches, only 12 have R-b tag.
>>
>> You could have sent a PR of the reviewed patches, and respin the
>> unreviewed patches separately.
>>
>> I also see you addressed some of my comments, so I'd have liked be able
>> to take time to review and eventually test your series.
>
>
> Sure take your time, however do consider that we should be able to fix bugs
> as a maintainer for RISC-V. I'm very able to separate well-tested changes
> from experimental code. e.g
>
> - https://github.com/michaeljclark/riscv-qemu/tree/wip-riscv-tcg-backend
>
> It will be some time before we submit patches for the RISC-V backend. It
> needs to be rock-solid before I would consider submitting it.

This is perhaps not the best possible approach. You will find
that it's easier to get code into QEMU (and many upstream projects)
if you do the development as a part of that upstream. So for
instance you can submit "RFC" patches if they're work in progress,
or initial versions of a patchset which implement a basic working
version with optimizations to follow. (You just need to be clear
in a patchset cover letter what is/isn't implemented, what you're
asking for review on, and so on.) If you do that then you won't
find yourself in the position of submitting what you think is a
complete finished feature and being asked to do significant rework.
You also are less likely find yourself two weeks before a codefreeze
with a large pile of unreviewed and uncommitted changes and
having huge difficulty getting your code into the tree.

In my experience the best way to work is "upstream first" and
incrementally, so that large features and bugfixes all go onto
the list, get reviewed and then go into upstream in small parts
over the whole QEMU development cycle. It's often possible
to put not-yet-finished work behind a feature-bit gate that means
that it doesn't appear to guests until it's finished and the
feature-bit is enabled.

Working incrementally also significantly 

Re: [Qemu-devel] [PATCH v4 2/2] i386/kvm: expose Hyper-V frequency MSRs with reenlightenment

[Eduardo Habkost]

On Thu, Mar 22, 2018 at 02:13:58PM +0100, Vitaly Kuznetsov wrote:
> We can also expose Hyper-V frequency MSRs when reenlightenment feature is
> enabled and TSC frequency is known, Hyper-V on KVM will provide stable TSC
> page clocksources to its guests.
> 
> Signed-off-by: Vitaly Kuznetsov 
> ---
> - Expose frequency MSRs only when either INVTSC or Reenlightenment is
>   provided [Paolo Bonzini]
> ---
>  target/i386/kvm.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/target/i386/kvm.c b/target/i386/kvm.c
> index 75f4e1d69e..2c3c19d690 100644
> --- a/target/i386/kvm.c
> +++ b/target/i386/kvm.c
> @@ -651,7 +651,8 @@ static int hyperv_handle_properties(CPUState *cs)
>  env->features[FEAT_HYPERV_EAX] |= HV_TIME_REF_COUNT_AVAILABLE;
>  env->features[FEAT_HYPERV_EAX] |= HV_REFERENCE_TSC_AVAILABLE;
>  
> -if (has_msr_hv_frequencies && tsc_is_stable_and_known(env)) {
> +if (has_msr_hv_frequencies && env->tsc_khz &&
> +(tsc_is_stable_and_known(env) || cpu->hyperv_reenlightenment)) {

Continuing the discussion we had in v3 about being possible to
crash when migrating to a host running an older kernel:

This patch doesn't fix the issue, because it is still implicitly
enabling hv-frequencies.

But I don't think this patch will make the situation any worse,
because we don't have any KVM versions that support
HV_X64_MSR_REENLIGHTENMENT_CONTROL but not
HV_X64_MSR_TSC_FREQUENCY.

This means that we can safely make "hv-reenlightenment=on"
automatically set "hv-frequencies=on", when we finally introduce
a hv-frequencies property.

Roman, do you agree?

The next question is: do we need to fix this and introduce a
hv-frequencies property in 2.12, or can this wait for 2.13?


>  env->features[FEAT_HYPERV_EAX] |= HV_ACCESS_FREQUENCY_MSRS;
>  env->features[FEAT_HYPERV_EDX] |= HV_FREQUENCY_MSRS_AVAILABLE;
>  }
> -- 
> 2.14.3
> 
> 

-- 
Eduardo

Re: [Qemu-devel] [PATCH v4 2/2] i386/kvm: expose Hyper-V frequency MSRs with reenlightenment

[Eduardo Habkost]

On Thu, Mar 22, 2018 at 02:13:58PM +0100, Vitaly Kuznetsov wrote:
> We can also expose Hyper-V frequency MSRs when reenlightenment feature is
> enabled and TSC frequency is known, Hyper-V on KVM will provide stable TSC
> page clocksources to its guests.
> 
> Signed-off-by: Vitaly Kuznetsov 
> ---
> - Expose frequency MSRs only when either INVTSC or Reenlightenment is
>   provided [Paolo Bonzini]
> ---
>  target/i386/kvm.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/target/i386/kvm.c b/target/i386/kvm.c
> index 75f4e1d69e..2c3c19d690 100644
> --- a/target/i386/kvm.c
> +++ b/target/i386/kvm.c
> @@ -651,7 +651,8 @@ static int hyperv_handle_properties(CPUState *cs)
>  env->features[FEAT_HYPERV_EAX] |= HV_TIME_REF_COUNT_AVAILABLE;
>  env->features[FEAT_HYPERV_EAX] |= HV_REFERENCE_TSC_AVAILABLE;
>  
> -if (has_msr_hv_frequencies && tsc_is_stable_and_known(env)) {
> +if (has_msr_hv_frequencies && env->tsc_khz &&

Why is the check for env->tsc_khz necessary?

Are there known circumstances where HV_X64_MSR_TSC_FREQUENCY will be supported
by KVM but ioctl(KVM_GET_TSC_KHZ) will return 0, or this is just for extra
safety?

> +(tsc_is_stable_and_known(env) || cpu->hyperv_reenlightenment)) {
>  env->features[FEAT_HYPERV_EAX] |= HV_ACCESS_FREQUENCY_MSRS;
>  env->features[FEAT_HYPERV_EDX] |= HV_FREQUENCY_MSRS_AVAILABLE;
>  }
> -- 
> 2.14.3
> 

-- 
Eduardo

Re: [Qemu-devel] [PATCH v3 2/2] i386/kvm: lower requirements for Hyper-V frequency MSRs exposure

[Eduardo Habkost]

On Thu, Mar 22, 2018 at 04:58:03PM +0300, Roman Kagan wrote:
> On Thu, Mar 22, 2018 at 10:22:18AM -0300, Eduardo Habkost wrote:
> > On Thu, Mar 22, 2018 at 04:00:14PM +0300, Roman Kagan wrote:
> > > On Wed, Mar 21, 2018 at 05:19:24PM -0300, Eduardo Habkost wrote:
> > > > On Wed, Mar 21, 2018 at 07:57:29PM +0300, Roman Kagan wrote:
> > > > > On Wed, Mar 21, 2018 at 02:18:54PM +0100, Vitaly Kuznetsov wrote:
> > > > > > Roman Kagan  writes:
> > > > > > 
> > > > > > > On Tue, Mar 20, 2018 at 06:35:00PM +0100, Vitaly Kuznetsov wrote:
> > > > > > >> Requiring tsc_is_stable_and_known() is too restrictive: even 
> > > > > > >> without INVTCS
> > > > > > >> nested Hyper-V-on-KVM enables TSC pages for its guests e.g. when
> > > > > > >> Reenlightenment MSRs are present. Presence of frequency MSRs 
> > > > > > >> doesn't mean
> > > > > > >> these frequencies are stable, it just means they're available 
> > > > > > >> for reading.
> > > > > > >> 
> > > > > > >> Signed-off-by: Vitaly Kuznetsov 
> > > > > > >> ---
> > > > > > >>  target/i386/kvm.c | 2 +-
> > > > > > >>  1 file changed, 1 insertion(+), 1 deletion(-)
> > > > > > >> 
> > > > > > >> diff --git a/target/i386/kvm.c b/target/i386/kvm.c
> > > > > > >> index 7d9f9ca0b1..74fc3d3b2c 100644
> > > > > > >> --- a/target/i386/kvm.c
> > > > > > >> +++ b/target/i386/kvm.c
> > > > > > >> @@ -651,7 +651,7 @@ static int hyperv_handle_properties(CPUState 
> > > > > > >> *cs)
> > > > > > >>  env->features[FEAT_HYPERV_EAX] |= 
> > > > > > >> HV_TIME_REF_COUNT_AVAILABLE;
> > > > > > >>  env->features[FEAT_HYPERV_EAX] |= 
> > > > > > >> HV_REFERENCE_TSC_AVAILABLE;
> > > > > > >>  
> > > > > > >> -if (has_msr_hv_frequencies && 
> > > > > > >> tsc_is_stable_and_known(env)) {
> > > > > > >> +if (has_msr_hv_frequencies && env->tsc_khz) {
> > > > > > >>  env->features[FEAT_HYPERV_EAX] |= 
> > > > > > >> HV_ACCESS_FREQUENCY_MSRS;
> > > > > > >>  env->features[FEAT_HYPERV_EDX] |= 
> > > > > > >> HV_FREQUENCY_MSRS_AVAILABLE;
> > > > > > >>  }
> > > > > > >
> > > > > > > I suggest that we add a corresponding cpu property here, too.  
> > > > > > > The guest
> > > > > > > may legitimately rely on these msrs when it sees the support in 
> > > > > > > CPUID,
> > > > > > > and migrating from a kernel with the feature supported (4.14+) to 
> > > > > > > an
> > > > > > > older one will make it crash.
> > > > > > >
> > > > > > 
> > > > > > This can be arranged, but what happens to people who use these 
> > > > > > features
> > > > > > today? Assuming they also passed 'invtsc' they have stable TSC page
> > > > > > clocksource already (when Hyper-V role is enabled) but when we start
> > > > > > requesting a new 'hv_frequency' cpu property they'll suddenly lose 
> > > > > > what
> > > > > > they have...
> > > > > 
> > > > > I see two cases here:
> > > > > 
> > > > > 1) people start a new VM, and discover that their old configuration is
> > > > >not enough for this feature to work.
> > > > > 
> > > > >They need to reconfigure and restart the VM.  This costs them some
> > > > >time investigating and restarting, but not data.
> > > > 
> > > > If we keep machine-type compatibility, people will need to do
> > > > that only if they change the machine-type (or use the "pc" or
> > > > "q35" aliases).  If they copy the old configuration, it will keep
> > > > working.
> > > 
> > > The problem is that the feature is not fixed by the machine-type, due to
> > > the forgotten property: it only depends on the KVM version.  So, once
> > > (if) we add the property and make the feature deterministic, we'll lose
> > > compatibility one way or another.
> > > 
> > > Or are you suggesting that for pre-2.12 machine types we leave the
> > > property at "decided by your KVM" state?
> > 
> > Yes, that's what I mean.  This looks like the only way to avoid
> > losing features by just cold-rebooting an existing VM.
> > 
> > The scenario I'm thinking is this:
> > 
> > 1) pc-2.11 VM started on host running QEMU 2.11
> > 2) VM migrated to a host containing this patch
> > 3) 1 year later, the VM is shut down and booted again.
> > 4) Things stop working inside the VM because hv-frequency is
> >unexpectedly gone.
> > 
> > Machine-type compatibility code would avoid (4).
> 
> Right, but (4) typically means that you fail to start your workload from
> a clean state, so you just go and fix it; no data is lost.
> 
> Compare this to a migration to an older KVM which results in your guest
> crashing, where you risk data loss and still have to meddle with
> configs.

True. To make it worse, we are already unable to avoid this crash
on existing VMs without a reboot.  The only case where we can fix
this is if live-migration to older KVM happens after the guest
was rebooted when running on a newer QEMU version.  :(



> 
> > > > machine-type compatibility also makes the following case a bit
> > > > safer:
> > > > 
> > > > > 
> > > > > 2) people migrate from a QEM

Re: [Qemu-devel] [PATCH for-2.12] hw/intc/arm_gicv3: Fix secure-GIC NS ICC_PMR and ICC_RPR accesses

[Peter Maydell]

On 22 March 2018 at 14:23, Andrew Jones  wrote:
> On Thu, Mar 15, 2018 at 01:34:41PM +, Peter Maydell wrote:
>> If the GIC has the security extension support enabled, then a
>> non-secure access to ICC_PMR must take account of the non-secure
>> view of interrupt priorities, where real priorities 0..0x7f
>> are secure-only and not visible to the non-secure guest, and
>> priorities 0x80..0xff are shown to the guest as if they were
>> 0x00..0xff. We had the logic here wrong:
>
> 0x00..0x7f

I think 0x00..0xff is correct. The conversion from actual
priority value to the NS-view is
   if (prio & 0x80 == 0) {
   nsview = 0;
   } else {
   nsview = (prio << 1) & 0xff;
   }

so:
   real priority NS view
0x80  0x00
0x90  0x20
0xa0  0x40
0xb0  0x60
0xc0  0x80
0xd0  0xa0
0xe0  0xc0
0xf0  0xe0

the NS view covers the whole 0x00..0xff range, but more sparsely.
(OK, technically you can't ever read 0xff, only 0xfe.)

> Reviewed-by: Andrew Jones 

Thanks for the review.

-- PMM

[Qemu-devel] [PATCH v2 0/6] postcopy block time calculation + ppc32 build fix

[Alexey Perevalov]

V1-V2
accidentally appeared __nocheck after rebase
this patch set also rebased after latest pull request

This patch set includes patches which were reverted by commit
ee86981bd, due to build problem on 32 powerpc/arm architecture.
Also it includes patch to fix build
([PATCH v4] migration: change blocktime type to uint32_t), but that
patch was merged into:
migration: add postcopy blocktime ctx into MigrationIncomingState
migration: calculate vCPU blocktime on dst side
migration: add postcopy total blocktime into query-migrate


based on
commit c6740fc88ecd8f5cf3cf3185ee112c3eea41caa2
"hw/rdma: Implementation of PVRDMA device"

Alexey Perevalov (6):
  migration: introduce postcopy-blocktime capability
  migration: add postcopy blocktime ctx into MigrationIncomingState
  migration: calculate vCPU blocktime on dst side
  migration: postcopy_blocktime documentation
  migration: add blocktime calculation into migration-test
  migration: add postcopy total blocktime into query-migrate

 docs/devel/migration.rst |  14 +++
 hmp.c|  15 +++
 migration/migration.c|  51 -
 migration/migration.h|  13 +++
 migration/postcopy-ram.c | 268 ++-
 migration/trace-events   |   6 +-
 qapi/migration.json  |  17 ++-
 tests/migration-test.c   |  16 +++
 8 files changed, 392 insertions(+), 8 deletions(-)

-- 
2.7.4

Re: [Qemu-devel] [PULL 00/25] RISC-V Post-merge spec conformance and cleanup

[Michael Clark]

On Thu, Mar 22, 2018 at 2:56 AM, Philippe Mathieu-Daudé 
wrote:

> Hi Michael,
>
> On 03/20/2018 07:25 PM, Michael Clark wrote:
> > The following changes since commit f1a63fcfcd92c88be8942b5ae71aef
> 9749a4f135:
> >
> >   Update version for v2.12.0-rc0 release (2018-03-20 19:04:22 +)
> >
> > are available in the git repository at:
> >
> >   https://github.com/riscv/riscv-qemu.git tags/riscv-qemu-2.12-fixes
> >
> > for you to fetch changes up to e1a247183ac0816284dfda61423f7030b6686679:
> >
> >   RISC-V: Remove erroneous comment from translate.c (2018-03-20 14:34:13
> -0700)
> >
> > 
> > This is a series of spec conformance bug fixes and code cleanups
> > that we would like to get in before the QEMU 2.12 release.
> >
> > * Implements WARL behavior for CSRs that don't support writes
> > * Improves specification conformance of the page table walker
> >   * Change access checks from ternary operator to if statements
> >   * Checks for misaligned PPNs
> >   * Disallow M-mode or S-mode from fetching from User pages
> >   * Adds reserved PTE flag check: W or W|X
> >   * Set READ flag for PTE X flag if mstatus.mxr is in effect
> >   * Improves page walker comments and general readability
> > * Several trivial code cleanups to hw/riscv
> >   * Replacing hard coded constants with reference to enums
> > or the machine memory maps.
> >   * Remove unnecessary class initialization boilerplate
> > * Adds bounds checks when writing device-tree to ROM
> > * Updates the cpu model to use a more modern interface
> > * Sets mtval/stval to zero on exceptions without addresses
> > * Fixes memory allocation bug in riscv_isa_string
> >
> > v4
> >
> > * added fix for memory allocation bug in riscv_isa_string
> >
> > v3
> >
> > * refactor rcu_read_lock in PTE update to use single unlock
> > * mstatus.mxr is in effect regardless of privilege mode
> > * remove unnecessary class init from riscv_hart
> > * set mtval/stval to zero on exceptions without addresses
> >
> > v2
> >
> > * remove unused class boilerplate retains qom parent_obj
> > * convert cpu definition towards future model
> > * honor mstatus.mxr flag in page table walker
> >
> > v1
> >
> > * initial post merge cleanup patch series
> >
> > 
> > Michael Clark (25):
> >   RISC-V: Make virt create_fdt interface consistent
> >   RISC-V: Replace hardcoded constants with enum values
> >   RISC-V: Make virt board description match spike
> >   RISC-V: Use ROM base address and size from memmap
> >   RISC-V: Remove identity_translate from load_elf
> >   RISC-V: Mark ROM read-only after copying in code
> >   RISC-V: Remove unused class definitions
> >   RISC-V: Make sure rom has space for fdt
> >   RISC-V: Include intruction hex in disassembly
> >   RISC-V: Hold rcu_read_lock when accessing memory
> >   RISC-V: Improve page table walker spec compliance
> >   RISC-V: Update E order and I extension order
> >   RISC-V: Make some header guards more specific
> >   RISC-V: Make virt header comment title consistent
> >   RISC-V: Use memory_region_is_ram in pte update
> >   RISC-V: Remove EM_RISCV ELF_MACHINE indirection
> >   RISC-V: Hardwire satp to 0 for no-mmu case
> >   RISC-V: Remove braces from satp case statement
> >   RISC-V: riscv-qemu port supports sv39 and sv48
> >   RISC-V: vectored traps are optional
> >   RISC-V: No traps on writes to misa,minstret,mcycle
> >   RISC-V: Remove support for adhoc X_COP interrupt
> >   RISC-V: Convert cpu definition towards future model
> >   RISC-V: Clear mtval/stval on exceptions without info
> >   RISC-V: Remove erroneous comment from translate.c
>
>
> I'm not a maintainer, so I'll just give my reviewer point of view, since
> I'm willing to review the RISC-V patches.
>
> From https://wiki.qemu.org/Contribute/FAQ:
>
>   "In order for your patch to be merged, it must either
>   (1) receive a Reviewed-by by a trusted reviewer on qemu-devel or
>   (2) be reviewed by a maintainer and accepted into their tree."
>

Re 2). I am listed as a maintainer for hw/riscv and target/riscv which is
the only area these patches touch and these patches have been in the
riscv.org tree for quite some time. Indeed, most folk are running RISC-V
QEMU from the riscv.org tree as it has previously been the only way to
access the port before it very recently made it upstream.

$ sed -n '/RISC-V/,/^$/p' MAINTAINERS
RISC-V
M: Michael Clark 
M: Palmer Dabbelt 
M: Sagar Karandikar 
M: Bastian Koppelmann 
S: Maintained
F: target/riscv/
F: hw/riscv/
F: include/hw/riscv/
F: disas/riscv.c

Besides some trivial cleanups (erroneous comments, dead-code), and the cpu
init work we were asked to work on by Peter Maydell, the focus of the
changes are specification conformance. e.g. cases where we were trapping on
CSR accesses when we should return 0 or cases 

Re: [Qemu-devel] [PATCH] linux-user/xtensa: remove stray syscall.h

[Peter Maydell]

On 22 March 2018 at 18:18, Max Filippov  wrote:
> Signed-off-by: Max Filippov 
> ---
>  linux-user/xtensa/syscall.h | 0
>  1 file changed, 0 insertions(+), 0 deletions(-)
>  delete mode 100644 linux-user/xtensa/syscall.h
>
> diff --git a/linux-user/xtensa/syscall.h b/linux-user/xtensa/syscall.h
> deleted file mode 100644
> index e69de29bb2d1..

Reviewed-by: Peter Maydell 

thanks
-- PMM

[Qemu-devel] [PATCH v2 6/6] migration: add postcopy total blocktime into query-migrate

[Alexey Perevalov]

Postcopy total blocktime is available on destination side only.
But query-migrate was possible only for source. This patch
adds ability to call query-migrate on destination.
To be able to see postcopy blocktime, need to request postcopy-blocktime
capability.

The query-migrate command will show following sample result:
{"return":
"postcopy-vcpu-blocktime": [115, 100],
"status": "completed",
"postcopy-blocktime": 100
}}

postcopy_vcpu_blocktime contains list, where the first item is the first
vCPU in QEMU.

This patch has a drawback, it combines states of incoming and
outgoing migration. Ongoing migration state will overwrite incoming
state. Looks like better to separate query-migrate for incoming and
outgoing migration or add parameter to indicate type of migration.

Reviewed-by: Dr. David Alan Gilbert 
Signed-off-by: Alexey Perevalov 
Reviewed-by: Juan Quintela 
Signed-off-by: Juan Quintela 
---
 hmp.c| 15 +
 migration/migration.c| 42 
 migration/migration.h|  4 
 migration/postcopy-ram.c | 56 
 migration/trace-events   |  1 +
 qapi/migration.json  | 11 +-
 6 files changed, 124 insertions(+), 5 deletions(-)

diff --git a/hmp.c b/hmp.c
index 679467d..6c51df5 100644
--- a/hmp.c
+++ b/hmp.c
@@ -274,6 +274,21 @@ void hmp_info_migrate(Monitor *mon, const QDict *qdict)
info->cpu_throttle_percentage);
 }
 
+if (info->has_postcopy_blocktime) {
+monitor_printf(mon, "postcopy blocktime: %u\n",
+   info->postcopy_blocktime);
+}
+
+if (info->has_postcopy_vcpu_blocktime) {
+Visitor *v;
+char *str;
+v = string_output_visitor_new(false, &str);
+visit_type_uint32List(v, NULL, &info->postcopy_vcpu_blocktime, NULL);
+visit_complete(v, &str);
+monitor_printf(mon, "postcopy vcpu blocktime: %s\n", str);
+g_free(str);
+visit_free(v);
+}
 qapi_free_MigrationInfo(info);
 qapi_free_MigrationCapabilityStatusList(caps);
 }
diff --git a/migration/migration.c b/migration/migration.c
index f95a7f3..71b0f19 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -630,14 +630,15 @@ static void populate_disk_info(MigrationInfo *info)
 }
 }
 
-MigrationInfo *qmp_query_migrate(Error **errp)
+static void fill_source_migration_info(MigrationInfo *info)
 {
-MigrationInfo *info = g_malloc0(sizeof(*info));
 MigrationState *s = migrate_get_current();
 
 switch (s->state) {
 case MIGRATION_STATUS_NONE:
 /* no migration has happened ever */
+/* do not overwrite destination migration status */
+return;
 break;
 case MIGRATION_STATUS_SETUP:
 info->has_status = true;
@@ -688,8 +689,6 @@ MigrationInfo *qmp_query_migrate(Error **errp)
 break;
 }
 info->status = s->state;
-
-return info;
 }
 
 /**
@@ -753,6 +752,41 @@ static bool migrate_caps_check(bool *cap_list,
 return true;
 }
 
+static void fill_destination_migration_info(MigrationInfo *info)
+{
+MigrationIncomingState *mis = migration_incoming_get_current();
+
+switch (mis->state) {
+case MIGRATION_STATUS_NONE:
+return;
+break;
+case MIGRATION_STATUS_SETUP:
+case MIGRATION_STATUS_CANCELLING:
+case MIGRATION_STATUS_CANCELLED:
+case MIGRATION_STATUS_ACTIVE:
+case MIGRATION_STATUS_POSTCOPY_ACTIVE:
+case MIGRATION_STATUS_FAILED:
+case MIGRATION_STATUS_COLO:
+info->has_status = true;
+break;
+case MIGRATION_STATUS_COMPLETED:
+info->has_status = true;
+fill_destination_postcopy_migration_info(info);
+break;
+}
+info->status = mis->state;
+}
+
+MigrationInfo *qmp_query_migrate(Error **errp)
+{
+MigrationInfo *info = g_malloc0(sizeof(*info));
+
+fill_destination_migration_info(info);
+fill_source_migration_info(info);
+
+return info;
+}
+
 void qmp_migrate_set_capabilities(MigrationCapabilityStatusList *params,
   Error **errp)
 {
diff --git a/migration/migration.h b/migration/migration.h
index 6d9aaeb..7c69598 100644
--- a/migration/migration.h
+++ b/migration/migration.h
@@ -77,6 +77,10 @@ struct MigrationIncomingState {
 
 MigrationIncomingState *migration_incoming_get_current(void);
 void migration_incoming_state_destroy(void);
+/*
+ * Functions to work with blocktime context
+ */
+void fill_destination_postcopy_migration_info(MigrationInfo *info);
 
 #define TYPE_MIGRATION "migration"
 
diff --git a/migration/postcopy-ram.c b/migration/postcopy-ram.c
index 6b01884..bbc1a95 100644
--- a/migration/postcopy-ram.c
+++ b/migration/postcopy-ram.c
@@ -139,6 +139,55 @@ static struct PostcopyBlocktimeContext 
*blocktime_context_new(void)
 return ctx;
 }
 
+static uint32List *get_vcpu_blocktime_list(PostcopyBlocktimeContext *ctx)
+{
+uint32List *list = NULL, *ent

[Qemu-devel] [PATCH v2 1/6] migration: introduce postcopy-blocktime capability

[Alexey Perevalov]

Right now it could be used on destination side to
enable vCPU blocktime calculation for postcopy live migration.
vCPU blocktime - it's time since vCPU thread was put into
interruptible sleep, till memory page was copied and thread awake.

Reviewed-by: Dr. David Alan Gilbert 
Signed-off-by: Alexey Perevalov 
Reviewed-by: Juan Quintela 
Signed-off-by: Juan Quintela 
---
 migration/migration.c | 9 +
 migration/migration.h | 1 +
 qapi/migration.json   | 6 +-
 3 files changed, 15 insertions(+), 1 deletion(-)

diff --git a/migration/migration.c b/migration/migration.c
index fc629e5..f95a7f3 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -1540,6 +1540,15 @@ bool migrate_zero_blocks(void)
 return s->enabled_capabilities[MIGRATION_CAPABILITY_ZERO_BLOCKS];
 }
 
+bool migrate_postcopy_blocktime(void)
+{
+MigrationState *s;
+
+s = migrate_get_current();
+
+return s->enabled_capabilities[MIGRATION_CAPABILITY_POSTCOPY_BLOCKTIME];
+}
+
 bool migrate_use_compression(void)
 {
 MigrationState *s;
diff --git a/migration/migration.h b/migration/migration.h
index 8d2f320..46a50bc 100644
--- a/migration/migration.h
+++ b/migration/migration.h
@@ -230,6 +230,7 @@ int migrate_compress_level(void);
 int migrate_compress_threads(void);
 int migrate_decompress_threads(void);
 bool migrate_use_events(void);
+bool migrate_postcopy_blocktime(void);
 
 /* Sending on the return path - generic and then for each message type */
 void migrate_send_rp_shut(MigrationIncomingState *mis,
diff --git a/qapi/migration.json b/qapi/migration.json
index 9d0bf82..24bfc19 100644
--- a/qapi/migration.json
+++ b/qapi/migration.json
@@ -354,16 +354,20 @@
 #
 # @x-multifd: Use more than one fd for migration (since 2.11)
 #
+#
 # @dirty-bitmaps: If enabled, QEMU will migrate named dirty bitmaps.
 # (since 2.12)
 #
+# @postcopy-blocktime: Calculate downtime for postcopy live migration
+# (since 2.13)
+#
 # Since: 1.2
 ##
 { 'enum': 'MigrationCapability',
   'data': ['xbzrle', 'rdma-pin-all', 'auto-converge', 'zero-blocks',
'compress', 'events', 'postcopy-ram', 'x-colo', 'release-ram',
'block', 'return-path', 'pause-before-switchover', 'x-multifd',
-   'dirty-bitmaps' ] }
+   'dirty-bitmaps', 'postcopy-blocktime' ] }
 
 ##
 # @MigrationCapabilityStatus:
-- 
2.7.4

[Qemu-devel] [PATCH v2 2/6] migration: add postcopy blocktime ctx into MigrationIncomingState

[Alexey Perevalov]

This patch adds request to kernel space for UFFD_FEATURE_THREAD_ID, in
case this feature is provided by kernel.

PostcopyBlocktimeContext is encapsulated inside postcopy-ram.c,
due to it being a postcopy-only feature.
Also it defines PostcopyBlocktimeContext's instance live time.
Information from PostcopyBlocktimeContext instance will be provided
much after postcopy migration end, instance of PostcopyBlocktimeContext
will live till QEMU exit, but part of it (vcpu_addr,
page_fault_vcpu_time) used only during calculation, will be released
when postcopy ended or failed.

To enable postcopy blocktime calculation on destination, need to
request proper compatibility (Patch for documentation will be at the
tail of the patch set).

As an example following command enable that capability, assume QEMU was
started with
-chardev socket,id=charmonitor,path=/var/lib/migrate-vm-monitor.sock
option to control it

[root@host]#printf "{\"execute\" : \"qmp_capabilities\"}\r\n \
{\"execute\": \"migrate-set-capabilities\" , \"arguments\":   {
\"capabilities\": [ { \"capability\": \"postcopy-blocktime\", \"state\":
true } ] } }" | nc -U /var/lib/migrate-vm-monitor.sock

Or just with HMP
(qemu) migrate_set_capability postcopy-blocktime on

Reviewed-by: Dr. David Alan Gilbert 
Signed-off-by: Alexey Perevalov 
Reviewed-by: Juan Quintela 
Signed-off-by: Juan Quintela 
---
 migration/migration.h|  8 +++
 migration/postcopy-ram.c | 61 
 2 files changed, 69 insertions(+)

diff --git a/migration/migration.h b/migration/migration.h
index 46a50bc..6d9aaeb 100644
--- a/migration/migration.h
+++ b/migration/migration.h
@@ -22,6 +22,8 @@
 #include "hw/qdev.h"
 #include "io/channel.h"
 
+struct PostcopyBlocktimeContext;
+
 /* State for the incoming migration */
 struct MigrationIncomingState {
 QEMUFile *from_src_file;
@@ -65,6 +67,12 @@ struct MigrationIncomingState {
 /* The coroutine we should enter (back) after failover */
 Coroutine *migration_incoming_co;
 QemuSemaphore colo_incoming_sem;
+
+/*
+ * PostcopyBlocktimeContext to keep information for postcopy
+ * live migration, to calculate vCPU block time
+ * */
+struct PostcopyBlocktimeContext *blocktime_ctx;
 };
 
 MigrationIncomingState *migration_incoming_get_current(void);
diff --git a/migration/postcopy-ram.c b/migration/postcopy-ram.c
index efd7793..66f1df9 100644
--- a/migration/postcopy-ram.c
+++ b/migration/postcopy-ram.c
@@ -90,6 +90,54 @@ int postcopy_notify(enum PostcopyNotifyReason reason, Error 
**errp)
 #include 
 #include 
 
+typedef struct PostcopyBlocktimeContext {
+/* time when page fault initiated per vCPU */
+uint32_t *page_fault_vcpu_time;
+/* page address per vCPU */
+uintptr_t *vcpu_addr;
+uint32_t total_blocktime;
+/* blocktime per vCPU */
+uint32_t *vcpu_blocktime;
+/* point in time when last page fault was initiated */
+uint32_t last_begin;
+/* number of vCPU are suspended */
+int smp_cpus_down;
+uint64_t start_time;
+
+/*
+ * Handler for exit event, necessary for
+ * releasing whole blocktime_ctx
+ */
+Notifier exit_notifier;
+} PostcopyBlocktimeContext;
+
+static void destroy_blocktime_context(struct PostcopyBlocktimeContext *ctx)
+{
+g_free(ctx->page_fault_vcpu_time);
+g_free(ctx->vcpu_addr);
+g_free(ctx->vcpu_blocktime);
+g_free(ctx);
+}
+
+static void migration_exit_cb(Notifier *n, void *data)
+{
+PostcopyBlocktimeContext *ctx = container_of(n, PostcopyBlocktimeContext,
+ exit_notifier);
+destroy_blocktime_context(ctx);
+}
+
+static struct PostcopyBlocktimeContext *blocktime_context_new(void)
+{
+PostcopyBlocktimeContext *ctx = g_new0(PostcopyBlocktimeContext, 1);
+ctx->page_fault_vcpu_time = g_new0(uint32_t, smp_cpus);
+ctx->vcpu_addr = g_new0(uintptr_t, smp_cpus);
+ctx->vcpu_blocktime = g_new0(uint32_t, smp_cpus);
+
+ctx->exit_notifier.notify = migration_exit_cb;
+ctx->start_time = qemu_clock_get_ms(QEMU_CLOCK_REALTIME);
+qemu_add_exit_notifier(&ctx->exit_notifier);
+return ctx;
+}
 
 /**
  * receive_ufd_features: check userfault fd features, to request only supported
@@ -182,6 +230,19 @@ static bool ufd_check_and_apply(int ufd, 
MigrationIncomingState *mis)
 }
 }
 
+#ifdef UFFD_FEATURE_THREAD_ID
+if (migrate_postcopy_blocktime() && mis &&
+UFFD_FEATURE_THREAD_ID & supported_features) {
+/* kernel supports that feature */
+/* don't create blocktime_context if it exists */
+if (!mis->blocktime_ctx) {
+mis->blocktime_ctx = blocktime_context_new();
+}
+
+asked_features |= UFFD_FEATURE_THREAD_ID;
+}
+#endif
+
 /*
  * request features, even if asked_features is 0, due to
  * kernel expects UFFD_API before UFFDIO_REGISTER, per
-- 
2.7.4

[Qemu-devel] [PATCH] linux-user/xtensa: remove stray syscall.h

[Max Filippov]

Signed-off-by: Max Filippov 
---
 linux-user/xtensa/syscall.h | 0
 1 file changed, 0 insertions(+), 0 deletions(-)
 delete mode 100644 linux-user/xtensa/syscall.h

diff --git a/linux-user/xtensa/syscall.h b/linux-user/xtensa/syscall.h
deleted file mode 100644
index e69de29bb2d1..
-- 
2.11.0

[Qemu-devel] [PATCH v2 4/6] migration: postcopy_blocktime documentation

[Alexey Perevalov]

Reviewed-by: Dr. David Alan Gilbert 
Signed-off-by: Alexey Perevalov 
Reviewed-by: Juan Quintela 
Signed-off-by: Juan Quintela 
---
 docs/devel/migration.rst | 14 ++
 1 file changed, 14 insertions(+)

diff --git a/docs/devel/migration.rst b/docs/devel/migration.rst
index e32b087..9342a8a 100644
--- a/docs/devel/migration.rst
+++ b/docs/devel/migration.rst
@@ -401,6 +401,20 @@ will now cause the transition from precopy to postcopy.
 It can be issued immediately after migration is started or any
 time later on.  Issuing it after the end of a migration is harmless.
 
+Blocktime is a postcopy live migration metric, intended to show how
+long the vCPU was in state of interruptable sleep due to pagefault.
+That metric is calculated both for all vCPUs as overlapped value, and
+separately for each vCPU. These values are calculated on destination
+side.  To enable postcopy blocktime calculation, enter following
+command on destination monitor:
+
+``migrate_set_capability postcopy-blocktime on``
+
+Postcopy blocktime can be retrieved by query-migrate qmp command.
+postcopy-blocktime value of qmp command will show overlapped blocking
+time for all vCPU, postcopy-vcpu-blocktime will show list of blocking
+time per vCPU.
+
 .. note::
   During the postcopy phase, the bandwidth limits set using
   ``migrate_set_speed`` is ignored (to avoid delaying requested pages that
-- 
2.7.4

[Qemu-devel] [PATCH v2 3/6] migration: calculate vCPU blocktime on dst side

[Alexey Perevalov]

This patch provides blocktime calculation per vCPU,
as a summary and as a overlapped value for all vCPUs.

This approach was suggested by Peter Xu, as an improvements of
previous approch where QEMU kept tree with faulted page address and cpus bitmask
in it. Now QEMU is keeping array with faulted page address as value and vCPU
as index. It helps to find proper vCPU at UFFD_COPY time. Also it keeps
list for blocktime per vCPU (could be traced with page_fault_addr)

Blocktime will not calculated if postcopy_blocktime field of
MigrationIncomingState wasn't initialized.

Signed-off-by: Alexey Perevalov 
Reviewed-by: Dr. David Alan Gilbert 
Reviewed-by: Juan Quintela 
Signed-off-by: Juan Quintela 
---
 migration/postcopy-ram.c | 151 ++-
 migration/trace-events   |   5 +-
 2 files changed, 154 insertions(+), 2 deletions(-)

diff --git a/migration/postcopy-ram.c b/migration/postcopy-ram.c
index 66f1df9..6b01884 100644
--- a/migration/postcopy-ram.c
+++ b/migration/postcopy-ram.c
@@ -636,6 +636,148 @@ int postcopy_request_shared_page(struct PostCopyFD *pcfd, 
RAMBlock *rb,
 return 0;
 }
 
+static int get_mem_fault_cpu_index(uint32_t pid)
+{
+CPUState *cpu_iter;
+
+CPU_FOREACH(cpu_iter) {
+if (cpu_iter->thread_id == pid) {
+trace_get_mem_fault_cpu_index(cpu_iter->cpu_index, pid);
+return cpu_iter->cpu_index;
+}
+}
+trace_get_mem_fault_cpu_index(-1, pid);
+return -1;
+}
+
+static uint32_t get_low_time_offset(PostcopyBlocktimeContext *dc)
+{
+int64_t start_time_offset = qemu_clock_get_ms(QEMU_CLOCK_REALTIME) -
+dc->start_time;
+return start_time_offset < 1 ? 1 : start_time_offset & UINT32_MAX;
+}
+
+/*
+ * This function is being called when pagefault occurs. It
+ * tracks down vCPU blocking time.
+ *
+ * @addr: faulted host virtual address
+ * @ptid: faulted process thread id
+ * @rb: ramblock appropriate to addr
+ */
+static void mark_postcopy_blocktime_begin(uintptr_t addr, uint32_t ptid,
+  RAMBlock *rb)
+{
+int cpu, already_received;
+MigrationIncomingState *mis = migration_incoming_get_current();
+PostcopyBlocktimeContext *dc = mis->blocktime_ctx;
+uint32_t low_time_offset;
+
+if (!dc || ptid == 0) {
+return;
+}
+cpu = get_mem_fault_cpu_index(ptid);
+if (cpu < 0) {
+return;
+}
+
+low_time_offset = get_low_time_offset(dc);
+if (dc->vcpu_addr[cpu] == 0) {
+atomic_inc(&dc->smp_cpus_down);
+}
+
+atomic_xchg(&dc->last_begin, low_time_offset);
+atomic_xchg(&dc->page_fault_vcpu_time[cpu], low_time_offset);
+atomic_xchg(&dc->vcpu_addr[cpu], addr);
+
+/* check it here, not at the begining of the function,
+ * due to, check could accur early than bitmap_set in
+ * qemu_ufd_copy_ioctl */
+already_received = ramblock_recv_bitmap_test(rb, (void *)addr);
+if (already_received) {
+atomic_xchg(&dc->vcpu_addr[cpu], 0);
+atomic_xchg(&dc->page_fault_vcpu_time[cpu], 0);
+atomic_dec(&dc->smp_cpus_down);
+}
+trace_mark_postcopy_blocktime_begin(addr, dc, 
dc->page_fault_vcpu_time[cpu],
+cpu, already_received);
+}
+
+/*
+ *  This function just provide calculated blocktime per cpu and trace it.
+ *  Total blocktime is calculated in mark_postcopy_blocktime_end.
+ *
+ *
+ * Assume we have 3 CPU
+ *
+ *  S1E1   S1   E1
+ * -***xxx***> CPU1
+ *
+ * S2E2
+ * xxx---> CPU2
+ *
+ * S3E3
+ * xxx---> CPU3
+ *
+ * We have sequence S1,S2,E1,S3,S1,E2,E3,E1
+ * S2,E1 - doesn't match condition due to sequence S1,S2,E1 doesn't include 
CPU3
+ * S3,S1,E2 - sequence includes all CPUs, in this case overlap will be S1,E2 -
+ *it's a part of total blocktime.
+ * S1 - here is last_begin
+ * Legend of the picture is following:
+ *  * - means blocktime per vCPU
+ *  x - means overlapped blocktime (total blocktime)
+ *
+ * @addr: host virtual address
+ */
+static void mark_postcopy_blocktime_end(uintptr_t addr)
+{
+MigrationIncomingState *mis = migration_incoming_get_current();
+PostcopyBlocktimeContext *dc = mis->blocktime_ctx;
+int i, affected_cpu = 0;
+bool vcpu_total_blocktime = false;
+uint32_t read_vcpu_time, low_time_offset;
+
+if (!dc) {
+return;
+}
+
+low_time_offset = get_low_time_offset(dc);
+/* lookup cpu, to clear it,
+ * that algorithm looks straighforward, but it's not
+ * optimal, more optimal algorithm is keeping tree or hash
+ * where key is address value is a list of  */
+for (i = 0; i < smp_cpus; i++) {
+

[Qemu-devel] [PATCH v2 5/6] migration: add blocktime calculation into migration-test

[Alexey Perevalov]

This patch just requests blocktime calculation,
and check it in case when UFFD_FEATURE_THREAD_ID feature is set
on the host.

Reviewed-by: Dr. David Alan Gilbert 
Signed-off-by: Alexey Perevalov 
Reviewed-by: Juan Quintela 
Signed-off-by: Juan Quintela 
---
 tests/migration-test.c | 16 
 1 file changed, 16 insertions(+)

diff --git a/tests/migration-test.c b/tests/migration-test.c
index 422bf1a..dde7c46 100644
--- a/tests/migration-test.c
+++ b/tests/migration-test.c
@@ -26,6 +26,7 @@
 const unsigned start_address = 1024 * 1024;
 const unsigned end_address = 100 * 1024 * 1024;
 bool got_stop;
+static bool uffd_feature_thread_id;
 
 #if defined(__linux__)
 #include 
@@ -55,6 +56,7 @@ static bool ufd_version_check(void)
 g_test_message("Skipping test: UFFDIO_API failed");
 return false;
 }
+uffd_feature_thread_id = api_struct.features & UFFD_FEATURE_THREAD_ID;
 
 ioctl_mask = (__u64)1 << _UFFDIO_REGISTER |
  (__u64)1 << _UFFDIO_UNREGISTER;
@@ -223,6 +225,16 @@ static uint64_t get_migration_pass(QTestState *who)
 return result;
 }
 
+static void read_blocktime(QTestState *who)
+{
+QDict *rsp, *rsp_return;
+
+rsp = wait_command(who, "{ 'execute': 'query-migrate' }");
+rsp_return = qdict_get_qdict(rsp, "return");
+g_assert(qdict_haskey(rsp_return, "postcopy-blocktime"));
+QDECREF(rsp);
+}
+
 static void wait_for_migration_complete(QTestState *who)
 {
 while (true) {
@@ -533,6 +545,7 @@ static void test_migrate(void)
 
 migrate_set_capability(from, "postcopy-ram", "true");
 migrate_set_capability(to, "postcopy-ram", "true");
+migrate_set_capability(to, "postcopy-blocktime", "true");
 
 /* We want to pick a speed slow enough that the test completes
  * quickly, but that it doesn't complete precopy even on a slow
@@ -559,6 +572,9 @@ static void test_migrate(void)
 wait_for_serial("dest_serial");
 wait_for_migration_complete(from);
 
+if (uffd_feature_thread_id) {
+read_blocktime(to);
+}
 g_free(uri);
 
 test_migrate_end(from, to, true);
-- 
2.7.4

[Qemu-devel] [PATCH 3/3] target/xtensa/import_core.sh: fix #include

[Max Filippov]

Change #include  to #include "xtensa-isa.h" in imported
files to make references to local files consistent.

Signed-off-by: Max Filippov 
---
 target/xtensa/import_core.sh | 1 +
 1 file changed, 1 insertion(+)

diff --git a/target/xtensa/import_core.sh b/target/xtensa/import_core.sh
index 58a42987d853..af6c6104792d 100755
--- a/target/xtensa/import_core.sh
+++ b/target/xtensa/import_core.sh
@@ -33,6 +33,7 @@ tar -xf "$OVERLAY" -O binutils/xtensa-modules.c | \
 -e '/^uint32 \*bypass_entry(int i)/,/}/d' \
 -e '/^#include "ansidecl.h"/d' \
 -e '/^Slot_[a-zA-Z0-9_]\+_decode (const xtensa_insnbuf insn)/,/^}/s/^  
return 0;$/  return XTENSA_UNDEFINED;/' \
+-e 's/#include /#include "xtensa-isa.h"/' \
 > "$TARGET"/xtensa-modules.inc.c
 
 cat < "${TARGET}.c"
-- 
2.11.0

[Qemu-devel] [PATCH 2/3] target/xtensa/import_core.sh: fix names of non-top level files

[Max Filippov]

Add .inc. to the names of files imported from configuration overlay in
the import_core.sh script to follow the rule of naming non-top level
source files.

Signed-off-by: Max Filippov 
---
 target/xtensa/import_core.sh | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/target/xtensa/import_core.sh b/target/xtensa/import_core.sh
index 32255eea9b1b..58a42987d853 100755
--- a/target/xtensa/import_core.sh
+++ b/target/xtensa/import_core.sh
@@ -22,7 +22,7 @@ mkdir -p "$TARGET"
 tar -xf "$OVERLAY" -C "$TARGET" --strip-components=1 \
 --xform='s/core/core-isa/' config/core.h
 tar -xf "$OVERLAY" -O gdb/xtensa-config.c | \
-sed -n '1,/*\//p;/XTREG/,/XTREG_END/p' > "$TARGET"/gdb-config.c
+sed -n '1,/*\//p;/XTREG/,/XTREG_END/p' > "$TARGET"/gdb-config.inc.c
 #
 # Fix up known issues in the xtensa-modules.c
 #
@@ -33,7 +33,7 @@ tar -xf "$OVERLAY" -O binutils/xtensa-modules.c | \
 -e '/^uint32 \*bypass_entry(int i)/,/}/d' \
 -e '/^#include "ansidecl.h"/d' \
 -e '/^Slot_[a-zA-Z0-9_]\+_decode (const xtensa_insnbuf insn)/,/^}/s/^  
return 0;$/  return XTENSA_UNDEFINED;/' \
-> "$TARGET"/xtensa-modules.c
+> "$TARGET"/xtensa-modules.inc.c
 
 cat < "${TARGET}.c"
 #include "qemu/osdep.h"
@@ -47,13 +47,13 @@ cat < "${TARGET}.c"
 #include "overlay_tool.h"
 
 #define xtensa_modules xtensa_modules_$NAME
-#include "core-$NAME/xtensa-modules.c"
+#include "core-$NAME/xtensa-modules.inc.c"
 
 static XtensaConfig $NAME __attribute__((unused)) = {
 .name = "$NAME",
 .gdb_regmap = {
 .reg = {
-#include "core-$NAME/gdb-config.c"
+#include "core-$NAME/gdb-config.inc.c"
 }
 },
 .isa_internal = &xtensa_modules,
-- 
2.11.0

[Qemu-devel] [PATCH 1/3] target/xtensa: add .inc. to non-top level source file names

[Max Filippov]

Fix definitions of existing cores and core importing script.

Signed-off-by: Max Filippov 
---
 target/xtensa/core-dc232b.c   | 4 ++--
 target/xtensa/core-dc232b/{gdb-config.c => gdb-config.inc.c}  | 0
 target/xtensa/core-dc232b/{xtensa-modules.c => xtensa-modules.inc.c}  | 0
 target/xtensa/core-dc233c.c   | 4 ++--
 target/xtensa/core-dc233c/{gdb-config.c => gdb-config.inc.c}  | 0
 target/xtensa/core-dc233c/{xtensa-modules.c => xtensa-modules.inc.c}  | 0
 target/xtensa/core-de212.c| 4 ++--
 target/xtensa/core-de212/{gdb-config.c => gdb-config.inc.c}   | 0
 target/xtensa/core-de212/{xtensa-modules.c => xtensa-modules.inc.c}   | 0
 target/xtensa/core-fsf.c  | 2 +-
 target/xtensa/core-fsf/{xtensa-modules.c => xtensa-modules.inc.c} | 0
 target/xtensa/core-sample_controller.c| 4 ++--
 .../xtensa/core-sample_controller/{gdb-config.c => gdb-config.inc.c}  | 0
 .../core-sample_controller/{xtensa-modules.c => xtensa-modules.inc.c} | 0
 14 files changed, 9 insertions(+), 9 deletions(-)
 rename target/xtensa/core-dc232b/{gdb-config.c => gdb-config.inc.c} (100%)
 rename target/xtensa/core-dc232b/{xtensa-modules.c => xtensa-modules.inc.c} 
(100%)
 rename target/xtensa/core-dc233c/{gdb-config.c => gdb-config.inc.c} (100%)
 rename target/xtensa/core-dc233c/{xtensa-modules.c => xtensa-modules.inc.c} 
(100%)
 rename target/xtensa/core-de212/{gdb-config.c => gdb-config.inc.c} (100%)
 rename target/xtensa/core-de212/{xtensa-modules.c => xtensa-modules.inc.c} 
(100%)
 rename target/xtensa/core-fsf/{xtensa-modules.c => xtensa-modules.inc.c} (100%)
 rename target/xtensa/core-sample_controller/{gdb-config.c => gdb-config.inc.c} 
(100%)
 rename target/xtensa/core-sample_controller/{xtensa-modules.c => 
xtensa-modules.inc.c} (100%)

diff --git a/target/xtensa/core-dc232b.c b/target/xtensa/core-dc232b.c
index fe80582df465..aa07018af4e7 100644
--- a/target/xtensa/core-dc232b.c
+++ b/target/xtensa/core-dc232b.c
@@ -35,7 +35,7 @@
 #include "overlay_tool.h"
 
 #define xtensa_modules xtensa_modules_dc232b
-#include "core-dc232b/xtensa-modules.c"
+#include "core-dc232b/xtensa-modules.inc.c"
 
 static XtensaConfig dc232b __attribute__((unused)) = {
 .name = "dc232b",
@@ -43,7 +43,7 @@ static XtensaConfig dc232b __attribute__((unused)) = {
 .num_regs = 120,
 .num_core_regs = 52,
 .reg = {
-#include "core-dc232b/gdb-config.c"
+#include "core-dc232b/gdb-config.inc.c"
 }
 },
 .isa_internal = &xtensa_modules,
diff --git a/target/xtensa/core-dc232b/gdb-config.c 
b/target/xtensa/core-dc232b/gdb-config.inc.c
similarity index 100%
rename from target/xtensa/core-dc232b/gdb-config.c
rename to target/xtensa/core-dc232b/gdb-config.inc.c
diff --git a/target/xtensa/core-dc232b/xtensa-modules.c 
b/target/xtensa/core-dc232b/xtensa-modules.inc.c
similarity index 100%
rename from target/xtensa/core-dc232b/xtensa-modules.c
rename to target/xtensa/core-dc232b/xtensa-modules.inc.c
diff --git a/target/xtensa/core-dc233c.c b/target/xtensa/core-dc233c.c
index 00301c28a2e8..8296e6fa109a 100644
--- a/target/xtensa/core-dc233c.c
+++ b/target/xtensa/core-dc233c.c
@@ -36,7 +36,7 @@
 #include "overlay_tool.h"
 
 #define xtensa_modules xtensa_modules_dc233c
-#include "core-dc233c/xtensa-modules.c"
+#include "core-dc233c/xtensa-modules.inc.c"
 
 static XtensaConfig dc233c __attribute__((unused)) = {
 .name = "dc233c",
@@ -44,7 +44,7 @@ static XtensaConfig dc233c __attribute__((unused)) = {
 .num_regs = 121,
 .num_core_regs = 52,
 .reg = {
-#include "core-dc233c/gdb-config.c"
+#include "core-dc233c/gdb-config.inc.c"
 }
 },
 .isa_internal = &xtensa_modules,
diff --git a/target/xtensa/core-dc233c/gdb-config.c 
b/target/xtensa/core-dc233c/gdb-config.inc.c
similarity index 100%
rename from target/xtensa/core-dc233c/gdb-config.c
rename to target/xtensa/core-dc233c/gdb-config.inc.c
diff --git a/target/xtensa/core-dc233c/xtensa-modules.c 
b/target/xtensa/core-dc233c/xtensa-modules.inc.c
similarity index 100%
rename from target/xtensa/core-dc233c/xtensa-modules.c
rename to target/xtensa/core-dc233c/xtensa-modules.inc.c
diff --git a/target/xtensa/core-de212.c b/target/xtensa/core-de212.c
index 466a467f7fa0..53775a97fae0 100644
--- a/target/xtensa/core-de212.c
+++ b/target/xtensa/core-de212.c
@@ -36,13 +36,13 @@
 #include "overlay_tool.h"
 
 #define xtensa_modules xtensa_modules_de212
-#include "core-de212/xtensa-modules.c"
+#include "core-de212/xtensa-modules.inc.c"
 
 static XtensaConfig de212 __attribute__((unused)) = {
 .name = "de212",
 .gdb_regmap = {
 .reg = {
-#include "core-de212/gdb-config.c"
+#include "core-de212/gdb-config.inc.c"
 }
 },
 .isa_internal = &xtensa_modules,
diff --git a/target/xtensa/core-de212/gdb-config.c 
b/targe

[Qemu-devel] [PATCH 0/3] target/xtensa: improvements for core-specific files

[Max Filippov]

Hello,

this series adds .inc. to the names of non-top level xtensa core-specific
files and fixes script import_core.sh so that it does it automatically.
It also adds a fixup to the script that changes #include 
to #include "xtensa-isa.h".

Max Filippov (3):
  target/xtensa: add .inc. to non-top level source file names
  target/xtensa/import_core.sh: fix names of non-top level files
  target/xtensa/import_core.sh: fix #include 

 target/xtensa/core-dc232b.c  | 4 ++--
 target/xtensa/core-dc232b/{gdb-config.c => gdb-config.inc.c} | 0
 .../core-dc232b/{xtensa-modules.c => xtensa-modules.inc.c}   | 0
 target/xtensa/core-dc233c.c  | 4 ++--
 target/xtensa/core-dc233c/{gdb-config.c => gdb-config.inc.c} | 0
 .../core-dc233c/{xtensa-modules.c => xtensa-modules.inc.c}   | 0
 target/xtensa/core-de212.c   | 4 ++--
 target/xtensa/core-de212/{gdb-config.c => gdb-config.inc.c}  | 0
 .../xtensa/core-de212/{xtensa-modules.c => xtensa-modules.inc.c} | 0
 target/xtensa/core-fsf.c | 2 +-
 .../xtensa/core-fsf/{xtensa-modules.c => xtensa-modules.inc.c}   | 0
 target/xtensa/core-sample_controller.c   | 4 ++--
 .../core-sample_controller/{gdb-config.c => gdb-config.inc.c}| 0
 .../{xtensa-modules.c => xtensa-modules.inc.c}   | 0
 target/xtensa/import_core.sh | 9 +
 15 files changed, 14 insertions(+), 13 deletions(-)
 rename target/xtensa/core-dc232b/{gdb-config.c => gdb-config.inc.c} (100%)
 rename target/xtensa/core-dc232b/{xtensa-modules.c => xtensa-modules.inc.c} 
(100%)
 rename target/xtensa/core-dc233c/{gdb-config.c => gdb-config.inc.c} (100%)
 rename target/xtensa/core-dc233c/{xtensa-modules.c => xtensa-modules.inc.c} 
(100%)
 rename target/xtensa/core-de212/{gdb-config.c => gdb-config.inc.c} (100%)
 rename target/xtensa/core-de212/{xtensa-modules.c => xtensa-modules.inc.c} 
(100%)
 rename target/xtensa/core-fsf/{xtensa-modules.c => xtensa-modules.inc.c} (100%)
 rename target/xtensa/core-sample_controller/{gdb-config.c => gdb-config.inc.c} 
(100%)
 rename target/xtensa/core-sample_controller/{xtensa-modules.c => 
xtensa-modules.inc.c} (100%)

-- 
2.11.0

Re: [Qemu-devel] [PATCH 0/4] coccinelle: re-run scripts from scripts/coccinelle

[Peter Maydell]

On 22 March 2018 at 17:57,   wrote:
> Checking PATCH 4/4: Remove unnecessary variables for function return value...
> ERROR: return is not a function, parentheses are not required
> #251: FILE: target/mips/dsp_helper.c:3281:
> +return (temp[1] << 63) | (temp[0] >> 1);

This looks like a bug in checkpatch. I guess to fix it you'd need
to make checkpatch count opening and closing parens in the line
to see if it goes to 0 somewhere other than just before the ';'...

thanks
-- PMM

Re: [Qemu-devel] [PATCH 0/4] coccinelle: re-run scripts from scripts/coccinelle

[no-reply]

Hi,

This series seems to have some coding style problems. See output below for
more information:

Type: series
Message-id: 20180322161226.29796-1-lviv...@redhat.com
Subject: [Qemu-devel] [PATCH 0/4] coccinelle: re-run scripts from 
scripts/coccinelle

=== TEST SCRIPT BEGIN ===
#!/bin/bash

BASE=base
n=1
total=$(git log --oneline $BASE.. | wc -l)
failed=0

git config --local diff.renamelimit 0
git config --local diff.renames True
git config --local diff.algorithm histogram

commits="$(git log --format=%H --reverse $BASE..)"
for c in $commits; do
echo "Checking PATCH $n/$total: $(git log -n 1 --format=%s $c)..."
if ! git show $c --format=email | ./scripts/checkpatch.pl --mailback -; then
failed=1
echo
fi
n=$((n+1))
done

exit $failed
=== TEST SCRIPT END ===

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
From https://github.com/patchew-project/qemu
 t [tag update]patchew/20180321144107.22770-1-kw...@redhat.com -> 
patchew/20180321144107.22770-1-kw...@redhat.com
Switched to a new branch 'test'
a1c7275b57 Remove unnecessary variables for function return value
a530fa5a72 qdict: remove useless cast
c1bfe26b11 error: Remove NULL checks on error_propagate() calls
db3d55e58e error: Strip trailing '\n' from error string arguments (again again)

=== OUTPUT BEGIN ===
Checking PATCH 1/4: error: Strip trailing '\n' from error string arguments 
(again again)...
Checking PATCH 2/4: error: Remove NULL checks on error_propagate() calls...
Checking PATCH 3/4: qdict: remove useless cast...
Checking PATCH 4/4: Remove unnecessary variables for function return value...
ERROR: return is not a function, parentheses are not required
#251: FILE: target/mips/dsp_helper.c:3281:
+return (temp[1] << 63) | (temp[0] >> 1);

ERROR: return is not a function, parentheses are not required
#270: FILE: target/mips/dsp_helper.c:3308:
+return (temp[1] << 63) | (temp[0] >> 1);

ERROR: return is not a function, parentheses are not required
#289: FILE: target/mips/dsp_helper.c:3341:
+return (temp[1] << 63) | (temp[0] >> 1);

total: 3 errors, 0 warnings, 813 lines checked

Your patch has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

=== OUTPUT END ===

Test command exited with code: 1


---
Email generated automatically by Patchew [http://patchew.org/].
Please send your feedback to patchew-de...@freelists.org

Re: [Qemu-devel] [PULL 0/5] Multiboot patches

[Peter Maydell]

On 21 March 2018 at 14:41, Kevin Wolf  wrote:
> The following changes since commit f1a63fcfcd92c88be8942b5ae71aef9749a4f135:
>
>   Update version for v2.12.0-rc0 release (2018-03-20 19:04:22 +)
>
> are available in the git repository at:
>
>   git://repo.or.cz/qemu/kevin.git tags/for-upstream
>
> for you to fetch changes up to e2679395d598bd40770c22a793c0152576ac211f:
>
>   tests/multiboot: Add .gitignore (2018-03-21 15:13:40 +0100)
>
> 
> Multiboot patches
>
> 
> Kevin Wolf (5):
>   multiboot: Reject kernels exceeding the address space
>   multiboot: Check validity of mh_header_addr
>   tests/multiboot: Test exit code for every qemu run
>   tests/multiboot: Add tests for the a.out kludge
>   tests/multiboot: Add .gitignore

Applied, thanks.

-- PMM

Re: [Qemu-devel] [PATCH 4/4] Remove unnecessary variables for function return value

[Peter Maydell]

On 22 March 2018 at 17:30, Eric Blake  wrote:
> I'm less certain of whether our Coccinelle scripts have easy ways to exclude
> specific files.  We already have scripts/cocci-macro-file.h to help
> Coccinelle not choke on some our existing files, but I'm not sure if
> Coccinelle has a config-file like way that is easy to maintain as a data
> file in-tree for blacklist files to leave alone (right now, when I run
> Coccinelle, I have to manually remember to pass a long command line cribbed
> out of the commit message of an earlier run to pick up things like
> cocci-macro-file.h, instead of an easy formula that points to a single
> config file to pull in all the usual options).

Coccinelle itself doesn't seem to have a config file mechanism.
We could probably approximate one by using a wrapper script
that skips some files and run the passed spatch file on the rest;
scripts/clean-includes has some "skip files we don't want to try
to run on" code that could perhaps be generalized.

thanks
-- PMM

Re: [Qemu-devel] [PATCH 1/1] block/file-posix.c: fix not reopened lock file descriptor

[Kevin Wolf]

[ Cc: qemu-block ]

Am 22.03.2018 um 18:20 hat Dion Bosschieter geschrieben:
> In commit 244a5668106297378391b768e7288eb157616f64 another
> file descriptor to BDRVRawState is added. When we try to issue the
> reopen command only s->fd is reopened; lock_fd could still hold an old
> file descriptor "possibly" pointing to another file.
> 
> - change raw_reopen_prepare so it checks use_lock from BDRVRawState and
> tries to reopen lock_fd accordingly
> - change raw_reopen_commit so it closes the old lock_fd on use_lock
> 
> Signed-off-by: Dion Bosschieter 

bdrv_reopen() is not meant for opening a different file, it is meant to
change the flags and options of the same file. Do you have a use case
where you would actually need to switch to a different file?

As far as I know, lock_fd was specifically introduced _because_ it stays
the same across reopen, so we don't need a racy release/reacquire pair.
Fam (CCed) should know more.

In any case, doesn't your patch drop all the locks without reacquiring
them on the new lock_fd?

Kevin

>  block/file-posix.c | 25 +
>  1 file changed, 25 insertions(+)
> 
> diff --git a/block/file-posix.c b/block/file-posix.c
> index d7fb772c14..16d83fc49e 100644
> --- a/block/file-posix.c
> +++ b/block/file-posix.c
> @@ -167,6 +167,7 @@ typedef struct BDRVRawState {
>  
>  typedef struct BDRVRawReopenState {
>  int fd;
> +int lock_fd;
>  int open_flags;
>  } BDRVRawReopenState;
>  
> @@ -795,6 +796,7 @@ static int raw_reopen_prepare(BDRVReopenState *state,
>  raw_parse_flags(state->flags, &rs->open_flags);
>  
>  rs->fd = -1;
> +rs->lock_fd = -1;
>  
>  int fcntl_flags = O_APPEND | O_NONBLOCK;
>  #ifdef O_NOATIME
> @@ -820,6 +822,17 @@ static int raw_reopen_prepare(BDRVReopenState *state,
>  rs->fd = -1;
>  }
>  }
> +
> +if (s->use_lock) {
> +rs->lock_fd = qemu_dup(s->lock_fd);
> +if (rs->lock_fd >= 0) {
> +ret = fcntl_setfl(rs->lock_fd, rs->open_flags);
> +if (ret) {
> +qemu_close(rs->lock_fd);
> +rs->lock_fd = -1;
> +}
> +}
> +}
>  }
>  
>  /* If we cannot use fcntl, or fcntl failed, fall back to qemu_open() */
> @@ -835,6 +848,14 @@ static int raw_reopen_prepare(BDRVReopenState *state,
>  error_setg_errno(errp, errno, "Could not reopen file");
>  ret = -1;
>  }
> +
> +if (s->use_lock) {
> +rs->lock_fd = qemu_open(normalized_filename, rs->open_flags);
> +if (rs->lock_fd == -1) {
> +error_setg_errno(errp, errno, "Could not reopen file for 
> locking");
> +ret = -1;
> +}
> +}
>  }
>  }
>  
> @@ -861,7 +882,11 @@ static void raw_reopen_commit(BDRVReopenState *state)
>  s->open_flags = rs->open_flags;
>  
>  qemu_close(s->fd);
> +if (s->use_lock) {
> +qemu_close(s->lock_fd);
> +}
>  s->fd = rs->fd;
> +s->lock_fd = rs->lock_fd;
>  
>  g_free(state->opaque);
>  state->opaque = NULL;
> -- 
> 2.14.2
> 

Re: [Qemu-devel] [PATCH v3 1/2] i386/kvm: add support for Hyper-V reenlightenment MSRs

[Vitaly Kuznetsov]

Marcelo Tosatti  writes:

> On Tue, Mar 20, 2018 at 06:34:59PM +0100, Vitaly Kuznetsov wrote:
>> KVM recently gained support for Hyper-V Reenlightenment MSRs which are
>> required to make KVM-on-Hyper-V enable TSC page clocksource to its guests
>> when INVTSC is not passed to it (and it is not passed by default in Qemu
>> as it effectively blocks migration).
>
> Hi Vitaly,
>
> From Microsoft's documentation:
>
> "An L1 hypervisor can request to be notified when its partition is
> migrated. This capability is enumerated in CPUID as
> AccessReenlightenmentControls privilege (see 2.4.10)."
>
> The L0 hypervisor exposes a synthetic MSR
> (HV_X64_MSR_REENLIGHTENMENT_CONTROL) that may be used by the L1
> hypervisor to configure an interrupt vector and target processor. The L0
> hypervisor will inject an interrupt with the specified vector after each
> migration.
>
> What prevents a guest from setting the enable bit, and expect
> to receive an interrupt, if the reenlightenment MSRs are exposed ?
>

This is actually desired: Hyper-V on KVM will set this bit and expect to
receive an interrupt. Currently, we don't send it because we don't
migrate nested workloads but eventually, when we learn how to do this in
KVM, sending an interrupt and doint TSC access emulation will be required.

Normal Windows on KVM won't use the feature as it doesn't need it: upon
migration we update TSC page in KVM and readings from it stay correct.

-- 
  Vitaly

Re: [Qemu-devel] [PATCH 4/4] Remove unnecessary variables for function return value

[Eric Blake]

On 03/22/2018 12:19 PM, Max Filippov wrote:


These files were autogenerated, fixing them doesn't make much sense.


Good to know. They have been already updated by a couple of patches:

8f0a3716e4 Clean up includes
d8e39b7062 Use #include "..." for our own headers, <...> for others

Perhaps import_core.sh can be updated?


Ok, I can add a fixup that changes #include  to #include
"xtensa-isa.h".
Adding #include "qemu/osdep.h" there seems pointless to me.


Or we can add more exceptions to our tooling to recognize the files as 
generated.  The scripts/clean-includes script knows to leave certain 
files alone, so add your files to that list.


I'm less certain of whether our Coccinelle scripts have easy ways to 
exclude specific files.  We already have scripts/cocci-macro-file.h to 
help Coccinelle not choke on some our existing files, but I'm not sure 
if Coccinelle has a config-file like way that is easy to maintain as a 
data file in-tree for blacklist files to leave alone (right now, when I 
run Coccinelle, I have to manually remember to pass a long command line 
cribbed out of the commit message of an earlier run to pick up things 
like cocci-macro-file.h, instead of an easy formula that points to a 
single config file to pull in all the usual options).


--
Eric Blake, Principal Software Engineer
Red Hat, Inc.   +1-919-301-3266
Virtualization:  qemu.org | libvirt.org

Re: [Qemu-devel] [PATCH 4/4] Remove unnecessary variables for function return value

[Max Filippov]

On Thu, Mar 22, 2018 at 10:23 AM, Peter Maydell
 wrote:
> On 22 March 2018 at 17:19, Max Filippov  wrote:
>> Ok, I can add a fixup that changes #include  to #include
>> "xtensa-isa.h".
>> Adding #include "qemu/osdep.h" there seems pointless to me.
>
> Every top level .c file must start with including osdep.h.
> Other headers that it might include rely on that.
> It looks like these files aren't actually top level .c files,
> but are only compiled by being #included from some other file.
> Our convention for that kind of thing is to give the file
> a .inc.c extension. If you follow that then clean-includes
> will skip the file rather than trying to apply the requirements
> for a top level .c file to it.

Ok, thanks. Will do that too.

-- 
Thanks.
-- Max

Re: [Qemu-devel] [PATCH 4/4] Remove unnecessary variables for function return value

[Peter Maydell]

On 22 March 2018 at 17:19, Max Filippov  wrote:
> Ok, I can add a fixup that changes #include  to #include
> "xtensa-isa.h".
> Adding #include "qemu/osdep.h" there seems pointless to me.

Every top level .c file must start with including osdep.h.
Other headers that it might include rely on that.
It looks like these files aren't actually top level .c files,
but are only compiled by being #included from some other file.
Our convention for that kind of thing is to give the file
a .inc.c extension. If you follow that then clean-includes
will skip the file rather than trying to apply the requirements
for a top level .c file to it.

thanks
-- PMM

Re: [Qemu-devel] [PATCH 4/4] Remove unnecessary variables for function return value

[Max Filippov]

On Thu, Mar 22, 2018 at 9:58 AM, Laurent Vivier  wrote:
> On 22/03/2018 17:51, Max Filippov wrote:
>> On Thu, Mar 22, 2018 at 9:12 AM, Laurent Vivier  wrote:
>>> Re-run Coccinelle script scripts/coccinelle/return_directly.cocci
>>>
>>> Signed-off-by: Laurent Vivier 
>>> ---
>>>  target/xtensa/core-dc232b/xtensa-modules.c | 56 
>>> ++
>>>  target/xtensa/core-dc233c/xtensa-modules.c | 56 
>>> ++
>>>  target/xtensa/core-de212/xtensa-modules.c  | 48 +--
>>>  target/xtensa/core-fsf/xtensa-modules.c| 32 -
>>>  .../xtensa/core-sample_controller/xtensa-modules.c | 24 +++---
>>
>> These files were autogenerated, fixing them doesn't make much sense.
>
> Good to know. They have been already updated by a couple of patches:
>
> 8f0a3716e4 Clean up includes
> d8e39b7062 Use #include "..." for our own headers, <...> for others
>
> Perhaps import_core.sh can be updated?

Ok, I can add a fixup that changes #include  to #include
"xtensa-isa.h".
Adding #include "qemu/osdep.h" there seems pointless to me.

-- 
Thanks.
-- Max

Re: [Qemu-devel] [PATCH v3 1/2] i386/kvm: add support for Hyper-V reenlightenment MSRs

[Marcelo Tosatti]

On Tue, Mar 20, 2018 at 06:34:59PM +0100, Vitaly Kuznetsov wrote:
> KVM recently gained support for Hyper-V Reenlightenment MSRs which are
> required to make KVM-on-Hyper-V enable TSC page clocksource to its guests
> when INVTSC is not passed to it (and it is not passed by default in Qemu
> as it effectively blocks migration).

Hi Vitaly,

>From Microsoft's documentation:

"An L1 hypervisor can request to be notified when its partition is
migrated. This capability is enumerated in CPUID as
AccessReenlightenmentControls privilege (see 2.4.10)."

The L0 hypervisor exposes a synthetic MSR
(HV_X64_MSR_REENLIGHTENMENT_CONTROL) that may be used by the L1
hypervisor to configure an interrupt vector and target processor. The L0
hypervisor will inject an interrupt with the specified vector after each
migration.

What prevents a guest from setting the enable bit, and expect
to receive an interrupt, if the reenlightenment MSRs are exposed ?

> 
> Signed-off-by: Vitaly Kuznetsov 
> ---
> Changes since v2:
> - add hv-reenlightenment CPU property [Roman Kagan, Paolo Bonzini]
> - add a comment to feature_word_info [Roman Kagan]
> ---
>  target/i386/cpu.c  |  4 +++-
>  target/i386/cpu.h  |  4 
>  target/i386/hyperv-proto.h |  9 -
>  target/i386/kvm.c  | 39 ++-
>  target/i386/machine.c  | 24 
>  5 files changed, 77 insertions(+), 3 deletions(-)
> 
> diff --git a/target/i386/cpu.c b/target/i386/cpu.c
> index 6bb4ce8719..02579f8234 100644
> --- a/target/i386/cpu.c
> +++ b/target/i386/cpu.c
> @@ -407,7 +407,8 @@ static FeatureWordInfo feature_word_info[FEATURE_WORDS] = 
> {
>  NULL /* hv_vpindex_access */, NULL /* hv_msr_reset_access */,
>  NULL /* hv_msr_stats_access */, NULL /* hv_reftsc_access */,
>  NULL /* hv_msr_idle_access */, NULL /* hv_msr_frequency_access 
> */,
> -NULL, NULL, NULL, NULL,
> +NULL /* hv_msr_debug_access */, NULL /* 
> hv_msr_reenlightenment_access */,
> +NULL, NULL,
>  NULL, NULL, NULL, NULL,
>  NULL, NULL, NULL, NULL,
>  NULL, NULL, NULL, NULL,
> @@ -4764,6 +4765,7 @@ static Property x86_cpu_properties[] = {
>  DEFINE_PROP_BOOL("hv-runtime", X86CPU, hyperv_runtime, false),
>  DEFINE_PROP_BOOL("hv-synic", X86CPU, hyperv_synic, false),
>  DEFINE_PROP_BOOL("hv-stimer", X86CPU, hyperv_stimer, false),
> +DEFINE_PROP_BOOL("hv-reenlightenment", X86CPU, hyperv_reenlightenment, 
> false),
>  DEFINE_PROP_BOOL("check", X86CPU, check_cpuid, true),
>  DEFINE_PROP_BOOL("enforce", X86CPU, enforce_cpuid, false),
>  DEFINE_PROP_BOOL("kvm", X86CPU, expose_kvm, true),
> diff --git a/target/i386/cpu.h b/target/i386/cpu.h
> index 2e2bab5ff3..98eed72937 100644
> --- a/target/i386/cpu.h
> +++ b/target/i386/cpu.h
> @@ -1174,6 +1174,9 @@ typedef struct CPUX86State {
>  uint64_t msr_hv_synic_sint[HV_SINT_COUNT];
>  uint64_t msr_hv_stimer_config[HV_STIMER_COUNT];
>  uint64_t msr_hv_stimer_count[HV_STIMER_COUNT];
> +uint64_t msr_hv_reenlightenment_control;
> +uint64_t msr_hv_tsc_emulation_control;
> +uint64_t msr_hv_tsc_emulation_status;
>  
>  uint64_t msr_rtit_ctrl;
>  uint64_t msr_rtit_status;
> @@ -1296,6 +1299,7 @@ struct X86CPU {
>  bool hyperv_runtime;
>  bool hyperv_synic;
>  bool hyperv_stimer;
> +bool hyperv_reenlightenment;
>  bool check_cpuid;
>  bool enforce_cpuid;
>  bool expose_kvm;
> diff --git a/target/i386/hyperv-proto.h b/target/i386/hyperv-proto.h
> index cb4d7f2b7a..93352ebd2a 100644
> --- a/target/i386/hyperv-proto.h
> +++ b/target/i386/hyperv-proto.h
> @@ -35,7 +35,7 @@
>  #define HV_RESET_AVAILABLE   (1u << 7)
>  #define HV_REFERENCE_TSC_AVAILABLE   (1u << 9)
>  #define HV_ACCESS_FREQUENCY_MSRS (1u << 11)
> -
> +#define HV_ACCESS_REENLIGHTENMENTS_CONTROL  (1u << 13)
>  
>  /*
>   * HV_CPUID_FEATURES.EDX bits
> @@ -129,6 +129,13 @@
>  #define HV_X64_MSR_CRASH_CTL0x4105
>  #define HV_CRASH_CTL_NOTIFY (1ull << 63)
>  
> +/*
> + * Reenlightenment notification MSRs
> + */
> +#define HV_X64_MSR_REENLIGHTENMENT_CONTROL  0x4106
> +#define HV_X64_MSR_TSC_EMULATION_CONTROL0x4107
> +#define HV_X64_MSR_TSC_EMULATION_STATUS 0x4108
> +
>  /*
>   * Hypercall status code
>   */
> diff --git a/target/i386/kvm.c b/target/i386/kvm.c
> index d23fff12f5..7d9f9ca0b1 100644
> --- a/target/i386/kvm.c
> +++ b/target/i386/kvm.c
> @@ -90,6 +90,7 @@ static bool has_msr_hv_runtime;
>  static bool has_msr_hv_synic;
>  static bool has_msr_hv_stimer;
>  static bool has_msr_hv_frequencies;
> +static bool has_msr_hv_reenlightenment;
>  static bool has_msr_xss;
>  static bool has_msr_spec_ctrl;
>  static bool has_msr_smi_count;
> @@ -583,7 +584,8 @@ static bool hyperv_enabled(X86CPU *cpu)
>  cpu->hyperv_vpindex ||
>  cpu->hyperv_runtime 

Re: [Qemu-devel] [PATCH 4/4] Remove unnecessary variables for function return value

[Max Filippov]

On Thu, Mar 22, 2018 at 9:59 AM, Eric Blake  wrote:
> On 03/22/2018 11:51 AM, Max Filippov wrote:
>>
>> On Thu, Mar 22, 2018 at 9:12 AM, Laurent Vivier 
>> wrote:
>>>
>>> Re-run Coccinelle script scripts/coccinelle/return_directly.cocci
>>>
>>> Signed-off-by: Laurent Vivier 
>>> ---
>>>   target/xtensa/core-dc232b/xtensa-modules.c | 56
>>> ++
>>>   target/xtensa/core-dc233c/xtensa-modules.c | 56
>>> ++
>>>   target/xtensa/core-de212/xtensa-modules.c  | 48
>>> +--
>>>   target/xtensa/core-fsf/xtensa-modules.c| 32 -
>>>   .../xtensa/core-sample_controller/xtensa-modules.c | 24 +++---
>>
>>
>> These files were autogenerated, fixing them doesn't make much sense.
>
>
> How frequently is the generator rerun?  Is it something where we are likely
> to revert the change because it needs to be rerun soon?  If so, then is it
> worth fixing the generator to output more concise code?

They were generated once and are not supposed to be regenerated.

> Conversely, if they were generated up front, but likely to remain unchanged
> into the future, then fixing them (even though the fix differs from the
> generator) will mean they no longer show up as false positives in future
> runs of the Coccinelle script.

Ok.

> I'm also fine removing the changes to these files as part of preparing the
> PULL request, if that's what you would prefer.

The changes are fine, if they make maintenance easier they should stay.

-- 
Thanks.
-- Max

Re: [Qemu-devel] [PATCH 4/4] Remove unnecessary variables for function return value

[Eric Blake]

On 03/22/2018 11:51 AM, Max Filippov wrote:

On Thu, Mar 22, 2018 at 9:12 AM, Laurent Vivier  wrote:

Re-run Coccinelle script scripts/coccinelle/return_directly.cocci

Signed-off-by: Laurent Vivier 
---
  target/xtensa/core-dc232b/xtensa-modules.c | 56 ++
  target/xtensa/core-dc233c/xtensa-modules.c | 56 ++
  target/xtensa/core-de212/xtensa-modules.c  | 48 +--
  target/xtensa/core-fsf/xtensa-modules.c| 32 -
  .../xtensa/core-sample_controller/xtensa-modules.c | 24 +++---


These files were autogenerated, fixing them doesn't make much sense.


How frequently is the generator rerun?  Is it something where we are 
likely to revert the change because it needs to be rerun soon?  If so, 
then is it worth fixing the generator to output more concise code?


Conversely, if they were generated up front, but likely to remain 
unchanged into the future, then fixing them (even though the fix differs 
from the generator) will mean they no longer show up as false positives 
in future runs of the Coccinelle script.


I'm also fine removing the changes to these files as part of preparing 
the PULL request, if that's what you would prefer.


--
Eric Blake, Principal Software Engineer
Red Hat, Inc.   +1-919-301-3266
Virtualization:  qemu.org | libvirt.org

Re: [Qemu-devel] [PATCH 4/4] Remove unnecessary variables for function return value

[Laurent Vivier]

On 22/03/2018 17:51, Max Filippov wrote:
> On Thu, Mar 22, 2018 at 9:12 AM, Laurent Vivier  wrote:
>> Re-run Coccinelle script scripts/coccinelle/return_directly.cocci
>>
>> Signed-off-by: Laurent Vivier 
>> ---
>>  target/xtensa/core-dc232b/xtensa-modules.c | 56 
>> ++
>>  target/xtensa/core-dc233c/xtensa-modules.c | 56 
>> ++
>>  target/xtensa/core-de212/xtensa-modules.c  | 48 +--
>>  target/xtensa/core-fsf/xtensa-modules.c| 32 -
>>  .../xtensa/core-sample_controller/xtensa-modules.c | 24 +++---
> 
> These files were autogenerated, fixing them doesn't make much sense.

Good to know. They have been already updated by a couple of patches:

8f0a3716e4 Clean up includes
d8e39b7062 Use #include "..." for our own headers, <...> for others

Perhaps import_core.sh can be updated?

Thanks,
Laurent

Re: [Qemu-devel] [PATCH 4/4] Remove unnecessary variables for function return value

[Max Filippov]

On Thu, Mar 22, 2018 at 9:12 AM, Laurent Vivier  wrote:
> Re-run Coccinelle script scripts/coccinelle/return_directly.cocci
>
> Signed-off-by: Laurent Vivier 
> ---
>  target/xtensa/core-dc232b/xtensa-modules.c | 56 
> ++
>  target/xtensa/core-dc233c/xtensa-modules.c | 56 
> ++
>  target/xtensa/core-de212/xtensa-modules.c  | 48 +--
>  target/xtensa/core-fsf/xtensa-modules.c| 32 -
>  .../xtensa/core-sample_controller/xtensa-modules.c | 24 +++---

These files were autogenerated, fixing them doesn't make much sense.

-- 
Thanks.
-- Max

Re: [Qemu-devel] [PULL 12/13] page_unprotect(): handle calls to pages that are PAGE_WRITE

[Laurent Vivier]

Le 22/03/2018 à 12:13, Laurent Vivier a écrit :
> Le 22/03/2018 à 12:07, Peter Maydell a écrit :
>> On 22 March 2018 at 11:05, Peter Maydell  wrote:
>>> On 22 March 2018 at 10:36, Laurent Vivier  wrote:
 It goes wrong in this part:

 + */
 +if (is_write && info->si_signo == SIGSEGV && info->si_code ==
 SEGV_ACCERR &&
 +h2g_valid(address)) {

 Because, on ppc, si_code is SEGV_MAPERR and not SEGV_ACCERR
 (on x86_64, si_code is SEGV_ACCERR as expected)
>>>
>>> So on PPC if you have a page mapped, and you access it with
>>> the wrong permissions, you get SEGV_MAPERR? This seems like
>>> a host kernel bug to me.
>>
>> ...in particular, kernel commit ecb101aed86156e (dated Dec 2017)
>> fixes a regression introduced in commit c3350602e876 that broke
>> the ppc kernels so they started returning SEGV_MAPERR here
>> instead of SEGV_ACCERR. Presumably your host kernel is missing
>> this fix.
> 
> Yes, you're right, my kernel is 4.14-rc1 (6e80ecd) with
> c3350602e876 but without ecb101aed86156e.
> 
> I'm going to update it.

Re-tested with 4.16-rc6 on ppc32 and it works fine.

Thanks,
Laurent

Re: [Qemu-devel] [PATCH 0/4] coccinelle: re-run scripts from scripts/coccinelle

[Eric Blake]

On 03/22/2018 11:12 AM, Laurent Vivier wrote:

I've re-run some scripts from the coccinelle directory,
and they have found some problems.

This series fixes them.

Laurent Vivier (4):
   error: Strip trailing '\n' from error string arguments (again again)
   error: Remove NULL checks on error_propagate() calls
   qdict: remove useless cast
   Remove unnecessary variables for function return value


I've gone through all the patches to double-check for no surprises; and 
found some formatting nits on 4 that can be touched up on pull request. 
Series:

Reviewed-by: Eric Blake 

--
Eric Blake, Principal Software Engineer
Red Hat, Inc.   +1-919-301-3266
Virtualization:  qemu.org | libvirt.org

Re: [Qemu-devel] [PATCH 4/4] qapi: new qmp command nbd-server-add-bitmap

[Vladimir Sementsov-Ogievskiy]

22.03.2018 19:19, Eric Blake wrote:

On 03/22/2018 10:43 AM, Vladimir Sementsov-Ogievskiy wrote:


+# Returns: error on one of the following conditions:
+#   - the server is not running
+#   - export is not found
+#   - bitmap is not found
+#   - bitmap is disabled
+#   - bitmap is locked


Do we really need to list all the error conditions?  My worry is 
that a list this specific might go stale, compared to the obvious 
default that the command succeeds only if it was able to expose the 
bitmap and that the error message is specific enough for a human to 
figure out what to fix if it failed.



Hmm, I've just doing it similar with other commands in the file. Is 
there any requirements on this part of qapi documentation? I can 
write only "# Returns: nothing on success" line, is it appropriate? 
blockdev-mirror do so, but other commands tries to describe errors. 
Looks like we lack some specified format for return code description 
like we have for parameters..


Yeah, for returns, it's been very ad hoc.  My personal feel (although 
it's not very well documented and certainly not enforced): all 
commands can reasonably return errors, presumably for a good reason; 
but exhaustively auditing WHICH errors is a huge task with little 
benefits. A few commands return non-generic errors, but if all error 
paths used error_setg(), there's nothing that a machine can do to tell 
the difference between the errors, so documenting different error 
reasons doesn't add much.


Thus, if a command returns nothing on success, I'm fine with omitting 
'Returns:' entirely, and the doc generator permits that. But if you 
have bothered to list Returns: for certain errors, I'm not going to 
blindly throw away the documentation work, even though the list may 
become incomplete over time.




So, only something interesting worth documenting.

Hmm, interesting: consider for example bloc-dirty-bitmap-remove. It says:
# Returns: nothing on success
#  If @node is not a valid block device or node, DeviceNotFound

But the code uses bdrv_lookup_bs, which uses simple error_setg, so it 
will return GenericError, isn't it? And, it's not the only place..


--
Best regards,
Vladimir

Re: [Qemu-devel] [PATCH 4/4] Remove unnecessary variables for function return value

[Eric Blake]

On 03/22/2018 11:12 AM, Laurent Vivier wrote:

Re-run Coccinelle script scripts/coccinelle/return_directly.cocci

Signed-off-by: Laurent Vivier 
---



+++ b/hw/arm/exynos4210.c
@@ -156,12 +156,7 @@ void exynos4210_write_secondary(ARMCPU *cpu,
  
  static uint64_t exynos4210_calc_affinity(int cpu)

  {
-uint64_t mp_affinity;
-
-/* Exynos4210 has 0x9 as cluster ID */
-mp_affinity = (0x9 << ARM_AFF1_SHIFT) | cpu;
-
-return mp_affinity;
+return (0x9 << ARM_AFF1_SHIFT) | cpu;


Unchanged by this rewrite, but since this is converting a signed 32-bit 
int to an unsigned 64-bit value, are we sure that the upper 32 bits are 
always set correctly?  (Using unsigned values earlier in the expression 
would require less head-scratching on whether it is correct).  Any 
changes should be a separate fix by the file's maintainer.




+++ b/hw/misc/mos6522.c
@@ -176,12 +176,8 @@ static void mos6522_set_sr_int(MOS6522State *s)
  
  static uint64_t mos6522_get_counter_value(MOS6522State *s, MOS6522Timer *ti)

  {
-uint64_t d;
-
-d = muldiv64(qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL) - ti->load_time,
+return muldiv64(qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL) - ti->load_time,
   ti->frequency, NANOSECONDS_PER_SECOND);


Coccinelle missed that indentation is now off here.


+++ b/hw/ppc/pnv_lpc.c
@@ -125,25 +125,15 @@ static int pnv_lpc_dt_xscom(PnvXScomInterface *dev, void 
*fdt, int xscom_offset)
  static bool opb_read(PnvLpcController *lpc, uint32_t addr, uint8_t *data,
   int sz)
  {
-bool success;
-
-/* XXX Handle access size limits and FW read caching here */
-success = !address_space_rw(&lpc->opb_as, addr, MEMTXATTRS_UNSPECIFIED,
+return !address_space_rw(&lpc->opb_as, addr, MEMTXATTRS_UNSPECIFIED,
  data, sz, false);


and here.


+++ b/target/xtensa/translate.c
@@ -1272,11 +1272,8 @@ XtensaOpcodeOps *
  xtensa_find_opcode_ops(const XtensaOpcodeTranslators *t,
 const char *name)
  {
-XtensaOpcodeOps *ops;
-
-ops = bsearch(name, t->opcode, t->num_opcodes,
+return bsearch(name, t->opcode, t->num_opcodes,
sizeof(XtensaOpcodeOps), compare_opcode_ops);


and here


--
Eric Blake, Principal Software Engineer
Red Hat, Inc.   +1-919-301-3266
Virtualization:  qemu.org | libvirt.org

Re: [Qemu-devel] [PATCH 0/3] coroutine: avoid co_queue_wakeup recursion

[Paolo Bonzini]

On 22/03/2018 16:28, Stefan Hajnoczi wrote:
> co_queue_wakeup is currently implemented in a recursive fashion.  Pathological
> patterns of aio_co_enter() between coroutines can cause stack exhaustion.
> 
> This patch series implements co_queue_wakeup iteratively and avoids stack
> exhaustion.
> 
> This issue was originally reported with qemu-img convert but I don't have a
> good reproducer.  See Patch 3 for a test-aio test case instead.
> 
> Stefan Hajnoczi (3):
>   queue: add QSIMPLEQ_PREPEND()
>   coroutine: avoid co_queue_wakeup recursion
>   coroutine: add test-aio coroutine queue chaining test case
> 
>  include/qemu/coroutine_int.h |   1 -
>  include/qemu/queue.h |   8 
>  block/io.c   |   3 +-
>  tests/test-aio.c |  65 -
>  util/qemu-coroutine-lock.c   |  34 -
>  util/qemu-coroutine.c| 110 
> +++
>  6 files changed, 120 insertions(+), 101 deletions(-)
> 

Reviewed-by: Paolo Bonzini 

I was a little surprised by the disappearing of the "do not use co
anymore" comments, but they seem unnecessary indeed with the new code.

Paolo

Re: [Qemu-devel] [PATCH v2 0/6] Extend vhost-user to support VFIO based accelerators

[Michael S. Tsirkin]

On Mon, Mar 19, 2018 at 03:15:31PM +0800, Tiwei Bie wrote:
> This patch set does some small extensions to vhost-user protocol
> to support VFIO based accelerators, and makes it possible to get
> the similar performance of VFIO based PCI passthru while keeping
> the virtio device emulation in QEMU.
> 
> How does accelerator accelerate vhost (data path)
> =
> 
> Any virtio ring compatible devices potentially can be used as the
> vhost data path accelerators. We can setup the accelerator based
> on the informations (e.g. memory table, features, ring info, etc)
> available on the vhost backend. And accelerator will be able to use
> the virtio ring provided by the virtio driver in the VM directly.
> So the virtio driver in the VM can exchange e.g. network packets
> with the accelerator directly via the virtio ring. That is to say,
> we will be able to use the accelerator to accelerate the vhost
> data path. We call it vDPA: vhost Data Path Acceleration.
> 
> Notice: Although the accelerator can talk with the virtio driver
> in the VM via the virtio ring directly. The control path events
> (e.g. device start/stop) in the VM will still be trapped and handled
> by QEMU, and QEMU will deliver such events to the vhost backend
> via standard vhost protocol.
> 
> Below link is an example showing how to setup a such environment
> via nested VM. In this case, the virtio device in the outer VM is
> the accelerator. It will be used to accelerate the virtio device
> in the inner VM. In reality, we could use virtio ring compatible
> hardware device as the accelerators.
> 
> http://dpdk.org/ml/archives/dev/2017-December/085044.html

I understand that it might be challenging due to
the tight coupling with VFIO. Still - isn't there
a way do make it easier to set a testing rig up?

In particular can we avoid the dpdk requirement for testing?



> In above example, it doesn't require any changes to QEMU, but
> it has lower performance compared with the traditional VFIO
> based PCI passthru. And that's the problem this patch set wants
> to solve.
> 
> The performance issue of vDPA/vhost-user and solutions
> ==
> 
> For vhost-user backend, the critical issue in vDPA is that the
> data path performance is relatively low and some host threads are
> needed for the data path, because some necessary mechanisms are
> missing to support:
> 
> 1) guest driver notifies the device directly;
> 2) device interrupts the guest directly;
> 
> So this patch set does some small extensions to the vhost-user
> protocol to make both of them possible. It leverages the same
> mechanisms (e.g. EPT and Posted-Interrupt on Intel platform) as
> the PCI passthru.

Not all platforms support posted interrupts, and EPT isn't
required for MMIO to be mapped to devices.

It probably makes sense to separate the more portable
host notification offload from the less portable
guest notification offload.



> A new protocol feature bit is added to negotiate the accelerator
> feature support. Two new slave message types are added to control
> the notify region and queue interrupt passthru for each queue.
> >From the view of vhost-user protocol design, it's very flexible.
> The passthru can be enabled/disabled for each queue individually,
> and it's possible to accelerate each queue by different devices.
> More design and implementation details can be found from the last
> patch.
> 
> Difference between vDPA and PCI passthru
> 
> 
> The key difference between PCI passthru and vDPA is that, in vDPA
> only the data path of the device (e.g. DMA ring, notify region and
> queue interrupt) is pass-throughed to the VM, the device control
> path (e.g. PCI configuration space and MMIO regions) is still
> defined and emulated by QEMU.
> 
> The benefits of keeping virtio device emulation in QEMU compared
> with virtio device PCI passthru include (but not limit to):
> 
> - consistent device interface for guest OS in the VM;
> - max flexibility on the hardware (i.e. the accelerators) design;
> - leveraging the existing virtio live-migration framework;
> 
> Why extend vhost-user for vDPA
> ==
> 
> We have already implemented various virtual switches (e.g. OVS-DPDK)
> based on vhost-user for VMs in the Cloud. They are purely software
> running on CPU cores. When we have accelerators for such NFVi applications,
> it's ideal if the applications could keep using the original interface
> (i.e. vhost-user netdev) with QEMU, and infrastructure is able to decide
> when and how to switch between CPU and accelerators within the interface.
> And the switching (i.e. switch between CPU and accelerators) can be done
> flexibly and quickly inside the applications.
> 
> More details about this can be found from the Cunming's discussions on
> the RFC patch set.
> 
> Update notes
> 
> 
> IOMMU feature bit check is removed in

Re: [Qemu-devel] [PATCH 0/4] coccinelle: re-run scripts from scripts/coccinelle

[Eric Blake]

On 03/22/2018 11:12 AM, Laurent Vivier wrote:

I've re-run some scripts from the coccinelle directory,
and they have found some problems.

This series fixes them.

Laurent Vivier (4):
   error: Strip trailing '\n' from error string arguments (again again)


This is user-visible, so appropriate during freeze as a bug fix.  It's 
closes to the error/qapi code that Markus normally maintains (and where 
I'm filling in while he is out), so I'll queue this one on my QAPI tree.



   error: Remove NULL checks on error_propagate() calls
   qdict: remove useless cast
   Remove unnecessary variables for function return value


These are cosmetic; they should be no-ops.  2 and 3 are again best 
through error/qapi, while 4 is more generic, but there's little reason 
to split up the series just because 4 is not as closely related.  Unless 
anyone has an opinion otherwise, I'm planning to also include them in my 
QAPI tree for 2.12.


--
Eric Blake, Principal Software Engineer
Red Hat, Inc.   +1-919-301-3266
Virtualization:  qemu.org | libvirt.org

Re: [Qemu-devel] [PATCH 3/4] qdict: remove useless cast

[Dr. David Alan Gilbert]

* Laurent Vivier (lviv...@redhat.com) wrote:
> Re-run Coccinelle script scripts/coccinelle/qobject.cocci
> 
> Signed-off-by: Laurent Vivier 
> ---
>  block/nvme.c | 11 +--
>  monitor.c|  2 +-
>  2 files changed, 6 insertions(+), 7 deletions(-)
> 
> diff --git a/block/nvme.c b/block/nvme.c
> index 8bca57aae6..c4f3a7bc94 100644
> --- a/block/nvme.c
> +++ b/block/nvme.c
> @@ -695,12 +695,11 @@ static void nvme_parse_filename(const char *filename, 
> QDict *options,
>  unsigned long ns;
>  const char *slash = strchr(tmp, '/');
>  if (!slash) {
> -qdict_put(options, NVME_BLOCK_OPT_DEVICE,
> -  qstring_from_str(tmp));
> +qdict_put_str(options, NVME_BLOCK_OPT_DEVICE, tmp);
>  return;
>  }
>  device = g_strndup(tmp, slash - tmp);
> -qdict_put(options, NVME_BLOCK_OPT_DEVICE, qstring_from_str(device));
> +qdict_put_str(options, NVME_BLOCK_OPT_DEVICE, device);
>  g_free(device);
>  namespace = slash + 1;
>  if (*namespace && qemu_strtoul(namespace, NULL, 10, &ns)) {
> @@ -708,8 +707,8 @@ static void nvme_parse_filename(const char *filename, 
> QDict *options,
> namespace);
>  return;
>  }
> -qdict_put(options, NVME_BLOCK_OPT_NAMESPACE,
> -  qstring_from_str(*namespace ? namespace : "1"));
> +qdict_put_str(options, NVME_BLOCK_OPT_NAMESPACE,
> +  *namespace ? namespace : "1");
>  }
>  }
>  
> @@ -1082,7 +1081,7 @@ static void nvme_refresh_filename(BlockDriverState *bs, 
> QDict *opts)
>   bs->drv->format_name);
>  }
>  
> -qdict_put(opts, "driver", qstring_from_str(bs->drv->format_name));
> +qdict_put_str(opts, "driver", bs->drv->format_name);
>  bs->full_open_options = opts;
>  }
>  
> diff --git a/monitor.c b/monitor.c
> index 6ccd2fc089..6e7667d82f 100644
> --- a/monitor.c
> +++ b/monitor.c
> @@ -4316,7 +4316,7 @@ static QObject *get_qmp_greeting(Monitor *mon)
>  /* Monitors that are not using IOThread won't support OOB */
>  continue;
>  }
> -qlist_append(cap_list, qstring_from_str(QMPCapability_str(cap)));
> +qlist_append_str(cap_list, QMPCapability_str(cap));
>  }
>  
>  return qobject_from_jsonf("{'QMP': {'version': %p, 'capabilities': %p}}",

For monitor:

Acked-by: Dr. David Alan Gilbert 

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

Re: [Qemu-devel] [PATCH 4/4] qapi: new qmp command nbd-server-add-bitmap

[Eric Blake]

On 03/22/2018 10:43 AM, Vladimir Sementsov-Ogievskiy wrote:


+# Returns: error on one of the following conditions:
+#   - the server is not running
+#   - export is not found
+#   - bitmap is not found
+#   - bitmap is disabled
+#   - bitmap is locked


Do we really need to list all the error conditions?  My worry is that 
a list this specific might go stale, compared to the obvious default 
that the command succeeds only if it was able to expose the bitmap and 
that the error message is specific enough for a human to figure out 
what to fix if it failed.



Hmm, I've just doing it similar with other commands in the file. Is 
there any requirements on this part of qapi documentation? I can write 
only "# Returns: nothing on success" line, is it appropriate? 
blockdev-mirror do so, but other commands tries to describe errors. 
Looks like we lack some specified format for return code description 
like we have for parameters..


Yeah, for returns, it's been very ad hoc.  My personal feel (although 
it's not very well documented and certainly not enforced): all commands 
can reasonably return errors, presumably for a good reason; but 
exhaustively auditing WHICH errors is a huge task with little benefits. 
A few commands return non-generic errors, but if all error paths used 
error_setg(), there's nothing that a machine can do to tell the 
difference between the errors, so documenting different error reasons 
doesn't add much.


Thus, if a command returns nothing on success, I'm fine with omitting 
'Returns:' entirely, and the doc generator permits that.  But if you 
have bothered to list Returns: for certain errors, I'm not going to 
blindly throw away the documentation work, even though the list may 
become incomplete over time.


--
Eric Blake, Principal Software Engineer
Red Hat, Inc.   +1-919-301-3266
Virtualization:  qemu.org | libvirt.org

Re: [Qemu-devel] [PATCH v2 6/6] vhost-user: add VFIO based accelerators support

[Michael S. Tsirkin]

On Mon, Mar 19, 2018 at 03:15:37PM +0800, Tiwei Bie wrote:
> This patch does some small extensions to vhost-user protocol to
> support VFIO based accelerators, and makes it possible to get the
> similar performance of VFIO based PCI passthru while keeping the
> virtio device emulation in QEMU.
> 
> Any virtio ring compatible devices potentially can be used as the
> vhost data path accelerators. We can setup the accelerator based
> on the informations (e.g. memory table, features, ring info, etc)
> available on the vhost backend. And accelerator will be able to use
> the virtio ring provided by the virtio driver in the VM directly.
> So the virtio driver in the VM can exchange e.g. network packets
> with the accelerator directly via the virtio ring.
> 
> But for vhost-user, the critical issue in this case is that the
> data path performance is relatively low and some host threads are
> needed for the data path, because some necessary mechanisms are
> missing to support:
> 
> 1) guest driver notifies the device directly;
> 2) device interrupts the guest directly;
> 
> So this patch does some small extensions to vhost-user protocol
> to make both of them possible. It leverages the same mechanisms
> as the VFIO based PCI passthru.
> 
> A new protocol feature bit is added to negotiate the accelerator
> feature support. Two new slave message types are added to control
> the notify region and queue interrupt passthru for each queue.
> >From the view of vhost-user protocol design, it's very flexible.
> The passthru can be enabled/disabled for each queue individually,
> and it's possible to accelerate each queue by different devices.
> 
> The key difference with PCI passthru is that, in this case only
> the data path of the device (e.g. DMA ring, notify region and
> queue interrupt) is pass-throughed to the VM, the device control
> path (e.g. PCI configuration space and MMIO regions) is still
> defined and emulated by QEMU.
> 
> The benefits of keeping virtio device emulation in QEMU compared
> with virtio device PCI passthru include (but not limit to):
> 
> - consistent device interface for guest OS in the VM;
> - max flexibility on the hardware (i.e. the accelerators) design;
> - leveraging the existing virtio live-migration framework;
> 
> Normally, vhost-user is meant for connecting to e.g. user-space
> switch which is shared between multiple VMs. Typically, a vhost
> accelerator isn't a simple NIC which is just for packet I/O, but
> e.g. an switch accelerator which is also shared between multiple
> VMs. This commit extends vhost-user to better support connecting
> to e.g. a user-space switch that has an accelerator.
> 
> Signed-off-by: Tiwei Bie 
> ---
>  docs/interop/vhost-user.txt|  57 
>  hw/virtio/vhost-user.c | 198 
> +
>  include/hw/virtio/vhost-user.h |  17 
>  3 files changed, 272 insertions(+)
> 
> diff --git a/docs/interop/vhost-user.txt b/docs/interop/vhost-user.txt
> index cb3a7595aa..264a58a800 100644
> --- a/docs/interop/vhost-user.txt
> +++ b/docs/interop/vhost-user.txt
> @@ -132,6 +132,15 @@ Depending on the request type, payload can be:
> Payload: Size bytes array holding the contents of the virtio
> device's configuration space
>  
> + * Vring area description
> +   ---
> +   | u64 | size | offset |
> +   ---
> +
> +   u64: a 64-bit unsigned integer
> +   Size: a 64-bit size
> +   Offset: a 64-bit offset
> +
>  In QEMU the vhost-user message is implemented with the following struct:
>  
>  typedef struct VhostUserMsg {

I see you modeled this after a generic message such as vring state
description, but that one is used in many msgs, that is
why it is not documented in a single place.

vring address description is a better model for how to document this
message.

> @@ -146,6 +155,7 @@ typedef struct VhostUserMsg {
>  VhostUserLog log;
>  struct vhost_iotlb_msg iotlb;
>  VhostUserConfig config;
> +VhostUserVringArea area;
>  };
>  } QEMU_PACKED VhostUserMsg;
>  
> @@ -358,6 +368,17 @@ The fd is provided via VHOST_USER_SET_SLAVE_REQ_FD 
> ancillary data.
>  A slave may then send VHOST_USER_SLAVE_* messages to the master
>  using this fd communication channel.
>  
> +VFIO based accelerators
> +---
> +
> +The VFIO based accelerators feature is a protocol extension. It is supported
> +when the protocol feature VHOST_USER_PROTOCOL_F_VFIO (bit 7) is set.
> +
> +The vhost-user backend will set the accelerator context via slave channel,
> +and QEMU just needs to handle those messages passively.

I didn't understand the above unfortunately.
accelerator context and passively do not seem to be defined anywhere.
What do these terms mean here?

How is the backend supposed to use this?
Could you describe this in a way that will make it possible
for backend writers to use?


> The accelerator
> +context will be set for each queue indepe

[Qemu-devel] [PATCH 3/4] qdict: remove useless cast

[Laurent Vivier]

Re-run Coccinelle script scripts/coccinelle/qobject.cocci

Signed-off-by: Laurent Vivier 
---
 block/nvme.c | 11 +--
 monitor.c|  2 +-
 2 files changed, 6 insertions(+), 7 deletions(-)

diff --git a/block/nvme.c b/block/nvme.c
index 8bca57aae6..c4f3a7bc94 100644
--- a/block/nvme.c
+++ b/block/nvme.c
@@ -695,12 +695,11 @@ static void nvme_parse_filename(const char *filename, 
QDict *options,
 unsigned long ns;
 const char *slash = strchr(tmp, '/');
 if (!slash) {
-qdict_put(options, NVME_BLOCK_OPT_DEVICE,
-  qstring_from_str(tmp));
+qdict_put_str(options, NVME_BLOCK_OPT_DEVICE, tmp);
 return;
 }
 device = g_strndup(tmp, slash - tmp);
-qdict_put(options, NVME_BLOCK_OPT_DEVICE, qstring_from_str(device));
+qdict_put_str(options, NVME_BLOCK_OPT_DEVICE, device);
 g_free(device);
 namespace = slash + 1;
 if (*namespace && qemu_strtoul(namespace, NULL, 10, &ns)) {
@@ -708,8 +707,8 @@ static void nvme_parse_filename(const char *filename, QDict 
*options,
namespace);
 return;
 }
-qdict_put(options, NVME_BLOCK_OPT_NAMESPACE,
-  qstring_from_str(*namespace ? namespace : "1"));
+qdict_put_str(options, NVME_BLOCK_OPT_NAMESPACE,
+  *namespace ? namespace : "1");
 }
 }
 
@@ -1082,7 +1081,7 @@ static void nvme_refresh_filename(BlockDriverState *bs, 
QDict *opts)
  bs->drv->format_name);
 }
 
-qdict_put(opts, "driver", qstring_from_str(bs->drv->format_name));
+qdict_put_str(opts, "driver", bs->drv->format_name);
 bs->full_open_options = opts;
 }
 
diff --git a/monitor.c b/monitor.c
index 6ccd2fc089..6e7667d82f 100644
--- a/monitor.c
+++ b/monitor.c
@@ -4316,7 +4316,7 @@ static QObject *get_qmp_greeting(Monitor *mon)
 /* Monitors that are not using IOThread won't support OOB */
 continue;
 }
-qlist_append(cap_list, qstring_from_str(QMPCapability_str(cap)));
+qlist_append_str(cap_list, QMPCapability_str(cap));
 }
 
 return qobject_from_jsonf("{'QMP': {'version': %p, 'capabilities': %p}}",
-- 
2.14.3

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

[Laurent Vivier]

Re-run Coccinelle script scripts/coccinelle/err-bad-newline.cocci,
and found new error_report() occurences with '\n'.

Signed-off-by: Laurent Vivier 
---
 target/i386/hvf/hvf.c | 24 
 1 file changed, 12 insertions(+), 12 deletions(-)

diff --git a/target/i386/hvf/hvf.c b/target/i386/hvf/hvf.c
index 15870a4f36..c36753954b 100644
--- a/target/i386/hvf/hvf.c
+++ b/target/i386/hvf/hvf.c
@@ -86,25 +86,25 @@ static void assert_hvf_ok(hv_return_t ret)
 
 switch (ret) {
 case HV_ERROR:
-error_report("Error: HV_ERROR\n");
+error_report("Error: HV_ERROR");
 break;
 case HV_BUSY:
-error_report("Error: HV_BUSY\n");
+error_report("Error: HV_BUSY");
 break;
 case HV_BAD_ARGUMENT:
-error_report("Error: HV_BAD_ARGUMENT\n");
+error_report("Error: HV_BAD_ARGUMENT");
 break;
 case HV_NO_RESOURCES:
-error_report("Error: HV_NO_RESOURCES\n");
+error_report("Error: HV_NO_RESOURCES");
 break;
 case HV_NO_DEVICE:
-error_report("Error: HV_NO_DEVICE\n");
+error_report("Error: HV_NO_DEVICE");
 break;
 case HV_UNSUPPORTED:
-error_report("Error: HV_UNSUPPORTED\n");
+error_report("Error: HV_UNSUPPORTED");
 break;
 default:
-error_report("Unknown Error\n");
+error_report("Unknown Error");
 }
 
 abort();
@@ -191,7 +191,7 @@ void hvf_set_phys_mem(MemoryRegionSection *section, bool 
add)
 if (mem) {
 mem->size = 0;
 if (do_hvf_set_memory(mem)) {
-error_report("Failed to reset overlapping slot\n");
+error_report("Failed to reset overlapping slot");
 abort();
 }
 }
@@ -211,7 +211,7 @@ void hvf_set_phys_mem(MemoryRegionSection *section, bool 
add)
 }
 
 if (x == hvf_state->num_slots) {
-error_report("No free slots\n");
+error_report("No free slots");
 abort();
 }
 
@@ -221,7 +221,7 @@ void hvf_set_phys_mem(MemoryRegionSection *section, bool 
add)
 mem->region = area;
 
 if (do_hvf_set_memory(mem)) {
-error_report("Error registering new memory slot\n");
+error_report("Error registering new memory slot");
 abort();
 }
 }
@@ -884,7 +884,7 @@ int hvf_vcpu_exec(CPUState *cpu)
 break;
 }
 default:
-error_report("Unrecognized CR %d\n", cr);
+error_report("Unrecognized CR %d", cr);
 abort();
 }
 RIP(env) += ins_len;
@@ -930,7 +930,7 @@ int hvf_vcpu_exec(CPUState *cpu)
 env->error_code = 0;
 break;
 default:
-error_report("%llx: unhandled exit %llx\n", rip, exit_reason);
+error_report("%llx: unhandled exit %llx", rip, exit_reason);
 }
 } while (ret == 0);
 
-- 
2.14.3

[Qemu-devel] [PATCH 4/4] Remove unnecessary variables for function return value

[Laurent Vivier]

Re-run Coccinelle script scripts/coccinelle/return_directly.cocci

Signed-off-by: Laurent Vivier 
---
 accel/tcg/translate-all.c  |  5 +-
 block/quorum.c |  6 +--
 hw/arm/exynos4210.c|  7 +--
 hw/block/vhost-user-blk.c  |  5 +-
 hw/hppa/dino.c |  5 +-
 hw/misc/mos6522.c  |  6 +--
 hw/net/ftgmac100.c |  5 +-
 hw/ppc/pnv_lpc.c   | 14 +-
 io/net-listener.c  |  6 +--
 target/i386/hax-darwin.c   | 10 ++--
 target/mips/dsp_helper.c   | 15 ++
 target/xtensa/core-dc232b/xtensa-modules.c | 56 ++
 target/xtensa/core-dc233c/xtensa-modules.c | 56 ++
 target/xtensa/core-de212/xtensa-modules.c  | 48 +--
 target/xtensa/core-fsf/xtensa-modules.c| 32 -
 .../xtensa/core-sample_controller/xtensa-modules.c | 24 +++---
 target/xtensa/translate.c  |  5 +-
 tests/m48t59-test.c|  6 +--
 tests/test-thread-pool.c   |  6 +--
 util/uri.c |  5 +-
 20 files changed, 75 insertions(+), 247 deletions(-)

diff --git a/accel/tcg/translate-all.c b/accel/tcg/translate-all.c
index 5ad1b919bc..55d822d410 100644
--- a/accel/tcg/translate-all.c
+++ b/accel/tcg/translate-all.c
@@ -644,11 +644,8 @@ static inline void *alloc_code_gen_buffer(void)
 static inline void *alloc_code_gen_buffer(void)
 {
 size_t size = tcg_ctx->code_gen_buffer_size;
-void *buf;
-
-buf = VirtualAlloc(NULL, size, MEM_RESERVE | MEM_COMMIT,
+return VirtualAlloc(NULL, size, MEM_RESERVE | MEM_COMMIT,
 PAGE_EXECUTE_READWRITE);
-return buf;
 }
 #else
 static inline void *alloc_code_gen_buffer(void)
diff --git a/block/quorum.c b/block/quorum.c
index 14333c18aa..304442ef65 100644
--- a/block/quorum.c
+++ b/block/quorum.c
@@ -608,7 +608,7 @@ static void read_quorum_children_entry(void *opaque)
 static int read_quorum_children(QuorumAIOCB *acb)
 {
 BDRVQuorumState *s = acb->bs->opaque;
-int i, ret;
+int i;
 
 acb->children_read = s->num_children;
 for (i = 0; i < s->num_children; i++) {
@@ -643,9 +643,7 @@ static int read_quorum_children(QuorumAIOCB *acb)
 qemu_coroutine_yield();
 }
 
-ret = acb->vote_ret;
-
-return ret;
+return acb->vote_ret;
 }
 
 static int read_fifo_child(QuorumAIOCB *acb)
diff --git a/hw/arm/exynos4210.c b/hw/arm/exynos4210.c
index 06f9d1ffa4..d5ce275b21 100644
--- a/hw/arm/exynos4210.c
+++ b/hw/arm/exynos4210.c
@@ -156,12 +156,7 @@ void exynos4210_write_secondary(ARMCPU *cpu,
 
 static uint64_t exynos4210_calc_affinity(int cpu)
 {
-uint64_t mp_affinity;
-
-/* Exynos4210 has 0x9 as cluster ID */
-mp_affinity = (0x9 << ARM_AFF1_SHIFT) | cpu;
-
-return mp_affinity;
+return (0x9 << ARM_AFF1_SHIFT) | cpu;
 }
 
 Exynos4210State *exynos4210_init(MemoryRegion *system_mem)
diff --git a/hw/block/vhost-user-blk.c b/hw/block/vhost-user-blk.c
index f840f07dfe..3f41ca9e26 100644
--- a/hw/block/vhost-user-blk.c
+++ b/hw/block/vhost-user-blk.c
@@ -196,7 +196,6 @@ static uint64_t vhost_user_blk_get_features(VirtIODevice 
*vdev,
 Error **errp)
 {
 VHostUserBlk *s = VHOST_USER_BLK(vdev);
-uint64_t get_features;
 
 /* Turn on pre-defined features */
 virtio_add_feature(&features, VIRTIO_BLK_F_SEG_MAX);
@@ -215,9 +214,7 @@ static uint64_t vhost_user_blk_get_features(VirtIODevice 
*vdev,
 virtio_add_feature(&features, VIRTIO_BLK_F_MQ);
 }
 
-get_features = vhost_get_features(&s->dev, user_feature_bits, features);
-
-return get_features;
+return vhost_get_features(&s->dev, user_feature_bits, features);
 }
 
 static void vhost_user_blk_handle_output(VirtIODevice *vdev, VirtQueue *vq)
diff --git a/hw/hppa/dino.c b/hw/hppa/dino.c
index 15aefde09c..c5dcf3104d 100644
--- a/hw/hppa/dino.c
+++ b/hw/hppa/dino.c
@@ -403,13 +403,10 @@ static void dino_set_irq(void *opaque, int irq, int level)
 static int dino_pci_map_irq(PCIDevice *d, int irq_num)
 {
 int slot = d->devfn >> 3;
-int local_irq;
 
 assert(irq_num >= 0 && irq_num <= 3);
 
-local_irq = slot & 0x03;
-
-return local_irq;
+return slot & 0x03;
 }
 
 static void dino_set_timer_irq(void *opaque, int irq, int level)
diff --git a/hw/misc/mos6522.c b/hw/misc/mos6522.c
index 8ad9fc831e..606532aa65 100644
--- a/hw/misc/mos6522.c
+++ b/hw/misc/mos6522.c
@@ -176,12 +176,8 @@ static void mos6522_set_sr_int(MOS6522State *s)
 
 static uint64_t mos6522_get_counter_value(MOS6522State *s, MOS6522Timer *ti)
 {
-uint64_t d;
-
-d = muldiv64(qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL) - t

[Qemu-devel] [PATCH 0/4] coccinelle: re-run scripts from scripts/coccinelle

[Laurent Vivier]

I've re-run some scripts from the coccinelle directory,
and they have found some problems.

This series fixes them.

Laurent Vivier (4):
  error: Strip trailing '\n' from error string arguments (again again)
  error: Remove NULL checks on error_propagate() calls
  qdict: remove useless cast
  Remove unnecessary variables for function return value

 accel/tcg/translate-all.c  |  5 +-
 block/nvme.c   | 11 ++---
 block/quorum.c |  6 +--
 hw/arm/exynos4210.c|  7 +--
 hw/block/vhost-user-blk.c  |  5 +-
 hw/hppa/dino.c |  5 +-
 hw/misc/mos6522.c  |  6 +--
 hw/net/ftgmac100.c |  5 +-
 hw/ppc/pnv_lpc.c   | 14 +-
 io/channel-websock.c   |  4 +-
 io/net-listener.c  |  6 +--
 monitor.c  |  2 +-
 target/i386/hax-darwin.c   | 10 ++--
 target/i386/hvf/hvf.c  | 24 +-
 target/mips/dsp_helper.c   | 15 ++
 target/xtensa/core-dc232b/xtensa-modules.c | 56 ++
 target/xtensa/core-dc233c/xtensa-modules.c | 56 ++
 target/xtensa/core-de212/xtensa-modules.c  | 48 +--
 target/xtensa/core-fsf/xtensa-modules.c| 32 -
 .../xtensa/core-sample_controller/xtensa-modules.c | 24 +++---
 target/xtensa/translate.c  |  5 +-
 tests/m48t59-test.c|  6 +--
 tests/test-thread-pool.c   |  6 +--
 util/uri.c |  5 +-
 24 files changed, 94 insertions(+), 269 deletions(-)

-- 
2.14.3

[Qemu-devel] [PATCH 2/4] error: Remove NULL checks on error_propagate() calls

[Laurent Vivier]

Re-run Coccinelle patch
scripts/coccinelle/error_propagate_null.cocci

Signed-off-by: Laurent Vivier 
---
 io/channel-websock.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/io/channel-websock.c b/io/channel-websock.c
index ec48a305f0..e6608b969d 100644
--- a/io/channel-websock.c
+++ b/io/channel-websock.c
@@ -586,9 +586,7 @@ static gboolean qio_channel_websock_handshake_io(QIOChannel 
*ioc,
 return TRUE;
 }
 
-if (err) {
-error_propagate(&wioc->io_err, err);
-}
+error_propagate(&wioc->io_err, err);
 
 trace_qio_channel_websock_handshake_reply(ioc);
 qio_channel_add_watch(
-- 
2.14.3

Re: [Qemu-devel] [PATCH v3 43/49] qapi: make s390 commands depend on TARGET_S390X

[Thomas Huth]

On 22.03.2018 10:41, Marc-André Lureau wrote:
> Hi
> 
> On Thu, Mar 22, 2018 at 6:42 AM, Thomas Huth  wrote:
>> On 21.03.2018 12:52, Marc-André Lureau wrote:
[...]
>>> diff --git a/qmp.c b/qmp.c
>>> index d8f80cb04e..14972b78df 100644
>>> --- a/qmp.c
>>> +++ b/qmp.c
>>> @@ -601,20 +601,6 @@ CpuModelExpansionInfo 
>>> *qmp_query_cpu_model_expansion(CpuModelExpansionType type,
>>>  return arch_query_cpu_model_expansion(type, model, errp);
>>>  }
>>>
>>> -CpuModelCompareInfo *qmp_query_cpu_model_comparison(CpuModelInfo *modela,
>>> -CpuModelInfo *modelb,
>>> -Error **errp)
>>> -{
>>> -return arch_query_cpu_model_comparison(modela, modelb, errp);
>>> -}
>>> -
>>> -CpuModelBaselineInfo *qmp_query_cpu_model_baseline(CpuModelInfo *modela,
>>> -   CpuModelInfo *modelb,
>>> -   Error **errp)
>>> -{
>>> -return arch_query_cpu_model_baseline(modela, modelb, errp);
>>> -}
>>
>> Not sure, but couldn't these two commands be implemented on other
>> architectures in the long run, too? So removing them now here seems
>> somewhat counterproductive?
> 
> They would have modify the qapi ifdef and implement the qmp handler in
> their target, similar to what would be done by implementing arch_query
> handlers. How counterproductive is that? The benefit is that we don't
> have to have stubs and "de-register" the commands.

Yes, thinking about that again, I guess you're right, this should be
fine. I first thought that there would likely be some common code
between the targets finally, but it's more likely that this CPU stuff is
completely different everywhere. So never mind!

 Thomas

[Qemu-devel] [Bug 1758091] [NEW] vmxnet3 unable to send IPv6 ESP packets

[Thomas Jansen]

Public bug reported:

My vmxnet3 network driver (in a closed source custom OS) is unable to
send network packets that are structured as follows: Ethernet-
Header(IPv6-Header(ESP(encrypted data))). I can verify that the packet
is sent in the VM but is dropped in qemu. I first encountered this
problem on qemu 2.10.1 but master is affected as well. After some debug
printing in qemu I could identify the following call chain as being
problematic:

eth_is_ip6_extension_header_type
eth_parse_ipv6_hdr
net_tx_pkt_parse_headers
net_tx_pkt_parse
vmxnet3_process_tx_queue

The problem seems to be the definition of the ESP header
(https://en.wikipedia.org/wiki/IPsec#Encapsulating_Security_Payload)
that does not follow the standard IPv6 extension header format starting
with next type and length. Thus the parsed ext_hdr in eth_parse_ipv6_hdr
does not contain valid data, in particular the length will contain bogus
data and lead to a info->full_hdr_len that is larger than the packet
itself and the loop would then try to read beyond the end of the packet.

Using the e1000 driver I can send these packets. My guess is that the
net_tx_pkt_parse function is not called in that case.

My guess for a fix would be to remove "case IP6_ESP:" from
eth_is_ip6_extension_header_type and not regard the ESP header as a IPv6
extension header. In a quick test this seems to fix the problem. But
that should be verified by someone who is familiar with the code.

** Affects: qemu
 Importance: Undecided
 Status: New

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

Title:
  vmxnet3 unable to send IPv6 ESP packets

Status in QEMU:
  New

Bug description:
  My vmxnet3 network driver (in a closed source custom OS) is unable to
  send network packets that are structured as follows: Ethernet-
  Header(IPv6-Header(ESP(encrypted data))). I can verify that the packet
  is sent in the VM but is dropped in qemu. I first encountered this
  problem on qemu 2.10.1 but master is affected as well. After some
  debug printing in qemu I could identify the following call chain as
  being problematic:

  eth_is_ip6_extension_header_type
  eth_parse_ipv6_hdr
  net_tx_pkt_parse_headers
  net_tx_pkt_parse
  vmxnet3_process_tx_queue

  The problem seems to be the definition of the ESP header
  (https://en.wikipedia.org/wiki/IPsec#Encapsulating_Security_Payload)
  that does not follow the standard IPv6 extension header format
  starting with next type and length. Thus the parsed ext_hdr in
  eth_parse_ipv6_hdr does not contain valid data, in particular the
  length will contain bogus data and lead to a info->full_hdr_len that
  is larger than the packet itself and the loop would then try to read
  beyond the end of the packet.

  Using the e1000 driver I can send these packets. My guess is that the
  net_tx_pkt_parse function is not called in that case.

  My guess for a fix would be to remove "case IP6_ESP:" from
  eth_is_ip6_extension_header_type and not regard the ESP header as a
  IPv6 extension header. In a quick test this seems to fix the problem.
  But that should be verified by someone who is familiar with the code.

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

Re: [Qemu-devel] [PATCH 4/4] qapi: new qmp command nbd-server-add-bitmap

[Vladimir Sementsov-Ogievskiy]

21.03.2018 20:33, Eric Blake wrote:

On 03/21/2018 07:19 AM, Vladimir Sementsov-Ogievskiy wrote:

Signed-off-by: Vladimir Sementsov-Ogievskiy 
---
  qapi/block.json | 27 +++
  blockdev-nbd.c  | 23 +++
  2 files changed, 50 insertions(+)

diff --git a/qapi/block.json b/qapi/block.json
index c694524002..4afbbcd7b7 100644
--- a/qapi/block.json
+++ b/qapi/block.json
@@ -269,6 +269,33 @@
    'data': {'name': 'str', '*mode': 'NbdServerRemoveMode'} }
    ##
+# @nbd-server-add-bitmap:
+#
+# Export dirty bitmap through selected export. Bitmaps are searched 
for in
+# device attached to the export and in all its backings. Exported 
bitmap

+# is locked until NBD export is removed.


Expose a dirty bitmap associated with the selected export.  The bitmap 
search starts at the device attached to the export, and includes all 
backing files.  The exported bitmap is then locked until the NBD 
export is removed.



+#
+# @name: Export name.
+#
+# @bitmap: Bitmap name to search.


s/search./search for./


+#
+# @bitmap-export-name: How the bitmap will be seen by nbd clients
+#  (default @bitmap)


Maybe mention that the client must use NBD_OPT_SET_META_CONTEXT with a 
query of "qemu-dirty-bitmap:NAME" (where NAME matches 
bitmap-export-name) to access the exposed bitmap.  (May need to be 
adjusted by my suggestion to use just the namespace "qemu:")


ok




+#
+#
+# Returns: error on one of the following conditions:
+#   - the server is not running
+#   - export is not found
+#   - bitmap is not found
+#   - bitmap is disabled
+#   - bitmap is locked


Do we really need to list all the error conditions?  My worry is that 
a list this specific might go stale, compared to the obvious default 
that the command succeeds only if it was able to expose the bitmap and 
that the error message is specific enough for a human to figure out 
what to fix if it failed.



Hmm, I've just doing it similar with other commands in the file. Is 
there any requirements on this part of qapi documentation? I can write 
only "# Returns: nothing on success" line, is it appropriate? 
blockdev-mirror do so, but other commands tries to describe errors. 
Looks like we lack some specified format for return code description 
like we have for parameters..





+#
+# Since: 2.13
+##
+  { 'command': 'nbd-server-add-bitmap',
+    'data': {'name': 'str', 'bitmap': 'str', '*bitmap-export-name': 
'str'} }

+





--
Best regards,
Vladimir

[Qemu-devel] [PATCH 3/3] coroutine: add test-aio coroutine queue chaining test case

[Stefan Hajnoczi]

Check that two coroutines can queue each other repeatedly without
hitting stack exhaustion.

Switch to qemu_init_main_loop() in main() because coroutines use
qemu_get_aio_context() - they don't know about test-aio's ctx variable.

Signed-off-by: Stefan Hajnoczi 
---
 tests/test-aio.c | 65 
 1 file changed, 52 insertions(+), 13 deletions(-)

diff --git a/tests/test-aio.c b/tests/test-aio.c
index 54e20d6ab1..86fb73b3d5 100644
--- a/tests/test-aio.c
+++ b/tests/test-aio.c
@@ -16,6 +16,8 @@
 #include "qemu/timer.h"
 #include "qemu/sockets.h"
 #include "qemu/error-report.h"
+#include "qemu/coroutine.h"
+#include "qemu/main-loop.h"
 
 static AioContext *ctx;
 
@@ -827,24 +829,59 @@ static void test_source_timer_schedule(void)
 timer_del(&data.timer);
 }
 
+/*
+ * Check that aio_co_enter() can chain many times
+ *
+ * Two coroutines should be able to invoke each other via aio_co_enter() many
+ * times without hitting a limit like stack exhaustion.  In other words, the
+ * calls should be chained instead of nested.
+ */
+
+typedef struct {
+Coroutine *other;
+unsigned i;
+unsigned max;
+} ChainData;
+
+static void coroutine_fn chain(void *opaque)
+{
+ChainData *data = opaque;
+
+for (data->i = 0; data->i < data->max; data->i++) {
+/* Queue up the other coroutine... */
+aio_co_enter(ctx, data->other);
+
+/* ...and give control to it */
+qemu_coroutine_yield();
+}
+}
+
+static void test_queue_chaining(void)
+{
+/* This number of iterations hit stack exhaustion in the past: */
+ChainData data_a = { .max = 25000 };
+ChainData data_b = { .max = 25000 };
+
+data_b.other = qemu_coroutine_create(chain, &data_a);
+data_a.other = qemu_coroutine_create(chain, &data_b);
+
+qemu_coroutine_enter(data_b.other);
+
+g_assert_cmpint(data_a.i, ==, data_a.max);
+g_assert_cmpint(data_b.i, ==, data_b.max - 1);
+
+/* Allow the second coroutine to terminate */
+qemu_coroutine_enter(data_a.other);
+
+g_assert_cmpint(data_b.i, ==, data_b.max);
+}
 
 /* End of tests.  */
 
 int main(int argc, char **argv)
 {
-Error *local_error = NULL;
-GSource *src;
-
-init_clocks(NULL);
-
-ctx = aio_context_new(&local_error);
-if (!ctx) {
-error_reportf_err(local_error, "Failed to create AIO Context: ");
-exit(1);
-}
-src = aio_get_g_source(ctx);
-g_source_attach(src, NULL);
-g_source_unref(src);
+qemu_init_main_loop(&error_fatal);
+ctx = qemu_get_aio_context();
 
 while (g_main_context_iteration(NULL, false));
 
@@ -864,6 +901,8 @@ int main(int argc, char **argv)
 g_test_add_func("/aio/external-client", test_aio_external_client);
 g_test_add_func("/aio/timer/schedule",  test_timer_schedule);
 
+g_test_add_func("/aio/coroutine/queue-chaining", test_queue_chaining);
+
 g_test_add_func("/aio-gsource/flush",   test_source_flush);
 g_test_add_func("/aio-gsource/bh/schedule", 
test_source_bh_schedule);
 g_test_add_func("/aio-gsource/bh/schedule10",   
test_source_bh_schedule10);
-- 
2.14.3

[Qemu-devel] [PATCH 1/3] queue: add QSIMPLEQ_PREPEND()

[Stefan Hajnoczi]

QSIMPLEQ_CONCAT(a, b) joins a = a + b.  The new QSIMPLEQ_PREPEND(a, b)
API joins a = b + a.

Signed-off-by: Stefan Hajnoczi 
---
 include/qemu/queue.h | 8 
 1 file changed, 8 insertions(+)

diff --git a/include/qemu/queue.h b/include/qemu/queue.h
index aa270d2b38..59fd1203a1 100644
--- a/include/qemu/queue.h
+++ b/include/qemu/queue.h
@@ -324,6 +324,14 @@ struct {   
 \
 }   \
 } while (/*CONSTCOND*/0)
 
+#define QSIMPLEQ_PREPEND(head1, head2) do { \
+if (!QSIMPLEQ_EMPTY((head2))) { \
+*(head2)->sqh_last = (head1)->sqh_first;\
+(head1)->sqh_first = (head2)->sqh_first;  \
+QSIMPLEQ_INIT((head2)); \
+}   \
+} while (/*CONSTCOND*/0)
+
 #define QSIMPLEQ_LAST(head, type, field)\
 (QSIMPLEQ_EMPTY((head)) ?   \
 NULL :  \
-- 
2.14.3

Re: [Qemu-devel] [PATCH 3/4] nbd/server: implement dirty bitmap export

[Vladimir Sementsov-Ogievskiy]

21.03.2018 19:57, Eric Blake wrote:

On 03/21/2018 07:19 AM, Vladimir Sementsov-Ogievskiy wrote:

Signed-off-by: Vladimir Sementsov-Ogievskiy 




[...]




+
+#define NBD_STATE_DIRTY 1


Does this belong better in nbd.h, alongside NBD_STATE_HOLE?  (And 
defining it as (1 << 0) might be nicer, too).


It's not used anywhere else, but it may be worth moving, for consistency 
and for future users. Ok, I'll move.



[...]


  +/* nbd_meta_bitmap_query
+ *
+ * Handle query to 'qemu-dirty-bitmap' namespace.
+ * 'len' is the amount of text remaining to be read from the current 
name, after

+ * the 'qemu-dirty-bitmap:' portion has been stripped.
+ *
+ * Return -errno on I/O error, 0 if option was completely handled by
+ * sending a reply about inconsistent lengths, or 1 on success. */
+static int nbd_meta_bitmap_query(NBDClient *client, 
NBDExportMetaContexts *meta,

+ uint32_t len, Error **errp)
+{
+    if (!client->exp->export_bitmap) {
+    return nbd_opt_skip(client, len, errp);
+    }
+
+    return nbd_meta_single_query(client, 
client->exp->export_bitmap_name, len,

+ &meta->dirty_bitmap, errp);


So if I'm reading this right, a client can do _LIST_ 
'qemu-dirty-bitmap:' (or 'qemu:dirty-bitmap:' if we choose a shorter 
initial namespace) and get back the exported bitmap name; or the user 
already knows the bitmap name and both _LIST_ and _SET_ 
'qemu-dirty-bitmap:name' return the one supported bitmap for that NBD 
server.


yes





  /* nbd_co_send_extents
+ *
+ * NBD_REPLY_FLAG_DONE is not set, don't forget to send it.
+ *
   * @extents should be in big-endian */
  static int nbd_co_send_extents(NBDClient *client, uint64_t handle,
 NBDExtent *extents, unsigned 
nb_extents,


Worth a bool flag that the caller can inform this function whether to 
include FLAG_DONE?


It was simpler to just send it separately, after all BLOCK_STATUS 
replies. So, I didn't need it.



+
+static int nbd_co_send_bitmap(NBDClient *client, uint64_t handle,
+  BdrvDirtyBitmap *bitmap, uint64_t offset,
+  uint64_t length, uint32_t context_id,
+  Error **errp)
+{
+    int ret;
+    unsigned nb_extents;
+    NBDExtent *extents = g_new(NBDExtent, NBD_MAX_BITMAP_EXTENTS);


Is this correct even when the client used NBD_CMD_FLAG_REQ_ONE?



Oops, looks like a bug.

--
Best regards,
Vladimir

[Qemu-devel] [PATCH 0/3] coroutine: avoid co_queue_wakeup recursion

[Stefan Hajnoczi]

co_queue_wakeup is currently implemented in a recursive fashion.  Pathological
patterns of aio_co_enter() between coroutines can cause stack exhaustion.

This patch series implements co_queue_wakeup iteratively and avoids stack
exhaustion.

This issue was originally reported with qemu-img convert but I don't have a
good reproducer.  See Patch 3 for a test-aio test case instead.

Stefan Hajnoczi (3):
  queue: add QSIMPLEQ_PREPEND()
  coroutine: avoid co_queue_wakeup recursion
  coroutine: add test-aio coroutine queue chaining test case

 include/qemu/coroutine_int.h |   1 -
 include/qemu/queue.h |   8 
 block/io.c   |   3 +-
 tests/test-aio.c |  65 -
 util/qemu-coroutine-lock.c   |  34 -
 util/qemu-coroutine.c| 110 +++
 6 files changed, 120 insertions(+), 101 deletions(-)

-- 
2.14.3

[Qemu-devel] [PATCH 2/3] coroutine: avoid co_queue_wakeup recursion

[Stefan Hajnoczi]

qemu_aio_coroutine_enter() is (indirectly) called recursively when
processing co_queue_wakeup.  This can lead to stack exhaustion.

This patch rewrites co_queue_wakeup in an iterative fashion (instead of
recursive) with bounded memory usage to prevent stack exhaustion.

qemu_co_queue_run_restart() is inlined into qemu_aio_coroutine_enter()
and the qemu_coroutine_enter() call is turned into a loop to avoid
recursion.

There is one change that is worth mentioning:  Previously, when
coroutine A queued coroutine B, qemu_co_queue_run_restart() entered
coroutine B from coroutine A.  If A was terminating then it would still
stay alive until B yielded.  After this patch B is entered by A's parent
so that a A can be deleted immediately if it is terminating.

It is safe to make this change since B could never interact with A if it
was terminating anyway.

Signed-off-by: Stefan Hajnoczi 
---
 include/qemu/coroutine_int.h |   1 -
 block/io.c   |   3 +-
 util/qemu-coroutine-lock.c   |  34 -
 util/qemu-coroutine.c| 110 +++
 4 files changed, 60 insertions(+), 88 deletions(-)

diff --git a/include/qemu/coroutine_int.h b/include/qemu/coroutine_int.h
index 59e8406398..bd6b0468e1 100644
--- a/include/qemu/coroutine_int.h
+++ b/include/qemu/coroutine_int.h
@@ -68,6 +68,5 @@ Coroutine *qemu_coroutine_new(void);
 void qemu_coroutine_delete(Coroutine *co);
 CoroutineAction qemu_coroutine_switch(Coroutine *from, Coroutine *to,
   CoroutineAction action);
-void coroutine_fn qemu_co_queue_run_restart(Coroutine *co);
 
 #endif
diff --git a/block/io.c b/block/io.c
index 2b09c656d0..bd9a19a9c4 100644
--- a/block/io.c
+++ b/block/io.c
@@ -249,8 +249,7 @@ static void coroutine_fn 
bdrv_co_yield_to_drain(BlockDriverState *bs,
 BdrvCoDrainData data;
 
 /* Calling bdrv_drain() from a BH ensures the current coroutine yields and
- * other coroutines run if they were queued from
- * qemu_co_queue_run_restart(). */
+ * other coroutines run if they were queued by aio_co_enter(). */
 
 assert(qemu_in_coroutine());
 data = (BdrvCoDrainData) {
diff --git a/util/qemu-coroutine-lock.c b/util/qemu-coroutine-lock.c
index 5a80c10690..27438a1858 100644
--- a/util/qemu-coroutine-lock.c
+++ b/util/qemu-coroutine-lock.c
@@ -68,40 +68,6 @@ void coroutine_fn qemu_co_queue_wait_impl(CoQueue *queue, 
QemuLockable *lock)
 }
 }
 
-/**
- * qemu_co_queue_run_restart:
- *
- * Enter each coroutine that was previously marked for restart by
- * qemu_co_queue_next() or qemu_co_queue_restart_all().  This function is
- * invoked by the core coroutine code when the current coroutine yields or
- * terminates.
- */
-void qemu_co_queue_run_restart(Coroutine *co)
-{
-Coroutine *next;
-QSIMPLEQ_HEAD(, Coroutine) tmp_queue_wakeup =
-QSIMPLEQ_HEAD_INITIALIZER(tmp_queue_wakeup);
-
-trace_qemu_co_queue_run_restart(co);
-
-/* Because "co" has yielded, any coroutine that we wakeup can resume it.
- * If this happens and "co" terminates, co->co_queue_wakeup becomes
- * invalid memory.  Therefore, use a temporary queue and do not touch
- * the "co" coroutine as soon as you enter another one.
- *
- * In its turn resumed "co" can populate "co_queue_wakeup" queue with
- * new coroutines to be woken up.  The caller, who has resumed "co",
- * will be responsible for traversing the same queue, which may cause
- * a different wakeup order but not any missing wakeups.
- */
-QSIMPLEQ_CONCAT(&tmp_queue_wakeup, &co->co_queue_wakeup);
-
-while ((next = QSIMPLEQ_FIRST(&tmp_queue_wakeup))) {
-QSIMPLEQ_REMOVE_HEAD(&tmp_queue_wakeup, co_queue_next);
-qemu_coroutine_enter(next);
-}
-}
-
 static bool qemu_co_queue_do_restart(CoQueue *queue, bool single)
 {
 Coroutine *next;
diff --git a/util/qemu-coroutine.c b/util/qemu-coroutine.c
index 9eff7fd450..1ba4191b84 100644
--- a/util/qemu-coroutine.c
+++ b/util/qemu-coroutine.c
@@ -104,57 +104,65 @@ static void coroutine_delete(Coroutine *co)
 
 void qemu_aio_coroutine_enter(AioContext *ctx, Coroutine *co)
 {
-Coroutine *self = qemu_coroutine_self();
-CoroutineAction ret;
-
-/* Cannot rely on the read barrier for co in aio_co_wake(), as there are
- * callers outside of aio_co_wake() */
-const char *scheduled = atomic_mb_read(&co->scheduled);
-
-trace_qemu_aio_coroutine_enter(ctx, self, co, co->entry_arg);
-
-/* if the Coroutine has already been scheduled, entering it again will
- * cause us to enter it twice, potentially even after the coroutine has
- * been deleted */
-if (scheduled) {
-fprintf(stderr,
-"%s: Co-routine was already scheduled in '%s'\n",
-__func__, scheduled);
-abort();
-}
-
-if (co->caller) {
-fprintf(stderr, "Co-routine re-entered recursively\n");
-abort();
-}
-
-co->caller = self;
-co->c