Re: [Qemu-devel] Different type of qcow2_get_cluster_type

[lampahome]

>
>
> Both values correspond to L2 entries with bit 0 set.  However,
> QCOW2_CLUSTER_ZERO_ALLOC is an entry that has a non-zero value in bits 9-55
> (the cluster has an allocated host location, we guarantee that things read
> as zero regardless of whether the host data actually contains zeroes at
> that offset, and writes go directly to that offset with no further
> allocation required); while QCOW2_CLUSTER_ZERO_PLAIN is an entry with all
> zeros in bits 9-55 (we guarantee things read as zero, but writes have to
> allocate a new cluster because we have not reserved any space in the host
> yet).
>

If I let one entry called l2_addr of l2 table is 1(also the
QCOW2_CLUSTER_ZERO_PLAIN)
to make it as discard.

After I run qemu-img commit image, and the l2_addr also commit to its
backing file.

But I saw the same entry l2_addr of l2 table in backing file doesn't show
1, and write corresponding cluster with zero.

Is that normal?

[Qemu-devel] [PATCH] hvf: drop unused variable

[Paolo Bonzini]

Signed-off-by: Paolo Bonzini 
---
 target/i386/hvf/hvf.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/target/i386/hvf/hvf.c b/target/i386/hvf/hvf.c
index 5db167df98..9f52bc413a 100644
--- a/target/i386/hvf/hvf.c
+++ b/target/i386/hvf/hvf.c
@@ -72,7 +72,6 @@
 #include "sysemu/sysemu.h"
 #include "target/i386/cpu.h"
 
-pthread_rwlock_t mem_lock = PTHREAD_RWLOCK_INITIALIZER;
 HVFState *hvf_state;
 int hvf_disabled = 1;
 
-- 
2.17.1

[Qemu-devel] [PATCH] MAINTAINERS: add myself as elf2dmp maintainer

[Viktor Prutyanov]

Add myself as contrib/elf2dmp maintainer and elf2dmp as maintained.

Signed-off-by: Viktor Prutyanov 
---
 MAINTAINERS | 5 +
 1 file changed, 5 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index d12518c08f..e70ff1e009 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1888,6 +1888,11 @@ S: Maintained
 F: include/qemu/iova-tree.h
 F: util/iova-tree.c
 
+elf2dmp
+M: Viktor Prutyanov 
+S: Maintained
+F: contrib/elf2dmp/
+
 Usermode Emulation
 --
 Overall
-- 
2.14.3

Re: [Qemu-devel] [PATCH 0/3] aio-posix: polling mode bug fixes

[Fam Zheng]

On Wed, 09/12 19:10, Paolo Bonzini wrote:
> Patch 1 fixes a too-strict assertion that could fire when aio_poll
> is called in parallel with aio_set_fd_handler.
> 
> Patch 2 and 3 reinstate the performance benefits of polling, which were
> essentially disabled by commit 70232b5253 ("aio-posix: Don't count
> ctx->notifier as progress when polling", 2018-08-15).
> 
> Please review carefully! :)

Queued, thanks!

Fam

Re: [Qemu-devel] [PATCH v4 0/8] discard blockstats

[Anton Nefedov]

ping

do you think we might proceed with this? or is there any general doubt
about the idea?

thanks,

On 21/8/2018 12:46 PM, Anton Nefedov wrote:

new in v4:
 - patch 7: discard and write-zeroes code paths had been separated in
   34fa110e: file-posix: Fix write_zeroes with unmap on block devices.
   This patch now only accounts discards that come explicitly
   through .bdrv_co_pdiscard handler.
 - qapi 'Since' clauses changed 3.0 -> 3.1

v3: http://lists.nongnu.org/archive/html/qemu-devel/2018-06/msg03688.html



qmp query-blockstats provides stats info for write/read/flush ops.

Patches 1-6 implement the similar for discard (unmap) command for scsi
and ide disks.
Discard stat "unmap_ops / unmap_bytes" is supposed to account the ops that
have completed without an error.

However, discard operation is advisory. Specifically,
  - common block layer ignores ENOTSUP error code.
That might be returned if the block driver does not support discard,
or discard has been configured to be ignored.
  - format drivers such as qcow2 may ignore discard if they were configured
to ignore that, or if the corresponding area is already marked unused
(unallocated / zero clusters).

And what is actually useful is the number of bytes actually discarded
down on the host filesystem.
To achieve that, driver-specific statistics has been added to blockstats
(patch 8).
With patch 7, file-posix driver accounts discard operations on its level too.

query-blockstat result:

(note the difference between blockdevice unmap and file discard stats. qcow2
sends fewer ops down to the file as the clusters are actually unallocated
on qcow2 level)

