Re: [Qemu-devel] [PATCH] raspi: Add Raspberry Pi 1 support

[Andrew Baumann via Qemu-devel]

Hi Omar,

> From: Omar Rizwan [mailto:omar.riz...@gmail.com]
> Sent: Friday, 7 April 2017 22:43

Did you do any testing of this? One of the reasons I never got around to 
upstreaming this was that I couldn't get Raspbian working reliably (there was 
some problem with stalled DMA reads from the MMC controller), and I didn't have 
the cycles to debug it further. Also, IIRC Raspbian on pi1 depended on the 
system timer devices, so it might not make sense to have this board-level 
support without adding that first.

> --- a/hw/arm/bcm2835_peripherals.c
> +++ b/hw/arm/bcm2835_peripherals.c
> @@ -34,6 +34,7 @@ static void bcm2835_peripherals_init(Object *obj)
>  /* Internal memory region for peripheral bus addresses (not exported) */
>  memory_region_init(>gpu_bus_mr, obj, "bcm2835-gpu", (uint64_t)1 <<
> 32);
>  object_property_add_child(obj, "gpu-bus", OBJECT(>gpu_bus_mr),
> NULL);
> +sysbus_init_mmio(SYS_BUS_DEVICE(s), >gpu_bus_mr);
> 
>  /* Internal memory region for request/response communication with
>   * mailbox-addressable peripherals (not exported)

This line looks like it might have snuck in by accident?

Andrew

[Qemu-devel] [PATCH] Removed trailing newline from error_report()

[Ishani Chugh]

Signed-off-by: Ishani Chugh 
---
 target/arm/kvm64.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/target/arm/kvm64.c b/target/arm/kvm64.c
index 609..a16abc8 100644
--- a/target/arm/kvm64.c
+++ b/target/arm/kvm64.c
@@ -940,7 +940,7 @@ bool kvm_arm_handle_debug(CPUState *cs, struct 
kvm_debug_exit_arch *debug_exit)
  * single step at this point so something has gone wrong.
  */
 error_report("%s: guest single-step while debugging unsupported"
- " (%"PRIx64", %"PRIx32")\n",
+ " (%"PRIx64", %"PRIx32")",
  __func__, env->pc, debug_exit->hsr);
 return false;
 }
@@ -965,7 +965,7 @@ bool kvm_arm_handle_debug(CPUState *cs, struct 
kvm_debug_exit_arch *debug_exit)
 break;
 }
 default:
-error_report("%s: unhandled debug exit (%"PRIx32", %"PRIx64")\n",
+error_report("%s: unhandled debug exit (%"PRIx32", %"PRIx64")",
  __func__, debug_exit->hsr, env->pc);
 }
 
-- 
2.7.4

Re: [Qemu-devel] [PATCH 04/21] ppc/pnv: enable only one LPC bus

[David Gibson]

On Fri, Apr 07, 2017 at 08:14:36AM +0200, Cédric Le Goater wrote:
> On 04/06/2017 11:53 PM, Benjamin Herrenschmidt wrote:
> > On Thu, 2017-04-06 at 14:35 +0200, Cédric Le Goater wrote:
> >> but real HW (2 sockets OpenPOWER systems, garrison and firestone)
> >> does 
> >> not expose the LPC bus on the second chip, should we do so in qemu ?
> > 
> > It's not so much HW as it it HostBoot. Not a huge deal.
> 
> So let's have QEMU populate the second LPC bus without the "primary"
> prop then. It's a different approach than hostboot, but it's good for
> testing anyhow.

Works for me.

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


signature.asc
Description: PGP signature

Re: [Qemu-devel] [PATCH v2] migration/block: use blk_pwrite_zeroes for each zero cluster

[858585 jemmy]

On Fri, Apr 7, 2017 at 6:10 PM, Fam Zheng  wrote:
> On Fri, 04/07 16:44, jemmy858...@gmail.com wrote:
>> From: Lidong Chen 
>>
>> BLOCK_SIZE is (1 << 20), qcow2 cluster size is 65536 by default,
>> this maybe cause the qcow2 file size is bigger after migration.
>> This patch check each cluster, use blk_pwrite_zeroes for each
>> zero cluster.
>>
>> Signed-off-by: Lidong Chen 
>> ---
>>  migration/block.c | 37 +++--
>>  1 file changed, 35 insertions(+), 2 deletions(-)
>>
>> diff --git a/migration/block.c b/migration/block.c
>> index 7734ff7..c32e046 100644
>> --- a/migration/block.c
>> +++ b/migration/block.c
>> @@ -885,6 +885,11 @@ static int block_load(QEMUFile *f, void *opaque, int 
>> version_id)
>>  int64_t total_sectors = 0;
>>  int nr_sectors;
>>  int ret;
>> +int i;
>> +int64_t addr_offset;
>> +uint8_t *buf_offset;
>
> Poor variable names, they are not offset, maybe "cur_addr" and "cur_buf"? And
> they can be moved to the loop block below.
ok, i will change.

>
>> +BlockDriverInfo bdi;
>> +int cluster_size;
>>
>>  do {
>>  addr = qemu_get_be64(f);
>> @@ -934,8 +939,36 @@ static int block_load(QEMUFile *f, void *opaque, int 
>> version_id)
>>  } else {
>>  buf = g_malloc(BLOCK_SIZE);
>>  qemu_get_buffer(f, buf, BLOCK_SIZE);
>> -ret = blk_pwrite(blk, addr * BDRV_SECTOR_SIZE, buf,
>> - nr_sectors * BDRV_SECTOR_SIZE, 0);
>> +
>> +ret = bdrv_get_info(blk_bs(blk), );
>> +cluster_size = bdi.cluster_size;
>> +
>> +if (ret == 0 && cluster_size > 0 &&
>> +cluster_size < BLOCK_SIZE &&
>
> I think cluster_size == BLOCK_SIZE should work too.
This case the (flags & BLK_MIG_FLAG_ZERO_BLOCK) should be true,
and will invoke blk_pwrite_zeroes before apply this patch.
but maybe the source qemu maybe not enabled zero flag.
so i think cluster_size <= BLOCK_SIZE is ok.

>
>> +BLOCK_SIZE % cluster_size == 0) {
>> +for (i = 0; i < BLOCK_SIZE / cluster_size; i++) {
>> +addr_offset = addr * BDRV_SECTOR_SIZE
>> ++ i * cluster_size;
>> +buf_offset = buf + i * cluster_size;
>> +
>> +if (buffer_is_zero(buf_offset, cluster_size)) {
>> +ret = blk_pwrite_zeroes(blk, addr_offset,
>> +cluster_size,
>> +BDRV_REQ_MAY_UNMAP);
>> +} else {
>> + ret = blk_pwrite(blk, addr_offset, buf_offset,
>> +  cluster_size, 0);
>> +}
>> +
>> +if (ret < 0) {
>> +g_free(buf);
>> +return ret;
>> +}
>> +}
>> +} else {
>> +ret = blk_pwrite(blk, addr * BDRV_SECTOR_SIZE, buf,
>> + nr_sectors * BDRV_SECTOR_SIZE, 0);
>> +}
>>  g_free(buf);
>>  }
>>
>> --
>> 1.8.3.1
>>
>
> Is it possible use (source) cluster size as the transfer chunk size, instead 
> of
> BDRV_SECTORS_PER_DIRTY_CHUNK? Then the existing BLK_MIG_FLAG_ZERO_BLOCK logic
> can help and you don't need to send zero bytes on the wire. This may still not
> be optimal if dest has larger cluster, but it should cover the common use case
> well.

yes, i also think BDRV_SECTORS_PER_DIRTY_CHUNK is too large.
This have two disadvantage:
1. it will cause the dest qcow2 file size is bigger after migration.
2. it will cause transfer not necessary data, and maybe cause the
migration can't be successful.

in my production environment, some vm only write 2MB/s, the dirty
block migrate speed is 70MB/s.
but it still migration timeout.

but if we change the size of BDRV_SECTORS_PER_DIRTY_CHUNK, it will
break the protocol.
the old version qemu will not be able to migrate to new version qemu.
there are not information about the length about the migration buffer.

so i think we should add new flags to indicate that there are an
additional byte about the length
of migration buffer. i will send another patch later, and test the result.

this patch is also valuable, there are many old version qemu in my
production environment.
and will be benefit with this patch.

>
> Fam

Re: [Qemu-devel] [PATCH v2 4/6] block: Quiesce old aio context during bdrv_set_aio_context

[Fam Zheng]

On Fri, 04/07 13:50, Stefan Hajnoczi wrote:
> On Fri, Apr 07, 2017 at 02:54:12PM +0800, Fam Zheng wrote:
> > @@ -4413,6 +4416,10 @@ void bdrv_set_aio_context(BlockDriverState *bs, 
> > AioContext *new_context)
> >  aio_context_acquire(new_context);
> >  bdrv_attach_aio_context(bs, new_context);
> >  aio_context_release(new_context);
> > +if (bs->job) {
> > +block_job_resume(bs->job);
> > +}
> 
> Should this be called before aio_context_release(new_context)?

Yes, and I'm going to replace it with bdrv_parent_drained_begin() as Kevin
suggested.

Fam

Re: [Qemu-devel] [PATCH v2 6/6] tests/block-job-txn: Don't start block job before adding to txn

[Fam Zheng]

