Re: [Qemu-devel] [PATCH v2] Update the avx2 configure test to be compatible with clang

[Richard Henderson]

On 8/8/19 9:29 PM, Rebecca Cran wrote:
> clang doesn't support the GCC pragma to enable AVX2, but instead
> requires the command line option -mavx2. Since GCC also supports that,
> remove the pragma lines and add the -mavx2 option when building the
> test. If AVX2 is supported, update QEMU_CFLAGS to include -mavx2 .
> 
> Signed-off-by: Rebecca Cran 
> ---
>  configure | 5 ++---
>  1 file changed, 2 insertions(+), 3 deletions(-)

No, this has the same problem as v1.


r~

[Qemu-devel] [PATCH v2] target-arm: Make the counter tick relative to cntfrq

[Andrew Jeffery]

The use of GTIMER_SCALE assumes the clock feeding the generic timer is
62.5MHz for all platforms. This is untrue in general, for example the
ASPEED AST2600 feeds the counter with either an 800 or 1200MHz clock,
and CNTFRQ is configured appropriately by u-boot.

To cope with these values we need to take care of the quantization
caused by the clock scaling in terms of nanoseconds per clock by
calculating an effective frequency such that NANOSECONDS_PER_SECOND is
an integer multiple of the effective frequency. Failing to account for
the quantisation leads to sticky behaviour in the VM as the guest timer
subsystems account for the difference between delay time and the counter
value.

Signed-off-by: Andrew Jeffery 
---
v2:
1. Removed the user-mode change that broke v1
2. Rearranged the implementation as a consequence of 1.

 target/arm/helper.c | 51 ++---
 1 file changed, 48 insertions(+), 3 deletions(-)

diff --git a/target/arm/helper.c b/target/arm/helper.c
index b74c23a9bc08..166a54daf278 100644
--- a/target/arm/helper.c
+++ b/target/arm/helper.c
@@ -2277,6 +2277,34 @@ static const ARMCPRegInfo v6k_cp_reginfo[] = {
 
 #ifndef CONFIG_USER_ONLY
 
+static void gt_recalc_timer(ARMCPU *cpu, int timeridx);
+static void gt_cntfrq_write(CPUARMState *env, const ARMCPRegInfo *ri,
+uint64_t value)
+{
+uint64_t scale;
+ARMCPU *cpu;
+int i;
+
+raw_write(env, ri, value);
+
+/* Fix up the timer scaling */
+cpu = env_archcpu(env);
+scale = MAX(1, NANOSECONDS_PER_SECOND / value);
+for (i = 0; i < NUM_GTIMERS; i++) {
+if (!cpu->gt_timer[i]) {
+continue;
+}
+
+cpu->gt_timer[i]->scale = scale;
+gt_recalc_timer(cpu, i);
+}
+}
+
+static void gt_cntfrq_reset(CPUARMState *env, const ARMCPRegInfo *opaque)
+{
+gt_cntfrq_write(env, opaque, opaque->resetvalue);
+}
+
 static CPAccessResult gt_cntfrq_access(CPUARMState *env, const ARMCPRegInfo 
*ri,
bool isread)
 {
@@ -2407,7 +2435,21 @@ static CPAccessResult gt_stimer_access(CPUARMState *env,
 
 static uint64_t gt_get_countervalue(CPUARMState *env)
 {
-return qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL) / GTIMER_SCALE;
+uint64_t effective;
+
+/*
+ * Deal with quantized clock scaling by calculating the effective frequency
+ * in terms of the timer scale.
+ */
+if (env->cp15.c14_cntfrq <= NANOSECONDS_PER_SECOND) {
+uint32_t scale = NANOSECONDS_PER_SECOND / env->cp15.c14_cntfrq;
+effective = NANOSECONDS_PER_SECOND / scale;
+} else {
+effective = NANOSECONDS_PER_SECOND;
+}
+
+return muldiv64(qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL), effective,
+NANOSECONDS_PER_SECOND);
 }
 
 static void gt_recalc_timer(ARMCPU *cpu, int timeridx)
@@ -2443,8 +2485,8 @@ static void gt_recalc_timer(ARMCPU *cpu, int timeridx)
  * set the timer for as far in the future as possible. When the
  * timer expires we will reset the timer for any remaining period.
  */
-if (nexttick > INT64_MAX / GTIMER_SCALE) {
-nexttick = INT64_MAX / GTIMER_SCALE;
+if (nexttick > INT64_MAX / cpu->gt_timer[timeridx]->scale) {
+nexttick = INT64_MAX / cpu->gt_timer[timeridx]->scale;
 }
 timer_mod(cpu->gt_timer[timeridx], nexttick);
 trace_arm_gt_recalc(timeridx, irqstate, nexttick);
@@ -2686,13 +2728,16 @@ static const ARMCPRegInfo generic_timer_cp_reginfo[] = {
 { .name = "CNTFRQ", .cp = 15, .crn = 14, .crm = 0, .opc1 = 0, .opc2 = 0,
   .type = ARM_CP_ALIAS,
   .access = PL1_RW | PL0_R, .accessfn = gt_cntfrq_access,
+  .writefn = gt_cntfrq_write,
   .fieldoffset = offsetoflow32(CPUARMState, cp15.c14_cntfrq),
 },
 { .name = "CNTFRQ_EL0", .state = ARM_CP_STATE_AA64,
   .opc0 = 3, .opc1 = 3, .crn = 14, .crm = 0, .opc2 = 0,
   .access = PL1_RW | PL0_R, .accessfn = gt_cntfrq_access,
+  .writefn = gt_cntfrq_write,
   .fieldoffset = offsetof(CPUARMState, cp15.c14_cntfrq),
   .resetvalue = (1000 * 1000 * 1000) / GTIMER_SCALE,
+  .resetfn = gt_cntfrq_reset,
 },
 /* overall control: mostly access permissions */
 { .name = "CNTKCTL", .state = ARM_CP_STATE_BOTH,
-- 
2.20.1

Re: [Qemu-devel] [PATCH] Update the avx2 configure test to be compatible with clang

[Richard Henderson]

On 8/8/19 9:19 PM, Rebecca Cran wrote:
> clang doesn't support the GCC pragma to enable AVX2, but instead
> requires the command line option -mavx2. Since GCC also supports that,
> remove the pragma lines and add the -mavx2 option when building the
> test.

No, this means we're not testing what we need:

We need to compile exactly one function using avx2.

The other functions should be compiled with sse4 and sse2, respectively, and we
choose between them by testing cpuid bits at startup.  If you supply -mavx2 to
the entire compilation, then the routine that is supposed to use only sse2 will
in fact use avx2, and then the runtime selection is moot.


r~

Re: [Qemu-devel] [PATCH v2] Update the avx2 configure test to be compatible with clang

[no-reply]

Patchew URL: https://patchew.org/QEMU/20190809042909.74988-1-rebe...@bsdio.com/



Hi,

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

=== TEST SCRIPT BEGIN ===
#!/bin/bash
make docker-image-fedora V=1 NETWORK=1
time make docker-test-debug@fedora TARGET_LIST=x86_64-softmmu J=14 NETWORK=1
=== TEST SCRIPT END ===

  CC  util/oslib-posix.o
  CC  util/qemu-openpty.o
  CC  util/qemu-thread-posix.o
/tmp/qemu-test/src/util/bufferiszero.c:71:13: error: unknown pragma ignored 
[-Werror,-Wunknown-pragmas]
#pragma GCC push_options
^
/tmp/qemu-test/src/util/bufferiszero.c:72:13: error: unknown pragma ignored 
[-Werror,-Wunknown-pragmas]
#pragma GCC target("sse2")
^
/tmp/qemu-test/src/util/bufferiszero.c:108:13: error: unknown pragma ignored 
[-Werror,-Wunknown-pragmas]
#pragma GCC pop_options
^
/tmp/qemu-test/src/util/bufferiszero.c:116:13: error: unknown pragma ignored 
[-Werror,-Wunknown-pragmas]
#pragma GCC push_options
^
/tmp/qemu-test/src/util/bufferiszero.c:117:13: error: unknown pragma ignored 
[-Werror,-Wunknown-pragmas]
#pragma GCC target("sse4")
^
/tmp/qemu-test/src/util/bufferiszero.c:148:13: error: unknown pragma ignored 
[-Werror,-Wunknown-pragmas]
#pragma GCC pop_options
^
/tmp/qemu-test/src/util/bufferiszero.c:149:13: error: unknown pragma ignored 
[-Werror,-Wunknown-pragmas]
#pragma GCC push_options
^
/tmp/qemu-test/src/util/bufferiszero.c:150:13: error: unknown pragma ignored 
[-Werror,-Wunknown-pragmas]
#pragma GCC target("avx2")
^
/tmp/qemu-test/src/util/bufferiszero.c:187:13: error: unknown pragma ignored 
[-Werror,-Wunknown-pragmas]
#pragma GCC pop_options
^
9 errors generated.


The full log is available at
http://patchew.org/logs/20190809042909.74988-1-rebe...@bsdio.com/testing.asan/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-de...@redhat.com

Re: [Qemu-devel] [PATCH v3] spapr: quantify error messages regarding capability settings

[David Gibson]

On Mon, Aug 05, 2019 at 03:09:58PM +1000, Daniel Black wrote:
> Its not immediately obvious how cap-X=Y setting need to be applied
> to the command line so, for spapr capability hints, this has been clarified 
> to:
> 
>   ..try appending -machine cap-X=Y
> 
> The wrong value messages have been left as is, as the user has found the right
> location.
> 
> Warning messages have been left as is for now.
> 
> Reviewed-by: Greg Kurz 
> Signed-off-by: Daniel Black 

So, I'm sorry to nitpick, but..
[snip]
> diff --git a/hw/ppc/spapr_caps.c b/hw/ppc/spapr_caps.c
> index bbb001f84a..b06faee7f6 100644
> --- a/hw/ppc/spapr_caps.c
> +++ b/hw/ppc/spapr_caps.c
> @@ -37,6 +37,8 @@
>  
>  #include "hw/ppc/spapr.h"
>  
> +#define CAPABILITY_HINT(X) " try appending -machine " X

.. I'd prefer to see the new messages just written inline rather than
this macro.  I think the increased duplication is less important than
the reduced greppability that the macro introduces.

> +
>  typedef struct SpaprCapPossible {
>  int num;/* size of vals array below */
>  const char *help;   /* help text for vals */
> @@ -194,10 +196,12 @@ static void cap_htm_apply(SpaprMachineState *spapr, 
> uint8_t val, Error **errp)
>  }
>  if (tcg_enabled()) {
>  error_setg(errp,
> -   "No Transactional Memory support in TCG, try 
> cap-htm=off");
> +   "No Transactional Memory support in TCG,"
> +   CAPABILITY_HINT("cap-htm=off"));
>  } else if (kvm_enabled() && !kvmppc_has_cap_htm()) {
>  error_setg(errp,
> -"KVM implementation does not support Transactional Memory, try cap-htm=off"
> +"KVM implementation does not support Transactional Memory,"
> +   CAPABILITY_HINT("cap-htm=off")
>  );
>  }
>  }
> @@ -215,7 +219,8 @@ static void cap_vsx_apply(SpaprMachineState *spapr, 
> uint8_t val, Error **errp)
>   * rid of anything that doesn't do VMX */
>  g_assert(env->insns_flags & PPC_ALTIVEC);
>  if (!(env->insns_flags2 & PPC2_VSX)) {
> -error_setg(errp, "VSX support not available, try cap-vsx=off");
> +error_setg(errp, "VSX support not available,"
> +   CAPABILITY_HINT("cap-vsx=off"));
>  }
>  }
>  
> @@ -229,7 +234,8 @@ static void cap_dfp_apply(SpaprMachineState *spapr, 
> uint8_t val, Error **errp)
>  return;
>  }
>  if (!(env->insns_flags2 & PPC2_DFP)) {
> -error_setg(errp, "DFP support not available, try cap-dfp=off");
> +error_setg(errp, "DFP support not available,"
> +   CAPABILITY_HINT("cap-dfp=off"));
>  }
>  }
>  
> @@ -253,7 +259,8 @@ static void cap_safe_cache_apply(SpaprMachineState 
> *spapr, uint8_t val,
> cap_cfpc_possible.vals[val]);
>  } else if (kvm_enabled() && (val > kvm_val)) {
>  error_setg(errp,
> -"Requested safe cache capability level not supported by kvm, try 
> cap-cfpc=%s",
> +   "Requested safe cache capability level not supported by 
> kvm,"
> +   CAPABILITY_HINT("cap-cfpc=%s"),
> cap_cfpc_possible.vals[kvm_val]);
>  }
>  
> @@ -281,7 +288,8 @@ static void cap_safe_bounds_check_apply(SpaprMachineState 
> *spapr, uint8_t val,
> cap_sbbc_possible.vals[val]);
>  } else if (kvm_enabled() && (val > kvm_val)) {
>  error_setg(errp,
> -"Requested safe bounds check capability level not supported by kvm, try 
> cap-sbbc=%s",
> +"Requested safe bounds check capability level not supported by kvm,"
> +   CAPABILITY_HINT("cap-sbbc=%s"),
> cap_sbbc_possible.vals[kvm_val]);
>  }
>  
> @@ -312,7 +320,8 @@ static void 
> cap_safe_indirect_branch_apply(SpaprMachineState *spapr,
> cap_ibs_possible.vals[val]);
>  } else if (kvm_enabled() && (val > kvm_val)) {
>  error_setg(errp,
> -"Requested safe indirect branch capability level not supported by kvm, try 
> cap-ibs=%s",
> +"Requested safe indirect branch capability level not supported by kvm,"
> +   CAPABILITY_HINT("cap-ibs=%s"),
> cap_ibs_possible.vals[kvm_val]);
>  }
>  
> @@ -401,11 +410,13 @@ static void cap_nested_kvm_hv_apply(SpaprMachineState 
> *spapr,
>  
>  if (tcg_enabled()) {
>  error_setg(errp,
> -   "No Nested KVM-HV support in tcg, try cap-nested-hv=off");
> +   "No Nested KVM-HV support in tcg,"
> +   CAPABILITY_HINT("cap-nested-hv=off"));
>  } else if (kvm_enabled()) {
>  if (!kvmppc_has_cap_nested_kvm_hv()) {
>  error_setg(errp,
> -"KVM implementation does not support Nested KVM-HV, try cap-nested-hv=off");
> +"KVM implementation does not support Nested KVM-HV,"
> +   CAPABILITY_HINT("cap-nested-hv=off"));
>  } else if (kvmppc_set_cap_nested_kvm_hv(val) < 0) {
>  error_setg(errp,
>  "Error enabling 

Re: [Qemu-devel] [PATCH] target-arm: Make the counter tick relative to cntfrq

[Andrew Jeffery]

On Fri, 9 Aug 2019, at 13:36, no-re...@patchew.org wrote:
> Patchew URL: https://patchew.org/QEMU/20190809031321.14760-1-and...@aj.id.au/
> 
> 
> 
> Hi,
> 
> This series failed build test on s390x host. Please find the details below.
> 
> === TEST SCRIPT BEGIN ===
> #!/bin/bash
> # Testing script will be invoked under the git checkout with
> # HEAD pointing to a commit that has the patches applied on top of "base"
> # branch
> set -e
> 
> echo
> echo "=== ENV ==="
> env
> 
> echo
> echo "=== PACKAGES ==="
> rpm -qa
> 
> echo
> echo "=== UNAME ==="
> uname -a
> 
> CC=$HOME/bin/cc
> INSTALL=$PWD/install
> BUILD=$PWD/build
> mkdir -p $BUILD $INSTALL
> SRC=$PWD
> cd $BUILD
> $SRC/configure --cc=$CC --prefix=$INSTALL
> make -j4
> # XXX: we need reliable clean up
> # make check -j4 V=1
> make install
> === TEST SCRIPT END ===
> 
>   CC  aarch64_be-linux-user/target/arm/arm-semi.o
>   CC  aarch64_be-linux-user/target/arm/helper.o
> /var/tmp/patchew-tester-tmp-hkd7ua1n/src/target/arm/helper.c: In 
> function ‘gt_virt_cnt_read’:
> /var/tmp/patchew-tester-tmp-hkd7ua1n/src/target/arm/helper.c:2921:12: 
> error: implicit declaration of function ‘gt_calc_tick’ 
> [-Werror=implicit-function-declaration]
>  2921 | return gt_calc_tick(env, cpu_get_clock());
>   |^~~~
> /var/tmp/patchew-tester-tmp-hkd7ua1n/src/target/arm/helper.c:2921:12: 
> error: nested extern declaration of ‘gt_calc_tick’ 
> [-Werror=nested-externs]
> cc1: all warnings being treated as errors
> make[1]: *** [/var/tmp/patchew-tester-tmp-hkd7ua1n/src/rules.mak:69: 
> target/arm/helper.o] Error 1
> make: *** [Makefile:472: aarch64_be-linux-user/all] Error 2
> 

Ah, I missed that I put the implementation inside the
#ifndef CONFIG_USER_ONLY` block. Maybe we can just not do the scaling
for userspace anyway?

Andrew

Re: [Qemu-devel] [PATCH] Update the avx2 configure test to be compatible with clang

[no-reply]

Patchew URL: https://patchew.org/QEMU/20190809041952.57302-1-rebe...@bsdio.com/



Hi,

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

=== TEST SCRIPT BEGIN ===
#!/bin/bash
make docker-image-fedora V=1 NETWORK=1
time make docker-test-debug@fedora TARGET_LIST=x86_64-softmmu J=14 NETWORK=1
=== TEST SCRIPT END ===

  CC  util/qemu-sockets.o
  CC  util/uri.o
  CC  util/notify.o
/tmp/qemu-test/src/util/bufferiszero.c:71:13: error: unknown pragma ignored 
[-Werror,-Wunknown-pragmas]
#pragma GCC push_options
^
/tmp/qemu-test/src/util/bufferiszero.c:72:13: error: unknown pragma ignored 
[-Werror,-Wunknown-pragmas]
#pragma GCC target("sse2")
^
/tmp/qemu-test/src/util/bufferiszero.c:108:13: error: unknown pragma ignored 
[-Werror,-Wunknown-pragmas]
#pragma GCC pop_options
^
/tmp/qemu-test/src/util/bufferiszero.c:116:13: error: unknown pragma ignored 
[-Werror,-Wunknown-pragmas]
#pragma GCC push_options
^
/tmp/qemu-test/src/util/bufferiszero.c:117:13: error: unknown pragma ignored 
[-Werror,-Wunknown-pragmas]
#pragma GCC target("sse4")
^
/tmp/qemu-test/src/util/bufferiszero.c:148:13: error: unknown pragma ignored 
[-Werror,-Wunknown-pragmas]
#pragma GCC pop_options
^
/tmp/qemu-test/src/util/bufferiszero.c:149:13: error: unknown pragma ignored 
[-Werror,-Wunknown-pragmas]
#pragma GCC push_options
^
/tmp/qemu-test/src/util/bufferiszero.c:150:13: error: unknown pragma ignored 
[-Werror,-Wunknown-pragmas]
#pragma GCC target("avx2")
^
/tmp/qemu-test/src/util/bufferiszero.c:187:13: error: unknown pragma ignored 
[-Werror,-Wunknown-pragmas]
#pragma GCC pop_options
^
9 errors generated.


The full log is available at
http://patchew.org/logs/20190809041952.57302-1-rebe...@bsdio.com/testing.asan/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-de...@redhat.com

Re: [Qemu-devel] [RFC PATCH 3/6] hw/ppc/pnv_homer: add homer/occ common area emulation for PowerNV

[David Gibson]

On Wed, Aug 07, 2019 at 09:54:55AM +0200, Cédric Le Goater wrote:
> On 07/08/2019 09:14, Balamuruhan S wrote:
> > Add mmio callback functions to enable homer/occ common area
> > to emulate pstate table, occ-sensors, slw, occ static and
> > dynamic values for Power8 and Power9 chips. It also works for
> > multiple chips as offset remains the same whereas the base
> > address are handled appropriately while initializing device
> > tree.
> > 
> > currently skiboot disables the homer/occ code path with
> > `QUIRK_NO_PBA`, this quirk have to be removed in skiboot
> > for it to use this infrastructure.
> 
> 
> I think this patch can come before the others as it is adding
> support without the python extra facilities.

Right.  In fact it seems to me having it as an entirely separate
series would be preferable.  I don't think we want to tie review of a
basic OCC extension to to the frankly not all that palatable idea of
adding arbitrary scripting into the MMIO path.

> 
> Some comments below, 
>  
> > Signed-off-by: Hariharan T.S 
> > Signed-off-by: Balamuruhan S 
> > ---
> >  hw/ppc/Makefile.objs   |   2 +-
> >  hw/ppc/pnv_homer.c | 185 
> > +
> >  include/hw/ppc/pnv.h   |  14 
> >  include/hw/ppc/pnv_homer.h |  41 ++
> >  4 files changed, 241 insertions(+), 1 deletion(-)
> >  create mode 100644 hw/ppc/pnv_homer.c
> >  create mode 100644 include/hw/ppc/pnv_homer.h
> > 
> > diff --git a/hw/ppc/Makefile.objs b/hw/ppc/Makefile.objs
> > index 9da93af905..7260b4a96c 100644
> > --- a/hw/ppc/Makefile.objs
> > +++ b/hw/ppc/Makefile.objs
> > @@ -7,7 +7,7 @@ obj-$(CONFIG_PSERIES) += spapr_pci.o spapr_rtc.o spapr_drc.o
> >  obj-$(CONFIG_PSERIES) += spapr_cpu_core.o spapr_ovec.o spapr_irq.o
> >  obj-$(CONFIG_SPAPR_RNG) +=  spapr_rng.o
> >  # IBM PowerNV
> > -obj-$(CONFIG_POWERNV) += pnv.o pnv_xscom.o pnv_core.o pnv_lpc.o pnv_psi.o 
> > pnv_occ.o pnv_bmc.o
> > +obj-$(CONFIG_POWERNV) += pnv.o pnv_xscom.o pnv_core.o pnv_lpc.o pnv_psi.o 
> > pnv_occ.o pnv_bmc.o pnv_homer.o
> 
> add an extra line.
> 
> >  ifeq ($(CONFIG_PCI)$(CONFIG_PSERIES)$(CONFIG_LINUX), yyy)
> >  obj-y += spapr_pci_vfio.o spapr_pci_nvlink2.o
> >  endif
> > diff --git a/hw/ppc/pnv_homer.c b/hw/ppc/pnv_homer.c
> > new file mode 100644
> > index 00..73a94856d0
> > --- /dev/null
> > +++ b/hw/ppc/pnv_homer.c
> > @@ -0,0 +1,185 @@
> > +/*
> > + * QEMU PowerPC PowerNV Homer and OCC common area region
> > + *
> > + * Copyright (c) 2019, IBM Corporation.
> > + *
> > + * This library is free software; you can redistribute it and/or
> > + * modify it under the terms of the GNU Lesser General Public
> > + * License as published by the Free Software Foundation; either
> > + * version 2 of the License, or (at your option) any later version.
> > + *
> > + * This library is distributed in the hope that it will be useful,
> > + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> > + * Lesser General Public License for more details.
> > + *
> > + * You should have received a copy of the GNU Lesser General Public
> > + * License along with this library; if not, see 
> > .
> > + */
> > +#include "qemu/osdep.h"
> > +#include "sysemu/hw_accel.h"
> > +#include "sysemu/cpus.h"
> > +#include "hw/ppc/pnv.h"
> > +
> > +static bool core_max_array(hwaddr addr)
> > +{
> > +char *cpu_type;
> > +hwaddr core_max_base = 0xe2819;
> 
> What is this representing ? 
> 
> > +MachineState *ms = MACHINE(qdev_get_machine());
> > +cpu_type = strstr(ms->cpu_type, "power8");
> 
> you need to get this information some other way. The PnvChip should have it.
> 
> > +if (cpu_type)
> > +core_max_base = 0x1f8810;
> 
> It could be a PnvChipClass value.
> 
> > +for (int i = 0; i <= ms->smp.cores; i++)
> > +   if (addr == (core_max_base + i))
> > +   return true;
> > +return false;
> > +}
> 
> 
> > +static uint64_t homer_read(void *opaque, hwaddr addr, unsigned width)
> > +{
> > +switch (addr) {
> 
> We should be using defines for the case statements below. 
> 
> Are we accessing one or more structures which are mapped at specific 
> addresses ? If so I would define them in this file and change the 
> memory ops to use well known offsets.
> 
> Are these structures the same on P9 and P8 ? 
> 
> Are there default values ? May be we could use a reset handler
> in this case.
> 
> > +case 0xe2006:  /* max pstate ultra turbo */
> > +case 0xe2018:  /* pstate id for 0 */
> > +case 0x1f8001: /* P8 occ pstate version */
> > +case 0x1f8003: /* P8 pstate min */
> > +case 0x1f8010: /* P8 pstate id for 0 */
> > +return 0;
> > +case 0xe2000:  /* occ data area */
> > +case 0xe2002:  /* occ_role master/slave*/
> > +case 0xe2004:  /* pstate nom */
> > +case 0xe2005:  /* pstate turbo */
> > +

Re: [Qemu-devel] [RFC PATCH 4/6] hw/ppc/pnv: initialize and realize homer/occ common area

[David Gibson]

On Wed, Aug 07, 2019 at 12:44:43PM +0530, Balamuruhan S wrote:
> homer and occ common area region base address are initialized
> to create device tree and realized to map the address with
> mmio callbacks during `pnv_chip_realize()`.
> 
> `SysBusNum` enum is introduced to set sysbus for XSCOM, ICP,
> HOMER and OCC appropriately and chip_num to initialize and
> retrieve base address + size contiguously on a PowerNV with
> multichip boot.

Same comments here as on the previous patch - I don't think this
belongs in the series with the scripting extensions.

> 
> Signed-off-by: Balamuruhan S 
> ---
>  hw/ppc/pnv.c | 49 +
>  include/hw/ppc/pnv.h |  1 +
>  2 files changed, 46 insertions(+), 4 deletions(-)
> 
> diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c
> index bd4531c822..f6e56e915d 100644
> --- a/hw/ppc/pnv.c
> +++ b/hw/ppc/pnv.c
> @@ -675,6 +675,7 @@ static void pnv_init(MachineState *machine)
>  Object *chip = object_new(chip_typename);
>  
>  pnv->chips[i] = PNV_CHIP(chip);
> +PNV_CHIP(chip)->chip_num = i;
>  
>  /* TODO: put all the memory in one node on chip 0 until we find a
>   * way to specify different ranges for each chip
> @@ -824,18 +825,20 @@ static void pnv_chip_icp_realize(Pnv8Chip *chip8, Error 
> **errp)
>   {
>  PnvChip *chip = PNV_CHIP(chip8);
>  PnvChipClass *pcc = PNV_CHIP_GET_CLASS(chip);
> +SysBusDevice *sbd = SYS_BUS_DEVICE(chip);
>  const char *typename = pnv_chip_core_typename(chip);
>  size_t typesize = object_type_get_instance_size(typename);
>  int i, j;
>  char *name;
>  XICSFabric *xi = XICS_FABRIC(qdev_get_machine());
>  
> +sbd->num_mmio = PNV_ICP_SYSBUS;
>  name = g_strdup_printf("icp-%x", chip->chip_id);
>  memory_region_init(>icp_mmio, OBJECT(chip), name, PNV_ICP_SIZE);
> -sysbus_init_mmio(SYS_BUS_DEVICE(chip), >icp_mmio);
> +sysbus_init_mmio(sbd, >icp_mmio);
>  g_free(name);
>  
> -sysbus_mmio_map(SYS_BUS_DEVICE(chip), 1, PNV_ICP_BASE(chip));
> +sysbus_mmio_map(sbd, PNV_ICP_SYSBUS, PNV_ICP_BASE(chip));
>  
>  /* Map the ICP registers for each thread */
>  for (i = 0; i < chip->nr_cores; i++) {
> @@ -866,7 +869,26 @@ static void pnv_chip_power8_realize(DeviceState *dev, 
> Error **errp)
>  error_propagate(errp, local_err);
>  return;
>  }
> -sysbus_mmio_map(SYS_BUS_DEVICE(chip), 0, PNV_XSCOM_BASE(chip));
> +sysbus_mmio_map(SYS_BUS_DEVICE(chip), PNV_XSCOM_SYSBUS,
> +   PNV_XSCOM_BASE(chip));
> +
> +/* homer */
> +pnv_homer_realize(chip, _err);
> +if (local_err) {
> +error_propagate(errp, local_err);
> +return;
> +}
> +sysbus_mmio_map(SYS_BUS_DEVICE(chip), PNV_HOMER_SYSBUS,
> +PNV_HOMER_BASE(chip));
> +
> +/* occ common area */
> +pnv_occ_common_area_realize(chip, _err);
> +if (local_err) {
> +error_propagate(errp, local_err);
> +return;
> +}
> +sysbus_mmio_map(SYS_BUS_DEVICE(chip), PNV_OCC_COMMON_AREA_SYSBUS,
> +PNV_OCC_COMMON_AREA(chip));
>  
>  pcc->parent_realize(dev, _err);
>  if (local_err) {
> @@ -1035,7 +1057,26 @@ static void pnv_chip_power9_realize(DeviceState *dev, 
> Error **errp)
>  error_propagate(errp, local_err);
>  return;
>  }
> -sysbus_mmio_map(SYS_BUS_DEVICE(chip), 0, PNV9_XSCOM_BASE(chip));
> +sysbus_mmio_map(SYS_BUS_DEVICE(chip), PNV_XSCOM_SYSBUS,
> +PNV9_XSCOM_BASE(chip));
> +
> +/* homer */
> +pnv_homer_realize(chip, _err);
> +if (local_err) {
> +error_propagate(errp, local_err);
> +return;
> +}
> +sysbus_mmio_map(SYS_BUS_DEVICE(chip), PNV_HOMER_SYSBUS,
> +PNV9_HOMER_BASE(chip));
> +
> +/* occ common area */
> +pnv_occ_common_area_realize(chip, _err);
> +if (local_err) {
> +error_propagate(errp, local_err);
> +return;
> +}
> +sysbus_mmio_map(SYS_BUS_DEVICE(chip), PNV_OCC_COMMON_AREA_SYSBUS,
> +PNV9_OCC_COMMON_AREA(chip));
>  
>  pcc->parent_realize(dev, _err);
>  if (local_err) {
> diff --git a/include/hw/ppc/pnv.h b/include/hw/ppc/pnv.h
> index 6464e32892..dea6772988 100644
> --- a/include/hw/ppc/pnv.h
> +++ b/include/hw/ppc/pnv.h
> @@ -57,6 +57,7 @@ typedef struct PnvChip {
>  
>  /*< public >*/
>  uint32_t chip_id;
> +uint32_t chip_num;
>  uint64_t ram_start;
>  uint64_t ram_size;
>  

-- 
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 PATCH 1/6] utils/python_api: add scripting interface for Qemu with python lib

[David Gibson]

On Thu, Aug 08, 2019 at 02:45:02PM +0200, Philippe Mathieu-Daudé wrote:
> On 8/8/19 12:49 PM, Daniel P. Berrangé wrote:
> > On Wed, Aug 07, 2019 at 12:44:40PM +0530, Balamuruhan S wrote:
> >> Adds scripting interface with python library to call functions in
> >> python modules from Qemu that can be used to feed input externally
> >> and without recompiling Qemu that can be used for early development,
> >> testing and can be extended to abstract some of Qemu code out to a
> >> python script to ease maintenance.
> > 
> > I admit the use case is interesting, but this is opening a can of
> > worms...
> > 
> > Historically the project has held the view that we do not wish
> > to have an mechanism to support loading out of tree code into the
> > QEMU process. Much previously talk was around dlopen'd C plugins,
> > but dynanically loaded Python plugins are doing the same thing
> > at a conceptual level.
> > 
> > We didn't wish to expose internals of QEMU in a plugin API to
> > avoid having any kind of API promise across releases.
> > 
> > There was also the question of licensing with plugins opening
> > the door for people to extend QEMU with non-free/closed source
> > functionality.
> > 
> > While this series only uses the plugin for one fairly obscure
> > device, once a python plugin feature is intergrated in QEMU
> > there will inevitably be requests to use it in further areas
> > of QEMU.
> > 
> > IOW, acceptance of this patch is a significant question for
> > the project, and a broader discussion point, than just this
> > PPC feature patch series.
> 
> Since performance is not an issue, we can use a QMP-PyMMIO bridge.
> Most of the functions required are already exposed, Damien completed the
> missing ones in his 'FAULT INJECTION FRAMEWORK' series:
> https://lists.gnu.org/archive/html/qemu-devel/2019-06/msg06230.html
> 
> Maybe we simply need a clearer (better documented) QMP 'MMIO' API?

I tend to agree.  If performance is not a consideration to the point
that we can use an embedded Python interpreter, it should also not be
an issue to the point that we can move the processing to an entirely
different process with a vetted protocol in between them (QMP or an
extension thereof being the obvious choice).  That seems safer than
embeddeding arbitrary scripting right into the MMIO paths.

-- 
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 PATCH 6/6] hw/ppc/pnv_homer: add python interface support for homer/occ common area

[David Gibson]

On Wed, Aug 07, 2019 at 12:44:45PM +0530, Balamuruhan S wrote:
> use python interface APIs in homer/occ common area emulation to
> interact with scripts if provided else fallback to normal flow,
> it shows how simple to use the interface to call python methods
> with any number of arguments in any script placed in common
> -module-path provided in qemu commandline.

What's the use case for this?

> 
> Signed-off-by: Balamuruhan S 
> ---
>  hw/ppc/pnv_homer.c  | 20 
>  hw/ppc/pnv_xscom.c  |  9 +
>  include/sysemu/sysemu.h |  4 
>  vl.c| 24 
>  4 files changed, 53 insertions(+), 4 deletions(-)
> 
> diff --git a/hw/ppc/pnv_homer.c b/hw/ppc/pnv_homer.c
> index 73a94856d0..6ae5e74f19 100644
> --- a/hw/ppc/pnv_homer.c
> +++ b/hw/ppc/pnv_homer.c
> @@ -16,7 +16,9 @@
>   * You should have received a copy of the GNU Lesser General Public
>   * License along with this library; if not, see 
> .
>   */
> +#include "sysemu/python_api.h"
>  #include "qemu/osdep.h"
> +#include "sysemu/sysemu.h"
>  #include "sysemu/hw_accel.h"
>  #include "sysemu/cpus.h"
>  #include "hw/ppc/pnv.h"
> @@ -37,6 +39,15 @@ static bool core_max_array(hwaddr addr)
>  
>  static uint64_t homer_read(void *opaque, hwaddr addr, unsigned width)
>  {
> +if (homer_module && homer) {
> +uint64_t homer_ret;
> +char **address = g_malloc(sizeof(uint64_t));
> +python_args_init_cast_long(address, addr, 0);
> +homer_ret = python_callback_int(module_path, homer_module, homer, 
> address, 1);
> +python_args_clean(address, 1);
> +g_free(address);
> +return homer_ret;
> +}
>  switch (addr) {
>  case 0xe2006:  /* max pstate ultra turbo */
>  case 0xe2018:  /* pstate id for 0 */
> @@ -106,6 +117,15 @@ const MemoryRegionOps pnv_homer_ops = {
>  
>  static uint64_t occ_common_area_read(void *opaque, hwaddr addr, unsigned 
> width)
>  {
> +if (occ_module && occ) {
> +uint64_t occ_ret;
> +char **address = g_malloc(sizeof(uint64_t));
> +python_args_init_cast_long(address, addr, 0);
> +occ_ret = python_callback_int(module_path, occ_module, occ, address, 
> 1);
> +python_args_clean(address, 1);
> +g_free(address);
> +return occ_ret;
> +}
>  switch (addr) {
>  /*
>   * occ-sensor sanity check that asserts the sensor
> diff --git a/hw/ppc/pnv_xscom.c b/hw/ppc/pnv_xscom.c
> index 18a780bcdf..5e41b7c953 100644
> --- a/hw/ppc/pnv_xscom.c
> +++ b/hw/ppc/pnv_xscom.c
> @@ -179,13 +179,14 @@ static uint64_t xscom_read(void *opaque, hwaddr addr, 
> unsigned width)
>  MemTxResult result;
>  
>  if (xscom_module && xscom_readp) {
> -char **args = g_malloc(2 * sizeof(uint64_t));
> +char **args = g_malloc(3 * sizeof(uint64_t));
>  PnvChipClass *pcc = PNV_CHIP_GET_CLASS(chip);
>  python_args_init_cast_long(args, pcba, 0);
> -python_args_init_cast_int(args, pcc->chip_type, 1);
> +python_args_init_cast_int(args, chip->chip_num, 1);
> +python_args_init_cast_int(args, pcc->chip_type, 2);
>  val = python_callback_int(module_path, xscom_module, xscom_readp,
> -  args, 2);
> -python_args_clean(args, 2);
> +  args, 3);
> +python_args_clean(args, 3);
>  g_free(args);
>  }
>  else {
> diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h
> index 9b8dc346d6..3c8119e040 100644
> --- a/include/sysemu/sysemu.h
> +++ b/include/sysemu/sysemu.h
> @@ -121,6 +121,10 @@ extern const char *module_path;
>  extern const char *xscom_module;
>  extern const char *xscom_readp;
>  extern const char *xscom_writep;
> +extern const char *homer_module;
> +extern const char *homer;
> +extern const char *occ_module;
> +extern const char *occ;
>  extern int mem_prealloc;
>  
>  #define MAX_NODES 128
> diff --git a/vl.c b/vl.c
> index 28f0dc1c1b..c96d35d907 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -144,6 +144,10 @@ const char *module_path = NULL;
>  const char *xscom_module = NULL;
>  const char *xscom_readp = NULL;
>  const char *xscom_writep = NULL;
> +const char *homer_module = NULL;
> +const char *homer = NULL;
> +const char *occ_module = NULL;
> +const char *occ = NULL;
>  int mem_prealloc = 0; /* force preallocation of physical target memory */
>  bool enable_mlock = false;
>  bool enable_cpu_pm = false;
> @@ -495,6 +499,22 @@ static QemuOptsList qemu_module_opts = {
>  .name = "xscom_write",
>  .type = QEMU_OPT_STRING,
>  },
> +{
> +.name = "homer_module",
> +.type = QEMU_OPT_STRING,
> +},
> +{
> +.name = "homer",
> +.type = QEMU_OPT_STRING,
> +},
> +{
> +.name = "occ_module",
> +.type = QEMU_OPT_STRING,
> +},
> + 

Re: [Qemu-devel] [RFC PATCH 0/6] Enhancing Qemu MMIO emulation with scripting interface

[David Gibson]

On Wed, Aug 07, 2019 at 10:15:48AM +0200, Cédric Le Goater wrote:
> On 07/08/2019 09:14, Balamuruhan S wrote:
> > Hi All,
> > 
> > This is a proposal to extend mmio callbacks in Qemu with scripting interface
> > that is prototyped with python in this implementation. It gives ability to
> > feed runtime data through callbacks without recompiling Qemu in generic way.
> > This patchset adds library that provides APIs for Qemu to talk with python
> > scripts placed in path -module-path and how existing xscom can be extended
> > with python interface infrastructure.
> > 
> > We have also added an hacky emulation for memory region (OCC common area 
> > and HOMER)
> > which is shared between core and un-core engine (ideally this should be via
> > sram device) to showcase the effectiveness of having the scripting interface
> > (uncore engine taken for discussion here is powerpc specificed called OCC).
> 
> We should try to merge this part first. It is useful as it is after some
> cleanups.
> 
> > Having scripting interface helps to emulate/test different uncore-core
> > interactions including uncore engine failure or hang. It also helps in 
> > feeding
> > randomized data at byte level access. This patchset is primarily to extend 
> > mmio
> > callbacks with scripting interface and to demonstrate effectiveness it.
> 
> It is already possible to feed device models with external data using QMP or
> external agents using a chardev backend transport. What are the benefits
> of using the embedded python approach ?  

Yeah, I also think this needs better justification.

In particular what's the case that Python makes this significantly
easier than hacking up experimental interactions with C.  I mean you
already have to understand POWER9 internals to work with this, right,
so I wouldn't expect Python's greater accessibility to be a big
concern here.

-- 
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 PATCH 5/6] hw/ppc/pnv_xscom: retrieve homer/occ base address from PBA BARs

[David Gibson]

On Wed, Aug 07, 2019 at 12:44:44PM +0530, Balamuruhan S wrote:
> During PowerNV boot skiboot populates the device tree by retrieving
> base address of homer/occ common area from PBA BARs and prd ipoll
> mask by accessing xscom read/write accesses.
> 
> Signed-off-by: Balamuruhan S 

Again seems unrelatedto the scripting.

> ---
>  hw/ppc/pnv_xscom.c | 27 +++
>  1 file changed, 23 insertions(+), 4 deletions(-)
> 
> diff --git a/hw/ppc/pnv_xscom.c b/hw/ppc/pnv_xscom.c
> index 5d5b5e9884..18a780bcdf 100644
> --- a/hw/ppc/pnv_xscom.c
> +++ b/hw/ppc/pnv_xscom.c
> @@ -77,6 +77,29 @@ static uint64_t xscom_read_default(PnvChip *chip, uint32_t 
> pcba)
>  case 0x18002:   /* ECID2 */
>  return 0;
>  
> +/* PBA BAR0 */
> +case 0x5012b00: /* P9 homer base address */
> +return PNV9_HOMER_BASE(chip);
> +case 0x2013f00: /* P8 homer base address */
> +return PNV_HOMER_BASE(chip);
> +
> +/* PBA BARMASK0 */
> +case 0x5012b04: /* P9 homer region size */
> +case 0x2013f04: /* P8 homer region size */
> +return PNV_HOMER_SIZE;
> +
> +/* PBA BAR2 */
> +case 0x5012b02: /* P9 occ common area */
> +return PNV9_OCC_COMMON_AREA(chip);
> +case 0x2013f02: /* P8 occ common area */
> +return PNV_OCC_COMMON_AREA(chip);
> +
> +/* PBA BARMASK2 */
> +case 0x5012b06: /* P9 occ common area size */
> +case 0x2013f06: /* P8 occ common area size */
> +return PNV_OCC_COMMON_AREA_SIZE;
> +
> +
>  case 0x1010c00: /* PIBAM FIR */
>  case 0x1010c03: /* PIBAM FIR MASK */
>  
> @@ -96,13 +119,9 @@ static uint64_t xscom_read_default(PnvChip *chip, 
> uint32_t pcba)
>  case 0x2020009: /* ADU stuff, error register */
>  case 0x202000f: /* ADU stuff, receive status register*/
>  return 0;
> -case 0x2013f00: /* PBA stuff */
>  case 0x2013f01: /* PBA stuff */
> -case 0x2013f02: /* PBA stuff */
>  case 0x2013f03: /* PBA stuff */
> -case 0x2013f04: /* PBA stuff */
>  case 0x2013f05: /* PBA stuff */
> -case 0x2013f06: /* PBA stuff */
>  case 0x2013f07: /* PBA stuff */
>  return 0;
>  case 0x2013028: /* CAPP stuff */

-- 
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] configure and submodules capstone, slirp, dtc

[David Gibson]

On Thu, Aug 08, 2019 at 05:17:36PM +0200, Markus Armbruster wrote:
> configure --help documents capstone like any other generic optional
> feature.  This leaves --enable-capstone=git and =system undocumented
> there.  Anyone care to improve this?
> 
> It documents slirp unlike other generic optional features, and shows
> only --disable-slirp.  Anyone care to improve this?
> 
> There's also --enable-dtc, which appears to select the system's dtc if
> it deems it usable, else the git submodule.  Should it support explicit
> --enable=dtc=git and =system, too?

I have no strong opinion on the matter, so I'm happy for it to be
consistent with whatever else we have.

-- 
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

[Qemu-devel] [PATCH v2] Update the avx2 configure test to be compatible with clang

[Rebecca Cran]

clang doesn't support the GCC pragma to enable AVX2, but instead
requires the command line option -mavx2. Since GCC also supports that,
remove the pragma lines and add the -mavx2 option when building the
test. If AVX2 is supported, update QEMU_CFLAGS to include -mavx2 .

Signed-off-by: Rebecca Cran 
---
 configure | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/configure b/configure
index 714e7fb6a1..227fce6561 100755
--- a/configure
+++ b/configure
@@ -5392,8 +5392,6 @@ fi
 
 if test "$cpuid_h" = "yes" && test "$avx2_opt" != "no"; then
   cat > $TMPC << EOF
-#pragma GCC push_options
-#pragma GCC target("avx2")
 #include 
 #include 
 static int bar(void *a) {
@@ -5402,7 +5400,8 @@ static int bar(void *a) {
 }
 int main(int argc, char *argv[]) { return bar(argv[0]); }
 EOF
-  if compile_object "" ; then
+  if compile_object "-mavx2" ; then
+QEMU_CFLAGS="-mavx2 $QEMU_CFLAGS"
 avx2_opt="yes"
   else
 avx2_opt="no"
-- 
2.22.0

[Qemu-devel] [PATCH] Update the avx2 configure test to be compatible with clang

[Rebecca Cran]

clang doesn't support the GCC pragma to enable AVX2, but instead
requires the command line option -mavx2. Since GCC also supports that,
remove the pragma lines and add the -mavx2 option when building the
test.

Signed-off-by: Rebecca Cran 
---
 configure | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/configure b/configure
index 714e7fb6a1..30d6c02ab4 100755
--- a/configure
+++ b/configure
@@ -5392,8 +5392,6 @@ fi
 
 if test "$cpuid_h" = "yes" && test "$avx2_opt" != "no"; then
   cat > $TMPC << EOF
-#pragma GCC push_options
-#pragma GCC target("avx2")
 #include 
 #include 
 static int bar(void *a) {
@@ -5402,7 +5400,7 @@ static int bar(void *a) {
 }
 int main(int argc, char *argv[]) { return bar(argv[0]); }
 EOF
-  if compile_object "" ; then
+  if compile_object "-mavx2" ; then
 avx2_opt="yes"
   else
 avx2_opt="no"
-- 
2.22.0

Re: [Qemu-devel] [PATCH] target-arm: Make the counter tick relative to cntfrq

[no-reply]

Patchew URL: https://patchew.org/QEMU/20190809031321.14760-1-and...@aj.id.au/



Hi,

This series failed build test on s390x host. Please find the details below.

=== TEST SCRIPT BEGIN ===
#!/bin/bash
# Testing script will be invoked under the git checkout with
# HEAD pointing to a commit that has the patches applied on top of "base"
# branch
set -e

echo
echo "=== ENV ==="
env

echo
echo "=== PACKAGES ==="
rpm -qa

echo
echo "=== UNAME ==="
uname -a

CC=$HOME/bin/cc
INSTALL=$PWD/install
BUILD=$PWD/build
mkdir -p $BUILD $INSTALL
SRC=$PWD
cd $BUILD
$SRC/configure --cc=$CC --prefix=$INSTALL
make -j4
# XXX: we need reliable clean up
# make check -j4 V=1
make install
=== TEST SCRIPT END ===

  CC  aarch64_be-linux-user/target/arm/arm-semi.o
  CC  aarch64_be-linux-user/target/arm/helper.o
/var/tmp/patchew-tester-tmp-hkd7ua1n/src/target/arm/helper.c: In function 
‘gt_virt_cnt_read’:
/var/tmp/patchew-tester-tmp-hkd7ua1n/src/target/arm/helper.c:2921:12: error: 
implicit declaration of function ‘gt_calc_tick’ 
[-Werror=implicit-function-declaration]
 2921 | return gt_calc_tick(env, cpu_get_clock());
  |^~~~
/var/tmp/patchew-tester-tmp-hkd7ua1n/src/target/arm/helper.c:2921:12: error: 
nested extern declaration of ‘gt_calc_tick’ [-Werror=nested-externs]
cc1: all warnings being treated as errors
make[1]: *** [/var/tmp/patchew-tester-tmp-hkd7ua1n/src/rules.mak:69: 
target/arm/helper.o] Error 1
make: *** [Makefile:472: aarch64_be-linux-user/all] Error 2


The full log is available at
http://patchew.org/logs/20190809031321.14760-1-and...@aj.id.au/testing.s390x/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-de...@redhat.com

[Qemu-devel] qemu icount mode timer accuracy

[Wu, Wentong]

Hi,

Recently I'm working to enable Qemu icount mode with TCG, with source code 
review I found that Qemu can give deterministic execution for guest code 
timeout. But for exact time point for guest OS, I have a question:

For armv7m_systick.c example, guest OS will use systick_read which will call "t 
= qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL); " to calculate his exact time point, 
and qemu_clock_get_ns will use qemu_icount. But from qemu_tcg_rr_cpu_thread_fn 
{ prepare_icount_for_run(cpu); r = tcg_cpu_exec(cpu); 
process_icount_data(cpu);}, we know qemu just update qemu_icount value after 
tcg_cpu_exec, so for each tcg_cpu_exec execution there is the same qemu_icount 
value, and then guest code will get the same time point for that one tcg 
execution. Can someone confirm that?



Thanks

[Qemu-devel] [PATCH] target-arm: Make the counter tick relative to cntfrq

[Andrew Jeffery]

The use of GTIMER_SCALE assumes the clock feeding the generic timer is
62.5MHz for all platforms. This is untrue in general, for example the
ASPEED AST2600 feeds the counter with either an 800 or 1200MHz clock,
and CNTFRQ is configured appropriately by u-boot.

To cope with these values we need to take care of the quantization
caused by the clock scaling in terms of nanoseconds per clock by
calculating an effective frequency such that NANOSECONDS_PER_SECOND is
an integer multiple of the effective frequency. Failing to account for
the quantisation leads to sticky behaviour in the VM as the guest timer
subsystems account for the difference between delay time and the counter
value.

Signed-off-by: Andrew Jeffery 
---
The timer rate assumptions seemed unusual, so I'm not sure if this patch is way
off-base or not. However it does make the AST2600 u-boot and kernel behave
correctly.

 target/arm/helper.c | 51 +
 1 file changed, 47 insertions(+), 4 deletions(-)

diff --git a/target/arm/helper.c b/target/arm/helper.c
index b74c23a9bc08..35d14c183630 100644
--- a/target/arm/helper.c
+++ b/target/arm/helper.c
@@ -2277,6 +2277,34 @@ static const ARMCPRegInfo v6k_cp_reginfo[] = {
 
 #ifndef CONFIG_USER_ONLY
 
+static void gt_recalc_timer(ARMCPU *cpu, int timeridx);
+static void gt_cntfrq_write(CPUARMState *env, const ARMCPRegInfo *ri,
+uint64_t value)
+{
+uint64_t scale;
+ARMCPU *cpu;
+int i;
+
+raw_write(env, ri, value);
+
+/* Fix up the timer scaling */
+cpu = env_archcpu(env);
+scale = MAX(1, NANOSECONDS_PER_SECOND / value);
+for (i = 0; i < NUM_GTIMERS; i++) {
+if (!cpu->gt_timer[i]) {
+continue;
+}
+
+cpu->gt_timer[i]->scale = scale;
+gt_recalc_timer(cpu, i);
+}
+}
+
+static void gt_cntfrq_reset(CPUARMState *env, const ARMCPRegInfo *opaque)
+{
+gt_cntfrq_write(env, opaque, opaque->resetvalue);
+}
+
 static CPAccessResult gt_cntfrq_access(CPUARMState *env, const ARMCPRegInfo 
*ri,
bool isread)
 {
@@ -2405,9 +2433,21 @@ static CPAccessResult gt_stimer_access(CPUARMState *env,
 }
 }
 
+static uint64_t gt_calc_tick(CPUARMState *env, uint64_t ns)
+{
+/*
+ * Deal with quantized clock scaling by calculating the effective frequency
+ * in terms of the timer scale.
+ */
+uint32_t scale = MAX(1, NANOSECONDS_PER_SECOND / env->cp15.c14_cntfrq);
+uint64_t effective = NANOSECONDS_PER_SECOND / scale;
+
+return muldiv64(ns, effective, NANOSECONDS_PER_SECOND);
+}
+
 static uint64_t gt_get_countervalue(CPUARMState *env)
 {
-return qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL) / GTIMER_SCALE;
+return gt_calc_tick(env, qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL));
 }
 
 static void gt_recalc_timer(ARMCPU *cpu, int timeridx)
@@ -2443,8 +2483,8 @@ static void gt_recalc_timer(ARMCPU *cpu, int timeridx)
  * set the timer for as far in the future as possible. When the
  * timer expires we will reset the timer for any remaining period.
  */
-if (nexttick > INT64_MAX / GTIMER_SCALE) {
-nexttick = INT64_MAX / GTIMER_SCALE;
+if (nexttick > INT64_MAX / cpu->gt_timer[timeridx]->scale) {
+nexttick = INT64_MAX / cpu->gt_timer[timeridx]->scale;
 }
 timer_mod(cpu->gt_timer[timeridx], nexttick);
 trace_arm_gt_recalc(timeridx, irqstate, nexttick);
@@ -2686,13 +2726,16 @@ static const ARMCPRegInfo generic_timer_cp_reginfo[] = {
 { .name = "CNTFRQ", .cp = 15, .crn = 14, .crm = 0, .opc1 = 0, .opc2 = 0,
   .type = ARM_CP_ALIAS,
   .access = PL1_RW | PL0_R, .accessfn = gt_cntfrq_access,
+  .writefn = gt_cntfrq_write,
   .fieldoffset = offsetoflow32(CPUARMState, cp15.c14_cntfrq),
 },
 { .name = "CNTFRQ_EL0", .state = ARM_CP_STATE_AA64,
   .opc0 = 3, .opc1 = 3, .crn = 14, .crm = 0, .opc2 = 0,
   .access = PL1_RW | PL0_R, .accessfn = gt_cntfrq_access,
+  .writefn = gt_cntfrq_write,
   .fieldoffset = offsetof(CPUARMState, cp15.c14_cntfrq),
   .resetvalue = (1000 * 1000 * 1000) / GTIMER_SCALE,
+  .resetfn = gt_cntfrq_reset,
 },
 /* overall control: mostly access permissions */
 { .name = "CNTKCTL", .state = ARM_CP_STATE_BOTH,
@@ -2875,7 +2918,7 @@ static uint64_t gt_virt_cnt_read(CPUARMState *env, const 
ARMCPRegInfo *ri)
  * can't call gt_get_countervalue(env), instead we directly
  * call the lower level functions.
  */
-return cpu_get_clock() / GTIMER_SCALE;
+return gt_calc_tick(env, cpu_get_clock());
 }
 
 static const ARMCPRegInfo generic_timer_cp_reginfo[] = {
-- 
2.20.1

Re: [Qemu-devel] [PATCH] numa: Introduce MachineClass::auto_enable_numa for implicit NUMA node

[David Gibson]

On Thu, Aug 08, 2019 at 04:35:00PM +1000, David Gibson wrote:
> On Wed, Aug 07, 2019 at 02:52:56PM -0300, Eduardo Habkost wrote:
> > On Tue, Aug 06, 2019 at 02:50:55PM +0200, Igor Mammedov wrote:
> > > On Mon,  5 Aug 2019 15:13:02 +0800
> > > Tao Xu  wrote:
> > > 
> > > > Add MachineClass::auto_enable_numa field. When it is true, a NUMA node
> > > > is expected to be created implicitly.
> > > > 
> > > > Acked-by: David Gibson 
> > > > Suggested-by: Igor Mammedov 
> > > > Suggested-by: Eduardo Habkost 
> > > > Signed-off-by: Tao Xu 
> > [...]
> > > > +mc->auto_enable_numa = true;
> > > 
> > > this will always create a numa node (that will affect not only RAM but
> > > also all other components that depends on numa state (like CPUs)),
> > > where as spapr_populate_memory() was only faking numa node in DT for RAM.
> > > It makes non-numa configuration impossible.
> > > Seeing David's ACK on the patch it might be fine, but I believe
> > > commit message should capture that and explain why the change in
> > > behavior is fine.
> > 
> > After a quick look, all spapr code seems to have the same
> > behavior when nb_numa_nodes==0 and nb_numa_nodes==1, but I'd like
> > to be sure.
> 
> That's certainly the intention.  If there are cases where it doesn't
> behave that way, it's a bug - although possible one we have to
> maintainer for machine compatibility.
> 
> > David and/or Tao Xu: do you confirm there's no ABI change at all
> > on spapr after implicitly creating a NUMA node?
> 
> I don't believe there is, no.

Oh, FWIW, the PAPR interface which is what defines the guest
environment has no notion of "non NUMA" except in the sense of a
system with exactly one NUMA node.

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


signature.asc
Description: PGP signature

Re: [Qemu-devel] [PATCH v3 09/33] add doc about Resettable interface

[David Gibson]

On Wed, Aug 07, 2019 at 11:34:41AM +0100, Peter Maydell wrote:
> On Wed, 31 Jul 2019 at 07:33, David Gibson  
> wrote:
> >
> > On Mon, Jul 29, 2019 at 04:56:30PM +0200, Damien Hedde wrote:
> > > +The function *resettable_reset* is used to trigger a reset on a given
> > > +object.
> > > +void resettable_reset(Object *obj, bool cold)
> > > +
> > > +The parameter *obj* must implement the Resettable interface.
> >
> > And what happens if it doesn't?  This function has no way to report an
> > error.
> 
> Trying to reset an object that isn't actually resettable would
> be a programming error -- I think asserting is a reasonable
> response to that.

Yeah, fair enough.

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


signature.asc
Description: PGP signature

Re: [Qemu-devel] [PATCH v3 05/33] Switch to new api in qdev/bus

[David Gibson]

On Wed, Jul 31, 2019 at 11:29:36AM +0200, Damien Hedde wrote:
> 
> 
> On 7/31/19 8:05 AM, David Gibson wrote:
> > On Mon, Jul 29, 2019 at 04:56:26PM +0200, Damien Hedde wrote:
> >> Deprecate old reset apis and make them use the new one while they
> >> are still used somewhere.
> >>
> >> Signed-off-by: Damien Hedde 
> >> ---
> >>  hw/core/qdev.c | 22 +++---
> >>  include/hw/qdev-core.h | 28 ++--
> >>  2 files changed, 25 insertions(+), 25 deletions(-)
> >>
> >> diff --git a/hw/core/qdev.c b/hw/core/qdev.c
> >> index 559ced070d..e9e5f2d5f9 100644
> >> --- a/hw/core/qdev.c
> >> +++ b/hw/core/qdev.c
> >> @@ -312,25 +312,9 @@ static void device_foreach_reset_child(Object *obj, 
> >> void (*func)(Object *))
> >>  }
> >>  }
> >>  
> >> -static int qdev_reset_one(DeviceState *dev, void *opaque)
> >> -{
> >> -device_legacy_reset(dev);
> >> -
> >> -return 0;
> >> -}
> >> -
> >> -static int qbus_reset_one(BusState *bus, void *opaque)
> >> -{
> >> -BusClass *bc = BUS_GET_CLASS(bus);
> >> -if (bc->reset) {
> >> -bc->reset(bus);
> >> -}
> >> -return 0;
> >> -}
> >> -
> >>  void qdev_reset_all(DeviceState *dev)
> >>  {
> >> -qdev_walk_children(dev, NULL, NULL, qdev_reset_one, qbus_reset_one, 
> >> NULL);
> >> +device_reset(dev, false);
> >>  }
> >>  
> >>  void qdev_reset_all_fn(void *opaque)
> >> @@ -340,7 +324,7 @@ void qdev_reset_all_fn(void *opaque)
> >>  
> >>  void qbus_reset_all(BusState *bus)
> >>  {
> >> -qbus_walk_children(bus, NULL, NULL, qdev_reset_one, qbus_reset_one, 
> >> NULL);
> >> +bus_reset(bus, false);
> >>  }
> >>  
> >>  void qbus_reset_all_fn(void *opaque)
> >> @@ -922,7 +906,7 @@ static void device_set_realized(Object *obj, bool 
> >> value, Error **errp)
> >>  }
> >>  }
> >>  if (dev->hotplugged) {
> >> -device_legacy_reset(dev);
> >> +device_reset(dev, true);
> > 
> > So.. is this change in the device_reset() signature really necessary?
> > Even if there are compelling reasons to handle warm reset in the new
> > API, that doesn't been you need to change device_reset() itself from
> > its established meaning of a cold (i.e. as per power cycle) reset.
> > Warm resets are generally called in rather more specific circumstances
> > (often under guest software direction) so it seems likely that users
> > would want to engage with the new reset API directly.  Or we could
> > just create a device_warm_reset() wrapper.  That would also avoid the
> > bare boolean parameter, which is not great for readability (you have
> > to look up the signature to have any idea what it means).
> 
> I've added device_reset_cold/warm wrapper functions to avoid having to
> pass the boolean parameter. it seems I forgot to use them in qdev.c
> I suppose, like you said, we could live with
> + no function with the boolean parameter
> + device_reset doing cold reset
> + device_reset_warm (or device_warm_reset) for the warm version

Ok, good.

I'm afraid the whole series still makes me pretty uncomfortable,
though, since the whole "warm reset" concept still seems way to vague
to me.

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


signature.asc
Description: PGP signature

Re: [Qemu-devel] [PATCH v3 05/33] Switch to new api in qdev/bus

[David Gibson]

On Wed, Jul 31, 2019 at 01:31:28PM +0200, Philippe Mathieu-Daudé wrote:
> On 7/31/19 11:29 AM, Damien Hedde wrote:
> > On 7/31/19 8:05 AM, David Gibson wrote:
> >> On Mon, Jul 29, 2019 at 04:56:26PM +0200, Damien Hedde wrote:
> >>> Deprecate old reset apis and make them use the new one while they
> >>> are still used somewhere.
> >>>
> >>> Signed-off-by: Damien Hedde 
> >>> ---
> >>>  hw/core/qdev.c | 22 +++---
> >>>  include/hw/qdev-core.h | 28 ++--
> >>>  2 files changed, 25 insertions(+), 25 deletions(-)
> >>>
> >>> diff --git a/hw/core/qdev.c b/hw/core/qdev.c
> >>> index 559ced070d..e9e5f2d5f9 100644
> >>> --- a/hw/core/qdev.c
> >>> +++ b/hw/core/qdev.c
> >>> @@ -312,25 +312,9 @@ static void device_foreach_reset_child(Object *obj, 
> >>> void (*func)(Object *))
> >>>  }
> >>>  }
> >>>  
> >>> -static int qdev_reset_one(DeviceState *dev, void *opaque)
> >>> -{
> >>> -device_legacy_reset(dev);
> >>> -
> >>> -return 0;
> >>> -}
> >>> -
> >>> -static int qbus_reset_one(BusState *bus, void *opaque)
> >>> -{
> >>> -BusClass *bc = BUS_GET_CLASS(bus);
> >>> -if (bc->reset) {
> >>> -bc->reset(bus);
> >>> -}
> >>> -return 0;
> >>> -}
> >>> -
> >>>  void qdev_reset_all(DeviceState *dev)
> >>>  {
> >>> -qdev_walk_children(dev, NULL, NULL, qdev_reset_one, qbus_reset_one, 
> >>> NULL);
> >>> +device_reset(dev, false);
> >>>  }
> >>>  
> >>>  void qdev_reset_all_fn(void *opaque)
> >>> @@ -340,7 +324,7 @@ void qdev_reset_all_fn(void *opaque)
> >>>  
> >>>  void qbus_reset_all(BusState *bus)
> >>>  {
> >>> -qbus_walk_children(bus, NULL, NULL, qdev_reset_one, qbus_reset_one, 
> >>> NULL);
> >>> +bus_reset(bus, false);
> >>>  }
> >>>  
> >>>  void qbus_reset_all_fn(void *opaque)
> >>> @@ -922,7 +906,7 @@ static void device_set_realized(Object *obj, bool 
> >>> value, Error **errp)
> >>>  }
> >>>  }
> >>>  if (dev->hotplugged) {
> >>> -device_legacy_reset(dev);
> >>> +device_reset(dev, true);
> >>
> >> So.. is this change in the device_reset() signature really necessary?
> >> Even if there are compelling reasons to handle warm reset in the new
> >> API, that doesn't been you need to change device_reset() itself from
> >> its established meaning of a cold (i.e. as per power cycle) reset.
> >> Warm resets are generally called in rather more specific circumstances
> >> (often under guest software direction) so it seems likely that users
> >> would want to engage with the new reset API directly.  Or we could
> >> just create a device_warm_reset() wrapper.  That would also avoid the
> >> bare boolean parameter, which is not great for readability (you have
> >> to look up the signature to have any idea what it means).
> 
> If the boolean is not meaningful, we can use an enum...

That's certainly better, but I'm not seeing a compelling reason not to
have separate function names.  It's just as clear and means less churn.

> 
> > I've added device_reset_cold/warm wrapper functions to avoid having to
> > pass the boolean parameter. it seems I forgot to use them in qdev.c
> > I suppose, like you said, we could live with
> > + no function with the boolean parameter
> > + device_reset doing cold reset
> > + device_reset_warm (or device_warm_reset) for the warm version
> > 
> > Damien
> > 
> 

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


signature.asc
Description: PGP signature

Re: [Qemu-devel] [PATCH] numa: Introduce MachineClass::auto_enable_numa for implicit NUMA node

[David Gibson]

On Wed, Aug 07, 2019 at 02:52:56PM -0300, Eduardo Habkost wrote:
> On Tue, Aug 06, 2019 at 02:50:55PM +0200, Igor Mammedov wrote:
> > On Mon,  5 Aug 2019 15:13:02 +0800
> > Tao Xu  wrote:
> > 
> > > Add MachineClass::auto_enable_numa field. When it is true, a NUMA node
> > > is expected to be created implicitly.
> > > 
> > > Acked-by: David Gibson 
> > > Suggested-by: Igor Mammedov 
> > > Suggested-by: Eduardo Habkost 
> > > Signed-off-by: Tao Xu 
> [...]
> > > +mc->auto_enable_numa = true;
> > 
> > this will always create a numa node (that will affect not only RAM but
> > also all other components that depends on numa state (like CPUs)),
> > where as spapr_populate_memory() was only faking numa node in DT for RAM.
> > It makes non-numa configuration impossible.
> > Seeing David's ACK on the patch it might be fine, but I believe
> > commit message should capture that and explain why the change in
> > behavior is fine.
> 
> After a quick look, all spapr code seems to have the same
> behavior when nb_numa_nodes==0 and nb_numa_nodes==1, but I'd like
> to be sure.

That's certainly the intention.  If there are cases where it doesn't
behave that way, it's a bug - although possible one we have to
maintainer for machine compatibility.

> David and/or Tao Xu: do you confirm there's no ABI change at all
> on spapr after implicitly creating a NUMA node?

I don't believe there is, no.

> 
> > 
> > >  smc->default_caps.caps[SPAPR_CAP_HTM] = SPAPR_CAP_OFF;
> > >  smc->default_caps.caps[SPAPR_CAP_VSX] = SPAPR_CAP_ON;
> > > diff --git a/include/hw/boards.h b/include/hw/boards.h
> > > index 2eb9a0b4e0..4a350b87d2 100644
> > > --- a/include/hw/boards.h
> > > +++ b/include/hw/boards.h
> > > @@ -220,6 +220,7 @@ struct MachineClass {
> > >  bool smbus_no_migration_support;
> > >  bool nvdimm_supported;
> > >  bool numa_mem_supported;
> > > +bool auto_enable_numa;
> > >  
> > >  HotplugHandler *(*get_hotplug_handler)(MachineState *machine,
> > > DeviceState *dev);
> > 
> 

-- 
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] [Fail] tests/test-util-filemonitor fails

[Wei Yang]

On Thu, Aug 08, 2019 at 10:22:13AM +0100, Daniel P. Berrangé wrote:
>On Thu, Aug 08, 2019 at 04:46:53PM +0800, Wei Yang wrote:
>> On Thu, Aug 08, 2019 at 09:02:29AM +0100, Daniel P. Berrangé wrote:
>> >On Thu, Aug 08, 2019 at 10:07:23AM +0800, Wei Yang wrote:
>> >> Current qemu fails tests/test-util-filemonitor.
>> >
>> >You'll need to provide more info. The test works for me and passes in all
>> >the QEMU CI environments.
>> >
>> 
>> The error message from my side is:
>> 
>> /util/filemonitor: Expected watch id 2 but got 1
>> **
>> ERROR:tests/test-util-filemonitor.c:665:test_file_monitor_events: assertion 
>> failed: (err == 0)
>> 
>> What else you'd prefer to have?
>
>Can you set the  "FILEMONITOR_DEBUG=1" env variable before running
>the test - it will print out lots more info
>

Here is the output with more info.

$ FILEMONITOR_DEBUG=1 QTEST_QEMU_BINARY=x86_64-softmmu/qemu-system-x86_64 
tests/test-util-filemonitor
/util/filemonitor: Add watch /tmp/test-util-filemonitor-151B6Z (null)
Watch ID 1
Add watch /tmp/test-util-filemonitor-151B6Z one.txt
Watch ID 10001
Add watch /tmp/test-util-filemonitor-151B6Z two.txt
Watch ID 10002
Create /tmp/test-util-filemonitor-151B6Z/one.txt
Event id=1 event=0 file=one.txt
Event id=10001 event=0 file=one.txt
Create /tmp/test-util-filemonitor-151B6Z/two.txt
Event id=1 event=0 file=two.txt
Event id=10002 event=0 file=two.txt
Create /tmp/test-util-filemonitor-151B6Z/three.txt
Event id=1 event=0 file=three.txt
Unlink /tmp/test-util-filemonitor-151B6Z/three.txt
Event id=1 event=2 file=three.txt
Rename /tmp/test-util-filemonitor-151B6Z/one.txt -> 
/tmp/test-util-filemonitor-151B6Z/two.txt
Event id=1 event=2 file=one.txt
Event id=10001 event=2 file=one.txt
Event id=1 event=0 file=two.txt
Event id=10002 event=0 file=two.txt
Append /tmp/test-util-filemonitor-151B6Z/two.txt
Event id=1 event=1 file=two.txt
Event id=10002 event=1 file=two.txt
Touch /tmp/test-util-filemonitor-151B6Z/two.txt
Event id=1 event=3 file=two.txt
Event id=10002 event=3 file=two.txt
Del watch /tmp/test-util-filemonitor-151B6Z 10001
Add watch /tmp/test-util-filemonitor-151B6Z one.txt
Watch ID 10003
Create /tmp/test-util-filemonitor-151B6Z/one.txt
Event id=1 event=0 file=one.txt
Event id=10003 event=0 file=one.txt
Del watch /tmp/test-util-filemonitor-151B6Z 10003
Unlink /tmp/test-util-filemonitor-151B6Z/one.txt
Event id=1 event=2 file=one.txt
Mkdir /tmp/test-util-filemonitor-151B6Z/fish
Event id=1 event=0 file=fish
Add watch /tmp/test-util-filemonitor-151B6Z fish/
Watch ID 2
Add watch /tmp/test-util-filemonitor-151B6Z fish/one.txt
Watch ID 20001
Create /tmp/test-util-filemonitor-151B6Z/fish/one.txt
Event id=2 event=0 file=one.txt
Event id=20001 event=0 file=one.txt
Del watch /tmp/test-util-filemonitor-151B6Z 20001
Rename /tmp/test-util-filemonitor-151B6Z/fish/one.txt -> 
/tmp/test-util-filemonitor-151B6Z/two.txt
Event id=2 event=2 file=one.txt
Event id=1 event=0 file=two.txt
Event id=10002 event=0 file=two.txt
Rmdir /tmp/test-util-filemonitor-151B6Z/fish
Event id=2 event=4 file=
Expected watch id 2 but got 1
**
ERROR:tests/test-util-filemonitor.c:665:test_file_monitor_events: assertion 
failed: (err == 0)
Aborted (core dumped)


>Also what operating system are you using, and what kernel version
>

OS: Ubuntu 18.04.2 LTS
Kernel: I built the kernel whose last commit is bed38c3e2dca.

>Regards,
>Daniel
>-- 
>|: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
>|: https://libvirt.org -o-https://fstop138.berrange.com :|
>|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|

-- 
Wei Yang
Help you, Help me

Re: [Qemu-devel] [PATCH] hw/ide/atapi: Use the ldst API

[John Snow]

On 8/8/19 9:04 AM, Philippe Mathieu-Daudé wrote:
> The big-endian load/store functions are already provided
> by "qemu/bswap.h".
> Avoid code duplication, use the generic API.
> 
> Signed-off-by: Philippe Mathieu-Daudé 
> ---
>  hw/ide/atapi.c | 80 ++
>  1 file changed, 28 insertions(+), 52 deletions(-)
> 
> diff --git a/hw/ide/atapi.c b/hw/ide/atapi.c
> index 1b0f66cc08..17a9d635d8 100644
> --- a/hw/ide/atapi.c
> +++ b/hw/ide/atapi.c
> @@ -45,30 +45,6 @@ static void padstr8(uint8_t *buf, int buf_size, const char 
> *src)
>  }
>  }
>  
> -static inline void cpu_to_ube16(uint8_t *buf, int val)
> -{
> -buf[0] = val >> 8;
> -buf[1] = val & 0xff;
> -}
> -
> -static inline void cpu_to_ube32(uint8_t *buf, unsigned int val)
> -{
> -buf[0] = val >> 24;
> -buf[1] = val >> 16;
> -buf[2] = val >> 8;
> -buf[3] = val & 0xff;
> -}
> -
> -static inline int ube16_to_cpu(const uint8_t *buf)
> -{
> -return (buf[0] << 8) | buf[1];
> -}
> -
> -static inline int ube32_to_cpu(const uint8_t *buf)
> -{
> -return (buf[0] << 24) | (buf[1] << 16) | (buf[2] << 8) | buf[3];
> -}
> -
>  static void lba_to_msf(uint8_t *buf, int lba)
>  {
>  lba += 150;
> @@ -485,7 +461,7 @@ static inline uint8_t ide_atapi_set_profile(uint8_t *buf, 
> uint8_t *index,
>  uint8_t *buf_profile = buf + 12; /* start of profiles */
>  
>  buf_profile += ((*index) * 4); /* start of indexed profile */
> -cpu_to_ube16 (buf_profile, profile);
> +stw_be_p(buf_profile, profile);
>  buf_profile[2] = ((buf_profile[0] == buf[6]) && (buf_profile[1] == 
> buf[7]));
>  
>  /* each profile adds 4 bytes to the response */
> @@ -518,9 +494,9 @@ static int ide_dvd_read_structure(IDEState *s, int format,
>  buf[7] = 0;   /* default densities */
>  
>  /* FIXME: 0x3 per spec? */
> -cpu_to_ube32(buf + 8, 0); /* start sector */
> -cpu_to_ube32(buf + 12, total_sectors - 1); /* end sector */
> -cpu_to_ube32(buf + 16, total_sectors - 1); /* l0 end sector 
> */
> +stl_be_p(buf + 8, 0); /* start sector */
> +stl_be_p(buf + 12, total_sectors - 1); /* end sector */
> +stl_be_p(buf + 16, total_sectors - 1); /* l0 end sector */
>  
>  /* Size of buffer, not including 2 byte size field */
>  stw_be_p(buf, 2048 + 2);
> @@ -839,7 +815,7 @@ static void cmd_get_configuration(IDEState *s, uint8_t 
> *buf)
>  }
>  
>  /* XXX: could result in alignment problems in some architectures */
> -max_len = ube16_to_cpu(buf + 7);
> +max_len = lduw_be_p(buf + 7);
>  
>  /*
>   * XXX: avoid overflow for io_buffer if max_len is bigger than
> @@ -859,16 +835,16 @@ static void cmd_get_configuration(IDEState *s, uint8_t 
> *buf)
>   * to use as current.  0 means there is no media
>   */
>  if (media_is_dvd(s)) {
> -cpu_to_ube16(buf + 6, MMC_PROFILE_DVD_ROM);
> +stw_be_p(buf + 6, MMC_PROFILE_DVD_ROM);
>  } else if (media_is_cd(s)) {
> -cpu_to_ube16(buf + 6, MMC_PROFILE_CD_ROM);
> +stw_be_p(buf + 6, MMC_PROFILE_CD_ROM);
>  }
>  
>  buf[10] = 0x02 | 0x01; /* persistent and current */
>  len = 12; /* headers: 8 + 4 */
>  len += ide_atapi_set_profile(buf, , MMC_PROFILE_DVD_ROM);
>  len += ide_atapi_set_profile(buf, , MMC_PROFILE_CD_ROM);
> -cpu_to_ube32(buf, len - 4); /* data length */
> +stl_be_p(buf, len - 4); /* data length */
>  
>  ide_atapi_cmd_reply(s, len, max_len);
>  }
> @@ -878,7 +854,7 @@ static void cmd_mode_sense(IDEState *s, uint8_t *buf)
>  int action, code;
>  int max_len;
>  
> -max_len = ube16_to_cpu(buf + 7);
> +max_len = lduw_be_p(buf + 7);
>  action = buf[2] >> 6;
>  code = buf[2] & 0x3f;
>  
> @@ -886,7 +862,7 @@ static void cmd_mode_sense(IDEState *s, uint8_t *buf)
>  case 0: /* current values */
>  switch(code) {
>  case MODE_PAGE_R_W_ERROR: /* error recovery */
> -cpu_to_ube16([0], 16 - 2);
> +stw_be_p([0], 16 - 2);
>  buf[2] = 0x70;
>  buf[3] = 0;
>  buf[4] = 0;
> @@ -905,7 +881,7 @@ static void cmd_mode_sense(IDEState *s, uint8_t *buf)
>  ide_atapi_cmd_reply(s, 16, max_len);
>  break;
>  case MODE_PAGE_AUDIO_CTL:
> -cpu_to_ube16([0], 24 - 2);
> +stw_be_p([0], 24 - 2);
>  buf[2] = 0x70;
>  buf[3] = 0;
>  buf[4] = 0;
> @@ -924,7 +900,7 @@ static void cmd_mode_sense(IDEState *s, uint8_t *buf)
>  ide_atapi_cmd_reply(s, 24, max_len);
>  break;
>  case MODE_PAGE_CAPABILITIES:
> -cpu_to_ube16([0], 30 - 2);
> +stw_be_p([0], 30 - 2);
>  buf[2] = 0x70;
>  buf[3] = 0;
>  buf[4] = 0;
> @@ -946,11 +922,11 @@ static void 

Re: [Qemu-devel] [PATCH] ide: ahci: add check to avoid null dereference (CVE-2019-12067)

[John Snow]

On 8/8/19 5:11 AM, Philippe Mathieu-Daudé wrote:
> Hi Prasad,
> 
> On 8/8/19 8:56 AM, P J P wrote:
>> From: Prasad J Pandit 
>>
>> AHCI emulator while committing DMA buffer in ahci_commit_buf()
>> may do a NULL dereference if the command header 'ad->cur_cmd'
>> is null. Add check to avoid it.
>>
>> Reported-by: Bugs SysSec 
>> Signed-off-by: Prasad J Pandit 
>> ---
>>  hw/ide/ahci.c | 6 --
>>  1 file changed, 4 insertions(+), 2 deletions(-)
>>
>> diff --git a/hw/ide/ahci.c b/hw/ide/ahci.c
>> index 00ba422a48..9fff94075b 100644
>> --- a/hw/ide/ahci.c
>> +++ b/hw/ide/ahci.c
>> @@ -1458,8 +1458,10 @@ static void ahci_commit_buf(IDEDMA *dma, uint32_t 
>> tx_bytes)
>>  {
>>  AHCIDevice *ad = DO_UPCAST(AHCIDevice, dma, dma);
>>  
>> -tx_bytes += le32_to_cpu(ad->cur_cmd->status);
>> -ad->cur_cmd->status = cpu_to_le32(tx_bytes);
>> +if (ad->cur_cmd) {
> 
> My 2 cents, John will correct me:
> 
> This is not a valid condition, so an assert() might be more appropriate
> here.
> 
> Why is dma_buf_commit() being called with a null command? This check
> should go elsewhere to avoid the call.
> 

Yes, something else is broken.

Can you please give a reproducer or more detailed report so I can fix
this properly?

This likely is just adding a whole host of new undefined problems
waiting to happen by adding an if (...), so while it's not nice to have
a host assert() it's probably better than taking the state machine off
the rails and seeing what else breaks.

>> +tx_bytes += le32_to_cpu(ad->cur_cmd->status);
>> +ad->cur_cmd->status = cpu_to_le32(tx_bytes);
>> +}
>>  }
>>  
>>  static int ahci_dma_rw_buf(IDEDMA *dma, int is_write)
>>

[Qemu-devel] [PATCH 6/7] target/arm: Simplify SMMLA, SMMLAR, SMMLS, SMMLSR

[Richard Henderson]

All of the inputs to these instructions are 32-bits.  Rather than
extend each input to 64-bits and then extract the high 32-bits of
the output, use tcg_gen_muls2_i32 and other 32-bit generator functions.

Signed-off-by: Richard Henderson 
---
 target/arm/translate.c | 72 +++---
 1 file changed, 26 insertions(+), 46 deletions(-)

diff --git a/target/arm/translate.c b/target/arm/translate.c
index ddc54e77e4..77154be743 100644
--- a/target/arm/translate.c
+++ b/target/arm/translate.c
@@ -391,34 +391,6 @@ static void gen_revsh(TCGv_i32 var)
 tcg_gen_ext16s_i32(var, var);
 }
 
-/* Return (b << 32) + a. Mark inputs as dead */
-static TCGv_i64 gen_addq_msw(TCGv_i64 a, TCGv_i32 b)
-{
-TCGv_i64 tmp64 = tcg_temp_new_i64();
-
-tcg_gen_extu_i32_i64(tmp64, b);
-tcg_temp_free_i32(b);
-tcg_gen_shli_i64(tmp64, tmp64, 32);
-tcg_gen_add_i64(a, tmp64, a);
-
-tcg_temp_free_i64(tmp64);
-return a;
-}
-
-/* Return (b << 32) - a. Mark inputs as dead. */
-static TCGv_i64 gen_subq_msw(TCGv_i64 a, TCGv_i32 b)
-{
-TCGv_i64 tmp64 = tcg_temp_new_i64();
-
-tcg_gen_extu_i32_i64(tmp64, b);
-tcg_temp_free_i32(b);
-tcg_gen_shli_i64(tmp64, tmp64, 32);
-tcg_gen_sub_i64(a, tmp64, a);
-
-tcg_temp_free_i64(tmp64);
-return a;
-}
-
 /* 32x32->64 multiply.  Marks inputs as dead.  */
 static TCGv_i64 gen_mulu_i64_i32(TCGv_i32 a, TCGv_i32 b)
 {
@@ -8872,23 +8844,27 @@ static void disas_arm_insn(DisasContext *s, unsigned 
int insn)
(SMMUL, SMMLA, SMMLS) */
 tmp = load_reg(s, rm);
 tmp2 = load_reg(s, rs);
-tmp64 = gen_muls_i64_i32(tmp, tmp2);
+tcg_gen_muls2_i32(tmp2, tmp, tmp, tmp2);
 
 if (rd != 15) {
-tmp = load_reg(s, rd);
+tmp3 = load_reg(s, rd);
 if (insn & (1 << 6)) {
-tmp64 = gen_subq_msw(tmp64, tmp);
+tcg_gen_sub_i32(tmp, tmp, tmp3);
 } else {
-tmp64 = gen_addq_msw(tmp64, tmp);
+tcg_gen_add_i32(tmp, tmp, tmp3);
 }
+tcg_temp_free_i32(tmp3);
 }
 if (insn & (1 << 5)) {
-tcg_gen_addi_i64(tmp64, tmp64, 0x8000u);
+/*
+ * Adding 0x8000 to the 64-bit quantity
+ * means that we have carry in to the high
+ * word when the low word has the high bit set.
+ */
+tcg_gen_shri_i32(tmp2, tmp2, 31);
+tcg_gen_add_i32(tmp, tmp, tmp2);
 }
-tcg_gen_shri_i64(tmp64, tmp64, 32);
-tmp = tcg_temp_new_i32();
-tcg_gen_extrl_i64_i32(tmp, tmp64);
-tcg_temp_free_i64(tmp64);
+tcg_temp_free_i32(tmp2);
 store_reg(s, rn, tmp);
 break;
 case 0:
@@ -10114,22 +10090,26 @@ static void disas_thumb2_insn(DisasContext *s, 
uint32_t insn)
   }
 break;
 case 5: case 6: /* 32 * 32 -> 32msb (SMMUL, SMMLA, SMMLS) */
-tmp64 = gen_muls_i64_i32(tmp, tmp2);
+tcg_gen_muls2_i32(tmp2, tmp, tmp, tmp2);
 if (rs != 15) {
-tmp = load_reg(s, rs);
+tmp3 = load_reg(s, rs);
 if (insn & (1 << 20)) {
-tmp64 = gen_addq_msw(tmp64, tmp);
+tcg_gen_add_i32(tmp, tmp, tmp3);
 } else {
-tmp64 = gen_subq_msw(tmp64, tmp);
+tcg_gen_sub_i32(tmp, tmp, tmp3);
 }
+tcg_temp_free_i32(tmp3);
 }
 if (insn & (1 << 4)) {
-tcg_gen_addi_i64(tmp64, tmp64, 0x8000u);
+/*
+ * Adding 0x8000 to the 64-bit quantity
+ * means that we have carry in to the high
+ * word when the low word has the high bit set.
+ */
+tcg_gen_shri_i32(tmp2, tmp2, 31);
+tcg_gen_add_i32(tmp, tmp, tmp2);
 }
-tcg_gen_shri_i64(tmp64, tmp64, 32);
-tmp = tcg_temp_new_i32();
-tcg_gen_extrl_i64_i32(tmp, tmp64);
-tcg_temp_free_i64(tmp64);
+tcg_temp_free_i32(tmp2);
 break;
 case 7: /* Unsigned sum of absolute differences.  */
 

[Qemu-devel] [PATCH 7/7] target/arm: Use tcg_gen_extrh_i64_i32 to extract the high word

[Richard Henderson]

Separate shift + extract low will result in one extra insn
for hosts like RISC-V, MIPS, and Sparc.

Signed-off-by: Richard Henderson 
---
 target/arm/translate.c | 18 ++
 1 file changed, 6 insertions(+), 12 deletions(-)

diff --git a/target/arm/translate.c b/target/arm/translate.c
index 77154be743..9e103e4fad 100644
--- a/target/arm/translate.c
+++ b/target/arm/translate.c
@@ -1761,8 +1761,7 @@ static int disas_iwmmxt_insn(DisasContext *s, uint32_t 
insn)
 if (insn & ARM_CP_RW_BIT) { /* TMRRC */
 iwmmxt_load_reg(cpu_V0, wrd);
 tcg_gen_extrl_i64_i32(cpu_R[rdlo], cpu_V0);
-tcg_gen_shri_i64(cpu_V0, cpu_V0, 32);
-tcg_gen_extrl_i64_i32(cpu_R[rdhi], cpu_V0);
+tcg_gen_extrh_i64_i32(cpu_R[rdhi], cpu_V0);
 } else {/* TMCRR */
 tcg_gen_concat_i32_i64(cpu_V0, cpu_R[rdlo], cpu_R[rdhi]);
 iwmmxt_store_reg(cpu_V0, wrd);
@@ -2807,8 +2806,7 @@ static int disas_dsp_insn(DisasContext *s, uint32_t insn)
 if (insn & ARM_CP_RW_BIT) { /* MRA */
 iwmmxt_load_reg(cpu_V0, acc);
 tcg_gen_extrl_i64_i32(cpu_R[rdlo], cpu_V0);
-tcg_gen_shri_i64(cpu_V0, cpu_V0, 32);
-tcg_gen_extrl_i64_i32(cpu_R[rdhi], cpu_V0);
+tcg_gen_extrh_i64_i32(cpu_R[rdhi], cpu_V0);
 tcg_gen_andi_i32(cpu_R[rdhi], cpu_R[rdhi], (1 << (40 - 32)) - 1);
 } else {/* MAR */
 tcg_gen_concat_i32_i64(cpu_V0, cpu_R[rdlo], cpu_R[rdhi]);
@@ -6005,8 +6003,7 @@ static int disas_neon_data_insn(DisasContext *s, uint32_t 
insn)
 gen_helper_neon_narrow_high_u16(tmp, cpu_V0);
 break;
 case 2:
-tcg_gen_shri_i64(cpu_V0, cpu_V0, 32);
-tcg_gen_extrl_i64_i32(tmp, cpu_V0);
+tcg_gen_extrh_i64_i32(tmp, cpu_V0);
 break;
 default: abort();
 }
@@ -6020,8 +6017,7 @@ static int disas_neon_data_insn(DisasContext *s, uint32_t 
insn)
 break;
 case 2:
 tcg_gen_addi_i64(cpu_V0, cpu_V0, 1u << 31);
-tcg_gen_shri_i64(cpu_V0, cpu_V0, 32);
-tcg_gen_extrl_i64_i32(tmp, cpu_V0);
+tcg_gen_extrh_i64_i32(tmp, cpu_V0);
 break;
 default: abort();
 }
@@ -7254,9 +7250,8 @@ static int disas_coproc_insn(DisasContext *s, uint32_t 
insn)
 tmp = tcg_temp_new_i32();
 tcg_gen_extrl_i64_i32(tmp, tmp64);
 store_reg(s, rt, tmp);
-tcg_gen_shri_i64(tmp64, tmp64, 32);
 tmp = tcg_temp_new_i32();
-tcg_gen_extrl_i64_i32(tmp, tmp64);
+tcg_gen_extrh_i64_i32(tmp, tmp64);
 tcg_temp_free_i64(tmp64);
 store_reg(s, rt2, tmp);
 } else {
@@ -7365,8 +7360,7 @@ static void gen_storeq_reg(DisasContext *s, int rlow, int 
rhigh, TCGv_i64 val)
 tcg_gen_extrl_i64_i32(tmp, val);
 store_reg(s, rlow, tmp);
 tmp = tcg_temp_new_i32();
-tcg_gen_shri_i64(val, val, 32);
-tcg_gen_extrl_i64_i32(tmp, val);
+tcg_gen_extrh_i64_i32(tmp, val);
 store_reg(s, rhigh, tmp);
 }
 
-- 
2.17.1

[Qemu-devel] [PATCH 3/7] target/arm: Remove redundant shift tests

[Richard Henderson]

The immediate shift generator functions already test for,
and eliminate, the case of a shift by zero.

Signed-off-by: Richard Henderson 
---
 target/arm/translate.c | 19 +++
 1 file changed, 7 insertions(+), 12 deletions(-)

diff --git a/target/arm/translate.c b/target/arm/translate.c
index 94170af134..3ddc404b3b 100644
--- a/target/arm/translate.c
+++ b/target/arm/translate.c
@@ -8826,8 +8826,7 @@ static void disas_arm_insn(DisasContext *s, unsigned int 
insn)
 shift = (insn >> 10) & 3;
 /* ??? In many cases it's not necessary to do a
rotate, a shift is sufficient.  */
-if (shift != 0)
-tcg_gen_rotri_i32(tmp, tmp, shift * 8);
+tcg_gen_rotri_i32(tmp, tmp, shift * 8);
 op1 = (insn >> 20) & 7;
 switch (op1) {
 case 0: gen_sxtb16(tmp);  break;
@@ -9904,8 +9903,7 @@ static void disas_thumb2_insn(DisasContext *s, uint32_t 
insn)
 shift = (insn >> 4) & 3;
 /* ??? In many cases it's not necessary to do a
rotate, a shift is sufficient.  */
-if (shift != 0)
-tcg_gen_rotri_i32(tmp, tmp, shift * 8);
+tcg_gen_rotri_i32(tmp, tmp, shift * 8);
 op = (insn >> 20) & 7;
 switch (op) {
 case 0: gen_sxth(tmp);   break;
@@ -10632,11 +10630,10 @@ static void disas_thumb2_insn(DisasContext *s, 
uint32_t insn)
 case 7:
 goto illegal_op;
 default: /* Saturate.  */
-if (shift) {
-if (op & 1)
-tcg_gen_sari_i32(tmp, tmp, shift);
-else
-tcg_gen_shli_i32(tmp, tmp, shift);
+if (op & 1) {
+tcg_gen_sari_i32(tmp, tmp, shift);
+} else {
+tcg_gen_shli_i32(tmp, tmp, shift);
 }
 tmp2 = tcg_const_i32(imm);
 if (op & 4) {
@@ -10827,9 +10824,7 @@ static void disas_thumb2_insn(DisasContext *s, uint32_t 
insn)
 goto illegal_op;
 }
 tmp = load_reg(s, rm);
-if (shift) {
-tcg_gen_shli_i32(tmp, tmp, shift);
-}
+tcg_gen_shli_i32(tmp, tmp, shift);
 tcg_gen_add_i32(addr, addr, tmp);
 tcg_temp_free_i32(tmp);
 break;
-- 
2.17.1

[Qemu-devel] [PATCH 1/7] target/arm: Use tcg_gen_extract_i32 for shifter_out_im

[Richard Henderson]

Extract is a compact combination of shift + and.

Signed-off-by: Richard Henderson 
---
 target/arm/translate.c | 9 +
 1 file changed, 1 insertion(+), 8 deletions(-)

diff --git a/target/arm/translate.c b/target/arm/translate.c
index 846052acea..43e005d191 100644
--- a/target/arm/translate.c
+++ b/target/arm/translate.c
@@ -620,14 +620,7 @@ static void gen_sar(TCGv_i32 dest, TCGv_i32 t0, TCGv_i32 
t1)
 
 static void shifter_out_im(TCGv_i32 var, int shift)
 {
-if (shift == 0) {
-tcg_gen_andi_i32(cpu_CF, var, 1);
-} else {
-tcg_gen_shri_i32(cpu_CF, var, shift);
-if (shift != 31) {
-tcg_gen_andi_i32(cpu_CF, cpu_CF, 1);
-}
-}
+tcg_gen_extract_i32(cpu_CF, var, shift, 1);
 }
 
 /* Shift by immediate.  Includes special handling for shift == 0.  */
-- 
2.17.1

[Qemu-devel] [PATCH 5/7] target/arm: Use tcg_gen_rotri_i32 for gen_swap_half

[Richard Henderson]

Rotate is the more compact and obvious way to swap 16-bit
elements of a 32-bit word.

Signed-off-by: Richard Henderson 
---
 target/arm/translate.c | 6 +-
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/target/arm/translate.c b/target/arm/translate.c
index b40f163bab..ddc54e77e4 100644
--- a/target/arm/translate.c
+++ b/target/arm/translate.c
@@ -459,11 +459,7 @@ static TCGv_i64 gen_muls_i64_i32(TCGv_i32 a, TCGv_i32 b)
 /* Swap low and high halfwords.  */
 static void gen_swap_half(TCGv_i32 var)
 {
-TCGv_i32 tmp = tcg_temp_new_i32();
-tcg_gen_shri_i32(tmp, var, 16);
-tcg_gen_shli_i32(var, var, 16);
-tcg_gen_or_i32(var, var, tmp);
-tcg_temp_free_i32(tmp);
+tcg_gen_rotri_i32(var, var, 16);
 }
 
 /* Dual 16-bit add.  Result placed in t0 and t1 is marked as dead.
-- 
2.17.1

[Qemu-devel] [PATCH 2/7] target/arm: Use tcg_gen_deposit_i32 for PKHBT, PKHTB

[Richard Henderson]

Use deposit as the composit operation to merge the
bits from the two inputs.

Signed-off-by: Richard Henderson 
---
 target/arm/translate.c | 26 ++
 1 file changed, 10 insertions(+), 16 deletions(-)

diff --git a/target/arm/translate.c b/target/arm/translate.c
index 43e005d191..94170af134 100644
--- a/target/arm/translate.c
+++ b/target/arm/translate.c
@@ -8769,19 +8769,16 @@ static void disas_arm_insn(DisasContext *s, unsigned 
int insn)
 shift = (insn >> 7) & 0x1f;
 if (insn & (1 << 6)) {
 /* pkhtb */
-if (shift == 0)
+if (shift == 0) {
 shift = 31;
+}
 tcg_gen_sari_i32(tmp2, tmp2, shift);
-tcg_gen_andi_i32(tmp, tmp, 0x);
-tcg_gen_ext16u_i32(tmp2, tmp2);
+tcg_gen_deposit_i32(tmp, tmp, tmp2, 0, 16);
 } else {
 /* pkhbt */
-if (shift)
-tcg_gen_shli_i32(tmp2, tmp2, shift);
-tcg_gen_ext16u_i32(tmp, tmp);
-tcg_gen_andi_i32(tmp2, tmp2, 0x);
+tcg_gen_shli_i32(tmp2, tmp2, shift);
+tcg_gen_deposit_i32(tmp, tmp2, tmp, 0, 16);
 }
-tcg_gen_or_i32(tmp, tmp, tmp2);
 tcg_temp_free_i32(tmp2);
 store_reg(s, rd, tmp);
 } else if ((insn & 0x00200020) == 0x0020) {
@@ -9817,19 +9814,16 @@ static void disas_thumb2_insn(DisasContext *s, uint32_t 
insn)
 shift = ((insn >> 10) & 0x1c) | ((insn >> 6) & 0x3);
 if (insn & (1 << 5)) {
 /* pkhtb */
-if (shift == 0)
+if (shift == 0) {
 shift = 31;
+}
 tcg_gen_sari_i32(tmp2, tmp2, shift);
-tcg_gen_andi_i32(tmp, tmp, 0x);
-tcg_gen_ext16u_i32(tmp2, tmp2);
+tcg_gen_deposit_i32(tmp, tmp, tmp2, 0, 16);
 } else {
 /* pkhbt */
-if (shift)
-tcg_gen_shli_i32(tmp2, tmp2, shift);
-tcg_gen_ext16u_i32(tmp, tmp);
-tcg_gen_andi_i32(tmp2, tmp2, 0x);
+tcg_gen_shli_i32(tmp2, tmp2, shift);
+tcg_gen_deposit_i32(tmp, tmp2, tmp, 0, 16);
 }
-tcg_gen_or_i32(tmp, tmp, tmp2);
 tcg_temp_free_i32(tmp2);
 store_reg(s, rd, tmp);
 } else {
-- 
2.17.1

[Qemu-devel] [PATCH 4/7] target/arm: Use ror32 instead of open-coding the operation

[Richard Henderson]

The helper function is more documentary, and also already
handles the case of rotate by zero.

Signed-off-by: Richard Henderson 
---
 target/arm/translate.c | 7 ++-
 1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/target/arm/translate.c b/target/arm/translate.c
index 3ddc404b3b..b40f163bab 100644
--- a/target/arm/translate.c
+++ b/target/arm/translate.c
@@ -7979,8 +7979,7 @@ static void disas_arm_insn(DisasContext *s, unsigned int 
insn)
 /* CPSR = immediate */
 val = insn & 0xff;
 shift = ((insn >> 8) & 0xf) * 2;
-if (shift)
-val = (val >> shift) | (val << (32 - shift));
+val = ror32(val, shift);
 i = ((insn & (1 << 22)) != 0);
 if (gen_set_psr_im(s, msr_mask(s, (insn >> 16) & 0xf, i),
i, val)) {
@@ -8243,9 +8242,7 @@ static void disas_arm_insn(DisasContext *s, unsigned int 
insn)
 /* immediate operand */
 val = insn & 0xff;
 shift = ((insn >> 8) & 0xf) * 2;
-if (shift) {
-val = (val >> shift) | (val << (32 - shift));
-}
+val = ror32(val, shift);
 tmp2 = tcg_temp_new_i32();
 tcg_gen_movi_i32(tmp2, val);
 if (logic_cc && shift) {
-- 
2.17.1

[Qemu-devel] [PATCH 0/7] target/arm: Misc cleanups

[Richard Henderson]

Some of these were cleanups that I was making simultaneous
with the decodetree split.  Let's do those beforehand to
make the split easier to read.

Some of these are new, noticed while I was in the area.


r~


Richard Henderson (7):
  target/arm: Use tcg_gen_extract_i32 for shifter_out_im
  target/arm: Use tcg_gen_deposit_i32 for PKHBT, PKHTB
  target/arm: Remove redundant shift tests
  target/arm: Use ror32 instead of open-coding the operation
  target/arm: Use tcg_gen_rotri_i32 for gen_swap_half
  target/arm: Simplify SMMLA, SMMLAR, SMMLS, SMMLSR
  target/arm: Use tcg_gen_extrh_i64_i32 to extract the high word

 target/arm/translate.c | 157 ++---
 1 file changed, 53 insertions(+), 104 deletions(-)

-- 
2.17.1

Re: [Qemu-devel] Is network backend netmap worth keeping?

[Vincenzo Maffione]

Yes, indeed.
Netmap is actively maintained on FreeBSD, and QEMU is packaged on FreeBSD
with netmap support enabled.
Also keep in mind that, differently from Linux, the (current) tap driver on
FreeBSD does not support offloads (e.g. IFF_VNET_HDR, TUNSETVNETHDRSIZE and
so on). On the contrary, netmap (VALE) supports the same offloads as tap
does on Linux. This makes inter-VM networking with VALE more appealing on
FreeBSD.

Cheers,
  Vincenzo

Il giorno gio 8 ago 2019 alle ore 15:36 Philippe Mathieu-Daudé <
phi...@redhat.com> ha scritto:

> On 8/8/19 7:38 AM, Jason Wang wrote:
> >
> > On 2019/8/8 下午12:48, Markus Armbruster wrote:
> >> Please excuse the attention-grabbing subject.
> >>
> >> Philippe Mathieu-Daudé  writes:
> >>
> >>> On 8/7/19 10:16 PM, Markus Armbruster wrote:
> >> [...]
>  Can you tell me offhand what I have to install so configure enables
>  CONFIG_NETMAP?
> >>> The steps are listed in tests/docker/dockerfiles/debian-amd64.docker,
> >>> but you can get to this point running:
> >>>
> >>>$ make docker-image-debian-amd64 V=1 DEBUG=1
> >>>
> >>> This will build the docker image with netmap (so you don't have to mess
> >>> with your workstation setup), then build QEMU within the image.
> >> So, to make use of QEMU's netmap backend (CONFIG_NETMAP), you have to
> >> build and install netmap software from sources.  Which pretty much
> >> ensures nobody uses it.  It was added in commit 58952137b0b (Nov 2013).
> >> The commit message points to ,
> >> which gives me "connection timed out" right now.
> >>
> >> On the other hand, it's covered in MAINTAINERS, and has seen
> >> non-janitorial activity as late as Dec 2018 (commit c693fc748a).
> >>
> >> Luigi, Giuseppe, Vincenzo, what's the status of the netmap project?
> >>
> >> Why is the QEMU netmap backend worth keeping?
> >>
> >> Who is using the netmap backend?
> >
> >
> > Netmap was supported by freebsd:
> > https://www.freebsd.org/cgi/man.cgi?query=netmap=4. So I guess
> > there should be real users.
> >
> >
> >>
> >> How do they obtain a netmap-enabled QEMU?  Compile it from sources
> >> themselves?
> >
> >
> > Yes.
>
> Hmm at least on the FreeBSD setup by vmtest (12.0-RELEASE r341666) we
> don't need to build it from source:
>
> $ make vm-build-freebsd V=1 DEBUG=1
> [...]
> netmap supportyes
> [...]
>
> $ fgrep -r CONFIG_NETMAP .
> ./config-host.mak:CONFIG_NETMAP=y
>

Re: [Qemu-devel] [PATCH v1 0/7] softfloat header cleanups

[no-reply]

Patchew URL: 
https://patchew.org/QEMU/20190808164117.23348-1-alex.ben...@linaro.org/



Hi,

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

Type: series
Subject: [Qemu-devel] [PATCH  v1 0/7] softfloat header cleanups
Message-id: 20190808164117.23348-1-alex.ben...@linaro.org

=== TEST SCRIPT BEGIN ===
#!/bin/bash
git rev-parse base > /dev/null || exit 0
git config --local diff.renamelimit 0
git config --local diff.renames True
git config --local diff.algorithm histogram
./scripts/checkpatch.pl --mailback base..
=== TEST SCRIPT END ===

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
From https://github.com/patchew-project/qemu
 - [tag update]  patchew/20190808164117.23348-1-alex.ben...@linaro.org -> 
patchew/20190808164117.23348-1-alex.ben...@linaro.org
Submodule 'capstone' (https://git.qemu.org/git/capstone.git) registered for 
path 'capstone'
Submodule 'dtc' (https://git.qemu.org/git/dtc.git) registered for path 'dtc'
Submodule 'roms/QemuMacDrivers' (https://git.qemu.org/git/QemuMacDrivers.git) 
registered for path 'roms/QemuMacDrivers'
Submodule 'roms/SLOF' (https://git.qemu.org/git/SLOF.git) registered for path 
'roms/SLOF'
Submodule 'roms/edk2' (https://git.qemu.org/git/edk2.git) registered for path 
'roms/edk2'
Submodule 'roms/ipxe' (https://git.qemu.org/git/ipxe.git) registered for path 
'roms/ipxe'
Submodule 'roms/openbios' (https://git.qemu.org/git/openbios.git) registered 
for path 'roms/openbios'
Submodule 'roms/openhackware' (https://git.qemu.org/git/openhackware.git) 
registered for path 'roms/openhackware'
Submodule 'roms/opensbi' (https://git.qemu.org/git/opensbi.git) registered for 
path 'roms/opensbi'
Submodule 'roms/qemu-palcode' (https://git.qemu.org/git/qemu-palcode.git) 
registered for path 'roms/qemu-palcode'
Submodule 'roms/seabios' (https://git.qemu.org/git/seabios.git/) registered for 
path 'roms/seabios'
Submodule 'roms/seabios-hppa' (https://git.qemu.org/git/seabios-hppa.git) 
registered for path 'roms/seabios-hppa'
Submodule 'roms/sgabios' (https://git.qemu.org/git/sgabios.git) registered for 
path 'roms/sgabios'
Submodule 'roms/skiboot' (https://git.qemu.org/git/skiboot.git) registered for 
path 'roms/skiboot'
Submodule 'roms/u-boot' (https://git.qemu.org/git/u-boot.git) registered for 
path 'roms/u-boot'
Submodule 'roms/u-boot-sam460ex' (https://git.qemu.org/git/u-boot-sam460ex.git) 
registered for path 'roms/u-boot-sam460ex'
Submodule 'slirp' (https://git.qemu.org/git/libslirp.git) registered for path 
'slirp'
Submodule 'tests/fp/berkeley-softfloat-3' 
(https://git.qemu.org/git/berkeley-softfloat-3.git) registered for path 
'tests/fp/berkeley-softfloat-3'
Submodule 'tests/fp/berkeley-testfloat-3' 
(https://git.qemu.org/git/berkeley-testfloat-3.git) registered for path 
'tests/fp/berkeley-testfloat-3'
Submodule 'ui/keycodemapdb' (https://git.qemu.org/git/keycodemapdb.git) 
registered for path 'ui/keycodemapdb'
Cloning into 'capstone'...
Submodule path 'capstone': checked out 
'22ead3e0bfdb87516656453336160e0a37b066bf'
Cloning into 'dtc'...
Submodule path 'dtc': checked out '88f18909db731a627456f26d779445f84e449536'
Cloning into 'roms/QemuMacDrivers'...
Submodule path 'roms/QemuMacDrivers': checked out 
'90c488d5f4a407342247b9ea869df1c2d9c8e266'
Cloning into 'roms/SLOF'...
Submodule path 'roms/SLOF': checked out 
'ba1ab360eebe6338bb8d7d83a9220ccf7e213af3'
Cloning into 'roms/edk2'...
Submodule path 'roms/edk2': checked out 
'20d2e5a125e34fc8501026613a71549b2a1a3e54'
Submodule 'SoftFloat' (https://github.com/ucb-bar/berkeley-softfloat-3.git) 
registered for path 'ArmPkg/Library/ArmSoftFloatLib/berkeley-softfloat-3'
Submodule 'CryptoPkg/Library/OpensslLib/openssl' 
(https://github.com/openssl/openssl) registered for path 
'CryptoPkg/Library/OpensslLib/openssl'
Cloning into 'ArmPkg/Library/ArmSoftFloatLib/berkeley-softfloat-3'...
Submodule path 'roms/edk2/ArmPkg/Library/ArmSoftFloatLib/berkeley-softfloat-3': 
checked out 'b64af41c3276f97f0e181920400ee056b9c88037'
Cloning into 'CryptoPkg/Library/OpensslLib/openssl'...
Submodule path 'roms/edk2/CryptoPkg/Library/OpensslLib/openssl': checked out 
'50eaac9f3337667259de725451f201e784599687'
Submodule 'boringssl' (https://boringssl.googlesource.com/boringssl) registered 
for path 'boringssl'
Submodule 'krb5' (https://github.com/krb5/krb5) registered for path 'krb5'
Submodule 'pyca.cryptography' (https://github.com/pyca/cryptography.git) 
registered for path 'pyca-cryptography'
Cloning into 'boringssl'...
Submodule path 'roms/edk2/CryptoPkg/Library/OpensslLib/openssl/boringssl': 
checked out '2070f8ad9151dc8f3a73bffaa146b5e6937a583f'
Cloning into 'krb5'...
Submodule path 'roms/edk2/CryptoPkg/Library/OpensslLib/openssl/krb5': checked 
out 'b9ad6c49505c96a088326b62a52568e3484f2168'
Cloning into 'pyca-cryptography'...
Submodule path 
'roms/edk2/CryptoPkg/Library/OpensslLib/openssl/pyca-cryptography': checked out 
'09403100de2f6f1cdd0d484dcb8e620f1c335c8f'
Cloning into 'roms/ipxe'...

Re: [Qemu-devel] [PATCH v3 03/15] target/arm/monitor: Introduce qmp_query_cpu_model_expansion

[Richard Henderson]

On 8/8/19 1:50 AM, Andrew Jones wrote:
> I'm not sure. Of course I'd need to experiment with it to be sure, but
> I'm reluctant to go through that exercise, because I believe that a
> deferred validation will result in less specific errors messages. For
> example, how would the validator know in which order the sve properties
> were provided? Which is necessary to complain that one length is not
> allowed when another has already been disabled, or vice versa.

My point is that we would not *need* to know in which order the properties are
provided, and do not care.  Indeed, removing this ordering malarkey is a
feature not a bug.

The error message would simply state, e.g., that sve256 may not be disabled
while sve512 is enabled.  The error message does not need to reference the
order in which they appeared.

> Also with deferred validation I think I'd need more vq states, at least
> for when KVM is enabled. For example, if 384-bit vector lengths are not
> supported on the KVM host, but the user does sve384=on, and all we do
> is record that, then we've lost the KVM unsupported information and won't
> find out until we attempt to enable it later at kvm vcpu init time. We'd
> need a kvm-unsupported-user-enabled state to track that, which also means
> we're not blindly recording user input in cpu_arm_set_sve_vq() anymore,
> but are instead applying logic to decide which state we transition to.

Or perhaps, less vq states.  You do not need to compute kvm states early.  You
can wait to collect those until validation time and keep them in their own
local bitmap.

bool validate_sve_properties(...)
{
// Populate uninitialized bits in sve_vq_map from sve_max_vq.
// Populate uninitialized bits in sve_vq_map from on bits in sve_vq_map.
// All remaining uninitialized bits are set to off.
// Reset sve_max_vq to the maximum bit set in sve_vq_map, plus 1.
// Diagnose off bits in sve_vq_map from on bits in sve_vq_map.

if (kvm_enabled()) {
DECLARE_BITMAP(test, ARM_MAX_VQ);
kvm_arm_sve_get_vls(CPU(cpu), test);
if (!bitmap_equal(test, s->sve_vq_map, s->sve_max_vq))  {
bitmap_xor(test, test, s->sve_vq_map, s->sve_max_vq);
// Diagnose the differences in TEST:
// test[i] & s->sve_vq_map[i] -> i is unsupported by kvm
// test[i] & !s->sve_vq_map[i] -> i is required by kvm
}
}
}

If you prefer not to experiment with this, then I will.


r~

Re: [Qemu-devel] [PATCH v1 7/7] targets (various): use softfloat-helpers.h where we can

[Richard Henderson]

On 8/8/19 9:41 AM, Alex Bennée wrote:
> Generally the cpu and non-FP helper files just want to manipulate the
> softfloat flags. For this they can just use the -helpers.h include
> which brings in a minimal number of inline helpers.
> 
> Signed-off-by: Alex Bennée 
> ---
>  target/alpha/helper.c   | 2 +-
>  target/microblaze/cpu.c | 2 +-
>  target/s390x/cpu.c  | 2 +-
>  target/sh4/cpu.c| 3 +--
>  target/tricore/helper.c | 2 +-
>  target/unicore32/cpu.c  | 1 -
>  6 files changed, 5 insertions(+), 7 deletions(-)

Reviewed-by: Richard Henderson 


r~

Re: [Qemu-devel] [PATCH v1 0/7] softfloat header cleanups

[no-reply]

Patchew URL: 
https://patchew.org/QEMU/20190808164117.23348-1-alex.ben...@linaro.org/



Hi,

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

Type: series
Subject: [Qemu-devel] [PATCH  v1 0/7] softfloat header cleanups
Message-id: 20190808164117.23348-1-alex.ben...@linaro.org

=== TEST SCRIPT BEGIN ===
#!/bin/bash
git rev-parse base > /dev/null || exit 0
git config --local diff.renamelimit 0
git config --local diff.renames True
git config --local diff.algorithm histogram
./scripts/checkpatch.pl --mailback base..
=== TEST SCRIPT END ===

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
From https://github.com/patchew-project/qemu
 - [tag update]  patchew/20190808164117.23348-1-alex.ben...@linaro.org -> 
patchew/20190808164117.23348-1-alex.ben...@linaro.org
Submodule 'capstone' (https://git.qemu.org/git/capstone.git) registered for 
path 'capstone'
Submodule 'dtc' (https://git.qemu.org/git/dtc.git) registered for path 'dtc'
Submodule 'roms/QemuMacDrivers' (https://git.qemu.org/git/QemuMacDrivers.git) 
registered for path 'roms/QemuMacDrivers'
Submodule 'roms/SLOF' (https://git.qemu.org/git/SLOF.git) registered for path 
'roms/SLOF'
Submodule 'roms/edk2' (https://git.qemu.org/git/edk2.git) registered for path 
'roms/edk2'
Submodule 'roms/ipxe' (https://git.qemu.org/git/ipxe.git) registered for path 
'roms/ipxe'
Submodule 'roms/openbios' (https://git.qemu.org/git/openbios.git) registered 
for path 'roms/openbios'
Submodule 'roms/openhackware' (https://git.qemu.org/git/openhackware.git) 
registered for path 'roms/openhackware'
Submodule 'roms/opensbi' (https://git.qemu.org/git/opensbi.git) registered for 
path 'roms/opensbi'
Submodule 'roms/qemu-palcode' (https://git.qemu.org/git/qemu-palcode.git) 
registered for path 'roms/qemu-palcode'
Submodule 'roms/seabios' (https://git.qemu.org/git/seabios.git/) registered for 
path 'roms/seabios'
Submodule 'roms/seabios-hppa' (https://git.qemu.org/git/seabios-hppa.git) 
registered for path 'roms/seabios-hppa'
Submodule 'roms/sgabios' (https://git.qemu.org/git/sgabios.git) registered for 
path 'roms/sgabios'
Submodule 'roms/skiboot' (https://git.qemu.org/git/skiboot.git) registered for 
path 'roms/skiboot'
Submodule 'roms/u-boot' (https://git.qemu.org/git/u-boot.git) registered for 
path 'roms/u-boot'
Submodule 'roms/u-boot-sam460ex' (https://git.qemu.org/git/u-boot-sam460ex.git) 
registered for path 'roms/u-boot-sam460ex'
Submodule 'slirp' (https://git.qemu.org/git/libslirp.git) registered for path 
'slirp'
Submodule 'tests/fp/berkeley-softfloat-3' 
(https://git.qemu.org/git/berkeley-softfloat-3.git) registered for path 
'tests/fp/berkeley-softfloat-3'
Submodule 'tests/fp/berkeley-testfloat-3' 
(https://git.qemu.org/git/berkeley-testfloat-3.git) registered for path 
'tests/fp/berkeley-testfloat-3'
Submodule 'ui/keycodemapdb' (https://git.qemu.org/git/keycodemapdb.git) 
registered for path 'ui/keycodemapdb'
Cloning into 'capstone'...
Submodule path 'capstone': checked out 
'22ead3e0bfdb87516656453336160e0a37b066bf'
Cloning into 'dtc'...
Submodule path 'dtc': checked out '88f18909db731a627456f26d779445f84e449536'
Cloning into 'roms/QemuMacDrivers'...
Submodule path 'roms/QemuMacDrivers': checked out 
'90c488d5f4a407342247b9ea869df1c2d9c8e266'
Cloning into 'roms/SLOF'...
Submodule path 'roms/SLOF': checked out 
'ba1ab360eebe6338bb8d7d83a9220ccf7e213af3'
Cloning into 'roms/edk2'...
Submodule path 'roms/edk2': checked out 
'20d2e5a125e34fc8501026613a71549b2a1a3e54'
Submodule 'SoftFloat' (https://github.com/ucb-bar/berkeley-softfloat-3.git) 
registered for path 'ArmPkg/Library/ArmSoftFloatLib/berkeley-softfloat-3'
Submodule 'CryptoPkg/Library/OpensslLib/openssl' 
(https://github.com/openssl/openssl) registered for path 
'CryptoPkg/Library/OpensslLib/openssl'
Cloning into 'ArmPkg/Library/ArmSoftFloatLib/berkeley-softfloat-3'...
Submodule path 'roms/edk2/ArmPkg/Library/ArmSoftFloatLib/berkeley-softfloat-3': 
checked out 'b64af41c3276f97f0e181920400ee056b9c88037'
Cloning into 'CryptoPkg/Library/OpensslLib/openssl'...
Submodule path 'roms/edk2/CryptoPkg/Library/OpensslLib/openssl': checked out 
'50eaac9f3337667259de725451f201e784599687'
Submodule 'boringssl' (https://boringssl.googlesource.com/boringssl) registered 
for path 'boringssl'
Submodule 'krb5' (https://github.com/krb5/krb5) registered for path 'krb5'
Submodule 'pyca.cryptography' (https://github.com/pyca/cryptography.git) 
registered for path 'pyca-cryptography'
Cloning into 'boringssl'...
Submodule path 'roms/edk2/CryptoPkg/Library/OpensslLib/openssl/boringssl': 
checked out '2070f8ad9151dc8f3a73bffaa146b5e6937a583f'
Cloning into 'krb5'...
Submodule path 'roms/edk2/CryptoPkg/Library/OpensslLib/openssl/krb5': checked 
out 'b9ad6c49505c96a088326b62a52568e3484f2168'
Cloning into 'pyca-cryptography'...
Submodule path 
'roms/edk2/CryptoPkg/Library/OpensslLib/openssl/pyca-cryptography': checked out 
'09403100de2f6f1cdd0d484dcb8e620f1c335c8f'
Cloning into 'roms/ipxe'...

Re: [Qemu-devel] [PATCH v1 6/7] target/riscv: rationalise softfloat includes

[Richard Henderson]

On 8/8/19 9:41 AM, Alex Bennée wrote:
> We should avoid including the whole of softfloat headers in cpu.h and
> explicitly include it only where we will be calling softfloat
> functions. We can use the -types.h and -helpers.h in cpu.h for the few
> bits that are global.
> 
> Signed-off-by: Alex Bennée 
> ---
>  target/riscv/cpu.c| 1 +
>  target/riscv/cpu.h| 2 +-
>  target/riscv/fpu_helper.c | 1 +
>  3 files changed, 3 insertions(+), 1 deletion(-)

Reviewed-by: Richard Henderson 


r~

Re: [Qemu-devel] [PATCH v1 5/7] target/mips: rationalise softfloat includes

[Richard Henderson]

On 8/8/19 9:41 AM, Alex Bennée wrote:
> index 21c0615e020..f146924623c 100644
> --- a/target/mips/cpu.h
> +++ b/target/mips/cpu.h
> @@ -5,7 +5,8 @@
>  
>  #include "cpu-qom.h"
>  #include "exec/cpu-defs.h"
> -#include "fpu/softfloat.h"
> +#include "fpu/softfloat-types.h"
> +#include "fpu/softfloat-helpers.h"

Do you really need softfloat-helpers.h here?
It appears as if this is only needed by target/mips/internal.h


r~

Re: [Qemu-devel] [PATCH v1 4/7] fpu: rename softfloat-specialize.h -> .inc.c

[Richard Henderson]

On 8/8/19 9:41 AM, Alex Bennée wrote:
> This is not a normal header and should only be included in the main
> softfloat.c file to bring in the various target specific
> specialisations. Indeed as it contains non-inlined C functions it is
> not even a legal header. Rename it to match our included C convention.
> 
> Signed-off-by: Alex Bennée 
> ---
>  fpu/{softfloat-specialize.h => softfloat-specialize.inc.c} | 0
>  fpu/softfloat.c| 2 +-
>  2 files changed, 1 insertion(+), 1 deletion(-)
>  rename fpu/{softfloat-specialize.h => softfloat-specialize.inc.c} (100%)

Reviewed-by: Richard Henderson 


r~

Re: [Qemu-devel] [PATCH v1 3/7] fpu: make softfloat-macros "self-contained"

[Richard Henderson]

On 8/8/19 9:41 AM, Alex Bennée wrote:
> The macros use the "flags" type and to be consistent if anyone just
> needs the macros we should bring in the header we need. There is an
> outstanding TODO to audit the use of "flags" and replace with bool at
> which point this include could be dropped.

Indeed.

Acked-by: Richard Henderson 


r~

Re: [Qemu-devel] [PATCH v3] migration/postcopy: use mis->bh instead of allocating a QEMUBH

[Dr. David Alan Gilbert]

* Wei Yang (richardw.y...@linux.intel.com) wrote:
> On Wed, Aug 07, 2019 at 07:35:34PM +0100, Dr. David Alan Gilbert wrote:
> >* Wei Yang (richardw.y...@linux.intel.com) wrote:
> >> For migration incoming side, it either quit in precopy or postcopy. It
> >> is safe to use the mis->bh for both instead of allocating a dedicated
> >> QEMUBH for postcopy.
> >> 
> >> Signed-off-by: Wei Yang 
> >> Reviewed-by: Dr. David Alan Gilbert 
> >
> >Hi Wei,
> >  Can you check this, the patchew tests came back with a failure which
> >seems bh related; I've not tried it, but can you just see if you can
> >reproduce it?
> >
> 
> I tried make check, which looks good.
> 
> (Some other upstream commit introduced error. I revert them to make check 
> work)

OK, I'll pick it up and see how we go.

Dave

> 
> >Dave
> >
> 
> -- 
> Wei Yang
> Help you, Help me
--
Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK

Re: [Qemu-devel] [PATCH v1 0/7] softfloat header cleanups

[no-reply]

Patchew URL: 
https://patchew.org/QEMU/20190808164117.23348-1-alex.ben...@linaro.org/



Hi,

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

Type: series
Subject: [Qemu-devel] [PATCH  v1 0/7] softfloat header cleanups
Message-id: 20190808164117.23348-1-alex.ben...@linaro.org

=== TEST SCRIPT BEGIN ===
#!/bin/bash
git rev-parse base > /dev/null || exit 0
git config --local diff.renamelimit 0
git config --local diff.renames True
git config --local diff.algorithm histogram
./scripts/checkpatch.pl --mailback base..
=== TEST SCRIPT END ===

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
From https://github.com/patchew-project/qemu
 - [tag update]  patchew/20190808164117.23348-1-alex.ben...@linaro.org -> 
patchew/20190808164117.23348-1-alex.ben...@linaro.org
Submodule 'capstone' (https://git.qemu.org/git/capstone.git) registered for 
path 'capstone'
Submodule 'dtc' (https://git.qemu.org/git/dtc.git) registered for path 'dtc'
Submodule 'roms/QemuMacDrivers' (https://git.qemu.org/git/QemuMacDrivers.git) 
registered for path 'roms/QemuMacDrivers'
Submodule 'roms/SLOF' (https://git.qemu.org/git/SLOF.git) registered for path 
'roms/SLOF'
Submodule 'roms/edk2' (https://git.qemu.org/git/edk2.git) registered for path 
'roms/edk2'
Submodule 'roms/ipxe' (https://git.qemu.org/git/ipxe.git) registered for path 
'roms/ipxe'
Submodule 'roms/openbios' (https://git.qemu.org/git/openbios.git) registered 
for path 'roms/openbios'
Submodule 'roms/openhackware' (https://git.qemu.org/git/openhackware.git) 
registered for path 'roms/openhackware'
Submodule 'roms/opensbi' (https://git.qemu.org/git/opensbi.git) registered for 
path 'roms/opensbi'
Submodule 'roms/qemu-palcode' (https://git.qemu.org/git/qemu-palcode.git) 
registered for path 'roms/qemu-palcode'
Submodule 'roms/seabios' (https://git.qemu.org/git/seabios.git/) registered for 
path 'roms/seabios'
Submodule 'roms/seabios-hppa' (https://git.qemu.org/git/seabios-hppa.git) 
registered for path 'roms/seabios-hppa'
Submodule 'roms/sgabios' (https://git.qemu.org/git/sgabios.git) registered for 
path 'roms/sgabios'
Submodule 'roms/skiboot' (https://git.qemu.org/git/skiboot.git) registered for 
path 'roms/skiboot'
Submodule 'roms/u-boot' (https://git.qemu.org/git/u-boot.git) registered for 
path 'roms/u-boot'
Submodule 'roms/u-boot-sam460ex' (https://git.qemu.org/git/u-boot-sam460ex.git) 
registered for path 'roms/u-boot-sam460ex'
Submodule 'slirp' (https://git.qemu.org/git/libslirp.git) registered for path 
'slirp'
Submodule 'tests/fp/berkeley-softfloat-3' 
(https://git.qemu.org/git/berkeley-softfloat-3.git) registered for path 
'tests/fp/berkeley-softfloat-3'
Submodule 'tests/fp/berkeley-testfloat-3' 
(https://git.qemu.org/git/berkeley-testfloat-3.git) registered for path 
'tests/fp/berkeley-testfloat-3'
Submodule 'ui/keycodemapdb' (https://git.qemu.org/git/keycodemapdb.git) 
registered for path 'ui/keycodemapdb'
Cloning into 'capstone'...
Submodule path 'capstone': checked out 
'22ead3e0bfdb87516656453336160e0a37b066bf'
Cloning into 'dtc'...
Submodule path 'dtc': checked out '88f18909db731a627456f26d779445f84e449536'
Cloning into 'roms/QemuMacDrivers'...
Submodule path 'roms/QemuMacDrivers': checked out 
'90c488d5f4a407342247b9ea869df1c2d9c8e266'
Cloning into 'roms/SLOF'...
Submodule path 'roms/SLOF': checked out 
'ba1ab360eebe6338bb8d7d83a9220ccf7e213af3'
Cloning into 'roms/edk2'...
Submodule path 'roms/edk2': checked out 
'20d2e5a125e34fc8501026613a71549b2a1a3e54'
Submodule 'SoftFloat' (https://github.com/ucb-bar/berkeley-softfloat-3.git) 
registered for path 'ArmPkg/Library/ArmSoftFloatLib/berkeley-softfloat-3'
Submodule 'CryptoPkg/Library/OpensslLib/openssl' 
(https://github.com/openssl/openssl) registered for path 
'CryptoPkg/Library/OpensslLib/openssl'
Cloning into 'ArmPkg/Library/ArmSoftFloatLib/berkeley-softfloat-3'...
Submodule path 'roms/edk2/ArmPkg/Library/ArmSoftFloatLib/berkeley-softfloat-3': 
checked out 'b64af41c3276f97f0e181920400ee056b9c88037'
Cloning into 'CryptoPkg/Library/OpensslLib/openssl'...
Submodule path 'roms/edk2/CryptoPkg/Library/OpensslLib/openssl': checked out 
'50eaac9f3337667259de725451f201e784599687'
Submodule 'boringssl' (https://boringssl.googlesource.com/boringssl) registered 
for path 'boringssl'
Submodule 'krb5' (https://github.com/krb5/krb5) registered for path 'krb5'
Submodule 'pyca.cryptography' (https://github.com/pyca/cryptography.git) 
registered for path 'pyca-cryptography'
Cloning into 'boringssl'...
Submodule path 'roms/edk2/CryptoPkg/Library/OpensslLib/openssl/boringssl': 
checked out '2070f8ad9151dc8f3a73bffaa146b5e6937a583f'
Cloning into 'krb5'...
Submodule path 'roms/edk2/CryptoPkg/Library/OpensslLib/openssl/krb5': checked 
out 'b9ad6c49505c96a088326b62a52568e3484f2168'
Cloning into 'pyca-cryptography'...
Submodule path 
'roms/edk2/CryptoPkg/Library/OpensslLib/openssl/pyca-cryptography': checked out 
'09403100de2f6f1cdd0d484dcb8e620f1c335c8f'
Cloning into 'roms/ipxe'...

Re: [Qemu-devel] [PATCH v2] migration: rename migration_bitmap_sync_range to ramblock_sync_dirty_bitmap

[Dr. David Alan Gilbert]

* Wei Yang (richardw.y...@linux.intel.com) wrote:
> Rename for better understanding of the code.
> 
> Suggested-by: Paolo Bonzini 
> Signed-off-by: Wei Yang 

Reviewed-by: Dr. David Alan Gilbert 

and queued.

> 
> ---
> v2:
>   * rebase on top of "just pass RAMBlock is enough"
> 
> ---
>  migration/ram.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/migration/ram.c b/migration/ram.c
> index eee68a7991..0d12fa8e92 100644
> --- a/migration/ram.c
> +++ b/migration/ram.c
> @@ -1748,7 +1748,7 @@ static inline bool 
> migration_bitmap_clear_dirty(RAMState *rs,
>  }
>  
>  /* Called with RCU critical section */
> -static void migration_bitmap_sync_range(RAMState *rs, RAMBlock *rb)
> +static void ramblock_sync_dirty_bitmap(RAMState *rs, RAMBlock *rb)
>  {
>  rs->migration_dirty_pages +=
>  cpu_physical_memory_sync_dirty_bitmap(rb, 0, rb->used_length,
> @@ -1840,7 +1840,7 @@ static void migration_bitmap_sync(RAMState *rs)
>  qemu_mutex_lock(>bitmap_mutex);
>  rcu_read_lock();
>  RAMBLOCK_FOREACH_NOT_IGNORED(block) {
> -migration_bitmap_sync_range(rs, block);
> +ramblock_sync_dirty_bitmap(rs, block);
>  }
>  ram_counters.remaining = ram_bytes_remaining();
>  rcu_read_unlock();
> @@ -4261,7 +4261,7 @@ static void colo_flush_ram_cache(void)
>  memory_global_dirty_log_sync();
>  rcu_read_lock();
>  RAMBLOCK_FOREACH_NOT_IGNORED(block) {
> -migration_bitmap_sync_range(ram_state, block);
> +ramblock_sync_dirty_bitmap(ram_state, block);
>  }
>  rcu_read_unlock();
>  
> -- 
> 2.17.1
> 
--
Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK

Re: [Qemu-devel] [PATCH v1 2/7] fpu: move inline helpers into a separate header

[Richard Henderson]

On 8/8/19 9:41 AM, Alex Bennée wrote:
> +static inline void set_float_detect_tininess(int val, float_status *status)
> +{
> +status->float_detect_tininess = val;
> +}
> +static inline void set_float_rounding_mode(int val, float_status *status)
> +{
> +status->float_rounding_mode = val;
> +}
> +static inline void set_float_exception_flags(int val, float_status *status)
> +{
> +status->float_exception_flags = val;
> +}

Can you please fix the spacing at the same time?  Otherwise,
Reviewed-by: Richard Henderson 


r~

Re: [Qemu-devel] [PATCH v1 1/7] fpu: move LIT64 helper to softfloat-types

[Richard Henderson]

On 8/8/19 9:41 AM, Alex Bennée wrote:
> This simple pasting helper can be used by those who don't need the
> entire softfloat api. Move it to the smaller types header.
> 
> Signed-off-by: Alex Bennée 
> ---
>  include/fpu/softfloat-types.h | 2 ++
>  include/fpu/softfloat.h   | 2 --
>  2 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/include/fpu/softfloat-types.h b/include/fpu/softfloat-types.h
> index 2aae6a89b19..7e88152dfc4 100644
> --- a/include/fpu/softfloat-types.h
> +++ b/include/fpu/softfloat-types.h
> @@ -80,6 +80,8 @@ this code that are retained.
>  #ifndef SOFTFLOAT_TYPES_H
>  #define SOFTFLOAT_TYPES_H
>  
> +#define LIT64( a ) a##LL

Better would be to replace all uses with {,U}INT64_C from .

But if you prefer this smaller patch for now,
Acked-by: Richard Henderson 


r~

Re: [Qemu-devel] [PATCH v1 0/7] softfloat header cleanups

[no-reply]

Patchew URL: 
https://patchew.org/QEMU/20190808164117.23348-1-alex.ben...@linaro.org/



Hi,

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

Type: series
Subject: [Qemu-devel] [PATCH  v1 0/7] softfloat header cleanups
Message-id: 20190808164117.23348-1-alex.ben...@linaro.org

=== TEST SCRIPT BEGIN ===
#!/bin/bash
git rev-parse base > /dev/null || exit 0
git config --local diff.renamelimit 0
git config --local diff.renames True
git config --local diff.algorithm histogram
./scripts/checkpatch.pl --mailback base..
=== TEST SCRIPT END ===

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
From https://github.com/patchew-project/qemu
 - [tag update]  patchew/20190805031240.6024-1-alx...@bu.edu -> 
patchew/20190805031240.6024-1-alx...@bu.edu
 * [new tag] patchew/20190808164117.23348-1-alex.ben...@linaro.org -> 
patchew/20190808164117.23348-1-alex.ben...@linaro.org
Submodule 'capstone' (https://git.qemu.org/git/capstone.git) registered for 
path 'capstone'
Submodule 'dtc' (https://git.qemu.org/git/dtc.git) registered for path 'dtc'
Submodule 'roms/QemuMacDrivers' (https://git.qemu.org/git/QemuMacDrivers.git) 
registered for path 'roms/QemuMacDrivers'
Submodule 'roms/SLOF' (https://git.qemu.org/git/SLOF.git) registered for path 
'roms/SLOF'
Submodule 'roms/edk2' (https://git.qemu.org/git/edk2.git) registered for path 
'roms/edk2'
Submodule 'roms/ipxe' (https://git.qemu.org/git/ipxe.git) registered for path 
'roms/ipxe'
Submodule 'roms/openbios' (https://git.qemu.org/git/openbios.git) registered 
for path 'roms/openbios'
Submodule 'roms/openhackware' (https://git.qemu.org/git/openhackware.git) 
registered for path 'roms/openhackware'
Submodule 'roms/opensbi' (https://git.qemu.org/git/opensbi.git) registered for 
path 'roms/opensbi'
Submodule 'roms/qemu-palcode' (https://git.qemu.org/git/qemu-palcode.git) 
registered for path 'roms/qemu-palcode'
Submodule 'roms/seabios' (https://git.qemu.org/git/seabios.git/) registered for 
path 'roms/seabios'
Submodule 'roms/seabios-hppa' (https://git.qemu.org/git/seabios-hppa.git) 
registered for path 'roms/seabios-hppa'
Submodule 'roms/sgabios' (https://git.qemu.org/git/sgabios.git) registered for 
path 'roms/sgabios'
Submodule 'roms/skiboot' (https://git.qemu.org/git/skiboot.git) registered for 
path 'roms/skiboot'
Submodule 'roms/u-boot' (https://git.qemu.org/git/u-boot.git) registered for 
path 'roms/u-boot'
Submodule 'roms/u-boot-sam460ex' (https://git.qemu.org/git/u-boot-sam460ex.git) 
registered for path 'roms/u-boot-sam460ex'
Submodule 'slirp' (https://git.qemu.org/git/libslirp.git) registered for path 
'slirp'
Submodule 'tests/fp/berkeley-softfloat-3' 
(https://git.qemu.org/git/berkeley-softfloat-3.git) registered for path 
'tests/fp/berkeley-softfloat-3'
Submodule 'tests/fp/berkeley-testfloat-3' 
(https://git.qemu.org/git/berkeley-testfloat-3.git) registered for path 
'tests/fp/berkeley-testfloat-3'
Submodule 'ui/keycodemapdb' (https://git.qemu.org/git/keycodemapdb.git) 
registered for path 'ui/keycodemapdb'
Cloning into 'capstone'...
Submodule path 'capstone': checked out 
'22ead3e0bfdb87516656453336160e0a37b066bf'
Cloning into 'dtc'...
Submodule path 'dtc': checked out '88f18909db731a627456f26d779445f84e449536'
Cloning into 'roms/QemuMacDrivers'...
Submodule path 'roms/QemuMacDrivers': checked out 
'90c488d5f4a407342247b9ea869df1c2d9c8e266'
Cloning into 'roms/SLOF'...
Submodule path 'roms/SLOF': checked out 
'ba1ab360eebe6338bb8d7d83a9220ccf7e213af3'
Cloning into 'roms/edk2'...
Submodule path 'roms/edk2': checked out 
'20d2e5a125e34fc8501026613a71549b2a1a3e54'
Submodule 'SoftFloat' (https://github.com/ucb-bar/berkeley-softfloat-3.git) 
registered for path 'ArmPkg/Library/ArmSoftFloatLib/berkeley-softfloat-3'
Submodule 'CryptoPkg/Library/OpensslLib/openssl' 
(https://github.com/openssl/openssl) registered for path 
'CryptoPkg/Library/OpensslLib/openssl'
Cloning into 'ArmPkg/Library/ArmSoftFloatLib/berkeley-softfloat-3'...
Submodule path 'roms/edk2/ArmPkg/Library/ArmSoftFloatLib/berkeley-softfloat-3': 
checked out 'b64af41c3276f97f0e181920400ee056b9c88037'
Cloning into 'CryptoPkg/Library/OpensslLib/openssl'...
Submodule path 'roms/edk2/CryptoPkg/Library/OpensslLib/openssl': checked out 
'50eaac9f3337667259de725451f201e784599687'
Submodule 'boringssl' (https://boringssl.googlesource.com/boringssl) registered 
for path 'boringssl'
Submodule 'krb5' (https://github.com/krb5/krb5) registered for path 'krb5'
Submodule 'pyca.cryptography' (https://github.com/pyca/cryptography.git) 
registered for path 'pyca-cryptography'
Cloning into 'boringssl'...
Submodule path 'roms/edk2/CryptoPkg/Library/OpensslLib/openssl/boringssl': 
checked out '2070f8ad9151dc8f3a73bffaa146b5e6937a583f'
Cloning into 'krb5'...
Submodule path 'roms/edk2/CryptoPkg/Library/OpensslLib/openssl/krb5': checked 
out 'b9ad6c49505c96a088326b62a52568e3484f2168'
Cloning into 'pyca-cryptography'...
Submodule path 

[Qemu-devel] [PATCH v1 5/7] target/mips: rationalise softfloat includes

[Alex Bennée]

We should avoid including the whole of softfloat headers in cpu.h and
explicitly include it only where we will be calling softfloat
functions. We can use the -types.h and -helpers.h in cpu.h for the few
bits that are global.

Signed-off-by: Alex Bennée 
---
 target/mips/cpu.h| 3 ++-
 target/mips/msa_helper.c | 1 +
 target/mips/op_helper.c  | 1 +
 3 files changed, 4 insertions(+), 1 deletion(-)

diff --git a/target/mips/cpu.h b/target/mips/cpu.h
index 21c0615e020..f146924623c 100644
--- a/target/mips/cpu.h
+++ b/target/mips/cpu.h
@@ -5,7 +5,8 @@
 
 #include "cpu-qom.h"
 #include "exec/cpu-defs.h"
-#include "fpu/softfloat.h"
+#include "fpu/softfloat-types.h"
+#include "fpu/softfloat-helpers.h"
 #include "mips-defs.h"
 
 #define TCG_GUEST_DEFAULT_MO (0)
diff --git a/target/mips/msa_helper.c b/target/mips/msa_helper.c
index a5a86572b4a..f24061e2af7 100644
--- a/target/mips/msa_helper.c
+++ b/target/mips/msa_helper.c
@@ -22,6 +22,7 @@
 #include "internal.h"
 #include "exec/exec-all.h"
 #include "exec/helper-proto.h"
+#include "fpu/softfloat.h"
 
 /* Data format min and max values */
 #define DF_BITS(df) (1 << ((df) + 3))
diff --git a/target/mips/op_helper.c b/target/mips/op_helper.c
index 9e2e02f8586..f88a3ab9043 100644
--- a/target/mips/op_helper.c
+++ b/target/mips/op_helper.c
@@ -25,6 +25,7 @@
 #include "exec/exec-all.h"
 #include "exec/cpu_ldst.h"
 #include "sysemu/kvm.h"
+#include "fpu/softfloat.h"
 
 /*/
 /* Exceptions processing helpers */
-- 
2.20.1

[Qemu-devel] [PATCH v1 7/7] targets (various): use softfloat-helpers.h where we can

[Alex Bennée]

Generally the cpu and non-FP helper files just want to manipulate the
softfloat flags. For this they can just use the -helpers.h include
which brings in a minimal number of inline helpers.

Signed-off-by: Alex Bennée 
---
 target/alpha/helper.c   | 2 +-
 target/microblaze/cpu.c | 2 +-
 target/s390x/cpu.c  | 2 +-
 target/sh4/cpu.c| 3 +--
 target/tricore/helper.c | 2 +-
 target/unicore32/cpu.c  | 1 -
 6 files changed, 5 insertions(+), 7 deletions(-)

diff --git a/target/alpha/helper.c b/target/alpha/helper.c
index 93b8e788b18..c6998348df4 100644
--- a/target/alpha/helper.c
+++ b/target/alpha/helper.c
@@ -21,7 +21,7 @@
 
 #include "cpu.h"
 #include "exec/exec-all.h"
-#include "fpu/softfloat.h"
+#include "fpu/softfloat-types.h"
 #include "exec/helper-proto.h"
 #include "qemu/qemu-print.h"
 
diff --git a/target/microblaze/cpu.c b/target/microblaze/cpu.c
index 0bec54b2f8a..9cfd7445e7d 100644
--- a/target/microblaze/cpu.c
+++ b/target/microblaze/cpu.c
@@ -28,7 +28,7 @@
 #include "hw/qdev-properties.h"
 #include "migration/vmstate.h"
 #include "exec/exec-all.h"
-#include "fpu/softfloat.h"
+#include "fpu/softfloat-helpers.h"
 
 static const struct {
 const char *name;
diff --git a/target/s390x/cpu.c b/target/s390x/cpu.c
index 736a7903e22..5db016672bb 100644
--- a/target/s390x/cpu.c
+++ b/target/s390x/cpu.c
@@ -42,7 +42,7 @@
 #include "sysemu/sysemu.h"
 #include "sysemu/tcg.h"
 #endif
-#include "fpu/softfloat.h"
+#include "fpu/softfloat-helpers.h"
 
 #define CR0_RESET   0xE0UL
 #define CR14_RESET  0xC200UL;
diff --git a/target/sh4/cpu.c b/target/sh4/cpu.c
index 816d6d7f311..d0a7707991f 100644
--- a/target/sh4/cpu.c
+++ b/target/sh4/cpu.c
@@ -25,8 +25,7 @@
 #include "cpu.h"
 #include "migration/vmstate.h"
 #include "exec/exec-all.h"
-#include "fpu/softfloat.h"
-
+#include "fpu/softfloat-helpers.h"
 
 static void superh_cpu_set_pc(CPUState *cs, vaddr value)
 {
diff --git a/target/tricore/helper.c b/target/tricore/helper.c
index a6803368506..d5db7b2c03f 100644
--- a/target/tricore/helper.c
+++ b/target/tricore/helper.c
@@ -19,7 +19,7 @@
 
 #include "cpu.h"
 #include "exec/exec-all.h"
-#include "fpu/softfloat.h"
+#include "fpu/softfloat-helpers.h"
 #include "qemu/qemu-print.h"
 
 enum {
diff --git a/target/unicore32/cpu.c b/target/unicore32/cpu.c
index 802e2f1eba5..b27fb9689ff 100644
--- a/target/unicore32/cpu.c
+++ b/target/unicore32/cpu.c
@@ -17,7 +17,6 @@
 #include "cpu.h"
 #include "migration/vmstate.h"
 #include "exec/exec-all.h"
-#include "fpu/softfloat.h"
 
 static void uc32_cpu_set_pc(CPUState *cs, vaddr value)
 {
-- 
2.20.1

[Qemu-devel] [PATCH v1 4/7] fpu: rename softfloat-specialize.h -> .inc.c

[Alex Bennée]

This is not a normal header and should only be included in the main
softfloat.c file to bring in the various target specific
specialisations. Indeed as it contains non-inlined C functions it is
not even a legal header. Rename it to match our included C convention.

Signed-off-by: Alex Bennée 
---
 fpu/{softfloat-specialize.h => softfloat-specialize.inc.c} | 0
 fpu/softfloat.c| 2 +-
 2 files changed, 1 insertion(+), 1 deletion(-)
 rename fpu/{softfloat-specialize.h => softfloat-specialize.inc.c} (100%)

diff --git a/fpu/softfloat-specialize.h b/fpu/softfloat-specialize.inc.c
similarity index 100%
rename from fpu/softfloat-specialize.h
rename to fpu/softfloat-specialize.inc.c
diff --git a/fpu/softfloat.c b/fpu/softfloat.c
index 2ba36ec3703..80a16458304 100644
--- a/fpu/softfloat.c
+++ b/fpu/softfloat.c
@@ -652,7 +652,7 @@ static inline float64 float64_pack_raw(FloatParts p)
 | are propagated from function inputs to output.  These details are target-
 | specific.
 **/
-#include "softfloat-specialize.h"
+#include "softfloat-specialize.inc.c"
 
 /* Canonicalize EXP and FRAC, setting CLS.  */
 static FloatParts sf_canonicalize(FloatParts part, const FloatFmt *parm,
-- 
2.20.1

[Qemu-devel] [PATCH v1 6/7] target/riscv: rationalise softfloat includes

[Alex Bennée]

We should avoid including the whole of softfloat headers in cpu.h and
explicitly include it only where we will be calling softfloat
functions. We can use the -types.h and -helpers.h in cpu.h for the few
bits that are global.

Signed-off-by: Alex Bennée 
---
 target/riscv/cpu.c| 1 +
 target/riscv/cpu.h| 2 +-
 target/riscv/fpu_helper.c | 1 +
 3 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index f8d07bd20ad..6d52f97d7c3 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -27,6 +27,7 @@
 #include "qemu/error-report.h"
 #include "hw/qdev-properties.h"
 #include "migration/vmstate.h"
+#include "fpu/softfloat-helpers.h"
 
 /* RISC-V CPU definitions */
 
diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
index 0adb307f329..240b31e2ebb 100644
--- a/target/riscv/cpu.h
+++ b/target/riscv/cpu.h
@@ -22,7 +22,7 @@
 
 #include "qom/cpu.h"
 #include "exec/cpu-defs.h"
-#include "fpu/softfloat.h"
+#include "fpu/softfloat-types.h"
 
 #define TCG_GUEST_DEFAULT_MO 0
 
diff --git a/target/riscv/fpu_helper.c b/target/riscv/fpu_helper.c
index b4f818a6465..0b79562a690 100644
--- a/target/riscv/fpu_helper.c
+++ b/target/riscv/fpu_helper.c
@@ -21,6 +21,7 @@
 #include "qemu/host-utils.h"
 #include "exec/exec-all.h"
 #include "exec/helper-proto.h"
+#include "fpu/softfloat.h"
 
 target_ulong riscv_cpu_get_fflags(CPURISCVState *env)
 {
-- 
2.20.1

[Qemu-devel] [PATCH v1 2/7] fpu: move inline helpers into a separate header

[Alex Bennée]

There are a bunch of users of the inline helpers who do not need
access to the entire softfloat API. Move those inline helpers into a
new header file which can be included without bringing in the rest of
the world.

Signed-off-by: Alex Bennée 
---
 include/fpu/softfloat-helpers.h | 118 
 include/fpu/softfloat.h |  63 +
 2 files changed, 119 insertions(+), 62 deletions(-)
 create mode 100644 include/fpu/softfloat-helpers.h

diff --git a/include/fpu/softfloat-helpers.h b/include/fpu/softfloat-helpers.h
new file mode 100644
index 000..de90663d384
--- /dev/null
+++ b/include/fpu/softfloat-helpers.h
@@ -0,0 +1,118 @@
+/*
+ * QEMU float support - standalone helpers
+ *
+ * This is provided for files that don't need the access to the full
+ * set of softfloat functions. Typically this is cpu initialisation
+ * code which wants to set default rounding and exceptions modes.
+ *
+ * The code in this source file is derived from release 2a of the SoftFloat
+ * IEC/IEEE Floating-point Arithmetic Package. Those parts of the code (and
+ * some later contributions) are provided under that license, as detailed 
below.
+ * It has subsequently been modified by contributors to the QEMU Project,
+ * so some portions are provided under:
+ *  the SoftFloat-2a license
+ *  the BSD license
+ *  GPL-v2-or-later
+ *
+ * Any future contributions to this file after December 1st 2014 will be
+ * taken to be licensed under the Softfloat-2a license unless specifically
+ * indicated otherwise.
+ */
+
+/*
+===
+This C header file is part of the SoftFloat IEC/IEEE Floating-point
+Arithmetic Package, Release 2a.
+
+Written by John R. Hauser.  This work was made possible in part by the
+International Computer Science Institute, located at Suite 600, 1947 Center
+Street, Berkeley, California 94704.  Funding was partially provided by the
+National Science Foundation under grant MIP-9311980.  The original version
+of this code was written as part of a project to build a fixed-point vector
+processor in collaboration with the University of California at Berkeley,
+overseen by Profs. Nelson Morgan and John Wawrzynek.  More information
+is available through the Web page `http://HTTP.CS.Berkeley.EDU/~jhauser/
+arithmetic/SoftFloat.html'.
+
+THIS SOFTWARE IS DISTRIBUTED AS IS, FOR FREE.  Although reasonable effort
+has been made to avoid it, THIS SOFTWARE MAY CONTAIN FAULTS THAT WILL AT
+TIMES RESULT IN INCORRECT BEHAVIOR.  USE OF THIS SOFTWARE IS RESTRICTED TO
+PERSONS AND ORGANIZATIONS WHO CAN AND WILL TAKE FULL RESPONSIBILITY FOR ANY
+AND ALL LOSSES, COSTS, OR OTHER PROBLEMS ARISING FROM ITS USE.
+
+Derivative works are acceptable, even for commercial purposes, so long as
+(1) they include prominent notice that the work is derivative, and (2) they
+include prominent notice akin to these four paragraphs for those parts of
+this code that are retained.
+
+===
+*/
+
+#ifndef _SOFTFLOAT_HELPERS_H_
+#define _SOFTFLOAT_HELPERS_H_
+
+#include "fpu/softfloat-types.h"
+
+static inline void set_float_detect_tininess(int val, float_status *status)
+{
+status->float_detect_tininess = val;
+}
+static inline void set_float_rounding_mode(int val, float_status *status)
+{
+status->float_rounding_mode = val;
+}
+static inline void set_float_exception_flags(int val, float_status *status)
+{
+status->float_exception_flags = val;
+}
+static inline void set_floatx80_rounding_precision(int val,
+   float_status *status)
+{
+status->floatx80_rounding_precision = val;
+}
+static inline void set_flush_to_zero(flag val, float_status *status)
+{
+status->flush_to_zero = val;
+}
+static inline void set_flush_inputs_to_zero(flag val, float_status *status)
+{
+status->flush_inputs_to_zero = val;
+}
+static inline void set_default_nan_mode(flag val, float_status *status)
+{
+status->default_nan_mode = val;
+}
+static inline void set_snan_bit_is_one(flag val, float_status *status)
+{
+status->snan_bit_is_one = val;
+}
+static inline int get_float_detect_tininess(float_status *status)
+{
+return status->float_detect_tininess;
+}
+static inline int get_float_rounding_mode(float_status *status)
+{
+return status->float_rounding_mode;
+}
+static inline int get_float_exception_flags(float_status *status)
+{
+return status->float_exception_flags;
+}
+static inline int get_floatx80_rounding_precision(float_status *status)
+{
+return status->floatx80_rounding_precision;
+}
+static inline flag get_flush_to_zero(float_status *status)
+{
+return status->flush_to_zero;
+}
+static inline flag get_flush_inputs_to_zero(float_status *status)
+{
+return status->flush_inputs_to_zero;
+}
+static inline flag get_default_nan_mode(float_status *status)
+{
+return 

[Qemu-devel] [PATCH v1 0/7] softfloat header cleanups

[Alex Bennée]

Hi Markus,

As promised here is a softfloat specific follow-up to your headers
clean-up series:

  From: Markus Armbruster 
  Date: Tue,  6 Aug 2019 17:14:06 +0200
  Message-Id: <20190806151435.10740-1-arm...@redhat.com>
  Subject: [Qemu-devel] [PATCH v2 00/29] Tame a few "touch this, recompile the 
world" headers

The first few patches do a little light re-organising of the header
files and some renaming. The remaining patches then rationalise the
header usage in the targets mostly by removing the inclusion of
softfloat.h from cpu.h which is most likely to trigger the largest
number of rebuilds.

I'm happy for you to pull these straight into your larger series if
you want otherwise I'll collect tags and submit once the tree
re-opens.

Alex Bennée (7):
  fpu: move LIT64 helper to softfloat-types
  fpu: move inline helpers into a separate header
  fpu: make softfloat-macros "self-contained"
  fpu: rename softfloat-specialize.h -> .inc.c
  target/mips: rationalise softfloat includes
  target/riscv: rationalise softfloat includes
  targets (various): use softfloat-helpers.h where we can

 ...pecialize.h => softfloat-specialize.inc.c} |   0
 fpu/softfloat.c   |   2 +-
 include/fpu/softfloat-helpers.h   | 118 ++
 include/fpu/softfloat-macros.h|   2 +
 include/fpu/softfloat-types.h |   2 +
 include/fpu/softfloat.h   |  65 +-
 target/alpha/helper.c |   2 +-
 target/microblaze/cpu.c   |   2 +-
 target/mips/cpu.h |   3 +-
 target/mips/msa_helper.c  |   1 +
 target/mips/op_helper.c   |   1 +
 target/riscv/cpu.c|   1 +
 target/riscv/cpu.h|   2 +-
 target/riscv/fpu_helper.c |   1 +
 target/s390x/cpu.c|   2 +-
 target/sh4/cpu.c  |   3 +-
 target/tricore/helper.c   |   2 +-
 target/unicore32/cpu.c|   1 -
 18 files changed, 136 insertions(+), 74 deletions(-)
 rename fpu/{softfloat-specialize.h => softfloat-specialize.inc.c} (100%)
 create mode 100644 include/fpu/softfloat-helpers.h

-- 
2.20.1

[Qemu-devel] [PATCH v1 3/7] fpu: make softfloat-macros "self-contained"

[Alex Bennée]

The macros use the "flags" type and to be consistent if anyone just
needs the macros we should bring in the header we need. There is an
outstanding TODO to audit the use of "flags" and replace with bool at
which point this include could be dropped.

Signed-off-by: Alex Bennée 
---
 include/fpu/softfloat-macros.h | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/include/fpu/softfloat-macros.h b/include/fpu/softfloat-macros.h
index c55aa6d1742..e698bca4e1d 100644
--- a/include/fpu/softfloat-macros.h
+++ b/include/fpu/softfloat-macros.h
@@ -82,6 +82,8 @@ this code that are retained.
 #ifndef FPU_SOFTFLOAT_MACROS_H
 #define FPU_SOFTFLOAT_MACROS_H
 
+#include "fpu/softfloat-types.h"
+
 /*
 | Shifts `a' right by the number of bits given in `count'.  If any nonzero
 | bits are shifted off, they are ``jammed'' into the least significant bit of
-- 
2.20.1

[Qemu-devel] [PATCH v1 1/7] fpu: move LIT64 helper to softfloat-types

[Alex Bennée]

This simple pasting helper can be used by those who don't need the
entire softfloat api. Move it to the smaller types header.

Signed-off-by: Alex Bennée 
---
 include/fpu/softfloat-types.h | 2 ++
 include/fpu/softfloat.h   | 2 --
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/include/fpu/softfloat-types.h b/include/fpu/softfloat-types.h
index 2aae6a89b19..7e88152dfc4 100644
--- a/include/fpu/softfloat-types.h
+++ b/include/fpu/softfloat-types.h
@@ -80,6 +80,8 @@ this code that are retained.
 #ifndef SOFTFLOAT_TYPES_H
 #define SOFTFLOAT_TYPES_H
 
+#define LIT64( a ) a##LL
+
 /* This 'flag' type must be able to hold at least 0 and 1. It should
  * probably be replaced with 'bool' but the uses would need to be audited
  * to check that they weren't accidentally relying on it being a larger type.
diff --git a/include/fpu/softfloat.h b/include/fpu/softfloat.h
index 3ff3fa52245..d9333eb65b8 100644
--- a/include/fpu/softfloat.h
+++ b/include/fpu/softfloat.h
@@ -82,8 +82,6 @@ this code that are retained.
 #ifndef SOFTFLOAT_H
 #define SOFTFLOAT_H
 
-#define LIT64( a ) a##LL
-
 /*
 | Software IEC/IEEE floating-point ordering relations
 **/
-- 
2.20.1

Re: [Qemu-devel] [PATCH 11/67] target/arm: Add stubs for aa32 decodetree

[Richard Henderson]

On 8/8/19 4:41 AM, Aleksandar Markovic wrote:
> +/*
> + * Include the generated decoders.
> + * Note that the T32 decoder reuses some of the trans_* functions
> + * initially declared by the A32 decoder, which results in duplicate
> + * declaration warnings.  Suppress them.
> + */
> +
> +#ifdef CONFIG_PRAGMA_DIAGNOSTIC_AVAILABLE
> +# pragma GCC diagnostic push
> +# pragma GCC diagnostic ignored "-Wredundant-decls"
> +# ifdef __clang__
> +#  pragma GCC diagnostic ignored "-Wtypedef-redefinition"
> +# endif
> +#endif
> +
> 
> 
> This looks more like a "band aid" solution rather than the right one.

What would the "right" solution be, would you say?

A couple of days ago Phil suggested moving these pragmas into the generated
code, so that this need not be done by hand in the several targets that use
multiple decoders.  That sounds reasonable to me.


r~

Re: [Qemu-devel] [PATCH v3 07/33] automatically add vmstate for reset support in devices

[Dr. David Alan Gilbert]

* Peter Maydell (peter.mayd...@linaro.org) wrote:
> On Mon, 29 Jul 2019 at 15:59, Damien Hedde  wrote:
> >
> > This add the reset related sections for every QOM
> > device.
> 
> A bit more detail in the commit message would help, I think --
> this is adding extra machinery which has to copy and modify
> the VMStateDescription passed in by the device in order to
> add the subsection that handles reset.
> 
> I've added Dave Gilbert to the already long cc list since this
> is migration related.

I don't like dynamically modifying all the vmsds.
Aren't you going to have to understand each devices reset behaviour
and make sure it does something sane? e.g. it might have a postload
that registers a timer or something that you wouldn't want to do if it's
in reset.

The easiest way is to write a macro that you can easily add to devices
you've checked subsection list (like the way we have a
VMSTATE_USB_DEVICE).

Dave

> 
> > Signed-off-by: Damien Hedde 
> > ---
> >  hw/core/qdev-vmstate.c | 41 +
> >  hw/core/qdev.c | 12 +++-
> >  include/hw/qdev-core.h |  3 +++
> >  stubs/Makefile.objs|  1 +
> >  stubs/device.c |  7 +++
> >  5 files changed, 63 insertions(+), 1 deletion(-)
> >  create mode 100644 stubs/device.c
> >
> > diff --git a/hw/core/qdev-vmstate.c b/hw/core/qdev-vmstate.c
> > index 07b010811f..24f8465c61 100644
> > --- a/hw/core/qdev-vmstate.c
> > +++ b/hw/core/qdev-vmstate.c
> > @@ -43,3 +43,44 @@ const struct VMStateDescription device_vmstate_reset = {
> >  VMSTATE_END_OF_LIST()
> >  },
> >  };
> > +
> > +static VMStateDescription *vmsd_duplicate_and_append(
> > +const VMStateDescription *old_vmsd,
> > +const VMStateDescription *new_subsection)
> > +{
> > +VMStateDescription *vmsd;
> > +int n = 0;
> > +
> > +assert(old_vmsd && new_subsection);
> > +
> > +vmsd = (VMStateDescription *) g_memdup(old_vmsd, sizeof(*vmsd));
> > +
> > +if (old_vmsd->subsections) {
> > +while (old_vmsd->subsections[n]) {
> > +n += 1;
> > +}
> > +}
> > +vmsd->subsections = g_new(const VMStateDescription *, n + 2);
> > +
> > +if (old_vmsd->subsections) {
> > +memcpy(vmsd->subsections, old_vmsd->subsections,
> > +   sizeof(VMStateDescription *) * n);
> > +}
> > +vmsd->subsections[n] = new_subsection;
> > +vmsd->subsections[n + 1] = NULL;
> > +
> > +return vmsd;
> > +}
> > +
> > +void device_class_build_extended_vmsd(DeviceClass *dc)
> > +{
> > +assert(dc->vmsd);
> > +assert(!dc->vmsd_ext);
> > +
> > +/* forge a subsection with proper name */
> > +VMStateDescription *reset;
> > +reset = g_memdup(_vmstate_reset, sizeof(*reset));
> > +reset->name = g_strdup_printf("%s/device_reset", dc->vmsd->name);
> > +
> > +dc->vmsd_ext = vmsd_duplicate_and_append(dc->vmsd, reset);
> > +}
> 
> This will allocate memory, but there is no corresponding
> code which frees it. This means you'll have a memory leak
> across device realize->unrealize for hotplug devices.
> 
> > diff --git a/hw/core/qdev.c b/hw/core/qdev.c
> > index e9e5f2d5f9..88387d3743 100644
> > --- a/hw/core/qdev.c
> > +++ b/hw/core/qdev.c
> > @@ -45,7 +45,17 @@ bool qdev_hot_removed = false;
> >  const VMStateDescription *qdev_get_vmsd(DeviceState *dev)
> >  {
> >  DeviceClass *dc = DEVICE_GET_CLASS(dev);
> > -return dc->vmsd;
> > +
> > +if (!dc->vmsd) {
> > +return NULL;
> > +}
> > +
> > +if (!dc->vmsd_ext) {
> > +/* build it first time we need it */
> > +device_class_build_extended_vmsd(dc);
> > +}
> > +
> > +return dc->vmsd_ext;
> >  }
> 
> Unfortunately not everything that wants the VMSD calls
> this function. migration/savevm.c:dump_vmstate_json_to_file()
> does a direct access to dc->vmsd, so we need to fix that first.
> 
> Devices which don't use dc->vmsd won't get this and so
> their reset state won't be migrated. That's OK for anything
> that's still not yet a QOM device, I guess -- it's not possible
> for them to be in a 'held in reset' state anyway, so the
> extra subsection would never be needed.
> 
> The one I'm less sure about is the 'virtio' devices, which
> have to do something odd with migration state for backwards
> compat reasons. At the moment they can't be in a situation
> where they're being held in reset when we do a migration,
> but since they're PCI devices they might in future be possible
> to put into new boards/pci controllers that would let them
> be in that situation.
> 
> >  static void bus_remove_child(BusState *bus, DeviceState *child)
> > diff --git a/include/hw/qdev-core.h b/include/hw/qdev-core.h
> > index 1670ae41bb..926d4bbcb1 100644
> > --- a/include/hw/qdev-core.h
> > +++ b/include/hw/qdev-core.h
> > @@ -120,6 +120,7 @@ typedef struct DeviceClass {
> >
> >  /* device state */
> >  const struct VMStateDescription *vmsd;
> > +const struct VMStateDescription 

[Qemu-devel] Does i386-linux-user build on an i686 host?

[Markus Armbruster]

Fails for me, but perhaps I'm doing it wrong:

$ uname -a
Linux gcc45 3.16.0-7-686-pae #1 SMP Debian 3.16.59-1 (2018-10-03) i686 GNU/Linux
$ ../configure --target-list=i386-linux-user
Install prefix/usr/local
BIOS directory/usr/local/share/qemu
firmware path /usr/local/share/qemu-firmware
binary directory  /usr/local/bin
library directory /usr/local/lib
module directory  /usr/local/lib/qemu
libexec directory /usr/local/libexec
include directory /usr/local/include
config directory  /usr/local/etc
local state directory   /usr/local/var
Manual directory  /usr/local/share/man
ELF interp prefix /usr/gnemul/qemu-%M
Source path   /home/armbru/qemu
GIT binarygit
GIT submodulesui/keycodemapdb tests/fp/berkeley-testfloat-3 
tests/fp/berkeley-softfloat-3 dtc capstone slirp
C compilercc
Host C compiler   cc
C++ compiler  c++
Objective-C compiler clang
ARFLAGS   rv
CFLAGS-O2 -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=2 -g 
QEMU_CFLAGS   -I/usr/include/pixman-1  -I$(SRC_PATH)/dtc/libfdt -Werror  
-pthread -I/usr/include/glib-2.0 -I/usr/lib/i386-linux-gnu/glib-2.0/include  
-fPIE -DPIE -m32 -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 -std=gnu99  
-Wendif-labels -Wno-missing-include-dirs -Wempty-body -Wnested-externs 
-Wformat-security -Wformat-y2k -Winit-self -Wignored-qualifiers 
-Wold-style-declaration -Wold-style-definition -Wtype-limits 
-fstack-protector-strong -Wno-missing-braces -I/usr/include/p11-kit-1
-I/usr/include/libpng12  -I$(SRC_PATH)/capstone/include
LDFLAGS   -Wl,--warn-common -Wl,-z,relro -Wl,-z,now -pie -m32 -g 
QEMU_LDFLAGS  -L$(BUILD_DIR)/dtc/libfdt 
make  make
install   install
pythonpython3 -B (3.4.2)
slirp support git 
smbd  /usr/sbin/smbd
module supportno
host CPU  i386
host big endian   no
target list   i386-linux-user
gprof enabled no
sparse enabledno
strip binariesyes
profiler  no
static build  no
SDL support   no 
SDL image support no
GTK support   no 
GTK GL supportno
VTE support   no 
TLS priority  NORMAL
GNUTLS supportyes
libgcrypt no
nettleyes (2.7.1)
libtasn1  yes
PAM   no
iconv support yes
curses supportno
virgl support no 
curl support  yes
mingw32 support   no
Audio drivers pa oss
Block whitelist (rw) 
Block whitelist (ro) 
VirtFS support
Multipath support 
VNC support   yes
VNC SASL support  no
VNC JPEG support  yes
VNC PNG support   yes
xen support   no
brlapi supportno
bluez  supportno
Documentation no
PIE   yes
vde support   no
netmap supportno
Linux AIO support yes
ATTR/XATTR support yes
Install blobs yes
KVM support   yes
HAX support   no
HVF support   no
WHPX support  no
TCG support   yes
TCG debug enabled no
TCG interpreter   no
malloc trim support yes
RDMA support  no
PVRDMA supportno
fdt support   git
membarrierno
preadv supportyes
fdatasync yes
madvise   yes
posix_madvise yes
posix_memalignyes
libcap-ng support no
vhost-net support yes
vhost-crypto support yes
vhost-scsi support yes
vhost-vsock support yes
vhost-user support yes
Trace backendslog
spice support no 
rbd support   no
xfsctl supportno
smartcard support no
libusbyes
usb net redir no
OpenGL supportno
OpenGL dmabufsno
libiscsi support  no
libnfs supportno
build guest agent yes
QGA VSS support   no
QGA w32 disk info no
QGA MSI support   no
seccomp support   no
coroutine backend ucontext
coroutine poolyes
debug stack usage no
mutex debugging   no
crypto afalg  no
GlusterFS support no
gcov  gcov
gcov enabled  no
TPM support   yes
libssh supportno
QOM debugging yes
Live block migration yes
lzo support   no
snappy supportno
bzip2 support yes
lzfse support no
NUMA host support yes
libxml2   yes
tcmalloc support  no
jemalloc support  no
avx2 optimization yes
replication support yes
VxHS block device no
bochs support yes
cloop support yes
dmg support   yes
qcow v1 support   yes
vdi support   yes
vvfat support yes
qed support   yes
parallels support yes
sheepdog support  yes
capstone  git
dockerno
libpmem support   no
libudev   yes
default devices   yes

NOTE: cross-compilers enabled:  'cc'
$ make
  CC  i386-linux-user/linux-user/syscall.o
/home/armbru/qemu/linux-user/ioctls.h:306:9: error: ‘SNDCTL_DSP_MAPINBUF’ 
undeclared here (not in a function)
   IOCTL(SNDCTL_DSP_MAPINBUF, IOC_R, MK_PTR(MK_STRUCT(STRUCT_buffmem_desc)))
 ^
/home/armbru/qemu/linux-user/syscall.c:5023:23: note: in definition of macro 
‘IOCTL’
 { TARGET_ ## cmd, cmd, #cmd, access, 0, {  

Re: [Qemu-devel] [PATCH] qtest: Rename qtest.c:qtest_init()

[Stefan Hajnoczi]

On Mon, Aug 05, 2019 at 03:13:01AM +, Oleinik, Alexander wrote:
> Both the qtest client, libqtest.c, and server, qtest.c, used the same
> name for initialization functions which can cause confusion.
> 
> Signed-off-by: Alexander Oleinik 
> ---
> Thank you, Thomas Huth for the suggestion.
> 
>  include/sysemu/qtest.h | 2 +-
>  qtest.c| 3 +--
>  vl.c   | 2 +-
>  3 files changed, 3 insertions(+), 4 deletions(-)

Reviewed-by: Stefan Hajnoczi 


signature.asc
Description: PGP signature

[Qemu-devel] configure and submodules capstone, slirp, dtc

[Markus Armbruster]

configure --help documents capstone like any other generic optional
feature.  This leaves --enable-capstone=git and =system undocumented
there.  Anyone care to improve this?

It documents slirp unlike other generic optional features, and shows
only --disable-slirp.  Anyone care to improve this?

There's also --enable-dtc, which appears to select the system's dtc if
it deems it usable, else the git submodule.  Should it support explicit
--enable=dtc=git and =system, too?

Re: [Qemu-devel] [PATCH v2 2/2] Add dbus-vmstate object

[Marc-André Lureau]

Hi

On Thu, Aug 8, 2019 at 7:03 PM Marc-André Lureau
 wrote:
>
> When instanciated, this object will connect to the given D-Bus
> bus. During migration, it will take the data from org.qemu.VMState1
> instances.

I forgot to update the commit message.

When instantiated, this object will connect to the given D-Bus peer.
During migration, it will load/save the data from the
org.qemu.VMState1 interface.

See documentation for further details.

>
>
> Signed-off-by: Marc-André Lureau 
> ---
>  MAINTAINERS   |   6 +
>  backends/Makefile.objs|   4 +
>  backends/dbus-vmstate.c   | 332 ++
>  configure |   7 +
>  docs/interop/dbus-vmstate.rst |  63 ++
>  docs/interop/index.rst|   1 +
>  tests/Makefile.include|  18 +-
>  tests/dbus-vmstate-test.c | 371 ++
>  tests/dbus-vmstate1.xml   |  12 ++
>  9 files changed, 813 insertions(+), 1 deletion(-)
>  create mode 100644 backends/dbus-vmstate.c
>  create mode 100644 docs/interop/dbus-vmstate.rst
>  create mode 100644 tests/dbus-vmstate-test.c
>  create mode 100644 tests/dbus-vmstate1.xml
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index d6de200453..d136bf4f4b 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -2143,6 +2143,12 @@ F: tests/migration-test.c
>  F: docs/devel/migration.rst
>  F: qapi/migration.json
>
> +DBus VMState
> +M: Marc-André Lureau 
> +S: Maintained
> +F: backends/dbus-vmstate.c
> +F: tests/dbus-vmstate*
> +
>  Seccomp
>  M: Eduardo Otubo 
>  S: Supported
> diff --git a/backends/Makefile.objs b/backends/Makefile.objs
> index 981e8e122f..dbbe12b225 100644
> --- a/backends/Makefile.objs
> +++ b/backends/Makefile.objs
> @@ -17,3 +17,7 @@ endif
>  common-obj-$(call land,$(CONFIG_VHOST_USER),$(CONFIG_VIRTIO)) += vhost-user.o
>
>  common-obj-$(CONFIG_LINUX) += hostmem-memfd.o
> +
> +common-obj-$(CONFIG_GIO) += dbus-vmstate.o
> +dbus-vmstate.o-cflags = $(GIO_CFLAGS)
> +dbus-vmstate.o-libs = $(GIO_LIBS)
> diff --git a/backends/dbus-vmstate.c b/backends/dbus-vmstate.c
> new file mode 100644
> index 00..a9d9cac388
> --- /dev/null
> +++ b/backends/dbus-vmstate.c
> @@ -0,0 +1,332 @@
> +/*
> + * QEMU dbus-vmstate
> + *
> + * Copyright (C) 2019 Red Hat Inc
> + *
> + * Authors:
> + *  Marc-André Lureau 
> + *
> + * This work is licensed under the terms of the GNU GPL, version 2 or later.
> + * See the COPYING file in the top-level directory.
> + */
> +
> +#include "qemu/osdep.h"
> +#include "qemu/error-report.h"
> +#include "qapi/error.h"
> +#include "qom/object_interfaces.h"
> +#include "qapi/qmp/qerror.h"
> +#include "migration/register.h"
> +#include "migration/qemu-file-types.h"
> +#include 
> +
> +typedef struct DBusVMState DBusVMState;
> +typedef struct DBusVMStateClass DBusVMStateClass;
> +
> +#define TYPE_DBUS_VMSTATE "dbus-vmstate"
> +#define DBUS_VMSTATE(obj)\
> +OBJECT_CHECK(DBusVMState, (obj), TYPE_DBUS_VMSTATE)
> +#define DBUS_VMSTATE_GET_CLASS(obj)  \
> +OBJECT_GET_CLASS(DBusVMStateClass, (obj), TYPE_DBUS_VMSTATE)
> +#define DBUS_VMSTATE_CLASS(klass)\
> +OBJECT_CLASS_CHECK(DBusVMStateClass, (klass), TYPE_DBUS_VMSTATE)
> +
> +struct DBusVMStateClass {
> +ObjectClass parent_class;
> +};
> +
> +struct DBusVMState {
> +Object parent;
> +
> +GDBusConnection *bus;
> +GDBusProxy *proxy;
> +char *id;
> +char *dbus_addr;
> +};
> +
> +static const GDBusPropertyInfo vmstate_property_info[] = {
> +{ -1, (char *) "Id", (char *) "s",
> +  G_DBUS_PROPERTY_INFO_FLAGS_READABLE, NULL },
> +};
> +
> +static const GDBusPropertyInfo * const vmstate_property_info_pointers[] = {
> +_property_info[0],
> +NULL
> +};
> +
> +static const GDBusInterfaceInfo vmstate1_interface_info = {
> +-1,
> +(char *) "org.qemu.VMState1",
> +(GDBusMethodInfo **) NULL,
> +(GDBusSignalInfo **) NULL,
> +(GDBusPropertyInfo **) _property_info_pointers,
> +NULL,
> +};
> +
> +#define DBUS_VMSTATE_SIZE_LIMIT (1 << 20) /* 1mb */
> +
> +
> +static char *
> +get_idstr(DBusVMState *self)
> +{
> +return g_strdup_printf("%s-%s", TYPE_DBUS_VMSTATE, self->id);
> +}
> +
> +static char *
> +dbus_proxy_get_id(GDBusProxy *proxy, GError **err)
> +{
> +char *id = NULL;
> +GVariant *result = NULL;
> +size_t size;
> +
> +result = g_dbus_proxy_get_cached_property(proxy, "Id");
> +if (!result) {
> +g_set_error_literal(err, G_IO_ERROR, G_IO_ERROR_FAILED,
> +"Failed to get Id property");
> +return NULL;
> +}
> +
> +id = g_variant_dup_string(result, );
> +g_clear_pointer(, g_variant_unref);
> +
> +if (size == 0 || size >= 256) {
> +g_set_error_literal(err, G_IO_ERROR, G_IO_ERROR_FAILED,
> +"Invalid Id property");
> +g_free(id);
> +return NULL;
> +}
> +
> +return id;
> 

[Qemu-devel] [PATCH v2 1/2] qemu-file: move qemu_{get, put}_counted_string() declarations

[Marc-André Lureau]

Move migration helpers for strings under include/, so they can be used
outside of migration/

Signed-off-by: Marc-André Lureau 
Reviewed-by: Juan Quintela 
---
 include/migration/qemu-file-types.h | 4 
 migration/qemu-file.h   | 4 
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/include/migration/qemu-file-types.h 
b/include/migration/qemu-file-types.h
index c0a1988155..2867e3da84 100644
--- a/include/migration/qemu-file-types.h
+++ b/include/migration/qemu-file-types.h
@@ -161,6 +161,10 @@ static inline void qemu_get_sbe64s(QEMUFile *f, int64_t 
*pv)
 qemu_get_be64s(f, (uint64_t *)pv);
 }
 
+size_t qemu_get_counted_string(QEMUFile *f, char buf[256]);
+
+void qemu_put_counted_string(QEMUFile *f, const char *name);
+
 int qemu_file_rate_limit(QEMUFile *f);
 
 #endif
diff --git a/migration/qemu-file.h b/migration/qemu-file.h
index 13baf896bd..185d3de505 100644
--- a/migration/qemu-file.h
+++ b/migration/qemu-file.h
@@ -155,8 +155,6 @@ QEMUFile *qemu_file_get_return_path(QEMUFile *f);
 void qemu_fflush(QEMUFile *f);
 void qemu_file_set_blocking(QEMUFile *f, bool block);
 
-size_t qemu_get_counted_string(QEMUFile *f, char buf[256]);
-
 void ram_control_before_iterate(QEMUFile *f, uint64_t flags);
 void ram_control_after_iterate(QEMUFile *f, uint64_t flags);
 void ram_control_load_hook(QEMUFile *f, uint64_t flags, void *data);
@@ -175,6 +173,4 @@ size_t ram_control_save_page(QEMUFile *f, ram_addr_t 
block_offset,
  ram_addr_t offset, size_t size,
  uint64_t *bytes_sent);
 
-void qemu_put_counted_string(QEMUFile *f, const char *name);
-
 #endif
-- 
2.23.0.rc1

[Qemu-devel] [PATCH v2 2/2] Add dbus-vmstate object

[Marc-André Lureau]

When instanciated, this object will connect to the given D-Bus
bus. During migration, it will take the data from org.qemu.VMState1
instances.

See documentation for further details.

Signed-off-by: Marc-André Lureau 
---
 MAINTAINERS   |   6 +
 backends/Makefile.objs|   4 +
 backends/dbus-vmstate.c   | 332 ++
 configure |   7 +
 docs/interop/dbus-vmstate.rst |  63 ++
 docs/interop/index.rst|   1 +
 tests/Makefile.include|  18 +-
 tests/dbus-vmstate-test.c | 371 ++
 tests/dbus-vmstate1.xml   |  12 ++
 9 files changed, 813 insertions(+), 1 deletion(-)
 create mode 100644 backends/dbus-vmstate.c
 create mode 100644 docs/interop/dbus-vmstate.rst
 create mode 100644 tests/dbus-vmstate-test.c
 create mode 100644 tests/dbus-vmstate1.xml

diff --git a/MAINTAINERS b/MAINTAINERS
index d6de200453..d136bf4f4b 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -2143,6 +2143,12 @@ F: tests/migration-test.c
 F: docs/devel/migration.rst
 F: qapi/migration.json
 
+DBus VMState
+M: Marc-André Lureau 
+S: Maintained
+F: backends/dbus-vmstate.c
+F: tests/dbus-vmstate*
+
 Seccomp
 M: Eduardo Otubo 
 S: Supported
diff --git a/backends/Makefile.objs b/backends/Makefile.objs
index 981e8e122f..dbbe12b225 100644
--- a/backends/Makefile.objs
+++ b/backends/Makefile.objs
@@ -17,3 +17,7 @@ endif
 common-obj-$(call land,$(CONFIG_VHOST_USER),$(CONFIG_VIRTIO)) += vhost-user.o
 
 common-obj-$(CONFIG_LINUX) += hostmem-memfd.o
+
+common-obj-$(CONFIG_GIO) += dbus-vmstate.o
+dbus-vmstate.o-cflags = $(GIO_CFLAGS)
+dbus-vmstate.o-libs = $(GIO_LIBS)
diff --git a/backends/dbus-vmstate.c b/backends/dbus-vmstate.c
new file mode 100644
index 00..a9d9cac388
--- /dev/null
+++ b/backends/dbus-vmstate.c
@@ -0,0 +1,332 @@
+/*
+ * QEMU dbus-vmstate
+ *
+ * Copyright (C) 2019 Red Hat Inc
+ *
+ * Authors:
+ *  Marc-André Lureau 
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ */
+
+#include "qemu/osdep.h"
+#include "qemu/error-report.h"
+#include "qapi/error.h"
+#include "qom/object_interfaces.h"
+#include "qapi/qmp/qerror.h"
+#include "migration/register.h"
+#include "migration/qemu-file-types.h"
+#include 
+
+typedef struct DBusVMState DBusVMState;
+typedef struct DBusVMStateClass DBusVMStateClass;
+
+#define TYPE_DBUS_VMSTATE "dbus-vmstate"
+#define DBUS_VMSTATE(obj)\
+OBJECT_CHECK(DBusVMState, (obj), TYPE_DBUS_VMSTATE)
+#define DBUS_VMSTATE_GET_CLASS(obj)  \
+OBJECT_GET_CLASS(DBusVMStateClass, (obj), TYPE_DBUS_VMSTATE)
+#define DBUS_VMSTATE_CLASS(klass)\
+OBJECT_CLASS_CHECK(DBusVMStateClass, (klass), TYPE_DBUS_VMSTATE)
+
+struct DBusVMStateClass {
+ObjectClass parent_class;
+};
+
+struct DBusVMState {
+Object parent;
+
+GDBusConnection *bus;
+GDBusProxy *proxy;
+char *id;
+char *dbus_addr;
+};
+
+static const GDBusPropertyInfo vmstate_property_info[] = {
+{ -1, (char *) "Id", (char *) "s",
+  G_DBUS_PROPERTY_INFO_FLAGS_READABLE, NULL },
+};
+
+static const GDBusPropertyInfo * const vmstate_property_info_pointers[] = {
+_property_info[0],
+NULL
+};
+
+static const GDBusInterfaceInfo vmstate1_interface_info = {
+-1,
+(char *) "org.qemu.VMState1",
+(GDBusMethodInfo **) NULL,
+(GDBusSignalInfo **) NULL,
+(GDBusPropertyInfo **) _property_info_pointers,
+NULL,
+};
+
+#define DBUS_VMSTATE_SIZE_LIMIT (1 << 20) /* 1mb */
+
+
+static char *
+get_idstr(DBusVMState *self)
+{
+return g_strdup_printf("%s-%s", TYPE_DBUS_VMSTATE, self->id);
+}
+
+static char *
+dbus_proxy_get_id(GDBusProxy *proxy, GError **err)
+{
+char *id = NULL;
+GVariant *result = NULL;
+size_t size;
+
+result = g_dbus_proxy_get_cached_property(proxy, "Id");
+if (!result) {
+g_set_error_literal(err, G_IO_ERROR, G_IO_ERROR_FAILED,
+"Failed to get Id property");
+return NULL;
+}
+
+id = g_variant_dup_string(result, );
+g_clear_pointer(, g_variant_unref);
+
+if (size == 0 || size >= 256) {
+g_set_error_literal(err, G_IO_ERROR, G_IO_ERROR_FAILED,
+"Invalid Id property");
+g_free(id);
+return NULL;
+}
+
+return id;
+}
+
+static int
+dbus_load_state_proxy(GDBusProxy *proxy, const uint8_t *data, size_t size)
+{
+GError *err = NULL;
+GVariant *value, *result;
+int ret = -1;
+
+value = g_variant_new_fixed_array(G_VARIANT_TYPE_BYTE,
+  data, size, sizeof(char));
+result = g_dbus_proxy_call_sync(proxy, "Load",
+g_variant_new("(@ay)", value),
+G_DBUS_CALL_FLAGS_NO_AUTO_START,
+-1, NULL, );
+if (!result) {
+ 

[Qemu-devel] [PATCH v2 0/2] Add dbus-vmstate

[Marc-André Lureau]

Hi,

With external processes or helpers participating to the VM support, it
becomes necessary to handle their migration. Various options exist to
transfer their state:
1) as the VM memory, RAM or devices (we could say that's how
   vhost-user devices can be handled today, they are expected to
   restore from ring state)
2) other "vmstate" (as with TPM emulator state blobs)
3) left to be handled by management layer

1) is not practical, since an external processes may legitimatelly
need arbitrary state date to back a device or a service, or may not
even have an associated device.

2) needs ad-hoc code for each helper, but is simple and working

3) is complicated for management layer, QEMU has the migration timing

The proposed "dbus-vmstate" object will connect to a given D-Bus
peer address, and save/load from org.qemu.VMState1 interface.

This way, helpers can have their state migrated with QEMU, without
implementing another ad-hoc support (such as done for TPM emulation)

I chose D-Bus as it is ubiquitous on Linux (it is systemd IPC), and
can be made to work on various other OSes. There are several
implementations and good bindings for various languages.
(the tests/dbus-vmstate-test.c is a good example of how simple
the implementation of services can be, even in C)

v2:
- D-Bus is most common and practical through a bus, but it requires a
  daemon to be running. I argue that the benefits outweight the cost
  of running an extra daemon in v1 in the context of multi-process
  qemu, but it is also possible to connect in p2p mode as done in this
  new version.

dbus-vmstate is put into use by the libvirt series "[PATCH v2 00/23]
Use a slirp helper process".

Marc-André Lureau (2):
  qemu-file: move qemu_{get,put}_counted_string() declarations
  Add dbus-vmstate object

 MAINTAINERS |   6 +
 backends/Makefile.objs  |   4 +
 backends/dbus-vmstate.c | 332 +
 configure   |   7 +
 docs/interop/dbus-vmstate.rst   |  63 +
 docs/interop/index.rst  |   1 +
 include/migration/qemu-file-types.h |   4 +
 migration/qemu-file.h   |   4 -
 tests/Makefile.include  |  18 +-
 tests/dbus-vmstate-test.c   | 371 
 tests/dbus-vmstate1.xml |  12 +
 11 files changed, 817 insertions(+), 5 deletions(-)
 create mode 100644 backends/dbus-vmstate.c
 create mode 100644 docs/interop/dbus-vmstate.rst
 create mode 100644 tests/dbus-vmstate-test.c
 create mode 100644 tests/dbus-vmstate1.xml

-- 
2.23.0.rc1

Re: [Qemu-devel] [PATCH v6 12/26] hw/s390x: Hard code size with MO_{8|16|32|64}

[Cornelia Huck]

On Wed, 7 Aug 2019 08:30:04 +
 wrote:

> Temporarily no-op size_memop was introduced to aid the conversion of
> memory_region_dispatch_{read|write} operand "unsigned size" into
> "MemOp op".
> 
> Now size_memop is implemented, again hard coded size but with

"Now that size_memop has been implemented properly, again hard code the
size but with..."

> MO_{8|16|32|64}. This is more expressive and avoid size_memop calls.

s/avoid/avoids/

> 
> Signed-off-by: Tony Nguyen 
> ---
>  hw/s390x/s390-pci-inst.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)

Reviewed-by: Cornelia Huck 

Re: [Qemu-devel] [PATCH v2 3/3] qcow2: add zstd cluster compression

[Max Reitz]

On 04.07.19 15:09, Denis Plotnikov wrote:
> zstd significantly reduces cluster compression time.
> It provides better compression performance maintaining
> the same level of compression ratio in comparison with
> zlib, which, by the moment, has been the only compression
> method available.
> 
> The performance test results:
> Test compresses and decompresses qemu qcow2 image with just
> installed rhel-7.6 guest.
> Image cluster size: 64K. Image on disk size: 2.2G
> 
> The test was conducted with brd disk to reduce the influence
> of disk subsystem to the test results.
> The results is given in seconds.
> 
> compress cmd:
>   time ./qemu-img convert -O qcow2 -c -o compression_type=[zlib|zstd]
>   src.img [zlib|zstd]_compressed.img
> decompress cmd
>   time ./qemu-img convert -O qcow2
>   [zlib|zstd]_compressed.img uncompressed.img
> 
>compression   decompression
>  zlib   zstd   zlib zstd
> 
> real 65.5   16.3 (-75 %)1.9  1.6 (-16 %)
> user 65.0   15.85.3  2.5
> sys   3.30.22.0  2.0
> 
> Both ZLIB and ZSTD gave the same compression ratio: 1.57
> compressed image size in both cases: 1.4G
> 
> Signed-off-by: Denis Plotnikov 
> ---
>  block/qcow2.c  | 99 ++
>  configure  | 32 ++
>  docs/interop/qcow2.txt | 19 
>  qapi/block-core.json   |  3 +-
>  4 files changed, 152 insertions(+), 1 deletion(-)
> 
> diff --git a/block/qcow2.c b/block/qcow2.c
> index a107f76e98..252eba636f 100644
> --- a/block/qcow2.c
> +++ b/block/qcow2.c

[...]

> @@ -4092,6 +4103,84 @@ static ssize_t qcow2_zlib_decompress(void *dest, 
> size_t dest_size,
>  return ret;
>  }
>  
> +#ifdef CONFIG_ZSTD
> +/*
> + * qcow2_zstd_compress()
> + *
> + * Compress @src_size bytes of data using zstd compression method
> + *
> + * @dest - destination buffer, @dest_size bytes
> + * @src - source buffer, @src_size bytes
> + *
> + * Returns: compressed size on success
> + *  a negative error code on fail
> + */
> +
> +static ssize_t qcow2_zstd_compress(void *dest, size_t dest_size,
> +   const void *src, size_t src_size)
> +{
> +ssize_t ret;
> +uint32_t *c_size = dest;
> +/* steal some bytes to store compressed chunk size */
> +char *d_buf = ((char *) dest) + sizeof(*c_size);
> +
> +if (dest_size < sizeof(*c_size)) {
> +return -ENOMEM;
> +}
> +
> +dest_size -= sizeof(*c_size);
> +
> +ret = ZSTD_compress(d_buf, dest_size, src, src_size, 5);
> +
> +if (ZSTD_isError(ret)) {
> +if (ret == ZSTD_error_dstSize_tooSmall) {

s/ret/ZSTD_getErrorCode(ret)/

> +return -ENOMEM;
> +} else {
> +return -EIO;
> +}
> +}
> +
> +/* store the compressed chunk size in the very beginning of the buffer */
> +*c_size = ret;

I think this should be stored in big endian.

> +
> +return ret + sizeof(ret);

s/sizeof(ret)/sizeof(*c_size)/

> +}
> +
> +/*
> + * qcow2_zstd_decompress()
> + *
> + * Decompress some data (not more than @src_size bytes) to produce exactly
> + * @dest_size bytes using zstd compression method
> + *
> + * @dest - destination buffer, @dest_size bytes
> + * @src - source buffer, @src_size bytes
> + *
> + * Returns: 0 on success
> + *  -EIO on fail
> + */
> +
> +static ssize_t qcow2_zstd_decompress(void *dest, size_t dest_size,
> + const void *src, size_t src_size)
> +{
> +ssize_t ret;
> +/*
> + * zstd decompress wants to know the exact lenght of the data

*length

> + * for that purpose, on the compression the length is stored in
> + * the very beginning of the compressed buffer
> + */
> +const uint32_t *s_size = src;
> +const char *s_buf = ((char *) src) + sizeof(*s_size);

If you want to be strict, s/(char *)/(const char *)/.

> +
> +ret = ZSTD_decompress(dest, dest_size, s_buf, *s_size);
> +
> +if (ZSTD_isError(ret)) {
> +return -EIO;
> +}
> +
> +return 0;
> +}
> +#endif
> +
>  #define MAX_COMPRESS_THREADS 4
>  
>  typedef ssize_t (*Qcow2CompressFunc)(void *dest, size_t dest_size,

[...]

> diff --git a/configure b/configure
> index 1c563a7027..57a80e38e7 100755
> --- a/configure
> +++ b/configure

[...]

> @@ -2374,6 +2380,29 @@ EOF
>  fi
>  fi
>  
> +#
> +# zstd check
> +
> +if test "$zstd" != "no" ; then
> +if $pkg_config --exists libzstd; then
> +zstd_cflags=$($pkg_config --cflags libzstd)
> +zstd_libs=$($pkg_config --libs libzstd)
> +QEMU_CFLAGS="$zstd_cflags $QEMU_CFLAGS"
> +LIBS="$zstd_libs $LIBS"
> +else
> +cat > $TMPC << EOF
> +#include 
> +int main(void) { ZSTD_versionNumber(); return 0; }
> +EOF
> +if 

Re: [Qemu-devel] [PATCH v2 2/3] qcow2: rework the cluster compression routine

[Max Reitz]

On 04.07.19 15:09, Denis Plotnikov wrote:
> The patch allow to process image compression type defined
> in the image header and choose an appropriate method for
> image clusters (de)compression.
> 
> Signed-off-by: Denis Plotnikov 
> ---
>  block/qcow2.c | 93 ---
>  1 file changed, 73 insertions(+), 20 deletions(-)
> 
> diff --git a/block/qcow2.c b/block/qcow2.c
> index 8fa932a349..a107f76e98 100644
> --- a/block/qcow2.c
> +++ b/block/qcow2.c

[...]

> @@ -4153,20 +4156,67 @@ qcow2_co_do_compress(BlockDriverState *bs, void 
> *dest, size_t dest_size,
>  return arg.ret;
>  }
>  
> +/*
> + * qcow2_co_compress()
> + *
> + * Compress @src_size bytes of data using the compression
> + * method defined by the image compression type
> + *
> + * @dest - destination buffer, @dest_size bytes
> + * @src - source buffer, @src_size bytes
> + *
> + * Returns: 0 on success

Actually, it returns the number of compressed bytes.

Max

> + *  a negative error code on fail
> + */



signature.asc
Description: OpenPGP digital signature

[Qemu-devel] [PATCH 6/6] net/eth: Remove the single use of udp_hdr structure

[Philippe Mathieu-Daudé]

Commit 75020a70215 introduced 2 very equivalent structures:
udp_header and udp_hdr.

Replace the single use of udp_hdr by udp_header (which has few more
uses in the codebase) and remove the now unused structure.

Signed-off-by: Philippe Mathieu-Daudé 
---
 hw/net/net_tx_pkt.c | 2 +-
 include/net/eth.h   | 7 ---
 2 files changed, 1 insertion(+), 8 deletions(-)

diff --git a/hw/net/net_tx_pkt.c b/hw/net/net_tx_pkt.c
index fc4514416c..25148cf01b 100644
--- a/hw/net/net_tx_pkt.c
+++ b/hw/net/net_tx_pkt.c
@@ -349,7 +349,7 @@ void net_tx_pkt_build_vheader(struct NetTxPkt *pkt, bool 
tso_enable,
 case IP_PROTO_UDP:
 pkt->virt_hdr.flags = VIRTIO_NET_HDR_F_NEEDS_CSUM;
 pkt->virt_hdr.csum_start = pkt->hdr_len;
-pkt->virt_hdr.csum_offset = offsetof(struct udp_hdr, uh_sum);
+pkt->virt_hdr.csum_offset = offsetof(struct udp_header, uh_sum);
 break;
 default:
 break;
diff --git a/include/net/eth.h b/include/net/eth.h
index 0b2584328a..740ec225fb 100644
--- a/include/net/eth.h
+++ b/include/net/eth.h
@@ -153,13 +153,6 @@ struct ip6_option_hdr {
 uint8_t len;
 };
 
-struct udp_hdr {
-  uint16_t uh_sport;   /* source port */
-  uint16_t uh_dport;   /* destination port */
-  uint16_t uh_ulen;/* udp length */
-  uint16_t uh_sum; /* udp checksum */
-};
-
 #define ip6_nxt  ip6_ctlun.ip6_un1.ip6_un1_nxt
 #define ip6_ecn_acc  ip6_ctlun.ip6_un3.ip6_un3_ecn
 
-- 
2.20.1

[Qemu-devel] [PATCH 5/6] net/eth: Remove the unused tcp_hdr structure

[Philippe Mathieu-Daudé]

Commit 75020a70215 introduced 2 very similar structures:
tcp_header and tcp_hdr.

We replaced the uses of struct tcp_hdr with the equivalent
struct tcp_header. Remove the unused one.

Signed-off-by: Philippe Mathieu-Daudé 
---
I prefer the TCP_FLAG_XXX name, but there is only one single use of
TCP_FLAG_ACK vs many use of the other set, so I'm keeping the set.

 include/net/eth.h | 40 
 1 file changed, 8 insertions(+), 32 deletions(-)

diff --git a/include/net/eth.h b/include/net/eth.h
index 7f45c678e7..0b2584328a 100644
--- a/include/net/eth.h
+++ b/include/net/eth.h
@@ -72,7 +72,14 @@ typedef struct tcp_header {
 #define TCP_HEADER_FLAGS(tcp) \
 TCP_FLAGS_ONLY(be16_to_cpu((tcp)->th_offset_flags))
 
-#define TCP_FLAG_ACK  0x10
+#define TH_FIN  0x01
+#define TH_SYN  0x02
+#define TH_RST  0x04
+#define TH_PUSH 0x08
+#define TH_ACK  0x10
+#define TH_URG  0x20
+#define TH_ECE  0x40
+#define TH_CWR  0x80
 
 #define TCP_HEADER_DATA_OFFSET(tcp) \
 (((be16_to_cpu((tcp)->th_offset_flags) >> 12) & 0xf) << 2)
@@ -153,37 +160,6 @@ struct udp_hdr {
   uint16_t uh_sum; /* udp checksum */
 };
 
-struct tcp_hdr {
-u_short th_sport;   /* source port */
-u_short th_dport;   /* destination port */
-uint32_tth_seq; /* sequence number */
-uint32_tth_ack; /* acknowledgment number */
-#ifdef HOST_WORDS_BIGENDIAN
-u_char  th_off : 4, /* data offset */
-th_x2:4;/* (unused) */
-#else
-u_char  th_x2 : 4,  /* (unused) */
-th_off:4;   /* data offset */
-#endif
-
-#define TH_ELN  0x1 /* explicit loss notification */
-#define TH_ECN  0x2 /* explicit congestion notification */
-#define TH_FS   0x4 /* fast start */
-
-u_char  th_flags;
-#define TH_FIN  0x01
-#define TH_SYN  0x02
-#define TH_RST  0x04
-#define TH_PUSH 0x08
-#define TH_ACK  0x10
-#define TH_URG  0x20
-#define TH_ECE  0x40
-#define TH_CWR  0x80
-u_short th_win;  /* window */
-u_short th_sum;  /* checksum */
-u_short th_urp;  /* urgent pointer */
-};
-
 #define ip6_nxt  ip6_ctlun.ip6_un1.ip6_un1_nxt
 #define ip6_ecn_acc  ip6_ctlun.ip6_un3.ip6_un3_ecn
 
-- 
2.20.1

Re: [Qemu-devel] [PATCH v6 08/26] hw/vfio: Access MemoryRegion with MemOp

[Cornelia Huck]

On Wed, 7 Aug 2019 08:28:40 +
 wrote:

> The memory_region_dispatch_{read|write} operand "unsigned size" is
> being converted into a "MemOp op".
> 
> Convert interfaces by using no-op size_memop.
> 
> After all interfaces are converted, size_memop will be implemented
> and the memory_region_dispatch_{read|write} operand "unsigned size"
> will be converted into a "MemOp op".
> 
> As size_memop is a no-op, this patch does not change any behaviour.
> 
> Signed-off-by: Tony Nguyen 
> Reviewed-by: Richard Henderson 
> ---
>  hw/vfio/pci-quirks.c | 6 --
>  1 file changed, 4 insertions(+), 2 deletions(-)

Reviewed-by: Cornelia Huck 

[Qemu-devel] [RFC PATCH 2/6] net/colo-compare: Use the tcp_header structure

[Philippe Mathieu-Daudé]

The tcp_header structure comes convenient macros to avoid
manipulating the TCP header flags/offset bits manually.
Replace the tcp_hdr structure by the tcp_header equivalent,
and use the macros.

Signed-off-by: Philippe Mathieu-Daudé 
---
RFC: Verify th_off endianess

 net/colo-compare.c | 14 +++---
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/net/colo-compare.c b/net/colo-compare.c
index 7489840bde..14ee4166ba 100644
--- a/net/colo-compare.c
+++ b/net/colo-compare.c
@@ -161,28 +161,28 @@ static void 
colo_compare_inconsistency_notify(CompareState *s)
 
 static gint seq_sorter(Packet *a, Packet *b, gpointer data)
 {
-struct tcp_hdr *atcp, *btcp;
+struct tcp_header *atcp, *btcp;
 
-atcp = (struct tcp_hdr *)(a->transport_header);
-btcp = (struct tcp_hdr *)(b->transport_header);
+atcp = (struct tcp_header *)(a->transport_header);
+btcp = (struct tcp_header *)(b->transport_header);
 return ntohl(atcp->th_seq) - ntohl(btcp->th_seq);
 }
 
 static void fill_pkt_tcp_info(void *data, uint32_t *max_ack)
 {
 Packet *pkt = data;
-struct tcp_hdr *tcphd;
+struct tcp_header *tcphd;
 
-tcphd = (struct tcp_hdr *)pkt->transport_header;
+tcphd = (struct tcp_header *)pkt->transport_header;
 
 pkt->tcp_seq = ntohl(tcphd->th_seq);
 pkt->tcp_ack = ntohl(tcphd->th_ack);
 *max_ack = *max_ack > pkt->tcp_ack ? *max_ack : pkt->tcp_ack;
 pkt->header_size = pkt->transport_header - (uint8_t *)pkt->data
-   + (tcphd->th_off << 2) - pkt->vnet_hdr_len;
+   + TCP_HEADER_DATA_OFFSET(tcphd) - pkt->vnet_hdr_len;
 pkt->payload_size = pkt->size - pkt->header_size;
 pkt->seq_end = pkt->tcp_seq + pkt->payload_size;
-pkt->flags = tcphd->th_flags;
+pkt->flags = TCP_HEADER_FLAGS(tcphd);
 }
 
 /*
-- 
2.20.1

[Qemu-devel] [RFC PATCH 4/6] hw/net/vmxnet3: Use the tcp_header structure

[Philippe Mathieu-Daudé]

The tcp_header structure comes convenient macros to avoid
manipulating the TCP header flags/offset bits manually.
Replace the tcp_hdr structure by the tcp_header equivalent,
and use the macros.

Since we will remove the duplicated TCP_FLAG_ACK definition
in the next commit, replace its use now.

Signed-off-by: Philippe Mathieu-Daudé 
---
RFC: Verify th_off endianess

 hw/net/net_rx_pkt.c | 2 +-
 hw/net/net_tx_pkt.c | 7 ---
 2 files changed, 5 insertions(+), 4 deletions(-)

diff --git a/hw/net/net_rx_pkt.c b/hw/net/net_rx_pkt.c
index 98a5030ace..7558f0646a 100644
--- a/hw/net/net_rx_pkt.c
+++ b/hw/net/net_rx_pkt.c
@@ -376,7 +376,7 @@ bool net_rx_pkt_is_tcp_ack(struct NetRxPkt *pkt)
 assert(pkt);
 
 if (pkt->istcp) {
-return TCP_HEADER_FLAGS(>l4hdr_info.hdr.tcp) & TCP_FLAG_ACK;
+return TCP_HEADER_FLAGS(>l4hdr_info.hdr.tcp) & TH_ACK;
 }
 
 return false;
diff --git a/hw/net/net_tx_pkt.c b/hw/net/net_tx_pkt.c
index 162f802dd7..fc4514416c 100644
--- a/hw/net/net_tx_pkt.c
+++ b/hw/net/net_tx_pkt.c
@@ -307,7 +307,7 @@ func_exit:
 void net_tx_pkt_build_vheader(struct NetTxPkt *pkt, bool tso_enable,
 bool csum_enable, uint32_t gso_size)
 {
-struct tcp_hdr l4hdr;
+struct tcp_header l4hdr;
 assert(pkt);
 
 /* csum has to be enabled if tso is. */
@@ -330,7 +330,8 @@ void net_tx_pkt_build_vheader(struct NetTxPkt *pkt, bool 
tso_enable,
 case VIRTIO_NET_HDR_GSO_TCPV6:
 iov_to_buf(>vec[NET_TX_PKT_PL_START_FRAG], pkt->payload_frags,
0, , sizeof(l4hdr));
-pkt->virt_hdr.hdr_len = pkt->hdr_len + l4hdr.th_off * sizeof(uint32_t);
+pkt->virt_hdr.hdr_len = pkt->hdr_len
++ TCP_HEADER_DATA_OFFSET() * 
sizeof(uint32_t);
 pkt->virt_hdr.gso_size = gso_size;
 break;
 
@@ -343,7 +344,7 @@ void net_tx_pkt_build_vheader(struct NetTxPkt *pkt, bool 
tso_enable,
 case IP_PROTO_TCP:
 pkt->virt_hdr.flags = VIRTIO_NET_HDR_F_NEEDS_CSUM;
 pkt->virt_hdr.csum_start = pkt->hdr_len;
-pkt->virt_hdr.csum_offset = offsetof(struct tcp_hdr, th_sum);
+pkt->virt_hdr.csum_offset = offsetof(struct tcp_header, th_sum);
 break;
 case IP_PROTO_UDP:
 pkt->virt_hdr.flags = VIRTIO_NET_HDR_F_NEEDS_CSUM;
-- 
2.20.1

[Qemu-devel] [PATCH 3/6] net/filter-rewriter: Use the tcp_header structure

[Philippe Mathieu-Daudé]

The tcp_header structure comes convenient macros to avoid
manipulating the TCP header flags/offset bits manually.
Replace the tcp_hdr structure by the tcp_header equivalent,
and use the TCP_HEADER_FLAGS macro.

Signed-off-by: Philippe Mathieu-Daudé 
---
 net/filter-rewriter.c | 37 +
 1 file changed, 21 insertions(+), 16 deletions(-)

diff --git a/net/filter-rewriter.c b/net/filter-rewriter.c
index 31da08a2f4..62e0de1293 100644
--- a/net/filter-rewriter.c
+++ b/net/filter-rewriter.c
@@ -73,23 +73,26 @@ static int handle_primary_tcp_pkt(RewriterState *rf,
   Connection *conn,
   Packet *pkt, ConnectionKey *key)
 {
-struct tcp_hdr *tcp_pkt;
+struct tcp_header *tcp_pkt;
+uint8_t tcp_flags;
+
+tcp_pkt = (struct tcp_header *)pkt->transport_header;
+tcp_flags = TCP_HEADER_FLAGS(tcp_pkt);
 
-tcp_pkt = (struct tcp_hdr *)pkt->transport_header;
 if (trace_event_get_state_backends(TRACE_COLO_FILTER_REWRITER_DEBUG)) {
 trace_colo_filter_rewriter_pkt_info(__func__,
 inet_ntoa(pkt->ip->ip_src), inet_ntoa(pkt->ip->ip_dst),
 ntohl(tcp_pkt->th_seq), ntohl(tcp_pkt->th_ack),
-tcp_pkt->th_flags);
+tcp_flags);
 trace_colo_filter_rewriter_conn_offset(conn->offset);
 }
 
-if (((tcp_pkt->th_flags & (TH_ACK | TH_SYN)) == (TH_ACK | TH_SYN)) &&
+if (((tcp_flags & (TH_ACK | TH_SYN)) == (TH_ACK | TH_SYN)) &&
 conn->tcp_state == TCPS_SYN_SENT) {
 conn->tcp_state = TCPS_ESTABLISHED;
 }
 
-if (((tcp_pkt->th_flags & (TH_ACK | TH_SYN)) == TH_SYN)) {
+if (((tcp_flags & (TH_ACK | TH_SYN)) == TH_SYN)) {
 /*
  * we use this flag update offset func
  * run once in independent tcp connection
@@ -97,7 +100,7 @@ static int handle_primary_tcp_pkt(RewriterState *rf,
 conn->tcp_state = TCPS_SYN_RECEIVED;
 }
 
-if (((tcp_pkt->th_flags & (TH_ACK | TH_SYN)) == TH_ACK)) {
+if (((tcp_flags & (TH_ACK | TH_SYN)) == TH_ACK)) {
 if (conn->tcp_state == TCPS_SYN_RECEIVED) {
 /*
  * offset = secondary_seq - primary seq
@@ -119,13 +122,13 @@ static int handle_primary_tcp_pkt(RewriterState *rf,
  * Passive close step 3
  */
 if ((conn->tcp_state == TCPS_LAST_ACK) &&
-(ntohl(tcp_pkt->th_ack) == (conn->fin_ack_seq + 1))) {
+(ldl_be_p(_pkt->th_ack) == (conn->fin_ack_seq + 1))) {
 conn->tcp_state = TCPS_CLOSED;
 g_hash_table_remove(rf->connection_track_table, key);
 }
 }
 
-if ((tcp_pkt->th_flags & TH_FIN) == TH_FIN) {
+if ((tcp_flags & TH_FIN) == TH_FIN) {
 /*
  * Passive close.
  * Step 1:
@@ -176,20 +179,22 @@ static int handle_secondary_tcp_pkt(RewriterState *rf,
 Connection *conn,
 Packet *pkt, ConnectionKey *key)
 {
-struct tcp_hdr *tcp_pkt;
+struct tcp_header *tcp_pkt;
+uint8_t tcp_flags;
 
-tcp_pkt = (struct tcp_hdr *)pkt->transport_header;
+tcp_pkt = (struct tcp_header *)pkt->transport_header;
+tcp_flags = TCP_HEADER_FLAGS(tcp_pkt);
 
 if (trace_event_get_state_backends(TRACE_COLO_FILTER_REWRITER_DEBUG)) {
 trace_colo_filter_rewriter_pkt_info(__func__,
 inet_ntoa(pkt->ip->ip_src), inet_ntoa(pkt->ip->ip_dst),
 ntohl(tcp_pkt->th_seq), ntohl(tcp_pkt->th_ack),
-tcp_pkt->th_flags);
+tcp_flags);
 trace_colo_filter_rewriter_conn_offset(conn->offset);
 }
 
 if (conn->tcp_state == TCPS_SYN_RECEIVED &&
-((tcp_pkt->th_flags & (TH_ACK | TH_SYN)) == (TH_ACK | TH_SYN))) {
+((tcp_flags & (TH_ACK | TH_SYN)) == (TH_ACK | TH_SYN))) {
 /*
  * save offset = secondary_seq and then
  * in handle_primary_tcp_pkt make offset
@@ -200,11 +205,11 @@ static int handle_secondary_tcp_pkt(RewriterState *rf,
 
 /* VM active connect */
 if (conn->tcp_state == TCPS_CLOSED &&
-((tcp_pkt->th_flags & (TH_ACK | TH_SYN)) == TH_SYN)) {
+((tcp_flags & (TH_ACK | TH_SYN)) == TH_SYN)) {
 conn->tcp_state = TCPS_SYN_SENT;
 }
 
-if ((tcp_pkt->th_flags & (TH_ACK | TH_SYN)) == TH_ACK) {
+if ((tcp_flags & (TH_ACK | TH_SYN)) == TH_ACK) {
 /* Only need to adjust seq while offset is Non-zero */
 if (conn->offset) {
 /* handle packets to the primary from the secondary*/
@@ -219,7 +224,7 @@ static int handle_secondary_tcp_pkt(RewriterState *rf,
  * Passive close step 2:
  */
 if (conn->tcp_state == TCPS_CLOSE_WAIT &&
-(tcp_pkt->th_flags & (TH_ACK | TH_FIN)) == (TH_ACK | TH_FIN)) {
+(tcp_flags & (TH_ACK | TH_FIN)) == (TH_ACK | TH_FIN)) {
 conn->fin_ack_seq = ntohl(tcp_pkt->th_seq);
 conn->tcp_state = 

[Qemu-devel] [RFC PATCH 1/6] hw/net/virtio-net: Use TCP_HEADER_FLAGS/TCP_HEADER_DATA_OFFSET macros

[Philippe Mathieu-Daudé]

"net/eth.h" provides convenient macros to avoid manipulating
the TCP header flags/offset bits manually, let's use them.

Signed-off-by: Philippe Mathieu-Daudé 
---
RFC: Check the macro uses the correct bits

 hw/net/virtio-net.c | 9 ++---
 1 file changed, 2 insertions(+), 7 deletions(-)

diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
index b9e1cd71cf..867f50613e 100644
--- a/hw/net/virtio-net.c
+++ b/hw/net/virtio-net.c
@@ -46,9 +46,6 @@
 
 #define VIRTIO_NET_IP4_ADDR_SIZE   8/* ipv4 saddr + daddr */
 
-#define VIRTIO_NET_TCP_FLAG 0x3F
-#define VIRTIO_NET_TCP_HDR_LENGTH   0xF000
-
 /* IPv4 max payload, 16 bits in the header */
 #define VIRTIO_NET_MAX_IP4_PAYLOAD (65535 - sizeof(struct ip_header))
 #define VIRTIO_NET_MAX_TCP_PAYLOAD 65535
@@ -1658,10 +1655,8 @@ static int 
virtio_net_rsc_tcp_ctrl_check(VirtioNetRscChain *chain,
 uint16_t tcp_hdr;
 uint16_t tcp_flag;
 
-tcp_flag = htons(tcp->th_offset_flags);
-tcp_hdr = (tcp_flag & VIRTIO_NET_TCP_HDR_LENGTH) >> 10;
-tcp_flag &= VIRTIO_NET_TCP_FLAG;
-tcp_flag = htons(tcp->th_offset_flags) & 0x3F;
+tcp_hdr = TCP_HEADER_DATA_OFFSET(tcp);
+tcp_flag = TCP_HEADER_FLAGS(tcp);
 if (tcp_flag & TH_SYN) {
 chain->stat.tcp_syn++;
 return RSC_BYPASS;
-- 
2.20.1

[Qemu-devel] [PATCH 0/6] net/eth: Remove duplicated tcp/udp_hdr structures

[Philippe Mathieu-Daudé]

This is a preparatory cleanup series.

Commit 75020a70215 introduced 4 very equivalent structures:
- tcp_header and tcp_hdr,
- udp_header and udp_hdr.

Choose the most widely use in the codebase, which happens to
provide convenient bitfields manipulation macros and is not
endian-specific.

Philippe Mathieu-Daudé (6):
  hw/net/virtio-net: Use TCP_HEADER_FLAGS/TCP_HEADER_DATA_OFFSET macros
  net/colo-compare: Use the tcp_header structure
  net/filter-rewriter: Use the tcp_header structure
  hw/net/vmxnet3: Use the tcp_header structure
  net/eth: Remove the unused tcp_hdr structure
  net/eth: Remove the single use of udp_hdr structure

 hw/net/net_rx_pkt.c   |  2 +-
 hw/net/net_tx_pkt.c   |  9 +
 hw/net/virtio-net.c   |  9 ++---
 include/net/eth.h | 47 ---
 net/colo-compare.c| 14 ++---
 net/filter-rewriter.c | 37 +++---
 6 files changed, 44 insertions(+), 74 deletions(-)

-- 
2.20.1

Re: [Qemu-devel] [PATCH v6 07/26] hw/virtio: Access MemoryRegion with MemOp

[Cornelia Huck]

On Wed, 7 Aug 2019 08:28:16 +
 wrote:

> The memory_region_dispatch_{read|write} operand "unsigned size" is
> being converted into a "MemOp op".
> 
> Convert interfaces by using no-op size_memop.
> 
> After all interfaces are converted, size_memop will be implemented
> and the memory_region_dispatch_{read|write} operand "unsigned size"
> will be converted into a "MemOp op".
> 
> As size_memop is a no-op, this patch does not change any behaviour.
> 
> Signed-off-by: Tony Nguyen 
> Reviewed-by: Richard Henderson 
> ---
>  hw/virtio/virtio-pci.c | 7 +--
>  1 file changed, 5 insertions(+), 2 deletions(-)

Reviewed-by: Cornelia Huck 

Re: [Qemu-devel] [PATCH v6 05/26] hw/s390x: Access MemoryRegion with MemOp

[Cornelia Huck]

On Wed, 7 Aug 2019 08:27:35 +
 wrote:

> The memory_region_dispatch_{read|write} operand "unsigned size" is
> being converted into a "MemOp op".
> 
> Convert interfaces by using no-op size_memop.
> 
> After all interfaces are converted, size_memop will be implemented
> and the memory_region_dispatch_{read|write} operand "unsigned size"
> will be converted into a "MemOp op".
> 
> As size_memop is a no-op, this patch does not change any behaviour.
> 
> Signed-off-by: Tony Nguyen 
> Reviewed-by: Richard Henderson 
> ---
>  hw/s390x/s390-pci-inst.c | 8 +---
>  1 file changed, 5 insertions(+), 3 deletions(-)

Reviewed-by: Cornelia Huck 

Re: [Qemu-devel] [PATCH v2 26/29] Clean up inclusion of sysemu/sysemu.h

[Alex Bennée]

Markus Armbruster  writes:

> Alex Bennée  writes:
>
>> Markus Armbruster  writes:
>>
>>> In my "build everything" tree, changing sysemu/sysemu.h triggers a
>>> recompile of some 5400 out of 6600 objects (not counting tests and
>>> objects that don't depend on qemu/osdep.h).
>>>
>>> 119 of 380 #include directives are actually superfluous.  Delete them.
>>> Downgrade two more to qapi/qapi-types-run-state.h, and move one from
>>> char/serial.h to char/serial.c.
>>>
>>> This doesn't reduce actual use much, as it's still included into
>>> widely included headers.  The next commit will tackle that.
>>>
>>> Signed-off-by: Markus Armbruster 
>>> ---
>> 
>>>  hw/semihosting/config.c | 1 +
>> 
>>>  stubs/semihost.c| 1 +
>> 
>>> diff --git a/hw/semihosting/config.c b/hw/semihosting/config.c
>>> index 2a8e7e1045..9807f10cb0 100644
>>> --- a/hw/semihosting/config.c
>>> +++ b/hw/semihosting/config.c
>>> @@ -24,6 +24,7 @@
>>>  #include "qemu/error-report.h"
>>>  #include "hw/semihosting/semihost.h"
>>>  #include "chardev/char.h"
>>> +#include "sysemu/sysemu.h"
>>>
>>>  QemuOptsList qemu_semihosting_config_opts = {
>>>  .name = "semihosting-config",
>>> diff --git a/hw/ssi/aspeed_smc.c b/hw/ssi/aspeed_smc.c
>>> index b8332150f1..9f3cff5fb6 100644
>> 
>>>
>>> diff --git a/stubs/semihost.c b/stubs/semihost.c
>>> index 4d5b3c0653..f90589259c 100644
>>> --- a/stubs/semihost.c
>>> +++ b/stubs/semihost.c
>>> @@ -12,6 +12,7 @@
>>>  #include "qemu/option.h"
>>>  #include "qemu/error-report.h"
>>>  #include "hw/semihosting/semihost.h"
>>> +#include "sysemu/sysemu.h"
>> 
>>
>> These additions seem out of place. If I comment them out I can still
>> build fine
>
> sysemu/sysemu.h declares qemu_semihosting_config_opts,
> hw/semihosting/config.c and stubs/semihost.c define it.
>
> Gcc warns when you do that for functions (-Wmissing-declarations
> -Wmissing-prototypes), but not for variables.  I like to include the
> header anyway, to make sure the compiler checks the declaration is
> consistent with the definition.
>
>>- I think the only place that needs them is vl.c so it has a
>> typedef for the semihosting configure options. Arguably the extern
>> declaration could be moved into semihostings own headers to avoid
>> polluting sysemu.h more than it needs to?
>
> I'm not sure I'm following you.
>
> What would you like me to move where?

extern QemuOptsList qemu_semihosting_config_opts;

from sysemu.h to semihosting.h - but given other options externs are
there maybe that is the best place for it.

--
Alex Bennée

Re: [Qemu-devel] RISC-V: Vector && DSP Extension

[Aleksandar Markovic]

On Thu, Aug 8, 2019 at 3:48 PM Chih-Min Chao 
wrote:

>
>
> On Thu, Aug 8, 2019 at 7:29 PM Aleksandar Markovic <
> aleksandar.m.m...@gmail.com> wrote:
>
>> On Thu, Aug 8, 2019 at 11:52 AM liuzhiwei  wrote:
>>
>> > Hi all,
>> >
>> > My workmate  and I have been working on Vector & Dsp extension, and
>> > I'd like to share develop status  with folks.
>> >
>> > The spec references for  Vector extension is riscv-v-spec-0.7.1, and
>> > riscv-p-spec-0.5 for DSP extension.
>>
>>
>> Hello, Liu.
>>
>> I will not answer your questions directly, however I want to bring to you
>> and others another perspective on this situation.
>>
>> First, please provide the link to the specifications. Via Google, I found
>> that "riscv-v-spec-0.7.1" is titled "Working draft of the proposed RISC-V
>> V
>> vector extension". I could not find "riscv-p-spec-0.5".
>>
>> I am not sure what the QEMU policy towards "working draft proposal" type
>> of
>> specification is. Peter, can you perhaps clarify that or any other related
>> issue?
>>
>
> Hi Aleksandar,
>
> As for riscv-v-spec 0.7.1, it is first stable spec for target software
> development
> though the name is working draft.
>

Hello, Chih-Min.

Too many unclear points here.

What does this sentence mean? What is "stable"? Is that the same as
"final"? If the document is stable, why the title "draft/proposal"? Can a
"draft" be stable? Can you, or anybody else, guarantee that the final
version of this document will not affect QEMU implementation, if it is done
by the current document? If not, why would you like QEMU upstream to
support something not fully specified? Why has the final document not been
released, if the situation is stable?.

Yours,
Aleksandar

  The architecture skeleton is fix and most of
> work are focusing the issues related to micro-architecture implementation
> complexity.
> Sifive has released an open source implementation on spike simulation and
> Imperas also
> provides another implementation with its binary simulator.  I think it is
> worth to include the extension
> in Qemu at this moment.
>
> As for riscv-p-spec-0.5, I think Andes has fully supported this extension
> and should release more
> detailed spec in the near future (described Riscv Technical Update
> 2019/06).
> They have implement lots of DSP kernel based on this extension and also
> provided impressed
> performance result.  It is also worth to be reviewed (at least [RFC]) if
> the detailed  spec is public.
>
>
> ref:
>  1.
> https://content.riscv.org/wp-content/uploads/2019/06/17.40-Vector_RISCV-20190611-Vectors.pdf
>  2.
> https://content.riscv.org/wp-content/uploads/2019/06/17.20-P-ext-RVW-Zurich-20190611.pdf
>  3.
> https://content.riscv.org/wp-content/uploads/2019/06/10.05-TechCommitteeUpdate-June-2019-Copy.pdf
>
>
> chihmin
>
>
> I would advice some caution in these cases. The major issue is backward
>> compatibility, but there are other issues too. Let's say, fairness. If we
>> let emulation of a component based on a "working draft proposal" be
>> integrated into QEMU, this will set a precedent, and many other developer
>> would rightfully ask for their contributions based on drafts to be
>> integrated into QEMU. Our policy should be as equal as possible to all
>> contribution, large or small, riscv or alpha, cpu or device, tcg or kvm -
>> in my honest opinion. QEMU upstream should not be a collecting place for
>> all imaginable experimentations, certain criteria on what is appropriate
>> for upstreaming exist and must continue to exist.
>>
>> Yours,
>> Aleksandar
>>
>>
>>
>>
>> > The code of vector extension is
>> > ready and under testing,  the first patch will be sent about two weeks
>> > later. After that we will forward working on DSP extension, and send the
>> > first patch in middle  October.
>> >
>> >  Could the maintainers  tell me whether the specs referenced are
>> > appropriate? Is anyone working on these extensions?  I'd like to get
>> > your status, and maybe discuss questions and work togather.
>> >
>> > Best Regards
>> >
>> > LIU Zhiwei
>> >
>> >
>> >
>> >
>>
>

Re: [Qemu-devel] [PATCH v6 02/26] tcg: TCGMemOp is now accelerator independent MemOp

[Cornelia Huck]

On Wed, 7 Aug 2019 08:26:23 +
 wrote:

> Preparation for collapsing the two byte swaps, adjust_endianness and
> handle_bswap, along the I/O path.
> 
> Target dependant attributes are conditionalize upon NEED_CPU_H.

s/conditionalize/conditionalized/ ?

> 
> Signed-off-by: Tony Nguyen 
> Acked-by: David Gibson 
> Reviewed-by: Richard Henderson 
> ---
>  MAINTAINERS |   1 +
>  accel/tcg/cputlb.c  |   2 +-
>  include/exec/memop.h| 110 ++
>  target/alpha/translate.c|   2 +-
>  target/arm/translate-a64.c  |  48 ++--
>  target/arm/translate-a64.h  |   2 +-
>  target/arm/translate-sve.c  |   2 +-
>  target/arm/translate.c  |  32 
>  target/arm/translate.h  |   2 +-
>  target/hppa/translate.c |  14 ++--
>  target/i386/translate.c | 132 
> 
>  target/m68k/translate.c |   2 +-
>  target/microblaze/translate.c   |   4 +-
>  target/mips/translate.c |   8 +-
>  target/openrisc/translate.c |   4 +-
>  target/ppc/translate.c  |  12 +--
>  target/riscv/insn_trans/trans_rva.inc.c |   8 +-
>  target/riscv/insn_trans/trans_rvi.inc.c |   4 +-
>  target/s390x/translate.c|   6 +-
>  target/s390x/translate_vx.inc.c |  10 +--
>  target/sparc/translate.c|  14 ++--
>  target/tilegx/translate.c   |  10 +--
>  target/tricore/translate.c  |   8 +-
>  tcg/README  |   2 +-
>  tcg/aarch64/tcg-target.inc.c|  26 +++
>  tcg/arm/tcg-target.inc.c|  26 +++
>  tcg/i386/tcg-target.inc.c   |  24 +++---
>  tcg/mips/tcg-target.inc.c   |  16 ++--
>  tcg/optimize.c  |   2 +-
>  tcg/ppc/tcg-target.inc.c|  12 +--
>  tcg/riscv/tcg-target.inc.c  |  20 ++---
>  tcg/s390/tcg-target.inc.c   |  14 ++--
>  tcg/sparc/tcg-target.inc.c  |   6 +-
>  tcg/tcg-op.c|  38 -
>  tcg/tcg-op.h|  86 ++---
>  tcg/tcg.c   |   2 +-
>  tcg/tcg.h   | 101 ++--
>  trace/mem-internal.h|   4 +-
>  trace/mem.h |   4 +-
>  39 files changed, 421 insertions(+), 399 deletions(-)
>  create mode 100644 include/exec/memop.h

Acked-by: Cornelia Huck 

Re: [Qemu-devel] [PATCH v6 01/26] configure: Define TARGET_ALIGNED_ONLY

[Cornelia Huck]

On Wed, 7 Aug 2019 08:25:37 +
 wrote:

> Rename ALIGNED_ONLY to TARGET_ALIGNED_ONLY for clarity and move
> defines out of target/foo/cpu.h into configure, as we do with
> TARGET_WORDS_BIGENDIAN, so that it is always defined early.
> 
> Poisoned TARGET_ALIGNED_ONLY to prevent use in common code.
> 
> Signed-off-by: Tony Nguyen 
> Reviewed-by: Philippe Mathieu-Daudé 
> Reviewed-by: Richard Henderson 
> Reviewed-by: Aleksandar Markovic 
> ---
>  configure | 10 +-
>  include/exec/poison.h |  1 +
>  include/qom/cpu.h |  2 +-
>  target/alpha/cpu.h|  2 --
>  target/hppa/cpu.h |  1 -
>  target/mips/cpu.h |  2 --
>  target/sh4/cpu.h  |  2 --
>  target/sparc/cpu.h|  2 --
>  target/xtensa/cpu.h   |  2 --
>  tcg/tcg.c |  2 +-
>  tcg/tcg.h |  8 +---
>  11 files changed, 17 insertions(+), 17 deletions(-)

Reviewed-by: Cornelia Huck 

Re: [Qemu-devel] [PATCH v2 1/3] qcow2: introduce compression type feature

[Kevin Wolf]

Am 08.08.2019 um 14:50 hat Max Reitz geschrieben:
> On 08.08.19 02:18, Eric Blake wrote:
> > On 7/4/19 8:09 AM, Denis Plotnikov wrote:
> >> The patch adds some preparation parts for incompatible compression type
> >> feature to QCOW2 header that indicates that *all* compressed clusters
> >> must be (de)compressed using a certain compression type.
> >>
> >> It is implied that the compression type is set on the image creation and
> >> can be changed only later by image conversion, thus compression type
> >> defines the only compression algorithm used for the image.
> >>
> >> The goal of the feature is to add support of other compression algorithms
> >> to qcow2. For example, ZSTD which is more effective on compression than 
> >> ZLIB.
> >> It works roughly 2x faster than ZLIB providing a comparable compression 
> >> ratio
> >> and therefore provide a performance advantage in backup scenarios.
> > 
> > provides
> > 
> >>
> >> The default compression is ZLIB. Images created with ZLIB compression type
> >> are backward compatible with older qemu versions.
> >>
> >> Signed-off-by: Denis Plotnikov 
> > 
> >> +++ b/docs/interop/qcow2.txt
> >> @@ -109,7 +109,12 @@ in the description of a field.
> >>  An External Data File Name header 
> >> extension may
> >>  be present if this bit is set.
> >>  
> >> -Bits 3-63:  Reserved (set to 0)
> >> +Bit 3:  Compression type bit. The bit must be set 
> >> if
> >> +the compression type differs from 
> >> default: zlib.
> > 
> > I'd word this 'from the default of zlib.'
> > 
> >> +If the compression type is default the 
> >> bit must
> >> +be unset.
> > 
> > Why? I see no reason to forbid a qcow2 image that has the incompatible
> > bit set but still uses zlib compression.  True, such an image is not as
> > friendly to older clients, but it is not technically wrong, and newer
> > clients would still be able to use the image if not for this sentence
> > telling them they must not.
> 
> Just because an image doesn’t adhere to the specification doesn’t mean
> you have to reject it, if the intention is clear.
> 
> > I'd drop this sentence.
> 
> I wouldn’t, I like it (in essence).  Though maybe the “must” is indeed
> too strong and it should be a “should” instead.

I'd agree that "should" is the best way for the spec.

In the code, I'd insist that QEMU doesn't add any extra code to verify
that this is the case (which previous versions of the series did).

Kevin


signature.asc
Description: PGP signature

Re: [Qemu-devel] [PATCH v2 11/28] riscv: sifive: Rename sifive_prci.{c, h} to sifive_e_prci.{c, h}

[Chih-Min Chao]

On Wed, Aug 7, 2019 at 6:11 PM Bin Meng  wrote:

> On Wed, Aug 7, 2019 at 4:54 PM Chih-Min Chao 
> wrote:
> >
> >
> >
> > On Wed, Aug 7, 2019 at 3:49 PM Bin Meng  wrote:
> >>
> >> Current SiFive PRCI model only works with sifive_e machine, as it
> >> only emulates registers or PRCI block in the FE310 SoC.
> >>
> >> Rename the file name to make it clear that it is for sifive_e.
> >>
> >> Signed-off-by: Bin Meng 
> >> ---
> >>
> >> Changes in v2: None
> >>
> >>  hw/riscv/Makefile.objs  |  2 +-
> >>  hw/riscv/sifive_e.c |  4 ++--
> >>  hw/riscv/{sifive_prci.c => sifive_e_prci.c} | 14 +++---
> >>  include/hw/riscv/{sifive_prci.h => sifive_e_prci.h} | 14 +++---
> >>  4 files changed, 17 insertions(+), 17 deletions(-)
> >>  rename hw/riscv/{sifive_prci.c => sifive_e_prci.c} (90%)
> >>  rename include/hw/riscv/{sifive_prci.h => sifive_e_prci.h} (82%)
> >>
> >> diff --git a/hw/riscv/Makefile.objs b/hw/riscv/Makefile.objs
> >> index eb9d4f9..c859697 100644
> >> --- a/hw/riscv/Makefile.objs
> >> +++ b/hw/riscv/Makefile.objs
> >> @@ -2,9 +2,9 @@ obj-y += boot.o
> >>  obj-$(CONFIG_SPIKE) += riscv_htif.o
> >>  obj-$(CONFIG_HART) += riscv_hart.o
> >>  obj-$(CONFIG_SIFIVE_E) += sifive_e.o
> >> +obj-$(CONFIG_SIFIVE_E) += sifive_e_prci.o
> >>  obj-$(CONFIG_SIFIVE) += sifive_clint.o
> >>  obj-$(CONFIG_SIFIVE) += sifive_gpio.o
> >> -obj-$(CONFIG_SIFIVE) += sifive_prci.o
> >>  obj-$(CONFIG_SIFIVE) += sifive_plic.o
> >>  obj-$(CONFIG_SIFIVE) += sifive_test.o
> >>  obj-$(CONFIG_SIFIVE_U) += sifive_u.o
> >> diff --git a/hw/riscv/sifive_e.c b/hw/riscv/sifive_e.c
> >> index 2a499d8..2d67670 100644
> >> --- a/hw/riscv/sifive_e.c
> >> +++ b/hw/riscv/sifive_e.c
> >> @@ -41,9 +41,9 @@
> >>  #include "hw/riscv/riscv_hart.h"
> >>  #include "hw/riscv/sifive_plic.h"
> >>  #include "hw/riscv/sifive_clint.h"
> >> -#include "hw/riscv/sifive_prci.h"
> >>  #include "hw/riscv/sifive_uart.h"
> >>  #include "hw/riscv/sifive_e.h"
> >> +#include "hw/riscv/sifive_e_prci.h"
> >>  #include "hw/riscv/boot.h"
> >>  #include "chardev/char.h"
> >>  #include "sysemu/arch_init.h"
> >> @@ -174,7 +174,7 @@ static void riscv_sifive_e_soc_realize(DeviceState
> *dev, Error **errp)
> >>  SIFIVE_SIP_BASE, SIFIVE_TIMECMP_BASE, SIFIVE_TIME_BASE);
> >>  sifive_mmio_emulate(sys_mem, "riscv.sifive.e.aon",
> >>  memmap[SIFIVE_E_AON].base, memmap[SIFIVE_E_AON].size);
> >> -sifive_prci_create(memmap[SIFIVE_E_PRCI].base);
> >> +sifive_e_prci_create(memmap[SIFIVE_E_PRCI].base);
> >>
> >>  /* GPIO */
> >>
> >
> > I  think adding infix to function name is sufficient and keeping the
> filename the same may be better.
> > The U board PRCI or variant implementation in future could be included
> in the same files with different create function
> >
>
> I considered such approach when working on this one, but later I chose
> to implement a new file for SiFive U machine.
>
> The SiFive U and E PRCI blocks have different register blocks so if we
> put two variants into one file, their functions don't have much in
> common and we end up just merely physically put them into one file
> which does not bring too much benefit IMHO.
>
> Regards,
> Bin
>

agree that the difference between u and e prci are  a lot and it make sense
to separate it

Reviewed-by: Chih-Min Chao 

Re: [Qemu-devel] [PATCH v2] scsi: lsi: exit infinite loop while executing script (CVE-2019-12068)

[Philippe Mathieu-Daudé]

On 8/8/19 3:46 PM, Marcelo Tosatti wrote:
> On Thu, Aug 08, 2019 at 11:31:02AM +0200, Philippe Mathieu-Daudé wrote:
>> On 8/8/19 11:06 AM, P J P wrote:
>>> From: Prasad J Pandit 
>>>
>>> When executing script in lsi_execute_script(), the LSI scsi
>>> adapter emulator advances 's->dsp' index to read next opcode.
>>> This can lead to an infinite loop if the next opcode is empty.
>>> Exit such loop after reading 10k empty opcodes.
>>>
>>> Reported-by: Bugs SysSec 
>>> Signed-off-by: Prasad J Pandit 
>>> ---
>>>  hw/scsi/lsi53c895a.c | 11 +--
>>>  1 file changed, 9 insertions(+), 2 deletions(-)
>>>
>>> Update v2: define LSI_MAX_INSN 1
>>>   -> https://lists.gnu.org/archive/html/qemu-devel/2019-08/msg01370.html
>>>
>>> diff --git a/hw/scsi/lsi53c895a.c b/hw/scsi/lsi53c895a.c
>>> index 10468c1ec1..2adab341b1 100644
>>> --- a/hw/scsi/lsi53c895a.c
>>> +++ b/hw/scsi/lsi53c895a.c
>>> @@ -185,6 +185,9 @@ static const char *names[] = {
>>>  /* Flag set if this is a tagged command.  */
>>>  #define LSI_TAG_VALID (1 << 16)
>>>  
>>> +/* Maximum instructions to process. */
>>> +#define LSI_MAX_INSN1
>>> +
>>>  typedef struct lsi_request {
>>>  SCSIRequest *req;
>>>  uint32_t tag;
>>> @@ -1132,7 +1135,10 @@ static void lsi_execute_script(LSIState *s)
>>>  
>>>  s->istat1 |= LSI_ISTAT1_SRUN;
>>>  again:
>>> -insn_processed++;
>>> +if (++insn_processed > LSI_MAX_INSN) {
>>> +s->waiting = LSI_NOWAIT;
>>> +goto exitloop;
>>> +}
>>
>> If I understand the datasheet correctly, the model should set the
>> DSTAT.IID bit.
>>
>>   Illegal Instruction Detected
>>
>>   This status bit is set any time an illegal or reserved
>>   instruction opcode is detected, whether the LSI53C895A
>>   is operating in single step mode or automatically
>>   executing SCSI SCRIPTS.
> 
> Sounds the correct thing to do (exiting the loop seems arbitrary). 
> 
>> We already have:
>>
>> trace_lsi_execute_script_tc_illegal();
>> lsi_script_dma_interrupt(s, LSI_DSTAT_IID);
>>
>> Cc'ing Marcelo Tosatti since it is hard to understand the "Windows SCSI
>> driver hack":
> 
> What this patch is, if an infinite loop is detected, to raise UDC
> exception (Unexpected Disconnect). This would cause the driver to 
> restart processing, which would work around the infinite loop problem.

Thanks for the explanation.
So we agree using DSTAT.IID is the correct thing to do.
Any volunteer to fix this? :)

>> $ git show ee4d919f30f
>> commit ee4d919f30f1378cda697dd94d5a21b2a7f4d90d
>> Author: aliguori 
>> Date:   Mon Sep 22 16:04:16 2008 +
>>
>> LSI SCSI: raise UDC on infinite loop (Marcelo Tosatti)
>>
>> Raise UDC (Unexpected Disconnect) when a large enough number of
>> instructions has been executed by the SCRIPTS processor. This "solution"
>> is much simpler than temporarily interrupting execution.
>>
>> This remedies the situation with Windows which downloads SCRIPTS code
>> that busy loops on guest main memory. Their drivers _do_ handle UDC
>> appropriately (at least XP and 2003).
>>
>> It would be nicer to actually detect infinite loops, but until then,
>> this bandaid seems acceptable.
>>
>> Since the situation seems to be rare enough, raise the number
>> of instructions to 1 (previously 1000).
>>
>> Three people other than myself had success with this patch.
>>
>> Signed-off-by: Marcelo Tosatti 
>> Signed-off-by: Anthony Liguori 
>>
>> $ git show 64c68080da4
>> commit 64c68080da429edf30a9857e3a698cb9ed335bd3
>> Author: pbrook 
>> Date:   Mon Sep 22 16:30:29 2008 +
>>
>> Add comment to windows SCSI hack.
>>
>> diff --git a/hw/lsi53c895a.c b/hw/lsi53c895a.c
>> index e45eefaef7..53a2add0df 100644
>> --- a/hw/lsi53c895a.c
>> +++ b/hw/lsi53c895a.c
>> @@ -1199,6 +1199,11 @@ again:
>>  }
>>  }
>>  if (insn_processed > 1 && !s->waiting) {
>> +/* Some windows drivers make the device spin waiting for a memory
>> +   location to change.  If we have been executed a lot of code then
>> +   assume this is the case and force an unexpected device
>> disconnect.
>> +   This is apparently sufficient to beat the drivers into
>> submission.
>> + */
>>  if (!(s->sien0 & LSI_SIST0_UDC))
>>  fprintf(stderr, "inf. loop with UDC masked\n");
>>  lsi_script_scsi_interrupt(s, LSI_SIST0_UDC, 0);
>>
>>>  insn = read_dword(s, s->dsp);
>>>  if (!insn) {
>>>  /* If we receive an empty opcode increment the DSP by 4 bytes
>>> @@ -1569,7 +1575,8 @@ again:
>>>  }
>>>  }
>>>  }
>>> -if (insn_processed > 1 && s->waiting == LSI_NOWAIT) {
>>> +exitloop:
>>> +if (insn_processed > LSI_MAX_INSN && s->waiting == LSI_NOWAIT) {
>>>  /* Some windows drivers make the device spin waiting for a memory
>>> location to change.  If we have been executed a lot of code then
>>> assume this is the case and force 

Re: [Qemu-devel] RISC-V: Vector && DSP Extension

[Chih-Min Chao]

On Thu, Aug 8, 2019 at 7:29 PM Aleksandar Markovic <
aleksandar.m.m...@gmail.com> wrote:

> On Thu, Aug 8, 2019 at 11:52 AM liuzhiwei  wrote:
>
> > Hi all,
> >
> > My workmate  and I have been working on Vector & Dsp extension, and
> > I'd like to share develop status  with folks.
> >
> > The spec references for  Vector extension is riscv-v-spec-0.7.1, and
> > riscv-p-spec-0.5 for DSP extension.
>
>
> Hello, Liu.
>
> I will not answer your questions directly, however I want to bring to you
> and others another perspective on this situation.
>
> First, please provide the link to the specifications. Via Google, I found
> that "riscv-v-spec-0.7.1" is titled "Working draft of the proposed RISC-V V
> vector extension". I could not find "riscv-p-spec-0.5".
>
> I am not sure what the QEMU policy towards "working draft proposal" type of
> specification is. Peter, can you perhaps clarify that or any other related
> issue?
>

Hi Aleksandar,

As for riscv-v-spec 0.7.1, it is first stable spec for target software
development
though the name is working draft.  The architecture skeleton is fix and
most of
work are focusing the issues related to micro-architecture implementation
complexity.
Sifive has released an open source implementation on spike simulation and
Imperas also
provides another implementation with its binary simulator.  I think it is
worth to include the extension
in Qemu at this moment.

As for riscv-p-spec-0.5, I think Andes has fully supported this extension
and should release more
detailed spec in the near future (described Riscv Technical Update
2019/06).
They have implement lots of DSP kernel based on this extension and also
provided impressed
performance result.  It is also worth to be reviewed (at least [RFC]) if
the detailed  spec is public.


ref:
 1.
https://content.riscv.org/wp-content/uploads/2019/06/17.40-Vector_RISCV-20190611-Vectors.pdf
 2.
https://content.riscv.org/wp-content/uploads/2019/06/17.20-P-ext-RVW-Zurich-20190611.pdf
 3.
https://content.riscv.org/wp-content/uploads/2019/06/10.05-TechCommitteeUpdate-June-2019-Copy.pdf


chihmin


I would advice some caution in these cases. The major issue is backward
> compatibility, but there are other issues too. Let's say, fairness. If we
> let emulation of a component based on a "working draft proposal" be
> integrated into QEMU, this will set a precedent, and many other developer
> would rightfully ask for their contributions based on drafts to be
> integrated into QEMU. Our policy should be as equal as possible to all
> contribution, large or small, riscv or alpha, cpu or device, tcg or kvm -
> in my honest opinion. QEMU upstream should not be a collecting place for
> all imaginable experimentations, certain criteria on what is appropriate
> for upstreaming exist and must continue to exist.
>
> Yours,
> Aleksandar
>
>
>
>
> > The code of vector extension is
> > ready and under testing,  the first patch will be sent about two weeks
> > later. After that we will forward working on DSP extension, and send the
> > first patch in middle  October.
> >
> >  Could the maintainers  tell me whether the specs referenced are
> > appropriate? Is anyone working on these extensions?  I'd like to get
> > your status, and maybe discuss questions and work togather.
> >
> > Best Regards
> >
> > LIU Zhiwei
> >
> >
> >
> >
>

Re: [Qemu-devel] [PATCH v2] scsi: lsi: exit infinite loop while executing script (CVE-2019-12068)

[Marcelo Tosatti]

On Thu, Aug 08, 2019 at 11:31:02AM +0200, Philippe Mathieu-Daudé wrote:
> On 8/8/19 11:06 AM, P J P wrote:
> > From: Prasad J Pandit 
> > 
> > When executing script in lsi_execute_script(), the LSI scsi
> > adapter emulator advances 's->dsp' index to read next opcode.
> > This can lead to an infinite loop if the next opcode is empty.
> > Exit such loop after reading 10k empty opcodes.
> > 
> > Reported-by: Bugs SysSec 
> > Signed-off-by: Prasad J Pandit 
> > ---
> >  hw/scsi/lsi53c895a.c | 11 +--
> >  1 file changed, 9 insertions(+), 2 deletions(-)
> > 
> > Update v2: define LSI_MAX_INSN 1
> >   -> https://lists.gnu.org/archive/html/qemu-devel/2019-08/msg01370.html
> > 
> > diff --git a/hw/scsi/lsi53c895a.c b/hw/scsi/lsi53c895a.c
> > index 10468c1ec1..2adab341b1 100644
> > --- a/hw/scsi/lsi53c895a.c
> > +++ b/hw/scsi/lsi53c895a.c
> > @@ -185,6 +185,9 @@ static const char *names[] = {
> >  /* Flag set if this is a tagged command.  */
> >  #define LSI_TAG_VALID (1 << 16)
> >  
> > +/* Maximum instructions to process. */
> > +#define LSI_MAX_INSN1
> > +
> >  typedef struct lsi_request {
> >  SCSIRequest *req;
> >  uint32_t tag;
> > @@ -1132,7 +1135,10 @@ static void lsi_execute_script(LSIState *s)
> >  
> >  s->istat1 |= LSI_ISTAT1_SRUN;
> >  again:
> > -insn_processed++;
> > +if (++insn_processed > LSI_MAX_INSN) {
> > +s->waiting = LSI_NOWAIT;
> > +goto exitloop;
> > +}
> 
> If I understand the datasheet correctly, the model should set the
> DSTAT.IID bit.
> 
>   Illegal Instruction Detected
> 
>   This status bit is set any time an illegal or reserved
>   instruction opcode is detected, whether the LSI53C895A
>   is operating in single step mode or automatically
>   executing SCSI SCRIPTS.

Sounds the correct thing to do (exiting the loop seems arbitrary). 

> We already have:
> 
> trace_lsi_execute_script_tc_illegal();
> lsi_script_dma_interrupt(s, LSI_DSTAT_IID);
> 
> Cc'ing Marcelo Tosatti since it is hard to understand the "Windows SCSI
> driver hack":

What this patch is, if an infinite loop is detected, to raise UDC
exception (Unexpected Disconnect). This would cause the driver to 
restart processing, which would work around the infinite loop problem.

> $ git show ee4d919f30f
> commit ee4d919f30f1378cda697dd94d5a21b2a7f4d90d
> Author: aliguori 
> Date:   Mon Sep 22 16:04:16 2008 +
> 
> LSI SCSI: raise UDC on infinite loop (Marcelo Tosatti)
> 
> Raise UDC (Unexpected Disconnect) when a large enough number of
> instructions has been executed by the SCRIPTS processor. This "solution"
> is much simpler than temporarily interrupting execution.
> 
> This remedies the situation with Windows which downloads SCRIPTS code
> that busy loops on guest main memory. Their drivers _do_ handle UDC
> appropriately (at least XP and 2003).
> 
> It would be nicer to actually detect infinite loops, but until then,
> this bandaid seems acceptable.
> 
> Since the situation seems to be rare enough, raise the number
> of instructions to 1 (previously 1000).
> 
> Three people other than myself had success with this patch.
> 
> Signed-off-by: Marcelo Tosatti 
> Signed-off-by: Anthony Liguori 
> 
> $ git show 64c68080da4
> commit 64c68080da429edf30a9857e3a698cb9ed335bd3
> Author: pbrook 
> Date:   Mon Sep 22 16:30:29 2008 +
> 
> Add comment to windows SCSI hack.
> 
> diff --git a/hw/lsi53c895a.c b/hw/lsi53c895a.c
> index e45eefaef7..53a2add0df 100644
> --- a/hw/lsi53c895a.c
> +++ b/hw/lsi53c895a.c
> @@ -1199,6 +1199,11 @@ again:
>  }
>  }
>  if (insn_processed > 1 && !s->waiting) {
> +/* Some windows drivers make the device spin waiting for a memory
> +   location to change.  If we have been executed a lot of code then
> +   assume this is the case and force an unexpected device
> disconnect.
> +   This is apparently sufficient to beat the drivers into
> submission.
> + */
>  if (!(s->sien0 & LSI_SIST0_UDC))
>  fprintf(stderr, "inf. loop with UDC masked\n");
>  lsi_script_scsi_interrupt(s, LSI_SIST0_UDC, 0);
> 
> >  insn = read_dword(s, s->dsp);
> >  if (!insn) {
> >  /* If we receive an empty opcode increment the DSP by 4 bytes
> > @@ -1569,7 +1575,8 @@ again:
> >  }
> >  }
> >  }
> > -if (insn_processed > 1 && s->waiting == LSI_NOWAIT) {
> > +exitloop:
> > +if (insn_processed > LSI_MAX_INSN && s->waiting == LSI_NOWAIT) {
> >  /* Some windows drivers make the device spin waiting for a memory
> > location to change.  If we have been executed a lot of code then
> > assume this is the case and force an unexpected device 
> > disconnect.
> > 

Re: [Qemu-devel] Is network backend netmap worth keeping?

[Philippe Mathieu-Daudé]

On 8/8/19 7:38 AM, Jason Wang wrote:
> 
> On 2019/8/8 下午12:48, Markus Armbruster wrote:
>> Please excuse the attention-grabbing subject.
>>
>> Philippe Mathieu-Daudé  writes:
>>
>>> On 8/7/19 10:16 PM, Markus Armbruster wrote:
>> [...]
 Can you tell me offhand what I have to install so configure enables
 CONFIG_NETMAP?
>>> The steps are listed in tests/docker/dockerfiles/debian-amd64.docker,
>>> but you can get to this point running:
>>>
>>>    $ make docker-image-debian-amd64 V=1 DEBUG=1
>>>
>>> This will build the docker image with netmap (so you don't have to mess
>>> with your workstation setup), then build QEMU within the image.
>> So, to make use of QEMU's netmap backend (CONFIG_NETMAP), you have to
>> build and install netmap software from sources.  Which pretty much
>> ensures nobody uses it.  It was added in commit 58952137b0b (Nov 2013).
>> The commit message points to ,
>> which gives me "connection timed out" right now.
>>
>> On the other hand, it's covered in MAINTAINERS, and has seen
>> non-janitorial activity as late as Dec 2018 (commit c693fc748a).
>>
>> Luigi, Giuseppe, Vincenzo, what's the status of the netmap project?
>>
>> Why is the QEMU netmap backend worth keeping?
>>
>> Who is using the netmap backend?
> 
> 
> Netmap was supported by freebsd:
> https://www.freebsd.org/cgi/man.cgi?query=netmap=4. So I guess
> there should be real users.
> 
> 
>>
>> How do they obtain a netmap-enabled QEMU?  Compile it from sources
>> themselves?
> 
> 
> Yes.

Hmm at least on the FreeBSD setup by vmtest (12.0-RELEASE r341666) we
don't need to build it from source:

$ make vm-build-freebsd V=1 DEBUG=1
[...]
netmap supportyes
[...]

$ fgrep -r CONFIG_NETMAP .
./config-host.mak:CONFIG_NETMAP=y

Re: [Qemu-devel] [PATCH v2 2/3] qcow2: rework the cluster compression routine

[Max Reitz]

On 04.07.19 15:09, Denis Plotnikov wrote:
> The patch allow to process image compression type defined
> in the image header and choose an appropriate method for
> image clusters (de)compression.
> 
> Signed-off-by: Denis Plotnikov 
> ---
>  block/qcow2.c | 93 ---
>  1 file changed, 73 insertions(+), 20 deletions(-)

I tried my best to rebase this patch on top of the current master, I
hope I did it right.

(A couple of the hunks in this patch are already in master.)

> diff --git a/block/qcow2.c b/block/qcow2.c
> index 8fa932a349..a107f76e98 100644
> --- a/block/qcow2.c
> +++ b/block/qcow2.c

[...]

> @@ -4059,8 +4062,8 @@ static ssize_t qcow2_compress(void *dest, size_t 
> dest_size,
>   * Returns: 0 on success
>   *  -1 on fail

This should be fixed to reflect the new return values.

>   */
> -static ssize_t qcow2_decompress(void *dest, size_t dest_size,
> -const void *src, size_t src_size)
> +static ssize_t qcow2_zlib_decompress(void *dest, size_t dest_size,
> + const void *src, size_t src_size)
>  {
>  int ret = 0;
>  z_stream strm;

[...]

> @@ -4153,20 +4156,67 @@ qcow2_co_do_compress(BlockDriverState *bs, void 
> *dest, size_t dest_size,
>  return arg.ret;
>  }
>  
> +/*
> + * qcow2_co_compress()
> + *
> + * Compress @src_size bytes of data using the compression
> + * method defined by the image compression type
> + *
> + * @dest - destination buffer, @dest_size bytes
> + * @src - source buffer, @src_size bytes
> + *
> + * Returns: 0 on success
> + *  a negative error code on fail
> + */
>  static ssize_t coroutine_fn
>  qcow2_co_compress(BlockDriverState *bs, void *dest, size_t dest_size,
>const void *src, size_t src_size)
>  {
> -return qcow2_co_do_compress(bs, dest, dest_size, src, src_size,
> -qcow2_compress);
> +BDRVQcow2State *s = bs->opaque;
> +Qcow2CompressFunc fn;
> +
> +switch (s->compression_type) {
> +case QCOW2_COMPRESSION_TYPE_ZLIB:
> +fn = qcow2_zlib_compress;
> +break;
> +
> +default:
> +return -ENOTSUP;

A plain abort() would work, too.  (Your choice.)

> +}
> +
> +return qcow2_co_do_compress(bs, dest, dest_size, src, src_size, fn);
>  }
>  
> +/*
> + * qcow2_co_decompress()
> + *
> + * Decompress some data (not more than @src_size bytes) to produce exactly
> + * @dest_size bytes using the compression method defined by the image
> + * compression type
> + *
> + * @dest - destination buffer, @dest_size bytes
> + * @src - source buffer, @src_size bytes
> + *
> + * Returns: 0 on success
> + *  a negative error code on fail
> + */
>  static ssize_t coroutine_fn
>  qcow2_co_decompress(BlockDriverState *bs, void *dest, size_t dest_size,
>  const void *src, size_t src_size)
>  {
> -return qcow2_co_do_compress(bs, dest, dest_size, src, src_size,
> -qcow2_decompress);
> +BDRVQcow2State *s = bs->opaque;
> +Qcow2CompressFunc fn;
> +
> +switch (s->compression_type) {
> +case QCOW2_COMPRESSION_TYPE_ZLIB:
> +fn = qcow2_zlib_decompress;
> +break;
> +
> +default:
> +return -ENOTSUP;

Same here.

Max

> +}
> +
> +return qcow2_co_do_compress(bs, dest, dest_size, src, src_size, fn);
>  }



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH v2 00/29] Tame a few "touch this, recompile the world" headers

[Markus Armbruster]

Alex Bennée  writes:

> Markus Armbruster  writes:
>
>> We have quite a few "touch this, recompile the world" headers.  My
>> "build everything" tree has some 6600 objects (not counting tests and
>> objects that don't depend on qemu/osdep.h).  Touching any of 54
>> headers triggers a recompile of more than half of them.
>>
>> This series reduces them to 46.
>
> I think this series is going the right way but there seems to be quite a
> lot of breakage introduced to the cross compiles:
>
>   https://app.shippable.com/github/stsquad/qemu/runs/939/summary/console
>
> I guess there is more subtlety involved when host != target. I'd
> recommend setting up a shippable account:
>
>   https://wiki.qemu.org/Testing/CI/Shippable

The failures I can see there are actually anything but subtle:

* 32 bit builds are broken

  Culprit is PATCH 04.  memory.h's inline functions need qemu/bswap.h.
  Before this patch, they reliably get it indirectly from
  hw/qdev-core.h.  After this patch, they still get it from
  qemu/int128.h, but only ifdef CONFIG_INT128.  Meh.  I hate conditional
  includion in headers.

  The obvious fix is to have memory.h include qemu/bswap.h directly.

  Perhaps we should make qemu/int128.h include it unconditionally
  regardless.

  Fun fact: I used the GCC compile farm's last 32 bit x86 box to debug
  this one.

* net/netmap.c is broken

  Philippe found it first :)

* target/i386/hax-all.c is broken

  Likewise.

* hw/intc/s390_flic_kvm.c is broken

  Patchew found it first.

> You can of course just run:
>
>   make docker-test-build J=n
>
> And watch your machine slowly grind through all the options.
>
> --
> Alex Bennée

Re: [Qemu-devel] Is network backend netmap worth keeping?

[Giuseppe Lettieri]

Dear Markus,

the netmap project is alive and well, if a bit understuffed. We have 
moved to github:


https://github.com/luigirizzo/netmap

We have users from FreeBSD, where it is part of the official kernel, and 
Linux, both from Academia and industry.


But you asked about the netmap backend in QEMU, in particular. When it 
was merged, the decision was made to disable it by default because it 
was not supported upstream in Linux. As Jason Wang says, this support is 
even more unlikely now than it was then.


The fact the the backend has to be explicitly enabled and built from the 
sources has obviously cut down the number of potential users. However, 
we still think it is useful and we have pending updates for it. If it's 
causing problems in the workflow, I am willing to help as much as I can.


Cheers,
Giuseppe

Il 08/08/19 06:48, Markus Armbruster ha scritto:

Please excuse the attention-grabbing subject.

Philippe Mathieu-Daudé  writes:


On 8/7/19 10:16 PM, Markus Armbruster wrote:

[...]

Can you tell me offhand what I have to install so configure enables
CONFIG_NETMAP?


The steps are listed in tests/docker/dockerfiles/debian-amd64.docker,
but you can get to this point running:

   $ make docker-image-debian-amd64 V=1 DEBUG=1

This will build the docker image with netmap (so you don't have to mess
with your workstation setup), then build QEMU within the image.


So, to make use of QEMU's netmap backend (CONFIG_NETMAP), you have to
build and install netmap software from sources.  Which pretty much
ensures nobody uses it.  It was added in commit 58952137b0b (Nov 2013).
The commit message points to ,
which gives me "connection timed out" right now.

On the other hand, it's covered in MAINTAINERS, and has seen
non-janitorial activity as late as Dec 2018 (commit c693fc748a).

Luigi, Giuseppe, Vincenzo, what's the status of the netmap project?

Why is the QEMU netmap backend worth keeping?

Who is using the netmap backend?

How do they obtain a netmap-enabled QEMU?  Compile it from sources
themselves?

Would it make sense to have netmap packaged in common Linux distros?




--
Dr. Ing. Giuseppe Lettieri
Dipartimento di Ingegneria della Informazione
Universita' di Pisa
Largo Lucio Lazzarino 1, 56122 Pisa - Italy
Ph. : (+39) 050-2217.649 (direct) .599 (switch)
Fax : (+39) 050-2217.600
e-mail: g.letti...@iet.unipi.it

[Qemu-devel] [PATCH] hw/ide/atapi: Use the ldst API

[Philippe Mathieu-Daudé]

The big-endian load/store functions are already provided
by "qemu/bswap.h".
Avoid code duplication, use the generic API.

Signed-off-by: Philippe Mathieu-Daudé 
---
 hw/ide/atapi.c | 80 ++
 1 file changed, 28 insertions(+), 52 deletions(-)

diff --git a/hw/ide/atapi.c b/hw/ide/atapi.c
index 1b0f66cc08..17a9d635d8 100644
--- a/hw/ide/atapi.c
+++ b/hw/ide/atapi.c
@@ -45,30 +45,6 @@ static void padstr8(uint8_t *buf, int buf_size, const char 
*src)
 }
 }
 
-static inline void cpu_to_ube16(uint8_t *buf, int val)
-{
-buf[0] = val >> 8;
-buf[1] = val & 0xff;
-}
-
-static inline void cpu_to_ube32(uint8_t *buf, unsigned int val)
-{
-buf[0] = val >> 24;
-buf[1] = val >> 16;
-buf[2] = val >> 8;
-buf[3] = val & 0xff;
-}
-
-static inline int ube16_to_cpu(const uint8_t *buf)
-{
-return (buf[0] << 8) | buf[1];
-}
-
-static inline int ube32_to_cpu(const uint8_t *buf)
-{
-return (buf[0] << 24) | (buf[1] << 16) | (buf[2] << 8) | buf[3];
-}
-
 static void lba_to_msf(uint8_t *buf, int lba)
 {
 lba += 150;
@@ -485,7 +461,7 @@ static inline uint8_t ide_atapi_set_profile(uint8_t *buf, 
uint8_t *index,
 uint8_t *buf_profile = buf + 12; /* start of profiles */
 
 buf_profile += ((*index) * 4); /* start of indexed profile */
-cpu_to_ube16 (buf_profile, profile);
+stw_be_p(buf_profile, profile);
 buf_profile[2] = ((buf_profile[0] == buf[6]) && (buf_profile[1] == 
buf[7]));
 
 /* each profile adds 4 bytes to the response */
@@ -518,9 +494,9 @@ static int ide_dvd_read_structure(IDEState *s, int format,
 buf[7] = 0;   /* default densities */
 
 /* FIXME: 0x3 per spec? */
-cpu_to_ube32(buf + 8, 0); /* start sector */
-cpu_to_ube32(buf + 12, total_sectors - 1); /* end sector */
-cpu_to_ube32(buf + 16, total_sectors - 1); /* l0 end sector */
+stl_be_p(buf + 8, 0); /* start sector */
+stl_be_p(buf + 12, total_sectors - 1); /* end sector */
+stl_be_p(buf + 16, total_sectors - 1); /* l0 end sector */
 
 /* Size of buffer, not including 2 byte size field */
 stw_be_p(buf, 2048 + 2);
@@ -839,7 +815,7 @@ static void cmd_get_configuration(IDEState *s, uint8_t *buf)
 }
 
 /* XXX: could result in alignment problems in some architectures */
-max_len = ube16_to_cpu(buf + 7);
+max_len = lduw_be_p(buf + 7);
 
 /*
  * XXX: avoid overflow for io_buffer if max_len is bigger than
@@ -859,16 +835,16 @@ static void cmd_get_configuration(IDEState *s, uint8_t 
*buf)
  * to use as current.  0 means there is no media
  */
 if (media_is_dvd(s)) {
-cpu_to_ube16(buf + 6, MMC_PROFILE_DVD_ROM);
+stw_be_p(buf + 6, MMC_PROFILE_DVD_ROM);
 } else if (media_is_cd(s)) {
-cpu_to_ube16(buf + 6, MMC_PROFILE_CD_ROM);
+stw_be_p(buf + 6, MMC_PROFILE_CD_ROM);
 }
 
 buf[10] = 0x02 | 0x01; /* persistent and current */
 len = 12; /* headers: 8 + 4 */
 len += ide_atapi_set_profile(buf, , MMC_PROFILE_DVD_ROM);
 len += ide_atapi_set_profile(buf, , MMC_PROFILE_CD_ROM);
-cpu_to_ube32(buf, len - 4); /* data length */
+stl_be_p(buf, len - 4); /* data length */
 
 ide_atapi_cmd_reply(s, len, max_len);
 }
@@ -878,7 +854,7 @@ static void cmd_mode_sense(IDEState *s, uint8_t *buf)
 int action, code;
 int max_len;
 
-max_len = ube16_to_cpu(buf + 7);
+max_len = lduw_be_p(buf + 7);
 action = buf[2] >> 6;
 code = buf[2] & 0x3f;
 
@@ -886,7 +862,7 @@ static void cmd_mode_sense(IDEState *s, uint8_t *buf)
 case 0: /* current values */
 switch(code) {
 case MODE_PAGE_R_W_ERROR: /* error recovery */
-cpu_to_ube16([0], 16 - 2);
+stw_be_p([0], 16 - 2);
 buf[2] = 0x70;
 buf[3] = 0;
 buf[4] = 0;
@@ -905,7 +881,7 @@ static void cmd_mode_sense(IDEState *s, uint8_t *buf)
 ide_atapi_cmd_reply(s, 16, max_len);
 break;
 case MODE_PAGE_AUDIO_CTL:
-cpu_to_ube16([0], 24 - 2);
+stw_be_p([0], 24 - 2);
 buf[2] = 0x70;
 buf[3] = 0;
 buf[4] = 0;
@@ -924,7 +900,7 @@ static void cmd_mode_sense(IDEState *s, uint8_t *buf)
 ide_atapi_cmd_reply(s, 24, max_len);
 break;
 case MODE_PAGE_CAPABILITIES:
-cpu_to_ube16([0], 30 - 2);
+stw_be_p([0], 30 - 2);
 buf[2] = 0x70;
 buf[3] = 0;
 buf[4] = 0;
@@ -946,11 +922,11 @@ static void cmd_mode_sense(IDEState *s, uint8_t *buf)
 buf[14] |= 1 << 1;
 }
 buf[15] = 0x00; /* No volume & mute control, no changer */
-cpu_to_ube16([16], 704); /* 4x read speed */
+stw_be_p([16], 704); /* 4x read speed */
 buf[18] = 0; /* Two volume levels */
 buf[19] = 2;

Re: [Qemu-devel] [PATCH-for-4.2 v1 1/9] s390x/mmu: Better ASC selection in s390_cpu_get_phys_page_debug()

[David Hildenbrand]

On 08.08.19 14:57, Cornelia Huck wrote:
> On Mon,  5 Aug 2019 17:29:39 +0200
> David Hildenbrand  wrote:
> 
>> Let's select the ASC before calling the function and use MMU_DATA_LOAD.
>> This is a preparation to:
>> - Remove the ASC magic depending on the access mode from mmu_translate
>> - Implement IEP support, where we could run into access exceptions
> 
> 'IEP' was instruction execution protection?
> 
>>   trying to fetch instructions
>>
>> Signed-off-by: David Hildenbrand 
>> ---
>>  target/s390x/helper.c | 10 +-
>>  1 file changed, 9 insertions(+), 1 deletion(-)
>>
>> diff --git a/target/s390x/helper.c b/target/s390x/helper.c
>> index 13ae9909ad..08166558a0 100644
>> --- a/target/s390x/helper.c
>> +++ b/target/s390x/helper.c
>> @@ -58,7 +58,15 @@ hwaddr s390_cpu_get_phys_page_debug(CPUState *cs, vaddr 
>> vaddr)
>>  vaddr &= 0x7fff;
>>  }
>>  
>> -if (mmu_translate(env, vaddr, MMU_INST_FETCH, asc, , , 
>> false)) {
>> +/*
>> + * We want to read the code, however, not run into access exceptions
>> + * (especially, IEP).
>> + */
>> +if (asc != PSW_ASC_HOME) {
>> +asc = PSW_ASC_PRIMARY;
>> +}
> 
> Previously, if we'd go in here specifying access register mode, we'd
> hw_error() out. Now, that would be rewritten to using primary. Could
> that be a problem, or do we filter out access register mode even
> earlier?

As this is just a debug interface it's not an issue (it actually makes
sure that we really read code and not data). Any guest that would be
trying to read data while in AR mode would immediately crash.

> 
> (As an aside, I'm not sure the guest crashing qemu by specifying access
> register mode was a good idea. Or do we get to slap the guest before
> that happens?)

I guess there isn't too much we can do - continue running the guest in a
wrong mode would lead to data corruption :/ And as it's core ISA, we
cannot really forbid switching to it. The only solution is to implement
AR mode :D

-- 

Thanks,

David / dhildenb

Re: [Qemu-devel] [PATCH-for-4.2 v1 1/9] s390x/mmu: Better ASC selection in s390_cpu_get_phys_page_debug()

[Cornelia Huck]

On Mon,  5 Aug 2019 17:29:39 +0200
David Hildenbrand  wrote:

> Let's select the ASC before calling the function and use MMU_DATA_LOAD.
> This is a preparation to:
> - Remove the ASC magic depending on the access mode from mmu_translate
> - Implement IEP support, where we could run into access exceptions

'IEP' was instruction execution protection?

>   trying to fetch instructions
> 
> Signed-off-by: David Hildenbrand 
> ---
>  target/s390x/helper.c | 10 +-
>  1 file changed, 9 insertions(+), 1 deletion(-)
> 
> diff --git a/target/s390x/helper.c b/target/s390x/helper.c
> index 13ae9909ad..08166558a0 100644
> --- a/target/s390x/helper.c
> +++ b/target/s390x/helper.c
> @@ -58,7 +58,15 @@ hwaddr s390_cpu_get_phys_page_debug(CPUState *cs, vaddr 
> vaddr)
>  vaddr &= 0x7fff;
>  }
>  
> -if (mmu_translate(env, vaddr, MMU_INST_FETCH, asc, , , 
> false)) {
> +/*
> + * We want to read the code, however, not run into access exceptions
> + * (especially, IEP).
> + */
> +if (asc != PSW_ASC_HOME) {
> +asc = PSW_ASC_PRIMARY;
> +}

Previously, if we'd go in here specifying access register mode, we'd
hw_error() out. Now, that would be rewritten to using primary. Could
that be a problem, or do we filter out access register mode even
earlier?

(As an aside, I'm not sure the guest crashing qemu by specifying access
register mode was a good idea. Or do we get to slap the guest before
that happens?)

> +
> +if (mmu_translate(env, vaddr, MMU_DATA_LOAD, asc, , , false)) 
> {
>  return -1;
>  }
>  return raddr;

Re: [Qemu-devel] [Virtio-fs] [PATCH 0/4] virtiofsd: multithreading preparation part 3

[Vivek Goyal]

On Thu, Aug 08, 2019 at 10:53:16AM +0100, Dr. David Alan Gilbert wrote:
> * Stefan Hajnoczi (stefa...@redhat.com) wrote:
> > On Wed, Aug 07, 2019 at 04:57:15PM -0400, Vivek Goyal wrote:
> > > Kernel also serializes MAP/UNMAP on one inode. So you will need to run
> > > multiple jobs operating on different inodes to see parallel MAP/UNMAP
> > > (atleast from kernel's point of view).
> > 
> > Okay, there is still room to experiment with how MAP and UNMAP are
> > handled by virtiofsd and QEMU even if the host kernel ultimately becomes
> > the bottleneck.
> > 
> > One possible optimization is to eliminate REMOVEMAPPING requests when
> > the guest driver knows a SETUPMAPPING will follow immediately.  I see
> > the following request pattern in a fio randread iodepth=64 job:
> > 
> >   unique: 995348, opcode: SETUPMAPPING (48), nodeid: 135, insize: 80, pid: 
> > 1351
> >   lo_setupmapping(ino=135, fi=0x(nil), foffset=3860856832, len=2097152, 
> > moffset=859832320, flags=0)
> >  unique: 995348, success, outsize: 16
> >   unique: 995350, opcode: REMOVEMAPPING (49), nodeid: 135, insize: 60, pid: 
> > 12
> >  unique: 995350, success, outsize: 16
> >   unique: 995352, opcode: SETUPMAPPING (48), nodeid: 135, insize: 80, pid: 
> > 1351
> >   lo_setupmapping(ino=135, fi=0x(nil), foffset=16777216, len=2097152, 
> > moffset=861929472, flags=0)
> >  unique: 995352, success, outsize: 16
> >   unique: 995354, opcode: REMOVEMAPPING (49), nodeid: 135, insize: 60, pid: 
> > 12
> >  unique: 995354, success, outsize: 16
> >   virtio_send_msg: elem 9: with 1 in desc of length 16
> >   unique: 995356, opcode: SETUPMAPPING (48), nodeid: 135, insize: 80, pid: 
> > 1351
> >   lo_setupmapping(ino=135, fi=0x(nil), foffset=383778816, len=2097152, 
> > moffset=864026624, flags=0)
> >  unique: 995356, success, outsize: 16
> >   unique: 995358, opcode: REMOVEMAPPING (49), nodeid: 135, insize: 60, pid: 
> > 12
> > 
> > The REMOVEMAPPING requests are unnecessary since we can map over the top
> > of the old mapping instead of taking the extra step of removing it
> > first.
> 
> Yep, those should go - I think Vivek likes to keep them for testing
> since they make things fail more completely if there's a screwup.

I like to keep them because otherwise they keep the resources busy
on host. If DAX range is being used immediately, then this optimization
makes more sense. I will keep this in mind.

> 
> > Some more questions to consider for DAX performance optimization:
> > 
> > 1. Is FUSE_READ/FUSE_WRITE more efficient than DAX for some I/O patterns?
> 
> Probably for cases where the data is only accessed once, and you can't
> preemptively map.
> Another variant on (1) is whether we could do read/writes while the mmap
> is happening to absorb the latency.

For small random I/O, dax might not be very effective. Overhead of
setting up mapping and tearing it down is significant.

Vivek

Re: [Qemu-devel] [PATCH v2 1/3] qcow2: introduce compression type feature

[Max Reitz]

On 08.08.19 02:18, Eric Blake wrote:
> On 7/4/19 8:09 AM, Denis Plotnikov wrote:
>> The patch adds some preparation parts for incompatible compression type
>> feature to QCOW2 header that indicates that *all* compressed clusters
>> must be (de)compressed using a certain compression type.
>>
>> It is implied that the compression type is set on the image creation and
>> can be changed only later by image conversion, thus compression type
>> defines the only compression algorithm used for the image.
>>
>> The goal of the feature is to add support of other compression algorithms
>> to qcow2. For example, ZSTD which is more effective on compression than ZLIB.
>> It works roughly 2x faster than ZLIB providing a comparable compression ratio
>> and therefore provide a performance advantage in backup scenarios.
> 
> provides
> 
>>
>> The default compression is ZLIB. Images created with ZLIB compression type
>> are backward compatible with older qemu versions.
>>
>> Signed-off-by: Denis Plotnikov 
> 
>> +++ b/docs/interop/qcow2.txt
>> @@ -109,7 +109,12 @@ in the description of a field.
>>  An External Data File Name header extension 
>> may
>>  be present if this bit is set.
>>  
>> -Bits 3-63:  Reserved (set to 0)
>> +Bit 3:  Compression type bit. The bit must be set if
>> +the compression type differs from default: 
>> zlib.
> 
> I'd word this 'from the default of zlib.'
> 
>> +If the compression type is default the bit 
>> must
>> +be unset.
> 
> Why? I see no reason to forbid a qcow2 image that has the incompatible
> bit set but still uses zlib compression.  True, such an image is not as
> friendly to older clients, but it is not technically wrong, and newer
> clients would still be able to use the image if not for this sentence
> telling them they must not.

Just because an image doesn’t adhere to the specification doesn’t mean
you have to reject it, if the intention is clear.

> I'd drop this sentence.

I wouldn’t, I like it (in essence).  Though maybe the “must” is indeed
too strong and it should be a “should” instead.

Max



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH v2 1/3] qcow2: introduce compression type feature

[Max Reitz]

On 08.08.19 02:09, Eric Blake wrote:
> On 8/7/19 6:12 PM, Max Reitz wrote:
> 
>>>  
>>> +static int check_compression_type(BDRVQcow2State *s, Error **errp)
>>> +{
>>> +switch (s->compression_type) {
>>> +case QCOW2_COMPRESSION_TYPE_ZLIB:
>>> +break;
>>> +
>>> +default:
>>> +error_setg(errp, "qcow2: unknown compression type: %u",
>>> +   s->compression_type);
>>> +return -ENOTSUP;
>>> +}
>>> +
>>> +/*
>>> + * if the compression type differs from QCOW2_COMPRESSION_TYPE_ZLIB
>>> + * the incompatible feature flag must be set
>>> + */
>>> +
>>> +if (s->compression_type != QCOW2_COMPRESSION_TYPE_ZLIB &&
>>> +!(s->incompatible_features & QCOW2_INCOMPAT_COMPRESSION_TYPE)) {
>>> +error_setg(errp, "qcow2: Invalid compression type setting");
>>> +return -EINVAL;
>>
>> (1) Why is this block indented twice?
>>
>> (2) Do we need this at all?  Sure, it’s a corruption, but do we need to
>> reject the image because of it?
> 
> Yes, because otherwise there is a high risk of some application
> misinterpreting the contents (whether an older qemu that silently
> ignores unrecognized headers, and so assumes it can decode compressed
> clusters with zlib even though the decode will only succeed with zstd,
> or can write a compressed cluster with zlib which then causes corruption
> when the newer qemu tries to read it with zstd).  The whole point of an
> incompatible bit is to reject opening an image that can't be interpreted
> correctly, and where writing may break later readers.

Fair enough.

>>> +}
>>> +
>>> +return 0;
>>> +}
>>> +
>>
>> Overall, I don’t see the purpose of this function.  I don’t see any need
>> to call it in qcow2_update_header().  And without “does non-zlib
>> compression imply the respective incompatible flag?” check, you can just
>> inline the rest (checking that we recognize the compression type) into
>> qcow2_do_open().
>>
> 
> Inlining may indeed be possible; the real question is whether the
> function expands later in the series to the point that inlining no
> longer makes sense.

A quick search says no.

Max



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [RFC PATCH 1/6] utils/python_api: add scripting interface for Qemu with python lib

[Philippe Mathieu-Daudé]

On 8/8/19 12:49 PM, Daniel P. Berrangé wrote:
> On Wed, Aug 07, 2019 at 12:44:40PM +0530, Balamuruhan S wrote:
>> Adds scripting interface with python library to call functions in
>> python modules from Qemu that can be used to feed input externally
>> and without recompiling Qemu that can be used for early development,
>> testing and can be extended to abstract some of Qemu code out to a
>> python script to ease maintenance.
> 
> I admit the use case is interesting, but this is opening a can of
> worms...
> 
> Historically the project has held the view that we do not wish
> to have an mechanism to support loading out of tree code into the
> QEMU process. Much previously talk was around dlopen'd C plugins,
> but dynanically loaded Python plugins are doing the same thing
> at a conceptual level.
> 
> We didn't wish to expose internals of QEMU in a plugin API to
> avoid having any kind of API promise across releases.
> 
> There was also the question of licensing with plugins opening
> the door for people to extend QEMU with non-free/closed source
> functionality.
> 
> While this series only uses the plugin for one fairly obscure
> device, once a python plugin feature is intergrated in QEMU
> there will inevitably be requests to use it in further areas
> of QEMU.
> 
> IOW, acceptance of this patch is a significant question for
> the project, and a broader discussion point, than just this
> PPC feature patch series.

Since performance is not an issue, we can use a QMP-PyMMIO bridge.
Most of the functions required are already exposed, Damien completed the
missing ones in his 'FAULT INJECTION FRAMEWORK' series:
https://lists.gnu.org/archive/html/qemu-devel/2019-06/msg06230.html

Maybe we simply need a clearer (better documented) QMP 'MMIO' API?