{
   "return": [
 {
   "device": "drive-scsi0-0-0-0",
   "parent": {

   "discard-bytes-ok": 262144,
   "discard-nb-ok": 4,

 "stats": {

 "unmap_operations": 0,
 "unmap_merged": 0,

   "flush_total_time_ns": 0,
   "wr_highest_offset": 8111718400,
   "wr_total_time_ns": 0,
   "failed_wr_operations": 0,
   "failed_rd_operations": 0,
   "wr_merged": 0,
   "wr_bytes": 0,
   "timed_stats": [
 
   ],

 "failed_unmap_operations": 0,

   "failed_flush_operations": 0,
   "account_invalid": false,
   "rd_total_time_ns": 0,

 "invalid_unmap_operations": 0,

   "flush_operations": 0,
   "wr_operations": 0,

 "unmap_bytes": 0,

   "rd_merged": 0,
   "rd_bytes": 0,

 "unmap_total_time_ns": 0,

   "invalid_flush_operations": 0,
   "account_failed": false,
   "rd_operations": 0,
   "invalid_wr_operations": 0,
   "invalid_rd_operations": 0
 },
 "node-name": "#block012",

   "driver": "file",
   "discard-nb-failed": 0

   },
   "stats": {

   "unmap_operations": 860,
   "unmap_merged": 0,

 "flush_total_time_ns": 21506733,
 "wr_highest_offset": 13411741696,
 "wr_total_time_ns": 2212749334,
 "failed_wr_operations": 0,
 "failed_rd_operations": 0,
 "wr_merged": 0,
 "wr_bytes": 3426304,
 "timed_stats": [
   
 ],

   "failed_unmap_operations": 0,

 "failed_flush_operations": 0,
 "account_invalid": true,
 "rd_total_time_ns": 3617478206,

   "invalid_unmap_operations": 0,

 "flush_operations": 24,
 "wr_operations": 309,

   "unmap_bytes": 11949633536,

 "rd_merged": 0,
 "rd_bytes": 141967360,

   "unmap_total_time_ns": 14871816,

 [..]

Anton Nefedov (8):
   qapi: group BlockDeviceStats fields
   qapi: add unmap to BlockDeviceStats
   ide: account UNMAP (TRIM) operations
   scsi: store unmap offset and nb_sectors in request struct
   scsi: move unmap error checking to the complete callback
   scsi: account unmap operations
   file-posix: account discard operations
   qapi: query-blockstat: add driver specific file-posix stats

  qapi/block-core.json   | 82 +++---
  include/block/accounting.h |  1 +
  include/block/block.h  |  1 +
  include/block/block_int.h  |  1 +
  block.c|  9 +
  block/file-posix.c | 45 +++--
  block/qapi.c   | 11 +++
  hw/ide/core.c  | 12 +++
  hw/scsi/scsi-disk.c| 29 +---
  tests/qemu-iotests/227.out | 18 ++
  10 files changed, 184 insertions(+), 25 deletions(-)

Re: [Qemu-devel] [PATCH v2] clean up callback when del virtqueue

[Jason Wang]

On 2018年09月17日 21:48, liujunjie wrote:

Before, we did not clear callback like handle_output when delete
the virtqueue which may result be segmentfault.
The scene is as follows:
1. Start a vm with multiqueue vhost-net,
2. then we write VIRTIO_PCI_GUEST_FEATURES in PCI configuration to
triger multiqueue disable in this vm which will delete the virtqueue.
In this step, the tx_bh is deleted but the callback virtio_net_handle_tx_bh
still exist.
3. Finally, we write VIRTIO_PCI_QUEUE_NOTIFY in PCI configuration to
notify the deleted virtqueue. In this way, virtio_net_handle_tx_bh
will be called and qemu will be crashed.

Although the way described above is uncommon, we had better reinforce it.

Signed-off-by: liujunjie 
---
  hw/virtio/virtio.c | 2 ++
  1 file changed, 2 insertions(+)

diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
index d4e4d98..dc8dcf8 100644
--- a/hw/virtio/virtio.c
+++ b/hw/virtio/virtio.c
@@ -1604,6 +1604,8 @@ void virtio_del_queue(VirtIODevice *vdev, int n)
  
  vdev->vq[n].vring.num = 0;

  vdev->vq[n].vring.num_default = 0;
+vdev->vq[n].handle_output = NULL;
+vdev->vq[n].handle_aio_output = NULL;
  }
  
  static void virtio_set_isr(VirtIODevice *vdev, int value)


Applied and queued for -stable.

Thanks

Re: [Qemu-devel] [PATCH] hvf: drop unused variable

[Thomas Huth]

On 2018-09-18 11:28, Paolo Bonzini wrote:
> Signed-off-by: Paolo Bonzini 
> ---
>  target/i386/hvf/hvf.c | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/target/i386/hvf/hvf.c b/target/i386/hvf/hvf.c
> index 5db167df98..9f52bc413a 100644
> --- a/target/i386/hvf/hvf.c
> +++ b/target/i386/hvf/hvf.c
> @@ -72,7 +72,6 @@
>  #include "sysemu/sysemu.h"
>  #include "target/i386/cpu.h"
>  
> -pthread_rwlock_t mem_lock = PTHREAD_RWLOCK_INITIALIZER;
>  HVFState *hvf_state;
>  int hvf_disabled = 1;
>  
> 

Reviewed-by: Thomas Huth 

Re: [Qemu-devel] [PATCH V12-fix-V2 01/19] filter-rewriter: Add TCP state machine and fix memory leak in connection_track_table

[Jason Wang]

On 2018年09月14日 09:47, Zhang Chen wrote:

We add almost full TCP state machine in filter-rewriter, except
TCPS_LISTEN and some simplify in VM active close FIN states.
The reason for this simplify job is because guest kernel will track
the TCP status and wait 2MSL time too, if client resend the FIN packet,
guest will resend the last ACK, so we needn't wait 2MSL time in filter-rewriter.

After a net connection is closed, we didn't clear its related resources
in connection_track_table, which will lead to memory leak.

Let's track the state of net connection, if it is closed, its related
resources will be cleared up.

Signed-off-by: zhanghailiang
Signed-off-by: Zhang Chen
Signed-off-by: Zhang Chen


Applied.

Thanks

[Qemu-devel] [PATCH 1/1] qmp, hmp: add PCI subsystem id and vendor id to PCI info

[Denis V. Lunev]

This is a long story. RedHat has relicensed Windows KVM device drivers
in 2018 and there was an agreement that to avoid WHQL driver conflict
software manufacturers should set proper PCI subsystem vendor ID in
their distributions. Thus PCI subsystem vendor id becomes actively used.

The problem is that this field is applied by us via hardware compats.
Thus technically it could be lost.

This patch adds PCI susbsystem id and vendor id to exportable parameters
for validation.

Signed-off-by: Denis V. Lunev 
CC: "Dr. David Alan Gilbert" 
CC: Eric Blake 
CC: Markus Armbruster 
---
 hmp.c| 2 ++
 hw/pci/pci.c | 3 +++
 qapi-schema.json | 7 ++-
 3 files changed, 11 insertions(+), 1 deletion(-)

diff --git a/hmp.c b/hmp.c
index 2874bcd789..8fb0957cfd 100644
--- a/hmp.c
+++ b/hmp.c
@@ -803,6 +803,8 @@ static void hmp_info_pci_device(Monitor *mon, const 
PciDeviceInfo *dev)
 
 monitor_printf(mon, ": PCI device %04" PRIx64 ":%04" PRIx64 "\n",
dev->id->vendor, dev->id->device);
+monitor_printf(mon, "  PCI subsystem %04" PRIx64 ":%04" PRIx64 "\n",
+   dev->id->subsystem_vendor, dev->id->subsystem);
 
 if (dev->has_irq) {
 monitor_printf(mon, "  IRQ %" PRId64 ".\n", dev->irq);
diff --git a/hw/pci/pci.c b/hw/pci/pci.c
index f0c98cd0ae..be70dd425c 100644
--- a/hw/pci/pci.c
+++ b/hw/pci/pci.c
@@ -1724,6 +1724,9 @@ static PciDeviceInfo *qmp_query_pci_device(PCIDevice 
*dev, PCIBus *bus,
 info->id = g_new0(PciDeviceId, 1);
 info->id->vendor = pci_get_word(dev->config + PCI_VENDOR_ID);
 info->id->device = pci_get_word(dev->config + PCI_DEVICE_ID);
+info->id->subsystem = pci_get_word(dev->config + PCI_SUBSYSTEM_ID);
+info->id->subsystem_vendor =
+pci_get_word(dev->config + PCI_SUBSYSTEM_VENDOR_ID);
 info->regions = qmp_query_pci_regions(dev);
 info->qdev_id = g_strdup(dev->qdev.id ? dev->qdev.id : "");
 
diff --git a/qapi/misc.json b/qapi/misc.json
index dfef6faf81..1704a8d437 100644
--- a/qapi/misc.json
+++ b/qapi/misc.json
@@ -2162,10 +2162,15 @@
 #
 # @vendor: the PCI vendor id
 #
+# @subsystem: the PCI subsystem id (since 3.1)
+#
+# @subsystem-vendor: the PCI subsystem vendor id (since 3.1)
+#
 # Since: 2.4
 ##
 { 'struct': 'PciDeviceId',
-  'data': {'device': 'int', 'vendor': 'int'} }
+  'data': {'device': 'int', 'vendor': 'int', 'subsystem': 'int',
+'subsystem-vendor': 'int'} }
 
 ##
 # @PciDeviceInfo:
-- 
2.17.1

Re: [Qemu-devel] [PATCH v2] vhost-user-blk: start vhost when guest kicks

[Yongji Xie]

Kindly ping...

On Thu, 7 Jun 2018 at 20:50, Stefan Hajnoczi  wrote:
>
> On Wed, Jun 06, 2018 at 09:24:48PM +0800, Yongji Xie wrote:
> > Some old guests (before commit 7a11370e5: "virtio_blk: enable VQs early")
> > kick virtqueue before setting VIRTIO_CONFIG_S_DRIVER_OK. This violates
> > the virtio spec. But virtio 1.0 transitional devices support this behaviour.
> > So we should start vhost when guest kicks in this case.
> >
> > Signed-off-by: Yongji Xie 
> > Signed-off-by: Chai Wen 
> > Signed-off-by: Ni Xun 
> > ---
> >  hw/block/vhost-user-blk.c |   25 +
> >  1 file changed, 25 insertions(+)
>
> Reviewed-by: Stefan Hajnoczi 

Re: [Qemu-devel] [PATCH] qom/object: add some interface asserts

[Paolo Bonzini]

On 12/09/2018 14:53, Marc-André Lureau wrote:
> An interface can't have any instance size or callback, or itself
> implement other interfaces (this is unsupported).
> 
> Signed-off-by: Marc-André Lureau 
> ---
>  qom/object.c | 9 -
>  1 file changed, 8 insertions(+), 1 deletion(-)
> 
> diff --git a/qom/object.c b/qom/object.c
> index 75d1d48944..9222b23172 100644
> --- a/qom/object.c
> +++ b/qom/object.c
> @@ -286,7 +286,14 @@ static void type_initialize(TypeImpl *ti)
>  if (ti->instance_size == 0) {
>  ti->abstract = true;
>  }
> -
> +if (type_is_ancestor(ti, type_interface)) {
> +assert(ti->instance_size == 0);
> +assert(ti->abstract);
> +assert(!ti->instance_init);
> +assert(!ti->instance_post_init);
> +assert(!ti->instance_finalize);
> +assert(!ti->num_interfaces);
> +}
>  ti->class = g_malloc0(ti->class_size);
>  
>  parent = type_get_parent(ti);
> 

Queued, thanks.

Paolo

Re: [Qemu-devel] [PATCH v5 3/3] target/s390x: implement CVB, CVBY and CVBG

[David Hildenbrand]

Am 02.09.18 um 02:33 schrieb Pavel Zbitskiy:
> Convert to Binary - counterparts of the already implemented Convert
> to Decimal (CVD*) instructions.
> Example from the Principles of Operation: 25594C becomes 63FA.
> 
> Signed-off-by: Pavel Zbitskiy 
> ---
>  target/s390x/helper.h   |  1 +
>  target/s390x/insn-data.def  |  4 +++
>  target/s390x/int_helper.c   | 52 +
>  target/s390x/translate.c| 11 +++
>  tests/tcg/s390x/Makefile.target |  1 +
>  tests/tcg/s390x/cvb.c   | 18 
>  6 files changed, 87 insertions(+)
>  create mode 100644 tests/tcg/s390x/cvb.c
> 
> diff --git a/target/s390x/helper.h b/target/s390x/helper.h
> index 97c60ca7bc..46baaee0ab 100644
> --- a/target/s390x/helper.h
> +++ b/target/s390x/helper.h
> @@ -88,6 +88,7 @@ DEF_HELPER_FLAGS_4(tcxb, TCG_CALL_NO_RWG_SE, i32, env, i64, 
> i64, i64)
>  DEF_HELPER_FLAGS_2(sqeb, TCG_CALL_NO_WG, i64, env, i64)
>  DEF_HELPER_FLAGS_2(sqdb, TCG_CALL_NO_WG, i64, env, i64)
>  DEF_HELPER_FLAGS_3(sqxb, TCG_CALL_NO_WG, i64, env, i64, i64)
> +DEF_HELPER_FLAGS_3(cvb, TCG_CALL_NO_WG, i64, env, i64, i32)
>  DEF_HELPER_FLAGS_1(cvd, TCG_CALL_NO_RWG_SE, i64, s32)
>  DEF_HELPER_FLAGS_4(pack, TCG_CALL_NO_WG, void, env, i32, i64, i64)
>  DEF_HELPER_FLAGS_4(pka, TCG_CALL_NO_WG, void, env, i64, i64, i32)
> diff --git a/target/s390x/insn-data.def b/target/s390x/insn-data.def
> index 9c7b434fca..0911180ca6 100644
> --- a/target/s390x/insn-data.def
> +++ b/target/s390x/insn-data.def
> @@ -284,6 +284,10 @@
>  D(0xec73, CLFIT,   RIE_a, GIE, r1_32u, i2_32u, 0, 0, ct, 0, 1)
>  D(0xec71, CLGIT,   RIE_a, GIE, r1_o, i2_32u, 0, 0, ct, 0, 1)
>  
> +/* CONVERT TO BINARY */
> +C(0x4f00, CVB, RX_a,  Z,   la2, 0, new, r1_32, cvb, 0)
> +C(0xe306, CVBY,RXY_a, LD,  la2, 0, new, r1_32, cvb, 0)
> +C(0xe30e, CVBG,RXY_a, Z,   la2, 0, r1, 0, cvb, 0)

This has to be r1_o, no? (otherwise the value is never actually written
to r1, but only to a copy).

However, I guess we should write the result directly from the
HELPER(cvb) - due to the fixed-point-divide exception checks. So pass to
the helper the register number instead.

>  /* CONVERT TO DECIMAL */
>  C(0x4e00, CVD, RX_a,  Z,   r1_o, a2, 0, 0, cvd, 0)
>  C(0xe326, CVDY,RXY_a, LD,  r1_o, a2, 0, 0, cvd, 0)
> diff --git a/target/s390x/int_helper.c b/target/s390x/int_helper.c
> index abf77a94e6..3b12c11cee 100644
> --- a/target/s390x/int_helper.c
> +++ b/target/s390x/int_helper.c
> @@ -24,6 +24,7 @@
>  #include "exec/exec-all.h"
>  #include "qemu/host-utils.h"
>  #include "exec/helper-proto.h"
> +#include "exec/cpu_ldst.h"
>  
>  /* #define DEBUG_HELPER */
>  #ifdef DEBUG_HELPER
> @@ -118,6 +119,57 @@ uint64_t HELPER(divu64)(CPUS390XState *env, uint64_t ah, 
> uint64_t al,
>  return ret;
>  }
>  
> +static void general_operand_exception(CPUS390XState *env, uintptr_t ra)
> +{
> +#ifndef CONFIG_USER_ONLY
> +LowCore *lowcore;
> +
> +lowcore = cpu_map_lowcore(env);
> +lowcore->data_exc_code = 0;
> +cpu_unmap_lowcore(lowcore);
> +#endif
> +s390_program_interrupt(env, PGM_DATA, ILEN_AUTO, ra);

Have a look at

http://patchwork.ozlabs.org/patch/963846/

Once that is merged (soon), you can make use of
tcg_s390_data_exception(), which will also take care of storing the dxc
into the FPC poperly.

> +}
> +
> +uint64_t HELPER(cvb)(CPUS390XState *env, uint64_t src, uint32_t n)
> +{
> +int i, j;
> +uint64_t tmpsrc;
> +const uintptr_t ra = GETPC();
> +int64_t dec, sign = 0, digit, val = 0, pow10 = 0;
> +
> +for (i = 0; i < n; i++) {
> +tmpsrc = wrap_address(env, src + (n - i - 1) * 8);
> +dec = cpu_ldq_data_ra(env, tmpsrc, ra);
> +for (j = 0; j < 16; j++, dec >>= 4) {
> +if (i == 0 && j == 0) {
> +sign = dec & 0xf;
> +if (sign < 0xa) {
> +general_operand_exception(env, ra);
> +}
> +continue;
> +}
> +digit = dec & 0xf;
> +if (digit > 0x9) {
> +general_operand_exception(env, ra);
> +}
> +if (i == 0 && j == 1) {
> +if (sign == 0xb || sign == 0xd) {
> +val = -digit;
> +pow10 = -10;
> +} else {
> +val = digit;
> +pow10 = 10;
> +}
> +} else {
> +val += digit * pow10;
> +pow10 *= 10;
> +}
> +}
> +}
> +return val;

We are missing the fixed-point-divide exception checks.

For n==1, the valid range is -2,147,483,648 - 2,147,483,647. So in case
our number no longer fits into 32bit (excluding sign extension), we have
to trigger that. Guess it could be something like this (please verify):

/* store the result before triggering fix-point-divide */
if (n ==1) {
env->regs[reg] &= 0xfff...
env->regs[reg] |= 

Re: [Qemu-devel] [PATCH 00/35] exec: drop BQL from interrupt handling

[David Hildenbrand]

Am 17.09.18 um 18:30 schrieb Emilio G. Cota:
> This series comes originally from a series of patches that Paolo
> sent to me a long time ago. I have kept most of his S-o-b tags,
> but I have done the forward port of the patches to the current
> QEMU code base, so please blame all possible bugs on me, not him.
> 
> The goal of this series is to push the BQL out of interrupt
> handling wherever possible. Many targets do not need the BQL
> to handle interrupts, so for those targets we get rid of the
> BQL in interrupt handling. In TCG mode, this by itself does
> not yield measurable performance gains. However, it is a
> necessary first step before tackling BQL contention when
> managing CPU states (i.e. exiting the exec loop always
> involves taking the BQL, which does not scale).
> 
> The patches:
> 
> - Patch 0 accesses icount_decr.u16 consistently with atomics.
>   This is important because we use it across threads to signal
>   that an interrupt is pending.
> 
> - 2-28: convert direct callers of cpu->interrupt_request
>   to use atomic accesses or CPU helper functions that use atomics.
>   I have broken these up into individual patches to ease review.
>   Note that in some cases I have added many atomic_reads, most
>   of which are probably unnecessary. However, I'd leave this up
>   to the maintainers, since I do not want to break things by
>   doing a single atomic_read(>interrupt_request) at the
>   beginning of a function and then miss subsequent updates to
>   cpu->interrupt_request.
> 
> - 29-31: drop the BQL requirement when handling cpu->interrupt_request
> 
> - 32-33: drop now unnecessary BQL acquisitions in i386 and ppc
> 
> - 34-35: push down the BQL acquisition to cpu->do_interrupt
>   and cpu->cpu_exec_interrupt. Here I am Cc'ing everyone so that
>   we can make sure I am not missing any target that requires
>   the BQL on either do_interrupt or cpu_exec_interrupt.
> 
> This series is checkpatch-clean, and can be fetched from:
>   https://github.com/cota/qemu/tree/cpu-lock
> 
> Note that the series applies on top of patches that are
> already on the qemu-devel list and will hopefully soon
> be upstream. This series begins after commit
> 4ece0cbd74 ("cpus: access .qemu_icount_bias with atomic64")
> in that tree.
> 
> Thanks,
> 
>   Emilio
> 

I just did a (very slow) make -j32 of QEMU on my 16 core notebook inside
a MTTCG s390x-softmmu guest with 32 VCPUs. I used the same test when
reworking floating interrupt support to make sure there are no races.

As I got no deadlock, I assume you series does not break anything for s390x.

Thanks!

-- 

Thanks,

David / dhildenb

Re: [Qemu-devel] [RFC v5 1/6] pci_expander_bridge: add type TYPE_PXB_PCIE_HOST

[Zihan Yang]

Gerd Hoffmann  于2018年9月18日周二 下午1:09写道：
>
>   Hi,
>
> > +static void pxb_pcie_host_get_mmcfg_base(Object *obj, Visitor *v, const 
> > char *name,
>
> > +void *opaque, Error **errp)
> > +{
> > +PCIExpressHost *e = PCIE_HOST_BRIDGE(obj);
> > +
> > +visit_type_uint64(v, name, >size, errp);
>   
>
> cut bug?

Thanks for pointing out. I was wondering how I got the right behavior with
wrong code, then I find I somehow fix it in patch 3/6... It will be solved in
next version.

> cheers,
>   Gerd
>

Re: [Qemu-devel] [PATCH 23/35] target/s390x: access cpu->interrupt_request with atomics

[David Hildenbrand]

Am 17.09.18 um 18:30 schrieb Emilio G. Cota:
> From: Paolo Bonzini 
> 
> Cc: Cornelia Huck 
> Cc: Christian Borntraeger 
> Cc: Alexander Graf 
> Cc: Richard Henderson 
> Cc: David Hildenbrand 
> Cc: qemu-s3...@nongnu.org
> Signed-off-by: Paolo Bonzini 
> Signed-off-by: Emilio G. Cota 
> ---
>  hw/intc/s390_flic.c | 2 +-
>  target/s390x/cpu.c  | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/hw/intc/s390_flic.c b/hw/intc/s390_flic.c
> index 5f8168f0f0..bce07c8a1e 100644
> --- a/hw/intc/s390_flic.c
> +++ b/hw/intc/s390_flic.c
> @@ -189,7 +189,7 @@ static void qemu_s390_flic_notify(uint32_t type)
>  CPU_FOREACH(cs) {
>  S390CPU *cpu = S390_CPU(cs);
>  
> -cs->interrupt_request |= CPU_INTERRUPT_HARD;
> +atomic_or(>interrupt_request, CPU_INTERRUPT_HARD);
>  
>  /* ignore CPUs that are not sleeping */
>  if (s390_cpu_get_state(cpu) != S390_CPU_STATE_OPERATING &&
> diff --git a/target/s390x/cpu.c b/target/s390x/cpu.c
> index 8ed4823d6e..d4c93abc29 100644
> --- a/target/s390x/cpu.c
> +++ b/target/s390x/cpu.c
> @@ -65,7 +65,7 @@ static bool s390_cpu_has_work(CPUState *cs)
>  return false;
>  }
>  
> -if (!(cs->interrupt_request & CPU_INTERRUPT_HARD)) {
> +if (!(atomic_read(>interrupt_request) & CPU_INTERRUPT_HARD)) {
>  return false;
>  }
>  
> 

Reviewed-by: David Hildenbrand 

-- 

Thanks,

David / dhildenb

Re: [Qemu-devel] [PATCH 0/5] contrib: add elf2dmp tool

[Paolo Bonzini]

On 18/09/2018 07:44, Viktor Prutyanov wrote:
> On Fri, 14 Sep 2018 17:22:14 +0200
> Paolo Bonzini  wrote:
> 
>> On 29/08/2018 14:41, Viktor Prutyanov wrote:
>>> elf2dmp is a converter from ELF dump (produced by
>>> 'dump-guest-memory') to Windows MEMORY.DMP format (also know as
>>> 'Complete Memory Dump') which can be opened in WinDbg.
>>>
>>> This tool can help if VMCoreInfo device/driver is absent in Windows
>>> VM and 'dump-guest-memory -w' is not available but dump can be
>>> created in ELF format.
>>>
>>> elf2dmp differs from other universal converters in method of
>>> determining of virtual memory layout. The tool uses register values
>>> from QEMU ELF dump file to do it. In particular, it uses
>>> KERNEL_GS_BASE value added to dump format in QEMU 3.0.
>>>
>>> Even if KERNEL_GS_BASEs are absent in ELF dump file, at least 1
>>> vCPU with kernel task can be found quite often and virtual memory
>>> layout can be determined.
>>>
>>> Viktor Prutyanov (5):
>>>   dump: move Windows dump structures definitions
>>>   contrib: add elf2dmp tool
>>>   contrib/elf2dmp: improve paging root selection
>>>   contrib/elf2dmp: add DMP file name as 2nd argument
>>>   contrib/elf2dmp: add 1GB and 2MB pages support
>>>
>>>  Makefile  |   5 +
>>>  Makefile.objs |   1 +
>>>  configure |   3 +
>>>  contrib/elf2dmp/Makefile.objs |   1 +
>>>  contrib/elf2dmp/addrspace.c   | 236 +
>>>  contrib/elf2dmp/addrspace.h   |  44 
>>>  contrib/elf2dmp/download.c|  50 
>>>  contrib/elf2dmp/download.h|  13 +
>>>  contrib/elf2dmp/err.h |  13 +
>>>  contrib/elf2dmp/kdbg.h| 194 ++
>>>  contrib/elf2dmp/main.c| 594
>>> ++
>>> contrib/elf2dmp/pdb.c | 331 +++
>>> contrib/elf2dmp/pdb.h | 241 +
>>> contrib/elf2dmp/pe.h  | 121 +
>>> contrib/elf2dmp/qemu_elf.c| 172 
>>> contrib/elf2dmp/qemu_elf.h|  51 
>>> include/qemu/win_dump_defs.h  | 179 +
>>> win_dump.h| 166 +--- 18 files changed,
>>> 2253 insertions(+), 162 deletions(-) create mode 100644
>>> contrib/elf2dmp/Makefile.objs create mode 100644
>>> contrib/elf2dmp/addrspace.c create mode 100644
>>> contrib/elf2dmp/addrspace.h create mode 100644
>>> contrib/elf2dmp/download.c create mode 100644
>>> contrib/elf2dmp/download.h create mode 100644 contrib/elf2dmp/err.h
>>>  create mode 100644 contrib/elf2dmp/kdbg.h
>>>  create mode 100644 contrib/elf2dmp/main.c
>>>  create mode 100644 contrib/elf2dmp/pdb.c
>>>  create mode 100644 contrib/elf2dmp/pdb.h
>>>  create mode 100644 contrib/elf2dmp/pe.h
>>>  create mode 100644 contrib/elf2dmp/qemu_elf.c
>>>  create mode 100644 contrib/elf2dmp/qemu_elf.h
>>>  create mode 100644 include/qemu/win_dump_defs.h
>>>   
>>
>> Queued, squashing patches 2-5.  Would you like to send a patch for
>> MAINTAINERS, adding yourself for elf2dmp?
>>
>> Paolo
> 
> If I add myself to MAINTAINERS, what I will be expected to do?

As a start, it's okay if you just review any patch.  Peter or I (or in
the future, the committer / misc tree maintainer) can take care of
applying them.

Paolo

[Qemu-devel] qemu-nbd performance

[lampahome]

I test nbd performance when I divide image into multiple backing files.
The image is 512GB, I divide it into 1, 16, 32, 64, and 128 backing files.

Ex: If I divide it into 16 files, each backing file is 512/16=32GB.
If I divide it into 64 files, each backing file is 512/64=8GB  and so on.

*Mount command: qemu-nbd -c /dev/nbd0 image*

*Cmd to test:*
*Read:*

fio -bs=1m -iodepth=16  -rw=read -ioengine=libaio -name=mytest -direct=1
 -size=512G -runtime=300 -filename=/dev/nbd0

*Write:*

fio -bs=1m -iodepth=16  -rw=read -ioengine=libaio -name=mytest -direct=1
 -size=512G -runtime=300 -filename=/dev/nbd0


All images are on the RAID0(3 SSD).
Below is the performance:

* image numberseq. read(MB/s)seq.
write(MB/s)1148060161453363214503664143036128140036*
The seq. read performance is very well than write.
1. Does nbd cache the data defaultly and make read so quickly?
2. The write performance isn't so good, Does nbd do something to decrease
the performance?

Re: [Qemu-devel] [PATCH 05/35] target/s390x: use cpu_reset_interrupt

[David Hildenbrand]

Am 17.09.18 um 18:30 schrieb Emilio G. Cota:
> From: Paolo Bonzini 
> 
> It will be changed to an atomic operation soon.
> 
> Cc: Cornelia Huck 
> Cc: Richard Henderson 
> Cc: Alexander Graf 
> Cc: David Hildenbrand 
> Cc: qemu-s3...@nongnu.org
> Signed-off-by: Paolo Bonzini 
> Signed-off-by: Emilio G. Cota 
> ---
>  target/s390x/excp_helper.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/target/s390x/excp_helper.c b/target/s390x/excp_helper.c
> index f0ce60cff2..f2b92d7cbc 100644
> --- a/target/s390x/excp_helper.c
> +++ b/target/s390x/excp_helper.c
> @@ -444,7 +444,7 @@ try_deliver:
>  
>  /* we might still have pending interrupts, but not deliverable */
>  if (!env->pending_int && !qemu_s390_flic_has_any(flic)) {
> -cs->interrupt_request &= ~CPU_INTERRUPT_HARD;
> +cpu_reset_interrupt(cs, CPU_INTERRUPT_HARD);
>  }
>  
>  /* WAIT PSW during interrupt injection or STOP interrupt */
> 

Reviewed-by: David Hildenbrand 

-- 

Thanks,

David / dhildenb

Re: [Qemu-devel] [PATCH 35/35] exec: push BQL down to cpu->cpu_exec_interrupt

[David Hildenbrand]

>  return false;
> diff --git a/target/s390x/excp_helper.c b/target/s390x/excp_helper.c
> index 931c0103c8..f2a93abf01 100644
> --- a/target/s390x/excp_helper.c
> +++ b/target/s390x/excp_helper.c
> @@ -480,10 +480,13 @@ bool s390_cpu_exec_interrupt(CPUState *cs, int 
> interrupt_request)
> the parent EXECUTE insn.  */
>  return false;
>  }
> +qemu_mutex_lock_iothread();
>  if (s390_cpu_has_int(cpu)) {
>  s390_cpu_do_interrupt(cs);

You can directly use s390_cpu_do_interrupt_locked() here.

> +qemu_mutex_unlock_iothread();
>  return true;
>  }
> +qemu_mutex_unlock_iothread();
>  if (env->psw.mask & PSW_MASK_WAIT) {
>  /* Woken up because of a floating interrupt but it has already
>   * been delivered. Go back to sleep. */
> 

-- 

Thanks,

David / dhildenb

Re: [Qemu-devel] [PATCH v5 2/3] target/s390x: exception on non-aligned LPSW(E)

[David Hildenbrand]

Am 02.09.18 um 02:33 schrieb Pavel Zbitskiy:
> Both LPSW and LPSWE should raise a specification exception when their
> operand is not doubleword aligned.
> 
> Signed-off-by: Pavel Zbitskiy 
> ---
>  target/s390x/translate.c | 6 --
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/target/s390x/translate.c b/target/s390x/translate.c
> index 7363aabf3a..59b1e5893c 100644
> --- a/target/s390x/translate.c
> +++ b/target/s390x/translate.c
> @@ -2835,7 +2835,8 @@ static DisasJumpType op_lpsw(DisasContext *s, DisasOps 
> *o)
>  
>  t1 = tcg_temp_new_i64();
>  t2 = tcg_temp_new_i64();
> -tcg_gen_qemu_ld32u(t1, o->in2, get_mem_index(s));
> +tcg_gen_qemu_ld_i64(t1, o->in2, get_mem_index(s),
> +MO_TEUL | MO_ALIGN_8);
>  tcg_gen_addi_i64(o->in2, o->in2, 4);
>  tcg_gen_qemu_ld32u(t2, o->in2, get_mem_index(s));
>  /* Convert the 32-bit PSW_MASK into the 64-bit PSW_MASK.  */
> @@ -2855,7 +2856,8 @@ static DisasJumpType op_lpswe(DisasContext *s, DisasOps 
> *o)
>  
>  t1 = tcg_temp_new_i64();
>  t2 = tcg_temp_new_i64();
> -tcg_gen_qemu_ld64(t1, o->in2, get_mem_index(s));
> +tcg_gen_qemu_ld_i64(t1, o->in2, get_mem_index(s),
> +MO_TEQ | MO_ALIGN_8);
>  tcg_gen_addi_i64(o->in2, o->in2, 8);
>  tcg_gen_qemu_ld64(t2, o->in2, get_mem_index(s));
>  gen_helper_load_psw(cpu_env, t1, t2);
> 

Reviewed-by: David Hildenbrand 

Thanks!

-- 

Thanks,

David / dhildenb

Re: [Qemu-devel] [PATCH] accel/tcg: Remove dead code

[Paolo Bonzini]

On 17/09/2018 19:08, Thomas Huth wrote:
> The global cpu_single_env variable has been removed more than 5 years
> ago, so apparently nobody used this dead debug code in that timeframe
> anymore. Thus let's remove it completely now.
> 
> Signed-off-by: Thomas Huth 
> ---
>  accel/tcg/translate-all.c | 9 -
>  1 file changed, 9 deletions(-)
> 
> diff --git a/accel/tcg/translate-all.c b/accel/tcg/translate-all.c
> index 898c3bb..e589c35 100644
> --- a/accel/tcg/translate-all.c
> +++ b/accel/tcg/translate-all.c
> @@ -2011,15 +2011,6 @@ void tb_invalidate_phys_page_fast(struct 
> page_collection *pages,
>  {
>  PageDesc *p;
>  
> -#if 0
> -if (1) {
> -qemu_log("modifying code at 0x%x size=%d EIP=%x PC=%08x\n",
> -  cpu_single_env->mem_io_vaddr, len,
> -  cpu_single_env->eip,
> -  cpu_single_env->eip +
> -  (intptr_t)cpu_single_env->segs[R_CS].base);
> -}
> -#endif
>  assert_memory_lock();
>  
>  p = page_find(start >> TARGET_PAGE_BITS);
> 

Queued, thanks.

Paolo

[Qemu-devel] [Bug 1793119] Re: Wrong floating-point emulation on AArch64 with FPCR set to zero

[Emilio G. Cota]

Not yet. There should be a fix before 3.1 is released.

Both 2.12 and 3.0 have this bug, so you might want to consider using
2.11 until the bug gets fixed.

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

Title:
  Wrong floating-point emulation on AArch64 with FPCR set to zero

Status in QEMU:
  Confirmed

Bug description:
  On AArch64, with FPCR set to Zero (i.e., FPU set to IEEE-754 compliant
  mode), floating-point emulation does not produce the same results as
  real hardware (e.g., Raspberry Pi 3 with AArch64 Linux).

  I attached a sample that reproduces the issue. It divides `x` by `y`
  and puts the result in `r`. The expected result of the operation is
  `q`.

  Output on real hardware:
  =
  fpcr = 0x0700.
  x = 0x03250f416dcdc6d0. y = 0x00029f4e5837c977. r = 0x7ff0. q = 
0x43300fde9cbcf023.
  fpcr = 0x.
  x = 0x03250f416dcdc6d0. y = 0x00029f4e5837c977. r = 0x43300fde9cbcf023. q = 
0x43300fde9cbcf023.
  =

  Notice that after setting FPCR to zero, `r` equals `q`.

  Output on qemu 3.0.0 (Linux user-mode emulation):
  =
  fpcr = 0x0700.
  x = 0x03250f416dcdc6d0. y = 0x00029f4e5837c977. r = 0x7ff0. q = 
0x43300fde9cbcf023.
  fpcr = 0x.
  x = 0x03250f416dcdc6d0. y = 0x00029f4e5837c977. r = 0x43300fde9cbcf024. q = 
0x43300fde9cbcf023.
  =

  Notice that after setting FPCR to zero, `r` is not equal to `q`.

  Also notice that, using another proprietary operating system, the same
  issue arises between a real board and QEMU. This might be an issue in
  emulation of the AArch64 instruction "fdiv".

  Build command line: aarch64-linux-gnu-gcc -static -O0 -o sample1
  sample1.c

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

Re: [Qemu-devel] [PATCH 04/11] hw/arm/aspeed: add a 'mmio-exec' property to boot from the FMC flash module

[Peter Maydell]

On 31 August 2018 at 11:38, Cédric Le Goater  wrote:
> Now that MMIO execution is supported, introduce a 'mmio-exec' property
> to boot directly from CE0 of the FMC controller using a memory region
> alias.

The name of this property seems to be a reference to QEMU's
internals: is there some other name that would make more sense
to a user who cares more about the guest and the emulated hardware?

> The overhead for the current firmware images using a custom U-Boot is
> around 2 seconds, which is fine, but with a U-Boot from mainline, it
> takes an extra 50 seconds or so to reach Linux. This might be related
> to the fact that a device tree is used.

Is this overhead just because we do a lot of execution from the
mmio region and that's slower than execution from RAM ?

> MMIO execution is not activated by default because until boot time is
> improved.

Can the guest tell the difference? I'm wondering how much we
might need to care about backward compatibility if in future
we fix the performance problem, or if we can just switch the
default to execute-from-flash without breaking guests.

thanks
-- PMM

Re: [Qemu-devel] [PATCH v2 02/12] util: add atomic64

[Emilio G. Cota]

On Tue, Sep 18, 2018 at 16:55:56 +0100, Peter Maydell wrote:
> On 11 September 2018 at 21:43, Emilio G. Cota  wrote:
> > On Tue, Sep 11, 2018 at 05:43:38 -0700, Richard Henderson wrote:
> >> Is this any better than using libatomic?
> >
> > I didn't think of using libatomic. I just checked the source
> > code and it's quite similar:
> > - It uses 64 locks instead of 16 ($page_size/$cache_line,
> >   but these are hard-coded for POSIX as 4096/64, respectively)
> > - We compute the cacheline size and corresponding padding
> >   at run-time, which is a little better than the above.
> > - The locks are implemented as pthread_mutex instead of
> >   spinlocks. I think spinlocks make more sense here because
> >   we do not expect huge contention (systems without
> >   !CONFIG_ATOMIC64 have few cores); for libatomic it makes
> >   sense to use mutexes because it might be used in many-core
> >   machines.
> >
> > So yes, we could have used libatomic. If you feel strongly
> > about it, I can give it a shot.
> 
> I guess the question I have is:
>  * will we need (now or in future) to emit atomic accesses
>into generated TCG code that need to interoperate with
>the atomic accesses we do from C code ?
>  * if so, does libatomic provide a mechanism for doing that?
>
> (maybe there's nothing you can do except to call out from
> the generated code to C code anyway)

We already have these. For instance:

- sTLB lookups can happen concurrently with invalidations
  to the same sTLB from another core (via tlb_reset_dirty)

- icount_decr is written to by cpu_exit, and is read
  at the beginning of each TB (with a tcg_gen_ld_32 generated
  by gen_tb_start).

The latter is always a 32-bit access, so all hosts can generate
it atomically. The former can be a 64-bit access if the guest
is 64-bit, which cannot be done if !CONFIG_ATOMIC64. But we
already disable mttcg for those combinations (we define
 TCG_OVERSIZED_GUEST for this).

If we wanted to support mttcg even for oversized guests,
then libatomic or our "atomic64" implementation could help,
but we'd have to insert a lock on every guest load/store
(either via the TCG backend or via a C helper). I do not think
we have enough users of these hosts to warrant support for this.
Moreover, these systems are unlikely to have many cores, and
therefore are unlikely to benefit much from mttcg.

Thanks,

Emilio

Re: [Qemu-devel] [PATCH v2 02/12] util: add atomic64

[Emilio G. Cota]

On Tue, Sep 18, 2018 at 10:23:32 -0300, Murilo Opsfelder Araujo wrote:
> On Tue, Sep 11, 2018 at 04:43:04PM -0400, Emilio G. Cota wrote:
> > On Tue, Sep 11, 2018 at 05:43:38 -0700, Richard Henderson wrote:
> > > On 09/10/2018 04:27 PM, Emilio G. Cota wrote:
> > > > +#define GEN_READ(name, type)\
> > > > +type name(const type *ptr)  \
> > > > +{   \
> > > > +QemuSpin *lock = addr_to_lock(ptr); \
> > > > +type ret;   \
> > > > +\
> > > > +qemu_spin_lock(lock);   \
> > > > +ret = *ptr; \
> > > > +qemu_spin_unlock(lock); \
> > > > +return ret; \
> > > > +}
> > > > +
> > > > +GEN_READ(atomic_read_i64, int64_t)
> > > > +GEN_READ(atomic_read_u64, uint64_t)
> > >
> > > Is there really a good reason to have two external
> > > functions instead of having one of them inline and
> > > perform a cast?
> >
> > Not really. I can do a follow-up patch if you want me to.
> >
> > > Is this any better than using libatomic?
> >
> > I didn't think of using libatomic. I just checked the source
> > code and it's quite similar:
> > - It uses 64 locks instead of 16 ($page_size/$cache_line,
> >   but these are hard-coded for POSIX as 4096/64, respectively)
> > - We compute the cacheline size and corresponding padding
> >   at run-time, which is a little better than the above.
> > - The locks are implemented as pthread_mutex instead of
> >   spinlocks. I think spinlocks make more sense here because
> >   we do not expect huge contention (systems without
> >   !CONFIG_ATOMIC64 have few cores); for libatomic it makes
> >   sense to use mutexes because it might be used in many-core
> >   machines.
> >
> > So yes, we could have used libatomic. If you feel strongly
> > about it, I can give it a shot.
> 
> Would allowing user to choose between libatomic or Emilio's implementation
> at configure time be a bad idea?
> 
> One could pass, for example, --with-libatomic to configure script to use
> libatomic instead of using Emilio's implementation, which could be the
> default implementation - or vice-versa.

If we decide to use libatomic then there's no need to keep
our own atomic64. As I said in another message, both
implementations are very similar.

libatomic supports more operations, but at the moment
we only need 64-bit atomic_read/write.

Thanks,

Emilio

Re: [Qemu-devel] [PATCH 07/11] aspeed/smc: add a 'sdram_base' and 'max-ram-size' properties

[Peter Maydell]

On 31 August 2018 at 11:38, Cédric Le Goater  wrote:
> The setting of the DRAM address of the DMA transaction depends on the
> DRAM base address and the maximun DRAM size of the SoC. Let's add a
> couple of properties to give this information to the SMC controller
> model.

In hardware, does the SMC controller really know the base address
of DRAM, or is it actually emitting transactions that the
bus fabric in the SoC sends to the right place? That is, would
it be more accurate to model it by passing the SMC controller a
MemoryRegion to use for emitting DMA transactions which was an
alias into the right part of the address space ?

thanks
-- PMM

[Qemu-devel] [PATCH v9 1/3] qmp: query-current-machine with wakeup-suspend-support

[Daniel Henrique Barboza]

When issuing the qmp/hmp 'system_wakeup' command, what happens in a
nutshell is:

- qmp_system_wakeup_request set runstate to RUNNING, sets a wakeup_reason
and notify the event
- in the main_loop, all vcpus are paused, a system reset is issued, all
subscribers of wakeup_notifiers receives a notification, vcpus are then
resumed and the wake up QAPI event is fired

Note that this procedure alone doesn't ensure that the guest will awake
from SUSPENDED state - the subscribers of the wake up event must take
action to resume the guest, otherwise the guest will simply reboot. At
this moment, only the ACPI machines via acpi_pm1_cnt_init has wake-up
from suspend support.

However, only the presence of 'system_wakeup' is required for QGA to
support 'guest-suspend-ram' and 'guest-suspend-hybrid' at this moment.
This means that the user/management will expect to suspend the guest using
one of those suspend commands and then resume execution using system_wakeup,
regardless of the support offered in system_wakeup in the first place.

This patch creates a new API called query-current-machine [1], that holds
a new flag called 'wakeup-suspend-support' that indicates if the guest
supports wake up from suspend via system_wakeup. The machine is considered
to implement wake-up support if a call to a new 'qemu_register_wakeup_support'
is made during its init, as it is now being done inside acpi_pm1_cnt_init.
This allows for any other machine type to declare wake-up support regardless
of ACPI state or wakeup_notifiers subscription, making easier for
newer implementations that might have its own mechanisms in the future.

This is the expected output of query-current-machine when running a x86
guest:

{"execute" : "query-current-machine"}
{"return": {"wakeup-suspend-support": true}}

Running the same x86 guest, but with the --no-acpi option:

{"execute" : "query-current-machine"}
{"return": {"wakeup-suspend-support": false}}

This is the output when running a pseries guest:

{"execute" : "query-current-machine"}
{"return": {"wakeup-suspend-support": false}}

With this extra tool, management can avoid situations where a guest
that does not have proper suspend/wake capabilities ends up in
inconsistent state (e.g.
https://github.com/open-power-host-os/qemu/issues/31).

[1] the decision of creating the query-current-machine API is based
on discussions in the QEMU mailing list where it was decided that
query-target wasn't a proper place to store the wake-up flag, neither
was query-machines because this isn't a static property of the
machine object. This new API can then be used to store other
dynamic machine properties that are scattered around the code
ATM. More info at:
https://lists.gnu.org/archive/html/qemu-devel/2018-05/msg04235.html

Reported-by: Balamuruhan S 
Signed-off-by: Daniel Henrique Barboza 
---
 hw/acpi/core.c  |  1 +
 include/sysemu/sysemu.h |  1 +
 qapi/misc.json  | 24 
 vl.c| 19 +++
 4 files changed, 45 insertions(+)

diff --git a/hw/acpi/core.c b/hw/acpi/core.c
index b8d39012cd..2dea893245 100644
--- a/hw/acpi/core.c
+++ b/hw/acpi/core.c
@@ -617,6 +617,7 @@ void acpi_pm1_cnt_init(ACPIREGS *ar, MemoryRegion *parent,
 ar->pm1.cnt.s4_val = s4_val;
 ar->wakeup.notify = acpi_notify_wakeup;
 qemu_register_wakeup_notifier(>wakeup);
+qemu_register_wakeup_support();
 memory_region_init_io(>pm1.cnt.io, memory_region_owner(parent),
   _pm_cnt_ops, ar, "acpi-cnt", 2);
 memory_region_add_subregion(parent, 4, >pm1.cnt.io);
diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h
index 8d6095d98b..0446adacc6 100644
--- a/include/sysemu/sysemu.h
+++ b/include/sysemu/sysemu.h
@@ -77,6 +77,7 @@ void qemu_register_suspend_notifier(Notifier *notifier);
 void qemu_system_wakeup_request(WakeupReason reason);
 void qemu_system_wakeup_enable(WakeupReason reason, bool enabled);
 void qemu_register_wakeup_notifier(Notifier *notifier);
+void qemu_register_wakeup_support(void);
 void qemu_system_shutdown_request(ShutdownCause reason);
 void qemu_system_powerdown_request(void);
 void qemu_register_powerdown_notifier(Notifier *notifier);
diff --git a/qapi/misc.json b/qapi/misc.json
index d450cfef21..b616d385a4 100644
--- a/qapi/misc.json
+++ b/qapi/misc.json
@@ -2003,6 +2003,30 @@
 ##
 { 'command': 'query-machines', 'returns': ['MachineInfo'] }
 
+##
+# @CurrentMachineParams:
+#
+# Information describing the running machine parameters.
+#
+# @wakeup-suspend-support: true if the target supports wake up from
+#  suspend
+#
+# Since: 3.1.0
+##
+{ 'struct': 'CurrentMachineParams',
+  'data': { 'wakeup-suspend-support': 'bool'} }
+
+##
+# @query-current-machine:
+#
+# Return a list of parameters of the running machine.
+#
+# Returns: CurrentMachineParams
+#
+# Since: 3.1.0
+##
+{ 'command': 'query-current-machine', 'returns': 'CurrentMachineParams' }
+
 ##
 # @CpuDefinitionInfo:
 #
diff --git 

Re: [Qemu-devel] [PATCH 09/11] aspeed/smc: add DMA calibration settings

[Peter Maydell]

On 31 August 2018 at 11:38, Cédric Le Goater  wrote:
> When doing calibration, the SPI clock rate in the CE0 Control Register
> and the read delay cycles in the Read Timing Compensation Register are
> replaced by bit[11:4] of the DMA Control Register.
>
> Signed-off-by: Cédric Le Goater 
> ---
>  hw/ssi/aspeed_smc.c | 54 +
>  1 file changed, 54 insertions(+)
>
> diff --git a/hw/ssi/aspeed_smc.c b/hw/ssi/aspeed_smc.c
> index 534faec4c111..983066f5ad1d 100644
> --- a/hw/ssi/aspeed_smc.c
> +++ b/hw/ssi/aspeed_smc.c
> @@ -695,6 +695,56 @@ static uint64_t aspeed_smc_read(void *opaque, hwaddr 
> addr, unsigned int size)
>  }
>  }
>
> +static uint8_t aspeed_smc_hclk_divisor(uint8_t hclk_mask)
> +{
> +/* HCLK/1 .. HCLK/16 */
> +const uint8_t hclk_divisors[] = {
> +15, 7, 14, 6, 13, 5, 12, 4, 11, 3, 10, 2, 9, 1, 8, 0
> +};
> +int i;
> +
> +for (i = 0; i < ARRAY_SIZE(hclk_divisors); i++) {
> +if (hclk_mask == hclk_divisors[i]) {
> +return i + 1;
> +}
> +}
> +
> +qemu_log_mask(LOG_GUEST_ERROR, "invalid HCLK mask %x", hclk_mask);
> +return 0;
> +}
> +
> +/*
> + * When doing calibration, the SPI clock rate in the CE0 Control
> + * Register and the read delay cycles in the Read Timing
> + * Compensation Register are replaced by bit[11:4] of the DMA
> + * Control Register.
> + */
> +static void aspeed_smc_dma_calibration(AspeedSMCState *s)
> +{
> +uint8_t delay =
> +(s->regs[R_DMA_CTRL] >> DMA_CTRL_DELAY_SHIFT) & DMA_CTRL_DELAY_MASK;
> +uint8_t hclk_mask =
> +(s->regs[R_DMA_CTRL] >> DMA_CTRL_FREQ_SHIFT) & DMA_CTRL_FREQ_MASK;
> +uint8_t hclk_div = aspeed_smc_hclk_divisor(hclk_mask);
> +uint32_t hclk_shift = (hclk_div - 1) << 2;
> +uint8_t cs;
> +
> +/* Only HCLK/1 - HCLK/5 have tunable delays */
> +if (hclk_div && hclk_div < 6) {
> +s->regs[s->r_timings] &= ~(0xf << hclk_shift);
> +s->regs[s->r_timings] |= delay << hclk_shift;
> +}
> +
> +/*
> + * TODO: choose CS depending on the DMA address. This is not used
> + * on the field.
> + */

Not entirely sure what you have in mind by "on the field" here?
Not used by Linux?

thanks
-- PMM

[Qemu-devel] [PATCH v9 0/3] wakeup-from-suspend and system_wakeup changes

[Daniel Henrique Barboza]

changes in v9, all proposed by Mike Roth:
- added a new 'qemu_register_wakeup_support' to be called by the wake-up
  implementations to register the support in vl.c (patch 1)
- changed versions from 3.0.0 to 3.1.0 (patch 1)
- added back the 'qemu_system_wakeup_request' call that was removed by
mistake in the previous version (patch 3)
- Previous series link:
https://lists.gnu.org/archive/html/qemu-devel/2018-07/msg01675.html


Daniel Henrique Barboza (3):
  qmp: query-current-machine with wakeup-suspend-support
  qga: update guest-suspend-ram and guest-suspend-hybrid descriptions
  qmp hmp: Make system_wakeup check wake-up support and run state

 hmp.c   |  5 -
 hw/acpi/core.c  |  1 +
 include/sysemu/sysemu.h |  2 ++
 qapi/misc.json  | 29 -
 qga/qapi-schema.json| 12 ++--
 qmp.c   | 10 ++
 vl.c| 19 +++
 7 files changed, 70 insertions(+), 8 deletions(-)

-- 
2.17.1

[Qemu-devel] [PATCH v9 3/3] qmp hmp: Make system_wakeup check wake-up support and run state

[Daniel Henrique Barboza]

The qmp/hmp command 'system_wakeup' is simply a direct call to
'qemu_system_wakeup_request' from vl.c. This function verifies if
runstate is SUSPENDED and if the wake up reason is valid before
proceeding. However, no error or warning is thrown if any of those
pre-requirements isn't met. There is no way for the caller to
differentiate between a successful wakeup or an error state caused
when trying to wake up a guest that wasn't suspended.

This means that system_wakeup is silently failing, which can be
considered a bug. Adding error handling isn't an API break in this
case - applications that didn't check the result will remain broken,
the ones that check it will have a chance to deal with it.

Adding to that, the commit before previous created a new QMP API called
query-current-machine, with a new flag called wakeup-suspend-support,
that indicates if the guest has the capability of waking up from suspended
state. Although such guest will never reach SUSPENDED state and erroring
it out in this scenario would suffice, it is more informative for the user
to differentiate between a failure because the guest isn't suspended versus
a failure because the guest does not have support for wake up at all.

All this considered, this patch changes qmp_system_wakeup to:

- check if the guest has the capability to wake-up from suspend. If
not, error out informing about the lack of support;

- make the runstate verification before proceeding to call
qemu_system_wakeup_request, firing up an error message if the user tries
to wake up a machine that isn't in SUSPENDED state.

After this patch, this is the output of system_wakeup in a guest that
does not have wake-up from suspend support (ppc64):

(qemu) system_wakeup
wake-up from suspend is not supported by this guest
(qemu)

And this is the output of system_wakeup in a x86 guest that has the
support but isn't suspended:

(qemu) system_wakeup
Unable to wake up: guest is not in suspended state
(qemu)

Reported-by: Balamuruhan S 
Signed-off-by: Daniel Henrique Barboza 
---
 hmp.c   |  5 -
 include/sysemu/sysemu.h |  1 +
 qapi/misc.json  |  5 -
 qmp.c   | 10 ++
 vl.c|  2 +-
 5 files changed, 20 insertions(+), 3 deletions(-)

diff --git a/hmp.c b/hmp.c
index 4975fa56b0..e98c24782f 100644
--- a/hmp.c
+++ b/hmp.c
@@ -1203,7 +1203,10 @@ void hmp_cont(Monitor *mon, const QDict *qdict)
 
 void hmp_system_wakeup(Monitor *mon, const QDict *qdict)
 {
-qmp_system_wakeup(NULL);
+Error *err = NULL;
+
+qmp_system_wakeup();
+hmp_handle_error(mon, );
 }
 
 void hmp_nmi(Monitor *mon, const QDict *qdict)
diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h
index 0446adacc6..8d91c5ad5b 100644
--- a/include/sysemu/sysemu.h
+++ b/include/sysemu/sysemu.h
@@ -74,6 +74,7 @@ void qemu_exit_preconfig_request(void);
 void qemu_system_reset_request(ShutdownCause reason);
 void qemu_system_suspend_request(void);
 void qemu_register_suspend_notifier(Notifier *notifier);
+bool qemu_wakeup_suspend_enabled(void);
 void qemu_system_wakeup_request(WakeupReason reason);
 void qemu_system_wakeup_enable(WakeupReason reason, bool enabled);
 void qemu_register_wakeup_notifier(Notifier *notifier);
diff --git a/qapi/misc.json b/qapi/misc.json
index b616d385a4..2f5995f45f 100644
--- a/qapi/misc.json
+++ b/qapi/misc.json
@@ -1230,7 +1230,10 @@
 ##
 # @system_wakeup:
 #
-# Wakeup guest from suspend.  Does nothing in case the guest isn't suspended.
+# If the guest has wake-up from suspend support enabled
+# (wakeup-suspend-support flag from query-current-machine), wakeup guest
+# from suspend if the guest is in SUSPENDED state. Returns an error
+# otherwise.
 #
 # Since:  1.1
 #
diff --git a/qmp.c b/qmp.c
index e7c0a2fd60..44e619d1be 100644
--- a/qmp.c
+++ b/qmp.c
@@ -183,6 +183,16 @@ void qmp_cont(Error **errp)
 
 void qmp_system_wakeup(Error **errp)
 {
+if (!qemu_wakeup_suspend_enabled()) {
+error_setg(errp,
+   "wake-up from suspend is not supported by this guest");
+return;
+}
+if (!runstate_check(RUN_STATE_SUSPENDED)) {
+error_setg(errp,
+   "Unable to wake up: guest is not in suspended state");
+return;
+}
 qemu_system_wakeup_request(QEMU_WAKEUP_REASON_OTHER);
 }
 
diff --git a/vl.c b/vl.c
index f78947f69c..35a33d61ba 100644
--- a/vl.c
+++ b/vl.c
@@ -1759,7 +1759,7 @@ void qemu_register_wakeup_support(void)
 wakeup_suspend_enabled = true;
 }
 
-static bool qemu_wakeup_suspend_enabled(void)
+bool qemu_wakeup_suspend_enabled(void)
 {
 return wakeup_suspend_enabled;
 }
-- 
2.17.1

[Qemu-devel] [PATCH v9 2/3] qga: update guest-suspend-ram and guest-suspend-hybrid descriptions

[Daniel Henrique Barboza]

This patch updates the descriptions of 'guest-suspend-ram' and
'guest-suspend-hybrid' to mention that both commands relies now
on the proper support for wake up from suspend, retrieved by the
'wakeup-suspend-support' attribute of the 'query-current-machine'
QMP command.

Reported-by: Balamuruhan S 
Signed-off-by: Daniel Henrique Barboza 
Reviewed-by: Michael Roth 
---
 qga/qapi-schema.json | 12 ++--
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/qga/qapi-schema.json b/qga/qapi-schema.json
index dfbc4a5e32..7ae7e2502c 100644
--- a/qga/qapi-schema.json
+++ b/qga/qapi-schema.json
@@ -567,9 +567,9 @@
 # For the best results it's strongly recommended to have the pm-utils
 # package installed in the guest.
 #
-# IMPORTANT: guest-suspend-ram requires QEMU to support the 'system_wakeup'
-# command.  Thus, it's *required* to query QEMU for the presence of the
-# 'system_wakeup' command before issuing guest-suspend-ram.
+# IMPORTANT: guest-suspend-ram requires working wakeup support in
+# QEMU. You *must* check QMP command query-current-machine returns
+# wakeup-suspend-support: true before issuing this command.
 #
 # This command does NOT return a response on success. There are two options
 # to check for success:
@@ -594,9 +594,9 @@
 #
 # This command requires the pm-utils package to be installed in the guest.
 #
-# IMPORTANT: guest-suspend-hybrid requires QEMU to support the 'system_wakeup'
-# command.  Thus, it's *required* to query QEMU for the presence of the
-# 'system_wakeup' command before issuing guest-suspend-hybrid.
+# IMPORTANT: guest-suspend-hybrid requires working wakeup support in
+# QEMU. You *must* check QMP command query-current-machine returns
+# wakeup-suspend-support: true before issuing this command.
 #
 # This command does NOT return a response on success. There are two options
 # to check for success:
-- 
2.17.1

Re: [Qemu-devel] [PATCH v2 02/12] util: add atomic64

[Peter Maydell]

On 18 September 2018 at 19:42, Emilio G. Cota  wrote:
> We already have these. For instance:
>
> - sTLB lookups can happen concurrently with invalidations
>   to the same sTLB from another core (via tlb_reset_dirty)
>
> - icount_decr is written to by cpu_exit, and is read
>   at the beginning of each TB (with a tcg_gen_ld_32 generated
>   by gen_tb_start).
>
> The latter is always a 32-bit access, so all hosts can generate
> it atomically. The former can be a 64-bit access if the guest
> is 64-bit, which cannot be done if !CONFIG_ATOMIC64. But we
> already disable mttcg for those combinations (we define
>  TCG_OVERSIZED_GUEST for this).

Does libatomic give us a firm guarantee that for 32-bit
types it will definitely produce an inline atomic access
insn that will interwork with what we're using? At the
moment our guard against this going wrong is that we don't
link against libatomic (so we get a compile failure if the
compiler decides to do something weird and out of line).

thanks
-- PMM

Re: [Qemu-devel] Freeze / spin in virtio blk...flatview do translate

[Frank Yang via Qemu-devel]

We also only get those reports from users with 4G RAM configured, so it
could also have to do with overflow.

On Tue, Sep 18, 2018 at 11:57 AM Frank Yang  wrote:

> That seems to be the case, since our 15 second detector is reset if the
> main loop runs its timers again, so no main loop iterations happened since
> that aio_dispatch_handlers call (we use a looper abstraction for it).
>
> On Tue, Sep 18, 2018 at 8:56 AM Paolo Bonzini  wrote:
>
>> On 15/09/2018 20:41, Frank Yang via Qemu-devel wrote:
>> > We have not reproduced this hang so far, this is from user crash reports
>> > that triggered our hang detector (where 15+ seconds pass without main
>> loop
>> > / VCPU threads being able to go back and ping their loopers in main
>> loop /
>> > vcpu threads.
>> >
>> > 0x0001024e9fcb(qemu-system-x86_64 -exec.c:511)flatview_translate
>> > 0x0001024f2390(qemu-system-x86_64
>> > -memory.h:1865)address_space_lduw_internal_cached
>> > 0x00010246ff11(qemu-system-x86_64
>> > -virtio-access.h:166)virtio_queue_set_notification
>> > 0x0001024fa2c9(qemu-system-x86_64+ 0x000a72c9)virtio_blk_handle_vq
>> > 0x0001024746ee(qemu-system-x86_64
>> > -virtio.c:1521)virtio_queue_host_notifier_aio_read
>> > 0x000103a5ed8a(qemu-system-x86_64
>> -aio-posix.c:406)aio_dispatch_handlers
>> > 0x000103a5ecc8(qemu-system-x86_64 -aio-posix.c:437)aio_dispatch
>> > 0x000103a5c158(qemu-system-x86_64 -async.c:261)aio_ctx_dispatch
>> > 0x000103a92103(qemu-system-x86_64
>> -gmain.c:3072)g_main_context_dispatch
>> > 0x000103a5e4ad(qemu-system-x86_64 -main-loop.c:224)main_loop_wait
>> > 0x000102468ab8(qemu-system-x86_64 -vl.c:2172)main_impl
>> > 0x000102461a3a(qemu-system-x86_64 -vl.c:3332)run_qemu_main
>> > 0x00010246eef3(qemu-system-x86_64
>> > -main.cpp:577)enter_qemu_main_loop(int, char**)
>> > 0x0001062b63a9(libQt5Core.5.dylib
>> > -qthread_unix.cpp:344)QThreadPrivate::start(void*)
>> > 0x7fff65118660
>> > 0x7fff6511850c
>> > 0x7fff65117bf8
>> > 0x0001062b623f(libQt5Core.5.dylib+ 0x0002623f)
>>
>> To be clear, is aio_dispatch_handlers running for 15+ seconds?
>>
>> None of the patches you point out are related however.
>>
>> Paolo
>>
>

Re: [Qemu-devel] [Bug 1793119] Re: Wrong floating-point emulation on AArch64 with FPCR set to zero

[Peter Maydell]

On 18 September 2018 at 19:18, Emilio G. Cota
<1793...@bugs.launchpad.net> wrote:
> Not yet. There should be a fix before 3.1 is released.
>
> Both 2.12 and 3.0 have this bug, so you might want to consider using
> 2.11 until the bug gets fixed.

On the other hand 2.11 has a different set of slightly-inaccurate-fp-result
bugs that are fixed in 3.0, so you get to pick your choice of bug there.

thanks
-- PMM

Re: [Qemu-devel] [PATCH v4 5/8] target/mips: R5900 DMULT[U], DDIV[U], LL, SC, LLD and SCD are user only

[Maciej W. Rozycki]

Hi Fredrik,

> I agree, that is important too. I will post an updated v5 soon. Another
> alternative change is to define check_insn_opc_user_only as
> 
> static inline void check_insn_opc_user_only(DisasContext *ctx, int flags)
> {
> #ifndef CONFIG_USER_ONLY
> check_insn_opc_removed(ctx, flags);
> #endif
> }
> 
> by referring to check_insn_opc_removed (instead of copying its definition).
> Would you consider this an improvement for v5 too?

 Yes, it does look like an improvement to me, reducing code duplication.  
Thanks for looking into it further.

> > here (and swapping the two former calls ought to be fixed separately; I 
> > haven't checked if there are more cases like this, but if so, then they 
> > would best be amended with a single change).
> 
> I'll defer other ordering and indentation fixes since I'm not sure whether
> such changes would be accepted.

 Sure, no need for you to rush doing that.  In the absence of someone 
willing to do such clean-ups voluntarily I would consider it a 
maintainer's duty really.

  Maciej

Re: [Qemu-devel] [PATCH 03/11] hw/arm/aspeed: Add an Aspeed machine class

[Peter Maydell]

On 31 August 2018 at 11:38, Cédric Le Goater  wrote:
> The code looks better, it removes duplicated lines and it will ease
> the introduction of common properties for the Aspeed machines.
>
> Signed-off-by: Cédric Le Goater 
> ---
>  include/hw/arm/aspeed.h |  46 +
>  hw/arm/aspeed.c | 212 +---
>  2 files changed, 116 insertions(+), 142 deletions(-)
>  create mode 100644 include/hw/arm/aspeed.h
>
> diff --git a/include/hw/arm/aspeed.h b/include/hw/arm/aspeed.h
> new file mode 100644
> index ..2b77f8d2b3c8
> --- /dev/null
> +++ b/include/hw/arm/aspeed.h
> @@ -0,0 +1,46 @@
> +/*
> + * Aspeed Machines
> + *
> + * Copyright 2018 IBM Corp.
> + *
> + * This code is licensed under the GPL version 2 or later.  See
> + * the COPYING file in the top-level directory.
> + */
> +#ifndef _ARM_ASPEED_H
> +#define _ARM_ASPEED_H

Leading underscore followed by capital is a C reserved
identifier. This should just be ARM_ASPEED_H.
Patch looks good otherwise.

thanks
--- PMM

Re: [Qemu-devel] [PATCH 08/11] aspeed/smc: add support for DMAs

[Peter Maydell]

On 31 August 2018 at 11:38, Cédric Le Goater  wrote:
> The FMC controller on the Aspeed SoCs support DMA to access the flash
> modules. It can operate in a normal mode, to copy to or from the flash
> module mapping window, or in a checksum calculation mode, to evaluate
> the best clock settings for reads.
>
> Our primary need is to support the checksum calculation mode and the
> model only implements synchronous DMA accesses. Something to improve
> in the future.
>
> Signed-off-by: Cédric Le Goater 

>
> +/*
> + * Accumulate the result of the reads to provide a checksum that will
> + * be used to validate the read timing settings.
> + */
> +static void aspeed_smc_dma_checksum(AspeedSMCState *s)
> +{
> +uint32_t data;
> +
> +if (s->regs[R_DMA_CTRL] & DMA_CTRL_WRITE) {
> +qemu_log_mask(LOG_GUEST_ERROR,
> +  "%s: invalid direction for DMA checksum\n",  __func__);
> +return;
> +}
> +
> +while (s->regs[R_DMA_LEN]) {
> +cpu_physical_memory_read(s->regs[R_DMA_FLASH_ADDR], , 4);

Ideally DMAing devices should not use cpu_physical_memory_*().
It's better to have the device take a MemoryRegion which it uses to
create an AddressSpace that it can use address_space_ld*. This (a)
allows you to correctly model that the DMAing device can't necessarily
see the same set of devices/memory that the CPU does and (b) detect
and handle read/write failures.

Commit 112a829f8f0ad is an example of converting a DMA controller from
old style cpu_physical_memory_*  to taking a MemoryRegion and using it.
(It doesn't do checking for read/write failure, but you should, by
passing a non-null MemTxResult* final argument.)

thanks
-- PMM

Re: [Qemu-devel] Freeze / spin in virtio blk...flatview do translate

[Frank Yang via Qemu-devel]

That seems to be the case, since our 15 second detector is reset if the
main loop runs its timers again, so no main loop iterations happened since
that aio_dispatch_handlers call (we use a looper abstraction for it).

On Tue, Sep 18, 2018 at 8:56 AM Paolo Bonzini  wrote:

> On 15/09/2018 20:41, Frank Yang via Qemu-devel wrote:
> > We have not reproduced this hang so far, this is from user crash reports
> > that triggered our hang detector (where 15+ seconds pass without main
> loop
> > / VCPU threads being able to go back and ping their loopers in main loop
> /
> > vcpu threads.
> >
> > 0x0001024e9fcb(qemu-system-x86_64 -exec.c:511)flatview_translate
> > 0x0001024f2390(qemu-system-x86_64
> > -memory.h:1865)address_space_lduw_internal_cached
> > 0x00010246ff11(qemu-system-x86_64
> > -virtio-access.h:166)virtio_queue_set_notification
> > 0x0001024fa2c9(qemu-system-x86_64+ 0x000a72c9)virtio_blk_handle_vq
> > 0x0001024746ee(qemu-system-x86_64
> > -virtio.c:1521)virtio_queue_host_notifier_aio_read
> > 0x000103a5ed8a(qemu-system-x86_64
> -aio-posix.c:406)aio_dispatch_handlers
> > 0x000103a5ecc8(qemu-system-x86_64 -aio-posix.c:437)aio_dispatch
> > 0x000103a5c158(qemu-system-x86_64 -async.c:261)aio_ctx_dispatch
> > 0x000103a92103(qemu-system-x86_64
> -gmain.c:3072)g_main_context_dispatch
> > 0x000103a5e4ad(qemu-system-x86_64 -main-loop.c:224)main_loop_wait
> > 0x000102468ab8(qemu-system-x86_64 -vl.c:2172)main_impl
> > 0x000102461a3a(qemu-system-x86_64 -vl.c:3332)run_qemu_main
> > 0x00010246eef3(qemu-system-x86_64
> > -main.cpp:577)enter_qemu_main_loop(int, char**)
> > 0x0001062b63a9(libQt5Core.5.dylib
> > -qthread_unix.cpp:344)QThreadPrivate::start(void*)
> > 0x7fff65118660
> > 0x7fff6511850c
> > 0x7fff65117bf8
> > 0x0001062b623f(libQt5Core.5.dylib+ 0x0002623f)
>
> To be clear, is aio_dispatch_handlers running for 15+ seconds?
>
> None of the patches you point out are related however.
>
> Paolo
>

Re: [Qemu-devel] [PATCH v2 02/12] util: add atomic64

[Richard Henderson]

On 9/18/18 12:04 PM, Peter Maydell wrote:
> Does libatomic give us a firm guarantee that for 32-bit
> types it will definitely produce an inline atomic access
> insn that will interwork with what we're using? At the
> moment our guard against this going wrong is that we don't
> link against libatomic (so we get a compile failure if the
> compiler decides to do something weird and out of line).

No, it does not.


r~

[Qemu-devel] [Bug 1793119] Re: Wrong floating-point emulation on AArch64 with FPCR set to zero

[Koutheir Attouchi]

Neither will be sufficient in my use case. IEEE-754 conformance is
essential. Thank you for the hints.

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

Title:
  Wrong floating-point emulation on AArch64 with FPCR set to zero

Status in QEMU:
  Confirmed

Bug description:
  On AArch64, with FPCR set to Zero (i.e., FPU set to IEEE-754 compliant
  mode), floating-point emulation does not produce the same results as
  real hardware (e.g., Raspberry Pi 3 with AArch64 Linux).

  I attached a sample that reproduces the issue. It divides `x` by `y`
  and puts the result in `r`. The expected result of the operation is
  `q`.

  Output on real hardware:
  =
  fpcr = 0x0700.
  x = 0x03250f416dcdc6d0. y = 0x00029f4e5837c977. r = 0x7ff0. q = 
0x43300fde9cbcf023.
  fpcr = 0x.
  x = 0x03250f416dcdc6d0. y = 0x00029f4e5837c977. r = 0x43300fde9cbcf023. q = 
0x43300fde9cbcf023.
  =

  Notice that after setting FPCR to zero, `r` equals `q`.

  Output on qemu 3.0.0 (Linux user-mode emulation):
  =
  fpcr = 0x0700.
  x = 0x03250f416dcdc6d0. y = 0x00029f4e5837c977. r = 0x7ff0. q = 
0x43300fde9cbcf023.
  fpcr = 0x.
  x = 0x03250f416dcdc6d0. y = 0x00029f4e5837c977. r = 0x43300fde9cbcf024. q = 
0x43300fde9cbcf023.
  =

  Notice that after setting FPCR to zero, `r` is not equal to `q`.

  Also notice that, using another proprietary operating system, the same
  issue arises between a real board and QEMU. This might be an issue in
  emulation of the AArch64 instruction "fdiv".

  Build command line: aarch64-linux-gnu-gcc -static -O0 -o sample1
  sample1.c

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

Re: [Qemu-devel] [PATCH v3] hw/arm: Add arm SBSA reference machine

[Peter Maydell]

On 9 September 2018 at 03:23, Hongbo Zhang  wrote:
> For the Aarch64, there is one machine 'virt', it is primarily meant to
> run on KVM and execute virtualization workloads, but we need an
> environment as faithful as possible to physical hardware, for supporting
> firmware and OS development for pysical Aarch64 machines.
>
> This patch introduces new machine type 'sbsa-ref' with main features:
>  - Based on 'virt' machine type.
>  - Re-designed memory map.
>  - EL2 and EL3 are enabled by default.
>  - GIC version 3 only.
>  - AHCI controller attached to system bus, and then CDROM and hard disc
>can be added to it.
>  - EHCI controller attached to system bus, with USB mouse and key board
>installed by default.
>  - E1000E ethernet card on PCIE bus.
>  - VGA display adaptor on PCIE bus.
>  - Default CPU type cortex-a57, 4 cores, and 1G bytes memory.
>  - No virtio functions enabled, since this is to emulate real hardware.
>  - No paravirtualized fw_cfg devicei either.
>  - No ACPI table, it should be supplied by firmware.
>
> Arm Trusted Firmware and UEFI porting to this are done accordingly.
>
> Signed-off-by: Hongbo Zhang 
> ---
> Changes since v2:
>  - rename the platform 'sbsa-ref'
>  - move all the codes to a separate file sbsa-ref.c
>  - remove paravirtualized fw_cfg device
>  - do not supply ACPI tables, since firmware will do it
>  - supply only necessary DT nodes
>  - and other minor code clean up
>
>  hw/arm/Makefile.objs  |2 +-
>  hw/arm/sbsa-ref.c | 1184 
> +
>  include/hw/arm/virt.h |2 +
>  3 files changed, 1187 insertions(+), 1 deletion(-)
>  create mode 100644 hw/arm/sbsa-ref.c
>
> diff --git a/hw/arm/Makefile.objs b/hw/arm/Makefile.objs
> index d51fcec..a8895eb 100644
> --- a/hw/arm/Makefile.objs
> +++ b/hw/arm/Makefile.objs
> @@ -1,4 +1,4 @@
> -obj-y += boot.o virt.o sysbus-fdt.o
> +obj-y += boot.o virt.o sbsa-ref.o sysbus-fdt.o
>  obj-$(CONFIG_ACPI) += virt-acpi-build.o
>  obj-$(CONFIG_DIGIC) += digic_boards.o
>  obj-$(CONFIG_EXYNOS4) += exynos4_boards.o
> diff --git a/hw/arm/sbsa-ref.c b/hw/arm/sbsa-ref.c
> new file mode 100644
> index 000..b5e19f4
> --- /dev/null
> +++ b/hw/arm/sbsa-ref.c
> @@ -0,0 +1,1184 @@
> +/*
> + * ARM SBSA Reference Platform emulation
> + *
> + * Copyright (c) 2018 Linaro Limited
> + * Written by Hongbo Zhang 
> + *
> + * Based on hw/arm/virt.c
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms and conditions of the GNU General Public License,
> + * version 2 or later, as published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope it will be useful, but WITHOUT
> + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
> + * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
> + * more details.
> + *
> + * You should have received a copy of the GNU General Public License along 
> with
> + * this program.  If not, see .
> + */
> +
> +#include "qemu/osdep.h"
> +#include "qapi/error.h"
> +#include "hw/sysbus.h"
> +#include "hw/arm/arm.h"
> +#include "hw/arm/primecell.h"
> +#include "hw/arm/virt.h"Please remove all the code you don't need, rather 
> than
just copin


> +#include "hw/devices.h"
> +#include "net/net.h"
> +#include "sysemu/device_tree.h"
> +#include "sysemu/numa.h"
> +#include "sysemu/sysemu.h"
> +#include "hw/compat.h"
> +#include "hw/loader.h"
> +#include "exec/address-spaces.h"
> +#include "qemu/bitops.h"
> +#include "qemu/error-report.h"
> +#include "hw/pci-host/gpex.h"
> +#include "hw/arm/sysbus-fdt.h"
> +#include "hw/platform-bus.h"
> +#include "hw/arm/fdt.h"
> +#include "hw/intc/arm_gic.h"
> +#include "hw/intc/arm_gicv3_common.h"
> +#include "kvm_arm.h"
> +#include "hw/smbios/smbios.h"
> +#include "qapi/visitor.h"
> +#include "standard-headers/linux/input.h"
> +#include "hw/arm/smmuv3.h"
> +#include "hw/ide/internal.h"
> +#include "hw/ide/ahci_internal.h"
> +#include "hw/usb.h"
> +#include "qemu/units.h"
> +
> +/* Number of external interrupt lines to configure the GIC with */
> +#define NUM_IRQS 256
> +
> +#define PLATFORM_BUS_NUM_IRQS 64
> +
> +#define SATA_NUM_PORTS 6
> +
> +#define RAMLIMIT_GB 255
> +#define RAMLIMIT_BYTES (RAMLIMIT_GB * 1024ULL * 1024 * 1024)
> +
> +static const MemMapEntry sbsa_ref_memmap[] = {
> +/* Space up to 0x800 is reserved for a boot ROM */
> +[VIRT_FLASH] =  {  0, 0x0800 },
> +[VIRT_CPUPERIPHS] = { 0x0800, 0x0002 },
> +/* GIC distributor and CPU interfaces sit inside the CPU peripheral 
> space */
> +[VIRT_GIC_DIST] =   { 0x0800, 0x0001 },
> +[VIRT_GIC_CPU] ={ 0x0801, 0x0001 },
> +[VIRT_GIC_V2M] ={ 0x0802, 0x1000 },
> +/* The space in between here is reserved for GICv3 CPU/vCPU/HYP */
> +[VIRT_GIC_ITS] ={ 0x0808, 0x0002 },
> +/* 

Re: [Qemu-devel] [PATCH v3] hw/arm: Add arm SBSA reference machine

[Peter Maydell]

On 18 September 2018 at 12:44, Peter Maydell  wrote:
> On 9 September 2018 at 03:23, Hongbo Zhang  wrote:
>> +#include "hw/arm/primecell.h"
>> +#include "hw/arm/virt.h"Please remove all the code you don't need, rather 
>> than
> just copin

Oops, please ignore this line, it is stray editor junk that I failed
to delete when writing my reply (my gmail window is very laggy here...)

thanks
-- PMM

[Qemu-devel] [Bug 1793183] [NEW] apt source --compile qemu-system-x86 fails on last ubuntu 18.04.1

[Dmitry Isaykin]

Public bug reported:

Error log:

/tmp/qemu-2.10+dfsg/util/memfd.c:40:12: error: static declaration of 
‘memfd_create’ follows non-static declaration
 static int memfd_create(const char *name, unsigned int flags)
^~~~
In file included from /usr/include/x86_64-linux-gnu/bits/mman-linux.h:115:0,
 from /usr/include/x86_64-linux-gnu/bits/mman.h:45,
 from /usr/include/x86_64-linux-gnu/sys/mman.h:41,
 from /tmp/qemu-2.10+dfsg/include/sysemu/os-posix.h:29,
 from /tmp/qemu-2.10+dfsg/include/qemu/osdep.h:104,
 from /tmp/qemu-2.10+dfsg/util/memfd.c:28:
/usr/include/x86_64-linux-gnu/bits/mman-shared.h:46:5: note: previous 
declaration of ‘memfd_create’ was here
 int memfd_create (const char *__name, unsigned int __flags) __THROW;
 ^~~~
/tmp/qemu-2.10+dfsg/rules.mak:66: recipe for target 'util/memfd.o' failed
make[1]: *** [util/memfd.o] Error 1
make[1]: *** Waiting for unfinished jobs
make[1]: Leaving directory '/tmp/qemu-2.10+dfsg/qemu-build'
debian/rules:121: recipe for target 'build-stamp' failed
make: *** [build-stamp] Error 2
dpkg-buildpackage: error: debian/rules build subprocess returned exit status 2

** Affects: qemu
 Importance: Undecided
 Status: New

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

Title:
  apt source --compile qemu-system-x86 fails on last ubuntu 18.04.1

Status in QEMU:
  New

Bug description:
  Error log:

  /tmp/qemu-2.10+dfsg/util/memfd.c:40:12: error: static declaration of 
‘memfd_create’ follows non-static declaration
   static int memfd_create(const char *name, unsigned int flags)
  ^~~~
  In file included from /usr/include/x86_64-linux-gnu/bits/mman-linux.h:115:0,
   from /usr/include/x86_64-linux-gnu/bits/mman.h:45,
   from /usr/include/x86_64-linux-gnu/sys/mman.h:41,
   from /tmp/qemu-2.10+dfsg/include/sysemu/os-posix.h:29,
   from /tmp/qemu-2.10+dfsg/include/qemu/osdep.h:104,
   from /tmp/qemu-2.10+dfsg/util/memfd.c:28:
  /usr/include/x86_64-linux-gnu/bits/mman-shared.h:46:5: note: previous 
declaration of ‘memfd_create’ was here
   int memfd_create (const char *__name, unsigned int __flags) __THROW;
   ^~~~
  /tmp/qemu-2.10+dfsg/rules.mak:66: recipe for target 'util/memfd.o' failed
  make[1]: *** [util/memfd.o] Error 1
  make[1]: *** Waiting for unfinished jobs
  make[1]: Leaving directory '/tmp/qemu-2.10+dfsg/qemu-build'
  debian/rules:121: recipe for target 'build-stamp' failed
  make: *** [build-stamp] Error 2
  dpkg-buildpackage: error: debian/rules build subprocess returned exit status 2

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

[Qemu-devel] [Bug 1793183] Re: apt source --compile qemu-system-x86 fails on last ubuntu 18.04.1

[Dmitry Isaykin]

** Patch added: "fix-memfd-conflict.patch"
   
https://bugs.launchpad.net/qemu/+bug/1793183/+attachment/5190282/+files/fix-memfd-conflict.patch

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

Title:
  apt source --compile qemu-system-x86 fails on last ubuntu 18.04.1

Status in QEMU:
  New

Bug description:
  Error log:

  /tmp/qemu-2.10+dfsg/util/memfd.c:40:12: error: static declaration of 
‘memfd_create’ follows non-static declaration
   static int memfd_create(const char *name, unsigned int flags)
  ^~~~
  In file included from /usr/include/x86_64-linux-gnu/bits/mman-linux.h:115:0,
   from /usr/include/x86_64-linux-gnu/bits/mman.h:45,
   from /usr/include/x86_64-linux-gnu/sys/mman.h:41,
   from /tmp/qemu-2.10+dfsg/include/sysemu/os-posix.h:29,
   from /tmp/qemu-2.10+dfsg/include/qemu/osdep.h:104,
   from /tmp/qemu-2.10+dfsg/util/memfd.c:28:
  /usr/include/x86_64-linux-gnu/bits/mman-shared.h:46:5: note: previous 
declaration of ‘memfd_create’ was here
   int memfd_create (const char *__name, unsigned int __flags) __THROW;
   ^~~~
  /tmp/qemu-2.10+dfsg/rules.mak:66: recipe for target 'util/memfd.o' failed
  make[1]: *** [util/memfd.o] Error 1
  make[1]: *** Waiting for unfinished jobs
  make[1]: Leaving directory '/tmp/qemu-2.10+dfsg/qemu-build'
  debian/rules:121: recipe for target 'build-stamp' failed
  make: *** [build-stamp] Error 2
  dpkg-buildpackage: error: debian/rules build subprocess returned exit status 2

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

[Qemu-devel] [Bug 1793183] Re: apt source --compile qemu-system-x86 fails on last ubuntu 18.04.1

[Dmitry Isaykin]

** Project changed: qemu => ubuntu

** Package changed: ubuntu => qemu (Ubuntu)

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

Title:
  apt source --compile qemu-system-x86 fails on last ubuntu 18.04.1

Status in qemu package in Ubuntu:
  New

Bug description:
  Error log:

  /tmp/qemu-2.10+dfsg/util/memfd.c:40:12: error: static declaration of 
‘memfd_create’ follows non-static declaration
   static int memfd_create(const char *name, unsigned int flags)
  ^~~~
  In file included from /usr/include/x86_64-linux-gnu/bits/mman-linux.h:115:0,
   from /usr/include/x86_64-linux-gnu/bits/mman.h:45,
   from /usr/include/x86_64-linux-gnu/sys/mman.h:41,
   from /tmp/qemu-2.10+dfsg/include/sysemu/os-posix.h:29,
   from /tmp/qemu-2.10+dfsg/include/qemu/osdep.h:104,
   from /tmp/qemu-2.10+dfsg/util/memfd.c:28:
  /usr/include/x86_64-linux-gnu/bits/mman-shared.h:46:5: note: previous 
declaration of ‘memfd_create’ was here
   int memfd_create (const char *__name, unsigned int __flags) __THROW;
   ^~~~
  /tmp/qemu-2.10+dfsg/rules.mak:66: recipe for target 'util/memfd.o' failed
  make[1]: *** [util/memfd.o] Error 1
  make[1]: *** Waiting for unfinished jobs
  make[1]: Leaving directory '/tmp/qemu-2.10+dfsg/qemu-build'
  debian/rules:121: recipe for target 'build-stamp' failed
  make: *** [build-stamp] Error 2
  dpkg-buildpackage: error: debian/rules build subprocess returned exit status 2

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

[Qemu-devel] [PATCH v9 9/9] qcow2: Explicit number replaced by a constant

[Leonid Bloch]

Signed-off-by: Leonid Bloch 
Reviewed-by: Alberto Garcia 
---
 block/qcow2.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/block/qcow2.c b/block/qcow2.c
index f885afa0ed..ffb4a9e4a1 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -1324,7 +1324,7 @@ static int coroutine_fn qcow2_do_open(BlockDriverState 
*bs, QDict *options,
 /* 2^(s->refcount_order - 3) is the refcount width in bytes */
 s->refcount_block_bits = s->cluster_bits - (s->refcount_order - 3);
 s->refcount_block_size = 1 << s->refcount_block_bits;
-bs->total_sectors = header.size / 512;
+bs->total_sectors = header.size / BDRV_SECTOR_SIZE;
 s->csize_shift = (62 - (s->cluster_bits - 8));
 s->csize_mask = (1 << (s->cluster_bits - 8)) - 1;
 s->cluster_offset_mask = (1LL << s->csize_shift) - 1;
@@ -3450,7 +3450,7 @@ static int coroutine_fn 
qcow2_co_truncate(BlockDriverState *bs, int64_t offset,
 goto fail;
 }
 
-old_length = bs->total_sectors * 512;
+old_length = bs->total_sectors * BDRV_SECTOR_SIZE;
 new_l1_size = size_to_l1(s, offset);
 
 if (offset < old_length) {
-- 
2.17.1

[Qemu-devel] [PATCH v9 1/9] qcow2: Options' documentation fixes

[Leonid Bloch]

Signed-off-by: Leonid Bloch 
---
 docs/qcow2-cache.txt | 20 +---
 qemu-options.hx  |  9 ++---
 2 files changed, 19 insertions(+), 10 deletions(-)

diff --git a/docs/qcow2-cache.txt b/docs/qcow2-cache.txt
index 8a09a5cc5f..013991e21c 100644
--- a/docs/qcow2-cache.txt
+++ b/docs/qcow2-cache.txt
@@ -77,7 +77,7 @@ aforementioned L2 cache, and its size can also be configured.
 Choosing the right cache sizes
 --
 In order to choose the cache sizes we need to know how they relate to
-the amount of allocated space.
+the amount of the allocated space.
 
 The amount of virtual disk that can be mapped by the L2 and refcount
 caches (in bytes) is:
@@ -86,7 +86,7 @@ caches (in bytes) is:
disk_size = refcount_cache_size * cluster_size * 8 / refcount_bits
 
 With the default values for cluster_size (64KB) and refcount_bits
-(16), that is
+(16), this becomes:
 
disk_size = l2_cache_size * 8192
disk_size = refcount_cache_size * 32768
@@ -97,12 +97,15 @@ need:
l2_cache_size = disk_size_GB * 131072
refcount_cache_size = disk_size_GB * 32768
 
-QEMU has a default L2 cache of 1MB (1048576 bytes) and a refcount
-cache of 256KB (262144 bytes), so using the formulas we've just seen
-we have
+For example, 1MB of L2 cache is needed to cover every 8 GB of the virtual
+image size (given that the default cluster size is used):
 
-   1048576 / 131072 = 8 GB of virtual disk covered by that cache
-262144 /  32768 = 8 GB
+   8 GB / 8192 = 1 MB
+
+A default refcount cache is 4 times the cluster size, which defaults to
+256 KB (262144 bytes). This is sufficient for 8 GB of image size:
+
+   262144 * 32768 = 8 GB
 
 
 How to configure the cache sizes
@@ -130,6 +133,9 @@ There are a few things that need to be taken into account:
memory as possible to the L2 cache before increasing the refcount
cache size.
 
+ - At most two of "l2-cache-size", "refcount-cache-size", and "cache-size"
+   can be set simultaneously.
+
 Unlike L2 tables, refcount blocks are not used during normal I/O but
 only during allocations and internal snapshots. In most cases they are
 accessed sequentially (even during random guest I/O) so increasing the
diff --git a/qemu-options.hx b/qemu-options.hx
index 654ef484d9..ab1a3b240e 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -742,15 +742,18 @@ image file)
 
 @item cache-size
 The maximum total size of the L2 table and refcount block caches in bytes
-(default: 1048576 bytes or 8 clusters, whichever is larger)
+(default: the sum of l2-cache-size and refcount-cache-size)
 
 @item l2-cache-size
 The maximum size of the L2 table cache in bytes
-(default: 4/5 of the total cache size)
+(default: if cache-size is not defined - 1048576 bytes or 8 clusters, whichever
+is larger; otherwise, as large as possible or needed within the cache-size,
+while permitting the requested or the minimal refcount cache size)
 
 @item refcount-cache-size
 The maximum size of the refcount block cache in bytes
-(default: 1/5 of the total cache size)
+(default: 4 times the cluster size; or if cache-size is specified, the part of
+it which is not used for the L2 cache)
 
 @item cache-clean-interval
 Clean unused entries in the L2 and refcount caches. The interval is in seconds.
-- 
2.17.1

[Qemu-devel] [PATCH v9 3/9] qcow2: Make sizes more humanly readable

[Leonid Bloch]

Signed-off-by: Leonid Bloch 
Reviewed-by: Alberto Garcia 
---
 block/qcow2.c | 2 +-
 block/qcow2.h | 9 +
 2 files changed, 6 insertions(+), 5 deletions(-)

diff --git a/block/qcow2.c b/block/qcow2.c
index ec9e6238a0..67cc82f0b9 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -830,7 +830,7 @@ static void read_cache_sizes(BlockDriverState *bs, QemuOpts 
*opts,
 }
 } else {
 if (!l2_cache_size_set) {
-*l2_cache_size = MAX(DEFAULT_L2_CACHE_BYTE_SIZE,
+*l2_cache_size = MAX(DEFAULT_L2_CACHE_SIZE,
  (uint64_t)DEFAULT_L2_CACHE_CLUSTERS
  * s->cluster_size);
 }
diff --git a/block/qcow2.h b/block/qcow2.h
index 81b844e936..a8d6f757b1 100644
--- a/block/qcow2.h
+++ b/block/qcow2.h
@@ -27,6 +27,7 @@
 
 #include "crypto/block.h"
 #include "qemu/coroutine.h"
+#include "qemu/units.h"
 
 //#define DEBUG_ALLOC
 //#define DEBUG_ALLOC2
@@ -43,11 +44,11 @@
 
 /* 8 MB refcount table is enough for 2 PB images at 64k cluster size
  * (128 GB for 512 byte clusters, 2 EB for 2 MB clusters) */
-#define QCOW_MAX_REFTABLE_SIZE 0x80
+#define QCOW_MAX_REFTABLE_SIZE S_8MiB
 
 /* 32 MB L1 table is enough for 2 PB images at 64k cluster size
  * (128 GB for 512 byte clusters, 2 EB for 2 MB clusters) */
-#define QCOW_MAX_L1_SIZE 0x200
+#define QCOW_MAX_L1_SIZE S_32MiB
 
 /* Allow for an average of 1k per snapshot table entry, should be plenty of
  * space for snapshot names and IDs */
@@ -75,9 +76,9 @@
 
 /* Whichever is more */
 #define DEFAULT_L2_CACHE_CLUSTERS 8 /* clusters */
-#define DEFAULT_L2_CACHE_BYTE_SIZE 1048576 /* bytes */
+#define DEFAULT_L2_CACHE_SIZE S_1MiB
 
-#define DEFAULT_CLUSTER_SIZE 65536
+#define DEFAULT_CLUSTER_SIZE S_64KiB
 
 
 #define QCOW2_OPT_LAZY_REFCOUNTS "lazy-refcounts"
-- 
2.17.1

[Qemu-devel] [PATCH v9 5/9] qcow2: Assign the L2 cache relatively to the image size

[Leonid Bloch]

Sufficient L2 cache can noticeably improve the performance when using
large images with frequent I/O.

Previously, unless 'cache-size' was specified and was large enough, the
L2 cache was set to a certain size without taking the virtual image size
into account.

Now, the L2 cache assignment is aware of the virtual size of the image,
and will cover the entire image, unless the cache size needed for that is
larger than a certain maximum. This maximum is set to 1 MB by default
(enough to cover an 8 GB image with the default cluster size) but can
be increased or decreased using the 'l2-cache-size' option. This option
was previously documented as the *maximum* L2 cache size, and this patch
makes it behave as such, instead of as a constant size. Also, the
existing option 'cache-size' can limit the sum of both L2 and refcount
caches, as previously.

Signed-off-by: Leonid Bloch 
---
 block/qcow2.c  | 21 +
 block/qcow2.h  |  4 +---
 docs/qcow2-cache.txt   | 15 ++-
 qemu-options.hx|  6 +++---
 tests/qemu-iotests/137 |  8 +++-
 tests/qemu-iotests/137.out |  4 +++-
 6 files changed, 33 insertions(+), 25 deletions(-)

diff --git a/block/qcow2.c b/block/qcow2.c
index 7949d15fc6..01c39c56c0 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -777,29 +777,35 @@ static void read_cache_sizes(BlockDriverState *bs, 
QemuOpts *opts,
  uint64_t *refcount_cache_size, Error **errp)
 {
 BDRVQcow2State *s = bs->opaque;
-uint64_t combined_cache_size;
+uint64_t combined_cache_size, l2_cache_max_setting;
 bool l2_cache_size_set, refcount_cache_size_set, combined_cache_size_set;
 int min_refcount_cache = MIN_REFCOUNT_CACHE_SIZE * s->cluster_size;
+uint64_t virtual_disk_size = bs->total_sectors * BDRV_SECTOR_SIZE;
+uint64_t max_l2_cache = virtual_disk_size / (s->cluster_size / 8);
 
 combined_cache_size_set = qemu_opt_get(opts, QCOW2_OPT_CACHE_SIZE);
 l2_cache_size_set = qemu_opt_get(opts, QCOW2_OPT_L2_CACHE_SIZE);
 refcount_cache_size_set = qemu_opt_get(opts, 
QCOW2_OPT_REFCOUNT_CACHE_SIZE);
 
 combined_cache_size = qemu_opt_get_size(opts, QCOW2_OPT_CACHE_SIZE, 0);
-*l2_cache_size = qemu_opt_get_size(opts, QCOW2_OPT_L2_CACHE_SIZE, 0);
+l2_cache_max_setting = qemu_opt_get_size(opts, QCOW2_OPT_L2_CACHE_SIZE,
+ DEFAULT_L2_CACHE_MAX_SIZE);
 *refcount_cache_size = qemu_opt_get_size(opts,
  QCOW2_OPT_REFCOUNT_CACHE_SIZE, 0);
 
 *l2_cache_entry_size = qemu_opt_get_size(
 opts, QCOW2_OPT_L2_CACHE_ENTRY_SIZE, s->cluster_size);
 
+*l2_cache_size = MIN(max_l2_cache, l2_cache_max_setting);
+
 if (combined_cache_size_set) {
 if (l2_cache_size_set && refcount_cache_size_set) {
 error_setg(errp, QCOW2_OPT_CACHE_SIZE ", " QCOW2_OPT_L2_CACHE_SIZE
" and " QCOW2_OPT_REFCOUNT_CACHE_SIZE " may not be set "
"at the same time");
 return;
-} else if (*l2_cache_size > combined_cache_size) {
+} else if (l2_cache_size_set &&
+   (l2_cache_max_setting > combined_cache_size)) {
 error_setg(errp, QCOW2_OPT_L2_CACHE_SIZE " may not exceed "
QCOW2_OPT_CACHE_SIZE);
 return;
@@ -814,9 +820,6 @@ static void read_cache_sizes(BlockDriverState *bs, QemuOpts 
*opts,
 } else if (refcount_cache_size_set) {
 *l2_cache_size = combined_cache_size - *refcount_cache_size;
 } else {
-uint64_t virtual_disk_size = bs->total_sectors * BDRV_SECTOR_SIZE;
-uint64_t max_l2_cache = virtual_disk_size / (s->cluster_size / 8);
-
 /* Assign as much memory as possible to the L2 cache, and
  * use the remainder for the refcount cache */
 if (combined_cache_size >= max_l2_cache + min_refcount_cache) {
@@ -828,12 +831,6 @@ static void read_cache_sizes(BlockDriverState *bs, 
QemuOpts *opts,
 *l2_cache_size = combined_cache_size - *refcount_cache_size;
 }
 }
-} else {
-if (!l2_cache_size_set) {
-*l2_cache_size = MAX(DEFAULT_L2_CACHE_SIZE,
- (uint64_t)DEFAULT_L2_CACHE_CLUSTERS
- * s->cluster_size);
-}
 }
 /* l2_cache_size and refcount_cache_size are ensured to have at least
  * their minimum values in qcow2_update_options_prepare() */
diff --git a/block/qcow2.h b/block/qcow2.h
index a8d6f757b1..2f8c1fd15c 100644
--- a/block/qcow2.h
+++ b/block/qcow2.h
@@ -74,9 +74,7 @@
 /* Must be at least 4 to cover all cases of refcount table growth */
 #define MIN_REFCOUNT_CACHE_SIZE 4 /* clusters */
 
-/* Whichever is more */
-#define DEFAULT_L2_CACHE_CLUSTERS 8 /* clusters */
-#define DEFAULT_L2_CACHE_SIZE S_1MiB
+#define DEFAULT_L2_CACHE_MAX_SIZE S_1MiB
 
 

Re: [Qemu-devel] [PATCH] fix setting the FPSCR[FR] bit

[Peter Maydell]

On 18 September 2018 at 15:34, Programmingkid  wrote:
> On Sep 17, 2018, at 7:46 PM, Peter Maydell  wrote:
>> See my previous email -- the spec suggests that "round" is
>> "inexact but not overflow".
>
> I couldn't find anything in my pdf document about round being defined as 
> inexact but not overflow. Could you send me a link to your document?

My suggestion was based on the doc at:
https://openpowerfoundation.org/?resource_lib=power-isa-version-3-0
which also lets you download the 2.07B spec, which says on page 115:

Floating-Point Fraction Inexact (FI)
 The last Arithmetic or Rounding and Conversion instruction either produced an
 inexact result during rounding or caused a disabled Overflow Exception.

which I took to mean that Inexact is "rounded or overflow", so rounded should be
"inexact but not overflow".

However, reading section 4.6 suggests that it's not quite that simple,
because FR is only set if the fraction part is *incremented*, whereas
Inexact covers also cases where the result is inexact but got rounded
down.

So I now think that we need to implement this flag properly in softfloat,
by having the code that does rounding and sets float_flag_inexact
also set float_flag_round in the correct places. I think also that
float_flag_round is a slightly confusing name for these semantics
when it's in the softfloat code rather than the ppc specific code,
because it isn't set only when the result is rounded, but when the
fractional part is specifically rounded up. Maybe float_flag_frac_inc ?

thanks
-- PMM

[Qemu-devel] [Bug 1793119] Re: Wrong floating-point emulation on AArch64 with FPCR set to zero

[Emilio G. Cota]

Thanks for your report. This is a known regression on our implementation
of f64_div, introduced by cf07323d49 ("fpu/softfloat: re-factor div",
2018-02-21).

We are working on improving FP tests to limit regressions, e.g. see this
thread, where the bug you report is first mentioned:
https://lists.gnu.org/archive/html/qemu-devel/2018-09/msg01703.html

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

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

Title:
  Wrong floating-point emulation on AArch64 with FPCR set to zero

Status in QEMU:
  Confirmed

Bug description:
  On AArch64, with FPCR set to Zero (i.e., FPU set to IEEE-754 compliant
  mode), floating-point emulation does not produce the same results as
  real hardware (e.g., Raspberry Pi 3 with AArch64 Linux).

  I attached a sample that reproduces the issue. It divides `x` by `y`
  and puts the result in `r`. The expected result of the operation is
  `q`.

  Output on real hardware:
  =
  fpcr = 0x0700.
  x = 0x03250f416dcdc6d0. y = 0x00029f4e5837c977. r = 0x7ff0. q = 
0x43300fde9cbcf023.
  fpcr = 0x.
  x = 0x03250f416dcdc6d0. y = 0x00029f4e5837c977. r = 0x43300fde9cbcf023. q = 
0x43300fde9cbcf023.
  =

  Notice that after setting FPCR to zero, `r` equals `q`.

  Output on qemu 3.0.0 (Linux user-mode emulation):
  =
  fpcr = 0x0700.
  x = 0x03250f416dcdc6d0. y = 0x00029f4e5837c977. r = 0x7ff0. q = 
0x43300fde9cbcf023.
  fpcr = 0x.
  x = 0x03250f416dcdc6d0. y = 0x00029f4e5837c977. r = 0x43300fde9cbcf024. q = 
0x43300fde9cbcf023.
  =

  Notice that after setting FPCR to zero, `r` is not equal to `q`.

  Also notice that, using another proprietary operating system, the same
  issue arises between a real board and QEMU. This might be an issue in
  emulation of the AArch64 instruction "fdiv".

  Build command line: aarch64-linux-gnu-gcc -static -O0 -o sample1
  sample1.c

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

[Qemu-devel] [PATCH v9 0/9] Take the image size into account when allocating the L2 cache

[Leonid Bloch]

Sorry for taking such a long pause after v8. I had several extremely
urgent issues to attend to.

This series makes the qcow2 L2 cache assignment aware of the image size,
with the intention for it to cover the entire image. The importance of
this change is in noticeable performance improvement, especially with
heavy random I/O. The memory overhead is not big in most cases, as only
1 MB of cache for every 8 GB of image size is used. For cases with very
large images and/or small cluster sizes, or systems with limited RAM
resources, there is an upper limit on the default L2 cache: 32 MB for
Linux systems, and 8 MB for non-Linux systems (the difference is caused
by the fact that it is currently impossible to perform scheduled cache
cleaning on non-Linux systems). To modify this upper limit one can use
the already existing 'l2-cache-size' and 'cache-size' options. Moreover,
this fixes the behavior of 'l2-cache-size', as it was documented as the
*maximum* L2 cache size, but in practice behaved as the absolute size.

To compensate the memory overhead which may (but not necesarily will) be
increased following this behavior, the default cache-clean-interval is set
to 10 minutes by default (was disabled by default before).

The L2 cache is also resized accordingly, by default, if the image is
resized.

Additionally, few minor changes are made (refactoring and documentation
fixes).

Differences from v1:
* .gitignore modification patch removed (unneeded).
* The grammar fix in conflicting cache sizing patch removed (merged).
* The update total_sectors when resizing patch squashed with the
  resizing patch.
* L2 cache is now capped at 32 MB.
* The default cache-clean-interval is set to 30 seconds.

Differences from v2:
* Made it clear in the documentation that setting cache-clean-interval
  to 0 disables this feature.

Differences from v3:
* The default cache-clean-interval is set to 10 minutes instead of 30
  seconds before.
* Commit message changes to better explain the patches.
* Some refactoring.

Differences from v4:
* Refactoring.
* Documentation and commit message fixes.

Differences from v5:
* A patch with cosmetic changes is split from the main patch
* A patch for avoiding duplication in refcount cache size assignment is
  split from the main patch.
* In the main patch the cap on the L2 cache size is set to only 1 MB (in
  order to be close to the previous behavior) and a separate patch sets
  it to 32 MB.
* Changes in the documentation fixes patch [1/8].

Differences from v6:
* A patch is added to make the defined sizes more humanly readable.

Differences from v7:
* Documentation and commit message changes.
* Fix: when cache-size is specified and is large enough, with the L2 and
  refcount caches not set, the L2 cache will be as large as needed to
  cover the entire image, and not as the default max. size. (Return to
  the previous behavior in this part, which was correct).
* Cosmetic changes patch not used.

Differences from v8:
* Made a lookup table for power-of-two sizes. This will enable writing
  pretty size values (most of the ones used are powers of two) while
  allowing correct stringification of these values.
* Made the max. L2 cache size 8 MB on non-Linux systems.
* Returned the test for too large L2 cache size.
* Minor documentation fixes.

Leonid Bloch (9):
  qcow2: Options' documentation fixes
  include: Add a lookup table of sizes
  qcow2: Make sizes more humanly readable
  qcow2: Avoid duplication in setting the refcount cache size
  qcow2: Assign the L2 cache relatively to the image size
  qcow2: Increase the default upper limit on the L2 cache size
  qcow2: Resize the cache upon image resizing
  qcow2: Set the default cache-clean-interval to 10 minutes
  qcow2: Explicit number replaced by a constant

 block/qcow2.c  | 42 -
 block/qcow2.h  | 16 ++-
 docs/qcow2-cache.txt   | 42 +++--
 include/qemu/units.h   | 55 ++
 qapi/block-core.json   |  3 ++-
 qemu-options.hx| 11 +---
 tests/qemu-iotests/137 |  8 +-
 tests/qemu-iotests/137.out |  4 ++-
 8 files changed, 136 insertions(+), 45 deletions(-)

-- 
2.17.1

Re: [Qemu-devel] Freeze / spin in virtio blk...flatview do translate

[Frank Yang via Qemu-devel]

And this one:

https://github.com/qemu/qemu/commit/a411c84b561baa94b28165c52f21c33517ee8f59

On Sat, Sep 15, 2018 at 4:42 PM Frank Yang  wrote:

> I notice at least two commits in upstream QEMU that might impact this:
>
>
> https://github.com/qemu/qemu/commit/ce3a9eaff4e5f29514dba35a001894cb7a238e07#diff-8bfe2ea13d8c6dab17a555f300ac2f66
>
> https://github.com/qemu/qemu/commit/45641dba38f6f44c3ea44c2d1c37b31619276ce3#diff-a9288ea1a561573c7d3036de7d7048e8
>
>
> On Sat, Sep 15, 2018 at 11:41 AM Frank Yang  wrote:
>
>> Hi qemu-devel,
>>
>> So we're using QEMU 2.12 for recent Android Emulator canaryies, and we're
>> seeing a lot of hangs on mac in flatview_translate in qemu 2.12.
>>
>> What would be some pointers for diagnosing excessive I/O?
>>
>> Especially, metrics to see if a system is on the verge of getting into
>> main loop spins.
>>
>> We have not reproduced this hang so far, this is from user crash reports
>> that triggered our hang detector (where 15+ seconds pass without main loop
>> / VCPU threads being able to go back and ping their loopers in main loop /
>> vcpu threads.
>>
>> 0x0001024e9fcb(qemu-system-x86_64 -exec.c:511)flatview_translate
>> 0x0001024f2390(qemu-system-x86_64
>> -memory.h:1865)address_space_lduw_internal_cached
>> 0x00010246ff11(qemu-system-x86_64
>> -virtio-access.h:166)virtio_queue_set_notification
>> 0x0001024fa2c9(qemu-system-x86_64+ 0x000a72c9)virtio_blk_handle_vq
>> 0x0001024746ee(qemu-system-x86_64
>> -virtio.c:1521)virtio_queue_host_notifier_aio_read
>> 0x000103a5ed8a(qemu-system-x86_64
>> -aio-posix.c:406)aio_dispatch_handlers
>> 0x000103a5ecc8(qemu-system-x86_64 -aio-posix.c:437)aio_dispatch
>> 0x000103a5c158(qemu-system-x86_64 -async.c:261)aio_ctx_dispatch
>> 0x000103a92103(qemu-system-x86_64
>> -gmain.c:3072)g_main_context_dispatch
>> 0x000103a5e4ad(qemu-system-x86_64 -main-loop.c:224)main_loop_wait
>> 0x000102468ab8(qemu-system-x86_64 -vl.c:2172)main_impl
>> 0x000102461a3a(qemu-system-x86_64 -vl.c:3332)run_qemu_main
>> 0x00010246eef3(qemu-system-x86_64
>> -main.cpp:577)enter_qemu_main_loop(int, char**)
>> 0x0001062b63a9(libQt5Core.5.dylib
>> -qthread_unix.cpp:344)QThreadPrivate::start(void*)
>> 0x7fff65118660
>> 0x7fff6511850c
>> 0x7fff65117bf8
>> 0x0001062b623f(libQt5Core.5.dylib+ 0x0002623f)
>>
>> Thanks,
>>
>> Frank
>>
>

[Qemu-devel] [PATCH v9 7/9] qcow2: Resize the cache upon image resizing

[Leonid Bloch]

The caches are now recalculated upon image resizing. This is done
because the new default behavior of assigning L2 cache relatively to
the image size, implies that the cache will be adapted accordingly
after an image resize.

Signed-off-by: Leonid Bloch 
Reviewed-by: Alberto Garcia 
---
 block/qcow2.c | 10 ++
 1 file changed, 10 insertions(+)

diff --git a/block/qcow2.c b/block/qcow2.c
index 01c39c56c0..1445cd5360 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -3418,6 +3418,7 @@ static int coroutine_fn 
qcow2_co_truncate(BlockDriverState *bs, int64_t offset,
 uint64_t old_length;
 int64_t new_l1_size;
 int ret;
+QDict *options;
 
 if (prealloc != PREALLOC_MODE_OFF && prealloc != PREALLOC_MODE_METADATA &&
 prealloc != PREALLOC_MODE_FALLOC && prealloc != PREALLOC_MODE_FULL)
@@ -3642,6 +3643,8 @@ static int coroutine_fn 
qcow2_co_truncate(BlockDriverState *bs, int64_t offset,
 }
 }
 
+bs->total_sectors = offset / BDRV_SECTOR_SIZE;
+
 /* write updated header.size */
 offset = cpu_to_be64(offset);
 ret = bdrv_pwrite_sync(bs->file, offsetof(QCowHeader, size),
@@ -3652,6 +3655,13 @@ static int coroutine_fn 
qcow2_co_truncate(BlockDriverState *bs, int64_t offset,
 }
 
 s->l1_vm_state_index = new_l1_size;
+
+/* Update cache sizes */
+options = qdict_clone_shallow(bs->options);
+ret = qcow2_update_options(bs, options, s->flags, errp);
+if (ret < 0) {
+goto fail;
+}
 ret = 0;
 fail:
 qemu_co_mutex_unlock(>lock);
-- 
2.17.1

[Qemu-devel] [PATCH v9 6/9] qcow2: Increase the default upper limit on the L2 cache size

[Leonid Bloch]

The upper limit on the L2 cache size is increased from 1 MB to 32 MB
on Linux platforms, and to 8 MB on other platforms (this difference is
caused by the ability to set intervals for cache cleaning on Linux
platforms only).

This is done in order to allow default full coverage with the L2 cache
for images of up to 256 GB in size (was 8 GB). Note, that only the
needed amount to cover the full image is allocated. The value which is
changed here is just the upper limit on the L2 cache size, beyond which
it will not grow, even if the size of the image will require it to.

Signed-off-by: Leonid Bloch 
---
 block/qcow2.h|  6 +-
 docs/qcow2-cache.txt | 15 +--
 qemu-options.hx  |  6 +++---
 3 files changed, 17 insertions(+), 10 deletions(-)

diff --git a/block/qcow2.h b/block/qcow2.h
index 2f8c1fd15c..0f0e3534bf 100644
--- a/block/qcow2.h
+++ b/block/qcow2.h
@@ -74,7 +74,11 @@
 /* Must be at least 4 to cover all cases of refcount table growth */
 #define MIN_REFCOUNT_CACHE_SIZE 4 /* clusters */
 
-#define DEFAULT_L2_CACHE_MAX_SIZE S_1MiB
+#ifdef CONFIG_LINUX
+#define DEFAULT_L2_CACHE_MAX_SIZE S_32MiB
+#else
+#define DEFAULT_L2_CACHE_MAX_SIZE S_8MiB
+#endif
 
 #define DEFAULT_CLUSTER_SIZE S_64KiB
 
diff --git a/docs/qcow2-cache.txt b/docs/qcow2-cache.txt
index c84cd69cc7..5965d3d094 100644
--- a/docs/qcow2-cache.txt
+++ b/docs/qcow2-cache.txt
@@ -124,12 +124,15 @@ There are a few things that need to be taken into account:
  - Both caches must have a size that is a multiple of the cluster size
(or the cache entry size: see "Using smaller cache sizes" below).
 
- - The maximum L2 cache size is 1 MB by default (enough for full coverage
-   of 8 GB images, with the default cluster size). This value can be
-   modified using the "l2-cache-size" option. QEMU will not use more memory
-   than needed to hold all of the image's L2 tables, regardless of this max.
-   value. The minimal L2 cache size is 2 clusters (or 2 cache entries, see
-   below).
+ - The maximum L2 cache size is 32 MB by default on Linux platforms (enough
+   for full coverage of 256 GB images, with the default cluster size). This
+   value can be modified using the "l2-cache-size" option. QEMU will not use
+   more memory than needed to hold all of the image's L2 tables, regardless
+   of this max. value.
+   On non-Linux platforms the maximal value is smaller by default (8 MB) and
+   this difference stems from the fact that on Linux the cache can be cleared
+   periodically if needed, using the "cache-clean-interval" option (see below).
+   The minimal L2 cache size is 2 clusters (or 2 cache entries, see below).
 
  - The default (and minimum) refcount cache size is 4 clusters.
 
diff --git a/qemu-options.hx b/qemu-options.hx
index f9fe43a4dc..d5f4bcadd4 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -746,9 +746,9 @@ The maximum total size of the L2 table and refcount block 
caches in bytes
 
 @item l2-cache-size
 The maximum size of the L2 table cache in bytes
-(default: if cache-size is not specified - 1M; otherwise, as large as possible
-within the cache-size, while permitting the requested or the minimal refcount
-cache size)
+(default: if cache-size is not specified - 32M on Linux platforms, and 8M on
+non-Linux platforms; otherwise, as large as possible within the cache-size,
+while permitting the requested or the minimal refcount cache size)
 
 @item refcount-cache-size
 The maximum size of the refcount block cache in bytes
-- 
2.17.1

[Qemu-devel] [PATCH v9 8/9] qcow2: Set the default cache-clean-interval to 10 minutes

[Leonid Bloch]

The default cache-clean-interval is set to 10 minutes, in order to lower
the overhead of the qcow2 caches (before the default was 0, i.e.
disabled).

Signed-off-by: Leonid Bloch 
Reviewed-by: Alberto Garcia 
---
 block/qcow2.c| 2 +-
 block/qcow2.h| 1 +
 docs/qcow2-cache.txt | 4 ++--
 qapi/block-core.json | 3 ++-
 qemu-options.hx  | 2 +-
 5 files changed, 7 insertions(+), 5 deletions(-)

diff --git a/block/qcow2.c b/block/qcow2.c
index 1445cd5360..f885afa0ed 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -944,7 +944,7 @@ static int qcow2_update_options_prepare(BlockDriverState 
*bs,
 /* New interval for cache cleanup timer */
 r->cache_clean_interval =
 qemu_opt_get_number(opts, QCOW2_OPT_CACHE_CLEAN_INTERVAL,
-s->cache_clean_interval);
+DEFAULT_CACHE_CLEAN_INTERVAL);
 #ifndef CONFIG_LINUX
 if (r->cache_clean_interval != 0) {
 error_setg(errp, QCOW2_OPT_CACHE_CLEAN_INTERVAL
diff --git a/block/qcow2.h b/block/qcow2.h
index 0f0e3534bf..8c863897b0 100644
--- a/block/qcow2.h
+++ b/block/qcow2.h
@@ -82,6 +82,7 @@
 
 #define DEFAULT_CLUSTER_SIZE S_64KiB
 
+#define DEFAULT_CACHE_CLEAN_INTERVAL 600  /* seconds */
 
 #define QCOW2_OPT_LAZY_REFCOUNTS "lazy-refcounts"
 #define QCOW2_OPT_DISCARD_REQUEST "pass-discard-request"
diff --git a/docs/qcow2-cache.txt b/docs/qcow2-cache.txt
index 5965d3d094..15ae797931 100644
--- a/docs/qcow2-cache.txt
+++ b/docs/qcow2-cache.txt
@@ -209,8 +209,8 @@ This example removes all unused cache entries every 15 
minutes:
 
-drive file=hd.qcow2,cache-clean-interval=900
 
-If unset, the default value for this parameter is 0 and it disables
-this feature.
+If unset, the default value for this parameter is 600. Setting it to 0
+disables this feature.
 
 Note that this functionality currently relies on the MADV_DONTNEED
 argument for madvise() to actually free the memory. This is a
diff --git a/qapi/block-core.json b/qapi/block-core.json
index 4c7a37afdc..08c27b9af7 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -2827,7 +2827,8 @@
 #
 # @cache-clean-interval:  clean unused entries in the L2 and refcount
 # caches. The interval is in seconds. The default value
-# is 0 and it disables this feature (since 2.5)
+# is 600, and 0 disables this feature. (since 2.5)
+#
 # @encrypt:   Image decryption options. Mandatory for
 # encrypted images, except when doing a metadata-only
 # probe of the image. (since 2.10)
diff --git a/qemu-options.hx b/qemu-options.hx
index d5f4bcadd4..2975fdf9f8 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -757,7 +757,7 @@ it which is not used for the L2 cache)
 
 @item cache-clean-interval
 Clean unused entries in the L2 and refcount caches. The interval is in seconds.
-The default value is 0 and it disables this feature.
+The default value is 600. Setting it to 0 disables this feature.
 
 @item pass-discard-request
 Whether discard requests to the qcow2 device should be forwarded to the data
-- 
2.17.1

Re: [Qemu-devel] [PATCH v3] linux-user: do setrlimit selectively

[Laurent Vivier]

Le 17/09/2018 à 20:13, Max Filippov a écrit :
> setrlimit guest calls that affect memory resources
> (RLIMIT_{AS,DATA,STACK}) may interfere with QEMU internal memory
> management. They may result in QEMU lockup because mprotect call in
> page_unprotect would fail with ENOMEM error code, causing infinite loop
> of SIGSEGV. E.g. it happens when running libstdc++ testsuite for xtensa
> target on x86_64 host.
> 
> Don't call host setrlimit for memory-related resources.
> 
> Reviewed-by: Peter Maydell 
> Signed-off-by: Max Filippov 
> ---
> Changes v2->v3:
> - add comment sugessted by Peter
> 
> Changes v1->v2:
> - don't limit change to 32-bit guest on 64-bit host case
> 
>  linux-user/syscall.c | 16 +++-
>  1 file changed, 15 insertions(+), 1 deletion(-)

rebased on master and applied on my branch linux-user-for-3.1

Thanks,
Laurent

[Qemu-devel] [PATCH v9 2/9] include: Add a lookup table of sizes

[Leonid Bloch]

Adding a lookup table for the powers of two, with the appropriate size
prefixes. This is needed when a size has to be stringified, in which
case something like '(1 * KiB)' would become a literal '(1 * (1L << 10))'
string. Powers of two are used very often for sizes, so such a table
will also make it easier and more intuitive to write them.

This table is generatred using the following AWK script:

BEGIN {
suffix="KMGTPE";
for(i=10; i<64; i++) {
val=2**i;
s=substr(suffix, int(i/10), 1);
n=2**(i%10);
pad=21-int(log(n)/log(10));
printf("#define S_%d%siB %*d\n", n, s, pad, val);
}
}

Signed-off-by: Leonid Bloch 
---
 include/qemu/units.h | 55 
 1 file changed, 55 insertions(+)

diff --git a/include/qemu/units.h b/include/qemu/units.h
index 692db3fbb2..68a7758650 100644
--- a/include/qemu/units.h
+++ b/include/qemu/units.h
@@ -17,4 +17,59 @@
 #define PiB (INT64_C(1) << 50)
 #define EiB (INT64_C(1) << 60)
 
+#define S_1KiB  1024
+#define S_2KiB  2048
+#define S_4KiB  4096
+#define S_8KiB  8192
+#define S_16KiB16384
+#define S_32KiB32768
+#define S_64KiB65536
+#define S_128KiB  131072
+#define S_256KiB  262144
+#define S_512KiB  524288
+#define S_1MiB   1048576
+#define S_2MiB   2097152
+#define S_4MiB   4194304
+#define S_8MiB   8388608
+#define S_16MiB 16777216
+#define S_32MiB 33554432
+#define S_64MiB 67108864
+#define S_128MiB   134217728
+#define S_256MiB   268435456
+#define S_512MiB   536870912
+#define S_1GiB1073741824
+#define S_2GiB2147483648
+#define S_4GiB4294967296
+#define S_8GiB8589934592
+#define S_16GiB  17179869184
+#define S_32GiB  34359738368
+#define S_64GiB  68719476736
+#define S_128GiB137438953472
+#define S_256GiB274877906944
+#define S_512GiB549755813888
+#define S_1TiB 1099511627776
+#define S_2TiB 219902322
+#define S_4TiB 4398046511104
+#define S_8TiB 8796093022208
+#define S_16TiB   17592186044416
+#define S_32TiB   35184372088832
+#define S_64TiB   70368744177664
+#define S_128TiB 140737488355328
+#define S_256TiB 281474976710656
+#define S_512TiB 562949953421312
+#define S_1PiB  1125899906842624
+#define S_2PiB  2251799813685248
+#define S_4PiB  4503599627370496
+#define S_8PiB  9007199254740992
+#define S_16PiB18014398509481984
+#define S_32PiB36028797018963968
+#define S_64PiB72057594037927936
+#define S_128PiB  144115188075855872
+#define S_256PiB  288230376151711744
+#define S_512PiB  576460752303423488
+#define S_1EiB   1152921504606846976
+#define S_2EiB   2305843009213693952
+#define S_4EiB   4611686018427387904
+#define S_8EiB   9223372036854775808
+
 #endif
-- 
2.17.1

[Qemu-devel] [PATCH v9 4/9] qcow2: Avoid duplication in setting the refcount cache size

[Leonid Bloch]

The refcount cache size does not need to be set to its minimum value in
read_cache_sizes(), as it is set to at least its minimum value in
qcow2_update_options_prepare().

Signed-off-by: Leonid Bloch 
Reviewed-by: Alberto Garcia 
---
 block/qcow2.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/block/qcow2.c b/block/qcow2.c
index 67cc82f0b9..7949d15fc6 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -834,10 +834,9 @@ static void read_cache_sizes(BlockDriverState *bs, 
QemuOpts *opts,
  (uint64_t)DEFAULT_L2_CACHE_CLUSTERS
  * s->cluster_size);
 }
-if (!refcount_cache_size_set) {
-*refcount_cache_size = min_refcount_cache;
-}
 }
+/* l2_cache_size and refcount_cache_size are ensured to have at least
+ * their minimum values in qcow2_update_options_prepare() */
 
 if (*l2_cache_entry_size < (1 << MIN_CLUSTER_BITS) ||
 *l2_cache_entry_size > s->cluster_size ||
-- 
2.17.1

Re: [Qemu-devel] [PATCH v10 6/6] tpm: add ACPI memory clear interface

[Marc-André Lureau]

Hi

On Tue, Sep 11, 2018 at 6:19 PM Laszlo Ersek  wrote:
>
> +Alex, due to mention of 21e00fa55f3fd
>
> On 09/10/18 15:03, Marc-André Lureau wrote:
> > Hi
> >
> > On Mon, Sep 10, 2018 at 2:44 PM Dr. David Alan Gilbert
> >  wrote:
> >> (I didn't know about guest_phys_block* and would have probably just used
> >> qemu_ram_foreach_block )
> >>
> >
> > guest_phys_block*() seems to fit, as it lists only the blocks actually
> > used, and already skip the device RAM.
> >
> > Laszlo, you wrote the functions
> > (https://git.qemu.org/?p=qemu.git;a=commit;h=c5d7f60f0614250bd925071e25220ce5958f75d0),
> > do you think it's appropriate to list the memory to clear, or we
> > should rather use qemu_ram_foreach_block() ?
>
> Originally, I would have said, "use either, doesn't matter". Namely,
> when I introduced the guest_phys_block*() functions, the original
> purpose was not related to RAM *contents*, but to RAM *addresses*
> (GPAs). This is evident if you look at the direct child commit of
> c5d7f60f0614, namely 56c4bfb3f07f, which put GuestPhysBlockList to use.
> And, for your use case (= wiping RAM), GPAs don't matter, only contents
> matter.
>
> However, with the commits I mentioned previously, namely e4dc3f5909ab9
> and 21e00fa55f3fd, we now filter out some RAM blocks from the dumping
> based on contents / backing as well. I think? So I believe we should
> honor that for the wiping to. I guess I'd (vaguely) suggest using
> guest_phys_block*().
>
> (And then, as Dave suggests, maybe extend the filter to consider pmem
> too, separately.)

I looked a bit into skipping pmem memory. The issue is that RamBlock
and MemoryRegion have no idea they are actually used for nvram (you
could rely on hostmem.pmem flag, but this is optional), and I don't
see a clear way to figure this out.

I can imagine to retrieve the MemoryRegion from guest phys address,
then check the owner is TYPE_NVDIMM for example. Is this a good
solution?

There is memory_region_from_host(), is there a memory_region_from_guest() ?

thanks


-- 
Marc-André Lureau

Re: [Qemu-devel] [PATCH v10 6/6] tpm: add ACPI memory clear interface

[Dr. David Alan Gilbert]

* Marc-André Lureau (marcandre.lur...@gmail.com) wrote:
> Hi
> 
> On Tue, Sep 11, 2018 at 6:19 PM Laszlo Ersek  wrote:
> >
> > +Alex, due to mention of 21e00fa55f3fd
> >
> > On 09/10/18 15:03, Marc-André Lureau wrote:
> > > Hi
> > >
> > > On Mon, Sep 10, 2018 at 2:44 PM Dr. David Alan Gilbert
> > >  wrote:
> > >> (I didn't know about guest_phys_block* and would have probably just used
> > >> qemu_ram_foreach_block )
> > >>
> > >
> > > guest_phys_block*() seems to fit, as it lists only the blocks actually
> > > used, and already skip the device RAM.
> > >
> > > Laszlo, you wrote the functions
> > > (https://git.qemu.org/?p=qemu.git;a=commit;h=c5d7f60f0614250bd925071e25220ce5958f75d0),
> > > do you think it's appropriate to list the memory to clear, or we
> > > should rather use qemu_ram_foreach_block() ?
> >
> > Originally, I would have said, "use either, doesn't matter". Namely,
> > when I introduced the guest_phys_block*() functions, the original
> > purpose was not related to RAM *contents*, but to RAM *addresses*
> > (GPAs). This is evident if you look at the direct child commit of
> > c5d7f60f0614, namely 56c4bfb3f07f, which put GuestPhysBlockList to use.
> > And, for your use case (= wiping RAM), GPAs don't matter, only contents
> > matter.
> >
> > However, with the commits I mentioned previously, namely e4dc3f5909ab9
> > and 21e00fa55f3fd, we now filter out some RAM blocks from the dumping
> > based on contents / backing as well. I think? So I believe we should
> > honor that for the wiping to. I guess I'd (vaguely) suggest using
> > guest_phys_block*().
> >
> > (And then, as Dave suggests, maybe extend the filter to consider pmem
> > too, separately.)
> 
> I looked a bit into skipping pmem memory. The issue is that RamBlock
> and MemoryRegion have no idea they are actually used for nvram (you
> could rely on hostmem.pmem flag, but this is optional), and I don't
> see a clear way to figure this out.

I think the pmem flag is what we should use; the problem though is we
have two different pieces of semantics:
a) PMEM - needs special flush instruction/calls
b) PMEM - my data is persistent, please don't clear me

Do those always go together?

(Copying in Junyan He who added the RAM_PMEM flag)

> I can imagine to retrieve the MemoryRegion from guest phys address,
> then check the owner is TYPE_NVDIMM for example. Is this a good
> solution?

No, I think it's upto whatever creates the region to set a flag
somewhere properly - there's no telling whether it'll always be NVDIMM
or some other object.

Dave

> There is memory_region_from_host(), is there a memory_region_from_guest() ?
> 
> thanks
> 
> 
> -- 
> Marc-André Lureau
--
Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK

Re: [Qemu-devel] [PATCH 1/1] qmp, hmp: add PCI subsystem id and vendor id to PCI info

[Eric Blake]

On 9/18/18 4:58 AM, Denis V. Lunev wrote:

This is a long story. RedHat has relicensed Windows KVM device drivers


s/RedHat/Red Hat/


in 2018 and there was an agreement that to avoid WHQL driver conflict
software manufacturers should set proper PCI subsystem vendor ID in
their distributions. Thus PCI subsystem vendor id becomes actively used.

The problem is that this field is applied by us via hardware compats.
Thus technically it could be lost.

This patch adds PCI susbsystem id and vendor id to exportable parameters
for validation.

Signed-off-by: Denis V. Lunev 
CC: "Dr. David Alan Gilbert" 
CC: Eric Blake 
CC: Markus Armbruster 
---


Reviewed-by: Eric Blake 

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

Re: [Qemu-devel] [PATCH v2 02/12] util: add atomic64

[Peter Maydell]

On 11 September 2018 at 21:43, Emilio G. Cota  wrote:
> On Tue, Sep 11, 2018 at 05:43:38 -0700, Richard Henderson wrote:
>> Is this any better than using libatomic?
>
> I didn't think of using libatomic. I just checked the source
> code and it's quite similar:
> - It uses 64 locks instead of 16 ($page_size/$cache_line,
>   but these are hard-coded for POSIX as 4096/64, respectively)
> - We compute the cacheline size and corresponding padding
>   at run-time, which is a little better than the above.
> - The locks are implemented as pthread_mutex instead of
>   spinlocks. I think spinlocks make more sense here because
>   we do not expect huge contention (systems without
>   !CONFIG_ATOMIC64 have few cores); for libatomic it makes
>   sense to use mutexes because it might be used in many-core
>   machines.
>
> So yes, we could have used libatomic. If you feel strongly
> about it, I can give it a shot.

I guess the question I have is:
 * will we need (now or in future) to emit atomic accesses
   into generated TCG code that need to interoperate with
   the atomic accesses we do from C code ?
 * if so, does libatomic provide a mechanism for doing that?

(maybe there's nothing you can do except to call out from
the generated code to C code anyway)

thanks
-- PMM

[Qemu-devel] qemu 2.12: qemu_coroutine_switch crash on Windows in SwitchToFiber (null CoroutineWin32*?)

[Frank Yang via Qemu-devel]

Hi qemu-devel,

We've been having crash reports in QEMU 2.12 on the anroid emulator in
SwitchToFiber that make it look like the coroutine or fiber getting
switched to is null.

Thread 16 (id: 0x13bc) CRASHED [EXCEPTION_ACCESS_VIOLATION_READ @
0x0010 ]
Stack Quality84%Show frame trust levels
0x7ffc1e694d90(KERNELBASE.dll+ 0x00064d90)SwitchToFiber
0x017b4b78(qemu-system-x86_64.exe
-coroutine-win32.c:59)qemu_coroutine_switch
0x017b3db6(qemu-system-x86_64.exe
-qemu-coroutine.c:161)qemu_aio_coroutine_enter
0x017a0a0f(qemu-system-x86_64.exe -async.c:496)aio_co_enter
0x005b76b2(qemu-system-x86_64.exe -block-backend.c:1364)blk_aio_prwv
0x005b77e7(qemu-system-x86_64.exe -block-backend.c:1458)blk_aio_preadv
0x00493b31(qemu-system-x86_64.exe
-virtio-blk.c:372)virtio_blk_submit_multireq
0x00494331(qemu-system-x86_64.exe -virtio-blk.c:620)virtio_blk_handle_vq
0x017a2aca(qemu-system-x86_64.exe -aio-win32.c:308)aio_dispatch_handlers
0x017a324e(qemu-system-x86_64.exe -aio-win32.c:355)aio_dispatch
0x017a02f8(qemu-system-x86_64.exe -async.c:261)aio_ctx_dispatch
0x017c64b8(qemu-system-x86_64.exe -gmain.c:3072)g_main_context_dispatch
0x017a2451(qemu-system-x86_64.exe -main-loop.c:496)os_host_main_loop_wait
0x017a2807(qemu-system-x86_64.exe -main-loop.c:530)main_loop_wait
0x00415016(qemu-system-x86_64.exe -vl.c:2172)main_impl
0x00415ad9(qemu-system-x86_64.exe -vl.c:3332)run_qemu_main
0x00415b54(qemu-system-x86_64.exe -main.cpp:586)enter_qemu_main_loop
0x6675de4b(Qt5Core.dll -qthread_win.cpp:391)QThreadPrivate::start
0x7ffc1fbbaa95(msvcrt.dll+ 0x0003aa95)beginthreadex
0x7ffc1fbbab6b(msvcrt.dll+ 0x0003ab6b)endthreadex
0x7ffc1feb3033(kernel32.dll+ 0x00013033)BaseThreadInitThunk
0x7ffc22161460(ntdll.dll+ 0x00071460)
0x7ffc1e71bf0f(KERNELBASE.dll+
0x000ebf0f)TerminateProcessOnMemoryExhaustion

Backing up the data flow a bit says that in qemu_aio_coroutine_enter,

Coroutine* to = QSIMPLEQ_FIRST();

and |to| is dererenced a few times before the call to qemu_coroutine_switch:

to->caller = from;
to->ctx = ctx;

smp_wmb();

qemu_coroutine_switch(from, to, COROUTINE_ENTER)

|to| is just some number so I don't think that is null. But what about
|to->fiber|? In coroutine-win32.c that's what's actually passed to
SwitchToFiber anyway:

CoroutineWin32 *to = DO_UPCAST(CoroutineWin32, base, to_); // just
arithmetic and it is a 64 bit process so I highly doubt anything is
under/overflowing

...

SwitchToFiber(to->fiber); // crash

I also see that the following functions affect fiber:

Coroutine *qemu_coroutine_new(void)
{
const size_t stack_size = COROUTINE_STACK_SIZE;
CoroutineWin32 *co;

co = g_malloc0(sizeof(*co));
co->fiber = CreateFiber(stack_size, coroutine_trampoline, >base);
return >base;
}

void qemu_coroutine_delete(Coroutine *co_)
{
CoroutineWin32 *co = DO_UPCAST(CoroutineWin32, base, co_);

DeleteFiber(co->fiber);
g_free(co);
}

Coroutine *qemu_coroutine_self(void)
{
Coroutine* res = QEMU_THREAD_LOCAL_GET(current);
if (!res) {
CoroutineWin32* pleader = QEMU_THREAD_LOCAL_GET_PTR(leader);
res = >base;
QEMU_THREAD_LOCAL_SET(current, res);
pleader->fiber = ConvertThreadToFiber(NULL);
}
return res;
}

which makes me think it could be a race condition with any of these three
functions or a failure in ConvertThreadToFiber. How likely is it that it is
a race condition? Or that the fiber was never initialized to begin with.

Frank

Re: [Qemu-devel] qemu 2.12: qemu_coroutine_switch crash on Windows in SwitchToFiber (null CoroutineWin32*?)

[Frank Yang via Qemu-devel]

BTW from https://bugs.launchpad.net/qemu/+bug/932487

that
says gcc version
is to blame, I don't think that's the case; we are using gcc 4.8 as well.
Perhaps it regressed. It would be helpful to know what kind of TLS fixees
for windows took place for QEMU and when.

On Tue, Sep 18, 2018 at 8:09 AM Frank Yang  wrote:

> Hi qemu-devel,
>
> We've been having crash reports in QEMU 2.12 on the anroid emulator in
> SwitchToFiber that make it look like the coroutine or fiber getting
> switched to is null.
>
> Thread 16 (id: 0x13bc) CRASHED [EXCEPTION_ACCESS_VIOLATION_READ @
> 0x0010 ]
> Stack Quality84%Show frame trust levels
> 0x7ffc1e694d90(KERNELBASE.dll+ 0x00064d90)SwitchToFiber
> 0x017b4b78(qemu-system-x86_64.exe
> -coroutine-win32.c:59)qemu_coroutine_switch
> 0x017b3db6(qemu-system-x86_64.exe
> -qemu-coroutine.c:161)qemu_aio_coroutine_enter
> 0x017a0a0f(qemu-system-x86_64.exe -async.c:496)aio_co_enter
> 0x005b76b2(qemu-system-x86_64.exe -block-backend.c:1364)blk_aio_prwv
> 0x005b77e7(qemu-system-x86_64.exe -block-backend.c:1458)blk_aio_preadv
> 0x00493b31(qemu-system-x86_64.exe
> -virtio-blk.c:372)virtio_blk_submit_multireq
> 0x00494331(qemu-system-x86_64.exe -virtio-blk.c:620)virtio_blk_handle_vq
> 0x017a2aca(qemu-system-x86_64.exe -aio-win32.c:308)aio_dispatch_handlers
> 0x017a324e(qemu-system-x86_64.exe -aio-win32.c:355)aio_dispatch
> 0x017a02f8(qemu-system-x86_64.exe -async.c:261)aio_ctx_dispatch
> 0x017c64b8(qemu-system-x86_64.exe -gmain.c:3072)g_main_context_dispatch
> 0x017a2451(qemu-system-x86_64.exe -main-loop.c:496)os_host_main_loop_wait
> 0x017a2807(qemu-system-x86_64.exe -main-loop.c:530)main_loop_wait
> 0x00415016(qemu-system-x86_64.exe -vl.c:2172)main_impl
> 0x00415ad9(qemu-system-x86_64.exe -vl.c:3332)run_qemu_main
> 0x00415b54(qemu-system-x86_64.exe -main.cpp:586)enter_qemu_main_loop
> 0x6675de4b(Qt5Core.dll -qthread_win.cpp:391)QThreadPrivate::start
> 0x7ffc1fbbaa95(msvcrt.dll+ 0x0003aa95)beginthreadex
> 0x7ffc1fbbab6b(msvcrt.dll+ 0x0003ab6b)endthreadex
> 0x7ffc1feb3033(kernel32.dll+ 0x00013033)BaseThreadInitThunk
> 0x7ffc22161460(ntdll.dll+ 0x00071460)
> 0x7ffc1e71bf0f(KERNELBASE.dll+
> 0x000ebf0f)TerminateProcessOnMemoryExhaustion
>
> Backing up the data flow a bit says that in qemu_aio_coroutine_enter,
>
> Coroutine* to = QSIMPLEQ_FIRST();
>
> and |to| is dererenced a few times before the call to
> qemu_coroutine_switch:
>
> to->caller = from;
> to->ctx = ctx;
>
> smp_wmb();
>
> qemu_coroutine_switch(from, to, COROUTINE_ENTER)
>
> |to| is just some number so I don't think that is null. But what about
> |to->fiber|? In coroutine-win32.c that's what's actually passed to
> SwitchToFiber anyway:
>
> CoroutineWin32 *to = DO_UPCAST(CoroutineWin32, base, to_); // just
> arithmetic and it is a 64 bit process so I highly doubt anything is
> under/overflowing
>
> ...
>
> SwitchToFiber(to->fiber); // crash
>
> I also see that the following functions affect fiber:
>
> Coroutine *qemu_coroutine_new(void)
> {
> const size_t stack_size = COROUTINE_STACK_SIZE;
> CoroutineWin32 *co;
>
> co = g_malloc0(sizeof(*co));
> co->fiber = CreateFiber(stack_size, coroutine_trampoline, >base);
> return >base;
> }
>
> void qemu_coroutine_delete(Coroutine *co_)
> {
> CoroutineWin32 *co = DO_UPCAST(CoroutineWin32, base, co_);
>
> DeleteFiber(co->fiber);
> g_free(co);
> }
>
> Coroutine *qemu_coroutine_self(void)
> {
> Coroutine* res = QEMU_THREAD_LOCAL_GET(current);
> if (!res) {
> CoroutineWin32* pleader = QEMU_THREAD_LOCAL_GET_PTR(leader);
> res = >base;
> QEMU_THREAD_LOCAL_SET(current, res);
> pleader->fiber = ConvertThreadToFiber(NULL);
> }
> return res;
> }
>
> which makes me think it could be a race condition with any of these three
> functions or a failure in ConvertThreadToFiber. How likely is it that it is
> a race condition? Or that the fiber was never initialized to begin with.
>
> Frank
>
>
>
>

Re: [Qemu-devel] [PATCH v2 12/17] mirror: Fix potential use-after-free in active commit

[Kevin Wolf]

Am 18.09.2018 um 16:11 hat Max Reitz geschrieben:
> On 17.09.18 13:37, Kevin Wolf wrote:
> > Am 17.09.2018 um 00:05 hat Max Reitz geschrieben:
> >> On 14.09.18 18:25, Kevin Wolf wrote:
> >>> Am 13.09.2018 um 22:55 hat Max Reitz geschrieben:
>  On 13.09.18 14:52, Kevin Wolf wrote:
> > When starting an active commit job, other callbacks can run before
> > mirror_start_job() calls bdrv_ref() where needed and cause the nodes to
> > go away. Add another pair of bdrv_ref/unref() around it to protect
> > against this case.
> >
> > Signed-off-by: Kevin Wolf 
> > ---
> >  block/mirror.c | 11 +++
> >  1 file changed, 11 insertions(+)
> 
>  Reviewed-by: Max Reitz 
> 
>  But...  How?
> >>>
> >>> Tried to reproduce the other bug Paolo was concerned about (good there
> >>> is something like 'git reflog'!) and dug up this one instead.
> >>>
> >>> So the scenario seems to be test_stream_commit_1 in qemu-iotests 030.
> >>>
> >>> The backtrace when the BDS is deleted is the following:
> >>>
> >>> (rr) bt
> >>> #0  0x7faeaf6145ec in __memset_sse2_unaligned_erms () at 
> >>> /lib64/libc.so.6
> >>> #1  0x7faeaf60414e in _int_free () at /lib64/libc.so.6
> >>> #2  0x7faeaf6093be in free () at /lib64/libc.so.6
> >>> #3  0x7faecaea6b4e in g_free () at /lib64/libglib-2.0.so.0
> >>> #4  0x55c0c94f9d6c in bdrv_delete (bs=0x55c0cbe69240) at block.c:3590
> >>> #5  0x55c0c94fbe82 in bdrv_unref (bs=0x55c0cbe69240) at block.c:4638
> >>> #6  0x55c0c94f6761 in bdrv_root_unref_child (child=0x55c0cbe57af0) at 
> >>> block.c:2188
> >>> #7  0x55c0c94fe239 in block_job_remove_all_bdrv (job=0x55c0ccf9e220) 
> >>> at blockjob.c:200
> >>> #8  0x55c0c94fdf1f in block_job_free (job=0x55c0ccf9e220) at 
> >>> blockjob.c:94
> >>> #9  0x55c0c94ffe0d in job_unref (job=0x55c0ccf9e220) at job.c:368
> >>> #10 0x55c0c95007cd in job_do_dismiss (job=0x55c0ccf9e220) at job.c:641
> >>> #11 0x55c0c95008d9 in job_conclude (job=0x55c0ccf9e220) at job.c:667
> >>> #12 0x55c0c9500b66 in job_finalize_single (job=0x55c0ccf9e220) at 
> >>> job.c:735
> >>> #13 0x55c0c94ff4fa in job_txn_apply (txn=0x55c0cbe19eb0, 
> >>> fn=0x55c0c9500a62 , lock=true) at job.c:151
> >>> #14 0x55c0c9500e92 in job_do_finalize (job=0x55c0ccf9e220) at 
> >>> job.c:822
> >>> #15 0x55c0c9501040 in job_completed_txn_success (job=0x55c0ccf9e220) 
> >>> at job.c:872
> >>> #16 0x55c0c950115c in job_completed (job=0x55c0ccf9e220, ret=0, 
> >>> error=0x0) at job.c:892
> >>> #17 0x55c0c92b572c in stream_complete (job=0x55c0ccf9e220, 
> >>> opaque=0x55c0cc050bc0) at block/stream.c:96
> >>
> >> But isn't this just deletion of node1, i.e. the intermediate node of the
> >> stream job?
> >>
> >> The commit job happens above that (node3 and upwards), so all its BDSs
> >> shouldn't be affected.  So even with the bdrv_ref()s introduced in this
> >> patch, I'd still expect node1 to be deleted exactly here.
> > 
> > Hm, I suppose that's true.
> > 
> > So it crashed because the drain in bdrv_reopen() didn't actually drain
> > the streaming job (which includes removing node1), so node1 ended up in
> > the ReopenQueue and when it disappeared in the middle of reopen, we got
> > a use after free.
> > 
> > Then it would be bug between job draining and bdrv_reopen() and the
> > active commit job would only be a victim of the bug without actually
> > doing anything wrong, and we can drop this patch.
> > 
> > Does this make sense?
> 
> The explanation makes sense, yes.
> 
> About dropping the patch...  We would have to be certain that you can
> only run a block job if none of the nodes involved can be deleted at any
> point.  After having started the job, this is the job's responsibility
> by having referenced everything it needs (through block_job_add_bdrv()).
>  So the critical point is when the job is starting, before it invoked
> that function.
> 
> The user cannot do any graph manipulation then because the monitor is
> blocked by starting the job.  So the only issue would be other
> operations that can make nodes go away without user intervention.
> Naturally the way to prevent conflicts would be with blockers, and
> currently our old job blockers should be sufficient to prevent them.
> But in theory we don't like them, so is this what we need the graph
> manipulation blocker for? :-)

I don't think so. Permissions are for users that are potentially
attached for a long time, not for protecting operations during which we
hold the BQL anyway.

Our problem here isn't that the graph is changing, but that nodes are
going away. The solution is to take references when we need them instead
of relying on someone else holding a reference for us when we don't know
who that someone is. Specifically, QMP commands might need to hold an
additional reference while working with a node.

An actual blocker for graph modification might make sense in the case of
a commit job, for example: If you break 

Re: [Qemu-devel] Dropped CPU feature names and backward compatibility

[Paolo Bonzini]

On 18/09/2018 16:22, Eduardo Habkost wrote:
> On Tue, Sep 18, 2018 at 04:02:54PM +0200, Paolo Bonzini wrote:
>> On 18/09/2018 15:14, Eduardo Habkost wrote:
>>> If it broke something, we should restore the option names and
>>> declare them as deprecated.
>>
>> I think in this particular case it's okay to add them back as no-ops,
>> especially we'd actually want them to be customizable for user-mode
>> emulation.
> 
> We can make the flag work on user-mode emulation, but I wouldn't
> like to keep QEMU silent if the option was explicitly set in the
> command-line in system emulation, because it means the user or
> some software component is really confused and trying to do
> something that is never going to work.

They just want to copy the host flags blindly into the guest.  osxsave
and ospke require some collaboration from the guest OS, but it should be
pretty harmless to have them on the command line, because in the end the
apps _should_ anyway be checking OSPKE.  If somebody is writing such an
app and is puzzled that OSPKE is missing, they should first of all ask
themselves if they have installed the right guest OS.

Paolo

Re: [Qemu-devel] Freeze / spin in virtio blk...flatview do translate

[Paolo Bonzini]

On 15/09/2018 20:41, Frank Yang via Qemu-devel wrote:
> We have not reproduced this hang so far, this is from user crash reports
> that triggered our hang detector (where 15+ seconds pass without main loop
> / VCPU threads being able to go back and ping their loopers in main loop /
> vcpu threads.
> 
> 0x0001024e9fcb(qemu-system-x86_64 -exec.c:511)flatview_translate
> 0x0001024f2390(qemu-system-x86_64
> -memory.h:1865)address_space_lduw_internal_cached
> 0x00010246ff11(qemu-system-x86_64
> -virtio-access.h:166)virtio_queue_set_notification
> 0x0001024fa2c9(qemu-system-x86_64+ 0x000a72c9)virtio_blk_handle_vq
> 0x0001024746ee(qemu-system-x86_64
> -virtio.c:1521)virtio_queue_host_notifier_aio_read
> 0x000103a5ed8a(qemu-system-x86_64 -aio-posix.c:406)aio_dispatch_handlers
> 0x000103a5ecc8(qemu-system-x86_64 -aio-posix.c:437)aio_dispatch
> 0x000103a5c158(qemu-system-x86_64 -async.c:261)aio_ctx_dispatch
> 0x000103a92103(qemu-system-x86_64 -gmain.c:3072)g_main_context_dispatch
> 0x000103a5e4ad(qemu-system-x86_64 -main-loop.c:224)main_loop_wait
> 0x000102468ab8(qemu-system-x86_64 -vl.c:2172)main_impl
> 0x000102461a3a(qemu-system-x86_64 -vl.c:3332)run_qemu_main
> 0x00010246eef3(qemu-system-x86_64
> -main.cpp:577)enter_qemu_main_loop(int, char**)
> 0x0001062b63a9(libQt5Core.5.dylib
> -qthread_unix.cpp:344)QThreadPrivate::start(void*)
> 0x7fff65118660
> 0x7fff6511850c
> 0x7fff65117bf8
> 0x0001062b623f(libQt5Core.5.dylib+ 0x0002623f)

To be clear, is aio_dispatch_handlers running for 15+ seconds?

None of the patches you point out are related however.

Paolo

Re: [Qemu-devel] [qemu-s390x] [PATCH v8 3/6] s390x/kvm: enable/disable AP instruction interpretation for guest

[Tony Krowiak]

On 09/17/2018 04:43 AM, David Hildenbrand wrote:

Am 12.09.18 um 22:08 schrieb Tony Krowiak:

From: Tony Krowiak 

Let's use the KVM_SET_DEVICE_ATTR ioctl to enable or disable
hardware interpretation of AP instructions executed on the guest.
If the S390_FEAT_AP feature is installed, AP instructions will
be interpreted by default; otherwise, they will be intercepted.
This attribute setting may be overridden by a device. For example,
a device may want to provide AP instructions to the guest (i.e.,
S390_FEAT_AP turned on), but it may want to emulate them. In this
case, the AP instructions executed on the guest must be
intercepted; so when the device is realized, it must disable
interpretation.

Signed-off-by: Tony Krowiak 
---
  target/s390x/kvm.c |   16 
  1 files changed, 16 insertions(+), 0 deletions(-)

diff --git a/target/s390x/kvm.c b/target/s390x/kvm.c
index c4bd84d..28a3900 100644
--- a/target/s390x/kvm.c
+++ b/target/s390x/kvm.c
@@ -2297,6 +2297,19 @@ void kvm_s390_get_host_cpu_model(S390CPUModel *model, 
Error **errp)
 S390_FEAT_MAX);
  }
  
+static void kvm_s390_configure_apie(int interpret)

+{
+uint64_t attr = KVM_S390_VM_CRYPTO_DISABLE_APIE;
+
+if (interpret) {
+attr = KVM_S390_VM_CRYPTO_ENABLE_APIE;
+}
+
+if (kvm_vm_check_attr(kvm_state, KVM_S390_VM_CRYPTO, attr)) {
+kvm_s390_set_attr(attr);
+}
+}
+
  void kvm_s390_apply_cpu_model(const S390CPUModel *model, Error **errp)
  {
  struct kvm_s390_vm_cpu_processor prop  = {
@@ -2346,6 +2359,9 @@ void kvm_s390_apply_cpu_model(const S390CPUModel *model, 
Error **errp)
  if (test_bit(S390_FEAT_CMM, model->features)) {
  kvm_s390_enable_cmma();
  }
+
+/* configure hardware interpretation of AP instructions */
+kvm_s390_configure_apie(test_bit(S390_FEAT_AP, model->features));
  }
  

As it is off as default,

if (test_bit(S390_FEAT_AP, model->features)) {
kvm_s390_configure_apie(true, model->features));
}

will save one ioctl without AP when starting a guest.


As mentioned as replay to patch #2, instead of KVM_S390_VM_CPU_FEAT_AP,
I think we can simply do the following in kvm_s390_get_host_cpu_model()


/* for now, we can only provide the AP feature with HW support */
if (kvm_vm_check_attr(kvm_state, KVM_S390_VM_CRYPTO,
 KVM_S390_VM_CRYPTO_ENABLE_APIE)) {
set_bit(S390_FEAT_AP, model->features);
}


Apart from that, looks good to me.


Let me summarize what I think you are suggesting.

For KVM:
1. Get rid of KVM_S390_VM_CPU_FEAT_AP in KVM
2. Make AP instruction interception the default.
3. Provide the KVM_S390_VM_CRYPTO_ENABLE_APIE attribute in KVM if the
   AP instructions are installed on the host

For QEMU:
1. In the kvm_s390_get_host_cpu_model(), set the S390_FEAT_AP CPU model
   feature bit if the KVM_S390_VM_CRYPTO_ENABLE_APIE attribute is available
   in KVM.
2. In the kvm_s390_apply_cpu_model() function, if the S390_FEAT_AP bit is
   set in the guest's CPU model (i.e., ap=on), use the KVM_SET_DEVICE_ATTR
   ioctl to set the KVM_S390_VM_CRYPTO_ENABLE_APIE attribute. This will
   ultimately result in ECA.28 being set to 1 (i.e., interpret AP instructions)
   for each vcpu.
3. Down the road, if a QEMU device for emulating AP is used, the
   KVM_S390_VM_CRYPTO_DISABLE_APIE attribute can be set to instruct KVM to
   intercept AP instructions. This can be done when the device is realized.

I've discussed this with Halil -- Pierre is out until next week. We
are in agreement that while these changes are viable, they result in
a slightly more complicated implementation compared to previous versions (e.g.
kernel v9 QEMU v7), and locks us into a model that may limit design choices
down the road if/when virtual and/or emulated AP devices are implemented.

Having said that, your proposal makes perfect sense under the assumptions that:
1) The default for ECA.28 has to be zero even if the guest is supposed to have
AP instructions installed, and
2) QEMU needs to set ECA.28 via ioctl.

I don't think, however, those assumptions are necessarily justified. What we
get does not expand on what we had in any meaningful way, but forces us to add
the two new KVM attributes (KVM_S390_VM_CRYPTO_ENABLE_APIE and 
KVM_S390_VM_CRYPTO_DISABLE_APIE)
which limits our choices for how we implement future enhancements to the AP
virtualization architecture.

Let's recap the previous behavior case by case and compare it with the one
proposed by you:

1) ap=off in the requested model:
   --> ECA.28 not set
   --> vfio-ap device has no bus to plug into in QEMU
   --> QEMU injects operation exceptions
2) ap=on in the requested model
 2.1)  if host does not have ap instr:
   --> KVM_S390_VM_CPU_FEAT_AP not advertised
   --> since we don't provide emulation compatibility fails
   --> guest does not start
   --> ECA.28 does not matter but remains 0
 

[Qemu-devel] [Bug 1793119] Re: Wrong floating-point emulation on AArch64 with FPCR set to zero

[Koutheir Attouchi]

Thanks for the update. Is there a fix/patch for the 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/1793119

Title:
  Wrong floating-point emulation on AArch64 with FPCR set to zero

Status in QEMU:
  Confirmed

Bug description:
  On AArch64, with FPCR set to Zero (i.e., FPU set to IEEE-754 compliant
  mode), floating-point emulation does not produce the same results as
  real hardware (e.g., Raspberry Pi 3 with AArch64 Linux).

  I attached a sample that reproduces the issue. It divides `x` by `y`
  and puts the result in `r`. The expected result of the operation is
  `q`.

  Output on real hardware:
  =
  fpcr = 0x0700.
  x = 0x03250f416dcdc6d0. y = 0x00029f4e5837c977. r = 0x7ff0. q = 
0x43300fde9cbcf023.
  fpcr = 0x.
  x = 0x03250f416dcdc6d0. y = 0x00029f4e5837c977. r = 0x43300fde9cbcf023. q = 
0x43300fde9cbcf023.
  =

  Notice that after setting FPCR to zero, `r` equals `q`.

  Output on qemu 3.0.0 (Linux user-mode emulation):
  =
  fpcr = 0x0700.
  x = 0x03250f416dcdc6d0. y = 0x00029f4e5837c977. r = 0x7ff0. q = 
0x43300fde9cbcf023.
  fpcr = 0x.
  x = 0x03250f416dcdc6d0. y = 0x00029f4e5837c977. r = 0x43300fde9cbcf024. q = 
0x43300fde9cbcf023.
  =

  Notice that after setting FPCR to zero, `r` is not equal to `q`.

  Also notice that, using another proprietary operating system, the same
  issue arises between a real board and QEMU. This might be an issue in
  emulation of the AArch64 instruction "fdiv".

  Build command line: aarch64-linux-gnu-gcc -static -O0 -o sample1
  sample1.c

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

Re: [Qemu-devel] [PATCH v4 5/8] target/mips: R5900 DMULT[U], DDIV[U], LL, SC, LLD and SCD are user only

[Fredrik Noring]

Hi Maciej,

Philippe -- thank you for your reviews,

On Mon, Sep 17, 2018 at 06:10:27PM +0100, Maciej W. Rozycki wrote:
>  Nitpicking here, but I think it's what makes code clean and pleasant to 
> read.

I agree, that is important too. I will post an updated v5 soon. Another
alternative change is to define check_insn_opc_user_only as

static inline void check_insn_opc_user_only(DisasContext *ctx, int flags)
{
#ifndef CONFIG_USER_ONLY
check_insn_opc_removed(ctx, flags);
#endif
}

by referring to check_insn_opc_removed (instead of copying its definition).
Would you consider this an improvement for v5 too?

>  I think it would make sense to keep the order of the checks consistent, 
> or otherwise code starts looking messy.
> 
>  The predominant order appears to be (as applicable) starting with a 
> check for the lowest ISA membership, followed by a check for the highest 
> ISA membership (R6 instruction cuts) and ending with processor-specific 
> special cases.  I think this order makes sense in that it starts with 
> checks that have a wider scope and then moves on to ones of a narrower 
> scope, and I think keeping it would be good (as would fixing where the 
> addition of R6 broke it).  Mode checks for otherwise existing 
> instructions then follow, which complements the pattern.

Sure, thanks for clarifying the ordering rules!

>  So please make it:
> 
> check_insn(ctx, ISA_MIPS3);
> check_insn_opc_user_only(ctx, INSN_R5900);
> check_mips_64(ctx);

Done.

> > @@ -25275,6 +25280,7 @@ static void decode_opc(CPUMIPSState *env, 
> > DisasContext *ctx)
> >  break;
> >  case OPC_SCD:
> >  check_insn_opc_removed(ctx, ISA_MIPS32R6);
> > +check_insn_opc_user_only(ctx, INSN_R5900);
> >  check_insn(ctx, ISA_MIPS3);
> >  check_mips_64(ctx);
> >  gen_st_cond(ctx, op, rt, rs, imm);
> 
>  And please make it:
> 
> check_insn_opc_removed(ctx, ISA_MIPS32R6);
> check_insn(ctx, ISA_MIPS3);
> check_insn_opc_user_only(ctx, INSN_R5900);
> check_mips_64(ctx);

Done.

> here (and swapping the two former calls ought to be fixed separately; I 
> haven't checked if there are more cases like this, but if so, then they 
> would best be amended with a single change).

I'll defer other ordering and indentation fixes since I'm not sure whether
such changes would be accepted.

Fredrik

Re: [Qemu-devel] [PATCH] tests/migration: Speed up the test on ppc64

[Laurent Vivier]

On 17/09/2018 19:12, Thomas Huth wrote:
> The SLOF boot process is always quite slow ... but we can speed it up
> a little bit by specifying "-nodefaults" and by using the "nvramrc"
> variable instead of "boot-command" (since "nvramrc" is evaluated earlier
> in the SLOF boot process than "boot-command").
> 
> Signed-off-by: Thomas Huth 
> ---
>  tests/migration-test.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/tests/migration-test.c b/tests/migration-test.c
> index 0e687b7..967e3d0 100644
> --- a/tests/migration-test.c
> +++ b/tests/migration-test.c
> @@ -438,11 +438,11 @@ static int test_migrate_start(QTestState **from, 
> QTestState **to,
>" -incoming %s",
>accel, tmpfs, bootpath, uri);
>  } else if (strcmp(arch, "ppc64") == 0) {
> -cmd_src = g_strdup_printf("-machine accel=%s -m 256M"
> +cmd_src = g_strdup_printf("-machine accel=%s -m 256M -nodefaults"
>" -name source,debug-threads=on"
>" -serial file:%s/src_serial"
> -  " -prom-env '"
> -  "boot-command=hex .\" _\" begin %x %x "
> +  " -prom-env 'use-nvramrc?=true' -prom-env "
> +  "'nvramrc=hex .\" _\" begin %x %x "
>"do i c@ 1 + i c! 1000 +loop .\" B\" 0 "
>"until'",  accel, tmpfs, end_address,
>start_address);
> 

Reviewed-by: Laurent Vivier 

[Qemu-devel] [PATCH 00/10] target/xtensa updates

[Max Filippov]

Hi Peter,

please pull the following batch of target/xtensa updates:

The following changes since commit 0abaa41d936becd914a16ee1fe2a981d96d19428:

  Merge remote-tracking branch 'remotes/ehabkost/tags/x86-next-pull-request' 
into staging (2018-08-17 09:46:00 +0100)

are available in the git repository at:

  git://github.com/OSLL/qemu-xtensa.git tags/20180918-xtensa

for you to fetch changes up to 5aa37f488fa22c07495edbc04aa63812fbcdb79c:

  target/xtensa: support input from chardev console (2018-09-17 11:09:04 -0700)


target/xtensa updates:

- fix gdbstub register counts;
- add big-endian core test_kc705_be;
- convert to do_transaction_failed and add test for failed memory
  transactions;
- fix couple FPU2000 bugs;
- fix s32c1i implementation;
- clean up exception handlers generation in xtensa tests;
- add support for semihosting console input through a chardev.


Max Filippov (10):
  target/xtensa: fix gdbstub register counts
  target/xtensa: clean up gdbstub register handling
  target/xtensa: add test_kc705_be core
  target/xtensa: convert to do_transaction_failed
  tests/tcg/xtensa: add test for failed memory transactions
  target/xtensa: fix FPU2000 bugs
  tests/tcg/xtensa: move exception handlers to separate section
  tests/tcg/xtensa: only generate defined exception handlers
  target/xtensa: fix s32c1i TCGMemOp flags
  target/xtensa: support input from chardev console

 target/xtensa/Makefile.objs| 1 +
 target/xtensa/core-test_kc705_be.c |52 +
 target/xtensa/core-test_kc705_be/core-isa.h|   575 +
 target/xtensa/core-test_kc705_be/gdb-config.inc.c  |   259 +
 .../xtensa/core-test_kc705_be/xtensa-modules.inc.c | 45117 +++
 target/xtensa/cpu.c| 2 +-
 target/xtensa/cpu.h| 9 +-
 target/xtensa/gdbstub.c|60 +-
 target/xtensa/helper.c |40 +-
 target/xtensa/op_helper.c  |12 +-
 target/xtensa/translate.c  | 8 +-
 target/xtensa/xtensa-semi.c|71 +-
 tests/tcg/xtensa/Makefile  | 1 +
 tests/tcg/xtensa/linker.ld.S   |37 +-
 tests/tcg/xtensa/test_phys_mem.S   |   124 +
 tests/tcg/xtensa/vectors.S |16 +
 16 files changed, 46318 insertions(+), 66 deletions(-)
 create mode 100644 target/xtensa/core-test_kc705_be.c
 create mode 100644 target/xtensa/core-test_kc705_be/core-isa.h
 create mode 100644 target/xtensa/core-test_kc705_be/gdb-config.inc.c
 create mode 100644 target/xtensa/core-test_kc705_be/xtensa-modules.inc.c
 create mode 100644 tests/tcg/xtensa/test_phys_mem.S

Thanks.
-- Max

Re: [Qemu-devel] [PATCH v2 5/8] x86_iommu/amd: Add interrupt remap support when VAPIC is not enabled

[Singh, Brijesh]

On 09/17/2018 10:53 PM, Peter Xu wrote:
[...]

>> IMHO we should not be using error_report_once() here. It's possible that
>> guest OS have DTE[IV]=1 but has not programmed the interrupt
>> remapping entries or have deactivated the remapping. I see that Linux
>> OS does it all the time and in those cases we will be printing out
>> error messages - which will give a wrong information.
> 
> I would be a bit curious on why the guest would like to switch the irq
> off if the device still needs to generate any... though this question
> could be out of topic so feel free to skip.

If you are interested then look at kernel irq domain [1] (Hierarchy IRQ
domain). There are 4 interfaces to the irq_domain:

- irq_domain_alloc_irqs() : when IR is enabled this will set DTE[IV]=1
- irq_domain_{activate,deactivate}: sets/clears the IRTE.fields.valid
   (aka IRTE.RemapEn bit).

You can find some additional info in this document on how a domain will
activate/deactivate resources.

[1] https://www.kernel.org/doc/Documentation/IRQ-domain.txt


> 
> But if that's normal then I would suggest you use a better naming of
> the trace point, "*_target_abort" seems to be a severe issue.
> 

I have been trying to adhere to IOMMU spec, if spec says that we should
cause an "target abort" then I try to use _target_abort tracepoint. In
case of vIOMMU we do not generate any IOMMU log event to communicate the
guest about the target aborts but on native system hardware will
generate a events which guest OS can parse to get additional info. Maybe
in future we can look at logging events from vIOMMU.


> Also, my suggestion is a general one - it still applies to the places
> that you think a well-behaved guest should not do.  It's optional so
> you can decide whether you want to use it.
> 

Agreed, I will certain look into it.


[...]

>> Honestly, I did struggled whether to use "exit" or error. Actually this
>> is a configuration error.
>>
>> AMD IOMMU does not have an explicit bit to indicate whether the IR
>> feature is supported. All versions of IOMMU supports IR. But in QEMU
>> we are using 'intermap' to enable/disable the feature. In case of AMD IOMMU,
>> we will not be able to link intremap to control the exposure of
>> this feature to the guest. So we need to do some checks during the
>> runtime. If we get asked to remap the interrupts (when intremap=off)
>> then IMHO its a configuration error and we should kill the guest.
>> Having said so, I am flexible and if needed then I can remove the
>> exit from next patch.
> 
> If all AMD IOMMU has IR, then I'm curious how the old AMD IOMMU
> emulation worked since that does not have an IR, do you know how the
> guest detects that?  (It must have done so otherwise IRQ won't work
> there, isn't it?)
> 
> And if that's true, instead of check intr_enabled here, I would
> suggest you check it in realize() - we'd better crash asap when we
> know this error, and we know it from the very beginning.
> 
>>


So far non of the guests were enabling the interrupt remap features
even when it was available. As I explained in previous patches (see
patch 6), Linux guest looks for a special IOAPIC device in IVHD before
enabling the interrupt remap. If you enable the amd_iommu_debug=1
in kernel cmdline then kernel bootup prints the message that its
attempting to enable the IR but since IOAPIC entry does not exist
in IVHD hence it disabled it.

Notes: for kicks you can apply patch 6 from this series in existing
code base and you will see that Linux OS will enable the IR (it does
care about intremap= from amd-iommu.


[...]

>>
>> This time used the pre-defined macro's from the apic-msidef.h hence I
>> thought there was no need to include the field definitions. But, I will
>> go ahead and include the field definition in the next rev.
>>
>> * The delivery_mode field from the MSI data is used to get the upstream
>>interrupt type.
>> * If the interrupt type is Fixed/Arbitrated, MSI data 10:0 is used
>>as an offset within the IRTE table to get the actual IRTE entry
>>(Fig 14).
>>
>> Please note that we don't emulate the HyperTransport hence instead we
>> should not treat MSI data 10:8 as index instead the MSI data 10:0 should
>> be used as "offset" within the IRTE table to get the interrupt remapping
>> table entries.
> 
> Frankly speaking until now I don't quite understand what's that
> HyperTransport thing... but ok I think I understand your point now:
> you mean that the "Interrupt type" mentioned in Table 19 is exactly
> the same bits (10:8) defined in general MSI data register.  I think
> that's the only knowledge that I'm missing from there. 


The MSI data register bit 10:8 definitions are similar to the Hyper
Transport MT encoding (Table 19), but they are not quite the same.

In our case we don't have HyperTransport so we need to use the
bit definition from the IOAPIC https://wiki.osdev.org/APIC

enum ioapic_irq_destination_types {
 dest_Fixed  = 0,
 dest_LowestPrio  

Re: [Qemu-devel] [PATCH v2 5/8] x86_iommu/amd: Add interrupt remap support when VAPIC is not enabled

[Singh, Brijesh]

Small correction


On 09/18/2018 03:27 PM, Singh, Brijesh wrote:>
> So far non of the guests were enabling the interrupt remap features
> even when it was available. As I explained in previous patches (see
> patch 6), Linux guest looks for a special IOAPIC device in IVHD before
> enabling the interrupt remap. If you enable the amd_iommu_debug=1
> in kernel cmdline then kernel bootup prints the message that its
> attempting to enable the IR but since IOAPIC entry does not exist
> in IVHD hence it disabled it.
> 
> Notes: for kicks you can apply patch 6 from this series in existing
> code base and you will see that Linux OS will enable the IR (it does
> care about intremap= from amd-iommu.
> 


I was meaning to say (it does *not* care about  intremap=...)

Re: [Qemu-devel] [PATCH 01/35] tcg: access cpu->icount_decr.u16.high with atomics

[Richard Henderson]

On 9/17/18 9:30 AM, Emilio G. Cota wrote:
> Consistently access u16.high with atomics to avoid
> undefined behaviour in MTTCG.
> 
> Note that icount_decr.u16.low is only used in icount mode,
> so regular accesses to it are OK.
> 
> Cc: Peter Crosthwaite 
> Cc: Richard Henderson 
> Signed-off-by: Emilio G. Cota 
> ---
>  accel/tcg/tcg-all.c   | 2 +-
>  accel/tcg/translate-all.c | 2 +-
>  qom/cpu.c | 2 +-
>  3 files changed, 3 insertions(+), 3 deletions(-)

Eh, sure, I guess.
Reviewed-by: Richard Henderson 


r~

Re: [Qemu-devel] [PATCH 05/35] target/s390x: use cpu_reset_interrupt

[Richard Henderson]

On 9/17/18 9:30 AM, Emilio G. Cota wrote:
> From: Paolo Bonzini 
> 
> It will be changed to an atomic operation soon.
> 
> Cc: Cornelia Huck 
> Cc: Richard Henderson 
> Cc: Alexander Graf 
> Cc: David Hildenbrand 
> Cc: qemu-s3...@nongnu.org
> Signed-off-by: Paolo Bonzini 
> Signed-off-by: Emilio G. Cota 
> ---
>  target/s390x/excp_helper.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Reviewed-by: Richard Henderson 


r~

Re: [Qemu-devel] [PATCH 06/35] exec: use cpu_reset_interrupt

[Richard Henderson]

On 9/17/18 9:30 AM, Emilio G. Cota wrote:
> From: Paolo Bonzini 
> 
> It will be changed to an atomic operation soon.
> 
> Cc: Peter Crosthwaite 
> Cc: Richard Henderson 
> Signed-off-by: Paolo Bonzini 
> Signed-off-by: Emilio G. Cota 
> ---
>  accel/tcg/cpu-exec.c | 6 +++---

Reviewed-by: Richard Henderson 

> diff --git a/exec.c b/exec.c
> index 6826c8337d..0b8e2420cf 100644
> --- a/exec.c
> +++ b/exec.c
> @@ -776,7 +776,7 @@ static int cpu_common_post_load(void *opaque, int 
> version_id)
>  
>  /* 0x01 was CPU_INTERRUPT_EXIT. This line can be removed when the
> version_id is increased. */
> -cpu->interrupt_request &= ~0x01;
> +cpu_reset_interrupt(cpu, 0x01);

I guess this doesn't hurt, although it's irrelevant.
We're in the middle of loadvm and there will not be
concurrent modification.


r~

Re: [Qemu-devel] [PATCH 02/35] target/i386: use cpu_reset_interrupt

[Richard Henderson]

On 9/17/18 9:30 AM, Emilio G. Cota wrote:
> From: Paolo Bonzini 
> 
> It will be changed to an atomic operation soon.
> 
> Cc: Richard Henderson 
> Cc: Eduardo Habkost 
> Cc: Marcelo Tosatti 
> Cc: k...@vger.kernel.org
> Signed-off-by: Paolo Bonzini 
> Signed-off-by: Emilio G. Cota 
> ---
>  target/i386/hax-all.c|  4 ++--
>  target/i386/hvf/x86hvf.c |  8 
>  target/i386/kvm.c| 14 +++---
>  target/i386/seg_helper.c | 13 ++---
>  target/i386/svm_helper.c |  2 +-
>  target/i386/whpx-all.c   | 10 +-
>  6 files changed, 25 insertions(+), 26 deletions(-)

Reviewed-by: Richard Henderson 


r~

Re: [Qemu-devel] [PATCH 07/35] target/alpha: access cpu->interrupt_request with atomics

[Richard Henderson]

On 9/17/18 9:30 AM, Emilio G. Cota wrote:
> From: Paolo Bonzini 
> 
> Cc: Richard Henderson 
> Signed-off-by: Paolo Bonzini 
> Signed-off-by: Emilio G. Cota 
> ---
>  target/alpha/cpu.c | 8 
>  1 file changed, 4 insertions(+), 4 deletions(-)

Reviewed-by: Richard Henderson 


r~

Re: [Qemu-devel] [PATCH 10/35] target/hppa: access cpu->interrupt_request with atomics

[Richard Henderson]

On 9/17/18 9:30 AM, Emilio G. Cota wrote:
> From: Paolo Bonzini 
> 
> Cc: Richard Henderson 
> Signed-off-by: Paolo Bonzini 
> Signed-off-by: Emilio G. Cota 
> ---
>  target/hppa/cpu.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Reviewed-by: Richard Henderson 


r~

Re: [Qemu-devel] [PATCH 10/35] target/hppa: access cpu->interrupt_request with atomics

[Richard Henderson]

On 9/17/18 9:30 AM, Emilio G. Cota wrote:
> From: Paolo Bonzini 
> 
> Cc: Richard Henderson 
> Signed-off-by: Paolo Bonzini 
> Signed-off-by: Emilio G. Cota 
> ---
>  target/hppa/cpu.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Reviewed-by: Richard Henderson 


r~

Re: [Qemu-devel] [Bug 1793183] [NEW] apt source --compile qemu-system-x86 fails on last ubuntu 18.04.1

[Peter Maydell]

On 18 September 2018 at 10:24, Dmitry Isaykin  wrote:
> Public bug reported:
>
> Error log:
>
> /tmp/qemu-2.10+dfsg/util/memfd.c:40:12: error: static declaration of 
> ‘memfd_create’ follows non-static declaration
>  static int memfd_create(const char *name, unsigned int flags)
> ^~~~
> In file included from /usr/include/x86_64-linux-gnu/bits/mman-linux.h:115:0,
>  from /usr/include/x86_64-linux-gnu/bits/mman.h:45,
>  from /usr/include/x86_64-linux-gnu/sys/mman.h:41,
>  from /tmp/qemu-2.10+dfsg/include/sysemu/os-posix.h:29,
>  from /tmp/qemu-2.10+dfsg/include/qemu/osdep.h:104,
>  from /tmp/qemu-2.10+dfsg/util/memfd.c:28:
> /usr/include/x86_64-linux-gnu/bits/mman-shared.h:46:5: note: previous 
> declaration of ‘memfd_create’ was here
>  int memfd_create (const char *__name, unsigned int __flags) __THROW;
>  ^~~~
> /tmp/qemu-2.10+dfsg/rules.mak:66: recipe for target 'util/memfd.o' failed

This is fixed in upstream QEMU in commit 75e5b70e6b5dcc, which is in
QEMU 2.12.0. If Ubuntu are interested in fixing this they can backport
that to the QEMU they are shipping, or alternatively move forward to 2.12.

thanks
-- PMM

Re: [Qemu-devel] [PATCH 11/35] target/i386: access cpu->interrupt_request with atomics

[Richard Henderson]

On 9/17/18 9:30 AM, Emilio G. Cota wrote:
>  cpu_reset(cs);
> -cs->interrupt_request = sipi;
> +atomic_mb_set(>interrupt_request, sipi);
>  memcpy(>start_init_save, >start_init_save,

Why does this need a memory barrier?

Anyway, I think a bare mechanical conversion would be best
for the first patch and then extra barriers added separately
and with a description of why.


r~

Re: [Qemu-devel] [PATCH 28/35] exec: access cpu->interrupt_request with atomics

[Richard Henderson]

On 9/17/18 9:30 AM, Emilio G. Cota wrote:
> From: Paolo Bonzini 
> 
> Cc: Peter Crosthwaite 
> Cc: Richard Henderson 
> Signed-off-by: Paolo Bonzini 
> Signed-off-by: Emilio G. Cota 
> ---
>  accel/tcg/cpu-exec.c  | 6 +++---
>  accel/tcg/tcg-all.c   | 3 +--
>  accel/tcg/translate-all.c | 2 +-
>  qom/cpu.c | 6 +++---
>  4 files changed, 8 insertions(+), 9 deletions(-)

Reviewed-by: Richard Henderson 


r~

Re: [Qemu-devel] [PATCH 29/35] exec: drop BQL from cpu_reset_interrupt

[Richard Henderson]

On 9/17/18 9:30 AM, Emilio G. Cota wrote:
> From: Paolo Bonzini 
> 
> Now that cpu->interrupt_request is accessed with atomics,
> we can drop the BQL around cpu_reset_interrupt, which is a
> step towards not taking the BQL mandatorily in cpu_exec_interrupt.
> 
> Cc: Peter Crosthwaite 
> Cc: Richard Henderson 
> Signed-off-by: Paolo Bonzini 
> Signed-off-by: Emilio G. Cota 
> ---
>  qom/cpu.c | 10 --
>  1 file changed, 10 deletions(-)

Reviewed-by: Richard Henderson 


r~

Re: [Qemu-devel] [PATCH 30/35] tcg: drop BQL assertion from tcg_handle_interrupt

[Richard Henderson]

On 9/17/18 9:30 AM, Emilio G. Cota wrote:
> From: Paolo Bonzini 
> 
> Cc: Peter Crosthwaite 
> Cc: Richard Henderson 
> Signed-off-by: Paolo Bonzini 
> Signed-off-by: Emilio G. Cota 
> ---
>  accel/tcg/tcg-all.c | 1 -
>  1 file changed, 1 deletion(-)

Reviewed-by: Richard Henderson 


r~

Re: [Qemu-devel] [PATCH v3 0/2] 40p: fix PCI interrupt routing

[Mark Cave-Ayland]

On 17/09/2018 04:54, David Gibson wrote:

> Mark,
> 
> I think we have all the necessary acks to go ahead with this.
> However, I'm afraid I've lost track of the various prereq patches that
> were necessary here.  Can you resend with all the necessary pieces
> rebased against ppc-for-3.1 and the appropriate acked-bys included?

Apologies for the delay on this one, but once I rebased onto your ppc
for-3.1 branch I noticed a regression whereby the 40p sandalfoot image
would fail to boot to userspace - it would hang towards the end of the
boot with:

Bad inittab entry:
Bad inittab entry:
No more tasks for init -- sleeping forever

In the end I bisected it down to this patch:

commit d12a22c5c7ccb6cb7e8438b7e6e1011c6b712ae2
Author: Roman Kapl 
Date:   Tue Sep 11 13:34:51 2018 +0200

target/ppc: add external PID support

Playing with this a bit more, it appears that the bug is only triggered
if I configure QEMU with --enable-debug which is my development default.

Roman, can you reproduce this locally at all? My setup is nothing
special, just Debian Stretch on amd64.


ATB,

Mark.

Re: [Qemu-devel] [PATCH 32/35] target/i386/kvm: do not acquire the BQL to call cpu_reset_interrupt

[Richard Henderson]

On 9/17/18 9:31 AM, Emilio G. Cota wrote:
> From: Paolo Bonzini 
> 
> It's not needed anymore.
> 
> Cc: Marcelo Tosatti 
> Cc: Richard Henderson 
> Cc: Eduardo Habkost 
> Cc: k...@vger.kernel.org
> Signed-off-by: Paolo Bonzini 
> Signed-off-by: Emilio G. Cota 
> ---
>  target/i386/kvm.c | 4 
>  1 file changed, 4 deletions(-)

Reviewed-by: Richard Henderson 


r~

Re: [Qemu-devel] [PATCH 31/35] translate-all: drop BQL assertion from cpu_interrupt

[Richard Henderson]

On 9/17/18 9:30 AM, Emilio G. Cota wrote:
> This patch explicitly drops the BQL assertion from
> the user-mode version; previous patches have taken
> care of softmmu's cpu_interrupt.
> 
> Cc: Peter Crosthwaite 
> Cc: Richard Henderson 
> Signed-off-by: Emilio G. Cota 
> ---
>  accel/tcg/translate-all.c | 1 -
>  1 file changed, 1 deletion(-)

Reviewed-by: Richard Henderson 


r~

Re: [Qemu-devel] [qemu-s390x] [PATCH v8 3/6] s390x/kvm: enable/disable AP instruction interpretation for guest

[Halil Pasic]

On 09/18/2018 06:59 PM, Tony Krowiak wrote:
> I've discussed this with Halil -- Pierre is out until next week. We
> are in agreement that while these changes are viable, they result in
> a slightly more complicated implementation compared to previous versions (e.g.
> kernel v9 QEMU v7), and locks us into a model that may limit design choices
> down the road if/when virtual and/or emulated AP devices are implemented.

Just want to confirm, that I did slightly change my mind compared to some weeks
ago regarding the introducing the dynamic toggle in this stage. Back then I was
like it's good, because it makes the interface complete. After thinking some
more, it appears to me alone it does not buy us (or userspace) a thing. If
userspace wants to emulate the ap instructions in the first place it can just
clear away the KVM_S390_VM_CPU_FEAT_AP bit (if offered) before applying the cpu
model. And I can't think of an situation where dynamically switching one way or
the other would make sense without additional kernel interfaces.

But in general my position stands. Any of the proposed solutions will work, and
I'm fine with any of them getting merged. That said, let me state my
preferences.  The most preferred would be a bulk operation based solution like
in v9, for the reasons pointed out by Tony. Next by preference is getting rid
of KVM_S390_VM_CPU_FEAT_AP as you proposed, for the reasons already stated
here. The least preferable would be the v10 solution, but it's also a working
solution, and that's what I care the most about at the moment. And about
vfio-ap getting merged ASAP.

Regards,
Halil

Re: [Qemu-devel] [PATCH v2 12/17] mirror: Fix potential use-after-free in active commit

[Max Reitz]

On 18.09.18 17:04, Kevin Wolf wrote:
> Am 18.09.2018 um 16:11 hat Max Reitz geschrieben:
>> On 17.09.18 13:37, Kevin Wolf wrote:
>>> Am 17.09.2018 um 00:05 hat Max Reitz geschrieben:
 On 14.09.18 18:25, Kevin Wolf wrote:
> Am 13.09.2018 um 22:55 hat Max Reitz geschrieben:
>> On 13.09.18 14:52, Kevin Wolf wrote:
>>> When starting an active commit job, other callbacks can run before
>>> mirror_start_job() calls bdrv_ref() where needed and cause the nodes to
>>> go away. Add another pair of bdrv_ref/unref() around it to protect
>>> against this case.
>>>
>>> Signed-off-by: Kevin Wolf 
>>> ---
>>>  block/mirror.c | 11 +++
>>>  1 file changed, 11 insertions(+)
>>
>> Reviewed-by: Max Reitz 
>>
>> But...  How?
>
> Tried to reproduce the other bug Paolo was concerned about (good there
> is something like 'git reflog'!) and dug up this one instead.
>
> So the scenario seems to be test_stream_commit_1 in qemu-iotests 030.
>
> The backtrace when the BDS is deleted is the following:
>
> (rr) bt
> #0  0x7faeaf6145ec in __memset_sse2_unaligned_erms () at 
> /lib64/libc.so.6
> #1  0x7faeaf60414e in _int_free () at /lib64/libc.so.6
> #2  0x7faeaf6093be in free () at /lib64/libc.so.6
> #3  0x7faecaea6b4e in g_free () at /lib64/libglib-2.0.so.0
> #4  0x55c0c94f9d6c in bdrv_delete (bs=0x55c0cbe69240) at block.c:3590
> #5  0x55c0c94fbe82 in bdrv_unref (bs=0x55c0cbe69240) at block.c:4638
> #6  0x55c0c94f6761 in bdrv_root_unref_child (child=0x55c0cbe57af0) at 
> block.c:2188
> #7  0x55c0c94fe239 in block_job_remove_all_bdrv (job=0x55c0ccf9e220) 
> at blockjob.c:200
> #8  0x55c0c94fdf1f in block_job_free (job=0x55c0ccf9e220) at 
> blockjob.c:94
> #9  0x55c0c94ffe0d in job_unref (job=0x55c0ccf9e220) at job.c:368
> #10 0x55c0c95007cd in job_do_dismiss (job=0x55c0ccf9e220) at job.c:641
> #11 0x55c0c95008d9 in job_conclude (job=0x55c0ccf9e220) at job.c:667
> #12 0x55c0c9500b66 in job_finalize_single (job=0x55c0ccf9e220) at 
> job.c:735
> #13 0x55c0c94ff4fa in job_txn_apply (txn=0x55c0cbe19eb0, 
> fn=0x55c0c9500a62 , lock=true) at job.c:151
> #14 0x55c0c9500e92 in job_do_finalize (job=0x55c0ccf9e220) at 
> job.c:822
> #15 0x55c0c9501040 in job_completed_txn_success (job=0x55c0ccf9e220) 
> at job.c:872
> #16 0x55c0c950115c in job_completed (job=0x55c0ccf9e220, ret=0, 
> error=0x0) at job.c:892
> #17 0x55c0c92b572c in stream_complete (job=0x55c0ccf9e220, 
> opaque=0x55c0cc050bc0) at block/stream.c:96

 But isn't this just deletion of node1, i.e. the intermediate node of the
 stream job?

 The commit job happens above that (node3 and upwards), so all its BDSs
 shouldn't be affected.  So even with the bdrv_ref()s introduced in this
 patch, I'd still expect node1 to be deleted exactly here.
>>>
>>> Hm, I suppose that's true.
>>>
>>> So it crashed because the drain in bdrv_reopen() didn't actually drain
>>> the streaming job (which includes removing node1), so node1 ended up in
>>> the ReopenQueue and when it disappeared in the middle of reopen, we got
>>> a use after free.
>>>
>>> Then it would be bug between job draining and bdrv_reopen() and the
>>> active commit job would only be a victim of the bug without actually
>>> doing anything wrong, and we can drop this patch.
>>>
>>> Does this make sense?
>>
>> The explanation makes sense, yes.
>>
>> About dropping the patch...  We would have to be certain that you can
>> only run a block job if none of the nodes involved can be deleted at any
>> point.  After having started the job, this is the job's responsibility
>> by having referenced everything it needs (through block_job_add_bdrv()).
>>  So the critical point is when the job is starting, before it invoked
>> that function.
>>
>> The user cannot do any graph manipulation then because the monitor is
>> blocked by starting the job.  So the only issue would be other
>> operations that can make nodes go away without user intervention.
>> Naturally the way to prevent conflicts would be with blockers, and
>> currently our old job blockers should be sufficient to prevent them.
>> But in theory we don't like them, so is this what we need the graph
>> manipulation blocker for? :-)
> 
> I don't think so. Permissions are for users that are potentially
> attached for a long time, not for protecting operations during which we
> hold the BQL anyway.
> 
> Our problem here isn't that the graph is changing, but that nodes are
> going away.

Hm, yeah.  Which they usually wouldn't unless the graph is changing, but
that's not a necessity.

Though the better explanation is: The node can only go away when it has
only one parent.  So even if that parent took the graph manipulation
blocker, there'd be nobody to care.

> The solution is to take 

[Qemu-devel] [resend PATCH v2] qga-win: add support for qmp_guest_fsfreeze_freeze_list

[Chen Hanxiao]

From: Chen Hanxiao 

This patch add support for freeze specified fs.

The valid mountpoints list member are [1]:

  The path of a mounted folder, for example, Y:\MountX\
  A drive letter, for example, D:\
  A volume GUID path of the form \\?\Volume{GUID}\,
  where GUID identifies the volume
  A UNC path that specifies a remote file share,
  for example, \\Clusterx\Share1\

[1] 
https://docs.microsoft.com/en-us/windows/desktop/api/vsbackup/nf-vsbackup-ivssbackupcomponents-addtosnapshotset

Cc: Michael Roth 
Signed-off-by: Chen Hanxiao 
---
v2:
  optimize internal logic blocks

 qga/commands-win32.c| 21 -
 qga/main.c  |  2 +-
 qga/vss-win32.c |  5 +-
 qga/vss-win32.h |  3 +-
 qga/vss-win32/requester.cpp | 92 ++---
 qga/vss-win32/requester.h   | 13 --
 6 files changed, 91 insertions(+), 45 deletions(-)

diff --git a/qga/commands-win32.c b/qga/commands-win32.c
index 98d9735389..1d627f73c1 100644
--- a/qga/commands-win32.c
+++ b/qga/commands-win32.c
@@ -776,6 +776,13 @@ GuestFsfreezeStatus qmp_guest_fsfreeze_status(Error **errp)
  * The frozen state is limited for up to 10 seconds by VSS.
  */
 int64_t qmp_guest_fsfreeze_freeze(Error **errp)
+{
+return qmp_guest_fsfreeze_freeze_list(false, NULL, errp);
+}
+
+int64_t qmp_guest_fsfreeze_freeze_list(bool has_mountpoints,
+   strList *mountpoints,
+   Error **errp)
 {
 int i;
 Error *local_err = NULL;
@@ -790,7 +797,7 @@ int64_t qmp_guest_fsfreeze_freeze(Error **errp)
 /* cannot risk guest agent blocking itself on a write in this state */
 ga_set_frozen(ga_state);
 
-qga_vss_fsfreeze(, true, _err);
+qga_vss_fsfreeze(, true, mountpoints, _err);
 if (local_err) {
 error_propagate(errp, local_err);
 goto error;
@@ -808,15 +815,6 @@ error:
 return 0;
 }
 
-int64_t qmp_guest_fsfreeze_freeze_list(bool has_mountpoints,
-   strList *mountpoints,
-   Error **errp)
-{
-error_setg(errp, QERR_UNSUPPORTED);
-
-return 0;
-}
-
 /*
  * Thaw local file systems using Volume Shadow-copy Service.
  */
@@ -829,7 +827,7 @@ int64_t qmp_guest_fsfreeze_thaw(Error **errp)
 return 0;
 }
 
-qga_vss_fsfreeze(, false, errp);
+qga_vss_fsfreeze(, false, NULL, errp);
 
 ga_unset_frozen(ga_state);
 return i;
@@ -1646,7 +1644,6 @@ GList *ga_command_blacklist_init(GList *blacklist)
 "guest-set-vcpus",
 "guest-get-memory-blocks", "guest-set-memory-blocks",
 "guest-get-memory-block-size",
-"guest-fsfreeze-freeze-list",
 NULL};
 char **p = (char **)list_unsupported;
 
diff --git a/qga/main.c b/qga/main.c
index 6d70242d05..13bfff5f0a 100644
--- a/qga/main.c
+++ b/qga/main.c
@@ -151,7 +151,7 @@ static void quit_handler(int sig)
 WaitForSingleObject(hEventTimeout, 0);
 CloseHandle(hEventTimeout);
 }
-qga_vss_fsfreeze(, false, );
+qga_vss_fsfreeze(, false, NULL, );
 if (err) {
 g_debug("Error unfreezing filesystems prior to exiting: %s",
 error_get_pretty(err));
diff --git a/qga/vss-win32.c b/qga/vss-win32.c
index a541f3ae01..f444a25a70 100644
--- a/qga/vss-win32.c
+++ b/qga/vss-win32.c
@@ -147,7 +147,8 @@ void ga_uninstall_vss_provider(void)
 }
 
 /* Call VSS requester and freeze/thaw filesystems and applications */
-void qga_vss_fsfreeze(int *nr_volume, bool freeze, Error **errp)
+void qga_vss_fsfreeze(int *nr_volume, bool freeze,
+  strList *mountpoints, Error **errp)
 {
 const char *func_name = freeze ? "requester_freeze" : "requester_thaw";
 QGAVSSRequesterFunc func;
@@ -164,5 +165,5 @@ void qga_vss_fsfreeze(int *nr_volume, bool freeze, Error 
**errp)
 return;
 }
 
-func(nr_volume, );
+func(nr_volume, mountpoints, );
 }
diff --git a/qga/vss-win32.h b/qga/vss-win32.h
index 4f8e39aa5c..ce2abe5a72 100644
--- a/qga/vss-win32.h
+++ b/qga/vss-win32.h
@@ -22,6 +22,7 @@ bool vss_initialized(void);
 int ga_install_vss_provider(void);
 void ga_uninstall_vss_provider(void);
 
-void qga_vss_fsfreeze(int *nr_volume, bool freeze, Error **errp);
+void qga_vss_fsfreeze(int *nr_volume, bool freeze,
+  strList *mountpints, Error **errp);
 
 #endif
diff --git a/qga/vss-win32/requester.cpp b/qga/vss-win32/requester.cpp
index 3d9c9716c0..5378c55d23 100644
--- a/qga/vss-win32/requester.cpp
+++ b/qga/vss-win32/requester.cpp
@@ -234,7 +234,7 @@ out:
 }
 }
 
-void requester_freeze(int *num_vols, ErrorSet *errset)
+void requester_freeze(int *num_vols, void *mountpoints, ErrorSet *errset)
 {
 COMPointer pAsync;
 HANDLE volume;
@@ -246,6 +246,7 @@ void requester_freeze(int *num_vols, ErrorSet *errset)
 WCHAR short_volume_name[64], *display_name = short_volume_name;
 DWORD wait_status;
 

[Qemu-devel] [PATCH 1/1] i386: Add new model of Cascadelake-Server

[Tao Xu]

New CPU models mostly inherit features from ancestor Skylake-Server,
while addin new features: AVX512_VNNI, Intel PT.
SSBD support for speculative execution
side channel mitigations.

Note:

On Cascadelake, some capabilities (RDCL_NO, IBRS_ALL, RSBA,
SKIP_L1DFL_VMENTRY and SSB_NO) are enumerated by MSR.
These features rely on MSR based feature support patch. 
Will be added later after that patch's in.
http://lists.nongnu.org/archive/html/qemu-devel/2018-09/msg00074.html

Signed-off-by: Tao Xu 
---
 target/i386/cpu.c | 54 +++
 1 file changed, 54 insertions(+)

diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index f24295e6e4..670898f32d 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -2386,6 +2386,60 @@ static X86CPUDefinition builtin_x86_defs[] = {
 .xlevel = 0x8008,
 .model_id = "Intel Xeon Processor (Skylake, IBRS)",
 },
+{
+.name = "Cascadelake-Server",
+.level = 0xd,
+.vendor = CPUID_VENDOR_INTEL,
+.family = 6,
+.model = 85,
+.stepping = 5,
+.features[FEAT_1_EDX] =
+CPUID_VME | CPUID_SSE2 | CPUID_SSE | CPUID_FXSR | CPUID_MMX |
+CPUID_CLFLUSH | CPUID_PSE36 | CPUID_PAT | CPUID_CMOV | CPUID_MCA |
+CPUID_PGE | CPUID_MTRR | CPUID_SEP | CPUID_APIC | CPUID_CX8 |
+CPUID_MCE | CPUID_PAE | CPUID_MSR | CPUID_TSC | CPUID_PSE |
+CPUID_DE | CPUID_FP87,
+.features[FEAT_1_ECX] =
+CPUID_EXT_AVX | CPUID_EXT_XSAVE | CPUID_EXT_AES |
+CPUID_EXT_POPCNT | CPUID_EXT_X2APIC | CPUID_EXT_SSE42 |
+CPUID_EXT_SSE41 | CPUID_EXT_CX16 | CPUID_EXT_SSSE3 |
+CPUID_EXT_PCLMULQDQ | CPUID_EXT_SSE3 |
+CPUID_EXT_TSC_DEADLINE_TIMER | CPUID_EXT_FMA | CPUID_EXT_MOVBE |
+CPUID_EXT_PCID | CPUID_EXT_F16C | CPUID_EXT_RDRAND,
+.features[FEAT_8000_0001_EDX] =
+CPUID_EXT2_LM | CPUID_EXT2_PDPE1GB | CPUID_EXT2_RDTSCP |
+CPUID_EXT2_NX | CPUID_EXT2_SYSCALL,
+.features[FEAT_8000_0001_ECX] =
+CPUID_EXT3_ABM | CPUID_EXT3_LAHF_LM | CPUID_EXT3_3DNOWPREFETCH,
+.features[FEAT_7_0_EBX] =
+CPUID_7_0_EBX_FSGSBASE | CPUID_7_0_EBX_BMI1 |
+CPUID_7_0_EBX_HLE | CPUID_7_0_EBX_AVX2 | CPUID_7_0_EBX_SMEP |
+CPUID_7_0_EBX_BMI2 | CPUID_7_0_EBX_ERMS | CPUID_7_0_EBX_INVPCID |
+CPUID_7_0_EBX_RTM | CPUID_7_0_EBX_RDSEED | CPUID_7_0_EBX_ADX |
+CPUID_7_0_EBX_SMAP | CPUID_7_0_EBX_MPX | CPUID_7_0_EBX_CLWB |
+CPUID_7_0_EBX_AVX512F | CPUID_7_0_EBX_AVX512DQ |
+CPUID_7_0_EBX_AVX512BW | CPUID_7_0_EBX_AVX512CD |
+CPUID_7_0_EBX_AVX512VL | CPUID_7_0_EBX_CLFLUSHOPT |
+CPUID_7_0_EBX_INTEL_PT,
+.features[FEAT_7_0_ECX] =
+CPUID_7_0_ECX_PKU | CPUID_7_0_ECX_OSPKE |
+CPUID_7_0_ECX_AVX512VNNI,
+.features[FEAT_7_0_EDX] =
+CPUID_7_0_EDX_SPEC_CTRL | CPUID_7_0_EDX_SPEC_CTRL_SSBD,
+/* Missing: XSAVES (not supported by some Linux versions,
+* including v4.1 to v4.12).
+* KVM doesn't yet expose any XSAVES state save component,
+* and the only one defined in Skylake (processor tracing)
+* probably will block migration anyway.
+*/
+.features[FEAT_XSAVE] =
+CPUID_XSAVE_XSAVEOPT | CPUID_XSAVE_XSAVEC |
+CPUID_XSAVE_XGETBV1,
+.features[FEAT_6_EAX] =
+CPUID_6_EAX_ARAT,
+.xlevel = 0x8008,
+.model_id = "Intel Xeon Processor (Cascadelake)",
+},
 {
 .name = "Icelake-Client",
 .level = 0xd,
-- 
2.17.1

[Qemu-devel] [PATCH 0/1] Add new model of Cascadelake-Server

[Tao Xu]

This patch defines the new guest CPU models of Cascadelake-Server.

Tao Xu (1):
  i386: Add new model of Cascadelake-Server

 target/i386/cpu.c | 54 +++
 1 file changed, 54 insertions(+)

-- 
2.17.1

Re: [Qemu-devel] [PATCH v4 0/3] file-posix: Simplifications on image locking

[Fam Zheng]

On Tue, 08/21 08:58, Fam Zheng wrote:
> v4: Fix test on systems without OFD. [Patchew]

Ping?

> 
> The first patch reduces chances of QEMU crash in unusual (but not unlikely)
> cases especially when used by Libvirt (see commit message).
> 
> The second patch halves fd for images.
> 
> The third adds some more test for patch one (would have caught the regression
> caused by v2).
> 
> Fam Zheng (3):
>   file-posix: Skip effectiveless OFD lock operations
>   file-posix: Drop s->lock_fd
>   tests: Add unit tests for image locking
> 
>  block/file-posix.c |  83 
>  tests/Makefile.include |   2 +
>  tests/test-image-locking.c | 154 +
>  3 files changed, 209 insertions(+), 30 deletions(-)
>  create mode 100644 tests/test-image-locking.c
> 
> -- 
> 2.17.1
> 
> 

[Qemu-devel] [Bug 1788665] Re: Low 2D graphics performance with Windows 10 (1803) VGA passthrough VM using "Spectre" protection

[George Amanakis]

snapshot_2 showing the pattern of vmentries/vmexits from the previous
comment ("zoom-in").

** Attachment added: "snapshot_2.png"
   
https://bugs.launchpad.net/qemu/+bug/1788665/+attachment/5190356/+files/snapshot_2.png

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

Title:
  Low 2D graphics performance with Windows 10 (1803) VGA passthrough VM
  using "Spectre" protection

Status in QEMU:
  New

Bug description:
  Windows 10 (1803) VM using VGA passthrough via qemu script.

  After upgrading Windows 10 Pro VM to version 1803, or possibly after
  applying the March/April security updates from Microsoft, the VM would
  show low 2D graphics performance (sluggishness in 2D applications and
  low Passmark results).

  Turning off Spectre vulnerability protection in Windows remedies the
  issue.

  Expected behavior:
  qemu/kvm hypervisor to expose firmware capabilities of host to guest OS - see 
https://docs.microsoft.com/en-us/virtualization/hyper-v-on-windows/CVE-2017-5715-and-hyper-v-vms

  Background:

  Starting in March or April Microsoft began to push driver updates in
  their updates / security updates. See https://support.microsoft.com
  /en-us/help/4073757/protect-your-windows-devices-against-spectre-
  meltdown

  One update concerns the Intel microcode - see
  https://support.microsoft.com/en-us/help/4100347. It is activated by
  default within Windows.

  Once the updates are applied within the Windows guest, 2D graphics
  performance drops significantly. Other performance benchmarks are not
  affected.

  A bare metal Windows installation does not display a performance loss
  after the update. See https://heiko-sieger.info/low-2d-graphics-
  benchmark-with-windows-10-1803-kvm-vm/

  Similar reports can be found here:
  
https://www.reddit.com/r/VFIO/comments/97unx4/passmark_lousy_2d_graphics_performance_on_windows/

  Hardware:

  6 core Intel Core i7-3930K (-MT-MCP-)

  Host OS:
  Linux Mint 19/Ubuntu 18.04
  Kernel: 4.15.0-32-generic x86_64
  Qemu: QEMU emulator version 2.11.1
  Intel microcode (host): 0x714
  dmesg | grep microcode
  [0.00] microcode: microcode updated early to revision 0x714, date = 
2018-05-08
  [2.810683] microcode: sig=0x206d7, pf=0x4, revision=0x714
  [2.813340] microcode: Microcode Update Driver: v2.2.

  Note: I manually updated the Intel microcode on the host from 0x713 to
  0x714. However, both microcode versions produce the same result in the
  Windows guest.

  Guest OS:
  Windows 10 Pro 64 bit, release 1803

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

[Qemu-devel] [Bug 1788665] Re: Low 2D graphics performance with Windows 10 (1803) VGA passthrough VM using "Spectre" protection

[George Amanakis]

David, your suggestion seemed helpful, at least there is a difference in
the pattern of vmentries and vmexits. See the snapshot attached.

Explanation of snapshot_1:
Two windows of kernelshark with trace.dats obtained using the command from 
above; the left window (trace.dat) is with spec-ctrl feature disabled, the 
right window with spec-ctrl enabled (default).
CPU9 runs the emulator (emulatorpin), and the spikes seen are kvm_set_irq every 
16ms. CPUs 2-7 and 19-15 run the vcpu threads.
Halfway through the trace in the snapshot the test begins (passmark 2d image 
rendering). The VM without spec-ctrl triggers vmentries/vmexits much more often 
than the VM with spec-ctrl. I could not spot a difference in the pattern of the 
vmentries/vmexits themselves (snapshot_2 below), only their frequency seems to 
differ.

Does anybody have an idea of what is going on?

George

** Attachment added: "snapshot_1.png"
   
https://bugs.launchpad.net/qemu/+bug/1788665/+attachment/5190355/+files/snapshot_1.png

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

Title:
  Low 2D graphics performance with Windows 10 (1803) VGA passthrough VM
  using "Spectre" protection

Status in QEMU:
  New

Bug description:
  Windows 10 (1803) VM using VGA passthrough via qemu script.

  After upgrading Windows 10 Pro VM to version 1803, or possibly after
  applying the March/April security updates from Microsoft, the VM would
  show low 2D graphics performance (sluggishness in 2D applications and
  low Passmark results).

  Turning off Spectre vulnerability protection in Windows remedies the
  issue.

  Expected behavior:
  qemu/kvm hypervisor to expose firmware capabilities of host to guest OS - see 
https://docs.microsoft.com/en-us/virtualization/hyper-v-on-windows/CVE-2017-5715-and-hyper-v-vms

  Background:

  Starting in March or April Microsoft began to push driver updates in
  their updates / security updates. See https://support.microsoft.com
  /en-us/help/4073757/protect-your-windows-devices-against-spectre-
  meltdown

  One update concerns the Intel microcode - see
  https://support.microsoft.com/en-us/help/4100347. It is activated by
  default within Windows.

  Once the updates are applied within the Windows guest, 2D graphics
  performance drops significantly. Other performance benchmarks are not
  affected.

  A bare metal Windows installation does not display a performance loss
  after the update. See https://heiko-sieger.info/low-2d-graphics-
  benchmark-with-windows-10-1803-kvm-vm/

  Similar reports can be found here:
  
https://www.reddit.com/r/VFIO/comments/97unx4/passmark_lousy_2d_graphics_performance_on_windows/

  Hardware:

  6 core Intel Core i7-3930K (-MT-MCP-)

  Host OS:
  Linux Mint 19/Ubuntu 18.04
  Kernel: 4.15.0-32-generic x86_64
  Qemu: QEMU emulator version 2.11.1
  Intel microcode (host): 0x714
  dmesg | grep microcode
  [0.00] microcode: microcode updated early to revision 0x714, date = 
2018-05-08
  [2.810683] microcode: sig=0x206d7, pf=0x4, revision=0x714
  [2.813340] microcode: Microcode Update Driver: v2.2.

  Note: I manually updated the Intel microcode on the host from 0x713 to
  0x714. However, both microcode versions produce the same result in the
  Windows guest.

  Guest OS:
  Windows 10 Pro 64 bit, release 1803

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

Re: [Qemu-devel] [RFC v5 0/6] pci_expander_brdige: support separate pci domain for pxb-pcie

[Gerd Hoffmann]

> > > 2. Only 4x devices is supported, you need to be careful not to overuse
> >
> > Could you elaborate on this please? What happens if you are not careful?
> > How does management know what the limits are?
> 
> It means the user might use more space than 768MB for mmconfig,
> which is [0x8000m 0xb000). If such situation happens,  seabios
> will ignore all extra pxb-pcie devices and not adding e820 entry for them,
> which makes all the devices under them unworkable.

Ypu should clearly note that this is a limitation of the seabios support
patches then, so not an issue of the qemu patches.

> As for the management, will some checks when adding mcfg be enough for
> management? Or I can maintain a variable to indicate how many space
> have been consumed and warn the user if they exceed the threshold?
> The latter allows us to do the check when the pxb-pcie is initializing.

I think qemu should not apply any restrictions here.

cheers,
  Gerd

Re: [Qemu-devel] [PATCH 1/1] qmp, hmp: add PCI subsystem id and vendor id to PCI info

[Dr. David Alan Gilbert]

* Denis V. Lunev (d...@openvz.org) wrote:
> This is a long story. RedHat has relicensed Windows KVM device drivers
> in 2018 and there was an agreement that to avoid WHQL driver conflict
> software manufacturers should set proper PCI subsystem vendor ID in
> their distributions. Thus PCI subsystem vendor id becomes actively used.
> 
> The problem is that this field is applied by us via hardware compats.
> Thus technically it could be lost.
> 
> This patch adds PCI susbsystem id and vendor id to exportable parameters
> for validation.
> 
> Signed-off-by: Denis V. Lunev 
> CC: "Dr. David Alan Gilbert" 
> CC: Eric Blake 
> CC: Markus Armbruster 

Reviewed-by: Dr. David Alan Gilbert 


I'll take it via HMP.

Dave

> ---
>  hmp.c| 2 ++
>  hw/pci/pci.c | 3 +++
>  qapi-schema.json | 7 ++-
>  3 files changed, 11 insertions(+), 1 deletion(-)
> 
> diff --git a/hmp.c b/hmp.c
> index 2874bcd789..8fb0957cfd 100644
> --- a/hmp.c
> +++ b/hmp.c
> @@ -803,6 +803,8 @@ static void hmp_info_pci_device(Monitor *mon, const 
> PciDeviceInfo *dev)
>  
>  monitor_printf(mon, ": PCI device %04" PRIx64 ":%04" PRIx64 "\n",
> dev->id->vendor, dev->id->device);
> +monitor_printf(mon, "  PCI subsystem %04" PRIx64 ":%04" PRIx64 "\n",
> +   dev->id->subsystem_vendor, dev->id->subsystem);
>  
>  if (dev->has_irq) {
>  monitor_printf(mon, "  IRQ %" PRId64 ".\n", dev->irq);
> diff --git a/hw/pci/pci.c b/hw/pci/pci.c
> index f0c98cd0ae..be70dd425c 100644
> --- a/hw/pci/pci.c
> +++ b/hw/pci/pci.c
> @@ -1724,6 +1724,9 @@ static PciDeviceInfo *qmp_query_pci_device(PCIDevice 
> *dev, PCIBus *bus,
>  info->id = g_new0(PciDeviceId, 1);
>  info->id->vendor = pci_get_word(dev->config + PCI_VENDOR_ID);
>  info->id->device = pci_get_word(dev->config + PCI_DEVICE_ID);
> +info->id->subsystem = pci_get_word(dev->config + PCI_SUBSYSTEM_ID);
> +info->id->subsystem_vendor =
> +pci_get_word(dev->config + PCI_SUBSYSTEM_VENDOR_ID);
>  info->regions = qmp_query_pci_regions(dev);
>  info->qdev_id = g_strdup(dev->qdev.id ? dev->qdev.id : "");
>  
> diff --git a/qapi/misc.json b/qapi/misc.json
> index dfef6faf81..1704a8d437 100644
> --- a/qapi/misc.json
> +++ b/qapi/misc.json
> @@ -2162,10 +2162,15 @@
>  #
>  # @vendor: the PCI vendor id
>  #
> +# @subsystem: the PCI subsystem id (since 3.1)
> +#
> +# @subsystem-vendor: the PCI subsystem vendor id (since 3.1)
> +#
>  # Since: 2.4
>  ##
>  { 'struct': 'PciDeviceId',
> -  'data': {'device': 'int', 'vendor': 'int'} }
> +  'data': {'device': 'int', 'vendor': 'int', 'subsystem': 'int',
> +'subsystem-vendor': 'int'} }
>  
>  ##
>  # @PciDeviceInfo:
> -- 
> 2.17.1
> 
--
Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK