[Qemu-devel] [PATCH] target-openrisc: Fix exception handling status registers

[Stafford Horne]

I am working on testing instruction emulation patches for the linux
kernel. During testing I found these 2 issues:

 - sets DSX (delay slot exception) but never clears it
 - EEAR for illegal insns should point to the bad exception (as per
   openrisc spec) but its not

This patch fixes these two issues by clearing the DSX flag when not in a
delay slot and by setting EEAR to exception PC when handling illegal
instruction exceptions.

After this patch the openrisc kernel with latest patches boots great on
qemu and instruction emulation works.

Cc: qemu-triv...@nongnu.org
Cc: openr...@lists.librecores.org
Signed-off-by: Stafford Horne 
---
 target/openrisc/interrupt.c | 7 +++
 1 file changed, 7 insertions(+)

diff --git a/target/openrisc/interrupt.c b/target/openrisc/interrupt.c
index 5fe3f11..e1b0142 100644
--- a/target/openrisc/interrupt.c
+++ b/target/openrisc/interrupt.c
@@ -38,10 +38,17 @@ void openrisc_cpu_do_interrupt(CPUState *cs)
 env->flags &= ~D_FLAG;
 env->sr |= SR_DSX;
 env->epcr -= 4;
+} else {
+env->sr &= ~SR_DSX;
 }
 if (cs->exception_index == EXCP_SYSCALL) {
 env->epcr += 4;
 }
+/* When we have an illegal instruction the error effective address
+   shall be set to the illegal instruction address.  */
+if (cs->exception_index == EXCP_ILLEGAL) {
+env->eear = env->pc;
+}
 
 /* For machine-state changed between user-mode and supervisor mode,
we need flush TLB when we enter EXCP.  */
-- 
2.9.3

[Qemu-devel] [PATCH] hw/core/null-machine: Add the possibility to instantiate a CPU, RAM and kernel

[Thomas Huth]

Sometimes it is useful to have just a machine with CPU and RAM, without
any further hardware in it, e.g. if you just want to do some instruction
debugging for TCG with a remote GDB attached to QEMU, or run some embedded
code with the "-semihosting" QEMU parameter. qemu-system-m68k already
features a "dummy" machine, and xtensa a "sim" machine for exactly this
purpose.
All target architectures have nowadays also a "none" machine, which would
be a perfect match for this, too - but it currently does not allow to add
CPU, RAM or a kernel yet. Thus let's add these possibilities in a generic
way to the "none" machine, too, so that we hopefully do not need additional
"dummy" machines in the future anymore (and maybe can also get rid of the
already existing "dummy"/"sim" machines one day).
Note that the default behaviour of the "none" machine is not changed, i.e.
no CPU and no RAM is instantiated by default. You've explicitely got to
specify the CPU model with "-cpu" and the amount of RAM with "-m" to get
these new features.

Signed-off-by: Thomas Huth 
---
 hw/core/Makefile.objs  |  2 +-
 hw/core/null-machine.c | 81 +-
 2 files changed, 81 insertions(+), 2 deletions(-)

diff --git a/hw/core/Makefile.objs b/hw/core/Makefile.objs
index a4c94e5..0b6c0f1 100644
--- a/hw/core/Makefile.objs
+++ b/hw/core/Makefile.objs
@@ -12,7 +12,6 @@ common-obj-$(CONFIG_XILINX_AXI) += stream.o
 common-obj-$(CONFIG_PTIMER) += ptimer.o
 common-obj-$(CONFIG_SOFTMMU) += sysbus.o
 common-obj-$(CONFIG_SOFTMMU) += machine.o
-common-obj-$(CONFIG_SOFTMMU) += null-machine.o
 common-obj-$(CONFIG_SOFTMMU) += loader.o
 common-obj-$(CONFIG_SOFTMMU) += qdev-properties-system.o
 common-obj-$(CONFIG_SOFTMMU) += register.o
@@ -20,3 +19,4 @@ common-obj-$(CONFIG_SOFTMMU) += or-irq.o
 common-obj-$(CONFIG_PLATFORM_BUS) += platform-bus.o
 
 obj-$(CONFIG_SOFTMMU) += generic-loader.o
+obj-$(CONFIG_SOFTMMU) += null-machine.o
diff --git a/hw/core/null-machine.c b/hw/core/null-machine.c
index 0351ba7..b2468ed 100644
--- a/hw/core/null-machine.c
+++ b/hw/core/null-machine.c
@@ -13,18 +13,97 @@
 
 #include "qemu/osdep.h"
 #include "qemu-common.h"
+#include "qemu/error-report.h"
 #include "hw/hw.h"
 #include "hw/boards.h"
+#include "hw/loader.h"
+#include "sysemu/sysemu.h"
+#include "exec/address-spaces.h"
+#include "cpu.h"
+#include "elf.h"
+
+#ifdef TARGET_WORDS_BIGENDIAN
+#define LOAD_ELF_ENDIAN_FLAG 1
+#else
+#define LOAD_ELF_ENDIAN_FLAG 0
+#endif
+
+static hwaddr cpu_initial_pc;
+
+static uint64_t translate_phys_addr(void *opaque, uint64_t addr)
+{
+return cpu_get_phys_page_debug(CPU(opaque), addr);
+}
+
+static void machine_none_load_kernel(CPUState *cpu, const char *kernel_fname,
+ ram_addr_t ram_size)
+{
+uint64_t elf_entry;
+int kernel_size;
+
+if (!ram_size) {
+error_report("You need RAM for loading a kernel");
+return;
+}
+
+kernel_size = load_elf(kernel_fname, translate_phys_addr, cpu, _entry,
+   NULL, NULL, LOAD_ELF_ENDIAN_FLAG, EM_NONE, 0, 0);
+cpu_initial_pc = elf_entry;
+if (kernel_size < 0) {
+kernel_size = load_uimage(kernel_fname, _initial_pc, NULL, NULL,
+  NULL, NULL);
+}
+if (kernel_size < 0) {
+kernel_size = load_image_targphys(kernel_fname, 0, ram_size);
+}
+if (kernel_size < 0) {
+error_report("Could not load kernel '%s'", kernel_fname);
+return;
+}
+}
+
+static void machine_none_cpu_reset(void *opaque)
+{
+CPUState *cpu = CPU(opaque);
+
+cpu_reset(cpu);
+cpu_set_pc(cpu, cpu_initial_pc);
+}
 
 static void machine_none_init(MachineState *machine)
 {
+ram_addr_t ram_size = machine->ram_size;
+MemoryRegion *ram;
+CPUState *cpu = NULL;
+
+/* Initialize CPU (if a model has been specified) */
+if (machine->cpu_model) {
+cpu = cpu_init(machine->cpu_model);
+if (!cpu) {
+error_report("Unable to initialize CPU");
+exit(1);
+}
+qemu_register_reset(machine_none_cpu_reset, cpu);
+cpu_reset(cpu);
+}
+
+/* RAM at address zero */
+if (ram_size) {
+ram = g_new(MemoryRegion, 1);
+memory_region_allocate_system_memory(ram, NULL, "ram", ram_size);
+memory_region_add_subregion(get_system_memory(), 0, ram);
+}
+
+if (machine->kernel_filename) {
+machine_none_load_kernel(cpu, machine->kernel_filename, ram_size);
+}
 }
 
 static void machine_none_machine_init(MachineClass *mc)
 {
 mc->desc = "empty machine";
 mc->init = machine_none_init;
-mc->max_cpus = 0;
+mc->default_ram_size = 0;
 }
 
 DEFINE_MACHINE("none", machine_none_machine_init)
-- 
1.8.3.1

[Qemu-devel] [PATCH] cryptodev: setiv only when really need

[Longpeng(Mike)]

ECB mode cipher doesn't need IV, if we setiv for it then qemu
crypto API would report "Expected IV size 0 not **", so we should
setiv only when the cipher algos really need.

