Re: [Qemu-devel] [PATCH 14/29] target-sparc: use direct address translation in hyperprivileged mode

2016-10-10 Thread Richard Henderson

On 10/01/2016 05:05 AM, Artyom Tarasenko wrote:

Implement translation behavior described in the chapter 13.7 of
"UltraSPARC T1™ Supplement to the UltraSPARC Architecture 2005".

Please note that QEMU doesn't impelement Real->Physical address
translation. The "Real Address" is always the "Physical Address".

Signed-off-by: Artyom Tarasenko 
---
 target-sparc/mmu_helper.c | 8 +---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/target-sparc/mmu_helper.c b/target-sparc/mmu_helper.c
index 32b629f..bef63f8 100644
--- a/target-sparc/mmu_helper.c
+++ b/target-sparc/mmu_helper.c
@@ -498,7 +498,8 @@ static int get_physical_address_data(CPUSPARCState *env,
 int is_user = (mmu_idx == MMU_USER_IDX ||
mmu_idx == MMU_USER_SECONDARY_IDX);

-if ((env->lsu & DMMU_E) == 0) { /* DMMU disabled */
+if ((env->lsu & DMMU_E) == 0 || cpu_hypervisor_mode(env)) {
+/* direct translation VA -> PA */
 *physical = ultrasparc_truncate_physical(address);
 *prot = PAGE_READ | PAGE_WRITE;
 return 0;
@@ -617,8 +618,9 @@ static int get_physical_address_code(CPUSPARCState *env,
 int is_user = (mmu_idx == MMU_USER_IDX ||
mmu_idx == MMU_USER_SECONDARY_IDX);

-if ((env->lsu & IMMU_E) == 0 || (env->pstate & PS_RED) != 0) {
-/* IMMU disabled */
+if (((env->lsu & IMMU_E) == 0) || (env->pstate & PS_RED) != 0
+|| cpu_hypervisor_mode(env)) {
+/* direct translation VA -> PA */


If we let this depend on my MMU_PHYS_IDX patch, this becomes



>From b1cb710579877de99fdbc03f30f43622057bb4ca Mon Sep 17 00:00:00 2001
From: Artyom Tarasenko 
Date: Sat, 1 Oct 2016 12:05:18 +0200
Subject: [PATCH 14/31] target-sparc: use direct address translation in
 hyperprivileged mode
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Implement translation behavior described in the chapter 13.7 of
"UltraSPARC T1™ Supplement to the UltraSPARC Architecture 2005".

Please note that QEMU doesn't impelement Real->Physical address
translation. The "Real Address" is always the "Physical Address".

[rth: Therefore we can merge MMU_HYPV_IDX and MMMU_PHYS_IDX.
This brings NB_MMU_MODES <= 6, which avoids the TLBs being
reduced in size, which will increase performance.]

Signed-off-by: Artyom Tarasenko 
Message-Id: <1475316333-9776-15-git-send-email-atar4q...@gmail.com>
Signed-off-by: Ricahrd Henderson 
---
 target-sparc/cpu.h   | 20 
 target-sparc/translate.c | 18 +-
 2 files changed, 29 insertions(+), 9 deletions(-)

diff --git a/target-sparc/cpu.h b/target-sparc/cpu.h
index b0df95f..4309df3 100644
--- a/target-sparc/cpu.h
+++ b/target-sparc/cpu.h
@@ -223,7 +223,7 @@ enum {
 #if !defined(TARGET_SPARC64)
 #define NB_MMU_MODES 3
 #else
-#define NB_MMU_MODES 7
+#define NB_MMU_MODES 6
 typedef struct trap_state {
 uint64_t tpc;
 uint64_t tnpc;
@@ -665,8 +665,7 @@ int cpu_sparc_signal_handler(int host_signum, void *pinfo, void *puc);
 #define MMU_KERNEL_IDX 2
 #define MMU_KERNEL_SECONDARY_IDX 3
 #define MMU_NUCLEUS_IDX 4
-#define MMU_HYPV_IDX   5
-#define MMU_PHYS_IDX   6
+#define MMU_PHYS_IDX   5
 #else
 #define MMU_USER_IDX   0
 #define MMU_KERNEL_IDX 1
@@ -688,6 +687,11 @@ static inline int cpu_supervisor_mode(CPUSPARCState *env1)
 {
 return env1->pstate & PS_PRIV;
 }
+#else
+static inline int cpu_supervisor_mode(CPUSPARCState *env1)
+{
+return env1->psrs;
+}
 #endif
 
 static inline int cpu_mmu_index(CPUSPARCState *env, bool ifetch)
@@ -707,7 +711,7 @@ static inline int cpu_mmu_index(CPUSPARCState *env, bool ifetch)
 : (env->lsu & DMMU_E) == 0) {
 return MMU_PHYS_IDX;
 } else if (cpu_hypervisor_mode(env)) {
-return MMU_HYPV_IDX;
+return MMU_PHYS_IDX;
 } else if (env->tl > 0) {
 return MMU_NUCLEUS_IDX;
 } else if (cpu_supervisor_mode(env)) {
@@ -755,6 +759,8 @@ trap_state* cpu_tsptr(CPUSPARCState* env);
 #define TB_FLAG_MMU_MASK 7
 #define TB_FLAG_FPU_ENABLED  (1 << 4)
 #define TB_FLAG_AM_ENABLED   (1 << 5)
+#define TB_FLAG_SUPER(1 << 6)
+#define TB_FLAG_HYPER(1 << 7)
 #define TB_FLAG_ASI_SHIFT24
 
 static inline void cpu_get_tb_cpu_state(CPUSPARCState *env, target_ulong *pc,
@@ -764,7 +770,13 @@ static inline void cpu_get_tb_cpu_state(CPUSPARCState *env, target_ulong *pc,
 *pc = env->pc;
 *cs_base = env->npc;
 flags = cpu_mmu_index(env, false);
+if (cpu_supervisor_mode(env)) {
+flags |= TB_FLAG_SUPER;
+}
 #ifdef TARGET_SPARC64
+if (cpu_hypervisor_mode(env)) {
+flags |= TB_FLAG_HYPER;
+}
 if (env->pstate & PS_AM) {
 flags |= TB_FLAG_AM_ENABLED;
 }
diff --git a/target-sparc/translate.c b/target-sparc/translate.c
index 5d7929b..cdec52b 100644
--- a/target-sparc/translate.c
+++ b/target-sparc/translate.c
@@ -72,9 +72,13 @@ typedef struct 

Re: [Qemu-devel] [PATCH 03/29] target-sparc: add UA2005 TTE bit #defines

2016-10-10 Thread Richard Henderson

On 10/10/2016 04:45 PM, Artyom Tarasenko wrote:

Hmm.  Would it make more sense to reorg these as

  TTE_US1_*
  TTE_UA2005_*

with some duplication for the bits that are shared?
As is, it's pretty hard to tell which actually change...


All of them :-)
I'm not sure about renaming: the US1 format is still used in T1 on the read
access.

On the other hand, it's not used in T2. And then again we don't have the T2
emulation yet.


Oh my.  Different on T2 as well?

I wonder if it would make sense to have different functions with which to fill 
in the CPUClass hooks (or invent new SPARCCPUClass hooks as necessary) for the 
major entry points.


E.g. sparc_cpu_handle_mmu_fault or get_physical_address could be hooked, so 
that the choice of how to handle the tlb miss is chosen at startup time, and 
not during each fault.  One can arrange subroutines as necessary to share code 
between the alternate routines, such as when T1 needs to use parts of US1.


Similarly for out-of-line ASI handling, which is already beyond messy, with 
handling for all cpus thrown in the same switch statement.



r~



[Qemu-devel] [PATCH v1 1/2] block/replication: prefect the logic to acquire 'top_id'

2016-10-10 Thread Changlong Xie
Only g_strdup(top_id) if 'top_id' is not NULL, although there
is no memory leak here

Signed-off-by: Changlong Xie 
---
 block/replication.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/block/replication.c b/block/replication.c
index 3bd1cf1..5b432d9 100644
--- a/block/replication.c
+++ b/block/replication.c
@@ -104,11 +104,11 @@ static int replication_open(BlockDriverState *bs, QDict 
*options,
 } else if (!strcmp(mode, "secondary")) {
 s->mode = REPLICATION_MODE_SECONDARY;
 top_id = qemu_opt_get(opts, REPLICATION_TOP_ID);
-s->top_id = g_strdup(top_id);
-if (!s->top_id) {
+if (!top_id) {
 error_setg(_err, "Missing the option top-id");
 goto fail;
 }
+s->top_id = g_strdup(top_id);
 } else {
 error_setg(_err,
"The option mode's value should be primary or secondary");
-- 
1.9.3






[Qemu-devel] [PATCH v1 0/2] block/replication fixes

2016-10-10 Thread Changlong Xie
Changlong Xie (2):
  block/replication: prefect the logic to acquire 'top_id'
  block/replication: Clarify 'top-id' parameter usage

 block/replication.c  | 9 +++--
 qapi/block-core.json | 3 ++-
 2 files changed, 9 insertions(+), 3 deletions(-)

-- 
1.9.3






[Qemu-devel] [PATCH v1 2/2] block/replication: Clarify 'top-id' parameter usage

2016-10-10 Thread Changlong Xie
Replication driver only support 'top-id' parameter in secondary side,
and it must not be supplied in primary side

Signed-off-by: Changlong Xie 
---
 block/replication.c  | 5 +
 qapi/block-core.json | 3 ++-
 2 files changed, 7 insertions(+), 1 deletion(-)

diff --git a/block/replication.c b/block/replication.c
index 5b432d9..1e8284b 100644
--- a/block/replication.c
+++ b/block/replication.c
@@ -101,6 +101,11 @@ static int replication_open(BlockDriverState *bs, QDict 
*options,
 
 if (!strcmp(mode, "primary")) {
 s->mode = REPLICATION_MODE_PRIMARY;
+top_id = qemu_opt_get(opts, REPLICATION_TOP_ID);
+if (top_id) {
+error_setg(_err, "The primary side do not support option 
top-id");
+goto fail;
+}
 } else if (!strcmp(mode, "secondary")) {
 s->mode = REPLICATION_MODE_SECONDARY;
 top_id = qemu_opt_get(opts, REPLICATION_TOP_ID);
diff --git a/qapi/block-core.json b/qapi/block-core.json
index 4badb97..ec92df4 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -2197,7 +2197,8 @@
 # @mode: the replication mode
 #
 # @top-id: #optional In secondary mode, node name or device ID of the root
-#  node who owns the replication node chain. Ignored in primary mode.
+#  node who owns the replication node chain. Must not be given in
+#  primary mode.
 #
 # Since: 2.8
 ##
-- 
1.9.3






[Qemu-devel] [PATCH] filter-dump: add missing "["

2016-10-10 Thread Changlong Xie
Signed-off-by: Changlong Xie 
---
 qemu-options.hx | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/qemu-options.hx b/qemu-options.hx
index b1fbdb0..c209b53 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -3902,7 +3902,7 @@ colo secondary:
 -object filter-redirector,id=f2,netdev=hn0,queue=rx,outdev=red1
 -object filter-rewriter,id=rew0,netdev=hn0,queue=all
 
-@item -object 
filter-dump,id=@var{id},netdev=@var{dev},file=@var{filename}][,maxlen=@var{len}]
+@item -object 
filter-dump,id=@var{id},netdev=@var{dev}[,file=@var{filename}][,maxlen=@var{len}]
 
 Dump the network traffic on netdev @var{dev} to the file specified by
 @var{filename}. At most @var{len} bytes (64k by default) per packet are stored.
-- 
1.9.3






Re: [Qemu-devel] [PATCH v3 8/8] vmxnet3: remove unnecessary internal msix state flag

2016-10-10 Thread Dmitry Fleytman

> On 11 Oct 2016, at 6:35, Cao jin  wrote:
> 
> 
> 
>> On 10/06/2016 05:39 PM, Dmitry Fleytman wrote:
>> 
> 
>> 
>> Hello,
>> 
>> Yes, from what I see, this call is wrong and leads to
>> reference leaks on device unload at migration target.
>> It should be removed.
>> 
>> Best regards,
>> Dmitry
>> 
> 
> Hi,Dmitry
> 
>From what I saw, current code will call msix_vector_use twice on migration 
> target, which is wrong. I am not quite follow you on "reference leak", are 
> you saying the same thing as me? if not, could you help to explain more?

Hello,

Yes, I meant exactly this :)

msix_vector_use() increments vector reference counter each time it is called,
so device holds two references to used vectors after migration.

The second reference will be leaked because there is no second call to 
msix_vector_unuse() on unload.

Best Regards,
Dmitry

> 
> -- 
> Yours Sincerely,
> 
> Cao jin
> 
> 



Re: [Qemu-devel] [PATCH v2] qtest: ask endianness of the target in qtest_init()

2016-10-10 Thread David Gibson
On Tue, Oct 11, 2016 at 12:24:29PM +1100, David Gibson wrote:
> On Mon, Oct 10, 2016 at 03:10:33PM +0100, Peter Maydell wrote:
> > On 10 October 2016 at 14:39, David Gibson  
> > wrote:
> > > In the overwhelming majority of cases the endianness of the device is
> > > known independent of the guest CPU and board.
> > 
> > We don't seem to model things that way:
> > 
> > $ git grep '.endianness = DEVICE_NATIVE' |wc -l
> > 341
> > $ git grep '.endianness = DEVICE_BIG' |wc -l
> > 18
> > $ git grep '.endianness = DEVICE_LITTLE' |wc -l
> > 172
> 
> Sigh.  So, most of these are themselves mistakes.
> 
> A lot of these are target specific devices that should be tagged with
> their target's (notional) endianness, rather than NATIVE.  That
> covers:

A couple of points of clarification here.

When I say the "device" endianness is known, that's a little sloppy.
I mean that the endianness of each specific register (or DMA field)
has a known endianness.  The device as a whole does not have well
defined endianness except insofar as one device will generally use the
same endianness for all its >8bit registers and/or DMA fields.

There are a few exceptions of course.  virtio-balloon (pre 1.0) on a
notionally BE platform is a horrible example: its PCI config space is
LE, the generic part of the virtio config space is BE ("native") and
the balloon specific part is LE again (due to a screwup in the spec).

Intermediate bridges in the system (on or off chip) won't affect this,
*as long as* they preserve byte address invariance.  If they *don't*
preserve byte address invariance, then:
1) Shoot your HW designer
2) Grudgingly work around with special cases in your tests


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


signature.asc
Description: PGP signature


Re: [Qemu-devel] [RFC 1/4] spapr_pci: Delegate placement of PCI host bridges to machine type

2016-10-10 Thread David Gibson
On Fri, Oct 07, 2016 at 04:10:08PM +1100, David Gibson wrote:
> On Fri, Oct 07, 2016 at 02:57:43PM +1100, Alexey Kardashevskiy wrote:
> > On 06/10/16 14:03, David Gibson wrote:
> > > The 'spapr-pci-host-bridge' represents the virtual PCI host bridge (PHB)
> > > for a PAPR guest.  Unlike on x86, it's routine on Power (both bare metal
> > > and PAPR guests) to have numerous independent PHBs, each controlling a
> > > separate PCI domain.
> > > 
> > > There are two ways of configuring the spapr-pci-host-bridge device: first
> > > it can be done fully manually, specifying the locations and sizes of all
> > > the IO windows.  This gives the most control, but is very awkward with 6
> > > mandatory parameters.  Alternatively just an "index" can be specified
> > > which essentially selects from an array of predefined PHB locations.
> > > The PHB at index 0 is automatically created as the default PHB.
> > > 
> > > The current set of default locations causes some problems for guests with
> > > large RAM (> 1 TiB) or PCI devices with very large BARs (e.g. big nVidia
> > > GPGPU cards via VFIO).  Obviously, for migration we can only change the
> > > locations on a new machine type, however.
> > > 
> > > This is awkward, because the placement is currently decided within the
> > > spapr-pci-host-bridge code, so it breaks abstraction to look inside the
> > > machine type version.
> > > 
> > > So, this patch delegates the "default mode" PHB placement from the
> > > spapr-pci-host-bridge device back to the machine type via a public method
> > > in sPAPRMachineClass.  It's still a bit ugly, but it's about the best we
> > > can do.
> > > 
> > > For now, this just changes where the calculation is done.  It doesn't
> > > change the actual location of the host bridges, or any other behaviour.
> > > 
> > > Signed-off-by: David Gibson 
> > > ---
> > >  hw/ppc/spapr.c  | 34 ++
> > >  hw/ppc/spapr_pci.c  | 22 --
> > >  include/hw/pci-host/spapr.h | 11 +--
> > >  include/hw/ppc/spapr.h  |  4 
> > >  4 files changed, 47 insertions(+), 24 deletions(-)
> > > 
> > > diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> > > index 03e3803..f6e9c2a 100644
> > > --- a/hw/ppc/spapr.c
> > > +++ b/hw/ppc/spapr.c
> > > @@ -2370,6 +2370,39 @@ static HotpluggableCPUList 
> > > *spapr_query_hotpluggable_cpus(MachineState *machine)
> > >  return head;
> > >  }
> > >  
> > > +static void spapr_phb_placement(sPAPRMachineState *spapr, uint32_t index,
> > > +uint64_t *buid, hwaddr *pio, hwaddr 
> > > *pio_size,
> > > +hwaddr *mmio, hwaddr *mmio_size,
> > > +unsigned n_dma, uint32_t *liobns, Error 
> > > **errp)
> > > +{
> > > +const uint64_t base_buid = 0x8002000ULL;
> > > +const hwaddr phb0_base = 0x100ULL; /* 1 TiB */
> > > +const hwaddr phb_spacing = 0x10ULL; /* 64 GiB */
> > > +const hwaddr mmio_offset = 0xa000; /* 2 GiB + 512 MiB */
> > > +const hwaddr pio_offset = 0x8000; /* 2 GiB */
> > > +const uint32_t max_index = 255;
> > > +
> > > +hwaddr phb_base;
> > > +int i;
> > > +
> > > +if (index > max_index) {
> > > +error_setg(errp, "\"index\" for PAPR PHB is too large (max %u)",
> > > +   max_index);
> > > +return;
> > > +}
> > > +
> > > +*buid = base_buid + index;
> > > +for (i = 0; i < n_dma; ++i) {
> > > +liobns[i] = SPAPR_PCI_LIOBN(index, i);
> > > +}
> > > +
> > > +phb_base = phb0_base + index * phb_spacing;
> > > +*pio = phb_base + pio_offset;
> > > +*pio_size = SPAPR_PCI_IO_WIN_SIZE;
> > > +*mmio = phb_base + mmio_offset;
> > > +*mmio_size = SPAPR_PCI_MMIO_WIN_SIZE;
> > > +}
> > > +
> > >  static void spapr_machine_class_init(ObjectClass *oc, void *data)
> > >  {
> > >  MachineClass *mc = MACHINE_CLASS(oc);
> > > @@ -2406,6 +2439,7 @@ static void spapr_machine_class_init(ObjectClass 
> > > *oc, void *data)
> > >  mc->query_hotpluggable_cpus = spapr_query_hotpluggable_cpus;
> > >  fwc->get_dev_path = spapr_get_fw_dev_path;
> > >  nc->nmi_monitor_handler = spapr_nmi;
> > > +smc->phb_placement = spapr_phb_placement;
> > >  }
> > >  
> > >  static const TypeInfo spapr_machine_info = {
> > > diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c
> > > index 4f00865..c0fc964 100644
> > > --- a/hw/ppc/spapr_pci.c
> > > +++ b/hw/ppc/spapr_pci.c
> > > @@ -1311,7 +1311,8 @@ static void spapr_phb_realize(DeviceState *dev, 
> > > Error **errp)
> > >  sphb->ddw_enabled ? SPAPR_PCI_DMA_MAX_WINDOWS : 1;
> > >  
> > >  if (sphb->index != (uint32_t)-1) {
> > > -hwaddr windows_base;
> > > +sPAPRMachineClass *smc = SPAPR_MACHINE_GET_CLASS(spapr);
> > > +Error *local_err = NULL;
> > >  
> > >  if ((sphb->buid != (uint64_t)-1) || (sphb->dma_liobn[0] != 

Re: [Qemu-devel] [PATCH v8 2/6] vfio: VFIO based driver for Mediated devices

2016-10-10 Thread Alex Williamson
On Tue, 11 Oct 2016 01:58:33 +0530
Kirti Wankhede  wrote:

> vfio_mdev driver registers with mdev core driver.
> MDEV core driver creates mediated device and calls probe routine of
> vfio_mdev driver for each device.
> Probe routine of vfio_mdev driver adds mediated device to VFIO core module
> 
> This driver forms a shim layer that pass through VFIO devices operations
> to vendor driver for mediated devices.
> 
> Signed-off-by: Kirti Wankhede 
> Signed-off-by: Neo Jia 
> Change-Id: I583f4734752971d3d112324d69e2508c88f359ec
> ---
>  drivers/vfio/mdev/Kconfig   |   6 ++
>  drivers/vfio/mdev/Makefile  |   1 +
>  drivers/vfio/mdev/vfio_mdev.c   | 171 
> 
>  drivers/vfio/pci/vfio_pci_private.h |   6 +-
>  4 files changed, 181 insertions(+), 3 deletions(-)
>  create mode 100644 drivers/vfio/mdev/vfio_mdev.c

Looking pretty good so far, a few preliminary comments below.  Thanks,

Alex

> 
> diff --git a/drivers/vfio/mdev/Kconfig b/drivers/vfio/mdev/Kconfig
> index 23d5b9d08a5c..e1b23697261d 100644
> --- a/drivers/vfio/mdev/Kconfig
> +++ b/drivers/vfio/mdev/Kconfig
> @@ -9,4 +9,10 @@ config VFIO_MDEV
>  
>  If you don't know what do here, say N.
>  
> +config VFIO_MDEV_DEVICE
> +tristate "VFIO support for Mediated devices"
> +depends on VFIO && VFIO_MDEV
> +default n
> +help
> +VFIO based driver for mediated devices.
>  
> diff --git a/drivers/vfio/mdev/Makefile b/drivers/vfio/mdev/Makefile
> index 56a75e689582..e5087ed83a34 100644
> --- a/drivers/vfio/mdev/Makefile
> +++ b/drivers/vfio/mdev/Makefile
> @@ -2,4 +2,5 @@
>  mdev-y := mdev_core.o mdev_sysfs.o mdev_driver.o
>  
>  obj-$(CONFIG_VFIO_MDEV) += mdev.o
> +obj-$(CONFIG_VFIO_MDEV_DEVICE) += vfio_mdev.o
>  
> diff --git a/drivers/vfio/mdev/vfio_mdev.c b/drivers/vfio/mdev/vfio_mdev.c
> new file mode 100644
> index ..1efc3f309510
> --- /dev/null
> +++ b/drivers/vfio/mdev/vfio_mdev.c
> @@ -0,0 +1,171 @@
> +/*
> + * VFIO based driver for Mediated device
> + *
> + * Copyright (c) 2016, NVIDIA CORPORATION. All rights reserved.
> + * Author: Neo Jia 
> + *  Kirti Wankhede 
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + */
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +#include "mdev_private.h"
> +
> +#define DRIVER_VERSION  "0.1"
> +#define DRIVER_AUTHOR   "NVIDIA Corporation"
> +#define DRIVER_DESC "VFIO based driver for Mediated device"
> +
> +struct vfio_mdev {
> + struct iommu_group *group;

a) this is never used, b) it's redundant, there are other ways to it.

> + struct mdev_device *mdev;

Which leaves us with just *mdev, so do we even need a struct vfio_mdev?

> +};
> +
> +static int vfio_mdev_open(void *device_data)
> +{
> + struct vfio_mdev *vmdev = device_data;
> + struct parent_device *parent = vmdev->mdev->parent;
> + int ret;
> +
> + if (unlikely(!parent->ops->open))
> + return -EINVAL;
> +
> + if (!try_module_get(THIS_MODULE))
> + return -ENODEV;
> +
> + ret = parent->ops->open(vmdev->mdev);
> + if (ret)
> + module_put(THIS_MODULE);
> +
> + return ret;
> +}
> +
> +static void vfio_mdev_release(void *device_data)
> +{
> + struct vfio_mdev *vmdev = device_data;
> + struct parent_device *parent = vmdev->mdev->parent;
> +
> + if (parent->ops->release)
> + parent->ops->release(vmdev->mdev);
> +
> + module_put(THIS_MODULE);
> +}
> +
> +static long vfio_mdev_unlocked_ioctl(void *device_data,
> +  unsigned int cmd, unsigned long arg)
> +{
> + struct vfio_mdev *vmdev = device_data;
> + struct parent_device *parent = vmdev->mdev->parent;
> +
> + if (unlikely(!parent->ops->ioctl))
> + return -EINVAL;
> +
> + return parent->ops->ioctl(vmdev->mdev, cmd, arg);
> +}
> +
> +static ssize_t vfio_mdev_read(void *device_data, char __user *buf,
> +   size_t count, loff_t *ppos)
> +{
> + struct vfio_mdev *vmdev = device_data;
> + struct parent_device *parent = vmdev->mdev->parent;
> +
> + if (unlikely(!parent->ops->read))
> + return -EINVAL;
> +
> + return parent->ops->read(vmdev->mdev, buf, count, *ppos);
> +}
> +
> +static ssize_t vfio_mdev_write(void *device_data, const char __user *buf,
> +size_t count, loff_t *ppos)
> +{
> + struct vfio_mdev *vmdev = device_data;
> + struct parent_device *parent = vmdev->mdev->parent;
> +
> + if (unlikely(!parent->ops->write))
> + return -EINVAL;
> +
> + return parent->ops->write(vmdev->mdev, (char *)buf, count, *ppos);


Re: [Qemu-devel] [PATCH v8 1/6] vfio: Mediated device Core driver

2016-10-10 Thread Alex Williamson
On Tue, 11 Oct 2016 01:58:32 +0530
Kirti Wankhede  wrote:
> ---
>  drivers/vfio/Kconfig |   1 +
>  drivers/vfio/Makefile|   1 +
>  drivers/vfio/mdev/Kconfig|  12 ++
>  drivers/vfio/mdev/Makefile   |   5 +
>  drivers/vfio/mdev/mdev_core.c| 363 
> +++
>  drivers/vfio/mdev/mdev_driver.c  | 131 ++
>  drivers/vfio/mdev/mdev_private.h |  41 +
>  drivers/vfio/mdev/mdev_sysfs.c   | 295 +++
>  include/linux/mdev.h | 178 +++
>  9 files changed, 1027 insertions(+)
>  create mode 100644 drivers/vfio/mdev/Kconfig
>  create mode 100644 drivers/vfio/mdev/Makefile
>  create mode 100644 drivers/vfio/mdev/mdev_core.c
>  create mode 100644 drivers/vfio/mdev/mdev_driver.c
>  create mode 100644 drivers/vfio/mdev/mdev_private.h
>  create mode 100644 drivers/vfio/mdev/mdev_sysfs.c
>  create mode 100644 include/linux/mdev.h


Overall this is heading in a good direction.  What kernel is this
series against?  I could only apply it to v4.7, yet some of the
dependencies claimed in the cover letter are only in v4.8.  linux-next
or v4.8 are both good baselines right now, as we move to v4.9-rc
releases, linux-next probably becomes a better target.

A few initial comments below, I'll likely have more as I wrap my head
around it.  Thanks,

Alex

> diff --git a/drivers/vfio/mdev/mdev_core.c b/drivers/vfio/mdev/mdev_core.c
> new file mode 100644
> index ..019c196e62d5
> --- /dev/null
> +++ b/drivers/vfio/mdev/mdev_core.c
> @@ -0,0 +1,363 @@
> +/*
> + * Mediated device Core Driver
> + *
> + * Copyright (c) 2016, NVIDIA CORPORATION. All rights reserved.
> + * Author: Neo Jia 
> + *  Kirti Wankhede 
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + */
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 

I don't see any vfio interfaces used here, is vfio.h necessary?

> +#include 
> +#include 
> +#include 
> +
> +#include "mdev_private.h"
> +
[snip]
> +int mdev_device_create(struct kobject *kobj, struct device *dev, uuid_le 
> uuid)
> +{
> + int ret;
> + struct mdev_device *mdev;
> + struct parent_device *parent;
> + struct mdev_type *type = to_mdev_type(kobj);
> +
> + parent = mdev_get_parent(type->parent);
> + if (!parent)
> + return -EINVAL;
> +
> + /* Check for duplicate */
> + mdev = __find_mdev_device(parent, uuid);
> + if (mdev) {
> + ret = -EEXIST;
> + goto create_err;
> + }
> +
> + mdev = kzalloc(sizeof(*mdev), GFP_KERNEL);
> + if (!mdev) {
> + ret = -ENOMEM;
> + goto create_err;
> + }
> +
> + memcpy(>uuid, , sizeof(uuid_le));
> + mdev->parent = parent;
> + kref_init(>ref);
> +
> + mdev->dev.parent  = dev;
> + mdev->dev.bus = _bus_type;
> + mdev->dev.release = mdev_device_release;
> + dev_set_name(>dev, "%pUl", uuid.b);
> +
> + ret = device_register(>dev);
> + if (ret) {
> + put_device(>dev);
> + goto create_err;
> + }
> +
> + ret = mdev_device_create_ops(kobj, mdev);
> + if (ret)
> + goto create_failed;
> +
> + ret = mdev_create_sysfs_files(>dev, type);
> + if (ret) {
> + mdev_device_remove_ops(mdev, true);
> + goto create_failed;
> + }
> +
> + mdev->type_kobj = kobj;
> + dev_dbg(>dev, "MDEV: created\n");
> +
> + return ret;
> +
> +create_failed:
> + device_unregister(>dev);
> +
> +create_err:
> + mdev_put_parent(parent);
> + return ret;
> +}
> +
> +int mdev_device_remove(struct device *dev, void *data)

I understand this void* is to be able to call this from
device_for_each_child(), but let's create a callback wrapper for that
path that converts data to bool, we really don't want to use void args
except where necessary.  IOW,

static int mdev_device_remove_cb(struct device *dev, void *data)
{
mdev_device_remove(dev, data ? *(bool *)data : true);
}

int mdev_device_remove(struct device *dev, bool force_remove)
> +{
> + struct mdev_device *mdev;
> + struct parent_device *parent;
> + struct mdev_type *type;
> + bool force_remove = true;
> + int ret = 0;
> +
> + if (!dev_is_mdev(dev))
> + return 0;
> +
> + mdev = to_mdev_device(dev);
> + parent = mdev->parent;
> + type = to_mdev_type(mdev->type_kobj);
> +
> + if (data)
> + force_remove = *(bool *)data;
> +
> + ret = mdev_device_remove_ops(mdev, force_remove);
> + if (ret)
> + return ret;
> +
> + mdev_remove_sysfs_files(dev, type);
> + device_unregister(dev);
> + mdev_put_parent(parent);
> + return ret;

Re: [Qemu-devel] [PATCH] colo-proxy: fix memory leak

2016-10-10 Thread Zhang Chen



On 10/11/2016 04:23 AM, Eric Blake wrote:

On 10/10/2016 04:46 AM, Zhang Chen wrote:

Fix memory leak in colo-compare.c and filter-rewriter.c
Report by Coverity.

Signed-off-by: Zhang Chen 
---
  net/colo-compare.c| 4 ++--
  net/filter-rewriter.c | 9 +
  2 files changed, 7 insertions(+), 6 deletions(-)

diff --git a/net/colo-compare.c b/net/colo-compare.c
index 22b1da1..47e7ebe 100644
--- a/net/colo-compare.c
+++ b/net/colo-compare.c
@@ -235,8 +235,8 @@ static int colo_packet_compare_tcp(Packet *spkt, Packet 
*ppkt)
  fprintf(stderr, "Secondary len = %d\n", spkt->size);
  qemu_hexdump((char *)spkt->data, stderr, "colo-compare", spkt->size);
  
-g_free(sdebug);

-g_free(ddebug);
+free(sdebug);
+free(ddebug);

Uggh. We should fix the original allocations of sdebug and ddebug to use
g_strdup_printf() rather than strdup(), so that we are using a
consistent allocator throughout.  Or even improve this code to use
traces instead of fprintf.


OK~~ I get your point.
I will send patch V2 later.

Thanks
Zhang Chen






--
Thanks
zhangchen






Re: [Qemu-devel] [PATCH v3 8/8] vmxnet3: remove unnecessary internal msix state flag

2016-10-10 Thread Cao jin



On 10/06/2016 05:39 PM, Dmitry Fleytman wrote:






Hello,

Yes, from what I see, this call is wrong and leads to
reference leaks on device unload at migration target.
It should be removed.

Best regards,
Dmitry



Hi,Dmitry

From what I saw, current code will call msix_vector_use twice on 
migration target, which is wrong. I am not quite follow you on 
"reference leak", are you saying the same thing as me? if not, could you 
help to explain more?


--
Yours Sincerely,

Cao jin





Re: [Qemu-devel] [PATCH v5 08/17] vfio: Pass an Error object to vfio_connect_container

2016-10-10 Thread David Gibson
On Mon, Oct 10, 2016 at 02:36:28PM +0200, Markus Armbruster wrote:
> Auger Eric  writes:
> 
> > Hi,
> >
> > On 10/10/2016 07:34, David Gibson wrote:
> >> On Fri, Oct 07, 2016 at 09:36:09AM +0200, Auger Eric wrote:
> >>> Hi,
> >>>
> >>> On 07/10/2016 09:01, Markus Armbruster wrote:
>  Eric Auger  writes:
> 
> > The error is currently simply reported in vfio_get_group. Don't
> > bother too much with the prefix which will be handled at upper level,
> > later on.
> >
> > Also return an error value in case container->error is not 0 and
> > the container is teared down.
> 
>  "torn down", I think.
> >>>
> >>> Sure. I had a wrong feeling when writing this ...
> 
>  Is this a bug fix?  See also below.
> 
> > On vfio_spapr_remove_window failure, we also report an error whereas
> > it was silent before.
> >
> > Signed-off-by: Eric Auger 
> > Reviewed-by: Markus Armbruster 
> >
> > ---
> >
> > v4 -> v5:
> > - set ret to container->error
> > - mention error report on vfio_spapr_remove_window failure in the commit
> >   message
> > ---
> >  hw/vfio/common.c | 40 +---
> >  1 file changed, 25 insertions(+), 15 deletions(-)
> >
> > diff --git a/hw/vfio/common.c b/hw/vfio/common.c
> > index 29188a1..85a7759 100644
> > --- a/hw/vfio/common.c
> > +++ b/hw/vfio/common.c
> [...]
> > @@ -1008,7 +1010,9 @@ static int vfio_connect_container(VFIOGroup 
> > *group, AddressSpace *as)
>container = g_malloc0(sizeof(*container));
>container->space = space;
>container->fd = fd;
>if (ioctl(fd, VFIO_CHECK_EXTENSION, VFIO_TYPE1_IOMMU) ||
>ioctl(fd, VFIO_CHECK_EXTENSION, VFIO_TYPE1v2_IOMMU)) {
> [...]
>} else if (ioctl(fd, VFIO_CHECK_EXTENSION, VFIO_SPAPR_TCE_IOMMU) ||
>   ioctl(fd, VFIO_CHECK_EXTENSION, 
> VFIO_SPAPR_TCE_v2_IOMMU)) {
>struct vfio_iommu_spapr_tce_info info;
>bool v2 = !!ioctl(fd, VFIO_CHECK_EXTENSION, 
> VFIO_SPAPR_TCE_v2_IOMMU);
> 
>ret = ioctl(group->fd, VFIO_GROUP_SET_CONTAINER, );
>if (ret) {
>error_setg_errno(errp, errno, "failed to set group 
> container");
>ret = -errno;
>goto free_container_exit;
>}
>container->iommu_type =
>v2 ? VFIO_SPAPR_TCE_v2_IOMMU : VFIO_SPAPR_TCE_IOMMU;
>ret = ioctl(fd, VFIO_SET_IOMMU, container->iommu_type);
>if (ret) {
>error_setg_errno(errp, errno, "failed to set iommu for 
> container");
>ret = -errno;
>goto free_container_exit;
>}
> 
>/*
> * The host kernel code implementing VFIO_IOMMU_DISABLE is 
> called
> * when container fd is closed so we do not call it explicitly
> * in this file.
> */
>if (!v2) {
>ret = ioctl(fd, VFIO_IOMMU_ENABLE);
>if (ret) {
>error_setg_errno(errp, errno, "failed to enable 
> container");
>ret = -errno;
>goto free_container_exit;
>}
>} else {
>container->prereg_listener = vfio_prereg_listener;
> 
>memory_listener_register(>prereg_listener,
> >   _space_memory);
> >  if (container->error) {
> 
>  I tried to see where non-zero container->error comes from, but failed.
>  Can you help?
> >>>
> >>> Added Alexey in CC
> >>>
> >>> It is set in vfio_prereg_listener_region_add (spapr.c)
> >>> There is a comment there saying:
> >>> /*
> >>>  * On the initfn path, store the first error in the container so we
> >>>  * can gracefully fail.  Runtime, there's not much we can do other
> >>>  * than throw a hardware error.
> >>>  */
> >>> 1) by the way I should also s/initfn/realize now.
> >>> 2) by gracefully fail I understand the error should be properly
> >>> cascaded. Also when looking at the other vfio_memory_listener
> >>> registration below, ret is set to container->error.
> >>> 3) I could use error_setg_errno ...
> >>>
> >>> David, Alexey, could you confirm we should set the returned value to the
> >>> container->error below?
> >> 
> >> I think the right approach is to change container->error from an int
> >> to an Error *.  As now, we stash the first error from the listener in
> >> there.
> >> 
> >> realize() would check for a non-NULL error in the container after
> >> registering the listener, and if present, propagate it up to the
> >> caller.
> >> 
> >>>
> >>> Thanks
> >>>
> >>> Eric
> 

Re: [Qemu-devel] [PATCH v8 0/6] Add Mediated device support

2016-10-10 Thread Jike Song
On 10/11/2016 04:28 AM, Kirti Wankhede wrote:
> This series adds Mediated device support to Linux host kernel. Purpose
> of this series is to provide a common interface for mediated device
> management that can be used by different devices. This series introduces
> Mdev core module that creates and manages mediated devices, VFIO based
> driver for mediated devices that are created by mdev core module and
> update VFIO type1 IOMMU module to support pinning & unpinning for mediated
> devices.
> 
> This change uses uuid_le_to_bin() to parse UUID string and convert to bin.
> This requires following commits from linux master branch:
> * commit bc9dc9d5eec908806f1b15c9ec2253d44dcf7835 :
> lib/uuid.c: use correct offset in uuid parser
> * commit 2b1b0d66704a8cafe83be7114ec4c15ab3a314ad :
> lib/uuid.c: introduce a few more generic helpers
> 
> Requires below commits from linux master branch for mmap region fault
> handler that uses remap_pfn_range() to setup EPT properly.
> * commit add6a0cd1c5ba51b201e1361b05a5df817083618
> KVM: MMU: try to fix up page faults before giving up
> * commit 92176a8ede577d0ff78ab3298e06701f67ad5f51 :
> KVM: MMU: prepare to support mapping of VM_IO and VM_PFNMAP frames
> 
> What changed in v8?
> mdev-core:
> - Removed start/stop or online/offline interfaces.
> - Added open() and close() interfaces that should be used to commit
>   resources for mdev devices from vendor driver.
> - Removed supported_config callback function. Introduced sysfs interface
>   for 'mdev_supported_types' as discussed earlier. It is mandatory to
>   provide supported types by vendor driver.
> - Removed 'mdev_create' and 'mdev_destroy' sysfs files from device's
>   directory. Added 'create' file in each supported type group that vendor
>   driver would define. Added 'remove' file in mdev device directory to
>   destroy mdev device.
> 
> vfio_mdev:
> - Added ioctl() callback. All ioctls should be handled in vendor driver
> - Common functions for SET_IRQS and GET_REGION_INFO ioctls are added to
>   reduce code duplication in vendor drivers.
> - This forms a shim layer that pass through VFIO devices operations to
>   vendor driver for mediated devices.

Hi Kirti,

While having not looked yet at the v8 details, I would say that this is
definitely the right way to go, as I have been proposing for a quite long
while :)

--
Thanks,
Jike

> 
> vfio_iommu_type1:
> - Handled the case if all devices attached to the normal IOMMU API domain
>   go away and mdev device still exist in domain. Updated page accounting
>   for local domain.
> - Similarly if device is attached to normal IOMMU API domain, mappings are
>   establised and page accounting is updated accordingly.
> - Tested hot-plug and hot-unplug of vGPU and GPU pass through device with
>   Linux VM.
> 
> Documentation:
> - Updated vfio-mediated-device.txt with current interface.
> - Added sample driver that simulates serial port over PCI card for a VM.
>   This driver is added as an example for how to use mediated device
>   framework.
> - Moved updated document and example driver to 'vfio-mdev' directory in
>   Documentation.
> 
> 
> Kirti Wankhede (6):
>   vfio: Mediated device Core driver
>   vfio: VFIO based driver for Mediated devices
>   vfio iommu: Add support for mediated devices
>   docs: Add Documentation for Mediated devices
>   Add simple sample driver for mediated device framework
>   Add common functions for SET_IRQS and GET_REGION_INFO ioctls
> 
>  Documentation/vfio-mdev/Makefile |   14 +
>  Documentation/vfio-mdev/mtty.c   | 1353 
> ++
>  Documentation/vfio-mdev/vfio-mediated-device.txt |  282 +
>  drivers/vfio/Kconfig |1 +
>  drivers/vfio/Makefile|1 +
>  drivers/vfio/mdev/Kconfig|   18 +
>  drivers/vfio/mdev/Makefile   |6 +
>  drivers/vfio/mdev/mdev_core.c|  363 ++
>  drivers/vfio/mdev/mdev_driver.c  |  131 +++
>  drivers/vfio/mdev/mdev_private.h |   41 +
>  drivers/vfio/mdev/mdev_sysfs.c   |  295 +
>  drivers/vfio/mdev/vfio_mdev.c|  171 +++
>  drivers/vfio/pci/vfio_pci.c  |  103 +-
>  drivers/vfio/pci/vfio_pci_private.h  |6 +-
>  drivers/vfio/vfio.c  |  233 
>  drivers/vfio/vfio_iommu_type1.c  |  685 +--
>  include/linux/mdev.h |  178 +++
>  include/linux/vfio.h |   20 +-
>  18 files changed, 3743 insertions(+), 158 deletions(-)
>  create mode 100644 Documentation/vfio-mdev/Makefile
>  create mode 100644 Documentation/vfio-mdev/mtty.c
>  create mode 100644 Documentation/vfio-mdev/vfio-mediated-device.txt
>  create mode 100644 drivers/vfio/mdev/Kconfig
>  create mode 100644 drivers/vfio/mdev/Makefile
>  create 

Re: [Qemu-devel] [PATCH 03/29] target-sparc: add UA2005 TTE bit #defines

2016-10-10 Thread Artyom Tarasenko
10 окт. 2016 г. 23:22 пользователь "Richard Henderson" 
написал:
>
> On 10/01/2016 05:05 AM, Artyom Tarasenko wrote:
>>
>>  #define TTE_VALID_BIT   (1ULL << 63)
>>  #define TTE_NFO_BIT (1ULL << 60)
>> +#define TTE_NFO_BIT_UA2005  (1ULL << 62)
>>  #define TTE_USED_BIT(1ULL << 41)
>> +#define TTE_USED_BIT_UA2005 (1ULL << 47)
>>  #define TTE_LOCKED_BIT  (1ULL <<  6)
>> +#define TTE_LOCKED_BIT_UA2005 (1ULL <<  61)
>>  #define TTE_SIDEEFFECT_BIT  (1ULL <<  3)
>> +#define TTE_SIDEEFFECT_BIT_UA2005 (1ULL <<  11)
>>  #define TTE_PRIV_BIT(1ULL <<  2)
>> +#define TTE_PRIV_BIT_UA2005 (1ULL <<  8)
>>  #define TTE_W_OK_BIT(1ULL <<  1)
>> +#define TTE_W_OK_BIT_UA2005 (1ULL <<  6)
>>  #define TTE_GLOBAL_BIT  (1ULL <<  0)
>
>
> Hmm.  Would it make more sense to reorg these as
>
>   TTE_US1_*
>   TTE_UA2005_*
>
> with some duplication for the bits that are shared?
> As is, it's pretty hard to tell which actually change...

All of them :-)
I'm not sure about renaming: the US1 format is still used in T1 on the read
access.

On the other hand, it's not used in T2. And then again we don't have the T2
emulation yet.

Artyom


Re: [Qemu-devel] [PATCH v5 00/17] Convert VFIO-PCI to realize

2016-10-10 Thread no-reply
Hi,

Your series failed automatic build test. Please find the testing commands and
their output below. If you have docker installed, you can probably reproduce it
locally.

Message-id: 1475770058-20409-1-git-send-email-eric.au...@redhat.com
Subject: [Qemu-devel] [PATCH v5 00/17] Convert VFIO-PCI to realize
Type: series

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

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
Switched to a new branch 'test'
f5c0d95 vfio/pci: Handle host oversight
aacb337 vfio/pci: Remove vfio_populate_device returned value
eb998d2 vfio/pci: Remove vfio_msix_early_setup returned value
fd8b442 vfio/pci: Conversion to realize
0e1ec19 vfio/platform: Pass an error object to vfio_base_device_init
b98a6d9 vfio/platform: fix a wrong returned value in vfio_populate_device
64991c5 vfio/platform: Pass an error object to vfio_populate_device
de3e688 vfio: Pass an error object to vfio_get_device
e8119bf vfio: Pass an error object to vfio_get_group
311e128 vfio: Pass an Error object to vfio_connect_container
9a8ade4 vfio/pci: Pass an error object to vfio_pci_igd_opregion_init
225894a vfio/pci: Pass an error object to vfio_add_capabilities
1b8c9cf vfio/pci: Pass an error object to vfio_intx_enable
0d8c535 vfio/pci: Pass an error object to vfio_msix_early_setup
c3086ef vfio/pci: Pass an error object to vfio_populate_device
d6d6ce0 vfio/pci: Pass an error object to vfio_populate_vga
98d6388 vfio/pci: Use local error object in vfio_initfn

=== OUTPUT BEGIN ===
Submodule 'dtc' (git://git.qemu-project.org/dtc.git) registered for path 'dtc'
Cloning into 'dtc'...
Submodule path 'dtc': checked out '65cc4d2748a2c2e6f27f1cf39e07a5dbabd80ebf'
  BUILD   centos6
=== OUTPUT END ===

Abort: command timeout (>3600 seconds)


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

Re: [Qemu-devel] [PATCH v2] qtest: ask endianness of the target in qtest_init()

2016-10-10 Thread David Gibson
On Mon, Oct 10, 2016 at 03:10:33PM +0100, Peter Maydell wrote:
> On 10 October 2016 at 14:39, David Gibson  wrote:
> > In the overwhelming majority of cases the endianness of the device is
> > known independent of the guest CPU and board.
> 
> We don't seem to model things that way:
> 
> $ git grep '.endianness = DEVICE_NATIVE' |wc -l
> 341
> $ git grep '.endianness = DEVICE_BIG' |wc -l
> 18
> $ git grep '.endianness = DEVICE_LITTLE' |wc -l
> 172

Sigh.  So, most of these are themselves mistakes.

A lot of these are target specific devices that should be tagged with
their target's (notional) endianness, rather than NATIVE.  That
covers:

ARM
hw/arm   68
hw/*/bcm283*  8
hw/*/imx*13
hw/*/omap*   27
hw/*/pl* 10
hw/*/exynos*  8
hw/net/lan9118.c  2
hw/*/pxa2xx*  8
hw/gpio/zaurus.c  1
hw/*/versatile*   3
hw/*/allwinner*   3
hw/*/arm*15
hw/*/a9*  3
hw/char/cadence*  2
hw/*/digic*   2
hw/*/stm32f2xx*   5
hw/usb/tusb6010.c 1
hw/misc/mst_fpga.c1
hw/net/smc91c111.c1
hw/net/stellaris_enet.c   1
hw/misc/zynq* 2
hw/dma/xlnx_dpdma.c   1
hw/display/xlnx_dp.c  4
hw/ssi/xilinx_spips.c 1
hw/display/tc6393xb.c 1
hw/audio/marvell_88w8618.c1
hw/block/onenand.c1
---
TOTAL   193 


CRIS
hw/cris   2
hw/*/etraxfs* 3
---
TOTAL 5

LM32
hw/*/milkymist*  10
hw/*/lm32*2
---
TOTAL12 
 

M68K
hw/m68k   4
hw/*/mcf* 2
---
TOTAL 6

MICROBLAZE
hw/dma/xilinx_axidma.c1
hw/char/xilinx_uartlite.c 1
hw/intc/xilinx_intc.c 1
hw/net/xilinx_ethlite.c   1
hw/timer/xilinx_timer.c   1
hw/ssi/xilinx_spi.c   1
---
TOTAL 6

MIPS
hw/mips   8
hw/*/mips*5
hw/display/jazz_led.c 1
hw/dma/rc4030.c   2
hw/net/dp8393x.c  1
hw/pci-host/bonito.c  5
---
TOTAL22

OPENRISC
hw/openrisc   1

PPC
hw/ppc7
hw/char/escc.c1
hw/ide/macio.c1
hw/misc/macio/cuda.c  1
hw/misc/applesmc.c2
hw/net/fsl_etsec/etsec.c  1
---
TOTAL13

SH
hw/sh44
hw/*/sh_* 3
hw/display/sm501.c4

Re: [Qemu-devel] [PATCH v6 01/12] virtio-crypto: introduce virtio_crypto.h

2016-10-10 Thread Gonglei (Arei)

> -Original Message-
> From: Eric Blake [mailto:ebl...@redhat.com]
> Sent: Tuesday, October 11, 2016 3:58 AM
> Subject: Re: [PATCH v6 01/12] virtio-crypto: introduce virtio_crypto.h
> 
> On 10/10/2016 03:43 AM, Gonglei wrote:
> > Introduce the virtio_crypto.h which follows
> > virtio-crypto specification.
> >
> > Signed-off-by: Gonglei 
> > ---
> >  include/standard-headers/linux/virtio_crypto.h | 381
> +
> >  1 file changed, 381 insertions(+)
> >  create mode 100644 include/standard-headers/linux/virtio_crypto.h
> >
> > diff --git a/include/standard-headers/linux/virtio_crypto.h
> b/include/standard-headers/linux/virtio_crypto.h
> > new file mode 100644
> > index 000..a62d192
> > --- /dev/null
> > +++ b/include/standard-headers/linux/virtio_crypto.h
> > @@ -0,0 +1,381 @@
> > +#ifndef _VIRTIO_CRYPTO_H
> > +#define _VIRTIO_CRYPTO_H
> 
> Missing a copyright and license notice.  Is this header copied from
> Linux, or is it new to qemu?  At any rate, being explicit about your
> license is important (in qemu, files without a license are GPLv2+;
> whereas the Linux kernel tends to favor GPLv2-only).
> 
It should be copied from Linux like other virtio devices do.
I'll add copyright for this in the next version.

> > +
> > +#include "standard-headers/linux/types.h"
> > +#include "standard-headers/linux/virtio_config.h"
> > +#include "standard-headers/linux/virtio_types.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)
> 
> Technically over-parenthesized, but doesn't hurt.
> 
> > +
> > +#define VIRTIO_CRYPTO_OPCODE(service, op)   ((service << 8) | (op))
> 
> Under-parenthesized.  You need more () around service.
> 
So, the best definition is:

#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_OPCODE(service, op)   (((service) << 8) | (op))

Will fix. Thanks~

Regards,
-Gonglei


Re: [Qemu-devel] [PATCH v6 00/12] virtio-crypto: introduce framework and device emulation

2016-10-10 Thread Gonglei (Arei)

> -Original Message-
> From: Eric Blake [mailto:ebl...@redhat.com]
> Sent: Tuesday, October 11, 2016 3:53 AM
> Subject: Re: [PATCH v6 00/12] virtio-crypto: introduce framework and device
> emulation
> 
> On 10/10/2016 03:43 AM, Gonglei wrote:
> > The virtio crypto is a virtual crypto 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 handled by the real crypto accelerators finally.
> > The second queue is the control queue used to create or
> > destroy sessions for symmetric algorithms and control
> > some advanced features in the future. The virtio crypto
> > device provides the following crypto services: CIPHER,
> > MAC, HASH, AEAD etc.
> >
> > TODO:
> >  - add vhost-user as a high performance cryptodev backend.
> >  - more crypto services support.
> >  - mirgration support.
> >
> > Changes since v5:
> >  - rebase the patch 14 in v5, using the correct at the beginning of whole
> patch serials. [Eric]
> 
> s/serials/series/  ('series' is one of those weird English words that is
> the same for both singular and plural usage)
> 
I see, thanks. :)


Regards,
-Gonglei

> >  - perfect algorithm chain support in patch 12.
> >  - more friendly error handler in both controlq and dataq.
> >  - drop patch "virtio-crypto: emulate virtio crypto as a legacy device by
> default" because
> >   we shouldn't support transitional virtio devices any more. [Michael]
> >  - drop patch "virtio-crypto-test: add qtest case for virtio-crypto" because
> >   libqtest doesn't support virtio-1.0 device yet.
> >  - rebase the patch set based on Michael's pull request:
> > [PULL 00/33] virtio, pc: fixes and features
> >
> 
> --
> Eric Blake   eblake redhat com+1-919-301-3266
> Libvirt virtualization library http://libvirt.org



Re: [Qemu-devel] [PATCH v4] timer: a9gtimer: remove loop to auto-increment comparator

2016-10-10 Thread no-reply
Hi,

Your series failed automatic build test. Please find the testing commands and
their output below. If you have docker installed, you can probably reproduce it
locally.

Message-id: 1475903058-729-1-git-send-email-ppan...@redhat.com
Subject: [Qemu-devel] [PATCH v4] timer: a9gtimer: remove loop to auto-increment 
comparator
Type: series

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

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
Switched to a new branch 'test'
5ae77cf timer: a9gtimer: remove loop to auto-increment comparator

=== OUTPUT BEGIN ===
Submodule 'dtc' (git://git.qemu-project.org/dtc.git) registered for path 'dtc'
Cloning into 'dtc'...
Submodule path 'dtc': checked out '65cc4d2748a2c2e6f27f1cf39e07a5dbabd80ebf'
  BUILD   centos6
  ARCHIVE qemu.tgz
  ARCHIVE dtc.tgz
  COPYRUNNER
  RUN test-quick in centos6
Packages installed:
SDL-devel-1.2.14-7.el6_7.1.x86_64
ccache-3.1.6-2.el6.x86_64
epel-release-6-8.noarch
gcc-4.4.7-17.el6.x86_64
git-1.7.1-4.el6_7.1.x86_64
glib2-devel-2.28.8-5.el6.x86_64
libfdt-devel-1.4.0-1.el6.x86_64
make-3.81-23.el6.x86_64
package g++ is not installed
pixman-devel-0.32.8-1.el6.x86_64
tar-1.23-15.el6_8.x86_64
zlib-devel-1.2.3-29.el6.x86_64

Environment variables:
PACKAGES=libfdt-devel ccache tar git make gcc g++ zlib-devel 
glib2-devel SDL-devel pixman-devel epel-release
HOSTNAME=0f4212dfef99
TERM=xterm
MAKEFLAGS= -j8
HISTSIZE=1000
J=8
USER=root
CCACHE_DIR=/var/tmp/ccache
EXTRA_CONFIGURE_OPTS=
V=
SHOW_ENV=1
MAIL=/var/spool/mail/root
PATH=/usr/lib/ccache:/usr/lib64/ccache:/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin
PWD=/
LANG=en_US.UTF-8
TARGET_LIST=
HISTCONTROL=ignoredups
SHLVL=1
HOME=/root
TEST_DIR=/tmp/qemu-test
LOGNAME=root
LESSOPEN=||/usr/bin/lesspipe.sh %s
FEATURES= dtc
DEBUG=
G_BROKEN_FILENAMES=1
CCACHE_HASHDIR=
_=/usr/bin/env

Configure options:
--enable-werror --target-list=x86_64-softmmu,aarch64-softmmu 
--prefix=/var/tmp/qemu-build/install
No C++ compiler available; disabling C++ specific optional code
Install prefix/var/tmp/qemu-build/install
BIOS directory/var/tmp/qemu-build/install/share/qemu
binary directory  /var/tmp/qemu-build/install/bin
library directory /var/tmp/qemu-build/install/lib
module directory  /var/tmp/qemu-build/install/lib/qemu
libexec directory /var/tmp/qemu-build/install/libexec
include directory /var/tmp/qemu-build/install/include
config directory  /var/tmp/qemu-build/install/etc
local state directory   /var/tmp/qemu-build/install/var
Manual directory  /var/tmp/qemu-build/install/share/man
ELF interp prefix /usr/gnemul/qemu-%M
Source path   /tmp/qemu-test/src
C compilercc
Host C compiler   cc
C++ compiler  
Objective-C compiler cc
ARFLAGS   rv
CFLAGS-O2 -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=2 -g 
QEMU_CFLAGS   -I/usr/include/pixman-1-pthread -I/usr/include/glib-2.0 
-I/usr/lib64/glib-2.0/include   -fPIE -DPIE -m64 -D_GNU_SOURCE 
-D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE -Wstrict-prototypes 
-Wredundant-decls -Wall -Wundef -Wwrite-strings -Wmissing-prototypes 
-fno-strict-aliasing -fno-common -fwrapv  -Wendif-labels -Wmissing-include-dirs 
-Wempty-body -Wnested-externs -Wformat-security -Wformat-y2k -Winit-self 
-Wignored-qualifiers -Wold-style-declaration -Wold-style-definition 
-Wtype-limits -fstack-protector-all
LDFLAGS   -Wl,--warn-common -Wl,-z,relro -Wl,-z,now -pie -m64 -g 
make  make
install   install
pythonpython -B
smbd  /usr/sbin/smbd
module supportno
host CPU  x86_64
host big endian   no
target list   x86_64-softmmu aarch64-softmmu
tcg debug enabled no
gprof enabled no
sparse enabledno
strip binariesyes
profiler  no
static build  no
pixmansystem
SDL support   yes (1.2.14)
GTK support   no 
GTK GL supportno
VTE support   no 
TLS priority  NORMAL
GNUTLS supportno
GNUTLS rndno
libgcrypt no
libgcrypt kdf no
nettleno 
nettle kdfno
libtasn1  no
curses supportno
virgl support no
curl support  no
mingw32 support   no
Audio drivers oss
Block whitelist (rw) 
Block whitelist (ro) 
VirtFS supportno
VNC support   yes
VNC SASL support  no
VNC JPEG support  no
VNC PNG support   no
xen support   no
brlapi supportno
bluez  supportno
Documentation no
PIE   yes
vde support   no
netmap supportno
Linux AIO support no
ATTR/XATTR support yes
Install blobs yes
KVM support   yes
RDMA support  no
TCG interpreter   no
fdt support   yes
preadv supportyes
fdatasync yes
madvise   yes
posix_madvise yes
libcap-ng support no
vhost-net support yes
vhost-scsi support yes
vhost-vsock support yes
Trace backendslog
spice 

Re: [Qemu-devel] [PATCH v4 6/8] intel_iommu: reject broken EIM

2016-10-10 Thread Peter Xu
On Mon, Oct 10, 2016 at 05:11:19PM +0200, Radim Krčmář wrote:

[...]

> > But that's really a matter of taste. So:
> 
> I'll currently go for an implicit else: (because 4 levels of indentation
> are getting helper-function worthy and it has less curly braces)
> 
> if (!kvm_irqchip_in_kernel()) {
> error("need split irqchip");
> return false;
> }
> if (!kvm_enable_x2apic()) {
> error("enable x2apic failed");
> return false;
> }

Good to me.

> 
> > Reviewed-by: Peter Xu 
> 
> I squashed [7/8] into this patch in v5 and the second one didn't have
> your r-b, so I made the change as I'd have to drop the r-b anyway.

Sure. Thanks,

-- peterx



Re: [Qemu-devel] [PATCH v4 2/8] apic: add send_msi() to APICCommonClass

2016-10-10 Thread Peter Xu
On Mon, Oct 10, 2016 at 03:35:26PM +0200, Radim Krčmář wrote:

[...]

> Yes, but it is a separate logical change, so I'd do that in a separate
> cleaup patch that amends it in the whole file ...
> (And I prefer to copy-paste code verbatim.)

Sure.

> 
> > And, this patch is assuming MSIMessage as host endianess (which is
> > good to me). Not sure whether we need fixes for the whole MSIMessage
> > cleanup (after all, kvm_irqchip_send_msi() is taking it as LE). Or we
> > can do it afterwards since it won't break anything AFAIU.
> 
> This patch doesn't affect existing callers, and IR known to be broken,
> so I think we can sort endianess out later ...

Agree. Patch:

  [PATCH] Revert "KVM: MSI: Swap payload to native endianness"

is for the cleanup actually. Though I am not sure whether that's
enough and correct. Looking forward to any review comments.

Thanks,

-- peterx



Re: [Qemu-devel] [PATCH v2 10/11] blockjob: refactor backup_start as backup_job_create

2016-10-10 Thread John Snow



On 10/10/2016 04:57 AM, Kevin Wolf wrote:

Am 07.10.2016 um 20:39 hat John Snow geschrieben:

On 09/30/2016 06:00 PM, John Snow wrote:

Refactor backup_start as backup_job_create, which only creates the job,
but does not automatically start it. The old interface, 'backup_start',
is not kept in favor of limiting the number of nearly-identical iterfaces


(Ah yes, 'iterfaces.')


that would have to be edited to keep up with QAPI changes in the future.

Callers that wish to synchronously start the backup_block_job can
instead just call block_job_start immediately after calling
backup_job_create.

Transactions are updated to use the new interface, calling block_job_start
only during the .commit phase, which helps prevent race conditions where
jobs may finish before we even finish building the transaction. This may
happen, for instance, during empty block backup jobs.



Sadly for me, I realized this patch has a potential problem. When we
were adding the bitmap operations, it became clear that the
atomicity point was during .prepare, not .commit.

e.g. the bitmap is cleared or created during prepare, and backup_run
installs its Write Notifier at that point in time, too.


Strictly speaking that's wrong then.



I agree, though I do remember this coming up during the bitmap review 
process that the current point-in-time spot is during prepare at the moment.


I do think that while it's at least a consistent model (The model where 
we do in fact commit during .prepare(), and simply undo or revert during 
.abort(), and only clean or remove undo-cache in .commit()) it certainly 
violates the principle of least surprise and is a little rude...



The write notifier doesn't really hurt because it is never triggered
between prepare and commit (we're holding the lock) and it can just be
removed again.

Clearing the bitmap is a bug because the caller could expect that the
bitmap is in its original state if the transaction fails. I doubt this
is a problem in practice, but we should fix it anyway.



We make a backup to undo the process if it fails. I only mention it to 
emphasize that the atomic point appears to be during prepare. In 
practice we hold the locks for the whole process, but... I think Paolo 
may be actively trying to change that.



By the way, why did we allow to add a 'bitmap' option for DriveBackup
without adding it to BlockdevBackup at the same time?



I don't remember. I'm not sure anyone ever audited it to convince 
themselves it was a useful or safe thing to do. I believe at the time I 
was pushing for bitmaps in DriveBackup, Fam was still authoring the 
BlockdevBackup interface.



By changing BlockJobs to only run on commit, we've severed the
atomicity point such that some actions will take effect during
prepare, and others at commit.

I still think it's the correct thing to do to delay the BlockJobs
until the commit phase, so I will start auditing the code to see how
hard it is to shift the atomicity point to commit instead. If it's
possible to do that, I think from the POV of the managing
application, having the atomicity point be

Feel free to chime in with suggestions and counterpoints until then.


I agree that jobs have to be started only at commit. There may be other
things that are currently happening in prepare that really should be
moved as well, but unless moving one thing but not the other doesn't
break anything that was working, we can fix one thing at a time.

Kevin



Alright, let's give this a whirl.

We have 8 transaction actions:

drive_backup
blockdev_backup
block_dirty_bitmap_add
block_dirty_bitmap_clear
abort
blockdev_snapshot
blockdev_snapshot_sync
blockdev_snapshot_internal_sync

Drive and Blockdev backup are already modified to behave point-in-time 
at time of .commit() by changing them to only begin running once the 
commit phase occurs.


Bitmap add and clear are trivial to rework; clear just moves the call to 
clear in commit, with possibly some action taken to prevent the bitmap 
from become used by some other process in the meantime. Add is easy to 
rework too, we can create it during prepare but reset it back to zero 
during commit if necessary.


Abort needs no changes.

blockdev_snapshot[_sync] actually appears to already be doing the right 
thing, by only installing the new top layer during commit, which makes 
this action inconsistent by current semantics, but requires no changes 
to move to the desired new semantics.


That leaves only the internal snapshot to worry about, which does 
admittedly look like quite the yak to shave. It's a bit out of scope for 
me, but Kevin, do you think this is possible?


Looks like implementations are qcow2, rbd, and sheepdog. I imagine this 
would need to be split into prepare and commit semantics to accommodate 
this change... though we don't have any meaningful control over the rbd 
implementation.


Any thoughts? I could conceivably just change everything over to working 
primarily during .commit(), and just argue that 

[Qemu-devel] [PATCH v4] build: Work around SIZE_MAX bug in OSX headers

2016-10-10 Thread Eric Blake
C99 requires SIZE_MAX to be declared with the same type as the
integral promotion of size_t, but OSX mistakenly defines it as
an 'unsigned long long' expression even though size_t is only
'unsigned long'.  Rather than futzing around with whether size_t
is 32- or 64-bits wide (which would be needed if we cared about
using SIZE_T in a #if expression), let the compiler get the right
type for us by virtue of integer promotion - if we later need it
during the preprocessor, the build will break on Mac until we
improve our replacement.

See also https://patchwork.ozlabs.org/patch/542327/ for an
instance where the wrong type trips us up if we don't fix it
for good in osdep.h.

Some versions of glibc make a similar mistake with SSIZE_MAX; the
goal is that the approach of this patch could be copied to work
around that problem if it ever becomes important to us.

Signed-off-by: Eric Blake 

---
v1 was here:
https://lists.gnu.org/archive/html/qemu-devel/2016-07/msg02520.html

The topic recently came up again, and I noticed this patch sitting
on one of my older branches, so I've taken another shot at it.
https://lists.gnu.org/archive/html/qemu-devel/2016-10/msg00950.html

v2: rewrite into a configure check (not sure if directly adding a
-D to QEMU_CFLAGS is the best, so advice welcome)

v3: Use config-host.mak rather than direct -D

v4: placate -Wunused builds

I lack easy access to a Mac box, so this is untested as to whether
it actually solves the issue...
---
 include/qemu/osdep.h |  8 
 configure| 16 
 2 files changed, 24 insertions(+)

diff --git a/include/qemu/osdep.h b/include/qemu/osdep.h
index 9e9fa61..e06ac47 100644
--- a/include/qemu/osdep.h
+++ b/include/qemu/osdep.h
@@ -141,6 +141,14 @@ extern int daemon(int, int);
 # error Unknown pointer size
 #endif

+/* Mac OSX has a  bug that incorrectly defines SIZE_MAX with
+ * the wrong type. Our replacement isn't usable in preprocessor
+ * expressions, but it is sufficient for our needs. */
+#if defined(HAVE_BROKEN_SIZE_MAX) && HAVE_BROKEN_SIZE_MAX
+#undef SIZE_MAX
+#define SIZE_MAX ((sizeof(char)) * -1)
+#endif
+
 #ifndef MIN
 #define MIN(a, b) (((a) < (b)) ? (a) : (b))
 #endif
diff --git a/configure b/configure
index 5751d8e..dd9e679 100755
--- a/configure
+++ b/configure
@@ -1725,6 +1725,19 @@ if test "$cocoa" = "yes"; then
 sdl=no
 fi

+# Some versions of Mac OS X incorrectly define SIZE_MAX
+cat > $TMPC << EOF
+#include 
+#include 
+int main(int argc, char *argv[]) {
+return printf("%zu", SIZE_MAX);
+}
+EOF
+have_broken_size_max=no
+if ! compile_object -Werror ; then
+have_broken_size_max=yes
+fi
+
 ##
 # L2TPV3 probe

@@ -5245,6 +5258,9 @@ fi
 if test "$have_ifaddrs_h" = "yes" ; then
 echo "HAVE_IFADDRS_H=y" >> $config_host_mak
 fi
+if test "$have_broken_size_max" = "yes" ; then
+echo "HAVE_BROKEN_SIZE_MAX=y" >> $config_host_mak
+fi

 # Work around a system header bug with some kernel/XFS header
 # versions where they both try to define 'struct fsxattr':
-- 
2.7.4




Re: [Qemu-devel] [PATCH qemu] sysemu: support up to 1024 vCPUs

2016-10-10 Thread Alexey Kardashevskiy
Ping, anyone?


On 04/10/16 11:33, Alexey Kardashevskiy wrote:
> From: Greg Kurz 
> 
> Some systems can already provide more than 255 hardware threads.
> 
> Bumping the QEMU limit to 1024 seems reasonable:
> - it has no visible overhead in top;
> - the limit itself has no effect on hot paths.
> 
> Signed-off-by: Greg Kurz 
> Signed-off-by: Alexey Kardashevskiy 
> ---
>  include/sysemu/sysemu.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h
> index ef2c50b..2ec0bd8 100644
> --- a/include/sysemu/sysemu.h
> +++ b/include/sysemu/sysemu.h
> @@ -173,7 +173,7 @@ extern int mem_prealloc;
>   *
>   * Note that cpu->get_arch_id() may be larger than MAX_CPUMASK_BITS.
>   */
> -#define MAX_CPUMASK_BITS 255
> +#define MAX_CPUMASK_BITS 1024
>  
>  #define MAX_OPTION_ROMS 16
>  typedef struct QEMUOptionRom {
> 


-- 
Alexey



Re: [Qemu-devel] [PATCH 12/29] target-sparc: implement UA2005 rdhpstate and wrhpstate instructions

2016-10-10 Thread Richard Henderson

On 10/01/2016 05:05 AM, Artyom Tarasenko wrote:

Signed-off-by: Artyom Tarasenko 
---
 target-sparc/translate.c | 7 +--
 1 file changed, 5 insertions(+), 2 deletions(-)


Reviewed-by: Richard Henderson 


r~



Re: [Qemu-devel] [PATCH 11/29] target-sparc: implement UA2005 GL register

2016-10-10 Thread Richard Henderson

On 10/01/2016 05:05 AM, Artyom Tarasenko wrote:

 case 16: // UA2005 gl
 CHECK_IU_FEATURE(dc, GL);
-tcg_gen_st32_tl(cpu_tmp0, cpu_env,
-offsetof(CPUSPARCState, gl));
+save_state(dc);
+gen_helper_wrgl(cpu_env, cpu_tmp0);
+dc->npc = DYNAMIC_PC;


Why the state save?  gen_helper_wrgl does not raise an exception...


r~



Re: [Qemu-devel] [PATCH 09/29] target-sparc: hypervisor mode takes over nucleus mode

2016-10-10 Thread Richard Henderson

On 10/01/2016 05:05 AM, Artyom Tarasenko wrote:

Signed-off-by: Artyom Tarasenko 
---
 target-sparc/cpu.h | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/target-sparc/cpu.h b/target-sparc/cpu.h
index 0b5c79f..fbeb8d7 100644
--- a/target-sparc/cpu.h
+++ b/target-sparc/cpu.h
@@ -699,10 +699,10 @@ static inline int cpu_mmu_index(CPUSPARCState *env1, bool 
ifetch)
 #elif !defined(TARGET_SPARC64)
 return env1->psrs;
 #else
-if (env1->tl > 0) {
-return MMU_NUCLEUS_IDX;
-} else if (cpu_hypervisor_mode(env1)) {
+if (cpu_hypervisor_mode(env1)) {
 return MMU_HYPV_IDX;
+} else if (env1->tl > 0) {
+return MMU_NUCLEUS_IDX;
 } else if (cpu_supervisor_mode(env1)) {
 return MMU_KERNEL_IDX;
 } else {



While playing with your patch set, I discovered that we also need a patch to 
get_asi for ASI_N et al to retain MMU_HYPV_IDX, and not decrease privilege. 
This happens *very* early in the prom boot, with the first casx (when casx is 
implemented inline).



r~



Re: [Qemu-devel] [Qemu-ppc] [QEMU PATCH v5 4/6] migration: migrate QTAILQ

2016-10-10 Thread Jianjun Duan


On 10/08/2016 12:28 PM, Halil Pasic wrote:
> 
> 
> On 10/08/2016 01:37 PM, Paolo Bonzini wrote:
>>> Even though most put/get have no issues now, when somebody writes a new
 put, he or she could run into issues if only checking the type
 signature. It makes the code more readable.
> 
>> No, it doesn't because one is left wondering what is VMS_LINKED about.
> 
> I agree with Paolo. IMHO VMS_LINKED is conceptually difficult in  a sense
> that it's quite different that what we have currently. I have the feeling,
> conceptually, we are trying to fit in something like data structures with a
> type parameter (the element type) here. AFAIU what vmstate currently can is
> directed acyclic graphs of certain stuff (and also completely custom
> handling based on custom put/get).
> 
You are right. What we have in vmstate now cannot handle a recursive or
cyclic structure in a generic way. VMS_LINKED is intended to indicate a
recursive structure, with the newly added parameters of put/get
providing information about the element type in the recursive structure.

Thanks,
Jianjun
>> What is the relation between linked datastructures and passing some
>> arguments as NULL/noon-NULL?
> 
> IFAIU we need those for the datastructure because it's linked and has
> a type parameter (element type). The two last arguments are for
> the element type. These were added by the previous patch because
> the old VMStateInfo did not need these. So we drastically changed
> the scope of VMStateInfo with the previous patch and I'm not
> sure I like this.
> 
> I will have to stare at this a bit longer to bring something more
> constructive than these (largely feeling-based) remarks.
> 
> Cheers,
> Halil
> 




Re: [Qemu-devel] [PATCH 08/29] target-sparc: implement UltraSPARC-T1 Strand status ASR

2016-10-10 Thread Richard Henderson

On 10/01/2016 05:05 AM, Artyom Tarasenko wrote:

Signed-off-by: Artyom Tarasenko 
---
 target-sparc/translate.c | 11 +++
 1 file changed, 11 insertions(+)


Reviewed-by: Richard Henderson 


r~



Re: [Qemu-devel] [PATCH 07/29] target-sparc: implement UA2005 scratchpad registers

2016-10-10 Thread Richard Henderson

On 10/01/2016 05:05 AM, Artyom Tarasenko wrote:

+case ASI_SCRATCHPAD: /* UA2005 privileged scratchpad */
+if (unlikely((addr >= 0x20) && (addr < 0x30))) {
+/* Hyperprivileged access only */
+cpu_unassigned_access(cs, addr, false, false, 1, size);
+}
+/* fall through */
+case ASI_HYP_SCRATCHPAD: /* UA2005 hyperprivileged scratchpad */
+{
+unsigned int i = (addr >> 3) & 0x7;
+ret = env->scratch[i];
+break;
+}


It *might* speed things up a teeny bit to implement ASI_HYP_SCRATCHPAD inline. 
E.g.


  case GET_ASI_HYP_SCRATCH:
{
  TCGv_ptr tmp = tcg_temp_new_ptr();
#if UINTPTR_MAX == UINT32_MAX
  tcg_gen_extrl_i64_i32(tmp, addr);
  tcg_gen_andi_i32(tmp, tmp, 7 << 3);
#else
  tcg_gen_andi_i64(tmp, addr, 7 << 3);
#endif
  tcg_gen_add_ptr(tmp, tmp, cpu_env);
  tcg_gen_ld_i64(dst, tmp, offsetof(CPUSPARCState, scratch));
  tcg_temp_free_ptr(tmp);
}
break;

Of course, you can't do that for ASI_SCRATCHPAD because of the dynamic check 
against ADDR.



@@ -2056,6 +2068,9 @@ void helper_st_asi(CPUSPARCState *env, target_ulong addr, 
target_ulong val,
 return;
 case ASI_INTR_RECEIVE: /* Interrupt data receive */
 env->ivec_status = val & 0x20;
+if (!env->ivec_status) {
+cpu_reset_interrupt(cs, CPU_INTERRUPT_HARD);
+}
 return;


This belongs in some other patch.


r~



Re: [Qemu-devel] [PATCH v3] build: Work around SIZE_MAX bug in OSX headers

2016-10-10 Thread Eric Blake
On 10/10/2016 01:27 PM, Eric Blake wrote:
> C99 requires SIZE_MAX to be declared with the same type as the
> integral promotion of size_t, but OSX mistakenly defines it as
> an 'unsigned long long' expression even though size_t is only
> 'unsigned long'.  Rather than futzing around with whether size_t
> is 32- or 64-bits wide (which would be needed if we cared about
> using SIZE_T in a #if expression), let the compiler get the right
> type for us by virtue of integer promotion - if we later need it
> during the preprocessor, the build will break on Mac until we
> improve our replacement.
> 
> See also https://patchwork.ozlabs.org/patch/542327/ for an
> instance where the wrong type trips us up if we don't fix it
> for good in osdep.h.
> 
> Some versions of glibc make a similar mistake with SSIZE_MAX; the
> goal is that the approach of this patch could be copied to work
> around that problem if it ever becomes important to us.
> 
> Signed-off-by: Eric Blake 
> 

> +/* Mac OSX has a  bug that incorrectly defines SIZE_MAX with
> + * the wrong type. Our replacement isn't usable in preprocessor
> + * expressions, but it is sufficient for our needs. */
> +#if HAVE_BROKEN_SIZE_MAX

Whoops, -Werror=undef doesn't like this one.  v4 coming up

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



signature.asc
Description: OpenPGP digital signature


Re: [Qemu-devel] [Qemu-ppc] [QEMU PATCH v5 4/6] migration: migrate QTAILQ

2016-10-10 Thread Jianjun Duan


On 10/08/2016 12:28 PM, Halil Pasic wrote:
> 
> 
> On 10/08/2016 01:37 PM, Paolo Bonzini wrote:
>>> Even though most put/get have no issues now, when somebody writes a new
 put, he or she could run into issues if only checking the type
 signature. It makes the code more readable.
> 
>> No, it doesn't because one is left wondering what is VMS_LINKED about.
> 
> I agree with Paolo. IMHO VMS_LINKED is conceptually difficult in  a sense
> that it's quite different that what we have currently. I have the feeling,
> conceptually, we are trying to fit in something like data structures with a
> type parameter (the element type) here. AFAIU what vmstate currently can is
> directed acyclic graphs of certain stuff (and also completely custom
> handling based on custom put/get).
> 
You are right. What we have in VMSTATE now cannot handle a recursive (or
cyclic as you call it) structure. The idea was to use VMS_LINKED to
indicate a recursive structure. In this patch is used on a queue. It can
also be used on list or trees.
In this regard, VMS_LINKED does represent something.

Thanks,
Jianjun

>> What is the relation between linked datastructures and passing some
>> arguments as NULL/noon-NULL?
> 
> IFAIU we need those for the datastructure because it's linked and has
> a type parameter (element type). The two last arguments are for
> the element type. These were added by the previous patch because
> the old VMStateInfo did not need these. So we drastically changed
> the scope of VMStateInfo with the previous patch and I'm not
> sure I like this.
> 
> I will have to stare at this a bit longer to bring something more
> constructive than these (largely feeling-based) remarks.
> 
> Cheers,
> Halil
> 




Re: [Qemu-devel] [PATCH 06/29] target-sparc: simplify replace_tlb_entry by using TTE_PGSIZE

2016-10-10 Thread Richard Henderson

On 10/01/2016 05:05 AM, Artyom Tarasenko wrote:

Signed-off-by: Artyom Tarasenko 
---
 target-sparc/ldst_helper.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)


Reviewed-by: Richard Henderson 


r~



Re: [Qemu-devel] [PATCH 05/29] target-sparc: on UA2005 don't deliver Interrupt_level_n IRQs in hypervisor mode

2016-10-10 Thread Richard Henderson

On 10/01/2016 05:05 AM, Artyom Tarasenko wrote:

As described in Chapter 5.7.6 of the UltraSPARC Architecture 2005,
outstanding disrupting exceptions that are destined for privileged mode can only
cause a trap when the virtual processor is in nonprivileged or privileged mode 
and
PSTATE.ie = 1. At all other times, they are held pending.

Signed-off-by: Artyom Tarasenko 
---
 target-sparc/cpu.h | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)


Reviewed-by: Richard Henderson 


r~



Re: [Qemu-devel] [PATCH 03/29] target-sparc: add UA2005 TTE bit #defines

2016-10-10 Thread Richard Henderson

On 10/01/2016 05:05 AM, Artyom Tarasenko wrote:

 #define TTE_VALID_BIT   (1ULL << 63)
 #define TTE_NFO_BIT (1ULL << 60)
+#define TTE_NFO_BIT_UA2005  (1ULL << 62)
 #define TTE_USED_BIT(1ULL << 41)
+#define TTE_USED_BIT_UA2005 (1ULL << 47)
 #define TTE_LOCKED_BIT  (1ULL <<  6)
+#define TTE_LOCKED_BIT_UA2005 (1ULL <<  61)
 #define TTE_SIDEEFFECT_BIT  (1ULL <<  3)
+#define TTE_SIDEEFFECT_BIT_UA2005 (1ULL <<  11)
 #define TTE_PRIV_BIT(1ULL <<  2)
+#define TTE_PRIV_BIT_UA2005 (1ULL <<  8)
 #define TTE_W_OK_BIT(1ULL <<  1)
+#define TTE_W_OK_BIT_UA2005 (1ULL <<  6)
 #define TTE_GLOBAL_BIT  (1ULL <<  0)


Hmm.  Would it make more sense to reorg these as

  TTE_US1_*
  TTE_UA2005_*

with some duplication for the bits that are shared?
As is, it's pretty hard to tell which actually change...


r~



Re: [Qemu-devel] [PATCH 02/29] target-sparc: use explicit mmu register pointers

2016-10-10 Thread Richard Henderson

On 10/01/2016 05:05 AM, Artyom Tarasenko wrote:

Use explicit register pointers while accessing D/I-MMU registers.
Trap on access to missing registers.


... unless the mmu is disabled; see patch 1.  ;-)


Signed-off-by: Artyom Tarasenko 
---
 target-sparc/cpu.h |  4 +++
 target-sparc/ldst_helper.c | 66 +-
 2 files changed, 58 insertions(+), 12 deletions(-)


Otherwise,

Reviewed-by: Richard Henderson 


r~



Re: [Qemu-devel] [PATCH 01/29] target-sparc: don't trap on MMU-fault if MMU is disabled

2016-10-10 Thread Richard Henderson

On 10/01/2016 05:05 AM, Artyom Tarasenko wrote:

 if (is_exec) {
-helper_raise_exception(env, TT_CODE_ACCESS);
+if (env->lsu & (IMMU_E)) {
+helper_raise_exception(env, TT_CODE_ACCESS);
+}
 } else {
-helper_raise_exception(env, TT_DATA_ACCESS);
+if (env->lsu & (DMMU_E)) {
+helper_raise_exception(env, TT_DATA_ACCESS);
+}


The cpu really does no kind of machine check for a hypervisor write to 
0x1122334455667788?



r~



Re: [Qemu-devel] [RFC v4 0/8] KVM PCI/MSI passthrough with mach-virt

2016-10-10 Thread no-reply
Hi,

Your series failed automatic build test. Please find the testing commands and
their output below. If you have docker installed, you can probably reproduce it
locally.

Message-id: 1475754090-22681-1-git-send-email-eric.au...@redhat.com
Subject: [Qemu-devel] [RFC v4 0/8] KVM PCI/MSI passthrough with mach-virt
Type: series

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

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
Switched to a new branch 'test'
9fa6e4b hw: vfio: common: Adapt vfio_listeners for reserved_iova region
6f58b94 hw: vfio: common: vfio_prepare_msi_mapping
e73c1f9 hw: platform-bus: Enable to map any memory region onto the platform-bus
313b445 memory: memory_region_find_by_name
afe3c21 memory: Add reserved_iova region type
403f6a2 hw: vfio: common: Introduce vfio_register_msi_iova
30cd237 hw: vfio: common: vfio_get_iommu_type1_info
22e4efb linux-headers: Partial update for MSI IOVA handling

=== OUTPUT BEGIN ===
Submodule 'dtc' (git://git.qemu-project.org/dtc.git) registered for path 'dtc'
Cloning into 'dtc'...
Submodule path 'dtc': checked out '65cc4d2748a2c2e6f27f1cf39e07a5dbabd80ebf'
  BUILD   centos6
  ARCHIVE qemu.tgz
  ARCHIVE dtc.tgz
  COPYRUNNER
  RUN test-quick in centos6
Packages installed:
SDL-devel-1.2.14-7.el6_7.1.x86_64
ccache-3.1.6-2.el6.x86_64
epel-release-6-8.noarch
gcc-4.4.7-17.el6.x86_64
git-1.7.1-4.el6_7.1.x86_64
glib2-devel-2.28.8-5.el6.x86_64
libfdt-devel-1.4.0-1.el6.x86_64
make-3.81-23.el6.x86_64
package g++ is not installed
pixman-devel-0.32.8-1.el6.x86_64
tar-1.23-15.el6_8.x86_64
zlib-devel-1.2.3-29.el6.x86_64

Environment variables:
PACKAGES=libfdt-devel ccache tar git make gcc g++ zlib-devel 
glib2-devel SDL-devel pixman-devel epel-release
HOSTNAME=415edbc0ef5f
TERM=xterm
MAKEFLAGS= -j8
HISTSIZE=1000
J=8
USER=root
CCACHE_DIR=/var/tmp/ccache
EXTRA_CONFIGURE_OPTS=
V=
SHOW_ENV=1
MAIL=/var/spool/mail/root
PATH=/usr/lib/ccache:/usr/lib64/ccache:/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin
PWD=/
LANG=en_US.UTF-8
TARGET_LIST=
HISTCONTROL=ignoredups
SHLVL=1
HOME=/root
TEST_DIR=/tmp/qemu-test
LOGNAME=root
LESSOPEN=||/usr/bin/lesspipe.sh %s
FEATURES= dtc
DEBUG=
G_BROKEN_FILENAMES=1
CCACHE_HASHDIR=
_=/usr/bin/env

Configure options:
--enable-werror --target-list=x86_64-softmmu,aarch64-softmmu 
--prefix=/var/tmp/qemu-build/install
No C++ compiler available; disabling C++ specific optional code
Install prefix/var/tmp/qemu-build/install
BIOS directory/var/tmp/qemu-build/install/share/qemu
binary directory  /var/tmp/qemu-build/install/bin
library directory /var/tmp/qemu-build/install/lib
module directory  /var/tmp/qemu-build/install/lib/qemu
libexec directory /var/tmp/qemu-build/install/libexec
include directory /var/tmp/qemu-build/install/include
config directory  /var/tmp/qemu-build/install/etc
local state directory   /var/tmp/qemu-build/install/var
Manual directory  /var/tmp/qemu-build/install/share/man
ELF interp prefix /usr/gnemul/qemu-%M
Source path   /tmp/qemu-test/src
C compilercc
Host C compiler   cc
C++ compiler  
Objective-C compiler cc
ARFLAGS   rv
CFLAGS-O2 -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=2 -g 
QEMU_CFLAGS   -I/usr/include/pixman-1-pthread -I/usr/include/glib-2.0 
-I/usr/lib64/glib-2.0/include   -fPIE -DPIE -m64 -D_GNU_SOURCE 
-D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE -Wstrict-prototypes 
-Wredundant-decls -Wall -Wundef -Wwrite-strings -Wmissing-prototypes 
-fno-strict-aliasing -fno-common -fwrapv  -Wendif-labels -Wmissing-include-dirs 
-Wempty-body -Wnested-externs -Wformat-security -Wformat-y2k -Winit-self 
-Wignored-qualifiers -Wold-style-declaration -Wold-style-definition 
-Wtype-limits -fstack-protector-all
LDFLAGS   -Wl,--warn-common -Wl,-z,relro -Wl,-z,now -pie -m64 -g 
make  make
install   install
pythonpython -B
smbd  /usr/sbin/smbd
module supportno
host CPU  x86_64
host big endian   no
target list   x86_64-softmmu aarch64-softmmu
tcg debug enabled no
gprof enabled no
sparse enabledno
strip binariesyes
profiler  no
static build  no
pixmansystem
SDL support   yes (1.2.14)
GTK support   no 
GTK GL supportno
VTE support   no 
TLS priority  NORMAL
GNUTLS supportno
GNUTLS rndno
libgcrypt no
libgcrypt kdf no
nettleno 
nettle kdfno
libtasn1  no
curses supportno
virgl support no
curl support  no
mingw32 support   no
Audio drivers oss
Block whitelist (rw) 
Block whitelist (ro) 
VirtFS supportno
VNC support   yes
VNC SASL support  no
VNC JPEG support  no
VNC PNG support   no
xen support   no
brlapi supportno
bluez  supportno
Documentation no
PIE   yes
vde support   

Re: [Qemu-devel] [PATCH v8 1/6] vfio: Mediated device Core driver

2016-10-10 Thread Eric Blake
On 10/10/2016 03:28 PM, Kirti Wankhede wrote:
> Design for Mediated Device Driver:
> Main purpose of this driver is to provide a common interface for mediated
> device management that can be used by different drivers of different
> devices.
> 

> 
> Signed-off-by: Kirti Wankhede 
> Signed-off-by: Neo Jia 
> Change-Id: I73a5084574270b14541c529461ea2f03c292d510
> ---

> +++ b/drivers/vfio/mdev/Kconfig
> @@ -0,0 +1,12 @@
> +
> +config VFIO_MDEV
> +tristate "Mediated device driver framework"
> +depends on VFIO
> +default n
> +help
> +Provides a framework to virtualize device.

Feels like a missing word or two here, maybe 'virtualize a _ device' ?

> + See Documentation/vfio-mdev/vfio-mediated-device.txt for more details.
> +
> +If you don't know what do here, say N.
> +

> +++ b/drivers/vfio/mdev/mdev_core.c
> @@ -0,0 +1,363 @@
> +/*
> + * Mediated device Core Driver
> + *
> + * Copyright (c) 2016, NVIDIA CORPORATION. All rights reserved.
> + * Author: Neo Jia 
> + *  Kirti Wankhede 
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.

Umm - "All rights reserved" is incompatible with GPLv2.  Either you
reserved the rights (and it is therefore not GPL), or it is GPL (and you
are specifically granting rights, and therefore not all rights are
reserved).

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



signature.asc
Description: OpenPGP digital signature


[Qemu-devel] [PATCH v8 5/6] Add simple sample driver for mediated device framework

2016-10-10 Thread Kirti Wankhede
Sample driver creates mdev device that simulates serial port over PCI card.

Signed-off-by: Kirti Wankhede 
Signed-off-by: Neo Jia 
Change-Id: I857f8f12f8b275f2498dfe8c628a5cdc7193b1b2
---
 Documentation/vfio-mdev/Makefile |   14 +
 Documentation/vfio-mdev/mtty.c   | 1353 ++
 Documentation/vfio-mdev/vfio-mediated-device.txt |   63 +
 3 files changed, 1430 insertions(+)
 create mode 100644 Documentation/vfio-mdev/Makefile
 create mode 100644 Documentation/vfio-mdev/mtty.c

diff --git a/Documentation/vfio-mdev/Makefile b/Documentation/vfio-mdev/Makefile
new file mode 100644
index ..ff6f8a324c85
--- /dev/null
+++ b/Documentation/vfio-mdev/Makefile
@@ -0,0 +1,14 @@
+#
+# Makefile for mtty.c file
+#
+KDIR:=/lib/modules/$(shell uname -r)/build
+
+obj-m:=mtty.o
+
+default:
+   $(MAKE) -C $(KDIR) SUBDIRS=$(PWD) modules
+
+clean:
+   @rm -rf .*.cmd *.mod.c *.o *.ko .tmp*
+   @rm -rf Module.* Modules.* modules.* .tmp_versions
+
diff --git a/Documentation/vfio-mdev/mtty.c b/Documentation/vfio-mdev/mtty.c
new file mode 100644
index ..497c90ebe257
--- /dev/null
+++ b/Documentation/vfio-mdev/mtty.c
@@ -0,0 +1,1353 @@
+/*
+ * Mediated virtual PCI serial host device driver
+ *
+ * Copyright (c) 2016, NVIDIA CORPORATION. All rights reserved.
+ * Author: Neo Jia 
+ * Kirti Wankhede 
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ * Sample driver that creates mdev device that simulates serial port over PCI
+ * card.
+ *
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+/*
+ * #defines
+ */
+
+#define VERSION_STRING  "0.1"
+#define DRIVER_AUTHOR   "NVIDIA Corporation"
+
+#define MTTY_CLASS_NAME "mtty"
+
+#define MTTY_NAME   "mtty"
+
+#define MTTY_CONFIG_SPACE_SIZE  0xff
+#define MTTY_IO_BAR_SIZE0x8
+#define MTTY_MMIO_BAR_SIZE  0x10
+
+#define STORE_LE16(addr, val)   (*(u16 *)addr = val)
+#define STORE_LE32(addr, val)   (*(u32 *)addr = val)
+
+#define MAX_FIFO_SIZE   16
+
+#define CIRCULAR_BUF_INC_IDX(idx)(idx = (idx + 1) & (MAX_FIFO_SIZE - 1))
+
+#define MTTY_VFIO_PCI_OFFSET_SHIFT   40
+
+#define MTTY_VFIO_PCI_OFFSET_TO_INDEX(off)   (off >> 
MTTY_VFIO_PCI_OFFSET_SHIFT)
+#define MTTY_VFIO_PCI_INDEX_TO_OFFSET(index) \
+   ((u64)(index) << MTTY_VFIO_PCI_OFFSET_SHIFT)
+#define MTTY_VFIO_PCI_OFFSET_MASK\
+   (((u64)(1) << MTTY_VFIO_PCI_OFFSET_SHIFT) - 1)
+
+
+/*
+ * Global Structures
+ */
+
+struct mtty_dev {
+   dev_t   vd_devt;
+   struct class*vd_class;
+   struct cdev vd_cdev;
+   struct idr  vd_idr;
+   struct device   dev;
+} mtty_dev;
+
+struct mdev_region_info {
+   u64 start;
+   u64 phys_start;
+   u32 size;
+   u64 vfio_offset;
+};
+
+#if defined(DEBUG_REGS)
+const char *wr_reg[] = {
+   "TX",
+   "IER",
+   "FCR",
+   "LCR",
+   "MCR",
+   "LSR",
+   "MSR",
+   "SCR"
+};
+
+const char *rd_reg[] = {
+   "RX",
+   "IER",
+   "IIR",
+   "LCR",
+   "MCR",
+   "LSR",
+   "MSR",
+   "SCR"
+};
+#endif
+
+/* loop back buffer */
+struct rxtx {
+   u8 fifo[MAX_FIFO_SIZE];
+   u8 head, tail;
+   u8 count;
+};
+
+struct serial_port {
+   u8 uart_reg[8]; /* 8 registers */
+   struct rxtx rxtx;   /* loop back buffer */
+   bool dlab;
+   bool overrun;
+   u16 divisor;
+   u8 fcr; /* FIFO control register */
+   u8 max_fifo_size;
+   u8 intr_trigger_level;  /* interrupt trigger level */
+};
+
+/* State of each mdev device */
+struct mdev_state {
+   int irq_fd;
+   struct file *intx_file;
+   struct file *msi_file;
+   int irq_index;
+   u8 *vconfig;
+   struct mutex ops_lock;
+   struct mdev_device *mdev;
+   struct mdev_region_info region_info[VFIO_PCI_NUM_REGIONS];
+   u32 bar_mask[VFIO_PCI_NUM_REGIONS];
+   struct list_head next;
+   struct serial_port s[2];
+   struct mutex rxtx_lock;
+   struct vfio_device_info dev_info;
+};
+
+struct mutex mdev_list_lock;
+struct list_head mdev_devices_list;
+
+static const struct file_operations vd_fops = {
+   .owner  = THIS_MODULE,
+};
+
+/* function prototypes */
+
+static int mtty_trigger_interrupt(uuid_le uuid);
+
+/* Helper functions */
+static struct mdev_state *find_mdev_state_by_uuid(uuid_le uuid)
+{
+   struct mdev_state *mds;
+
+   list_for_each_entry(mds, _devices_list, next) {
+   if (uuid_le_cmp(mds->mdev->uuid, uuid) == 

[Qemu-devel] [PATCH v8 4/6] docs: Add Documentation for Mediated devices

2016-10-10 Thread Kirti Wankhede
Add file Documentation/vfio-mediated-device.txt that include details of
mediated device framework.

Signed-off-by: Kirti Wankhede 
Signed-off-by: Neo Jia 
Change-Id: I137dd646442936090d92008b115908b7b2c7bc5d
---
 Documentation/vfio-mdev/vfio-mediated-device.txt | 219 +++
 1 file changed, 219 insertions(+)
 create mode 100644 Documentation/vfio-mdev/vfio-mediated-device.txt

diff --git a/Documentation/vfio-mdev/vfio-mediated-device.txt 
b/Documentation/vfio-mdev/vfio-mediated-device.txt
new file mode 100644
index ..c1eacb83807b
--- /dev/null
+++ b/Documentation/vfio-mdev/vfio-mediated-device.txt
@@ -0,0 +1,219 @@
+/*
+ * VFIO Mediated devices
+ *
+ * Copyright (c) 2016, NVIDIA CORPORATION. All rights reserved.
+ * Author: Neo Jia 
+ * Kirti Wankhede 
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+VFIO Mediated devices [1]
+-
+
+There are more and more use cases/demands to virtualize the DMA devices which
+don't have SR_IOV capability built-in. To do this, drivers of different
+devices had to develop their own management interface and set of APIs and then
+integrate it to user space software. We've identified common requirements and
+unified management interface for such devices to make user space software
+integration easier and simplify the device driver implementation to support 
such
+I/O virtualization solution.
+
+The VFIO driver framework provides unified APIs for direct device access from
+user space. It is an IOMMU/device agnostic framework for exposing direct device
+access to user space, in a secure, IOMMU protected environment. This framework
+is used for multiple devices like GPUs, network adapters and compute
+accelerators. With direct device access, virtual machines or user space
+applications have direct access of physical device. This framework is reused
+for mediated devices.
+
+Mediated core driver provides a common interface for mediated device management
+that can be used by drivers of different devices. This module provides a 
generic
+interface to create/destroy a mediated device, add/remove it to mediated bus
+driver and add/remove device to an IOMMU group. It also provides an interface 
to
+register bus driver, for example, Mediated VFIO mdev driver is designed for
+mediated devices and supports VFIO APIs. Mediated bus driver add/delete 
mediated
+device to VFIO Group.
+
+Below is the high Level block diagram, with NVIDIA, Intel and IBM devices
+as example, since these are the devices which are going to actively use
+this module as of now.
+
+ +---+
+ |   |
+ | +---+ |  mdev_register_driver() +--+
+ | |   | +<+  |
+ | |  mdev | | |  |
+ | |  bus  | +>+ vfio_mdev.ko |<-> VFIO user
+ | |  driver   | | probe()/remove()|  |APIs
+ | |   | | +--+
+ | +---+ |
+ |   |
+ |  MDEV CORE|
+ |   MODULE  |
+ |   mdev.ko |
+ | +---+ |  mdev_register_device() +--+
+ | |   | +<+  |
+ | |   | | |  nvidia.ko   |<-> physical
+ | |   | +>+  |device
+ | |   | |callbacks+--+
+ | | Physical  | |
+ | |  device   | |  mdev_register_device() +--+
+ | | interface | |<+  |
+ | |   | | |  i915.ko |<-> physical
+ | |   | +>+  |device
+ | |   | |callbacks+--+
+ | |   | |
+ | |   | |  mdev_register_device() +--+
+ | |   | +<+  |
+ | |   | | | ccw_device.ko|<-> physical
+ | |   | +>+  |device
+ | |   | |callbacks+--+
+ | +---+ |
+ +---+
+
+
+Registration Interfaces:
+
+
+Mediated core driver provides two types of registration interfaces:
+
+1. Registration interface for mediated bus driver:
+--
+ /*
+  * struct mdev_driver [2] - Mediated device's driver
+  * @name: driver name
+  * @probe: called when new device created
+  * @remove: called when device removed
+  * @driver: device driver structure
+

[Qemu-devel] [PATCH v8 6/6] Add common functions for SET_IRQS and GET_REGION_INFO ioctls

2016-10-10 Thread Kirti Wankhede
Add common functions for SET_IRQS and to add capability buffer for
GET_REGION_INFO ioctls

Signed-off-by: Kirti Wankhede 
Signed-off-by: Neo Jia 
Change-Id: Id9e976a2c08b9b2b37da77dac4365ae8f6024b4a
---
 drivers/vfio/pci/vfio_pci.c | 103 +++
 drivers/vfio/vfio.c | 116 
 include/linux/vfio.h|   7 +++
 3 files changed, 162 insertions(+), 64 deletions(-)

diff --git a/drivers/vfio/pci/vfio_pci.c b/drivers/vfio/pci/vfio_pci.c
index 188b1ff03f5f..f312cbb0eebc 100644
--- a/drivers/vfio/pci/vfio_pci.c
+++ b/drivers/vfio/pci/vfio_pci.c
@@ -478,12 +478,12 @@ static int vfio_pci_for_each_slot_or_bus(struct pci_dev 
*pdev,
 }
 
 static int msix_sparse_mmap_cap(struct vfio_pci_device *vdev,
+   struct vfio_region_info *info,
struct vfio_info_cap *caps)
 {
-   struct vfio_info_cap_header *header;
struct vfio_region_info_cap_sparse_mmap *sparse;
size_t end, size;
-   int nr_areas = 2, i = 0;
+   int nr_areas = 2, i = 0, ret;
 
end = pci_resource_len(vdev->pdev, vdev->msix_bar);
 
@@ -494,13 +494,10 @@ static int msix_sparse_mmap_cap(struct vfio_pci_device 
*vdev,
 
size = sizeof(*sparse) + (nr_areas * sizeof(*sparse->areas));
 
-   header = vfio_info_cap_add(caps, size,
-  VFIO_REGION_INFO_CAP_SPARSE_MMAP, 1);
-   if (IS_ERR(header))
-   return PTR_ERR(header);
+   sparse = kzalloc(size, GFP_KERNEL);
+   if (!sparse)
+   return -ENOMEM;
 
-   sparse = container_of(header,
- struct vfio_region_info_cap_sparse_mmap, header);
sparse->nr_areas = nr_areas;
 
if (vdev->msix_offset & PAGE_MASK) {
@@ -516,24 +513,14 @@ static int msix_sparse_mmap_cap(struct vfio_pci_device 
*vdev,
i++;
}
 
-   return 0;
-}
-
-static int region_type_cap(struct vfio_pci_device *vdev,
-  struct vfio_info_cap *caps,
-  unsigned int type, unsigned int subtype)
-{
-   struct vfio_info_cap_header *header;
-   struct vfio_region_info_cap_type *cap;
+   info->flags |= VFIO_REGION_INFO_FLAG_CAPS;
 
-   header = vfio_info_cap_add(caps, sizeof(*cap),
-  VFIO_REGION_INFO_CAP_TYPE, 1);
-   if (IS_ERR(header))
-   return PTR_ERR(header);
+   ret = vfio_info_add_capability(info, caps,
+ VFIO_REGION_INFO_CAP_SPARSE_MMAP, sparse);
+   kfree(sparse);
 
-   cap = container_of(header, struct vfio_region_info_cap_type, header);
-   cap->type = type;
-   cap->subtype = subtype;
+   if (ret)
+   return ret;
 
return 0;
 }
@@ -628,7 +615,8 @@ static long vfio_pci_ioctl(void *device_data,
IORESOURCE_MEM && info.size >= PAGE_SIZE) {
info.flags |= VFIO_REGION_INFO_FLAG_MMAP;
if (info.index == vdev->msix_bar) {
-   ret = msix_sparse_mmap_cap(vdev, );
+   ret = msix_sparse_mmap_cap(vdev, ,
+  );
if (ret)
return ret;
}
@@ -676,6 +664,9 @@ static long vfio_pci_ioctl(void *device_data,
 
break;
default:
+   {
+   struct vfio_region_info_cap_type cap_type;
+
if (info.index >=
VFIO_PCI_NUM_REGIONS + vdev->num_regions)
return -EINVAL;
@@ -684,29 +675,26 @@ static long vfio_pci_ioctl(void *device_data,
 
info.offset = VFIO_PCI_INDEX_TO_OFFSET(info.index);
info.size = vdev->region[i].size;
-   info.flags = vdev->region[i].flags;
+   info.flags = vdev->region[i].flags |
+VFIO_REGION_INFO_FLAG_CAPS;
 
-   ret = region_type_cap(vdev, ,
- vdev->region[i].type,
- vdev->region[i].subtype);
+   cap_type.type = vdev->region[i].type;
+   cap_type.subtype = vdev->region[i].subtype;
+
+   ret = vfio_info_add_capability(, ,
+ VFIO_REGION_INFO_CAP_TYPE,
+ _type);
if (ret)
return ret;
+
+   }
}
 
-   if (caps.size) {
-   

[Qemu-devel] [PATCH v8 3/6] vfio iommu: Add support for mediated devices

2016-10-10 Thread Kirti Wankhede
VFIO IOMMU drivers are designed for the devices which are IOMMU capable.
Mediated device only uses IOMMU APIs, the underlying hardware can be
managed by an IOMMU domain.

Aim of this change is:
- To use most of the code of TYPE1 IOMMU driver for mediated devices
- To support direct assigned device and mediated device in single module

Added two new callback functions to struct vfio_iommu_driver_ops. Backend
IOMMU module that supports pining and unpinning pages for mdev devices
should provide these functions.
Added APIs for pining and unpining pages to VFIO module. These calls back
into backend iommu module to actually pin and unpin pages.

This change adds pin and unpin support for mediated device to TYPE1 IOMMU
backend module. More details:
- When iommu_group of mediated devices is attached, task structure is
  cached which is used later to pin pages and page accounting.
- It keeps track of pinned pages for mediated domain. This data is used to
  verify unpinning request and to unpin remaining pages while detaching, if
  there are any.
- Used existing mechanism for page accounting. If iommu capable domain
  exist in the container then all pages are already pinned and accounted.
  Accouting for mdev device is only done if there is no iommu capable
  domain in the container.
- Page accouting is updated on hot plug and unplug mdev device and pass
  through device.

Tested by assigning below combinations of devices to a single VM:
- GPU pass through only
- vGPU device only
- One GPU pass through and one vGPU device
- Linux VM hot plug and unplug vGPU device while GPU pass through device
  exist
- Linux VM hot plug and unplug GPU pass through device while vGPU device
  exist

Signed-off-by: Kirti Wankhede 
Signed-off-by: Neo Jia 
Change-Id: I295d6f0f2e0579b8d9882bfd8fd5a4194b97bd9a
---
 drivers/vfio/vfio.c | 117 +++
 drivers/vfio/vfio_iommu_type1.c | 685 ++--
 include/linux/vfio.h|  13 +-
 3 files changed, 724 insertions(+), 91 deletions(-)

diff --git a/drivers/vfio/vfio.c b/drivers/vfio/vfio.c
index 6fd6fa5469de..e3e342861e04 100644
--- a/drivers/vfio/vfio.c
+++ b/drivers/vfio/vfio.c
@@ -1782,6 +1782,123 @@ void vfio_info_cap_shift(struct vfio_info_cap *caps, 
size_t offset)
 }
 EXPORT_SYMBOL_GPL(vfio_info_cap_shift);
 
+static struct vfio_group *vfio_group_from_dev(struct device *dev)
+{
+   struct vfio_device *device;
+   struct vfio_group *group;
+   int ret;
+
+   device = vfio_device_get_from_dev(dev);
+   if (!device)
+   return ERR_PTR(-EINVAL);
+
+   group = device->group;
+   if (!atomic_inc_not_zero(>container_users)) {
+   ret = -EINVAL;
+   goto err_ret;
+   }
+
+   if (group->noiommu) {
+   atomic_dec(>container_users);
+   ret = -EPERM;
+   goto err_ret;
+   }
+
+   if (!group->container->iommu_driver ||
+   !vfio_group_viable(group)) {
+   atomic_dec(>container_users);
+   ret = -EINVAL;
+   goto err_ret;
+   }
+
+   vfio_device_put(device);
+   return group;
+
+err_ret:
+   vfio_device_put(device);
+   return ERR_PTR(ret);
+}
+
+/*
+ * Pin a set of guest PFNs and return their associated host PFNs for local
+ * domain only.
+ * @dev [in] : device
+ * @user_pfn [in]: array of user/guest PFNs
+ * @npage [in]: count of array elements
+ * @prot [in] : protection flags
+ * @phys_pfn[out] : array of host PFNs
+ */
+long vfio_pin_pages(struct device *dev, unsigned long *user_pfn,
+   long npage, int prot, unsigned long *phys_pfn)
+{
+   struct vfio_container *container;
+   struct vfio_group *group;
+   struct vfio_iommu_driver *driver;
+   ssize_t ret = -EINVAL;
+
+   if (!dev || !user_pfn || !phys_pfn)
+   return -EINVAL;
+
+   group = vfio_group_from_dev(dev);
+   if (IS_ERR(group))
+   return PTR_ERR(group);
+
+   container = group->container;
+   if (IS_ERR(container))
+   return PTR_ERR(container);
+
+   down_read(>group_lock);
+
+   driver = container->iommu_driver;
+   if (likely(driver && driver->ops->pin_pages))
+   ret = driver->ops->pin_pages(container->iommu_data, user_pfn,
+npage, prot, phys_pfn);
+
+   up_read(>group_lock);
+   vfio_group_try_dissolve_container(group);
+
+   return ret;
+
+}
+EXPORT_SYMBOL(vfio_pin_pages);
+
+/*
+ * Unpin set of host PFNs for local domain only.
+ * @dev [in] : device
+ * @pfn [in] : array of host PFNs to be unpinned.
+ * @npage [in] :count of elements in array, that is number of pages.
+ */
+long vfio_unpin_pages(struct device *dev, unsigned long *pfn, long npage)
+{
+   struct vfio_container *container;
+   struct vfio_group *group;
+   struct vfio_iommu_driver *driver;
+   ssize_t ret = 

[Qemu-devel] [PATCH v8 1/6] vfio: Mediated device Core driver

2016-10-10 Thread Kirti Wankhede
Design for Mediated Device Driver:
Main purpose of this driver is to provide a common interface for mediated
device management that can be used by different drivers of different
devices.

This module provides a generic interface to create the device, add it to
mediated bus, add device to IOMMU group and then add it to vfio group.

Below is the high Level block diagram, with Nvidia, Intel and IBM devices
as example, since these are the devices which are going to actively use
this module as of now.

 +---+
 |   |
 | +---+ |  mdev_register_driver() +--+
 | |   | +<+ __init() |
 | |  mdev | | |  |
 | |  bus  | +>+  |<-> VFIO user
 | |  driver   | | probe()/remove()| vfio_mdev.ko |APIs
 | |   | | |  |
 | +---+ | +--+
 |   |
 |  MDEV CORE|
 |   MODULE  |
 |   mdev.ko |
 | +---+ |  mdev_register_device() +--+
 | |   | +<+  |
 | |   | | |  nvidia.ko   |<-> physical
 | |   | +>+  |device
 | |   | |callback +--+
 | | Physical  | |
 | |  device   | |  mdev_register_device() +--+
 | | interface | |<+  |
 | |   | | |  i915.ko |<-> physical
 | |   | +>+  |device
 | |   | |callback +--+
 | |   | |
 | |   | |  mdev_register_device() +--+
 | |   | +<+  |
 | |   | | | ccw_device.ko|<-> physical
 | |   | +>+  |device
 | |   | |callback +--+
 | +---+ |
 +---+

Core driver provides two types of registration interfaces:
1. Registration interface for mediated bus driver:

/**
  * struct mdev_driver - Mediated device's driver
  * @name: driver name
  * @probe: called when new device created
  * @remove:called when device removed
  * @driver:device driver structure
  *
  **/
struct mdev_driver {
 const char *name;
 int  (*probe)  (struct device *dev);
 void (*remove) (struct device *dev);
 struct device_driverdriver;
};

Mediated bus driver for mdev device should use this interface to register
and unregister with core driver respectively:

int  mdev_register_driver(struct mdev_driver *drv, struct module *owner);
void mdev_unregister_driver(struct mdev_driver *drv);

Medisted bus driver is responsible to add/delete mediated devices to/from
VFIO group when devices are bound and unbound to the driver.

2. Physical device driver interface
This interface provides vendor driver the set APIs to manage physical
device related work in its driver. APIs are :

* dev_attr_groups: attributes of the parent device.
* mdev_attr_groups: attributes of the mediated device.
* supported_type_groups: attributes to define supported type. This is
 mandatory field.
* create: to allocate basic resources in driver for a mediated device.
* remove: to free resources in driver when mediated device is destroyed.
* open: open callback of mediated device
* release: release callback of mediated device
* read : read emulation callback.
* write: write emulation callback.
* mmap: mmap emulation callback.
* ioctl: ioctl callback.

Drivers should use these interfaces to register and unregister device to
mdev core driver respectively:

extern int  mdev_register_device(struct device *dev,
 const struct parent_ops *ops);
extern void mdev_unregister_device(struct device *dev);

There are no locks to serialize above callbacks in mdev driver and
vfio_mdev driver. If required, vendor driver can have locks to serialize
above APIs in their driver.

Signed-off-by: Kirti Wankhede 
Signed-off-by: Neo Jia 
Change-Id: I73a5084574270b14541c529461ea2f03c292d510
---
 drivers/vfio/Kconfig |   1 +
 drivers/vfio/Makefile|   1 +
 drivers/vfio/mdev/Kconfig|  12 ++
 drivers/vfio/mdev/Makefile   |   5 +
 drivers/vfio/mdev/mdev_core.c| 363 +++
 drivers/vfio/mdev/mdev_driver.c  | 131 ++
 drivers/vfio/mdev/mdev_private.h |  41 +
 drivers/vfio/mdev/mdev_sysfs.c   | 295 +++
 include/linux/mdev.h | 178 +++
 9 files changed, 1027 insertions(+)
 create mode 100644 drivers/vfio/mdev/Kconfig
 create mode 100644 drivers/vfio/mdev/Makefile
 create mode 100644 drivers/vfio/mdev/mdev_core.c
 create mode 

[Qemu-devel] [PATCH v8 0/6] Add Mediated device support

2016-10-10 Thread Kirti Wankhede
This series adds Mediated device support to Linux host kernel. Purpose
of this series is to provide a common interface for mediated device
management that can be used by different devices. This series introduces
Mdev core module that creates and manages mediated devices, VFIO based
driver for mediated devices that are created by mdev core module and
update VFIO type1 IOMMU module to support pinning & unpinning for mediated
devices.

This change uses uuid_le_to_bin() to parse UUID string and convert to bin.
This requires following commits from linux master branch:
* commit bc9dc9d5eec908806f1b15c9ec2253d44dcf7835 :
lib/uuid.c: use correct offset in uuid parser
* commit 2b1b0d66704a8cafe83be7114ec4c15ab3a314ad :
lib/uuid.c: introduce a few more generic helpers

Requires below commits from linux master branch for mmap region fault
handler that uses remap_pfn_range() to setup EPT properly.
* commit add6a0cd1c5ba51b201e1361b05a5df817083618
KVM: MMU: try to fix up page faults before giving up
* commit 92176a8ede577d0ff78ab3298e06701f67ad5f51 :
KVM: MMU: prepare to support mapping of VM_IO and VM_PFNMAP frames

What changed in v8?
mdev-core:
- Removed start/stop or online/offline interfaces.
- Added open() and close() interfaces that should be used to commit
  resources for mdev devices from vendor driver.
- Removed supported_config callback function. Introduced sysfs interface
  for 'mdev_supported_types' as discussed earlier. It is mandatory to
  provide supported types by vendor driver.
- Removed 'mdev_create' and 'mdev_destroy' sysfs files from device's
  directory. Added 'create' file in each supported type group that vendor
  driver would define. Added 'remove' file in mdev device directory to
  destroy mdev device.

vfio_mdev:
- Added ioctl() callback. All ioctls should be handled in vendor driver
- Common functions for SET_IRQS and GET_REGION_INFO ioctls are added to
  reduce code duplication in vendor drivers.
- This forms a shim layer that pass through VFIO devices operations to
  vendor driver for mediated devices.

vfio_iommu_type1:
- Handled the case if all devices attached to the normal IOMMU API domain
  go away and mdev device still exist in domain. Updated page accounting
  for local domain.
- Similarly if device is attached to normal IOMMU API domain, mappings are
  establised and page accounting is updated accordingly.
- Tested hot-plug and hot-unplug of vGPU and GPU pass through device with
  Linux VM.

Documentation:
- Updated vfio-mediated-device.txt with current interface.
- Added sample driver that simulates serial port over PCI card for a VM.
  This driver is added as an example for how to use mediated device
  framework.
- Moved updated document and example driver to 'vfio-mdev' directory in
  Documentation.


Kirti Wankhede (6):
  vfio: Mediated device Core driver
  vfio: VFIO based driver for Mediated devices
  vfio iommu: Add support for mediated devices
  docs: Add Documentation for Mediated devices
  Add simple sample driver for mediated device framework
  Add common functions for SET_IRQS and GET_REGION_INFO ioctls

 Documentation/vfio-mdev/Makefile |   14 +
 Documentation/vfio-mdev/mtty.c   | 1353 ++
 Documentation/vfio-mdev/vfio-mediated-device.txt |  282 +
 drivers/vfio/Kconfig |1 +
 drivers/vfio/Makefile|1 +
 drivers/vfio/mdev/Kconfig|   18 +
 drivers/vfio/mdev/Makefile   |6 +
 drivers/vfio/mdev/mdev_core.c|  363 ++
 drivers/vfio/mdev/mdev_driver.c  |  131 +++
 drivers/vfio/mdev/mdev_private.h |   41 +
 drivers/vfio/mdev/mdev_sysfs.c   |  295 +
 drivers/vfio/mdev/vfio_mdev.c|  171 +++
 drivers/vfio/pci/vfio_pci.c  |  103 +-
 drivers/vfio/pci/vfio_pci_private.h  |6 +-
 drivers/vfio/vfio.c  |  233 
 drivers/vfio/vfio_iommu_type1.c  |  685 +--
 include/linux/mdev.h |  178 +++
 include/linux/vfio.h |   20 +-
 18 files changed, 3743 insertions(+), 158 deletions(-)
 create mode 100644 Documentation/vfio-mdev/Makefile
 create mode 100644 Documentation/vfio-mdev/mtty.c
 create mode 100644 Documentation/vfio-mdev/vfio-mediated-device.txt
 create mode 100644 drivers/vfio/mdev/Kconfig
 create mode 100644 drivers/vfio/mdev/Makefile
 create mode 100644 drivers/vfio/mdev/mdev_core.c
 create mode 100644 drivers/vfio/mdev/mdev_driver.c
 create mode 100644 drivers/vfio/mdev/mdev_private.h
 create mode 100644 drivers/vfio/mdev/mdev_sysfs.c
 create mode 100644 drivers/vfio/mdev/vfio_mdev.c
 create mode 100644 include/linux/mdev.h

-- 
2.7.0




[Qemu-devel] [PATCH v8 2/6] vfio: VFIO based driver for Mediated devices

2016-10-10 Thread Kirti Wankhede
vfio_mdev driver registers with mdev core driver.
MDEV core driver creates mediated device and calls probe routine of
vfio_mdev driver for each device.
Probe routine of vfio_mdev driver adds mediated device to VFIO core module

This driver forms a shim layer that pass through VFIO devices operations
to vendor driver for mediated devices.

Signed-off-by: Kirti Wankhede 
Signed-off-by: Neo Jia 
Change-Id: I583f4734752971d3d112324d69e2508c88f359ec
---
 drivers/vfio/mdev/Kconfig   |   6 ++
 drivers/vfio/mdev/Makefile  |   1 +
 drivers/vfio/mdev/vfio_mdev.c   | 171 
 drivers/vfio/pci/vfio_pci_private.h |   6 +-
 4 files changed, 181 insertions(+), 3 deletions(-)
 create mode 100644 drivers/vfio/mdev/vfio_mdev.c

diff --git a/drivers/vfio/mdev/Kconfig b/drivers/vfio/mdev/Kconfig
index 23d5b9d08a5c..e1b23697261d 100644
--- a/drivers/vfio/mdev/Kconfig
+++ b/drivers/vfio/mdev/Kconfig
@@ -9,4 +9,10 @@ config VFIO_MDEV
 
 If you don't know what do here, say N.
 
+config VFIO_MDEV_DEVICE
+tristate "VFIO support for Mediated devices"
+depends on VFIO && VFIO_MDEV
+default n
+help
+VFIO based driver for mediated devices.
 
diff --git a/drivers/vfio/mdev/Makefile b/drivers/vfio/mdev/Makefile
index 56a75e689582..e5087ed83a34 100644
--- a/drivers/vfio/mdev/Makefile
+++ b/drivers/vfio/mdev/Makefile
@@ -2,4 +2,5 @@
 mdev-y := mdev_core.o mdev_sysfs.o mdev_driver.o
 
 obj-$(CONFIG_VFIO_MDEV) += mdev.o
+obj-$(CONFIG_VFIO_MDEV_DEVICE) += vfio_mdev.o
 
diff --git a/drivers/vfio/mdev/vfio_mdev.c b/drivers/vfio/mdev/vfio_mdev.c
new file mode 100644
index ..1efc3f309510
--- /dev/null
+++ b/drivers/vfio/mdev/vfio_mdev.c
@@ -0,0 +1,171 @@
+/*
+ * VFIO based driver for Mediated device
+ *
+ * Copyright (c) 2016, NVIDIA CORPORATION. All rights reserved.
+ * Author: Neo Jia 
+ *Kirti Wankhede 
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include "mdev_private.h"
+
+#define DRIVER_VERSION  "0.1"
+#define DRIVER_AUTHOR   "NVIDIA Corporation"
+#define DRIVER_DESC "VFIO based driver for Mediated device"
+
+struct vfio_mdev {
+   struct iommu_group *group;
+   struct mdev_device *mdev;
+};
+
+static int vfio_mdev_open(void *device_data)
+{
+   struct vfio_mdev *vmdev = device_data;
+   struct parent_device *parent = vmdev->mdev->parent;
+   int ret;
+
+   if (unlikely(!parent->ops->open))
+   return -EINVAL;
+
+   if (!try_module_get(THIS_MODULE))
+   return -ENODEV;
+
+   ret = parent->ops->open(vmdev->mdev);
+   if (ret)
+   module_put(THIS_MODULE);
+
+   return ret;
+}
+
+static void vfio_mdev_release(void *device_data)
+{
+   struct vfio_mdev *vmdev = device_data;
+   struct parent_device *parent = vmdev->mdev->parent;
+
+   if (parent->ops->release)
+   parent->ops->release(vmdev->mdev);
+
+   module_put(THIS_MODULE);
+}
+
+static long vfio_mdev_unlocked_ioctl(void *device_data,
+unsigned int cmd, unsigned long arg)
+{
+   struct vfio_mdev *vmdev = device_data;
+   struct parent_device *parent = vmdev->mdev->parent;
+
+   if (unlikely(!parent->ops->ioctl))
+   return -EINVAL;
+
+   return parent->ops->ioctl(vmdev->mdev, cmd, arg);
+}
+
+static ssize_t vfio_mdev_read(void *device_data, char __user *buf,
+ size_t count, loff_t *ppos)
+{
+   struct vfio_mdev *vmdev = device_data;
+   struct parent_device *parent = vmdev->mdev->parent;
+
+   if (unlikely(!parent->ops->read))
+   return -EINVAL;
+
+   return parent->ops->read(vmdev->mdev, buf, count, *ppos);
+}
+
+static ssize_t vfio_mdev_write(void *device_data, const char __user *buf,
+  size_t count, loff_t *ppos)
+{
+   struct vfio_mdev *vmdev = device_data;
+   struct parent_device *parent = vmdev->mdev->parent;
+
+   if (unlikely(!parent->ops->write))
+   return -EINVAL;
+
+   return parent->ops->write(vmdev->mdev, (char *)buf, count, *ppos);
+}
+
+static int vfio_mdev_mmap(void *device_data, struct vm_area_struct *vma)
+{
+   struct vfio_mdev *vmdev = device_data;
+   struct parent_device *parent = vmdev->mdev->parent;
+
+   if (unlikely(!parent->ops->mmap))
+   return -EINVAL;
+
+   return parent->ops->mmap(vmdev->mdev, vma);
+}
+
+static const struct vfio_device_ops vfio_mdev_dev_ops = {
+   .name   = "vfio-mdev",
+   .open   = vfio_mdev_open,
+   .release= vfio_mdev_release,
+   .ioctl   

Re: [Qemu-devel] [PATCH] colo-proxy: fix memory leak

2016-10-10 Thread Eric Blake
On 10/10/2016 04:46 AM, Zhang Chen wrote:
> Fix memory leak in colo-compare.c and filter-rewriter.c
> Report by Coverity.
> 
> Signed-off-by: Zhang Chen 
> ---
>  net/colo-compare.c| 4 ++--
>  net/filter-rewriter.c | 9 +
>  2 files changed, 7 insertions(+), 6 deletions(-)
> 
> diff --git a/net/colo-compare.c b/net/colo-compare.c
> index 22b1da1..47e7ebe 100644
> --- a/net/colo-compare.c
> +++ b/net/colo-compare.c
> @@ -235,8 +235,8 @@ static int colo_packet_compare_tcp(Packet *spkt, Packet 
> *ppkt)
>  fprintf(stderr, "Secondary len = %d\n", spkt->size);
>  qemu_hexdump((char *)spkt->data, stderr, "colo-compare", spkt->size);
>  
> -g_free(sdebug);
> -g_free(ddebug);
> +free(sdebug);
> +free(ddebug);

Uggh. We should fix the original allocations of sdebug and ddebug to use
g_strdup_printf() rather than strdup(), so that we are using a
consistent allocator throughout.  Or even improve this code to use
traces instead of fprintf.


-- 
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 03/24] qmp: teach qmp_dispatch() to take a pre-filled QDict

2016-10-10 Thread Eric Blake
On 10/10/2016 04:22 AM, Marc-André Lureau wrote:
> Give an optionnal qdict for the dispatch call to be used for the

s/optionnal/optional/

> reply. The qemu monitor has the request "id" pre-filled, simplifying a
> bit the code.

simplifying the code a bit

> 
> Signed-off-by: Marc-André Lureau 
> ---
>  monitor.c   | 26 +++---
>  qapi/qmp-dispatch.c |  5 ++---
>  qga/main.c  |  2 +-
>  tests/test-qmp-commands.c   |  8 
>  include/qapi/qmp/dispatch.h |  2 +-
>  5 files changed, 19 insertions(+), 24 deletions(-)
> 

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] rbd: shift byte count as a 64-bit value

2016-10-10 Thread Eric Blake
On 10/10/2016 03:00 PM, Paolo Bonzini wrote:
> Otherwise, reads of more than 2GB fail.  Until commit
> 7bbca9e290a9c7c217b5a24fc6094e91e54bd05d, reads of 2^41
> bytes succeeded at least theoretically.

I don't think the block layer ever passes a read request down to drivers
that large in the first place; we have a lot of code in io.c that
clamps/asserts things to be smaller than 2^31 (ie.
BDRV_REQUEST_MAX_SECTORS).  So this may just be cleaning up something
that would become latent with a future block change to support larger
sizes, rather than an actual bug fix today.  But it is worth making.

> In fact, pdiscard ought to receive a 64-bit integer as the
> count for the same reason.

Yes, auditing ALL of the block code for eliminating arbitrary 32-bit
limits would be a big project.

> 
> Reported by Coverity.
> 
> Fixes: 7bbca9e290a9c7c217b5a24fc6094e91e54bd05d
> Cc: qemu-sta...@nongnu.org
> Cc: kw...@redhat.com
> Cc: ebl...@redhat.com
> Signed-off-by: Paolo Bonzini 
> ---
>  block/rbd.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)

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 18/29] target-sparc: use SparcV9MMU type for sparc64 I/D-MMUs

2016-10-10 Thread Richard Henderson

On 10/01/2016 05:05 AM, Artyom Tarasenko wrote:

-//typedef struct SparcMMU
-union {
-uint64_t immuregs[16];
-struct {
-uint64_t tsb_tag_target;
-uint64_t unused_mmu_primary_context;   // use DMMU
-uint64_t unused_mmu_secondary_context; // use DMMU
-uint64_t sfsr;
-uint64_t sfar;
-uint64_t tsb;
-uint64_t tag_access;
-uint64_t virtual_watchpoint;
-uint64_t physical_watchpoint;
-} immu;
-};
-union {
-uint64_t dmmuregs[16];
-struct {
-uint64_t tsb_tag_target;
-uint64_t mmu_primary_context;
-uint64_t mmu_secondary_context;
-uint64_t sfsr;
-uint64_t sfar;
-uint64_t tsb;
-uint64_t tag_access;
-uint64_t virtual_watchpoint;
-uint64_t physical_watchpoint;
-} dmmu;
-};
+SparcV9MMU immu;
+SparcV9MMU dmmu;


You'll find this broke sparc64-linux-user.  You'll need

diff --git a/linux-user/main.c b/linux-user/main.c
index bb48260..b1f1347 100644
--- a/linux-user/main.c
+++ b/linux-user/main.c
@@ -1163,7 +1163,7 @@ void cpu_loop (CPUSPARCState *env)
 /* XXX: check env->error_code */
 info.si_code = TARGET_SEGV_MAPERR;
 if (trapnr == TT_DFAULT)
-info._sifields._sigfault._addr = env->dmmuregs[4];
+info._sifields._sigfault._addr = env->dmmu.mmuregs[4];
 else
 info._sifields._sigfault._addr = cpu_tsptr(env)->tpc;
 queue_signal(env, info.si_signo, QEMU_SI_FAULT, );


r~



Re: [Qemu-devel] [PATCH 02/24] tests: change /0.15/* tests to /qmp/*

2016-10-10 Thread Eric Blake
On 10/10/2016 04:22 AM, Marc-André Lureau wrote:
> Presumably 0.15 was the version it was first introduced, but
> qmp keeps evolving. There is no point in having that version
> as test prefix, 'qmp' makes more sense here.
> 
> Signed-off-by: Marc-André Lureau 
> ---
>  tests/test-qmp-commands.c | 10 +-
>  1 file changed, 5 insertions(+), 5 deletions(-)

Reviewed-by: Eric Blake 

> 
> diff --git a/tests/test-qmp-commands.c b/tests/test-qmp-commands.c
> index 81cbe54..5ff2f9b 100644
> --- a/tests/test-qmp-commands.c
> +++ b/tests/test-qmp-commands.c
> @@ -267,11 +267,11 @@ int main(int argc, char **argv)
>  {
>  g_test_init(, , NULL);
>  
> -g_test_add_func("/0.15/dispatch_cmd", test_dispatch_cmd);
> -g_test_add_func("/0.15/dispatch_cmd_failure", test_dispatch_cmd_failure);
> -g_test_add_func("/0.15/dispatch_cmd_io", test_dispatch_cmd_io);
> -g_test_add_func("/0.15/dealloc_types", test_dealloc_types);
> -g_test_add_func("/0.15/dealloc_partial", test_dealloc_partial);
> +g_test_add_func("/qmp/dispatch_cmd", test_dispatch_cmd);
> +g_test_add_func("/qmp/dispatch_cmd_failure", test_dispatch_cmd_failure);
> +g_test_add_func("/qmp/dispatch_cmd_io", test_dispatch_cmd_io);
> +g_test_add_func("/qmp/dealloc_types", test_dealloc_types);
> +g_test_add_func("/qmp/dealloc_partial", test_dealloc_partial);
>  
>  module_call_init(MODULE_INIT_QAPI);
>  g_test_run();
> 

-- 
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 01/24] tests: start generic qemu-qmp tests

2016-10-10 Thread Eric Blake
On 10/10/2016 04:22 AM, Marc-André Lureau wrote:
> These 2 tests exhibit two qmp bugs fixed by the previous patches.

It looks like this is a respin because it was removed from an earlier
pull request; now that the previous patches mentioned have landed and
this is no longer immediately adjacent to those patches, is it worth
tweaking the commit message to call out commit ids?

> 
> Signed-off-by: Marc-André Lureau 
> Reviewed-by: Daniel P. Berrange 
> Reviewed-by: Eric Blake 
> Message-Id: <20160922203927.28241-4-marcandre.lur...@redhat.com>
> [Rename tests/test-qemu-qmp.c to tests/qmp-test.c, cover it in
> MAINTAINERS, add a file comment]
> Reviewed-by: Markus Armbruster 
> Signed-off-by: Markus Armbruster 

Since you reworked the patch to get it to compile, you may want to drop
the [] parenthetical inserted by Markus, as this is now a new revision
of your patch (although crediting him for contributions is still
appropriate).

> +++ b/tests/qmp-test.c
> @@ -0,0 +1,79 @@
> +/*
> + * QTest testcase for QMP
> + *
> + * Copyright (c) 2016 Red Hat, Inc.

We aren't very consistent on (C) vs. (c). I don't know if a lawyer would
complain.

-- 
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 0/2] qemu-img: change img_open() and opening method in dd

2016-10-10 Thread Max Reitz
On 07.10.2016 17:36, Reda Sallahi wrote:
> The patch series include the previously submitted patch:
> [PATCH v5] qemu-img: change opening method for the output in dd
> 
> Changes from v5:
> * Replace access() with a modified version of img_open() in img_dd()
> 
> Depends on:
> [PATCH v3] qemu-img: add conv=notrunc option to dd

BTW, I didn't apply this patch so far because it actually doesn't
implement conv=notrunc. It makes specifying it mandatory, but when you
do, the output is still going to be truncated.

Patch 2 of this series fixes this, and while one normally says that
everything should work between patches as intended and not just after
applying the whole series, I'm fine with just applying that patch and
this series (once I think it's good) together as they are, because
getting everything in the right order would actually be rather
difficult, I think.

Also, you could argue that that patch fixes the "qemu-img dd does not
require conv=notrunc" "bug" and then patch 2 of this series fixes the
"qemu-img dd should not truncate an existing output file" "bug". So
since it is possible to argue that these patches fix two distinct bugs,
I think it should be fine to keep it broken in between...

tl;dr: I'll apply said patch together with this series once the latter
is in a good state.

Max

> Reda Sallahi (2):
>   qemu-img: make img_open() able to surpress file opening errors
>   qemu-img: change opening method for the output in dd
> 
>  qemu-img.c | 176 
> +
>  1 file changed, 106 insertions(+), 70 deletions(-)
> 




signature.asc
Description: OpenPGP digital signature


[Qemu-devel] [PATCH] rbd: shift byte count as a 64-bit value

2016-10-10 Thread Paolo Bonzini
Otherwise, reads of more than 2GB fail.  Until commit
7bbca9e290a9c7c217b5a24fc6094e91e54bd05d, reads of 2^41
bytes succeeded at least theoretically.

In fact, pdiscard ought to receive a 64-bit integer as the
count for the same reason.

Reported by Coverity.

Fixes: 7bbca9e290a9c7c217b5a24fc6094e91e54bd05d
Cc: qemu-sta...@nongnu.org
Cc: kw...@redhat.com
Cc: ebl...@redhat.com
Signed-off-by: Paolo Bonzini 
---
 block/rbd.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/block/rbd.c b/block/rbd.c
index 6f9eb6f..f6e1d4b 100644
--- a/block/rbd.c
+++ b/block/rbd.c
@@ -733,7 +733,7 @@ static BlockAIOCB *qemu_rbd_aio_readv(BlockDriverState *bs,
   void *opaque)
 {
 return rbd_start_aio(bs, sector_num << BDRV_SECTOR_BITS, qiov,
- nb_sectors << BDRV_SECTOR_BITS, cb, opaque,
+ (int64_t) nb_sectors << BDRV_SECTOR_BITS, cb, opaque,
  RBD_AIO_READ);
 }
 
@@ -745,7 +745,7 @@ static BlockAIOCB *qemu_rbd_aio_writev(BlockDriverState *bs,
void *opaque)
 {
 return rbd_start_aio(bs, sector_num << BDRV_SECTOR_BITS, qiov,
- nb_sectors << BDRV_SECTOR_BITS, cb, opaque,
+ (int64_t) nb_sectors << BDRV_SECTOR_BITS, cb, opaque,
  RBD_AIO_WRITE);
 }
 
-- 
2.7.4




Re: [Qemu-devel] [PATCH v6 01/12] virtio-crypto: introduce virtio_crypto.h

2016-10-10 Thread Eric Blake
On 10/10/2016 03:43 AM, Gonglei wrote:
> Introduce the virtio_crypto.h which follows
> virtio-crypto specification.
> 
> Signed-off-by: Gonglei 
> ---
>  include/standard-headers/linux/virtio_crypto.h | 381 
> +
>  1 file changed, 381 insertions(+)
>  create mode 100644 include/standard-headers/linux/virtio_crypto.h
> 
> diff --git a/include/standard-headers/linux/virtio_crypto.h 
> b/include/standard-headers/linux/virtio_crypto.h
> new file mode 100644
> index 000..a62d192
> --- /dev/null
> +++ b/include/standard-headers/linux/virtio_crypto.h
> @@ -0,0 +1,381 @@
> +#ifndef _VIRTIO_CRYPTO_H
> +#define _VIRTIO_CRYPTO_H

Missing a copyright and license notice.  Is this header copied from
Linux, or is it new to qemu?  At any rate, being explicit about your
license is important (in qemu, files without a license are GPLv2+;
whereas the Linux kernel tends to favor GPLv2-only).

> +
> +#include "standard-headers/linux/types.h"
> +#include "standard-headers/linux/virtio_config.h"
> +#include "standard-headers/linux/virtio_types.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)

Technically over-parenthesized, but doesn't hurt.

> +
> +#define VIRTIO_CRYPTO_OPCODE(service, op)   ((service << 8) | (op))

Under-parenthesized.  You need more () around service.

-- 
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 v6 00/12] virtio-crypto: introduce framework and device emulation

2016-10-10 Thread Eric Blake
On 10/10/2016 03:43 AM, Gonglei wrote:
> The virtio crypto is a virtual crypto 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 handled by the real crypto accelerators finally.
> The second queue is the control queue used to create or
> destroy sessions for symmetric algorithms and control
> some advanced features in the future. The virtio crypto
> device provides the following crypto services: CIPHER,
> MAC, HASH, AEAD etc.
> 
> TODO:
>  - add vhost-user as a high performance cryptodev backend.
>  - more crypto services support.
>  - mirgration support.
> 
> Changes since v5:
>  - rebase the patch 14 in v5, using the correct at the beginning of whole 
> patch serials. [Eric]

s/serials/series/  ('series' is one of those weird English words that is
the same for both singular and plural usage)

>  - perfect algorithm chain support in patch 12.
>  - more friendly error handler in both controlq and dataq.
>  - drop patch "virtio-crypto: emulate virtio crypto as a legacy device by 
> default" because
>   we shouldn't support transitional virtio devices any more. [Michael] 
>  - drop patch "virtio-crypto-test: add qtest case for virtio-crypto" because
>   libqtest doesn't support virtio-1.0 device yet.
>  - rebase the patch set based on Michael's pull request:
> [PULL 00/33] virtio, pc: fixes and features
> 

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



signature.asc
Description: OpenPGP digital signature


Re: [Qemu-devel] [PATCH 2/2] qemu-img: change opening method for the output in dd

2016-10-10 Thread Max Reitz
On 07.10.2016 17:36, Reda Sallahi wrote:
> The subcommand dd was creating an output image regardless of whether there
> was one already created. With this patch we try to check first if the output
> image exists and resize it if necessary.
> 
> Signed-off-by: Reda Sallahi 
> ---
>  qemu-img.c | 124 
> ++---
>  tests/qemu-iotests/160 |   1 -
>  2 files changed, 75 insertions(+), 50 deletions(-)
> 
> diff --git a/qemu-img.c b/qemu-img.c
> index 6c088bd..9b590d4 100644
> --- a/qemu-img.c
> +++ b/qemu-img.c

[...]

> @@ -4083,31 +4053,87 @@ static int img_dd(int argc, char **argv)

[...]

>  blk2 = img_open(image_opts, out.filename, out_fmt, BDRV_O_RDWR,
> -false, false, true);
> +false, false, false);
>  
>  if (!blk2) {

[...]

> +} else {
> +int64_t blk2sz = 0;
> +
> +if (!(dd.conv & C_NOTRUNC)) {
> +/* We make conv=notrunc mandatory for the moment to avoid
> +   accidental destruction of the output image. Needs to be
> +   changed when a better solution is found */
> +error_report("conv=notrunc not specified");
> +ret = -1;
> +goto out;
> +}
> +
> +blk2sz = blk_getlength(blk2);
> +if (blk2sz < 0) {
> +error_report("Failed to get size for '%s'", in.filename);
> +ret = -1;
> +goto out;
> +}
> +
> +if (in.offset <= INT64_MAX / in.bsz && size >= in.offset * in.bsz) {
> +if (blk2sz < out_size) {

Looks good to me overall, but wouldn't the second condition (blk2sz <
out_size) suffice alone? In my opinion, dropping the first "if" would
make the code easier to read, too.

Max

> +blk_truncate(blk2, out_size);
> +}
> +}
>  }
>  
>  if (dd.flags & C_SKIP && (in.offset > INT64_MAX / in.bsz ||



signature.asc
Description: OpenPGP digital signature


Re: [Qemu-devel] [PATCH 2/2] hw/arm/virt: no ITS on older machine types

2016-10-10 Thread Auger Eric
Hi Drew,

On 10/10/2016 18:35, Andrew Jones wrote:
> We should avoid exposing new hardware (through DT and ACPI) on older
> machine types. This patch keeps 2.7 and older from changing, despite
> the introduction of ITS support for 2.8.
> 
> Signed-off-by: Andrew Jones 
> ---
> 
> As Eduardo pointed out long ago for a different reason, we should
> probably replace VirtGuestInfo with direct use of VirtMachineClass,
> like x86 has done to replace PcGuestInfo with direct use of
> PCMachineClass. I'll work on that, but wanted to get this ITS
> fixup in sooner than later, so I'm posting this patch now, which
> requires 'no_its' to be duplicated.
> 
> Also, this patch will have a trivial conflict with Wei's PMU
> property series.
> 
> 
>  hw/arm/virt-acpi-build.c |  2 +-
>  hw/arm/virt.c| 15 +++
>  include/hw/arm/virt-acpi-build.h |  1 +
>  3 files changed, 13 insertions(+), 5 deletions(-)
> 
> diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
> index c31349561c95..48820f372830 100644
> --- a/hw/arm/virt-acpi-build.c
> +++ b/hw/arm/virt-acpi-build.c
> @@ -556,7 +556,7 @@ build_madt(GArray *table_data, BIOSLinker *linker, 
> VirtGuestInfo *guest_info)
>  gicr->base_address = cpu_to_le64(memmap[VIRT_GIC_REDIST].base);
>  gicr->range_length = cpu_to_le32(memmap[VIRT_GIC_REDIST].size);
>  
> -if (its_class_name()) {
> +if (its_class_name() && !guest_info->no_its) {
>  gic_its = acpi_data_push(table_data, sizeof *gic_its);
>  gic_its->type = ACPI_APIC_GENERIC_TRANSLATOR;
>  gic_its->length = sizeof(*gic_its);
> diff --git a/hw/arm/virt.c b/hw/arm/virt.c
> index 0f6305d3c7f6..e3734d47df81 100644
> --- a/hw/arm/virt.c
> +++ b/hw/arm/virt.c
> @@ -84,6 +84,7 @@ typedef struct {
>  MachineClass parent;
>  VirtBoardInfo *daughterboard;
>  bool disallow_affinity_adjustment;
> +bool no_its;
>  } VirtMachineClass;
>  
>  typedef struct {
> @@ -552,7 +553,8 @@ static void create_v2m(VirtBoardInfo *vbi, qemu_irq *pic)
>  fdt_add_v2m_gic_node(vbi);
>  }
>  
> -static void create_gic(VirtBoardInfo *vbi, qemu_irq *pic, int type, bool 
> secure)
> +static void create_gic(VirtBoardInfo *vbi, qemu_irq *pic, int type,
> +   bool secure, bool no_its)
>  {
>  /* We create a standalone GIC */
>  DeviceState *gicdev;
> @@ -616,9 +618,9 @@ static void create_gic(VirtBoardInfo *vbi, qemu_irq *pic, 
> int type, bool secure)
>  
>  fdt_add_gic_node(vbi, type);
>  
> -if (type == 3) {
> +if (type == 3 && !no_its) {
>  create_its(vbi, gicdev);
> -} else {
> +} else if (type == 2) {
>  create_v2m(vbi, pic);
>  }
>  }
> @@ -1376,7 +1378,7 @@ static void machvirt_init(MachineState *machine)
>  
>  create_flash(vbi, sysmem, secure_sysmem ? secure_sysmem : sysmem);
>  
> -create_gic(vbi, pic, gic_version, vms->secure);
> +create_gic(vbi, pic, gic_version, vms->secure, vmc->no_its);
>  
>  fdt_add_pmu_nodes(vbi, gic_version);
>  
> @@ -1408,6 +1410,7 @@ static void machvirt_init(MachineState *machine)
>  guest_info->irqmap = vbi->irqmap;
>  guest_info->use_highmem = vms->highmem;
>  guest_info->gic_version = gic_version;
> +guest_info->no_its = vmc->no_its;
>  guest_info_state->machine_done.notify = virt_guest_info_machine_done;
>  qemu_add_machine_init_done_notifier(_info_state->machine_done);
>  
> @@ -1562,8 +1565,12 @@ static void virt_2_7_instance_init(Object *obj)
>  
>  static void virt_machine_2_7_options(MachineClass *mc)
>  {
> +VirtMachineClass *vmc = VIRT_MACHINE_CLASS(OBJECT_CLASS(mc));
> +
>  virt_machine_2_8_options(mc);
>  SET_MACHINE_COMPAT(mc, VIRT_COMPAT_2_7);
> +/* ITS was introduced with 2.8 */
> +vmc->no_its = true;
>  }
>  DEFINE_VIRT_MACHINE(2, 7)
>  
> diff --git a/include/hw/arm/virt-acpi-build.h 
> b/include/hw/arm/virt-acpi-build.h
> index e43330ad659b..f5ec749b8fea 100644
> --- a/include/hw/arm/virt-acpi-build.h
> +++ b/include/hw/arm/virt-acpi-build.h
> @@ -33,6 +33,7 @@ typedef struct VirtGuestInfo {
>  const int *irqmap;
>  bool use_highmem;
>  int gic_version;
> +bool no_its;
>  } VirtGuestInfo;
>  
>  
> 
Reviewed-by: Eric Auger 

Eric



Re: [Qemu-devel] [PATCH 1/2] qemu-img: make img_open() able to surpress file opening errors

2016-10-10 Thread Max Reitz
On 07.10.2016 17:36, Reda Sallahi wrote:
> Previously img_open() always printed file opening errors even if the intended
> action is just to check the file existence as is sometimes the case in
> qemu-img dd.
> 
> This adds an argument (openerror) to img_open() that specifies whether to 
> print
> an opening error or not.
> 
> Signed-off-by: Reda Sallahi 
> ---
>  qemu-img.c | 54 
> ++
>  tests/qemu-iotests/160 |  1 +
>  2 files changed, 33 insertions(+), 22 deletions(-)
> 
> diff --git a/qemu-img.c b/qemu-img.c
> index 219fd86..6c088bd 100644
> --- a/qemu-img.c
> +++ b/qemu-img.c
> @@ -269,7 +269,7 @@ static int img_open_password(BlockBackend *blk, const 
> char *filename,
>  
>  static BlockBackend *img_open_opts(const char *optstr,
> QemuOpts *opts, int flags, bool 
> writethrough,
> -   bool quiet)
> +   bool quiet, bool openerror)

I think it would be better to introduce an own function which checks
whether the image file exists or not. There are three reasons for this:

First, if we ever have the infrastructure to actually check this, using
it there is more straightforward. Not too good of a reason, though,
since that's a big "if".

Second, img_open_opts() invokes img_open_password(). We probably don't
want to do that if we just want to check the existence of an image file.
That's a better reason, and however you decide, you should make sure
that img_open_password() is not called when we just want to confirm the
existence of an image file.

And third, having an own function would make this patch simpler because
you don't have to change all the previous places that call img_open_opts().

So the new function (static bool img_check_existence(const char *optstr,
QemuOpts *opts, int flags) or something) would just invoke
qemu_opts_to_qdict(), blk_new_open() (with NULL as errp) and then return
false if blk_new_open() failed or call blk_unref() on the returned BB if
it succeeded and then return true.

Max

>  {
>  QDict *options;
>  Error *local_err = NULL;
> @@ -277,7 +277,9 @@ static BlockBackend *img_open_opts(const char *optstr,
>  options = qemu_opts_to_qdict(opts, NULL);
>  blk = blk_new_open(NULL, NULL, options, flags, _err);
>  if (!blk) {
> -error_reportf_err(local_err, "Could not open '%s': ", optstr);
> +if (openerror) {
> +error_reportf_err(local_err, "Could not open '%s': ", optstr);
> +}
>  return NULL;
>  }
>  blk_set_enable_write_cache(blk, !writethrough);
> @@ -291,7 +293,8 @@ static BlockBackend *img_open_opts(const char *optstr,
>  
>  static BlockBackend *img_open_file(const char *filename,
> const char *fmt, int flags,
> -   bool writethrough, bool quiet)
> +   bool writethrough, bool quiet,
> +   bool openerror)
>  {
>  BlockBackend *blk;
>  Error *local_err = NULL;
> @@ -304,7 +307,9 @@ static BlockBackend *img_open_file(const char *filename,
>  
>  blk = blk_new_open(filename, NULL, options, flags, _err);
>  if (!blk) {
> -error_reportf_err(local_err, "Could not open '%s': ", filename);
> +if (openerror) {
> +error_reportf_err(local_err, "Could not open '%s': ", filename);
> +}
>  return NULL;
>  }
>  blk_set_enable_write_cache(blk, !writethrough);
> @@ -320,7 +325,7 @@ static BlockBackend *img_open_file(const char *filename,
>  static BlockBackend *img_open(bool image_opts,
>const char *filename,
>const char *fmt, int flags, bool writethrough,
> -  bool quiet)
> +  bool quiet, bool openerror)
>  {
>  BlockBackend *blk;
>  if (image_opts) {
> @@ -334,9 +339,11 @@ static BlockBackend *img_open(bool image_opts,
>  if (!opts) {
>  return NULL;
>  }
> -blk = img_open_opts(filename, opts, flags, writethrough, quiet);
> +blk = img_open_opts(filename, opts, flags, writethrough, quiet,
> +openerror);
>  } else {
> -blk = img_open_file(filename, fmt, flags, writethrough, quiet);
> +blk = img_open_file(filename, fmt, flags, writethrough, quiet,
> +openerror);
>  }
>  return blk;
>  }
> @@ -706,7 +713,7 @@ static int img_check(int argc, char **argv)
>  return 1;
>  }
>  
> -blk = img_open(image_opts, filename, fmt, flags, writethrough, quiet);
> +blk = img_open(image_opts, filename, fmt, flags, writethrough, quiet, 
> true);
>  if (!blk) {
>  return 1;
>  }
> @@ -898,7 +905,7 @@ static int img_commit(int argc, char **argv)
>  return 1;
>  }
>  
> -

[Qemu-devel] [PATCH] target-i386: Unset cannot_destroy_with_object_finalize_yet

2016-10-10 Thread Eduardo Habkost
TYPE_X86_CPU now call cpu_exec_init() on realize, so we don't
need to set cannot_destroy_with_object_finalize_yet anymore.

Signed-off-by: Eduardo Habkost 
---
 target-i386/cpu.c | 5 -
 1 file changed, 5 deletions(-)

diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index 1e8127b..33129db 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -3575,11 +3575,6 @@ static void x86_cpu_common_class_init(ObjectClass *oc, 
void *data)
 cc->cpu_exec_exit = x86_cpu_exec_exit;
 
 dc->cannot_instantiate_with_device_add_yet = false;
-/*
- * Reason: x86_cpu_initfn() calls cpu_exec_init(), which saves the
- * object in cpus -> dangling pointer after final object_unref().
- */
-dc->cannot_destroy_with_object_finalize_yet = true;
 }
 
 static const TypeInfo x86_cpu_type_info = {
-- 
2.7.4




Re: [Qemu-devel] [PATCH v2 02/11] blockjob: centralize QMP event emissions

2016-10-10 Thread Eric Blake
On 10/10/2016 01:36 PM, John Snow wrote:
  But if it cannot figure out which upper-layer disk the event is
 associated with, it just drops the event.  So I think that from the
 libvirt perspective, you are okay if if you always report job events,
 even for internal jobs.  (Do we have a quick and easy way to set up an
 internal job event, to double-check if I can produce some sort of
 libvirt scenario to trigger the event and see that it gets safely
 ignored?)

>>>
>>> Not in a QEMU release yet, I think.
>>
>> If not from an official QEMU release, it'd still be useful to work out a
>> a way to reproduce what Eric asked even from upstream Git master.
>>
> 
> I'll be honest that I don't know; this is related to Replication which I
> know reasonably little about overall. It got added in the 2.8 timeframe,
> so the behavior it currently exhibits is not a good or meaningful
> reference for maintaining compatibility.
> 
> We can /change/ the behavior before releases with no love lost.

And if Replication is the only way to trigger internal use of jobs, then
we aren't breaking libvirt (which doesn't know how to drive replication
yet) by changing anything on that front.

> 
> Or, what if you just didn't get events for internal jobs? Are events for
> un-managed jobs useful in any sense beyond understanding the stateful
> availability of the drive to participate in a new job?

If libvirt isn't driving replication, then it's a moot point. And even
though replication in libvirt is not supported yet, I suspect that down
the road when support is added, the easiest thing for libvirt will be to
state that replication and libvirt-controlled block jobs are mutually
exclusive; there's probably enough lurking dragons that if your system
MUST be high-reliance by replication, you probably don't want to be
confusing things by changing the backing file depth manually with
streams, pulls, or other manual actions at the same time as replication
is managing the system, because how can you guarantee that both primary
and secondary see the same manual actions at all the right times?

At any rate, not seeing internal-only jobs is probably the most
reasonable, even if it means an internal-only job can block the attempt
to do a manual job.

-- 
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 00/33] virtio, pc: fixes and features

2016-10-10 Thread Michael S. Tsirkin
On Mon, Oct 10, 2016 at 07:32:32PM +0100, Peter Maydell wrote:
> On 10 October 2016 at 15:13, Peter Maydell  wrote:
> > On 10 October 2016 at 03:57, Michael S. Tsirkin  wrote:
> >> The following changes since commit 
> >> 48f592118ab42f83a1a7561c4bfd2b72a100f241:
> >>
> >>   bsd-user: fix FreeBSD build after d148d90e (2016-10-07 15:17:53 +0100)
> >>
> >> are available in the git repository at:
> >>
> >>   git://git.kernel.org/pub/scm/virt/kvm/mst/qemu.git tags/for_upstream
> >>
> >> for you to fetch changes up to dea651a95af6dad0997b840241a0bf6059d9a776:
> >>
> >>   intel-iommu: Check IOAPIC's Trigger Mode against the one in IRTE 
> >> (2016-10-10 02:38:14 +0300)
> >>
> >> 
> >> virtio, pc: fixes and features
> >>
> >> more guest error handling for virtio devices
> >> virtio migration rework
> >> pc fixes
> >>
> >> Signed-off-by: Michael S. Tsirkin 
> >
> > This failed 'make check' on aarch64 host (everything else was ok):
> >
> > TEST: tests/pxe-test... (pid=11699)
> >   /ppc64/pxe/virtio:   **
> > ERROR:/home/petmay01/qemu/tests/boot-sector.c:120:boot_sector_test:
> > assertion failed (signature == SIGN
> > ATURE): (0x2020 == 0xdead)
> > FAIL
> > GTester: last random seed: R02S87a02de849c98998299177b1a4c7a31b
> > (pid=19477)
> >   /ppc64/pxe/spapr-vlan:   **
> > ERROR:/home/petmay01/qemu/tests/boot-sector.c:120:boot_sector_test:
> > assertion failed (signature == SIGN
> > ATURE): (0x2020 == 0xdead)
> > FAIL
> > GTester: last random seed: R02Sf9cf55ad239a137dd20d3085abf91524
> > (pid=24055)
> > FAIL: tests/pxe-test
> 
> Several subsequent test runs passed, so I'm inclined to suspect
> this is not related to the pull request but is actually an
> over-enthusiastic timeout and the build machine was heavily
> loaded or something.
> 
> Anyway, I've pushed the merge to master.
> 
> thanks
> -- PMM


Could be. I think we should have code to detect this though.
How about using e.g. time to report elapsed system and user time?

-- 
MST



Re: [Qemu-devel] [PATCH 1/2] dma-helpers: explicitly pass alignment into dma-helpers

2016-10-10 Thread Mark Cave-Ayland
On 10/10/16 17:34, Eric Blake wrote:

> On 10/09/2016 11:43 AM, Mark Cave-Ayland wrote:
>> The hard-coded default alignment is BDRV_SECTOR_SIZE, however this is not
>> necessarily the case for all platforms. Use this as the default alignment for
>> all current callers.
>>
>> Signed-off-by: Mark Cave-Ayland 
>> ---
> 
>> @@ -160,8 +161,8 @@ static void dma_blk_cb(void *opaque, int ret)
>>  return;
>>  }
>>  
>> -if (dbs->iov.size & ~BDRV_SECTOR_MASK) {
>> -qemu_iovec_discard_back(>iov, dbs->iov.size & 
>> ~BDRV_SECTOR_MASK);
>> +if (dbs->iov.size & (dbs->align - 1)) {
>> +qemu_iovec_discard_back(>iov, dbs->iov.size & (dbs->align - 
>> 1));
> 
> Would it be any smarter to use osdep.h's QEMU_IS_ALIGNED(dbs->iov.size,
> dbs->align) and QEMU_ALIGN_DOWN(dbs->iov.size, dbs->align)?
> Semantically it is the same, but the macros make it obvious what the
> bit-twiddling is doing.
> 
> Unless you think that needs a tweak,
> Reviewed-by: Eric Blake 

I can't say I feel too strongly about it since there are plenty of other
examples of this style in the codebase, so I'm happy to go with whatever
John/Paolo are most happy with.


ATB,

Mark.




Re: [Qemu-devel] [PATCH 18/22] qapi: add md5 checksum of last dirty bitmap level to query-block

2016-10-10 Thread Eric Blake
On 10/10/2016 12:03 PM, Max Reitz wrote:
>> I'm not sure what the right way would be to get this information out
>> (...maybe make it optional and set it only if qtest_enabled() is true?),
>> but in my opinion this is not the right way.
> 
> By the way, the cleanest way I can come up with (which I didn't write
> about in my first reply because it's not so trivial) would be some kind
> of debugging QMP command convention. For instance, we could say that all
> debugging commands have an x-debug- prefix, and then you could add an
> x-debug-get-bitmap-md5 to read the MD5 hash of a named bitmap. That
> would appear to be the cleanest way to do this to me.

Anything named x- is automatically not stable, and therefore free to use
in the testsuite without having to worry about keeping it backwards
compatible (libvirt won't touch x- commands).  The suggestion of
x-debug- to mark it as specifically for debug use is reasonable.

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



signature.asc
Description: OpenPGP digital signature


Re: [Qemu-devel] [RFC QEMU PATCH 8/8] qmp: add a qmp command 'query-nvdimms' to get plugged NVDIMM devices

2016-10-10 Thread Eric Blake
On 10/09/2016 07:34 PM, Haozhong Zhang wrote:
> Xen uses this command to get the backend resource, guest SPA and size of
> NVDIMM devices so as to map them to guest.
> 
> Signed-off-by: Haozhong Zhang 
> ---
> Cc: Markus Armbruster 

> +++ b/docs/qmp-commands.txt
> @@ -3800,3 +3800,39 @@ Example for pc machine type started with
>  "props": {"core-id": 0, "socket-id": 0, "thread-id": 0}
>   }
> ]}
> +
> +EQMP
> +
> +{
> +.name   = "query-nvdimms",
> +.args_type  = "",
> +.mhandler.cmd_new = qmp_marshal_query_nvdimms,

Needs rebasing - we no longer need SQMP/EQMP sections or callouts to the
initializers, now that commit bd6092e4 has automated the mapping of QAPI
to command registration.

> +},
> +
> +SQMP
> +Show plugged NVDIMM devices
> +---
> +
> +Arguments: None.
> +
> +Example for pc machine type started with
> +-object memory-backend-file,id=mem1,mem-path=/path/to/nvm1,size=4G
> +-device nvdimm,id=nvdimm1,memdev=mem1
> +-object memory-backend-file,id=mem2,mem-path=/path/to/nvm2,size=8G
> +-device nvdimm,id=nvdimm2,memdev=mem2:
> +
> +-> { "execute": "query-nvdimms" }
> +<- { "returns": [
> +  {
> + "mem-path": "/path/to/nvm1",
> +  "slot": 0,

TAB damage; please fix.

> +  "spa": 17179869184,
> +  "length": 4294967296
> +  },
> +  {
> + "mem-path": "/path/to/nvm2",
> +  "slot": 1,
> +  "spa": 21474836480,
> +  "length": 8589934592
> +  }
> +   ]}

> +++ b/qapi-schema.json
> @@ -4646,3 +4646,32 @@
>  # Since: 2.7
>  ##
>  { 'command': 'query-hotpluggable-cpus', 'returns': ['HotpluggableCPU'] }
> +
> +##
> +# @NvdimmInfo
> +#
> +# Information about an NVDIMM device.
> +#
> +# @mem-path: the backend file of the NVDIMM device
> +#
> +# @slot: the slot index of the NVDIMM device
> +#
> +# @spa: the 64-bit SPA base address of the NVDIMM device
> +#
> +# @length: the 64-bit size in bytes of the NVDIMM device
> +#
> +# Since 2.8
> +##
> +{ 'struct': 'NvdimmInfo',
> +  'data': {'mem-path' : 'str', 'slot': 'int', 'spa': 'int', 'length': 'int'} 
> }
> +
> +##
> +# @query-nvdimms:
> +#
> +# Returns information about each NVDIMM device
> +#
> +# Returns: a list of @NvdimmInfo for each device
> +#
> +# Since: 2.8
> +##
> +{ 'command': 'query-nvdimms', 'returns': ['NvdimmInfo'] }
> 

Is this something that can be added to the existing query-memdev or
query-memory-devices command, instead of needing a new command?

-- 
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 v10 09/16] block: Add QMP support for streaming to an intermediate layer

2016-10-10 Thread Eric Blake
On 10/06/2016 08:02 AM, Alberto Garcia wrote:
> This patch makes the 'device' parameter of the 'block-stream' command
> accept a node name that is not a root node.
> 
> In addition to that, operation blockers will be checked in all
> intermediate nodes between the top and the base node.
> 
> Signed-off-by: Alberto Garcia 
> ---
>  blockdev.c   | 11 +--
>  qapi/block-core.json | 10 +++---
>  2 files changed, 16 insertions(+), 5 deletions(-)
> 

> +++ b/qapi/block-core.json
> @@ -1469,6 +1469,10 @@
>  # with query-block-jobs.  The operation can be stopped before it has 
> completed
>  # using the block-job-cancel command.
>  #
> +# The node that receives the data is called the top image, can be located in
> +# any part of the chain (but always above the base image; see below) and can 
> be
> +# specified using its device or node name.
> +#
>  # If a base file is specified then sectors are not copied from that base 
> file and
>  # its backing chain.  When streaming completes the image file will have the 
> base
>  # file as its backing file.  This can be used to stream a subset of the 
> backing
> @@ -1480,12 +1484,12 @@
>  # @job-id: #optional identifier for the newly-created block job. If
>  #  omitted, the device name will be used. (Since 2.7)
>  #
> -# @device: the device name or node-name of a root node
> +# @device: the device or node name of the top image
>  #
>  # @base:   #optional the common backing file name
>  #
> -# @backing-file: #optional The backing file string to write into the active
> -#  layer. This filename is not validated.
> +# @backing-file: #optional The backing file string to write into the top
> +#  image. This filename is not validated.

No change to the actual introspection. What's the easiest way for
libvirt to learn if the new style command will work, short of actually
trying it to see if it succeeds or fails?  (That may be sufficient, but
the error message quality can often be improved in libvirt if it is
known up front if qemu is new enough rather than taking the 'try and
see' approach and getting stuck with qemu's error message)

-- 
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 for-2.5] iotests: drop thread spun work-around

2016-10-10 Thread Max Reitz
On 10.10.2016 04:57, Michael S. Tsirkin wrote:
> We've disabled the warning, there should be no need for test to work
> around it.
> 
> Signed-off-by: Michael S. Tsirkin 
> ---
> 
> This is on top of
> main-loop: suppress warnings under qtest
> 
> I just tested this by running make check.
> Is this enough?

It should be, but I'm not sure why you want this patch for 2.5. Anyway:

Reviewed-by: Max Reitz 

>  tests/qemu-iotests/common.filter | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/tests/qemu-iotests/common.filter 
> b/tests/qemu-iotests/common.filter
> index cfdb633..49217b0 100644
> --- a/tests/qemu-iotests/common.filter
> +++ b/tests/qemu-iotests/common.filter
> @@ -164,7 +164,6 @@ _filter_qemu()
>  {
>  sed -e "s#\\(^\\|(qemu) \\)$(basename $QEMU_PROG):#\1QEMU_PROG:#" \
>  -e 's#^QEMU [0-9]\+\.[0-9]\+\.[0-9]\+ monitor#QEMU X.Y.Z monitor#' \
> --e '/main-loop: WARNING: I\/O thread spun for [0-9]\+ iterations/d' \
>  -e $'s#\r##' # QEMU monitor uses \r\n line endings
>  }
>  
> 




signature.asc
Description: OpenPGP digital signature


Re: [Qemu-devel] [PATCH] socket: add atomic QEMU_SOCK_NONBLOCK flag

2016-10-10 Thread no-reply
Hi,

Your series failed automatic build test. Please find the testing commands and
their output below. If you have docker installed, you can probably reproduce it
locally.

Message-id: 1475848458-1285-1-git-send-email-stefa...@redhat.com
Subject: [Qemu-devel] [PATCH] socket: add atomic QEMU_SOCK_NONBLOCK flag
Type: series

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

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
Switched to a new branch 'test'
9338e02 socket: add atomic QEMU_SOCK_NONBLOCK flag

=== OUTPUT BEGIN ===
Submodule 'dtc' (git://git.qemu-project.org/dtc.git) registered for path 'dtc'
Cloning into 'dtc'...
Submodule path 'dtc': checked out '65cc4d2748a2c2e6f27f1cf39e07a5dbabd80ebf'
  BUILD   centos6
  ARCHIVE qemu.tgz
  ARCHIVE dtc.tgz
  COPYRUNNER
  RUN test-quick in centos6
Packages installed:
SDL-devel-1.2.14-7.el6_7.1.x86_64
ccache-3.1.6-2.el6.x86_64
epel-release-6-8.noarch
gcc-4.4.7-17.el6.x86_64
git-1.7.1-4.el6_7.1.x86_64
glib2-devel-2.28.8-5.el6.x86_64
libfdt-devel-1.4.0-1.el6.x86_64
make-3.81-23.el6.x86_64
package g++ is not installed
pixman-devel-0.32.8-1.el6.x86_64
tar-1.23-15.el6_8.x86_64
zlib-devel-1.2.3-29.el6.x86_64

Environment variables:
PACKAGES=libfdt-devel ccache tar git make gcc g++ zlib-devel 
glib2-devel SDL-devel pixman-devel epel-release
HOSTNAME=f2f51b60ec8e
TERM=xterm
MAKEFLAGS= -j8
HISTSIZE=1000
J=8
USER=root
CCACHE_DIR=/var/tmp/ccache
EXTRA_CONFIGURE_OPTS=
V=
SHOW_ENV=1
MAIL=/var/spool/mail/root
PATH=/usr/lib/ccache:/usr/lib64/ccache:/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin
PWD=/
LANG=en_US.UTF-8
TARGET_LIST=
HISTCONTROL=ignoredups
SHLVL=1
HOME=/root
TEST_DIR=/tmp/qemu-test
LOGNAME=root
LESSOPEN=||/usr/bin/lesspipe.sh %s
FEATURES= dtc
DEBUG=
G_BROKEN_FILENAMES=1
CCACHE_HASHDIR=
_=/usr/bin/env

Configure options:
--enable-werror --target-list=x86_64-softmmu,aarch64-softmmu 
--prefix=/var/tmp/qemu-build/install
No C++ compiler available; disabling C++ specific optional code
Install prefix/var/tmp/qemu-build/install
BIOS directory/var/tmp/qemu-build/install/share/qemu
binary directory  /var/tmp/qemu-build/install/bin
library directory /var/tmp/qemu-build/install/lib
module directory  /var/tmp/qemu-build/install/lib/qemu
libexec directory /var/tmp/qemu-build/install/libexec
include directory /var/tmp/qemu-build/install/include
config directory  /var/tmp/qemu-build/install/etc
local state directory   /var/tmp/qemu-build/install/var
Manual directory  /var/tmp/qemu-build/install/share/man
ELF interp prefix /usr/gnemul/qemu-%M
Source path   /tmp/qemu-test/src
C compilercc
Host C compiler   cc
C++ compiler  
Objective-C compiler cc
ARFLAGS   rv
CFLAGS-O2 -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=2 -g 
QEMU_CFLAGS   -I/usr/include/pixman-1-pthread -I/usr/include/glib-2.0 
-I/usr/lib64/glib-2.0/include   -fPIE -DPIE -m64 -D_GNU_SOURCE 
-D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE -Wstrict-prototypes 
-Wredundant-decls -Wall -Wundef -Wwrite-strings -Wmissing-prototypes 
-fno-strict-aliasing -fno-common -fwrapv  -Wendif-labels -Wmissing-include-dirs 
-Wempty-body -Wnested-externs -Wformat-security -Wformat-y2k -Winit-self 
-Wignored-qualifiers -Wold-style-declaration -Wold-style-definition 
-Wtype-limits -fstack-protector-all
LDFLAGS   -Wl,--warn-common -Wl,-z,relro -Wl,-z,now -pie -m64 -g 
make  make
install   install
pythonpython -B
smbd  /usr/sbin/smbd
module supportno
host CPU  x86_64
host big endian   no
target list   x86_64-softmmu aarch64-softmmu
tcg debug enabled no
gprof enabled no
sparse enabledno
strip binariesyes
profiler  no
static build  no
pixmansystem
SDL support   yes (1.2.14)
GTK support   no 
GTK GL supportno
VTE support   no 
TLS priority  NORMAL
GNUTLS supportno
GNUTLS rndno
libgcrypt no
libgcrypt kdf no
nettleno 
nettle kdfno
libtasn1  no
curses supportno
virgl support no
curl support  no
mingw32 support   no
Audio drivers oss
Block whitelist (rw) 
Block whitelist (ro) 
VirtFS supportno
VNC support   yes
VNC SASL support  no
VNC JPEG support  no
VNC PNG support   no
xen support   no
brlapi supportno
bluez  supportno
Documentation no
PIE   yes
vde support   no
netmap supportno
Linux AIO support no
ATTR/XATTR support yes
Install blobs yes
KVM support   yes
RDMA support  no
TCG interpreter   no
fdt support   yes
preadv supportyes
fdatasync yes
madvise   yes
posix_madvise yes
libcap-ng support no
vhost-net support yes
vhost-scsi support yes
vhost-vsock support yes
Trace backendslog
spice support no 
rbd support   

Re: [Qemu-devel] 答复: Re: [PATCH v3 2/3] qapi: auto generate enum value strings

2016-10-10 Thread Eric Blake
On 10/10/2016 10:09 AM, Lin Ma wrote:
> 
> 
 Eric Blake  2016/9/27 星期二 上午 4:17 >>>
>> On 09/26/2016 05:38 AM, Daniel P. Berrange wrote:
>>> On Mon, Sep 26, 2016 at 06:16:26PM +0800, Lin Ma wrote:
 Automatically generate enum value strings that containing the acceptable 
 values.
 (Borrowwed Daniel's code.)
>>
>> s/Borrowwed/Borrowed/
> Sorry for the late reply, I was on vacation.
> Thanks for the review.
>>

 Signed-off-by: Lin Ma 
 ---
  scripts/qapi-types.py | 2 ++
  scripts/qapi.py   | 9 +
  2 files changed, 11 insertions(+)
>>>
>>> This will need some test case coverage in tests/ somewhere, but I'm
>>> not sure exactly which place is best - Eric/Markus can probably advise
>>
>> tests/test-qmp-commands.c is the first one that comes to mind, for
>> adding another test case to an existing program.
>>
> I'm not familiar with how to write qapi generator code and related test
> code at all. I'll start to dig, Any guidance is appreciated.
> For adding test case, Only this tests/test-qmp-commands.c needs to be
> modified, right? 

Yes, I think the easiest approach is to add a new line in the main()
file that calls out to a new function, and the new function tests that
an existing QAPI enum (from tests/qapi-schema/qapi-schema-test.json) has
a sane conversion to a string listing all its members.  Markus may have
better ideas on where to place a new test, though.

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



signature.asc
Description: OpenPGP digital signature


Re: [Qemu-devel] [PATCH 1/2] hw/arm/virt-acpi-build: fix MADT generation

2016-10-10 Thread Auger Eric
Hi Drew,
On 10/10/2016 18:35, Andrew Jones wrote:
> We can't return early from build_* functions, as build_header is
> only called at the end.
> 
> Signed-off-by: Andrew Jones 
> ---
>  hw/arm/virt-acpi-build.c | 14 ++
>  1 file changed, 6 insertions(+), 8 deletions(-)
> 
> diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
> index 7b39b1d2d676..c31349561c95 100644
> --- a/hw/arm/virt-acpi-build.c
> +++ b/hw/arm/virt-acpi-build.c
> @@ -556,15 +556,13 @@ build_madt(GArray *table_data, BIOSLinker *linker, 
> VirtGuestInfo *guest_info)
>  gicr->base_address = cpu_to_le64(memmap[VIRT_GIC_REDIST].base);
>  gicr->range_length = cpu_to_le32(memmap[VIRT_GIC_REDIST].size);
>  
> -if (!its_class_name()) {
> -return;
> +if (its_class_name()) {
> +gic_its = acpi_data_push(table_data, sizeof *gic_its);
> +gic_its->type = ACPI_APIC_GENERIC_TRANSLATOR;
> +gic_its->length = sizeof(*gic_its);
> +gic_its->translation_id = 0;
> +gic_its->base_address = cpu_to_le64(memmap[VIRT_GIC_ITS].base);
>  }
> -
> -gic_its = acpi_data_push(table_data, sizeof *gic_its);
> -gic_its->type = ACPI_APIC_GENERIC_TRANSLATOR;
> -gic_its->length = sizeof(*gic_its);
> -gic_its->translation_id = 0;
> -gic_its->base_address = cpu_to_le64(memmap[VIRT_GIC_ITS].base);
>  } else {
>  gic_msi = acpi_data_push(table_data, sizeof *gic_msi);
>  gic_msi->type = ACPI_APIC_GENERIC_MSI_FRAME;
> 
Thanks for spotting the issue

Reviewed-by: Eric Auger 

Eric



Re: [Qemu-devel] [PATCH v2] build: Work around SIZE_MAX bug in OSX headers

2016-10-10 Thread Eric Blake
On 10/10/2016 01:38 PM, Eric Blake wrote:
> On 10/10/2016 10:12 AM, Peter Maydell wrote:
> 
>>> v2: rewrite into a configure check (not sure if directly adding a
>>> -D to QEMU_CFLAGS is the best, so advice welcome)
>>
>> Writing into config-host.mak would be preferable I think.
>>
> 
> Okay, attempted in v4.

I can't type today. I just sent v3, not v4.

> 
>>> I lack easy access to a Mac box, so this is untested as to whether
>>> it actually solves the issue...
>>
>> I ran a test with one of the cast workarounds locally removed,
>> and this patch does allow the %zu version to build OK on OSX.
> 
> Thanks. But v3 leaves off Tested-by, because I still need to make sure
> my configure magic worked correctly :)
> 

-- 
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 v2] build: Work around SIZE_MAX bug in OSX headers

2016-10-10 Thread Eric Blake
On 10/10/2016 10:12 AM, Peter Maydell wrote:

>> v2: rewrite into a configure check (not sure if directly adding a
>> -D to QEMU_CFLAGS is the best, so advice welcome)
> 
> Writing into config-host.mak would be preferable I think.
> 

Okay, attempted in v4.

>> I lack easy access to a Mac box, so this is untested as to whether
>> it actually solves the issue...
> 
> I ran a test with one of the cast workarounds locally removed,
> and this patch does allow the %zu version to build OK on OSX.

Thanks. But v3 leaves off Tested-by, because I still need to make sure
my configure magic worked correctly :)

-- 
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 v2 02/11] blockjob: centralize QMP event emissions

2016-10-10 Thread John Snow



On 10/10/2016 12:45 PM, Kashyap Chamarthy wrote:

On Wed, Oct 05, 2016 at 05:00:29PM -0400, John Snow wrote:

[Arbitrarily chiming here, and still catching up with the details of the
thread.]


On 10/05/2016 03:24 PM, Eric Blake wrote:

On 10/05/2016 01:49 PM, John Snow wrote:


[...]


Hmm, do we want to make it so some jobs are invisible and others are
not? Because as it stands right now, neither case is strictly true. We
only emit cancelled/completed events if it was started via QMP, however
we do emit events for error and ready regardless of who started the job.


Libvirt tries to mirror any block job event it receives to upper layers.
 But if it cannot figure out which upper-layer disk the event is
associated with, it just drops the event.  So I think that from the
libvirt perspective, you are okay if if you always report job events,
even for internal jobs.  (Do we have a quick and easy way to set up an
internal job event, to double-check if I can produce some sort of
libvirt scenario to trigger the event and see that it gets safely ignored?)



Not in a QEMU release yet, I think.


If not from an official QEMU release, it'd still be useful to work out a
a way to reproduce what Eric asked even from upstream Git master.



I'll be honest that I don't know; this is related to Replication which I 
know reasonably little about overall. It got added in the 2.8 timeframe, 
so the behavior it currently exhibits is not a good or meaningful 
reference for maintaining compatibility.


We can /change/ the behavior before releases with no love lost.


That didn't seem particularly consistent to me; either all events should
be controlled by the job layer itself or none of them should be.

I opted for "all."

For "internal" jobs that did not previously emit any events, is it not
true that these jobs still appear in the block job list and are
effectively public regardless? I'd argue that these messages may be of
value for management utilities who are still blocked by these jobs
whether or not they are 'internal' or not.


It'd certainly be useful durign debugging (for the said management
utilities), if it's possible to distinguish an event that was triggerred
by an internal block job vs. an event emitted by a job explicitly
triggerred by a user action.



Or, what if you just didn't get events for internal jobs? Are events for 
un-managed jobs useful in any sense beyond understanding the stateful 
availability of the drive to participate in a new job?



For example, OpenStack Nova calls libvirt API virDomainBlockRebase(),
which internally calls QMP `drive-mirror` that emits events.  An "event
origin classification" system, if were to exist, allows one to pay
attention to only those events that are emitted due to an explicit
action and ignore all the rest ('internal').

But I'm not quite sure if it's desirable to have this event
classification for cleanliness reasons as Eric points out below.


I'll push for keeping it mandatory and explicit. If it becomes a
problem, we can always add a 'silent' job property that silences ALL qmp
events, including all completion, error, and ready notices.


Completely silencing an internal job seems a little cleaner than having
events for the job but not being able to query it.  But if nothing
breaks by exposing the internal jobs, that seems even easier than trying
to decide which jobs are internal and hidden vs. user-requested and public.



Well, at the moment anything requested directly via blockdev.c is "public."
Before 2.8, all jobs were public ones, with the exception of those in
qemu-img which is a bit of a different/special case.

We have this block/replication.c beast now, though, and it uses backup_start
and commit_active_start as it sees fit without direct user intervention.

As it stands, I believe the jobs that replication creates are user-visible
via query, will not issue cancellation or completion events, but WILL emit
error events. It may emit ready events for the mirror job it uses, but I
haven't traced that. (It could, at least.)


Thanks, the above is useful to know.






Re: [Qemu-devel] [PULL 00/33] virtio, pc: fixes and features

2016-10-10 Thread Peter Maydell
On 10 October 2016 at 15:13, Peter Maydell  wrote:
> On 10 October 2016 at 03:57, Michael S. Tsirkin  wrote:
>> The following changes since commit 48f592118ab42f83a1a7561c4bfd2b72a100f241:
>>
>>   bsd-user: fix FreeBSD build after d148d90e (2016-10-07 15:17:53 +0100)
>>
>> are available in the git repository at:
>>
>>   git://git.kernel.org/pub/scm/virt/kvm/mst/qemu.git tags/for_upstream
>>
>> for you to fetch changes up to dea651a95af6dad0997b840241a0bf6059d9a776:
>>
>>   intel-iommu: Check IOAPIC's Trigger Mode against the one in IRTE 
>> (2016-10-10 02:38:14 +0300)
>>
>> 
>> virtio, pc: fixes and features
>>
>> more guest error handling for virtio devices
>> virtio migration rework
>> pc fixes
>>
>> Signed-off-by: Michael S. Tsirkin 
>
> This failed 'make check' on aarch64 host (everything else was ok):
>
> TEST: tests/pxe-test... (pid=11699)
>   /ppc64/pxe/virtio:   **
> ERROR:/home/petmay01/qemu/tests/boot-sector.c:120:boot_sector_test:
> assertion failed (signature == SIGN
> ATURE): (0x2020 == 0xdead)
> FAIL
> GTester: last random seed: R02S87a02de849c98998299177b1a4c7a31b
> (pid=19477)
>   /ppc64/pxe/spapr-vlan:   **
> ERROR:/home/petmay01/qemu/tests/boot-sector.c:120:boot_sector_test:
> assertion failed (signature == SIGN
> ATURE): (0x2020 == 0xdead)
> FAIL
> GTester: last random seed: R02Sf9cf55ad239a137dd20d3085abf91524
> (pid=24055)
> FAIL: tests/pxe-test

Several subsequent test runs passed, so I'm inclined to suspect
this is not related to the pull request but is actually an
over-enthusiastic timeout and the build machine was heavily
loaded or something.

Anyway, I've pushed the merge to master.

thanks
-- PMM



[Qemu-devel] [PATCH v3] build: Work around SIZE_MAX bug in OSX headers

2016-10-10 Thread Eric Blake
C99 requires SIZE_MAX to be declared with the same type as the
integral promotion of size_t, but OSX mistakenly defines it as
an 'unsigned long long' expression even though size_t is only
'unsigned long'.  Rather than futzing around with whether size_t
is 32- or 64-bits wide (which would be needed if we cared about
using SIZE_T in a #if expression), let the compiler get the right
type for us by virtue of integer promotion - if we later need it
during the preprocessor, the build will break on Mac until we
improve our replacement.

See also https://patchwork.ozlabs.org/patch/542327/ for an
instance where the wrong type trips us up if we don't fix it
for good in osdep.h.

Some versions of glibc make a similar mistake with SSIZE_MAX; the
goal is that the approach of this patch could be copied to work
around that problem if it ever becomes important to us.

Signed-off-by: Eric Blake 

---
v1 was here:
https://lists.gnu.org/archive/html/qemu-devel/2016-07/msg02520.html

The topic recently came up again, and I noticed this patch sitting
on one of my older branches, so I've taken another shot at it.
https://lists.gnu.org/archive/html/qemu-devel/2016-10/msg00950.html

v2: rewrite into a configure check (not sure if directly adding a
-D to QEMU_CFLAGS is the best, so advice welcome)

v3: Use config-host.mak rather than direct -D

I lack easy access to a Mac box, so again this needs testing...
---
 include/qemu/osdep.h |  8 
 configure| 16 
 2 files changed, 24 insertions(+)

diff --git a/include/qemu/osdep.h b/include/qemu/osdep.h
index 9e9fa61..2520e63 100644
--- a/include/qemu/osdep.h
+++ b/include/qemu/osdep.h
@@ -141,6 +141,14 @@ extern int daemon(int, int);
 # error Unknown pointer size
 #endif

+/* Mac OSX has a  bug that incorrectly defines SIZE_MAX with
+ * the wrong type. Our replacement isn't usable in preprocessor
+ * expressions, but it is sufficient for our needs. */
+#if HAVE_BROKEN_SIZE_MAX
+#undef SIZE_MAX
+#define SIZE_MAX ((sizeof(char)) * -1)
+#endif
+
 #ifndef MIN
 #define MIN(a, b) (((a) < (b)) ? (a) : (b))
 #endif
diff --git a/configure b/configure
index 5751d8e..dd9e679 100755
--- a/configure
+++ b/configure
@@ -1725,6 +1725,19 @@ if test "$cocoa" = "yes"; then
 sdl=no
 fi

+# Some versions of Mac OS X incorrectly define SIZE_MAX
+cat > $TMPC << EOF
+#include 
+#include 
+int main(int argc, char *argv[]) {
+return printf("%zu", SIZE_MAX);
+}
+EOF
+have_broken_size_max=no
+if ! compile_object -Werror ; then
+have_broken_size_max=yes
+fi
+
 ##
 # L2TPV3 probe

@@ -5245,6 +5258,9 @@ fi
 if test "$have_ifaddrs_h" = "yes" ; then
 echo "HAVE_IFADDRS_H=y" >> $config_host_mak
 fi
+if test "$have_broken_size_max" = "yes" ; then
+echo "HAVE_BROKEN_SIZE_MAX=y" >> $config_host_mak
+fi

 # Work around a system header bug with some kernel/XFS header
 # versions where they both try to define 'struct fsxattr':
-- 
2.7.4




Re: [Qemu-devel] [Qemu-block] [PATCH] iotests: drop thread spun work-around

2016-10-10 Thread Max Reitz
On 10.10.2016 04:57, Michael S. Tsirkin wrote:
> We've disabled the warning, no need for test to work
> around it.
> 
> Signed-off-by: Michael S. Tsirkin 
> ---
>  tests/qemu-iotests/common.filter | 1 -
>  1 file changed, 1 deletion(-)

Thanks, I've applied the patch to my block branch:

https://github.com/XanClic/qemu/commits/block

Max



signature.asc
Description: OpenPGP digital signature


[Qemu-devel] [PATCH 1/2] 9p: v9fs add writepages.

2016-10-10 Thread Edward Shishkin
Add a v9fs private ->writepages() method of address_space
operations for merging pages into long 9p messages.

Signed-off-by: Edward Shishkin 
---
 fs/9p/v9fs.c  |  46 +++
 fs/9p/v9fs.h  |  22 +++-
 fs/9p/vfs_addr.c  | 357 ++
 fs/9p/vfs_super.c |   8 +-
 4 files changed, 431 insertions(+), 2 deletions(-)

diff --git a/fs/9p/v9fs.c b/fs/9p/v9fs.c
index 072e759..3b49daf 100644
--- a/fs/9p/v9fs.c
+++ b/fs/9p/v9fs.c
@@ -32,6 +32,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -309,6 +310,49 @@ static int v9fs_parse_options(struct v9fs_session_info 
*v9ses, char *opts)
return ret;
 }
 
+void put_flush_set(struct v9fs_flush_set *fset)
+{
+   if (!fset)
+   return;
+   if (fset->pages)
+   kfree(fset->pages);
+   if (fset->buf)
+   kfree(fset->buf);
+   kfree(fset);
+}
+
+/**
+ * Allocate and initalize flush set
+ * Pre-conditions: valid msize is set
+ */
+int alloc_init_flush_set(struct v9fs_session_info *v9ses)
+{
+   int ret = -ENOMEM;
+   int num_pages;
+   struct v9fs_flush_set *fset = NULL;
+
+   num_pages = v9ses->clnt->msize >> PAGE_SHIFT;
+   if (num_pages < 2)
+   /* speedup impossible */
+   return 0;
+   fset = kzalloc(sizeof(*fset), GFP_KERNEL);
+   if (!fset)
+   goto error;
+   fset->num_pages = num_pages;
+   fset->pages = kzalloc(num_pages * sizeof(*fset->pages), GFP_KERNEL);
+   if (!fset->pages)
+   goto error;
+   fset->buf = kzalloc(num_pages << PAGE_SHIFT, GFP_USER);
+   if (!fset->buf)
+   goto error;
+   spin_lock_init(&(fset->lock));
+   v9ses->flush = fset;
+   return 0;
+ error:
+   put_flush_set(fset);
+   return ret;
+}
+
 /**
  * v9fs_session_init - initialize session
  * @v9ses: session information structure
@@ -444,6 +488,8 @@ void v9fs_session_close(struct v9fs_session_info *v9ses)
kfree(v9ses->uname);
kfree(v9ses->aname);
 
+   put_flush_set(v9ses->flush);
+
bdi_destroy(>bdi);
 
spin_lock(_sessionlist_lock);
diff --git a/fs/9p/v9fs.h b/fs/9p/v9fs.h
index 6877050..d1092e4 100644
--- a/fs/9p/v9fs.h
+++ b/fs/9p/v9fs.h
@@ -23,6 +23,7 @@
 #ifndef FS_9P_V9FS_H
 #define FS_9P_V9FS_H
 
+#include 
 #include 
 
 /**
@@ -69,6 +70,13 @@ enum p9_cache_modes {
CACHE_FSCACHE,
 };
 
+struct v9fs_flush_set {
+struct page **pages;
+   int num_pages;
+char *buf;
+   spinlock_t lock;
+};
+
 /**
  * struct v9fs_session_info - per-instance session information
  * @flags: session options of type _session_flags
@@ -105,7 +113,7 @@ struct v9fs_session_info {
char *cachetag;
struct fscache_cookie *fscache;
 #endif
-
+   struct v9fs_flush_set *flush; /* flush set for writepages */
char *uname;/* user name to mount as */
char *aname;/* name of remote hierarchy being mounted */
unsigned int maxdata;   /* max data for client interface */
@@ -158,6 +166,8 @@ extern const struct inode_operations 
v9fs_symlink_inode_operations_dotl;
 extern struct inode *v9fs_inode_from_fid_dotl(struct v9fs_session_info *v9ses,
  struct p9_fid *fid,
  struct super_block *sb, int new);
+extern int alloc_init_flush_set(struct v9fs_session_info *v9ses);
+extern void put_flush_set(struct v9fs_flush_set *fset);
 
 /* other default globals */
 #define V9FS_PORT  564
@@ -222,4 +232,14 @@ v9fs_get_new_inode_from_fid(struct v9fs_session_info 
*v9ses, struct p9_fid *fid,
return v9fs_inode_from_fid(v9ses, fid, sb, 1);
 }
 
+static inline int spin_trylock_flush_set(struct v9fs_flush_set *fset)
+{
+   return spin_trylock(&(fset->lock));
+}
+
+static inline void spin_unlock_flush_set(struct v9fs_flush_set *fset)
+{
+   spin_unlock(&(fset->lock));
+}
+
 #endif
diff --git a/fs/9p/vfs_addr.c b/fs/9p/vfs_addr.c
index 6181ad7..e871886 100644
--- a/fs/9p/vfs_addr.c
+++ b/fs/9p/vfs_addr.c
@@ -36,6 +36,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #include "v9fs.h"
 #include "v9fs_vfs.h"
@@ -209,6 +210,361 @@ static int v9fs_vfs_writepage(struct page *page, struct 
writeback_control *wbc)
return retval;
 }
 
+static void redirty_pages_for_writeback(struct page **pages, int nr,
+   struct writeback_control *wbc)
+{
+   int i;
+   for (i = 0; i < nr; i++) {
+   lock_page(pages[i]);
+   redirty_page_for_writepage(wbc, pages[i]);
+   unlock_page(pages[i]);
+   }
+}
+
+static void set_pages_error(struct page **pages, int nr, int error)
+{
+   int i;
+   for (i = 0; i < nr; i++) {
+   lock_page(pages[i]);
+   SetPageError(pages[i]);
+   

[Qemu-devel] [RFC][PATCH 0/2] 9p: v9fs read and write speedup

2016-10-10 Thread Edward Shishkin

Hello everyone,

The progress in virtualization and cloud technologies has resulted in
a popularity of file sets shared on the host machines by Plan 9 File
Protocol (the sharing setup is also known as VirtFS). Another sharing
setup which uses NFS protocol is less popular because of number of
reasons.

Unfortunately performance of default VirtFS setup is poor.
We analyzed the reasons in our Labs of Huawei Technologies and found
that typical bottleneck is caused by the fact that transfer of any
portion of data by many small 9p messages is slower than transfer of
the same amount of data by a single 9p message.

It is possible to reduce number of 9P messages (and, hence, to improve
performance) by a number of ways(*), however, some "hardcoded"
bottlenecks are still present in the v9fs driver of the guest kernel.
Specifically, this is poor implementation of read-ahead and
write-behind paths of v9fs. With current implementations there is no
chances that more than PAGE_SIZE bytes of data will be transmitted at
one time.

To improve the situation we have introduced a special layer, which
allows to coalesce specified number of adjacent pages (if any) into a
single 9P message. This layer is represented by private
implementations of ->readpages() and ->writepages() address space
operations for v9fs.

To merge adjacent pages we use a special buffer of size, which depends
on specified (per mount session) msize. For read-ahead paths we
allocate such buffers by demand. For writeback paths we use a single
buffer pre-allocated at mount time. This is because at writeback paths
the file system usually responds to memory pressure notification, so
we can not afford allocation by-demand at writeback paths.

All pages which are supposed to merge are coped to the buffer at
respective offsets. Then we construct and transmit a long read(write)
9P message (**).

Thus, we managed to speedup only one writeback-ing thread. Other
concurrent threads, which failed to obtain the buffer, will go by
usual (slow) ways. If interesting, I'll implement a solution with N
pre-allocated buffers (where N is number of CPUs).

This approach allows to increase VirtFS bandwidth up to 3 times, and
thus, to make it close to the bandwidth of VirtIO-blk (see the numbers
in the Appendix A below).

Comment. Our patches improve only asynchronous operations, which
involve the page cache. Direct reads and writes will be unaffected by
obvious reasons.
NOTE, that by default v9fs works in direct mode, so in order to see an
effect, you should specify respective v9fs mount option (e.g.
"fscache").


(*)  Specifying larger mszie (maximal possible size of 9P message)
allow to reduce number of 9P messages in direct operations performed
by large chunks.
Disabling v9fs ACL and security labels in the guest kernel (if it is
not needed) allows to avoid extra-messages.

(**) 9P, Plan 9 File Protocol specifications
https://swtch.com/plan9port/man/man9/intro.html


Appendix A.

   iozone -e -r chunk_size -s 6G -w -f

   Throughput in mBytes/sec


operation  chunk_size   (1)   (2)   (3)

write  1M   391   127   330
read   1M   469   221   432
write  4K   410   128   297
read   4K   465   192   327
random write   1M   403   145   313
random read1M   347   161   195
random write   4K   344   119   131
random read4K444164


Legend:

(1): VirtIO-blk
(2): VirtIO-9p
(3): VirtIO-9p: guest kernel is patched with our stuff

Hardware & Software:

Host:  8 CPU Intel(R) Core(TM) Intel(R) Core(TM) i7-4770 CPU @ 3.40GHz
16G RAM, SSD: Noname. Throughput: Write: 410 M/sec, Read: 470 M/sec.
Fedora 24, Kernel-4.7.4-200.fc24.x86_64, kvm+qemu-2.7.0, fs: ext4

Guest: 2 CPU: GenuineIntel 3.4GHz, 2G RAM, Network model: VirtIO
Fedora 21, Kernel: 4.7.6

Settings:
VirtIO-blk: Guest FS: ext4;
VirtIO-9p:  mount option:
"trans=virtio,version=9p2000.L,msize=131096,fscache"

Caches of the host and guest were dropped before every iozone's phase.

CC-ed QEMU developers list for possible comments and ACKs.

Please, consider for inclusion.

Thanks,
Edward.



[Qemu-devel] [PATCH 2/2] 9p: v9fs new readpages.

2016-10-10 Thread Edward Shishkin
Modify v9fs private ->readpages() method of address_space
operations for merging pages into long 9p messages.

Signed-off-by: Edward Shishkin 
---
 fs/9p/vfs_addr.c | 156 ++-
 1 file changed, 155 insertions(+), 1 deletion(-)

diff --git a/fs/9p/vfs_addr.c b/fs/9p/vfs_addr.c
index e871886..4ad248e 100644
--- a/fs/9p/vfs_addr.c
+++ b/fs/9p/vfs_addr.c
@@ -34,6 +34,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -99,6 +100,148 @@ static int v9fs_vfs_readpage(struct file *filp, struct 
page *page)
return v9fs_fid_readpage(filp->private_data, page);
 }
 
+/*
+ * Context for "fast readpages"
+ */
+struct v9fs_readpages_ctx {
+   struct file *filp;
+   struct address_space *mapping;
+   pgoff_t start_index; /* index of the first page with actual data */
+   char *buf; /* buffer with actual data */
+   int len; /* length of the actual data */
+   int num_pages; /* maximal data chunk (in pages) that can be
+ passed per transmission */
+};
+
+static int init_readpages_ctx(struct v9fs_readpages_ctx *ctx,
+ struct file *filp,
+ struct address_space *mapping,
+ int num_pages)
+{
+   memset(ctx, 0, sizeof(*ctx));
+   ctx->buf = kmalloc(num_pages << PAGE_SHIFT, GFP_USER);
+   if (!ctx->buf)
+   return -ENOMEM;
+   ctx->filp = filp;
+   ctx->mapping = mapping;
+   ctx->num_pages = num_pages;
+   return 0;
+}
+
+static void done_readpages_ctx(struct v9fs_readpages_ctx *ctx)
+{
+   kfree(ctx->buf);
+}
+
+static int receive_buffer(struct file *filp,
+ char *buf,
+ off_t offset, /* offset in the file */
+ int len,
+ int *err)
+{
+   struct kvec kvec;
+   struct iov_iter iter;
+
+   kvec.iov_base = buf;
+   kvec.iov_len = len;
+   iov_iter_kvec(, READ | ITER_KVEC, , 1, len);
+
+   return p9_client_read(filp->private_data, offset, , err);
+}
+
+static int fast_filler(struct v9fs_readpages_ctx *ctx, struct page *page)
+{
+   int err;
+   int ret = 0;
+   char *kdata;
+   int to_page;
+   off_t off_in_buf;
+   struct inode *inode = page->mapping->host;
+
+   BUG_ON(!PageLocked(page));
+   /*
+* first, validate the buffer
+*/
+   if (page->index < ctx->start_index ||
+   ctx->start_index + ctx->num_pages < page->index) {
+   /*
+* No actual data in the buffer,
+* so actualize it
+*/
+   ret = receive_buffer(ctx->filp,
+ctx->buf,
+page_offset(page),
+ctx->num_pages << PAGE_SHIFT,
+);
+   if (err) {
+   printk("failed to receive buffer off=%llu (%d)\n",
+  (unsigned long long)page_offset(page),
+  err);
+   ret = err;
+   goto done;
+   }
+   ctx->start_index = page->index;
+   ctx->len = ret;
+   ret = 0;
+   }
+   /*
+* fill the page with buffer's data
+*/
+   off_in_buf = (page->index - ctx->start_index) << PAGE_SHIFT;
+   if (off_in_buf >= ctx->len) {
+   /*
+* No actual data to fill the page with
+*/
+   ret = -1;
+   goto done;
+   }
+   to_page = ctx->len - off_in_buf;
+   if (to_page >= PAGE_SIZE)
+   to_page = PAGE_SIZE;
+
+   kdata = kmap_atomic(page);
+   memcpy(kdata, ctx->buf + off_in_buf, to_page);
+   memset(kdata + to_page, 0, PAGE_SIZE - to_page);
+   kunmap_atomic(kdata);
+
+   flush_dcache_page(page);
+   SetPageUptodate(page);
+   v9fs_readpage_to_fscache(inode, page);
+ done:
+   unlock_page(page);
+   return ret;
+}
+
+/**
+ * Try to read pages by groups. For every such group we issue only one
+ * read request to the server.
+ * @num_pages: maximal chunk of data (in pages) that can be passed per
+ * such request
+ */
+static int v9fs_readpages_tryfast(struct file *filp,
+ struct address_space *mapping,
+ struct list_head *pages,
+ int num_pages)
+{
+   int ret;
+   struct v9fs_readpages_ctx ctx;
+
+   ret = init_readpages_ctx(, filp, mapping, num_pages);
+   if (ret)
+   /*
+* Can not allocate resources for the fast path,
+* so do it by slow way
+*/
+   return read_cache_pages(mapping, pages,
+  

Re: [Qemu-devel] [PATCH v2] Avoid additional GET_FEATURES call on vhost-user

2016-10-10 Thread Michael S. Tsirkin
On Mon, Oct 10, 2016 at 03:31:05PM +, Felipe Franciosi wrote:
> 
> > On 10 Oct 2016, at 16:24, Michael S. Tsirkin  wrote:
> > 
> > On Mon, Oct 10, 2016 at 03:17:36PM +, Felipe Franciosi wrote:
> >> 
> >>> On 9 Oct 2016, at 23:33, Michael S. Tsirkin  wrote:
> >>> 
> >>> On Thu, Sep 22, 2016 at 01:13:41PM +0100, Felipe Franciosi wrote:
>  Vhost-user requires an early GET_FEATURES call to determine if the
>  slave supports protocol feature negotiation. An extra GET_FEATURES
>  call is made after vhost_backend_init() to actually set the device
>  features.
>  
>  This patch moves the actual setting of the device features to both
>  implementations (kernel/user) of vhost_backend_init(), therefore
>  eliminating the need for a second call.
>  
>  Signed-off-by: Felipe Franciosi 
> >>> 
> >>> 
> >>> Thanks!
> >>> Thinking about this, I think it makes sense to keep
> >>> both messages around.
> >>> This allow backend to change the feature bitmap
> >>> depending on the protocol features negotiated.
> >> 
> >> Hi Michael,
> >> 
> >> Sounds interesting, but I'm struggling a bit to see how that would work. 
> >> IIUC, the "feature" negotiation should relate to anything between device 
> >> and driver. The "protocol feature" should relate to things between backend 
> >> and Qemu. The obvious exception is the "protocol feature bit" itself, 
> >> which is part of the "feature" since protocol features weren't part of the 
> >> original spec.
> >> 
> >> If the protocol or the implementation have deviated from that, we should 
> >> try to make it consistent and clear to avoid further problems.
> >> 
> >> So on the double message, I guess my questions would be:
> >> 1) What features could a backend advertise differently and how that 
> >> relates to protocol features? In other words, how would a protocol feature 
> >> affect the actual relationship between the backend and the driver?
> > 
> > I have some vague ideas, e.g. dealing with migration. I'll Cc you when I do 
> > a writeup (RSN).
> 
> Awesome. Look forward to it... I always had the feeling that all the stuff 
> related with migration should really be on a "set protocol feature" message 
> (and not "set feature"), for the reasons outlined above (it's really about 
> Qemu<->Backend and not Driver<->Backend).
> 
> > 
> > 
> >> 2) What guarantees would a backend have that Qemu is going to ask again 
> >> for the feature bitmap? In other words, if a backend is going to implement 
> >> something differently (regarding handling the driver) depending on what 
> >> protocol bits are supported by Qemu, can it count on Qemu asking for it 
> >> again?
> > 
> > There would be a protocol feature for this. If it's not acked by qemu,
> > it must assume it's done once.
> 
> But wouldn't that be too late in the negotiation? The first thing Qemu
> needs to do is ask for supported features (to determine whether
> protocol features are supported). Meaning, any backend that wants to
> do change their exposed features depending on what protocol features
> are supported will have to implement it both ways (in case the
> required protocol features aren't there). Sounds awfully complicated
> from a backend point of view.

Yes, but that's the cost of compatibility unfortunately.
If you don't care about that, you don't have to implement it.

> But perhaps it will make sense depending
> on what you have in mind for (1) above. Will ping you again on this
> when the text comes.
> 
> > 
> > 
> >> 3) If we stick to the double message, should we document it as intentional 
> >> and exemplify how it could be used to avoid someone else spotting this and 
> >> trying to "fix" it again?
> >> 
> >> Thanks,
> >> Felipe
> > 
> > We can do that when we land such a feature.
> 
> Sure makes sense.
> 
> Thanks!
> Felipe
> 
> > 
> >>> 
>  ---
>  v2. Rebase on d1b4259f
>  
>  hw/virtio/vhost-backend.c | 27 ++-
>  hw/virtio/vhost-user.c|  1 +
>  hw/virtio/vhost.c |  9 -
>  3 files changed, 19 insertions(+), 18 deletions(-)
>  
>  diff --git a/hw/virtio/vhost-backend.c b/hw/virtio/vhost-backend.c
>  index 272a5ec..0ffa4d1 100644
>  --- a/hw/virtio/vhost-backend.c
>  +++ b/hw/virtio/vhost-backend.c
>  @@ -25,15 +25,6 @@ static int vhost_kernel_call(struct vhost_dev *dev, 
>  unsigned long int request,
> return ioctl(fd, request, arg);
>  }
>  
>  -static int vhost_kernel_init(struct vhost_dev *dev, void *opaque)
>  -{
>  -assert(dev->vhost_ops->backend_type == VHOST_BACKEND_TYPE_KERNEL);
>  -
>  -dev->opaque = opaque;
>  -
>  -return 0;
>  -}
>  -
>  static int vhost_kernel_cleanup(struct vhost_dev *dev)
>  {
> int fd = (uintptr_t) dev->opaque;
>  @@ -185,6 +176,24 @@ static int vhost_kernel_vsock_set_running(struct 
>  vhost_dev 

[Qemu-devel] [PATCH v2 1/4] target-arm: Infrastucture changes to enable handling of tagged address loading into PC

2016-10-10 Thread Thomas Hanson
When capturing the current CPU state for the TB, extract the TBI0 and TBI1
values from the correct TCR for the current EL and then add them to the TB
flags field.

Then, at the start of code generation for the block, copy the TBI fields
into the DisasContext structure.

Signed-off-by: Thomas Hanson 
---
 target-arm/cpu.h   | 39 +--
 target-arm/helper.c| 46 ++
 target-arm/translate-a64.c |  2 ++
 target-arm/translate.h |  2 ++
 4 files changed, 87 insertions(+), 2 deletions(-)

diff --git a/target-arm/cpu.h b/target-arm/cpu.h
index 76d824d..699e6e5 100644
--- a/target-arm/cpu.h
+++ b/target-arm/cpu.h
@@ -2191,7 +2191,11 @@ static inline bool 
arm_cpu_data_is_big_endian(CPUARMState *env)
 #define ARM_TBFLAG_BE_DATA_SHIFT20
 #define ARM_TBFLAG_BE_DATA_MASK (1 << ARM_TBFLAG_BE_DATA_SHIFT)
 
-/* Bit usage when in AArch64 state: currently we have no A64 specific bits */
+/* Bit usage when in AArch64 state */
+#define ARM_TBFLAG_TBI0_SHIFT 0/* TBI0 for EL0/1 or TBI for EL2/3 */
+#define ARM_TBFLAG_TBI0_MASK (0x1ull << ARM_TBFLAG_TBI0_SHIFT)
+#define ARM_TBFLAG_TBI1_SHIFT 1/* TBI1 for EL0/1  */
+#define ARM_TBFLAG_TBI1_MASK (0x1ull << ARM_TBFLAG_TBI1_SHIFT)
 
 /* some convenience accessor macros */
 #define ARM_TBFLAG_AARCH64_STATE(F) \
@@ -,6 +2226,10 @@ static inline bool 
arm_cpu_data_is_big_endian(CPUARMState *env)
 (((F) & ARM_TBFLAG_NS_MASK) >> ARM_TBFLAG_NS_SHIFT)
 #define ARM_TBFLAG_BE_DATA(F) \
 (((F) & ARM_TBFLAG_BE_DATA_MASK) >> ARM_TBFLAG_BE_DATA_SHIFT)
+#define ARM_TBFLAG_TBI0(F) \
+(((F) & ARM_TBFLAG_TBI0_MASK) >> ARM_TBFLAG_TBI0_SHIFT)
+#define ARM_TBFLAG_TBI1(F) \
+(((F) & ARM_TBFLAG_TBI1_MASK) >> ARM_TBFLAG_TBI1_SHIFT)
 
 static inline bool bswap_code(bool sctlr_b)
 {
@@ -2319,12 +2327,38 @@ static inline bool arm_cpu_bswap_data(CPUARMState *env)
 }
 #endif
 
+/**
+ * arm_regime_tbi0:
+ * @env: CPUARMState
+ * @mmu_idx: MMU index indicating required translation regime
+ *
+ * Extracts the TBI0 value from the appropriate TCR for the current EL
+ *
+ * Returns: the TBI0 value.
+ */
+extern uint32_t arm_regime_tbi0(CPUARMState *env, ARMMMUIdx mmu_idx);
+
+/**
+ * arm_regime_tbi1:
+ * @env: CPUARMState
+ * @mmu_idx: MMU index indicating required translation regime
+ *
+ * Extracts the TBI1 value from the appropriate TCR for the current EL
+ *
+ * Returns: the TBI1 value.
+ */
+extern uint32_t arm_regime_tbi1(CPUARMState *env, ARMMMUIdx mmu_idx);
+
 static inline void cpu_get_tb_cpu_state(CPUARMState *env, target_ulong *pc,
 target_ulong *cs_base, uint32_t *flags)
 {
+ARMMMUIdx mmu_idx = cpu_mmu_index(env, false);
 if (is_a64(env)) {
 *pc = env->pc;
 *flags = ARM_TBFLAG_AARCH64_STATE_MASK;
+/* Get control bits for tagged addresses */
+*flags |= (arm_regime_tbi0(env, mmu_idx) << ARM_TBFLAG_TBI0_SHIFT);
+*flags |= (arm_regime_tbi1(env, mmu_idx) << ARM_TBFLAG_TBI1_SHIFT);
 } else {
 *pc = env->regs[15];
 *flags = (env->thumb << ARM_TBFLAG_THUMB_SHIFT)
@@ -2343,7 +2377,8 @@ static inline void cpu_get_tb_cpu_state(CPUARMState *env, 
target_ulong *pc,
<< ARM_TBFLAG_XSCALE_CPAR_SHIFT);
 }
 
-*flags |= (cpu_mmu_index(env, false) << ARM_TBFLAG_MMUIDX_SHIFT);
+*flags |= (mmu_idx << ARM_TBFLAG_MMUIDX_SHIFT);
+
 /* The SS_ACTIVE and PSTATE_SS bits correspond to the state machine
  * states defined in the ARM ARM for software singlestep:
  *  SS_ACTIVE   PSTATE.SS   State
diff --git a/target-arm/helper.c b/target-arm/helper.c
index 25f612d..70e2742 100644
--- a/target-arm/helper.c
+++ b/target-arm/helper.c
@@ -6720,6 +6720,52 @@ static inline TCR *regime_tcr(CPUARMState *env, 
ARMMMUIdx mmu_idx)
 return >cp15.tcr_el[regime_el(env, mmu_idx)];
 }
 
+/* Returns TBI0 value for current regime el */
+uint32_t arm_regime_tbi0(CPUARMState *env, ARMMMUIdx mmu_idx)
+{
+TCR *tcr;
+uint32_t el;
+
+/* For EL0 and EL1, TBI is controlled by stage 1's TCR, so convert
+   * a stage 1+2 mmu index into the appropriate stage 1 mmu index.
+   */
+if (mmu_idx == ARMMMUIdx_S12NSE0 || mmu_idx == ARMMMUIdx_S12NSE1) {
+mmu_idx += ARMMMUIdx_S1NSE0;
+}
+
+tcr = regime_tcr(env, mmu_idx);
+el = regime_el(env, mmu_idx);
+
+if (el > 1) {
+return extract64(tcr->raw_tcr, 20, 1);
+} else {
+return extract64(tcr->raw_tcr, 37, 1);
+}
+}
+
+/* Returns TBI1 value for current regime el */
+uint32_t arm_regime_tbi1(CPUARMState *env, ARMMMUIdx mmu_idx)
+{
+TCR *tcr;
+uint32_t el;
+
+/* For EL0 and EL1, TBI is controlled by stage 1's TCR, so convert
+   * a stage 1+2 mmu index into the appropriate stage 1 mmu index.
+   */
+if (mmu_idx == ARMMMUIdx_S12NSE0 || mmu_idx == ARMMMUIdx_S12NSE1) {
+mmu_idx += ARMMMUIdx_S1NSE0;
+}
+
+

[Qemu-devel] [PATCH v2 0/4] target-arm: Handle tagged addresses when loading PC

2016-10-10 Thread Thomas Hanson
If tagged addresses are enabled, then addresses being loaded into the 
PC must be cleaned up by overwriting the tag bits with either all 0's 
or all 1's as specified in the ARM ARM spec.  The decision process is 
dependent on whether the code will be running in EL0/1 or in EL2/3 and 
is controlled by a combination of Top Byte Ignored (TBI) bits in the 
TCR and the value of bit 55 in the address being loaded. 

TBI values are extracted from the appropriate TCR and made available 
to TCG code generation routines by inserting them into the TB flags 
field and then transferring them to DisasContext structure in 
gen_intermediate_code_a64().

New function gen_a64_set_pc_reg() encapsulates the logic required to 
determine whether clean up of the tag byte is required and then 
generating the code to correctly load the PC.
  
In addition to those instruction which can directly load a tagged 
address into the PC, there are others which increment or add a value to
the PC.  If 56 bit addressing is used, these instructions can cause an 
arithmetic roll-over into the tag bits.  The ARM ARM specification for 
handling tagged addresses requires that these cases also be addressed
by cleaning up the tag field.  This work has been deferred because 
there is currently no CPU model available for testing with 56 bit 
addresses.

v1->v2:
  - Updated patch descriptions per Peter's commments
  - Added function header and other comments as recommended
  - Change return type from long to unit32_t for arm_regime_tbi0() &
arm_regime_tbi1()
  - Moved prototype of gen_a64_set_pc_reg() from patch 1 to patch 2
  - Moved assignment of dc->tbi0 & dc->tbi1 from patch 2 to patch 1
  - Split out documentation comments into separate patch.

  Still looking into handling of tagged addresses for exceptions and
  exception returns.  Will handle that as a separate patch set.


Thomas Hanson (4):
  target-arm: Infrastucture changes to enable handling of tagged address
loading into PC
  target-arm: Code changes to implement overwrite of tag field on PC
load
  target-arm: Comments to mark location of pending work for 56 bit
addresses
  target-arm: Comments added to identify cases in a switch

 target-arm/cpu.h   |  39 -
 target-arm/helper.c|  46 +
 target-arm/translate-a64.c | 101 +
 target-arm/translate.h |   3 ++
 4 files changed, 179 insertions(+), 10 deletions(-)

-- 
1.9.1




[Qemu-devel] [PATCH v2 4/4] target-arm: Comments added to identify cases in a switch

2016-10-10 Thread Thomas Hanson
3 cases in a switch in disas_exc() require reference to the
ARM ARM spec in order to determine what case they're handling.

Signed-off-by: Thomas Hanson 
---
 target-arm/translate-a64.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/target-arm/translate-a64.c b/target-arm/translate-a64.c
index a1e5f2c..728e929 100644
--- a/target-arm/translate-a64.c
+++ b/target-arm/translate-a64.c
@@ -1678,12 +1678,12 @@ static void disas_exc(DisasContext *s, uint32_t insn)
  * instruction works properly.
  */
 switch (op2_ll) {
-case 1:
+case 1: /* SVC */
 gen_ss_advance(s);
 gen_exception_insn(s, 0, EXCP_SWI, syn_aa64_svc(imm16),
default_exception_el(s));
 break;
-case 2:
+case 2: /* HVC */
 if (s->current_el == 0) {
 unallocated_encoding(s);
 break;
@@ -1696,7 +1696,7 @@ static void disas_exc(DisasContext *s, uint32_t insn)
 gen_ss_advance(s);
 gen_exception_insn(s, 0, EXCP_HVC, syn_aa64_hvc(imm16), 2);
 break;
-case 3:
+case 3: /* SMC */
 if (s->current_el == 0) {
 unallocated_encoding(s);
 break;
-- 
1.9.1




[Qemu-devel] [PATCH v2 3/4] target-arm: Comments to mark location of pending work for 56 bit addresses

2016-10-10 Thread Thomas Hanson
Certain instructions which can not directly load a tagged address value
may trigger a corner case when the address size is 56 bits.  This is
because incrementing or offsetting from the current PC can cause an
arithetic roll-over into the tag bits.  Per the ARM ARM spec, these cases
should also be addressed by cleaning up the tag field.

Signed-off-by: Thomas Hanson 
---
 target-arm/translate-a64.c | 12 
 1 file changed, 12 insertions(+)

diff --git a/target-arm/translate-a64.c b/target-arm/translate-a64.c
index 14e91fb..a1e5f2c 100644
--- a/target-arm/translate-a64.c
+++ b/target-arm/translate-a64.c
@@ -1222,6 +1222,9 @@ static inline AArch64DecodeFn *lookup_disas_fn(const 
AArch64DecodeTable *table,
  */
 static void disas_uncond_b_imm(DisasContext *s, uint32_t insn)
 {
+/*If/when address size is 56 bits, this could overflow into address tag
+ * byte, and that byte should be fixed per ARM ARM spec.
+ */
 uint64_t addr = s->pc + sextract32(insn, 0, 26) * 4 - 4;
 
 if (insn & (1U << 31)) {
@@ -1249,6 +1252,9 @@ static void disas_comp_b_imm(DisasContext *s, uint32_t 
insn)
 sf = extract32(insn, 31, 1);
 op = extract32(insn, 24, 1); /* 0: CBZ; 1: CBNZ */
 rt = extract32(insn, 0, 5);
+/*If/when address size is 56 bits, this could overflow into address tag
+ * byte, and that byte should be fixed per ARM ARM spec.
+ */
 addr = s->pc + sextract32(insn, 5, 19) * 4 - 4;
 
 tcg_cmp = read_cpu_reg(s, rt, sf);
@@ -1277,6 +1283,9 @@ static void disas_test_b_imm(DisasContext *s, uint32_t 
insn)
 
 bit_pos = (extract32(insn, 31, 1) << 5) | extract32(insn, 19, 5);
 op = extract32(insn, 24, 1); /* 0: TBZ; 1: TBNZ */
+/*If/when address size is 56 bits, this could overflow into address tag
+ * byte, and that byte should be fixed per ARM ARM spec.
+ */
 addr = s->pc + sextract32(insn, 5, 14) * 4 - 4;
 rt = extract32(insn, 0, 5);
 
@@ -1306,6 +1315,9 @@ static void disas_cond_b_imm(DisasContext *s, uint32_t 
insn)
 unallocated_encoding(s);
 return;
 }
+/*If/when address size is 56 bits, this could overflow into address tag
+ * byte, and that byte should be fixed per ARM ARM spec.
+ */
 addr = s->pc + sextract32(insn, 5, 19) * 4 - 4;
 cond = extract32(insn, 0, 4);
 
-- 
1.9.1




[Qemu-devel] [PATCH v2 2/4] target-arm: Code changes to implement overwrite of tag field on PC load

2016-10-10 Thread Thomas Hanson
For BR, BLR and RET instructions, if tagged addresses are enabled, the
tag field in the address must be cleared out prior to loading the
address into the PC.  Depending on the current EL, it will be set to
either all 0's or all 1's.

Signed-off-by: Thomas Hanson 
---
 target-arm/translate-a64.c | 81 +++---
 target-arm/translate.h |  1 +
 2 files changed, 77 insertions(+), 5 deletions(-)

diff --git a/target-arm/translate-a64.c b/target-arm/translate-a64.c
index 3b15d2c..14e91fb 100644
--- a/target-arm/translate-a64.c
+++ b/target-arm/translate-a64.c
@@ -41,6 +41,7 @@ static TCGv_i64 cpu_pc;
 
 /* Load/store exclusive handling */
 static TCGv_i64 cpu_exclusive_high;
+static TCGv_i64 cpu_reg(DisasContext *s, int reg);
 
 static const char *regnames[] = {
 "x0", "x1", "x2", "x3", "x4", "x5", "x6", "x7",
@@ -176,6 +177,75 @@ void gen_a64_set_pc_im(uint64_t val)
 tcg_gen_movi_i64(cpu_pc, val);
 }
 
+/* Load the PC from a register.
+ *
+ * If address tagging is enabled via the TCR TBI bits, then loading
+ * an address into the PC will clear out any tag in the it:
+ *  + for EL2 and EL3 there is only one TBI bit, and if it is set
+ *then the address is zero-extended, clearing bits [63:56]
+ *  + for EL0 and EL1, TBI0 controls addresses with bit 55 == 0
+ *and TBI1 controls addressses with bit 55 == 1.
+ *If the appropriate TBI bit is set for the address then
+ *the address is sign-extended from bit 55 into bits [63:56]
+ *
+ * We can avoid doing this for relative-branches, because the
+ * PC + offset can never overflow into the tag bits (assuming
+ * that virtual addresses are less than 56 bits wide, as they
+ * are currently), but we must handle it for branch-to-register.
+ */
+void gen_a64_set_pc_reg(DisasContext *s, unsigned int rn)
+{
+if (s->current_el <= 1) {
+/* Test if NEITHER or BOTH TBI values are set.  If so, no need to
+ * examine bit 55 of address, can just generate code.
+ * If mixed, then test via generated code
+ */
+if (s->tbi0 && s->tbi1) {
+TCGv_i64 tmp_reg = tcg_temp_new_i64();
+/* Both bits set, sign extension from bit 55 into [63:56] will
+ * cover both cases
+ */
+tcg_gen_shli_i64(tmp_reg, cpu_reg(s, rn), 8);
+tcg_gen_sari_i64(cpu_pc, tmp_reg, 8);
+tcg_temp_free_i64(tmp_reg);
+} else if (!s->tbi0 && !s->tbi1) {
+/* Neither bit set, just load it as-is */
+tcg_gen_mov_i64(cpu_pc, cpu_reg(s, rn));
+} else {
+TCGv_i64 tcg_tmpval = tcg_temp_new_i64();
+TCGv_i64 tcg_bit55  = tcg_temp_new_i64();
+TCGv_i64 tcg_zero   = tcg_const_i64(0);
+
+tcg_gen_andi_i64(tcg_bit55, cpu_reg(s, rn), (1ull << 55));
+
+if (s->tbi0) {
+/* tbi0==1, tbi1==0, so 0-fill upper byte if bit 55 = 0 */
+tcg_gen_andi_i64(tcg_tmpval, cpu_reg(s, rn),
+ 0x00FFull);
+tcg_gen_movcond_i64(TCG_COND_EQ, cpu_pc, tcg_bit55, tcg_zero,
+tcg_tmpval, cpu_reg(s, rn));
+} else {
+/* tbi0==0, tbi1==1, so 1-fill upper byte if bit 55 = 1 */
+tcg_gen_ori_i64(tcg_tmpval, cpu_reg(s, rn),
+0xFF00ull);
+tcg_gen_movcond_i64(TCG_COND_NE, cpu_pc, tcg_bit55, tcg_zero,
+tcg_tmpval, cpu_reg(s, rn));
+}
+tcg_temp_free_i64(tcg_zero);
+tcg_temp_free_i64(tcg_bit55);
+tcg_temp_free_i64(tcg_tmpval);
+}
+} else {  /* EL > 1 */
+if (s->tbi0) {
+/* Force tag byte to all zero */
+tcg_gen_andi_i64(cpu_pc, cpu_reg(s, rn), 0x00FFull);
+} else {
+/* Load unmodified address */
+tcg_gen_mov_i64(cpu_pc, cpu_reg(s, rn));
+}
+ }
+}
+
 typedef struct DisasCompare64 {
 TCGCond cond;
 TCGv_i64 value;
@@ -1704,12 +1774,13 @@ static void disas_uncond_b_reg(DisasContext *s, 
uint32_t insn)
 
 switch (opc) {
 case 0: /* BR */
-case 2: /* RET */
-tcg_gen_mov_i64(cpu_pc, cpu_reg(s, rn));
-break;
 case 1: /* BLR */
-tcg_gen_mov_i64(cpu_pc, cpu_reg(s, rn));
-tcg_gen_movi_i64(cpu_reg(s, 30), s->pc);
+case 2: /* RET */
+gen_a64_set_pc_reg(s, rn);
+/* BLR also needs to load return address */
+if (opc == 1) {
+tcg_gen_movi_i64(cpu_reg(s, 30), s->pc);
+}
 break;
 case 4: /* ERET */
 if (s->current_el == 0) {
diff --git a/target-arm/translate.h b/target-arm/translate.h
index a53f25a..49c042e 100644
--- a/target-arm/translate.h
+++ b/target-arm/translate.h
@@ -129,6 +129,7 @@ static inline int default_exception_el(DisasContext *s)
 void 

Re: [Qemu-devel] [PATCH 20/22] qcow2-dirty-bitmap: refcounts

2016-10-10 Thread Max Reitz
On 30.09.2016 12:53, Vladimir Sementsov-Ogievskiy wrote:
> Calculate refcounts for dirty bitmaps.

The commit message should mention that this is for qcow2's qemu-img
check implementation.

> 
> Signed-off-by: Vladimir Sementsov-Ogievskiy 
> ---
>  block/qcow2-bitmap.c   | 60 
> ++
>  block/qcow2-refcount.c | 14 +++-
>  block/qcow2.h  |  5 +
>  3 files changed, 74 insertions(+), 5 deletions(-)
> 
> diff --git a/block/qcow2-bitmap.c b/block/qcow2-bitmap.c
> index 76f7e2b..6d9394a 100644
> --- a/block/qcow2-bitmap.c
> +++ b/block/qcow2-bitmap.c
> @@ -901,3 +901,63 @@ out:
>  g_free(new_dir);
>  g_free(dir);
>  }
> +
> +int qcow2_dirty_bitmaps_refcounts(BlockDriverState *bs,
> +  BdrvCheckResult *res,
> +  void **refcount_table,
> +  int64_t *refcount_table_size)

I'd rename this function to make clear that this is for checking the
refcounts, e.g. to "qcow2_check_dirty_bitmaps_refcounts" or
"qcow2_count_dirty_bitmaps_refcounts" or just
"qcow2_check_dirty_bitmaps". Probably the last one is the best because
this function should ideally perform a full check of the dirty bitmaps
extension.

> +{
> +BDRVQcow2State *s = bs->opaque;
> +uint8_t *dir;
> +Qcow2BitmapDirEntry *e;
> +Error *local_err = NULL;
> +
> +int i;
> +int ret = inc_refcounts(bs, res, refcount_table, refcount_table_size,
> +s->bitmap_directory_offset,
> +s->bitmap_directory_size);
> +if (ret < 0) {
> +return ret;
> +}
> +
> +dir = directory_read(bs, s->bitmap_directory_offset,
> + s->bitmap_directory_size, _err);
> +if (dir == NULL) {
> +error_report_err(local_err);

I think you should increment res->corruptions here.

> +return -EINVAL;
> +}
> +
> +for_each_bitmap_dir_entry(e, dir, s->bitmap_directory_size) {
> +uint64_t *bitmap_table = NULL;

I think you should call check_constraints() here.

> +
> +ret = inc_refcounts(bs, res, refcount_table, refcount_table_size,
> +e->bitmap_table_offset,
> +e->bitmap_table_size);

Probably rather e->bitmap_table_size * sizeof(uint64_t).

> +if (ret < 0) {
> +return ret;
> +}
> +
> +ret = bitmap_table_load(bs, e, _table);
> +if (ret < 0) {

Again, it would make sense to increment res->corruptions here.

> +return ret;
> +}
> +
> +for (i = 0; i < e->bitmap_table_size; ++i) {
> +uint64_t off = be64_to_cpu(bitmap_table[i]);
> +if (off <= 1) {
> +continue;
> +}

As I said in some other patch, I'd write this condition differently
(with an offset mask, etc.).

Also, you should check that the offset is aligned to a cluster boundary
and that no unknown flags are set.

> +
> +ret = inc_refcounts(bs, res, refcount_table, refcount_table_size,
> +off, s->cluster_size);
> +if (ret < 0) {
> +g_free(bitmap_table);
> +return ret;
> +}
> +}
> +
> +g_free(bitmap_table);
> +}
> +
> +return 0;

dir is leaked here.

> +}
> diff --git a/block/qcow2-refcount.c b/block/qcow2-refcount.c
> index cbfb3fe..05bcc57 100644
> --- a/block/qcow2-refcount.c
> +++ b/block/qcow2-refcount.c
> @@ -1309,11 +1309,9 @@ static int realloc_refcount_array(BDRVQcow2State *s, 
> void **array,
>   *
>   * Modifies the number of errors in res.
>   */
> -static int inc_refcounts(BlockDriverState *bs,
> - BdrvCheckResult *res,
> - void **refcount_table,
> - int64_t *refcount_table_size,
> - int64_t offset, int64_t size)
> +int inc_refcounts(BlockDriverState *bs, BdrvCheckResult *res,
> +  void **refcount_table, int64_t *refcount_table_size,
> +  int64_t offset, int64_t size)

First, if you make this function public, you have to give it a qcow2_
prefix.

Second, if this function is public, it should have a name that makes
sense. inc_refcounts() sounds as if it's the same as update_refcount()
with an addend of 1. I'd rename it qcow2_inc_refcounts_imrt(), because
that's probably the shortest name I can come up with (and
qcow2-refcount.c explains what an IMRT is in some comment).

>  {
>  BDRVQcow2State *s = bs->opaque;
>  uint64_t start, last, cluster_offset, k, refcount;
> @@ -1843,6 +1841,12 @@ static int calculate_refcounts(BlockDriverState *bs, 
> BdrvCheckResult *res,
>  return ret;
>  }
>  
> +/* dirty bitmaps */
> +ret = qcow2_dirty_bitmaps_refcounts(bs, res, refcount_table, 
> nb_clusters);
> +if (ret < 0) {
> +return ret;
> +}

Re: [Qemu-devel] [PATCH v5 6/7] intel_iommu: reject broken EIM

2016-10-10 Thread Eduardo Habkost
On Mon, Oct 10, 2016 at 05:28:47PM +0200, Radim Krčmář wrote:
> Cluster x2APIC cannot work without KVM's x2apic API when the maximal
> APIC ID is greater than 8 and only KVM's LAPIC can support x2APIC, so we
> forbid other APICs and also the old KVM case with less than 9, to
> simplify the code.
> 
> There is no point in enabling EIM in forbidden APICs, so we keep it
> enabled only for the KVM APIC;  unconditionally, because making the
> option depend on KVM version would be a maintanance burden.
> 
> Old QEMUs would enable eim whenever intremap was on, which would trick
> guests into thinking that they can enable cluster x2APIC even if any
> interrupt destination would get clamped to 8 bits.
> Depending on your configuration, QEMU could notice that the destination
> LAPIC is not present and report it with a very non-obvious:
> 
>   KVM: injection failed, MSI lost (Operation not permitted)
> 
> Or the guest could say something about unexpected interrupts, because
> clamping leads to aliasing so interrupts were being delivered to
> incorrect VCPUs.
> 
> KVM_X2APIC_API is the feature that allows us to enable EIM for KVM.
> 
> QEMU 2.7 allowed EIM whenever interrupt remapping was enabled.  In order
> to keep backward compatibility, we again allow guests to misbehave in
> non-obvious ways, and make it the default for old machine types.
> 
> A user can enable the buggy mode it with "x-buggy-eim=on".
> 
> Signed-off-by: Radim Krčmář 

Reviewed-by: Eduardo Habkost 

-- 
Eduardo



[Qemu-devel] Deprecating old machine-types (was Re: [PATCH v4 7/8] intel_iommu: keep buggy EIM enabled in 2.7 machine type)

2016-10-10 Thread Eduardo Habkost
On Thu, Oct 06, 2016 at 05:55:25PM +0200, Radim Krčmář wrote:
[...]
> pc-0.10 seems to be the first machine type ever (2009), is there already
> a plan to deprecate it?

I don't think we have a plan, but I would support deprecating and
removing very old machine-types. The question is: how old is too
old?

For reference, the commits and dates when each machine-type was
added are below:

machine   commitcommit date  release  release date
pc-0.10   e8b2a1c6  Jul 8 2009   v0.11.0  Sep 22 2009
pc-0.13   95747581  Jul 22 2009  v0.12.0  Dec 19 2009
pc-0.12   2cae6f5e  Jan 8 2010   v0.13.0  Oct 14 2010
pc-0.13   d76fa62d  Feb 15 2010  v0.13.0  Oct 14 2010
pc-0.14   b903a0f7  Nov 11 2010  v0.14.0  Feb 16 2011
pc-0.15   ce01a508  Dec 18 2011  v1.1.0   Jun 1 2012
pc-1.019857e62  Nov 7 2011   v1.0 Dec 1 2011
pc-1.1382b3a68  Feb 21 2012  v1.1.0   Jun 1 2012
pc-1.2f1dacf1c  Jun 11 2012  v1.2.0   Sep 5 2012
pc-1.3f4306941  Sep 13 2012  v1.3.0   Dec 3 2012
pc-*-1.4  f1ae2e38  Dec 4 2012   v1.4.0   Feb 15 2013
pc-*-1.5  bf3caa3d  Feb 8 2013   v1.5.0   May 20 2013
pc-*-1.6  45053fde  May 27 2013  v1.6.0   Aug 15 2013
pc-*-1.7  e9845f09  Aug 2 2013   v1.7.0   Nov 27 2013
pc-*-2.0  aeca6e8d  Dec 2 2013   v2.0.0   Apr 17 2014
pc-*-2.1  3458b2b0  Apr 24 2014  v2.1.0   Aug 1 2014
pc-*-2.2  f9f21873  Jul 30 2014  v2.2.0   Dec 9 2014
pc-*-2.3  64bbd372  Dec 5 2014   v2.3.0   Apr 24 2015
pc-*-2.4  5cb50e0a  Apr 23 2015  v2.4.0   Aug 11 2015
pc-*-2.5  87e896ab  Sep 11 2015  v2.5.0   Dec 16 2015
pc-*-2.6  240240d5  Nov 30 2015  v2.6.0   May 11 2016
pc-*-2.7  d86c1451  May 17 2016  v2.7.0   Sep 2 2016
pc-*-2.8  a4d3c834  Sep 7 2016   --

-- 
Eduardo



Re: [Qemu-devel] [PATCH] kvm-all: don't use stale dbg_data->cpu

2016-10-10 Thread Christian Borntraeger
On 10/10/2016 06:39 PM, Paolo Bonzini wrote:
> 
> 
> On 10/10/2016 17:46, Alex Bennée wrote:
>> The changes to run_on_cpu and friends mean that all helpers are passed
>> the CPUState of vCPU they are running on. The conversion missed the
>> field in commit e0eeb4a21a3ca4b296220ce4449d8acef9de9049 which
>> introduced bugs.
>>
>> Reported-by: Claudio Imbrenda 
>> Signed-off-by: Alex Bennée 
>> ---
>>  kvm-all.c | 5 ++---
>>  1 file changed, 2 insertions(+), 3 deletions(-)
>>
>> diff --git a/kvm-all.c b/kvm-all.c
>> index efb5fe3..3dcce16 100644
>> --- a/kvm-all.c
>> +++ b/kvm-all.c
>> @@ -2215,15 +2215,14 @@ int kvm_sw_breakpoints_active(CPUState *cpu)
>>  
>>  struct kvm_set_guest_debug_data {
>>  struct kvm_guest_debug dbg;
>> -CPUState *cpu;
>>  int err;
>>  };
>>  
>> -static void kvm_invoke_set_guest_debug(CPUState *unused_cpu, void *data)
>> +static void kvm_invoke_set_guest_debug(CPUState *cpu, void *data)
>>  {
>>  struct kvm_set_guest_debug_data *dbg_data = data;
>>  
>> -dbg_data->err = kvm_vcpu_ioctl(dbg_data->cpu, KVM_SET_GUEST_DEBUG,
>> +dbg_data->err = kvm_vcpu_ioctl(cpu, KVM_SET_GUEST_DEBUG,
>> _data->dbg);
>>  }
>>  
>>
> 
> Queued, thanks - or if Christian wants to send a pull request himself,
> he can go ahead.

Lets wait for Claudio to verify and then it can go via your tree.

PS: Can you also have a look at Claudios 2nd patch?





Re: [Qemu-devel] [PATCH v6 1/3] target-i386: Move warning code outside x86_cpu_filter_features()

2016-10-10 Thread Eduardo Habkost
On Mon, Oct 10, 2016 at 01:57:08PM +0200, Igor Mammedov wrote:
> On Fri,  7 Oct 2016 17:29:00 -0300
> Eduardo Habkost  wrote:
> 
> > x86_cpu_filter_features() will be reused by code that shouldn't
> > print any warning. Move the warning code to a new
> > x86_cpu_report_filtered_features() function, and call it from
> > x86_cpu_realizefn().
> > 
> > Signed-off-by: Eduardo Habkost 
> Reviewed-by: Igor Mammedov 

Thanks. Applied to x86-next.

-- 
Eduardo



Re: [Qemu-devel] [PATCH 19/22] iotests: test qcow2 persistent dirty bitmap

2016-10-10 Thread Max Reitz
On 30.09.2016 12:53, Vladimir Sementsov-Ogievskiy wrote:
> Signed-off-by: Vladimir Sementsov-Ogievskiy 
> ---
>  tests/qemu-iotests/165 | 87 
> ++
>  tests/qemu-iotests/165.out |  5 +++
>  tests/qemu-iotests/group   |  1 +
>  3 files changed, 93 insertions(+)
>  create mode 100755 tests/qemu-iotests/165
>  create mode 100644 tests/qemu-iotests/165.out
> 
> diff --git a/tests/qemu-iotests/165 b/tests/qemu-iotests/165
> new file mode 100755
> index 000..a69799c
> --- /dev/null
> +++ b/tests/qemu-iotests/165
> @@ -0,0 +1,87 @@
> +#!/usr/bin/env python
> +#
> +# Tests for persistent dirty bitmaps.
> +#
> +# Copyright: Vladimir Sementsov-Ogievskiy 2015-2016
> +#
> +# This program is free software; you can redistribute it and/or modify
> +# it under the terms of the GNU General Public License as published by
> +# the Free Software Foundation; either version 2 of the License, or
> +# (at your option) any later version.
> +#
> +# This program is distributed in the hope that it will be useful,
> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +# GNU General Public License for more details.
> +#
> +# You should have received a copy of the GNU General Public License
> +# along with this program.  If not, see .
> +#
> +
> +import os
> +import iotests
> +from iotests import qemu_img
> +
> +disk = os.path.join(iotests.test_dir, 'disk')
> +disk_size = 0x4000 # 1G
> +
> +# regions for qemu_io: (start, count) in bytes
> +regions1 = ((0,0x10),
> +(0x20, 0x10))
> +
> +regions2 = ((0x1000, 0x2),
> +(0x3999, 0x1))

Did you mean 0x3fff?

Looks good apart from that (and the fact that I don't like the @md5
field in query-block too much, but that's a matter for patch 18).

Max

> +
> +class TestPersistentDirtyBitmap(iotests.QMPTestCase):
> +
> +def setUp(self):
> +qemu_img('create', '-f', iotests.imgfmt, disk, str(disk_size))
> +
> +def tearDown(self):
> +os.remove(disk)
> +
> +def mkVm(self):
> +return iotests.VM().add_drive(disk)
> +
> +def getMd5(self):
> +result = self.vm.qmp('query-block');
> +return result['return'][0]['dirty-bitmaps'][0]['md5']
> +
> +def checkBitmap(self, md5):
> +result = self.vm.qmp('query-block');
> +self.assert_qmp(result, 'return[0]/dirty-bitmaps[0]/md5', md5);
> +
> +def writeRegions(self, regions):
> +for r in regions:
> +  self.vm.hmp_qemu_io('drive0',
> +  'write %d %d' % r)
> +
> +def qmpAddBitmap(self):
> +self.vm.qmp('block-dirty-bitmap-add', node='drive0',
> +name='bitmap0', persistent=True, autoload=True)
> +
> +def test_persistent(self):
> +self.vm = self.mkVm()
> +self.vm.launch()
> +self.qmpAddBitmap()
> +
> +self.writeRegions(regions1)
> +md5 = self.getMd5()
> +
> +self.vm.shutdown()
> +self.vm = self.mkVm()
> +self.vm.launch()
> +
> +self.checkBitmap(md5)
> +self.writeRegions(regions2)
> +md5 = self.getMd5()
> +
> +self.vm.shutdown()
> +self.vm.launch()
> +
> +self.checkBitmap(md5)
> +
> +self.vm.shutdown()
> +
> +if __name__ == '__main__':
> +iotests.main(supported_fmts=['qcow2'])
> diff --git a/tests/qemu-iotests/165.out b/tests/qemu-iotests/165.out
> new file mode 100644
> index 000..ae1213e
> --- /dev/null
> +++ b/tests/qemu-iotests/165.out
> @@ -0,0 +1,5 @@
> +.
> +--
> +Ran 1 tests
> +
> +OK
> diff --git a/tests/qemu-iotests/group b/tests/qemu-iotests/group
> index 503eb27..ed14294 100644
> --- a/tests/qemu-iotests/group
> +++ b/tests/qemu-iotests/group
> @@ -161,3 +161,4 @@
>  159 rw auto quick
>  160 rw auto quick
>  162 auto quick
> +165 rw auto quick
> 




signature.asc
Description: OpenPGP digital signature


Re: [Qemu-devel] [PATCH 00/16] target-sparc improvements

2016-10-10 Thread no-reply
Hi,

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

Subject: [Qemu-devel] [PATCH 00/16] target-sparc improvements
Message-id: 1476112628-12241-1-git-send-email-...@twiddle.net
Type: series

=== 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 show --no-patch --format=%s $c)..."
if ! git show $c --format=email | ./scripts/checkpatch.pl --mailback -; then
failed=1
echo
fi
n=$((n+1))
done

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

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
From https://github.com/patchew-project/qemu
 - [tag update]  
patchew/1476031419-6805-1-git-send-email-mark.cave-ayl...@ilande.co.uk -> 
patchew/1476031419-6805-1-git-send-email-mark.cave-ayl...@ilande.co.uk
 * [new tag] patchew/1476112628-12241-1-git-send-email-...@twiddle.net 
-> patchew/1476112628-12241-1-git-send-email-...@twiddle.net
 * [new tag] 
patchew/1476117341-32690-1-git-send-email-drjo...@redhat.com -> 
patchew/1476117341-32690-1-git-send-email-drjo...@redhat.com
Switched to a new branch 'test'
4974aa6 target-sparc: Use tcg_gen_atomic_cmpxchg_tl
b769d76 target-sparc: Use tcg_gen_atomic_xchg_tl
8ba7e23 target-sparc: Optmize writeback of cpu_cond
ac44dbe target-sparc: Remove MMU_MODE*_SUFFIX
9ad35ee target-sparc: Allow 4-byte alignment on fp mem ops
ee1cd04 target-sparc: Implement ldqf and stqf inline
105cd23 target-sparc: Remove asi helper code handled inline
8e2be2e target-sparc: Implement BCOPY/BFILL inline
38cbb5d target-sparc: Implement cas_asi/casx_asi inline
6ca4c14 target-sparc: Implement ldstub_asi inline
d90214c target-sparc: Implement swap_asi inline
e652593 target-sparc: Handle more twinx asis
9af8f40 target-sparc: Use MMU_PHYS_IDX for bypass asis
1f3996d target-sparc: Add MMU_PHYS_IDX
38fa924 target-sparc: Introduce cpu_raise_exception_ra
30abd6e target-sparc: Use overalignment flags for twinx and block asis

=== OUTPUT BEGIN ===
Checking PATCH 1/16: target-sparc: Use overalignment flags for twinx and block 
asis...
Checking PATCH 2/16: target-sparc: Introduce cpu_raise_exception_ra...
Checking PATCH 3/16: target-sparc: Add MMU_PHYS_IDX...
Checking PATCH 4/16: target-sparc: Use MMU_PHYS_IDX for bypass asis...
Checking PATCH 5/16: target-sparc: Handle more twinx asis...
Checking PATCH 6/16: target-sparc: Implement swap_asi inline...
Checking PATCH 7/16: target-sparc: Implement ldstub_asi inline...
Checking PATCH 8/16: target-sparc: Implement cas_asi/casx_asi inline...
Checking PATCH 9/16: target-sparc: Implement BCOPY/BFILL inline...
Checking PATCH 10/16: target-sparc: Remove asi helper code handled inline...
Checking PATCH 11/16: target-sparc: Implement ldqf and stqf inline...
Checking PATCH 12/16: target-sparc: Allow 4-byte alignment on fp mem ops...
ERROR: spaces required around that '/' (ctx:VxV)
#47: FILE: target-sparc/translate.c:2484:
+tcg_gen_qemu_ld_i64(cpu_fpr[rd/2+1], addr, da.mem_idx,
   ^

ERROR: spaces required around that '+' (ctx:VxV)
#47: FILE: target-sparc/translate.c:2484:
+tcg_gen_qemu_ld_i64(cpu_fpr[rd/2+1], addr, da.mem_idx,
 ^

total: 2 errors, 0 warnings, 175 lines checked

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

Checking PATCH 13/16: target-sparc: Remove MMU_MODE*_SUFFIX...
Checking PATCH 14/16: target-sparc: Optmize writeback of cpu_cond...
Checking PATCH 15/16: target-sparc: Use tcg_gen_atomic_xchg_tl...
Checking PATCH 16/16: target-sparc: Use tcg_gen_atomic_cmpxchg_tl...
=== OUTPUT END ===

Test command exited with code: 1


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

Re: [Qemu-devel] [PATCH 18/22] qapi: add md5 checksum of last dirty bitmap level to query-block

2016-10-10 Thread Max Reitz
On 10.10.2016 18:44, Max Reitz wrote:
> On 30.09.2016 12:53, Vladimir Sementsov-Ogievskiy wrote:
>> Reviewed-by: John Snow 
>> Reviewed-by: Eric Blake 
>> Signed-off-by: Vladimir Sementsov-Ogievskiy 
>> ---
>>  block/dirty-bitmap.c   | 1 +
>>  include/qemu/hbitmap.h | 8 
>>  qapi/block-core.json   | 5 -
>>  util/hbitmap.c | 8 
>>  4 files changed, 21 insertions(+), 1 deletion(-)
> 
> Having read John's and Eric's comments, I won't block this patch, but I
> won't give an R-b either.
> 
> It's probably true that this will not significantly slow down the
> query-block call, but doing this only for debugging does not seem right
> to me.
> 
> I'm not sure what the right way would be to get this information out
> (...maybe make it optional and set it only if qtest_enabled() is true?),
> but in my opinion this is not the right way.

By the way, the cleanest way I can come up with (which I didn't write
about in my first reply because it's not so trivial) would be some kind
of debugging QMP command convention. For instance, we could say that all
debugging commands have an x-debug- prefix, and then you could add an
x-debug-get-bitmap-md5 to read the MD5 hash of a named bitmap. That
would appear to be the cleanest way to do this to me.

Max

> Since I'm not the maintainer of the bitmap code (Fam and John are, even
> though their MAINTAINERS patch is not in master still...), I can't and
> won't block this, though.
> 
> Max
> 




signature.asc
Description: OpenPGP digital signature


Re: [Qemu-devel] [PATCH v6 3/3] target-i386: Return runnability information on query-cpu-definitions

2016-10-10 Thread Eduardo Habkost
On Mon, Oct 10, 2016 at 02:27:49PM +0200, Igor Mammedov wrote:
> On Fri,  7 Oct 2016 17:29:02 -0300
> Eduardo Habkost  wrote:
> 
> > Fill the "unavailable-features" field on the x86 implementation
> > of query-cpu-definitions.
> > 
> > Cc: Jiri Denemark 
> > Cc: libvir-l...@redhat.com
> > Signed-off-by: Eduardo Habkost 
> > ---
> > Changes v5 -> v6:
> > * Call x86_cpu_filter_features(), now that x86_cpu_load_features()
> >   won't run it automatically
> > 
> > Changes v4 -> v5:
> > * (none)
> > 
> > Changes v3 -> v4:
> > * Handle missing XSAVE components cleanly, but looking up
> >   the original feature that required it
> > * Use x86_cpu_load_features() function
> > 
> > Changes v2 -> v3:
> > * Create a x86_cpu_feature_name() function, to
> >   isolate the code that returns the property name
> > 
> > Changes v1 -> v2:
> > * Updated to the new schema: no @runnable field, and
> >   always report @unavailable-features as present
> > ---
> >  target-i386/cpu.c | 76 
> > +++
> >  1 file changed, 76 insertions(+)
> > 
> > diff --git a/target-i386/cpu.c b/target-i386/cpu.c
> > index 23cc19b..63330ce 100644
> > --- a/target-i386/cpu.c
> > +++ b/target-i386/cpu.c
> > @@ -1945,6 +1945,27 @@ static inline void feat2prop(char *s)
> >  }
> >  }
> >  
> > +/* Return the feature property name for a feature flag bit */
> > +static const char *x86_cpu_feature_name(FeatureWord w, int bitnr)
> > +{
> > +/* XSAVE components are automatically enabled by other features,
> > + * so return the original feature name instead
> > + */
> > +if (w == FEAT_XSAVE_COMP_LO || w == FEAT_XSAVE_COMP_HI) {
> > +int comp = (w == FEAT_XSAVE_COMP_HI) ? bitnr + 32 : bitnr;
> > +
> > +if (comp < ARRAY_SIZE(x86_ext_save_areas) &&
> > +x86_ext_save_areas[comp].bits) {
> > +w = x86_ext_save_areas[comp].feature;
> > +bitnr = ctz32(x86_ext_save_areas[comp].bits);
> > +}
> > +}
> > +
> > +assert(bitnr < 32);
> > +assert(w < FEATURE_WORDS);
> > +return feature_word_info[w].feat_names[bitnr];
> > +}
> > +
> >  /* Compatibily hack to maintain legacy +-feat semantic,
> >   * where +-feat overwrites any feature set by
> >   * feat=on|feat even if the later is parsed after +-feat
> > @@ -2030,6 +2051,59 @@ static void x86_cpu_parse_featurestr(const char 
> > *typename, char *features,
> >  }
> >  }
> >  
> > +static void x86_cpu_load_features(X86CPU *cpu, Error **errp);
> > +static int x86_cpu_filter_features(X86CPU *cpu);
> > +
> > +/* Check for missing features that may prevent the CPU class from
> > + * running using the current machine and accelerator.
> > + */
> > +static void x86_cpu_class_check_missing_features(X86CPUClass *xcc,
> > + strList **missing_feats)
> > +{
> > +X86CPU *xc;
> > +FeatureWord w;
> > +Error *err = NULL;
> > +strList **next = missing_feats;
> > +
> > +if (xcc->kvm_required && !kvm_enabled()) {
> > +strList *new = g_new0(strList, 1);
> > +new->value = g_strdup("kvm");;
> > +*missing_feats = new;
> > +return;
> > +}
> > +
> > +xc = X86_CPU(object_new(object_class_get_name(OBJECT_CLASS(xcc;
> > +
> > +x86_cpu_load_features(xc, );
> > +if (err) {
> > +/* Errors at x86_cpu_load_features should never happen,
> > + * but in case it does, just report the model as not
> > + * runnable at all using the "type" property.
> > + */
> > +strList *new = g_new0(strList, 1);
> > +new->value = g_strdup("type");
> > +*next = new;
> > +next = >next;
> > +}
> > +
> > +x86_cpu_filter_features(xc);
> > +
> > +for (w = 0; w < FEATURE_WORDS; w++) {
> > +uint32_t filtered = xc->filtered_features[w];
> > +int i;
> > +for (i = 0; i < 32; i++) {
> > +if (filtered & (1UL << i)) {
> > +strList *new = g_new0(strList, 1);
> > +new->value = g_strdup(x86_cpu_feature_name(w, i));
> > +*next = new;
> > +next = >next;
> > +}
> > +}
> > +}
> Shouldn't you add 
>if (IS_AMD_CPU(env)) { 
> fixup here, that realize does right after calling x86_cpu_filter_features()

What would it be useful for? The IS_AMD_CPU fixup runs after
x86_cpu_filter_features() (so it doesn't affect filtered_features
at all), and filtered_features is the only field used as input to
build missing_feats.

-- 
Eduardo



[Qemu-devel] [PATCH] fixup! target-i386: x86_cpu_load_features() function

2016-10-10 Thread Eduardo Habkost
On Mon, Oct 10, 2016 at 02:25:32PM +0200, Igor Mammedov wrote:
> On Fri,  7 Oct 2016 17:29:01 -0300
> Eduardo Habkost  wrote:
> 
> > When probing for CPU model information, we need to reuse the code
> > that initializes CPUID fields, but not the remaining side-effects
> > of x86_cpu_realizefn(). Move that code to a separate function
> > that can be reused later.
> > 
> > Signed-off-by: Eduardo Habkost 
> > ---
> > Changes v5 -> v6:
> > * Move x86_cpu_filter_features() outside x86_cpu_load_features(),
> >   as the CPU model querying API won't run
> >   x86_cpu_filter_features() on most cases
> 
> > 
> > Changes v4 -> v5:
> > * Fix typo on x86_cpu_load_features() comment
> >   Reported-by: Paolo Bonzini 
> > 
> > Changes series v3 -> v4:
> > * New patch added to series
> > ---
> >  target-i386/cpu.c | 67 
> > +++
> >  1 file changed, 43 insertions(+), 24 deletions(-)
> > 
> > diff --git a/target-i386/cpu.c b/target-i386/cpu.c
> > index 1e8127b..23cc19b 100644
> > --- a/target-i386/cpu.c
> > +++ b/target-i386/cpu.c
> > @@ -2993,34 +2993,13 @@ static void x86_cpu_enable_xsave_components(X86CPU 
> > *cpu)
> >  env->features[FEAT_XSAVE_COMP_HI] = mask >> 32;
> >  }
> >  
> > -#define IS_INTEL_CPU(env) ((env)->cpuid_vendor1 == CPUID_VENDOR_INTEL_1 && 
> > \
> > -   (env)->cpuid_vendor2 == CPUID_VENDOR_INTEL_2 && 
> > \
> > -   (env)->cpuid_vendor3 == CPUID_VENDOR_INTEL_3)
> > -#define IS_AMD_CPU(env) ((env)->cpuid_vendor1 == CPUID_VENDOR_AMD_1 && \
> > - (env)->cpuid_vendor2 == CPUID_VENDOR_AMD_2 && \
> > - (env)->cpuid_vendor3 == CPUID_VENDOR_AMD_3)
> > -static void x86_cpu_realizefn(DeviceState *dev, Error **errp)
> > +/* Load CPUID data based on configured features */
> > +static void x86_cpu_load_features(X86CPU *cpu, Error **errp)
> >  {
> > -CPUState *cs = CPU(dev);
> > -X86CPU *cpu = X86_CPU(dev);
> > -X86CPUClass *xcc = X86_CPU_GET_CLASS(dev);
> >  CPUX86State *env = >env;
> > -Error *local_err = NULL;
> > -static bool ht_warned;
> >  FeatureWord w;
> >  GList *l;
> > -
> > -if (xcc->kvm_required && !kvm_enabled()) {
> > -char *name = x86_cpu_class_get_model_name(xcc);
> > -error_setg(_err, "CPU model '%s' requires KVM", name);
> > -g_free(name);
> > -goto out;
> > -}
> > -
> > -if (cpu->apic_id == UNASSIGNED_APIC_ID) {
> > -error_setg(errp, "apic-id property was not initialized properly");
> > -return;
> > -}
> > +Error *local_err = NULL;
> >  
> >  /*TODO: cpu->host_features incorrectly overwrites features
> >   * set using "feat=on|off". Once we fix this, we can convert
> > @@ -3086,6 +3065,46 @@ static void x86_cpu_realizefn(DeviceState *dev, 
> > Error **errp)
> >  env->cpuid_xlevel2 = env->cpuid_min_xlevel2;
> >  }
> >  
> > +out:
> > +if (local_err != NULL) {
> > +error_propagate(errp, local_err);
> > +}
> > +}
> > +
> > +#define IS_INTEL_CPU(env) ((env)->cpuid_vendor1 == CPUID_VENDOR_INTEL_1 && 
> > \
> > +   (env)->cpuid_vendor2 == CPUID_VENDOR_INTEL_2 && 
> > \
> > +   (env)->cpuid_vendor3 == CPUID_VENDOR_INTEL_3)
> > +#define IS_AMD_CPU(env) ((env)->cpuid_vendor1 == CPUID_VENDOR_AMD_1 && \
> > + (env)->cpuid_vendor2 == CPUID_VENDOR_AMD_2 && \
> > + (env)->cpuid_vendor3 == CPUID_VENDOR_AMD_3)
> > +static void x86_cpu_realizefn(DeviceState *dev, Error **errp)
> > +{
> > +CPUState *cs = CPU(dev);
> > +X86CPU *cpu = X86_CPU(dev);
> > +X86CPUClass *xcc = X86_CPU_GET_CLASS(dev);
> > +CPUX86State *env = >env;
> > +Error *local_err = NULL;
> > +static bool ht_warned;
> > +
> > +if (xcc->kvm_required && !kvm_enabled()) {
> > +char *name = x86_cpu_class_get_model_name(xcc);
> > +error_setg(_err, "CPU model '%s' requires KVM", name);
> > +g_free(name);
> > +goto out;
> > +}
> > +
> > +if (cpu->apic_id == UNASSIGNED_APIC_ID) {
> > +error_setg(errp, "apic-id property was not initialized properly");
> > +return;
> > +}
> > +
> > +x86_cpu_load_features(cpu, _err);
> > +if (local_err) {
> > +goto out;
> > +}
> > +
> > +x86_cpu_filter_features(cpu);
> that makes 2 invocations of ^^ inside realize,
> see followup line 

Oops, leftover from v5. Thanks for spotting that! Fixup below.

Signed-off-by: Eduardo Habkost 
---
 target-i386/cpu.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index 23cc19b..4294746 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -3103,8 +3103,6 @@ static void x86_cpu_realizefn(DeviceState *dev, Error 
**errp)
 goto out;
 }
 
-

Re: [Qemu-devel] [RFC QEMU PATCH 0/8] Implement vNVDIMM for Xen HVM guest

2016-10-10 Thread no-reply
Hi,

Your series failed automatic build test. Please find the testing commands and
their output below. If you have docker installed, you can probably reproduce it
locally.

Message-id: 20161010003423.4333-1-haozhong.zh...@intel.com
Subject: [Qemu-devel] [RFC QEMU PATCH 0/8] Implement vNVDIMM for Xen HVM guest
Type: series

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

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
Switched to a new branch 'test'
455b9b0 qmp: add a qmp command 'query-nvdimms' to get plugged NVDIMM devices
bea844c xen-hvm: create hotplug memory region for HVM guest
ba5ca9a hostmem: add a host memory backend for Xen
02dc103 nvdimm acpi: build and copy NVDIMM namespace devices to guest on Xen
a3e39ca nvdimm acpi: build and copy NFIT to guest on Xen
1671ab8 nvdimm acpi: do not use fw_cfg on Xen
f7fa2ca xen-hvm: add a function to copy ACPI to guest
d6a1929 nvdimm: do not initialize label_data if label_size is zero

=== OUTPUT BEGIN ===
Submodule 'dtc' (git://git.qemu-project.org/dtc.git) registered for path 'dtc'
Cloning into 'dtc'...
Submodule path 'dtc': checked out '65cc4d2748a2c2e6f27f1cf39e07a5dbabd80ebf'
  BUILD   centos6
  ARCHIVE qemu.tgz
  ARCHIVE dtc.tgz
  COPYRUNNER
  RUN test-quick in centos6
Packages installed:
SDL-devel-1.2.14-7.el6_7.1.x86_64
ccache-3.1.6-2.el6.x86_64
epel-release-6-8.noarch
gcc-4.4.7-17.el6.x86_64
git-1.7.1-4.el6_7.1.x86_64
glib2-devel-2.28.8-5.el6.x86_64
libfdt-devel-1.4.0-1.el6.x86_64
make-3.81-23.el6.x86_64
package g++ is not installed
pixman-devel-0.32.8-1.el6.x86_64
tar-1.23-15.el6_8.x86_64
zlib-devel-1.2.3-29.el6.x86_64

Environment variables:
PACKAGES=libfdt-devel ccache tar git make gcc g++ zlib-devel 
glib2-devel SDL-devel pixman-devel epel-release
HOSTNAME=edbfa25803c4
TERM=xterm
MAKEFLAGS= -j8
HISTSIZE=1000
J=8
USER=root
CCACHE_DIR=/var/tmp/ccache
EXTRA_CONFIGURE_OPTS=
V=
SHOW_ENV=1
MAIL=/var/spool/mail/root
PATH=/usr/lib/ccache:/usr/lib64/ccache:/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin
PWD=/
LANG=en_US.UTF-8
TARGET_LIST=
HISTCONTROL=ignoredups
SHLVL=1
HOME=/root
TEST_DIR=/tmp/qemu-test
LOGNAME=root
LESSOPEN=||/usr/bin/lesspipe.sh %s
FEATURES= dtc
DEBUG=
G_BROKEN_FILENAMES=1
CCACHE_HASHDIR=
_=/usr/bin/env

Configure options:
--enable-werror --target-list=x86_64-softmmu,aarch64-softmmu 
--prefix=/var/tmp/qemu-build/install
No C++ compiler available; disabling C++ specific optional code
Install prefix/var/tmp/qemu-build/install
BIOS directory/var/tmp/qemu-build/install/share/qemu
binary directory  /var/tmp/qemu-build/install/bin
library directory /var/tmp/qemu-build/install/lib
module directory  /var/tmp/qemu-build/install/lib/qemu
libexec directory /var/tmp/qemu-build/install/libexec
include directory /var/tmp/qemu-build/install/include
config directory  /var/tmp/qemu-build/install/etc
local state directory   /var/tmp/qemu-build/install/var
Manual directory  /var/tmp/qemu-build/install/share/man
ELF interp prefix /usr/gnemul/qemu-%M
Source path   /tmp/qemu-test/src
C compilercc
Host C compiler   cc
C++ compiler  
Objective-C compiler cc
ARFLAGS   rv
CFLAGS-O2 -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=2 -g 
QEMU_CFLAGS   -I/usr/include/pixman-1-pthread -I/usr/include/glib-2.0 
-I/usr/lib64/glib-2.0/include   -fPIE -DPIE -m64 -D_GNU_SOURCE 
-D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE -Wstrict-prototypes 
-Wredundant-decls -Wall -Wundef -Wwrite-strings -Wmissing-prototypes 
-fno-strict-aliasing -fno-common -fwrapv  -Wendif-labels -Wmissing-include-dirs 
-Wempty-body -Wnested-externs -Wformat-security -Wformat-y2k -Winit-self 
-Wignored-qualifiers -Wold-style-declaration -Wold-style-definition 
-Wtype-limits -fstack-protector-all
LDFLAGS   -Wl,--warn-common -Wl,-z,relro -Wl,-z,now -pie -m64 -g 
make  make
install   install
pythonpython -B
smbd  /usr/sbin/smbd
module supportno
host CPU  x86_64
host big endian   no
target list   x86_64-softmmu aarch64-softmmu
tcg debug enabled no
gprof enabled no
sparse enabledno
strip binariesyes
profiler  no
static build  no
pixmansystem
SDL support   yes (1.2.14)
GTK support   no 
GTK GL supportno
VTE support   no 
TLS priority  NORMAL
GNUTLS supportno
GNUTLS rndno
libgcrypt no
libgcrypt kdf no
nettleno 
nettle kdfno
libtasn1  no
curses supportno
virgl support no
curl support  no
mingw32 support   no
Audio drivers oss
Block whitelist (rw) 
Block whitelist (ro) 
VirtFS supportno
VNC support   yes
VNC SASL support  no
VNC JPEG support  no
VNC PNG support   no
xen support   no
brlapi supportno
bluez  supportno
Documentation no
PIE   

Re: [Qemu-devel] [PATCH 0/5] More thread sanitizer fixes and atomic.h improvements

2016-10-10 Thread no-reply
Hi,

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

Message-id: 1476107947-31430-1-git-send-email-pbonz...@redhat.com
Subject: [Qemu-devel] [PATCH 0/5] More thread sanitizer fixes and atomic.h 
improvements
Type: series

=== 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 show --no-patch --format=%s $c)..."
if ! git show $c --format=email | ./scripts/checkpatch.pl --mailback -; then
failed=1
echo
fi
n=$((n+1))
done

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

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
From https://github.com/patchew-project/qemu
 - [tag update]  patchew/1475847779-7336-1-git-send-email-kw...@redhat.com 
-> patchew/1475847779-7336-1-git-send-email-kw...@redhat.com
 - [tag update]  
patchew/1476099707-526-1-git-send-email-berra...@redhat.com -> 
patchew/1476099707-526-1-git-send-email-berra...@redhat.com
 * [new tag] 
patchew/1476107947-31430-1-git-send-email-pbonz...@redhat.com -> 
patchew/1476107947-31430-1-git-send-email-pbonz...@redhat.com
 * [new tag] 
patchew/1476113163-24578-1-git-send-email-peter.mayd...@linaro.org -> 
patchew/1476113163-24578-1-git-send-email-peter.mayd...@linaro.org
 * [new tag] patchew/20161010152848.17902-1-rkrc...@redhat.com -> 
patchew/20161010152848.17902-1-rkrc...@redhat.com
 - [tag update]  patchew/cover.1475757437.git.be...@igalia.com -> 
patchew/cover.1475757437.git.be...@igalia.com
Switched to a new branch 'test'
8e1cfa9 atomic: base mb_read/mb_set on load-acquire and store-release
5cfb494 rcu: simplify memory barriers
5886ce0 qemu-thread: use acquire/release to clarify semantics of QemuEvent
7a5cb66 cpus: use atomic_read to read seqlock-protected variables
e663e45 atomic: introduce smp_mb_acquire and smp_mb_release

=== OUTPUT BEGIN ===
Checking PATCH 1/5: atomic: introduce smp_mb_acquire and smp_mb_release...
ERROR: line over 90 characters
#195: FILE: include/qemu/atomic.h:75:
+#define smp_mb() ({ barrier(); 
__atomic_thread_fence(__ATOMIC_SEQ_CST); })

ERROR: memory barrier without comment
#195: FILE: include/qemu/atomic.h:75:
+#define smp_mb() ({ barrier(); 
__atomic_thread_fence(__ATOMIC_SEQ_CST); })

ERROR: line over 90 characters
#196: FILE: include/qemu/atomic.h:76:
+#define smp_mb_release() ({ barrier(); 
__atomic_thread_fence(__ATOMIC_RELEASE); })

ERROR: line over 90 characters
#197: FILE: include/qemu/atomic.h:77:
+#define smp_mb_acquire() ({ barrier(); 
__atomic_thread_fence(__ATOMIC_ACQUIRE); })

ERROR: line over 90 characters
#205: FILE: include/qemu/atomic.h:84:
+#define smp_read_barrier_depends()   ({ barrier(); 
__atomic_thread_fence(__ATOMIC_CONSUME); })

ERROR: memory barrier without comment
#205: FILE: include/qemu/atomic.h:84:
+#define smp_read_barrier_depends()   ({ barrier(); 
__atomic_thread_fence(__ATOMIC_CONSUME); })

ERROR: spaces required around that ':' (ctx:OxW)
#241: FILE: include/qemu/atomic.h:266:
+#define smp_wmb()  ({ asm volatile("eieio" ::: "memory"); (void)0; })
  ^

ERROR: memory barrier without comment
#241: FILE: include/qemu/atomic.h:266:
+#define smp_wmb()  ({ asm volatile("eieio" ::: "memory"); (void)0; })

ERROR: spaces required around that ':' (ctx:OxW)
#244: FILE: include/qemu/atomic.h:268:
+#define smp_mb_release()   ({ asm volatile("lwsync" ::: "memory"); (void)0; })
   ^

ERROR: spaces required around that ':' (ctx:OxW)
#245: FILE: include/qemu/atomic.h:269:
+#define smp_mb_acquire()   ({ asm volatile("lwsync" ::: "memory"); (void)0; })
   ^

ERROR: spaces required around that ':' (ctx:OxW)
#248: FILE: include/qemu/atomic.h:271:
+#define smp_mb_release()   ({ asm volatile("sync" ::: "memory"); (void)0; })
 ^

ERROR: spaces required around that ':' (ctx:OxW)
#249: FILE: include/qemu/atomic.h:272:
+#define smp_mb_acquire()   ({ asm volatile("sync" ::: "memory"); (void)0; })
 ^

ERROR: spaces required around that ':' (ctx:OxW)
#252: FILE: include/qemu/atomic.h:274:
+#define smp_mb()   ({ asm volatile("sync" ::: "memory"); (void)0; })
 ^

ERROR: memory barrier without comment
#252: FILE: include/qemu/atomic.h:274:
+#define smp_mb()   ({ asm volatile("sync" ::: "memory"); (void)0; })

ERROR: memory barrier without comment
#265: FILE: include/qemu/atomic.h:285:
+#define smp_mb()   __sync_synchronize()

ERROR: memory barrier without 

Re: [Qemu-devel] [PATCH 2/2] macio: switch over to new byte-aligned DMA helpers

2016-10-10 Thread Eric Blake
On 10/09/2016 11:43 AM, Mark Cave-Ayland wrote:
> Now that the DMA helpers are byte-aligned they can be called directly from
> the macio routines rather than emulating byte-aligned accesses via multiple
> block-level accesses.
> 
> Signed-off-by: Mark Cave-Ayland 
> ---
>  hw/ide/macio.c |  213 
> 
>  1 file changed, 28 insertions(+), 185 deletions(-)
> 

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


  1   2   3   4   >