On Fri, 04/07 14:05, John Snow wrote:
> 
> 
> On 04/07/2017 09:28 AM, Stefan Hajnoczi wrote:
> > On Fri, Apr 07, 2017 at 02:54:14PM +0800, Fam Zheng wrote:
> >> Previously, before test_block_job_start returns, the job can already
> >> complete, as a result, the transactional state of other jobs added to
> >> the same txn later cannot be handled correctly.
> >>
> >> Move the block_job_start() calls to callers after
> >> block_job_txn_add_job() calls.
> >>
> >> Signed-off-by: Fam Zheng 
> >> ---
> >>  tests/test-blockjob-txn.c | 6 +-
> >>  1 file changed, 5 insertions(+), 1 deletion(-)
> > 
> > CCing John Snow because he looked at block jobs completing during txn
> > setup recently.
> > 
> > Stefan
> > 
> 
> This matches the changes we made to qmp_transaction, but I forgot to (or
> didn't take care to)  change the qtest as it didn't cause a regression
> at the time.
> 
> I wonder if I should make it a runtime error to add a job to a
> transaction which has already "started" to make sure that this interface
> is not misused, as this test highlights that there were still some
> remaining "bad" uses of the interface.
> 
> Regardless...
> 
> Thanks for the CC. ACK

So, shall we merge this for 2.9?

Fam

[Qemu-devel] [Bug 1654137] Re: Ctrl-A b not working in 2.8.0

[Paul Goyette]

I can confirm that this bug is fixed in qemu 2.8.1

Thanks!

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

Title:
  Ctrl-A b not working in 2.8.0

Status in QEMU:
  New

Bug description:
  With a recent update from 2.7.0 to 2.8.0 I have discovered that I can
  no longer send a "break" to the VM.  Ctrl-A b is simply ignored.
  Other Ctrl-A sequences seem to work correctly.

  This is on a NetBSD amd64 system, version 7.99.53, and qemu was
  installed on this system from source.

  Reverting to the previous install restores "break" capability.

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

Re: [Qemu-devel] [PATCH v2 5/6] coroutine: Explicitly specify AioContext when entering coroutine

[Fam Zheng]

On Fri, 04/07 14:26, Stefan Hajnoczi wrote:
> I wasn't expecting the patch to touch so many places.  If a coroutine
> can be permanently associated with an AioContext then there's no need to
> keep assigning co->ctx on each qemu_coroutine_enter().

Maybe, but bs->job->co->ctx changes when bdrv_set_aio_context(bs->ctx) is
called. In v1 the co->ctx is associated at qemu_coroutine_create() time but
failed to handle bdrv_set_aio_context.

Fam

Re: [Qemu-devel] [Qemu-ppc] [PATCH v2 0/3] Enable MTTCG on PPC64

[luigi burdo]

Tested on PowerMac G5 Quad  and 380% of system load and working on

Fedora 25 PPC64 host and Ubuntu Mate 17.04 guest  (patched the 2.9 rc3)


The machine configuration was this

sudo ./qemu-system-ppc64 -cpu POWER8 -vga none -machine pseries-2.5,usb=off -m 
2G  -smp 4,cores=4,threads=1 -accel tcg,thread=multi  -kerne vmlinuz-append 
root=/dev/sda   -device ich9-ahci,id=ahci   -device ide-drive,drive=disk0 
-drive file=/dev/sda4,if=none,id=disk0   -net nic,model=pcnet -net user 
-soundhw hda  -display sdl -vga virtio


vga virtio working too

here a shot

https://scontent-mxp1-1.xx.fbcdn.net/v/t1.0-9/17796379_10208795258860396_7825547329794577576_n.jpg?oh=526d6ddeb67c817053582d5b9ee56c71=594D7BDF


Thanks

Luigi

Re: [Qemu-devel] [PATCH 10/10] blockjob: use deferred_to_main_loop to indicate the coroutine has ended

[John Snow]

On 03/23/2017 01:39 PM, Paolo Bonzini wrote:
> All block jobs are using block_job_defer_to_main_loop as the final
> step just before the coroutine terminates.  At this point,
> block_job_enter should do nothing, but currently it restarts
> the freed coroutine.
> 
> Now, the job->co states should probably be changed to an enum
> (e.g. BEFORE_START, STARTED, YIELDED, COMPLETED) subsuming

Yes, I'd love to formalize the FSM for jobs.

> block_job_started, job->deferred_to_main_loop and job->busy.
> For now, this patch eliminates the problematic reenter by
> removing the reset of job->deferred_to_main_loop (which served
> no purpose, as far as I could see) and checking the flag in
> block_job_enter.

Not sure -- the original commit 794f01414 makes it seem like it should
stay so that the correct AIO context can be acquired. Probably a race as
jobs don't often stay long once they've deferred to the main loop, but I
think the reset is harmless as you say.

> 
> Signed-off-by: Paolo Bonzini 
> ---
>  blockjob.c   | 10 --
>  include/block/blockjob_int.h |  3 ++-
>  2 files changed, 10 insertions(+), 3 deletions(-)
> 
> diff --git a/blockjob.c b/blockjob.c
> index 3fa2885..2d80dae 100644
> --- a/blockjob.c
> +++ b/blockjob.c
> @@ -750,7 +750,14 @@ void block_job_resume_all(void)
>  
>  void block_job_enter(BlockJob *job)
>  {
> -if (job->co && !job->busy) {
> +if (!block_job_started(job)) {
> +return;
> +}
> +if (job->deferred_to_main_loop) {
> +return;
> +}
> +
> +if (!job->busy) {
>  qemu_coroutine_enter(job->co);
>  }
>  }
> @@ -874,7 +881,6 @@ static void block_job_defer_to_main_loop_bh(void *opaque)
>  aio_context_acquire(aio_context);
>  }
>  
> -data->job->deferred_to_main_loop = false;
>  data->fn(data->job, data->opaque);
>  
>  if (aio_context != data->aio_context) {
> diff --git a/include/block/blockjob_int.h b/include/block/blockjob_int.h
> index 97ffc43..4d287ba 100644
> --- a/include/block/blockjob_int.h
> +++ b/include/block/blockjob_int.h
> @@ -227,7 +227,8 @@ typedef void BlockJobDeferToMainLoopFn(BlockJob *job, 
> void *opaque);
>   * @fn: The function to run in the main loop
>   * @opaque: The opaque value that is passed to @fn
>   *
> - * Execute a given function in the main loop with the BlockDriverState
> + * This function must be called by the main job coroutine just before it
> + * returns.  @fn is executed in the main loop with the BlockDriverState
>   * AioContext acquired.  Block jobs must call bdrv_unref(), bdrv_close(), and
>   * anything that uses bdrv_drain_all() in the main loop.
>   *
> 

Reviewed-by: John Snow 

Re: [Qemu-devel] [PATCH 09/10] blockjob: reorganize block_job_completed_txn_abort

[John Snow]

On 03/23/2017 01:39 PM, Paolo Bonzini wrote:
> This splits the part that touches job states from the part that invokes
> callbacks.  It will be a bit simpler to understand once job states will
> be protected by a different mutex than the AioContext lock.
> 

Maybe easier to review then, too :)

> Signed-off-by: Paolo Bonzini 
> ---
>  blockjob.c | 165 
> -
>  1 file changed, 88 insertions(+), 77 deletions(-)
> 
> diff --git a/blockjob.c b/blockjob.c
> index 093962b..3fa2885 100644
> --- a/blockjob.c
> +++ b/blockjob.c
> @@ -76,6 +76,39 @@ BlockJob *block_job_get(const char *id)
>  return NULL;
>  }
>  
> +BlockJobTxn *block_job_txn_new(void)
> +{
> +BlockJobTxn *txn = g_new0(BlockJobTxn, 1);
> +QLIST_INIT(>jobs);
> +txn->refcnt = 1;
> +return txn;
> +}
> +
> +static void block_job_txn_ref(BlockJobTxn *txn)
> +{
> +txn->refcnt++;
> +}
> +
> +void block_job_txn_unref(BlockJobTxn *txn)
> +{
> +if (txn && --txn->refcnt == 0) {
> +g_free(txn);
> +}
> +}
> +
> +void block_job_txn_add_job(BlockJobTxn *txn, BlockJob *job)
> +{
> +if (!txn) {
> +return;
> +}
> +
> +assert(!job->txn);
> +job->txn = txn;
> +
> +QLIST_INSERT_HEAD(>jobs, job, txn_list);
> +block_job_txn_ref(txn);
> +}
> +
>  static void block_job_pause(BlockJob *job)
>  {
>  job->pause_count++;
> @@ -336,6 +369,8 @@ void block_job_start(BlockJob *job)
>  
>  static void block_job_completed_single(BlockJob *job)
>  {
> +assert(job->completed);
> +
>  if (!job->ret) {
>  if (job->driver->commit) {
>  job->driver->commit(job);
> @@ -376,14 +411,49 @@ static void block_job_completed_single(BlockJob *job)
>  static void block_job_cancel_async(BlockJob *job)
>  {
>  job->cancelled = true;
> -block_job_iostatus_reset(job);
> +if (!job->completed) {
> +block_job_iostatus_reset(job);
> +}
> +}
> +
> +static int block_job_finish_sync(BlockJob *job,
> + void (*finish)(BlockJob *, Error **errp),
> + Error **errp)
> +{
> +Error *local_err = NULL;
> +int ret;
> +
> +assert(blk_bs(job->blk)->job == job);
> +
> +block_job_ref(job);
> +
> +if (finish) {
> +finish(job, _err);
> +}
> +if (local_err) {
> +error_propagate(errp, local_err);
> +block_job_unref(job);
> +return -EBUSY;
> +}
> +/* block_job_drain calls block_job_enter, and it should be enough to
> + * induce progress until the job completes or moves to the main thread.
> +*/
> +while (!job->deferred_to_main_loop && !job->completed) {
> +block_job_drain(job);
> +}
> +while (!job->completed) {
> +aio_poll(qemu_get_aio_context(), true);
> +}
> +ret = (job->cancelled && job->ret == 0) ? -ECANCELED : job->ret;
> +block_job_unref(job);
> +return ret;
>  }
>  
>  static void block_job_completed_txn_abort(BlockJob *job)
>  {
>  AioContext *ctx;
>  BlockJobTxn *txn = job->txn;
> -BlockJob *other_job, *next;
> +BlockJob *other_job;
>  
>  if (txn->aborting) {
>  /*
> @@ -392,29 +462,34 @@ static void block_job_completed_txn_abort(BlockJob *job)
>  return;
>  }
>  txn->aborting = true;
> +block_job_txn_ref(txn);
> +
>  /* We are the first failed job. Cancel other jobs. */
>  QLIST_FOREACH(other_job, >jobs, txn_list) {
>  ctx = blk_get_aio_context(other_job->blk);
>  aio_context_acquire(ctx);
>  }
> +
> +/* Other jobs are "effectively" cancelled by us, set the status for
> + * them; this job, however, may or may not be cancelled, depending
> + * on the caller, so leave it. */
>  QLIST_FOREACH(other_job, >jobs, txn_list) {
> -if (other_job == job || other_job->completed) {
> -/* Other jobs are "effectively" cancelled by us, set the status 
> for
> - * them; this job, however, may or may not be cancelled, 
> depending
> - * on the caller, so leave it. */
> -if (other_job != job) {
> -block_job_cancel_async(other_job);
> -}
> -continue;
> +if (other_job != job) {
> +block_job_cancel_async(other_job);
>  }
> -block_job_cancel_sync(other_job);
> -assert(other_job->completed);
>  }
> -QLIST_FOREACH_SAFE(other_job, >jobs, txn_list, next) {
> +while (!QLIST_EMPTY(>jobs)) {
> +other_job = QLIST_FIRST(>jobs);
>  ctx = blk_get_aio_context(other_job->blk);
> +if (!other_job->completed) {
> +assert(other_job->cancelled);
> +block_job_finish_sync(other_job, NULL, NULL);
> +}
>  block_job_completed_single(other_job);
>  aio_context_release(ctx);
>  }
> +
> +block_job_txn_unref(txn);
>  }
>  
>  static void 

Re: [Qemu-devel] [PATCH 08/10] blockjob: introduce block_job_cancel_async

[John Snow]

On 03/23/2017 01:39 PM, Paolo Bonzini wrote:
> Signed-off-by: Paolo Bonzini 

What was the bad design that required you to fix the previous test? :)

> ---
>  blockjob.c | 11 ---
>  1 file changed, 8 insertions(+), 3 deletions(-)
> 
> diff --git a/blockjob.c b/blockjob.c
> index c9cb5b1..093962b 100644
> --- a/blockjob.c
> +++ b/blockjob.c
> @@ -373,6 +373,12 @@ static void block_job_completed_single(BlockJob *job)
>  block_job_unref(job);
>  }
>  
> +static void block_job_cancel_async(BlockJob *job)
> +{
> +job->cancelled = true;
> +block_job_iostatus_reset(job);
> +}
> +
>  static void block_job_completed_txn_abort(BlockJob *job)
>  {
>  AioContext *ctx;
> @@ -397,7 +403,7 @@ static void block_job_completed_txn_abort(BlockJob *job)
>   * them; this job, however, may or may not be cancelled, 
> depending
>   * on the caller, so leave it. */
>  if (other_job != job) {
> -other_job->cancelled = true;
> +block_job_cancel_async(other_job);

Adds an ioreset here, which I think is probably fine...

>  }
>  continue;
>  }
> @@ -489,8 +495,7 @@ void block_job_user_resume(BlockJob *job)
>  void block_job_cancel(BlockJob *job)
>  {
>  if (block_job_started(job)) {
> -job->cancelled = true;
> -block_job_iostatus_reset(job);
> +block_job_cancel_async(job);
>  block_job_enter(job);
>  } else {
>  block_job_completed(job, -ECANCELED);
> 

Reviewed-by: John Snow 

[Qemu-devel] [PATCH 3/4] pam: disable pc.rom when pam is disabled

[Anthony Xu]

pc.rom depends on pam. When pam is disabled, pc.rom is useless

Signed-off-by: Anthony Xu 
---
 hw/i386/pc.c | 13 +++--
 1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index 9d154c2..455f7fe 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -1358,7 +1358,7 @@ void pc_memory_init(PCMachineState *pcms,
 MemoryRegion **ram_memory)
 {
 int linux_boot, i;
-MemoryRegion *ram, *option_rom_mr;
+MemoryRegion *ram;
 MemoryRegion *ram_below_4g, *ram_above_4g;
 FWCfgState *fw_cfg;
 MachineState *machine = MACHINE(pcms);
@@ -1445,15 +1445,16 @@ void pc_memory_init(PCMachineState *pcms,
 /* Initialize PC system firmware */
 pc_system_firmware_init(rom_memory, !pcmc->pci_enabled);
 
-option_rom_mr = g_malloc(sizeof(*option_rom_mr));
-memory_region_init_ram(option_rom_mr, NULL, "pc.rom", PC_ROM_SIZE,
+if (pcms->pam) {
+MemoryRegion *option_rom_mr = g_malloc(sizeof(*option_rom_mr));
+memory_region_init_ram(option_rom_mr, NULL, "pc.rom", PC_ROM_SIZE,
_fatal);
-vmstate_register_ram_global(option_rom_mr);
-memory_region_add_subregion_overlap(rom_memory,
+vmstate_register_ram_global(option_rom_mr);
+memory_region_add_subregion_overlap(rom_memory,
 PC_ROM_MIN_VGA,
 option_rom_mr,
 1);
-
+}
 fw_cfg = bochs_bios_init(_space_memory, pcms);
 
 rom_set_fw(fw_cfg);
-- 
1.8.3.1

[Qemu-devel] [PATCH 2/4] pam: Make PAM configurable

[Anthony Xu]

by default PAM is enabled. when PAM is disabled,
*_init_pam and *_update_pam are dummy functions

Signed-off-by: Anthony Xu 
---
 hw/i386/pc.c | 18 ++
 hw/pci-host/piix.c   | 36 
 hw/pci-host/q35.c| 34 +++---
 include/hw/i386/pc.h |  2 ++
 4 files changed, 59 insertions(+), 31 deletions(-)

diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index d24388e..9d154c2 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -2210,6 +2210,20 @@ static void pc_machine_set_pit(Object *obj, bool value, 
Error **errp)
 pcms->pit = value;
 }
 
+static bool pc_machine_get_pam(Object *obj, Error **errp)
+{
+PCMachineState *pcms = PC_MACHINE(obj);
+
+return pcms->pam;
+}
+
+static void pc_machine_set_pam(Object *obj, bool value, Error **errp)
+{
+PCMachineState *pcms = PC_MACHINE(obj);
+
+pcms->pam = value;
+}
+
 static void pc_machine_initfn(Object *obj)
 {
 PCMachineState *pcms = PC_MACHINE(obj);
@@ -2224,6 +2238,7 @@ static void pc_machine_initfn(Object *obj)
 pcms->smbus = true;
 pcms->sata = true;
 pcms->pit = true;
+pcms->pam = true;
 }
 
 static void pc_machine_reset(void)
@@ -2372,6 +2387,9 @@ static void pc_machine_class_init(ObjectClass *oc, void 
*data)
 
 object_class_property_add_bool(oc, PC_MACHINE_PIT,
 pc_machine_get_pit, pc_machine_set_pit, _abort);
+
+object_class_property_add_bool(oc, PC_MACHINE_PAM,
+pc_machine_get_pam, pc_machine_set_pam, _abort);
 }
 
 static const TypeInfo pc_machine_info = {
diff --git a/hw/pci-host/piix.c b/hw/pci-host/piix.c
index ff4e8b5..7497516 100644
--- a/hw/pci-host/piix.c
+++ b/hw/pci-host/piix.c
@@ -152,16 +152,18 @@ static void i440fx_update_smram(PCII440FXState *d)
 
 static void i440fx_update_pam(PCII440FXState *d)
 {
-int i;
-PCIDevice *pd = PCI_DEVICE(d);
-memory_region_transaction_begin();
-pam_update(>pam_regions[0], 0,
-   pd->config[I440FX_PAM]);
-for (i = 1; i < 13; i++) {
-pam_update(>pam_regions[i], i,
-   pd->config[I440FX_PAM + ((i + 1) / 2)]);
+if (PC_MACHINE(current_machine)->pam) {
+int i;
+PCIDevice *pd = PCI_DEVICE(d);
+memory_region_transaction_begin();
+pam_update(>pam_regions[0], 0,
+   pd->config[I440FX_PAM]);
+for (i = 1; i < 13; i++) {
+pam_update(>pam_regions[i], i,
+   pd->config[I440FX_PAM + ((i + 1) / 2)]);
+}
+memory_region_transaction_commit();
 }
-memory_region_transaction_commit();
 }
 
 static void i440fx_update_memory_mappings(PCII440FXState *d)
@@ -173,14 +175,16 @@ static void i440fx_update_memory_mappings(PCII440FXState 
*d)
 
 static void i440fx_init_pam(PCII440FXState *d)
 {
-int i;
-init_pam(DEVICE(d), d->ram_memory, d->system_memory,
- d->pci_address_space, >pam_regions[0],
- PAM_BIOS_BASE,  PAM_BIOS_SIZE);
-for (i = 0; i < 12; ++i) {
+if (PC_MACHINE(current_machine)->pam) {
+int i;
 init_pam(DEVICE(d), d->ram_memory, d->system_memory,
- d->pci_address_space, >pam_regions[i + 1],
- PAM_EXPAN_BASE + i * PAM_EXPAN_SIZE, PAM_EXPAN_SIZE);
+ d->pci_address_space, >pam_regions[0],
+ PAM_BIOS_BASE,  PAM_BIOS_SIZE);
+for (i = 0; i < 12; ++i) {
+init_pam(DEVICE(d), d->ram_memory, d->system_memory,
+ d->pci_address_space, >pam_regions[i + 1],
+ PAM_EXPAN_BASE + i * PAM_EXPAN_SIZE, PAM_EXPAN_SIZE);
+}
 }
 }
 
diff --git a/hw/pci-host/q35.c b/hw/pci-host/q35.c
index 8866357..e63b057 100644
--- a/hw/pci-host/q35.c
+++ b/hw/pci-host/q35.c
@@ -310,15 +310,17 @@ static void mch_update_pciexbar(MCHPCIState *mch)
 /* PAM */
 static void mch_update_pam(MCHPCIState *mch)
 {
-PCIDevice *pd = PCI_DEVICE(mch);
-int i;
-
-memory_region_transaction_begin();
-for (i = 0; i < 13; i++) {
-pam_update(>pam_regions[i], i,
-   pd->config[MCH_HOST_BRIDGE_PAM0 + ((i + 1) / 2)]);
+if (PC_MACHINE(current_machine)->pam) {
+PCIDevice *pd = PCI_DEVICE(mch);
+int i;
+
+memory_region_transaction_begin();
+for (i = 0; i < 13; i++) {
+pam_update(>pam_regions[i], i,
+   pd->config[MCH_HOST_BRIDGE_PAM0 + ((i + 1) / 2)]);
+}
+memory_region_transaction_commit();
 }
-memory_region_transaction_commit();
 }
 
 /* SMRAM */
@@ -462,14 +464,16 @@ static void mch_reset(DeviceState *qdev)
 
 static void mch_init_pam(MCHPCIState *mch)
 {
-int i;
-init_pam(DEVICE(mch), mch->ram_memory, mch->system_memory,
- mch->pci_address_space, >pam_regions[0],
- PAM_BIOS_BASE, PAM_BIOS_SIZE);
-for (i = 0; i < 12; ++i) {
+if (PC_MACHINE(current_machine)->pam) {
+int i;
 init_pam(DEVICE(mch), 

[Qemu-devel] [PATCH 4/4] pam: setup pc.bios

[Anthony Xu]

when pam is disabled, set pc.bios and isa.bios region as writeable,
and add isa.bios to system memory region.

Signed-off-by: Anthony Xu 
---
 hw/i386/pc.c |  2 +-
 hw/i386/pc_sysfw.c   | 30 +-
 include/hw/i386/pc.h |  4 +++-
 3 files changed, 25 insertions(+), 11 deletions(-)

diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index 455f7fe..3e23f58 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -1443,7 +1443,7 @@ void pc_memory_init(PCMachineState *pcms,
 }
 
 /* Initialize PC system firmware */
-pc_system_firmware_init(rom_memory, !pcmc->pci_enabled);
+pc_system_firmware_init(system_memory, rom_memory, !pcmc->pci_enabled);
 
 if (pcms->pam) {
 MemoryRegion *option_rom_mr = g_malloc(sizeof(*option_rom_mr));
diff --git a/hw/i386/pc_sysfw.c b/hw/i386/pc_sysfw.c
index f915ad0..e5ac615 100644
--- a/hw/i386/pc_sysfw.c
+++ b/hw/i386/pc_sysfw.c
@@ -173,7 +173,8 @@ static void pc_system_flash_init(MemoryRegion *rom_memory)
 }
 }
 
-static void old_pc_system_rom_init(MemoryRegion *rom_memory, bool isapc_ram_fw)
+static void old_pc_system_rom_init(MemoryRegion *system_memory,
+   MemoryRegion *rom_memory, bool isapc_ram_fw)
 {
 char *filename;
 MemoryRegion *bios, *isa_bios;
@@ -197,8 +198,11 @@ static void old_pc_system_rom_init(MemoryRegion 
*rom_memory, bool isapc_ram_fw)
 bios = g_malloc(sizeof(*bios));
 memory_region_init_ram(bios, NULL, "pc.bios", bios_size, _fatal);
 vmstate_register_ram_global(bios);
-if (!isapc_ram_fw) {
-memory_region_set_readonly(bios, true);
+if (PC_MACHINE(current_machine)->pam) {
+/* if PAM is disabled, set it as readwrite */
+if (!isapc_ram_fw) {
+memory_region_set_readonly(bios, true);
+}
 }
 ret = rom_add_file_fixed(bios_name, (uint32_t)(-bios_size), -1);
 if (ret != 0) {
@@ -216,21 +220,29 @@ static void old_pc_system_rom_init(MemoryRegion 
*rom_memory, bool isapc_ram_fw)
 isa_bios = g_malloc(sizeof(*isa_bios));
 memory_region_init_alias(isa_bios, NULL, "isa-bios", bios,
  bios_size - isa_bios_size, isa_bios_size);
-memory_region_add_subregion_overlap(rom_memory,
+if (PC_MACHINE(current_machine)->pam) {
+memory_region_add_subregion_overlap(rom_memory,
+0x10 - isa_bios_size,
+isa_bios,
+1);
+if (!isapc_ram_fw) {
+memory_region_set_readonly(isa_bios, true);
+}
+} else {
+/* if PAM is disabed, add isa-bios to system memory region */
+memory_region_add_subregion_overlap(system_memory,
 0x10 - isa_bios_size,
 isa_bios,
 1);
-if (!isapc_ram_fw) {
-memory_region_set_readonly(isa_bios, true);
 }
-
 /* map all the bios at the top of memory */
 memory_region_add_subregion(rom_memory,
 (uint32_t)(-bios_size),
 bios);
 }
 
-void pc_system_firmware_init(MemoryRegion *rom_memory, bool isapc_ram_fw)
+void pc_system_firmware_init(MemoryRegion *system_memory,
+ MemoryRegion *rom_memory, bool isapc_ram_fw)
 {
 DriveInfo *pflash_drv;
 
@@ -238,7 +250,7 @@ void pc_system_firmware_init(MemoryRegion *rom_memory, bool 
isapc_ram_fw)
 
 if (isapc_ram_fw || pflash_drv == NULL) {
 /* When a pflash drive is not found, use rom-mode */
-old_pc_system_rom_init(rom_memory, isapc_ram_fw);
+old_pc_system_rom_init(system_memory, rom_memory, isapc_ram_fw);
 return;
 }
 
diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
index 05f4df5..eb5b853 100644
--- a/include/hw/i386/pc.h
+++ b/include/hw/i386/pc.h
@@ -354,7 +354,9 @@ static inline bool isa_ne2000_init(ISABus *bus, int base, 
int irq, NICInfo *nd)
 }
 
 /* pc_sysfw.c */
-void pc_system_firmware_init(MemoryRegion *rom_memory,
+
+void pc_system_firmware_init(MemoryRegion *system_memory,
+ MemoryRegion *rom_memory,
  bool isapc_ram_fw);
 
 /* pvpanic.c */
-- 
1.8.3.1

[Qemu-devel] [PATCH 0/4] pam: make pam configurable

[Anthony Xu]

Qemu creates 13 enabled memory regions and 39 disabled memory regions
for pam. in normal setup, more than half of all memory regions in
system root region are memory regions for pam. Which slow down all
memory region related operations. 

This patch makes pam configurable, by default it is enabled.
it keeps the old behavior by default.

you can disable pam by appending pam=off to -machine. If pam is
disabled, all memory regions for pam are gone, and some memory
region operations are gone since these memory regions are gone,
and memory region operations is fast because there are much less
memory regions. This patch works on both seabios and qboot,
it reduces Qemu heap size from ~12MB to ~9MB

if pam is disabled, pc.rom is useless, so it is disabled as well.

when pam is disabled, pc.bios and isa.bios are writeable memory
region, and isa.bios is put under system memory region, otherwise
isa.bios is acctually disabled because it is under pci memory region
which has lower priority than pc.ram region.

Anthony Xu (4):
  pam: refactor PAM related code
  pam: Make PAM configurable
  pam: disable pc.rom when pam is disabled
  pam: setup pc.bios

 hw/i386/pc.c | 33 ++--
 hw/i386/pc_sysfw.c   | 30 +
 hw/pci-host/piix.c   | 62 ++--
 hw/pci-host/q35.c| 43 ++--
 include/hw/i386/pc.h |  6 -
 5 files changed, 123 insertions(+), 51 deletions(-)

-- 
1.8.3.1

[Qemu-devel] [PATCH 1/4] pam:refactor PAM related code

[Anthony Xu]

split PAM SMRAM functions in piix.c
create mch_init_pam in q35.c

Signed-off-by: Anthony Xu 
---
 hw/pci-host/piix.c | 58 ++
 hw/pci-host/q35.c  | 23 +-
 2 files changed, 55 insertions(+), 26 deletions(-)

diff --git a/hw/pci-host/piix.c b/hw/pci-host/piix.c
index f9218aa..ff4e8b5 100644
--- a/hw/pci-host/piix.c
+++ b/hw/pci-host/piix.c
@@ -138,16 +138,11 @@ static int pci_slot_get_pirq(PCIDevice *pci_dev, int 
pci_intx)
 return (pci_intx + slot_addend) & 3;
 }
 
-static void i440fx_update_memory_mappings(PCII440FXState *d)
+static void i440fx_update_smram(PCII440FXState *d)
 {
-int i;
 PCIDevice *pd = PCI_DEVICE(d);
 
 memory_region_transaction_begin();
-for (i = 0; i < 13; i++) {
-pam_update(>pam_regions[i], i,
-   pd->config[I440FX_PAM + ((i + 1) / 2)]);
-}
 memory_region_set_enabled(>smram_region,
   !(pd->config[I440FX_SMRAM] & SMRAM_D_OPEN));
 memory_region_set_enabled(>smram,
@@ -155,6 +150,39 @@ static void i440fx_update_memory_mappings(PCII440FXState 
*d)
 memory_region_transaction_commit();
 }
 
+static void i440fx_update_pam(PCII440FXState *d)
+{
+int i;
+PCIDevice *pd = PCI_DEVICE(d);
+memory_region_transaction_begin();
+pam_update(>pam_regions[0], 0,
+   pd->config[I440FX_PAM]);
+for (i = 1; i < 13; i++) {
+pam_update(>pam_regions[i], i,
+   pd->config[I440FX_PAM + ((i + 1) / 2)]);
+}
+memory_region_transaction_commit();
+}
+
+static void i440fx_update_memory_mappings(PCII440FXState *d)
+{
+i440fx_update_pam(d);
+i440fx_update_smram(d);
+}
+
+
+static void i440fx_init_pam(PCII440FXState *d)
+{
+int i;
+init_pam(DEVICE(d), d->ram_memory, d->system_memory,
+ d->pci_address_space, >pam_regions[0],
+ PAM_BIOS_BASE,  PAM_BIOS_SIZE);
+for (i = 0; i < 12; ++i) {
+init_pam(DEVICE(d), d->ram_memory, d->system_memory,
+ d->pci_address_space, >pam_regions[i + 1],
+ PAM_EXPAN_BASE + i * PAM_EXPAN_SIZE, PAM_EXPAN_SIZE);
+}
+}
 
 static void i440fx_write_config(PCIDevice *dev,
 uint32_t address, uint32_t val, int len)
@@ -163,9 +191,12 @@ static void i440fx_write_config(PCIDevice *dev,
 
 /* XXX: implement SMRAM.D_LOCK */
 pci_default_write_config(dev, address, val, len);
-if (ranges_overlap(address, len, I440FX_PAM, I440FX_PAM_SIZE) ||
-range_covers_byte(address, len, I440FX_SMRAM)) {
-i440fx_update_memory_mappings(d);
+if (ranges_overlap(address, len, I440FX_PAM, I440FX_PAM_SIZE)) {
+i440fx_update_pam(d);
+}
+
+if (range_covers_byte(address, len, I440FX_SMRAM)) {
+i440fx_update_smram(d);
 }
 }
 
@@ -335,7 +366,6 @@ PCIBus *i440fx_init(const char *host_type, const char 
*pci_type,
 PCIHostState *s;
 PIIX3State *piix3;
 PCII440FXState *f;
-unsigned i;
 I440FXState *i440fx;
 
 dev = qdev_create(NULL, host_type);
@@ -378,13 +408,7 @@ PCIBus *i440fx_init(const char *host_type, const char 
*pci_type,
 object_property_add_const_link(qdev_get_machine(), "smram",
OBJECT(>smram), _abort);
 
-init_pam(dev, f->ram_memory, f->system_memory, f->pci_address_space,
- >pam_regions[0], PAM_BIOS_BASE, PAM_BIOS_SIZE);
-for (i = 0; i < 12; ++i) {
-init_pam(dev, f->ram_memory, f->system_memory, f->pci_address_space,
- >pam_regions[i+1], PAM_EXPAN_BASE + i * PAM_EXPAN_SIZE,
- PAM_EXPAN_SIZE);
-}
+i440fx_init_pam(f);
 
 /* Xen supports additional interrupt routes from the PCI devices to
  * the IOAPIC: the four pins of each PCI device on the bus are also
diff --git a/hw/pci-host/q35.c b/hw/pci-host/q35.c
index 344f77b..8866357 100644
--- a/hw/pci-host/q35.c
+++ b/hw/pci-host/q35.c
@@ -460,9 +460,21 @@ static void mch_reset(DeviceState *qdev)
 mch_update(mch);
 }
 
-static void mch_realize(PCIDevice *d, Error **errp)
+static void mch_init_pam(MCHPCIState *mch)
 {
 int i;
+init_pam(DEVICE(mch), mch->ram_memory, mch->system_memory,
+ mch->pci_address_space, >pam_regions[0],
+ PAM_BIOS_BASE, PAM_BIOS_SIZE);
+for (i = 0; i < 12; ++i) {
+init_pam(DEVICE(mch), mch->ram_memory, mch->system_memory,
+ mch->pci_address_space, >pam_regions[i + 1],
+ PAM_EXPAN_BASE + i * PAM_EXPAN_SIZE, PAM_EXPAN_SIZE);
+}
+}
+
+static void mch_realize(PCIDevice *d, Error **errp)
+{
 MCHPCIState *mch = MCH_PCI_DEVICE(d);
 
 /* setup pci memory mapping */
@@ -510,14 +522,7 @@ static void mch_realize(PCIDevice *d, Error **errp)
 object_property_add_const_link(qdev_get_machine(), "smram",
OBJECT(>smram), _abort);
 
-init_pam(DEVICE(mch), mch->ram_memory, 

Re: [Qemu-devel] [PATCH 07/10] blockjob: strengthen a bit test-blockjob-txn

[John Snow]

On 03/23/2017 01:39 PM, Paolo Bonzini wrote:
> Unlike test-blockjob-txn, QMP releases the reference to the transaction
> before the jobs finish.  Thus, while working on the next patch,
> qemu-iotest 124 showed a failure that the unit tests did not have.
> Make the unit test just a little nastier, so that it fails too.
> 
> Signed-off-by: Paolo Bonzini 
> ---
>  tests/test-blockjob-txn.c | 7 +--
>  1 file changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/tests/test-blockjob-txn.c b/tests/test-blockjob-txn.c
> index 4ccbda1..bfc2aaa 100644
> --- a/tests/test-blockjob-txn.c
> +++ b/tests/test-blockjob-txn.c
> @@ -165,6 +165,11 @@ static void test_pair_jobs(int expected1, int expected2)
>  job2 = test_block_job_start(2, true, expected2, );
>  block_job_txn_add_job(txn, job2);
>  

^ Oh, this might cause you grief too. Should be add-add-start-start, not
start-add-start-add. Fam sent a patch fixing this recently.

> +/* Release our reference now to trigger as many nice
> + * use-after-free bugs as possible.
> + */
> +block_job_txn_unref(txn);
> +

This is fine, though.

>  if (expected1 == -ECANCELED) {
>  block_job_cancel(job1);
>  }
> @@ -185,8 +190,6 @@ static void test_pair_jobs(int expected1, int expected2)
>  
>  g_assert_cmpint(result1, ==, expected1);
>  g_assert_cmpint(result2, ==, expected2);
> -
> -block_job_txn_unref(txn);
>  }
>  
>  static void test_pair_jobs_success(void)
> 

Reviewed-by: John Snow 

Re: [Qemu-devel] [PATCH 06/10] blockjob: move iostatus reset inside block_job_user_resume

[John Snow]

On 03/23/2017 01:39 PM, Paolo Bonzini wrote:
> Outside blockjob.c, the block_job_iostatus_reset function is used once
> in the monitor and once in BlockBackend.  When we introduce the block
> job mutex, block_job_iostatus_reset's client is going to be the block
> layer (for which blockjob.c will take the block job mutex) rather than
> the monitor (which will take the block job mutex by itself).
> 

Ah, sure. So we leave the block-backend call alone, here. We're moving
it from "public API" to "specifically block-layer API" to help
facilitate different mutex semantics. OK.

> The monitor's call to block_job_iostatus_reset from the monitor comes
> just before the sole call to block_job_user_resume, so reset the
> iostatus directly from block_job_iostatus_reset.  This will avoid
> the need to introduce separate block_job_iostatus_reset and
> block_job_iostatus_reset_locked APIs.
> 
> After making this change, move the function together with the others
> that were moved in the previous patch.
> 
> Signed-off-by: Paolo Bonzini 

Reviewed-by: John Snow 

> ---
>  blockdev.c |  1 -
>  blockjob.c | 11 ++-
>  2 files changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/blockdev.c b/blockdev.c
> index c5b2c2c..fbc376b 100644
> --- a/blockdev.c
> +++ b/blockdev.c
> @@ -3726,7 +3726,6 @@ void qmp_block_job_resume(const char *device, Error 
> **errp)
>  }
>  
>  trace_qmp_block_job_resume(job);
> -block_job_iostatus_reset(job);
>  block_job_user_resume(job);
>  aio_context_release(aio_context);
>  }
> diff --git a/blockjob.c b/blockjob.c
> index c5f1d19..c9cb5b1 100644
> --- a/blockjob.c
> +++ b/blockjob.c
> @@ -481,6 +481,7 @@ void block_job_user_resume(BlockJob *job)
>  {
>  if (job && job->user_paused && job->pause_count > 0) {
>  job->user_paused = false;
> +block_job_iostatus_reset(job);
>  block_job_resume(job);
>  }
>  }
> @@ -496,11 +497,6 @@ void block_job_cancel(BlockJob *job)
>  }
>  }
>  
> -void block_job_iostatus_reset(BlockJob *job)
> -{
> -job->iostatus = BLOCK_DEVICE_IO_STATUS_OK;
> -}
> -
>  static int block_job_finish_sync(BlockJob *job,
>   void (*finish)(BlockJob *, Error **errp),
>   Error **errp)
> @@ -751,6 +747,11 @@ void block_job_yield(BlockJob *job)
>  block_job_pause_point(job);
>  }
>  
> +void block_job_iostatus_reset(BlockJob *job)
> +{
> +job->iostatus = BLOCK_DEVICE_IO_STATUS_OK;
> +}
> +
>  void block_job_event_ready(BlockJob *job)
>  {
>  job->ready = true;
> 

[Qemu-devel] [Bug 1680991] [NEW] System Timer returning 0 tick count for raspi2 emulation

[Adam Clark]

Public bug reported:

In a small hobby kernel for Raspberry Pi 2B, I am using the system timer
to control wait durations.  This timer is located at 0x3f003000 and the
timer counts are located at 0x3f003004 (CLO) and 0x3f004008 (CHI).
Reading these memory locations returns 0 for both.

The basic code for this function is:
@
@@ uint64_t ReadSysTimerCount() -- read the system time running count
@
ReadSysTimerCount:
ldr r0,=ST_CLO  @ load the base address of the 
system timer
ldrdr0,r1,[r0]  @ Get the 64-bit timer "count" into 
r1:r0
mov pc,lr   @ return

Tracing back the definition of ST_CLO in my code:
#define ST_CLO  (ST_BASE+4) // Counter Lower 32 bits
#define ST_BASE (HW_BASE+0x3000)// System Timer base 
address
#define HW_BASE (0x3f00)// this is the base 
address for all hardware I/O addresses

I have tested a similar program that I know to work on real hardware
with qemu-system-arm reading the same mmio register and have the same
issue, so I'm pretty sure the issue is not with my code.

My Host PC is a VM on vmWare esxi running FC25 (8 cores, 8GB RAM): 
[adam@os-dev ~]$ uname -a
Linux os-dev.jammin 4.10.8-200.fc25.x86_64 #1 SMP Fri Mar 31 13:20:22 UTC 2017 
x86_64 x86_64 x86_64 GNU/Linux

I have confirmed this issue on QEMU 2.7.1 (fc25 Distro) and 2.9.0-rc3
(git).

adam@os-dev ~]$ qemu-system-arm --version
QEMU emulator version 2.7.1(qemu-2.7.1-4.fc25), Copyright (c) 2003-2016 Fabrice 
Bellard and the QEMU Project developers

[adam@os-dev ~]$ ./workspace/qemu/bin/debug/native/arm-softmmu/qemu-system-arm 
--version
QEMU emulator version 2.8.93 (v2.9.0-rc3-15-g5daf9b3)
Copyright (c) 2003-2017 Fabrice Bellard and the QEMU Project developers

A remote debugger for my kernel shows the following:
(gdb) info reg
r0 0x0  0
r1 0x0  0
r2 0x96 150
r3 0x0  0
r4 0xa000   40960
r5 0x0  0
r6 0x0  0
r7 0x0  0
r8 0x0  0
r9 0xa000   40960
r100x0  0
r110x7fdc   32732
r120x0  0
sp 0x7fc8   0x7fc8
lr 0x8194   33172
pc 0x80a4   0x80a4
cpsr   0x81d3   -2147483181
(gdb) stepi
0x80a8 in ?? ()
(gdb) info reg
r0 0x3f003004   1056976900
r1 0x0  0
r2 0x96 150
r3 0x0  0
r4 0xa000   40960
r5 0x0  0
r6 0x0  0
r7 0x0  0
r8 0x0  0
r9 0xa000   40960
r100x0  0
r110x7fdc   32732
r120x0  0
sp 0x7fc8   0x7fc8
lr 0x8194   33172
pc 0x80a8   0x80a8
cpsr   0x81d3   -2147483181
(gdb) stepi
0x80ac in ?? ()
(gdb) info reg
r0 0x0  0
r1 0x0  0
r2 0x96 150
r3 0x0  0
r4 0xa000   40960
r5 0x0  0
r6 0x0  0
r7 0x0  0
r8 0x0  0
r9 0xa000   40960
r100x0  0
r110x7fdc   32732
r120x0  0
sp 0x7fc8   0x7fc8
lr 0x8194   33172
pc 0x80ac   0x80ac
cpsr   0x81d3   -2147483181

Notice r0 is loaded with the address for CLO and then cleared with 0
when read.

I am writing my code against the documented specifications in "BCM2835
ARM Peripherals" (attached for convenience), section "12 System Timer".


Please let me know if you need anything else from me.

** Affects: qemu
 Importance: Undecided
 Status: New

** Attachment added: "BSM2835 ARM Peripherals"
   
https://bugs.launchpad.net/bugs/1680991/+attachment/4857848/+files/SoC-Peripherals.pdf

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

Title:
  System Timer returning 0 tick count for raspi2 emulation

Status in QEMU:
  New

Bug description:
  In a small hobby kernel for Raspberry Pi 2B, I am using the system
  timer to control wait durations.  This timer is located at 0x3f003000
  and the timer counts are located at 0x3f003004 (CLO) and 0x3f004008
  (CHI).  Reading these memory locations returns 0 for both.

  The basic code for this function is:
  
@
  @@ uint64_t ReadSysTimerCount() -- read the system time running count
  

[Qemu-devel] [Bug 1680991] Re: System Timer returning 0 tick count for raspi2 emulation

[Adam Clark]

The command lines are:

[adam@os-dev ~]$ qemu-system-aarch64 -m 256 -M raspi2 -serial stdio
-kernel bin/rpi2b/kernel.elf

[adam@os-dev workspace]$ ./qemu/bin/debug/native/arm-softmmu/qemu-
system-arm -m 256 -M raspi2 -serial stdio -kernel
century/bin/rpi2b/kernel.elf


A sample kernel is also attached for your convenience.

** Attachment added: "Sample kernel to reproduce the problem"
   
https://bugs.launchpad.net/qemu/+bug/1680991/+attachment/4857850/+files/kernel.elf

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

Title:
  System Timer returning 0 tick count for raspi2 emulation

Status in QEMU:
  New

Bug description:
  In a small hobby kernel for Raspberry Pi 2B, I am using the system
  timer to control wait durations.  This timer is located at 0x3f003000
  and the timer counts are located at 0x3f003004 (CLO) and 0x3f004008
  (CHI).  Reading these memory locations returns 0 for both.

  The basic code for this function is:
  
@
  @@ uint64_t ReadSysTimerCount() -- read the system time running count
  
@
  ReadSysTimerCount:
ldr r0,=ST_CLO  @ load the base address of the 
system timer
ldrdr0,r1,[r0]  @ Get the 64-bit timer "count" into 
r1:r0
mov pc,lr   @ return

  Tracing back the definition of ST_CLO in my code:
  #define ST_CLO  (ST_BASE+4) // Counter Lower 32 
bits
  #define ST_BASE (HW_BASE+0x3000)// System Timer base 
address
  #define HW_BASE (0x3f00)// this is the base 
address for all hardware I/O addresses

  I have tested a similar program that I know to work on real hardware
  with qemu-system-arm reading the same mmio register and have the same
  issue, so I'm pretty sure the issue is not with my code.

  My Host PC is a VM on vmWare esxi running FC25 (8 cores, 8GB RAM): 
  [adam@os-dev ~]$ uname -a
  Linux os-dev.jammin 4.10.8-200.fc25.x86_64 #1 SMP Fri Mar 31 13:20:22 UTC 
2017 x86_64 x86_64 x86_64 GNU/Linux

  I have confirmed this issue on QEMU 2.7.1 (fc25 Distro) and 2.9.0-rc3
  (git).

  adam@os-dev ~]$ qemu-system-arm --version
  QEMU emulator version 2.7.1(qemu-2.7.1-4.fc25), Copyright (c) 2003-2016 
Fabrice Bellard and the QEMU Project developers

  [adam@os-dev ~]$ 
./workspace/qemu/bin/debug/native/arm-softmmu/qemu-system-arm --version
  QEMU emulator version 2.8.93 (v2.9.0-rc3-15-g5daf9b3)
  Copyright (c) 2003-2017 Fabrice Bellard and the QEMU Project developers

  A remote debugger for my kernel shows the following:
  (gdb) info reg
  r0 0x00
  r1 0x00
  r2 0x96   150
  r3 0x00
  r4 0xa000 40960
  r5 0x00
  r6 0x00
  r7 0x00
  r8 0x00
  r9 0xa000 40960
  r100x00
  r110x7fdc 32732
  r120x00
  sp 0x7fc8 0x7fc8
  lr 0x8194 33172
  pc 0x80a4 0x80a4
  cpsr   0x81d3 -2147483181
  (gdb) stepi
  0x80a8 in ?? ()
  (gdb) info reg
  r0 0x3f003004 1056976900
  r1 0x00
  r2 0x96   150
  r3 0x00
  r4 0xa000 40960
  r5 0x00
  r6 0x00
  r7 0x00
  r8 0x00
  r9 0xa000 40960
  r100x00
  r110x7fdc 32732
  r120x00
  sp 0x7fc8 0x7fc8
  lr 0x8194 33172
  pc 0x80a8 0x80a8
  cpsr   0x81d3 -2147483181
  (gdb) stepi
  0x80ac in ?? ()
  (gdb) info reg
  r0 0x00
  r1 0x00
  r2 0x96   150
  r3 0x00
  r4 0xa000 40960
  r5 0x00
  r6 0x00
  r7 0x00
  r8 0x00
  r9 0xa000 40960
  r100x00
  r110x7fdc 32732
  r120x00
  sp 0x7fc8 0x7fc8
  lr 0x8194 33172
  pc 0x80ac 0x80ac
  cpsr   0x81d3 -2147483181

  Notice r0 is loaded with the address for CLO and then cleared with 0
  when read.

  I am writing my code against the documented specifications in "BCM2835
  ARM Peripherals" (attached for convenience), section "12 System
  Timer".

  
  Please let me know if you need anything else from me.

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

Re: [Qemu-devel] [PATCH 02/10] blockjob: remove iostatus_reset callback

[Philippe Mathieu-Daudé]

On 04/07/2017 08:12 PM, John Snow wrote:

On 03/23/2017 01:39 PM, Paolo Bonzini wrote:

This is unused since commit 66a0fae ("blockjob: Don't touch BDS iostatus",
2016-05-19).

Signed-off-by: Paolo Bonzini 
---
 blockjob.c   | 3 ---
 include/block/blockjob_int.h | 3 ---
 2 files changed, 6 deletions(-)

diff --git a/blockjob.c b/blockjob.c
index a9fb624..24d1e12 100644
--- a/blockjob.c
+++ b/blockjob.c
@@ -555,9 +555,6 @@ bool block_job_is_cancelled(BlockJob *job)
 void block_job_iostatus_reset(BlockJob *job)
 {
 job->iostatus = BLOCK_DEVICE_IO_STATUS_OK;
-if (job->driver->iostatus_reset) {
-job->driver->iostatus_reset(job);
-}
 }

 static int block_job_finish_sync(BlockJob *job,
diff --git a/include/block/blockjob_int.h b/include/block/blockjob_int.h
index 3f86cc5..bfcc5d1 100644
--- a/include/block/blockjob_int.h
+++ b/include/block/blockjob_int.h
@@ -44,9 +44,6 @@ struct BlockJobDriver {
 /** Optional callback for job types that support setting a speed limit */
 void (*set_speed)(BlockJob *job, int64_t speed, Error **errp);

-/** Optional callback for job types that need to forward I/O status reset 
*/
-void (*iostatus_reset)(BlockJob *job);
-
 /** Mandatory: Entrypoint for the Coroutine. */
 CoroutineEntry *start;




Good find.

Reviewed-by: John Snow 


Reviewed-by: Philippe Mathieu-Daudé 

Re: [Qemu-devel] [PATCH 05/10] blockjob: separate monitor and blockjob APIs

[John Snow]

On 03/23/2017 01:39 PM, Paolo Bonzini wrote:
> We already have different locking policies for APIs called by the monitor
> and the block job.  Monitor APIs need consistency across block_job_get
> and the actual operation (e.g. block_job_set_speed), so currently there
> are explicit aio_context_acquire/release calls in blockdev.c.
> 
> When a block job needs to do something instead it doesn't care about locking,
> because the whole coroutine runs under the AioContext lock.  When moving
> away from the AioContext lock, the monitor will have to call new
> block_job_lock/unlock APIs, while blockjob APIs will take care of this
> for the users.
> 
> In preparation for that, keep all the blockjob APIs together in the
> blockjob.c file.
> 
> Signed-off-by: Paolo Bonzini 
> ---
>  blockjob.c | 206 
> +++--

Did you guys order... like, fifty million pizzas?

Hooray for
$ diff -u <(sed -n 's/^-//p' patch) <(sed -n 's/^\+//p' patch)

Looks clean, though it may be useful to do a few more things;

- Demarcate what you think is the monitor API in this file
- Organize blockjob.h to match to serve as a useful reference.

>  1 file changed, 105 insertions(+), 101 deletions(-)
> 
> diff --git a/blockjob.c b/blockjob.c
> index caca718..c5f1d19 100644
> --- a/blockjob.c
> +++ b/blockjob.c
> @@ -334,11 +334,6 @@ void block_job_start(BlockJob *job)
>  qemu_coroutine_enter(job->co);
>  }
>  
> -void block_job_fail(BlockJob *job)
> -{
> -block_job_unref(job);
> -}
> -
>  static void block_job_completed_single(BlockJob *job)
>  {
>  if (!job->ret) {
> @@ -440,21 +435,6 @@ static void block_job_completed_txn_success(BlockJob 
> *job)
>  }
>  }
>  
> -void block_job_completed(BlockJob *job, int ret)
> -{
> -assert(blk_bs(job->blk)->job == job);
> -assert(!job->completed);
> -job->completed = true;
> -job->ret = ret;
> -if (!job->txn) {
> -block_job_completed_single(job);
> -} else if (ret < 0 || block_job_is_cancelled(job)) {
> -block_job_completed_txn_abort(job);
> -} else {
> -block_job_completed_txn_success(job);
> -}
> -}
> -
>  void block_job_set_speed(BlockJob *job, int64_t speed, Error **errp)
>  {
>  Error *local_err = NULL;
> @@ -492,44 +472,11 @@ void block_job_user_pause(BlockJob *job)
>  block_job_pause(job);
>  }
>  
> -static bool block_job_should_pause(BlockJob *job)
> -{
> -return job->pause_count > 0;
> -}
> -
>  bool block_job_user_paused(BlockJob *job)
>  {
>  return job->user_paused;
>  }
>  
> -void coroutine_fn block_job_pause_point(BlockJob *job)
> -{
> -assert(job && block_job_started(job));
> -
> -if (!block_job_should_pause(job)) {
> -return;
> -}
> -if (block_job_is_cancelled(job)) {
> -return;
> -}
> -
> -if (job->driver->pause) {
> -job->driver->pause(job);
> -}
> -
> -if (block_job_should_pause(job) && !block_job_is_cancelled(job)) {
> -job->paused = true;
> -job->busy = false;
> -qemu_coroutine_yield(); /* wait for block_job_resume() */
> -job->busy = true;
> -job->paused = false;
> -}
> -
> -if (job->driver->resume) {
> -job->driver->resume(job);
> -}
> -}
> -
>  void block_job_user_resume(BlockJob *job)
>  {
>  if (job && job->user_paused && job->pause_count > 0) {
> @@ -538,13 +485,6 @@ void block_job_user_resume(BlockJob *job)
>  }
>  }
>  
> -void block_job_enter(BlockJob *job)
> -{
> -if (job->co && !job->busy) {
> -qemu_coroutine_enter(job->co);
> -}
> -}
> -
>  void block_job_cancel(BlockJob *job)
>  {
>  if (block_job_started(job)) {
> @@ -556,11 +496,6 @@ void block_job_cancel(BlockJob *job)
>  }
>  }
>  
> -bool block_job_is_cancelled(BlockJob *job)
> -{
> -return job->cancelled;
> -}
> -
>  void block_job_iostatus_reset(BlockJob *job)
>  {
>  job->iostatus = BLOCK_DEVICE_IO_STATUS_OK;
> @@ -628,42 +563,6 @@ int block_job_complete_sync(BlockJob *job, Error **errp)
>  return block_job_finish_sync(job, _job_complete, errp);
>  }
>  
> -void block_job_sleep_ns(BlockJob *job, QEMUClockType type, int64_t ns)
> -{
> -assert(job->busy);
> -
> -/* Check cancellation *before* setting busy = false, too!  */
> -if (block_job_is_cancelled(job)) {
> -return;
> -}
> -
> -job->busy = false;
> -if (!block_job_should_pause(job)) {
> -co_aio_sleep_ns(blk_get_aio_context(job->blk), type, ns);
> -}
> -job->busy = true;
> -
> -block_job_pause_point(job);
> -}
> -
> -void block_job_yield(BlockJob *job)
> -{
> -assert(job->busy);
> -
> -/* Check cancellation *before* setting busy = false, too!  */
> -if (block_job_is_cancelled(job)) {
> -return;
> -}
> -
> -job->busy = false;
> -if (!block_job_should_pause(job)) {
> -qemu_coroutine_yield();
> -}
> -job->busy = true;
> -
> -

[Qemu-devel] [Bug 1680679] Re: qemu cannot run twice

[Roger Lawhorn]

I meant to say you cannot run without the intel card in windows if you
have optimus. I am glad seabios somehow hides optimus.

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

Title:
  qemu cannot run twice

Status in QEMU:
  Invalid

Bug description:
  After using qemu with gpu passthrough and then shutting down windows 7 
properly I cannot boot windows 7 a second time.
  Only a full reboot of linux fixes this issue.
  Qemu appears to corrupt something in linux when exiting.
  I get no error messages but windows 7 never finishes booting during the 2nd 
try.
  Apparently I do try to run vfiobind each time the script is run.
  Wondering if rerunning vfiobind can cause an issue?

  
  My specs:
  
-
  System:Host: GT70-2PE Kernel: 4.5.4-040504-generic x86_64 (64 bit gcc: 
5.3.1)
 Desktop: Cinnamon 3.2.7 (Gtk 3.18.9) Distro: Linux Mint 18.1 Serena
  Machine:   Mobo: Micro-Star model: MS-1763 v: REV:0.C Bios: American 
Megatrends v: E1763IMS.51B date: 01/29/2015
  CPU:   Quad core Intel Core i7-4810MQ (-HT-MCP-) cache: 6144 KB
 flags: (lm nx sse sse2 sse3 sse4_1 sse4_2 ssse3 vmx) bmips: 22347
 clock speeds: max: 2801 MHz 1: 2801 MHz 2: 2801 MHz 3: 2801 MHz 4: 
2801 MHz 5: 2801 MHz 6: 2801 MHz
 7: 2801 MHz 8: 2801 MHz
  Graphics:  Card-1: Intel 4th Gen Core Processor Integrated Graphics 
Controller bus-ID: 00:02.0
 Card-2: NVIDIA GK104M [GeForce GTX 880M] bus-ID: 01:00.0
 Display Server: X.Org 1.18.4 drivers: intel (unloaded: fbdev,vesa)
 Resolution: 1920x1080@60.02hz, 1920x1080@60.00hz
 GLX Renderer: Mesa DRI Intel Haswell Mobile GLX Version: 3.0 Mesa 
12.0.6 Direct Rendering: Yes

  
  My script:
  
---
  #!/bin/bash

  cd ~/qemu
  sudo ./up.sh tap0

  configfile=~/qemu/vfio-pci1.cfg

  vfiobind() {
  dev="$1"
  vendor=$(cat /sys/bus/pci/devices/$dev/vendor)
  device=$(cat /sys/bus/pci/devices/$dev/device)
  if [ -e /sys/bus/pci/devices/$dev/driver ]; then
  echo $dev > /sys/bus/pci/devices/$dev/driver/unbind
  fi
  echo $vendor $device > /sys/bus/pci/drivers/vfio-pci/new_id

  }

  modprobe vfio-pci

  cat $configfile | while read line;do
  echo $line | grep ^# >/dev/null 2>&1 && continue
  vfiobind $line
  done

  sudo qemu-system-x86_64 -machine type=q35,accel=kvm -cpu host,kvm=off \
  -smp 8,sockets=1,cores=4,threads=2 \
  -bios /usr/share/seabios/bios.bin \
  -serial none \
  -parallel none \
  -vga none \
  -m 4G \
  -mem-path /run/hugepages/kvm \
  -mem-prealloc \
  -balloon none \
  -rtc clock=host,base=localtime \
  -device 
ioh3420,bus=pcie.0,addr=1c.0,multifunction=on,port=1,chassis=1,id=root.1 \
  -device vfio-pci,host=01:00.0,bus=root.1,addr=00.0,multifunction=on,x-vga=on \
  -device virtio-scsi-pci,id=scsi \
  -drive 
id=disk0,if=virtio,cache=none,format=raw,file=/home/dad/qemu/windows7.img \
  -drive 
file=/home/dad/1TB-Backup/Iso/SP1ForWin7.iso,id=isocd,format=raw,if=none 
-device scsi-cd,drive=isocd \
  -net nic -net tap,ifname=tap0,script=no,downscript=no \
  -usbdevice host:413c:a503 \
  -usbdevice host:13fe:3100 \
  -usbdevice host:0bc2:ab21 \
  -boot menu=on \
  -boot order=c

  sudo ./down.sh tap0

  exit 0

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

Re: [Qemu-devel] [PATCH 04/10] blockjob: introduce block_job_pause/resume_all

[John Snow]

On 03/23/2017 01:39 PM, Paolo Bonzini wrote:
> Remove use of block_job_pause/resume from outside blockjob.c, thus
> making them static.  Again, one reason to do this is that
> block_job_pause/resume will have different locking rules compared
> to everything else that block.c and block/io.c use.
> 

Ominous

> Signed-off-by: Paolo Bonzini 
> ---
>  block/io.c   |  18 +---
>  blockjob.c   | 114 
> ---
>  include/block/blockjob.h |  14 +++---
>  3 files changed, 77 insertions(+), 69 deletions(-)
> 
> diff --git a/block/io.c b/block/io.c
> index 2709a70..1cc9318 100644
> --- a/block/io.c
> +++ b/block/io.c
> @@ -284,16 +284,9 @@ void bdrv_drain_all_begin(void)
>  bool waited = true;
>  BlockDriverState *bs;
>  BdrvNextIterator it;
> -BlockJob *job = NULL;
>  GSList *aio_ctxs = NULL, *ctx;
>  
> -while ((job = block_job_next(job))) {
> -AioContext *aio_context = blk_get_aio_context(job->blk);
> -
> -aio_context_acquire(aio_context);
> -block_job_pause(job);
> -aio_context_release(aio_context);
> -}
> +block_job_pause_all();
>  

Cool. Well, maybe cool. I think it would be cool if we didn't have to
explicitly pause jobs here at all. This is better at least, though.

>  for (bs = bdrv_first(); bs; bs = bdrv_next()) {
>  AioContext *aio_context = bdrv_get_aio_context(bs);
> @@ -337,7 +330,6 @@ void bdrv_drain_all_end(void)
>  {
>  BlockDriverState *bs;
>  BdrvNextIterator it;
> -BlockJob *job = NULL;
>  
>  for (bs = bdrv_first(); bs; bs = bdrv_next()) {
>  AioContext *aio_context = bdrv_get_aio_context(bs);
> @@ -348,13 +340,7 @@ void bdrv_drain_all_end(void)
>  aio_context_release(aio_context);
>  }
>  
> -while ((job = block_job_next(job))) {
> -AioContext *aio_context = blk_get_aio_context(job->blk);
> -
> -aio_context_acquire(aio_context);
> -block_job_resume(job);
> -aio_context_release(aio_context);
> -}
> +block_job_resume_all();
>  }
>  
>  void bdrv_drain_all(void)
> diff --git a/blockjob.c b/blockjob.c
> index 1c63d15..caca718 100644
> --- a/blockjob.c
> +++ b/blockjob.c
> @@ -55,36 +55,6 @@ struct BlockJobTxn {
>  
>  static QLIST_HEAD(, BlockJob) block_jobs = 
> QLIST_HEAD_INITIALIZER(block_jobs);
>  
> -static char *child_job_get_parent_desc(BdrvChild *c)
> -{
> -BlockJob *job = c->opaque;
> -return g_strdup_printf("%s job '%s'",
> -   BlockJobType_lookup[job->driver->job_type],
> -   job->id);
> -}
> -
> -static const BdrvChildRole child_job = {
> -.get_parent_desc= child_job_get_parent_desc,
> -.stay_at_node   = true,
> -};
> -
> -static void block_job_drained_begin(void *opaque)
> -{
> -BlockJob *job = opaque;
> -block_job_pause(job);
> -}
> -
> -static void block_job_drained_end(void *opaque)
> -{
> -BlockJob *job = opaque;
> -block_job_resume(job);
> -}
> -
> -static const BlockDevOps block_job_dev_ops = {
> -.drained_begin = block_job_drained_begin,
> -.drained_end = block_job_drained_end,
> -};
> -
>  BlockJob *block_job_next(BlockJob *job)
>  {
>  if (!job) {
> @@ -106,6 +76,21 @@ BlockJob *block_job_get(const char *id)
>  return NULL;
>  }
>  
> +static void block_job_pause(BlockJob *job)
> +{
> +job->pause_count++;
> +}
> +
> +static void block_job_resume(BlockJob *job)
> +{
> +assert(job->pause_count > 0);
> +job->pause_count--;
> +if (job->pause_count) {
> +return;
> +}
> +block_job_enter(job);
> +}
> +
>  static void block_job_ref(BlockJob *job)
>  {
>  ++job->refcnt;
> @@ -171,6 +156,36 @@ static void block_job_detach_aio_context(void *opaque)
>  block_job_unref(job);
>  }
>  
> +static char *child_job_get_parent_desc(BdrvChild *c)
> +{
> +BlockJob *job = c->opaque;
> +return g_strdup_printf("%s job '%s'",
> +   BlockJobType_lookup[job->driver->job_type],
> +   job->id);
> +}
> +
> +static const BdrvChildRole child_job = {
> +.get_parent_desc= child_job_get_parent_desc,
> +.stay_at_node   = true,
> +};
> +
> +static void block_job_drained_begin(void *opaque)
> +{
> +BlockJob *job = opaque;
> +block_job_pause(job);
> +}
> +
> +static void block_job_drained_end(void *opaque)
> +{
> +BlockJob *job = opaque;
> +block_job_resume(job);
> +}
> +
> +static const BlockDevOps block_job_dev_ops = {
> +.drained_begin = block_job_drained_begin,
> +.drained_end = block_job_drained_end,
> +};
> +

Movement should probably be its own patch, but I'm not a cop or nothin'

>  void block_job_remove_all_bdrv(BlockJob *job)
>  {
>  GSList *l;
> @@ -471,11 +486,6 @@ void block_job_complete(BlockJob *job, Error **errp)
>  job->driver->complete(job, errp);
>  }
>  
> -void block_job_pause(BlockJob *job)
> -{
> -

Re: [Qemu-devel] [PATCH 03/10] blockjob: introduce block_job_fail

[John Snow]

On 03/23/2017 01:39 PM, Paolo Bonzini wrote:
> Outside blockjob.c, block_job_unref is only used when a block job fails
> to start, and block_job_ref is not used at all.  The reference counting
> thus is pretty well hidden.  Introduce a separate function to be used
> by block jobs; because block_job_ref and block_job_unref now become
> static, move them earlier in blockjob.c.
> 

Early into the series, "I guess so."

It makes it a lot less obvious and more vague as to what exactly is
happening here, so I like it less.

> Later on, block_job_fail will also have different locking than
> block_job_unref.
> 
> Signed-off-by: Paolo Bonzini 
> ---
>  block/backup.c   |  2 +-
>  block/commit.c   |  2 +-
>  block/mirror.c   |  2 +-
>  blockjob.c   | 47 
> ++--
>  include/block/blockjob_int.h | 15 +++---
>  tests/test-blockjob.c| 10 +-
>  6 files changed, 39 insertions(+), 39 deletions(-)
> 
> diff --git a/block/backup.c b/block/backup.c
> index a4fb288..f284fd5 100644
> --- a/block/backup.c
> +++ b/block/backup.c
> @@ -692,7 +692,7 @@ BlockJob *backup_job_create(const char *job_id, 
> BlockDriverState *bs,
>  }
>  if (job) {
>  backup_clean(>common);
> -block_job_unref(>common);
> +block_job_fail(>common);
>  }
>  
>  return NULL;
> diff --git a/block/commit.c b/block/commit.c
> index 2832482..9b9ea39 100644
> --- a/block/commit.c
> +++ b/block/commit.c
> @@ -424,7 +424,7 @@ fail:
>  if (commit_top_bs) {
>  bdrv_set_backing_hd(overlay_bs, top, _abort);
>  }
> -block_job_unref(>common);
> +block_job_fail(>common);
>  }
>  
>  
> diff --git a/block/mirror.c b/block/mirror.c
> index ca4baa5..5cb2134 100644
> --- a/block/mirror.c
> +++ b/block/mirror.c
> @@ -1243,7 +1243,7 @@ fail:
>  if (s) {
>  g_free(s->replaces);
>  blk_unref(s->target);
> -block_job_unref(>common);
> +block_job_fail(>common);
>  }
>  
>  bdrv_child_try_set_perm(mirror_top_bs->backing, 0, BLK_PERM_ALL,
> diff --git a/blockjob.c b/blockjob.c
> index 24d1e12..1c63d15 100644
> --- a/blockjob.c
> +++ b/blockjob.c
> @@ -106,6 +106,32 @@ BlockJob *block_job_get(const char *id)
>  return NULL;
>  }
>  
> +static void block_job_ref(BlockJob *job)
> +{
> +++job->refcnt;
> +}
> +
> +static void block_job_attached_aio_context(AioContext *new_context,
> +   void *opaque);
> +static void block_job_detach_aio_context(void *opaque);
> +

^ Some tagalongs to the party?

> +static void block_job_unref(BlockJob *job)
> +{
> +if (--job->refcnt == 0) {
> +BlockDriverState *bs = blk_bs(job->blk);
> +bs->job = NULL;
> +block_job_remove_all_bdrv(job);
> +blk_remove_aio_context_notifier(job->blk,
> +block_job_attached_aio_context,
> +block_job_detach_aio_context, job);
> +blk_unref(job->blk);
> +error_free(job->blocker);
> +g_free(job->id);
> +QLIST_REMOVE(job, job_list);
> +g_free(job);
> +}
> +}
> +
>  static void block_job_attached_aio_context(AioContext *new_context,
> void *opaque)
>  {
> @@ -293,26 +319,9 @@ void block_job_start(BlockJob *job)
>  qemu_coroutine_enter(job->co);
>  }
>  
> -void block_job_ref(BlockJob *job)
> -{
> -++job->refcnt;
> -}
> -
> -void block_job_unref(BlockJob *job)
> +void block_job_fail(BlockJob *job)
>  {
> -if (--job->refcnt == 0) {
> -BlockDriverState *bs = blk_bs(job->blk);
> -bs->job = NULL;
> -block_job_remove_all_bdrv(job);
> -blk_remove_aio_context_notifier(job->blk,
> -block_job_attached_aio_context,
> -block_job_detach_aio_context, job);
> -blk_unref(job->blk);
> -error_free(job->blocker);
> -g_free(job->id);
> -QLIST_REMOVE(job, job_list);
> -g_free(job);
> -}
> +block_job_unref(job);
>  }
>  
>  static void block_job_completed_single(BlockJob *job)
> diff --git a/include/block/blockjob_int.h b/include/block/blockjob_int.h
> index bfcc5d1..97ffc43 100644
> --- a/include/block/blockjob_int.h
> +++ b/include/block/blockjob_int.h
> @@ -156,21 +156,12 @@ void block_job_sleep_ns(BlockJob *job, QEMUClockType 
> type, int64_t ns);
>  void block_job_yield(BlockJob *job);
>  
>  /**
> - * block_job_ref:
> + * block_job_fail:
>   * @bs: The block device.
>   *
> - * Grab a reference to the block job. Should be paired with block_job_unref.
> + * The block job could not be started, free it.
>   */
> -void block_job_ref(BlockJob *job);
> -
> -/**
> - * block_job_unref:
> - * @bs: The block device.
> - *
> - * Release reference to the block job and release resources if it is the 

Re: [Qemu-devel] [PATCH 01/10] blockjob: remove unnecessary check

[John Snow]

On 04/07/2017 06:54 PM, Philippe Mathieu-Daudé wrote:
> Hi Paolo,
> 
> On 03/23/2017 02:39 PM, Paolo Bonzini wrote:
>> !job is always checked prior to the call, drop it from here.
> 
> I agree with you this is currently true, *but* block_job_user_paused()
> is exported in "block/blockjob.h" so any future access (external to
> blockdev.c) could eventually happen with job == NULL.
> I'd  NACK this patch for for this reason, but I checked and there is no
> access to this function outside of blockdev.c, so I think the best is to
> make block_job_user_paused() static (removing the public declaration)
> and safely drop the !job check, what do you think?
> 

Sure, but I would consider this a strict improvement as asking for the
paused status of NULL should be an error, not zero.

Anyway, this function exists almost entirely for the sake of blockdev,
so deleting it out of the public jobs interface isn't a very nice thing
to do.

The "proper" thing might be to add *errp, but that's a lot of paint for
a really tiny shed.

"IMO, etc." I've already spent more time on this email than it'd take to
code that, so whichever way the wind blows is fine with me.

--js

>>
>> Signed-off-by: Paolo Bonzini 
>> ---
>>  blockjob.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/blockjob.c b/blockjob.c
>> index 9b619f385..a9fb624 100644
>> --- a/blockjob.c
>> +++ b/blockjob.c
>> @@ -480,7 +480,7 @@ static bool block_job_should_pause(BlockJob *job)
>>
>>  bool block_job_user_paused(BlockJob *job)
>>  {
>> -return job ? job->user_paused : 0;
>> +return job->user_paused;
>>  }
>>
>>  void coroutine_fn block_job_pause_point(BlockJob *job)
>>

Re: [Qemu-devel] [PATCH 02/10] blockjob: remove iostatus_reset callback

[John Snow]

On 03/23/2017 01:39 PM, Paolo Bonzini wrote:
> This is unused since commit 66a0fae ("blockjob: Don't touch BDS iostatus",
> 2016-05-19).
> 
> Signed-off-by: Paolo Bonzini 
> ---
>  blockjob.c   | 3 ---
>  include/block/blockjob_int.h | 3 ---
>  2 files changed, 6 deletions(-)
> 
> diff --git a/blockjob.c b/blockjob.c
> index a9fb624..24d1e12 100644
> --- a/blockjob.c
> +++ b/blockjob.c
> @@ -555,9 +555,6 @@ bool block_job_is_cancelled(BlockJob *job)
>  void block_job_iostatus_reset(BlockJob *job)
>  {
>  job->iostatus = BLOCK_DEVICE_IO_STATUS_OK;
> -if (job->driver->iostatus_reset) {
> -job->driver->iostatus_reset(job);
> -}
>  }
>  
>  static int block_job_finish_sync(BlockJob *job,
> diff --git a/include/block/blockjob_int.h b/include/block/blockjob_int.h
> index 3f86cc5..bfcc5d1 100644
> --- a/include/block/blockjob_int.h
> +++ b/include/block/blockjob_int.h
> @@ -44,9 +44,6 @@ struct BlockJobDriver {
>  /** Optional callback for job types that support setting a speed limit */
>  void (*set_speed)(BlockJob *job, int64_t speed, Error **errp);
>  
> -/** Optional callback for job types that need to forward I/O status 
> reset */
> -void (*iostatus_reset)(BlockJob *job);
> -
>  /** Mandatory: Entrypoint for the Coroutine. */
>  CoroutineEntry *start;
>  
> 

Good find.

Reviewed-by: John Snow 

[Qemu-devel] [Bug 1635339] Re: qxl_pre_save assertion failure on vm "save"

[elmarco]

Your approach works ok Gerd with a migration blocker. Are you going to
send a patch?

I am afraid we would have to make this code permanent though, since
there has been several releases of this driver already, and it's much
nicer to block migration than to crash a VM.

I have reached out to wddm driver about the bug.

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

Title:
  qxl_pre_save assertion failure on vm "save"

Status in QEMU:
  Confirmed

Bug description:
  When I try and save my Windows 10 VM, I see an assertion failure, and
  the machine is shut down.

  I see the following in the log:

  main_channel_handle_parsed: agent start
  qemu-system-x86_64: /build/qemu-Zwynhi/qemu-2.5+dfsg/hw/display/qxl.c:2101: 
qxl_pre_save: Assertion `d->last_release_offset < d->vga.vram_size' failed.
  2016-10-20 11:52:42.713+: shutting down

  Please let me know what other information would be relevant!

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

[Qemu-devel] [Bug 1670175] Re: qemu-system-sparc64 with tribblix-sparc-0m16.iso ends with "panic - kernel: no nucleus hblk8 to allocate"

[Michal Nowak]

Hi Mark, thank you for your effort on SPARC64 emulation in QEMU!

Thanks for the explanation on what might be wrong. Is there a way to
workaround the PCI problems?

Tribblix is indeed an illumos (a community fork of OpenSolaris)
distribution. Contrary to Milax, which looks abandoned to me as
OpenSolaris is, Tribblix and DilOS reflect recent illumos development
and until OpenIndiana SPARC edition materialize, probably should be a
reference solarish (sic) platforms.

As I hope to use qemu-system-sparc64 for automated validation of illumos
distributions, currently I am unable to provide anything but testing/QA
:).

Let me know should there be anything to test.

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

Title:
  qemu-system-sparc64 with tribblix-sparc-0m16.iso ends with "panic -
  kernel: no nucleus hblk8 to allocate"

Status in QEMU:
  New

Bug description:
  > qemu-system-sparc64 -m 1024 -cdrom Downloads/tribblix-sparc-0m16.iso -boot 
d -nographic
  OpenBIOS for Sparc64
  Configuration device id QEMU version 1 machine id 0
  kernel cmdline 
  CPUs: 1 x SUNW,UltraSPARC-IIi
  UUID: ----
  Welcome to OpenBIOS v1.1 built on Nov 24 2016 21:23
Type 'help' for detailed information
  Trying cdrom:f...
  Not a bootable ELF image
  Not a bootable a.out image

  Loading FCode image...
  Loaded 7120 bytes
  entry point is 0x4000
  Evaluating FCode...
  Evaluating FCode...
  Ignoring failed claim for va 10a96a0 memsz 19!
  Ignoring failed claim for va 100 memsz d1fb6!
  Ignoring failed claim for va 1402000 memsz 32518!
  Ignoring failed claim for va 180 memsz 52ac8!
  SunOS Release 5.11 Version tribblix-m16 64-bit
  Copyright (c) 1983, 2010, Oracle and/or its affiliates. All rights reserved.
  could not find debugger-vocabulary-hook>threads:interpret: exception -13 
caught
  interpret \ ident "%Z%%M% %I% %E% SMI"
  \ Copyright 2005 Sun Microsystems, Inc.  All rights reserved.
  \ Use is subject to license terms.
  \
  \ CDDL HEADER START
  \
  \ The contents of this file are subject to the terms of the
  \ Common Development and Distribution License, Version 1.0 only
  \ (the "License").  You may not use this file except in compliance
  \ with the License.
  \
  \ You can obtain a copy of the license at usr/src/OPENSOLARIS.LICENSE
  \ or http://www.opensolaris.org/os/licensing.
  \ See the License for 
  WARNING: add_spec: No major number for sf
  panic - kernel: no nucleus hblk8 to allocate
  EXIT

  QEMU keeps running (CPU is on 100 % all the time), I can interact with
  the prompt:

  0 > boot
  Not a Linux kernel image
  Not a bootable ELF image
  Not a bootable a.out image

  Loading FCode image...
  Unhandled Exception 0x0018
  PC = 0xffd25310 NPC = 0xffd25314
  Stopping execution

  > qemu-system-sparc64 -version
  QEMU emulator version 2.8.0(Virtualization:Staging / SLE_12_SP2)

  from
  https://build.opensuse.org/package/show/Virtualization:Staging/qemu on
  openSUSE Leap 42.2.

  ISO: http://pkgs.tribblix.org/iso/tribblix-sparc-0m16.iso.

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

Re: [Qemu-devel] [PATCH 01/10] blockjob: remove unnecessary check

[Philippe Mathieu-Daudé]

Hi Paolo,

On 03/23/2017 02:39 PM, Paolo Bonzini wrote:

!job is always checked prior to the call, drop it from here.


I agree with you this is currently true, *but* block_job_user_paused() 
is exported in "block/blockjob.h" so any future access (external to 
blockdev.c) could eventually happen with job == NULL.
I'd  NACK this patch for for this reason, but I checked and there is no 
access to this function outside of blockdev.c, so I think the best is to 
make block_job_user_paused() static (removing the public declaration) 
and safely drop the !job check, what do you think?




Signed-off-by: Paolo Bonzini 
---
 blockjob.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/blockjob.c b/blockjob.c
index 9b619f385..a9fb624 100644
--- a/blockjob.c
+++ b/blockjob.c
@@ -480,7 +480,7 @@ static bool block_job_should_pause(BlockJob *job)

 bool block_job_user_paused(BlockJob *job)
 {
-return job ? job->user_paused : 0;
+return job->user_paused;
 }

 void coroutine_fn block_job_pause_point(BlockJob *job)

Re: [Qemu-devel] [PATCH 01/10] blockjob: remove unnecessary check

[John Snow]

On 03/23/2017 01:39 PM, Paolo Bonzini wrote:
> !job is always checked prior to the call, drop it from here.
> 
> Signed-off-by: Paolo Bonzini 
> ---
>  blockjob.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/blockjob.c b/blockjob.c
> index 9b619f385..a9fb624 100644
> --- a/blockjob.c
> +++ b/blockjob.c
> @@ -480,7 +480,7 @@ static bool block_job_should_pause(BlockJob *job)
>  
>  bool block_job_user_paused(BlockJob *job)
>  {
> -return job ? job->user_paused : 0;
> +return job->user_paused;
>  }
>  
>  void coroutine_fn block_job_pause_point(BlockJob *job)
> 

Reviewed-by: John Snow 

[Qemu-devel] [Bug 1635339] Re: qxl_pre_save assertion failure on vm "save"

[Gerd Hoffmann]

Not sure we want a failure mode for pre_save().

If we go for option (a) (from comment 9), I'd add a check when reading
the commands from the ring, not at migration time, so we don't run enter
a state where pre_save() can fail in the first place.  Because that will
break the windows drivers we might add a warning only for 2.9, then for
2.10 raise an error irq.  Something like this:

--- a/hw/display/qxl.c
+++ b/hw/display/qxl.c
@@ -639,6 +639,24 @@ static int interface_get_command(QXLInstance *sin, struct 
QXLCommandExt *ext)
 qxl->guest_primary.commands++;
 qxl_track_command(qxl, ext);
 qxl_log_command(qxl, "cmd", ext);
+{
+void *msg = qxl_phys2virt(qxl, ext->cmd.data, ext->group_id);
+if (msg < (void *)qxl->vga.vram_ptr ||
+msg > ((void *)qxl->vga.vram_ptr + qxl->vga.vram_size)) {
+#if 1
+/* temporary, for 2.9 */
+static int once;
+if (!once) {
+fprintf(stderr, "qxl: guest bug: command not in ram bar, "
+"guest not migratable\n");
+once = true;
+}
+#else
+qxl_set_guest_bug(qxl, "command not in ram bar");
+return false;
+#endif
+}
+}
 trace_qxl_ring_command_get(qxl->id, qxl_mode_to_string(qxl->mode));
 return true;
 default:

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

Title:
  qxl_pre_save assertion failure on vm "save"

Status in QEMU:
  Confirmed

Bug description:
  When I try and save my Windows 10 VM, I see an assertion failure, and
  the machine is shut down.

  I see the following in the log:

  main_channel_handle_parsed: agent start
  qemu-system-x86_64: /build/qemu-Zwynhi/qemu-2.5+dfsg/hw/display/qxl.c:2101: 
qxl_pre_save: Assertion `d->last_release_offset < d->vga.vram_size' failed.
  2016-10-20 11:52:42.713+: shutting down

  Please let me know what other information would be relevant!

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

[Qemu-devel] [PATCH v2 4/4] qga: improve error handling in transfer_memory_block

[Philippe Mathieu-Daudé]

Suggested-by: Michael Roth 
Signed-off-by: Philippe Mathieu-Daudé 
---

Michael should I use Signed-off-by instead of the Suggested-by (since it is your
code)?

 qga/commands-posix.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/qga/commands-posix.c b/qga/commands-posix.c
index fc45102a1e..ca5a24b2c9 100644
--- a/qga/commands-posix.c
+++ b/qga/commands-posix.c
@@ -2128,6 +2128,9 @@ static void transfer_memory_block(GuestMemoryBlock 
*mem_blk, bool sys2memblk,
 if (errno == ENOENT) {
 result->response =
 GUEST_MEMORY_BLOCK_RESPONSE_TYPE_OPERATION_NOT_SUPPORTED;
+} else {
+result->response =
+GUEST_MEMORY_BLOCK_RESPONSE_TYPE_OPERATION_FAILED;
 }
 goto out1;
 }
-- 
2.11.0

[Qemu-devel] [PATCH v2 2/4] device_tree: fix compiler warnings (clang 5)

[Philippe Mathieu-Daudé]

static code analyzer complain:

device_tree.c:155:18: warning: Null pointer passed as an argument to a 
'nonnull' parameter
while ((de = readdir(d)) != NULL) {
 ^~

Reported-by: Clang Static Analyzer
Signed-off-by: Philippe Mathieu-Daudé 
Reviewed-by: Marc-André Lureau 
---
 device_tree.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/device_tree.c b/device_tree.c
index 6e06320830..a24ddff02b 100644
--- a/device_tree.c
+++ b/device_tree.c
@@ -148,6 +148,7 @@ static void read_fstree(void *fdt, const char *dirname)
 d = opendir(dirname);
 if (!d) {
 error_setg(_fatal, "%s cannot open %s", __func__, dirname);
+return;
 }
 
 while ((de = readdir(d)) != NULL) {
-- 
2.11.0

[Qemu-devel] [PATCH v2 1/4] usb-ccid: make ccid_write_data_block() cope with null buffers

[Philippe Mathieu-Daudé]

static code analyzer complain:

hw/usb/dev-smartcard-reader.c:816:5: warning: Null pointer passed as an 
argument to a 'nonnull' parameter
memcpy(p->abData, data, len);
^~~~

Reported-by: Clang Static Analyzer
Signed-off-by: Philippe Mathieu-Daudé 
---
 hw/usb/dev-smartcard-reader.c | 5 -
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/hw/usb/dev-smartcard-reader.c b/hw/usb/dev-smartcard-reader.c
index 757b8b3f5a..49cb1829b5 100644
--- a/hw/usb/dev-smartcard-reader.c
+++ b/hw/usb/dev-smartcard-reader.c
@@ -813,7 +813,10 @@ static void ccid_write_data_block(USBCCIDState *s, uint8_t 
slot, uint8_t seq,
 if (p->b.bError) {
 DPRINTF(s, D_VERBOSE, "error %d\n", p->b.bError);
 }
-memcpy(p->abData, data, len);
+if (len) {
+g_assert_nonnull(data);
+memcpy(p->abData, data, len);
+}
 ccid_reset_error_status(s);
 usb_wakeup(s->bulk, 0);
 }
-- 
2.11.0

[Qemu-devel] [PATCH v2 3/4] qga: fix compiler warnings (clang 5)

[Philippe Mathieu-Daudé]

static code analyzer complain:

qga/commands-posix.c:2127:9: warning: Null pointer passed as an argument to a 
'nonnull' parameter
closedir(dp);
^~~~

Reported-by: Clang Static Analyzer
Signed-off-by: Philippe Mathieu-Daudé 
Reviewed-by: Marc-André Lureau 
---
 qga/commands-posix.c | 8 +---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/qga/commands-posix.c b/qga/commands-posix.c
index 915df9ed90..fc45102a1e 100644
--- a/qga/commands-posix.c
+++ b/qga/commands-posix.c
@@ -2124,9 +2124,11 @@ static void transfer_memory_block(GuestMemoryBlock 
*mem_blk, bool sys2memblk,
  * we think this VM does not support online/offline memory block,
  * any other solution?
  */
-if (!dp && errno == ENOENT) {
-result->response =
-GUEST_MEMORY_BLOCK_RESPONSE_TYPE_OPERATION_NOT_SUPPORTED;
+if (!dp) {
+if (errno == ENOENT) {
+result->response =
+GUEST_MEMORY_BLOCK_RESPONSE_TYPE_OPERATION_NOT_SUPPORTED;
+}
 goto out1;
 }
 closedir(dp);
-- 
2.11.0

[Qemu-devel] [PATCH v2 0/4] 3 easy-to-fix clang warnings, 1 error code fixed

[Philippe Mathieu-Daudé]

This patchset fixes three easy-to-fix unrelated clang warnings, so it was
probably better to send them separately the first time.

v2:
- hw/usb/dev-smartcard-reader.c: The first patch was treating NULL and 0 as
  errors but those values are legal in some cases, i.e. no (virtual)
  smartcard inserted into the card reader (Gerd Hoffmann's review).
- device_tree.c: Added Marc-André Lureau's Reviewed-by.
- qga/commands-posix.c: Added a commit to improve failed operation, as
  suggested by Michael Roth.

Philippe Mathieu-Daudé (4):
  usb-ccid: make ccid_write_data_block() cope with null buffers
  device_tree: fix compiler warnings (clang 5)
  qga: fix compiler warnings (clang 5)
  qga: improve error handling in transfer_memory_block

 device_tree.c |  1 +
 hw/usb/dev-smartcard-reader.c |  5 -
 qga/commands-posix.c  | 11 ---
 3 files changed, 13 insertions(+), 4 deletions(-)

-- 
2.11.0

[Qemu-devel] [Bug 1680679] Re: qemu cannot run twice

[Roger Lawhorn]

It works using seabios. I assume that the nvidia driver cannot see
optimus, and expect an intel card also, unless I use OVMF. I do know
that you cannot run off the intel card in windows. It's a no-no. Thanks
for the bios tip. Maybe I can hide the optimus feature from OVMF and
windows.

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

Title:
  qemu cannot run twice

Status in QEMU:
  Invalid

Bug description:
  After using qemu with gpu passthrough and then shutting down windows 7 
properly I cannot boot windows 7 a second time.
  Only a full reboot of linux fixes this issue.
  Qemu appears to corrupt something in linux when exiting.
  I get no error messages but windows 7 never finishes booting during the 2nd 
try.
  Apparently I do try to run vfiobind each time the script is run.
  Wondering if rerunning vfiobind can cause an issue?

  
  My specs:
  
-
  System:Host: GT70-2PE Kernel: 4.5.4-040504-generic x86_64 (64 bit gcc: 
5.3.1)
 Desktop: Cinnamon 3.2.7 (Gtk 3.18.9) Distro: Linux Mint 18.1 Serena
  Machine:   Mobo: Micro-Star model: MS-1763 v: REV:0.C Bios: American 
Megatrends v: E1763IMS.51B date: 01/29/2015
  CPU:   Quad core Intel Core i7-4810MQ (-HT-MCP-) cache: 6144 KB
 flags: (lm nx sse sse2 sse3 sse4_1 sse4_2 ssse3 vmx) bmips: 22347
 clock speeds: max: 2801 MHz 1: 2801 MHz 2: 2801 MHz 3: 2801 MHz 4: 
2801 MHz 5: 2801 MHz 6: 2801 MHz
 7: 2801 MHz 8: 2801 MHz
  Graphics:  Card-1: Intel 4th Gen Core Processor Integrated Graphics 
Controller bus-ID: 00:02.0
 Card-2: NVIDIA GK104M [GeForce GTX 880M] bus-ID: 01:00.0
 Display Server: X.Org 1.18.4 drivers: intel (unloaded: fbdev,vesa)
 Resolution: 1920x1080@60.02hz, 1920x1080@60.00hz
 GLX Renderer: Mesa DRI Intel Haswell Mobile GLX Version: 3.0 Mesa 
12.0.6 Direct Rendering: Yes

  
  My script:
  
---
  #!/bin/bash

  cd ~/qemu
  sudo ./up.sh tap0

  configfile=~/qemu/vfio-pci1.cfg

  vfiobind() {
  dev="$1"
  vendor=$(cat /sys/bus/pci/devices/$dev/vendor)
  device=$(cat /sys/bus/pci/devices/$dev/device)
  if [ -e /sys/bus/pci/devices/$dev/driver ]; then
  echo $dev > /sys/bus/pci/devices/$dev/driver/unbind
  fi
  echo $vendor $device > /sys/bus/pci/drivers/vfio-pci/new_id

  }

  modprobe vfio-pci

  cat $configfile | while read line;do
  echo $line | grep ^# >/dev/null 2>&1 && continue
  vfiobind $line
  done

  sudo qemu-system-x86_64 -machine type=q35,accel=kvm -cpu host,kvm=off \
  -smp 8,sockets=1,cores=4,threads=2 \
  -bios /usr/share/seabios/bios.bin \
  -serial none \
  -parallel none \
  -vga none \
  -m 4G \
  -mem-path /run/hugepages/kvm \
  -mem-prealloc \
  -balloon none \
  -rtc clock=host,base=localtime \
  -device 
ioh3420,bus=pcie.0,addr=1c.0,multifunction=on,port=1,chassis=1,id=root.1 \
  -device vfio-pci,host=01:00.0,bus=root.1,addr=00.0,multifunction=on,x-vga=on \
  -device virtio-scsi-pci,id=scsi \
  -drive 
id=disk0,if=virtio,cache=none,format=raw,file=/home/dad/qemu/windows7.img \
  -drive 
file=/home/dad/1TB-Backup/Iso/SP1ForWin7.iso,id=isocd,format=raw,if=none 
-device scsi-cd,drive=isocd \
  -net nic -net tap,ifname=tap0,script=no,downscript=no \
  -usbdevice host:413c:a503 \
  -usbdevice host:13fe:3100 \
  -usbdevice host:0bc2:ab21 \
  -boot menu=on \
  -boot order=c

  sudo ./down.sh tap0

  exit 0

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

[Qemu-devel] [Bug 1680679] Re: qemu cannot run twice

[Alex Williamson]

I suspect the 'M' in GTX880M is the biggest contributor to the Code 43.
The fact you can get it to work once per host boot on SeaBIOS is a
fluke.  If you can get custom ROMs, you might try playing with the vfio-
pci x-pci-device-id option to masquerade as a discrete card, maybe that
would avoid mobile code in the NVIDIA driver that would expect Optimus.
Obviously do so at your own risk.

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

Title:
  qemu cannot run twice

Status in QEMU:
  Invalid

Bug description:
  After using qemu with gpu passthrough and then shutting down windows 7 
properly I cannot boot windows 7 a second time.
  Only a full reboot of linux fixes this issue.
  Qemu appears to corrupt something in linux when exiting.
  I get no error messages but windows 7 never finishes booting during the 2nd 
try.
  Apparently I do try to run vfiobind each time the script is run.
  Wondering if rerunning vfiobind can cause an issue?

  
  My specs:
  
-
  System:Host: GT70-2PE Kernel: 4.5.4-040504-generic x86_64 (64 bit gcc: 
5.3.1)
 Desktop: Cinnamon 3.2.7 (Gtk 3.18.9) Distro: Linux Mint 18.1 Serena
  Machine:   Mobo: Micro-Star model: MS-1763 v: REV:0.C Bios: American 
Megatrends v: E1763IMS.51B date: 01/29/2015
  CPU:   Quad core Intel Core i7-4810MQ (-HT-MCP-) cache: 6144 KB
 flags: (lm nx sse sse2 sse3 sse4_1 sse4_2 ssse3 vmx) bmips: 22347
 clock speeds: max: 2801 MHz 1: 2801 MHz 2: 2801 MHz 3: 2801 MHz 4: 
2801 MHz 5: 2801 MHz 6: 2801 MHz
 7: 2801 MHz 8: 2801 MHz
  Graphics:  Card-1: Intel 4th Gen Core Processor Integrated Graphics 
Controller bus-ID: 00:02.0
 Card-2: NVIDIA GK104M [GeForce GTX 880M] bus-ID: 01:00.0
 Display Server: X.Org 1.18.4 drivers: intel (unloaded: fbdev,vesa)
 Resolution: 1920x1080@60.02hz, 1920x1080@60.00hz
 GLX Renderer: Mesa DRI Intel Haswell Mobile GLX Version: 3.0 Mesa 
12.0.6 Direct Rendering: Yes

  
  My script:
  
---
  #!/bin/bash

  cd ~/qemu
  sudo ./up.sh tap0

  configfile=~/qemu/vfio-pci1.cfg

  vfiobind() {
  dev="$1"
  vendor=$(cat /sys/bus/pci/devices/$dev/vendor)
  device=$(cat /sys/bus/pci/devices/$dev/device)
  if [ -e /sys/bus/pci/devices/$dev/driver ]; then
  echo $dev > /sys/bus/pci/devices/$dev/driver/unbind
  fi
  echo $vendor $device > /sys/bus/pci/drivers/vfio-pci/new_id

  }

  modprobe vfio-pci

  cat $configfile | while read line;do
  echo $line | grep ^# >/dev/null 2>&1 && continue
  vfiobind $line
  done

  sudo qemu-system-x86_64 -machine type=q35,accel=kvm -cpu host,kvm=off \
  -smp 8,sockets=1,cores=4,threads=2 \
  -bios /usr/share/seabios/bios.bin \
  -serial none \
  -parallel none \
  -vga none \
  -m 4G \
  -mem-path /run/hugepages/kvm \
  -mem-prealloc \
  -balloon none \
  -rtc clock=host,base=localtime \
  -device 
ioh3420,bus=pcie.0,addr=1c.0,multifunction=on,port=1,chassis=1,id=root.1 \
  -device vfio-pci,host=01:00.0,bus=root.1,addr=00.0,multifunction=on,x-vga=on \
  -device virtio-scsi-pci,id=scsi \
  -drive 
id=disk0,if=virtio,cache=none,format=raw,file=/home/dad/qemu/windows7.img \
  -drive 
file=/home/dad/1TB-Backup/Iso/SP1ForWin7.iso,id=isocd,format=raw,if=none 
-device scsi-cd,drive=isocd \
  -net nic -net tap,ifname=tap0,script=no,downscript=no \
  -usbdevice host:413c:a503 \
  -usbdevice host:13fe:3100 \
  -usbdevice host:0bc2:ab21 \
  -boot menu=on \
  -boot order=c

  sudo ./down.sh tap0

  exit 0

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

[Qemu-devel] [Bug 1680679] Re: qemu cannot run twice

[Roger Lawhorn]

ok, so I will research qemu and nvidia optimus. I have a custom BIOS
made by an MSI employee. I have hundreds of bios options to play with.

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

Title:
  qemu cannot run twice

Status in QEMU:
  Invalid

Bug description:
  After using qemu with gpu passthrough and then shutting down windows 7 
properly I cannot boot windows 7 a second time.
  Only a full reboot of linux fixes this issue.
  Qemu appears to corrupt something in linux when exiting.
  I get no error messages but windows 7 never finishes booting during the 2nd 
try.
  Apparently I do try to run vfiobind each time the script is run.
  Wondering if rerunning vfiobind can cause an issue?

  
  My specs:
  
-
  System:Host: GT70-2PE Kernel: 4.5.4-040504-generic x86_64 (64 bit gcc: 
5.3.1)
 Desktop: Cinnamon 3.2.7 (Gtk 3.18.9) Distro: Linux Mint 18.1 Serena
  Machine:   Mobo: Micro-Star model: MS-1763 v: REV:0.C Bios: American 
Megatrends v: E1763IMS.51B date: 01/29/2015
  CPU:   Quad core Intel Core i7-4810MQ (-HT-MCP-) cache: 6144 KB
 flags: (lm nx sse sse2 sse3 sse4_1 sse4_2 ssse3 vmx) bmips: 22347
 clock speeds: max: 2801 MHz 1: 2801 MHz 2: 2801 MHz 3: 2801 MHz 4: 
2801 MHz 5: 2801 MHz 6: 2801 MHz
 7: 2801 MHz 8: 2801 MHz
  Graphics:  Card-1: Intel 4th Gen Core Processor Integrated Graphics 
Controller bus-ID: 00:02.0
 Card-2: NVIDIA GK104M [GeForce GTX 880M] bus-ID: 01:00.0
 Display Server: X.Org 1.18.4 drivers: intel (unloaded: fbdev,vesa)
 Resolution: 1920x1080@60.02hz, 1920x1080@60.00hz
 GLX Renderer: Mesa DRI Intel Haswell Mobile GLX Version: 3.0 Mesa 
12.0.6 Direct Rendering: Yes

  
  My script:
  
---
  #!/bin/bash

  cd ~/qemu
  sudo ./up.sh tap0

  configfile=~/qemu/vfio-pci1.cfg

  vfiobind() {
  dev="$1"
  vendor=$(cat /sys/bus/pci/devices/$dev/vendor)
  device=$(cat /sys/bus/pci/devices/$dev/device)
  if [ -e /sys/bus/pci/devices/$dev/driver ]; then
  echo $dev > /sys/bus/pci/devices/$dev/driver/unbind
  fi
  echo $vendor $device > /sys/bus/pci/drivers/vfio-pci/new_id

  }

  modprobe vfio-pci

  cat $configfile | while read line;do
  echo $line | grep ^# >/dev/null 2>&1 && continue
  vfiobind $line
  done

  sudo qemu-system-x86_64 -machine type=q35,accel=kvm -cpu host,kvm=off \
  -smp 8,sockets=1,cores=4,threads=2 \
  -bios /usr/share/seabios/bios.bin \
  -serial none \
  -parallel none \
  -vga none \
  -m 4G \
  -mem-path /run/hugepages/kvm \
  -mem-prealloc \
  -balloon none \
  -rtc clock=host,base=localtime \
  -device 
ioh3420,bus=pcie.0,addr=1c.0,multifunction=on,port=1,chassis=1,id=root.1 \
  -device vfio-pci,host=01:00.0,bus=root.1,addr=00.0,multifunction=on,x-vga=on \
  -device virtio-scsi-pci,id=scsi \
  -drive 
id=disk0,if=virtio,cache=none,format=raw,file=/home/dad/qemu/windows7.img \
  -drive 
file=/home/dad/1TB-Backup/Iso/SP1ForWin7.iso,id=isocd,format=raw,if=none 
-device scsi-cd,drive=isocd \
  -net nic -net tap,ifname=tap0,script=no,downscript=no \
  -usbdevice host:413c:a503 \
  -usbdevice host:13fe:3100 \
  -usbdevice host:0bc2:ab21 \
  -boot menu=on \
  -boot order=c

  sudo ./down.sh tap0

  exit 0

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

Re: [Qemu-devel] [PATCH RESEND 1/3] usb-ccid: make ccid_write_data_block() cope with null buffers

[Philippe Mathieu-Daudé]

Hi Markus, Gerd.

On 03/23/2017 11:08 AM, Markus Armbruster wrote:

Gerd Hoffmann  writes:


On Do, 2017-03-23 at 13:41 +0100, Markus Armbruster wrote:

Gerd Hoffmann  writes:


  Hi,


 oops, there are hard-coded calls with NULL/0. I suppose to fix clang
warning, it would need to check if data != null for memcpy.


I'd check for len > 0, and in that if branch we can also assert on data
== NULL and thereby check that len and data are consistent.


If len is non-zero but data is null, memcpy() will crash just fine by
itself, so why bother asserting.


To make clang happy?  But maybe clang is clever enough to figure data
can't be null at that point in case we call memcpy with len != 0
only ...


If Clang needs another hint to become happy, then an assertion is a fine
way to provide it.


Apparently Clang isn't clever enough using an assertion.

I'll resend checking len.

Re: [Qemu-devel] [PATCH RESEND 3/3] qga: fix compiler warnings (clang 5)

[Philippe Mathieu-Daudé]

Hi Michael,

On 03/22/2017 06:22 PM, Michael Roth wrote:

Quoting Philippe Mathieu-Daudé (2017-03-22 15:48:44)

static code analyzer complain:

qga/commands-posix.c:2127:9: warning: Null pointer passed as an argument to a 
'nonnull' parameter
closedir(dp);
^~~~

Reported-by: Clang Static Analyzer
Signed-off-by: Philippe Mathieu-Daudé 
Reviewed-by: Marc-André Lureau 
---
 qga/commands-posix.c | 8 +---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/qga/commands-posix.c b/qga/commands-posix.c
index 73d93eb5ce..8028141534 100644
--- a/qga/commands-posix.c
+++ b/qga/commands-posix.c
@@ -2122,9 +2122,11 @@ static void transfer_memory_block(GuestMemoryBlock 
*mem_blk, bool sys2memblk,
  * we think this VM does not support online/offline memory block,
  * any other solution?
  */
-if (!dp && errno == ENOENT) {
-result->response =
-GUEST_MEMORY_BLOCK_RESPONSE_TYPE_OPERATION_NOT_SUPPORTED;
+if (!dp) {
+if (errno == ENOENT) {
+result->response =
+GUEST_MEMORY_BLOCK_RESPONSE_TYPE_OPERATION_NOT_SUPPORTED;
+}
 goto out1;
 }


I think this should be:

if (!dp) {
if (errno == ENOENT) {
result->response =
GUEST_MEMORY_BLOCK_RESPONSE_TYPE_OPERATION_NOT_SUPPORTED;
} else {
result->response =
GUEST_MEMORY_BLOCK_RESPONSE_TYPE_OPERATION_FAILED;
}
goto out1;
}

otherwise we might set result->error_code while still indicating
success for the operation. That wasn't handled properly before your
patch either, it's just more apparent now.


Indeed, I'll resend it in 2 patches It seems easier to me to understand.




 closedir(dp);
--
2.11.0

Re: [Qemu-devel] [PATCH for-2.9?] tests/check-qdict: Fix missing brackets

[Philippe Mathieu-Daudé]

On 04/06/2017 12:55 PM, Eric Blake wrote:

On 04/06/2017 10:49 AM, Eric Blake wrote:

On 04/06/2017 10:41 AM, Dr. David Alan Gilbert (git) wrote:

From: "Dr. David Alan Gilbert" 

Gcc 7 (on Fedora 26) spotted odd use of integers instead of a
boolean; it's got a point.

Signed-off-by: Dr. David Alan Gilbert 
---
 tests/check-qdict.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/check-qdict.c b/tests/check-qdict.c
index 81162ee572..6f3fbcf9c1 100644
--- a/tests/check-qdict.c
+++ b/tests/check-qdict.c
@@ -559,7 +559,7 @@ static void qdict_join_test(void)
 g_assert(qdict_size(dict1) == 2);
 g_assert(qdict_size(dict2) == !overwrite);

-g_assert(qdict_get_int(dict1, "foo") == overwrite ? 84 : 42);
+g_assert(qdict_get_int(dict1, "foo") == (overwrite ? 84 : 42));


How is the test passing pre-patch, and why does it not change the test
post-patch?  Does that mean that overwrite is not doing what we expected?


Replying to myself:

Pre-patch, it was reached twice (once for overwrite=false, once for
overwrite=true), as:

(42 == false) ? 84 : 42
(84 == true) ? 84 : 42

which simplifies to 42 on both iterations, and g_assert(42) succeeds.
In fact, this is a tautology - no matter WHAT value we encounter, the
assert succeeds, so we are not actually testing that the value matters.

Post-patch, it becomes:

42 == (false ? 84 : 42)
84 == (true ? 84 : 42)

which is then asserting that we actually have the value we expect.

Reviewed-by: Eric Blake 

Safe for 2.9, but late enough that it also doesn't matter if it slips
until 2.10.



another case which demonstrate human _optimized_ C is more bug prone 
than verbose basic code...


Most cpp generates the same asm code with the following C:

if (overwrite) {
g_assert(qdict_get_int(dict1, "foo") == 84);
} else {
g_assert(qdict_get_int(dict1, "foo") == 42);
}

Reviewed-by: Philippe Mathieu-Daudé 

[Qemu-devel] [Bug 1680679] Re: qemu cannot run twice

[Roger Lawhorn]

My uefi build script. If you see an error or issue that could cause
error 43 please confirm it:

#!/bin/bash

cd /home/dad/qemu/qemu2
sudo ./up.sh tap0

configfile=./vfio-pci1.cfg

vfiobind() {
dev="$1"
vendor=$(cat /sys/bus/pci/devices/$dev/vendor)
device=$(cat /sys/bus/pci/devices/$dev/device)
if [ -e /sys/bus/pci/devices/$dev/driver ]; then
echo $dev > /sys/bus/pci/devices/$dev/driver/unbind
fi
echo $vendor $device > /sys/bus/pci/drivers/vfio-pci/new_id

}

modprobe vfio-pci

cat $configfile | while read line;do
echo $line | grep ^# >/dev/null 2>&1 && continue
vfiobind $line
done

vmname="windows10vm"

if ps -A | grep -q $vmname; then
   echo "$vmname is already running." &
   exit 1

else

# use pulseaudio
export QEMU_AUDIO_DRV=pa

cp /usr/share/OVMF/OVMF_VARS.fd /tmp/my_vars.fd

qemu-system-x86_64 \
  -name $vmname,process=$vmname \
  -machine type=pc-i440fx-2.6,accel=kvm \
  -cpu host,kvm=off \
  -smp 8,sockets=1,cores=4,threads=2 \
  -enable-kvm \
  -m 4G \
  -mem-path /run/hugepages/kvm \
  -mem-prealloc \
  -balloon none \
  -rtc clock=host,base=localtime \
  -serial none \
  -parallel none \
  -vga none \
  -device vfio-pci,host=01:00.0,multifunction=on \
  -device vfio-pci,host=01:00.1 \
  -drive 
if=pflash,format=raw,readonly,file=/home/dad/qemu/qemu2/OVMF-pure-efi.fd \
  -drive if=pflash,format=raw,file=/tmp/my_vars.fd \
  -boot order=dc -boot menu=on \
  -device virtio-scsi-pci,id=scsi \
  -drive 
id=disk0,if=virtio,cache=none,format=raw,file=/home/dad/qemu/qemu2/win7.img \
  -drive 
file=/home/dad/qemu/qemu2/virtio-win-0.1.126.iso,id=virtiocd,format=raw,if=none 
-device ide-cd,bus=ide.1,drive=virtiocd \
  -netdev type=tap,id=net0,ifname=tap0,vhost=on,script=no,downscript=no \
  -device virtio-net-pci,netdev=net0,mac=00:16:3e:00:01:01 \
  -usb -usbdevice host:413c:a503 -usbdevice host:13fe:3100

sudo ./down.sh tap0
   exit 0
fi

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

Title:
  qemu cannot run twice

Status in QEMU:
  Invalid

Bug description:
  After using qemu with gpu passthrough and then shutting down windows 7 
properly I cannot boot windows 7 a second time.
  Only a full reboot of linux fixes this issue.
  Qemu appears to corrupt something in linux when exiting.
  I get no error messages but windows 7 never finishes booting during the 2nd 
try.
  Apparently I do try to run vfiobind each time the script is run.
  Wondering if rerunning vfiobind can cause an issue?

  
  My specs:
  
-
  System:Host: GT70-2PE Kernel: 4.5.4-040504-generic x86_64 (64 bit gcc: 
5.3.1)
 Desktop: Cinnamon 3.2.7 (Gtk 3.18.9) Distro: Linux Mint 18.1 Serena
  Machine:   Mobo: Micro-Star model: MS-1763 v: REV:0.C Bios: American 
Megatrends v: E1763IMS.51B date: 01/29/2015
  CPU:   Quad core Intel Core i7-4810MQ (-HT-MCP-) cache: 6144 KB
 flags: (lm nx sse sse2 sse3 sse4_1 sse4_2 ssse3 vmx) bmips: 22347
 clock speeds: max: 2801 MHz 1: 2801 MHz 2: 2801 MHz 3: 2801 MHz 4: 
2801 MHz 5: 2801 MHz 6: 2801 MHz
 7: 2801 MHz 8: 2801 MHz
  Graphics:  Card-1: Intel 4th Gen Core Processor Integrated Graphics 
Controller bus-ID: 00:02.0
 Card-2: NVIDIA GK104M [GeForce GTX 880M] bus-ID: 01:00.0
 Display Server: X.Org 1.18.4 drivers: intel (unloaded: fbdev,vesa)
 Resolution: 1920x1080@60.02hz, 1920x1080@60.00hz
 GLX Renderer: Mesa DRI Intel Haswell Mobile GLX Version: 3.0 Mesa 
12.0.6 Direct Rendering: Yes

  
  My script:
  
---
  #!/bin/bash

  cd ~/qemu
  sudo ./up.sh tap0

  configfile=~/qemu/vfio-pci1.cfg

  vfiobind() {
  dev="$1"
  vendor=$(cat /sys/bus/pci/devices/$dev/vendor)
  device=$(cat /sys/bus/pci/devices/$dev/device)
  if [ -e /sys/bus/pci/devices/$dev/driver ]; then
  echo $dev > /sys/bus/pci/devices/$dev/driver/unbind
  fi
  echo $vendor $device > /sys/bus/pci/drivers/vfio-pci/new_id

  }

  modprobe vfio-pci

  cat $configfile | while read line;do
  echo $line | grep ^# >/dev/null 2>&1 && continue
  vfiobind $line
  done

  sudo qemu-system-x86_64 -machine type=q35,accel=kvm -cpu host,kvm=off \
  -smp 8,sockets=1,cores=4,threads=2 \
  -bios /usr/share/seabios/bios.bin \
  -serial none \
  -parallel none \
  -vga none \
  -m 4G \
  -mem-path /run/hugepages/kvm \
  -mem-prealloc \
  -balloon none \
  -rtc clock=host,base=localtime \
  -device 
ioh3420,bus=pcie.0,addr=1c.0,multifunction=on,port=1,chassis=1,id=root.1 \
  -device vfio-pci,host=01:00.0,bus=root.1,addr=00.0,multifunction=on,x-vga=on \
  -device virtio-scsi-pci,id=scsi \
  -drive 

[Qemu-devel] [Bug 1680679] Re: qemu cannot run twice

[Roger Lawhorn]

GTX880M - uefi firmware built in, 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/1680679

Title:
  qemu cannot run twice

Status in QEMU:
  Invalid

Bug description:
  After using qemu with gpu passthrough and then shutting down windows 7 
properly I cannot boot windows 7 a second time.
  Only a full reboot of linux fixes this issue.
  Qemu appears to corrupt something in linux when exiting.
  I get no error messages but windows 7 never finishes booting during the 2nd 
try.
  Apparently I do try to run vfiobind each time the script is run.
  Wondering if rerunning vfiobind can cause an issue?

  
  My specs:
  
-
  System:Host: GT70-2PE Kernel: 4.5.4-040504-generic x86_64 (64 bit gcc: 
5.3.1)
 Desktop: Cinnamon 3.2.7 (Gtk 3.18.9) Distro: Linux Mint 18.1 Serena
  Machine:   Mobo: Micro-Star model: MS-1763 v: REV:0.C Bios: American 
Megatrends v: E1763IMS.51B date: 01/29/2015
  CPU:   Quad core Intel Core i7-4810MQ (-HT-MCP-) cache: 6144 KB
 flags: (lm nx sse sse2 sse3 sse4_1 sse4_2 ssse3 vmx) bmips: 22347
 clock speeds: max: 2801 MHz 1: 2801 MHz 2: 2801 MHz 3: 2801 MHz 4: 
2801 MHz 5: 2801 MHz 6: 2801 MHz
 7: 2801 MHz 8: 2801 MHz
  Graphics:  Card-1: Intel 4th Gen Core Processor Integrated Graphics 
Controller bus-ID: 00:02.0
 Card-2: NVIDIA GK104M [GeForce GTX 880M] bus-ID: 01:00.0
 Display Server: X.Org 1.18.4 drivers: intel (unloaded: fbdev,vesa)
 Resolution: 1920x1080@60.02hz, 1920x1080@60.00hz
 GLX Renderer: Mesa DRI Intel Haswell Mobile GLX Version: 3.0 Mesa 
12.0.6 Direct Rendering: Yes

  
  My script:
  
---
  #!/bin/bash

  cd ~/qemu
  sudo ./up.sh tap0

  configfile=~/qemu/vfio-pci1.cfg

  vfiobind() {
  dev="$1"
  vendor=$(cat /sys/bus/pci/devices/$dev/vendor)
  device=$(cat /sys/bus/pci/devices/$dev/device)
  if [ -e /sys/bus/pci/devices/$dev/driver ]; then
  echo $dev > /sys/bus/pci/devices/$dev/driver/unbind
  fi
  echo $vendor $device > /sys/bus/pci/drivers/vfio-pci/new_id

  }

  modprobe vfio-pci

  cat $configfile | while read line;do
  echo $line | grep ^# >/dev/null 2>&1 && continue
  vfiobind $line
  done

  sudo qemu-system-x86_64 -machine type=q35,accel=kvm -cpu host,kvm=off \
  -smp 8,sockets=1,cores=4,threads=2 \
  -bios /usr/share/seabios/bios.bin \
  -serial none \
  -parallel none \
  -vga none \
  -m 4G \
  -mem-path /run/hugepages/kvm \
  -mem-prealloc \
  -balloon none \
  -rtc clock=host,base=localtime \
  -device 
ioh3420,bus=pcie.0,addr=1c.0,multifunction=on,port=1,chassis=1,id=root.1 \
  -device vfio-pci,host=01:00.0,bus=root.1,addr=00.0,multifunction=on,x-vga=on \
  -device virtio-scsi-pci,id=scsi \
  -drive 
id=disk0,if=virtio,cache=none,format=raw,file=/home/dad/qemu/windows7.img \
  -drive 
file=/home/dad/1TB-Backup/Iso/SP1ForWin7.iso,id=isocd,format=raw,if=none 
-device scsi-cd,drive=isocd \
  -net nic -net tap,ifname=tap0,script=no,downscript=no \
  -usbdevice host:413c:a503 \
  -usbdevice host:13fe:3100 \
  -usbdevice host:0bc2:ab21 \
  -boot menu=on \
  -boot order=c

  sudo ./down.sh tap0

  exit 0

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

[Qemu-devel] [Bug 1680679] Re: qemu cannot run twice

[Alex Williamson]

Perhaps you get Code 43 because mobile NVIDIA chips make use of Optimus
which requires significant proprietary firmware support.  QEMU/VFIO has
never claimed to work with such devices.  The further speculation in the
original report that QEMU corrupted something in Linux seems
unjustified, the device simply doesn't work a second time.  This might
be lack of proper reset support in the GPU itself or poor interaction
with aforementioned proprietary firmware.

** Changed in: qemu
   Status: New => Invalid

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

Title:
  qemu cannot run twice

Status in QEMU:
  Invalid

Bug description:
  After using qemu with gpu passthrough and then shutting down windows 7 
properly I cannot boot windows 7 a second time.
  Only a full reboot of linux fixes this issue.
  Qemu appears to corrupt something in linux when exiting.
  I get no error messages but windows 7 never finishes booting during the 2nd 
try.
  Apparently I do try to run vfiobind each time the script is run.
  Wondering if rerunning vfiobind can cause an issue?

  
  My specs:
  
-
  System:Host: GT70-2PE Kernel: 4.5.4-040504-generic x86_64 (64 bit gcc: 
5.3.1)
 Desktop: Cinnamon 3.2.7 (Gtk 3.18.9) Distro: Linux Mint 18.1 Serena
  Machine:   Mobo: Micro-Star model: MS-1763 v: REV:0.C Bios: American 
Megatrends v: E1763IMS.51B date: 01/29/2015
  CPU:   Quad core Intel Core i7-4810MQ (-HT-MCP-) cache: 6144 KB
 flags: (lm nx sse sse2 sse3 sse4_1 sse4_2 ssse3 vmx) bmips: 22347
 clock speeds: max: 2801 MHz 1: 2801 MHz 2: 2801 MHz 3: 2801 MHz 4: 
2801 MHz 5: 2801 MHz 6: 2801 MHz
 7: 2801 MHz 8: 2801 MHz
  Graphics:  Card-1: Intel 4th Gen Core Processor Integrated Graphics 
Controller bus-ID: 00:02.0
 Card-2: NVIDIA GK104M [GeForce GTX 880M] bus-ID: 01:00.0
 Display Server: X.Org 1.18.4 drivers: intel (unloaded: fbdev,vesa)
 Resolution: 1920x1080@60.02hz, 1920x1080@60.00hz
 GLX Renderer: Mesa DRI Intel Haswell Mobile GLX Version: 3.0 Mesa 
12.0.6 Direct Rendering: Yes

  
  My script:
  
---
  #!/bin/bash

  cd ~/qemu
  sudo ./up.sh tap0

  configfile=~/qemu/vfio-pci1.cfg

  vfiobind() {
  dev="$1"
  vendor=$(cat /sys/bus/pci/devices/$dev/vendor)
  device=$(cat /sys/bus/pci/devices/$dev/device)
  if [ -e /sys/bus/pci/devices/$dev/driver ]; then
  echo $dev > /sys/bus/pci/devices/$dev/driver/unbind
  fi
  echo $vendor $device > /sys/bus/pci/drivers/vfio-pci/new_id

  }

  modprobe vfio-pci

  cat $configfile | while read line;do
  echo $line | grep ^# >/dev/null 2>&1 && continue
  vfiobind $line
  done

  sudo qemu-system-x86_64 -machine type=q35,accel=kvm -cpu host,kvm=off \
  -smp 8,sockets=1,cores=4,threads=2 \
  -bios /usr/share/seabios/bios.bin \
  -serial none \
  -parallel none \
  -vga none \
  -m 4G \
  -mem-path /run/hugepages/kvm \
  -mem-prealloc \
  -balloon none \
  -rtc clock=host,base=localtime \
  -device 
ioh3420,bus=pcie.0,addr=1c.0,multifunction=on,port=1,chassis=1,id=root.1 \
  -device vfio-pci,host=01:00.0,bus=root.1,addr=00.0,multifunction=on,x-vga=on \
  -device virtio-scsi-pci,id=scsi \
  -drive 
id=disk0,if=virtio,cache=none,format=raw,file=/home/dad/qemu/windows7.img \
  -drive 
file=/home/dad/1TB-Backup/Iso/SP1ForWin7.iso,id=isocd,format=raw,if=none 
-device scsi-cd,drive=isocd \
  -net nic -net tap,ifname=tap0,script=no,downscript=no \
  -usbdevice host:413c:a503 \
  -usbdevice host:13fe:3100 \
  -usbdevice host:0bc2:ab21 \
  -boot menu=on \
  -boot order=c

  sudo ./down.sh tap0

  exit 0

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

[Qemu-devel] [Bug 1680679] Re: qemu cannot run twice

[Alex Williamson]

Comments #3 & #4 are not relevant to this bug and inaccurate
speculation.  There is no known incompatibility between NVIDIA drivers
and OVMF.  Many users, including myself, use this combination daily.
The issue is far more likely to be a VM configuration issue or lack of
UEFI support in the GPU ROM.

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

Title:
  qemu cannot run twice

Status in QEMU:
  New

Bug description:
  After using qemu with gpu passthrough and then shutting down windows 7 
properly I cannot boot windows 7 a second time.
  Only a full reboot of linux fixes this issue.
  Qemu appears to corrupt something in linux when exiting.
  I get no error messages but windows 7 never finishes booting during the 2nd 
try.
  Apparently I do try to run vfiobind each time the script is run.
  Wondering if rerunning vfiobind can cause an issue?

  
  My specs:
  
-
  System:Host: GT70-2PE Kernel: 4.5.4-040504-generic x86_64 (64 bit gcc: 
5.3.1)
 Desktop: Cinnamon 3.2.7 (Gtk 3.18.9) Distro: Linux Mint 18.1 Serena
  Machine:   Mobo: Micro-Star model: MS-1763 v: REV:0.C Bios: American 
Megatrends v: E1763IMS.51B date: 01/29/2015
  CPU:   Quad core Intel Core i7-4810MQ (-HT-MCP-) cache: 6144 KB
 flags: (lm nx sse sse2 sse3 sse4_1 sse4_2 ssse3 vmx) bmips: 22347
 clock speeds: max: 2801 MHz 1: 2801 MHz 2: 2801 MHz 3: 2801 MHz 4: 
2801 MHz 5: 2801 MHz 6: 2801 MHz
 7: 2801 MHz 8: 2801 MHz
  Graphics:  Card-1: Intel 4th Gen Core Processor Integrated Graphics 
Controller bus-ID: 00:02.0
 Card-2: NVIDIA GK104M [GeForce GTX 880M] bus-ID: 01:00.0
 Display Server: X.Org 1.18.4 drivers: intel (unloaded: fbdev,vesa)
 Resolution: 1920x1080@60.02hz, 1920x1080@60.00hz
 GLX Renderer: Mesa DRI Intel Haswell Mobile GLX Version: 3.0 Mesa 
12.0.6 Direct Rendering: Yes

  
  My script:
  
---
  #!/bin/bash

  cd ~/qemu
  sudo ./up.sh tap0

  configfile=~/qemu/vfio-pci1.cfg

  vfiobind() {
  dev="$1"
  vendor=$(cat /sys/bus/pci/devices/$dev/vendor)
  device=$(cat /sys/bus/pci/devices/$dev/device)
  if [ -e /sys/bus/pci/devices/$dev/driver ]; then
  echo $dev > /sys/bus/pci/devices/$dev/driver/unbind
  fi
  echo $vendor $device > /sys/bus/pci/drivers/vfio-pci/new_id

  }

  modprobe vfio-pci

  cat $configfile | while read line;do
  echo $line | grep ^# >/dev/null 2>&1 && continue
  vfiobind $line
  done

  sudo qemu-system-x86_64 -machine type=q35,accel=kvm -cpu host,kvm=off \
  -smp 8,sockets=1,cores=4,threads=2 \
  -bios /usr/share/seabios/bios.bin \
  -serial none \
  -parallel none \
  -vga none \
  -m 4G \
  -mem-path /run/hugepages/kvm \
  -mem-prealloc \
  -balloon none \
  -rtc clock=host,base=localtime \
  -device 
ioh3420,bus=pcie.0,addr=1c.0,multifunction=on,port=1,chassis=1,id=root.1 \
  -device vfio-pci,host=01:00.0,bus=root.1,addr=00.0,multifunction=on,x-vga=on \
  -device virtio-scsi-pci,id=scsi \
  -drive 
id=disk0,if=virtio,cache=none,format=raw,file=/home/dad/qemu/windows7.img \
  -drive 
file=/home/dad/1TB-Backup/Iso/SP1ForWin7.iso,id=isocd,format=raw,if=none 
-device scsi-cd,drive=isocd \
  -net nic -net tap,ifname=tap0,script=no,downscript=no \
  -usbdevice host:413c:a503 \
  -usbdevice host:13fe:3100 \
  -usbdevice host:0bc2:ab21 \
  -boot menu=on \
  -boot order=c

  sudo ./down.sh tap0

  exit 0

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

[Qemu-devel] [PATCH v2 for-2.10 7/8] block/rbd - update variable names to more apt names

[Jeff Cody]

Update 'clientname' to be 'user', which tracks better with both
the QAPI and rados variable naming.

Update 'name' to be 'image_name', as it indicates the rbd image.
Naming it 'image' would have been ideal, but we are using that for
the rados_image_t value returned by rbd_open().

Reviewed-by: Stefan Hajnoczi 
Signed-off-by: Jeff Cody 
---
 block/rbd.c | 33 +
 1 file changed, 17 insertions(+), 16 deletions(-)

diff --git a/block/rbd.c b/block/rbd.c
index 1c43171..35853c9 100644
--- a/block/rbd.c
+++ b/block/rbd.c
@@ -94,7 +94,7 @@ typedef struct BDRVRBDState {
 rados_t cluster;
 rados_ioctx_t io_ctx;
 rbd_image_t image;
-char *name;
+char *image_name;
 char *snap;
 } BDRVRBDState;
 
@@ -350,7 +350,7 @@ static int qemu_rbd_create(const char *filename, QemuOpts 
*opts, Error **errp)
 int64_t bytes = 0;
 int64_t objsize;
 int obj_order = 0;
-const char *pool, *name, *conf, *clientname, *keypairs;
+const char *pool, *image_name, *conf, *user, *keypairs;
 const char *secretid;
 rados_t cluster;
 rados_ioctx_t io_ctx;
@@ -393,11 +393,11 @@ static int qemu_rbd_create(const char *filename, QemuOpts 
*opts, Error **errp)
  */
 pool   = qdict_get_try_str(options, "pool");
 conf   = qdict_get_try_str(options, "conf");
-clientname = qdict_get_try_str(options, "user");
-name   = qdict_get_try_str(options, "image");
+user   = qdict_get_try_str(options, "user");
+image_name = qdict_get_try_str(options, "image");
 keypairs   = qdict_get_try_str(options, "=keyvalue-pairs");
 
-ret = rados_create(, clientname);
+ret = rados_create(, user);
 if (ret < 0) {
 error_setg_errno(errp, -ret, "error initializing");
 goto exit;
@@ -434,7 +434,7 @@ static int qemu_rbd_create(const char *filename, QemuOpts 
*opts, Error **errp)
 goto shutdown;
 }
 
-ret = rbd_create(io_ctx, name, bytes, _order);
+ret = rbd_create(io_ctx, image_name, bytes, _order);
 if (ret < 0) {
 error_setg_errno(errp, -ret, "error rbd create");
 }
@@ -540,7 +540,7 @@ static int qemu_rbd_open(BlockDriverState *bs, QDict 
*options, int flags,
  Error **errp)
 {
 BDRVRBDState *s = bs->opaque;
-const char *pool, *snap, *conf, *clientname, *name, *keypairs;
+const char *pool, *snap, *conf, *user, *image_name, *keypairs;
 const char *secretid;
 QemuOpts *opts;
 Error *local_err = NULL;
@@ -567,24 +567,24 @@ static int qemu_rbd_open(BlockDriverState *bs, QDict 
*options, int flags,
 pool   = qemu_opt_get(opts, "pool");
 conf   = qemu_opt_get(opts, "conf");
 snap   = qemu_opt_get(opts, "snapshot");
-clientname = qemu_opt_get(opts, "user");
-name   = qemu_opt_get(opts, "image");
+user   = qemu_opt_get(opts, "user");
+image_name = qemu_opt_get(opts, "image");
 keypairs   = qemu_opt_get(opts, "=keyvalue-pairs");
 
-if (!pool || !name) {
+if (!pool || !image_name) {
 error_setg(errp, "Parameters 'pool' and 'image' are required");
 r = -EINVAL;
 goto failed_opts;
 }
 
-r = rados_create(>cluster, clientname);
+r = rados_create(>cluster, user);
 if (r < 0) {
 error_setg_errno(errp, -r, "error initializing");
 goto failed_opts;
 }
 
 s->snap = g_strdup(snap);
-s->name = g_strdup(name);
+s->image_name = g_strdup(image_name);
 
 /* try default location when conf=NULL, but ignore failure */
 r = rados_conf_read_file(s->cluster, conf);
@@ -636,9 +636,10 @@ static int qemu_rbd_open(BlockDriverState *bs, QDict 
*options, int flags,
 }
 
 /* rbd_open is always r/w */
-r = rbd_open(s->io_ctx, s->name, >image, s->snap);
+r = rbd_open(s->io_ctx, s->image_name, >image, s->snap);
 if (r < 0) {
-error_setg_errno(errp, -r, "error reading header from %s", s->name);
+error_setg_errno(errp, -r, "error reading header from %s",
+ s->image_name);
 goto failed_open;
 }
 
@@ -660,7 +661,7 @@ failed_open:
 failed_shutdown:
 rados_shutdown(s->cluster);
 g_free(s->snap);
-g_free(s->name);
+g_free(s->image_name);
 failed_opts:
 qemu_opts_del(opts);
 g_free(mon_host);
@@ -674,7 +675,7 @@ static void qemu_rbd_close(BlockDriverState *bs)
 rbd_close(s->image);
 rados_ioctx_destroy(s->io_ctx);
 g_free(s->snap);
-g_free(s->name);
+g_free(s->image_name);
 rados_shutdown(s->cluster);
 }
 
-- 
2.9.3

[Qemu-devel] [PATCH v2 for-2.10 5/8] block: introduce bdrv_can_set_read_only()

[Jeff Cody]

Introduce check function for setting read_only flags.  Will return < 0 on
error, with appropriate Error value set.  Does not alter any flags.

Signed-off-by: Jeff Cody 
---
 block.c   | 14 +-
 include/block/block.h |  1 +
 2 files changed, 14 insertions(+), 1 deletion(-)

diff --git a/block.c b/block.c
index a05ad48..1514ae9 100644
--- a/block.c
+++ b/block.c
@@ -197,7 +197,7 @@ bool bdrv_is_read_only(BlockDriverState *bs)
 return bs->read_only;
 }
 
-int bdrv_set_read_only(BlockDriverState *bs, bool read_only, Error **errp)
+int bdrv_can_set_read_only(BlockDriverState *bs, bool read_only, Error **errp)
 {
 /* Do not set read_only if copy_on_read is enabled */
 if (bs->copy_on_read && read_only) {
@@ -213,6 +213,18 @@ int bdrv_set_read_only(BlockDriverState *bs, bool 
read_only, Error **errp)
 return -EPERM;
 }
 
+return 0;
+}
+
+int bdrv_set_read_only(BlockDriverState *bs, bool read_only, Error **errp)
+{
+int ret = 0;
+
+ret = bdrv_can_set_read_only(bs, read_only, errp);
+if (ret < 0) {
+return ret;
+}
+
 bs->read_only = read_only;
 return 0;
 }
diff --git a/include/block/block.h b/include/block/block.h
index beb563a..dfd8694 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -426,6 +426,7 @@ int bdrv_is_allocated_above(BlockDriverState *top, 
BlockDriverState *base,
 int64_t sector_num, int nb_sectors, int *pnum);
 
 bool bdrv_is_read_only(BlockDriverState *bs);
+int bdrv_can_set_read_only(BlockDriverState *bs, bool read_only, Error **errp);
 int bdrv_set_read_only(BlockDriverState *bs, bool read_only, Error **errp);
 bool bdrv_is_sg(BlockDriverState *bs);
 bool bdrv_is_inserted(BlockDriverState *bs);
-- 
2.9.3

[Qemu-devel] [PATCH v2 for-2.10 8/8] block/rbd: Add support for reopen()

[Jeff Cody]

This adds support for reopen in rbd, for changing between r/w and r/o.

Note, that this is only a flag change, but we will block a change from
r/o to r/w if we are using an RBD internal snapshot.

Reviewed-by: Stefan Hajnoczi 
Signed-off-by: Jeff Cody 
---
 block/rbd.c | 21 +
 1 file changed, 21 insertions(+)

diff --git a/block/rbd.c b/block/rbd.c
index 35853c9..6471f4f 100644
--- a/block/rbd.c
+++ b/block/rbd.c
@@ -668,6 +668,26 @@ failed_opts:
 return r;
 }
 
+
+/* Since RBD is currently always opened R/W via the API,
+ * we just need to check if we are using a snapshot or not, in
+ * order to determine if we will allow it to be R/W */
+static int qemu_rbd_reopen_prepare(BDRVReopenState *state,
+   BlockReopenQueue *queue, Error **errp)
+{
+BDRVRBDState *s = state->bs->opaque;
+int ret = 0;
+
+if (s->snap && state->flags & BDRV_O_RDWR) {
+error_setg(errp,
+   "Cannot change node '%s' to r/w when using RBD snapshot",
+   bdrv_get_device_or_node_name(state->bs));
+ret = -EINVAL;
+}
+
+return ret;
+}
+
 static void qemu_rbd_close(BlockDriverState *bs)
 {
 BDRVRBDState *s = bs->opaque;
@@ -1074,6 +1094,7 @@ static BlockDriver bdrv_rbd = {
 .bdrv_parse_filename= qemu_rbd_parse_filename,
 .bdrv_file_open = qemu_rbd_open,
 .bdrv_close = qemu_rbd_close,
+.bdrv_reopen_prepare= qemu_rbd_reopen_prepare,
 .bdrv_create= qemu_rbd_create,
 .bdrv_has_zero_init = bdrv_has_zero_init_1,
 .bdrv_get_info  = qemu_rbd_getinfo,
-- 
2.9.3

[Qemu-devel] [PATCH v2 for-2.10 2/8] block: do not set BDS read_only if copy_on_read enabled

[Jeff Cody]

A few block drivers will set the BDS read_only flag from their
.bdrv_open() function.  This means the bs->read_only flag could
be set after we enable copy_on_read, as the BDRV_O_COPY_ON_READ
flag check occurs prior to the call to bdrv->bdrv_open().

This adds an error return to bdrv_set_read_only(), and an error will be
return if we try to set the BDS to read_only while copy_on_read is
enabled.

This patch also changes the behavior of vvfat.  Before, vvfat could
override the drive 'readonly' flag with its own, internal 'rw' flag.

For instance, this -drive parameter would result in a writable image:

"-drive format=vvfat,dir=/tmp/vvfat,rw,if=virtio,readonly=on"

This is not correct.  Now, attempting to use the above -drive parameter
will result in an error (i.e., 'rw' is incompatible with 'readonly=on').

Signed-off-by: Jeff Cody 
---
 block.c   | 10 +-
 block/bochs.c |  5 -
 block/cloop.c |  5 -
 block/dmg.c   |  6 +-
 block/rbd.c   | 11 ++-
 block/vvfat.c | 19 +++
 include/block/block.h |  2 +-
 7 files changed, 48 insertions(+), 10 deletions(-)

diff --git a/block.c b/block.c
index 7b4c7ef..c9eb613 100644
--- a/block.c
+++ b/block.c
@@ -192,9 +192,17 @@ void path_combine(char *dest, int dest_size,
 }
 }
 
-void bdrv_set_read_only(BlockDriverState *bs, bool read_only)
+int bdrv_set_read_only(BlockDriverState *bs, bool read_only, Error **errp)
 {
+/* Do not set read_only if copy_on_read is enabled */
+if (bs->copy_on_read && read_only) {
+error_setg(errp, "Can't set node '%s' to r/o with copy-on-read 
enabled",
+   bdrv_get_device_or_node_name(bs));
+return -EINVAL;
+}
+
 bs->read_only = read_only;
+return 0;
 }
 
 void bdrv_get_full_backing_filename_from_filename(const char *backed,
diff --git a/block/bochs.c b/block/bochs.c
index bdc2831..a759b6e 100644
--- a/block/bochs.c
+++ b/block/bochs.c
@@ -110,7 +110,10 @@ static int bochs_open(BlockDriverState *bs, QDict 
*options, int flags,
 return -EINVAL;
 }
 
-bdrv_set_read_only(bs, true); /* no write support yet */
+ret = bdrv_set_read_only(bs, true, errp); /* no write support yet */
+if (ret < 0) {
+return ret;
+}
 
 ret = bdrv_pread(bs->file, 0, , sizeof(bochs));
 if (ret < 0) {
diff --git a/block/cloop.c b/block/cloop.c
index 11f17c8..d6597fc 100644
--- a/block/cloop.c
+++ b/block/cloop.c
@@ -72,7 +72,10 @@ static int cloop_open(BlockDriverState *bs, QDict *options, 
int flags,
 return -EINVAL;
 }
 
-bdrv_set_read_only(bs, true);
+ret = bdrv_set_read_only(bs, true, errp);
+if (ret < 0) {
+return ret;
+}
 
 /* read header */
 ret = bdrv_pread(bs->file, 128, >block_size, 4);
diff --git a/block/dmg.c b/block/dmg.c
index 27ce4a6..900ae5a 100644
--- a/block/dmg.c
+++ b/block/dmg.c
@@ -419,8 +419,12 @@ static int dmg_open(BlockDriverState *bs, QDict *options, 
int flags,
 return -EINVAL;
 }
 
+ret = bdrv_set_read_only(bs, true, errp);
+if (ret < 0) {
+return ret;
+}
+
 block_module_load_one("dmg-bz2");
-bdrv_set_read_only(bs, true);
 
 s->n_chunks = 0;
 s->offsets = s->lengths = s->sectors = s->sectorcounts = NULL;
diff --git a/block/rbd.c b/block/rbd.c
index 6ad2904..1c43171 100644
--- a/block/rbd.c
+++ b/block/rbd.c
@@ -635,13 +635,22 @@ static int qemu_rbd_open(BlockDriverState *bs, QDict 
*options, int flags,
 goto failed_shutdown;
 }
 
+/* rbd_open is always r/w */
 r = rbd_open(s->io_ctx, s->name, >image, s->snap);
 if (r < 0) {
 error_setg_errno(errp, -r, "error reading header from %s", s->name);
 goto failed_open;
 }
 
-bdrv_set_read_only(bs, (s->snap != NULL));
+/* If we are using an rbd snapshot, we must be r/o, otherwise
+ * leave as-is */
+if (s->snap != NULL) {
+r = bdrv_set_read_only(bs, true, _err);
+if (r < 0) {
+error_propagate(errp, local_err);
+goto failed_open;
+}
+}
 
 qemu_opts_del(opts);
 return 0;
diff --git a/block/vvfat.c b/block/vvfat.c
index d4ce6d7..b509d55 100644
--- a/block/vvfat.c
+++ b/block/vvfat.c
@@ -1156,8 +1156,6 @@ static int vvfat_open(BlockDriverState *bs, QDict 
*options, int flags,
 
 s->current_cluster=0x;
 
-/* read only is the default for safety */
-bdrv_set_read_only(bs, true);
 s->qcow = NULL;
 s->qcow_filename = NULL;
 s->fat2 = NULL;
@@ -1169,11 +1167,24 @@ static int vvfat_open(BlockDriverState *bs, QDict 
*options, int flags,
 s->sector_count = cyls * heads * secs - (s->first_sectors_number - 1);
 
 if (qemu_opt_get_bool(opts, "rw", false)) {
-ret = enable_write_target(bs, errp);
+if (!bdrv_is_read_only(bs)) {
+ret = enable_write_target(bs, errp);
+if (ret < 0) {
+goto fail;
+}
+ 

[Qemu-devel] [PATCH v2 for-2.10 4/8] block: code movement

[Jeff Cody]

Move bdrv_is_read_only() up with its friends.

Reviewed-by: Stefan Hajnoczi 
Reviewed-by: John Snow 
Signed-off-by: Jeff Cody 
---
 block.c | 10 +-
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/block.c b/block.c
index 68a18b0..a05ad48 100644
--- a/block.c
+++ b/block.c
@@ -192,6 +192,11 @@ void path_combine(char *dest, int dest_size,
 }
 }
 
+bool bdrv_is_read_only(BlockDriverState *bs)
+{
+return bs->read_only;
+}
+
 int bdrv_set_read_only(BlockDriverState *bs, bool read_only, Error **errp)
 {
 /* Do not set read_only if copy_on_read is enabled */
@@ -3367,11 +3372,6 @@ void bdrv_get_geometry(BlockDriverState *bs, uint64_t 
*nb_sectors_ptr)
 *nb_sectors_ptr = nb_sectors < 0 ? 0 : nb_sectors;
 }
 
-bool bdrv_is_read_only(BlockDriverState *bs)
-{
-return bs->read_only;
-}
-
 bool bdrv_is_sg(BlockDriverState *bs)
 {
 return bs->sg;
-- 
2.9.3

[Qemu-devel] [PATCH v2 for-2.10 6/8] block: use bdrv_can_set_read_only() during reopen

[Jeff Cody]

Signed-off-by: Jeff Cody 
---
 block.c | 14 --
 1 file changed, 8 insertions(+), 6 deletions(-)

diff --git a/block.c b/block.c
index 1514ae9..5d560f5 100644
--- a/block.c
+++ b/block.c
@@ -2785,6 +2785,7 @@ int bdrv_reopen_prepare(BDRVReopenState *reopen_state, 
BlockReopenQueue *queue,
 BlockDriver *drv;
 QemuOpts *opts;
 const char *value;
+bool read_only;
 
 assert(reopen_state != NULL);
 assert(reopen_state->bs->drv != NULL);
@@ -2813,12 +2814,13 @@ int bdrv_reopen_prepare(BDRVReopenState *reopen_state, 
BlockReopenQueue *queue,
 qdict_put(reopen_state->options, "driver", qstring_from_str(value));
 }
 
-/* if we are to stay read-only, do not allow permission change
- * to r/w */
-if (!(reopen_state->bs->open_flags & BDRV_O_ALLOW_RDWR) &&
-reopen_state->flags & BDRV_O_RDWR) {
-error_setg(errp, "Node '%s' is read only",
-   bdrv_get_device_or_node_name(reopen_state->bs));
+/* If we are to stay read-only, do not allow permission change
+ * to r/w. Attempting to set to r/w may fail if either BDRV_O_ALLOW_RDWR is
+ * not set, or if the BDS still has copy_on_read enabled */
+read_only = !(reopen_state->flags & BDRV_O_RDWR);
+ret = bdrv_can_set_read_only(reopen_state->bs, read_only, _err);
+if (local_err) {
+error_propagate(errp, local_err);
 goto error;
 }
 
-- 
2.9.3

[Qemu-devel] [PATCH v2 for-2.10 3/8] block: honor BDRV_O_ALLOW_RDWR when clearing bs->read_only

[Jeff Cody]

The BDRV_O_ALLOW_RDWR flag allows / prohibits the changing of
the BDS 'read_only' state, but there are a few places where it
is ignored.  In the bdrv_set_read_only() helper, make sure to
honor the flag.

Signed-off-by: Jeff Cody 
---
 block.c | 7 +++
 1 file changed, 7 insertions(+)

diff --git a/block.c b/block.c
index c9eb613..68a18b0 100644
--- a/block.c
+++ b/block.c
@@ -201,6 +201,13 @@ int bdrv_set_read_only(BlockDriverState *bs, bool 
read_only, Error **errp)
 return -EINVAL;
 }
 
+/* Do not clear read_only if it is prohibited */
+if (!read_only && !(bs->open_flags & BDRV_O_ALLOW_RDWR)) {
+error_setg(errp, "Node '%s' is read only",
+   bdrv_get_device_or_node_name(bs));
+return -EPERM;
+}
+
 bs->read_only = read_only;
 return 0;
 }
-- 
2.9.3

[Qemu-devel] [PATCH v2 for-2.10 1/8] block: add bdrv_set_read_only() helper function

[Jeff Cody]

We have a helper wrapper for checking for the BDS read_only flag,
add a helper wrapper to set the read_only flag as well.

Reviewed-by: Stefan Hajnoczi 
Signed-off-by: Jeff Cody 
---
 block.c   | 5 +
 block/bochs.c | 2 +-
 block/cloop.c | 2 +-
 block/dmg.c   | 2 +-
 block/rbd.c   | 2 +-
 block/vvfat.c | 4 ++--
 include/block/block.h | 1 +
 7 files changed, 12 insertions(+), 6 deletions(-)

diff --git a/block.c b/block.c
index 927ba89..7b4c7ef 100644
--- a/block.c
+++ b/block.c
@@ -192,6 +192,11 @@ void path_combine(char *dest, int dest_size,
 }
 }
 
+void bdrv_set_read_only(BlockDriverState *bs, bool read_only)
+{
+bs->read_only = read_only;
+}
+
 void bdrv_get_full_backing_filename_from_filename(const char *backed,
   const char *backing,
   char *dest, size_t sz,
diff --git a/block/bochs.c b/block/bochs.c
index 516da56..bdc2831 100644
--- a/block/bochs.c
+++ b/block/bochs.c
@@ -110,7 +110,7 @@ static int bochs_open(BlockDriverState *bs, QDict *options, 
int flags,
 return -EINVAL;
 }
 
-bs->read_only = true; /* no write support yet */
+bdrv_set_read_only(bs, true); /* no write support yet */
 
 ret = bdrv_pread(bs->file, 0, , sizeof(bochs));
 if (ret < 0) {
diff --git a/block/cloop.c b/block/cloop.c
index a6c7b9d..11f17c8 100644
--- a/block/cloop.c
+++ b/block/cloop.c
@@ -72,7 +72,7 @@ static int cloop_open(BlockDriverState *bs, QDict *options, 
int flags,
 return -EINVAL;
 }
 
-bs->read_only = true;
+bdrv_set_read_only(bs, true);
 
 /* read header */
 ret = bdrv_pread(bs->file, 128, >block_size, 4);
diff --git a/block/dmg.c b/block/dmg.c
index a7d25fc..27ce4a6 100644
--- a/block/dmg.c
+++ b/block/dmg.c
@@ -420,7 +420,7 @@ static int dmg_open(BlockDriverState *bs, QDict *options, 
int flags,
 }
 
 block_module_load_one("dmg-bz2");
-bs->read_only = true;
+bdrv_set_read_only(bs, true);
 
 s->n_chunks = 0;
 s->offsets = s->lengths = s->sectors = s->sectorcounts = NULL;
diff --git a/block/rbd.c b/block/rbd.c
index 1ceeeb5..6ad2904 100644
--- a/block/rbd.c
+++ b/block/rbd.c
@@ -641,7 +641,7 @@ static int qemu_rbd_open(BlockDriverState *bs, QDict 
*options, int flags,
 goto failed_open;
 }
 
-bs->read_only = (s->snap != NULL);
+bdrv_set_read_only(bs, (s->snap != NULL));
 
 qemu_opts_del(opts);
 return 0;
diff --git a/block/vvfat.c b/block/vvfat.c
index af5153d..d4ce6d7 100644
--- a/block/vvfat.c
+++ b/block/vvfat.c
@@ -1157,7 +1157,7 @@ static int vvfat_open(BlockDriverState *bs, QDict 
*options, int flags,
 s->current_cluster=0x;
 
 /* read only is the default for safety */
-bs->read_only = true;
+bdrv_set_read_only(bs, true);
 s->qcow = NULL;
 s->qcow_filename = NULL;
 s->fat2 = NULL;
@@ -1173,7 +1173,7 @@ static int vvfat_open(BlockDriverState *bs, QDict 
*options, int flags,
 if (ret < 0) {
 goto fail;
 }
-bs->read_only = false;
+bdrv_set_read_only(bs, false);
 }
 
 bs->total_sectors = cyls * heads * secs;
diff --git a/include/block/block.h b/include/block/block.h
index 5149260..06c9032 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -426,6 +426,7 @@ int bdrv_is_allocated_above(BlockDriverState *top, 
BlockDriverState *base,
 int64_t sector_num, int nb_sectors, int *pnum);
 
 bool bdrv_is_read_only(BlockDriverState *bs);
+void bdrv_set_read_only(BlockDriverState *bs, bool read_only);
 bool bdrv_is_sg(BlockDriverState *bs);
 bool bdrv_is_inserted(BlockDriverState *bs);
 int bdrv_media_changed(BlockDriverState *bs);
-- 
2.9.3

[Qemu-devel] [PATCH v2 for-2.10 0/8] RBD reopen, read_only cleanup

[Jeff Cody]

Changes from v1:

Patch 2: Has v1 patch 8 (do not blindly xset bs->read_only) squashed into it
 (thanks Stefan)
 COW -> "copy-on-read" (Thanks John)
 Drop unneeded call in vvfat, and bypass enable_write_target (Stefan)

Patch 5: Rename bdrv_try_... to bdrv_can_set_read_only() (Thanks John, Stefan)

Patch 6: Use "reopen_state->flags" not "reopen_state->bs->open_flags"
 (Thanks John)



This series does two things:

1.) Cleans up some of the logic behind setting the read_only flag
for a BDS in the block layer, so that it is done consistently
(and rules are applied consistently), and

2.) Adds .bdrv_reopen_prepare() implementation for RBD, so that block
jobs can be run on backing chains that have rbd protocol nodes.

Jeff Cody (8):
  block: add bdrv_set_read_only() helper function
  block: do not set BDS read_only if copy_on_read enabled
  block: honor BDRV_O_ALLOW_RDWR when clearing bs->read_only
  block: code movement
  block: introduce bdrv_can_set_read_only()
  block: use bdrv_can_set_read_only() during reopen
  block/rbd - update variable names to more apt names
  block/rbd: Add support for reopen()

 block.c   | 56 +++-
 block/bochs.c |  5 +++-
 block/cloop.c |  5 +++-
 block/dmg.c   |  6 -
 block/rbd.c   | 65 +--
 block/vvfat.c | 19 +++
 include/block/block.h |  2 ++
 7 files changed, 123 insertions(+), 35 deletions(-)

-- 
2.9.3

[Qemu-devel] [Bug 1680679] Re: qemu cannot run twice

[Roger Lawhorn]

NOTE: I only get gpu passthrough to work when using seabios. UEFI will
not work with modern nvidia drivers. You get the 'code 43' error because
whatever standards nvidia expects when talking to uefi are not met by
the OVMF firmware used by qemu. This issue happens to windows users
also. They had to update their uefi firmware to get around the code 43
issue.

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

Title:
  qemu cannot run twice

Status in QEMU:
  New

Bug description:
  After using qemu with gpu passthrough and then shutting down windows 7 
properly I cannot boot windows 7 a second time.
  Only a full reboot of linux fixes this issue.
  Qemu appears to corrupt something in linux when exiting.
  I get no error messages but windows 7 never finishes booting during the 2nd 
try.
  Apparently I do try to run vfiobind each time the script is run.
  Wondering if rerunning vfiobind can cause an issue?

  
  My specs:
  
-
  System:Host: GT70-2PE Kernel: 4.5.4-040504-generic x86_64 (64 bit gcc: 
5.3.1)
 Desktop: Cinnamon 3.2.7 (Gtk 3.18.9) Distro: Linux Mint 18.1 Serena
  Machine:   Mobo: Micro-Star model: MS-1763 v: REV:0.C Bios: American 
Megatrends v: E1763IMS.51B date: 01/29/2015
  CPU:   Quad core Intel Core i7-4810MQ (-HT-MCP-) cache: 6144 KB
 flags: (lm nx sse sse2 sse3 sse4_1 sse4_2 ssse3 vmx) bmips: 22347
 clock speeds: max: 2801 MHz 1: 2801 MHz 2: 2801 MHz 3: 2801 MHz 4: 
2801 MHz 5: 2801 MHz 6: 2801 MHz
 7: 2801 MHz 8: 2801 MHz
  Graphics:  Card-1: Intel 4th Gen Core Processor Integrated Graphics 
Controller bus-ID: 00:02.0
 Card-2: NVIDIA GK104M [GeForce GTX 880M] bus-ID: 01:00.0
 Display Server: X.Org 1.18.4 drivers: intel (unloaded: fbdev,vesa)
 Resolution: 1920x1080@60.02hz, 1920x1080@60.00hz
 GLX Renderer: Mesa DRI Intel Haswell Mobile GLX Version: 3.0 Mesa 
12.0.6 Direct Rendering: Yes

  
  My script:
  
---
  #!/bin/bash

  cd ~/qemu
  sudo ./up.sh tap0

  configfile=~/qemu/vfio-pci1.cfg

  vfiobind() {
  dev="$1"
  vendor=$(cat /sys/bus/pci/devices/$dev/vendor)
  device=$(cat /sys/bus/pci/devices/$dev/device)
  if [ -e /sys/bus/pci/devices/$dev/driver ]; then
  echo $dev > /sys/bus/pci/devices/$dev/driver/unbind
  fi
  echo $vendor $device > /sys/bus/pci/drivers/vfio-pci/new_id

  }

  modprobe vfio-pci

  cat $configfile | while read line;do
  echo $line | grep ^# >/dev/null 2>&1 && continue
  vfiobind $line
  done

  sudo qemu-system-x86_64 -machine type=q35,accel=kvm -cpu host,kvm=off \
  -smp 8,sockets=1,cores=4,threads=2 \
  -bios /usr/share/seabios/bios.bin \
  -serial none \
  -parallel none \
  -vga none \
  -m 4G \
  -mem-path /run/hugepages/kvm \
  -mem-prealloc \
  -balloon none \
  -rtc clock=host,base=localtime \
  -device 
ioh3420,bus=pcie.0,addr=1c.0,multifunction=on,port=1,chassis=1,id=root.1 \
  -device vfio-pci,host=01:00.0,bus=root.1,addr=00.0,multifunction=on,x-vga=on \
  -device virtio-scsi-pci,id=scsi \
  -drive 
id=disk0,if=virtio,cache=none,format=raw,file=/home/dad/qemu/windows7.img \
  -drive 
file=/home/dad/1TB-Backup/Iso/SP1ForWin7.iso,id=isocd,format=raw,if=none 
-device scsi-cd,drive=isocd \
  -net nic -net tap,ifname=tap0,script=no,downscript=no \
  -usbdevice host:413c:a503 \
  -usbdevice host:13fe:3100 \
  -usbdevice host:0bc2:ab21 \
  -boot menu=on \
  -boot order=c

  sudo ./down.sh tap0

  exit 0

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

[Qemu-devel] [Bug 1680679] Re: qemu cannot run twice

[Roger Lawhorn]

NOTE: Article showing windows users updating their motherboard uefi firmware to 
get around code 43:
https://devtalk.nvidia.com/default/topic/861244/cuda-setup-and-installation/geforce-740m-asus-x550l-code-43-after-windows-10-update/3

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

Title:
  qemu cannot run twice

Status in QEMU:
  New

Bug description:
  After using qemu with gpu passthrough and then shutting down windows 7 
properly I cannot boot windows 7 a second time.
  Only a full reboot of linux fixes this issue.
  Qemu appears to corrupt something in linux when exiting.
  I get no error messages but windows 7 never finishes booting during the 2nd 
try.
  Apparently I do try to run vfiobind each time the script is run.
  Wondering if rerunning vfiobind can cause an issue?

  
  My specs:
  
-
  System:Host: GT70-2PE Kernel: 4.5.4-040504-generic x86_64 (64 bit gcc: 
5.3.1)
 Desktop: Cinnamon 3.2.7 (Gtk 3.18.9) Distro: Linux Mint 18.1 Serena
  Machine:   Mobo: Micro-Star model: MS-1763 v: REV:0.C Bios: American 
Megatrends v: E1763IMS.51B date: 01/29/2015
  CPU:   Quad core Intel Core i7-4810MQ (-HT-MCP-) cache: 6144 KB
 flags: (lm nx sse sse2 sse3 sse4_1 sse4_2 ssse3 vmx) bmips: 22347
 clock speeds: max: 2801 MHz 1: 2801 MHz 2: 2801 MHz 3: 2801 MHz 4: 
2801 MHz 5: 2801 MHz 6: 2801 MHz
 7: 2801 MHz 8: 2801 MHz
  Graphics:  Card-1: Intel 4th Gen Core Processor Integrated Graphics 
Controller bus-ID: 00:02.0
 Card-2: NVIDIA GK104M [GeForce GTX 880M] bus-ID: 01:00.0
 Display Server: X.Org 1.18.4 drivers: intel (unloaded: fbdev,vesa)
 Resolution: 1920x1080@60.02hz, 1920x1080@60.00hz
 GLX Renderer: Mesa DRI Intel Haswell Mobile GLX Version: 3.0 Mesa 
12.0.6 Direct Rendering: Yes

  
  My script:
  
---
  #!/bin/bash

  cd ~/qemu
  sudo ./up.sh tap0

  configfile=~/qemu/vfio-pci1.cfg

  vfiobind() {
  dev="$1"
  vendor=$(cat /sys/bus/pci/devices/$dev/vendor)
  device=$(cat /sys/bus/pci/devices/$dev/device)
  if [ -e /sys/bus/pci/devices/$dev/driver ]; then
  echo $dev > /sys/bus/pci/devices/$dev/driver/unbind
  fi
  echo $vendor $device > /sys/bus/pci/drivers/vfio-pci/new_id

  }

  modprobe vfio-pci

  cat $configfile | while read line;do
  echo $line | grep ^# >/dev/null 2>&1 && continue
  vfiobind $line
  done

  sudo qemu-system-x86_64 -machine type=q35,accel=kvm -cpu host,kvm=off \
  -smp 8,sockets=1,cores=4,threads=2 \
  -bios /usr/share/seabios/bios.bin \
  -serial none \
  -parallel none \
  -vga none \
  -m 4G \
  -mem-path /run/hugepages/kvm \
  -mem-prealloc \
  -balloon none \
  -rtc clock=host,base=localtime \
  -device 
ioh3420,bus=pcie.0,addr=1c.0,multifunction=on,port=1,chassis=1,id=root.1 \
  -device vfio-pci,host=01:00.0,bus=root.1,addr=00.0,multifunction=on,x-vga=on \
  -device virtio-scsi-pci,id=scsi \
  -drive 
id=disk0,if=virtio,cache=none,format=raw,file=/home/dad/qemu/windows7.img \
  -drive 
file=/home/dad/1TB-Backup/Iso/SP1ForWin7.iso,id=isocd,format=raw,if=none 
-device scsi-cd,drive=isocd \
  -net nic -net tap,ifname=tap0,script=no,downscript=no \
  -usbdevice host:413c:a503 \
  -usbdevice host:13fe:3100 \
  -usbdevice host:0bc2:ab21 \
  -boot menu=on \
  -boot order=c

  sudo ./down.sh tap0

  exit 0

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

Re: [Qemu-devel] host stalls when qemu-system-aarch64 with kvm and pflash

[Wei Huang]

On 03/30/2017 05:51 AM, Marc Zyngier wrote:
> On 29/03/17 19:56, Christoffer Dall wrote:
>> On Tue, Mar 28, 2017 at 01:24:15PM -0700, Radha Mohan wrote:
>>> On Tue, Mar 28, 2017 at 1:16 PM, Christoffer Dall  wrote:
 Hi Radha,

 On Tue, Mar 28, 2017 at 12:58:24PM -0700, Radha Mohan wrote:
> Hi,
> I am seeing an issue with qemu-system-aarch64 when using pflash
> (booting kernel via UEFI bios).
>
> Host kernel: 4.11.0-rc3-next-20170323
> Qemu version: v2.9.0-rc1
>
> Command used:
> ./aarch64-softmmu/qemu-system-aarch64 -cpu host -enable-kvm -M
> virt,gic_version=3 -nographic -smp 1 -m 2048 -drive
> if=none,id=hd0,file=/root/zesty-server-cloudimg-arm64.img,id=0 -device
> virtio-blk-device,drive=hd0 -pflash /root/flash0.img -pflash
> /root/flash1.img
>
>
> As soon as the guest kernel boots the host starts to stall and prints
> the below messages. And the system never recovers. I can neither
> poweroff the guest nor the host. So I have resort to external power
> reset of the host.
>
> ==
> [  116.199077] NMI watchdog: BUG: soft lockup - CPU#25 stuck for 23s!
> [kworker/25:1:454]
> [  116.206901] Modules linked in: binfmt_misc nls_iso8859_1 aes_ce_blk
> shpchp crypto_simd gpio_keys cryptd aes_ce_cipher ghash_ce sha2_ce
> sha1_ce uio_pdrv_genirq uio autofs4 btrfs raid10 rai
> d456 async_raid6_recov async_memcpy async_pq async_xor async_tx xor
> raid6_pq libcrc32c raid1 raid0 multipath linear ast i2c_algo_bit ttm
> drm_kms_helper syscopyarea sysfillrect sysimgblt fb_s
> ys_fops drm nicvf ahci nicpf libahci thunder_bgx thunder_xcv
> mdio_thunder mdio_cavium
>
> [  116.206995] CPU: 25 PID: 454 Comm: kworker/25:1 Not tainted
> 4.11.0-rc3-next-20170323 #1
> [  116.206997] Hardware name: www.cavium.com crb-1s/crb-1s, BIOS 0.3 Feb 
> 23 2017
> [  116.207010] Workqueue: events netstamp_clear
> [  116.207015] task: 801f906b5400 task.stack: 801f901a4000
> [  116.207020] PC is at smp_call_function_many+0x284/0x2e8
> [  116.207023] LR is at smp_call_function_many+0x244/0x2e8
> [  116.207026] pc : [] lr : []
> pstate: 8145
> [  116.207028] sp : 801f901a7be0
> [  116.207030] x29: 801f901a7be0 x28: 09139000
> [  116.207036] x27: 09139434 x26: 0080
> [  116.207041] x25:  x24: 081565d0
> [  116.207047] x23: 0001 x22: 08e11e00
> [  116.207052] x21: 801f6d5cff00 x20: 801f6d5cff08
> [  116.207057] x19: 09138e38 x18: 0a03
> [  116.207063] x17: b77c9028 x16: 082e81d8
> [  116.207068] x15: 3d0d6dd44d08 x14: 0036312196549b4a
> [  116.207073] x13: 58dabe4c x12: 0018
> [  116.207079] x11: 366e2f04 x10: 09f0
> [  116.207084] x9 : 801f901a7d30 x8 : 0002
> [  116.207089] x7 :  x6 : 
> [  116.207095] x5 :  x4 : 0020
> [  116.207100] x3 : 0020 x2 : 
> [  116.207105] x1 : 801f6d682578 x0 : 0003
>
> [  150.443116] INFO: rcu_sched self-detected stall on CPU
> [  150.448261]  25-...: (14997 ticks this GP)
> idle=47a/141/0 softirq=349/349 fqs=7495
> [  150.451115] INFO: rcu_sched detected stalls on CPUs/tasks:
> [  150.451123]  25-...: (14997 ticks this GP)
> idle=47a/141/0 softirq=349/349 fqs=7495
> [  150.451124]  (detected by 13, t=15002 jiffies, g=805, c=804, q=8384)
> [  150.451136] Task dump for CPU 25:
> [  150.451138] kworker/25:1R  running task0   454  2 
> 0x0002
> [  150.451155] Workqueue: events netstamp_clear
> [  150.451158] Call trace:
> [  150.451164] [] __switch_to+0x90/0xa8
> [  150.451172] [] static_key_slow_inc+0x128/0x138
> [  150.451175] [] static_key_enable+0x34/0x60
> [  150.451178] [] netstamp_clear+0x68/0x80
> [  150.451181] [] process_one_work+0x158/0x478
> [  150.451183] [] worker_thread+0x50/0x4a8
> [  150.451187] [] kthread+0x108/0x138
> [  150.451190] [] ret_from_fork+0x10/0x50
> [  150.477451]   (t=15008 jiffies g=805 c=804 q=8384)
> [  150.482242] Task dump for CPU 25:
> [  150.482245] kworker/25:1R  running task0   454  2 
> 0x0002
> [  150.482259] Workqueue: events netstamp_clear
> [  150.482264] Call trace:
> [  150.482271] [] dump_backtrace+0x0/0x2b0
> [  150.482277] [] show_stack+0x24/0x30
> [  150.482281] [] sched_show_task+0x128/0x178
> [  150.482285] [] dump_cpu_task+0x48/0x58
> [  150.482288] [] rcu_dump_cpu_stacks+0xa0/0xe8
> [  150.482297] [] rcu_check_callbacks+0x774/0x938
> [  150.482305] [] update_process_times+0x34/0x60
> [  

[Qemu-devel] [Bug 1014681] Re: BSOD with newer host kernels (x64) and W2k8S guest (x64)

[Thomas Huth]

** Changed in: qemu
   Status: Incomplete => Won't Fix

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

Title:
  BSOD with newer host kernels (x64) and W2k8S guest (x64)

Status in QEMU:
  Won't Fix

Bug description:
  Hallo, I attempted to move virtual machines from one host to another
  but got stuck with Windows-BSODs on the target host. The host-side
  console message is "virtio_ioport_write: unexpected address 0x13 value
  0x1". Eventually there are overlaps to bug #990364, but I'm not sure.

  Host machine: 2x Opteron 4238 a 6 cores, 32GB RAM, Linux x86_64
  Guest machine(s): Windows 2008 Server R2 x64

  I tried different combinations of component versions, but only kernel
  2.6.34 could run the guests (but has other difficulties). See testet
  variants in comment.

  Run arguments are attached. Minidump follows immediately.

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

Re: [Qemu-devel] [PATCH v2 2/3] applesmc: implement error status port

[Philippe Mathieu-Daudé]

On 04/04/2017 02:01 PM, Gabriel L. Somlo wrote:

As of release 10.12.4, OS X (Sierra) refuses to boot unless the
AppleSMC supports an additional I/O port, expected to provide an
error status code.

Update the [cmd|data]_write() and data_read() methods to implement
the required state machine, and add I/O region & methods to handle
access to the error port.

Originally proposed by Eric Shelton 

Signed-off-by: Gabriel Somlo 


Reviewed-by: Philippe Mathieu-Daudé 


---
 hw/misc/applesmc.c | 141 +++--
 1 file changed, 115 insertions(+), 26 deletions(-)

diff --git a/hw/misc/applesmc.c b/hw/misc/applesmc.c
index 6381197..0d882e8 100644
--- a/hw/misc/applesmc.c
+++ b/hw/misc/applesmc.c
@@ -43,6 +43,7 @@
 enum {
 APPLESMC_DATA_PORT   = 0x00,
 APPLESMC_CMD_PORT= 0x04,
+APPLESMC_ERR_PORT= 0x1e,
 APPLESMC_NUM_PORTS   = 0x20,
 };

@@ -53,6 +54,24 @@ enum {
 APPLESMC_GET_KEY_TYPE_CMD= 0x13,
 };

+enum {
+APPLESMC_ST_CMD_DONE = 0x00,
+APPLESMC_ST_DATA_READY   = 0x01,
+APPLESMC_ST_BUSY = 0x02,
+APPLESMC_ST_ACK  = 0x04,
+APPLESMC_ST_NEW_CMD  = 0x08,
+};
+
+enum {
+APPLESMC_ST_1E_CMD_INTRUPTED = 0x80,
+APPLESMC_ST_1E_STILL_BAD_CMD = 0x81,
+APPLESMC_ST_1E_BAD_CMD   = 0x82,
+APPLESMC_ST_1E_NOEXIST   = 0x84,
+APPLESMC_ST_1E_WRITEONLY = 0x85,
+APPLESMC_ST_1E_READONLY  = 0x86,
+APPLESMC_ST_1E_BAD_INDEX = 0xb8,
+};
+
 #ifdef DEBUG_SMC
 #define smc_debug(...) fprintf(stderr, "AppleSMC: " __VA_ARGS__)
 #else
@@ -77,9 +96,12 @@ struct AppleSMCState {

 MemoryRegion io_data;
 MemoryRegion io_cmd;
+MemoryRegion io_err;
 uint32_t iobase;
 uint8_t cmd;
 uint8_t status;
+uint8_t status_1e;
+uint8_t last_ret;
 char key[4];
 uint8_t read_pos;
 uint8_t data_len;
@@ -93,89 +115,138 @@ static void applesmc_io_cmd_write(void *opaque, hwaddr 
addr, uint64_t val,
   unsigned size)
 {
 AppleSMCState *s = opaque;
+uint8_t status = s->status & 0x0f;

-smc_debug("CMD Write B: %#x = %#x\n", addr, val);
+smc_debug("CMD received: 0x%02x\n", (uint8_t)val);
 switch (val) {
 case APPLESMC_READ_CMD:
-s->status = 0x0c;
+/* did last command run through OK? */
+if (status == APPLESMC_ST_CMD_DONE || status == APPLESMC_ST_NEW_CMD) {
+s->cmd = val;
+s->status = APPLESMC_ST_NEW_CMD | APPLESMC_ST_ACK;
+} else {
+smc_debug("ERROR: previous command interrupted!\n");
+s->status = APPLESMC_ST_NEW_CMD;
+s->status_1e = APPLESMC_ST_1E_CMD_INTRUPTED;
+}
 break;
+default:
+smc_debug("UNEXPECTED CMD 0x%02x\n", (uint8_t)val);
+s->status = APPLESMC_ST_NEW_CMD;
+s->status_1e = APPLESMC_ST_1E_BAD_CMD;
 }
-s->cmd = val;
 s->read_pos = 0;
 s->data_pos = 0;
 }

-static void applesmc_fill_data(AppleSMCState *s)
+static struct AppleSMCData *applesmc_find_key(AppleSMCState *s)
 {
 struct AppleSMCData *d;

 QLIST_FOREACH(d, >data_def, node) {
 if (!memcmp(d->key, s->key, 4)) {
-smc_debug("Key matched (%s Len=%d Data=%s)\n", d->key,
-  d->len, d->data);
-memcpy(s->data, d->data, d->len);
-return;
+return d;
 }
 }
+return NULL;
 }

 static void applesmc_io_data_write(void *opaque, hwaddr addr, uint64_t val,
unsigned size)
 {
 AppleSMCState *s = opaque;
+struct AppleSMCData *d;

-smc_debug("DATA Write B: %#x = %#x\n", addr, val);
+smc_debug("DATA received: 0x%02x\n", (uint8_t)val);
 switch (s->cmd) {
 case APPLESMC_READ_CMD:
+if ((s->status & 0x0f) == APPLESMC_ST_CMD_DONE) {
+break;
+}
 if (s->read_pos < 4) {
 s->key[s->read_pos] = val;
-s->status = 0x04;
+s->status = APPLESMC_ST_ACK;
 } else if (s->read_pos == 4) {
-s->data_len = val;
-s->status = 0x05;
-s->data_pos = 0;
-smc_debug("Key = %c%c%c%c Len = %d\n", s->key[0],
-  s->key[1], s->key[2], s->key[3], val);
-applesmc_fill_data(s);
+d = applesmc_find_key(s);
+if (d != NULL) {
+memcpy(s->data, d->data, d->len);
+s->data_len = d->len;
+s->data_pos = 0;
+s->status = APPLESMC_ST_ACK | APPLESMC_ST_DATA_READY;
+s->status_1e = APPLESMC_ST_CMD_DONE;  /* clear on valid key */
+} else {
+smc_debug("READ_CMD: key '%c%c%c%c' not found!\n",
+  s->key[0], s->key[1], s->key[2], 

[Qemu-devel] [Bug 1678466] Re: using x-vga=on with vfio-pci leads to segfault

[Thomas Huth]

** Changed in: qemu
   Status: New => Fix Committed

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

Title:
  using x-vga=on with vfio-pci leads to segfault

Status in QEMU:
  Fix Committed

Bug description:
  bug occures at least with qemu 2.8.0 and 2.8.1 in 64bit-system

  stripped cmd for minimal config:
  qemu-system-i386 -m 2048 -M q35  -enable-kvm -nodefaults -nodefconfig -device 
ioh3420,bus=pcie.0,addr=0x9,multifunction=on,port=1,chassis=1,id=root.1 -device 
vfio-pci,host=01:00.0,bus=root.1,addr=01.0,x-vga=on

  Backtrace is:
  #0  0x557ca836 in memory_region_update_container_subregions 
(subregion=0x5828f2f0) at qemu-2.8.1/memory.c:2030
  #1  0x557ca9dc in memory_region_add_subregion_common (mr=0x0, 
offset=8, subregion=0x5828f2f0) at qemu-2.8.1/memory.c:2049
  #2  0x557caa9a in memory_region_add_subregion_overlap (mr=0x0, 
offset=8, subregion=0x5828f2f0, priority=1) at qemu-2.8.1/memory.c:2066
  #3  0x55832e48 in vfio_probe_nvidia_bar5_quirk (vdev=0x5805aef0, 
nr=5) at qemu-2.8.1/hw/vfio/pci-quirks.c:689
  #4  0x55835433 in vfio_bar_quirk_setup (vdev=0x5805aef0, nr=5) at 
qemu-2.8.1/hw/vfio/pci-quirks.c:1652
  #5  0x5582f122 in vfio_realize (pdev=0x5805aef0, 
errp=0x7fffdc78) at qemu-2.8.1/hw/vfio/pci.c:2777
  #6  0x55a86195 in pci_qdev_realize (qdev=0x5805aef0, 
errp=0x7fffdcf0) at hw/pci/pci.c:1966
  #7  0x559be7b7 in device_set_realized (obj=0x5805aef0, 
value=true, errp=0x7fffdeb0) at hw/core/qdev.c:918
  #8  0x55bb017f in property_set_bool (obj=0x5805aef0, 
v=0x5805ced0, name=0x56071b56 "realized", opaque=0x57f15860, 
errp=0x7fffdeb0) at qom/object.c:1854
  #9  0x55bae2e6 in object_property_set (obj=0x5805aef0, 
v=0x5805ced0, name=0x56071b56 "realized", errp=0x7fffdeb0) at 
qom/object.c:1088
  #10 0x55bb184f in object_property_set_qobject (obj=0x5805aef0, 
value=0x5805cd70, name=0x56071b56 "realized", errp=0x7fffdeb0) at 
qom/qom-qobject.c:27
  #11 0x55bae637 in object_property_set_bool (obj=0x5805aef0, 
value=true, name=0x56071b56 "realized", errp=0x7fffdeb0) at 
qom/object.c:1157
  #12 0x558fee4b in qdev_device_add (opts=0x56b15160, 
errp=0x7fffdf28) at qdev-monitor.c:623
  #13 0x559142c1 in device_init_func (opaque=0x0, opts=0x56b15160, 
errp=0x0) at vl.c:2373
  #14 0x55cc3bb7 in qemu_opts_foreach (list=0x56548b80 
, func=0x55914283 , opaque=0x0, 
errp=0x0) at util/qemu-option.c:1116
  #15 0x559198aa in main (argc=12, argv=0x7fffe388, 
envp=0x7fffe3f0) at vl.c:4574

  as I can see, it happens during initialization of the device-option.

  seems that the code tries to loop over a memory-region mr, which is
  null from at least three calls before it crashes.

  because there seems to be special handling for nvidia-cards, here're the 
pci-infos of the card:
  01:00.0 VGA compatible controller [0300]: NVIDIA Corporation G72 [GeForce 
7300 GS] [10de:01df] (rev a1) (prog-if 00 [VGA controller])
Subsystem: Gigabyte Technology Co., Ltd Device [1458:342a]
Flags: fast devsel, IRQ 16
Memory at de00 (32-bit, non-prefetchable) [disabled] [size=16M]
Memory at c000 (64-bit, prefetchable) [disabled] [size=256M]
Memory at dd00 (64-bit, non-prefetchable) [disabled] [size=16M]
Expansion ROM at df00 [disabled] [size=128K]
Capabilities: [60] Power Management version 2
Capabilities: [68] MSI: Enable- Count=1/1 Maskable- 64bit+
Capabilities: [78] Express Endpoint, MSI 00
Capabilities: [100] Virtual Channel
Capabilities: [128] Power Budgeting 
Kernel driver in use: vfio-pci

  at least with a similar card in another slot the crash does not occure.
  (sorry, can't change the slots at the moment)

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

Re: [Qemu-devel] [Qemu-ppc] [PATCH v2 0/3] Enable MTTCG on PPC64

[G 3]

On Apr 7, 2017, at 1:49 PM, luigi burdo wrote:


Tested on PowerMac G5 Quad  and 380% of system load and working on
Fedora 25 PPC64 host and Ubuntu Mate 17.04 guest  (patched the 2.9  
rc3)


The machine configuration was this

sudo ./qemu-system-ppc64 -cpu POWER8 -vga none -machine  
pseries-2.5,usb=off -m 2G  -smp 4,cores=4,threads=1 -accel  
tcg,thread=multi  -kerne vmlinuz-append root=/dev/sda   -device  
ich9-ahci,id=ahci   -device ide-drive,drive=disk0 -drive file=/dev/ 
sda4,if=none,id=disk0   -net nic,model=pcnet -net user -soundhw  
hda  -display sdl -vga virtio


vga virtio working too
here a shot
https://scontent-mxp1-1.xx.fbcdn.net/v/ 
t1.0-9/17796379_10208795258860396_7825547329794577576_n.jpg? 
oh=526d6ddeb67c817053582d5b9ee56c71=594D7BDF


Thanks
Luigi


Do you have any timings? Did the guest run faster?

Re: [Qemu-devel] [PATCH 03/14] cg3: remove unused width and height variables

[Philippe Mathieu-Daudé]

On 04/05/2017 05:35 AM, Mark Cave-Ayland wrote:

These aren't required since we can use the display width and height directly.

Signed-off-by: Mark Cave-Ayland 


Reviewed-by: Philippe Mathieu-Daudé 


---
 hw/display/cg3.c |   15 ++-
 1 file changed, 6 insertions(+), 9 deletions(-)

diff --git a/hw/display/cg3.c b/hw/display/cg3.c
index b42f60e..178a6dd 100644
--- a/hw/display/cg3.c
+++ b/hw/display/cg3.c
@@ -93,14 +93,11 @@ static void cg3_update_display(void *opaque)
 uint32_t *data;
 uint32_t dval;
 int x, y, y_start;
-unsigned int width, height;
 ram_addr_t page, page_min, page_max;

 if (surface_bits_per_pixel(surface) != 32) {
 return;
 }
-width = s->width;
-height = s->height;

 y_start = -1;
 page_min = -1;
@@ -110,11 +107,11 @@ static void cg3_update_display(void *opaque)
 data = (uint32_t *)surface_data(surface);

 memory_region_sync_dirty_bitmap(>vram_mem);
-for (y = 0; y < height; y++) {
+for (y = 0; y < s->height; y++) {
 int update = s->full_update;

-page = y * width;
-update |= memory_region_get_dirty(>vram_mem, page, width,
+page = y * s->width;
+update |= memory_region_get_dirty(>vram_mem, page, s->width,
   DIRTY_MEMORY_VGA);
 if (update) {
 if (y_start < 0) {
@@ -127,7 +124,7 @@ static void cg3_update_display(void *opaque)
 page_max = page;
 }

-for (x = 0; x < width; x++) {
+for (x = 0; x < s->width; x++) {
 dval = *pix++;
 dval = (s->r[dval] << 16) | (s->g[dval] << 8) | s->b[dval];
 *data++ = dval;
@@ -137,8 +134,8 @@ static void cg3_update_display(void *opaque)
 dpy_gfx_update(s->con, 0, y_start, s->width, y - y_start);
 y_start = -1;
 }
-pix += width;
-data += width;
+pix += s->width;
+data += s->width;
 }
 }
 s->full_update = 0;

Re: [Qemu-devel] Hacks for building on gcc 7 / Fedora 26

[Philippe Mathieu-Daudé]

Hi Dave,

On 04/07/2017 11:38 AM, Dr. David Alan Gilbert wrote:

Hi,
   Fedora 26 has gcc 7.0.1 which has the normal compliment
of new fussy warnings; so far I've posted :

tests/check-qdict: Fix missing brackets
slirp/smb: Replace constant strings by glib string

that fix one actual mistake and work around something it's being
fussy over.

But I've also got a pile of hacks, attached below that I'm
not too sure what I'll do with them yet, but they're attached
for anyone else trying to build.  Note they're smoke-only-tested.

I also have gcc bug https://gcc.gnu.org/bugzilla/show_bug.cgi?id=80346
filed for what I reckon is a couple of overly pessimistic warnings.

Enjoy,

Dave

From 15353ce59e35e1d85927138982241491ea65cee2 Mon Sep 17 00:00:00 2001
From: "Dr. David Alan Gilbert" 
Date: Thu, 6 Apr 2017 15:44:50 +0100
Subject: [HACK!] Hacks for f26 build

---
 block/blkdebug.c | 4 ++--
 block/blkverify.c| 4 ++--
 hw/usb/bus.c | 5 +++--
 include/qemu/iov.h   | 4 ++--
 tests/bios-tables-test.c | 2 +-
 5 files changed, 10 insertions(+), 9 deletions(-)

diff --git a/block/blkdebug.c b/block/blkdebug.c
index 67e8024e36..34c645d095 100644
--- a/block/blkdebug.c
+++ b/block/blkdebug.c
@@ -689,9 +689,9 @@ static void blkdebug_refresh_filename(BlockDriverState *bs, 
QDict *options)
 }

 if (!force_json && bs->file->bs->exact_filename[0]) {
-snprintf(bs->exact_filename, sizeof(bs->exact_filename),
+g_assert_cmpint(snprintf(bs->exact_filename, 
sizeof(bs->exact_filename),
  "blkdebug:%s:%s", s->config_file ?: "",
- bs->file->bs->exact_filename);
+ bs->file->bs->exact_filename), <, sizeof(bs->exact_filename));


I'm not sure this works as expected if you compile with CPPFLAGS 
+=-DG_DISABLE_ASSERT.


I added in me docker TODO "also build with -DG_DISABLE_ASSERT".


 }

 opts = qdict_new();
diff --git a/block/blkverify.c b/block/blkverify.c
index 9a1e21c6ad..d038947a5a 100644
--- a/block/blkverify.c
+++ b/block/blkverify.c
@@ -305,10 +305,10 @@ static void blkverify_refresh_filename(BlockDriverState 
*bs, QDict *options)
 if (bs->file->bs->exact_filename[0]
 && s->test_file->bs->exact_filename[0])
 {
-snprintf(bs->exact_filename, sizeof(bs->exact_filename),
+g_assert_cmpint(snprintf(bs->exact_filename, 
sizeof(bs->exact_filename),
  "blkverify:%s:%s",
  bs->file->bs->exact_filename,
- s->test_file->bs->exact_filename);
+ s->test_file->bs->exact_filename), <, 
sizeof(bs->exact_filename));
 }
 }

diff --git a/hw/usb/bus.c b/hw/usb/bus.c
index 24f1608b4b..6023f3b419 100644
--- a/hw/usb/bus.c
+++ b/hw/usb/bus.c
@@ -8,6 +8,7 @@
 #include "monitor/monitor.h"
 #include "trace.h"
 #include "qemu/cutils.h"
+#include 

 static void usb_bus_dev_print(Monitor *mon, DeviceState *qdev, int indent);

@@ -407,8 +408,8 @@ void usb_register_companion(const char *masterbus, USBPort 
*ports[],
 void usb_port_location(USBPort *downstream, USBPort *upstream, int portnr)
 {
 if (upstream) {
-snprintf(downstream->path, sizeof(downstream->path), "%s.%d",
- upstream->path, portnr);
+g_assert_cmpint(snprintf(downstream->path, sizeof(downstream->path), 
"%s.%d",
+ upstream->path, portnr), <, sizeof(downstream->path));
 downstream->hubcount = upstream->hubcount + 1;
 } else {
 snprintf(downstream->path, sizeof(downstream->path), "%d", portnr);
diff --git a/include/qemu/iov.h b/include/qemu/iov.h
index bd9fd55b0a..ebb0221140 100644
--- a/include/qemu/iov.h
+++ b/include/qemu/iov.h
@@ -46,7 +46,7 @@ static inline size_t
 iov_from_buf(const struct iovec *iov, unsigned int iov_cnt,
  size_t offset, const void *buf, size_t bytes)
 {
-if (__builtin_constant_p(bytes) && iov_cnt &&
+if (__builtin_constant_p(bytes) && iov_cnt && bytes <= INT_MAX &&
 offset <= iov[0].iov_len && bytes <= iov[0].iov_len - offset) {
 memcpy(iov[0].iov_base + offset, buf, bytes);
 return bytes;
@@ -59,7 +59,7 @@ static inline size_t
 iov_to_buf(const struct iovec *iov, const unsigned int iov_cnt,
size_t offset, void *buf, size_t bytes)
 {
-if (__builtin_constant_p(bytes) && iov_cnt &&
+if (__builtin_constant_p(bytes) && iov_cnt && bytes <= INT_MAX &&
 offset <= iov[0].iov_len && bytes <= iov[0].iov_len - offset) {
 memcpy(buf, iov[0].iov_base + offset, bytes);
 return bytes;
diff --git a/tests/bios-tables-test.c b/tests/bios-tables-test.c
index 88dbf97853..c55de4f65b 100644
--- a/tests/bios-tables-test.c
+++ b/tests/bios-tables-test.c
@@ -98,7 +98,7 @@ static void test_acpi_rsdt_table(test_data *data)
 AcpiRsdtDescriptorRev1 *rsdt_table = >rsdt_table;
 uint32_t addr = data->rsdp_table.rsdt_physical_address;
 uint32_t *tables;
-int tables_nr;
+unsigned int tables_nr;

Re: [Qemu-devel] [PATCH v2 11/12] cpus: call cpu_update_icount on read

[Richard Henderson]

On 04/07/2017 06:14 AM, Alex Bennée wrote:

Sorry, I meant we should generate TCG opcodes for the translation in
gen_io_start.

Ahh OK. I think this is 2.10 stuff though right?



Definitely.


r~

Re: [Qemu-devel] [PATCH v2] hw/arm/virt: generate 64-bit addressable ACPI objects

[Michael S. Tsirkin]

On Fri, Apr 07, 2017 at 03:41:38PM +0100, Ard Biesheuvel wrote:
> Our current ACPI table generation code limits the placement of ACPI
> tables to 32-bit addressable memory, in order to be able to emit the
> root pointer (RSDP) and root table (RSDT) using table types from the
> ACPI 1.0 days.
> 
> Since ARM was not supported by ACPI before version 5.0, it makes sense
> to lift this restriction. This is not crucial for mach-virt, which is
> guaranteed to have some memory available below the 4 GB mark, but it
> is a nice to have for QEMU machines that do not have any 32-bit
> addressable memory, which is not uncommon for real world 64-bit ARM
> systems.
> 
> Since we already emit a version of the RSDP root pointer that has a
> secondary 64-bit wide address field for the 64-bit root table (XSDT),
> all we need to do is replace the RSDT generation with the generation
> of an XSDT table, and use a different slot in the FADT table to refer
> to the DSDT.
> 
> Signed-off-by: Ard Biesheuvel 
> Reviewed-by: Andrew Jones 
> Acked-by: Laszlo Ersek 
> ---
> v2: - move new build_xsdt() function to hw/acpi/aml-build.c
> - tweak commit log text
> - add Drew's and Laszlo's acks
> 
>  hw/acpi/aml-build.c | 27 
>  hw/arm/virt-acpi-build.c| 26 +--
>  include/hw/acpi/acpi-defs.h | 11 
>  include/hw/acpi/aml-build.h |  3 +++
>  4 files changed, 54 insertions(+), 13 deletions(-)
> 
> diff --git a/hw/acpi/aml-build.c b/hw/acpi/aml-build.c
> index c6f2032decb1..4ddfb68b247f 100644
> --- a/hw/acpi/aml-build.c
> +++ b/hw/acpi/aml-build.c
> @@ -1599,6 +1599,33 @@ build_rsdt(GArray *table_data, BIOSLinker *linker, 
> GArray *table_offsets,
>   (void *)rsdt, "RSDT", rsdt_len, 1, oem_id, oem_table_id);
>  }
>  
> +/* Build xsdt table */
> +void
> +build_xsdt(GArray *table_data, BIOSLinker *linker, GArray *table_offsets,
> +   const char *oem_id, const char *oem_table_id)
> +{
> +int i;
> +unsigned xsdt_entries_offset;
> +AcpiXsdtDescriptorRev2 *xsdt;
> +const unsigned table_data_len = (sizeof(uint64_t) * table_offsets->len);
> +const unsigned xsdt_entry_size = sizeof(xsdt->table_offset_entry[0]);
> +const size_t xsdt_len = sizeof(*xsdt) + table_data_len;
> +
> +xsdt = acpi_data_push(table_data, xsdt_len);
> +xsdt_entries_offset = (char *)xsdt->table_offset_entry - 
> table_data->data;
> +for (i = 0; i < table_offsets->len; ++i) {
> +uint64_t ref_tbl_offset = g_array_index(table_offsets, uint32_t, i);
> +uint64_t xsdt_entry_offset = xsdt_entries_offset + xsdt_entry_size * 
> i;
> +
> +/* xsdt->table_offset_entry to be filled by Guest linker */
> +bios_linker_loader_add_pointer(linker,
> +ACPI_BUILD_TABLE_FILE, xsdt_entry_offset, xsdt_entry_size,
> +ACPI_BUILD_TABLE_FILE, ref_tbl_offset);
> +}
> +build_header(linker, table_data,
> + (void *)xsdt, "XSDT", xsdt_len, 1, oem_id, oem_table_id);
> +}
> +
>  void build_srat_memory(AcpiSratMemoryAffinity *numamem, uint64_t base,
> uint64_t len, int node, MemoryAffinityFlags flags)
>  {
> diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
> index b173bd109b91..2177f60544ce 100644
> --- a/hw/arm/virt-acpi-build.c
> +++ b/hw/arm/virt-acpi-build.c
> @@ -391,12 +391,12 @@ static void acpi_dsdt_add_power_button(Aml *scope)
>  
>  /* RSDP */
>  static GArray *
> -build_rsdp(GArray *rsdp_table, BIOSLinker *linker, unsigned rsdt_tbl_offset)
> +build_rsdp(GArray *rsdp_table, BIOSLinker *linker, unsigned xsdt_tbl_offset)
>  {
>  AcpiRsdpDescriptor *rsdp = acpi_data_push(rsdp_table, sizeof *rsdp);
> -unsigned rsdt_pa_size = sizeof(rsdp->rsdt_physical_address);
> -unsigned rsdt_pa_offset =
> -(char *)>rsdt_physical_address - rsdp_table->data;
> +unsigned xsdt_pa_size = sizeof(rsdp->xsdt_physical_address);
> +unsigned xsdt_pa_offset =
> +(char *)>xsdt_physical_address - rsdp_table->data;
>  
>  bios_linker_loader_alloc(linker, ACPI_BUILD_RSDP_FILE, rsdp_table, 16,
>   true /* fseg memory */);
> @@ -408,8 +408,8 @@ build_rsdp(GArray *rsdp_table, BIOSLinker *linker, 
> unsigned rsdt_tbl_offset)
>  
>  /* Address to be filled by Guest linker */
>  bios_linker_loader_add_pointer(linker,
> -ACPI_BUILD_RSDP_FILE, rsdt_pa_offset, rsdt_pa_size,
> -ACPI_BUILD_TABLE_FILE, rsdt_tbl_offset);
> +ACPI_BUILD_RSDP_FILE, xsdt_pa_offset, xsdt_pa_size,
> +ACPI_BUILD_TABLE_FILE, xsdt_tbl_offset);
>  
>  /* Checksum to be filled by Guest linker */
>  bios_linker_loader_add_checksum(linker, ACPI_BUILD_RSDP_FILE,
> @@ -686,7 +686,7 @@ static void build_fadt(GArray *table_data, BIOSLinker 
> *linker,
> VirtMachineState *vms, unsigned dsdt_tbl_offset)
>  {
>  AcpiFadtDescriptorRev5_1 *fadt 

Re: [Qemu-devel] [PATCH v2 3/3] target/ppc: Generate fence operations

[Richard Henderson]

On 04/06/2017 11:07 PM, Nikunj A Dadhania wrote:

Signed-off-by: Nikunj A Dadhania 
---
 target/ppc/translate.c | 8 
 1 file changed, 8 insertions(+)


Reviewed-by: Richard Henderson 


r~

Re: [Qemu-devel] [PATCH v2 1/3] target/ppc: Emulate LL/SC using cmpxchg helpers

[Richard Henderson]

On 04/06/2017 11:07 PM, Nikunj A Dadhania wrote:

Emulating LL/SC with cmpxchg is not correct, since it can suffer from
the ABA problem. However, portable parallel code is written assuming
only cmpxchg which means that in practice this is a viable alternative.

Signed-off-by: Nikunj A Dadhania 
---
 target/ppc/translate.c | 29 +++--
 1 file changed, 23 insertions(+), 6 deletions(-)


Reviewed-by: Richard Henderson 


r~

Re: [Qemu-devel] [PATCH RFC v1 3/3] target/ppc: Generate fence operations

[Richard Henderson]

On 04/06/2017 10:21 PM, Nikunj A Dadhania wrote:

We do that in the macro:

  if (len > 1) {  \
  gen_check_align(ctx, t0, (len) - 1);\
  }   \

Would we still need a barrier before the alignment check?


Ah, that's where it's been placed.

So, the MO_ALIGN flag to tcg_gen_atomic_* takes care of the alignment check 
too.  So we could move this down into the failure path.



r~

Re: [Qemu-devel] [PATCH v2 6/6] tests/block-job-txn: Don't start block job before adding to txn

[John Snow]

On 04/07/2017 09:28 AM, Stefan Hajnoczi wrote:
> On Fri, Apr 07, 2017 at 02:54:14PM +0800, Fam Zheng wrote:
>> Previously, before test_block_job_start returns, the job can already
>> complete, as a result, the transactional state of other jobs added to
>> the same txn later cannot be handled correctly.
>>
>> Move the block_job_start() calls to callers after
>> block_job_txn_add_job() calls.
>>
>> Signed-off-by: Fam Zheng 
>> ---
>>  tests/test-blockjob-txn.c | 6 +-
>>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> CCing John Snow because he looked at block jobs completing during txn
> setup recently.
> 
> Stefan
> 

This matches the changes we made to qmp_transaction, but I forgot to (or
didn't take care to)  change the qtest as it didn't cause a regression
at the time.

I wonder if I should make it a runtime error to add a job to a
transaction which has already "started" to make sure that this interface
is not misused, as this test highlights that there were still some
remaining "bad" uses of the interface.

Regardless...

Thanks for the CC. ACK

[Qemu-devel] [Bug 1680679] Re: qemu cannot run twice

[Roger Lawhorn]

I have been told that getting this to work with a laptop is rare.
I own an MSI GT70-2PE.

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

Title:
  qemu cannot run twice

Status in QEMU:
  New

Bug description:
  After using qemu with gpu passthrough and then shutting down windows 7 
properly I cannot boot windows 7 a second time.
  Only a full reboot of linux fixes this issue.
  Qemu appears to corrupt something in linux when exiting.
  I get no error messages but windows 7 never finishes booting during the 2nd 
try.
  Apparently I do try to run vfiobind each time the script is run.
  Wondering if rerunning vfiobind can cause an issue?

  
  My specs:
  
-
  System:Host: GT70-2PE Kernel: 4.5.4-040504-generic x86_64 (64 bit gcc: 
5.3.1)
 Desktop: Cinnamon 3.2.7 (Gtk 3.18.9) Distro: Linux Mint 18.1 Serena
  Machine:   Mobo: Micro-Star model: MS-1763 v: REV:0.C Bios: American 
Megatrends v: E1763IMS.51B date: 01/29/2015
  CPU:   Quad core Intel Core i7-4810MQ (-HT-MCP-) cache: 6144 KB
 flags: (lm nx sse sse2 sse3 sse4_1 sse4_2 ssse3 vmx) bmips: 22347
 clock speeds: max: 2801 MHz 1: 2801 MHz 2: 2801 MHz 3: 2801 MHz 4: 
2801 MHz 5: 2801 MHz 6: 2801 MHz
 7: 2801 MHz 8: 2801 MHz
  Graphics:  Card-1: Intel 4th Gen Core Processor Integrated Graphics 
Controller bus-ID: 00:02.0
 Card-2: NVIDIA GK104M [GeForce GTX 880M] bus-ID: 01:00.0
 Display Server: X.Org 1.18.4 drivers: intel (unloaded: fbdev,vesa)
 Resolution: 1920x1080@60.02hz, 1920x1080@60.00hz
 GLX Renderer: Mesa DRI Intel Haswell Mobile GLX Version: 3.0 Mesa 
12.0.6 Direct Rendering: Yes

  
  My script:
  
---
  #!/bin/bash

  cd ~/qemu
  sudo ./up.sh tap0

  configfile=~/qemu/vfio-pci1.cfg

  vfiobind() {
  dev="$1"
  vendor=$(cat /sys/bus/pci/devices/$dev/vendor)
  device=$(cat /sys/bus/pci/devices/$dev/device)
  if [ -e /sys/bus/pci/devices/$dev/driver ]; then
  echo $dev > /sys/bus/pci/devices/$dev/driver/unbind
  fi
  echo $vendor $device > /sys/bus/pci/drivers/vfio-pci/new_id

  }

  modprobe vfio-pci

  cat $configfile | while read line;do
  echo $line | grep ^# >/dev/null 2>&1 && continue
  vfiobind $line
  done

  sudo qemu-system-x86_64 -machine type=q35,accel=kvm -cpu host,kvm=off \
  -smp 8,sockets=1,cores=4,threads=2 \
  -bios /usr/share/seabios/bios.bin \
  -serial none \
  -parallel none \
  -vga none \
  -m 4G \
  -mem-path /run/hugepages/kvm \
  -mem-prealloc \
  -balloon none \
  -rtc clock=host,base=localtime \
  -device 
ioh3420,bus=pcie.0,addr=1c.0,multifunction=on,port=1,chassis=1,id=root.1 \
  -device vfio-pci,host=01:00.0,bus=root.1,addr=00.0,multifunction=on,x-vga=on \
  -device virtio-scsi-pci,id=scsi \
  -drive 
id=disk0,if=virtio,cache=none,format=raw,file=/home/dad/qemu/windows7.img \
  -drive 
file=/home/dad/1TB-Backup/Iso/SP1ForWin7.iso,id=isocd,format=raw,if=none 
-device scsi-cd,drive=isocd \
  -net nic -net tap,ifname=tap0,script=no,downscript=no \
  -usbdevice host:413c:a503 \
  -usbdevice host:13fe:3100 \
  -usbdevice host:0bc2:ab21 \
  -boot menu=on \
  -boot order=c

  sudo ./down.sh tap0

  exit 0

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

Re: [Qemu-devel] [PATCH v2 1/3] libqtest: Ignore QMP events when parsing the response for HMP commands

[Dr. David Alan Gilbert]

* John Snow (js...@redhat.com) wrote:
> 
> 
> On 04/04/2017 03:31 AM, Thomas Huth wrote:
> > On 03.04.2017 21:09, John Snow wrote:
> >>
> >>
> >> On 03/30/2017 03:50 AM, Thomas Huth wrote:
> >>> When running certain HMP commands (like "device_del") via QMP, we
> >>> can sometimes get a QMP event in the response first, so that the
> >>> "g_assert(ret)" statement in qtest_hmp() triggers and the test
> >>> fails. Fix this by ignoring such QMP events while looking for the
> >>> real return value from QMP.
> >>>
> >>> Signed-off-by: Thomas Huth 
> >>> ---
> >>>  tests/libqtest.c | 6 ++
> >>>  1 file changed, 6 insertions(+)
> >>>
> >>> diff --git a/tests/libqtest.c b/tests/libqtest.c
> >>> index a5c3d2b..c9b2d76 100644
> >>> --- a/tests/libqtest.c
> >>> +++ b/tests/libqtest.c
> >>> @@ -580,6 +580,12 @@ char *qtest_hmpv(QTestState *s, const char *fmt, 
> >>> va_list ap)
> >>>   " 'arguments': {'command-line': %s}}",
> >>>   cmd);
> >>>  ret = g_strdup(qdict_get_try_str(resp, "return"));
> >>> +while (ret == NULL && qdict_get_try_str(resp, "event")) {
> >>> +/* Ignore asynchronous QMP events */
> >>> +QDECREF(resp);
> >>> +resp = qtest_qmp_receive(s);
> >>> +ret = g_strdup(qdict_get_try_str(resp, "return"));
> >>> +}
> >>>  g_assert(ret);
> >>>  QDECREF(resp);
> >>>  g_free(cmd);
> >>>
> >>
> >> You've probably been asked this, but can you just shove the QMP response
> >> you don't want into the event queue for consumption by other calls?
> > 
> > Well, this is the qtest_hmpv() function, so I assume that the caller
> > just wants to execute a HMP command and does not really care about QMP
> > events. If you care about QMP events, you should use the qmp functions
> > instead.
> > 
> >  Thomas
> > 
> 
> I don't think it's obvious that using HMP functions should cause the QMP
> stream to become faulty, though.
> 
> If someone uses an HMP function and then tries to wait on a QMP event to
> confirm that some key condition has occurred (pausing or resuming, for
> instance) it would not be immediately apparent from the user's POV that
> this function just eats replies because it was convenient to do so.
> 
> I guess the event queue only exists in python though, so it's not as
> trivial as I was thinking it would be...

I think it's OK to discard the QMP events - it feels rare to mix and match
in a test; if you care about QMP events you'll probably be basing the
test around QMP rather than HMP.

Dave

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

Re: [Qemu-devel] [PATCH 15/15] COLO: flush host dirty ram from cache

[Dr. David Alan Gilbert]

* zhanghailiang (zhang.zhanghaili...@huawei.com) wrote:
> Don't need to flush all VM's ram from cache, only
> flush the dirty pages since last checkpoint
> 
> Cc: Juan Quintela 
> Signed-off-by: Li Zhijian 
> Signed-off-by: Zhang Chen 
> Signed-off-by: zhanghailiang 
> ---
>  migration/ram.c | 10 ++
>  1 file changed, 10 insertions(+)
> 
> diff --git a/migration/ram.c b/migration/ram.c
> index 6227b94..e9ba740 100644
> --- a/migration/ram.c
> +++ b/migration/ram.c
> @@ -2702,6 +2702,7 @@ int colo_init_ram_cache(void)
>  migration_bitmap_rcu = g_new0(struct BitmapRcu, 1);
>  migration_bitmap_rcu->bmap = bitmap_new(ram_cache_pages);
>  migration_dirty_pages = 0;
> +memory_global_dirty_log_start();

Shouldn't there be a stop somewhere?
(Probably if you failover to the secondary and colo stops?)

>  return 0;
>  
> @@ -2750,6 +2751,15 @@ void colo_flush_ram_cache(void)
>  void *src_host;
>  ram_addr_t offset = 0;
>  
> +memory_global_dirty_log_sync();
> +qemu_mutex_lock(_bitmap_mutex);
> +rcu_read_lock();
> +QLIST_FOREACH_RCU(block, _list.blocks, next) {
> +migration_bitmap_sync_range(block->offset, block->used_length);
> +}
> +rcu_read_unlock();
> +qemu_mutex_unlock(_bitmap_mutex);

Again this might have some fun merging with Juan's recent changes - what's
really unusual about your set is that you're using this bitmap on the 
destination;
I suspect Juan's recent changes that trickier.
Check 'Creating RAMState for migration' and 'Split migration bitmaps by 
ramblock'.

Dave
>  trace_colo_flush_ram_cache_begin(migration_dirty_pages);
>  rcu_read_lock();
>  block = QLIST_FIRST_RCU(_list.blocks);
> -- 
> 1.8.3.1
> 
> 
> 
--
Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK

Re: [Qemu-devel] [PATCH 12/15] savevm: split the process of different stages for loadvm/savevm

[Dr. David Alan Gilbert]

* zhanghailiang (zhang.zhanghaili...@huawei.com) wrote:
> There are several stages during loadvm/savevm process. In different stage,
> migration incoming processes different types of sections.
> We want to control these stages more accuracy, it will benefit COLO
> performance, we don't have to save type of QEMU_VM_SECTION_START
> sections everytime while do checkpoint, besides, we want to separate
> the process of saving/loading memory and devices state.
> 
> So we add three new helper functions: qemu_loadvm_state_begin(),
> qemu_load_device_state() and qemu_savevm_live_state() to achieve
> different process during migration.
> 
> Besides, we make qemu_loadvm_state_main() and qemu_save_device_state()
> public.
> 
> Cc: Juan Quintela 
> Signed-off-by: zhanghailiang 
> Signed-off-by: Li Zhijian 
> Reviewed-by: Dr. David Alan Gilbert 
> ---
>  include/sysemu/sysemu.h |  6 ++
>  migration/savevm.c  | 55 
> ++---
>  2 files changed, 54 insertions(+), 7 deletions(-)
> 
> diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h
> index 7ed665a..95cae41 100644
> --- a/include/sysemu/sysemu.h
> +++ b/include/sysemu/sysemu.h
> @@ -132,7 +132,13 @@ void qemu_savevm_send_postcopy_ram_discard(QEMUFile *f, 
> const char *name,
> uint64_t *start_list,
> uint64_t *length_list);
>  
> +void qemu_savevm_live_state(QEMUFile *f);
> +int qemu_save_device_state(QEMUFile *f);
> +
>  int qemu_loadvm_state(QEMUFile *f);
> +int qemu_loadvm_state_begin(QEMUFile *f);
> +int qemu_loadvm_state_main(QEMUFile *f, MigrationIncomingState *mis);
> +int qemu_load_device_state(QEMUFile *f);
>  
>  extern int autostart;
>  
> diff --git a/migration/savevm.c b/migration/savevm.c
> index 9c2d239..dac478b 100644
> --- a/migration/savevm.c
> +++ b/migration/savevm.c
> @@ -54,6 +54,7 @@
>  #include "qemu/cutils.h"
>  #include "io/channel-buffer.h"
>  #include "io/channel-file.h"
> +#include "migration/colo.h"
>  
>  #ifndef ETH_P_RARP
>  #define ETH_P_RARP 0x8035
> @@ -1279,13 +1280,21 @@ done:
>  return ret;
>  }
>  
> -static int qemu_save_device_state(QEMUFile *f)
> +void qemu_savevm_live_state(QEMUFile *f)
>  {
> -SaveStateEntry *se;
> +/* save QEMU_VM_SECTION_END section */
> +qemu_savevm_state_complete_precopy(f, true);
> +qemu_put_byte(f, QEMU_VM_EOF);
> +}
>  
> -qemu_put_be32(f, QEMU_VM_FILE_MAGIC);
> -qemu_put_be32(f, QEMU_VM_FILE_VERSION);
> +int qemu_save_device_state(QEMUFile *f)
> +{
> +SaveStateEntry *se;
>  
> +if (!migration_in_colo_state()) {
> +qemu_put_be32(f, QEMU_VM_FILE_MAGIC);
> +qemu_put_be32(f, QEMU_VM_FILE_VERSION);
> +}

Note that got split out into qemu_savevm_state_header() at some point.

Dave

>  cpu_synchronize_all_states();
>  
>  QTAILQ_FOREACH(se, _state.handlers, entry) {
> @@ -1336,8 +1345,6 @@ enum LoadVMExitCodes {
>  LOADVM_QUIT =  1,
>  };
>  
> -static int qemu_loadvm_state_main(QEMUFile *f, MigrationIncomingState *mis);
> -
>  /* -- incoming postcopy messages -- */
>  /* 'advise' arrives before any transfers just to tell us that a postcopy
>   * *might* happen - it might be skipped if precopy transferred everything
> @@ -1942,7 +1949,7 @@ qemu_loadvm_section_part_end(QEMUFile *f, 
> MigrationIncomingState *mis)
>  return 0;
>  }
>  
> -static int qemu_loadvm_state_main(QEMUFile *f, MigrationIncomingState *mis)
> +int qemu_loadvm_state_main(QEMUFile *f, MigrationIncomingState *mis)
>  {
>  uint8_t section_type;
>  int ret = 0;
> @@ -2080,6 +2087,40 @@ int qemu_loadvm_state(QEMUFile *f)
>  return ret;
>  }
>  
> +int qemu_loadvm_state_begin(QEMUFile *f)
> +{
> +MigrationIncomingState *mis = migration_incoming_get_current();
> +Error *local_err = NULL;
> +int ret;
> +
> +if (qemu_savevm_state_blocked(_err)) {
> +error_report_err(local_err);
> +return -EINVAL;
> +}
> +/* Load QEMU_VM_SECTION_START section */
> +ret = qemu_loadvm_state_main(f, mis);
> +if (ret < 0) {
> +error_report("Failed to loadvm begin work: %d", ret);
> +}
> +return ret;
> +}
> +
> +int qemu_load_device_state(QEMUFile *f)
> +{
> +MigrationIncomingState *mis = migration_incoming_get_current();
> +int ret;
> +
> +/* Load QEMU_VM_SECTION_FULL section */
> +ret = qemu_loadvm_state_main(f, mis);
> +if (ret < 0) {
> +error_report("Failed to load device state: %d", ret);
> +return ret;
> +}
> +
> +cpu_synchronize_all_post_init();
> +return 0;
> +}
> +
>  int save_vmstate(Monitor *mon, const char *name)
>  {
>  BlockDriverState *bs, *bs1;
> -- 
> 1.8.3.1
> 
> 
--
Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK

Re: [Qemu-devel] [PATCH] vhost: skip RAM device memory sections

[Paolo Bonzini]

On 08/04/2017 09:16, Wang guang wrote:
> From: ZhiPeng Lu 
> 
> A RAM device represents a mapping to a physical device, such as to a PCI
> * MMIO BAR of an vfio-pci assigned device.
> Vhost listens to this region,and increases the region's reference count
> while passthrough?for?network adapters (Physical Function, PF or Virtual 
> Function, VF).
> After detaching   network adapters with  vhost backend dirver or vhost user 
> dirver,
> it unregister vhost listen function  by memory_listener_unregister.
> After detaching the passthrough pf  or vf,
> the RAM device region's reference by  vhost listener increated can not be 
> released,
> due to vhost listen function does not exist.So let's just skip RAM device 
> memory.
> 
> Signed-off-by: ZhiPeng Lu 
> ---
>  hw/virtio/vhost.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c
> index 613494d..c1ff98f 100644
> --- a/hw/virtio/vhost.c
> +++ b/hw/virtio/vhost.c
> @@ -611,7 +611,8 @@ static void vhost_set_memory(MemoryListener *listener,
>  static bool vhost_section(MemoryRegionSection *section)
>  {
>  return memory_region_is_ram(section->mr) &&
> -!memory_region_is_rom(section->mr);
> +!memory_region_is_rom(section->mr) &&
> +!memory_region_is_skip_dump(section->mr);
>  }

Why not memory_region_is_ram_device?

Paolo

Re: [Qemu-devel] [RFC] Proposed qcow2 extension: subcluster allocation

[Max Reitz]

On 06.04.2017 17:01, Alberto Garcia wrote:
> Hi all,
> 
> over the past couple of months I discussed with some of you the
> possibility to extend the qcow2 format in order to improve its
> performance and reduce its memory requirements (particularly with very
> large images).
> 
> After some discussion in the mailing list and the #qemu IRC channel I
> decided to write a prototype of a new extension for qcow2 so I could
> understand better the scope of the changes and have some preliminary
> data about its effects.
> 
> This e-mail is the formal presentation of my proposal to extend the
> on-disk qcow2 format. As you can see this is still an RFC. Due to the
> nature of the changes I would like to get as much feedback as possible
> before going forward.
> 
> === Problem ===
> 
> The original problem that I wanted to address is the memory
> requirements of qcow2 files if you want to have very large images and
> still keep good I/O performance. This is a consequence of its very
> structure, which I'm going to describe now.
> 
> A qcow2 image is divided into units of constant size called clusters,
> and among other things it contains metadata that maps guest addresses
> to host addresses (the so-called L1 and L2 tables).
> 
> There are two basic problems that result from this:
> 
> 1) Reading from or writing to a qcow2 image involves reading the
>corresponding entry on the L2 table that maps the guest address to
>the host address. This is very slow because it involves two I/O
>operations: one on the L2 table and the other one on the actual
>data cluster.
> 
> 2) A cluster is the smallest unit of allocation. Therefore writing a
>mere 512 bytes to an empty disk requires allocating a complete
>cluster and filling it with zeroes (or with data from the backing
>image if there is one). This wastes more disk space and also has a
>negative impact on I/O.
> 
> Problem (1) can be solved by keeping in memory a cache of the L2
> tables (QEMU has an "l2_cache_size" parameter for this purpose). The
> amount of disk space used by L2 tables depends on two factors: the
> disk size and the cluster size.
> 
> The cluster size can be configured when the image is created, and it
> can be any power of two between 512 bytes and 2 MB (it defaults to 64
> KB).
> 
> The maximum amount of space needed for the L2 tables can be calculated
> with the following formula:
> 
>max_l2_size = virtual_disk_size * 8 / cluster_size
> 
> Large images require a large amount of metadata, and therefore a large
> amount of memory for the L2 cache. With the default cluster size
> (64KB) that's 128MB of L2 cache for a 1TB qcow2 image.
> 
> The only way to reduce the size of the L2 tables is therefore
> increasing the cluster size, but this makes the image less efficient
> as described earlier in (2).
> 
> === The proposal ===
> 
> The idea of this proposal is to extend the qcow2 format by allowing
> subcluster allocation. There would be an optional feature that would
> need to be enabled when creating the image. The on-disk format would
> remain essentially the same, except that each data cluster would be
> internally divided into a number of subclusters of equal size.
> 
> What this means in practice is that each entry on an L2 table would be
> accompanied by a bitmap indicating the allocation state of each one of
> the subclusters for that cluster. There are several alternatives for
> storing the bitmap, described below.
> 
> Other than L2 entries, all other data structures would remain
> unchanged, but for data clusters the smallest unit of allocation would
> now be the subcluster. Reference counting would still be at the
> cluster level, because there's no way to reference individual
> subclusters. Copy-on-write on internal snapshots would need to copy
> complete clusters, so that scenario would not benefit from this
> change.
> 
> I see two main use cases for this feature:
> 
> a) The qcow2 image is not too large / the L2 cache is not a problem,
>but you want to increase the allocation performance. In this case
>you can have something like a 128KB cluster with 4KB subclusters
>(with 4KB being a common block size in ext4 and other filesystems)
> 
> b) The qcow2 image is very large and you want to save metadata space
>in order to have a smaller L2 cache. In this case you can go for
>the maximum cluster size (2MB) but you want to have smaller
>subclusters to increase the allocation performance and optimize the
>disk usage. This was actually my original use case.
> 
> === Test results ===
> 
> I have a basic working prototype of this. It's still incomplete -and
> buggy :)- but it gives an idea of what we can expect from it. In my
> implementation each data cluster has 8 subclusters, but that's not set
> in stone (see below).
> 
> I made all tests on an SSD drive, writing to an empty qcow2 image with
> a fully populated 40GB backing image, performing random writes using
> fio with a block 

Re: [Qemu-devel] [PATCH 07/15] COLO: Load PVM's dirty pages into SVM's RAM cache temporarily

[Dr. David Alan Gilbert]

* zhanghailiang (zhang.zhanghaili...@huawei.com) wrote:
> We should not load PVM's state directly into SVM, because there maybe some
> errors happen when SVM is receving data, which will break SVM.
> 
> We need to ensure receving all data before load the state into SVM. We use
> an extra memory to cache these data (PVM's ram). The ram cache in secondary 
> side
> is initially the same as SVM/PVM's memory. And in the process of checkpoint,
> we cache the dirty pages of PVM into this ram cache firstly, so this ram cache
> always the same as PVM's memory at every checkpoint, then we flush this 
> cached ram
> to SVM after we receive all PVM's state.

You're probably going to find this interesting to merge with Juan's recent RAM 
block series.
Probably not too hard, but he's touching a lot of the same code and rearranging 
things.

Dave


> Cc: Juan Quintela 
> Signed-off-by: zhanghailiang 
> Signed-off-by: Li Zhijian 
> Reviewed-by: Dr. David Alan Gilbert 
> ---
>  include/exec/ram_addr.h   |  1 +
>  include/migration/migration.h |  4 +++
>  migration/colo.c  | 14 +
>  migration/ram.c   | 73 
> ++-
>  4 files changed, 91 insertions(+), 1 deletion(-)
> 
> diff --git a/include/exec/ram_addr.h b/include/exec/ram_addr.h
> index 3e79466..44e1190 100644
> --- a/include/exec/ram_addr.h
> +++ b/include/exec/ram_addr.h
> @@ -27,6 +27,7 @@ struct RAMBlock {
>  struct rcu_head rcu;
>  struct MemoryRegion *mr;
>  uint8_t *host;
> +uint8_t *colo_cache; /* For colo, VM's ram cache */
>  ram_addr_t offset;
>  ram_addr_t used_length;
>  ram_addr_t max_length;
> diff --git a/include/migration/migration.h b/include/migration/migration.h
> index 1735d66..93c6148 100644
> --- a/include/migration/migration.h
> +++ b/include/migration/migration.h
> @@ -379,4 +379,8 @@ int ram_save_queue_pages(MigrationState *ms, const char 
> *rbname,
>  PostcopyState postcopy_state_get(void);
>  /* Set the state and return the old state */
>  PostcopyState postcopy_state_set(PostcopyState new_state);
> +
> +/* ram cache */
> +int colo_init_ram_cache(void);
> +void colo_release_ram_cache(void);
>  #endif
> diff --git a/migration/colo.c b/migration/colo.c
> index 1e3e975..edb7f00 100644
> --- a/migration/colo.c
> +++ b/migration/colo.c
> @@ -551,6 +551,7 @@ void *colo_process_incoming_thread(void *opaque)
>  uint64_t total_size;
>  uint64_t value;
>  Error *local_err = NULL;
> +int ret;
>  
>  qemu_sem_init(>colo_incoming_sem, 0);
>  
> @@ -572,6 +573,12 @@ void *colo_process_incoming_thread(void *opaque)
>   */
>  qemu_file_set_blocking(mis->from_src_file, true);
>  
> +ret = colo_init_ram_cache();
> +if (ret < 0) {
> +error_report("Failed to initialize ram cache");
> +goto out;
> +}
> +
>  bioc = qio_channel_buffer_new(COLO_BUFFER_BASE_SIZE);
>  fb = qemu_fopen_channel_input(QIO_CHANNEL(bioc));
>  object_unref(OBJECT(bioc));
> @@ -705,11 +712,18 @@ out:
>  if (fb) {
>  qemu_fclose(fb);
>  }
> +/*
> + * We can ensure BH is hold the global lock, and will join COLO
> + * incoming thread, so here it is not necessary to lock here again,
> + * Or there will be a deadlock error.
> + */
> +colo_release_ram_cache();
>  
>  /* Hope this not to be too long to loop here */
>  qemu_sem_wait(>colo_incoming_sem);
>  qemu_sem_destroy(>colo_incoming_sem);
>  /* Must be called after failover BH is completed */
> +
>  if (mis->to_src_file) {
>  qemu_fclose(mis->to_src_file);
>  }
> diff --git a/migration/ram.c b/migration/ram.c
> index f289fcd..b588990 100644
> --- a/migration/ram.c
> +++ b/migration/ram.c
> @@ -219,6 +219,7 @@ static RAMBlock *last_sent_block;
>  static ram_addr_t last_offset;
>  static QemuMutex migration_bitmap_mutex;
>  static uint64_t migration_dirty_pages;
> +static bool ram_cache_enable;
>  static uint32_t last_version;
>  static bool ram_bulk_stage;
>  
> @@ -2227,6 +2228,20 @@ static inline void 
> *host_from_ram_block_offset(RAMBlock *block,
>  return block->host + offset;
>  }
>  
> +static inline void *colo_cache_from_block_offset(RAMBlock *block,
> + ram_addr_t offset)
> +{
> +if (!offset_in_ramblock(block, offset)) {
> +return NULL;
> +}
> +if (!block->colo_cache) {
> +error_report("%s: colo_cache is NULL in block :%s",
> + __func__, block->idstr);
> +return NULL;
> +}
> +return block->colo_cache + offset;
> +}
> +
>  /*
>   * If a page (or a whole RDMA chunk) has been
>   * determined to be zero, then zap it.
> @@ -2542,7 +2557,12 @@ static int ram_load(QEMUFile *f, void *opaque, int 
> version_id)
>   RAM_SAVE_FLAG_COMPRESS_PAGE | RAM_SAVE_FLAG_XBZRLE)) {
> 

[Qemu-devel] [Bug 1481272] Re: main-loop: WARNING: I/O thread spun for 1000 iterations

[Marcin Mielniczuk]

The command used is: qemu-system-x86_64 -drive file=../minix-img/minix_work.img 
-net user,hostfwd=tcp::2-:22 -net nic,model=virtio -m 1024M 
qemu 1.8.0 in this case, I'll test against 1.8.1 when it gets into the repos.

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

Title:
  main-loop: WARNING: I/O thread spun for 1000 iterations

Status in QEMU:
  New

Bug description:
  I compile the latest qemu to launch a VM but the monitor output the
  "main-loop: WARNING: I/O thread spun for 1000 iterations".

  # /usr/local/bin/qemu-system-x86_64 -name rhel6 -S -no-kvm -m 1024M -realtime 
mlock=off -smp 1,sockets=1,cores=1,threads=1 -uuid 
c9dd2a5c-40f2-fd3d-3c54-9cd84f8b9174 -rtc base=utc  -drive 
file=/home/samba-share/ubuntu.img,if=none,id=drive-virtio-disk0,format=qcow2,cache=none
 -device 
virtio-blk-pci,bus=pci.0,addr=0x4,drive=drive-virtio-disk0,id=disk,serial=425618d4-871f-4021-bc5d-bcd7f1b5ca9c,bootindex=0
 -vnc :1 -boot menu=on -monitor stdio
  QEMU 2.3.93 monitor - type 'help' for more information
  (qemu) c
  (qemu) main-loop: WARNING: I/O thread spun for 1000 iterations   
<---

  qemu]# git branch -v
  * master   e95edef Merge remote-tracking branch 
'remotes/sstabellini/tags/xen-migration-2.4-tag' into staging

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

[Qemu-devel] [Bug 1481272] Re: main-loop: WARNING: I/O thread spun for 1000 iterations

[Marcin Mielniczuk]

I experience the same while running Minix guest under an Arch Linux
host. Sometimes during heavy I/O the whole machine freezes with this
error message.

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

Title:
  main-loop: WARNING: I/O thread spun for 1000 iterations

Status in QEMU:
  New

Bug description:
  I compile the latest qemu to launch a VM but the monitor output the
  "main-loop: WARNING: I/O thread spun for 1000 iterations".

  # /usr/local/bin/qemu-system-x86_64 -name rhel6 -S -no-kvm -m 1024M -realtime 
mlock=off -smp 1,sockets=1,cores=1,threads=1 -uuid 
c9dd2a5c-40f2-fd3d-3c54-9cd84f8b9174 -rtc base=utc  -drive 
file=/home/samba-share/ubuntu.img,if=none,id=drive-virtio-disk0,format=qcow2,cache=none
 -device 
virtio-blk-pci,bus=pci.0,addr=0x4,drive=drive-virtio-disk0,id=disk,serial=425618d4-871f-4021-bc5d-bcd7f1b5ca9c,bootindex=0
 -vnc :1 -boot menu=on -monitor stdio
  QEMU 2.3.93 monitor - type 'help' for more information
  (qemu) c
  (qemu) main-loop: WARNING: I/O thread spun for 1000 iterations   
<---

  qemu]# git branch -v
  * master   e95edef Merge remote-tracking branch 
'remotes/sstabellini/tags/xen-migration-2.4-tag' into staging

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

[Qemu-devel] [Bug 1635339] Re: qxl_pre_save assertion failure on vm "save"

[Dr. David Alan Gilbert]

An untested hack that might fail cleaner might be:

  error_report("Broken driver . migration failed");
  qemu_file_set_error(migrate_get_current()->to_dst_file, -EINVAL);

(untested, probably needs checking it works with savevm).

I must go around and add a return value to pre_save().

We probably also need to make sure some migration testing gets added to
the driver dev

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

Title:
  qxl_pre_save assertion failure on vm "save"

Status in QEMU:
  Confirmed

Bug description:
  When I try and save my Windows 10 VM, I see an assertion failure, and
  the machine is shut down.

  I see the following in the log:

  main_channel_handle_parsed: agent start
  qemu-system-x86_64: /build/qemu-Zwynhi/qemu-2.5+dfsg/hw/display/qxl.c:2101: 
qxl_pre_save: Assertion `d->last_release_offset < d->vga.vram_size' failed.
  2016-10-20 11:52:42.713+: shutting down

  Please let me know what other information would be relevant!

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

[Qemu-devel] [Bug 1635339] Re: qxl_pre_save assertion failure on vm "save"

[Gerd Hoffmann]

Well.  qxl commands are expected to live in bar 0 (same bar where the
rings are too).  vram bar was added as surface storage.

Now the windows drivers started to us vram for qxl commands.  Problem is
we simply can't live-migrate such a guest.  At least not without
changing the vmstate format.  Which isn't something I would attempt just
a few days before release.

We can't throw an error in qxl_pre_save either (and fail migration
instead of aborting).

I don't see an easy way out for 2.9.

Long term options are (a) revert the driver change, and probably add
some checks to qxl to make sure guests don't use vram for commands, or
(b) extend qxl vmstate so we can handle that case.

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

Title:
  qxl_pre_save assertion failure on vm "save"

Status in QEMU:
  Confirmed

Bug description:
  When I try and save my Windows 10 VM, I see an assertion failure, and
  the machine is shut down.

  I see the following in the log:

  main_channel_handle_parsed: agent start
  qemu-system-x86_64: /build/qemu-Zwynhi/qemu-2.5+dfsg/hw/display/qxl.c:2101: 
qxl_pre_save: Assertion `d->last_release_offset < d->vga.vram_size' failed.
  2016-10-20 11:52:42.713+: shutting down

  Please let me know what other information would be relevant!

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

Re: [Qemu-devel] [PATCH 04/15] COLO: integrate colo compare with colo frame

[Dr. David Alan Gilbert]

* zhanghailiang (zhang.zhanghaili...@huawei.com) wrote:
> For COLO FT, both the PVM and SVM run at the same time,
> only sync the state while it needs.
> 
> So here, let SVM runs while not doing checkpoint,
> Besides, change DEFAULT_MIGRATE_X_CHECKPOINT_DELAY to 200*100.
> 
> Cc: Jason Wang 
> Signed-off-by: zhanghailiang 

Reviewed-by: Dr. David Alan Gilbert 

> ---
>  migration/colo.c  | 25 +
>  migration/migration.c |  2 +-
>  2 files changed, 26 insertions(+), 1 deletion(-)
> 
> diff --git a/migration/colo.c b/migration/colo.c
> index 712308e..fb8d8fd 100644
> --- a/migration/colo.c
> +++ b/migration/colo.c
> @@ -19,8 +19,11 @@
>  #include "qemu/error-report.h"
>  #include "qapi/error.h"
>  #include "migration/failover.h"
> +#include "net/colo-compare.h"
> +#include "net/colo.h"
>  
>  static bool vmstate_loading;
> +static Notifier packets_compare_notifier;
>  
>  #define COLO_BUFFER_BASE_SIZE (4 * 1024 * 1024)
>  
> @@ -263,6 +266,7 @@ static int colo_do_checkpoint_transaction(MigrationState 
> *s,
>  if (local_err) {
>  goto out;
>  }
> +
>  /* Reset channel-buffer directly */
>  qio_channel_io_seek(QIO_CHANNEL(bioc), 0, 0, NULL);
>  bioc->usage = 0;
> @@ -283,6 +287,11 @@ static int colo_do_checkpoint_transaction(MigrationState 
> *s,
>  goto out;
>  }
>  
> +colo_notify_compares_event(NULL, COLO_CHECKPOINT, _err);
> +if (local_err) {
> +goto out;
> +}
> +
>  /* Disable block migration */
>  s->params.blk = 0;
>  s->params.shared = 0;
> @@ -341,6 +350,11 @@ out:
>  return ret;
>  }
>  
> +static void colo_compare_notify_checkpoint(Notifier *notifier, void *data)
> +{
> +colo_checkpoint_notify(data);
> +}
> +
>  static void colo_process_checkpoint(MigrationState *s)
>  {
>  QIOChannelBuffer *bioc;
> @@ -357,6 +371,9 @@ static void colo_process_checkpoint(MigrationState *s)
>  goto out;
>  }
>  
> +packets_compare_notifier.notify = colo_compare_notify_checkpoint;
> +colo_compare_register_notifier(_compare_notifier);
> +
>  /*
>   * Wait for Secondary finish loading VM states and enter COLO
>   * restore.
> @@ -402,6 +419,7 @@ out:
>  qemu_fclose(fb);
>  }
>  
> +colo_compare_unregister_notifier(_compare_notifier);
>  timer_del(s->colo_delay_timer);
>  
>  /* Hope this not to be too long to wait here */
> @@ -518,6 +536,11 @@ void *colo_process_incoming_thread(void *opaque)
>  goto out;
>  }
>  
> +qemu_mutex_lock_iothread();
> +vm_stop_force_state(RUN_STATE_COLO);
> +trace_colo_vm_state_change("run", "stop");
> +qemu_mutex_unlock_iothread();
> +
>  /* FIXME: This is unnecessary for periodic checkpoint mode */
>  colo_send_message(mis->to_src_file, COLO_MESSAGE_CHECKPOINT_REPLY,
>   _err);
> @@ -571,6 +594,8 @@ void *colo_process_incoming_thread(void *opaque)
>  }
>  
>  vmstate_loading = false;
> +vm_start();
> +trace_colo_vm_state_change("stop", "run");
>  qemu_mutex_unlock_iothread();
>  
>  if (failover_get_state() == FAILOVER_STATUS_RELAUNCH) {
> diff --git a/migration/migration.c b/migration/migration.c
> index c6ae69d..2339be7 100644
> --- a/migration/migration.c
> +++ b/migration/migration.c
> @@ -66,7 +66,7 @@
>  /* The delay time (in ms) between two COLO checkpoints
>   * Note: Please change this default value to 1 when we support hybrid 
> mode.
>   */
> -#define DEFAULT_MIGRATE_X_CHECKPOINT_DELAY 200
> +#define DEFAULT_MIGRATE_X_CHECKPOINT_DELAY (200 * 100)
>  
>  static NotifierList migration_state_notifiers =
>  NOTIFIER_LIST_INITIALIZER(migration_state_notifiers);
> -- 
> 1.8.3.1
> 
> 
--
Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK

Re: [Qemu-devel] [RFC PATCH 4/4] nvdimm acpi: build flush hint address structure if required

[Dan Williams]

On Thu, Apr 6, 2017 at 3:13 AM, Stefan Hajnoczi  wrote:
> On Fri, Mar 31, 2017 at 04:41:47PM +0800, Haozhong Zhang wrote:
>>
>> Add an boolean option 'flush-hint' to device 'nvdimm'. If it's on, a
>> flush hint address structure will be constructed for each nvdimm
>> device.
>
> Users should not need to set the flush hint option.  NVDIMM
> configurations that persist data properly without Flush Hint Addresses
> shouldn't use them (for best performance).  Configurations that rely on
> flush hints *must* use them to guarantee data integrity.
>
> I don't remember if there's a way to detect the configuration from host
> userspace, but we should focus on setting this correctly without
> requiring users to know which setting is necessary.
>
>> Signed-off-by: Haozhong Zhang 
>> ---
>>  hw/acpi/nvdimm.c| 106 
>> +---
>>  hw/i386/pc.c|   5 ++-
>>  hw/mem/nvdimm.c |  26 
>>  include/hw/mem/nvdimm.h |   2 +-
>>  4 files changed, 132 insertions(+), 7 deletions(-)
>>
>> diff --git a/hw/acpi/nvdimm.c b/hw/acpi/nvdimm.c
>> index ea2ac3e..a7ff0b2 100644
>> --- a/hw/acpi/nvdimm.c
>> +++ b/hw/acpi/nvdimm.c
>> @@ -32,6 +32,8 @@
>>  #include "hw/acpi/bios-linker-loader.h"
>>  #include "hw/nvram/fw_cfg.h"
>>  #include "hw/mem/nvdimm.h"
>> +#include "exec/address-spaces.h"
>> +#include "qapi/error.h"
>>
>>  static int nvdimm_device_list(Object *obj, void *opaque)
>>  {
>> @@ -168,6 +170,22 @@ struct NvdimmNfitControlRegion {
>>  typedef struct NvdimmNfitControlRegion NvdimmNfitControlRegion;
>>
>>  /*
>> + * NVDIMM Flush Hint Address Structure
>> + *
>> + * It enables the data durability mechanism via ACPI.
>> + */
>> +struct NvdimmNfitFlushHintAddress {
>> +uint16_t type;
>> +uint16_t length;
>> +uint32_t nfit_handle;
>> +uint16_t nr_flush_hint_addr;
>> +uint8_t  reserved[6];
>> +#define NR_FLUSH_HINT_ADDR 1
>> +uint64_t flush_hint_addr[NR_FLUSH_HINT_ADDR];
>
> How should the number of flush hints be set?  This patch hardcodes it to
> 1 but the Linux nvdimm drivers tries to balance between flush hint
> addresses to improve performance (to prevent cache line bouncing?).

No, 1 should be fine for qemu. The reason for multiple flush hints is
to accommodate multiple flush queues in the hardware. Since each flush
takes some amount of time you can get more parallelism  if multiple
threads are waiting for the same flush event, rather than being queued
serially. I don't think you can realize the same parallelism with
fsync() calls.

[Qemu-devel] [PATCH v3 11/12] cpus: call cpu_update_icount on read

[Alex Bennée]

This ensures each time the vCPU thread reads the icount we update the
master timer_state.qemu_icount field. This way as long as updates are
in BQL protected sections (which they should be) the main-loop can
never come to update the log and find time has gone backwards.

Signed-off-by: Alex Bennée 
---
 cpus.c | 10 ++
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/cpus.c b/cpus.c
index 14c2149987..a6a85bfdba 100644
--- a/cpus.c
+++ b/cpus.c
@@ -253,19 +253,21 @@ void cpu_update_icount(CPUState *cpu)
 
 int64_t cpu_get_icount_raw(void)
 {
-int64_t icount;
 CPUState *cpu = current_cpu;
 
-icount = atomic_read(_state.qemu_icount);
 if (cpu && cpu->running) {
 if (!cpu->can_do_io) {
 fprintf(stderr, "Bad icount read\n");
 exit(1);
 }
 /* Take into account what has run */
-icount += cpu_get_icount_executed(cpu);
+cpu_update_icount(cpu);
 }
-return icount;
+#ifdef CONFIG_ATOMIC64
+return atomic_read__nocheck(_state.qemu_icount);
+#else /* FIXME: we need 64bit atomics to do this safely */
+return timers_state.qemu_icount;
+#endif
 }
 
 /* Return the virtual CPU time, based on the instruction counter.  */
-- 
2.11.0

Re: [Qemu-devel] [Qemu-block] [PATCH v2 for-2.10 13/16] block/qcow2: qcow2_calc_size_usage() for truncate

[Max Reitz]

On 06.04.2017 15:04, Stefan Hajnoczi wrote:
> On Mon, Apr 03, 2017 at 06:09:33PM +0200, Max Reitz wrote:
>> +BDRVQcow2State *s = bs->opaque;
>> +uint64_t aligned_cur_size = align_offset(current_size, 
>> cluster_size);
>> +uint64_t creftable_length;
>> +uint64_t i;
>> +
>> +/* new total size of L2 tables */
>> +nl2e = aligned_total_size / cluster_size;
>> +nl2e = align_offset(nl2e, cluster_size / sizeof(uint64_t));
>> +meta_size += nl2e * sizeof(uint64_t);
>> +
>> +/* Subtract L2 tables which are already present */
>> +for (i = 0; i < s->l1_size; i++) {
>> +if (s->l1_table[i] & L1E_OFFSET_MASK) {
>> +meta_size -= cluster_size;
>> +}
>> +}
> 
> Allocated L1 table entries are 'A', unallocated L1 table entries in
> the existing image are '-', and new L1 table entries after growth are
> '+':
> 
>   |-A-AAA--+|
> 
> This for loop includes '-' entries.  Should we count them or just the
> '+' entries?

Hm, good question, thanks. I suppose we should count them if we wanted
to fully allocate the image now. But that's not the intention, we just
want to fully allocate the new area.

While you can make the calculation faster if you take that into account,
it also gets a bit more complicated: We can subtract all L2 tables that
do not point to the new area. But there may be L2 tables already which
do point there which we therefore do not have to allocate. So we'd still
need to do this loop, but the starting index would be the first L2 table
index to point into the area.

So the above loop may calculate more space to be required than actually
is the case; I could argue that's fine because this function may
overcommit (especially if the image wasn't fully allocated before). But
modifying it to ignore all L2 tables before the new area doesn't seem to
be too complicated, so I'll do it.

>> -/* total size of refcount tables */
>> -nreftablee = nrefblocke / refblock_size;
>> -nreftablee = align_offset(nreftablee, cluster_size / sizeof(uint64_t));
>> -meta_size += nreftablee * sizeof(uint64_t);
>> +/* Do not add L1 table size because the only caller of this path
>> + * (qcow2_truncate) has increased its size already. */
>>  
>> -return aligned_total_size + meta_size;
>> +/* Calculate size of the additional refblocks (this assumes that 
>> all of
>> + * the existing image is covered by refblocks, which is extremely
>> + * likely); this may result in overallocation because parts of the 
>> newly
>> + * added space may be covered by existing refblocks, but that is 
>> fine.
>> + *
>> + * This only considers the newly added space. Since we cannot 
>> update the
>> + * reftable in-place, we will have to able to store both the old 
>> and the
>> + * new one at the same time, though. Therefore, we need to add the 
>> size
>> + * of the old reftable here.
>> + */
>> +creftable_length = ROUND_UP(s->refcount_table_size * 
>> sizeof(uint64_t),
>> +cluster_size);
>> +nrefblocke = ((aligned_total_size - aligned_cur_size) + meta_size +
>> +  creftable_length + cluster_size)
>> +   / (cluster_size - rces -
>> +  rces * sizeof(uint64_t) / cluster_size);
>> +meta_size += DIV_ROUND_UP(nrefblocke, refblock_size) * cluster_size;
>> +
>> +/* total size of the new refcount table (again, may be too much 
>> because
>> + * it assumes that the new area is not covered by any refcount 
>> blocks
>> + * yet) */
>> +nreftablee = s->max_refcount_table_index + 1 +
>> + nrefblocke / refblock_size;
>> +nreftablee = align_offset(nreftablee, cluster_size / 
>> sizeof(uint64_t));
>> +meta_size += nreftablee * sizeof(uint64_t);
>> +
>> +return (aligned_total_size - aligned_cur_size) + meta_size;
> 
> This calculation is an approximation.  Here is a simpler approximation:
> 
>   current_usage = qcow2_calc_size_usage(current_size, ...);
>   new_usage = qcow2_calc_size_usage(new_size, ...);
>   delta = new_usage - current_usage;
> 
> (Perhaps the new refcount_table clusters need to be added to new_size
> too.)
> 
> Is there an advantage of the more elaborate calculation implemented in
> this patch?

Now that you mention it... Well, the important thing is it's a
conservative approximation. It's supposed to never underestimate the
correct result.

Now the existing image doesn't need to be fully allocated. However, your
snippet assumes that it is. Often, this actually wouldn't be an issue.
But it is for clusters that are "shared" between the existing image and
the new area:

1. The old final L2 table may point into the new area. Your code assumes
it is already present but it may not be.

2. current_size need not be aligned to clusters. 

Re: [Qemu-devel] [PATCH] vhost: skip RAM device memory sections

[Michael S. Tsirkin]

On Sat, Apr 08, 2017 at 09:24:10AM +0800, ZhiPeng Lu wrote:
> A RAM device represents a mapping to a physical device, such as to a PCI
> * MMIO BAR of an vfio-pci assigned device.
> Vhost listens to this region,and increases the region's reference count
> while passthrough?for?network adapters (Physical Function, PF or Virtual 
> Function, VF).
> After detaching   network adapters with  vhost backend dirver or vhost user 
> dirver,
> it unregister vhost listen function  by memory_listener_unregister.

Shouldn't that drop all references? That might be a cleaner fix.

> After detaching the passthrough pf  or vf,
> the RAM device region's reference by  vhost listener increated can not be 
> released,
> due to vhost listen function does not exist.So let's just skip RAM device 
> memory.
> 
> Signed-off-by: ZhiPeng Lu 
> ---
>  hw/virtio/vhost.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c
> index 613494d..c1ff98f 100644
> --- a/hw/virtio/vhost.c
> +++ b/hw/virtio/vhost.c
> @@ -611,7 +611,8 @@ static void vhost_set_memory(MemoryListener *listener,
>  static bool vhost_section(MemoryRegionSection *section)
>  {
>  return memory_region_is_ram(section->mr) &&
> -!memory_region_is_rom(section->mr);
> +!memory_region_is_rom(section->mr) &&
> +!memory_region_is_skip_dump(section->mr);
>  }
>  
>  static void vhost_begin(MemoryListener *listener)
> -- 
> 1.8.3.1
> 

[Qemu-devel] [PATCH v3 07/12] cpus: move icount preparation out of tcg_exec_cpu

[Alex Bennée]

As icount is only supported for single-threaded execution due to the
requirement for determinism let's remove it from the common
tcg_exec_cpu path.

Also remove the additional fiddling which shouldn't be required as the
icount counters should all be rectified as you enter the loop.

Signed-off-by: Alex Bennée 

---
v2
  - only clear u16.low
  - drop BQL assert
---
 cpus.c | 67 --
 1 file changed, 45 insertions(+), 22 deletions(-)

diff --git a/cpus.c b/cpus.c
index 18b1746770..d9cb9407a2 100644
--- a/cpus.c
+++ b/cpus.c
@@ -1179,47 +1179,64 @@ static void handle_icount_deadline(void)
 }
 }
 
-static int tcg_cpu_exec(CPUState *cpu)
+static void prepare_icount_for_run(CPUState *cpu)
 {
-int ret;
-#ifdef CONFIG_PROFILER
-int64_t ti;
-#endif
-
-#ifdef CONFIG_PROFILER
-ti = profile_getclock();
-#endif
 if (use_icount) {
 int64_t count;
 int decr;
-timers_state.qemu_icount -= (cpu->icount_decr.u16.low
-+ cpu->icount_extra);
-cpu->icount_decr.u16.low = 0;
-cpu->icount_extra = 0;
+
+/* These should always be cleared by process_icount_data after
+ * each vCPU execution. However u16.high can be raised
+ * asynchronously by cpu_exit/cpu_interrupt/tcg_handle_interrupt
+ */
+g_assert(cpu->icount_decr.u16.low == 0);
+g_assert(cpu->icount_extra == 0);
+
+
 count = tcg_get_icount_limit();
+
 timers_state.qemu_icount += count;
 decr = (count > 0x) ? 0x : count;
 count -= decr;
 cpu->icount_decr.u16.low = decr;
 cpu->icount_extra = count;
 }
-qemu_mutex_unlock_iothread();
-cpu_exec_start(cpu);
-ret = cpu_exec(cpu);
-cpu_exec_end(cpu);
-qemu_mutex_lock_iothread();
-#ifdef CONFIG_PROFILER
-tcg_time += profile_getclock() - ti;
-#endif
+}
+
+static void process_icount_data(CPUState *cpu)
+{
 if (use_icount) {
 /* Fold pending instructions back into the
instruction counter, and clear the interrupt flag.  */
 timers_state.qemu_icount -= (cpu->icount_decr.u16.low
 + cpu->icount_extra);
-cpu->icount_decr.u32 = 0;
+
+/* Reset the counters */
+cpu->icount_decr.u16.low = 0;
 cpu->icount_extra = 0;
 replay_account_executed_instructions();
 }
+}
+
+
+static int tcg_cpu_exec(CPUState *cpu)
+{
+int ret;
+#ifdef CONFIG_PROFILER
+int64_t ti;
+#endif
+
+#ifdef CONFIG_PROFILER
+ti = profile_getclock();
+#endif
+qemu_mutex_unlock_iothread();
+cpu_exec_start(cpu);
+ret = cpu_exec(cpu);
+cpu_exec_end(cpu);
+qemu_mutex_lock_iothread();
+#ifdef CONFIG_PROFILER
+tcg_time += profile_getclock() - ti;
+#endif
 return ret;
 }
 
@@ -1306,7 +1323,13 @@ static void *qemu_tcg_rr_cpu_thread_fn(void *arg)
 
 if (cpu_can_run(cpu)) {
 int r;
+
+prepare_icount_for_run(cpu);
+
 r = tcg_cpu_exec(cpu);
+
+process_icount_data(cpu);
+
 if (r == EXCP_DEBUG) {
 cpu_handle_guest_debug(cpu);
 break;
-- 
2.11.0

[Qemu-devel] [PATCH v3 09/12] cpus: introduce cpu_update_icount helper

[Alex Bennée]

By holding off updates to timer_state.qemu_icount we can run into
trouble when the non-vCPU thread needs to know the time. This helper
ensures we atomically update timers_state.qemu_icount based on what
has been currently executed.

Signed-off-by: Alex Bennée 
---
 cpus.c   | 23 +--
 include/qemu/timer.h |  1 +
 2 files changed, 22 insertions(+), 2 deletions(-)

diff --git a/cpus.c b/cpus.c
index 88eabdc19f..3854f17c35 100644
--- a/cpus.c
+++ b/cpus.c
@@ -232,12 +232,31 @@ static int64_t cpu_get_icount_executed(CPUState *cpu)
 return cpu->icount_budget - (cpu->icount_decr.u16.low + cpu->icount_extra);
 }
 
+/*
+ * Update the global shared timer_state.qemu_icount to take into
+ * account executed instructions. This is done by the TCG vCPU
+ * thread so the main-loop can see time has moved forward.
+ */
+void cpu_update_icount(CPUState *cpu)
+{
+int64_t executed = cpu_get_icount_executed(cpu);
+cpu->icount_budget -= executed;
+
+#ifdef CONFIG_ATOMIC64
+atomic_set__nocheck(_state.qemu_icount,
+atomic_read__nocheck(_state.qemu_icount) +
+executed);
+#else /* FIXME: we need 64bit atomics to do this safely */
+timers_state.qemu_icount += executed;
+#endif
+}
+
 int64_t cpu_get_icount_raw(void)
 {
 int64_t icount;
 CPUState *cpu = current_cpu;
 
-icount = timers_state.qemu_icount;
+icount = atomic_read(_state.qemu_icount);
 if (cpu && cpu->running) {
 if (!cpu->can_do_io) {
 fprintf(stderr, "Bad icount read\n");
@@ -1220,7 +1239,7 @@ static void process_icount_data(CPUState *cpu)
 {
 if (use_icount) {
 /* Account for executed instructions */
-timers_state.qemu_icount += cpu_get_icount_executed(cpu);
+cpu_update_icount(cpu);
 
 /* Reset the counters */
 cpu->icount_decr.u16.low = 0;
diff --git a/include/qemu/timer.h b/include/qemu/timer.h
index e1742f2f3d..8a1eb74839 100644
--- a/include/qemu/timer.h
+++ b/include/qemu/timer.h
@@ -869,6 +869,7 @@ int64_t cpu_get_icount_raw(void);
 int64_t cpu_get_icount(void);
 int64_t cpu_get_clock(void);
 int64_t cpu_icount_to_ns(int64_t icount);
+voidcpu_update_icount(CPUState *cpu);
 
 /***/
 /* host CPU ticks (if available) */
-- 
2.11.0

Re: [Qemu-devel] [PATCH 01/15] net/colo: Add notifier/callback related helpers for filter

[Dr. David Alan Gilbert]

* zhanghailiang (zhang.zhanghaili...@huawei.com) wrote:
> We will use this notifier to help COLO to notify filter object
> to do something, like do checkpoint, or process failover event.
> 
> Cc: Jason Wang 
> Signed-off-by: zhanghailiang 
> ---
>  net/colo.c | 92 
> ++
>  net/colo.h | 18 
>  2 files changed, 110 insertions(+)
> 

<..>

> +FilterNotifier *filter_noitifier_new(FilterNotifierCallback *cb,
  ^ Typo - no*i*tifier

(I've not looked at this patch much, I'll leave networking stuff to Jason)

Dave

> +void *opaque, Error **errp)
> +{
> +FilterNotifier *notify;
> +int ret;
> +
> +notify = (FilterNotifier *)g_source_new(_source_funcs,
> +sizeof(FilterNotifier));
> +ret = event_notifier_init(>event, false);
> +if (ret < 0) {
> +error_setg_errno(errp, -ret, "Failed to initialize event notifier");
> +goto fail;
> +}
> +notify->pfd.fd = event_notifier_get_fd(>event);
> +notify->pfd.events = G_IO_IN | G_IO_HUP | G_IO_ERR;
> +notify->cb = cb;
> +notify->opaque = opaque;
> +g_source_add_poll(>source, >pfd);
> +
> +return notify;
> +
> +fail:
> +g_source_destroy(>source);
> +return NULL;
> +}
> +
> +int filter_notifier_set(FilterNotifier *notify, uint64_t value)
> +{
> +ssize_t ret;
> +
> +do {
> +ret = write(notify->event.wfd, , sizeof(value));
> +} while (ret < 0 && errno == EINTR);
> +
> +/* EAGAIN is fine, a read must be pending.  */
> +if (ret < 0 && errno != EAGAIN) {
> +return -errno;
> +}
> +return 0;
> +}
> diff --git a/net/colo.h b/net/colo.h
> index cd9027f..00f03b5 100644
> --- a/net/colo.h
> +++ b/net/colo.h
> @@ -19,6 +19,7 @@
>  #include "qemu/jhash.h"
>  #include "qemu/timer.h"
>  #include "slirp/tcp.h"
> +#include "qemu/event_notifier.h"
>  
>  #define HASHTABLE_MAX_SIZE 16384
>  
> @@ -89,4 +90,21 @@ void connection_hashtable_reset(GHashTable 
> *connection_track_table);
>  Packet *packet_new(const void *data, int size);
>  void packet_destroy(void *opaque, void *user_data);
>  
> +typedef void FilterNotifierCallback(void *opaque, int value);
> +typedef struct FilterNotifier {
> +GSource source;
> +EventNotifier event;
> +GPollFD pfd;
> +FilterNotifierCallback *cb;
> +void *opaque;
> +} FilterNotifier;
> +
> +FilterNotifier *filter_noitifier_new(FilterNotifierCallback *cb,
> +void *opaque, Error **errp);
> +int filter_notifier_set(FilterNotifier *notify, uint64_t value);
> +
> +enum {
> +COLO_CHECKPOINT = 2,
> +COLO_FAILOVER,
> +};
>  #endif /* QEMU_COLO_PROXY_H */
> -- 
> 1.8.3.1
> 
> 
> 
--
Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK

[Qemu-devel] [PATCH v3 03/12] scripts/replay-dump.py: replay log dumper

[Alex Bennée]

This script is a debugging tool for looking through the contents of a
replay log file. It is incomplete but should fail gracefully at events
it doesn't understand.

It currently understands two different log formats as the audio
record/replay support was merged during since MTTCG. It was written to
help debug what has caused the BQL changes to break replay support.

Signed-off-by: Alex Bennée 
---
 scripts/replay-dump.py | 272 +
 1 file changed, 272 insertions(+)
 create mode 100755 scripts/replay-dump.py

diff --git a/scripts/replay-dump.py b/scripts/replay-dump.py
new file mode 100755
index 00..fdd178aba0
--- /dev/null
+++ b/scripts/replay-dump.py
@@ -0,0 +1,272 @@
+#!/usr/bin/env python
+# -*- coding: utf-8 -*-
+#
+# Dump the contents of a recorded execution stream
+#
+#  Copyright (c) 2017 Alex Bennée 
+#
+# This library is free software; you can redistribute it and/or
+# modify it under the terms of the GNU Lesser General Public
+# License as published by the Free Software Foundation; either
+# version 2 of the License, or (at your option) any later version.
+#
+# This library is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+# Lesser General Public License for more details.
+#
+# You should have received a copy of the GNU Lesser General Public
+# License along with this library; if not, see .
+
+import argparse
+import struct
+from collections import namedtuple
+
+# This mirrors some of the global replay state which some of the
+# stream loading refers to. Some decoders may read the next event so
+# we need handle that case. Calling reuse_event will ensure the next
+# event is read from the cache rather than advancing the file.
+
+class ReplayState(object):
+def __init__(self):
+self.event = -1
+self.event_count = 0
+self.already_read = False
+self.current_checkpoint = 0
+self.checkpoint = 0
+
+def set_event(self, ev):
+self.event = ev
+self.event_count += 1
+
+def get_event(self):
+self.already_read = False
+return self.event
+
+def reuse_event(self, ev):
+self.event = ev
+self.already_read = True
+
+def set_checkpoint(self):
+self.checkpoint = self.event - self.checkpoint_start
+
+def get_checkpoint(self):
+return self.checkpoint
+
+replay_state = ReplayState()
+
+# Simple read functions that mirror replay-internal.c
+# The file-stream is big-endian and manually written out a byte at a time.
+
+def read_byte(fin):
+"Read a single byte"
+return struct.unpack('>B', fin.read(1))[0]
+
+def read_event(fin):
+"Read a single byte event, but save some state"
+if replay_state.already_read:
+return replay_state.get_event()
+else:
+replay_state.set_event(read_byte(fin))
+return replay_state.event
+
+def read_word(fin):
+"Read a 16 bit word"
+return struct.unpack('>H', fin.read(2))[0]
+
+def read_dword(fin):
+"Read a 32 bit word"
+return struct.unpack('>I', fin.read(4))[0]
+
+def read_qword(fin):
+"Read a 64 bit word"
+return struct.unpack('>Q', fin.read(8))[0]
+
+# Generic decoder structure
+Decoder = namedtuple("Decoder", "eid name fn")
+
+def call_decode(table, index, dumpfile):
+"Search decode table for next step"
+decoder = next((d for d in table if d.eid == index), None)
+if not decoder:
+print "Could not decode index: %d" % (index)
+print "Entry is: %s" % (decoder)
+print "Decode Table is:\n%s" % (table)
+return False
+else:
+return decoder.fn(decoder.eid, decoder.name, dumpfile)
+
+# Print event
+def print_event(eid, name, string=None, event_count=None):
+"Print event with count"
+if not event_count:
+event_count = replay_state.event_count
+
+if string:
+print "%d:%s(%d) %s" % (event_count, name, eid, string)
+else:
+print "%d:%s(%d)" % (event_count, name, eid)
+
+
+# Decoders for each event type
+
+def decode_unimp(eid, name, _unused_dumpfile):
+"Unimplimented decoder, will trigger exit"
+print "%s not handled - will now stop" % (name)
+return False
+
+# Checkpoint decoder
+def swallow_async_qword(eid, name, dumpfile):
+"Swallow a qword of data without looking at it"
+step_id = read_qword(dumpfile)
+print "  %s(%d) @ %d" % (name, eid, step_id)
+return True
+
+async_decode_table = [ Decoder(0, "REPLAY_ASYNC_EVENT_BH", 
swallow_async_qword),
+   Decoder(1, "REPLAY_ASYNC_INPUT", decode_unimp),
+   Decoder(2, "REPLAY_ASYNC_INPUT_SYNC", decode_unimp),
+   Decoder(3, "REPLAY_ASYNC_CHAR_READ", decode_unimp),
+   Decoder(4, 

[Qemu-devel] [PATCH v3 06/12] cpus: check cpu->running in cpu_get_icount_raw()

[Alex Bennée]

The lifetime of current_cpu is now the lifetime of the vCPU thread.
However get_icount_raw() can apply a fudge factor if called while code
is running to take into account the current executed instruction
count.

To ensure this is always the case we also check cpu->running.

Signed-off-by: Alex Bennée 
Reviewed-by: Philippe Mathieu-Daudé 
Reviewed-by: Richard Henderson 
---
 cpus.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/cpus.c b/cpus.c
index 4e48b9c4ad..18b1746770 100644
--- a/cpus.c
+++ b/cpus.c
@@ -229,7 +229,7 @@ int64_t cpu_get_icount_raw(void)
 CPUState *cpu = current_cpu;
 
 icount = timers_state.qemu_icount;
-if (cpu) {
+if (cpu && cpu->running) {
 if (!cpu->can_do_io) {
 fprintf(stderr, "Bad icount read\n");
 exit(1);
-- 
2.11.0

[Qemu-devel] [PATCH v3 08/12] cpus: don't credit executed instructions before they have run

[Alex Bennée]

Outside of the vCPU thread icount time will only be tracked against
timers_state.qemu_icount. We no longer credit cycles until they have
completed the run. Inside the vCPU thread we adjust for passage of
time by looking at how many have run so far. This is only valid inside
the vCPU thread while it is running.

Signed-off-by: Alex Bennée 
---
 cpus.c| 25 +++--
 include/qom/cpu.h |  1 +
 2 files changed, 20 insertions(+), 6 deletions(-)

diff --git a/cpus.c b/cpus.c
index d9cb9407a2..88eabdc19f 100644
--- a/cpus.c
+++ b/cpus.c
@@ -223,6 +223,15 @@ void qemu_tcg_configure(QemuOpts *opts, Error **errp)
 }
 }
 
+/* The current number of executed instructions is based on what we
+ * originally budgeted minus the current state of the decrementing
+ * icount counters in extra/u16.low.
+ */
+static int64_t cpu_get_icount_executed(CPUState *cpu)
+{
+return cpu->icount_budget - (cpu->icount_decr.u16.low + cpu->icount_extra);
+}
+
 int64_t cpu_get_icount_raw(void)
 {
 int64_t icount;
@@ -234,7 +243,8 @@ int64_t cpu_get_icount_raw(void)
 fprintf(stderr, "Bad icount read\n");
 exit(1);
 }
-icount -= (cpu->icount_decr.u16.low + cpu->icount_extra);
+/* Take into account what has run */
+icount += cpu_get_icount_executed(cpu);
 }
 return icount;
 }
@@ -1195,7 +1205,10 @@ static void prepare_icount_for_run(CPUState *cpu)
 
 count = tcg_get_icount_limit();
 
-timers_state.qemu_icount += count;
+/* To calculate what we have executed so far we need to know
+ * what we originally budgeted to run this cycle */
+cpu->icount_budget = count;
+
 decr = (count > 0x) ? 0x : count;
 count -= decr;
 cpu->icount_decr.u16.low = decr;
@@ -1206,14 +1219,14 @@ static void prepare_icount_for_run(CPUState *cpu)
 static void process_icount_data(CPUState *cpu)
 {
 if (use_icount) {
-/* Fold pending instructions back into the
-   instruction counter, and clear the interrupt flag.  */
-timers_state.qemu_icount -= (cpu->icount_decr.u16.low
-+ cpu->icount_extra);
+/* Account for executed instructions */
+timers_state.qemu_icount += cpu_get_icount_executed(cpu);
 
 /* Reset the counters */
 cpu->icount_decr.u16.low = 0;
 cpu->icount_extra = 0;
+cpu->icount_budget = 0;
+
 replay_account_executed_instructions();
 }
 }
diff --git a/include/qom/cpu.h b/include/qom/cpu.h
index c3292efe1c..5d10359c8f 100644
--- a/include/qom/cpu.h
+++ b/include/qom/cpu.h
@@ -332,6 +332,7 @@ struct CPUState {
 /* updates protected by BQL */
 uint32_t interrupt_request;
 int singlestep_enabled;
+int64_t icount_budget;
 int64_t icount_extra;
 sigjmp_buf jmp_env;
 
-- 
2.11.0

[Qemu-devel] [PATCH v3 12/12] replay: assert time only goes forward

[Alex Bennée]

If we find ourselves trying to add an event to the log where time has
gone backwards it is because a vCPU event has occurred and the
main-loop is not yet aware of time moving forward. This should not
happen and if it does its better to fail early than generate a log
that will have weird behaviour.

Signed-off-by: Alex Bennée 
---
 replay/replay-internal.c | 4 
 replay/replay.c  | 4 
 2 files changed, 8 insertions(+)

diff --git a/replay/replay-internal.c b/replay/replay-internal.c
index bea7b4aa6b..fca8514012 100644
--- a/replay/replay-internal.c
+++ b/replay/replay-internal.c
@@ -195,6 +195,10 @@ void replay_save_instructions(void)
 if (replay_file && replay_mode == REPLAY_MODE_RECORD) {
 replay_mutex_lock();
 int diff = (int)(replay_get_current_step() - 
replay_state.current_step);
+
+/* Time can only go forward */
+assert(diff >= 0);
+
 if (diff > 0) {
 replay_put_event(EVENT_INSTRUCTION);
 replay_put_dword(diff);
diff --git a/replay/replay.c b/replay/replay.c
index 9e0724e756..f810628cac 100644
--- a/replay/replay.c
+++ b/replay/replay.c
@@ -84,6 +84,10 @@ void replay_account_executed_instructions(void)
 if (replay_state.instructions_count > 0) {
 int count = (int)(replay_get_current_step()
   - replay_state.current_step);
+
+/* Time can only go forward */
+assert(count >= 0);
+
 replay_state.instructions_count -= count;
 replay_state.current_step += count;
 if (replay_state.instructions_count == 0) {
-- 
2.11.0

[Qemu-devel] [PATCH v3 04/12] target/i386/misc_helper: wrap BQL around another IRQ generator

[Alex Bennée]

Anything that calls into HW emulation must be protected by the BQL.

Signed-off-by: Alex Bennée 
Reviewed-by: Richard Henderson 
Acked-by: Eduardo Habkost 
---
 target/i386/misc_helper.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/target/i386/misc_helper.c b/target/i386/misc_helper.c
index ca2ea09f54..628f64aad5 100644
--- a/target/i386/misc_helper.c
+++ b/target/i386/misc_helper.c
@@ -18,6 +18,7 @@
  */
 
 #include "qemu/osdep.h"
+#include "qemu/main-loop.h"
 #include "cpu.h"
 #include "exec/helper-proto.h"
 #include "exec/exec-all.h"
@@ -156,7 +157,9 @@ void helper_write_crN(CPUX86State *env, int reg, 
target_ulong t0)
 break;
 case 8:
 if (!(env->hflags2 & HF2_VINTR_MASK)) {
+qemu_mutex_lock_iothread();
 cpu_set_apic_tpr(x86_env_get_cpu(env)->apic_state, t0);
+qemu_mutex_unlock_iothread();
 }
 env->v_tpr = t0 & 0x0f;
 break;
-- 
2.11.0

[Qemu-devel] [PATCH v3 02/12] scripts/qemu-gdb/timers.py: new helper to dump timer state

[Alex Bennée]

This introduces the qemu-gdb command "qemu timers" which will dump the
state of the main timers in the system.

Signed-off-by: Alex Bennée 
Reviewed-by: Philippe Mathieu-Daudé 
---
 scripts/qemu-gdb.py   |  3 ++-
 scripts/qemugdb/timers.py | 54 +++
 2 files changed, 56 insertions(+), 1 deletion(-)
 create mode 100644 scripts/qemugdb/timers.py

diff --git a/scripts/qemu-gdb.py b/scripts/qemu-gdb.py
index b3f8e04f77..3e7adb87dc 100644
--- a/scripts/qemu-gdb.py
+++ b/scripts/qemu-gdb.py
@@ -26,7 +26,7 @@ import os, sys
 
 sys.path.append(os.path.dirname(__file__))
 
-from qemugdb import aio, mtree, coroutine
+from qemugdb import aio, mtree, coroutine, timers
 
 class QemuCommand(gdb.Command):
 '''Prefix for QEMU debug support commands'''
@@ -38,6 +38,7 @@ QemuCommand()
 coroutine.CoroutineCommand()
 mtree.MtreeCommand()
 aio.HandlersCommand()
+timers.TimersCommand()
 
 coroutine.CoroutineSPFunction()
 coroutine.CoroutinePCFunction()
diff --git a/scripts/qemugdb/timers.py b/scripts/qemugdb/timers.py
new file mode 100644
index 00..be71a001e3
--- /dev/null
+++ b/scripts/qemugdb/timers.py
@@ -0,0 +1,54 @@
+#!/usr/bin/python
+# GDB debugging support
+#
+# Copyright 2017 Linaro Ltd
+#
+# Author: Alex Bennée 
+#
+# This work is licensed under the terms of the GNU GPL, version 2.  See
+# the COPYING file in the top-level directory.
+
+# 'qemu timers' -- display the current timerlists
+
+import gdb
+
+class TimersCommand(gdb.Command):
+'''Display the current QEMU timers'''
+
+def __init__(self):
+'Register the class as a gdb command'
+gdb.Command.__init__(self, 'qemu timers', gdb.COMMAND_DATA,
+ gdb.COMPLETE_NONE)
+
+def dump_timers(self, timer):
+"Follow a timer and recursively dump each one in the list."
+# timer should be of type QemuTimer
+gdb.write("timer %s/%s (cb:%s,opq:%s)\n" % (
+timer['expire_time'],
+timer['scale'],
+timer['cb'],
+timer['opaque']))
+
+if int(timer['next']) > 0:
+self.dump_timers(timer['next'])
+
+
+def process_timerlist(self, tlist, ttype):
+gdb.write("Processing %s timers\n" % (ttype))
+gdb.write("  clock %s is enabled:%s, last:%s\n" % (
+tlist['clock']['type'],
+tlist['clock']['enabled'],
+tlist['clock']['last']))
+if int(tlist['active_timers']) > 0:
+self.dump_timers(tlist['active_timers'])
+
+
+def invoke(self, arg, from_tty):
+'Run the command'
+main_timers = gdb.parse_and_eval("main_loop_tlg")
+
+# This will break if QEMUClockType in timer.h is redfined
+self.process_timerlist(main_timers['tl'][0], "Realtime")
+self.process_timerlist(main_timers['tl'][1], "Virtual")
+self.process_timerlist(main_timers['tl'][2], "Host")
+self.process_timerlist(main_timers['tl'][3], "Virtual RT")
-- 
2.11.0

[Qemu-devel] [PATCH v3 00/12] icount and misc MTTCG fixes for 2.9-rc4

[Alex Bennée]

Hi,

This doesn't change much from v2 earlier this week:

  https://lists.nongnu.org/archive/html/qemu-devel/2017-04/msg00661.html

There was a minor improvement to the cpu_update_icount to not use a
potentially expensive atomic_add (which is CST) when you can get away
with weaker semantics with a single writer.

I've also fixed a compile error on 32bit guests. It only falls back to
non-atomic code if CONFIG_ATOMIC64 is not set which isn't the case for
ARMv7 as it can use load/store-exclusive pair to do the operation.
This does require using the __no_check atomic variants though.

I still plan to roll a pull-request on Monday and get it sent to Peter
in good time for cutting -rc4.

Cheers,


Alex Bennée (12):
  scripts/qemugdb/mtree.py: fix up mtree dump
  scripts/qemu-gdb/timers.py: new helper to dump timer state
  scripts/replay-dump.py: replay log dumper
  target/i386/misc_helper: wrap BQL around another IRQ generator
  cpus: remove icount handling from qemu_tcg_cpu_thread_fn
  cpus: check cpu->running in cpu_get_icount_raw()
  cpus: move icount preparation out of tcg_exec_cpu
  cpus: don't credit executed instructions before they have run
  cpus: introduce cpu_update_icount helper
  cpu-exec: update icount after each TB_EXIT
  cpus: call cpu_update_icount on read
  replay: assert time only goes forward

 cpu-exec.c|  14 +--
 cpus.c| 109 ++-
 include/qemu/timer.h  |   1 +
 include/qom/cpu.h |   1 +
 replay/replay-internal.c  |   4 +
 replay/replay.c   |   4 +
 scripts/qemu-gdb.py   |   3 +-
 scripts/qemugdb/mtree.py  |  12 +-
 scripts/qemugdb/timers.py |  54 +
 scripts/replay-dump.py| 272 ++
 target/i386/misc_helper.c |   3 +
 11 files changed, 437 insertions(+), 40 deletions(-)
 create mode 100644 scripts/qemugdb/timers.py
 create mode 100755 scripts/replay-dump.py

-- 
2.11.0

[Qemu-devel] [PATCH v3 01/12] scripts/qemugdb/mtree.py: fix up mtree dump

[Alex Bennée]

Since QEMU has been able to build with native Int128 support this was
broken as it attempts to fish values out of the non-existent
structure. Also the alias print was trying to make a %x out of
gdb.ValueType directly which didn't seem to work.

Signed-off-by: Alex Bennée 
---
 scripts/qemugdb/mtree.py | 12 ++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/scripts/qemugdb/mtree.py b/scripts/qemugdb/mtree.py
index cc8131c2e7..e6791b7885 100644
--- a/scripts/qemugdb/mtree.py
+++ b/scripts/qemugdb/mtree.py
@@ -21,7 +21,15 @@ def isnull(ptr):
 return ptr == gdb.Value(0).cast(ptr.type)
 
 def int128(p):
-return int(p['lo']) + (int(p['hi']) << 64)
+'''Read an Int128 type to a python integer.
+
+QEMU can be built with native Int128 support so we need to detect
+if the value is a structure or the native type.
+'''
+if p.type.code == gdb.TYPE_CODE_STRUCT:
+return int(p['lo']) + (int(p['hi']) << 64)
+else:
+return int(("%s" % p), 16)
 
 class MtreeCommand(gdb.Command):
 '''Display the memory tree hierarchy'''
@@ -69,7 +77,7 @@ class MtreeCommand(gdb.Command):
 gdb.write('%salias: %s@%016x (@ %s)\n' %
   ('  ' * level,
alias['name'].string(),
-   ptr['alias_offset'],
+   int(ptr['alias_offset']),
alias,
),
   gdb.STDOUT)
-- 
2.11.0

[Qemu-devel] [PATCH v3 10/12] cpu-exec: update icount after each TB_EXIT

[Alex Bennée]

There is no particular reason we shouldn't update the global system
icount time as we exit each TranslationBlock run. This ensures the
main-loop doesn't have to wait until we exit to the outer loop for
executed instructions to be credited to timer_state.

The prepare_icount_for_run function is slightly tweaked to match the
logic we run in cpu_loop_exec_tb.

Based on Paolo's original suggestion.

Signed-off-by: Alex Bennée 
---
 cpu-exec.c | 14 +++---
 cpus.c | 18 +-
 2 files changed, 12 insertions(+), 20 deletions(-)

diff --git a/cpu-exec.c b/cpu-exec.c
index 748cb66bca..63a56d0407 100644
--- a/cpu-exec.c
+++ b/cpu-exec.c
@@ -600,13 +600,13 @@ static inline void cpu_loop_exec_tb(CPUState *cpu, 
TranslationBlock *tb,
 /* Instruction counter expired.  */
 assert(use_icount);
 #ifndef CONFIG_USER_ONLY
-if (cpu->icount_extra) {
-/* Refill decrementer and continue execution.  */
-cpu->icount_extra += insns_left;
-insns_left = MIN(0x, cpu->icount_extra);
-cpu->icount_extra -= insns_left;
-cpu->icount_decr.u16.low = insns_left;
-} else {
+/* Ensure global icount has gone forward */
+cpu_update_icount(cpu);
+/* Refill decrementer and continue execution.  */
+insns_left = MIN(0x, cpu->icount_budget);
+cpu->icount_decr.u16.low = insns_left;
+cpu->icount_extra = cpu->icount_budget - insns_left;
+if (!cpu->icount_extra) {
 /* Execute any remaining instructions, then let the main loop
  * handle the next event.
  */
diff --git a/cpus.c b/cpus.c
index 3854f17c35..14c2149987 100644
--- a/cpus.c
+++ b/cpus.c
@@ -1211,8 +1211,7 @@ static void handle_icount_deadline(void)
 static void prepare_icount_for_run(CPUState *cpu)
 {
 if (use_icount) {
-int64_t count;
-int decr;
+int insns_left;
 
 /* These should always be cleared by process_icount_data after
  * each vCPU execution. However u16.high can be raised
@@ -1221,17 +1220,10 @@ static void prepare_icount_for_run(CPUState *cpu)
 g_assert(cpu->icount_decr.u16.low == 0);
 g_assert(cpu->icount_extra == 0);
 
-
-count = tcg_get_icount_limit();
-
-/* To calculate what we have executed so far we need to know
- * what we originally budgeted to run this cycle */
-cpu->icount_budget = count;
-
-decr = (count > 0x) ? 0x : count;
-count -= decr;
-cpu->icount_decr.u16.low = decr;
-cpu->icount_extra = count;
+cpu->icount_budget = tcg_get_icount_limit();
+insns_left = MIN(0x, cpu->icount_budget);
+cpu->icount_decr.u16.low = insns_left;
+cpu->icount_extra = cpu->icount_budget - insns_left;
 }
 }
 
-- 
2.11.0