Signed-off-by: Longpeng(Mike) 
---
 backends/cryptodev-builtin.c | 10 ++
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/backends/cryptodev-builtin.c b/backends/cryptodev-builtin.c
index 82a068e..b24a299 100644
--- a/backends/cryptodev-builtin.c
+++ b/backends/cryptodev-builtin.c
@@ -320,10 +320,12 @@ static int cryptodev_builtin_sym_operation(
 
 sess = builtin->sessions[op_info->session_id];
 
-ret = qcrypto_cipher_setiv(sess->cipher, op_info->iv,
-   op_info->iv_len, errp);
-if (ret < 0) {
-return -VIRTIO_CRYPTO_ERR;
+if (op_info->iv_len > 0) {
+ret = qcrypto_cipher_setiv(sess->cipher, op_info->iv,
+   op_info->iv_len, errp);
+if (ret < 0) {
+return -VIRTIO_CRYPTO_ERR;
+}
 }
 
 if (sess->direction == VIRTIO_CRYPTO_OP_ENCRYPT) {
-- 
1.8.3.1

Re: [Qemu-devel] [PATCH 0/2] virtio: revert virtio_queue_set_notification() nesting

[Richard Henderson]

On 01/12/2017 03:46 AM, Stefan Hajnoczi wrote:

The virtio_queue_set_notification() nesting introduced for AioContext polling
raised an assertion with virtio-net (even in non-polling mode).  Converting
virtio-net and virtio-crypto to use virtio_queue_set_notification() in a
nesting fashion would be invasive and isn't worth it.

Patch 1 contains the revert to resolve the bug that Doug noticed.

Patch 2 is a less efficient but safe alternative.

Stefan Hajnoczi (2):
  Revert "virtio: turn vq->notification into a nested counter"
  virtio: disable notifications again after poll succeeded

 hw/virtio/virtio.c | 21 +
 1 file changed, 9 insertions(+), 12 deletions(-)


Tested-by: Richard Henderson 

This problem affected Alpha as well.  I tested the two patches together.


r~

Re: [Qemu-devel] [PATCH v7 3/7] trace: [tcg] Delay changes to dynamic state when translating

[Richard Henderson]

On 01/13/2017 12:48 PM, Lluís Vilanova wrote:

This keeps consistency across all decisions taken during translation
when the dynamic state of a vCPU is changed in the middle of translating
some guest code.

Signed-off-by: Lluís Vilanova 
---
 include/qom/cpu.h  |3 +++
 qom/cpu.c  |2 ++
 trace/control-target.c |   21 ++---
 3 files changed, 23 insertions(+), 3 deletions(-)


Reviewed-by: Richard Henderson 


r~

Re: [Qemu-devel] [PATCH v7 2/7] trace: Make trace_get_vcpu_event_count() inlinable

[Richard Henderson]

On 01/13/2017 12:48 PM, Lluís Vilanova wrote:

uring and controlling the state of tracing events.
  *
- * Copyright (C) 2011-2016 Lluís Vilanova 
+ * Copyright (C) 2011-2017 Lluís Vilanova 
  *
  * This work is licensed under the terms of the GNU GPL, version 2 or later.
  * See the COPYING file in the top-level directory.
@@ -237,7 +237,7 @@ char *trace_opt_parse(const char *optarg);
  *
  * Return the number of known vcpu-specific events
  */
-uint32_t trace_get_vcpu_event_count(void);
+static uint32_t trace_get_vcpu_event_count(void);



Why is this declaration still here?  It's redundant with the inline.


r~

Re: [Qemu-devel] [PATCH RFC v3 00/14] VT-d: vfio enablement and misc enhances

[Peter Xu]

On Fri, Jan 13, 2017 at 05:58:02PM +0200, Michael S. Tsirkin wrote:
> On Fri, Jan 13, 2017 at 11:06:26AM +0800, Peter Xu wrote:
> > v3:
> > - fix style error reported by patchew
> > - fix comment in domain switch patch: use "IOMMU address space" rather
> >   than "IOMMU region" [Kevin]
> > - add ack-by for Paolo in patch:
> >   "memory: add section range info for IOMMU notifier"
> >   (this is seperately collected besides this thread)
> > - remove 3 patches which are merged already (from Jason)
> > - rebase to master b6c0897
> 
> So 1-6 look like nice cleanups to me. Should I merge them now?

That'll be great if you'd like to merge them. Then I can further
shorten this series for the next post.

Regarding to the error_report() issue that Jason has mentioned, I can
touch them up in the future when needed - after all, most of the patch
content are about converting DPRINT()s into traces.

Thanks!

-- peterx

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

[Gonglei (Arei)]

> 
> On Fri, Jan 13, 2017 at 02:54:29AM +, Gonglei (Arei) wrote:
> >
> > >
> > > On Thu, Jan 12, 2017 at 12:26:24PM +, Gonglei (Arei) wrote:
> > > > Hi,
> > > >
> > > >
> > > > >
> > > > > On 01/04/2017 11:10 AM, Gonglei (Arei) wrote:
> > > > > > Hi all,
> > > > > >
> > > > > > I attach the diff files between v14 and v15 for better review.
> > > > > >
> > > > > Hi,
> > > > >
> > > > > only had a quick look. Will try to come back to this later.
> > > > >
> > > > That's cool.
> > > >
> > > > > > diff --git a/virtio-crypto.tex b/virtio-crypto.tex
> > > > > > index 9f7faf0..884ee95 100644
> > > > > > --- a/virtio-crypto.tex
> > > > > > +++ b/virtio-crypto.tex
> > > > > > @@ -2,8 +2,8 @@
> > > > > >
> > > > > >  The virtio crypto device is a virtual cryptography device as well 
> > > > > > as a
> kind
> > > of
> > > > > >  virtual hardware accelerator for virtual machines. The encryption
> and
> > > > > > -decryption requests are placed in the data queue and are ultimately
> > > handled
> > > > > by the
> > > > > > -backend crypto accelerators. The second queue is the control queue
> used
> > > to
> > > > > create
> > > > > > +decryption requests are placed in any of the data active queues and
> are
> > > > > ultimately handled by the
> > > > > s/data active/active data/
> > > > > > +backend crypto accelerators. The second kind of queue is the 
> > > > > > control
> > > queue
> > > > > used to create
> > > > > >  or destroy sessions for symmetric algorithms and will control some
> > > > > advanced
> > > > > >  features in the future. The virtio crypto device provides the 
> > > > > > following
> > > crypto
> > > > > >  services: CIPHER, MAC, HASH, and AEAD.
> > > > >
> > > > > [..]
> > > > >
> > > > > > ===The below diff shows the changes of add
> non-session
> > > mode
> > > > > support:
> > > > > >
> > > > > > diff --git a/virtio-crypto.tex b/virtio-crypto.tex
> > > > > > index 884ee95..44819f9 100644
> > > > > > --- a/virtio-crypto.tex
> > > > > > +++ b/virtio-crypto.tex
> > > > > > @@ -26,7 +26,10 @@ N is set by \field{max_dataqueues}.
> > > > > >
> > > > > >  \subsection{Feature bits}\label{sec:Device Types / Crypto Device /
> > > Feature
> > > > > bits}
> > > > > >
> > > > > > -None currently defined.
> > > > > > +VIRTIO_CRYPTO_F_CIPHER_SESSION_MODE (1) Session mode is
> > > available
> > > > > for CIPHER service.
> > > > > > +VIRTIO_CRYPTO_F_HASH_SESSION_MODE (2) Session mode is
> available
> > > for
> > > > > HASH service.
> > > > > > +VIRTIO_CRYPTO_F_MAC_SESSION_MODE (3) Session mode is
> available
> > > for
> > > > > MAC service.
> > > > > > +VIRTIO_CRYPTO_F_AEAD_SESSION_MODE (4) Session mode is
> available
> > > for
> > > > > AEAD service.
> > > > > >
> > > > > >  \subsection{Device configuration layout}\label{sec:Device Types /
> > > Crypto
> > > > > Device / Device configuration layout}
> > > > > >
> > > > > > @@ -208,6 +211,9 @@ Operation parameters are algorithm-specific
> > > > > parameters, output data is the
> > > > > >  data that should be utilized in operations, and input data is 
> > > > > > equal to
> > > > > >  "operation result + result data".
> > > > > >
> > > > > > +The device can support both session mode (See \ref{sec:Device Types
> /
> > > > > Crypto Device / Device Operation / Control Virtqueue / Session
> operation})
> > > and
> > > > > non-session mode, for example,
> > > > > > +As VIRTIO_CRYPTO_F_CIPHER_SESSION feature bit is negotiated,
> the
> > > driver
> > > > > can use session mode for CIPHER service, otherwise it can only use
> > > non-session
> > > > > mode.
> > > > > > +
> > > > >
> > > > > As far as I understand you are adding non-session mode to the mix but
> > > > > providing feature bits for session mode. Would this render the the
> current
> > > > > implementation non-compliant?
> > > > >
> > > > You are right, shall we use feature bits for non-session mode for
> compliancy?
> > > > Or because the spec is on the fly, and some structures in the
> virtio_crypto.h
> > > need to
> > > > be modified, can we keep the compliancy completely?
> > > >
> > > > Thanks,
> > > > -Gonglei
> > >
> > > Since there's a linux driver upstream you must at least
> > > keep compatibility with that.
> > >
> > Sure. We use feature bits for non-session mode then.
> > For structures modification, do we need to do some specific
> > actions for compatibility?  Take CIPHER service as an example:
> >
> > The present structure:
> >
> > struct virtio_crypto_cipher_para {
> > /*
> >  * Byte Length of valid IV/Counter data pointed to by the below iv data.
> >  *
> >  * For block ciphers in CBC or F8 mode, or for Kasumi in F8 mode, or for
> >  *   SNOW3G in UEA2 mode, this is the length of the IV (which
> >  *   must be the same as the block length of the cipher).
> >  * For block ciphers in CTR mode, this is the length of the counter
> >  *   (which must be the same as the block length of the cipher).
> >  */
> > le32 iv_len;
> > /* 

Re: [Qemu-devel] [PATCH] virtio_crypto: header update

[Gonglei (Arei)]

> -Original Message-
> From: Michael S. Tsirkin [mailto:m...@redhat.com]
> Sent: Saturday, January 14, 2017 12:18 AM
> To: qemu-devel@nongnu.org
> Cc: Gonglei (Arei)
> Subject: [PATCH] virtio_crypto: header update
> 
> Update header from latest linux driver.  Session creation structs gain
> padding to make them same size. Formatting cleanups.
> 
> Signed-off-by: Michael S. Tsirkin 
> ---
>  include/standard-headers/linux/virtio_crypto.h | 481 
> +
>  1 file changed, 251 insertions(+), 230 deletions(-)
> 
Tested-by: Gonglei 
Reviewed-by: Gonglei 

Re: [Qemu-devel] [PATCH] libvhost-user: Start VQs on SET_VRING_CALL

[Felipe Franciosi]

> On 13 Jan 2017, at 10:18, Michael S. Tsirkin  wrote:
> 
> On Fri, Jan 13, 2017 at 05:15:22PM +, Felipe Franciosi wrote:
>> 
>>> On 13 Jan 2017, at 09:04, Michael S. Tsirkin  wrote:
>>> 
>>> On Fri, Jan 13, 2017 at 03:09:46PM +, Felipe Franciosi wrote:
 Hi Marc-Andre,
 
> On 13 Jan 2017, at 07:03, Marc-André Lureau  wrote:
> 
> Hi
> 
> - Original Message -
>> Currently, VQs are started as soon as a SET_VRING_KICK is received. That
>> is too early in the VQ setup process, as the backend might not yet have
> 
> I think we may want to reconsider queue_set_started(), move it elsewhere, 
> since kick/call fds aren't mandatory to process the rings.
 
 Hmm. The fds aren't mandatory, but I imagine in that case we should still 
 receive SET_VRING_KICK/CALL messages without an fd (ie. with the 
 VHOST_MSG_VQ_NOFD_MASK flag set). Wouldn't that be the case?
>>> 
>>> Please look at docs/specs/vhost-user.txt, Starting and stopping rings
>>> 
>>> The spec says:
>>> Client must start ring upon receiving a kick (that is, detecting that
>>> file descriptor is readable) on the descriptor specified by
>>> VHOST_USER_SET_VRING_KICK, and stop ring upon receiving
>>> VHOST_USER_GET_VRING_BASE.
>> 
>> Yes I have seen the spec, but there is a race with the current libvhost-user 
>> code which needs attention. My initial proposal (which got turned down) was 
>> to send a spurious notification upon seeing a callfd. Then I came up with 
>> this proposal. See below.
>> 
>>> 
>>> 
> 
>> a callfd to notify in case it received a kick and fully processed the
>> request/command. This patch only starts a VQ when a SET_VRING_CALL is
>> received.
> 
> I don't like that much, as soon as the kick fd is received, it should 
> start polling it imho. callfd is optional, it may have one and not the 
> other.
 
 So the question is whether we should be receiving a SET_VRING_CALL anyway 
 or not, regardless of an fd being sent. (I think we do, but I haven't done 
 extensive testing with other device types.)
>>> 
>>> I would say not, only KICK is mandatory and that is also not enough
>>> to process ring. You must wait for it to be readable.
>> 
>> The problem is that Qemu takes time between sending the kickfd and the 
>> callfd. Hence the race. Consider this scenario:
>> 
>> 1) Guest configures the device
>> 2) Guest put a request on a virtq
>> 3) Guest kicks
>> 4) Qemu starts configuring the backend
>> 4.a) Qemu sends the masked callfds
>> 4.b) Qemu sends the virtq sizes and addresses
>> 4.c) Qemu sends the kickfds
>> 
>> (When using MQ, Qemu will only send the callfd once all VQs are configured)
>> 
>> 5) The backend starts listening on the kickfd upon receiving it
>> 6) The backend picks up the guest's request
>> 7) The backend processes the request
>> 8) The backend puts the response on the used ring
>> 9) The backend notifies the masked callfd
>> 
>> 4.d) Qemu sends the callfds
>> 
>> At which point the guest missed the notification and gets stuck.
>> 
>> Perhaps you prefer my initial proposal of sending a spurious notification 
>> when the backend sees a callfd?
>> 
>> Felipe
> 
> I thought we read the masked callfd when we unmask it,
> and forward the interrupt. See kvm_irqfd_assign:
> 
>/*
> * Check if there was an event already pending on the eventfd
> * before we registered, and trigger it as if we didn't miss it.
> */
>events = f.file->f_op->poll(f.file, >pt);
> 
>if (events & POLLIN)
>schedule_work(>inject);
> 
> 
> 
> Is this a problem you observe in practice?

Thanks for pointing out to this code; I wasn't aware of it.

Indeed I'm encountering it in practice. And I've checked that my kernel has the 
code above.

Starts to sound like a race:
Qemu registers the new notifier with kvm
Backend kicks the (now no longer registered) maskfd
Qemu sends the new callfd to the application

It's not hard to repro. How could this situation be avoided?

Cheers,
Felipe


> 
>> 
>>> 
> 
> Perhaps it's best for now to delay the callfd notification with a flag 
> until it is received?
 
 The other idea is to always kick when we receive the callfd. I remember 
 discussing that alternative with you before libvhost-user went in. The 
 protocol says both the driver and the backend must handle spurious kicks. 
 This approach also fixes the bug.
 
 I'm happy with whatever alternative you want, as long it makes 
 libvhost-user usable for storage devices.
 
 Thanks,
 Felipe
 
 
> 
> 
>> Signed-off-by: Felipe Franciosi 
>> ---
>> contrib/libvhost-user/libvhost-user.c | 26 +-
>> 1 file changed, 13 insertions(+), 13 deletions(-)
>> 
>> diff --git 

[Qemu-devel] [PATCH v6 3/9] block: Avoid BlockDriverState.filename

[Max Reitz]

In places which directly pass a filename to the OS, we should not use
the filename field at all but exact_filename instead (although the
former currently equals the latter if that is set).

In raw_open_common(), we do not need to access BDS.filename because we
already have a local variable pointing to the filename.

Signed-off-by: Max Reitz 
---
 block.c| 5 +++--
 block/file-posix.c | 6 +++---
 block/file-win32.c | 4 ++--
 block/gluster.c| 3 ++-
 4 files changed, 10 insertions(+), 8 deletions(-)

diff --git a/block.c b/block.c
index 95015251d5..9943d8eff6 100644
--- a/block.c
+++ b/block.c
@@ -1146,8 +1146,9 @@ static int bdrv_open_common(BlockDriverState *bs, 
BdrvChild *file,
 if (ret < 0) {
 if (local_err) {
 error_propagate(errp, local_err);
-} else if (bs->filename[0]) {
-error_setg_errno(errp, -ret, "Could not open '%s'", bs->filename);
+} else if (bs->exact_filename[0]) {
+error_setg_errno(errp, -ret, "Could not open '%s'",
+ bs->exact_filename);
 } else {
 error_setg_errno(errp, -ret, "Could not open image");
 }
diff --git a/block/file-posix.c b/block/file-posix.c
index 28b47d977b..2e3acd622d 100644
--- a/block/file-posix.c
+++ b/block/file-posix.c
@@ -590,7 +590,7 @@ static int raw_reopen_prepare(BDRVReopenState *state,
 
 /* If we cannot use fcntl, or fcntl failed, fall back to qemu_open() */
 if (rs->fd == -1) {
-const char *normalized_filename = state->bs->filename;
+const char *normalized_filename = state->bs->exact_filename;
 ret = raw_normalize_devicepath(_filename);
 if (ret < 0) {
 error_setg_errno(errp, -ret, "Could not normalize device path");
@@ -2075,7 +2075,7 @@ static bool hdev_is_sg(BlockDriverState *bs)
 int sg_version;
 int ret;
 
-if (stat(bs->filename, ) < 0 || !S_ISCHR(st.st_mode)) {
+if (stat(bs->exact_filename, ) < 0 || !S_ISCHR(st.st_mode)) {
 return false;
 }
 
@@ -2510,7 +2510,7 @@ static int cdrom_reopen(BlockDriverState *bs)
  */
 if (s->fd >= 0)
 qemu_close(s->fd);
-fd = qemu_open(bs->filename, s->open_flags, 0644);
+fd = qemu_open(bs->exact_filename, s->open_flags, 0644);
 if (fd < 0) {
 s->fd = -1;
 return -EIO;
diff --git a/block/file-win32.c b/block/file-win32.c
index 800fabdd72..040c71830e 100644
--- a/block/file-win32.c
+++ b/block/file-win32.c
@@ -457,7 +457,7 @@ static void raw_close(BlockDriverState *bs)
 
 CloseHandle(s->hfile);
 if (bs->open_flags & BDRV_O_TEMPORARY) {
-unlink(bs->filename);
+unlink(bs->exact_filename);
 }
 }
 
@@ -525,7 +525,7 @@ static int64_t raw_get_allocated_file_size(BlockDriverState 
*bs)
   DWORD * high);
 get_compressed_t get_compressed;
 struct _stati64 st;
-const char *filename = bs->filename;
+const char *filename = bs->exact_filename;
 /* WinNT support GetCompressedFileSize to determine allocate size */
 get_compressed =
 (get_compressed_t) GetProcAddress(GetModuleHandle("kernel32"),
diff --git a/block/gluster.c b/block/gluster.c
index ded13fb5fb..f6c2484c11 100644
--- a/block/gluster.c
+++ b/block/gluster.c
@@ -878,7 +878,8 @@ static int qemu_gluster_reopen_prepare(BDRVReopenState 
*state,
 gconf->has_debug = true;
 gconf->logfile = g_strdup(s->logfile);
 gconf->has_logfile = true;
-reop_s->glfs = qemu_gluster_init(gconf, state->bs->filename, NULL, errp);
+reop_s->glfs = qemu_gluster_init(gconf, state->bs->exact_filename, NULL,
+ errp);
 if (reop_s->glfs == NULL) {
 ret = -errno;
 goto exit;
-- 
2.11.0

[Qemu-devel] [PATCH v6 0/9] block: Drop BDS.filename

[Max Reitz]

*** This series is based on v4 of my ***
*** "block: Fix some filename generation issues" series ***


The BDS filename field is generally only used when opening disk images
or emitting error or warning messages, the only exception to this rule
is the map command of qemu-img. However, using exact_filename there
instead should not be a problem. Therefore, we can drop the filename
field from the BlockDriverState and use a function instead which builds
the filename from scratch when called.

This is slower than reading a static char array but the problem of that
static array is that it may become obsolete due to changes in any
BlockDriverState or in the BDS graph. Using a function which rebuilds
the filename every time it is called resolves this problem.

The disadvantage of worse performance is negligible, on the other hand.
After patch 3 of this series, which replaces some queries of
BDS.filename by reads from somewhere else (mostly BDS.exact_filename),
the filename field is only used when a disk image is opened or some
message should be emitted, both of which cases do not suffer from the
performance hit.


http://lists.nongnu.org/archive/html/qemu-devel/2015-06/msg07025.html
gives an example of how to achieve an outdated filename, so we do need
this series. We can technically fix it differently (by appropriately
invoking bdrv_refresh_filename()), but that is pretty difficult™. I find
this series simpler and it it's something we wanted to do anyway.

Regarding the fear that this might change current behavior, especially
for libvirt which used filenames to identify nodes in the BDS graph:
Since bdrv_open() already calls bdrv_refresh_filename() today, and
apparently everything works fine, this series will most likely not break
anything. The only thing that will change is if the BDS graph is
dynamically reconfigured at runtime, and in that case it's most likely a
bug fix. Also, I don't think libvirt supports such cases already.


tl;dr: This series is a bug fix and I don't think it'll break legacy
management applications relying on the filename to identify a BDS; as
long as they are not trying to continue that practice with more advanced
configurations (e.g. with Quorum).


v6:
- Rebased after more than a year
- Patch 1 is newly required for patch 6
- Patch 3: Rebase conflicts
- Patch 4: Related bug fix; fits well into this series because it
   requires that format drivers do not query their
   bs->exact_filename, and this series can ensure exactly that
   (and after patch 3, we can be pretty sure this is the case)
- Patch 5: Rebase conflicts, improved a comment, noticed that I'm now
   replacing all of the existing bdrv_refresh_filename() calls
   and changed the commit message accordingly
- Patch 6: Rewritten because the affected code has been rewritten in the
   meantime.
- Patch 7: Rebase conflicts
- Patch 8: Added in this version. It makes the series a bit more
   rigorous, and I think that's good.
- Patch 9: Rebase conflict


git-backport-diff against v5:

Key:
[] : patches are identical
[] : number of functional differences between upstream/downstream patch
[down] : patch is downstream-only
The flags [FC] indicate (F)unctional and (C)ontextual differences, respectively

001/9:[down] 'block: Always set *file in get_block_status'
002/9:[] [-C] 'block: Change bdrv_get_encrypted_filename()'
003/9:[0011] [FC] 'block: Avoid BlockDriverState.filename'
004/9:[down] 'block: Do not blindly copy filename from file'
005/9:[0020] [FC] 'block: Add bdrv_filename()'
006/9:[0041] [FC] 'qemu-img: Use bdrv_filename() for map'
007/9:[0072] [FC] 'block: Drop BlockDriverState.filename'
008/9:[down] 'block: Complete move to pull filename updates'
009/9:[0003] [FC] 'iotests: Test changed Quorum filename'


Max Reitz (9):
  block: Always set *file in get_block_status
  block: Change bdrv_get_encrypted_filename()
  block: Avoid BlockDriverState.filename
  block: Do not blindly copy filename from file
  block: Add bdrv_filename()
  qemu-img: Use bdrv_filename() for map
  block: Drop BlockDriverState.filename
  block: Complete move to pull filename updates
  iotests: Test changed Quorum filename

 include/block/block.h |  3 +-
 include/block/block_int.h | 13 ++-
 block.c   | 96 ++-
 block/commit.c|  4 +-
 block/file-posix.c|  6 +--
 block/file-win32.c|  4 +-
 block/gluster.c   |  3 +-
 block/io.c|  6 ++-
 block/mirror.c| 16 ++--
 block/nbd.c   |  5 ++-
 block/nfs.c   |  4 +-
 block/qapi.c  |  4 +-
 block/raw-format.c|  4 +-
 block/replication.c   |  2 -
 block/vhdx-log.c  |  7 +++-
 block/vmdk.c  | 24 +---
 block/vpc.c   |  7 +++-
 blockdev.c| 33 +++-
 monitor.c |  5 ++-
 qemu-img.c| 

[Qemu-devel] [PATCH v6 1/9] block: Always set *file in get_block_status

[Max Reitz]

*file should always be set (to NULL, if nothing else) instead of leaving
it dangling sometimes. This should also be documented so callers can
rely on this behavior.

Signed-off-by: Max Reitz 
---
 block/io.c | 6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/block/io.c b/block/io.c
index 4f005623f7..ff693d7f73 100644
--- a/block/io.c
+++ b/block/io.c
@@ -1709,7 +1709,8 @@ typedef struct BdrvCoGetBlockStatusData {
  * beyond the end of the disk image it will be clamped.
  *
  * If returned value is positive and BDRV_BLOCK_OFFSET_VALID bit is set, 'file'
- * points to the BDS which the sector range is allocated in.
+ * points to the BDS which the sector range is allocated in. If the block 
driver
+ * does not set 'file', it will be set to NULL.
  */
 static int64_t coroutine_fn bdrv_co_get_block_status(BlockDriverState *bs,
  int64_t sector_num,
@@ -1720,6 +1721,8 @@ static int64_t coroutine_fn 
bdrv_co_get_block_status(BlockDriverState *bs,
 int64_t n;
 int64_t ret, ret2;
 
+*file = NULL;
+
 total_sectors = bdrv_nb_sectors(bs);
 if (total_sectors < 0) {
 return total_sectors;
@@ -1744,7 +1747,6 @@ static int64_t coroutine_fn 
bdrv_co_get_block_status(BlockDriverState *bs,
 return ret;
 }
 
-*file = NULL;
 bdrv_inc_in_flight(bs);
 ret = bs->drv->bdrv_co_get_block_status(bs, sector_num, nb_sectors, pnum,
 file);
-- 
2.11.0

[Qemu-devel] [PATCH v7 7/7] trace: [trivial] Statically enable all guest events

[Lluís Vilanova]

The optimizations of this series makes it feasible to have them
available on all builds.

Signed-off-by: Lluís Vilanova 
---
 trace-events |6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/trace-events b/trace-events
index f74e1d3d22..0a0f4d9cd6 100644
--- a/trace-events
+++ b/trace-events
@@ -159,7 +159,7 @@ vcpu guest_cpu_reset(void)
 #
 # Mode: user, softmmu
 # Targets: TCG(all)
-disable vcpu tcg guest_mem_before(TCGv vaddr, uint8_t info) "info=%d", 
"vaddr=0x%016"PRIx64" info=%d"
+vcpu tcg guest_mem_before(TCGv vaddr, uint8_t info) "info=%d", 
"vaddr=0x%016"PRIx64" info=%d"
 
 # @num: System call number.
 # @arg*: System call argument value.
@@ -168,7 +168,7 @@ disable vcpu tcg guest_mem_before(TCGv vaddr, uint8_t info) 
"info=%d", "vaddr=0x
 #
 # Mode: user
 # Targets: TCG(all)
-disable vcpu guest_user_syscall(uint64_t num, uint64_t arg1, uint64_t arg2, 
uint64_t arg3, uint64_t arg4, uint64_t arg5, uint64_t arg6, uint64_t arg7, 
uint64_t arg8) "num=0x%016"PRIx64" arg1=0x%016"PRIx64" arg2=0x%016"PRIx64" 
arg3=0x%016"PRIx64" arg4=0x%016"PRIx64" arg5=0x%016"PRIx64" arg6=0x%016"PRIx64" 
arg7=0x%016"PRIx64" arg8=0x%016"PRIx64
+vcpu guest_user_syscall(uint64_t num, uint64_t arg1, uint64_t arg2, uint64_t 
arg3, uint64_t arg4, uint64_t arg5, uint64_t arg6, uint64_t arg7, uint64_t 
arg8) "num=0x%016"PRIx64" arg1=0x%016"PRIx64" arg2=0x%016"PRIx64" 
arg3=0x%016"PRIx64" arg4=0x%016"PRIx64" arg5=0x%016"PRIx64" arg6=0x%016"PRIx64" 
arg7=0x%016"PRIx64" arg8=0x%016"PRIx64
 
 # @num: System call number.
 # @ret: System call result value.
@@ -177,4 +177,4 @@ disable vcpu guest_user_syscall(uint64_t num, uint64_t 
arg1, uint64_t arg2, uint
 #
 # Mode: user
 # Targets: TCG(all)
-disable vcpu guest_user_syscall_ret(uint64_t num, uint64_t ret) 
"num=0x%016"PRIx64" ret=0x%016"PRIx64
+vcpu guest_user_syscall_ret(uint64_t num, uint64_t ret) "num=0x%016"PRIx64" 
ret=0x%016"PRIx64

[Qemu-devel] [PATCH v7 6/7] trace: [tcg, trivial] Re-align generated code

[Lluís Vilanova]

Last patch removed a nesting level in generated code. Re-align all code
generated by backends to be 4-column aligned.

Signed-off-by: Lluís Vilanova 
---
 scripts/tracetool/backend/dtrace.py |4 ++--
 scripts/tracetool/backend/ftrace.py |   20 ++--
 scripts/tracetool/backend/log.py|   19 ++-
 scripts/tracetool/backend/simple.py |4 ++--
 scripts/tracetool/backend/syslog.py |6 +++---
 scripts/tracetool/backend/ust.py|4 ++--
 6 files changed, 29 insertions(+), 28 deletions(-)

diff --git a/scripts/tracetool/backend/dtrace.py 
b/scripts/tracetool/backend/dtrace.py
index 79505c6b1a..c29ffb4fa0 100644
--- a/scripts/tracetool/backend/dtrace.py
+++ b/scripts/tracetool/backend/dtrace.py
@@ -6,7 +6,7 @@ DTrace/SystemTAP backend.
 """
 
 __author__ = "Lluís Vilanova "
-__copyright__  = "Copyright 2012-2016, Lluís Vilanova "
+__copyright__  = "Copyright 2012-2017, Lluís Vilanova "
 __license__= "GPL version 2 or (at your option) any later version"
 
 __maintainer__ = "Stefan Hajnoczi"
@@ -41,6 +41,6 @@ def generate_h_begin(events, group):
 
 
 def generate_h(event, group):
-out('QEMU_%(uppername)s(%(argnames)s);',
+out('QEMU_%(uppername)s(%(argnames)s);',
 uppername=event.name.upper(),
 argnames=", ".join(event.args.names()))
diff --git a/scripts/tracetool/backend/ftrace.py 
b/scripts/tracetool/backend/ftrace.py
index db9fe7ad57..dd0eda4441 100644
--- a/scripts/tracetool/backend/ftrace.py
+++ b/scripts/tracetool/backend/ftrace.py
@@ -29,17 +29,17 @@ def generate_h(event, group):
 if len(event.args) > 0:
 argnames = ", " + argnames
 
-out('{',
-'char ftrace_buf[MAX_TRACE_STRLEN];',
-'int unused __attribute__ ((unused));',
-'int trlen;',
-'if (trace_event_get_state(%(event_id)s)) {',
-'trlen = snprintf(ftrace_buf, MAX_TRACE_STRLEN,',
-' "%(name)s " %(fmt)s "\\n" 
%(argnames)s);',
-'trlen = MIN(trlen, MAX_TRACE_STRLEN - 1);',
-'unused = write(trace_marker_fd, ftrace_buf, trlen);',
-'}',
+out('{',
+'char ftrace_buf[MAX_TRACE_STRLEN];',
+'int unused __attribute__ ((unused));',
+'int trlen;',
+'if (trace_event_get_state(%(event_id)s)) {',
+'trlen = snprintf(ftrace_buf, MAX_TRACE_STRLEN,',
+' "%(name)s " %(fmt)s "\\n" 
%(argnames)s);',
+'trlen = MIN(trlen, MAX_TRACE_STRLEN - 1);',
+'unused = write(trace_marker_fd, ftrace_buf, trlen);',
 '}',
+'}',
 name=event.name,
 args=event.args,
 event_id="TRACE_" + event.name.upper(),
diff --git a/scripts/tracetool/backend/log.py b/scripts/tracetool/backend/log.py
index 4f4a4d38b1..54f0a69886 100644
--- a/scripts/tracetool/backend/log.py
+++ b/scripts/tracetool/backend/log.py
@@ -6,7 +6,7 @@ Stderr built-in backend.
 """
 
 __author__ = "Lluís Vilanova "
-__copyright__  = "Copyright 2012-2016, Lluís Vilanova "
+__copyright__  = "Copyright 2012-2017, Lluís Vilanova "
 __license__= "GPL version 2 or (at your option) any later version"
 
 __maintainer__ = "Stefan Hajnoczi"
@@ -35,14 +35,15 @@ def generate_h(event, group):
 else:
 cond = "trace_event_get_state(%s)" % ("TRACE_" + event.name.upper())
 
-out('if (%(cond)s) {',
-'struct timeval _now;',
-'gettimeofday(&_now, NULL);',
-'qemu_log_mask(LOG_TRACE, "%%d@%%zd.%%06zd:%(name)s " 
%(fmt)s "\\n",',
-'  getpid(),',
-'  (size_t)_now.tv_sec, (size_t)_now.tv_usec',
-'  %(argnames)s);',
-'}',
+out('if (%(cond)s) {',
+'struct timeval _now;',
+'gettimeofday(&_now, NULL);',
+'qemu_log_mask(LOG_TRACE,',
+'  "%%d@%%zd.%%06zd:%(name)s " %(fmt)s "\\n",',
+'  getpid(),',
+'  (size_t)_now.tv_sec, (size_t)_now.tv_usec',
+'  %(argnames)s);',
+'}',
 cond=cond,
 name=event.name,
 fmt=event.fmt.rstrip("\n"),
diff --git a/scripts/tracetool/backend/simple.py 
b/scripts/tracetool/backend/simple.py
index 85f61028e2..9554673cb9 100644
--- a/scripts/tracetool/backend/simple.py
+++ b/scripts/tracetool/backend/simple.py
@@ -6,7 +6,7 @@ Simple built-in backend.
 """
 
 __author__ = "Lluís Vilanova "
-__copyright__  = "Copyright 2012-2014, Lluís 

[Qemu-devel] [PATCH v6 5/9] block: Add bdrv_filename()

[Max Reitz]

Split the part which actually refreshes the BlockDriverState.filename
field off of bdrv_refresh_filename() into a more generic function
bdrv_filename(), which first calls bdrv_refresh_filename() and then
stores a qemu-usable filename in the given buffer instead of
BlockDriverState.filename.

Since bdrv_refresh_filename() therefore no longer refreshes that field,
all of the existing calls to that function have to be replaced by calls
to bdrv_filename() "manually" refreshing the BDS filename field (this is
only temporary).

Signed-off-by: Max Reitz 
---
 include/block/block.h |  1 +
 block.c   | 50 +-
 block/replication.c   |  2 +-
 blockdev.c|  3 ++-
 4 files changed, 45 insertions(+), 11 deletions(-)

diff --git a/include/block/block.h b/include/block/block.h
index 3425e9fa79..8abc3da69f 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -252,6 +252,7 @@ BlockDriverState *bdrv_find_backing_image(BlockDriverState 
*bs,
 const char *backing_file);
 int bdrv_get_backing_file_depth(BlockDriverState *bs);
 void bdrv_refresh_filename(BlockDriverState *bs);
+char *bdrv_filename(BlockDriverState *bs, char *dest, size_t sz);
 int bdrv_truncate(BlockDriverState *bs, int64_t offset);
 int64_t bdrv_nb_sectors(BlockDriverState *bs);
 int64_t bdrv_getlength(BlockDriverState *bs);
diff --git a/block.c b/block.c
index 19f8a84d03..a631d94702 100644
--- a/block.c
+++ b/block.c
@@ -1731,7 +1731,8 @@ static BlockDriverState 
*bdrv_append_temp_snapshot(BlockDriverState *bs,
 bdrv_append(bs_snapshot, bs);
 
 bs_snapshot->backing_overridden = true;
-bdrv_refresh_filename(bs_snapshot);
+bdrv_filename(bs_snapshot, bs_snapshot->filename,
+  sizeof(bs_snapshot->filename));
 
 g_free(tmp_filename);
 return bs_snapshot;
@@ -1923,7 +1924,7 @@ static BlockDriverState *bdrv_open_inherit(const char 
*filename,
 }
 }
 
-bdrv_refresh_filename(bs);
+bdrv_filename(bs, bs->filename, sizeof(bs->filename));
 
 /* Check if any unknown options were used */
 if (options && (qdict_size(options) != 0)) {
@@ -4101,9 +4102,6 @@ static bool append_significant_runtime_options(QDict *d, 
BlockDriverState *bs)
  *  - full_open_options: Options which, when given when opening a block device
  *   (without a filename), result in a BDS (mostly)
  *   equalling the given one
- *  - filename: If exact_filename is set, it is copied here. Otherwise,
- *  full_open_options is converted to a JSON object, prefixed with
- *  "json:" (for use through the JSON pseudo protocol) and put 
here.
  */
 void bdrv_refresh_filename(BlockDriverState *bs)
 {
@@ -4120,7 +4118,8 @@ void bdrv_refresh_filename(BlockDriverState *bs)
 /* This BDS's file name may depend on any of its children's file names, so
  * refresh those first */
 QLIST_FOREACH(child, >children, next) {
-bdrv_refresh_filename(child->bs);
+bdrv_filename(child->bs, child->bs->filename,
+  sizeof(child->bs->filename));
 
 if (child->role == _backing && child->bs->backing_overridden) {
 bs->backing_overridden = true;
@@ -4184,15 +4183,48 @@ void bdrv_refresh_filename(BlockDriverState *bs)
 strcpy(bs->exact_filename, bs->file->bs->exact_filename);
 }
 }
+}
+
+/* First refreshes exact_filename and full_open_options by calling
+ * bdrv_refresh_filename(). Then, if exact_filename is set, it is copied into
+ * the target buffer. Otherwise, full_open_options is converted to a JSON
+ * object, prefixed with "json:" (for use through the JSON pseudo protocol) and
+ * put there.
+ *
+ * If @dest is not NULL, the filename will be truncated to @sz - 1 bytes and
+ * placed there. If @sz > 0, it will always be null-terminated.
+ *
+ * If @dest is NULL, @sz is ignored and a new buffer will be allocated which is
+ * large enough to hold the filename and the trailing '\0'. This buffer is then
+ * returned and has to be freed by the caller when it is no longer needed.
+ *
+ * Returns @dest if it is not NULL, and the newly allocated buffer otherwise.
+ */
+char *bdrv_filename(BlockDriverState *bs, char *dest, size_t sz)
+{
+bdrv_refresh_filename(bs);
+
+if (sz > INT_MAX) {
+sz = INT_MAX;
+}
 
 if (bs->exact_filename[0]) {
-pstrcpy(bs->filename, sizeof(bs->filename), bs->exact_filename);
+if (dest) {
+pstrcpy(dest, sz, bs->exact_filename);
+} else {
+dest = g_strdup(bs->exact_filename);
+}
 } else {
 QString *json = qobject_to_json(QOBJECT(bs->full_open_options));
-snprintf(bs->filename, sizeof(bs->filename), "json:%s",
- qstring_get_str(json));
+if (dest) {
+snprintf(dest, sz, "json:%s", qstring_get_str(json));
+} else {
+dest = g_strdup_printf("json:%s", 

[Qemu-devel] [PATCH v7 3/7] trace: [tcg] Delay changes to dynamic state when translating

[Lluís Vilanova]

This keeps consistency across all decisions taken during translation
when the dynamic state of a vCPU is changed in the middle of translating
some guest code.

Signed-off-by: Lluís Vilanova 
---
 include/qom/cpu.h  |3 +++
 qom/cpu.c  |2 ++
 trace/control-target.c |   21 ++---
 3 files changed, 23 insertions(+), 3 deletions(-)

diff --git a/include/qom/cpu.h b/include/qom/cpu.h
index 3f79a8e955..31c3e6018d 100644
--- a/include/qom/cpu.h
+++ b/include/qom/cpu.h
@@ -295,6 +295,8 @@ struct qemu_work_item;
  * @kvm_fd: vCPU file descriptor for KVM.
  * @work_mutex: Lock to prevent multiple access to queued_work_*.
  * @queued_work_first: First asynchronous work pending.
+ * @trace_dstate_delayed: Delayed changes to trace_dstate (includes all changes
+ *to @trace_dstate).
  * @trace_dstate: Dynamic tracing state of events for this vCPU (bitmask).
  *
  * State of one CPU core or thread.
@@ -370,6 +372,7 @@ struct CPUState {
  * Dynamically allocated based on bitmap requried to hold up to
  * trace_get_vcpu_event_count() entries.
  */
+unsigned long *trace_dstate_delayed;
 unsigned long *trace_dstate;
 
 /* TODO Move common fields from CPUArchState here. */
diff --git a/qom/cpu.c b/qom/cpu.c
index 03d9190f8c..8e981ac6c7 100644
--- a/qom/cpu.c
+++ b/qom/cpu.c
@@ -367,6 +367,7 @@ static void cpu_common_initfn(Object *obj)
 QTAILQ_INIT(>breakpoints);
 QTAILQ_INIT(>watchpoints);
 
+cpu->trace_dstate_delayed = bitmap_new(trace_get_vcpu_event_count());
 cpu->trace_dstate = bitmap_new(trace_get_vcpu_event_count());
 
 cpu_exec_initfn(cpu);
@@ -375,6 +376,7 @@ static void cpu_common_initfn(Object *obj)
 static void cpu_common_finalize(Object *obj)
 {
 CPUState *cpu = CPU(obj);
+g_free(cpu->trace_dstate_delayed);
 g_free(cpu->trace_dstate);
 }
 
diff --git a/trace/control-target.c b/trace/control-target.c
index 7ebf6e0bcb..dba3b21bb0 100644
--- a/trace/control-target.c
+++ b/trace/control-target.c
@@ -1,13 +1,14 @@
 /*
  * Interface for configuring and controlling the state of tracing events.
  *
- * Copyright (C) 2014-2016 Lluís Vilanova 
+ * Copyright (C) 2014-2017 Lluís Vilanova 
  *
  * This work is licensed under the terms of the GNU GPL, version 2 or later.
  * See the COPYING file in the top-level directory.
  */
 
 #include "qemu/osdep.h"
+#include "qom/cpu.h"
 #include "cpu.h"
 #include "trace.h"
 #include "trace/control.h"
@@ -57,6 +58,13 @@ void trace_event_set_state_dynamic(TraceEvent *ev, bool 
state)
 }
 }
 
+static void trace_event_synchronize_vcpu_state_dynamic(
+CPUState *vcpu, run_on_cpu_data ignored)
+{
+bitmap_copy(vcpu->trace_dstate, vcpu->trace_dstate_delayed,
+trace_get_vcpu_event_count());
+}
+
 void trace_event_set_vcpu_state_dynamic(CPUState *vcpu,
 TraceEvent *ev, bool state)
 {
@@ -69,13 +77,20 @@ void trace_event_set_vcpu_state_dynamic(CPUState *vcpu,
 if (state_pre != state) {
 if (state) {
 trace_events_enabled_count++;
-set_bit(vcpu_id, vcpu->trace_dstate);
+set_bit(vcpu_id, vcpu->trace_dstate_delayed);
 (*ev->dstate)++;
 } else {
 trace_events_enabled_count--;
-clear_bit(vcpu_id, vcpu->trace_dstate);
+clear_bit(vcpu_id, vcpu->trace_dstate_delayed);
 (*ev->dstate)--;
 }
+/*
+ * Delay changes until next TB; we want all TBs to be built from a
+ * single set of dstate values to ensure consistency of generated
+ * tracing code.
+ */
+async_run_on_cpu(vcpu, trace_event_synchronize_vcpu_state_dynamic,
+ RUN_ON_CPU_NULL);
 }
 }
 

[Qemu-devel] [PATCH v6 4/9] block: Do not blindly copy filename from file

[Max Reitz]

bdrv_refresh_filename() can do the same and it has some checks whether
the filename can actually be inherited or not, so we can let it do its
job in bdrv_open_inherit() after bdrv_open_common() has been called.

The only thing we need to set in bdrv_open_common() is the
exact_filename of a BDS without an underlying file, for two reasons:
(1) It cannot be inherited from an underlying file BDS, so it has to be
set somewhere.
(2) The driver may need the filename in its bdrv_file_open()
implementation (format drivers do not need their own filename,
though they may need their file BDS's name).

Signed-off-by: Max Reitz 
---
 block.c | 7 +++
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/block.c b/block.c
index 9943d8eff6..19f8a84d03 100644
--- a/block.c
+++ b/block.c
@@ -1116,12 +1116,11 @@ static int bdrv_open_common(BlockDriverState *bs, 
BdrvChild *file,
 bs->detect_zeroes = value;
 }
 
-if (filename != NULL) {
-pstrcpy(bs->filename, sizeof(bs->filename), filename);
+if (!file && filename) {
+pstrcpy(bs->exact_filename, sizeof(bs->exact_filename), filename);
 } else {
-bs->filename[0] = '\0';
+assert(!drv->bdrv_needs_filename);
 }
-pstrcpy(bs->exact_filename, sizeof(bs->exact_filename), bs->filename);
 
 bs->drv = drv;
 bs->opaque = g_malloc0(drv->instance_size);
-- 
2.11.0

[Qemu-devel] [PATCH v6 2/9] block: Change bdrv_get_encrypted_filename()

[Max Reitz]

Instead of returning a pointer to the filename, g_strdup() it. This will
become necessary once we do not have BlockDriverState.filename anymore.

Signed-off-by: Max Reitz 
Reviewed-by: Eric Blake 
Reviewed-by: Kevin Wolf 
---
 include/block/block.h |  2 +-
 block.c   | 17 ++---
 monitor.c |  5 -
 3 files changed, 15 insertions(+), 9 deletions(-)

diff --git a/include/block/block.h b/include/block/block.h
index 7bcbd05338..3425e9fa79 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -432,7 +432,7 @@ void bdrv_round_to_clusters(BlockDriverState *bs,
 int64_t *cluster_offset,
 unsigned int *cluster_bytes);
 
-const char *bdrv_get_encrypted_filename(BlockDriverState *bs);
+char *bdrv_get_encrypted_filename(BlockDriverState *bs);
 void bdrv_get_backing_filename(BlockDriverState *bs,
char *filename, int filename_size);
 char *bdrv_get_full_backing_filename(BlockDriverState *bs, Error **errp);
diff --git a/block.c b/block.c
index c63fc1b2da..95015251d5 100644
--- a/block.c
+++ b/block.c
@@ -2858,10 +2858,12 @@ void bdrv_add_key(BlockDriverState *bs, const char 
*key, Error **errp)
 }
 } else {
 if (bdrv_key_required(bs)) {
+char *enc_filename = bdrv_get_encrypted_filename(bs);
 error_set(errp, ERROR_CLASS_DEVICE_ENCRYPTED,
   "'%s' (%s) is encrypted",
   bdrv_get_device_or_node_name(bs),
-  bdrv_get_encrypted_filename(bs));
+  enc_filename ?: "");
+g_free(enc_filename);
 }
 }
 }
@@ -3111,14 +3113,15 @@ bool bdrv_can_write_zeroes_with_unmap(BlockDriverState 
*bs)
 return false;
 }
 
-const char *bdrv_get_encrypted_filename(BlockDriverState *bs)
+char *bdrv_get_encrypted_filename(BlockDriverState *bs)
 {
-if (bs->backing && bs->backing->bs->encrypted)
-return bs->backing_file;
-else if (bs->encrypted)
-return bs->filename;
-else
+if (bs->backing && bs->backing->bs->encrypted) {
+return g_strdup(bs->backing_file);
+} else if (bs->encrypted) {
+return g_strdup(bs->filename);
+} else {
 return NULL;
+}
 }
 
 void bdrv_get_backing_filename(BlockDriverState *bs,
diff --git a/monitor.c b/monitor.c
index 0841d436b0..bb3000a2df 100644
--- a/monitor.c
+++ b/monitor.c
@@ -4056,10 +4056,13 @@ int monitor_read_bdrv_key_start(Monitor *mon, 
BlockDriverState *bs,
 BlockCompletionFunc *completion_cb,
 void *opaque)
 {
+char *enc_filename;
 int err;
 
+enc_filename = bdrv_get_encrypted_filename(bs);
 monitor_printf(mon, "%s (%s) is encrypted.\n", bdrv_get_device_name(bs),
-   bdrv_get_encrypted_filename(bs));
+   enc_filename ?: "");
+g_free(enc_filename);
 
 mon->password_completion_cb = completion_cb;
 mon->password_opaque = opaque;
-- 
2.11.0

[Qemu-devel] [PATCH v7 4/7] exec: [tcg] Use different TBs according to the vCPU's dynamic tracing state

[Lluís Vilanova]

Every vCPU now uses a separate set of TBs for each set of dynamic
tracing event state values. Each set of TBs can be used by any number of
vCPUs to maximize TB reuse when vCPUs have the same tracing state.

This feature is later used by tracetool to optimize tracing of guest
code events.

The maximum number of TB sets is defined as 2^E, where E is the number
of events that have the 'vcpu' property (their state is stored in
CPUState->trace_dstate).

For this to work, a change on the dynamic tracing state of a vCPU will
force it to flush its virtual TB cache (which is only indexed by
address), and fall back to the physical TB cache (which now contains the
vCPU's dynamic tracing state as part of the hashing function).

Signed-off-by: Lluís Vilanova 
Reviewed-by: Richard Henderson 
---
 cpu-exec.c|   22 +-
 include/exec/exec-all.h   |5 +
 include/exec/tb-hash-xx.h |8 +++-
 include/exec/tb-hash.h|5 +++--
 include/qemu-common.h |3 +++
 tests/qht-bench.c |2 +-
 trace/control-target.c|1 +
 trace/control.h   |3 +++
 translate-all.c   |   16 ++--
 9 files changed, 54 insertions(+), 11 deletions(-)

diff --git a/cpu-exec.c b/cpu-exec.c
index 4188fed3c6..36709cba1f 100644
--- a/cpu-exec.c
+++ b/cpu-exec.c
@@ -261,6 +261,7 @@ struct tb_desc {
 CPUArchState *env;
 tb_page_addr_t phys_page1;
 uint32_t flags;
+TRACE_QHT_VCPU_DSTATE_TYPE trace_vcpu_dstate;
 };
 
 static bool tb_cmp(const void *p, const void *d)
@@ -272,6 +273,7 @@ static bool tb_cmp(const void *p, const void *d)
 tb->page_addr[0] == desc->phys_page1 &&
 tb->cs_base == desc->cs_base &&
 tb->flags == desc->flags &&
+tb->trace_vcpu_dstate == desc->trace_vcpu_dstate &&
 !atomic_read(>invalid)) {
 /* check next page if needed */
 if (tb->page_addr[1] == -1) {
@@ -293,7 +295,8 @@ static bool tb_cmp(const void *p, const void *d)
 static TranslationBlock *tb_htable_lookup(CPUState *cpu,
   target_ulong pc,
   target_ulong cs_base,
-  uint32_t flags)
+  uint32_t flags,
+  uint32_t trace_vcpu_dstate)
 {
 tb_page_addr_t phys_pc;
 struct tb_desc desc;
@@ -302,10 +305,11 @@ static TranslationBlock *tb_htable_lookup(CPUState *cpu,
 desc.env = (CPUArchState *)cpu->env_ptr;
 desc.cs_base = cs_base;
 desc.flags = flags;
+desc.trace_vcpu_dstate = trace_vcpu_dstate;
 desc.pc = pc;
 phys_pc = get_page_addr_code(desc.env, pc);
 desc.phys_page1 = phys_pc & TARGET_PAGE_MASK;
-h = tb_hash_func(phys_pc, pc, flags);
+h = tb_hash_func(phys_pc, pc, flags, trace_vcpu_dstate);
 return qht_lookup(_ctx.tb_ctx.htable, tb_cmp, , h);
 }
 
@@ -317,16 +321,24 @@ static inline TranslationBlock *tb_find(CPUState *cpu,
 TranslationBlock *tb;
 target_ulong cs_base, pc;
 uint32_t flags;
+unsigned long trace_vcpu_dstate_bitmap;
+TRACE_QHT_VCPU_DSTATE_TYPE trace_vcpu_dstate;
 bool have_tb_lock = false;
 
+bitmap_copy(_vcpu_dstate_bitmap, cpu->trace_dstate,
+trace_get_vcpu_event_count());
+memcpy(_vcpu_dstate, _vcpu_dstate_bitmap,
+   sizeof(trace_vcpu_dstate));
+
 /* we record a subset of the CPU state. It will
always be the same before a given translated block
is executed. */
 cpu_get_tb_cpu_state(env, , _base, );
 tb = atomic_rcu_read(>tb_jmp_cache[tb_jmp_cache_hash_func(pc)]);
 if (unlikely(!tb || tb->pc != pc || tb->cs_base != cs_base ||
- tb->flags != flags)) {
-tb = tb_htable_lookup(cpu, pc, cs_base, flags);
+ tb->flags != flags ||
+ tb->trace_vcpu_dstate != trace_vcpu_dstate)) {
+tb = tb_htable_lookup(cpu, pc, cs_base, flags, trace_vcpu_dstate);
 if (!tb) {
 
 /* mmap_lock is needed by tb_gen_code, and mmap_lock must be
@@ -340,7 +352,7 @@ static inline TranslationBlock *tb_find(CPUState *cpu,
 /* There's a chance that our desired tb has been translated while
  * taking the locks so we check again inside the lock.
  */
-tb = tb_htable_lookup(cpu, pc, cs_base, flags);
+tb = tb_htable_lookup(cpu, pc, cs_base, flags, trace_vcpu_dstate);
 if (!tb) {
 /* if no translated code available, then translate it now */
 tb = tb_gen_code(cpu, pc, cs_base, flags, 0);
diff --git a/include/exec/exec-all.h b/include/exec/exec-all.h
index 57cd978578..ae74f61ea2 100644
--- a/include/exec/exec-all.h
+++ b/include/exec/exec-all.h
@@ -200,6 +200,10 @@ static inline void tlb_flush_by_mmuidx(CPUState *cpu, ...)
 #define USE_DIRECT_JUMP
 #endif
 
+/**
+ * TranslationBlock:
+ * 

[Qemu-devel] [PATCH v7 2/7] trace: Make trace_get_vcpu_event_count() inlinable

[Lluís Vilanova]

Later patches will make use of it.

Signed-off-by: Lluís Vilanova 
---
 trace/control-internal.h |7 ++-
 trace/control.c  |   11 +++
 trace/control.h  |4 ++--
 3 files changed, 11 insertions(+), 11 deletions(-)

diff --git a/trace/control-internal.h b/trace/control-internal.h
index a9d395a587..a8beb44847 100644
--- a/trace/control-internal.h
+++ b/trace/control-internal.h
@@ -1,7 +1,7 @@
 /*
  * Interface for configuring and controlling the state of tracing events.
  *
- * Copyright (C) 2011-2016 Lluís Vilanova 
+ * Copyright (C) 2011-2017 Lluís Vilanova 
  *
  * This work is licensed under the terms of the GNU GPL, version 2 or later.
  * See the COPYING file in the top-level directory.
@@ -16,6 +16,7 @@
 
 
 extern int trace_events_enabled_count;
+extern uint32_t trace_next_vcpu_id;
 
 
 static inline bool trace_event_is_pattern(const char *str)
@@ -82,6 +83,10 @@ static inline bool 
trace_event_get_vcpu_state_dynamic(CPUState *vcpu,
 return trace_event_get_vcpu_state_dynamic_by_vcpu_id(vcpu, vcpu_id);
 }
 
+static inline uint32_t trace_get_vcpu_event_count(void)
+{
+return trace_next_vcpu_id;
+}
 
 void trace_event_register_group(TraceEvent **events);
 
diff --git a/trace/control.c b/trace/control.c
index 1a7bee6ddc..8a52c7fc39 100644
--- a/trace/control.c
+++ b/trace/control.c
@@ -1,7 +1,7 @@
 /*
  * Interface for configuring and controlling the state of tracing events.
  *
- * Copyright (C) 2011-2016 Lluís Vilanova 
+ * Copyright (C) 2011-2017 Lluís Vilanova 
  *
  * This work is licensed under the terms of the GNU GPL, version 2 or later.
  * See the COPYING file in the top-level directory.
@@ -36,7 +36,7 @@ typedef struct TraceEventGroup {
 static TraceEventGroup *event_groups;
 static size_t nevent_groups;
 static uint32_t next_id;
-static uint32_t next_vcpu_id;
+uint32_t trace_next_vcpu_id;
 
 QemuOptsList qemu_trace_opts = {
 .name = "trace",
@@ -65,7 +65,7 @@ void trace_event_register_group(TraceEvent **events)
 for (i = 0; events[i] != NULL; i++) {
 events[i]->id = next_id++;
 if (events[i]->vcpu_id != TRACE_VCPU_EVENT_NONE) {
-events[i]->vcpu_id = next_vcpu_id++;
+events[i]->vcpu_id = trace_next_vcpu_id++;
 }
 }
 event_groups = g_renew(TraceEventGroup, event_groups, nevent_groups + 1);
@@ -299,8 +299,3 @@ char *trace_opt_parse(const char *optarg)
 
 return trace_file;
 }
-
-uint32_t trace_get_vcpu_event_count(void)
-{
-return next_vcpu_id;
-}
diff --git a/trace/control.h b/trace/control.h
index ccaeac8552..6dfbc53c8d 100644
--- a/trace/control.h
+++ b/trace/control.h
@@ -1,7 +1,7 @@
 /*
  * Interface for configuring and controlling the state of tracing events.
  *
- * Copyright (C) 2011-2016 Lluís Vilanova 
+ * Copyright (C) 2011-2017 Lluís Vilanova 
  *
  * This work is licensed under the terms of the GNU GPL, version 2 or later.
  * See the COPYING file in the top-level directory.
@@ -237,7 +237,7 @@ char *trace_opt_parse(const char *optarg);
  *
  * Return the number of known vcpu-specific events
  */
-uint32_t trace_get_vcpu_event_count(void);
+static uint32_t trace_get_vcpu_event_count(void);
 
 
 #include "trace/control-internal.h"

[Qemu-devel] [PATCH v7 5/7] trace: [tcg] Do not generate TCG code to trace dinamically-disabled events

[Lluís Vilanova]

If an event is dynamically disabled, the TCG code that calls the
execution-time tracer is not generated.

Removes the overheads of execution-time tracers for dynamically disabled
events. As a bonus, also avoids checking the event state when the
execution-time tracer is called from TCG-generated code (since otherwise
TCG would simply not call it).

Signed-off-by: Lluís Vilanova 
---
 scripts/tracetool/__init__.py|3 ++-
 scripts/tracetool/format/h.py|   26 +++---
 scripts/tracetool/format/tcg_h.py|   21 +
 scripts/tracetool/format/tcg_helper_c.py |5 +++--
 4 files changed, 41 insertions(+), 14 deletions(-)

diff --git a/scripts/tracetool/__init__.py b/scripts/tracetool/__init__.py
index 365446fa53..5f1576539e 100644
--- a/scripts/tracetool/__init__.py
+++ b/scripts/tracetool/__init__.py
@@ -6,7 +6,7 @@ Machinery for generating tracing-related intermediate files.
 """
 
 __author__ = "Lluís Vilanova "
-__copyright__  = "Copyright 2012-2016, Lluís Vilanova "
+__copyright__  = "Copyright 2012-2017, Lluís Vilanova "
 __license__= "GPL version 2 or (at your option) any later version"
 
 __maintainer__ = "Stefan Hajnoczi"
@@ -264,6 +264,7 @@ class Event(object):
 return self._FMT.findall(self.fmt)
 
 QEMU_TRACE   = "trace_%(name)s"
+QEMU_TRACE_NOCHECK   = "_nocheck__" + QEMU_TRACE
 QEMU_TRACE_TCG   = QEMU_TRACE + "_tcg"
 QEMU_DSTATE  = "_TRACE_%(NAME)s_DSTATE"
 QEMU_EVENT   = "_TRACE_%(NAME)s_EVENT"
diff --git a/scripts/tracetool/format/h.py b/scripts/tracetool/format/h.py
index 3682f4e6a8..aecf249d66 100644
--- a/scripts/tracetool/format/h.py
+++ b/scripts/tracetool/format/h.py
@@ -6,7 +6,7 @@ trace/generated-tracers.h
 """
 
 __author__ = "Lluís Vilanova "
-__copyright__  = "Copyright 2012-2016, Lluís Vilanova "
+__copyright__  = "Copyright 2012-2017, Lluís Vilanova "
 __license__= "GPL version 2 or (at your option) any later version"
 
 __maintainer__ = "Stefan Hajnoczi"
@@ -49,6 +49,19 @@ def generate(events, backend, group):
 backend.generate_begin(events, group)
 
 for e in events:
+# tracer without checks
+out('',
+'static inline void %(api)s(%(args)s)',
+'{',
+api=e.api(e.QEMU_TRACE_NOCHECK),
+args=e.args)
+
+if "disable" not in e.properties:
+backend.generate(e, group)
+
+out('}')
+
+# tracer wrapper with checks (per-vCPU tracing)
 if "vcpu" in e.properties:
 trace_cpu = next(iter(e.args))[1]
 cond = "trace_event_get_vcpu_state(%(cpu)s,"\
@@ -63,16 +76,15 @@ def generate(events, backend, group):
 'static inline void %(api)s(%(args)s)',
 '{',
 'if (%(cond)s) {',
+'%(api_nocheck)s(%(names)s);',
+'}',
+'}',
 api=e.api(),
+api_nocheck=e.api(e.QEMU_TRACE_NOCHECK),
 args=e.args,
+names=", ".join(e.args.names()),
 cond=cond)
 
-if "disable" not in e.properties:
-backend.generate(e, group)
-
-out('}',
-'}')
-
 backend.generate_end(events, group)
 
 out('#endif /* TRACE_%s_GENERATED_TRACERS_H */' % group.upper())
diff --git a/scripts/tracetool/format/tcg_h.py 
b/scripts/tracetool/format/tcg_h.py
index 5f213f6cba..9a73b5b268 100644
--- a/scripts/tracetool/format/tcg_h.py
+++ b/scripts/tracetool/format/tcg_h.py
@@ -6,7 +6,7 @@ Generate .h file for TCG code generation.
 """
 
 __author__ = "Lluís Vilanova "
-__copyright__  = "Copyright 2012-2016, Lluís Vilanova "
+__copyright__  = "Copyright 2012-2017, Lluís Vilanova "
 __license__= "GPL version 2 or (at your option) any later version"
 
 __maintainer__ = "Stefan Hajnoczi"
@@ -41,7 +41,7 @@ def generate(events, backend, group):
 
 for e in events:
 # just keep one of them
-if "tcg-trans" not in e.properties:
+if "tcg-exec" not in e.properties:
 continue
 
 out('static inline void %(name_tcg)s(%(args)s)',
@@ -53,12 +53,25 @@ def generate(events, backend, group):
 args_trans = e.original.event_trans.args
 args_exec = tracetool.vcpu.transform_args(
 "tcg_helper_c", e.original.event_exec, "wrapper")
+if "vcpu" in e.properties:
+trace_cpu = e.args.names()[0]
+cond = "trace_event_get_vcpu_state(%(cpu)s,"\
+   " TRACE_%(id)s)"\
+   % dict(
+   cpu=trace_cpu,
+   id=e.original.event_exec.name.upper())
+else:
+cond 

[Qemu-devel] [PATCH v7 0/7] trace: [tcg] Optimize per-vCPU tracing states with separate TB caches

[Lluís Vilanova]

Optimizes tracing of events with the 'tcg' and 'vcpu' properties (e.g., memory
accesses), making it feasible to statically enable them by default on all QEMU
builds.

Some quick'n'dirty numbers with 400.perlbench (SPECcpu2006) on the train input
(medium size - suns.pl) and the guest_mem_before event:

* vanilla, statically disabled
real0m2,259s
user0m2,252s
sys 0m0,004s

* vanilla, statically enabled (overhead: 2.18x)
real0m4,921s
user0m4,912s
sys 0m0,008s

* multi-tb, statically disabled (overhead: 0.99x) [within noise range]
real0m2,228s
user0m2,216s
sys 0m0,008s

* multi-tb, statically enabled (overhead: 0.99x) [within noise range]
real0m2,229s
user0m2,224s
sys 0m0,004s


Right now, events with the 'tcg' property always generate TCG code to trace that
event at guest code execution time, where the event's dynamic state is checked.

This series adds a performance optimization where TCG code for events with the
'tcg' and 'vcpu' properties is not generated if the event is dynamically
disabled. This optimization raises two issues:

* An event can be dynamically disabled/enabled after the corresponding TCG code
  has been generated (i.e., a new TB with the corresponding code should be
  used).

* Each vCPU can have a different dynamic state for the same event (i.e., tracing
  the memory accesses of only one process pinned to a vCPU).

To handle both issues, this series integrates the dynamic tracing event state
into the TB hashing function, so that vCPUs tracing different events will use
separate TBs. Note that only events with the 'vcpu' property are used for
hashing (as stored in the bitmap of CPUState->trace_dstate).

This makes dynamic event state changes on vCPUs very efficient, since they can
use TBs produced by other vCPUs while on the same event state combination (or
produced by the same vCPU, earlier).

Discarded alternatives:

* Emitting TCG code to check if an event needs tracing, where we should still
  move the tracing call code to either a cold path (making tracing performance
  worse), or leave it inlined (making non-tracing performance worse).

* Eliding TCG code only when *zero* vCPUs are tracing an event, since enabling
  it on a single vCPU will impact the performance of all other vCPUs that are
  not tracing that event.

Signed-off-by: Lluís Vilanova 
---

Changes in v7
=

* Fix delayed dstate changes (now uses async_run_on_cpu() as suggested by Paolo
  Bonzini).

* Note to Richard: patch 4 has been adapted to the new patch 3 async callback,
  but is essentially the same.


Changes in v6
=

* Check hashing size error with QEMU_BUILD_BUG_ON [Richard Henderson].


Changes in v5
=

* Move define into "qemu-common.h" to allow compilation of tests.


Changes in v4
=

* Incorporate trace_dstate into the TB hashing function instead of using
  multiple physical TB caches [suggested by Richard Henderson].


Changes in v3
=

* Rebase on 0737f32daf.
* Do not use reserved symbol prefixes ("__") [Stefan Hajnoczi].
* Refactor trace_get_vcpu_event_count() to be inlinable.
* Optimize cpu_tb_cache_set_requested() (hottest path).


Changes in v2
=

* Fix bitmap copy in cpu_tb_cache_set_apply().
* Split generated code re-alignment into a separate patch [Daniel P. Berrange].


Lluís Vilanova (7):
  exec: [tcg] Refactor flush of per-CPU virtual TB cache
  trace: Make trace_get_vcpu_event_count() inlinable
  trace: [tcg] Delay changes to dynamic state when translating
  exec: [tcg] Use different TBs according to the vCPU's dynamic tracing 
state
  trace: [tcg] Do not generate TCG code to trace dinamically-disabled events
  trace: [tcg,trivial] Re-align generated code
  trace: [trivial] Statically enable all guest events


 cpu-exec.c   |   22 +-
 cputlb.c |2 +-
 include/exec/exec-all.h  |   11 +++
 include/exec/tb-hash-xx.h|8 +++-
 include/exec/tb-hash.h   |5 +++--
 include/qemu-common.h|3 +++
 include/qom/cpu.h|3 +++
 qom/cpu.c|2 ++
 scripts/tracetool/__init__.py|3 ++-
 scripts/tracetool/backend/dtrace.py  |4 ++--
 scripts/tracetool/backend/ftrace.py  |   20 ++--
 scripts/tracetool/backend/log.py |   19 ++-
 scripts/tracetool/backend/simple.py  |4 ++--
 scripts/tracetool/backend/syslog.py  |6 +++---
 scripts/tracetool/backend/ust.py |4 ++--
 scripts/tracetool/format/h.py|   26 +++---
 scripts/tracetool/format/tcg_h.py|   21 +
 scripts/tracetool/format/tcg_helper_c.py |5 +++--
 tests/qht-bench.c|2 +-
 trace-events

[Qemu-devel] [PATCH v7 1/7] exec: [tcg] Refactor flush of per-CPU virtual TB cache

[Lluís Vilanova]

The function is reused in later patches.

Signed-off-by: Lluís Vilanova 
Reviewed-by: Richard Henderson 
---
 cputlb.c|2 +-
 include/exec/exec-all.h |6 ++
 translate-all.c |   14 +-
 3 files changed, 16 insertions(+), 6 deletions(-)

diff --git a/cputlb.c b/cputlb.c
index 813279f3bc..9bf9960e1b 100644
--- a/cputlb.c
+++ b/cputlb.c
@@ -80,7 +80,7 @@ void tlb_flush(CPUState *cpu, int flush_global)
 
 memset(env->tlb_table, -1, sizeof(env->tlb_table));
 memset(env->tlb_v_table, -1, sizeof(env->tlb_v_table));
-memset(cpu->tb_jmp_cache, 0, sizeof(cpu->tb_jmp_cache));
+tb_flush_jmp_cache_all(cpu);
 
 env->vtlb_index = 0;
 env->tlb_flush_addr = -1;
diff --git a/include/exec/exec-all.h b/include/exec/exec-all.h
index a8c13cee66..57cd978578 100644
--- a/include/exec/exec-all.h
+++ b/include/exec/exec-all.h
@@ -256,6 +256,12 @@ struct TranslationBlock {
 };
 
 void tb_free(TranslationBlock *tb);
+/**
+ * tb_flush_jmp_cache_all:
+ *
+ * Flush the virtual translation block cache.
+ */
+void tb_flush_jmp_cache_all(CPUState *env);
 void tb_flush(CPUState *cpu);
 void tb_phys_invalidate(TranslationBlock *tb, tb_page_addr_t page_addr);
 
diff --git a/translate-all.c b/translate-all.c
index 3dd9214904..29ccb9e546 100644
--- a/translate-all.c
+++ b/translate-all.c
@@ -941,11 +941,7 @@ static void do_tb_flush(CPUState *cpu, run_on_cpu_data 
tb_flush_count)
 }
 
 CPU_FOREACH(cpu) {
-int i;
-
-for (i = 0; i < TB_JMP_CACHE_SIZE; ++i) {
-atomic_set(>tb_jmp_cache[i], NULL);
-}
+tb_flush_jmp_cache_all(cpu);
 }
 
 tcg_ctx.tb_ctx.nb_tbs = 0;
@@ -1741,6 +1737,14 @@ void tb_check_watchpoint(CPUState *cpu)
 }
 }
 
+void tb_flush_jmp_cache_all(CPUState *cpu)
+{
+int i;
+for (i = 0; i < TB_JMP_CACHE_SIZE; ++i) {
+atomic_set(>tb_jmp_cache[i], NULL);
+}
+}
+
 #ifndef CONFIG_USER_ONLY
 /* in deterministic execution mode, instructions doing device I/Os
must be at the end of the TB */

Re: [Qemu-devel] [PATCH v3 5/5] target-m68k: increment/decrement with SP

[Richard Henderson]

On 01/13/2017 10:36 AM, Laurent Vivier wrote:
> On 680x0 family only.
> 
> Address Register indirect With postincrement:
> 
> When using the stack pointer (A7) with byte size data, the register
> is incremented by two.
> 
> Address Register indirect With predecrement:
> 
> When using the stack pointer (A7) with byte size data, the register
> is decremented by two.
> 
> Signed-off-by: Laurent Vivier 
> Reviewed-by: Thomas Huth 
> ---
>  target/m68k/translate.c | 14 --
>  1 file changed, 12 insertions(+), 2 deletions(-)

Reviewed-by: Richard Henderson 


r~

Re: [Qemu-devel] [PATCH v6 3/7] trace: [tcg] Delay changes to dynamic state when translating

[Lluís Vilanova]

Paolo Bonzini writes:

> On 12/01/2017 20:37, Lluís Vilanova wrote:
>> Stefan Hajnoczi writes:
>> 
>>> On Tue, Jan 10, 2017 at 05:31:37PM +0100, Paolo Bonzini wrote:
 On 09/01/2017 18:01, Stefan Hajnoczi wrote:
> Or use a simpler scheme:
> 
> struct CPUState {
> ...
> uint32_t dstate_update_count;
> };
> 
> In trace_event_set_vcpu_state_dynamic():
> 
> if (state) {
> trace_events_enabled_count++;
> set_bit(vcpu_id, vcpu->trace_dstate_delayed);
> atomic_inc(>dstate_update_count, 1);
> (*ev->dstate)++;
> } ...
> 
> In cpu_exec() and friends:
> 
> last_dstate_update_count = atomic_read(>dstate_update_count);
> 
> tb = tb_find(cpu, last_tb, tb_exit);
> cpu_loop_exec_tb(cpu, tb, _tb, _exit, );
> 
> /* apply and disable delayed dstate changes */
> if (unlikely(atomic_read(>dstate_update_count) != 
> last_dstate_update_count)) {
> bitmap_copy(cpu->trace_dstate, cpu->trace_dstate_delayed,
> trace_get_vcpu_event_count());
> }
> 
> (You'll need to adjust the details but the update counter approach
> should be workable.)
 
 Would it work to use async_run_on_cpu?
>> 
>>> I think so.
>> 
>> AFAIU we cannot use async_run_on_cpu(), since we need to reset the local
>> variable "last_tb" to avoid chaining TBs with different dstates, and we 
>> cannot
>> use cpu_loop_exit() inside the callback.

> async_run_on_cpu would run as soon as the currently executing TB
> finishes, and would leave cpu_exec completely, so there would be no
> chaining.

Aha, I've re-read the internals used by async and that'll be sufficient.


Thanks,
  Lluis

[Qemu-devel] [PULL 2/4] target/arm: Fix ubfx et al for aarch64

[Richard Henderson]

The patch in 59a71b4c5b4e suffered from a merge failure
when compared to the original patch in

  http://lists.nongnu.org/archive/html/qemu-devel/2016-12/msg00137.html

Signed-off-by: Richard Henderson 
---
 target/arm/translate-a64.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/target/arm/translate-a64.c b/target/arm/translate-a64.c
index 4f09dfb..d0352e2 100644
--- a/target/arm/translate-a64.c
+++ b/target/arm/translate-a64.c
@@ -3217,7 +3217,7 @@ static void disas_bitfield(DisasContext *s, uint32_t insn)
 tcg_tmp = read_cpu_reg(s, rn, 1);
 
 /* Recognize simple(r) extractions.  */
-if (si <= ri) {
+if (si >= ri) {
 /* Wd = Wn */
 len = (si - ri) + 1;
 if (opc == 0) { /* SBFM: ASR, SBFX, SXTB, SXTH, SXTW */
-- 
2.9.3

[Qemu-devel] [PULL 0/4] tcg fixes

[Richard Henderson]

Two problems found with my most recent tcg-2.9 queued patches;
two patches for tcg/aarch64 that had been submitted but somehow
dropped off the patch queue.

With this, aarch64 risu passes on aarch64 again.


r~


The following changes since commit b6af8ea60282df514f87d32e36afd1c9aeee28c8:

  Merge remote-tracking branch 
'remotes/ehabkost/tags/x86-and-machine-pull-request' into staging (2017-01-13 
14:38:21 +)

are available in the git repository at:

  git://github.com/rth7680/qemu.git tags/pull-tcg-20170113

for you to fetch changes up to 8cf9a3d3f7a4b95f33e0bda5416b9c93ec887dd3:

  tcg/aarch64: Fix tcg_out_movi (2017-01-13 11:47:29 -0800)


Fixes and more queued patches


Richard Henderson (4):
  tcg/s390: Fix merge error with facilities
  target/arm: Fix ubfx et al for aarch64
  tcg/aarch64: Fix addsub2 for 0+C
  tcg/aarch64: Fix tcg_out_movi

 target/arm/translate-a64.c   |  2 +-
 tcg/aarch64/tcg-target.inc.c | 66 ++--
 tcg/s390/tcg-target.inc.c|  2 +-
 3 files changed, 35 insertions(+), 35 deletions(-)

[Qemu-devel] [PULL 4/4] tcg/aarch64: Fix tcg_out_movi

[Richard Henderson]

There were some patterns, like 0x___00ff, for which we
would select to begin a multi-insn sequence with MOVN, but would
fail to set the 0x lane back from 0x.

Cc: qemu-sta...@nongnu.org
Signed-off-by: Richard Henderson 
Message-Id: <20161207180727.6286-3-...@twiddle.net>
---
 tcg/aarch64/tcg-target.inc.c | 57 +++-
 1 file changed, 24 insertions(+), 33 deletions(-)

diff --git a/tcg/aarch64/tcg-target.inc.c b/tcg/aarch64/tcg-target.inc.c
index deb5967..6d227a5 100644
--- a/tcg/aarch64/tcg-target.inc.c
+++ b/tcg/aarch64/tcg-target.inc.c
@@ -580,11 +580,9 @@ static void tcg_out_logicali(TCGContext *s, AArch64Insn 
insn, TCGType ext,
 static void tcg_out_movi(TCGContext *s, TCGType type, TCGReg rd,
  tcg_target_long value)
 {
-AArch64Insn insn;
 int i, wantinv, shift;
 tcg_target_long svalue = value;
 tcg_target_long ivalue = ~value;
-tcg_target_long imask;
 
 /* For 32-bit values, discard potential garbage in value.  For 64-bit
values within [2**31, 2**32-1], we can create smaller sequences by
@@ -630,42 +628,35 @@ static void tcg_out_movi(TCGContext *s, TCGType type, 
TCGReg rd,
 
 /* Would it take fewer insns to begin with MOVN?  For the value and its
inverse, count the number of 16-bit lanes that are 0.  */
-for (i = wantinv = imask = 0; i < 64; i += 16) {
+for (i = wantinv = 0; i < 64; i += 16) {
 tcg_target_long mask = 0xull << i;
-if ((value & mask) == 0) {
-wantinv -= 1;
-}
-if ((ivalue & mask) == 0) {
-wantinv += 1;
-imask |= mask;
-}
+wantinv -= ((value & mask) == 0);
+wantinv += ((ivalue & mask) == 0);
 }
 
-/* If we had more 0x than 0x, invert VALUE and use MOVN.  */
-insn = I3405_MOVZ;
-if (wantinv > 0) {
-value = ivalue;
-insn = I3405_MOVN;
-}
-
-/* Find the lowest lane that is not 0x.  */
-shift = ctz64(value) & (63 & -16);
-tcg_out_insn_3405(s, insn, type, rd, value >> shift, shift);
-
-if (wantinv > 0) {
-/* Re-invert the value, so MOVK sees non-inverted bits.  */
-value = ~value;
-/* Clear out all the 0x lanes.  */
-value ^= imask;
-}
-/* Clear out the lane that we just set.  */
-value &= ~(0xUL << shift);
-
-/* Iterate until all lanes have been set, and thus cleared from VALUE.  */
-while (value) {
+if (wantinv <= 0) {
+/* Find the lowest lane that is not 0x.  */
 shift = ctz64(value) & (63 & -16);
-tcg_out_insn(s, 3405, MOVK, type, rd, value >> shift, shift);
+tcg_out_insn(s, 3405, MOVZ, type, rd, value >> shift, shift);
+/* Clear out the lane that we just set.  */
 value &= ~(0xUL << shift);
+/* Iterate until all non-zero lanes have been processed.  */
+while (value) {
+shift = ctz64(value) & (63 & -16);
+tcg_out_insn(s, 3405, MOVK, type, rd, value >> shift, shift);
+value &= ~(0xUL << shift);
+}
+} else {
+/* Like above, but with the inverted value and MOVN to start.  */
+shift = ctz64(ivalue) & (63 & -16);
+tcg_out_insn(s, 3405, MOVN, type, rd, ivalue >> shift, shift);
+ivalue &= ~(0xUL << shift);
+while (ivalue) {
+shift = ctz64(ivalue) & (63 & -16);
+/* Provide MOVK with the non-inverted value.  */
+tcg_out_insn(s, 3405, MOVK, type, rd, ~(ivalue >> shift), shift);
+ivalue &= ~(0xUL << shift);
+}
 }
 }
 
-- 
2.9.3

[Qemu-devel] [PULL 1/4] tcg/s390: Fix merge error with facilities

[Richard Henderson]

The variable was renamed s390_facilities.

Signed-off-by: Richard Henderson 
---
 tcg/s390/tcg-target.inc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tcg/s390/tcg-target.inc.c b/tcg/s390/tcg-target.inc.c
index 0682d01..a679280 100644
--- a/tcg/s390/tcg-target.inc.c
+++ b/tcg/s390/tcg-target.inc.c
@@ -1096,7 +1096,7 @@ static int tgen_cmp(TCGContext *s, TCGType type, TCGCond 
c, TCGReg r1,
 /* If we only got here because of load-and-test,
and we couldn't use that, then we need to load
the constant into a register.  */
-if (!(facilities & FACILITY_EXT_IMM)) {
+if (!(s390_facilities & FACILITY_EXT_IMM)) {
 c2 = TCG_TMP0;
 tcg_out_movi(s, type, c2, 0);
 goto do_reg;
-- 
2.9.3

[Qemu-devel] [PULL 3/4] tcg/aarch64: Fix addsub2 for 0+C

[Richard Henderson]

When al == xzr, we cannot use addi/subi because that encodes xsp.
Force a zero into the temp register for that (rare) case.

Cc: qemu-sta...@nongnu.org
Signed-off-by: Richard Henderson 
Message-Id: <20161207180727.6286-2-...@twiddle.net>
---
 tcg/aarch64/tcg-target.inc.c | 9 +
 1 file changed, 9 insertions(+)

diff --git a/tcg/aarch64/tcg-target.inc.c b/tcg/aarch64/tcg-target.inc.c
index 585b0d6..deb5967 100644
--- a/tcg/aarch64/tcg-target.inc.c
+++ b/tcg/aarch64/tcg-target.inc.c
@@ -964,6 +964,15 @@ static inline void tcg_out_addsub2(TCGContext *s, int ext, 
TCGReg rl,
 insn = I3401_SUBSI;
 bl = -bl;
 }
+if (unlikely(al == TCG_REG_XZR)) {
+/* ??? We want to allow al to be zero for the benefit of
+   negation via subtraction.  However, that leaves open the
+   possibility of adding 0+const in the low part, and the
+   immediate add instructions encode XSP not XZR.  Don't try
+   anything more elaborate here than loading another zero.  */
+al = TCG_REG_TMP;
+tcg_out_movi(s, ext, al, 0);
+}
 tcg_out_insn_3401(s, insn, ext, rl, al, bl);
 } else {
 tcg_out_insn_3502(s, sub ? I3502_SUBS : I3502_ADDS, ext, rl, al, bl);
-- 
2.9.3

Re: [Qemu-devel] [PATCH 39/40] char: move parallel chardev in its own file

[Eric Blake]

On 01/11/2017 11:29 AM, Marc-André Lureau wrote:
> Signed-off-by: Marc-André Lureau 
> ---
>  chardev/char-parallel.h |   9 ++
>  chardev/char-parallel.c | 293 
> 
>  chardev/char.c  | 288 +--
>  chardev/Makefile.objs   |   1 +
>  4 files changed, 304 insertions(+), 287 deletions(-)
>  create mode 100644 chardev/char-parallel.h
>  create mode 100644 chardev/char-parallel.c
> 

> +++ b/chardev/char.c

> @@ -123,7 +109,6 @@ void qemu_chr_be_generic_open(Chardev *s)
>  qemu_chr_be_event(s, CHR_EVENT_OPENED);
>  }
>  
> -
>  /* Not reporting errors from writing to logfile, as logs are
>   * defined to be "best effort" only */
>  static void qemu_chr_fe_write_log(Chardev *s,

Spurious hunk?

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



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH 40/40] char: headers clean-up

[Eric Blake]

On 01/11/2017 11:29 AM, Marc-André Lureau wrote:
> Those could probably be squashed with earlier patches, however I
> couldn't easily identify them, test them or check if there are still
> necessary on various platforms.
> 
> Signed-off-by: Marc-André Lureau 
> ---

Reviewed-by: Eric Blake 

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



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH 29/40] char: move win chardev base class in its own file

[Eric Blake]

On 01/11/2017 11:29 AM, Marc-André Lureau wrote:
> Signed-off-by: Marc-André Lureau 
> ---
>  chardev/char-win.h|  30 ++
>  chardev/char-win.c| 242 +++
>  chardev/char.c| 253 
> +-
>  chardev/Makefile.objs |   1 +
>  4 files changed, 276 insertions(+), 250 deletions(-)
>  create mode 100644 chardev/char-win.h
>  create mode 100644 chardev/char-win.c
> 

> +++ b/chardev/Makefile.objs
> @@ -4,3 +4,4 @@ chardev-obj-y += char-io.o
>  chardev-obj-y += char-mux.o
>  chardev-obj-y += char-null.o
>  chardev-obj-y += char-ringbuf.o
> +chardev-obj-$(CONFIG_WIN32) += char-win.o

This part is the neatest - trading #ifdefs in the code for conditionally
compiling an entire .c file.

I have not closely read 28-39, so I don't know if R-b is appropriate,
but they are all mechanical and the fact that it still compiles is good.
 So maybe we go with the weaker:
Acked-by: Eric Blake 

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



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PULL 000/180] QAPI patches for 2017-01-13

[Eric Blake]

On 01/13/2017 10:44 AM, Markus Armbruster wrote:
> This is Marc-André's "[PATCH v8 00/21] qapi doc generation (whole
> version, squashed)" with a few commit messages tweaked, and "[PATCH v8
> 14/21] (SQUASHED) move doc to schema" unsquashed into 161 patches.
> 
> We did all the respins with in this squashed form to reduce noise.
> However, since the unsquashed form is better suited for review, and
> probably nicer if we have to revisit this part of the work down the
> road, I'm proposing to merge this unsquashed.
> 
> If you want me to post the unsquashed patches, I'm happy to redo this
> pull request.
> 
> If you'd rather pull the squashed version, likewise.
> 
> I'm afraid this is a bit of a doc conflict magnet.  The sooner we can
> get it in, the easier for Marc-André and me.

Indeed - there's already a merge conflict with commit e1ff3c6, which
landed between the time you created the pull request and now.

Also, in trying to merge your branch locally, I get a rejection message
from my git hooks:

$ git commit
tests/qapi-schema/comments.out:7: new blank line at EOF.
tests/qapi-schema/event-case.out:8: new blank line at EOF.
tests/qapi-schema/ident-with-escape.out:10: new blank line at EOF.
tests/qapi-schema/include-relpath.out:7: new blank line at EOF.
tests/qapi-schema/include-repetition.out:7: new blank line at EOF.
tests/qapi-schema/include-simple.out:7: new blank line at EOF.
tests/qapi-schema/indented-expr.out:13: new blank line at EOF.
tests/qapi-schema/qapi-schema-test.out:446: new blank line at EOF.

Worth respinning to fix those issues?

>   qmp-commands: move 'query-memdev' doc to schema

This one is the conflict with current master.

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



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH] ahci: advertise HOST_CAP_64

[John Snow]

On 01/13/2017 02:01 PM, Ladi Prosek wrote:
> On Fri, Jan 13, 2017 at 7:31 PM, John Snow  wrote:
>>
>>
>> On 01/13/2017 01:12 PM, Ladi Prosek wrote:
>>> On Fri, Jan 13, 2017 at 6:23 PM, John Snow  wrote:
 On 01/13/2017 06:02 AM, Ladi Prosek wrote:
> The AHCI emulation code supports 64-bit addressing and should advertise 
> this
> fact in the Host Capabilities register. Both Linux and Windows drivers 
> test
> this bit to decide if the upper 32 bits of various registers may be 
> written
> to, and at least some versions of Windows have a bug where DMA is 
> attempted
> with an address above 4GB but, in the absence of HOST_CAP_64, the upper 32
> bits are left unititialized which leads to a memory corruption.
>
> Signed-off-by: Ladi Prosek 
> ---
>  hw/ide/ahci.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/hw/ide/ahci.c b/hw/ide/ahci.c
> index 3c19bda..6a17acf 100644
> --- a/hw/ide/ahci.c
> +++ b/hw/ide/ahci.c
> @@ -488,7 +488,7 @@ static void ahci_reg_init(AHCIState *s)
>  s->control_regs.cap = (s->ports - 1) |
>(AHCI_NUM_COMMAND_SLOTS << 8) |
>(AHCI_SUPPORTED_SPEED_GEN1 << 
> AHCI_SUPPORTED_SPEED) |
> -  HOST_CAP_NCQ | HOST_CAP_AHCI;
> +  HOST_CAP_NCQ | HOST_CAP_AHCI | HOST_CAP_64;
>
>  s->control_regs.impl = (1 << s->ports) - 1;
>
>

 Was this tested? What was the use case that prompted this patch, and do
 you have a public bugzilla to point to?
>>>
>>> Sorry, I thought you were aware. Here's the BZ with details:
>>> https://bugzilla.redhat.com/show_bug.cgi?id=1411105
>>>
>>
>> It's for the public record :)
>>
>> I'm going to amend your commit message, OK?
>>
>> https://github.com/jnsnow/qemu/commit/f037be92fc0c4580b846134e0355a69d0eccd0d9
> 
> Minor nit: the OS we know for sure is affected is Windows Server 2008,
> without the "R2". Thanks!
> 

I misunderstood. It looked like the initial report was for "SP2," though
I see you saying that the R2 version actually worked okay, but by
coincidence.

I didn't think there *was* an "SP2," for Windows Server, so I
interpreted that to mean R2.

So this only manifests with vanilla 2008?

>>> In short, Windows guests run into issues in certain virtual HW
>>> configurations if the bit is not set. I have tested the patch and
>>> verified that it fixes the failing cases.
>>>
>>
>> Great. I'm CCing qemu-stable as this should probably be included in
>> 2.7.2 / 2.8.1 / etc.
>>
 It looks correct via the spec, thank you for finding this oversight. It
 looks like everywhere this bit would make a difference we already do the
 correct thing for having this bit set.

 From what I can gather from the spec, it looks as if even 32bit guests
 must ensure that the upper 32bit registers are cleared, so I think we're
 all set here.

 Reviewed-by: John Snow 
>>
>> Thanks, applied to my IDE tree:
>>
>> https://github.com/jnsnow/qemu/commits/ide
>> https://github.com/jnsnow/qemu.git
>>
>> --js

Re: [Qemu-devel] [PATCH] ahci: advertise HOST_CAP_64

[Ladi Prosek]

On Fri, Jan 13, 2017 at 7:31 PM, John Snow  wrote:
>
>
> On 01/13/2017 01:12 PM, Ladi Prosek wrote:
>> On Fri, Jan 13, 2017 at 6:23 PM, John Snow  wrote:
>>> On 01/13/2017 06:02 AM, Ladi Prosek wrote:
 The AHCI emulation code supports 64-bit addressing and should advertise 
 this
 fact in the Host Capabilities register. Both Linux and Windows drivers test
 this bit to decide if the upper 32 bits of various registers may be written
 to, and at least some versions of Windows have a bug where DMA is attempted
 with an address above 4GB but, in the absence of HOST_CAP_64, the upper 32
 bits are left unititialized which leads to a memory corruption.

 Signed-off-by: Ladi Prosek 
 ---
  hw/ide/ahci.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

 diff --git a/hw/ide/ahci.c b/hw/ide/ahci.c
 index 3c19bda..6a17acf 100644
 --- a/hw/ide/ahci.c
 +++ b/hw/ide/ahci.c
 @@ -488,7 +488,7 @@ static void ahci_reg_init(AHCIState *s)
  s->control_regs.cap = (s->ports - 1) |
(AHCI_NUM_COMMAND_SLOTS << 8) |
(AHCI_SUPPORTED_SPEED_GEN1 << 
 AHCI_SUPPORTED_SPEED) |
 -  HOST_CAP_NCQ | HOST_CAP_AHCI;
 +  HOST_CAP_NCQ | HOST_CAP_AHCI | HOST_CAP_64;

  s->control_regs.impl = (1 << s->ports) - 1;


>>>
>>> Was this tested? What was the use case that prompted this patch, and do
>>> you have a public bugzilla to point to?
>>
>> Sorry, I thought you were aware. Here's the BZ with details:
>> https://bugzilla.redhat.com/show_bug.cgi?id=1411105
>>
>
> It's for the public record :)
>
> I'm going to amend your commit message, OK?
>
> https://github.com/jnsnow/qemu/commit/f037be92fc0c4580b846134e0355a69d0eccd0d9

Minor nit: the OS we know for sure is affected is Windows Server 2008,
without the "R2". Thanks!

>> In short, Windows guests run into issues in certain virtual HW
>> configurations if the bit is not set. I have tested the patch and
>> verified that it fixes the failing cases.
>>
>
> Great. I'm CCing qemu-stable as this should probably be included in
> 2.7.2 / 2.8.1 / etc.
>
>>> It looks correct via the spec, thank you for finding this oversight. It
>>> looks like everywhere this bit would make a difference we already do the
>>> correct thing for having this bit set.
>>>
>>> From what I can gather from the spec, it looks as if even 32bit guests
>>> must ensure that the upper 32bit registers are cleared, so I think we're
>>> all set here.
>>>
>>> Reviewed-by: John Snow 
>
> Thanks, applied to my IDE tree:
>
> https://github.com/jnsnow/qemu/commits/ide
> https://github.com/jnsnow/qemu.git
>
> --js

[Qemu-devel] [PATCH] target-i386: Remove AMD feature flag aliases from Opteron models

[Eduardo Habkost]

When CPU vendor is set to AMD, the AMD feature alias bits on
CPUID[0x8001].EDX are already automatically copied from CPUID[1].EDX
on x86_cpu_realizefn(). When CPU vendor is Intel, those bits are
reserved and should be zero. On either case, those bits shouldn't be set
in the CPU model table.

Commit 726a8ff68677d8d5fba17eb0ffb85076bfb598dc removed those
bits from most CPU models, but the Opteron_* entries still have
them. Remove the alias bits from Opteron_* too.

Add an assert() to x86_register_cpudef_type() to ensure we don't
make the same mistake again.

Signed-off-by: Eduardo Habkost 
---
 target/i386/cpu.c | 46 --
 1 file changed, 12 insertions(+), 34 deletions(-)

diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index a149c8dc42..af726a8fd0 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -1339,12 +1339,7 @@ static X86CPUDefinition builtin_x86_defs[] = {
 .features[FEAT_1_ECX] =
 CPUID_EXT_SSE3,
 .features[FEAT_8000_0001_EDX] =
-CPUID_EXT2_LM | CPUID_EXT2_FXSR | CPUID_EXT2_MMX |
-CPUID_EXT2_NX | CPUID_EXT2_PSE36 | CPUID_EXT2_PAT |
-CPUID_EXT2_CMOV | CPUID_EXT2_MCA | CPUID_EXT2_PGE |
-CPUID_EXT2_MTRR | CPUID_EXT2_SYSCALL | CPUID_EXT2_APIC |
-CPUID_EXT2_CX8 | CPUID_EXT2_MCE | CPUID_EXT2_PAE | CPUID_EXT2_MSR |
-CPUID_EXT2_TSC | CPUID_EXT2_PSE | CPUID_EXT2_DE | CPUID_EXT2_FPU,
+CPUID_EXT2_LM | CPUID_EXT2_NX | CPUID_EXT2_SYSCALL,
 .xlevel = 0x8008,
 .model_id = "AMD Opteron 240 (Gen 1 Class Opteron)",
 },
@@ -1365,13 +1360,7 @@ static X86CPUDefinition builtin_x86_defs[] = {
 CPUID_EXT_CX16 | CPUID_EXT_SSE3,
 /* Missing: CPUID_EXT2_RDTSCP */
 .features[FEAT_8000_0001_EDX] =
-CPUID_EXT2_LM | CPUID_EXT2_FXSR |
-CPUID_EXT2_MMX | CPUID_EXT2_NX | CPUID_EXT2_PSE36 |
-CPUID_EXT2_PAT | CPUID_EXT2_CMOV | CPUID_EXT2_MCA |
-CPUID_EXT2_PGE | CPUID_EXT2_MTRR | CPUID_EXT2_SYSCALL |
-CPUID_EXT2_APIC | CPUID_EXT2_CX8 | CPUID_EXT2_MCE |
-CPUID_EXT2_PAE | CPUID_EXT2_MSR | CPUID_EXT2_TSC | CPUID_EXT2_PSE |
-CPUID_EXT2_DE | CPUID_EXT2_FPU,
+CPUID_EXT2_LM | CPUID_EXT2_NX | CPUID_EXT2_SYSCALL,
 .features[FEAT_8000_0001_ECX] =
 CPUID_EXT3_SVM | CPUID_EXT3_LAHF_LM,
 .xlevel = 0x8008,
@@ -1395,13 +1384,7 @@ static X86CPUDefinition builtin_x86_defs[] = {
 CPUID_EXT_SSE3,
 /* Missing: CPUID_EXT2_RDTSCP */
 .features[FEAT_8000_0001_EDX] =
-CPUID_EXT2_LM | CPUID_EXT2_FXSR |
-CPUID_EXT2_MMX | CPUID_EXT2_NX | CPUID_EXT2_PSE36 |
-CPUID_EXT2_PAT | CPUID_EXT2_CMOV | CPUID_EXT2_MCA |
-CPUID_EXT2_PGE | CPUID_EXT2_MTRR | CPUID_EXT2_SYSCALL |
-CPUID_EXT2_APIC | CPUID_EXT2_CX8 | CPUID_EXT2_MCE |
-CPUID_EXT2_PAE | CPUID_EXT2_MSR | CPUID_EXT2_TSC | CPUID_EXT2_PSE |
-CPUID_EXT2_DE | CPUID_EXT2_FPU,
+CPUID_EXT2_LM | CPUID_EXT2_NX | CPUID_EXT2_SYSCALL,
 .features[FEAT_8000_0001_ECX] =
 CPUID_EXT3_MISALIGNSSE | CPUID_EXT3_SSE4A |
 CPUID_EXT3_ABM | CPUID_EXT3_SVM | CPUID_EXT3_LAHF_LM,
@@ -1428,13 +1411,8 @@ static X86CPUDefinition builtin_x86_defs[] = {
 CPUID_EXT_SSE3,
 /* Missing: CPUID_EXT2_RDTSCP */
 .features[FEAT_8000_0001_EDX] =
-CPUID_EXT2_LM |
-CPUID_EXT2_PDPE1GB | CPUID_EXT2_FXSR | CPUID_EXT2_MMX |
-CPUID_EXT2_NX | CPUID_EXT2_PSE36 | CPUID_EXT2_PAT |
-CPUID_EXT2_CMOV | CPUID_EXT2_MCA | CPUID_EXT2_PGE |
-CPUID_EXT2_MTRR | CPUID_EXT2_SYSCALL | CPUID_EXT2_APIC |
-CPUID_EXT2_CX8 | CPUID_EXT2_MCE | CPUID_EXT2_PAE | CPUID_EXT2_MSR |
-CPUID_EXT2_TSC | CPUID_EXT2_PSE | CPUID_EXT2_DE | CPUID_EXT2_FPU,
+CPUID_EXT2_LM | CPUID_EXT2_PDPE1GB | CPUID_EXT2_NX |
+CPUID_EXT2_SYSCALL,
 .features[FEAT_8000_0001_ECX] =
 CPUID_EXT3_FMA4 | CPUID_EXT3_XOP |
 CPUID_EXT3_3DNOWPREFETCH | CPUID_EXT3_MISALIGNSSE |
@@ -1464,13 +1442,8 @@ static X86CPUDefinition builtin_x86_defs[] = {
 CPUID_EXT_SSSE3 | CPUID_EXT_PCLMULQDQ | CPUID_EXT_SSE3,
 /* Missing: CPUID_EXT2_RDTSCP */
 .features[FEAT_8000_0001_EDX] =
-CPUID_EXT2_LM |
-CPUID_EXT2_PDPE1GB | CPUID_EXT2_FXSR | CPUID_EXT2_MMX |
-CPUID_EXT2_NX | CPUID_EXT2_PSE36 | CPUID_EXT2_PAT |
-CPUID_EXT2_CMOV | CPUID_EXT2_MCA | CPUID_EXT2_PGE |
-CPUID_EXT2_MTRR | CPUID_EXT2_SYSCALL | CPUID_EXT2_APIC |
-CPUID_EXT2_CX8 | CPUID_EXT2_MCE | CPUID_EXT2_PAE | CPUID_EXT2_MSR |
-CPUID_EXT2_TSC | CPUID_EXT2_PSE | CPUID_EXT2_DE | CPUID_EXT2_FPU,
+CPUID_EXT2_LM | CPUID_EXT2_PDPE1GB | CPUID_EXT2_NX |
+CPUID_EXT2_SYSCALL,
 

[Qemu-devel] [PATCH v3 2/5] target-m68k: fix gen_flush_flags()

[Laurent Vivier]

gen_flush_flags() is setting unconditionally cc_op_synced to 1
and s->cc_op to CC_OP_FLAGS, whereas env->cc_op can be set
to something else by a previous tcg fragment.

We fix that by not setting cc_op_synced to 1
(except for gen_helper_flush_flags() that updates env->cc_op)

FIX: https://github.com/vivier/qemu-m68k/issues/19

Signed-off-by: Laurent Vivier 
Reviewed-by: Richard Henderson 
---
 target/m68k/translate.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/target/m68k/translate.c b/target/m68k/translate.c
index 410f56a..0e97900 100644
--- a/target/m68k/translate.c
+++ b/target/m68k/translate.c
@@ -595,18 +595,19 @@ static void gen_flush_flags(DisasContext *s)
 
 case CC_OP_DYNAMIC:
 gen_helper_flush_flags(cpu_env, QREG_CC_OP);
+s->cc_op_synced = 1;
 break;
 
 default:
 t0 = tcg_const_i32(s->cc_op);
 gen_helper_flush_flags(cpu_env, t0);
 tcg_temp_free(t0);
+s->cc_op_synced = 1;
 break;
 }
 
 /* Note that flush_flags also assigned to env->cc_op.  */
 s->cc_op = CC_OP_FLAGS;
-s->cc_op_synced = 1;
 }
 
 static inline TCGv gen_extend(TCGv val, int opsize, int sign)
-- 
2.7.4

[Qemu-devel] [PATCH v3 1/5] target-m68k: fix bit operation with immediate value

[Laurent Vivier]

M680x0 bit operations with an immediate value use 9 bits of the 16bit
value, while coldfire ones use only 8 bits.

Signed-off-by: Laurent Vivier 
Reviewed-by: Richard Henderson 
---
 target/m68k/translate.c | 13 ++---
 1 file changed, 10 insertions(+), 3 deletions(-)

diff --git a/target/m68k/translate.c b/target/m68k/translate.c
index 5f7357e..410f56a 100644
--- a/target/m68k/translate.c
+++ b/target/m68k/translate.c
@@ -1801,9 +1801,16 @@ DISAS_INSN(bitop_im)
 op = (insn >> 6) & 3;
 
 bitnum = read_im16(env, s);
-if (bitnum & 0xff00) {
-disas_undef(env, s, insn);
-return;
+if (m68k_feature(s->env, M68K_FEATURE_M68000)) {
+if (bitnum & 0xfe00) {
+disas_undef(env, s, insn);
+return;
+}
+} else {
+if (bitnum & 0xff00) {
+disas_undef(env, s, insn);
+return;
+}
 }
 
 SRC_EA(env, src1, opsize, 0, op ? : NULL);
-- 
2.7.4

[Qemu-devel] [PATCH v3 4/5] target-m68k: CAS doesn't need aligned access

[Laurent Vivier]

Signed-off-by: Laurent Vivier 
Reviewed-by: Richard Henderson 
---
 target/m68k/translate.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/target/m68k/translate.c b/target/m68k/translate.c
index 23e2b06..cf5d8dd 100644
--- a/target/m68k/translate.c
+++ b/target/m68k/translate.c
@@ -1934,7 +1934,6 @@ DISAS_INSN(cas)
 default:
 g_assert_not_reached();
 }
-opc |= MO_ALIGN;
 
 ext = read_im16(env, s);
 
-- 
2.7.4

[Qemu-devel] [PATCH v3 5/5] target-m68k: increment/decrement with SP

[Laurent Vivier]

On 680x0 family only.

Address Register indirect With postincrement:

When using the stack pointer (A7) with byte size data, the register
is incremented by two.

Address Register indirect With predecrement:

When using the stack pointer (A7) with byte size data, the register
is decremented by two.

Signed-off-by: Laurent Vivier 
Reviewed-by: Thomas Huth 
---
 target/m68k/translate.c | 14 --
 1 file changed, 12 insertions(+), 2 deletions(-)

diff --git a/target/m68k/translate.c b/target/m68k/translate.c
index cf5d8dd..9f60fbc 100644
--- a/target/m68k/translate.c
+++ b/target/m68k/translate.c
@@ -725,7 +725,12 @@ static TCGv gen_lea_mode(CPUM68KState *env, DisasContext 
*s,
 }
 reg = get_areg(s, reg0);
 tmp = tcg_temp_new();
-tcg_gen_subi_i32(tmp, reg, opsize_bytes(opsize));
+if (reg0 == 7 && opsize == OS_BYTE &&
+m68k_feature(s->env, M68K_FEATURE_M68000)) {
+tcg_gen_subi_i32(tmp, reg, 2);
+} else {
+tcg_gen_subi_i32(tmp, reg, opsize_bytes(opsize));
+}
 return tmp;
 case 5: /* Indirect displacement.  */
 reg = get_areg(s, reg0);
@@ -801,7 +806,12 @@ static TCGv gen_ea_mode(CPUM68KState *env, DisasContext 
*s, int mode, int reg0,
 result = gen_ldst(s, opsize, reg, val, what);
 if (what == EA_STORE || !addrp) {
 TCGv tmp = tcg_temp_new();
-tcg_gen_addi_i32(tmp, reg, opsize_bytes(opsize));
+if (reg0 == 7 && opsize == OS_BYTE &&
+m68k_feature(s->env, M68K_FEATURE_M68000)) {
+tcg_gen_addi_i32(tmp, reg, 2);
+} else {
+tcg_gen_addi_i32(tmp, reg, opsize_bytes(opsize));
+}
 delay_set_areg(s, reg0, tmp, true);
 }
 return result;
-- 
2.7.4

[Qemu-devel] [PATCH v3 0/5] Fixes for target/m68k

[Laurent Vivier]

This is a series of fixes for target/m68k found:
- with RISU (bit operation with immediate)
- while debugging package build under chroot
  (gen_flush_flags() and CAS address modes)
- while I was working on the softmmu mode
  (CAS alignment and SP address modes)

v2:
- Don't align stack access on coldfire.

v3:
- Fix v2 :( that has introduced a subi instead of
  an addi

Laurent Vivier (5):
  target-m68k: fix bit operation with immediate value
  target-m68k: fix gen_flush_flags()
  target-m68k: manage pre-dec et post-inc in CAS
  target-m68k: CAS doesn't need aligned access
  target-m68k: increment/decrement with SP

 target/m68k/translate.c | 40 +---
 1 file changed, 33 insertions(+), 7 deletions(-)

-- 
2.7.4

[Qemu-devel] [PATCH v3 3/5] target-m68k: manage pre-dec et post-inc in CAS

[Laurent Vivier]

In these cases we must update the address register after
the operation.

Signed-off-by: Laurent Vivier 
Reviewed-by: Richard Henderson 
---
 target/m68k/translate.c | 9 +
 1 file changed, 9 insertions(+)

diff --git a/target/m68k/translate.c b/target/m68k/translate.c
index 0e97900..23e2b06 100644
--- a/target/m68k/translate.c
+++ b/target/m68k/translate.c
@@ -1963,6 +1963,15 @@ DISAS_INSN(cas)
 gen_partset_reg(opsize, DREG(ext, 0), load);
 
 tcg_temp_free(load);
+
+switch (extract32(insn, 3, 3)) {
+case 3: /* Indirect postincrement.  */
+tcg_gen_addi_i32(AREG(insn, 0), addr, opsize_bytes(opsize));
+break;
+case 4: /* Indirect predecrememnt.  */
+tcg_gen_mov_i32(AREG(insn, 0), addr);
+break;
+}
 }
 
 DISAS_INSN(cas2w)
-- 
2.7.4

Re: [Qemu-devel] [PATCH] ahci: advertise HOST_CAP_64

[John Snow]

On 01/13/2017 01:12 PM, Ladi Prosek wrote:
> On Fri, Jan 13, 2017 at 6:23 PM, John Snow  wrote:
>> On 01/13/2017 06:02 AM, Ladi Prosek wrote:
>>> The AHCI emulation code supports 64-bit addressing and should advertise this
>>> fact in the Host Capabilities register. Both Linux and Windows drivers test
>>> this bit to decide if the upper 32 bits of various registers may be written
>>> to, and at least some versions of Windows have a bug where DMA is attempted
>>> with an address above 4GB but, in the absence of HOST_CAP_64, the upper 32
>>> bits are left unititialized which leads to a memory corruption.
>>>
>>> Signed-off-by: Ladi Prosek 
>>> ---
>>>  hw/ide/ahci.c | 2 +-
>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/hw/ide/ahci.c b/hw/ide/ahci.c
>>> index 3c19bda..6a17acf 100644
>>> --- a/hw/ide/ahci.c
>>> +++ b/hw/ide/ahci.c
>>> @@ -488,7 +488,7 @@ static void ahci_reg_init(AHCIState *s)
>>>  s->control_regs.cap = (s->ports - 1) |
>>>(AHCI_NUM_COMMAND_SLOTS << 8) |
>>>(AHCI_SUPPORTED_SPEED_GEN1 << 
>>> AHCI_SUPPORTED_SPEED) |
>>> -  HOST_CAP_NCQ | HOST_CAP_AHCI;
>>> +  HOST_CAP_NCQ | HOST_CAP_AHCI | HOST_CAP_64;
>>>
>>>  s->control_regs.impl = (1 << s->ports) - 1;
>>>
>>>
>>
>> Was this tested? What was the use case that prompted this patch, and do
>> you have a public bugzilla to point to?
> 
> Sorry, I thought you were aware. Here's the BZ with details:
> https://bugzilla.redhat.com/show_bug.cgi?id=1411105
> 

It's for the public record :)

I'm going to amend your commit message, OK?

https://github.com/jnsnow/qemu/commit/f037be92fc0c4580b846134e0355a69d0eccd0d9

> In short, Windows guests run into issues in certain virtual HW
> configurations if the bit is not set. I have tested the patch and
> verified that it fixes the failing cases.
> 

Great. I'm CCing qemu-stable as this should probably be included in
2.7.2 / 2.8.1 / etc.

>> It looks correct via the spec, thank you for finding this oversight. It
>> looks like everywhere this bit would make a difference we already do the
>> correct thing for having this bit set.
>>
>> From what I can gather from the spec, it looks as if even 32bit guests
>> must ensure that the upper 32bit registers are cleared, so I think we're
>> all set here.
>>
>> Reviewed-by: John Snow 

Thanks, applied to my IDE tree:

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

--js

Re: [Qemu-devel] [PATCH v2 1/5] target-m68k: fix bit operation with immediate value

[Richard Henderson]

On 01/13/2017 10:23 AM, Laurent Vivier wrote:
> See "BSET Instruction Format:BIT NUMBER STATIC, SPECIFIED AS IMMEDIATE
> DATA", p. 4.58 of "M68000 FAMILY PROGRAMMER’S REFERENCE MANUAL", "BIT
> NUMBER" is bits 0 to 8 of extended word.

Oops, yep.  I misread that.

Reviewed-by: Richard Henderson 


r~

Re: [Qemu-devel] [PATCH v2 1/5] target-m68k: fix bit operation with immediate value

[Laurent Vivier]

Le 13/01/2017 à 18:55, Richard Henderson a écrit :
> On 01/13/2017 04:51 AM, Laurent Vivier wrote:
>> M680x0 bit operations with an immediate value use 9 bits of the 16bit
>> value, while coldfire ones use only 8 bits.
> 
> I don't see that in the reference manual.  Where do you get this from?

See "BSET Instruction Format:BIT NUMBER STATIC, SPECIFIED AS IMMEDIATE
DATA", p. 4.58 of "M68000 FAMILY PROGRAMMER’S REFERENCE MANUAL", "BIT
NUMBER" is bits 0 to 8 of extended word.

See "Test a Bit and Set", p. 4-22 of "ColdFire Family Programmer’s
Reference Manual, Rev. 3" (link given by Thomas), "Bit Number" is bits 0
to 7 of extended word.

I've found the problem with RISU.

Thanks,
Laurent

Re: [Qemu-devel] [PATCH v3 2/2] memory: hmp: add "-f" for "info mtree"

[Dr. David Alan Gilbert]

* Peter Xu (pet...@redhat.com) wrote:
> Adding one more option "-f" for "info mtree" to dump the flat views of
> all the address spaces.
> 
> This will be useful to debug the memory rendering logic, also it'll be
> much easier with it to know what memory region is handling what address
> range.
> 
> Signed-off-by: Peter Xu 
> ---
>  hmp-commands-info.hx  |  6 +++---
>  include/exec/memory.h |  2 +-
>  memory.c  | 40 +++-
>  monitor.c |  4 +++-
>  4 files changed, 46 insertions(+), 6 deletions(-)
> 
> diff --git a/hmp-commands-info.hx b/hmp-commands-info.hx
> index 55d50c4..b0f35e6 100644
> --- a/hmp-commands-info.hx
> +++ b/hmp-commands-info.hx
> @@ -249,9 +249,9 @@ ETEXI
>  
>  {
>  .name   = "mtree",
> -.args_type  = "",
> -.params = "",
> -.help   = "show memory tree",
> +.args_type  = "flatview:-f",
> +.params = "[-f]",
> +.help   = "show memory tree (-f: dump flat view for address 
> spaces)",
>  .cmd= hmp_info_mtree,
>  },
>  
> diff --git a/include/exec/memory.h b/include/exec/memory.h
> index bec9756..71db380 100644
> --- a/include/exec/memory.h
> +++ b/include/exec/memory.h
> @@ -1254,7 +1254,7 @@ void memory_global_dirty_log_start(void);
>   */
>  void memory_global_dirty_log_stop(void);
>  
> -void mtree_info(fprintf_function mon_printf, void *f);
> +void mtree_info(fprintf_function mon_printf, void *f, bool flatview);
>  
>  /**
>   * memory_region_dispatch_read: perform a read directly to the specified
> diff --git a/memory.c b/memory.c
> index c42bde4..25f1c25 100644
> --- a/memory.c
> +++ b/memory.c
> @@ -2564,12 +2564,50 @@ static void mtree_print_mr(fprintf_function 
> mon_printf, void *f,
>  }
>  }
>  
> -void mtree_info(fprintf_function mon_printf, void *f)
> +static void mtree_print_flatview(fprintf_function p, void *f,
> + AddressSpace *as)
> +{
> +FlatView *view = address_space_get_flatview(as);
> +FlatRange *range = >ranges[0];
> +MemoryRegion *mr;
> +int n = view->nr;
> +
> +if (n <= 0) {
> +p(f, MTREE_INDENT "No rendered FlatView for "
> +  "address space '%s'\n", as->name);

Do you need an unref there?

Dave

> +return;
> +}
> +
> +while (n--) {
> +mr = range->mr;
> +p(f, MTREE_INDENT TARGET_FMT_plx "-"
> +  TARGET_FMT_plx " (prio %d, %s): %s\n",
> +  int128_get64(range->addr.start),
> +  int128_get64(range->addr.start) + MR_SIZE(range->addr.size),
> +  mr->priority,
> +  memory_region_type(mr),
> +  memory_region_name(mr));
> +range++;
> +}
> +
> +flatview_unref(view);
> +}
> +
> +void mtree_info(fprintf_function mon_printf, void *f, bool flatview)
>  {
>  MemoryRegionListHead ml_head;
>  MemoryRegionList *ml, *ml2;
>  AddressSpace *as;
>  
> +if (flatview) {
> +QTAILQ_FOREACH(as, _spaces, address_spaces_link) {
> +mon_printf(f, "address-space (flat view): %s\n", as->name);
> +mtree_print_flatview(mon_printf, f, as);
> +mon_printf(f, "\n");
> +}
> +return;
> +}
> +
>  QTAILQ_INIT(_head);
>  
>  QTAILQ_FOREACH(as, _spaces, address_spaces_link) {
> diff --git a/monitor.c b/monitor.c
> index 0841d43..679cd52 100644
> --- a/monitor.c
> +++ b/monitor.c
> @@ -1529,7 +1529,9 @@ static void hmp_boot_set(Monitor *mon, const QDict 
> *qdict)
>  
>  static void hmp_info_mtree(Monitor *mon, const QDict *qdict)
>  {
> -mtree_info((fprintf_function)monitor_printf, mon);
> +bool flatview = qdict_get_try_bool(qdict, "flatview", false);
> +
> +mtree_info((fprintf_function)monitor_printf, mon, flatview);
>  }
>  
>  static void hmp_info_numa(Monitor *mon, const QDict *qdict)
> -- 
> 2.7.4
> 
--
Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK

Re: [Qemu-devel] [PATCH] libvhost-user: Start VQs on SET_VRING_CALL

[Michael S. Tsirkin]

On Fri, Jan 13, 2017 at 05:15:22PM +, Felipe Franciosi wrote:
> 
> > On 13 Jan 2017, at 09:04, Michael S. Tsirkin  wrote:
> > 
> > On Fri, Jan 13, 2017 at 03:09:46PM +, Felipe Franciosi wrote:
> >> Hi Marc-Andre,
> >> 
> >>> On 13 Jan 2017, at 07:03, Marc-André Lureau  wrote:
> >>> 
> >>> Hi
> >>> 
> >>> - Original Message -
>  Currently, VQs are started as soon as a SET_VRING_KICK is received. That
>  is too early in the VQ setup process, as the backend might not yet have
> >>> 
> >>> I think we may want to reconsider queue_set_started(), move it elsewhere, 
> >>> since kick/call fds aren't mandatory to process the rings.
> >> 
> >> Hmm. The fds aren't mandatory, but I imagine in that case we should still 
> >> receive SET_VRING_KICK/CALL messages without an fd (ie. with the 
> >> VHOST_MSG_VQ_NOFD_MASK flag set). Wouldn't that be the case?
> > 
> > Please look at docs/specs/vhost-user.txt, Starting and stopping rings
> > 
> > The spec says:
> > Client must start ring upon receiving a kick (that is, detecting that
> > file descriptor is readable) on the descriptor specified by
> > VHOST_USER_SET_VRING_KICK, and stop ring upon receiving
> > VHOST_USER_GET_VRING_BASE.
> 
> Yes I have seen the spec, but there is a race with the current libvhost-user 
> code which needs attention. My initial proposal (which got turned down) was 
> to send a spurious notification upon seeing a callfd. Then I came up with 
> this proposal. See below.
> 
> > 
> > 
> >>> 
>  a callfd to notify in case it received a kick and fully processed the
>  request/command. This patch only starts a VQ when a SET_VRING_CALL is
>  received.
> >>> 
> >>> I don't like that much, as soon as the kick fd is received, it should 
> >>> start polling it imho. callfd is optional, it may have one and not the 
> >>> other.
> >> 
> >> So the question is whether we should be receiving a SET_VRING_CALL anyway 
> >> or not, regardless of an fd being sent. (I think we do, but I haven't done 
> >> extensive testing with other device types.)
> > 
> > I would say not, only KICK is mandatory and that is also not enough
> > to process ring. You must wait for it to be readable.
> 
> The problem is that Qemu takes time between sending the kickfd and the 
> callfd. Hence the race. Consider this scenario:
> 
> 1) Guest configures the device
> 2) Guest put a request on a virtq
> 3) Guest kicks
> 4) Qemu starts configuring the backend
> 4.a) Qemu sends the masked callfds
> 4.b) Qemu sends the virtq sizes and addresses
> 4.c) Qemu sends the kickfds
> 
> (When using MQ, Qemu will only send the callfd once all VQs are configured)
> 
> 5) The backend starts listening on the kickfd upon receiving it
> 6) The backend picks up the guest's request
> 7) The backend processes the request
> 8) The backend puts the response on the used ring
> 9) The backend notifies the masked callfd
> 
> 4.d) Qemu sends the callfds
> 
> At which point the guest missed the notification and gets stuck.
> 
> Perhaps you prefer my initial proposal of sending a spurious notification 
> when the backend sees a callfd?
> 
> Felipe

I thought we read the masked callfd when we unmask it,
and forward the interrupt. See kvm_irqfd_assign:

/*
 * Check if there was an event already pending on the eventfd
 * before we registered, and trigger it as if we didn't miss it.
 */
events = f.file->f_op->poll(f.file, >pt);

if (events & POLLIN)
schedule_work(>inject);



Is this a problem you observe in practice?

> 
> > 
> >>> 
> >>> Perhaps it's best for now to delay the callfd notification with a flag 
> >>> until it is received?
> >> 
> >> The other idea is to always kick when we receive the callfd. I remember 
> >> discussing that alternative with you before libvhost-user went in. The 
> >> protocol says both the driver and the backend must handle spurious kicks. 
> >> This approach also fixes the bug.
> >> 
> >> I'm happy with whatever alternative you want, as long it makes 
> >> libvhost-user usable for storage devices.
> >> 
> >> Thanks,
> >> Felipe
> >> 
> >> 
> >>> 
> >>> 
>  Signed-off-by: Felipe Franciosi 
>  ---
>  contrib/libvhost-user/libvhost-user.c | 26 +-
>  1 file changed, 13 insertions(+), 13 deletions(-)
>  
>  diff --git a/contrib/libvhost-user/libvhost-user.c
>  b/contrib/libvhost-user/libvhost-user.c
>  index af4faad..a46ef90 100644
>  --- a/contrib/libvhost-user/libvhost-user.c
>  +++ b/contrib/libvhost-user/libvhost-user.c
>  @@ -607,19 +607,6 @@ vu_set_vring_kick_exec(VuDev *dev, VhostUserMsg 
>  *vmsg)
> DPRINT("Got kick_fd: %d for vq: %d\n", vmsg->fds[0], index);
> }
>  
>  -dev->vq[index].started = true;
>  -if (dev->iface->queue_set_started) {
>  -dev->iface->queue_set_started(dev, index, true);

Re: [Qemu-devel] [PATCH] ahci: advertise HOST_CAP_64

[Ladi Prosek]

On Fri, Jan 13, 2017 at 6:23 PM, John Snow  wrote:
> On 01/13/2017 06:02 AM, Ladi Prosek wrote:
>> The AHCI emulation code supports 64-bit addressing and should advertise this
>> fact in the Host Capabilities register. Both Linux and Windows drivers test
>> this bit to decide if the upper 32 bits of various registers may be written
>> to, and at least some versions of Windows have a bug where DMA is attempted
>> with an address above 4GB but, in the absence of HOST_CAP_64, the upper 32
>> bits are left unititialized which leads to a memory corruption.
>>
>> Signed-off-by: Ladi Prosek 
>> ---
>>  hw/ide/ahci.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/hw/ide/ahci.c b/hw/ide/ahci.c
>> index 3c19bda..6a17acf 100644
>> --- a/hw/ide/ahci.c
>> +++ b/hw/ide/ahci.c
>> @@ -488,7 +488,7 @@ static void ahci_reg_init(AHCIState *s)
>>  s->control_regs.cap = (s->ports - 1) |
>>(AHCI_NUM_COMMAND_SLOTS << 8) |
>>(AHCI_SUPPORTED_SPEED_GEN1 << 
>> AHCI_SUPPORTED_SPEED) |
>> -  HOST_CAP_NCQ | HOST_CAP_AHCI;
>> +  HOST_CAP_NCQ | HOST_CAP_AHCI | HOST_CAP_64;
>>
>>  s->control_regs.impl = (1 << s->ports) - 1;
>>
>>
>
> Was this tested? What was the use case that prompted this patch, and do
> you have a public bugzilla to point to?

Sorry, I thought you were aware. Here's the BZ with details:
https://bugzilla.redhat.com/show_bug.cgi?id=1411105

In short, Windows guests run into issues in certain virtual HW
configurations if the bit is not set. I have tested the patch and
verified that it fixes the failing cases.

> It looks correct via the spec, thank you for finding this oversight. It
> looks like everywhere this bit would make a difference we already do the
> correct thing for having this bit set.
>
> From what I can gather from the spec, it looks as if even 32bit guests
> must ensure that the upper 32bit registers are cleared, so I think we're
> all set here.
>
> Reviewed-by: John Snow 

Re: [Qemu-devel] [PATCH] hw/i386: check if nvdimm is enabled before plugging

[Eduardo Habkost]

On Fri, Jan 13, 2017 at 01:17:27PM +, Stefan Hajnoczi wrote:
> On Fri, Jan 13, 2017 at 07:56:51PM +0800, Haozhong Zhang wrote:
> > The missing of 'nvdimm' in the machine type option '-M' means NVDIMM
> > is disabled. QEMU should refuse to plug any NVDIMM device in this case
> > and report the misconfiguration.
> > 
> > Reported-by: Stefan Hajnoczi 
> > Signed-off-by: Haozhong Zhang 
> > Message-Id: 20170112110928.GF4621@stefanha-x1.localdomain
> > Message-Id: 20170111093630.2088-1-stefa...@redhat.com
> > ---
> >  hw/i386/pc.c | 5 +
> >  1 file changed, 5 insertions(+)
> > 
> > diff --git a/hw/i386/pc.c b/hw/i386/pc.c
> > index 25e8586..3907609 100644
> > --- a/hw/i386/pc.c
> > +++ b/hw/i386/pc.c
> > @@ -1715,6 +1715,11 @@ static void pc_dimm_plug(HotplugHandler *hotplug_dev,
> >  }
> >  
> >  if (object_dynamic_cast(OBJECT(dev), TYPE_NVDIMM)) {
> > +if (!pcms->acpi_nvdimm_state.is_enabled) {
> > +error_setg(_err,
> > +   "nvdimm is not enabled: missing 'nvdimm' in '-M'");
> > +goto out;
> > +}
> 
> A warning is definitely useful to notify users of a possible
> configuration error.
> 
> I wonder what happens when you plug an NVDIMM into a motherboard where
> the firmware lacks support.  Does it:
>  * Refuse to boot?
>  * Treat the DIMM as regular RAM?
>  * Boot but the DIMM will not be used by firmware and kernel?
> 
> QEMU should act the same way as real hardware.

If real hardware behavior is not useful in any way (e.g. first
and third options above), is there a good reason for QEMU to not
implement an additional safety mechanism preventing NVDIMM from
being connected to a machine that doesn't support it?

-- 
Eduardo

Re: [Qemu-devel] [PATCH v2 5/5] target-m68k: increment/decrement with SP

[Richard Henderson]

On 01/13/2017 04:52 AM, Laurent Vivier wrote:
> +if (reg0 == 7 && opsize == OS_BYTE &&
> +m68k_feature(s->env, M68K_FEATURE_M68000)) {
> +tcg_gen_subi_i32(tmp, reg, 2);
> +} else {
> +tcg_gen_addi_i32(tmp, reg, opsize_bytes(opsize));
> +}

post-increment add.


r~

Re: [Qemu-devel] [PATCH v2 4/5] target-m68k: CAS doesn't need aligned access

[Richard Henderson]

On 01/13/2017 04:52 AM, Laurent Vivier wrote:
> Signed-off-by: Laurent Vivier 
> ---
>  target/m68k/translate.c | 1 -
>  1 file changed, 1 deletion(-)

Huh.  Wow.  Ok.

Reviewed-by: Richard Henderson 


r~

Re: [Qemu-devel] [PATCH v2 3/5] target-m68k: manage pre-dec et post-inc in CAS

[Richard Henderson]

On 01/13/2017 04:52 AM, Laurent Vivier wrote:
> In these cases we must update the address register after
> the operation.
> 
> Signed-off-by: Laurent Vivier 
> ---
>  target/m68k/translate.c | 9 +
>  1 file changed, 9 insertions(+)

Reviewed-by: Richard Henderson 


r~

Re: [Qemu-devel] [PATCH v2 2/5] target-m68k: fix gen_flush_flags()

[Richard Henderson]

On 01/13/2017 04:52 AM, Laurent Vivier wrote:
> gen_flush_flags() is setting unconditionally cc_op_synced to 1
> and s->cc_op to CC_OP_FLAGS, whereas env->cc_op can be set
> to something else by a previous tcg fragment.
> 
> We fix that by not setting cc_op_synced to 1
> (except for gen_helper_flush_flags() that updates env->cc_op)
> 
> FIX: https://github.com/vivier/qemu-m68k/issues/19
> 
> Signed-off-by: Laurent Vivier 
> ---
>  target/m68k/translate.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)

Reviewed-by: Richard Henderson 


r~

Re: [Qemu-devel] [PATCH v2 1/5] target-m68k: fix bit operation with immediate value

[Richard Henderson]

On 01/13/2017 04:51 AM, Laurent Vivier wrote:
> M680x0 bit operations with an immediate value use 9 bits of the 16bit
> value, while coldfire ones use only 8 bits.

I don't see that in the reference manual.  Where do you get this from?


r~

Re: [Qemu-devel] bug in new ubfx code

[Richard Henderson]

On 01/13/2017 09:27 AM, Ard Biesheuvel wrote:
> Hi all,
> 
> I tracked down a boot issue I was having with running the kernel under
> AArch64 system emulation to commit
> 
> It appears that ubfx is executing incorrectly: the following code
> 
> .global _start
> _start:
> mov x1, #0x1124
> ubfx x2, x1, #28, #4
> 
> built with
> 
> aarch64-linux-gnu-gcc -o /tmp/ubfx /tmp/ubfx.s -nostartfiles
> aarch64-linux-gnu-objcopy -O binary /tmp/ubfx.bin /tmp/ubfx
> 
> and executed with
> 
> qemu-system-aarch64 -M virt -cpu cortex-a53 -kernel /tmp/ubfx -s -S -nographic
> 
> produces the value 0x11240 in x2 (as can be observed via GDB),
> while the expected value is 0

Ho hum.  Somehow the relevant patch was corrupted.  It has half of
Alex's fix in

  http://lists.nongnu.org/archive/html/qemu-devel/2016-12/msg00137.html

but is also missing half.  I'll fix it up asap.


r~

[Qemu-devel] [PATCH v2] hw/arm/virt-acpi - reserve ECAM space as PNP0C02 device

[Ard Biesheuvel]

Linux for arm64 v4.10 and later will complain if the ECAM config space is
not reserved in the ACPI namespace:

  acpi PNP0A08:00: [Firmware Bug]: ECAM area [mem 0x3f00-0x3fff] not 
reserved in ACPI namespace

The rationale is that OSes that don't consume the MCFG table should still
be able to infer that the PCI config space MMIO region is occupied.

So update the ACPI table generation routine to add this reservation.

Signed-off-by: Ard Biesheuvel 
---
 hw/arm/virt-acpi-build.c | 7 +++
 1 file changed, 7 insertions(+)

diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
index 085a61117378..50d52f685f68 100644
--- a/hw/arm/virt-acpi-build.c
+++ b/hw/arm/virt-acpi-build.c
@@ -310,6 +310,13 @@ static void acpi_dsdt_add_pci(Aml *scope, const 
MemMapEntry *memmap,
 Aml *dev_rp0 = aml_device("%s", "RP0");
 aml_append(dev_rp0, aml_name_decl("_ADR", aml_int(0)));
 aml_append(dev, dev_rp0);
+
+Aml *dev_res0 = aml_device("%s", "RES0");
+aml_append(dev_res0, aml_name_decl("_HID", aml_string("PNP0C02")));
+crs = aml_resource_template();
+aml_append(crs, aml_memory32_fixed(base_ecam, size_ecam, AML_READ_WRITE));
+aml_append(dev_res0, aml_name_decl("_CRS", crs));
+aml_append(dev, dev_res0);
 aml_append(scope, dev);
 }
 
-- 
2.7.4

Re: [Qemu-devel] [PATCH] libvhost-user: Start VQs on SET_VRING_CALL

[Felipe Franciosi]

> On 13 Jan 2017, at 09:04, Michael S. Tsirkin  wrote:
> 
> On Fri, Jan 13, 2017 at 03:09:46PM +, Felipe Franciosi wrote:
>> Hi Marc-Andre,
>> 
>>> On 13 Jan 2017, at 07:03, Marc-André Lureau  wrote:
>>> 
>>> Hi
>>> 
>>> - Original Message -
 Currently, VQs are started as soon as a SET_VRING_KICK is received. That
 is too early in the VQ setup process, as the backend might not yet have
>>> 
>>> I think we may want to reconsider queue_set_started(), move it elsewhere, 
>>> since kick/call fds aren't mandatory to process the rings.
>> 
>> Hmm. The fds aren't mandatory, but I imagine in that case we should still 
>> receive SET_VRING_KICK/CALL messages without an fd (ie. with the 
>> VHOST_MSG_VQ_NOFD_MASK flag set). Wouldn't that be the case?
> 
> Please look at docs/specs/vhost-user.txt, Starting and stopping rings
> 
> The spec says:
>   Client must start ring upon receiving a kick (that is, detecting that
>   file descriptor is readable) on the descriptor specified by
>   VHOST_USER_SET_VRING_KICK, and stop ring upon receiving
>   VHOST_USER_GET_VRING_BASE.

Yes I have seen the spec, but there is a race with the current libvhost-user 
code which needs attention. My initial proposal (which got turned down) was to 
send a spurious notification upon seeing a callfd. Then I came up with this 
proposal. See below.

> 
> 
>>> 
 a callfd to notify in case it received a kick and fully processed the
 request/command. This patch only starts a VQ when a SET_VRING_CALL is
 received.
>>> 
>>> I don't like that much, as soon as the kick fd is received, it should start 
>>> polling it imho. callfd is optional, it may have one and not the other.
>> 
>> So the question is whether we should be receiving a SET_VRING_CALL anyway or 
>> not, regardless of an fd being sent. (I think we do, but I haven't done 
>> extensive testing with other device types.)
> 
> I would say not, only KICK is mandatory and that is also not enough
> to process ring. You must wait for it to be readable.

The problem is that Qemu takes time between sending the kickfd and the callfd. 
Hence the race. Consider this scenario:

1) Guest configures the device
2) Guest put a request on a virtq
3) Guest kicks
4) Qemu starts configuring the backend
4.a) Qemu sends the masked callfds
4.b) Qemu sends the virtq sizes and addresses
4.c) Qemu sends the kickfds

(When using MQ, Qemu will only send the callfd once all VQs are configured)

5) The backend starts listening on the kickfd upon receiving it
6) The backend picks up the guest's request
7) The backend processes the request
8) The backend puts the response on the used ring
9) The backend notifies the masked callfd

4.d) Qemu sends the callfds

At which point the guest missed the notification and gets stuck.

Perhaps you prefer my initial proposal of sending a spurious notification when 
the backend sees a callfd?

Felipe


> 
>>> 
>>> Perhaps it's best for now to delay the callfd notification with a flag 
>>> until it is received?
>> 
>> The other idea is to always kick when we receive the callfd. I remember 
>> discussing that alternative with you before libvhost-user went in. The 
>> protocol says both the driver and the backend must handle spurious kicks. 
>> This approach also fixes the bug.
>> 
>> I'm happy with whatever alternative you want, as long it makes libvhost-user 
>> usable for storage devices.
>> 
>> Thanks,
>> Felipe
>> 
>> 
>>> 
>>> 
 Signed-off-by: Felipe Franciosi 
 ---
 contrib/libvhost-user/libvhost-user.c | 26 +-
 1 file changed, 13 insertions(+), 13 deletions(-)
 
 diff --git a/contrib/libvhost-user/libvhost-user.c
 b/contrib/libvhost-user/libvhost-user.c
 index af4faad..a46ef90 100644
 --- a/contrib/libvhost-user/libvhost-user.c
 +++ b/contrib/libvhost-user/libvhost-user.c
 @@ -607,19 +607,6 @@ vu_set_vring_kick_exec(VuDev *dev, VhostUserMsg *vmsg)
DPRINT("Got kick_fd: %d for vq: %d\n", vmsg->fds[0], index);
}
 
 -dev->vq[index].started = true;
 -if (dev->iface->queue_set_started) {
 -dev->iface->queue_set_started(dev, index, true);
 -}
 -
 -if (dev->vq[index].kick_fd != -1 && dev->vq[index].handler) {
 -dev->set_watch(dev, dev->vq[index].kick_fd, VU_WATCH_IN,
 -   vu_kick_cb, (void *)(long)index);
 -
 -DPRINT("Waiting for kicks on fd: %d for vq: %d\n",
 -   dev->vq[index].kick_fd, index);
 -}
 -
return false;
 }
 
 @@ -661,6 +648,19 @@ vu_set_vring_call_exec(VuDev *dev, VhostUserMsg *vmsg)
 
DPRINT("Got call_fd: %d for vq: %d\n", vmsg->fds[0], index);
 
 +dev->vq[index].started = true;
 +if (dev->iface->queue_set_started) {
 +dev->iface->queue_set_started(dev, index, true);
 +}
 +
 

Re: [Qemu-devel] bug in new ubfx code

[Ard Biesheuvel]

On 13 January 2017 at 17:27, Ard Biesheuvel  wrote:
> Hi all,
>
> I tracked down a boot issue I was having with running the kernel under
> AArch64 system emulation to commit
>

59a71b4c5b4e target-arm: Use new deposit and extract ops


> It appears that ubfx is executing incorrectly: the following code
>
> .global _start
> _start:
> mov x1, #0x1124
> ubfx x2, x1, #28, #4
>
> built with
>
> aarch64-linux-gnu-gcc -o /tmp/ubfx /tmp/ubfx.s -nostartfiles
> aarch64-linux-gnu-objcopy -O binary /tmp/ubfx.bin /tmp/ubfx
>
> and executed with
>
> qemu-system-aarch64 -M virt -cpu cortex-a53 -kernel /tmp/ubfx -s -S -nographic
>
> produces the value 0x11240 in x2 (as can be observed via GDB),
> while the expected value is 0
>
> Regards,
> Ard.

[Qemu-devel] bug in new ubfx code

[Ard Biesheuvel]

Hi all,

I tracked down a boot issue I was having with running the kernel under
AArch64 system emulation to commit

It appears that ubfx is executing incorrectly: the following code

.global _start
_start:
mov x1, #0x1124
ubfx x2, x1, #28, #4

built with

aarch64-linux-gnu-gcc -o /tmp/ubfx /tmp/ubfx.s -nostartfiles
aarch64-linux-gnu-objcopy -O binary /tmp/ubfx.bin /tmp/ubfx

and executed with

qemu-system-aarch64 -M virt -cpu cortex-a53 -kernel /tmp/ubfx -s -S -nographic

produces the value 0x11240 in x2 (as can be observed via GDB),
while the expected value is 0

Regards,
Ard.

Re: [Qemu-devel] [PATCH] qemu-capabilities: Fix query-cpu-model-expansion on s390 with older kernel

[Eduardo Habkost]

On Fri, Jan 13, 2017 at 09:06:52AM -0500, Jason J. Herne wrote:
> On 01/13/2017 05:04 AM, Jiri Denemark wrote:
> > On Thu, Jan 12, 2017 at 11:18:11 -0500, Collin L. Walling wrote:
> > > When running on s390 with a kernel that does not support cpu model 
> > > checking and
> > > with a Qemu new enough to support query-cpu-model-expansion, the 
> > > gathering of qemu
> > > capabilities will fail. Qemu responds to the query-cpu-model-expansion qmp
> > > command with an error because the needed kernel ioct does not exist. When 
> > > this
> > > happens a guest cannot even be defined due to missing qemu capabilities 
> > > data.
> > > 
> > > This patch fixes the problem by silently ignoring generic errors stemming 
> > > from
> > > calls to query-cpu-model-expansion.
> > > 
> > > Reported-by: Farhan Ali 
> > > Signed-off-by: Collin L. Walling 
> > > Signed-off-by: Jason J. Herne 
> > > ---
> > >  src/qemu/qemu_monitor_json.c | 9 +
> > >  1 file changed, 9 insertions(+)
> > > 
> > > diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
> > > index e767437..1662749 100644
> > > --- a/src/qemu/qemu_monitor_json.c
> > > +++ b/src/qemu/qemu_monitor_json.c
> > > @@ -5041,6 +5041,15 @@ qemuMonitorJSONGetCPUModelExpansion(qemuMonitorPtr 
> > > mon,
> > >  if (qemuMonitorJSONCommand(mon, cmd, ) < 0)
> > >  goto cleanup;
> > > 
> > > +/* Some QEMU architectures have the query-cpu-model-expansion
> > > + * command, but return 'GenericError' instead of simply omitting
> > > + * the command entirely.
> > > + */
> > 
> > Hmm, this comment says something a bit different than the commit
> > message. I hope the issue described by this comment was fixed in QEMU
> > within the same release the query-cpu-model-expansion command was
> > introduced. But we need to fix this nevertheless to address what the
> > commit message describes.
> > 
> 
> The issue still exists in Qemu. I can work up a patch to handle it. My first
> idea is to simply test that the ioctl exists and functions at Qemu
> initialization and deregister the query-cpu-model-expansion command in the
> event that the ioctl does not exist or fails. Thoughts?

I'm not sure, probably unregistering the command after KVM is
initialized is too late. Even if today QMP is available only
after the accelerator is already initialized, we might want to
delay accelerator initialization in the future (so the
accelerator could be configured using QMP commands).

Also, I'm not sure when/how exactly the query-cpu-model-expansion
call fails. If the ioctl isn't available, shouldn't
query-cpu-model-expansion fail only when querying "host", but let
the other CPU models to be queried? In this case, unregistering
the command doesn't seem to be the right solution. Probably
omitting "host" (or flagging it as unavailable?) on
query-cpu-model-definitions would be better.

-- 
Eduardo

Re: [Qemu-devel] [PATCH] ahci: advertise HOST_CAP_64

[John Snow]

On 01/13/2017 06:02 AM, Ladi Prosek wrote:
> The AHCI emulation code supports 64-bit addressing and should advertise this
> fact in the Host Capabilities register. Both Linux and Windows drivers test
> this bit to decide if the upper 32 bits of various registers may be written
> to, and at least some versions of Windows have a bug where DMA is attempted
> with an address above 4GB but, in the absence of HOST_CAP_64, the upper 32
> bits are left unititialized which leads to a memory corruption.
> 
> Signed-off-by: Ladi Prosek 
> ---
>  hw/ide/ahci.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/hw/ide/ahci.c b/hw/ide/ahci.c
> index 3c19bda..6a17acf 100644
> --- a/hw/ide/ahci.c
> +++ b/hw/ide/ahci.c
> @@ -488,7 +488,7 @@ static void ahci_reg_init(AHCIState *s)
>  s->control_regs.cap = (s->ports - 1) |
>(AHCI_NUM_COMMAND_SLOTS << 8) |
>(AHCI_SUPPORTED_SPEED_GEN1 << 
> AHCI_SUPPORTED_SPEED) |
> -  HOST_CAP_NCQ | HOST_CAP_AHCI;
> +  HOST_CAP_NCQ | HOST_CAP_AHCI | HOST_CAP_64;
>  
>  s->control_regs.impl = (1 << s->ports) - 1;
>  
> 

Was this tested? What was the use case that prompted this patch, and do
you have a public bugzilla to point to?

It looks correct via the spec, thank you for finding this oversight. It
looks like everywhere this bit would make a difference we already do the
correct thing for having this bit set.

>From what I can gather from the spec, it looks as if even 32bit guests
must ensure that the upper 32bit registers are cleared, so I think we're
all set here.

Reviewed-by: John Snow 

[Qemu-devel] [PATCH] 9pfs: fix off-by-one error in PDU free list

[Greg Kurz]

The server can handle MAX_REQ - 1 PDUs at a time and the virtio-9p
device has a MAX_REQ sized virtqueue. If the client manages to fill
up the virtqueue, pdu_alloc() will fail and the request won't be
processed without any notice to the client (it actually causes the
linux 9p client to hang).

This has been there since the beginning (commit 9f10751365b2 "virtio-9p:
Add a virtio 9p device to qemu"), but it needs an agressive workload to
run in the guest to show up.

We actually allocate MAX_REQ PDUs and I see no reason not to link them
all into the free list, so let's fix the init loop.

Reported-by: Tuomas Tynkkynen 
Suggested-by: Al Viro 
Signed-off-by: Greg Kurz 
---
 hw/9pfs/9p.c |2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/9pfs/9p.c b/hw/9pfs/9p.c
index fa58877570f6..965c8edc8030 100644
--- a/hw/9pfs/9p.c
+++ b/hw/9pfs/9p.c
@@ -3454,7 +3454,7 @@ int v9fs_device_realize_common(V9fsState *s, Error **errp)
 /* initialize pdu allocator */
 QLIST_INIT(>free_list);
 QLIST_INIT(>active_list);
-for (i = 0; i < (MAX_REQ - 1); i++) {
+for (i = 0; i < MAX_REQ; i++) {
 QLIST_INSERT_HEAD(>free_list, >pdus[i], next);
 s->pdus[i].s = s;
 s->pdus[i].idx = i;

Re: [Qemu-devel] [PULL 0/6] x86 and machine queue, 2017-01-17

[Peter Maydell]

On 12 January 2017 at 17:53, Eduardo Habkost  wrote:
> The following changes since commit 204febd17f9ebb9e94b1980b42c7f2c2307851c1:
>
>   libqtest: handle zero length memwrite/memread (2017-01-12 10:45:59 +)
>
> are available in the git repository at:
>
>   git://github.com/ehabkost/qemu.git tags/x86-and-machine-pull-request
>
> for you to fetch changes up to 8ed877b78498c89aa7ce5c76aa20841ff5072796:
>
>   qmp: Report QOM type name on query-cpu-definitions (2017-01-12 15:51:36 
> -0200)
>
> 
> x86 and machine queue, 2017-01-17
>
> Includes i386, CPU, NUMA, and memory backends changes.
>
> i386:
>  target/i386: Fix bad patch application to translate.c
>
> CPU:
>  qmp: Report QOM type name on query-cpu-definitions
>
> NUMA:
>  numa: make -numa parser dynamically allocate CPUs masks
>
> Memory backends:
>  qom: remove unused header
>  monitor: reuse user_creatable_add_opts() instead of user_creatable_add()
>  monitor: fix qmp/hmp query-memdev not reporting IDs of memory backends
>
> 

Applied, thanks.

-- PMM

Re: [Qemu-devel] [PATCH] libvhost-user: Start VQs on SET_VRING_CALL

[Michael S. Tsirkin]

On Fri, Jan 13, 2017 at 03:09:46PM +, Felipe Franciosi wrote:
> Hi Marc-Andre,
> 
> > On 13 Jan 2017, at 07:03, Marc-André Lureau  wrote:
> > 
> > Hi
> > 
> > - Original Message -
> >> Currently, VQs are started as soon as a SET_VRING_KICK is received. That
> >> is too early in the VQ setup process, as the backend might not yet have
> > 
> > I think we may want to reconsider queue_set_started(), move it elsewhere, 
> > since kick/call fds aren't mandatory to process the rings.
> 
> Hmm. The fds aren't mandatory, but I imagine in that case we should still 
> receive SET_VRING_KICK/CALL messages without an fd (ie. with the 
> VHOST_MSG_VQ_NOFD_MASK flag set). Wouldn't that be the case?

Please look at docs/specs/vhost-user.txt, Starting and stopping rings

The spec says:
Client must start ring upon receiving a kick (that is, detecting that
file descriptor is readable) on the descriptor specified by
VHOST_USER_SET_VRING_KICK, and stop ring upon receiving
VHOST_USER_GET_VRING_BASE.


> > 
> >> a callfd to notify in case it received a kick and fully processed the
> >> request/command. This patch only starts a VQ when a SET_VRING_CALL is
> >> received.
> > 
> > I don't like that much, as soon as the kick fd is received, it should start 
> > polling it imho. callfd is optional, it may have one and not the other.
> 
> So the question is whether we should be receiving a SET_VRING_CALL anyway or 
> not, regardless of an fd being sent. (I think we do, but I haven't done 
> extensive testing with other device types.)

I would say not, only KICK is mandatory and that is also not enough
to process ring. You must wait for it to be readable.

> > 
> > Perhaps it's best for now to delay the callfd notification with a flag 
> > until it is received?
> 
> The other idea is to always kick when we receive the callfd. I remember 
> discussing that alternative with you before libvhost-user went in. The 
> protocol says both the driver and the backend must handle spurious kicks. 
> This approach also fixes the bug.
> 
> I'm happy with whatever alternative you want, as long it makes libvhost-user 
> usable for storage devices.
> 
> Thanks,
> Felipe
> 
> 
> > 
> > 
> >> Signed-off-by: Felipe Franciosi 
> >> ---
> >> contrib/libvhost-user/libvhost-user.c | 26 +-
> >> 1 file changed, 13 insertions(+), 13 deletions(-)
> >> 
> >> diff --git a/contrib/libvhost-user/libvhost-user.c
> >> b/contrib/libvhost-user/libvhost-user.c
> >> index af4faad..a46ef90 100644
> >> --- a/contrib/libvhost-user/libvhost-user.c
> >> +++ b/contrib/libvhost-user/libvhost-user.c
> >> @@ -607,19 +607,6 @@ vu_set_vring_kick_exec(VuDev *dev, VhostUserMsg *vmsg)
> >> DPRINT("Got kick_fd: %d for vq: %d\n", vmsg->fds[0], index);
> >> }
> >> 
> >> -dev->vq[index].started = true;
> >> -if (dev->iface->queue_set_started) {
> >> -dev->iface->queue_set_started(dev, index, true);
> >> -}
> >> -
> >> -if (dev->vq[index].kick_fd != -1 && dev->vq[index].handler) {
> >> -dev->set_watch(dev, dev->vq[index].kick_fd, VU_WATCH_IN,
> >> -   vu_kick_cb, (void *)(long)index);
> >> -
> >> -DPRINT("Waiting for kicks on fd: %d for vq: %d\n",
> >> -   dev->vq[index].kick_fd, index);
> >> -}
> >> -
> >> return false;
> >> }
> >> 
> >> @@ -661,6 +648,19 @@ vu_set_vring_call_exec(VuDev *dev, VhostUserMsg *vmsg)
> >> 
> >> DPRINT("Got call_fd: %d for vq: %d\n", vmsg->fds[0], index);
> >> 
> >> +dev->vq[index].started = true;
> >> +if (dev->iface->queue_set_started) {
> >> +dev->iface->queue_set_started(dev, index, true);
> >> +}
> >> +
> >> +if (dev->vq[index].kick_fd != -1 && dev->vq[index].handler) {
> >> +dev->set_watch(dev, dev->vq[index].kick_fd, VU_WATCH_IN,
> >> +   vu_kick_cb, (void *)(long)index);
> >> +
> >> +DPRINT("Waiting for kicks on fd: %d for vq: %d\n",
> >> +   dev->vq[index].kick_fd, index);
> >> +}
> >> +
> >> return false;
> >> }
> >> 
> >> --
> >> 1.9.4
> >> 
> >> 
> 

Re: [Qemu-devel] [PATCH] hw/arm/virt-acpi - reserve ECAM space as PNP0C02 device

[Ard Biesheuvel]

On 13 January 2017 at 16:36, Ard Biesheuvel  wrote:
> Linux for arm64 v4.10 and later will complain if the ECAM config space is
> not reserved in the ACPI namespace:
>
>   acpi PNP0A08:00: [Firmware Bug]: ECAM area [mem 0x3f00-0x3fff] not 
> reserved in ACPI namespace
>
> The rationale is that OSes that don't consume the MCFG table should still
> be able to infer that the PCI config space MMIO region is occupied.
>
> So update the ACPI table generation routine to add this reservation.
>
> Signed-off-by: Ard Biesheuvel 

Please disregard -- I failed to add the PNP0C02 HID

> ---
>  hw/arm/virt-acpi-build.c | 6 ++
>  1 file changed, 6 insertions(+)
>
> diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
> index 085a61117378..e8769dc6288f 100644
> --- a/hw/arm/virt-acpi-build.c
> +++ b/hw/arm/virt-acpi-build.c
> @@ -310,6 +310,12 @@ static void acpi_dsdt_add_pci(Aml *scope, const 
> MemMapEntry *memmap,
>  Aml *dev_rp0 = aml_device("%s", "RP0");
>  aml_append(dev_rp0, aml_name_decl("_ADR", aml_int(0)));
>  aml_append(dev, dev_rp0);
> +
> +Aml *dev_res0 = aml_device("%s", "RES0");
> +crs = aml_resource_template();
> +aml_append(crs, aml_memory32_fixed(base_ecam, size_ecam, 
> AML_READ_WRITE));
> +aml_append(dev_res0, aml_name_decl("_CRS", crs));
> +aml_append(dev, dev_res0);
>  aml_append(scope, dev);
>  }
>
> --
> 2.7.4
>

Re: [Qemu-devel] [PATCH v8 12/21] qapi.py: fix line break before binary operator pep8

[Markus Armbruster]

Marc-André Lureau  writes:

> Hi
>
> - Original Message -
>> Marc-André Lureau  writes:
>> 
>> > Python code style accepts both form, but pep8 complains. Better to clean
>> > up the single warning for now, so new errors stand out more easily.
>> >
>> > Fix scripts/qapi.py:1539:21: W503 line break before binary operator
>> >
>> > Signed-off-by: Marc-André Lureau 
>> 
>> I'm dropping this patch, as per review of v7.
>
> Sorry, I thought I dropped it. I think I screwed up with my two branches 
> (maintaining both split/squash series was quite a pain, I am relieved we are 
> nearing the end)

Don't worry, you did all the non-trivial changes correctly, so I'm happy
to do a few trivial ones :)

[Qemu-devel] [PULL 000/180] QAPI patches for 2017-01-13

[Markus Armbruster]

This is Marc-André's "[PATCH v8 00/21] qapi doc generation (whole
version, squashed)" with a few commit messages tweaked, and "[PATCH v8
14/21] (SQUASHED) move doc to schema" unsquashed into 161 patches.

We did all the respins with in this squashed form to reduce noise.
However, since the unsquashed form is better suited for review, and
probably nicer if we have to revisit this part of the work down the
road, I'm proposing to merge this unsquashed.

If you want me to post the unsquashed patches, I'm happy to redo this
pull request.

If you'd rather pull the squashed version, likewise.

I'm afraid this is a bit of a doc conflict magnet.  The sooner we can
get it in, the easier for Marc-André and me.

The following changes since commit fdbd92f738693abfda60d2d1fc075cd796f33f80:

  Merge remote-tracking branch 'remotes/stsquad/tags/pull-travis-20170112-1' 
into staging (2017-01-13 14:02:30 +)

are available in the git repository at:

  git://repo.or.cz/qemu/armbru.git tags/pull-qapi-2017-01-13

for you to fetch changes up to 3b4ce60a8bb5f48d87c925a4a783d005914605d1:

  build-sys: add qapi doc generation targets (2017-01-13 17:32:16 +0100)


QAPI patches for 2017-01-13


Marc-André Lureau (180):
  qapi: replace 'o' for list items
  qapi: move QKeyCode doc body at the top
  qapi: Format TODO comments more consistently
  qapi: improve device_add schema
  qapi: improve TransactionAction doc
  qga/schema: improve guest-set-vcpus Returns: section
  qapi: Reorder doc comments for future doc generator
  qapi: Move "command is experimental" notes down
  qapi: add some sections in docs
  docs: add master qapi texi files
  qapi: rework qapi Exception
  texi2pod: learn quotation, deftp and deftypefn
  qmp-commands: move 'add_client' doc to schema
  qmp-commands: move 'query-name' doc to schema
  qmp-commands: move 'query-kvm' doc to schema
  qmp-commands: move 'query-status' doc to schema
  qmp-commands: move 'query-uuid' doc to schema
  qmp-commands: move 'query-chardev' doc to schema
  qmp-commands: move 'query-chardev-backends' doc to schema
  qmp-commands: move 'ringbuf-write' doc to schema
  qmp-commands: move 'ringbuf-read' doc to schema
  qmp-commands: move 'query-events' doc to schema
  qmp-commands: move 'query-migrate' doc to schema
  qmp-commands: move 'migrate-set-capabilities' doc to schema
  qmp-commands: move 'query-migrate-capabilities' doc to schema
  qmp-commands: move 'migrate-set-parameters' doc to schema
  qmp-commands: move 'query-migrate-parameters' doc to schema
  qmp-commands: move 'client_migrate_info' doc to schema
  qmp-commands: move 'migrate-start-postcopy' doc to schema
  qmp-commands: move 'query-mice' doc to schema
  qmp-commands: move 'query-cpus' doc to schema
  qmp-commands: move 'query-iothreads' doc to schema
  qmp-commands: move 'query-vnc' doc to schema
  qmp-commands: move 'query-spice' doc to schema
  qmp-commands: move 'query-balloon' doc to schema
  qmp-commands: move 'query-pci' doc to schema
  qmp-commands: move 'quit' doc to schema
  qmp-commands: move 'stop' doc to schema
  qmp-commands: move 'system_reset' doc to schema
  qmp-commands: move 'system_powerdown' doc to schema
  qmp-commands: move 'cpu-add' doc to schema
  qmp-commands: move 'memsave' doc to schema
  qmp-commands: move 'pmemsave' doc to schema
  qmp-commands: move 'cont' doc to schema
  qmp-commands: move 'system_wakeup' doc to schema
  qmp-commands: move 'inject-nmi' doc to schema
  qmp-commands: move 'set_link' doc to schema
  qmp-commands: move 'balloon' doc to schema
  qmp-commands: move 'transaction' doc to schema
  qmp-commands: move 'human-monitor-command' doc to schema
  qmp-commands: move 'migrate_cancel' doc to schema
  qmp-commands: move 'migrate_set_downtime' doc to schema
  qmp-commands: move 'migrate_set_speed' doc to schema
  qmp-commands: move 'query-migrate-cache-size' doc to schema
  qmp-commands: move 'set_password' doc to schema
  qmp-commands: move 'expire_password' doc to schema
  qmp-commands: move 'change' doc to schema
  qmp-commands: move 'migrate' doc to schema
  qmp-commands: move 'migrate-incoming' doc to schema
  qmp-commands: move 'xen-save-devices-state' doc to schema
  qmp-commands: move 'xen-set-global-dirty-log' doc to schema
  qmp-commands: move 'device_del' doc to schema
  qmp-commands: move 'dump-guest-memory' doc to schema
  qmp-commands: move 'query-dump-guest-memory-capability' doc to schema
  qmp-commands: move 'dump-skeys' doc to schema
  qmp-commands: move 'netdev_add' doc to schema
  qmp-commands: move 'netdev_del' doc to schema
  qmp-commands: move 'object-add' doc to 

Re: [Qemu-devel] [PATCH 27/40] char: move QIOChannel-related in char-io.h

[Marc-André Lureau]

Hi

- Original Message -
> On 01/11/2017 11:29 AM, Marc-André Lureau wrote:
> 
> Grammar in subject is a bit terse; maybe:
> 
> char: move QIOChannel-related stuff to char-io.h
> 
> > Signed-off-by: Marc-André Lureau 
> > ---
> >  chardev/char-io.h |  24 +++
> >  chardev/char-io.c | 168
> >  
> >  chardev/char.c| 174
> >  +-
> >  chardev/Makefile.objs |   1 +
> >  4 files changed, 194 insertions(+), 173 deletions(-)
> >  create mode 100644 chardev/char-io.h
> >  create mode 100644 chardev/char-io.c
> > 
> > diff --git a/chardev/char-io.h b/chardev/char-io.h
> > new file mode 100644
> > index 00..ea559fd124
> > --- /dev/null
> > +++ b/chardev/char-io.h
> > @@ -0,0 +1,24 @@
> > +#ifndef CHAR_IO_H
> > +#define CHAR_IO_H
> 
> Must... resist... the temptation to repeat myself!

Ok, I'll just copy the original copyright from qemu-char.c in all the files I 
created in the series.

> 
> > +
> > +#include "qemu/osdep.h"
> 
> .h files should NOT include osdep.h; since all .c files included it
> before any other .h file, then all things in osdep.h are already in
> scope at the point the .h file starts.

ok

> 
> 
> > +static gboolean io_watch_poll_prepare(GSource *source,
> > +  gint *timeout_)
> > +{
> 
> Why the weird spelling of timeout_ ?  Maybe timeout_unused is better?
> 
> 
> > +++ b/chardev/char.c
> 
> > -static gboolean io_watch_poll_prepare(GSource *source,
> > -  gint *timeout_)
> 
> Then again, it's code motion.  Again, up to you if you want to tweak
> style things during code motion, or split them into a general cleanup
> patch separately.

yeah, weird style, any idea why Anthony wrote it that way in commit 96c638477?

I guess I may just drop the _ during the move

[Qemu-devel] [PATCH] hw/arm/virt-acpi - reserve ECAM space as PNP0C02 device

[Ard Biesheuvel]

Linux for arm64 v4.10 and later will complain if the ECAM config space is
not reserved in the ACPI namespace:

  acpi PNP0A08:00: [Firmware Bug]: ECAM area [mem 0x3f00-0x3fff] not 
reserved in ACPI namespace

The rationale is that OSes that don't consume the MCFG table should still
be able to infer that the PCI config space MMIO region is occupied.

So update the ACPI table generation routine to add this reservation.

Signed-off-by: Ard Biesheuvel 
---
 hw/arm/virt-acpi-build.c | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
index 085a61117378..e8769dc6288f 100644
--- a/hw/arm/virt-acpi-build.c
+++ b/hw/arm/virt-acpi-build.c
@@ -310,6 +310,12 @@ static void acpi_dsdt_add_pci(Aml *scope, const 
MemMapEntry *memmap,
 Aml *dev_rp0 = aml_device("%s", "RP0");
 aml_append(dev_rp0, aml_name_decl("_ADR", aml_int(0)));
 aml_append(dev, dev_rp0);
+
+Aml *dev_res0 = aml_device("%s", "RES0");
+crs = aml_resource_template();
+aml_append(crs, aml_memory32_fixed(base_ecam, size_ecam, AML_READ_WRITE));
+aml_append(dev_res0, aml_name_decl("_CRS", crs));
+aml_append(dev, dev_res0);
 aml_append(scope, dev);
 }
 
-- 
2.7.4

Re: [Qemu-devel] [PATCH V4 net-next] vhost_net: device IOTLB support

[Michael S. Tsirkin]

On Fri, Jan 13, 2017 at 10:45:09AM +0800, Jason Wang wrote:
> 
> 
> On 2017年01月12日 22:17, Michael S. Tsirkin wrote:
> > On Wed, Jan 11, 2017 at 12:32:12PM +0800, Jason Wang wrote:
> > > This patches implements Device IOTLB support for vhost kernel. This is
> > > done through:
> > > 
> > > 1) switch to use dma helpers when map/unmap vrings from vhost codes
> > > 2) introduce a set of VhostOps to:
> > > - setting up device IOTLB request callback
> > > - processing device IOTLB request
> > > - processing device IOTLB invalidation
> > > 2) kernel support for Device IOTLB API:
> > > 
> > > - allow vhost-net to query the IOMMU IOTLB entry through eventfd
> > > - enable the ability for qemu to update a specified mapping of vhost
> > > - through ioctl.
> > > - enable the ability to invalidate a specified range of iova for the
> > >device IOTLB of vhost through ioctl. In x86/intel_iommu case this is
> > >triggered through iommu memory region notifier from device IOTLB
> > >invalidation descriptor processing routine.
> > > 
> > > With all the above, kernel vhost_net can co-operate with userspace
> > > IOMMU. For vhost-user, the support could be easily done on top by
> > > implementing the VhostOps.
> > > 
> > > Cc: Michael S. Tsirkin
> > > Signed-off-by: Jason Wang
> > Applied, thanks!
> > 
> > > ---
> > > Changes from V4:
> > > - set iotlb callback only when IOMMU_PLATFORM is negotiated (fix
> > >vhost-user qtest failure)
> > In fact this only checks virtio_host_has_feature - which is
> > the right thing to do, we can't trust the guest.
> > 
> > > - whitelist VIRTIO_F_IOMMU_PLATFORM instead of manually add it
> > > - keep cpu_physical_memory_map() in vhost_memory_map()
> > One further enhancement might be to detect that guest disabled
> > iommu (e.g. globally, or using iommu=pt) and disable
> > the iotlb to avoid overhead for guests which use DPDK
> > for assigned devices but not for vhost.
> > 
> > 
> 
> Yes, it's in my todo list.
> 
> Thanks

Something that I just noticed is that when user requests iommu_platform
but vhost can not provide it, this patches will just let vhost continue
without.  I think that's wrong, since iommu_platform is a security
feature, when it's not supported I think we should fail init.

-- 
MST

Re: [Qemu-devel] Proposal PCI/PCIe device placement on PAPR guests

[Greg Kurz]

On Fri, 13 Jan 2017 09:57:36 +1100
David Gibson  wrote:

> On Thu, Jan 12, 2017 at 11:31:35AM +0100, Andrea Bolognani wrote:
> > On Mon, 2017-01-09 at 10:46 +1100, David Gibson wrote:  
> > > > >* To allow for hotplugged devices, libvirt should also add a number
> > > > >  of additional, empty vPHBs (the PAPR spec allows for hotplug of
> > > > >  PHBs, but this is not yet implemented in qemu).  
> > > > 
> > > > "A number" here will have to mean "one", same number of
> > > > empty PCIe Root Ports libvirt will add to a newly-defined
> > > > q35 guest.  
> > > 
> > > Umm.. why?  
> > 
> > Because some applications using libvirt would inevitably
> > start relying on the fact that such spare PHBs are
> > available, locking us into providing at least the same
> > number forever. In other words, increasing the amount at
> > a later time is always possible, but decreasing it isn't.
> > We did the same when we started automatically adding PCIe
> > Root Ports to q35 machines.
> > 
> > The rationale is that having a single spare hotpluggable
> > slot is extremely convenient for basic usage, eg. a simple
> > guest created by someone who's not necessarily very
> > familiar with virtualization; on the other hand, if you
> > are actually deploying in production you ought to conduct
> > proper capacity planning and figure out in advance how
> > many devices you're likely to need to hotplug throughout
> > the guest's life.  
> 
> Hm, ok.  Well I guess the limitation is the same as on x86, so it
> shouldn't surprise people.
> 
> > Of course this all will be moot once we can hotplug PHBs :)  
> 
> Yes.  Unfortunately, nobody's actually working on that at present.
> 

Well, there might be someone now :)

Michael Roth had posted a RFC patchset back in 2015:

https://lists.gnu.org/archive/html/qemu-ppc/2015-04/msg00275.html

I'll start from here.

Cheers.

--
Greg


pgpp3OcuV9NgA.pgp
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH v8 12/21] qapi.py: fix line break before binary operator pep8

[Marc-André Lureau]

Hi

- Original Message -
> Marc-André Lureau  writes:
> 
> > Python code style accepts both form, but pep8 complains. Better to clean
> > up the single warning for now, so new errors stand out more easily.
> >
> > Fix scripts/qapi.py:1539:21: W503 line break before binary operator
> >
> > Signed-off-by: Marc-André Lureau 
> 
> I'm dropping this patch, as per review of v7.

Sorry, I thought I dropped it. I think I screwed up with my two branches 
(maintaining both split/squash series was quite a pain, I am relieved we are 
nearing the end)

[Qemu-devel] [PATCH] update-linux-headers.sh: support __bitwise

[Michael S. Tsirkin]

In 4.10, Linux is switching from __bitwise__ to use __bitwise
exclusively. Update our script accordingly.

Signed-off-by: Michael S. Tsirkin 
---
 scripts/update-linux-headers.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/scripts/update-linux-headers.sh b/scripts/update-linux-headers.sh
index 08c4c4a..72cf1fb 100755
--- a/scripts/update-linux-headers.sh
+++ b/scripts/update-linux-headers.sh
@@ -51,7 +51,7 @@ cp_portable() {
 -e 's/__be\([0-9][0-9]*\)/uint\1_t/g' \
 -e 's/"\(input-event-codes\.h\)"/"standard-headers\/linux\/\1"/' \
 -e 's/]*\)>/"standard-headers\/linux\/\1"/' \
--e 's/__bitwise__//' \
+-e 's/__bitwise//' \
 -e 's/__attribute__((packed))/QEMU_PACKED/' \
 -e 's/__inline__/inline/' \
 -e '/sys\/ioctl.h/d' \
-- 
MST

[Qemu-devel] [PATCH] virtio_crypto: header update

[Michael S. Tsirkin]

Update header from latest linux driver.  Session creation structs gain
padding to make them same size. Formatting cleanups.

Signed-off-by: Michael S. Tsirkin 
---
 include/standard-headers/linux/virtio_crypto.h | 481 +
 1 file changed, 251 insertions(+), 230 deletions(-)

diff --git a/include/standard-headers/linux/virtio_crypto.h 
b/include/standard-headers/linux/virtio_crypto.h
index 82275a8..5ff0b4e 100644
--- a/include/standard-headers/linux/virtio_crypto.h
+++ b/include/standard-headers/linux/virtio_crypto.h
@@ -1,5 +1,5 @@
-#ifndef _LINUX_VIRTIO_CRYPTO_H
-#define _LINUX_VIRTIO_CRYPTO_H
+#ifndef _VIRTIO_CRYPTO_H
+#define _VIRTIO_CRYPTO_H
 /* This header is BSD licensed so anyone can use the definitions to implement
  * compatible drivers/servers.
  *
@@ -14,52 +14,54 @@
  * 3. Neither the name of IBM nor the names of its contributors
  *may be used to endorse or promote products derived from this software
  *without specific prior written permission.
- * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS ``AS 
IS''
- * AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
- * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
- * ARE DISCLAIMED.  IN NO EVENT SHALL IBM OR CONTRIBUTORS BE LIABLE
- * FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL
- * DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS
- * OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION)
- * HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT
- * LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY
- * OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
- * SUCH DAMAGE. */
-
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+ * ``AS IS'' AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+ * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS
+ * FOR A PARTICULAR PURPOSE ARE DISCLAIMED.  IN NO EVENT SHALL IBM OR
+ * CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+ * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+ * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF
+ * USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND
+ * ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY,
+ * OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT
+ * OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
+ * SUCH DAMAGE.
+ */
 #include "standard-headers/linux/types.h"
-#include "standard-headers/linux/virtio_config.h"
 #include "standard-headers/linux/virtio_types.h"
+#include "standard-headers/linux/virtio_ids.h"
+#include "standard-headers/linux/virtio_config.h"
 
 
 #define VIRTIO_CRYPTO_SERVICE_CIPHER 0
-#define VIRTIO_CRYPTO_SERVICE_HASH 1
-#define VIRTIO_CRYPTO_SERVICE_MAC  2
-#define VIRTIO_CRYPTO_SERVICE_AEAD 3
+#define VIRTIO_CRYPTO_SERVICE_HASH   1
+#define VIRTIO_CRYPTO_SERVICE_MAC2
+#define VIRTIO_CRYPTO_SERVICE_AEAD   3
 
 #define VIRTIO_CRYPTO_OPCODE(service, op)   (((service) << 8) | (op))
 
 struct virtio_crypto_ctrl_header {
 #define VIRTIO_CRYPTO_CIPHER_CREATE_SESSION \
-   VIRTIO_CRYPTO_OPCODE(VIRTIO_CRYPTO_SERVICE_CIPHER, 0x02)
+  VIRTIO_CRYPTO_OPCODE(VIRTIO_CRYPTO_SERVICE_CIPHER, 0x02)
 #define VIRTIO_CRYPTO_CIPHER_DESTROY_SESSION \
-   VIRTIO_CRYPTO_OPCODE(VIRTIO_CRYPTO_SERVICE_CIPHER, 0x03)
+  VIRTIO_CRYPTO_OPCODE(VIRTIO_CRYPTO_SERVICE_CIPHER, 0x03)
 #define VIRTIO_CRYPTO_HASH_CREATE_SESSION \
-   VIRTIO_CRYPTO_OPCODE(VIRTIO_CRYPTO_SERVICE_HASH, 0x02)
+  VIRTIO_CRYPTO_OPCODE(VIRTIO_CRYPTO_SERVICE_HASH, 0x02)
 #define VIRTIO_CRYPTO_HASH_DESTROY_SESSION \
-   VIRTIO_CRYPTO_OPCODE(VIRTIO_CRYPTO_SERVICE_HASH, 0x03)
+  VIRTIO_CRYPTO_OPCODE(VIRTIO_CRYPTO_SERVICE_HASH, 0x03)
 #define VIRTIO_CRYPTO_MAC_CREATE_SESSION \
-   VIRTIO_CRYPTO_OPCODE(VIRTIO_CRYPTO_SERVICE_MAC, 0x02)
+  VIRTIO_CRYPTO_OPCODE(VIRTIO_CRYPTO_SERVICE_MAC, 0x02)
 #define VIRTIO_CRYPTO_MAC_DESTROY_SESSION \
-   VIRTIO_CRYPTO_OPCODE(VIRTIO_CRYPTO_SERVICE_MAC, 0x03)
+  VIRTIO_CRYPTO_OPCODE(VIRTIO_CRYPTO_SERVICE_MAC, 0x03)
 #define VIRTIO_CRYPTO_AEAD_CREATE_SESSION \
-   VIRTIO_CRYPTO_OPCODE(VIRTIO_CRYPTO_SERVICE_AEAD, 0x02)
+  VIRTIO_CRYPTO_OPCODE(VIRTIO_CRYPTO_SERVICE_AEAD, 0x02)
 #define VIRTIO_CRYPTO_AEAD_DESTROY_SESSION \
-   VIRTIO_CRYPTO_OPCODE(VIRTIO_CRYPTO_SERVICE_AEAD, 0x03)
-__virtio32 opcode;
-__virtio32 algo;
-__virtio32 flag;
-/* data virtqueue id */
-__virtio32 queue_id;
+  VIRTIO_CRYPTO_OPCODE(VIRTIO_CRYPTO_SERVICE_AEAD, 0x03)
+   uint32_t opcode;
+   uint32_t algo;
+   uint32_t flag;
+   /* data virtqueue id */
+   uint32_t queue_id;
 };
 
 struct virtio_crypto_cipher_session_para {
@@ -78,26 +80,27 @@ struct 

Re: [Qemu-devel] [PATCH v8 12/21] qapi.py: fix line break before binary operator pep8

[Markus Armbruster]

Marc-André Lureau  writes:

> Python code style accepts both form, but pep8 complains. Better to clean
> up the single warning for now, so new errors stand out more easily.
>
> Fix scripts/qapi.py:1539:21: W503 line break before binary operator
>
> Signed-off-by: Marc-André Lureau 

I'm dropping this patch, as per review of v7.

Re: [Qemu-devel] [PATCH v8 08/21] qapi: move experimental note down

[Markus Armbruster]

Marc-André Lureau  writes:

> Use a better 'Note:' section, move it below parameters following
> guidelines.
>
> Signed-off-by: Marc-André Lureau 

As proposed in review of v7, I'm replacing commit message by

  qapi: Move "command is experimental" notes down

  Move these notes down and prefix with "Note:", to please the doc
  generator we're going to add.

if that's okay with you.

Re: [Qemu-devel] [PATCH v8 07/21] qapi: avoid interleaving sections and parameters

[Markus Armbruster]

Marc-André Lureau  writes:

> Follow documentation guideline, body, parameters then additional
> sections.
>
> Signed-off-by: Marc-André Lureau 

As proposed in review of v7, I'm replacing commit message by

  qapi: Reorder doc comments for future doc generator
  
  The doc generator we're going to add expects a fairly rigid doc
  comment structure.  Reorder / rephrase some doc comments to please it.

if that's okay with you.

Re: [Qemu-devel] [PATCH v5 0/3] Add litmus tests for MTTCG consistency tests

[no-reply]

Hi,

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

Type: series
Message-id: 20161201052844.31819-1-bobby.pr...@gmail.com
Subject: [Qemu-devel] [PATCH v5 0/3] Add litmus tests for MTTCG consistency 
tests

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

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

# Useful git options
git config --local diff.renamelimit 0
git config --local diff.renames True

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

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

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
fatal: unable to access 'https://github.com/patchew-project/qemu/': The 
requested URL returned error: 504
error: Could not fetch 3c8cf5a9c21ff8782164d1def7f44bd888713384
Traceback (most recent call last):
  File "/usr/bin/patchew", line 406, in test_one
git_clone_repo(clone, r["repo"], r["head"], logf)
  File "/usr/bin/patchew", line 47, in git_clone_repo
stdout=logf, stderr=logf)
  File "/usr/lib64/python3.4/subprocess.py", line 561, in check_call
raise CalledProcessError(retcode, cmd)
subprocess.CalledProcessError: Command '['git', 'remote', 'add', '-f', 
'--mirror=fetch', '3c8cf5a9c21ff8782164d1def7f44bd888713384', 
'https://github.com/patchew-project/qemu']' returned non-zero exit status 1



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

Re: [Qemu-devel] [PATCH v8 03/21] qapi: make TODOs named-sections

[Marc-André Lureau]

Hi

- Original Message -
> Marc-André Lureau  writes:
> 
> > Have the TODO in the TAG: format, so they will stand out in the
> > generated documentation.
> >
> > Signed-off-by: Marc-André Lureau 
> 
> As proposed in review of v7, I'm replacing commit message by
> 
>   qapi: Format TODO comments more consistently
> 
>   Consistently put a colon after TODO.  This will make the TODOs stand
>   out in the documentation we're going to generate.
> 
> if that's okay with you.
> 

sure, sorry I forgot to update it.

Re: [Qemu-devel] [PATCH v8 03/21] qapi: make TODOs named-sections

[Markus Armbruster]

Marc-André Lureau  writes:

> Have the TODO in the TAG: format, so they will stand out in the
> generated documentation.
>
> Signed-off-by: Marc-André Lureau 

As proposed in review of v7, I'm replacing commit message by

  qapi: Format TODO comments more consistently

  Consistently put a colon after TODO.  This will make the TODOs stand
  out in the documentation we're going to generate.

if that's okay with you.

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

[Michael S. Tsirkin]

On Fri, Jan 13, 2017 at 02:54:29AM +, Gonglei (Arei) wrote:
> 
> > 
> > On Thu, Jan 12, 2017 at 12:26:24PM +, Gonglei (Arei) wrote:
> > > Hi,
> > >
> > >
> > > >
> > > > On 01/04/2017 11:10 AM, Gonglei (Arei) wrote:
> > > > > Hi all,
> > > > >
> > > > > I attach the diff files between v14 and v15 for better review.
> > > > >
> > > > Hi,
> > > >
> > > > only had a quick look. Will try to come back to this later.
> > > >
> > > That's cool.
> > >
> > > > > diff --git a/virtio-crypto.tex b/virtio-crypto.tex
> > > > > index 9f7faf0..884ee95 100644
> > > > > --- a/virtio-crypto.tex
> > > > > +++ b/virtio-crypto.tex
> > > > > @@ -2,8 +2,8 @@
> > > > >
> > > > >  The virtio crypto device is a virtual cryptography device as well as 
> > > > > a kind
> > of
> > > > >  virtual hardware accelerator for virtual machines. The encryption and
> > > > > -decryption requests are placed in the data queue and are ultimately
> > handled
> > > > by the
> > > > > -backend crypto accelerators. The second queue is the control queue 
> > > > > used
> > to
> > > > create
> > > > > +decryption requests are placed in any of the data active queues and 
> > > > > are
> > > > ultimately handled by the
> > > > s/data active/active data/
> > > > > +backend crypto accelerators. The second kind of queue is the control
> > queue
> > > > used to create
> > > > >  or destroy sessions for symmetric algorithms and will control some
> > > > advanced
> > > > >  features in the future. The virtio crypto device provides the 
> > > > > following
> > crypto
> > > > >  services: CIPHER, MAC, HASH, and AEAD.
> > > >
> > > > [..]
> > > >
> > > > > ===The below diff shows the changes of add non-session
> > mode
> > > > support:
> > > > >
> > > > > diff --git a/virtio-crypto.tex b/virtio-crypto.tex
> > > > > index 884ee95..44819f9 100644
> > > > > --- a/virtio-crypto.tex
> > > > > +++ b/virtio-crypto.tex
> > > > > @@ -26,7 +26,10 @@ N is set by \field{max_dataqueues}.
> > > > >
> > > > >  \subsection{Feature bits}\label{sec:Device Types / Crypto Device /
> > Feature
> > > > bits}
> > > > >
> > > > > -None currently defined.
> > > > > +VIRTIO_CRYPTO_F_CIPHER_SESSION_MODE (1) Session mode is
> > available
> > > > for CIPHER service.
> > > > > +VIRTIO_CRYPTO_F_HASH_SESSION_MODE (2) Session mode is available
> > for
> > > > HASH service.
> > > > > +VIRTIO_CRYPTO_F_MAC_SESSION_MODE (3) Session mode is available
> > for
> > > > MAC service.
> > > > > +VIRTIO_CRYPTO_F_AEAD_SESSION_MODE (4) Session mode is available
> > for
> > > > AEAD service.
> > > > >
> > > > >  \subsection{Device configuration layout}\label{sec:Device Types /
> > Crypto
> > > > Device / Device configuration layout}
> > > > >
> > > > > @@ -208,6 +211,9 @@ Operation parameters are algorithm-specific
> > > > parameters, output data is the
> > > > >  data that should be utilized in operations, and input data is equal 
> > > > > to
> > > > >  "operation result + result data".
> > > > >
> > > > > +The device can support both session mode (See \ref{sec:Device Types /
> > > > Crypto Device / Device Operation / Control Virtqueue / Session 
> > > > operation})
> > and
> > > > non-session mode, for example,
> > > > > +As VIRTIO_CRYPTO_F_CIPHER_SESSION feature bit is negotiated, the
> > driver
> > > > can use session mode for CIPHER service, otherwise it can only use
> > non-session
> > > > mode.
> > > > > +
> > > >
> > > > As far as I understand you are adding non-session mode to the mix but
> > > > providing feature bits for session mode. Would this render the the 
> > > > current
> > > > implementation non-compliant?
> > > >
> > > You are right, shall we use feature bits for non-session mode for 
> > > compliancy?
> > > Or because the spec is on the fly, and some structures in the 
> > > virtio_crypto.h
> > need to
> > > be modified, can we keep the compliancy completely?
> > >
> > > Thanks,
> > > -Gonglei
> > 
> > Since there's a linux driver upstream you must at least
> > keep compatibility with that.
> > 
> Sure. We use feature bits for non-session mode then.
> For structures modification, do we need to do some specific
> actions for compatibility?  Take CIPHER service as an example:
> 
> The present structure:
> 
> struct virtio_crypto_cipher_para {
> /*
>  * Byte Length of valid IV/Counter data pointed to by the below iv data.
>  *
>  * For block ciphers in CBC or F8 mode, or for Kasumi in F8 mode, or for
>  *   SNOW3G in UEA2 mode, this is the length of the IV (which
>  *   must be the same as the block length of the cipher).
>  * For block ciphers in CTR mode, this is the length of the counter
>  *   (which must be the same as the block length of the cipher).
>  */
> le32 iv_len;
> /* length of source data */
> le32 src_data_len;
> /* length of destination data */
> le32 dst_data_len;
> };
> 
> The future structure if supporting non-session based operations:
> 
> struct virtio_crypto_cipher_para {
> struct {
>   

Re: [Qemu-devel] [PATCH RFC v3 00/14] VT-d: vfio enablement and misc enhances

[Michael S. Tsirkin]

On Fri, Jan 13, 2017 at 11:06:26AM +0800, Peter Xu wrote:
> v3:
> - fix style error reported by patchew
> - fix comment in domain switch patch: use "IOMMU address space" rather
>   than "IOMMU region" [Kevin]
> - add ack-by for Paolo in patch:
>   "memory: add section range info for IOMMU notifier"
>   (this is seperately collected besides this thread)
> - remove 3 patches which are merged already (from Jason)
> - rebase to master b6c0897

So 1-6 look like nice cleanups to me. Should I merge them now?

> v2:
> - change comment for "end" parameter in vtd_page_walk() [Tianyu]
> - change comment for "a iova" to "an iova" [Yi]
> - fix fault printed val for GPA address in vtd_page_walk_level (debug
>   only)
> - rebased to master (rather than Aviv's v6 series) and merged Aviv's
>   series v6: picked patch 1 (as patch 1 in this series), dropped patch
>   2, re-wrote patch 3 (as patch 17 of this series).
> - picked up two more bugfix patches from Jason's DMAR series
> - picked up the following patch as well:
>   "[PATCH v3] intel_iommu: allow dynamic switch of IOMMU region"
> 
> This RFC series is a re-work for Aviv B.D.'s vfio enablement series
> with vt-d:
> 
>   https://lists.gnu.org/archive/html/qemu-devel/2016-11/msg01452.html
> 
> Aviv has done a great job there, and what we still lack there are
> mostly the following:
> 
> (1) VFIO got duplicated IOTLB notifications due to splitted VT-d IOMMU
> memory region.
> 
> (2) VT-d still haven't provide a correct replay() mechanism (e.g.,
> when IOMMU domain switches, things will broke).
> 
> This series should have solved the above two issues.
> 
> Online repo:
> 
>   https://github.com/xzpeter/qemu/tree/vtd-vfio-enablement-v2
> 
> I would be glad to hear about any review comments for above patches.
> 
> =
> Test Done
> =
> 
> Build test passed for x86_64/arm/ppc64.
> 
> Simply tested with x86_64, assigning two PCI devices to a single VM,
> boot the VM using:
> 
> bin=x86_64-softmmu/qemu-system-x86_64
> $bin -M q35,accel=kvm,kernel-irqchip=split -m 1G \
>  -device intel-iommu,intremap=on,eim=off,cache-mode=on \
>  -netdev user,id=net0,hostfwd=tcp::-:22 \
>  -device virtio-net-pci,netdev=net0 \
>  -device vfio-pci,host=03:00.0 \
>  -device vfio-pci,host=02:00.0 \
>  -trace events=".trace.vfio" \
>  /var/lib/libvirt/images/vm1.qcow2
> 
> pxdev:bin [vtd-vfio-enablement]# cat .trace.vfio
> vtd_page_walk*
> vtd_replay*
> vtd_inv_desc*
> 
> Then, in the guest, run the following tool:
> 
>   
> https://github.com/xzpeter/clibs/blob/master/gpl/userspace/vfio-bind-group/vfio-bind-group.c
> 
> With parameter:
> 
>   ./vfio-bind-group 00:03.0 00:04.0
> 
> Check host side trace log, I can see pages are replayed and mapped in
> 00:04.0 device address space, like:
> 
> ...
> vtd_replay_ce_valid replay valid context device 00:04.00 hi 0x401 lo 
> 0x38fe1001
> vtd_page_walk Page walk for ce (0x401, 0x38fe1001) iova range 0x0 - 
> 0x80
> vtd_page_walk_level Page walk (base=0x38fe1000, level=3) iova range 0x0 - 
> 0x80
> vtd_page_walk_level Page walk (base=0x35d31000, level=2) iova range 0x0 - 
> 0x4000
> vtd_page_walk_level Page walk (base=0x34979000, level=1) iova range 0x0 - 
> 0x20
> vtd_page_walk_one Page walk detected map level 0x1 iova 0x0 -> gpa 0x22dc3000 
> mask 0xfff perm 3
> vtd_page_walk_one Page walk detected map level 0x1 iova 0x1000 -> gpa 
> 0x22e25000 mask 0xfff perm 3
> vtd_page_walk_one Page walk detected map level 0x1 iova 0x2000 -> gpa 
> 0x22e12000 mask 0xfff perm 3
> vtd_page_walk_one Page walk detected map level 0x1 iova 0x3000 -> gpa 
> 0x22e2d000 mask 0xfff perm 3
> vtd_page_walk_one Page walk detected map level 0x1 iova 0x4000 -> gpa 
> 0x12a49000 mask 0xfff perm 3
> vtd_page_walk_one Page walk detected map level 0x1 iova 0x5000 -> gpa 
> 0x129bb000 mask 0xfff perm 3
> vtd_page_walk_one Page walk detected map level 0x1 iova 0x6000 -> gpa 
> 0x128db000 mask 0xfff perm 3
> vtd_page_walk_one Page walk detected map level 0x1 iova 0x7000 -> gpa 
> 0x12a8 mask 0xfff perm 3
> vtd_page_walk_one Page walk detected map level 0x1 iova 0x8000 -> gpa 
> 0x12a7e000 mask 0xfff perm 3
> vtd_page_walk_one Page walk detected map level 0x1 iova 0x9000 -> gpa 
> 0x12b22000 mask 0xfff perm 3
> vtd_page_walk_one Page walk detected map level 0x1 iova 0xa000 -> gpa 
> 0x12b41000 mask 0xfff perm 3
> ...
> 
> =
> Todo List
> =
> 
> - error reporting for the assigned devices (as Tianyu has mentioned)
> 
> - per-domain address-space: A better solution in the future may be -
>   we maintain one address space per IOMMU domain in the guest (so
>   multiple devices can share a same address space if they are sharing
>   the same IOMMU domains in the guest), rather than one address space
>   per device (which is current implementation of vt-d). However that's
>   a step further than this series, and let's see whether we can first
>   provide a workable version of device assignment with 

Re: [Qemu-devel] [PATCH 2/2] virtio: disable notifications again after poll succeeded

[Michael S. Tsirkin]

On Thu, Jan 12, 2017 at 11:46:11AM +, Stefan Hajnoczi wrote:
> While AioContext is in polling mode virtqueue notifications are not
> necessary.  Some device virtqueue handlers enable notifications.  Make
> sure they stay disabled to avoid unnecessary vmexits.
> 
> Signed-off-by: Stefan Hajnoczi 

So I'll put just the revert in today's pull request,
let's make sure this one is not causing regressions.

> ---
>  hw/virtio/virtio.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
> index f04ab7a..34065c7 100644
> --- a/hw/virtio/virtio.c
> +++ b/hw/virtio/virtio.c
> @@ -2126,6 +2126,9 @@ static bool virtio_queue_host_notifier_aio_poll(void 
> *opaque)
>  }
>  
>  virtio_queue_notify_aio_vq(vq);
> +
> +/* In case the handler function re-enabled notifications */
> +virtio_queue_set_notification(vq, 0);
>  return true;
>  }
>  
> -- 
> 2.9.3

Re: [Qemu-devel] [PATCH RFC v3 1/2] block/qapi: reduce the coupling between the bdrv_query_stats and bdrv_query_bds_stats

[Eric Blake]

On 01/04/2017 12:58 AM, Dou Liyang wrote:
> the bdrv_query_stats and bdrv_query_bds_stats functions need to call
> each other, that increases the coupling. it also makes the program
> complicated and makes some unnecessary judgements.

s/judgements/judgments/ - although I wonder if 'tests' would be a
simpler word.

> 
> remove the call from bdrv_query_bds_stats to bdrv_query_stats, just
> take some recursion to make it clearly.
> 
> avoid judging whether the blk is NULL during querying the bds stats.

Again, 'testing' might read nicer than 'judging'.

> it is unnecessary.
> 
> Signed-off-by: Dou Liyang 
> ---
>  block/qapi.c | 26 ++
>  1 file changed, 14 insertions(+), 12 deletions(-)
> 

> -static void bdrv_query_bds_stats(BlockStats *s, const BlockDriverState *bs,
> +static BlockStats *bdrv_query_bds_stats(const BlockDriverState *bs,
>   bool query_backing)
>  {
> +BlockStats *s = NULL;
> +
> +s = g_malloc0(sizeof(*s));

Dead initialization to NULL since the very next statement overwrites it.

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



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH 18/40] char: remove class kind field

[Marc-André Lureau]

Hi

- Original Message -
> On 01/11/2017 11:29 AM, Marc-André Lureau wrote:
> > The class kind is necessary to lookup the chardev name in
> > qmp_chardev_add() after calling qemu_chr_new_from_opts() and to set
> > the appropriate ChardevBackend (mainly to free the right
> > fields).
> > 
> > qemu_chr_new_from_opts() can be changed to use a non-qmp function
> > using the chardev class typename. Introduce qemu_chardev_add() to be
> > called from qemu_chr_new_from_opts() and remove the class chardev kind
> > field. Set the backend->type in the parse callback (when non-common
> > fields are added).
> > 
> > Signed-off-by: Marc-André Lureau 
> > ---
> 
> >  
> > +static Chardev *qemu_chardev_add(const char *id, const char *typename,
> > + ChardevBackend *backend, Error **errp)
> > +{
> > +Chardev *chr;
> > +
> > +chr = qemu_chr_find(id);
> > +if (chr) {
> > +error_setg(errp, "Chardev '%s' already exists", id);
> > +return NULL;
> > +}
> > +
> > +chr = qemu_chardev_new(id, typename, backend, errp);
> > +if (!chr) {
> > +return NULL;
> > +}
> > +
> > +QTAILQ_INSERT_TAIL(, chr, next);
> > +return chr;
> > +}
> > +
> 
> This part seems okay.
> 
> > @@ -4222,22 +4235,22 @@ Chardev *qemu_chr_new_from_opts(QemuOpts *opts,
> >  
> >  cc = char_get_class(name, errp);
> >  if (cc == NULL) {
> > -goto err;
> > +return NULL;
> >  }
> >  
> >  backend = g_new0(ChardevBackend, 1);
> > +backend->type = CHARDEV_BACKEND_KIND_NULL;
> >  
> >  if (qemu_opt_get_bool(opts, "mux", 0)) {
> >  bid = g_strdup_printf("%s-base", id);
> >  }
> >  
> >  chr = NULL;
> > -backend->type = cc->kind;
> 
> I'm not sure I follow this hunk - we used to set backend->type
> dynamically and now it is forced to KIND_NULL.  Is the point that we
> don't need to set backend->type here because the later call to
> qemu_chardev_add()...

I tried to explain some of the reasons in the commit message.

We need backend->type to be set correctly for the qapi_free_ChardevBackend() to 
work. The kind field used to be on a class member, but we try to get rid of it. 
So we move that information to _parse(), and change the backend type 
appropriately there.

KIND_NULL is the common backend (sharing ChardevCommon).

> 
> > @@ -4245,37 +4258,33 @@ Chardev *qemu_chr_new_from_opts(QemuOpts *opts,
> >  backend->u.null.data = ccom; /* Any ChardevCommon member would
> >  work */
> >  }
> >  
> > -ret = qmp_chardev_add(bid ? bid : id, backend, errp);
> > -if (!ret) {
> > -goto qapi_out;
> > +chr = qemu_chardev_add(bid ? bid : id,
> > +   object_class_get_name(OBJECT_CLASS(cc)),
> > +   backend, errp);
> 
> ...now passes the Object type which can be used to derive the same same
> information?  In that case, was the assignment to backend->type =
> KIND_NULL dead?

0 is KIND_FILE, which would lead to invalid free.
 
> > +if (chr == NULL) {
> > +goto out;
> >  }
> >  
> >  if (bid) {
> > +Chardev *mux;
> >  qapi_free_ChardevBackend(backend);
> > -qapi_free_ChardevReturn(ret);
> >  backend = g_new0(ChardevBackend, 1);
> > -backend->u.mux.data = g_new0(ChardevMux, 1);
> >  backend->type = CHARDEV_BACKEND_KIND_MUX;
> > +backend->u.mux.data = g_new0(ChardevMux, 1);
> 
> Why the churn on the assignment to backend->u.mux.data?

fixed

> 
> >  backend->u.mux.data->chardev = g_strdup(bid);
> > -ret = qmp_chardev_add(id, backend, errp);
> > -if (!ret) {
> > -chr = qemu_chr_find(bid);
> > +mux = qemu_chardev_add(id, TYPE_CHARDEV_MUX, backend, errp);
> > +if (mux == NULL) {
> >  qemu_chr_delete(chr);
> >  chr = NULL;
> > -goto qapi_out;
> > +goto out;
> >  }
> > +chr = mux;
> >  }
> >  
> > -chr = qemu_chr_find(id);
> > -
> > -qapi_out:
> > +out:
> >  qapi_free_ChardevBackend(backend);
> > -qapi_free_ChardevReturn(ret);
> >  g_free(bid);
> >  return chr;
> > -
> > -err:
> > -return NULL;
> >  }
> >  
> 
> > @@ -5010,24 +5014,18 @@ Chardev *qemu_chardev_new(const char *id, const
> > char *typename,
> >  ChardevReturn *qmp_chardev_add(const char *id, ChardevBackend *backend,
> > Error **errp)
> >  {
> > -const ChardevClass *cc;
> >  ChardevReturn *ret;
> > +const ChardevClass *cc;
> 
> Why the churn on this declaration?

sorry, too many rebases, conflicts.. fixed

> 
> >  Chardev *chr;
> >  
> > -chr = qemu_chr_find(id);
> > -if (chr) {
> > -error_setg(errp, "Chardev '%s' already exists", id);
> > -return NULL;
> > -}
> > -
> >  cc = char_get_class(ChardevBackendKind_lookup[backend->type], errp);
> > -if (!cc) {
> > +

Re: [Qemu-devel] [PATCH 0/2] virtio: revert virtio_queue_set_notification() nesting

[Doug Goldstein]

On 1/13/17 6:02 AM, Stefan Hajnoczi wrote:
> On Thu, Jan 12, 2017 at 10:57:53AM -0600, Doug Goldstein wrote:
>> On 1/12/17 5:46 AM, Stefan Hajnoczi wrote:
>>> The virtio_queue_set_notification() nesting introduced for AioContext 
>>> polling
>>> raised an assertion with virtio-net (even in non-polling mode).  Converting
>>> virtio-net and virtio-crypto to use virtio_queue_set_notification() in a
>>> nesting fashion would be invasive and isn't worth it.
>>>
>>> Patch 1 contains the revert to resolve the bug that Doug noticed.
>>>
>>> Patch 2 is a less efficient but safe alternative.
>>>
>>> Stefan Hajnoczi (2):
>>>   Revert "virtio: turn vq->notification into a nested counter"
>>>   virtio: disable notifications again after poll succeeded
>>>
>>>  hw/virtio/virtio.c | 21 +
>>>  1 file changed, 9 insertions(+), 12 deletions(-)
>>>
>>
>> So I just gave this series a whirl and it fixes the assert but causes
>> another issue for me. While iPXE is getting a DHCP address the screen
>> immediately flashes over to the UEFI shell. Its like a timeout is
>> getting hit and just dropping me to the shell.
> 
> Sounds like an separate problem.
> 
> Stefan
> 

Is there any debug output that I can provide to help troubleshoot it?
I've built 23425cc and UEFI via OVMF is able to get an IP address via
DHCP inside of iPXE. I've also taken master and only applied the first
patch in this series (the revert) and it too works. Its only when I add
the 2nd patch into the mix or don't revert out the "virtio: turn
vq->notification into a nested counter" patch that it fails.

-- 
Doug Goldstein



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH] libvhost-user: Start VQs on SET_VRING_CALL

[Felipe Franciosi]

Hi Marc-Andre,

> On 13 Jan 2017, at 07:03, Marc-André Lureau  wrote:
> 
> Hi
> 
> - Original Message -
>> Currently, VQs are started as soon as a SET_VRING_KICK is received. That
>> is too early in the VQ setup process, as the backend might not yet have
> 
> I think we may want to reconsider queue_set_started(), move it elsewhere, 
> since kick/call fds aren't mandatory to process the rings.

Hmm. The fds aren't mandatory, but I imagine in that case we should still 
receive SET_VRING_KICK/CALL messages without an fd (ie. with the 
VHOST_MSG_VQ_NOFD_MASK flag set). Wouldn't that be the case?

> 
>> a callfd to notify in case it received a kick and fully processed the
>> request/command. This patch only starts a VQ when a SET_VRING_CALL is
>> received.
> 
> I don't like that much, as soon as the kick fd is received, it should start 
> polling it imho. callfd is optional, it may have one and not the other.

So the question is whether we should be receiving a SET_VRING_CALL anyway or 
not, regardless of an fd being sent. (I think we do, but I haven't done 
extensive testing with other device types.)

> 
> Perhaps it's best for now to delay the callfd notification with a flag until 
> it is received?

The other idea is to always kick when we receive the callfd. I remember 
discussing that alternative with you before libvhost-user went in. The protocol 
says both the driver and the backend must handle spurious kicks. This approach 
also fixes the bug.

I'm happy with whatever alternative you want, as long it makes libvhost-user 
usable for storage devices.

Thanks,
Felipe


> 
> 
>> Signed-off-by: Felipe Franciosi 
>> ---
>> contrib/libvhost-user/libvhost-user.c | 26 +-
>> 1 file changed, 13 insertions(+), 13 deletions(-)
>> 
>> diff --git a/contrib/libvhost-user/libvhost-user.c
>> b/contrib/libvhost-user/libvhost-user.c
>> index af4faad..a46ef90 100644
>> --- a/contrib/libvhost-user/libvhost-user.c
>> +++ b/contrib/libvhost-user/libvhost-user.c
>> @@ -607,19 +607,6 @@ vu_set_vring_kick_exec(VuDev *dev, VhostUserMsg *vmsg)
>> DPRINT("Got kick_fd: %d for vq: %d\n", vmsg->fds[0], index);
>> }
>> 
>> -dev->vq[index].started = true;
>> -if (dev->iface->queue_set_started) {
>> -dev->iface->queue_set_started(dev, index, true);
>> -}
>> -
>> -if (dev->vq[index].kick_fd != -1 && dev->vq[index].handler) {
>> -dev->set_watch(dev, dev->vq[index].kick_fd, VU_WATCH_IN,
>> -   vu_kick_cb, (void *)(long)index);
>> -
>> -DPRINT("Waiting for kicks on fd: %d for vq: %d\n",
>> -   dev->vq[index].kick_fd, index);
>> -}
>> -
>> return false;
>> }
>> 
>> @@ -661,6 +648,19 @@ vu_set_vring_call_exec(VuDev *dev, VhostUserMsg *vmsg)
>> 
>> DPRINT("Got call_fd: %d for vq: %d\n", vmsg->fds[0], index);
>> 
>> +dev->vq[index].started = true;
>> +if (dev->iface->queue_set_started) {
>> +dev->iface->queue_set_started(dev, index, true);
>> +}
>> +
>> +if (dev->vq[index].kick_fd != -1 && dev->vq[index].handler) {
>> +dev->set_watch(dev, dev->vq[index].kick_fd, VU_WATCH_IN,
>> +   vu_kick_cb, (void *)(long)index);
>> +
>> +DPRINT("Waiting for kicks on fd: %d for vq: %d\n",
>> +   dev->vq[index].kick_fd, index);
>> +}
>> +
>> return false;
>> }
>> 
>> --
>> 1.9.4
>> 
>> 

[Qemu-devel] [PULL 1/3] qom/cpu: move tlb_flush to cpu_common_reset

[Alex Bennée]

It is a common thing amongst the various cpu reset functions want to
flush the SoftMMU's TLB entries. This is done either by calling
tlb_flush directly or by way of a general memset of the CPU
structure (sometimes both).

This moves the tlb_flush call to the common reset function and
additionally ensures it is only done for the CONFIG_SOFTMMU case and
when tcg is enabled.

In some target cases we add an empty end_of_reset_fields structure to the
target vCPU structure so have a clear end point for any memset which
is resetting value in the structure before CPU_COMMON (where the TLB
structures are).

While this is a nice clean-up in general it is also a precursor for
changes coming to cputlb for MTTCG where the clearing of entries
can't be done arbitrarily across vCPUs. Currently the cpu_reset
function is usually called from the context of another vCPU as the
architectural power up sequence is run. By using the cputlb API
functions we can ensure the right behaviour in the future.

Signed-off-by: Alex Bennée 
Reviewed-by: Richard Henderson 
Reviewed-by: David Gibson 
---
 qom/cpu.c   | 4 
 target/arm/cpu.c| 5 ++---
 target/arm/cpu.h| 5 -
 target/cris/cpu.c   | 3 +--
 target/cris/cpu.h   | 9 ++---
 target/i386/cpu.c   | 2 --
 target/i386/cpu.h   | 6 --
 target/lm32/cpu.c   | 3 +--
 target/lm32/cpu.h   | 3 +++
 target/m68k/cpu.c   | 3 +--
 target/m68k/cpu.h   | 3 +++
 target/microblaze/cpu.c | 3 +--
 target/microblaze/cpu.h | 3 +++
 target/mips/cpu.c   | 3 +--
 target/mips/cpu.h   | 3 +++
 target/moxie/cpu.c  | 4 +---
 target/moxie/cpu.h  | 3 +++
 target/openrisc/cpu.c   | 9 +
 target/openrisc/cpu.h   | 3 +++
 target/ppc/translate_init.c | 3 ---
 target/s390x/cpu.c  | 7 ++-
 target/s390x/cpu.h  | 5 +++--
 target/sh4/cpu.c| 3 +--
 target/sh4/cpu.h| 3 +++
 target/sparc/cpu.c  | 3 +--
 target/sparc/cpu.h  | 3 +++
 target/tilegx/cpu.c | 3 +--
 target/tilegx/cpu.h | 3 +++
 target/tricore/cpu.c| 2 --
 29 files changed, 62 insertions(+), 50 deletions(-)

diff --git a/qom/cpu.c b/qom/cpu.c
index 03d9190f8c..cc51de2a8c 100644
--- a/qom/cpu.c
+++ b/qom/cpu.c
@@ -273,6 +273,10 @@ static void cpu_common_reset(CPUState *cpu)
 for (i = 0; i < TB_JMP_CACHE_SIZE; ++i) {
 atomic_set(>tb_jmp_cache[i], NULL);
 }
+
+#ifdef CONFIG_SOFTMMU
+tlb_flush(cpu, 0);
+#endif
 }
 
 static bool cpu_common_has_work(CPUState *cs)
diff --git a/target/arm/cpu.c b/target/arm/cpu.c
index f5cb30af6c..91046111d9 100644
--- a/target/arm/cpu.c
+++ b/target/arm/cpu.c
@@ -122,7 +122,8 @@ static void arm_cpu_reset(CPUState *s)
 
 acc->parent_reset(s);
 
-memset(env, 0, offsetof(CPUARMState, features));
+memset(env, 0, offsetof(CPUARMState, end_reset_fields));
+
 g_hash_table_foreach(cpu->cp_regs, cp_reg_reset, cpu);
 g_hash_table_foreach(cpu->cp_regs, cp_reg_check_reset, cpu);
 
@@ -226,8 +227,6 @@ static void arm_cpu_reset(CPUState *s)
   >vfp.fp_status);
 set_float_detect_tininess(float_tininess_before_rounding,
   >vfp.standard_fp_status);
-tlb_flush(s, 1);
-
 #ifndef CONFIG_USER_ONLY
 if (kvm_enabled()) {
 kvm_arm_reset_vcpu(cpu);
diff --git a/target/arm/cpu.h b/target/arm/cpu.h
index ab119e62ab..7bd16eec18 100644
--- a/target/arm/cpu.h
+++ b/target/arm/cpu.h
@@ -491,9 +491,12 @@ typedef struct CPUARMState {
 struct CPUBreakpoint *cpu_breakpoint[16];
 struct CPUWatchpoint *cpu_watchpoint[16];
 
+/* Fields up to this point are cleared by a CPU reset */
+struct {} end_reset_fields;
+
 CPU_COMMON
 
-/* These fields after the common ones so they are preserved on reset.  */
+/* Fields after CPU_COMMON are preserved across CPU reset. */
 
 /* Internal CPU feature flags.  */
 uint64_t features;
diff --git a/target/cris/cpu.c b/target/cris/cpu.c
index 2e9ab9700e..5f766f09d6 100644
--- a/target/cris/cpu.c
+++ b/target/cris/cpu.c
@@ -52,9 +52,8 @@ static void cris_cpu_reset(CPUState *s)
 ccc->parent_reset(s);
 
 vr = env->pregs[PR_VR];
-memset(env, 0, offsetof(CPUCRISState, load_info));
+memset(env, 0, offsetof(CPUCRISState, end_reset_fields));
 env->pregs[PR_VR] = vr;
-tlb_flush(s, 1);
 
 #if defined(CONFIG_USER_ONLY)
 /* start in user mode with interrupts enabled.  */
diff --git a/target/cris/cpu.h b/target/cris/cpu.h
index 43d5f9d1da..920e1c33ba 100644
--- a/target/cris/cpu.h
+++ b/target/cris/cpu.h
@@ -167,10 +167,13 @@ typedef struct CPUCRISState {
 */
 TLBSet tlbsets[2][4][16];
 
-   CPU_COMMON
+/* Fields up to this point are cleared by a CPU reset */
+struct {} end_reset_fields;
 
-/* Members from load_info on 

[Qemu-devel] [PULL 3/3] cputlb: drop flush_global flag from tlb_flush

[Alex Bennée]

We have never has the concept of global TLB entries which would avoid
the flush so we never actually use this flag. Drop it and make clear
that tlb_flush is the sledge-hammer it has always been.

Signed-off-by: Alex Bennée 
Reviewed-by: Richard Henderson 
[DG: ppc portions]
Acked-by: David Gibson 
---
 cputlb.c   | 21 ++---
 exec.c |  4 ++--
 hw/sh4/sh7750.c|  2 +-
 include/exec/exec-all.h| 14 ++
 target/alpha/cpu.c |  2 +-
 target/alpha/sys_helper.c  |  2 +-
 target/arm/helper.c| 26 +-
 target/i386/fpu_helper.c   |  2 +-
 target/i386/helper.c   |  8 
 target/i386/machine.c  |  2 +-
 target/i386/misc_helper.c  |  2 +-
 target/i386/svm_helper.c   |  2 +-
 target/microblaze/mmu.c|  2 +-
 target/mips/cpu.h  |  2 +-
 target/mips/helper.c   |  6 +++---
 target/mips/op_helper.c|  8 
 target/openrisc/interrupt.c|  2 +-
 target/openrisc/interrupt_helper.c |  2 +-
 target/openrisc/sys_helper.c   |  2 +-
 target/ppc/helper_regs.h   |  4 ++--
 target/ppc/misc_helper.c   |  4 ++--
 target/ppc/mmu_helper.c| 32 
 target/s390x/gdbstub.c |  2 +-
 target/s390x/mem_helper.c  |  8 
 target/sh4/helper.c|  2 +-
 target/sparc/ldst_helper.c | 12 ++--
 target/unicore32/cpu.c |  2 +-
 target/unicore32/helper.c  |  2 +-
 target/xtensa/op_helper.c  |  2 +-
 29 files changed, 85 insertions(+), 96 deletions(-)

diff --git a/cputlb.c b/cputlb.c
index 813279f3bc..6c39927455 100644
--- a/cputlb.c
+++ b/cputlb.c
@@ -60,24 +60,15 @@
 /* statistics */
 int tlb_flush_count;
 
-/* NOTE:
- * If flush_global is true (the usual case), flush all tlb entries.
- * If flush_global is false, flush (at least) all tlb entries not
- * marked global.
- *
- * Since QEMU doesn't currently implement a global/not-global flag
- * for tlb entries, at the moment tlb_flush() will also flush all
- * tlb entries in the flush_global == false case. This is OK because
- * CPU architectures generally permit an implementation to drop
- * entries from the TLB at any time, so flushing more entries than
- * required is only an efficiency issue, not a correctness issue.
+/* This is OK because CPU architectures generally permit an
+ * implementation to drop entries from the TLB at any time, so
+ * flushing more entries than required is only an efficiency issue,
+ * not a correctness issue.
  */
-void tlb_flush(CPUState *cpu, int flush_global)
+void tlb_flush(CPUState *cpu)
 {
 CPUArchState *env = cpu->env_ptr;
 
-tlb_debug("(%d)\n", flush_global);
-
 memset(env->tlb_table, -1, sizeof(env->tlb_table));
 memset(env->tlb_v_table, -1, sizeof(env->tlb_v_table));
 memset(cpu->tb_jmp_cache, 0, sizeof(cpu->tb_jmp_cache));
@@ -144,7 +135,7 @@ void tlb_flush_page(CPUState *cpu, target_ulong addr)
   TARGET_FMT_lx "/" TARGET_FMT_lx ")\n",
   env->tlb_flush_addr, env->tlb_flush_mask);
 
-tlb_flush(cpu, 1);
+tlb_flush(cpu);
 return;
 }
 
diff --git a/exec.c b/exec.c
index 47835c1dc1..401a9127c2 100644
--- a/exec.c
+++ b/exec.c
@@ -544,7 +544,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;
-tlb_flush(cpu, 1);
+tlb_flush(cpu);
 
 return 0;
 }
@@ -2426,7 +2426,7 @@ static void tcg_commit(MemoryListener *listener)
  */
 d = atomic_rcu_read(>as->dispatch);
 atomic_rcu_set(>memory_dispatch, d);
-tlb_flush(cpuas->cpu, 1);
+tlb_flush(cpuas->cpu);
 }
 
 void address_space_init_dispatch(AddressSpace *as)
diff --git a/hw/sh4/sh7750.c b/hw/sh4/sh7750.c
index 3132d559d7..166e4bd947 100644
--- a/hw/sh4/sh7750.c
+++ b/hw/sh4/sh7750.c
@@ -417,7 +417,7 @@ static void sh7750_mem_writel(void *opaque, hwaddr addr,
 case SH7750_PTEH_A7:
 /* If asid changes, clear all registered tlb entries. */
 if ((s->cpu->env.pteh & 0xff) != (mem_value & 0xff)) {
-tlb_flush(CPU(s->cpu), 1);
+tlb_flush(CPU(s->cpu));
 }
 s->cpu->env.pteh = mem_value;
 return;
diff --git a/include/exec/exec-all.h b/include/exec/exec-all.h
index a8c13cee66..bbc9478a50 100644
--- a/include/exec/exec-all.h
+++ b/include/exec/exec-all.h
@@ -95,15 +95,13 @@ void tlb_flush_page(CPUState *cpu, target_ulong addr);
 /**
  * tlb_flush:
  * @cpu: CPU whose TLB should be flushed
- * @flush_global: ignored
  *
- * Flush the entire TLB for the specified CPU.
- * The flush_global flag is in theory an 

Re: [Qemu-devel] [PATCH 0/2] virtio: revert virtio_queue_set_notification() nesting

[Doug Goldstein]

On 1/12/17 2:05 PM, Michael S. Tsirkin wrote:
> On Thu, Jan 12, 2017 at 10:57:53AM -0600, Doug Goldstein wrote:
>> On 1/12/17 5:46 AM, Stefan Hajnoczi wrote:
>>> The virtio_queue_set_notification() nesting introduced for AioContext 
>>> polling
>>> raised an assertion with virtio-net (even in non-polling mode).  Converting
>>> virtio-net and virtio-crypto to use virtio_queue_set_notification() in a
>>> nesting fashion would be invasive and isn't worth it.
>>>
>>> Patch 1 contains the revert to resolve the bug that Doug noticed.
>>>
>>> Patch 2 is a less efficient but safe alternative.
>>>
>>> Stefan Hajnoczi (2):
>>>   Revert "virtio: turn vq->notification into a nested counter"
>>>   virtio: disable notifications again after poll succeeded
>>>
>>>  hw/virtio/virtio.c | 21 +
>>>  1 file changed, 9 insertions(+), 12 deletions(-)
>>>
>>
>> So I just gave this series a whirl and it fixes the assert but causes
>> another issue for me. While iPXE is getting a DHCP address the screen
>> immediately flashes over to the UEFI shell. Its like a timeout is
>> getting hit and just dropping me to the shell.
>>
>> -- 
>> Doug Goldstein
>>
> 
> Is this just with UEFI or with seabios as well?
> 

Sorry for the delay on responding. I've now tested seabios and UEFI
(using OVMF). The issue is only happening via UEFI and it works fine
with seabios.

-- 
Doug Goldstein



signature.asc
Description: OpenPGP digital signature

[Qemu-devel] [PATCH v8 21/21] build-sys: add qapi doc generation targets

[Marc-André Lureau]

Generate and install the man, txt and html versions of QAPI
documentation (generate and install qemu-doc.txt too).

Add it also to optional pdf/info targets.

Signed-off-by: Marc-André Lureau 
---
 .gitignore |  9 +
 Makefile   | 43 ---
 configure  |  2 +-
 docs/qmp-intro.txt |  3 +--
 rules.mak  |  2 ++
 5 files changed, 49 insertions(+), 10 deletions(-)

diff --git a/.gitignore b/.gitignore
index 568c4bf9d3..78f180a020 100644
--- a/.gitignore
+++ b/.gitignore
@@ -105,6 +105,15 @@
 /pc-bios/optionrom/kvmvapic.img
 /pc-bios/s390-ccw/s390-ccw.elf
 /pc-bios/s390-ccw/s390-ccw.img
+/docs/qemu-ga-ref.html
+/docs/qemu-ga-ref.txt
+/docs/qemu-qmp-ref.html
+/docs/qemu-qmp-ref.txt
+docs/qemu-ga-ref.info*
+docs/qemu-qmp-ref.info*
+/qemu-ga-qapi.texi
+/qemu-qapi.texi
+*.tps
 .stgit-*
 cscope.*
 tags
diff --git a/Makefile b/Makefile
index 3280da201b..9f8968d047 100644
--- a/Makefile
+++ b/Makefile
@@ -91,6 +91,8 @@ HELPERS-$(CONFIG_LINUX) = qemu-bridge-helper$(EXESUF)
 
 ifdef BUILD_DOCS
 DOCS=qemu-doc.html qemu-doc.txt qemu.1 qemu-img.1 qemu-nbd.8 qemu-ga.8
+DOCS+=docs/qemu-qmp-ref.html docs/qemu-qmp-ref.txt docs/qemu-qmp-ref.7
+DOCS+=docs/qemu-ga-ref.html docs/qemu-ga-ref.txt docs/qemu-ga-ref.7
 ifdef CONFIG_VIRTFS
 DOCS+=fsdev/virtfs-proxy-helper.1
 endif
@@ -265,6 +267,7 @@ qemu-ga$(EXESUF): QEMU_CFLAGS += -I qga/qapi-generated
 gen-out-type = $(subst .,-,$(suffix $@))
 
 qapi-py = $(SRC_PATH)/scripts/qapi.py $(SRC_PATH)/scripts/ordereddict.py
+qapi-py += $(SRC_PATH)/scripts/qapi2texi.py
 
 qga/qapi-generated/qga-qapi-types.c qga/qapi-generated/qga-qapi-types.h :\
 $(SRC_PATH)/qga/qapi-schema.json $(SRC_PATH)/scripts/qapi-types.py $(qapi-py)
@@ -393,6 +396,11 @@ distclean: clean
rm -f qemu-doc.vr
rm -f config.log
rm -f linux-headers/asm
+   rm -f qemu-ga-qapi.texi qemu-qapi.texi
+   rm -f docs/qemu-qmp-ref.7 docs/qemu-ga-ref.7
+   rm -f docs/qemu-qmp-ref.txt docs/qemu-ga-ref.txt
+   rm -f docs/qemu-qmp-ref.pdf docs/qemu-ga-ref.pdf
+   rm -f docs/qemu-qmp-ref.html docs/qemu-ga-ref.html
for d in $(TARGET_DIRS); do \
rm -rf $$d || exit 1 ; \
 done
@@ -430,9 +438,13 @@ install-doc: $(DOCS)
$(INSTALL_DIR) "$(DESTDIR)$(qemu_docdir)"
$(INSTALL_DATA) qemu-doc.html "$(DESTDIR)$(qemu_docdir)"
$(INSTALL_DATA) qemu-doc.txt "$(DESTDIR)$(qemu_docdir)"
+   $(INSTALL_DATA) docs/qemu-qmp-ref.html "$(DESTDIR)$(qemu_docdir)"
+   $(INSTALL_DATA) docs/qemu-qmp-ref.txt "$(DESTDIR)$(qemu_docdir)"
 ifdef CONFIG_POSIX
$(INSTALL_DIR) "$(DESTDIR)$(mandir)/man1"
$(INSTALL_DATA) qemu.1 "$(DESTDIR)$(mandir)/man1"
+   $(INSTALL_DIR) "$(DESTDIR)$(mandir)/man7"
+   $(INSTALL_DATA) docs/qemu-qmp-ref.7 "$(DESTDIR)$(mandir)/man7"
 ifneq ($(TOOLS),)
$(INSTALL_DATA) qemu-img.1 "$(DESTDIR)$(mandir)/man1"
$(INSTALL_DIR) "$(DESTDIR)$(mandir)/man8"
@@ -440,6 +452,9 @@ ifneq ($(TOOLS),)
 endif
 ifneq (,$(findstring qemu-ga,$(TOOLS)))
$(INSTALL_DATA) qemu-ga.8 "$(DESTDIR)$(mandir)/man8"
+   $(INSTALL_DATA) docs/qemu-ga-ref.html "$(DESTDIR)$(qemu_docdir)"
+   $(INSTALL_DATA) docs/qemu-ga-ref.txt "$(DESTDIR)$(qemu_docdir)"
+   $(INSTALL_DATA) docs/qemu-ga-ref.7 "$(DESTDIR)$(mandir)/man7"
 endif
 endif
 ifdef CONFIG_VIRTFS
@@ -527,9 +542,10 @@ ui/console-gl.o: $(SRC_PATH)/ui/console-gl.c \
ui/shader/texture-blit-vert.h ui/shader/texture-blit-frag.h
 
 # documentation
-MAKEINFO=makeinfo
+MAKEINFO=makeinfo -D 'VERSION $(VERSION)'
 MAKEINFOFLAGS=--no-split --number-sections
-TEXIFLAG=$(if $(V),,--quiet)
+TEXIFLAG=$(if $(V),,--quiet) --command='@set VERSION $(VERSION)'
+
 %.html: %.texi
$(call quiet-command,LC_ALL=C $(MAKEINFO) $(MAKEINFOFLAGS) --no-headers 
\
--html $< -o $@,"GEN","$@")
@@ -542,7 +558,7 @@ TEXIFLAG=$(if $(V),,--quiet)
--plaintext $< -o $@,"GEN","$@")
 
 %.pdf: %.texi
-   $(call quiet-command,texi2pdf $(TEXIFLAG) -I . $<,"GEN","$@")
+   $(call quiet-command,texi2pdf $(TEXIFLAG) -I $(SRC_PATH) -I . $< -o 
$@,"GEN","$@")
 
 qemu-options.texi: $(SRC_PATH)/qemu-options.hx $(SRC_PATH)/scripts/hxtool
$(call quiet-command,sh $(SRC_PATH)/scripts/hxtool -t < $< > 
$@,"GEN","$@")
@@ -556,6 +572,12 @@ qemu-monitor-info.texi: $(SRC_PATH)/hmp-commands-info.hx 
$(SRC_PATH)/scripts/hxt
 qemu-img-cmds.texi: $(SRC_PATH)/qemu-img-cmds.hx $(SRC_PATH)/scripts/hxtool
$(call quiet-command,sh $(SRC_PATH)/scripts/hxtool -t < $< > 
$@,"GEN","$@")
 
+qemu-qapi.texi: $(qapi-modules) $(qapi-py)
+   $(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi2texi.py $< > 
$@,"GEN" "$@")
+
+qemu-ga-qapi.texi: $(SRC_PATH)/qga/qapi-schema.json $(qapi-py)
+   $(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi2texi.py $< > 
$@,"GEN","$@")
+
 qemu.1: qemu-doc.texi qemu-options.texi qemu-monitor.texi 
qemu-monitor-info.texi
 qemu.1: qemu-option-trace.texi

Re: [Qemu-devel] [PATCH] libvhost-user: Start VQs on SET_VRING_CALL

[Marc-André Lureau]

Hi

- Original Message -
> Currently, VQs are started as soon as a SET_VRING_KICK is received. That
> is too early in the VQ setup process, as the backend might not yet have

I think we may want to reconsider queue_set_started(), move it elsewhere, since 
kick/call fds aren't mandatory to process the rings.

> a callfd to notify in case it received a kick and fully processed the
> request/command. This patch only starts a VQ when a SET_VRING_CALL is
> received.

I don't like that much, as soon as the kick fd is received, it should start 
polling it imho. callfd is optional, it may have one and not the other.

Perhaps it's best for now to delay the callfd notification with a flag until it 
is received?


> Signed-off-by: Felipe Franciosi 
> ---
>  contrib/libvhost-user/libvhost-user.c | 26 +-
>  1 file changed, 13 insertions(+), 13 deletions(-)
> 
> diff --git a/contrib/libvhost-user/libvhost-user.c
> b/contrib/libvhost-user/libvhost-user.c
> index af4faad..a46ef90 100644
> --- a/contrib/libvhost-user/libvhost-user.c
> +++ b/contrib/libvhost-user/libvhost-user.c
> @@ -607,19 +607,6 @@ vu_set_vring_kick_exec(VuDev *dev, VhostUserMsg *vmsg)
>  DPRINT("Got kick_fd: %d for vq: %d\n", vmsg->fds[0], index);
>  }
>  
> -dev->vq[index].started = true;
> -if (dev->iface->queue_set_started) {
> -dev->iface->queue_set_started(dev, index, true);
> -}
> -
> -if (dev->vq[index].kick_fd != -1 && dev->vq[index].handler) {
> -dev->set_watch(dev, dev->vq[index].kick_fd, VU_WATCH_IN,
> -   vu_kick_cb, (void *)(long)index);
> -
> -DPRINT("Waiting for kicks on fd: %d for vq: %d\n",
> -   dev->vq[index].kick_fd, index);
> -}
> -
>  return false;
>  }
>  
> @@ -661,6 +648,19 @@ vu_set_vring_call_exec(VuDev *dev, VhostUserMsg *vmsg)
>  
>  DPRINT("Got call_fd: %d for vq: %d\n", vmsg->fds[0], index);
>  
> +dev->vq[index].started = true;
> +if (dev->iface->queue_set_started) {
> +dev->iface->queue_set_started(dev, index, true);
> +}
> +
> +if (dev->vq[index].kick_fd != -1 && dev->vq[index].handler) {
> +dev->set_watch(dev, dev->vq[index].kick_fd, VU_WATCH_IN,
> +   vu_kick_cb, (void *)(long)index);
> +
> +DPRINT("Waiting for kicks on fd: %d for vq: %d\n",
> +   dev->vq[index].kick_fd, index);
> +}
> +
>  return false;
>  }
>  
> --
> 1.9.4
> 
> 

[Qemu-devel] [PULL 0/3] Common TLB reset changes

[Alex Bennée]

The following changes since commit b6c08970bc989bfddcf830684ea7a96b7a4d62a7:

  Merge remote-tracking branch 
'remotes/bkoppelmann/tags/pull-tricore-2017-01-11-2' into staging (2017-01-12 
18:29:49 +)

are available in the git repository at:

  https://github.com/stsquad/qemu.git tags/pull-tcg-common-tlb-reset-20170113-r1

for you to fetch changes up to d10eb08f5d8389c814b554d01aa2882ac58221bf:

  cputlb: drop flush_global flag from tlb_flush (2017-01-13 14:24:37 +)


This is the same as the v3 posted except a re-base and a few extra signoffs


Alex Bennée (3):
  qom/cpu: move tlb_flush to cpu_common_reset
  cpu_common_reset: wrap TCG specific code in tcg_enabled()
  cputlb: drop flush_global flag from tlb_flush

 cputlb.c   | 21 ++---
 exec.c |  4 ++--
 hw/sh4/sh7750.c|  2 +-
 include/exec/exec-all.h| 14 ++
 qom/cpu.c  | 10 --
 target/alpha/cpu.c |  2 +-
 target/alpha/sys_helper.c  |  2 +-
 target/arm/cpu.c   |  5 ++---
 target/arm/cpu.h   |  5 -
 target/arm/helper.c| 26 +-
 target/cris/cpu.c  |  3 +--
 target/cris/cpu.h  |  9 ++---
 target/i386/cpu.c  |  2 --
 target/i386/cpu.h  |  6 --
 target/i386/fpu_helper.c   |  2 +-
 target/i386/helper.c   |  8 
 target/i386/machine.c  |  2 +-
 target/i386/misc_helper.c  |  2 +-
 target/i386/svm_helper.c   |  2 +-
 target/lm32/cpu.c  |  3 +--
 target/lm32/cpu.h  |  3 +++
 target/m68k/cpu.c  |  3 +--
 target/m68k/cpu.h  |  3 +++
 target/microblaze/cpu.c|  3 +--
 target/microblaze/cpu.h|  3 +++
 target/microblaze/mmu.c|  2 +-
 target/mips/cpu.c  |  3 +--
 target/mips/cpu.h  |  5 -
 target/mips/helper.c   |  6 +++---
 target/mips/op_helper.c|  8 
 target/moxie/cpu.c |  4 +---
 target/moxie/cpu.h |  3 +++
 target/openrisc/cpu.c  |  9 +
 target/openrisc/cpu.h  |  3 +++
 target/openrisc/interrupt.c|  2 +-
 target/openrisc/interrupt_helper.c |  2 +-
 target/openrisc/sys_helper.c   |  2 +-
 target/ppc/helper_regs.h   |  4 ++--
 target/ppc/misc_helper.c   |  4 ++--
 target/ppc/mmu_helper.c| 32 
 target/ppc/translate_init.c|  3 ---
 target/s390x/cpu.c |  7 ++-
 target/s390x/cpu.h |  5 +++--
 target/s390x/gdbstub.c |  2 +-
 target/s390x/mem_helper.c  |  8 
 target/sh4/cpu.c   |  3 +--
 target/sh4/cpu.h   |  3 +++
 target/sh4/helper.c|  2 +-
 target/sparc/cpu.c |  3 +--
 target/sparc/cpu.h |  3 +++
 target/sparc/ldst_helper.c | 12 ++--
 target/tilegx/cpu.c|  3 +--
 target/tilegx/cpu.h|  3 +++
 target/tricore/cpu.c   |  2 --
 target/unicore32/cpu.c |  2 +-
 target/unicore32/helper.c  |  2 +-
 target/xtensa/op_helper.c  |  2 +-
 57 files changed, 151 insertions(+), 148 deletions(-)


--
2.11.0

[Qemu-devel] [PATCH v8 20/21] build-sys: add txt documentation rules

[Marc-André Lureau]

Build plain text documentation, and install it.

Signed-off-by: Marc-André Lureau 
Reviewed-by: Markus Armbruster 
---
 .gitignore |  1 +
 Makefile   | 12 +---
 2 files changed, 10 insertions(+), 3 deletions(-)

diff --git a/.gitignore b/.gitignore
index 3338bdc876..568c4bf9d3 100644
--- a/.gitignore
+++ b/.gitignore
@@ -40,6 +40,7 @@
 /qmp-marshal.c
 /qemu-doc.html
 /qemu-doc.info
+/qemu-doc.txt
 /qemu-img
 /qemu-nbd
 /qemu-options.def
diff --git a/Makefile b/Makefile
index 4a4a34ed8b..3280da201b 100644
--- a/Makefile
+++ b/Makefile
@@ -81,7 +81,7 @@ Makefile: ;
 configure: ;
 
 .PHONY: all clean cscope distclean html info install install-doc \
-   pdf recurse-all speed test dist msi FORCE
+   pdf txt recurse-all speed test dist msi FORCE
 
 $(call set-vpath, $(SRC_PATH))
 
@@ -90,7 +90,7 @@ LIBS+=-lz $(LIBS_TOOLS)
 HELPERS-$(CONFIG_LINUX) = qemu-bridge-helper$(EXESUF)
 
 ifdef BUILD_DOCS
-DOCS=qemu-doc.html qemu.1 qemu-img.1 qemu-nbd.8 qemu-ga.8
+DOCS=qemu-doc.html qemu-doc.txt qemu.1 qemu-img.1 qemu-nbd.8 qemu-ga.8
 ifdef CONFIG_VIRTFS
 DOCS+=fsdev/virtfs-proxy-helper.1
 endif
@@ -429,6 +429,7 @@ endif
 install-doc: $(DOCS)
$(INSTALL_DIR) "$(DESTDIR)$(qemu_docdir)"
$(INSTALL_DATA) qemu-doc.html "$(DESTDIR)$(qemu_docdir)"
+   $(INSTALL_DATA) qemu-doc.txt "$(DESTDIR)$(qemu_docdir)"
 ifdef CONFIG_POSIX
$(INSTALL_DIR) "$(DESTDIR)$(mandir)/man1"
$(INSTALL_DATA) qemu.1 "$(DESTDIR)$(mandir)/man1"
@@ -536,6 +537,10 @@ TEXIFLAG=$(if $(V),,--quiet)
 %.info: %.texi
$(call quiet-command,$(MAKEINFO) $(MAKEINFOFLAGS) $< -o $@,"GEN","$@")
 
+%.txt: %.texi
+   $(call quiet-command,LC_ALL=C $(MAKEINFO) $(MAKEINFOFLAGS) --no-headers 
\
+   --plaintext $< -o $@,"GEN","$@")
+
 %.pdf: %.texi
$(call quiet-command,texi2pdf $(TEXIFLAG) -I . $<,"GEN","$@")
 
@@ -561,6 +566,7 @@ qemu-ga.8: qemu-ga.texi
 html: qemu-doc.html
 info: qemu-doc.info
 pdf: qemu-doc.pdf
+txt: qemu-doc.txt
 
 qemu-doc.html qemu-doc.info qemu-doc.pdf: \
qemu-img.texi qemu-nbd.texi qemu-options.texi qemu-option-trace.texi \
@@ -659,7 +665,7 @@ help:
@echo  '  docker  - Help about targets running tests inside 
Docker containers'
@echo  ''
@echo  'Documentation targets:'
-   @echo  '  html info pdf'
+   @echo  '  html info pdf txt'
@echo  '  - Build documentation in specified format'
@echo  ''
 ifdef CONFIG_WIN32
-- 
2.11.0

[Qemu-devel] [PATCH v8 16/21] docs: add qemu logo to pdf

[Marc-André Lureau]

Add a logo to texi2pdf output. Other formats (info/html) are left as
future improvements.

The PDF (needed by texi2pdf for vectorized images) was generated from
pc-bios/qemu_logo.svg like this:

inkscape --export-pdf=docs/qemu_logo.pdf pc-bios/qemu_logo.svg

Signed-off-by: Marc-André Lureau 
Reviewed-by: Markus Armbruster 
---
 docs/qemu-ga-ref.texi  |   4 
 docs/qemu-qmp-ref.texi |   4 
 docs/qemu_logo.pdf | Bin 0 -> 9117 bytes
 3 files changed, 8 insertions(+)
 create mode 100644 docs/qemu_logo.pdf

diff --git a/docs/qemu-ga-ref.texi b/docs/qemu-ga-ref.texi
index b8898027dc..87cc8d01a5 100644
--- a/docs/qemu-ga-ref.texi
+++ b/docs/qemu-ga-ref.texi
@@ -6,6 +6,10 @@
 
 @settitle QEMU Guest Agent Protocol Reference
 
+@iftex
+@center @image{docs/qemu_logo}
+@end iftex
+
 @copying
 This is the QEMU Guest Agent Protocol reference manual.
 
diff --git a/docs/qemu-qmp-ref.texi b/docs/qemu-qmp-ref.texi
index efb3370a24..818e52573b 100644
--- a/docs/qemu-qmp-ref.texi
+++ b/docs/qemu-qmp-ref.texi
@@ -6,6 +6,10 @@
 
 @settitle QEMU QMP Reference Manual
 
+@iftex
+@center @image{docs/qemu_logo}
+@end iftex
+
 @copying
 This is the QEMU QMP reference manual.
 
diff --git a/docs/qemu_logo.pdf b/docs/qemu_logo.pdf
new file mode 100644
index 
..294cb7dec50de73c786925671300fb0abdf9d641
GIT binary patch
literal 9117
zcmd5?2|QF^`%elfQWRNlwoqesGqx7H>`T^?!C)*ini-
zlq^4#U7MYvi2uDarj$PK|9#)j`*}a_T%U38bDnd~^E~G{-*e9Qj*O|64h*S?<(4TL
z(0|LewJPiPq*yuGb;qMxXf2Yrrc>;>)5rxe(}mw!_IBZ
zBWn5r%A#k#rGWQNo&)#KDAvR8x0=tc_AK7FZkNj5=6K3XBu`00v0e4uQlqTXa{EK^pC6URTA>12kjw8+SN
zFL;DT>nnVNJ9Uq*i!-qH#ln^D>ACfRJ_j`?HMf@?@jNTxflU55Zj$}?+0HD(+jedv
zsK^#=)qCfsdYUJ$wa)c?4(M-vH?`g0kBxo1J)9j`)_>X;B@~wx>R4K+FqOBp`!xB!
zYKk3}T*uAA-Y$KeWcRpI5+{BuFY#omOBB^5Vs^djKT6SmH9b!`dlmmaNI~J^
zjhaIL<6~y+qD@B~K30`}Of;%hE1bESUHuGxc_QqQg4HY2nclzcN$iJ3-3#cc0hp{c
zy<3R6;a%}Tr^$xj_YZd`?t32RE2i$=QC7C+f&?{Ht6y(`IGs*^kgG1CxPJQO;rX-K
zqbvRQ-n}mIZI9i->s9SXlwd0Z-)STpY7g`9DQu+Vcix|ylVCSlqiqSJKCGLROGORi
z;~vdL4}>0hReysnT{AIsU5Z=eLjIN9LeG(#e>oLhxsg(nPCXa|sBAxwcvHo$7qz`L
zzF`mF+!cqM>Bn3jyHyX}d#uXcUE)Nqdckppx$%
z9tC}RYDDtvV8xkqOBHjhW=1uKc7gM`fjT2i$CVqd1WajXh4ZG~Fo(5ux(Q^;S5-bK
zNn(qtkEuUX-|kNw(tZw%jR}6;;H-4&+FQHLdyx1w=eKl{Gfbac=L;(6f^#T+X4P{x
zJ7+{w-KxEw_B^GEhSDO1JcG6ww|tVHemwOBU*2~3q|GI{2V0JEcDO6K2y1TlS=)bT
zm4ZRAs2E$v+a1DTIf1}U!UcH_r~GQ~*Xhxw2k=?qU%+d
zWyVR<-GG(a%7(!Nx~Jp(R@-a#zSo$qHs&{TUt5Q-Bp-Rj71(Y~|NF3nPiyLJY{r<7
zLJ-VRcOx7A(8*O5oK9QWKAdsVnHSE{W;bnEpo~2feFA#Woy2-otTl%VunR
zabGKE;)441crKUVgF|_yMZ-IdbKUsd>LN#Pv1NbX8!sFwc86>9Wn5eS%?3cQi7=TRk_t$f(16+
zkiH~e|5X?VQW|D_n){%QJ;IzVeIHHxq8p$WRMQTz#w5PAfrQC5`{lSU<
z>6lPkrP2zCAiSbEX2r(67JB%1LK}BpIbpaBqkA^imU^iw?Q9BcqY3`+b9N?O^|$=f
z0{EWBM}|AJjSfbIE0n0e!>XiOJUabGQZ^w)CuRoUSJR+Hl=4^LuJgNosy^7V`Mac~
zYk9rn#C1t1oASf}}pySTiteAizyd9I}=WkLm%Ik*bG3v83fR#y|+MoLdCIf-}Zj{fE?c#$4q#{>7Cm>U~E
z-*tR$a{S%Y_y&_ZC)f;gtWI(h$u5hy3*JG94Z$?@9|US78m|l~K#!;w$ZGXD1
z1s#NMKurx2fFC#k0X_h5%{>4TfIAbwZxGZM%VW3yJK;
zXoKdC1+Y*#sOgRaaE#kUJsk>}zNA-I7y^a>l$SIH>Ma_LM1W$}GGrOxLj>Ta1UDkX
z15*N(2sVUdRfHQ6T}T8Cia%fviVy$_k5t6qFbE|8udIks#vrgb-~gkhCTIXyl?FhI
z9CJm}rBZx-mKeqCiy>u2zyg3M5}Dx>l4U<)(M1GH
zfc#Bh3?P?w@pJ6KHT$3M;zxgs<`wbEI1E~8vCCh3fQEW8=RlvKBw9=ch9l_3STO=b
z@uE;Id$
zDEnEm2yzT~V?y1|oy=0^RAt_?@^@KvnmZB=;wqFFKhbeNp_S#J*7*jpCRU^Csfw7Wn=w7)(x^I>7@@0WBWQ%hYh53O#
z5U8`zhiR`7f#%5=4$x3TUvCnbLL<=`ISbbXkjz5>Xuu+TEwBGe=;0PnbKvl?W)fpv
zVKfgdGmM)(`WApI!HdQ?L$FX0S}|C5u_h$LzJH90V0wVpBFsauwN+9>V>)+~P
zN%JfO{

[Qemu-devel] [PATCH v8 19/21] build-sys: use a generic TEXI2MAN rule

[Marc-André Lureau]

The recipe for making a man page from .texi is duplicated several
times over.  Capture it in suitable pattern rules instead.

Signed-off-by: Marc-André Lureau 
Reviewed-by: Markus Armbruster 
---
 Makefile  | 24 
 rules.mak | 10 ++
 2 files changed, 10 insertions(+), 24 deletions(-)

diff --git a/Makefile b/Makefile
index 1d6a31deb3..4a4a34ed8b 100644
--- a/Makefile
+++ b/Makefile
@@ -552,35 +552,11 @@ qemu-img-cmds.texi: $(SRC_PATH)/qemu-img-cmds.hx 
$(SRC_PATH)/scripts/hxtool
$(call quiet-command,sh $(SRC_PATH)/scripts/hxtool -t < $< > 
$@,"GEN","$@")
 
 qemu.1: qemu-doc.texi qemu-options.texi qemu-monitor.texi 
qemu-monitor-info.texi
-   $(call quiet-command, \
- perl -Ww -- $(SRC_PATH)/scripts/texi2pod.pl $< qemu.pod && \
- $(POD2MAN) --section=1 --center=" " --release=" " qemu.pod > $@, \
- "GEN","$@")
 qemu.1: qemu-option-trace.texi
-
 qemu-img.1: qemu-img.texi qemu-option-trace.texi qemu-img-cmds.texi
-   $(call quiet-command, \
- perl -Ww -- $(SRC_PATH)/scripts/texi2pod.pl $< qemu-img.pod && \
- $(POD2MAN) --section=1 --center=" " --release=" " qemu-img.pod > $@, \
- "GEN","$@")
-
 fsdev/virtfs-proxy-helper.1: fsdev/virtfs-proxy-helper.texi
-   $(call quiet-command, \
- perl -Ww -- $(SRC_PATH)/scripts/texi2pod.pl $< 
fsdev/virtfs-proxy-helper.pod && \
- $(POD2MAN) --section=1 --center=" " --release=" " 
fsdev/virtfs-proxy-helper.pod > $@, \
- "GEN","$@")
-
 qemu-nbd.8: qemu-nbd.texi qemu-option-trace.texi
-   $(call quiet-command, \
- perl -Ww -- $(SRC_PATH)/scripts/texi2pod.pl $< qemu-nbd.pod && \
- $(POD2MAN) --section=8 --center=" " --release=" " qemu-nbd.pod > $@, \
- "GEN","$@")
-
 qemu-ga.8: qemu-ga.texi
-   $(call quiet-command, \
- perl -Ww -- $(SRC_PATH)/scripts/texi2pod.pl $< qemu-ga.pod && \
- $(POD2MAN) --section=8 --center=" " --release=" " qemu-ga.pod > $@, \
- "GEN","$@")
 
 html: qemu-doc.html
 info: qemu-doc.info
diff --git a/rules.mak b/rules.mak
index ce9e7e6ffe..a7b6c0b020 100644
--- a/rules.mak
+++ b/rules.mak
@@ -363,3 +363,13 @@ define unnest-vars
 $(eval -include $(patsubst %.o,%.d,$(patsubst %.mo,%.d,$($v
 $(eval $v := $(filter-out %/,$($v
 endef
+
+TEXI2MAN = $(call quiet-command, \
+   perl -Ww -- $(SRC_PATH)/scripts/texi2pod.pl $< $@.pod && \
+   $(POD2MAN) --section=$(subst .,,$(suffix $@)) --center=" " --release=" 
" $@.pod > $@, \
+   "GEN","$@")
+
+%.1:
+   $(call TEXI2MAN)
+%.8:
+   $(call TEXI2MAN)
-- 
2.11.0

[Qemu-devel] [PATCH v8 11/21] qapi: rework qapi Exception

[Marc-André Lureau]

Use a base class QAPIError, and QAPIParseError for parser errors and
QAPISemError for semantic errors, suggested by Markus Armbruster.

Signed-off-by: Marc-André Lureau 
Reviewed-by: Markus Armbruster 
---
 scripts/qapi.py | 334 ++--
 1 file changed, 156 insertions(+), 178 deletions(-)

diff --git a/scripts/qapi.py b/scripts/qapi.py
index 21bc32fda3..1483ec09f5 100644
--- a/scripts/qapi.py
+++ b/scripts/qapi.py
@@ -91,35 +91,38 @@ def error_path(parent):
 return res
 
 
-class QAPISchemaError(Exception):
-def __init__(self, schema, msg):
+class QAPIError(Exception):
+def __init__(self, fname, line, col, incl_info, msg):
 Exception.__init__(self)
-self.fname = schema.fname
+self.fname = fname
+self.line = line
+self.col = col
+self.info = incl_info
 self.msg = msg
-self.col = 1
-self.line = schema.line
-for ch in schema.src[schema.line_pos:schema.pos]:
-if ch == '\t':
-self.col = (self.col + 7) % 8 + 1
-else:
-self.col += 1
-self.info = schema.incl_info
 
 def __str__(self):
-return error_path(self.info) + \
-"%s:%d:%d: %s" % (self.fname, self.line, self.col, self.msg)
+loc = "%s:%d" % (self.fname, self.line)
+if self.col is not None:
+loc += ":%s" % self.col
+return error_path(self.info) + "%s: %s" % (loc, self.msg)
 
 
-class QAPIExprError(Exception):
-def __init__(self, expr_info, msg):
-Exception.__init__(self)
-assert expr_info
-self.info = expr_info
-self.msg = msg
+class QAPIParseError(QAPIError):
+def __init__(self, parser, msg):
+col = 1
+for ch in parser.src[parser.line_pos:parser.pos]:
+if ch == '\t':
+col = (col + 7) % 8 + 1
+else:
+col += 1
+QAPIError.__init__(self, parser.fname, parser.line, col,
+   parser.incl_info, msg)
 
-def __str__(self):
-return error_path(self.info['parent']) + \
-"%s:%d: %s" % (self.info['file'], self.info['line'], self.msg)
+
+class QAPISemError(QAPIError):
+def __init__(self, info, msg):
+QAPIError.__init__(self, info['file'], info['line'], None,
+   info['parent'], msg)
 
 
 class QAPISchemaParser(object):
@@ -140,25 +143,24 @@ class QAPISchemaParser(object):
 self.accept()
 
 while self.tok is not None:
-expr_info = {'file': fname, 'line': self.line,
- 'parent': self.incl_info}
+info = {'file': fname, 'line': self.line,
+'parent': self.incl_info}
 expr = self.get_expr(False)
 if isinstance(expr, dict) and "include" in expr:
 if len(expr) != 1:
-raise QAPIExprError(expr_info,
-"Invalid 'include' directive")
+raise QAPISemError(info, "Invalid 'include' directive")
 include = expr["include"]
 if not isinstance(include, str):
-raise QAPIExprError(expr_info,
-"Value of 'include' must be a string")
+raise QAPISemError(info,
+   "Value of 'include' must be a string")
 incl_abs_fname = os.path.join(os.path.dirname(abs_fname),
   include)
 # catch inclusion cycle
-inf = expr_info
+inf = info
 while inf:
 if incl_abs_fname == os.path.abspath(inf['file']):
-raise QAPIExprError(expr_info, "Inclusion loop for %s"
-% include)
+raise QAPISemError(info, "Inclusion loop for %s"
+   % include)
 inf = inf['parent']
 # skip multiple include of the same file
 if incl_abs_fname in previously_included:
@@ -166,14 +168,13 @@ class QAPISchemaParser(object):
 try:
 fobj = open(incl_abs_fname, 'r')
 except IOError as e:
-raise QAPIExprError(expr_info,
-'%s: %s' % (e.strerror, include))
+raise QAPISemError(info, '%s: %s' % (e.strerror, include))
 exprs_include = QAPISchemaParser(fobj, previously_included,
- expr_info)
+ info)
 self.exprs.extend(exprs_include.exprs)
 else:
 expr_elem = {'expr': expr,
- 

[Qemu-devel] [PATCH v8 18/21] build-sys: remove dvi doc generation

[Marc-André Lureau]

There is no clear reason to have rules to generate dvi format
documentation, pdf is generally better supported nowadays.

Signed-off-by: Marc-André Lureau 
Reviewed-by: Markus Armbruster 
---
 .gitignore |  1 -
 Makefile   | 12 
 2 files changed, 4 insertions(+), 9 deletions(-)

diff --git a/.gitignore b/.gitignore
index e43c3044dc..3338bdc876 100644
--- a/.gitignore
+++ b/.gitignore
@@ -60,7 +60,6 @@
 *.a
 *.aux
 *.cp
-*.dvi
 *.exe
 *.msi
 *.dll
diff --git a/Makefile b/Makefile
index 82ee20150a..1d6a31deb3 100644
--- a/Makefile
+++ b/Makefile
@@ -80,7 +80,7 @@ GENERATED_HEADERS += module_block.h
 Makefile: ;
 configure: ;
 
-.PHONY: all clean cscope distclean dvi html info install install-doc \
+.PHONY: all clean cscope distclean html info install install-doc \
pdf recurse-all speed test dist msi FORCE
 
 $(call set-vpath, $(SRC_PATH))
@@ -387,7 +387,7 @@ distclean: clean
rm -f config-all-devices.mak config-all-disas.mak config.status
rm -f po/*.mo tests/qemu-iotests/common.env
rm -f roms/seabios/config.mak roms/vgabios/config.mak
-   rm -f qemu-doc.info qemu-doc.aux qemu-doc.cp qemu-doc.cps qemu-doc.dvi
+   rm -f qemu-doc.info qemu-doc.aux qemu-doc.cp qemu-doc.cps
rm -f qemu-doc.fn qemu-doc.fns qemu-doc.info qemu-doc.ky qemu-doc.kys
rm -f qemu-doc.log qemu-doc.pdf qemu-doc.pg qemu-doc.toc qemu-doc.tp
rm -f qemu-doc.vr
@@ -529,9 +529,6 @@ ui/console-gl.o: $(SRC_PATH)/ui/console-gl.c \
 MAKEINFO=makeinfo
 MAKEINFOFLAGS=--no-split --number-sections
 TEXIFLAG=$(if $(V),,--quiet)
-%.dvi: %.texi
-   $(call quiet-command,texi2dvi $(TEXIFLAG) -I . $<,"GEN","$@")
-
 %.html: %.texi
$(call quiet-command,LC_ALL=C $(MAKEINFO) $(MAKEINFOFLAGS) --no-headers 
\
--html $< -o $@,"GEN","$@")
@@ -585,12 +582,11 @@ qemu-ga.8: qemu-ga.texi
  $(POD2MAN) --section=8 --center=" " --release=" " qemu-ga.pod > $@, \
  "GEN","$@")
 
-dvi: qemu-doc.dvi
 html: qemu-doc.html
 info: qemu-doc.info
 pdf: qemu-doc.pdf
 
-qemu-doc.dvi qemu-doc.html qemu-doc.info qemu-doc.pdf: \
+qemu-doc.html qemu-doc.info qemu-doc.pdf: \
qemu-img.texi qemu-nbd.texi qemu-options.texi qemu-option-trace.texi \
qemu-monitor.texi qemu-img-cmds.texi qemu-ga.texi \
qemu-monitor-info.texi
@@ -687,7 +683,7 @@ help:
@echo  '  docker  - Help about targets running tests inside 
Docker containers'
@echo  ''
@echo  'Documentation targets:'
-   @echo  '  dvi html info pdf'
+   @echo  '  html info pdf'
@echo  '  - Build documentation in specified format'
@echo  ''
 ifdef CONFIG_WIN32
-- 
2.11.0