Re: [Qemu-devel] ARM: SVE while issue

[Laurent Desnogues]

Hello,

On Tue, Jul 31, 2018 at 6:36 PM, Richard Henderson
 wrote:
> On 07/31/2018 09:52 AM, Laurent Desnogues wrote:
>> Hello Richard,
>>
>> according to SVE specification, whilels/whilele instructions have a
>> special case where if the second operand is the maximum (un)signed
>> integer then the result is an all-true predicate.  The current code in
>> trans_WHILE doesn't seem to capture that requirement.  I'm afraid the
>> fix won't be as trivial as for the other two bugs I reported :-)
>
> Still not too bad.
> Testing a fix for my branch now.

I gave it a look and tested it, it works fine!

Thanks,

Laurent

> Annoying that risu didn't randomly generate this case,
> and I totally mis-read the documentation for this.
>
>
> r~
>

Re: [Qemu-devel] [PATCH v2] target/ppc: simplify bcdadd/sub functions

[David Gibson]

On Mon, Jul 30, 2018 at 05:09:17PM +, Yasmin Beatriz wrote:
> After solving a corner case in bcdsub, this patch simplifies the logic
> of both bcdadd/sub instructions by removing some unnecessary local flags.
> This commit also rearranges some if-else conditions in bcdadd to make it
> easier to read.
> 
> Signed-off-by: Yasmin Beatriz 

Applied to ppc-for-3.1, thanks.

> ---
>  target/ppc/int_helper.c | 49 
> ++---
>  1 file changed, 18 insertions(+), 31 deletions(-)
> 
> diff --git a/target/ppc/int_helper.c b/target/ppc/int_helper.c
> index fa18e6e..1906f22 100644
> --- a/target/ppc/int_helper.c
> +++ b/target/ppc/int_helper.c
> @@ -2671,16 +2671,14 @@ static int bcd_cmp_mag(ppc_avr_t *a, ppc_avr_t *b)
>  return 0;
>  }
>  
> -static int bcd_add_mag(ppc_avr_t *t, ppc_avr_t *a, ppc_avr_t *b, int 
> *invalid,
> +static void bcd_add_mag(ppc_avr_t *t, ppc_avr_t *a, ppc_avr_t *b, int 
> *invalid,
> int *overflow)
>  {
>  int carry = 0;
>  int i;
> -int is_zero = 1;
>  for (i = 1; i <= 31; i++) {
>  uint8_t digit = bcd_get_digit(a, i, invalid) +
>  bcd_get_digit(b, i, invalid) + carry;
> -is_zero &= (digit == 0);
>  if (digit > 9) {
>  carry = 1;
>  digit -= 10;
> @@ -2689,26 +2687,20 @@ static int bcd_add_mag(ppc_avr_t *t, ppc_avr_t *a, 
> ppc_avr_t *b, int *invalid,
>  }
>  
>  bcd_put_digit(t, digit, i);
> -
> -if (unlikely(*invalid)) {
> -return -1;
> -}
>  }
>  
>  *overflow = carry;
> -return is_zero;
>  }
>  
> -static int bcd_sub_mag(ppc_avr_t *t, ppc_avr_t *a, ppc_avr_t *b, int 
> *invalid,
> +static void bcd_sub_mag(ppc_avr_t *t, ppc_avr_t *a, ppc_avr_t *b, int 
> *invalid,
> int *overflow)
>  {
>  int carry = 0;
>  int i;
> -int is_zero = 1;
> +
>  for (i = 1; i <= 31; i++) {
>  uint8_t digit = bcd_get_digit(a, i, invalid) -
>  bcd_get_digit(b, i, invalid) + carry;
> -is_zero &= (digit == 0);
>  if (digit & 0x80) {
>  carry = -1;
>  digit += 10;
> @@ -2717,14 +2709,9 @@ static int bcd_sub_mag(ppc_avr_t *t, ppc_avr_t *a, 
> ppc_avr_t *b, int *invalid,
>  }
>  
>  bcd_put_digit(t, digit, i);
> -
> -if (unlikely(*invalid)) {
> -return -1;
> -}
>  }
>  
>  *overflow = carry;
> -return is_zero;
>  }
>  
>  uint32_t helper_bcdadd(ppc_avr_t *r,  ppc_avr_t *a, ppc_avr_t *b, uint32_t 
> ps)
> @@ -2734,26 +2721,28 @@ uint32_t helper_bcdadd(ppc_avr_t *r,  ppc_avr_t *a, 
> ppc_avr_t *b, uint32_t ps)
>  int sgnb = bcd_get_sgn(b);
>  int invalid = (sgna == 0) || (sgnb == 0);
>  int overflow = 0;
> -int zero = 0;
>  uint32_t cr = 0;
>  ppc_avr_t result = { .u64 = { 0, 0 } };
>  
>  if (!invalid) {
>  if (sgna == sgnb) {
>  result.u8[BCD_DIG_BYTE(0)] = bcd_preferred_sgn(sgna, ps);
> -zero = bcd_add_mag(, a, b, , );
> -cr = (sgna > 0) ? CRF_GT : CRF_LT;
> -} else if (bcd_cmp_mag(a, b) > 0) {
> -result.u8[BCD_DIG_BYTE(0)] = bcd_preferred_sgn(sgna, ps);
> -zero = bcd_sub_mag(, a, b, , );
> -cr = (sgna > 0) ? CRF_GT : CRF_LT;
> -} else if (bcd_cmp_mag(a, b) == 0) {
> -result.u8[BCD_DIG_BYTE(0)] = bcd_preferred_sgn(0, ps);
> -zero = bcd_sub_mag(, b, a, , );
> +bcd_add_mag(, a, b, , );
> +cr = bcd_cmp_zero();
>  } else {
> -result.u8[BCD_DIG_BYTE(0)] = bcd_preferred_sgn(sgnb, ps);
> -zero = bcd_sub_mag(, b, a, , );
> -cr = (sgnb > 0) ? CRF_GT : CRF_LT;
> +int magnitude = bcd_cmp_mag(a, b);
> +if (magnitude > 0) {
> +result.u8[BCD_DIG_BYTE(0)] = bcd_preferred_sgn(sgna, ps);
> +bcd_sub_mag(, a, b, , );
> +cr = (sgna > 0) ? CRF_GT : CRF_LT;
> +} else if (magnitude < 0) {
> +result.u8[BCD_DIG_BYTE(0)] = bcd_preferred_sgn(sgnb, ps);
> +bcd_sub_mag(, b, a, , );
> +cr = (sgnb > 0) ? CRF_GT : CRF_LT;
> +} else {
> +result.u8[BCD_DIG_BYTE(0)] = bcd_preferred_sgn(0, ps);
> +cr = CRF_EQ;
> +}
>  }
>  }
>  
> @@ -2762,8 +2751,6 @@ uint32_t helper_bcdadd(ppc_avr_t *r,  ppc_avr_t *a, 
> ppc_avr_t *b, uint32_t ps)
>  cr = CRF_SO;
>  } else if (overflow) {
>  cr |= CRF_SO;
> -} else if (zero) {
> -cr = CRF_EQ;
>  }
>  
>  *r = result;

-- 
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 v6 1/4] spapr: Add a pseries-3.1 machine type

[David Gibson]

On Mon, Jul 30, 2018 at 04:11:31PM +0200, Cédric Le Goater wrote:
> Signed-off-by: Cédric Le Goater 

Applied to ppc-for-3.1, thanks.

> ---
>  include/hw/compat.h |  3 +++
>  hw/ppc/spapr.c  | 23 +--
>  2 files changed, 24 insertions(+), 2 deletions(-)
> 
> diff --git a/include/hw/compat.h b/include/hw/compat.h
> index c08f4040bb80..6f4d5fc64704 100644
> --- a/include/hw/compat.h
> +++ b/include/hw/compat.h
> @@ -1,6 +1,9 @@
>  #ifndef HW_COMPAT_H
>  #define HW_COMPAT_H
>  
> +#define HW_COMPAT_3_0 \
> +/* empty */
> +
>  #define HW_COMPAT_2_12 \
>  {\
>  .driver   = "migration",\
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index 421b2dd09b51..3c72173c7e0f 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -4058,19 +4058,38 @@ static const TypeInfo spapr_machine_info = {
>  }\
>  type_init(spapr_machine_register_##suffix)
>  
> + /*
> + * pseries-3.1
> + */
> +static void spapr_machine_3_1_instance_options(MachineState *machine)
> +{
> +}
> +
> +static void spapr_machine_3_1_class_options(MachineClass *mc)
> +{
> +/* Defaults for the latest behaviour inherited from the base class */
> +}
> +
> +DEFINE_SPAPR_MACHINE(3_1, "3.1", true);
> +
>  /*
>   * pseries-3.0
>   */
> +#define SPAPR_COMPAT_3_0  \
> +HW_COMPAT_3_0
> +
>  static void spapr_machine_3_0_instance_options(MachineState *machine)
>  {
> +spapr_machine_3_1_instance_options(machine);
>  }
>  
>  static void spapr_machine_3_0_class_options(MachineClass *mc)
>  {
> -/* Defaults for the latest behaviour inherited from the base class */
> +spapr_machine_3_1_class_options(mc);
> +SET_MACHINE_COMPAT(mc, SPAPR_COMPAT_3_0);
>  }
>  
> -DEFINE_SPAPR_MACHINE(3_0, "3.0", true);
> +DEFINE_SPAPR_MACHINE(3_0, "3.0", false);
>  
>  /*
>   * pseries-2.12

-- 
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 for-3.1] xics: don't include "target/ppc/cpu-qom.h" in "hw/ppc/xics.h"

[David Gibson]

On Tue, Jul 31, 2018 at 12:56:49PM +0200, Greg Kurz wrote:
> The last user of the PowerPCCPU typedef in "hw/ppc/xics.h" vanished with
> commit b1fd36c363d73969841468146ebfb9fd84a5ee52. It isn't necessary to
> include "target/ppc/cpu-qom.h" there anymore.
> 
> Signed-off-by: Greg Kurz 

Applied to ppc-for-3.1.

> ---
>  include/hw/ppc/xics.h |1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/include/hw/ppc/xics.h b/include/hw/ppc/xics.h
> index 6ac8a9392da6..9c2916c9b23a 100644
> --- a/include/hw/ppc/xics.h
> +++ b/include/hw/ppc/xics.h
> @@ -29,7 +29,6 @@
>  #define XICS_H
>  
>  #include "hw/qdev.h"
> -#include "target/ppc/cpu-qom.h"
>  
>  #define XICS_IPI0x2
>  #define XICS_BUID   0x1
> 

-- 
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] [PULL 2/2] sam460ex: Fix PCI interrupts with multiple devices

[David Gibson]

From: BALATON Zoltan 

The four interrupts of the PCI bus are connected to the same UIC pin
on the real Sam460ex. Evidence for this can be found in the UBoot
source for the Sam460ex in the Sam460ex.c file where
PCI_INTERRUPT_LINE is written. Change the ppc440_pcix model to behave
more like this.

This fixes the problem that can be observed when adding further PCI
cards that got their interrupt rotated to other interrupts than PCI
INT A. In particular, the bug was observed with an additional OHCI PCI
card or an ES1370 sound device.

Signed-off-by: Sebastian Bauer 
Signed-off-by: BALATON Zoltan 
Tested-by: Sebastian Bauer 
Signed-off-by: David Gibson 
---
 hw/ppc/ppc440_pcix.c | 21 -
 hw/ppc/sam460ex.c|  6 ++
 2 files changed, 10 insertions(+), 17 deletions(-)

diff --git a/hw/ppc/ppc440_pcix.c b/hw/ppc/ppc440_pcix.c
index d8af04b70f..64ed07afa6 100644
--- a/hw/ppc/ppc440_pcix.c
+++ b/hw/ppc/ppc440_pcix.c
@@ -57,7 +57,7 @@ typedef struct PPC440PCIXState {
 struct PLBOutMap pom[PPC440_PCIX_NR_POMS];
 struct PLBInMap pim[PPC440_PCIX_NR_PIMS];
 uint32_t sts;
-qemu_irq irq[PCI_NUM_PINS];
+qemu_irq irq;
 AddressSpace bm_as;
 MemoryRegion bm;
 
@@ -418,21 +418,20 @@ static void ppc440_pcix_reset(DeviceState *dev)
  * This may need further refactoring for other boards. */
 static int ppc440_pcix_map_irq(PCIDevice *pci_dev, int irq_num)
 {
-int slot = pci_dev->devfn >> 3;
-trace_ppc440_pcix_map_irq(pci_dev->devfn, irq_num, slot);
-return slot - 1;
+trace_ppc440_pcix_map_irq(pci_dev->devfn, irq_num, 0);
+return 0;
 }
 
 static void ppc440_pcix_set_irq(void *opaque, int irq_num, int level)
 {
-qemu_irq *pci_irqs = opaque;
+qemu_irq *pci_irq = opaque;
 
 trace_ppc440_pcix_set_irq(irq_num);
 if (irq_num < 0) {
 error_report("%s: PCI irq %d", __func__, irq_num);
 return;
 }
-qemu_set_irq(pci_irqs[irq_num], level);
+qemu_set_irq(*pci_irq, level);
 }
 
 static AddressSpace *ppc440_pcix_set_iommu(PCIBus *b, void *opaque, int devfn)
@@ -471,19 +470,15 @@ static int ppc440_pcix_initfn(SysBusDevice *dev)
 {
 PPC440PCIXState *s;
 PCIHostState *h;
-int i;
 
 h = PCI_HOST_BRIDGE(dev);
 s = PPC440_PCIX_HOST_BRIDGE(dev);
 
-for (i = 0; i < ARRAY_SIZE(s->irq); i++) {
-sysbus_init_irq(dev, >irq[i]);
-}
-
+sysbus_init_irq(dev, >irq);
 memory_region_init(>busmem, OBJECT(dev), "pci bus memory", UINT64_MAX);
 h->bus = pci_register_root_bus(DEVICE(dev), NULL, ppc440_pcix_set_irq,
- ppc440_pcix_map_irq, s->irq, >busmem,
- get_system_io(), PCI_DEVFN(0, 0), 4, TYPE_PCI_BUS);
+ ppc440_pcix_map_irq, >irq, >busmem,
+ get_system_io(), PCI_DEVFN(0, 0), 1, TYPE_PCI_BUS);
 
 s->dev = pci_create_simple(h->bus, PCI_DEVFN(0, 0), "ppc4xx-host-bridge");
 
diff --git a/hw/ppc/sam460ex.c b/hw/ppc/sam460ex.c
index 0999efcc1e..9c77183006 100644
--- a/hw/ppc/sam460ex.c
+++ b/hw/ppc/sam460ex.c
@@ -515,10 +515,8 @@ static void sam460ex_init(MachineState *machine)
 
 /* PCI bus */
 ppc460ex_pcie_init(env);
-/* FIXME: is this correct? */
-dev = sysbus_create_varargs("ppc440-pcix-host", 0xc0ec0,
-uic[1][0], uic[1][20], uic[1][21], uic[1][22],
-NULL);
+/* All PCI irqs are connected to the same UIC pin (cf. UBoot source) */
+dev = sysbus_create_simple("ppc440-pcix-host", 0xc0ec0, uic[1][0]);
 pci_bus = (PCIBus *)qdev_get_child_bus(dev, "pci.0");
 if (!pci_bus) {
 error_report("couldn't create PCI controller!");
-- 
2.17.1

[Qemu-devel] [PULL 1/2] hw/misc/macio: Fix device introspection problems in macio devices

[David Gibson]

From: Thomas Huth 

Valgrind reports an error when introspecting the macio devices, e.g.:

echo "{'execute':'qmp_capabilities'} {'execute':'device-list-properties'," \
 "'arguments':{'typename':'macio-newworld'}}" \
 "{'execute': 'human-monitor-command', " \
 "'arguments': {'command-line': 'info qtree'}}" | \
 valgrind -q ppc64-softmmu/qemu-system-ppc64 -M none,accel=qtest -qmp stdio
[...]
==30768== Invalid read of size 8
==30768==at 0x5BC1EA: qdev_print (qdev-monitor.c:686)
==30768==by 0x5BC1EA: qbus_print (qdev-monitor.c:719)
==30768==by 0x43E458: handle_hmp_command (monitor.c:3446)
[...]

Use the new function sysbus_init_child_obj() to initialize the objects
here, to get the reference counting of the objects right, so that they
are cleaned up correctly when the parent gets removed.

Signed-off-by: Thomas Huth 
Signed-off-by: David Gibson 
---
 hw/misc/macio/cuda.c  |  5 ++---
 hw/misc/macio/macio.c | 24 
 hw/misc/macio/pmu.c   |  5 ++---
 3 files changed, 12 insertions(+), 22 deletions(-)

diff --git a/hw/misc/macio/cuda.c b/hw/misc/macio/cuda.c
index 9651ed9744..c4f7a2f39b 100644
--- a/hw/misc/macio/cuda.c
+++ b/hw/misc/macio/cuda.c
@@ -554,9 +554,8 @@ static void cuda_init(Object *obj)
 CUDAState *s = CUDA(obj);
 SysBusDevice *sbd = SYS_BUS_DEVICE(obj);
 
-object_initialize(>mos6522_cuda, sizeof(s->mos6522_cuda),
-  TYPE_MOS6522_CUDA);
-qdev_set_parent_bus(DEVICE(>mos6522_cuda), sysbus_get_default());
+sysbus_init_child_obj(obj, "mos6522-cuda", >mos6522_cuda,
+  sizeof(s->mos6522_cuda), TYPE_MOS6522_CUDA);
 
 memory_region_init_io(>mem, obj, _cuda_ops, s, "cuda", 0x2000);
 sysbus_init_mmio(sbd, >mem);
diff --git a/hw/misc/macio/macio.c b/hw/misc/macio/macio.c
index d135e3bc2b..52aa3775f4 100644
--- a/hw/misc/macio/macio.c
+++ b/hw/misc/macio/macio.c
@@ -209,14 +209,11 @@ static void macio_oldworld_realize(PCIDevice *d, Error 
**errp)
 static void macio_init_ide(MacIOState *s, MACIOIDEState *ide, size_t ide_size,
int index)
 {
-gchar *name;
+gchar *name = g_strdup_printf("ide[%i]", index);
 
-object_initialize(ide, ide_size, TYPE_MACIO_IDE);
-qdev_set_parent_bus(DEVICE(ide), sysbus_get_default());
+sysbus_init_child_obj(OBJECT(s), name, ide, ide_size, TYPE_MACIO_IDE);
 memory_region_add_subregion(>bar, 0x1f000 + ((index + 1) * 0x1000),
 >mem);
-name = g_strdup_printf("ide[%i]", index);
-object_property_add_child(OBJECT(s), name, OBJECT(ide), NULL);
 g_free(name);
 }
 
@@ -232,9 +229,7 @@ static void macio_oldworld_init(Object *obj)
  qdev_prop_allow_set_link_before_realize,
  0, NULL);
 
-object_initialize(>cuda, sizeof(s->cuda), TYPE_CUDA);
-qdev_set_parent_bus(DEVICE(>cuda), sysbus_get_default());
-object_property_add_child(obj, "cuda", OBJECT(>cuda), NULL);
+sysbus_init_child_obj(obj, "cuda", >cuda, sizeof(s->cuda), TYPE_CUDA);
 
 object_initialize(>nvram, sizeof(os->nvram), TYPE_MACIO_NVRAM);
 dev = DEVICE(>nvram);
@@ -390,8 +385,8 @@ static void macio_newworld_init(Object *obj)
  qdev_prop_allow_set_link_before_realize,
  0, NULL);
 
-object_initialize(>gpio, sizeof(ns->gpio), TYPE_MACIO_GPIO);
-qdev_set_parent_bus(DEVICE(>gpio), sysbus_get_default());
+sysbus_init_child_obj(obj, "gpio", >gpio, sizeof(ns->gpio),
+  TYPE_MACIO_GPIO);
 
 for (i = 0; i < 2; i++) {
 macio_init_ide(s, >ide[i], sizeof(ns->ide[i]), i);
@@ -404,13 +399,10 @@ static void macio_instance_init(Object *obj)
 
 memory_region_init(>bar, obj, "macio", 0x8);
 
-object_initialize(>dbdma, sizeof(s->dbdma), TYPE_MAC_DBDMA);
-qdev_set_parent_bus(DEVICE(>dbdma), sysbus_get_default());
-object_property_add_child(obj, "dbdma", OBJECT(>dbdma), NULL);
+sysbus_init_child_obj(obj, "dbdma", >dbdma, sizeof(s->dbdma),
+  TYPE_MAC_DBDMA);
 
-object_initialize(>escc, sizeof(s->escc), TYPE_ESCC);
-qdev_set_parent_bus(DEVICE(>escc), sysbus_get_default());
-object_property_add_child(obj, "escc", OBJECT(>escc), NULL);
+sysbus_init_child_obj(obj, "escc", >escc, sizeof(s->escc), TYPE_ESCC);
 }
 
 static const VMStateDescription vmstate_macio_oldworld = {
diff --git a/hw/misc/macio/pmu.c b/hw/misc/macio/pmu.c
index e246b0fd41..d25344f888 100644
--- a/hw/misc/macio/pmu.c
+++ b/hw/misc/macio/pmu.c
@@ -770,9 +770,8 @@ static void pmu_init(Object *obj)
  qdev_prop_allow_set_link_before_realize,
  0, NULL);
 
-object_initialize(>mos6522_pmu, sizeof(s->mos6522_pmu),
-  TYPE_MOS6522_PMU);
-qdev_set_parent_bus(DEVICE(>mos6522_pmu), sysbus_get_default());
+sysbus_init_child_obj(obj, "mos6522-pmu", >mos6522_pmu,
+

[Qemu-devel] [PULL 0/2] ppc-for-3.0 queue 20180801

[David Gibson]

The following changes since commit f7502360397d291be04bc040e9f96c92ff2d8030:

  Update version for v3.0.0-rc3 release (2018-07-31 19:30:17 +0100)

are available in the Git repository at:

  git://github.com/dgibson/qemu.git tags/ppc-for-3.0-20180801

for you to fetch changes up to 6484ab3dffadc79020a71376010f517d60b81b83:

  sam460ex: Fix PCI interrupts with multiple devices (2018-08-01 11:01:38 +1000)


ppc patch queue for 2018-08-01

Here are a final couple of fixes for the 3.0 release.


BALATON Zoltan (1):
  sam460ex: Fix PCI interrupts with multiple devices

Thomas Huth (1):
  hw/misc/macio: Fix device introspection problems in macio devices

 hw/misc/macio/cuda.c  |  5 ++---
 hw/misc/macio/macio.c | 24 
 hw/misc/macio/pmu.c   |  5 ++---
 hw/ppc/ppc440_pcix.c  | 21 -
 hw/ppc/sam460ex.c |  6 ++
 5 files changed, 22 insertions(+), 39 deletions(-)

Re: [Qemu-devel] [PATCH] sam460ex: Fix PCI interrupts with multiple devices

[David Gibson]

On Tue, Jul 31, 2018 at 11:55:02PM +0200, Sebastian Bauer wrote:
> Am 2018-07-31 13:08, schrieb BALATON Zoltan:
> > The four interrupts of the PCI bus are connected to the same UIC pin
> > on the real Sam460ex. Evidence for this can be found in the UBoot
> > source for the Sam460ex in the Sam460ex.c file where
> > PCI_INTERRUPT_LINE is written. Change the ppc440_pcix model to behave
> > more like this.
> > 
> > This fixes the problem that can be observed when adding further PCI
> > cards that got their interrupt rotated to other interrupts than PCI
> > INT A. In particular, the bug was observed with an additional OHCI PCI
> > card or an ES1370 sound device.
> [...]
> > diff --git a/hw/ppc/sam460ex.c b/hw/ppc/sam460ex.c
> > index 0999efc..9c77183 100644
> > --- a/hw/ppc/sam460ex.c
> > +++ b/hw/ppc/sam460ex.c
> > @@ -515,10 +515,8 @@ static void sam460ex_init(MachineState *machine)
> > 
> >  /* PCI bus */
> >  ppc460ex_pcie_init(env);
> > -/* FIXME: is this correct? */
> > -dev = sysbus_create_varargs("ppc440-pcix-host", 0xc0ec0,
> > -uic[1][0], uic[1][20], uic[1][21],
> > uic[1][22],
> > -NULL);
> > +/* All PCI irqs are connected to the same UIC pin (cf. UBoot
> > source) */
> > +dev = sysbus_create_simple("ppc440-pcix-host", 0xc0ec0,
> > uic[1][0]);
> >  pci_bus = (PCIBus *)qdev_get_child_bus(dev, "pci.0");
> >  if (!pci_bus) {
> >  error_report("couldn't create PCI controller!");
> 
> I'm fine with that change. I tested it with an additional OHCI controller on
> an emulated SAM machine under guest AmigaOS and it works.
> 
> Tested-by: Sebastian Bauer 
> 
> Better usage of QOM etc. can IMO be done at a later time point as this fixes
> an unpleasant bug and any side change will just increase the probability to
> introduce new bugs.

Thanks, applied to ppc-for-3.0.  I hope to send a pull request for
that today.

-- 
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] target/mips: Initial support for MIPS R5900

[Maciej W. Rozycki]

On Sat, 7 Jul 2018, Fredrik Noring wrote:

> The MIPS R5900 is normally taken to be MIPS3, but it has MOVN, MOVZ and PREF
> defined in MIPS4 which is why ISA_MIPS4 is chosen for this patch.

 It also has several instructions removed, so I don't think you can really 
just mark it MIPS IV without special-casing those instructions, or the 
emulation won't be accurate (and consequently programs that use them won't 
trigger exceptions that they are supposed to).

> Some flags in the mips_defs array are marked FIXME as I don't know the
> proper values.

 Well, the FPU is non-standard so until you implement it I'd rather kept 
it disabled and then all the FPU-related settings can go for now.  For the 
rest see below.

> --- a/target/mips/translate_init.inc.c
> +++ b/target/mips/translate_init.inc.c
> @@ -411,6 +411,26 @@ const mips_def_t mips_defs[] =
>  .mmu_type = MMU_TYPE_R4000,
>  },
>  {
> +.name = "R5900",
> +.CP0_PRid = 0x3800,
> +/* No L2 cache, icache size 32k, dcache size 32k, uncached 
> coherency. */
> +.CP0_Config0 = (1 << 17) | (0x3 << 9) | (0x3 << 6) | (0x2 << 
> CP0C0_K0),
> +/* Note: Config1 is only used internally, the R5900 has only 
> Config0. */
> +.CP0_Config1 = (1 << CP0C1_FP) | (47 << CP0C1_MMU),

 So I'd clear CP0C1_FP then; also make sure accessing CP0.Config1 from 
emulated code does what it does on actual hardware.

> +.CP0_LLAddr_rw_bitmask = 0x, /* FIXME */
> +.CP0_LLAddr_shift = 4,   /* FIXME */

 No LL/SC in the R5900, so the LLAddr settings can go.

> +.SYNCI_Step = 16,/* FIXME */

 SYNCI is MIPS32r2+, so this can go.

> +.CCRes = 2,  /* FIXME */

 Likewise, CCRes is MIPS32r2+, so this can go.

> +.CP0_Status_rw_bitmask = 0x3678, /* FIXME */

 This has to indicate which bits in CP0.Status are writable.  Check with 
the manual and/or actual hardware.

> +.CP1_fcr0 = (0x38 << FCR0_PRID) | (0x0 << FCR0_REV),
> +.CP1_fcr31 = 0,
> +.CP1_fcr31_rw_bitmask = 0x0183,  /* FIXME */

 This is all FPU stuff and it can go.

> +.SEGBITS = 40,   /* FIXME */

 This is the number of virtual address bits.  Determined by the highest 
writable CP0.EntryHi.VPN2 bit.

> +.PABITS = 36,/* FIXME */

 Likewise physical address bits.  Determined by the highest writable 
CP0.EntryLo0.PFN and CP0.EntryLo1.PFN bit.

> +.insn_flags = CPU_R5900,
> +.mmu_type = MMU_TYPE_R4000,  /* FIXME */

 This looks right to me.

 FWIW; I don't have any authority for QEMU maintenance.

  Maciej

Re: [Qemu-devel] [PATCH] scsi: mptsas: Mark as storage device

[Fam Zheng]

On Tue, 07/31 15:28, Guenter Roeck wrote:
> mptsas1068 is currently listed as uncategorized device.
> Mark it as storage device.
> 
> Signed-off-by: Guenter Roeck 
> ---
>  hw/scsi/mptsas.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/hw/scsi/mptsas.c b/hw/scsi/mptsas.c
> index 4176e87..929404f 100644
> --- a/hw/scsi/mptsas.c
> +++ b/hw/scsi/mptsas.c
> @@ -1431,6 +1431,7 @@ static void mptsas1068_class_init(ObjectClass *oc, void 
> *data)
>  dc->reset = mptsas_reset;
>  dc->vmsd = _mptsas;
>  dc->desc = "LSI SAS 1068";
> +set_bit(DEVICE_CATEGORY_STORAGE, dc->categories);
>  }
>  
>  static const TypeInfo mptsas_info = {
> -- 
> 2.7.4
> 

Reviewed-by: Fam Zheng 

Re: [Qemu-devel] [PATCH 2/3] migration: Add qmp command for migrate_set_max_cpu_throttle

[Li Qiang]

2018-07-31 23:50 GMT+08:00 Daniel P. Berrangé :

> On Tue, Jul 31, 2018 at 04:48:35PM +0200, Juan Quintela wrote:
> > "Dr. David Alan Gilbert"  wrote:
> > > * Li Qiang (liq...@gmail.com) wrote:
> > >> The default max cpu throttle is 99, this is too big that may
> > >> influence the guest loads. Add a qmp to config it can make it
> > >> more flexible.
> > >>
> > >> Signed-off-by: Li Qiang 
> > >
> > > This should be done as a migration parameter rather than a new command.
> > > For example, follow the cpu-throttle-increment parameter; and this
> > > should work just like it.
> >
>


Hello all,

Thanks for your review. I will prepare another migration parameter version
patch set
for this.


Thanks,
Li Qiang

> I was about to comment this one.
> >
> > migrate_set_downtime, migrate_set_speed, migrate-set-cache-size,
> > query-migrate-cache-size are marked as deprecated.  Any way that we
> > could have make more clear that one should use
> > migrate_set/get_parameter/capability?
>
> Where are they marked as deprecated ?
>
They're not included in our official list of deprecated features, so per
> our deprecation policy, they are still considered supported.
>
>   https://qemu.weilnetz.de/doc/qemu-doc.html#Deprecated-features


>
> To deprecate something it needs to be added to qemu-deprecated.texi, and
> ideally also made to print a message to stderr when triggered.
>
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 :|
>

Re: [Qemu-devel] [PULL 1/1] slirp: fix ICMP handling on macOS hosts

[Andrew Oates via Qemu-devel]

On Tue, Jul 31, 2018 at 6:22 AM Peter Maydell 
wrote:

> On 31 July 2018 at 02:16, Andrew Oates  wrote:
> > Yeah, I suspect (but haven't tested) that this applies to all BSDs.  We
> > could switch CONFIG_DARWIN to CONFIG_BSD (happy to resend the patch, just
> > LMK).
> >
> > Agreed that platform-specific ifdefs are gross, but I don't see a better
> way
> > here :/  One option would be to look at the packet length and contents to
> > try to determine if there's an IP header before the ICMP header, but that
> > seems pretty iffy.  Creating ICMP sockets often requires special
> privileges
> > or configuration (e.g. on Linux) so I don't think it could easily be
> done at
> > configure-time to test the host machine's configuration.
>
> Mmm. I guess using CONFIG_BSD, or perhaps even not-CONFIG_LINUX,
> would be best. Is there an easy way to test this?
> (Our other two supported host OSes are the Solarises and Haiku;
> no idea if either of those support ICMP sockets or what their
> behaviour is here...)
>

Both CONFIG_BSD and not-CONFIG_LINUX work on macOS.  I unfortunately don't
have access to any other BSDs to test them, though.

~Andrew

>
> thanks
> -- PMM
>

[Qemu-devel] [Bug 1783362] Re: qemu-user: mmap should return failure (MAP_FAILED, -1) instead of success (NULL, 0) when len==0

[umarcor]

Alex, Laurent, I'm new to this management/development system. So, first
off, thanks for working on this bug.

I have a few (probably silly) questions:

1. What is 'the r-b' that Alex used in #14?
2. When should I change the status of the bug? I can already see it in GitHub's 
mirror and in https://git.qemu.org/?p=qemu.git;a=summary. But not in the 
Changelog: https://wiki.qemu.org/ChangeLog/3.0#User-mode_emulation. I am not 
sure if it is in 'Fix Committed' or 'Fix Released' state.
3. Where did you push these commits to before they where merge in 
https://git.qemu.org/?p=qemu.git;a=summary? I cannot find your personal 
forks/branches. Are commits automatically created from the mailing list?

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

Title:
  qemu-user: mmap should return failure (MAP_FAILED, -1) instead of
  success (NULL, 0) when len==0

Status in QEMU:
  In Progress
Status in qemu package in Ubuntu:
  In Progress

Bug description:
  As shown in https://github.com/beehive-
  lab/mambo/issues/19#issuecomment-407420602, with len==0 mmap returns
  success (NULL, 0) instead of failure (MAP_FAILED, -1) in a x86_64 host
  executing a ELF 64-bit LSB executable, ARM aarch64 binary.

  Steps to reproduce the bug:

  - (cross-)compile the attached source file:

  $ aarch64-linux-gnu-gcc -static -std=gnu99 -lpthread test/mmap_qemu.c
  -o mmap_qemu

  - Execute in a x86_64 host with qemu-user and qemu-user-binfmt:

  $ ./mmap_qemu
  alloc: 0
  MAP_FAILED: -1
  errno: 0
  mmap_qemu: test/mmap_qemu.c:15: main: Assertion `alloc == MAP_FAILED' failed.
  qemu: uncaught target signal 6 (Aborted) - core dumped
  Aborted (core dumped)

  - Execute in a ARM host without any additional dependecy:

  $ ./mmap_qemu
  alloc: -1
  MAP_FAILED: -1
  errno: 22

  The bug is present in Fedora:

  $ qemu-aarch64 --version
  qemu-aarch64 version 2.11.2(qemu-2.11.2-1.fc28)
  Copyright (c) 2003-2017 Fabrice Bellard and the QEMU Project developers
  $ uname -r
  4.17.7-200.fc28.x86_64

  And also in Ubuntu:

  $ qemu-aarch64 --version
  qemu-aarch64 version 2.12.0 (Debian 1:2.12+dfsg-3ubuntu3)
  Copyright (c) 2003-2017 Fabrice Bellard and the QEMU Project developers
  $ uname -r
  4.15.0-23-generic

  Possibly related to:

  - https://lists.freebsd.org/pipermail/freebsd-hackers/2009-July/029109.html
  - https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=203852

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

[Qemu-devel] [PATCH] hw/sd: Mark sd-card as storage device

[Guenter Roeck]

sd-card is currently listed as uncategorized device.
Mark it as storage device.

Signed-off-by: Guenter Roeck 
---
 hw/sd/sd.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index d4356e9..aaab15f 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -2121,6 +2121,7 @@ static void sd_class_init(ObjectClass *klass, void *data)
 dc->vmsd = _vmstate;
 dc->reset = sd_reset;
 dc->bus_type = TYPE_SD_BUS;
+set_bit(DEVICE_CATEGORY_STORAGE, dc->categories);
 
 sc->set_voltage = sd_set_voltage;
 sc->get_dat_lines = sd_get_dat_lines;
-- 
2.7.4

[Qemu-devel] [PATCH] scsi: mptsas: Mark as storage device

[Guenter Roeck]

mptsas1068 is currently listed as uncategorized device.
Mark it as storage device.

Signed-off-by: Guenter Roeck 
---
 hw/scsi/mptsas.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/hw/scsi/mptsas.c b/hw/scsi/mptsas.c
index 4176e87..929404f 100644
--- a/hw/scsi/mptsas.c
+++ b/hw/scsi/mptsas.c
@@ -1431,6 +1431,7 @@ static void mptsas1068_class_init(ObjectClass *oc, void 
*data)
 dc->reset = mptsas_reset;
 dc->vmsd = _mptsas;
 dc->desc = "LSI SAS 1068";
+set_bit(DEVICE_CATEGORY_STORAGE, dc->categories);
 }
 
 static const TypeInfo mptsas_info = {
-- 
2.7.4

Re: [Qemu-devel] [PATCH] sam460ex: Fix PCI interrupts with multiple devices

[Sebastian Bauer]

Am 2018-07-31 13:08, schrieb BALATON Zoltan:

The four interrupts of the PCI bus are connected to the same UIC pin
on the real Sam460ex. Evidence for this can be found in the UBoot
source for the Sam460ex in the Sam460ex.c file where
PCI_INTERRUPT_LINE is written. Change the ppc440_pcix model to behave
more like this.

This fixes the problem that can be observed when adding further PCI
cards that got their interrupt rotated to other interrupts than PCI
INT A. In particular, the bug was observed with an additional OHCI PCI
card or an ES1370 sound device.

[...]

diff --git a/hw/ppc/sam460ex.c b/hw/ppc/sam460ex.c
index 0999efc..9c77183 100644
--- a/hw/ppc/sam460ex.c
+++ b/hw/ppc/sam460ex.c
@@ -515,10 +515,8 @@ static void sam460ex_init(MachineState *machine)

 /* PCI bus */
 ppc460ex_pcie_init(env);
-/* FIXME: is this correct? */
-dev = sysbus_create_varargs("ppc440-pcix-host", 0xc0ec0,
-uic[1][0], uic[1][20], uic[1][21], 
uic[1][22],

-NULL);
+/* All PCI irqs are connected to the same UIC pin (cf. UBoot 
source) */
+dev = sysbus_create_simple("ppc440-pcix-host", 0xc0ec0, 
uic[1][0]);

 pci_bus = (PCIBus *)qdev_get_child_bus(dev, "pci.0");
 if (!pci_bus) {
 error_report("couldn't create PCI controller!");


I'm fine with that change. I tested it with an additional OHCI 
controller on an emulated SAM machine under guest AmigaOS and it works.


Tested-by: Sebastian Bauer 

Better usage of QOM etc. can IMO be done at a later time point as this 
fixes an unpleasant bug and any side change will just increase the 
probability to introduce new bugs.


Bye
Sebastian

Re: [Qemu-devel] [PATCH v2 0/4] Balloon inhibit enhancements, vfio restriction

[Alex Williamson]

On Tue, 31 Jul 2018 16:07:46 +0100
"Dr. David Alan Gilbert"  wrote:

> * Alex Williamson (alex.william...@redhat.com) wrote:
> > On Tue, 31 Jul 2018 15:29:17 +0300
> > "Michael S. Tsirkin"  wrote:
> >   
> > > On Mon, Jul 30, 2018 at 05:13:26PM -0600, Alex Williamson wrote:  
> > > > v2:
> > > >  - Use atomic ops for balloon inhibit counter (Peter)
> > > >  - Allow endpoint driver opt-in for ballooning, vfio-ccw opt-in by
> > > >default, vfio-pci opt-in by device option, only allowed for mdev
> > > >devices, no support added for platform as there are no platform
> > > >mdev devices.
> > > > 
> > > > See patch 3/4 for detailed explanation why ballooning and device
> > > > assignment typically don't mix.  If this eventually changes, flags
> > > > on the iommu info struct or perhaps device info struct can inform
> > > > us for automatic opt-in.  Thanks,
> > > > 
> > > > Alex
> > > 
> > > So this patch seems to block ballooning when vfio is added.
> > > But what if balloon is added and inflated first?  
> > 
> > Good point.
> >
> > > I'd suggest making qemu_balloon_inhibit fail in that case,
> > > and then vfio realize will fail as well.  
> > 
> > That might be the correct behavior for vfio, but I wonder about the
> > existing postcopy use case.  Dave Gilbert, what do you think?  We might
> > need a separate interface for callers that cannot tolerate existing
> > ballooned pages.  Of course we'll also need another atomic counter to
> > keep a tally of ballooned pages.  Thanks,  
> 
> For postcopy, preinflation isn't a problem; our only issue is ballooning
> during the postcopy phase itself.

On further consideration, I think device assignment is in the same
category.  The balloon inhibitor does not actually stop the guest
balloon driver from grabbing and freeing pages, it only changes whether
QEMU releases the pages with madvise DONTNEED.  The problem we have
with ballooning and device assignment is when we have an existing HPA
mapping in the IOMMU that isn't invalidated on DONTNEED and becomes
inconsistent when the page is re-populated.  Zapped pages at the time
an assigned device is added do not trigger this, those pages will be
repopulated when pages are pinned for the assigned device.  This is the
identical scenario to a freshly started VM that doesn't use memory
preallocation and therefore faults in pages on demand.  When an
assigned device is attached to such a VM, page pinning will fault in
and lock all of those pages.

This is observable behavior, for example if I start a VM with 16GB of
RAM, booted to a command prompt the VM shows less that 1GB of RAM
resident in the host.  If I set the balloon to 2048, there's no
observable change in the QEMU process size on the host.  If I hot-add
an assigned device while we're ballooned down, the resident memory size
from the host jumps up to 16GB.  All of the zapped pages have been
reclaimed.  Adjusting ballooning at this point only changes the balloon
size in the guest, inflating the balloon no longer zaps pages from the
process.

The only oddity I see is the one Dave noted in the commit introducing
balloon inhibiting (371ff5a3f04c):

Queueing the requests until after migration would be nice, but is
non-trivial, since the set of inflate/deflate requests have to
be compared with the state of the page to know what the final
outcome is allowed to be.

So for this example of a 16GB VM ballooned down to 2GB then an assigned
device added and subsequently removed, the resident memory remains 16GB
and I need to deflate the balloon and reinflate it in order to zap them
from the QEMU process.  Therefore, I think that with respect to this
inquiry, the series stands as is.  Thanks,

Alex

[Qemu-devel] [PATCH] imx_spi: Unset XCH when TX FIFO becomes empty

[Trent Piepho via Qemu-devel]

The current emulation will clear the XCH bit when a burst finishes.
This is not quite correct.  According to the i.MX7d referemce manual,
Rev 0.1, §10.1.7.3:

This bit [XCH] is cleared automatically when all data in the TXFIFO
and the shift register has been shifted out.

So XCH should be cleared when the FIFO empties, not on completion of a
burst.  The FIFO is 64 x 32 bits = 2048 bits, while the max burst size
is larger at 4096 bits.  So it's possible that the burst is not finished
after the TXFIFO empties.

Sending a large block (> 2048 bits) with the Linux driver will use a
burst that is larger than the TXFIFO.  After the TXFIFO has emptied XCH
does not become unset, as the burst is not yet finished.

What should happen after the TXFIFO empties is the driver will refill it
and set XCH.  The rising edge of XCH will trigger another transfer to
begin.  However, since the emulation does not set XCH to 0, there is no
rising edge and the next trasfer never begins.

Signed-off-by: Trent Piepho 
---
 hw/ssi/imx_spi.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/hw/ssi/imx_spi.c b/hw/ssi/imx_spi.c
index b66505ca49..02c38c9e47 100644
--- a/hw/ssi/imx_spi.c
+++ b/hw/ssi/imx_spi.c
@@ -208,8 +208,6 @@ static void imx_spi_flush_txfifo(IMXSPIState *s)
 }
 
 if (s->burst_length <= 0) {
-s->regs[ECSPI_CONREG] &= ~ECSPI_CONREG_XCH;
-
 if (!imx_spi_is_multiple_master_burst(s)) {
 s->regs[ECSPI_STATREG] |= ECSPI_STATREG_TC;
 break;
@@ -219,6 +217,7 @@ static void imx_spi_flush_txfifo(IMXSPIState *s)
 
 if (fifo32_is_empty(>tx_fifo)) {
 s->regs[ECSPI_STATREG] |= ECSPI_STATREG_TC;
+s->regs[ECSPI_CONREG] &= ~ECSPI_CONREG_XCH;
 }
 
 /* TODO: We should also use TDR and RDR bits */
-- 
2.14.4

Re: [Qemu-devel] [PATCH v2 for 3.0 0/2] fix for bug #1783362

[no-reply]

Hi,

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

Type: series
Message-id: 20180730134321.19898-1-alex.ben...@linaro.org
Subject: [Qemu-devel] [PATCH v2 for 3.0 0/2] fix for bug #1783362

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

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

git config --local diff.renamelimit 0
git config --local diff.renames True
git config --local diff.algorithm histogram

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

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

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
Switched to a new branch 'test'
b48e0e0795 tests: add check_invalid_maps to test-mmap
b531edb72a linux-user/mmap.c: handle invalid len maps correctly

=== OUTPUT BEGIN ===
Checking PATCH 1/2: linux-user/mmap.c: handle invalid len maps correctly...
Checking PATCH 2/2: tests: add check_invalid_maps to test-mmap...
ERROR: code indent should never use tabs
#61: FILE: tests/tcg/multiarch/test-mmap.c:498:
+^Icheck_invalid_mmaps();$

total: 1 errors, 0 warnings, 40 lines checked

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

=== OUTPUT END ===

Test command exited with code: 1


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

Re: [Qemu-devel] [PATCH v3] block/gluster: Handle changed glfs_ftruncate signature

[Jeff Cody]

On Tue, Jul 31, 2018 at 11:18:02AM +0200, Niels de Vos wrote:
> On Mon, Jul 30, 2018 at 03:27:29PM -0400, Jeff Cody wrote:
> > On Mon, Jul 30, 2018 at 10:07:27AM -0500, Eric Blake wrote:
> > > On 07/28/2018 02:50 AM, Niels de Vos wrote:
> > > >>
> > > >>Part of me wishes that libgfapi had just created a new function
> > > >>'glfs_ftruncate2', so that existing users don't need to handle the api
> > > >>change.  But I guess in the grand scheme, not a huge deal either way.
> > > >
> > > >Gluster uses versioned symbols, so older binaries will keep working with
> > > >new libraries. It is (hopefully) rare that existing symbols get updated.
> > > >We try to send patches for these kind of changes to the projects we know
> > > >well in advance, reducing the number of surprises.
> > > 
> > > >>I can go ahead and add that to the comment in my branch after applying, 
> > > >>if
> > > >>Niels can let me know what that version is/will be (if known).
> > > >
> > > >The new glfs_ftruncate() will be part of glusterfs-5 (planned for
> > > >October). We're changing the numbering scheme, it was expected to come
> > > >in glusterfs-4.2, but that is a version that never will be released.
> > > >
> > > 
> > > Wait - so you're saying gluster has not yet released the incompatible
> > > change? Now would be the right time to get rid of the API breakage, before
> > > you bake it in, rather than relying solely on the versioned symbols to 
> > > avoid
> > > an ABI breakage but forcing all clients to compensate to the API breakage.
> > > 
> > 
> > If this is not yet in a released version of Gluster, I'm not real eager to
> > pollute the QEMU driver codebase with #ifdef's, especially if there is a
> > possibility the API change may not actually materialize.
> > 
> > Is there any reason that this change is being approached in a way that
> > breaks API usage, and is it too late in the Gluster development pipeline to
> > change that?
> 
> There recently have been a few changes like this in libgfapi. These have
> been introduced to improve performance in common use-cases where an
> updated 'struct stat' is needed after an operation. Some functions have
> been adapted in previous releases, glfs_ftruncate() landed too late for
> that. I hope we'll get a complete coherent API with glusterfs-5 again.
> 

Am I correct in assuming the API changes that happened in previous libgfapi
release are for APIs that QEMU does not use (i.e. no action needed from
us?)

> For QEMU this means additional changes will come related to
> glfs_fallocate(), glfs_zerofill() and probably more. The current
> glusterfs-4.1 version will be maintained for one year, after which the
> detection/#ifdef can be removed as the than maintained versions should
> all have the updated API. I'm sorry for the inconvenience that this
> causes.
> 

Understood.  I don't want to make a huge deal out of it.  I guess my
recommendation/request is that API enhancements happen in a way that don't
break existing APIs (e.g. use parallel APIs), because QEMU version usage and
supported glusterfs usage may not always follow supported versions in the
wild.  I know versioned symbols helps with this, but it still complicates
the code (and couples QEMU's code to gluster support windows).


> If you prefer, I can wait with sending patches for QEMU with future
> Gluster releases until additional changes have landed in libgfapi.
>

I think generally, we don't want to start introducing #ifdef's and new APi
usage for unreleased versions of libgfapi if possible (as unreleased APIs,
it could technically change, and then QEMU releases would have 'dead' API
support in it).

If glusterfs-5 releases in October, that may line up with 3.1 or 3.2.


-Jeff

[Qemu-devel] [ANNOUNCE] QEMU 3.0.0-rc3 is now available

[Michael Roth]

Hello,

On behalf of the QEMU Team, I'd like to announce the availability of the
fourth release candidate for the QEMU 3.0 release.  This release is meant
for testing purposes and should not be used in a production environment.

  http://download.qemu-project.org/qemu-3.0.0-rc3.tar.xz
  http://download.qemu-project.org/qemu-3.0.0-rc3.tar.xz.sig

A note from the maintainer:

  In an ideal world, rc3 will be our final release candidate before
  final release. If we find no further showstopper regression bugs
  that would require a new release candidate, we'll do the final
  release on the 7th. If we do need to roll an rc4, final release
  will likely be on the 14th.

You can help improve the quality of the QEMU 3.0 release by testing this
release and reporting bugs on Launchpad:

  https://bugs.launchpad.net/qemu/

The release plan, as well a documented known issues for release
candidates, are available at:

  http://wiki.qemu.org/Planning/3.0

Please add entries to the ChangeLog for the 3.0 release below:

  http://wiki.qemu.org/ChangeLog/3.0

Thank you to everyone involved!

Changes since rc2:

f750236039: Update version for v3.0.0-rc3 release (Peter Maydell)
9a1054061c: monitor: temporary fix for dead-lock on event recursion (Marc-André 
Lureau)
5d9f3ea081: linux-user: ppc64: don't use volatile register during safe_syscall 
(Shivaprasad G Bhat)
28cbb997d6: tests: add check_invalid_maps to test-mmap (Alex Bennée)
38138fab93: linux-user/mmap.c: handle invalid len maps correctly (Alex Bennée)
408e5ace51: s390x/sclp: fix maxram calculation (Christian Borntraeger)
0261fb805c: target/arm: Remove duplicate 'host' entry in '-cpu ?' output 
(Philippe Mathieu-Daudé)
218fe5ce40: hw/misc/tz-mpc: Zero the LUT on initialization, not just reset 
(Peter Maydell)
984b0c100f: hw/arm/iotkit: Fix IRQ number for timer1 (Peter Maydell)
942566ffc1: armv7m_nvic: Fix m-security subsection name (Peter Maydell)
d1fb710a9b: hw/arm/sysbus-fdt: Fix assertion in copy_properties_from_host() 
(Geert Uytterhoeven)
758b71f7a3: arm/smmuv3: Fix missing VMSD terminator (Dr. David Alan Gilbert)
1239ac241f: qemu-iotests: Test query-blockstats with -drive and -blockdev 
(Kevin Wolf)
567dcb31f2: block/qapi: Include anonymous BBs in query-blockstats (Kevin Wolf)
5a9cb5a97f: block/qapi: Add 'qdev' field to query-blockstats result (Kevin Wolf)
34fa110e42: file-posix: Fix write_zeroes with unmap on block devices (Kevin 
Wolf)
52ebcb2682: block: Fix documentation for BDRV_REQ_MAY_UNMAP (Kevin Wolf)
8ba4f10fa6: iotests: Add test for 'qemu-img convert -C' compatibility (Fam 
Zheng)
e11ce12f5e: qemu-img: Add -C option for convert with copy offloading (Fam Zheng)
b85504314f: Revert "qemu-img: Document copy offloading implications with -S and 
-c" (Fam Zheng)
ac49c189b4: iotests: Don't lock /dev/null in 226 (Fam Zheng)
f4a1b6536f: docs: Describe using images in writing iotests (Fam Zheng)
a1c81f4f16: file-posix: Handle EINTR in preallocation=full write (Fam Zheng)
308999e9d4: qcow2: A grammar fix in conflicting cache sizing error message 
(Leonid Bloch)
41b6513436: qcow: fix a reference leak (KONRAD Frederic)
cc4c77e12b: backends/cryptodev: remove dead code (Jay Zhou)
e4dab9449a: timer: remove replay clock probe in deadline calculation (Pavel 
Dovgalyuk)
1d3db6bdbb: i386: implement MSR_SMI_COUNT for TCG (Paolo Bonzini)
990e0be260: i386: do not migrate MSR_SMI_COUNT on machine types <2.12 (Paolo 
Bonzini)
ba891d68b4: qstring: Move qstring_from_substr()'s @end one to the right (Markus 
Armbruster)
b65ab77b3a: qstring: Assert size calculations don't overflow (Markus Armbruster)
ad63c549ec: qstring: Fix qstring_from_substr() not to provoke int overflow 
(liujunjie)

Re: [Qemu-devel] [PATCH v5 00/76] Add nanoMIPS support to QEMU

[Richard Henderson]

On 07/30/2018 12:11 PM, Aleksandar Markovic wrote:
> From: Aleksandar Markovic 
> 
> v4->v5:
> 
>   - merged series "Mips maintenance and misc fixes and improvements"
> and this one for easier handling (there are build dependencies)
>   - eliminated shadow variables from translate.c
>   - replaced shift/mask combination with extract32()
>   - added new function gen_op_addr_addi()
>   - added "fall through" comments at appropriate places
>   - eliminated micromips flag from I7200 definition
>   - numerous other enhancements originating from reviewer's
> comments
>   - some of the patches split into two or more for easier
> handling and review
>   - rebased to the latest code

That concludes my review of this revision.
It's quite a bit better, but there is still a lot of work to do.


r~

Re: [Qemu-devel] [PATCH v5 43/76] target/mips: Add emulation of DSP ASE for nanoMIPS - part 6

[Richard Henderson]

On 07/31/2018 02:58 PM, Richard Henderson wrote:
> On 07/30/2018 12:12 PM, Aleksandar Markovic wrote:
>> +switch (extract32(ctx->opcode, 12, 1)) {
>> +case 0:
>> +/* NM_SHRA_QB */
>> +check_dspr2(ctx);
>> +gen_helper_shra_qb(cpu_gpr[ret], t0, v1_t);
> More unprotected use of cpu_gpr[0].
> 
> I think you need some sort of solution that prevents this completely, without
> having to think about it.  E.g. global replace cpu_gpr[x] -> read_gpr(ctx, x) 
> /
> dest_gpr(ctx, x), where the two functions allocate tcg temporaries on demand.

I forgot to say... and then poison cpu_gpr so that uses cannot creep back into
the codebase.  See e.g. include/exec/poison.h.


r~

Re: [Qemu-devel] [PATCH v5 45/76] target/mips: Implement emulation of nanoMIPS LLWP/SCWP pair

[Richard Henderson]

On 07/30/2018 12:12 PM, Aleksandar Markovic wrote:
> diff --git a/linux-user/mips/cpu_loop.c b/linux-user/mips/cpu_loop.c
> index 084ad6a..1d3dc9e 100644
> --- a/linux-user/mips/cpu_loop.c
> +++ b/linux-user/mips/cpu_loop.c
> @@ -397,10 +397,13 @@ static int do_store_exclusive(CPUMIPSState *env)
>  target_ulong addr;
>  target_ulong page_addr;
>  target_ulong val;
> +uint32_t val_wp = 0;
> +uint32_t llnewval_wp = 0;
>  int flags;
>  int segv = 0;
>  int reg;
>  int d;
> +int wp;
>  
>  addr = env->lladdr;
>  page_addr = addr & TARGET_PAGE_MASK;
> @@ -412,19 +415,31 @@ static int do_store_exclusive(CPUMIPSState *env)
>  } else {
>  reg = env->llreg & 0x1f;
>  d = (env->llreg & 0x20) != 0;
> -if (d) {
> -segv = get_user_s64(val, addr);
> +wp = (env->llreg & 0x40) != 0;
> +if (!wp) {
> +if (d) {
> +segv = get_user_s64(val, addr);
> +} else {
> +segv = get_user_s32(val, addr);
> +}
>  } else {
>  segv = get_user_s32(val, addr);
> +segv |= get_user_s32(val_wp, addr);
> +llnewval_wp = env->llnewval_wp;
>  }
>  if (!segv) {
> -if (val != env->llval) {
> +if (val != env->llval && val_wp == llnewval_wp) {
>  env->active_tc.gpr[reg] = 0;
>  } else {
> -if (d) {
> -segv = put_user_u64(env->llnewval, addr);
> +if (!wp) {
> +if (d) {
> +segv = put_user_u64(env->llnewval, addr);
> +} else {
> +segv = put_user_u32(env->llnewval, addr);
> +}
>  } else {
>  segv = put_user_u32(env->llnewval, addr);
> +segv |= put_user_u32(env->llnewval_wp, addr + 4);
>  }
>  if (!segv) {
>  env->active_tc.gpr[reg] = 1;
...
> diff --git a/target/mips/helper.h b/target/mips/helper.h
> index b2a780a..deca307 100644
> --- a/target/mips/helper.h
> +++ b/target/mips/helper.h
> @@ -14,6 +14,8 @@ DEF_HELPER_4(swr, void, env, tl, tl, int)
>  #ifndef CONFIG_USER_ONLY
>  DEF_HELPER_3(ll, tl, env, tl, int)
>  DEF_HELPER_4(sc, tl, env, tl, tl, int)
> +DEF_HELPER_5(llwp, void, env, tl, i32, i32, i32)
> +DEF_HELPER_4(scwp, tl, env, tl, i64, int)
>  #ifdef TARGET_MIPS64
>  DEF_HELPER_3(lld, tl, env, tl, int)
>  DEF_HELPER_4(scd, tl, env, tl, tl, int)
> diff --git a/target/mips/op_helper.c b/target/mips/op_helper.c
> index b3eef9f..cb83b6d 100644
> --- a/target/mips/op_helper.c
> +++ b/target/mips/op_helper.c
> @@ -380,6 +380,19 @@ HELPER_LD_ATOMIC(lld, ld, 0x7)
>  #endif
>  #undef HELPER_LD_ATOMIC
>  
> +void helper_llwp(CPUMIPSState *env, target_ulong addr, uint32_t reg1,
> + uint32_t reg2, uint32_t mem_idx)
> +{
> +if (addr & 0x7) {
> +env->CP0_BadVAddr = addr;
> +do_raise_exception(env, EXCP_AdEL, GETPC());
> +}
> +env->lladdr = do_translate_address(env, addr, 0, GETPC());
> +env->active_tc.gpr[reg1] = env->llval = do_lw(env, addr, mem_idx, 
> GETPC());
> +env->active_tc.gpr[reg2] = env->llval_wp = do_lw(env, addr + 4, mem_idx,
> + GETPC());
> +}
> +
>  #define HELPER_ST_ATOMIC(name, ld_insn, st_insn, almask) 
>  \
>  target_ulong helper_##name(CPUMIPSState *env, target_ulong arg1, 
>  \
> target_ulong arg2, int mem_idx)   
>  \
> @@ -406,6 +419,28 @@ HELPER_ST_ATOMIC(sc, lw, sw, 0x3)
>  HELPER_ST_ATOMIC(scd, ld, sd, 0x7)
>  #endif
>  #undef HELPER_ST_ATOMIC
> +
> +target_ulong helper_scwp(CPUMIPSState *env, target_ulong addr,
> + uint64_t data, int mem_idx)
> +{
> +uint32_t tmp;
> +uint32_t tmp2;
> +
> +if (addr & 0x7) {
> +env->CP0_BadVAddr = addr;
> +do_raise_exception(env, EXCP_AdES, GETPC());
> +}
> +if (do_translate_address(env, addr, 1, GETPC()) == env->lladdr) {
> +tmp = do_lw(env, addr, mem_idx, GETPC());
> +tmp2 = do_lw(env, addr + 4, mem_idx, GETPC());
> +if (tmp == env->llval && tmp2 == env->llval_wp) {
> +do_sw(env, addr, (uint32_t) data, mem_idx, GETPC());
> +do_sw(env, addr + 4, (uint32_t) *( + 4), mem_idx, GETPC());
> +return 1;
> +}
> +}
> +return 0;
> +}
>  #endif
>  
>  #ifdef TARGET_WORDS_BIGENDIAN

All of this should be unused code.


> diff --git a/target/mips/translate.c b/target/mips/translate.c
> index ea6fdeb..c4b6a26 100644
> --- a/target/mips/translate.c
> +++ b/target/mips/translate.c
> @@ -1459,6 +1459,7 @@ typedef struct DisasContext {
>  bool nan2008;
>  bool abs2008;
>  bool has_isa_mode;
> +bool xnp;
>  } DisasContext;
>  
>  #define DISAS_STOP   

Re: [Qemu-devel] [PATCH v5 59/76] gdbstub: Disable handling of nanoMIPS ISA bit in the MIPS gdbstub

[Richard Henderson]

On 07/30/2018 12:12 PM, Aleksandar Markovic wrote:
> From: James Hogan 
> 
> nanoMIPS has no ISA bit in the PC, so remove the handling of the low bit
> of the PC in the MIPS gdbstub for nanoMIPS. This prevents the PC being
> read as e.g. 0xbfc1, and prevents writing to the PC clearing
> MIPS_HFLAG_M16.
> 
> Signed-off-by: James Hogan 
> Signed-off-by: Yongbok Kim 
> Signed-off-by: Aleksandar Markovic 
> Signed-off-by: Stefan Markovic 
> ---
>  target/mips/gdbstub.c | 13 -
>  1 file changed, 8 insertions(+), 5 deletions(-)

Likewise re nanomips not setting M16.


r~

Re: [Qemu-devel] [PATCH v5 52/76] target/mips: Fix ERET/ERETNC behavior related to ADEL exception

[Richard Henderson]

On 07/30/2018 12:12 PM, Aleksandar Markovic wrote:
> diff --git a/target/mips/op_helper.c b/target/mips/op_helper.c
> index c55a1e6..e6749c5 100644
> --- a/target/mips/op_helper.c
> +++ b/target/mips/op_helper.c
> @@ -2430,6 +2430,13 @@ static void set_pc(CPUMIPSState *env, target_ulong 
> error_pc)
>  env->active_tc.PC = error_pc & ~(target_ulong)1;
>  if (env->insn_flags & ISA_NANOMIPS32) {
>  /* Don't clear MIPS_HFLAG_M16 */
> +if (error_pc & 1) {
> +if (!(env->hflags & MIPS_HFLAG_DM)) {
> +env->CP0_BadVAddr = error_pc;
> +}
> +env->active_tc.PC = error_pc;
> +do_raise_exception(env, EXCP_AdEL, 0);
> +}

Isn't this taken care of by decode_nanomips_opc?
What do you gain by checking twice?


r~

Re: [Qemu-devel] [PATCH v5 50/76] target/mips: Adjust set_hflags_for_handler() for nanoMIPS

[Richard Henderson]

On 07/30/2018 12:12 PM, Aleksandar Markovic wrote:
> From: James Hogan 
> 
> We shouldn't clear M16 mode when entering an interrupt on nanoMIPS,
> otherwise we'll start interpreting the code as normal MIPS code.

Likewise re nanomips not setting M16.


r~

Re: [Qemu-devel] [PATCH v5 51/76] target/mips: Adjust set_pc() for nanoMIPS

[Richard Henderson]

On 07/30/2018 12:12 PM, Aleksandar Markovic wrote:
> From: James Hogan 
> 
> ERET and ERETNC shouldn't clear MIPS_HFLAG_M16 for nanoMIPS since there
> is no ISA bit, so fix set_pc() to skip the hflags update.
> 
> Signed-off-by: James Hogan 
> Signed-off-by: Yongbok Kim 
> Signed-off-by: Aleksandar Markovic 
> Signed-off-by: Stefan Markovic 
> Reviewed-by: Aleksandar Markovic 
> ---
>  target/mips/op_helper.c | 4 
>  1 file changed, 4 insertions(+)
> 
> diff --git a/target/mips/op_helper.c b/target/mips/op_helper.c
> index 5e10286..c55a1e6 100644
> --- a/target/mips/op_helper.c
> +++ b/target/mips/op_helper.c
> @@ -2428,6 +2428,10 @@ static void debug_post_eret(CPUMIPSState *env)
>  static void set_pc(CPUMIPSState *env, target_ulong error_pc)
>  {
>  env->active_tc.PC = error_pc & ~(target_ulong)1;

You don't want this for nanomips either, surely.

> +if (env->insn_flags & ISA_NANOMIPS32) {
> +/* Don't clear MIPS_HFLAG_M16 */
> +return;
> +}

Likewise re nanomips not setting M16.


r~

Re: [Qemu-devel] [PATCH v5 49/76] target/mips: Adjust exception_resume_pc() for nanoMIPS

[Richard Henderson]

On 07/30/2018 12:12 PM, Aleksandar Markovic wrote:
> -isa_mode = !!(env->hflags & MIPS_HFLAG_M16);
> +isa_mode = env->hflags & MIPS_HFLAG_M16 &&
> +!(env->insn_flags & ISA_NANOMIPS32);

Likewise re nanomips not setting M16.


r~

Re: [Qemu-devel] [PATCH v5 48/76] target/mips: Adjust behavior of Config3's ISAOnExc bit for nanoMIPS

[Richard Henderson]

On 07/30/2018 12:12 PM, Aleksandar Markovic wrote:
> From: Yongbok Kim 
> 
> Config3.ISAOnExc is read only in nanoMIPS.
> 
> Signed-off-by: Yongbok Kim 
> Signed-off-by: Aleksandar Markovic 
> Signed-off-by: Stefan Markovic 
> ---
>  target/mips/op_helper.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/target/mips/op_helper.c b/target/mips/op_helper.c
> index cb83b6d..5e10286 100644
> --- a/target/mips/op_helper.c
> +++ b/target/mips/op_helper.c
> @@ -1730,7 +1730,8 @@ void helper_mtc0_config2(CPUMIPSState *env, 
> target_ulong arg1)
>  
>  void helper_mtc0_config3(CPUMIPSState *env, target_ulong arg1)
>  {
> -if (env->insn_flags & ASE_MICROMIPS) {
> +if ((env->insn_flags & ASE_MICROMIPS) &&
> +!(env->insn_flags & ISA_NANOMIPS32)) {'

Didn't we say nanomips does not include micromips?


r~

Re: [Qemu-devel] [PATCH v5 47/76] target/mips: Implement CP0 Config0.WR bit functionality

[Richard Henderson]

On 07/30/2018 12:12 PM, Aleksandar Markovic wrote:
> From: Stefan Markovic 
> 
> Add testing Config0.WR bit into watch exception handling logic.
> 
> Signed-off-by: Aleksandar Markovic 
> Signed-off-by: Stefan Markovic 
> ---
>  target/mips/helper.c| 12 +++-
>  target/mips/translate.c | 22 --
>  2 files changed, 27 insertions(+), 7 deletions(-)

Reviewed-by: Richard Henderson 


r~

Re: [Qemu-devel] [PATCH v5 46/76] target/mips: Add updating BadInstr, BadInstrP, BadInstrX for nanoMIPS

[Richard Henderson]

On 07/30/2018 12:12 PM, Aleksandar Markovic wrote:
> +if ((env->CP0_Config3 & (1 << CP0C3_BP)) &&
> +(env->hflags & MIPS_HFLAG_BMASK)) {
> +if (!(env->hflags & MIPS_HFLAG_B16)) {
> +env->CP0_BadInstrP = cpu_ldl_code(env, env->active_tc.PC - 
> 4);
> +} else {
> +env->CP0_BadInstrP =
> +(cpu_lduw_code(env, env->active_tc.PC - 2)) << 16;
> +}
> +}

Why would BMASK ever be set for nanomips?
There are no delayed branches.


r~

Re: [Qemu-devel] [PATCH v5 10/76] linux-user: Add preprocessor availability control to some syscalls

[Richard Henderson]

On 07/30/2018 12:11 PM, Aleksandar Markovic wrote:
> From: Aleksandar Rikalo 
> 
> Add ability to target platforms to individually include user-mode
> support for system calls from "stat" group of system calls.
> 
> This change is related to new nanoMIPS platform in the sense that
> it supports a different set of "stat" system calls than any other
> target. nanoMIPS does not support structures stat and stat64 at
> all. Also, support for certain number of other system calls is
> dropped in nanoMIPS (those are most of the time obsoleted system
> calls).
> 
> Without this patch, build for nanoMIPS would fail.
> 
> Signed-off-by: Aleksandar Markovic 
> Signed-off-by: Stefan Markovic 
> Reviewed-by: Aleksandar Markovic 
> ---
>  linux-user/strace.c  | 14 +-
>  linux-user/syscall.c | 29 +
>  2 files changed, 42 insertions(+), 1 deletion(-)

Reviewed-by: Richard Henderson 


r~

Re: [Qemu-devel] [virtio-dev] [PATCH v3 3/3] Add "Group Identifier" support to Red Hat PCI Express bridge.

[Marcel Apfelbaum]

On 07/31/2018 07:03 PM, Michael S. Tsirkin wrote:

On Tue, Jul 31, 2018 at 10:58:37AM -0500, Venu Busireddy wrote:

On 2018-07-07 15:14:11 +0300, Marcel Apfelbaum wrote:

Hi Venu,

On 06/30/2018 01:19 AM, Venu Busireddy wrote:

Add a new bridge device "pcie-downstream" with a
Vendor ID of PCI_VENDOR_ID_REDHAT and a Device ID of
PCI_DEVICE_ID_REDHAT_DOWNPORT_FAILOVER.

Can't we use the current pcie-pci-bridge device (see
hw/pci-bridge/pcie_pci_bridge.c)
by adding the new vendor capability to it so we will not need to support a
new bridge?

Sorry for the delayed response. I was away on vacation.

This question is probably better answered by Michael, but me let me try.
Michael suggested adding a new device to avoid confusion with existing
bridge devices.

Venu

It's similar to the hotseat trick - dev id is easier to match against.


I understand, but even so, we can maybe use the current pcie-pci-brigde
as a base class and derive from it changing only the VENDOR/DEVICE ID
and adding the new properties, instead of a full implementation.

Only a suggestion, of course.

Thanks,
Marcel




Thanks,
Marcel


   Also, add the "Vendor-Specific"
capability to the bridge to contain the "Group Identifier" that will be
used to pair a virtio device with the passthrough device attached to
that bridge.

This capability is added to the bridge iff the "failover-group-id"
option is specified for the bridge.

Signed-off-by: Venu Busireddy 
---
   default-configs/arm-softmmu.mak|   1 +
   default-configs/i386-softmmu.mak   |   1 +
   default-configs/x86_64-softmmu.mak |   1 +
   hw/pci-bridge/Makefile.objs|   1 +
   hw/pci-bridge/pcie_downstream.c| 220 +
   hw/pci-bridge/pcie_downstream.h|  10 ++
   include/hw/pci/pci.h   |   1 +
   7 files changed, 235 insertions(+)
   create mode 100644 hw/pci-bridge/pcie_downstream.c
   create mode 100644 hw/pci-bridge/pcie_downstream.h

diff --git a/default-configs/arm-softmmu.mak b/default-configs/arm-softmmu.mak
index 834d45cfaf..b86c6fb122 100644
--- a/default-configs/arm-softmmu.mak
+++ b/default-configs/arm-softmmu.mak
@@ -139,6 +139,7 @@ CONFIG_IMX_I2C=y
   CONFIG_PCIE_PORT=y
   CONFIG_XIO3130=y
   CONFIG_IOH3420=y
+CONFIG_PCIE_DOWNSTREAM=y
   CONFIG_I82801B11=y
   CONFIG_ACPI=y
   CONFIG_SMBIOS=y
diff --git a/default-configs/i386-softmmu.mak b/default-configs/i386-softmmu.mak
index 8c7d4a0fa0..a900c8f052 100644
--- a/default-configs/i386-softmmu.mak
+++ b/default-configs/i386-softmmu.mak
@@ -56,6 +56,7 @@ CONFIG_ACPI_NVDIMM=y
   CONFIG_PCIE_PORT=y
   CONFIG_XIO3130=y
   CONFIG_IOH3420=y
+CONFIG_PCIE_DOWNSTREAM=y
   CONFIG_I82801B11=y
   CONFIG_SMBIOS=y
   CONFIG_HYPERV_TESTDEV=$(CONFIG_KVM)
diff --git a/default-configs/x86_64-softmmu.mak 
b/default-configs/x86_64-softmmu.mak
index 0390b4303c..481e4764be 100644
--- a/default-configs/x86_64-softmmu.mak
+++ b/default-configs/x86_64-softmmu.mak
@@ -56,6 +56,7 @@ CONFIG_ACPI_NVDIMM=y
   CONFIG_PCIE_PORT=y
   CONFIG_XIO3130=y
   CONFIG_IOH3420=y
+CONFIG_PCIE_DOWNSTREAM=y
   CONFIG_I82801B11=y
   CONFIG_SMBIOS=y
   CONFIG_HYPERV_TESTDEV=$(CONFIG_KVM)
diff --git a/hw/pci-bridge/Makefile.objs b/hw/pci-bridge/Makefile.objs
index 47065f87d9..5b42212edc 100644
--- a/hw/pci-bridge/Makefile.objs
+++ b/hw/pci-bridge/Makefile.objs
@@ -3,6 +3,7 @@ common-obj-$(CONFIG_PCIE_PORT) += pcie_root_port.o 
gen_pcie_root_port.o pcie_pci
   common-obj-$(CONFIG_PXB) += pci_expander_bridge.o
   common-obj-$(CONFIG_XIO3130) += xio3130_upstream.o xio3130_downstream.o
   common-obj-$(CONFIG_IOH3420) += ioh3420.o
+common-obj-$(CONFIG_PCIE_DOWNSTREAM) += pcie_downstream.o
   common-obj-$(CONFIG_I82801B11) += i82801b11.o
   # NewWorld PowerMac
   common-obj-$(CONFIG_DEC_PCI) += dec.o
diff --git a/hw/pci-bridge/pcie_downstream.c b/hw/pci-bridge/pcie_downstream.c
new file mode 100644
index 00..49b0d1e933
--- /dev/null
+++ b/hw/pci-bridge/pcie_downstream.c
@@ -0,0 +1,220 @@
+/*
+ * Red Hat PCI Express downstream port.
+ *
+ * pcie_downstream.c
+ * Most of this code is copied from xio3130_downstream.c
+ *
+ * Copyright (c) 2018 Oracle and/or its affiliates.
+ * Author: Venu Busireddy 
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License along
+ * with this program; if not, see .
+ */
+
+#include "qemu/osdep.h"
+#include "hw/pci/pci_ids.h"
+#include "hw/pci/msi.h"
+#include "hw/pci/pcie.h"
+#include "pcie_downstream.h"
+#include 

Re: [Qemu-devel] [PATCH v5 44/76] target/mips: Add handling of branch delay slots for nanoMIPS

[Richard Henderson]

On 07/30/2018 12:12 PM, Aleksandar Markovic wrote:
> -int lowbit = !!(ctx->hflags & MIPS_HFLAG_M16);
> +int lowbit = ctx->has_isa_mode && !!(ctx->hflags & MIPS_HFLAG_M16);

I believe I asked why M16 is set for nanoMIPS at all.

You go out of your way to ignore it in lots of places,
and avoid it being cleared in others.  And I noted some
places where it seemed to have been forgotten to be ignored.


r~

Re: [Qemu-devel] [PATCH v5 43/76] target/mips: Add emulation of DSP ASE for nanoMIPS - part 6

[Richard Henderson]

On 07/30/2018 12:12 PM, Aleksandar Markovic wrote:
> +switch (extract32(ctx->opcode, 12, 1)) {
> +case 0:
> +/* NM_SHRA_QB */
> +check_dspr2(ctx);
> +gen_helper_shra_qb(cpu_gpr[ret], t0, v1_t);
More unprotected use of cpu_gpr[0].

I think you need some sort of solution that prevents this completely, without
having to think about it.  E.g. global replace cpu_gpr[x] -> read_gpr(ctx, x) /
dest_gpr(ctx, x), where the two functions allocate tcg temporaries on demand.

I think the model used in target/alpha/translate.c is ideal.  However, there
are variations on this theme in target/arm/translate-a64.c,
target/sparc/translate.c, and target/openrisc/translate.c.


r~

Re: [Qemu-devel] [PATCH v5 42/76] target/mips: Add emulation of DSP ASE for nanoMIPS - part 5

[Richard Henderson]

On 07/30/2018 12:12 PM, Aleksandar Markovic wrote:
> +static void gen_pool32axf_4_nanomips_insn(DisasContext *ctx, uint32_t opc,
> +  int ret, int v1, int v2)
> +{
> +TCGv t0;
> +TCGv v0_t;
> +TCGv v1_t;
> +
> +t0 = tcg_temp_new();
> +
> +v0_t = tcg_temp_new();
> +v1_t = tcg_temp_new();
> +
> +gen_load_gpr(v0_t, ret);

Again with loading a result,

> +gen_load_gpr(v1_t, v1);
> +
> +switch (opc) {
> +case NM_ABSQ_S_QB:
> +check_dspr2(ctx);
> +gen_helper_absq_s_qb(cpu_gpr[ret], v0_t, cpu_env);
> +break;

and unprotected use of cpu_gpr[0].


r~

Re: [Qemu-devel] [PATCH v5 41/76] target/mips: Add emulation of DSP ASE for nanoMIPS - part 4

[Richard Henderson]

On 07/30/2018 12:12 PM, Aleksandar Markovic wrote:
 +static void gen_pool32axf_2_nanomips_insn(DisasContext *ctx, uint32_t opc,
> +  int ret, int v1, int v2)
> +{
> +TCGv t0;
> +TCGv t1;
> +
> +TCGv v0_t;
> +TCGv v1_t;
> +
> +t0 = tcg_temp_new();
> +t1 = tcg_temp_new();
> +
> +v0_t = tcg_temp_new();
> +v1_t = tcg_temp_new();
> +
> +gen_load_gpr(v0_t, ret);
> +gen_load_gpr(v1_t, v1);
> +
> +switch (opc) {
> +case NM_POOL32AXF_2_0_7:
> +switch (extract32(ctx->opcode, 9, 3)) {
> +case NM_DPA_W_PH:
> +case NM_DPAQ_S_W_PH:
> +case NM_DPS_W_PH:
> +case NM_DPSQ_S_W_PH:
> +gen_pool32axf_2_multiply(ctx, opc, ret, v1, v2);

This structure, in which you have loaded gpr values, then discard them only to
load them up again in a different helper function, could use some improvement.

> +case NM_BALIGN:
> +gen_load_gpr(t0, v1);
> +v2 &= 3;
> +if (v2 != 0 && v2 != 2) {
> +tcg_gen_shli_tl(cpu_gpr[ret], cpu_gpr[ret], 8 * v2);

More unprotected uses of cpu_gpr[0].


r~

Re: [Qemu-devel] [PATCH v5 40/76] target/mips: Add emulation of DSP ASE for nanoMIPS - part 3

[Richard Henderson]

On 07/30/2018 12:12 PM, Aleksandar Markovic wrote:
> From: Stefan Markovic 
> 
> Add emulation of DSP ASE instructions for nanoMIPS - part 3.
> 
> Signed-off-by: Aleksandar Markovic 
> Signed-off-by: Stefan Markovic 
> ---
>  target/mips/translate.c | 180 
> 
>  1 file changed, 180 insertions(+)
> 
> diff --git a/target/mips/translate.c b/target/mips/translate.c
> index 055be7e..e597b35 100644
> --- a/target/mips/translate.c
> +++ b/target/mips/translate.c
> @@ -16872,13 +16872,191 @@ static void 
> gen_pool32a0_nanomips_insn(CPUMIPSState *env, DisasContext *ctx)
>  }
>  }
>  
> +/* dsp */
> +static void gen_pool32axf_1_5_nanomips_insn(DisasContext *ctx, uint32_t opc,
> +int ret, int v1, int v2)
> +{
> +TCGv_i32 t0;
> +TCGv v0_t;
> +TCGv v1_t;
> +
> +t0 = tcg_temp_new_i32();
> +
> +v0_t = tcg_temp_new();
> +v1_t = tcg_temp_new();
> +
> +tcg_gen_movi_i32(t0, v2 >> 3);
> +
> +gen_load_gpr(v0_t, ret);
> +gen_load_gpr(v1_t, v1);
> +
> +switch (opc) {
> +case NM_MAQ_S_W_PHR:
> +check_dsp(ctx);
> +gen_helper_maq_s_w_phr(t0, v1_t, v0_t, cpu_env);
> +break;
> +case NM_MAQ_S_W_PHL:
> +check_dsp(ctx);
> +gen_helper_maq_s_w_phl(t0, v1_t, v0_t, cpu_env);
> +break;
> +case NM_MAQ_SA_W_PHR:
> +check_dsp(ctx);
> +gen_helper_maq_sa_w_phr(t0, v1_t, v0_t, cpu_env);
> +break;
> +case NM_MAQ_SA_W_PHL:
> +check_dsp(ctx);
> +gen_helper_maq_sa_w_phl(t0, v1_t, v0_t, cpu_env);
> +break;
> +default:
> +generate_exception_end(ctx, EXCP_RI);
> +break;
> +}
> +
> +tcg_temp_free_i32(t0);
> +
> +tcg_temp_free(v0_t);
> +tcg_temp_free(v1_t);
> +}
> +
> +
> +static void gen_pool32axf_1_nanomips_insn(DisasContext *ctx, uint32_t opc,
> +int ret, int v1, int v2)
> +{
> +int16_t imm;
> +
> +TCGv t0;
> +TCGv t1;
> +TCGv v0_t;
> +TCGv v1_t;
> +
> +t0 = tcg_temp_new();
> +t1 = tcg_temp_new();
> +
> +v0_t = tcg_temp_new();
> +v1_t = tcg_temp_new();
> +
> +gen_load_gpr(v0_t, ret);

Did you really mean to load the result?

> +gen_load_gpr(v1_t, v1);
> +
> +switch (opc) {
> +case NM_POOL32AXF_1_0:
> +switch (extract32(ctx->opcode, 12, 2)) {
> +case NM_MFHI:
> +gen_HILO(ctx, OPC_MFHI, v2 >> 3, ret);
> +break;
> +case NM_MFLO:
> +gen_HILO(ctx, OPC_MFLO, v2 >> 3, ret);
> +break;

The result isn't stored.

> +case NM_POOL32AXF_1_3:
> +imm = extract32(ctx->opcode, 14, 7);
> +switch (extract32(ctx->opcode, 12, 2)) {
> +case NM_RDDSP:
> +tcg_gen_movi_tl(t0, imm);
> +gen_helper_rddsp(cpu_gpr[ret], t0, cpu_env);
> +break;

Unprotected use of cpu_gpr[0].
And many others.


r~

Re: [Qemu-devel] qemu NBD client behaviour when device size is not a multiple of 512

[Eric Blake]

On 07/31/2018 01:00 PM, Richard W.M. Jones wrote:


Hi Eric.  Is this a bug?

   $ nbdkit -fv random size=1023

(You can choose any size which is not a multiple of 512.)

   $ qemu-img convert nbd:localhost:10809 /var/tmp/test
   qemu-img: error while reading sector 0: Invalid argument


Or more directly:

$ qemu-io -r -f raw --trace=nbd_\* nbd://localhost:10809
...
422833@1533060368.492178:nbd_receive_negotiate_size_flags Size is 1023, 
export flags 0x3

qemu-io> r -v 0 1023
422833@1533060426.523422:nbd_send_request Sending request to server: { 
.from = 0, .len = 1024, .handle = 94749973992480, .flags = 0x0, .type = 
0 (read) }
422833@1533060426.523632:nbd_receive_simple_reply Got simple reply: { 
.error = 22 (EINVAL), handle = 94749973992480 }

read failed: Invalid argument

Yep. Looks likes the client code is trying to enforce 512-byte 
alignment, and qemu rounds ALL disk sizes up to the next sector boundary 
(even when the actual disk is unaligned).  I'll have to patch things to 
not round past image end on the final partial sector.


Note that with qemu as server:

$ truncate --size=1023 file
$ qemu-nbd -f raw -x '' file

the client now sees:

423068@1533060754.229106:nbd_opt_go_info_block_size Block sizes are 
0x200, 0x1000, 0x200

...
423068@1533060754.229156:nbd_receive_negotiate_size_flags Size is 1024, 
export flags 0x1ed


where qemu as server is 0-padding the source file in all read requests; 
and that probably explains why I haven't noticed the problem with 
unaligned images in the client before (since qemu as NBD server doesn't 
advertise unaligned images).  What's more, a write request to the tail 
of the partial sector (when not using a read-only connection) succeeds, 
and resizes the file in the process (at least, on protocols that support 
live resizing); but I wouldn't count on it behaving sanely for all 
protocols.


What SHOULD happen is that if qemu as server advertises a block size, it 
should truncate the advertised image size DOWN (not round up).  I'm also 
not sure why qemu as server is advertising 512-byte blocks when backed 
by a file - it should be able to advertise 1-byte blocks.  Likewise, 
qemu as client should pay attention to the size advertised by the 
server: if it is not sector-aligned and the server did not advertise 
block size, then assume that byte alignment works; if the server DID 
advertise block size (greater than 1) then qemu should round the image 
size down (as obeying the advertised block size implies that the partial 
sector at the end is inaccessible without violating NBD protocol - 
although the NBD protocol also states that servers SHOULD advertise a 
size that is a multiple of the block size).


At any rate, you've given me plenty to work with for fixing things, 
although it probably won't make it into 3.0-rc3 and I don't know if 
we'll have an -rc4.


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

Re: [Qemu-devel] [PATCH v6 2/4] spapr: introduce a fixed IRQ number space

[Greg Kurz]

On Mon, 30 Jul 2018 16:11:32 +0200
Cédric Le Goater  wrote:

> This proposal introduces a new IRQ number space layout using static
> numbers for all devices, depending on a device index, and a bitmap
> allocator for the MSI IRQ numbers which are negotiated by the guest at
> runtime.
> 
> As the VIO device model does not have a device index but a "reg"
> property, we introduce a formula to compute an IRQ number from a "reg"
> value. It should minimize most of the collisions.
> 
> The previous layout is kept in pre-3.1 machines raising the
> 'legacy_irq_allocation' machine class flag.
> 
> Signed-off-by: Cédric Le Goater 
> ---
>  include/hw/ppc/spapr.h |  5 +++
>  include/hw/ppc/spapr_irq.h | 32 ++
>  hw/ppc/spapr.c | 32 ++
>  hw/ppc/spapr_events.c  | 12 ---
>  hw/ppc/spapr_irq.c | 56 
>  hw/ppc/spapr_pci.c | 29 +
>  hw/ppc/spapr_vio.c | 66 ++
>  hw/ppc/Makefile.objs   |  2 +-
>  8 files changed, 216 insertions(+), 18 deletions(-)
>  create mode 100644 include/hw/ppc/spapr_irq.h
>  create mode 100644 hw/ppc/spapr_irq.c
> 
> diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h
> index 7e5de1a6fd42..73067f5ee8aa 100644
> --- a/include/hw/ppc/spapr.h
> +++ b/include/hw/ppc/spapr.h
> @@ -8,6 +8,7 @@
>  #include "hw/ppc/spapr_drc.h"
>  #include "hw/mem/pc-dimm.h"
>  #include "hw/ppc/spapr_ovec.h"
> +#include "hw/ppc/spapr_irq.h"
>  
>  struct VIOsPAPRBus;
>  struct sPAPRPHBState;
> @@ -101,6 +102,8 @@ struct sPAPRMachineClass {
>  bool dr_lmb_enabled;   /* enable dynamic-reconfig/hotplug of LMBs */
>  bool use_ohci_by_default;  /* use USB-OHCI instead of XHCI */
>  bool pre_2_10_has_unused_icps;
> +bool legacy_irq_allocation;
> +
>  void (*phb_placement)(sPAPRMachineState *spapr, uint32_t index,
>uint64_t *buid, hwaddr *pio, 
>hwaddr *mmio32, hwaddr *mmio64,
> @@ -167,6 +170,8 @@ struct sPAPRMachineState {
>  char *kvm_type;
>  
>  const char *icp_type;
> +int32_t irq_map_nr;
> +unsigned long *irq_map;
>  
>  bool cmd_line_caps[SPAPR_CAP_NUM];
>  sPAPRCapabilities def, eff, mig;
> diff --git a/include/hw/ppc/spapr_irq.h b/include/hw/ppc/spapr_irq.h
> new file mode 100644
> index ..6f7f50548809
> --- /dev/null
> +++ b/include/hw/ppc/spapr_irq.h
> @@ -0,0 +1,32 @@
> +/*
> + * QEMU PowerPC sPAPR IRQ backend definitions
> + *
> + * Copyright (c) 2018, IBM Corporation.
> + *
> + * This code is licensed under the GPL version 2 or later. See the
> + * COPYING file in the top-level directory.
> + */
> +
> +#ifndef HW_SPAPR_IRQ_H
> +#define HW_SPAPR_IRQ_H
> +
> +/*
> + * IRQ range offsets per device type
> + */
> +#define SPAPR_IRQ_EPOW   0x1000  /* XICS_IRQ_BASE offset */
> +#define SPAPR_IRQ_HOTPLUG0x1001
> +#define SPAPR_IRQ_VIO0x1100  /* 256 VIO devices */
> +#define SPAPR_IRQ_PCI_LSI0x1200  /* 32+ PHBs devices */
> +
> +#define SPAPR_IRQ_MSI0x1300  /* Offset of the dynamic range covered
> +  * by the bitmap allocator */
> +
> +typedef struct sPAPRMachineState sPAPRMachineState;
> +

Old compilers (GCC < 4.6) might complain about 'redefinition of typedef' if
some file, say hw/ppc/spapr.c, includes both this header and "hw/ppc/xics.h".
We had several build breaks detected by 'make docker-test-build@centos6'...
The correct way to address this would be to move the typedef to the
"qemu/typedefs.h" header.

This being said, docker-test-build@centos6 vanished with commit e7b3af81597,
so I guess we don't support such old distros anymore, and we can live with
duplicate typedefs.

> +void spapr_irq_msi_init(sPAPRMachineState *spapr, uint32_t nr_msis);
> +int spapr_irq_msi_alloc(sPAPRMachineState *spapr, uint32_t num, bool align,
> +Error **errp);
> +void spapr_irq_msi_free(sPAPRMachineState *spapr, int irq, uint32_t num);
> +void spapr_irq_msi_reset(sPAPRMachineState *spapr);
> +
> +#endif
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index 3c72173c7e0f..792e24453d8b 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -189,6 +189,11 @@ static void xics_system_init(MachineState *machine, int 
> nr_irqs, Error **errp)
>  sPAPRMachineState *spapr = SPAPR_MACHINE(machine);
>  Error *local_err = NULL;
>  
> +/* Initialize the MSI IRQ allocator. */
> +if (!SPAPR_MACHINE_GET_CLASS(spapr)->legacy_irq_allocation) {
> +spapr_irq_msi_init(spapr, XICS_IRQ_BASE + nr_irqs - SPAPR_IRQ_MSI);
> +}
> +
>  if (kvm_enabled()) {
>  if (machine_kernel_irqchip_allowed(machine) &&
>  !xics_kvm_init(spapr, _err)) {
> @@ -1636,6 +1641,10 @@ static void spapr_machine_reset(void)
>  ppc_set_compat(first_ppc_cpu, spapr->max_compat_pvr, _fatal);
>  }
>  
> +if 

Re: [Qemu-devel] [PULL 0/1] Monitor patches for 2018-07-31 (3.0.0-rc3)

[Peter Maydell]

On 31 July 2018 at 16:51, Markus Armbruster  wrote:
> The following changes since commit 42e76456cf68dc828b8dbd3c7e255197e9b5e57d:
>
>   Merge remote-tracking branch 
> 'remotes/vivier2/tags/linux-user-for-3.0-pull-request' into staging 
> (2018-07-31 13:52:03 +0100)
>
> are available in the Git repository at:
>
>   git://repo.or.cz/qemu/armbru.git tags/pull-monitor-2018-07-31
>
> for you to fetch changes up to 9a1054061c62311f6efcd88e4df0aa5203c7b39a:
>
>   monitor: temporary fix for dead-lock on event recursion (2018-07-31 
> 17:42:57 +0200)
>
> 
> Monitor patches for 2018-07-31 (3.0.0-rc3)
>
> 
> Marc-André Lureau (1):
>   monitor: temporary fix for dead-lock on event recursion
>
>  monitor.c | 44 +++-
>  1 file changed, 43 insertions(+), 1 deletion(-)

Applied, thanks.

-- PMM

[Qemu-devel] [PATCH v4 10/10] iotests: test nbd reconnect

[Vladimir Sementsov-Ogievskiy]

Add test, which starts backup to nbd target and restarts nbd server
during backup.

Signed-off-by: Vladimir Sementsov-Ogievskiy 
---
 tests/qemu-iotests/220| 67 +++
 tests/qemu-iotests/220.out|  7 +
 tests/qemu-iotests/group  |  1 +
 tests/qemu-iotests/iotests.py |  4 +++
 4 files changed, 79 insertions(+)
 create mode 100755 tests/qemu-iotests/220
 create mode 100644 tests/qemu-iotests/220.out

diff --git a/tests/qemu-iotests/220 b/tests/qemu-iotests/220
new file mode 100755
index 00..c9702a7dad
--- /dev/null
+++ b/tests/qemu-iotests/220
@@ -0,0 +1,67 @@
+#!/usr/bin/env python
+#
+# Test nbd reconnect
+#
+# Copyright (c) 2018 Virtuozzo International GmbH
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 2 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see .
+#
+
+import time
+
+import iotests
+from iotests import qemu_img_create, file_path, qemu_nbd_popen
+
+disk_a, disk_b, nbd_sock = file_path('disk_a', 'disk_b', 'nbd-sock')
+nbd_uri = 'nbd+unix:///exp?socket=' + nbd_sock
+
+qemu_img_create('-f', iotests.imgfmt, disk_a, '5M')
+qemu_img_create('-f', iotests.imgfmt, disk_b, '5M')
+srv = qemu_nbd_popen('-k', nbd_sock, '-x', 'exp', '-f', iotests.imgfmt, disk_b)
+time.sleep(1)
+
+vm = iotests.VM().add_drive(disk_a)
+vm.launch()
+vm.hmp_qemu_io('drive0', 'write 0 5M')
+
+print 'blockdev-add:', vm.qmp('blockdev-add', node_name='backup0', 
driver='raw',
+  file={'driver':'nbd',
+'export': 'exp',
+'server': {'type': 'unix',
+   'path': nbd_sock}})
+print 'blockdev-backup:', vm.qmp('blockdev-backup', device='drive0',
+ sync='full', target='backup0')
+
+time.sleep(1)
+print 'Kill NBD server'
+srv.kill()
+
+jobs = vm.qmp('query-block-jobs')['return']
+if jobs and jobs[0]['offset'] < jobs[0]['len']:
+print 'Backup job is still in progress'
+
+time.sleep(1)
+
+print 'Start NBD server'
+srv = qemu_nbd_popen('-k', nbd_sock, '-x', 'exp', '-f', iotests.imgfmt, disk_b)
+
+try:
+e = vm.event_wait('BLOCK_JOB_COMPLETED')
+print e['event'], ':', e['data']
+except:
+pass
+
+print 'blockdev-del:', vm.qmp('blockdev-del', node_name='backup0')
+srv.kill()
+vm.shutdown()
diff --git a/tests/qemu-iotests/220.out b/tests/qemu-iotests/220.out
new file mode 100644
index 00..dae1a49d9f
--- /dev/null
+++ b/tests/qemu-iotests/220.out
@@ -0,0 +1,7 @@
+blockdev-add: {u'return': {}}
+blockdev-backup: {u'return': {}}
+Kill NBD server
+Backup job is still in progress
+Start NBD server
+BLOCK_JOB_COMPLETED : {u'device': u'drive0', u'type': u'backup', u'speed': 0, 
u'len': 5242880, u'offset': 5242880}
+blockdev-del: {u'return': {}}
diff --git a/tests/qemu-iotests/group b/tests/qemu-iotests/group
index b973dc842d..ee2473c6a3 100644
--- a/tests/qemu-iotests/group
+++ b/tests/qemu-iotests/group
@@ -219,6 +219,7 @@
 217 rw auto quick
 218 rw auto quick
 219 rw auto
+220 rw auto quick
 221 rw auto quick
 222 rw auto quick
 223 rw auto quick
diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py
index 4e67fbbe96..17bc8c8e32 100644
--- a/tests/qemu-iotests/iotests.py
+++ b/tests/qemu-iotests/iotests.py
@@ -185,6 +185,10 @@ def qemu_nbd(*args):
 '''Run qemu-nbd in daemon mode and return the parent's exit code'''
 return subprocess.call(qemu_nbd_args + ['--fork'] + list(args))
 
+def qemu_nbd_popen(*args):
+'''Run qemu-nbd in daemon mode and return the parent's exit code'''
+return subprocess.Popen(qemu_nbd_args + ['--persistent'] + list(args))
+
 def compare_images(img1, img2, fmt1=imgfmt, fmt2=imgfmt):
 '''Return True if two image files are identical'''
 return qemu_img('compare', '-f', fmt1,
-- 
2.11.1

[Qemu-devel] [PATCH v4 02/10] block/nbd: move connection code from block/nbd to block/nbd-client

[Vladimir Sementsov-Ogievskiy]

Keep all connection code in one file, to be able to implement reconnect
in further patches.

Signed-off-by: Vladimir Sementsov-Ogievskiy 
---
 block/nbd-client.h |  2 +-
 block/nbd-client.c | 37 +++--
 block/nbd.c| 40 ++--
 3 files changed, 38 insertions(+), 41 deletions(-)

diff --git a/block/nbd-client.h b/block/nbd-client.h
index cfc90550b9..2f047ba614 100644
--- a/block/nbd-client.h
+++ b/block/nbd-client.h
@@ -41,7 +41,7 @@ typedef struct NBDClientSession {
 NBDClientSession *nbd_get_client_session(BlockDriverState *bs);
 
 int nbd_client_init(BlockDriverState *bs,
-QIOChannelSocket *sock,
+SocketAddress *saddr,
 const char *export_name,
 QCryptoTLSCreds *tlscreds,
 const char *hostname,
diff --git a/block/nbd-client.c b/block/nbd-client.c
index 1efc3d36cc..a0d8f2523e 100644
--- a/block/nbd-client.c
+++ b/block/nbd-client.c
@@ -976,8 +976,31 @@ void nbd_client_close(BlockDriverState *bs)
 nbd_teardown_connection(bs);
 }
 
+static QIOChannelSocket *nbd_establish_connection(SocketAddress *saddr,
+  Error **errp)
+{
+QIOChannelSocket *sioc;
+Error *local_err = NULL;
+
+sioc = qio_channel_socket_new();
+qio_channel_set_name(QIO_CHANNEL(sioc), "nbd-client");
+
+qio_channel_socket_connect_sync(sioc,
+saddr,
+_err);
+if (local_err) {
+object_unref(OBJECT(sioc));
+error_propagate(errp, local_err);
+return NULL;
+}
+
+qio_channel_set_delay(QIO_CHANNEL(sioc), false);
+
+return sioc;
+}
+
 int nbd_client_init(BlockDriverState *bs,
-QIOChannelSocket *sioc,
+SocketAddress *saddr,
 const char *export,
 QCryptoTLSCreds *tlscreds,
 const char *hostname,
@@ -987,6 +1010,15 @@ int nbd_client_init(BlockDriverState *bs,
 NBDClientSession *client = nbd_get_client_session(bs);
 int ret;
 
+/* establish TCP connection, return error if it fails
+ * TODO: Configurable retry-until-timeout behaviour.
+ */
+QIOChannelSocket *sioc = nbd_establish_connection(saddr, errp);
+
+if (!sioc) {
+return -ECONNREFUSED;
+}
+
 /* NBD handshake */
 logout("session init %s\n", export);
 qio_channel_set_blocking(QIO_CHANNEL(sioc), true, NULL);
@@ -1001,12 +1033,14 @@ int nbd_client_init(BlockDriverState *bs,
 g_free(client->info.x_dirty_bitmap);
 if (ret < 0) {
 logout("Failed to negotiate with the NBD server\n");
+object_unref(OBJECT(sioc));
 return ret;
 }
 if (client->info.flags & NBD_FLAG_READ_ONLY &&
 !bdrv_is_read_only(bs)) {
 error_setg(errp,
"request for write access conflicts with read-only export");
+object_unref(OBJECT(sioc));
 return -EACCES;
 }
 if (client->info.flags & NBD_FLAG_SEND_FUA) {
@@ -1020,7 +1054,6 @@ int nbd_client_init(BlockDriverState *bs,
 qemu_co_mutex_init(>send_mutex);
 qemu_co_queue_init(>free_sema);
 client->sioc = sioc;
-object_ref(OBJECT(client->sioc));
 
 if (!client->ioc) {
 client->ioc = QIO_CHANNEL(sioc);
diff --git a/block/nbd.c b/block/nbd.c
index e87699fb73..9db5eded89 100644
--- a/block/nbd.c
+++ b/block/nbd.c
@@ -295,30 +295,6 @@ NBDClientSession *nbd_get_client_session(BlockDriverState 
*bs)
 return >client;
 }
 
-static QIOChannelSocket *nbd_establish_connection(SocketAddress *saddr,
-  Error **errp)
-{
-QIOChannelSocket *sioc;
-Error *local_err = NULL;
-
-sioc = qio_channel_socket_new();
-qio_channel_set_name(QIO_CHANNEL(sioc), "nbd-client");
-
-qio_channel_socket_connect_sync(sioc,
-saddr,
-_err);
-if (local_err) {
-object_unref(OBJECT(sioc));
-error_propagate(errp, local_err);
-return NULL;
-}
-
-qio_channel_set_delay(QIO_CHANNEL(sioc), false);
-
-return sioc;
-}
-
-
 static QCryptoTLSCreds *nbd_get_tls_creds(const char *id, Error **errp)
 {
 Object *obj;
@@ -394,7 +370,6 @@ static int nbd_open(BlockDriverState *bs, QDict *options, 
int flags,
 BDRVNBDState *s = bs->opaque;
 QemuOpts *opts = NULL;
 Error *local_err = NULL;
-QIOChannelSocket *sioc = NULL;
 QCryptoTLSCreds *tlscreds = NULL;
 const char *hostname = NULL;
 int ret = -EINVAL;
@@ -434,22 +409,11 @@ static int nbd_open(BlockDriverState *bs, QDict *options, 
int flags,
 hostname = s->saddr->u.inet.host;
 }
 
-/* establish TCP connection, return error if it fails
- * TODO: Configurable retry-until-timeout behaviour.
- */
-sioc = nbd_establish_connection(s->saddr, errp);

[Qemu-devel] [PATCH v4 06/10] block/nbd-client: move from quit to state

[Vladimir Sementsov-Ogievskiy]

To implement reconnect we need several states for the client:
CONNECTED, QUIT and two CONNECTING states. CONNECTING states will
be realized in the following patches. This patch implements CONNECTED
and QUIT.

QUIT means, that we should close the connection and fail all current
and further requests (like old quit = true).

CONNECTED means that connection is ok, we can send requests (like old
quit = false).

For receiving loop we use a comparison of the current state with QUIT,
because reconnect will be in the same loop, so it should be looping
until the end.

Opposite, for requests we use a comparison of the current state with
CONNECTED, as we don't want to send requests in CONNECTING states (
which are unreachable now, but will be reachable after the following
commits)

Signed-off-by: Vladimir Sementsov-Ogievskiy 
---
 block/nbd-client.h |  9 -
 block/nbd-client.c | 55 --
 2 files changed, 41 insertions(+), 23 deletions(-)

diff --git a/block/nbd-client.h b/block/nbd-client.h
index 2f047ba614..5367425774 100644
--- a/block/nbd-client.h
+++ b/block/nbd-client.h
@@ -23,6 +23,13 @@ typedef struct {
 bool receiving; /* waiting for read_reply_co? */
 } NBDClientRequest;
 
+typedef enum NBDClientState {
+NBD_CLIENT_CONNECTING_WAIT,
+NBD_CLIENT_CONNECTING_NOWAIT,
+NBD_CLIENT_CONNECTED,
+NBD_CLIENT_QUIT
+} NBDClientState;
+
 typedef struct NBDClientSession {
 QIOChannelSocket *sioc; /* The master data channel */
 QIOChannel *ioc; /* The current I/O channel which may differ (eg TLS) */
@@ -32,10 +39,10 @@ typedef struct NBDClientSession {
 CoQueue free_sema;
 Coroutine *read_reply_co;
 int in_flight;
+NBDClientState state;
 
 NBDClientRequest requests[MAX_NBD_REQUESTS];
 NBDReply reply;
-bool quit;
 } NBDClientSession;
 
 NBDClientSession *nbd_get_client_session(BlockDriverState *bs);
diff --git a/block/nbd-client.c b/block/nbd-client.c
index 7eaf0149f0..a91fd3ea3e 100644
--- a/block/nbd-client.c
+++ b/block/nbd-client.c
@@ -34,6 +34,12 @@
 #define HANDLE_TO_INDEX(bs, handle) ((handle) ^ (uint64_t)(intptr_t)(bs))
 #define INDEX_TO_HANDLE(bs, index)  ((index)  ^ (uint64_t)(intptr_t)(bs))
 
+/* @ret would be used for reconnect in future */
+static void nbd_channel_error(NBDClientSession *s, int ret)
+{
+s->state = NBD_CLIENT_QUIT;
+}
+
 static void nbd_recv_coroutines_wake_all(NBDClientSession *s)
 {
 int i;
@@ -73,14 +79,15 @@ static coroutine_fn void nbd_read_reply_entry(void *opaque)
 int ret = 0;
 Error *local_err = NULL;
 
-while (!s->quit) {
+while (s->state != NBD_CLIENT_QUIT) {
 assert(s->reply.handle == 0);
 ret = nbd_receive_reply(s->ioc, >reply, _err);
 if (local_err) {
 error_report_err(local_err);
 }
 if (ret <= 0) {
-break;
+nbd_channel_error(s, ret ? ret : -EIO);
+continue;
 }
 
 /* There's no need for a mutex on the receive side, because the
@@ -93,7 +100,8 @@ static coroutine_fn void nbd_read_reply_entry(void *opaque)
 !s->requests[i].receiving ||
 (nbd_reply_is_structured(>reply) && !s->info.structured_reply))
 {
-break;
+nbd_channel_error(s, -EINVAL);
+continue;
 }
 
 /* We're woken up again by the request itself.  Note that there
@@ -111,7 +119,6 @@ static coroutine_fn void nbd_read_reply_entry(void *opaque)
 qemu_coroutine_yield();
 }
 
-s->quit = true;
 nbd_recv_coroutines_wake_all(s);
 s->read_reply_co = NULL;
 }
@@ -121,12 +128,18 @@ static int nbd_co_send_request(BlockDriverState *bs,
QEMUIOVector *qiov)
 {
 NBDClientSession *s = nbd_get_client_session(bs);
-int rc, i;
+int rc, i = -1;
 
 qemu_co_mutex_lock(>send_mutex);
 while (s->in_flight == MAX_NBD_REQUESTS) {
 qemu_co_queue_wait(>free_sema, >send_mutex);
 }
+
+if (s->state != NBD_CLIENT_CONNECTED) {
+rc = -EIO;
+goto err;
+}
+
 s->in_flight++;
 
 for (i = 0; i < MAX_NBD_REQUESTS; i++) {
@@ -144,16 +157,12 @@ static int nbd_co_send_request(BlockDriverState *bs,
 
 request->handle = INDEX_TO_HANDLE(s, i);
 
-if (s->quit) {
-rc = -EIO;
-goto err;
-}
 assert(s->ioc);
 
 if (qiov) {
 qio_channel_set_cork(s->ioc, true);
 rc = nbd_send_request(s->ioc, request);
-if (rc >= 0 && !s->quit) {
+if (rc >= 0 && s->state == NBD_CLIENT_CONNECTED) {
 if (qio_channel_writev_all(s->ioc, qiov->iov, qiov->niov,
NULL) < 0) {
 rc = -EIO;
@@ -168,9 +177,11 @@ static int nbd_co_send_request(BlockDriverState *bs,
 
 err:
 if (rc < 0) {
-s->quit = true;
-s->requests[i].coroutine = NULL;
-s->in_flight--;
+nbd_channel_error(s, rc);
+if (i != 

[Qemu-devel] [PATCH v4 01/10] block/nbd-client: split channel errors from export errors

[Vladimir Sementsov-Ogievskiy]

To implement nbd reconnect in further patches, we need to distinguish
error codes, returned by nbd server, from channel errors, to reconnect
only in the latter case.

Signed-off-by: Vladimir Sementsov-Ogievskiy 
Reviewed-by: Eric Blake 
---
 block/nbd-client.c | 83 +++---
 1 file changed, 47 insertions(+), 36 deletions(-)

diff --git a/block/nbd-client.c b/block/nbd-client.c
index 9686ecbd5e..1efc3d36cc 100644
--- a/block/nbd-client.c
+++ b/block/nbd-client.c
@@ -501,11 +501,11 @@ static coroutine_fn int nbd_co_do_receive_one_chunk(
  */
 static coroutine_fn int nbd_co_receive_one_chunk(
 NBDClientSession *s, uint64_t handle, bool only_structured,
-QEMUIOVector *qiov, NBDReply *reply, void **payload, Error **errp)
+int *request_ret, QEMUIOVector *qiov, NBDReply *reply, void **payload,
+Error **errp)
 {
-int request_ret;
 int ret = nbd_co_do_receive_one_chunk(s, handle, only_structured,
-  _ret, qiov, payload, errp);
+  request_ret, qiov, payload, errp);
 
 if (ret < 0) {
 s->quit = true;
@@ -515,7 +515,6 @@ static coroutine_fn int nbd_co_receive_one_chunk(
 *reply = s->reply;
 }
 s->reply.handle = 0;
-ret = request_ret;
 }
 
 if (s->read_reply_co) {
@@ -527,22 +526,17 @@ static coroutine_fn int nbd_co_receive_one_chunk(
 
 typedef struct NBDReplyChunkIter {
 int ret;
-bool fatal;
+int request_ret;
 Error *err;
 bool done, only_structured;
 } NBDReplyChunkIter;
 
-static void nbd_iter_error(NBDReplyChunkIter *iter, bool fatal,
-   int ret, Error **local_err)
+static void nbd_iter_channel_error(NBDReplyChunkIter *iter,
+   int ret, Error **local_err)
 {
 assert(ret < 0);
 
-if ((fatal && !iter->fatal) || iter->ret == 0) {
-if (iter->ret != 0) {
-error_free(iter->err);
-iter->err = NULL;
-}
-iter->fatal = fatal;
+if (!iter->ret) {
 iter->ret = ret;
 error_propagate(>err, *local_err);
 } else {
@@ -552,6 +546,15 @@ static void nbd_iter_error(NBDReplyChunkIter *iter, bool 
fatal,
 *local_err = NULL;
 }
 
+static void nbd_iter_request_error(NBDReplyChunkIter *iter, int ret)
+{
+assert(ret < 0);
+
+if (!iter->request_ret) {
+iter->request_ret = ret;
+}
+}
+
 /* NBD_FOREACH_REPLY_CHUNK
  */
 #define NBD_FOREACH_REPLY_CHUNK(s, iter, handle, structured, \
@@ -567,13 +570,13 @@ static bool nbd_reply_chunk_iter_receive(NBDClientSession 
*s,
  QEMUIOVector *qiov, NBDReply *reply,
  void **payload)
 {
-int ret;
+int ret, request_ret;
 NBDReply local_reply;
 NBDStructuredReplyChunk *chunk;
 Error *local_err = NULL;
 if (s->quit) {
 error_setg(_err, "Connection closed");
-nbd_iter_error(iter, true, -EIO, _err);
+nbd_iter_channel_error(iter, -EIO, _err);
 goto break_loop;
 }
 
@@ -587,10 +590,12 @@ static bool nbd_reply_chunk_iter_receive(NBDClientSession 
*s,
 }
 
 ret = nbd_co_receive_one_chunk(s, handle, iter->only_structured,
-   qiov, reply, payload, _err);
+   _ret, qiov, reply, payload,
+   _err);
 if (ret < 0) {
-/* If it is a fatal error s->quit is set by nbd_co_receive_one_chunk */
-nbd_iter_error(iter, s->quit, ret, _err);
+nbd_iter_channel_error(iter, ret, _err);
+} else if (request_ret < 0) {
+nbd_iter_request_error(iter, request_ret);
 }
 
 /* Do not execute the body of NBD_FOREACH_REPLY_CHUNK for simple reply. */
@@ -627,7 +632,7 @@ break_loop:
 }
 
 static int nbd_co_receive_return_code(NBDClientSession *s, uint64_t handle,
-  Error **errp)
+  int *request_ret, Error **errp)
 {
 NBDReplyChunkIter iter;
 
@@ -636,12 +641,13 @@ static int nbd_co_receive_return_code(NBDClientSession 
*s, uint64_t handle,
 }
 
 error_propagate(errp, iter.err);
+*request_ret = iter.request_ret;
 return iter.ret;
 }
 
 static int nbd_co_receive_cmdread_reply(NBDClientSession *s, uint64_t handle,
 uint64_t offset, QEMUIOVector *qiov,
-Error **errp)
+int *request_ret, Error **errp)
 {
 NBDReplyChunkIter iter;
 NBDReply reply;
@@ -666,7 +672,7 @@ static int nbd_co_receive_cmdread_reply(NBDClientSession 
*s, uint64_t handle,
 offset, qiov, _err);
 if (ret < 0) {
 s->quit = true;
-nbd_iter_error(, true, ret, _err);
+

[Qemu-devel] [PATCH v4 07/10] block/nbd-client: rename read_reply_co to connection_co

[Vladimir Sementsov-Ogievskiy]

This coroutine will serve nbd reconnects, so, rename it to be something
more generic.

Signed-off-by: Vladimir Sementsov-Ogievskiy 
---
 block/nbd-client.h |  4 ++--
 block/nbd-client.c | 24 
 2 files changed, 14 insertions(+), 14 deletions(-)

diff --git a/block/nbd-client.h b/block/nbd-client.h
index 5367425774..e7bda4d3c4 100644
--- a/block/nbd-client.h
+++ b/block/nbd-client.h
@@ -20,7 +20,7 @@
 typedef struct {
 Coroutine *coroutine;
 uint64_t offset;/* original offset of the request */
-bool receiving; /* waiting for read_reply_co? */
+bool receiving; /* waiting for connection_co? */
 } NBDClientRequest;
 
 typedef enum NBDClientState {
@@ -37,7 +37,7 @@ typedef struct NBDClientSession {
 
 CoMutex send_mutex;
 CoQueue free_sema;
-Coroutine *read_reply_co;
+Coroutine *connection_co;
 int in_flight;
 NBDClientState state;
 
diff --git a/block/nbd-client.c b/block/nbd-client.c
index a91fd3ea3e..c184a9f0e9 100644
--- a/block/nbd-client.c
+++ b/block/nbd-client.c
@@ -63,7 +63,7 @@ static void nbd_teardown_connection(BlockDriverState *bs)
 qio_channel_shutdown(client->ioc,
  QIO_CHANNEL_SHUTDOWN_BOTH,
  NULL);
-BDRV_POLL_WHILE(bs, client->read_reply_co);
+BDRV_POLL_WHILE(bs, client->connection_co);
 
 nbd_client_detach_aio_context(bs);
 object_unref(OBJECT(client->sioc));
@@ -72,7 +72,7 @@ static void nbd_teardown_connection(BlockDriverState *bs)
 client->ioc = NULL;
 }
 
-static coroutine_fn void nbd_read_reply_entry(void *opaque)
+static coroutine_fn void nbd_connection_entry(void *opaque)
 {
 NBDClientSession *s = opaque;
 uint64_t i;
@@ -105,14 +105,14 @@ static coroutine_fn void nbd_read_reply_entry(void 
*opaque)
 }
 
 /* We're woken up again by the request itself.  Note that there
- * is no race between yielding and reentering read_reply_co.  This
+ * is no race between yielding and reentering connection_co.  This
  * is because:
  *
  * - if the request runs on the same AioContext, it is only
  *   entered after we yield
  *
  * - if the request runs on a different AioContext, reentering
- *   read_reply_co happens through a bottom half, which can only
+ *   connection_co happens through a bottom half, which can only
  *   run after we yield.
  */
 aio_co_wake(s->requests[i].coroutine);
@@ -120,7 +120,7 @@ static coroutine_fn void nbd_read_reply_entry(void *opaque)
 }
 
 nbd_recv_coroutines_wake_all(s);
-s->read_reply_co = NULL;
+s->connection_co = NULL;
 }
 
 static int nbd_co_send_request(BlockDriverState *bs,
@@ -428,7 +428,7 @@ static coroutine_fn int nbd_co_do_receive_one_chunk(
 }
 *request_ret = 0;
 
-/* Wait until we're woken up by nbd_read_reply_entry.  */
+/* Wait until we're woken up by nbd_connection_entry.  */
 s->requests[i].receiving = true;
 qemu_coroutine_yield();
 s->requests[i].receiving = false;
@@ -503,7 +503,7 @@ static coroutine_fn int nbd_co_do_receive_one_chunk(
 }
 
 /* nbd_co_receive_one_chunk
- * Read reply, wake up read_reply_co and set s->quit if needed.
+ * Read reply, wake up connection_co and set s->quit if needed.
  * Return value is a fatal error code or normal nbd reply error code
  */
 static coroutine_fn int nbd_co_receive_one_chunk(
@@ -517,15 +517,15 @@ static coroutine_fn int nbd_co_receive_one_chunk(
 if (ret < 0) {
 nbd_channel_error(s, ret);
 } else {
-/* For assert at loop start in nbd_read_reply_entry */
+/* For assert at loop start in nbd_connection_entry */
 if (reply) {
 *reply = s->reply;
 }
 s->reply.handle = 0;
 }
 
-if (s->read_reply_co) {
-aio_co_wake(s->read_reply_co);
+if (s->connection_co) {
+aio_co_wake(s->connection_co);
 }
 
 return ret;
@@ -966,7 +966,7 @@ void nbd_client_attach_aio_context(BlockDriverState *bs,
 {
 NBDClientSession *client = nbd_get_client_session(bs);
 qio_channel_attach_aio_context(QIO_CHANNEL(client->ioc), new_context);
-aio_co_schedule(new_context, client->read_reply_co);
+aio_co_schedule(new_context, client->connection_co);
 }
 
 void nbd_client_close(BlockDriverState *bs)
@@ -1066,7 +1066,7 @@ static int nbd_client_connect(BlockDriverState *bs,
 /* Now that we're connected, set the socket to be non-blocking and
  * kick the reply mechanism.  */
 qio_channel_set_blocking(QIO_CHANNEL(sioc), false, NULL);
-client->read_reply_co = qemu_coroutine_create(nbd_read_reply_entry, 
client);
+client->connection_co = qemu_coroutine_create(nbd_connection_entry, 
client);
 nbd_client_attach_aio_context(bs, bdrv_get_aio_context(bs));
 
 logout("Established connection with NBD server\n");
-- 
2.11.1

[Qemu-devel] [PATCH v4 03/10] block/nbd-client: split connection from initialization

[Vladimir Sementsov-Ogievskiy]

Split connection code to reuse it for reconnect.

Signed-off-by: Vladimir Sementsov-Ogievskiy 
---
 block/nbd-client.c | 33 -
 1 file changed, 24 insertions(+), 9 deletions(-)

diff --git a/block/nbd-client.c b/block/nbd-client.c
index a0d8f2523e..a1813cbfe1 100644
--- a/block/nbd-client.c
+++ b/block/nbd-client.c
@@ -999,13 +999,13 @@ static QIOChannelSocket 
*nbd_establish_connection(SocketAddress *saddr,
 return sioc;
 }
 
-int nbd_client_init(BlockDriverState *bs,
-SocketAddress *saddr,
-const char *export,
-QCryptoTLSCreds *tlscreds,
-const char *hostname,
-const char *x_dirty_bitmap,
-Error **errp)
+static int nbd_client_connect(BlockDriverState *bs,
+  SocketAddress *saddr,
+  const char *export,
+  QCryptoTLSCreds *tlscreds,
+  const char *hostname,
+  const char *x_dirty_bitmap,
+  Error **errp)
 {
 NBDClientSession *client = nbd_get_client_session(bs);
 int ret;
@@ -1051,8 +1051,6 @@ int nbd_client_init(BlockDriverState *bs,
 bs->supported_zero_flags |= BDRV_REQ_MAY_UNMAP;
 }
 
-qemu_co_mutex_init(>send_mutex);
-qemu_co_queue_init(>free_sema);
 client->sioc = sioc;
 
 if (!client->ioc) {
@@ -1069,3 +1067,20 @@ int nbd_client_init(BlockDriverState *bs,
 logout("Established connection with NBD server\n");
 return 0;
 }
+
+int nbd_client_init(BlockDriverState *bs,
+SocketAddress *saddr,
+const char *export,
+QCryptoTLSCreds *tlscreds,
+const char *hostname,
+const char *x_dirty_bitmap,
+Error **errp)
+{
+NBDClientSession *client = nbd_get_client_session(bs);
+
+qemu_co_mutex_init(>send_mutex);
+qemu_co_queue_init(>free_sema);
+
+return nbd_client_connect(bs, saddr, export, tlscreds, hostname,
+  x_dirty_bitmap, errp);
+}
-- 
2.11.1

[Qemu-devel] [PATCH v4 04/10] block/nbd-client: fix nbd_reply_chunk_iter_receive

[Vladimir Sementsov-Ogievskiy]

Use exported report, not the variable to be reused (should not really
matter).

Signed-off-by: Vladimir Sementsov-Ogievskiy 
---
 block/nbd-client.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/block/nbd-client.c b/block/nbd-client.c
index a1813cbfe1..263d1721f9 100644
--- a/block/nbd-client.c
+++ b/block/nbd-client.c
@@ -599,7 +599,7 @@ static bool nbd_reply_chunk_iter_receive(NBDClientSession 
*s,
 }
 
 /* Do not execute the body of NBD_FOREACH_REPLY_CHUNK for simple reply. */
-if (nbd_reply_is_simple(>reply) || s->quit) {
+if (nbd_reply_is_simple(reply) || s->quit) {
 goto break_loop;
 }
 
-- 
2.11.1

[Qemu-devel] [PATCH v4 09/10] block/nbd-client: nbd reconnect

[Vladimir Sementsov-Ogievskiy]

Implement reconnect. To achieve this:

1. add new modes:
   connecting-wait: means, that reconnecting is in progress, and there
 were small number of reconnect attempts, so all requests are
 waiting for the connection.
   connecting-nowait: reconnecting is in progress, there were a lot of
 attempts of reconnect, all requests will return errors.

   two old modes are used too:
   connected: normal state
   quit: exiting after fatal error or on close

Possible transitions are:

   * -> quit
   connecting-* -> connected
   connecting-wait -> connecting-nowait (transition is done after
  reconnect-delay seconds in connecting-wait mode)
   connected -> connecting-wait

2. Implement reconnect in connection_co. So, in connecting-* mode,
connection_co, tries to reconnect unlimited times.

3. Retry nbd queries on channel error, if we are in connecting-wait
state.

Signed-off-by: Vladimir Sementsov-Ogievskiy 
---
 block/nbd-client.h |   4 +
 block/nbd-client.c | 304 +++--
 2 files changed, 255 insertions(+), 53 deletions(-)

diff --git a/block/nbd-client.h b/block/nbd-client.h
index ef8a6a9239..52e4ec66be 100644
--- a/block/nbd-client.h
+++ b/block/nbd-client.h
@@ -40,6 +40,10 @@ typedef struct NBDClientSession {
 Coroutine *connection_co;
 int in_flight;
 NBDClientState state;
+bool receiving;
+int connect_status;
+Error *connect_err;
+bool wait_in_flight;
 
 NBDClientRequest requests[MAX_NBD_REQUESTS];
 NBDReply reply;
diff --git a/block/nbd-client.c b/block/nbd-client.c
index 41e6e6e702..b09907096d 100644
--- a/block/nbd-client.c
+++ b/block/nbd-client.c
@@ -34,10 +34,26 @@
 #define HANDLE_TO_INDEX(bs, handle) ((handle) ^ (uint64_t)(intptr_t)(bs))
 #define INDEX_TO_HANDLE(bs, index)  ((index)  ^ (uint64_t)(intptr_t)(bs))
 
-/* @ret would be used for reconnect in future */
+static int nbd_client_connect(BlockDriverState *bs,
+  SocketAddress *saddr,
+  const char *export,
+  QCryptoTLSCreds *tlscreds,
+  const char *hostname,
+  const char *x_dirty_bitmap,
+  Error **errp);
+
 static void nbd_channel_error(NBDClientSession *s, int ret)
 {
-s->state = NBD_CLIENT_QUIT;
+if (ret == -EIO) {
+if (s->state == NBD_CLIENT_CONNECTED) {
+s->state = NBD_CLIENT_CONNECTING_WAIT;
+}
+} else {
+if (s->state == NBD_CLIENT_CONNECTED) {
+qio_channel_shutdown(s->ioc, QIO_CHANNEL_SHUTDOWN_BOTH, NULL);
+}
+s->state = NBD_CLIENT_QUIT;
+}
 }
 
 static void nbd_recv_coroutines_wake_all(NBDClientSession *s)
@@ -57,33 +73,151 @@ static void nbd_teardown_connection(BlockDriverState *bs)
 {
 NBDClientSession *client = nbd_get_client_session(bs);
 
-assert(client->ioc);
-
-/* finish any pending coroutines */
-qio_channel_shutdown(client->ioc,
- QIO_CHANNEL_SHUTDOWN_BOTH,
- NULL);
+if (client->state == NBD_CLIENT_CONNECTED) {
+/* finish any pending coroutines */
+assert(client->ioc);
+qio_channel_shutdown(client->ioc, QIO_CHANNEL_SHUTDOWN_BOTH, NULL);
+}
+client->state = NBD_CLIENT_QUIT;
 BDRV_POLL_WHILE(bs, client->connection_co);
+}
+
+typedef struct NBDConnection {
+BlockDriverState *bs;
+SocketAddress *saddr;
+const char *export;
+QCryptoTLSCreds *tlscreds;
+const char *hostname;
+const char *x_dirty_bitmap;
+uint32_t reconnect_delay;
+} NBDConnection;
+
+static bool nbd_client_connecting(NBDClientSession *client)
+{
+return client->state == NBD_CLIENT_CONNECTING_WAIT ||
+   client->state == NBD_CLIENT_CONNECTING_NOWAIT;
+}
+
+static bool nbd_client_connecting_wait(NBDClientSession *client)
+{
+return client->state == NBD_CLIENT_CONNECTING_WAIT;
+}
+
+static coroutine_fn void nbd_reconnect_attempt(NBDConnection *con)
+{
+NBDClientSession *s = nbd_get_client_session(con->bs);
+Error *local_err = NULL;
+
+assert(nbd_client_connecting(s));
+
+/* Wait completion of all in-flight requests */
+
+qemu_co_mutex_lock(>send_mutex);
+
+while (s->in_flight > 0) {
+qemu_co_mutex_unlock(>send_mutex);
+nbd_recv_coroutines_wake_all(s);
+s->wait_in_flight = true;
+qemu_coroutine_yield();
+s->wait_in_flight = false;
+qemu_co_mutex_lock(>send_mutex);
+}
+
+qemu_co_mutex_unlock(>send_mutex);
+
+/* Now we are sure, that nobody accessing the channel now and nobody
+ * will try to access the channel, until we set state to CONNECTED
+ */
+
+/* Finalize previous connection if any */
+if (s->ioc) {
+nbd_client_detach_aio_context(con->bs);
+object_unref(OBJECT(s->sioc));
+s->sioc = NULL;
+object_unref(OBJECT(s->ioc));
+   

[Qemu-devel] [PATCH v4 08/10] block/nbd: add cmdline and qapi parameter reconnect-delay

[Vladimir Sementsov-Ogievskiy]

Reconnect will be implemented in the following commit, so for now,
in semantics below, disconnect itself is a "serious error".

Signed-off-by: Vladimir Sementsov-Ogievskiy 
---
 qapi/block-core.json | 12 +++-
 block/nbd-client.h   |  1 +
 block/nbd-client.c   |  1 +
 block/nbd.c  | 16 +++-
 4 files changed, 28 insertions(+), 2 deletions(-)

diff --git a/qapi/block-core.json b/qapi/block-core.json
index 5b9084a394..cf03402ec5 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -3511,13 +3511,23 @@
 #  traditional "base:allocation" block status (see
 #  NBD_OPT_LIST_META_CONTEXT in the NBD protocol) (since 3.0)
 #
+# @reconnect-delay: Reconnect delay. On disconnect, nbd client tries to connect
+#   again, until success or serious error. During first
+#   @reconnect-delay seconds of reconnecting loop all requests
+#   are paused and have a chance to rerun, if successful
+#   connect occures during this time. After @reconnect-delay
+#   seconds all delayed requests are failed and all following
+#   requests will be failed to (until successfull reconnect).
+#   Default 300 seconds (Since 3.1)
+#
 # Since: 2.9
 ##
 { 'struct': 'BlockdevOptionsNbd',
   'data': { 'server': 'SocketAddress',
 '*export': 'str',
 '*tls-creds': 'str',
-'*x-dirty-bitmap': 'str' } }
+'*x-dirty-bitmap': 'str',
+'*reconnect-delay': 'uint32' } }
 
 ##
 # @BlockdevOptionsRaw:
diff --git a/block/nbd-client.h b/block/nbd-client.h
index e7bda4d3c4..ef8a6a9239 100644
--- a/block/nbd-client.h
+++ b/block/nbd-client.h
@@ -53,6 +53,7 @@ int nbd_client_init(BlockDriverState *bs,
 QCryptoTLSCreds *tlscreds,
 const char *hostname,
 const char *x_dirty_bitmap,
+uint32_t reconnect_delay,
 Error **errp);
 void nbd_client_close(BlockDriverState *bs);
 
diff --git a/block/nbd-client.c b/block/nbd-client.c
index c184a9f0e9..41e6e6e702 100644
--- a/block/nbd-client.c
+++ b/block/nbd-client.c
@@ -1079,6 +1079,7 @@ int nbd_client_init(BlockDriverState *bs,
 QCryptoTLSCreds *tlscreds,
 const char *hostname,
 const char *x_dirty_bitmap,
+uint32_t reconnect_delay,
 Error **errp)
 {
 NBDClientSession *client = nbd_get_client_session(bs);
diff --git a/block/nbd.c b/block/nbd.c
index 9db5eded89..b40fb5a977 100644
--- a/block/nbd.c
+++ b/block/nbd.c
@@ -360,6 +360,18 @@ static QemuOptsList nbd_runtime_opts = {
 .help = "experimental: expose named dirty bitmap in place of "
 "block status",
 },
+{
+.name = "reconnect-delay",
+.type = QEMU_OPT_NUMBER,
+.help = "Reconnect delay. On disconnect, nbd client tries to"
+"connect again, until success or serious error. During"
+"first @reconnect-delay seconds of reconnecting loop all"
+"requests are paused and have a chance to rerun, if"
+"successful connect occures during this time. After"
+"@reconnect-delay seconds all delayed requests are failed"
+"and all following requests will be failed to (until"
+"successfull reconnect). Default 300 seconds",
+},
 { /* end of list */ }
 },
 };
@@ -411,7 +423,9 @@ static int nbd_open(BlockDriverState *bs, QDict *options, 
int flags,
 
 /* NBD handshake */
 ret = nbd_client_init(bs, s->saddr, s->export, tlscreds, hostname,
-  qemu_opt_get(opts, "x-dirty-bitmap"), errp);
+  qemu_opt_get(opts, "x-dirty-bitmap"),
+  qemu_opt_get_number(opts, "reconnect-delay", 300),
+  errp);
 
  error:
 if (tlscreds) {
-- 
2.11.1

[Qemu-devel] [PATCH v4 00/10] NBD reconnect

[Vladimir Sementsov-Ogievskiy]

Hi all.

Here is NBD reconnect. Previously, if connection failed all current
and future requests will fail. After the series, nbd-client driver
will try to reconnect unlimited times. During first @reconnect-delay
seconds of reconnecting all requests will wait for the connection,
and if it is established requests will be resent. After
@reconnect-delay period all requests will be failed (until successful
reconnect).

v4: - add Eric's r-b to 01.
- drop CONNECTING_INIT mode, don't reconnect on _open.
- new api: only one parameter @reconnect-delay
- new interval scheme between reconnect attempts
(1 - 2 - 4 - 8 - 16 - 16 ... seconds)
- fixes and refactorings in main patch (09), including merge with
  old 08 patch


v3:
06: fix build error in function 'nbd_co_send_request':
 error: 'i' may be used uninitialized in this function

v2 notes:
Here is v2 of NBD reconnect, but it is very very different from v1, so,
forget about v1.
The series includes my "NBD reconnect: preliminary refactoring", with
changes in 05: leave asserts (Eric).

Vladimir Sementsov-Ogievskiy (10):
  block/nbd-client: split channel errors from export errors
  block/nbd: move connection code from block/nbd to block/nbd-client
  block/nbd-client: split connection from initialization
  block/nbd-client: fix nbd_reply_chunk_iter_receive
  block/nbd-client: don't check ioc
  block/nbd-client: move from quit to state
  block/nbd-client: rename read_reply_co to connection_co
  block/nbd: add cmdline and qapi parameter reconnect-delay
  block/nbd-client: nbd reconnect
  iotests: test nbd reconnect

 qapi/block-core.json  |  12 +-
 block/nbd-client.h|  20 +-
 block/nbd-client.c| 515 +++---
 block/nbd.c   |  56 ++---
 tests/qemu-iotests/220|  67 ++
 tests/qemu-iotests/220.out|   7 +
 tests/qemu-iotests/group  |   1 +
 tests/qemu-iotests/iotests.py |   4 +
 8 files changed, 512 insertions(+), 170 deletions(-)
 create mode 100755 tests/qemu-iotests/220
 create mode 100644 tests/qemu-iotests/220.out

-- 
2.11.1

[Qemu-devel] [PATCH v4 05/10] block/nbd-client: don't check ioc

[Vladimir Sementsov-Ogievskiy]

We have several paranoiac checks for ioc != NULL. But ioc may become
NULL only on close, which should not happen during requests handling.
Also, we check ioc only sometimes, not after each yield, which is
inconsistent. Let's drop these checks. However, for safety, lets leave
asserts instead.

Signed-off-by: Vladimir Sementsov-Ogievskiy 
---
 block/nbd-client.c | 16 +---
 1 file changed, 5 insertions(+), 11 deletions(-)

diff --git a/block/nbd-client.c b/block/nbd-client.c
index 263d1721f9..7eaf0149f0 100644
--- a/block/nbd-client.c
+++ b/block/nbd-client.c
@@ -51,9 +51,7 @@ static void nbd_teardown_connection(BlockDriverState *bs)
 {
 NBDClientSession *client = nbd_get_client_session(bs);
 
-if (!client->ioc) { /* Already closed */
-return;
-}
+assert(client->ioc);
 
 /* finish any pending coroutines */
 qio_channel_shutdown(client->ioc,
@@ -150,10 +148,7 @@ static int nbd_co_send_request(BlockDriverState *bs,
 rc = -EIO;
 goto err;
 }
-if (!s->ioc) {
-rc = -EPIPE;
-goto err;
-}
+assert(s->ioc);
 
 if (qiov) {
 qio_channel_set_cork(s->ioc, true);
@@ -426,10 +421,11 @@ static coroutine_fn int nbd_co_do_receive_one_chunk(
 s->requests[i].receiving = true;
 qemu_coroutine_yield();
 s->requests[i].receiving = false;
-if (!s->ioc || s->quit) {
+if (s->quit) {
 error_setg(errp, "Connection closed");
 return -EIO;
 }
+assert(s->ioc);
 
 assert(s->reply.handle == handle);
 
@@ -967,9 +963,7 @@ void nbd_client_close(BlockDriverState *bs)
 NBDClientSession *client = nbd_get_client_session(bs);
 NBDRequest request = { .type = NBD_CMD_DISC };
 
-if (client->ioc == NULL) {
-return;
-}
+assert(client->ioc);
 
 nbd_send_request(client->ioc, );
 
-- 
2.11.1

Re: [Qemu-devel] [PATCH v5 39/76] target/mips: Add emulation of DSP ASE for nanoMIPS - part 2

[Richard Henderson]

On 07/30/2018 12:12 PM, Aleksandar Markovic wrote:

> +case NM_BPOSGE32C:
> +check_dsp(ctx);
> +{
> +int32_t imm = ctx->opcode;
> +imm >>= 1;
> +imm &= 0x1fff;
> +imm |= (ctx->opcode & 1) << 13;

extract32.

> +
> +gen_compute_branch(ctx, OPC_BPOSGE32, 4, -1, -2,
> +   (int32_t)imm, 4);

Useless cast.  And see my other response re gen_compute_branch.


r~

Re: [Qemu-devel] [PATCH v5 38/76] target/mips: Add emulation of DSP ASE for nanoMIPS - part 1

[Richard Henderson]

On 07/30/2018 12:12 PM, Aleksandar Markovic wrote:
> +case OPC_CMPGU_EQ_QB:
> +check_dsp(ctx);
> +gen_helper_cmpgu_eq_qb(cpu_gpr[ret], v1_t, v2_t);
> +break;

Missing a test for ret == 0, here and many other places.
I believe some (many?) of these have side-effects, so you'll
probably want to store into e.g. v1_t and gen_store_gpr.


r~

Re: [Qemu-devel] [PATCH v3 3/7] hw/arm: make bitbanded IO optional on ARM M Profile

[Peter Maydell]

On 25 July 2018 at 09:59, Stefan Hajnoczi  wrote:
> Some ARM CPUs have bitbanded IO, a memory region that allows convenient
> bit access via 32-bit memory loads/stores.  This eliminates the need for
> read-modify-update instruction sequences.
>
> This patch makes this optional feature a ARMMProfile qdev property,
> allowing boards to choose whether they want bitbanding or not.
>
> Status of boards:
>  * iotkit (Cortex M33), no bitband
>  * mps2 (Cortex M3), bitband
>  * msf2 (Cortex M3), bitband
>  * stellaris (Cortex M3), bitband
>  * stm32f205 (Cortex M3), bitband
>
> Signed-off-by: Stefan Hajnoczi 

This fixes a bug in the MPS2 boards, incidentally, where the
Ethernet controller is inaccessible because it's hidden by
the shouldn't-be-present bitbanding region...

thanks
-- PMM

Re: [Qemu-devel] [PATCH v5 36/76] target/mips: Add emulation of nanoMIPS 32-bit branch instructions

[Richard Henderson]

On 07/30/2018 12:12 PM, Aleksandar Markovic wrote:
> +/* Immediate Value Compact Branches */
> +static void gen_compute_imm_branch(DisasContext *ctx, uint32_t opc,
> +   int rt, int32_t imm, int32_t offset)
> +{
> +TCGCond cond;
> +int bcond_compute = 0;
> +TCGv t0 = tcg_temp_new();
> +TCGv t1 = tcg_temp_new();
> +
> +if (ctx->hflags & MIPS_HFLAG_BMASK) {
> +#ifdef MIPS_DEBUG_DISAS
> +LOG_DISAS("Branch in delay / forbidden slot at PC 0x" TARGET_FMT_lx
> +  "\n", ctx->base.pc_next);
> +#endif
> +generate_exception_end(ctx, EXCP_RI);
> +goto out;
> +}

What is this,

> +ctx->hflags |= MIPS_HFLAG_FBNSLOT;

and this (and several other occurrences), given that forbidden slots have been
removed from the nanomips architecture?

I think perhaps all of your branching code, utilizing the existing expanders,
needs a re-think.


r~

Re: [Qemu-devel] [PATCH v5 35/76] target/mips: Add emulation of nanoMIPS 32-bit load and store instructions

[Richard Henderson]

On 07/30/2018 12:12 PM, Aleksandar Markovic wrote:
>  case NM_P_GP_BH:
> +{
> +uint32_t u = extract32(ctx->opcode, 0, 18);
> +switch (extract32(ctx->opcode, 18, 3)) {
> +case NM_LBGP:
> +gen_ld(ctx, OPC_LB, rt, 28, u);
> +break;
> +case NM_SBGP:
> +gen_st(ctx, OPC_SB, rt, 28, u);
> +break;
> +case NM_LBUGP:
> +gen_ld(ctx, OPC_LBU, rt, 28, u);
> +break;
> +case NM_ADDIUGP_B:
> +if (rt != 0) {
> +uint32_t offset = extract32(ctx->opcode, 0, 18);

Duplicate extract of U.

Otherwise,
Reviewed-by: Richard Henderson 


r~

Re: [Qemu-devel] [PATCH v5 32/76] target/mips: Add emulation of misc nanoMIPS instructions (p_lsx)

[Richard Henderson]

On 07/30/2018 12:12 PM, Aleksandar Markovic wrote:
> +static void gen_p_lsx(DisasContext *ctx, int rd, int rs, int rt)
> +{
> +TCGv t0, t1;
> +t0 = tcg_temp_new();
> +t1 = tcg_temp_new();
> +
> +gen_load_gpr(t0, rs);
> +gen_load_gpr(t1, rt);
> +
> +if ((extract32(ctx->opcode, 6, 1)) == 1) {
> +/* PP.LSXS instructions require shifting */
> +switch (extract32(ctx->opcode, 7, 4)) {
> +case NM_LHXS:
> +case NM_SHXS:
> +case NM_LHUXS:
> +tcg_gen_shli_tl(t0, t0, 1);
> +break;
> +case NM_LWXS:
> +case NM_SWXS:
> +case NM_LWC1XS:
> +case NM_SWC1XS:
> +tcg_gen_shli_tl(t0, t0, 2);
> +break;
> +case NM_LDC1XS:
> +case NM_SDC1XS:
> +tcg_gen_shli_tl(t0, t0, 3);
> +break;
> +}
> +}
> +gen_op_addr_add(ctx, t0, t0, t1);
> +
> +switch (extract32(ctx->opcode, 7, 4)) {
> +case NM_LBX:
> +tcg_gen_qemu_ld_tl(t0, t0, ctx->mem_idx,
> +   MO_SB);
> +gen_store_gpr(t0, rd);
> +break;
> +case NM_LHX:
> +/*case NM_LHXS:*/
> +tcg_gen_qemu_ld_tl(t0, t0, ctx->mem_idx,
> +   MO_TESW);

Elsewhere in translate you use ctx->default_tcg_memop_mask.

You probably should tidy this up with a table or another switch that computes
the TCGMemOp for the size and sign of the operation.  Then you don't have to
modify quite so many places fixing this.  Something like

unsigned opc = extract32(ctx->opcode, 7, 4);
TCGMemOp mop;

switch (opc) {
case NM_LBX:
mop = MO_SB;
break;
...
}
mop |= ctx->default_tcg_memop_mask;

if (extract32(ctx->opcode, 6, 1)) {
tcg_gen_shli_tl(t0, t0, mop & MO_SIZE);
}
gen_op_addr_add(ctx, t0, t0, t1);

switch (opc) {
case NM_LBX: // and all other integer loads
   tcg_gen_qemu_ld_tl(t0, t0, ctx->mem_idx, mop);
   gen_store_gpr(t0, rd);
   break;
case NM_SBX: // and all other integer stores
   gen_load_gpr(t1, td);
   tcg_gen_qemu_st_tl(t1, t0, ctx->mem_idx, mop);
   break;
case NM_LWC1X: // and all other fp ops
   if (ctx->CP0_Config1...) {
  ...
   }
}


r~

Re: [Qemu-devel] [PATCH] throttle-groups: fix hang when group member leaves

[Alberto Garcia]

On Wed 04 Jul 2018 04:54:10 PM CEST, Stefan Hajnoczi wrote:
> Throttle groups consist of members sharing one throttling state
> (including bps/iops limits).  Round-robin scheduling is used to ensure
> fairness.  If a group member already has a timer pending then other
> groups members do not schedule their own timers.  The next group
> member will have its turn when the existing timer expires.
>
> A hang may occur when a group member leaves while it had a timer
> scheduled.

Ok, I can reproduce this if I run fio with iodepth=1.

We're draining the BDS before removing it from a throttle group, and
therefore there cannot be any pending requests.

So the problem seems to be that when throttle_co_drain_begin() runs the
pending requests from a member using throttle_group_co_restart_queue(),
it simply uses qemu_co_queue_next() and doesn't touch the timer at all.

So it can happen that there's a request in the queue waiting for a
timer, and after that call the request is gone but the timer remains.

The current patch is perhaps not worth touching at this point (we're
about to release QEMU 3.0), but I think that a better solution would be
to either

a) cancel the existing timer and reset tg->any_timer_armed on the given
   tgm after throttle_group_co_restart_queue() and before
   schedule_next_request() if the queue is empty.

b) force the existing timer to run immediately instead of calling
   throttle_group_co_restart_queue(). Seems cleaner, but I haven't tried
   this one yet.

I'll explore them a bit and send a patch.

Berto

Re: [Qemu-devel] [PATCH QEMU] hw/char/sh_serial: Add timeout handling to unbreak serial input

[Rob Landley]

On 07/30/2018 10:18 AM, Ulrich Hecht wrote:>> On July 30, 2018 at 3:02 PM Geert
Uytterhoeven  wrote:
>> Fix this by adding basic timeout handling.  As the QEMU SCIF emulation
>> ignores any serial speed programming, the timeout value used conforms to
>> a default speed of 9600 bps, which is fine for any interative console.
>>
>> Reported-by: Rob Landley 
>> Signed-off-by: Geert Uytterhoeven 
> 
> Works for me, kernel 4.18-rc7 for rts7751r2dplus.

Works for me too.

Tested-by: Rob Landley 

Rob

Re: [Qemu-devel] ARM: SVE while issue

[Richard Henderson]

On 07/31/2018 09:52 AM, Laurent Desnogues wrote:
> Hello Richard,
> 
> according to SVE specification, whilels/whilele instructions have a
> special case where if the second operand is the maximum (un)signed
> integer then the result is an all-true predicate.  The current code in
> trans_WHILE doesn't seem to capture that requirement.  I'm afraid the
> fix won't be as trivial as for the other two bugs I reported :-)

Still not too bad.
Testing a fix for my branch now.

Annoying that risu didn't randomly generate this case,
and I totally mis-read the documentation for this.


r~

Re: [Qemu-devel] [PATCH v2] monitor: temporary fix for dead-lock on event recursion

[Markus Armbruster]

Eric Blake  writes:

> On 07/31/2018 10:01 AM, Marc-André Lureau wrote:
>> With a Spice port chardev, it is possible to reenter
>> monitor_qapi_event_queue() (when the client disconnects for
>> example). This will dead-lock on monitor_lock.
>>
>> Instead, use some TLS variables to check for recursion and queue the
>> events.
>>
>
>>
>> diff --git a/monitor.c b/monitor.c
>> index d8d8211ae4..4d9c1873bc 100644
>> --- a/monitor.c
>> +++ b/monitor.c
>> @@ -633,7 +633,7 @@ static void monitor_qapi_event_handler(void *opaque);
>>* applying any rate limiting if required.
>>*/
>>   static void
>> -monitor_qapi_event_queue(QAPIEvent event, QDict *qdict, Error **errp)
>> +monitor_qapi_event_queue_no_recurse(QAPIEvent event, QDict *qdict)
>>   {
>>   MonitorQAPIEventConf *evconf;
>>   MonitorQAPIEventState *evstate;
>> @@ -688,6 +688,48 @@ monitor_qapi_event_queue(QAPIEvent event, QDict *qdict, 
>> Error **errp)
>>   qemu_mutex_unlock(_lock);
>>   }
>>   +static void
>> +monitor_qapi_event_queue(QAPIEvent event, QDict *qdict, Error **errp)
>> +{
>> +/*
>> + * monitor_qapi_event_queue_no_recurse() is not reentrant: it
>> + * would deadlock on monitor_lock.  Work around by queueing
>> + * events in thread-local storage.
>> + * TODO: remove this, make it re-enter safe.
>> + */
>> +static __thread bool reentered;
>> +typedef struct MonitorQapiEvent {
>> +QAPIEvent event;
>> +QDict *qdict;
>> +QSIMPLEQ_ENTRY(MonitorQapiEvent) entry;
>> +} MonitorQapiEvent;
>> +MonitorQapiEvent *ev;
>> +static __thread QSIMPLEQ_HEAD(, MonitorQapiEvent) event_queue;
>> +
>> +if (!reentered) {
>> +QSIMPLEQ_INIT(_queue);
>> +}
>
> Is it safe to call QSIMPLEQ_INIT() on an already-initialized but empty
> queue?

Yes.

#define QSIMPLEQ_HEAD(name, type)   \
struct name {   \
struct type *sqh_first;/* first element */  \
struct type **sqh_last;/* addr of last next element */  \
}

#define QSIMPLEQ_INIT(head) do {\
(head)->sqh_first = NULL;   \
(head)->sqh_last = &(head)->sqh_first;  \
} while (/*CONSTCOND*/0)


Removing the last member results in the same state as QSIMPLEQ_INIT():

#define QSIMPLEQ_REMOVE_HEAD(head, field) do {  \
if (((head)->sqh_first = (head)->sqh_first->field.sqe_next) == NULL)\
(head)->sqh_last = &(head)->sqh_first;  \
} while (/*CONSTCOND*/0)

Calling QSIMPLEQ_INIT() again then is a no-op.

>
>> +
>> +ev = g_new(MonitorQapiEvent, 1);
>> +ev->qdict = qobject_ref(qdict);
>> +ev->event = event;
>> +QSIMPLEQ_INSERT_TAIL(_queue, ev, entry);
>> +if (reentered) {
>> +return;
>> +}
>> +
>> +reentered = true;
>> +
>> +while ((ev = QSIMPLEQ_FIRST(_queue)) != NULL) {
>> +QSIMPLEQ_REMOVE_HEAD(_queue, entry);
>> +monitor_qapi_event_queue_no_recurse(ev->event, ev->qdict);
>> +qobject_unref(ev->qdict);
>> +g_free(ev);
>> +}
>> +
>> +reentered = false;
>> +}
>> +
>>   /*
>>* This function runs evconf->rate ns after sending a throttled
>>* event.
>>

Re: [Qemu-devel] [PATCH v2] monitor: temporary fix for dead-lock on event recursion

[Marc-André Lureau]

Hi

On Tue, Jul 31, 2018 at 6:03 PM, Eric Blake  wrote:
> On 07/31/2018 10:01 AM, Marc-André Lureau wrote:
>>
>> With a Spice port chardev, it is possible to reenter
>> monitor_qapi_event_queue() (when the client disconnects for
>> example). This will dead-lock on monitor_lock.
>>
>> Instead, use some TLS variables to check for recursion and queue the
>> events.
>>
>
>>
>> diff --git a/monitor.c b/monitor.c
>> index d8d8211ae4..4d9c1873bc 100644
>> --- a/monitor.c
>> +++ b/monitor.c
>> @@ -633,7 +633,7 @@ static void monitor_qapi_event_handler(void *opaque);
>>* applying any rate limiting if required.
>>*/
>>   static void
>> -monitor_qapi_event_queue(QAPIEvent event, QDict *qdict, Error **errp)
>> +monitor_qapi_event_queue_no_recurse(QAPIEvent event, QDict *qdict)
>>   {
>>   MonitorQAPIEventConf *evconf;
>>   MonitorQAPIEventState *evstate;
>> @@ -688,6 +688,48 @@ monitor_qapi_event_queue(QAPIEvent event, QDict
>> *qdict, Error **errp)
>>   qemu_mutex_unlock(_lock);
>>   }
>>   +static void
>> +monitor_qapi_event_queue(QAPIEvent event, QDict *qdict, Error **errp)
>> +{
>> +/*
>> + * monitor_qapi_event_queue_no_recurse() is not reentrant: it
>> + * would deadlock on monitor_lock.  Work around by queueing
>> + * events in thread-local storage.
>> + * TODO: remove this, make it re-enter safe.
>> + */
>> +static __thread bool reentered;
>> +typedef struct MonitorQapiEvent {
>> +QAPIEvent event;
>> +QDict *qdict;
>> +QSIMPLEQ_ENTRY(MonitorQapiEvent) entry;
>> +} MonitorQapiEvent;
>> +MonitorQapiEvent *ev;
>> +static __thread QSIMPLEQ_HEAD(, MonitorQapiEvent) event_queue;
>> +
>> +if (!reentered) {
>> +QSIMPLEQ_INIT(_queue);
>> +}
>
>
> Is it safe to call QSIMPLEQ_INIT() on an already-initialized but empty
> queue?

#define QSIMPLEQ_INIT(head) do {\
(head)->sqh_first = NULL;   \
(head)->sqh_last = &(head)->sqh_first;  \
} while (/*CONSTCOND*/0)


It looks safe to me. There is no allocation in QSIMPLEQ macros, and
some of them do call QSIMPLEQ_INIT on a number of operations on
already initialized queues.

Note that I would rather use QSIMPLEQ_HEAD_INITIALIZER, but there is an error:


/home/elmarco/src/qemu/monitor.c: In function ‘monitor_qapi_event_queue’:
/home/elmarco/src/qemu/include/qemu/queue.h:247:13: error: initializer
element is not constant
 { NULL, &(head).sqh_first }

Tbh, I don't really understand the problem! :)

>
>
>> +
>> +ev = g_new(MonitorQapiEvent, 1);
>> +ev->qdict = qobject_ref(qdict);
>> +ev->event = event;
>> +QSIMPLEQ_INSERT_TAIL(_queue, ev, entry);
>> +if (reentered) {
>> +return;
>> +}
>> +
>> +reentered = true;
>> +
>> +while ((ev = QSIMPLEQ_FIRST(_queue)) != NULL) {
>> +QSIMPLEQ_REMOVE_HEAD(_queue, entry);
>> +monitor_qapi_event_queue_no_recurse(ev->event, ev->qdict);
>> +qobject_unref(ev->qdict);
>> +g_free(ev);
>> +}
>> +
>> +reentered = false;
>> +}
>> +
>>   /*
>>* This function runs evconf->rate ns after sending a throttled
>>* event.
>>
>
> --
> Eric Blake, Principal Software Engineer
> Red Hat, Inc.   +1-919-301-3266
> Virtualization:  qemu.org | libvirt.org
>



-- 
Marc-André Lureau

Re: [Qemu-devel] [virtio-dev] [PATCH v3 3/3] Add "Group Identifier" support to Red Hat PCI Express bridge.

[Michael S. Tsirkin]

On Tue, Jul 31, 2018 at 10:58:37AM -0500, Venu Busireddy wrote:
> On 2018-07-07 15:14:11 +0300, Marcel Apfelbaum wrote:
> > Hi Venu,
> > 
> > On 06/30/2018 01:19 AM, Venu Busireddy wrote:
> > > Add a new bridge device "pcie-downstream" with a
> > > Vendor ID of PCI_VENDOR_ID_REDHAT and a Device ID of
> > > PCI_DEVICE_ID_REDHAT_DOWNPORT_FAILOVER.
> > 
> > Can't we use the current pcie-pci-bridge device (see
> > hw/pci-bridge/pcie_pci_bridge.c)
> > by adding the new vendor capability to it so we will not need to support a
> > new bridge?
> 
> Sorry for the delayed response. I was away on vacation.
> 
> This question is probably better answered by Michael, but me let me try.
> Michael suggested adding a new device to avoid confusion with existing
> bridge devices.
> 
> Venu

It's similar to the hotseat trick - dev id is easier to match against.

> 
> > 
> > Thanks,
> > Marcel
> > 
> > >   Also, add the "Vendor-Specific"
> > > capability to the bridge to contain the "Group Identifier" that will be
> > > used to pair a virtio device with the passthrough device attached to
> > > that bridge.
> > > 
> > > This capability is added to the bridge iff the "failover-group-id"
> > > option is specified for the bridge.
> > > 
> > > Signed-off-by: Venu Busireddy 
> > > ---
> > >   default-configs/arm-softmmu.mak|   1 +
> > >   default-configs/i386-softmmu.mak   |   1 +
> > >   default-configs/x86_64-softmmu.mak |   1 +
> > >   hw/pci-bridge/Makefile.objs|   1 +
> > >   hw/pci-bridge/pcie_downstream.c| 220 +
> > >   hw/pci-bridge/pcie_downstream.h|  10 ++
> > >   include/hw/pci/pci.h   |   1 +
> > >   7 files changed, 235 insertions(+)
> > >   create mode 100644 hw/pci-bridge/pcie_downstream.c
> > >   create mode 100644 hw/pci-bridge/pcie_downstream.h
> > > 
> > > diff --git a/default-configs/arm-softmmu.mak 
> > > b/default-configs/arm-softmmu.mak
> > > index 834d45cfaf..b86c6fb122 100644
> > > --- a/default-configs/arm-softmmu.mak
> > > +++ b/default-configs/arm-softmmu.mak
> > > @@ -139,6 +139,7 @@ CONFIG_IMX_I2C=y
> > >   CONFIG_PCIE_PORT=y
> > >   CONFIG_XIO3130=y
> > >   CONFIG_IOH3420=y
> > > +CONFIG_PCIE_DOWNSTREAM=y
> > >   CONFIG_I82801B11=y
> > >   CONFIG_ACPI=y
> > >   CONFIG_SMBIOS=y
> > > diff --git a/default-configs/i386-softmmu.mak 
> > > b/default-configs/i386-softmmu.mak
> > > index 8c7d4a0fa0..a900c8f052 100644
> > > --- a/default-configs/i386-softmmu.mak
> > > +++ b/default-configs/i386-softmmu.mak
> > > @@ -56,6 +56,7 @@ CONFIG_ACPI_NVDIMM=y
> > >   CONFIG_PCIE_PORT=y
> > >   CONFIG_XIO3130=y
> > >   CONFIG_IOH3420=y
> > > +CONFIG_PCIE_DOWNSTREAM=y
> > >   CONFIG_I82801B11=y
> > >   CONFIG_SMBIOS=y
> > >   CONFIG_HYPERV_TESTDEV=$(CONFIG_KVM)
> > > diff --git a/default-configs/x86_64-softmmu.mak 
> > > b/default-configs/x86_64-softmmu.mak
> > > index 0390b4303c..481e4764be 100644
> > > --- a/default-configs/x86_64-softmmu.mak
> > > +++ b/default-configs/x86_64-softmmu.mak
> > > @@ -56,6 +56,7 @@ CONFIG_ACPI_NVDIMM=y
> > >   CONFIG_PCIE_PORT=y
> > >   CONFIG_XIO3130=y
> > >   CONFIG_IOH3420=y
> > > +CONFIG_PCIE_DOWNSTREAM=y
> > >   CONFIG_I82801B11=y
> > >   CONFIG_SMBIOS=y
> > >   CONFIG_HYPERV_TESTDEV=$(CONFIG_KVM)
> > > diff --git a/hw/pci-bridge/Makefile.objs b/hw/pci-bridge/Makefile.objs
> > > index 47065f87d9..5b42212edc 100644
> > > --- a/hw/pci-bridge/Makefile.objs
> > > +++ b/hw/pci-bridge/Makefile.objs
> > > @@ -3,6 +3,7 @@ common-obj-$(CONFIG_PCIE_PORT) += pcie_root_port.o 
> > > gen_pcie_root_port.o pcie_pci
> > >   common-obj-$(CONFIG_PXB) += pci_expander_bridge.o
> > >   common-obj-$(CONFIG_XIO3130) += xio3130_upstream.o xio3130_downstream.o
> > >   common-obj-$(CONFIG_IOH3420) += ioh3420.o
> > > +common-obj-$(CONFIG_PCIE_DOWNSTREAM) += pcie_downstream.o
> > >   common-obj-$(CONFIG_I82801B11) += i82801b11.o
> > >   # NewWorld PowerMac
> > >   common-obj-$(CONFIG_DEC_PCI) += dec.o
> > > diff --git a/hw/pci-bridge/pcie_downstream.c 
> > > b/hw/pci-bridge/pcie_downstream.c
> > > new file mode 100644
> > > index 00..49b0d1e933
> > > --- /dev/null
> > > +++ b/hw/pci-bridge/pcie_downstream.c
> > > @@ -0,0 +1,220 @@
> > > +/*
> > > + * Red Hat PCI Express downstream port.
> > > + *
> > > + * pcie_downstream.c
> > > + * Most of this code is copied from xio3130_downstream.c
> > > + *
> > > + * Copyright (c) 2018 Oracle and/or its affiliates.
> > > + * Author: Venu Busireddy 
> > > + *
> > > + * This program is free software; you can redistribute it and/or modify
> > > + * it under the terms of the GNU General Public License as published by
> > > + * the Free Software Foundation; either version 2 of the License, or
> > > + * (at your option) any later version.
> > > + *
> > > + * This program is distributed in the hope that it will be useful,
> > > + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> > > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> > > + * GNU General Public License 

Re: [Qemu-devel] [PATCH v2] monitor: temporary fix for dead-lock on event recursion

[Eric Blake]

On 07/31/2018 10:01 AM, Marc-André Lureau wrote:

With a Spice port chardev, it is possible to reenter
monitor_qapi_event_queue() (when the client disconnects for
example). This will dead-lock on monitor_lock.

Instead, use some TLS variables to check for recursion and queue the
events.





diff --git a/monitor.c b/monitor.c
index d8d8211ae4..4d9c1873bc 100644
--- a/monitor.c
+++ b/monitor.c
@@ -633,7 +633,7 @@ static void monitor_qapi_event_handler(void *opaque);
   * applying any rate limiting if required.
   */
  static void
-monitor_qapi_event_queue(QAPIEvent event, QDict *qdict, Error **errp)
+monitor_qapi_event_queue_no_recurse(QAPIEvent event, QDict *qdict)
  {
  MonitorQAPIEventConf *evconf;
  MonitorQAPIEventState *evstate;
@@ -688,6 +688,48 @@ monitor_qapi_event_queue(QAPIEvent event, QDict *qdict, 
Error **errp)
  qemu_mutex_unlock(_lock);
  }
  
+static void

+monitor_qapi_event_queue(QAPIEvent event, QDict *qdict, Error **errp)
+{
+/*
+ * monitor_qapi_event_queue_no_recurse() is not reentrant: it
+ * would deadlock on monitor_lock.  Work around by queueing
+ * events in thread-local storage.
+ * TODO: remove this, make it re-enter safe.
+ */
+static __thread bool reentered;
+typedef struct MonitorQapiEvent {
+QAPIEvent event;
+QDict *qdict;
+QSIMPLEQ_ENTRY(MonitorQapiEvent) entry;
+} MonitorQapiEvent;
+MonitorQapiEvent *ev;
+static __thread QSIMPLEQ_HEAD(, MonitorQapiEvent) event_queue;
+
+if (!reentered) {
+QSIMPLEQ_INIT(_queue);
+}


Is it safe to call QSIMPLEQ_INIT() on an already-initialized but empty 
queue?



+
+ev = g_new(MonitorQapiEvent, 1);
+ev->qdict = qobject_ref(qdict);
+ev->event = event;
+QSIMPLEQ_INSERT_TAIL(_queue, ev, entry);
+if (reentered) {
+return;
+}
+
+reentered = true;
+
+while ((ev = QSIMPLEQ_FIRST(_queue)) != NULL) {
+QSIMPLEQ_REMOVE_HEAD(_queue, entry);
+monitor_qapi_event_queue_no_recurse(ev->event, ev->qdict);
+qobject_unref(ev->qdict);
+g_free(ev);
+}
+
+reentered = false;
+}
+
  /*
   * This function runs evconf->rate ns after sending a throttled
   * event.



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

Re: [Qemu-devel] [virtio-dev] [PATCH v3 3/3] Add "Group Identifier" support to Red Hat PCI Express bridge.

[Venu Busireddy]

On 2018-07-07 15:14:11 +0300, Marcel Apfelbaum wrote:
> Hi Venu,
> 
> On 06/30/2018 01:19 AM, Venu Busireddy wrote:
> > Add a new bridge device "pcie-downstream" with a
> > Vendor ID of PCI_VENDOR_ID_REDHAT and a Device ID of
> > PCI_DEVICE_ID_REDHAT_DOWNPORT_FAILOVER.
> 
> Can't we use the current pcie-pci-bridge device (see
> hw/pci-bridge/pcie_pci_bridge.c)
> by adding the new vendor capability to it so we will not need to support a
> new bridge?

Sorry for the delayed response. I was away on vacation.

This question is probably better answered by Michael, but me let me try.
Michael suggested adding a new device to avoid confusion with existing
bridge devices.

Venu


> 
> Thanks,
> Marcel
> 
> >   Also, add the "Vendor-Specific"
> > capability to the bridge to contain the "Group Identifier" that will be
> > used to pair a virtio device with the passthrough device attached to
> > that bridge.
> > 
> > This capability is added to the bridge iff the "failover-group-id"
> > option is specified for the bridge.
> > 
> > Signed-off-by: Venu Busireddy 
> > ---
> >   default-configs/arm-softmmu.mak|   1 +
> >   default-configs/i386-softmmu.mak   |   1 +
> >   default-configs/x86_64-softmmu.mak |   1 +
> >   hw/pci-bridge/Makefile.objs|   1 +
> >   hw/pci-bridge/pcie_downstream.c| 220 +
> >   hw/pci-bridge/pcie_downstream.h|  10 ++
> >   include/hw/pci/pci.h   |   1 +
> >   7 files changed, 235 insertions(+)
> >   create mode 100644 hw/pci-bridge/pcie_downstream.c
> >   create mode 100644 hw/pci-bridge/pcie_downstream.h
> > 
> > diff --git a/default-configs/arm-softmmu.mak 
> > b/default-configs/arm-softmmu.mak
> > index 834d45cfaf..b86c6fb122 100644
> > --- a/default-configs/arm-softmmu.mak
> > +++ b/default-configs/arm-softmmu.mak
> > @@ -139,6 +139,7 @@ CONFIG_IMX_I2C=y
> >   CONFIG_PCIE_PORT=y
> >   CONFIG_XIO3130=y
> >   CONFIG_IOH3420=y
> > +CONFIG_PCIE_DOWNSTREAM=y
> >   CONFIG_I82801B11=y
> >   CONFIG_ACPI=y
> >   CONFIG_SMBIOS=y
> > diff --git a/default-configs/i386-softmmu.mak 
> > b/default-configs/i386-softmmu.mak
> > index 8c7d4a0fa0..a900c8f052 100644
> > --- a/default-configs/i386-softmmu.mak
> > +++ b/default-configs/i386-softmmu.mak
> > @@ -56,6 +56,7 @@ CONFIG_ACPI_NVDIMM=y
> >   CONFIG_PCIE_PORT=y
> >   CONFIG_XIO3130=y
> >   CONFIG_IOH3420=y
> > +CONFIG_PCIE_DOWNSTREAM=y
> >   CONFIG_I82801B11=y
> >   CONFIG_SMBIOS=y
> >   CONFIG_HYPERV_TESTDEV=$(CONFIG_KVM)
> > diff --git a/default-configs/x86_64-softmmu.mak 
> > b/default-configs/x86_64-softmmu.mak
> > index 0390b4303c..481e4764be 100644
> > --- a/default-configs/x86_64-softmmu.mak
> > +++ b/default-configs/x86_64-softmmu.mak
> > @@ -56,6 +56,7 @@ CONFIG_ACPI_NVDIMM=y
> >   CONFIG_PCIE_PORT=y
> >   CONFIG_XIO3130=y
> >   CONFIG_IOH3420=y
> > +CONFIG_PCIE_DOWNSTREAM=y
> >   CONFIG_I82801B11=y
> >   CONFIG_SMBIOS=y
> >   CONFIG_HYPERV_TESTDEV=$(CONFIG_KVM)
> > diff --git a/hw/pci-bridge/Makefile.objs b/hw/pci-bridge/Makefile.objs
> > index 47065f87d9..5b42212edc 100644
> > --- a/hw/pci-bridge/Makefile.objs
> > +++ b/hw/pci-bridge/Makefile.objs
> > @@ -3,6 +3,7 @@ common-obj-$(CONFIG_PCIE_PORT) += pcie_root_port.o 
> > gen_pcie_root_port.o pcie_pci
> >   common-obj-$(CONFIG_PXB) += pci_expander_bridge.o
> >   common-obj-$(CONFIG_XIO3130) += xio3130_upstream.o xio3130_downstream.o
> >   common-obj-$(CONFIG_IOH3420) += ioh3420.o
> > +common-obj-$(CONFIG_PCIE_DOWNSTREAM) += pcie_downstream.o
> >   common-obj-$(CONFIG_I82801B11) += i82801b11.o
> >   # NewWorld PowerMac
> >   common-obj-$(CONFIG_DEC_PCI) += dec.o
> > diff --git a/hw/pci-bridge/pcie_downstream.c 
> > b/hw/pci-bridge/pcie_downstream.c
> > new file mode 100644
> > index 00..49b0d1e933
> > --- /dev/null
> > +++ b/hw/pci-bridge/pcie_downstream.c
> > @@ -0,0 +1,220 @@
> > +/*
> > + * Red Hat PCI Express downstream port.
> > + *
> > + * pcie_downstream.c
> > + * Most of this code is copied from xio3130_downstream.c
> > + *
> > + * Copyright (c) 2018 Oracle and/or its affiliates.
> > + * Author: Venu Busireddy 
> > + *
> > + * This program is free software; you can redistribute it and/or modify
> > + * it under the terms of the GNU General Public License as published by
> > + * the Free Software Foundation; either version 2 of the License, or
> > + * (at your option) any later version.
> > + *
> > + * This program is distributed in the hope that it will be useful,
> > + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> > + * GNU General Public License for more details.
> > + *
> > + * You should have received a copy of the GNU General Public License along
> > + * with this program; if not, see .
> > + */
> > +
> > +#include "qemu/osdep.h"
> > +#include "hw/pci/pci_ids.h"
> > +#include "hw/pci/msi.h"
> > +#include "hw/pci/pcie.h"
> > +#include "pcie_downstream.h"
> > +#include "qapi/error.h"
> > +
> 

[Qemu-devel] [PULL 0/1] Monitor patches for 2018-07-31 (3.0.0-rc3)

[Markus Armbruster]

The following changes since commit 42e76456cf68dc828b8dbd3c7e255197e9b5e57d:

  Merge remote-tracking branch 
'remotes/vivier2/tags/linux-user-for-3.0-pull-request' into staging (2018-07-31 
13:52:03 +0100)

are available in the Git repository at:

  git://repo.or.cz/qemu/armbru.git tags/pull-monitor-2018-07-31

for you to fetch changes up to 9a1054061c62311f6efcd88e4df0aa5203c7b39a:

  monitor: temporary fix for dead-lock on event recursion (2018-07-31 17:42:57 
+0200)


Monitor patches for 2018-07-31 (3.0.0-rc3)


Marc-André Lureau (1):
  monitor: temporary fix for dead-lock on event recursion

 monitor.c | 44 +++-
 1 file changed, 43 insertions(+), 1 deletion(-)

-- 
2.17.1

[Qemu-devel] [PULL 1/1] monitor: temporary fix for dead-lock on event recursion

[Markus Armbruster]

From: Marc-André Lureau 

With a Spice port chardev, it is possible to reenter
monitor_qapi_event_queue() (when the client disconnects for
example). This will dead-lock on monitor_lock.

Instead, use some TLS variables to check for recursion and queue the
events.

Fixes:
 (gdb) bt
 #0  0x7fa69e7217fd in __lll_lock_wait () at /lib64/libpthread.so.0
 #1  0x7fa69e71acf4 in pthread_mutex_lock () at /lib64/libpthread.so.0
 #2  0x563303567619 in qemu_mutex_lock_impl (mutex=0x563303d3e220 
, file=0x5633036589a8 "/home/elmarco/src/qq/monitor.c", line=645) 
at /home/elmarco/src/qq/util/qemu-thread-posix.c:66
 #3  0x563302fa6c25 in monitor_qapi_event_queue 
(event=QAPI_EVENT_SPICE_DISCONNECTED, qdict=0x56330602bde0, 
errp=0x7ffc6ab5e728) at /home/elmarco/src/qq/monitor.c:645
 #4  0x563303549aca in qapi_event_send_spice_disconnected 
(server=0x563305afd630, client=0x563305745360, errp=0x563303d8d0f0 
) at qapi/qapi-events-ui.c:149
 #5  0x5633033e600f in channel_event (event=3, info=0x5633061b0050) at 
/home/elmarco/src/qq/ui/spice-core.c:235
 #6  0x7fa69f6c86bb in reds_handle_channel_event (reds=, 
event=3, info=0x5633061b0050) at reds.c:316
 #7  0x7fa69f6b193b in main_dispatcher_self_handle_channel_event 
(info=0x5633061b0050, event=3, self=0x563304e088c0) at main-dispatcher.c:197
 #8  0x7fa69f6b193b in main_dispatcher_channel_event (self=0x563304e088c0, 
event=event@entry=3, info=0x5633061b0050) at main-dispatcher.c:197
 #9  0x7fa69f6d0833 in red_stream_push_channel_event 
(s=s@entry=0x563305ad8f50, event=event@entry=3) at red-stream.c:414
 #10 0x7fa69f6d086b in red_stream_free (s=0x563305ad8f50) at 
red-stream.c:388
 #11 0x7fa69f6b7ddc in red_channel_client_finalize (object=0x563304df2360) 
at red-channel-client.c:347
 #12 0x7fa6a56b7fb9 in g_object_unref () at /lib64/libgobject-2.0.so.0
 #13 0x7fa69f6ba212 in red_channel_client_push (rcc=0x563304df2360) at 
red-channel-client.c:1341
 #14 0x7fa69f68b259 in red_char_device_send_msg_to_client 
(client=, msg=0x5633059b6310, dev=0x563304e08bc0) at 
char-device.c:305
 #15 0x7fa69f68b259 in red_char_device_send_msg_to_clients 
(msg=0x5633059b6310, dev=0x563304e08bc0) at char-device.c:305
 #16 0x7fa69f68b259 in red_char_device_read_from_device 
(dev=0x563304e08bc0) at char-device.c:353
 #17 0x56330317d01d in spice_chr_write (chr=0x563304cafe20, 
buf=0x563304cc50b0 "{\"timestamp\": {\"seconds\": 1532944763, \"microseconds\": 
326636}, \"event\": \"SHUTDOWN\", \"data\": {\"guest\": false}}\r\n", len=111) 
at /home/elmarco/src/qq/chardev/spice.c:199
 #18 0x5633034deee7 in qemu_chr_write_buffer (s=0x563304cafe20, 
buf=0x563304cc50b0 "{\"timestamp\": {\"seconds\": 1532944763, \"microseconds\": 
326636}, \"event\": \"SHUTDOWN\", \"data\": {\"guest\": false}}\r\n", len=111, 
offset=0x7ffc6ab5ea70, write_all=false) at 
/home/elmarco/src/qq/chardev/char.c:112
 #19 0x5633034df054 in qemu_chr_write (s=0x563304cafe20, buf=0x563304cc50b0 
"{\"timestamp\": {\"seconds\": 1532944763, \"microseconds\": 326636}, 
\"event\": \"SHUTDOWN\", \"data\": {\"guest\": false}}\r\n", len=111, 
write_all=false) at /home/elmarco/src/qq/chardev/char.c:147
 #20 0x5633034e1e13 in qemu_chr_fe_write (be=0x563304dbb800, 
buf=0x563304cc50b0 "{\"timestamp\": {\"seconds\": 1532944763, \"microseconds\": 
326636}, \"event\": \"SHUTDOWN\", \"data\": {\"guest\": false}}\r\n", len=111) 
at /home/elmarco/src/qq/chardev/char-fe.c:42
 #21 0x563302fa6334 in monitor_flush_locked (mon=0x563304dbb800) at 
/home/elmarco/src/qq/monitor.c:425
 #22 0x563302fa6520 in monitor_puts (mon=0x563304dbb800, str=0x563305de7e9e 
"") at /home/elmarco/src/qq/monitor.c:468
 #23 0x563302fa680c in qmp_send_response (mon=0x563304dbb800, 
rsp=0x563304df5730) at /home/elmarco/src/qq/monitor.c:517
 #24 0x563302fa6905 in qmp_queue_response (mon=0x563304dbb800, 
rsp=0x563304df5730) at /home/elmarco/src/qq/monitor.c:538
 #25 0x563302fa6b5b in monitor_qapi_event_emit (event=QAPI_EVENT_SHUTDOWN, 
qdict=0x563304df5730) at /home/elmarco/src/qq/monitor.c:624
 #26 0x563302fa6c4b in monitor_qapi_event_queue (event=QAPI_EVENT_SHUTDOWN, 
qdict=0x563304df5730, errp=0x7ffc6ab5ed00) at /home/elmarco/src/qq/monitor.c:649
 #27 0x563303548cce in qapi_event_send_shutdown (guest=false, 
errp=0x563303d8d0f0 ) at qapi/qapi-events-run-state.c:58
 #28 0x56330313bcd7 in main_loop_should_exit () at 
/home/elmarco/src/qq/vl.c:1822
 #29 0x56330313bde3 in main_loop () at /home/elmarco/src/qq/vl.c:1862
 #30 0x563303143781 in main (argc=3, argv=0x7ffc6ab5f068, 
envp=0x7ffc6ab5f088) at /home/elmarco/src/qq/vl.c:4644

Note that error report is now moved to the first caller, which may
receive an error for a recursed event. This is probably fine (95% of
callers use _abort, the rest have NULL error and ignore it)

Signed-off-by: Marc-André Lureau 
Message-Id: <20180731150144.14022-1-marcandre.lur...@redhat.com>
Reviewed-by: Markus Armbruster 
[*_no_recurse 

Re: [Qemu-devel] [PATCH 2/3] migration: Add qmp command for migrate_set_max_cpu_throttle

[Daniel P . Berrangé]

On Tue, Jul 31, 2018 at 04:48:35PM +0200, Juan Quintela wrote:
> "Dr. David Alan Gilbert"  wrote:
> > * Li Qiang (liq...@gmail.com) wrote:
> >> The default max cpu throttle is 99, this is too big that may
> >> influence the guest loads. Add a qmp to config it can make it
> >> more flexible.
> >> 
> >> Signed-off-by: Li Qiang 
> >
> > This should be done as a migration parameter rather than a new command.
> > For example, follow the cpu-throttle-increment parameter; and this
> > should work just like it.
> 
> I was about to comment this one.
> 
> migrate_set_downtime, migrate_set_speed, migrate-set-cache-size,
> query-migrate-cache-size are marked as deprecated.  Any way that we
> could have make more clear that one should use
> migrate_set/get_parameter/capability?

Where are they marked as deprecated ?

They're not included in our official list of deprecated features, so per
our deprecation policy, they are still considered supported.

  https://qemu.weilnetz.de/doc/qemu-doc.html#Deprecated-features

To deprecate something it needs to be added to qemu-deprecated.texi, and
ideally also made to print a message to stderr when triggered.

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

Re: [Qemu-devel] [PATCH 0/7 V11] nvdimm: guarantee persistence of QEMU writes to persistent memory

[Stefan Hajnoczi]

On Fri, Jul 27, 2018 at 04:06:23PM +0300, Michael S. Tsirkin wrote:
> On Fri, Jul 27, 2018 at 01:49:17PM +0100, Stefan Hajnoczi wrote:
> > On Wed, Jul 18, 2018 at 03:47:56PM +0800, junyan...@gmx.com wrote:
> > > From: Junyan He 
> > > 
> > > QEMU writes to vNVDIMM backends in the vNVDIMM label emulation and live 
> > > migration.
> > > If the backend is on the persistent memory, QEMU needs to take proper 
> > > operations to
> > > ensure its writes persistent on the persistent memory. Otherwise, a host 
> > > power failure
> > > may result in the loss the guest data on the persistent memory.
> > 
> > Ping Michael.  Can this go through your tree?
> 
> OK but do you really think it's appropriate in this release cycle?
> If not pls repost after the release including the acks.

I agree, it's too late for QEMU 3.0.

Have you considered keeping a -next tree?  It's more
contributor-friendly if you merge patches into a -next tree so they
don't need to worry about it anymore.

Stefan


signature.asc
Description: PGP signature

Re: [Qemu-devel] [PATCH] monitor: accept input on resume

[Markus Armbruster]

Marc-André Lureau  writes:

> Hi
>
> On Tue, Jul 31, 2018 at 1:30 PM, Markus Armbruster  wrote:
>> Marc-André Lureau  writes:
>>
>>> A chardev may stop trying to write if the associated can_read()
>>> callback returned 0. This happens when the monitor is suspended.
>>> The frontend is supposed to call qemu_chr_fe_accept_input() when it is
>>> ready to accept data again.
>>>
>>> Signed-off-by: Marc-André Lureau 
>>
>> Impact?
>
> I have observed the issue with a spice port, all pending commands
> aren't flushed. Most chardev don't use the accept_input() callback,
> and instead poll regularly, like char-socket.c:tcp_chr_read_poll()

The ones that do use it are braille, mux, msmouse, spice (abstract),
spicevmc, spiceport, wctablet.

A description of the impact should be worked into the commit message.

>> Is this to be considered for 3.0?
>
> This is not a regression, afaik, and doesn't impact common monitor
> chardev. So it could be delayed. But it's also a small fix for Spice
> chardev that shouldn't create regression, so it should be fine for
> 3.0.

If the callbacks we now call all work fine, we should be good.  But
that's a chardev question, and I'm the guy for the monitor questions.  I
think the decision needs to be made by maintainers of the chardev
subsystem.

Preferably with an improved commit message:
Acked-by: Markus Armbruster 

Re: [Qemu-devel] [PATCH 2/3] migration: Add qmp command for migrate_set_max_cpu_throttle

[Dr. David Alan Gilbert]

* Juan Quintela (quint...@redhat.com) wrote:
> "Dr. David Alan Gilbert"  wrote:
> > * Li Qiang (liq...@gmail.com) wrote:
> >> The default max cpu throttle is 99, this is too big that may
> >> influence the guest loads. Add a qmp to config it can make it
> >> more flexible.
> >> 
> >> Signed-off-by: Li Qiang 
> >
> > This should be done as a migration parameter rather than a new command.
> > For example, follow the cpu-throttle-increment parameter; and this
> > should work just like it.
> 
> I was about to comment this one.
> 
> migrate_set_downtime, migrate_set_speed, migrate-set-cache-size,
> query-migrate-cache-size are marked as deprecated.  Any way that we
> could have make more clear that one should use
> migrate_set/get_parameter/capability?

A big hairy comment?

Dave

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

Re: [Qemu-devel] [PATCH v2] monitor: temporary fix for dead-lock on event recursion

[Marc-André Lureau]

On Tue, Jul 31, 2018 at 5:25 PM, Markus Armbruster  wrote:
> Marc-André Lureau  writes:
>
>> With a Spice port chardev, it is possible to reenter
>> monitor_qapi_event_queue() (when the client disconnects for
>> example). This will dead-lock on monitor_lock.
>>
>> Instead, use some TLS variables to check for recursion and queue the
>> events.
>>
>> Fixes:
>>  (gdb) bt
>>  #0  0x7fa69e7217fd in __lll_lock_wait () at /lib64/libpthread.so.0
>>  #1  0x7fa69e71acf4 in pthread_mutex_lock () at /lib64/libpthread.so.0
>>  #2  0x563303567619 in qemu_mutex_lock_impl (mutex=0x563303d3e220 
>> , file=0x5633036589a8 "/home/elmarco/src/qq/monitor.c", 
>> line=645) at /home/elmarco/src/qq/util/qemu-thread-posix.c:66
>>  #3  0x563302fa6c25 in monitor_qapi_event_queue 
>> (event=QAPI_EVENT_SPICE_DISCONNECTED, qdict=0x56330602bde0, 
>> errp=0x7ffc6ab5e728) at /home/elmarco/src/qq/monitor.c:645
>>  #4  0x563303549aca in qapi_event_send_spice_disconnected 
>> (server=0x563305afd630, client=0x563305745360, errp=0x563303d8d0f0 
>> ) at qapi/qapi-events-ui.c:149
>>  #5  0x5633033e600f in channel_event (event=3, info=0x5633061b0050) at 
>> /home/elmarco/src/qq/ui/spice-core.c:235
>>  #6  0x7fa69f6c86bb in reds_handle_channel_event (reds=, 
>> event=3, info=0x5633061b0050) at reds.c:316
>>  #7  0x7fa69f6b193b in main_dispatcher_self_handle_channel_event 
>> (info=0x5633061b0050, event=3, self=0x563304e088c0) at main-dispatcher.c:197
>>  #8  0x7fa69f6b193b in main_dispatcher_channel_event 
>> (self=0x563304e088c0, event=event@entry=3, info=0x5633061b0050) at 
>> main-dispatcher.c:197
>>  #9  0x7fa69f6d0833 in red_stream_push_channel_event 
>> (s=s@entry=0x563305ad8f50, event=event@entry=3) at red-stream.c:414
>>  #10 0x7fa69f6d086b in red_stream_free (s=0x563305ad8f50) at 
>> red-stream.c:388
>>  #11 0x7fa69f6b7ddc in red_channel_client_finalize 
>> (object=0x563304df2360) at red-channel-client.c:347
>>  #12 0x7fa6a56b7fb9 in g_object_unref () at /lib64/libgobject-2.0.so.0
>>  #13 0x7fa69f6ba212 in red_channel_client_push (rcc=0x563304df2360) at 
>> red-channel-client.c:1341
>>  #14 0x7fa69f68b259 in red_char_device_send_msg_to_client 
>> (client=, msg=0x5633059b6310, dev=0x563304e08bc0) at 
>> char-device.c:305
>>  #15 0x7fa69f68b259 in red_char_device_send_msg_to_clients 
>> (msg=0x5633059b6310, dev=0x563304e08bc0) at char-device.c:305
>>  #16 0x7fa69f68b259 in red_char_device_read_from_device 
>> (dev=0x563304e08bc0) at char-device.c:353
>>  #17 0x56330317d01d in spice_chr_write (chr=0x563304cafe20, 
>> buf=0x563304cc50b0 "{\"timestamp\": {\"seconds\": 1532944763, 
>> \"microseconds\": 326636}, \"event\": \"SHUTDOWN\", \"data\": {\"guest\": 
>> false}}\r\n", len=111) at /home/elmarco/src/qq/chardev/spice.c:199
>>  #18 0x5633034deee7 in qemu_chr_write_buffer (s=0x563304cafe20, 
>> buf=0x563304cc50b0 "{\"timestamp\": {\"seconds\": 1532944763, 
>> \"microseconds\": 326636}, \"event\": \"SHUTDOWN\", \"data\": {\"guest\": 
>> false}}\r\n", len=111, offset=0x7ffc6ab5ea70, write_all=false) at 
>> /home/elmarco/src/qq/chardev/char.c:112
>>  #19 0x5633034df054 in qemu_chr_write (s=0x563304cafe20, 
>> buf=0x563304cc50b0 "{\"timestamp\": {\"seconds\": 1532944763, 
>> \"microseconds\": 326636}, \"event\": \"SHUTDOWN\", \"data\": {\"guest\": 
>> false}}\r\n", len=111, write_all=false) at 
>> /home/elmarco/src/qq/chardev/char.c:147
>>  #20 0x5633034e1e13 in qemu_chr_fe_write (be=0x563304dbb800, 
>> buf=0x563304cc50b0 "{\"timestamp\": {\"seconds\": 1532944763, 
>> \"microseconds\": 326636}, \"event\": \"SHUTDOWN\", \"data\": {\"guest\": 
>> false}}\r\n", len=111) at /home/elmarco/src/qq/chardev/char-fe.c:42
>>  #21 0x563302fa6334 in monitor_flush_locked (mon=0x563304dbb800) at 
>> /home/elmarco/src/qq/monitor.c:425
>>  #22 0x563302fa6520 in monitor_puts (mon=0x563304dbb800, 
>> str=0x563305de7e9e "") at /home/elmarco/src/qq/monitor.c:468
>>  #23 0x563302fa680c in qmp_send_response (mon=0x563304dbb800, 
>> rsp=0x563304df5730) at /home/elmarco/src/qq/monitor.c:517
>>  #24 0x563302fa6905 in qmp_queue_response (mon=0x563304dbb800, 
>> rsp=0x563304df5730) at /home/elmarco/src/qq/monitor.c:538
>>  #25 0x563302fa6b5b in monitor_qapi_event_emit 
>> (event=QAPI_EVENT_SHUTDOWN, qdict=0x563304df5730) at 
>> /home/elmarco/src/qq/monitor.c:624
>>  #26 0x563302fa6c4b in monitor_qapi_event_queue 
>> (event=QAPI_EVENT_SHUTDOWN, qdict=0x563304df5730, errp=0x7ffc6ab5ed00) at 
>> /home/elmarco/src/qq/monitor.c:649
>>  #27 0x563303548cce in qapi_event_send_shutdown (guest=false, 
>> errp=0x563303d8d0f0 ) at qapi/qapi-events-run-state.c:58
>>  #28 0x56330313bcd7 in main_loop_should_exit () at 
>> /home/elmarco/src/qq/vl.c:1822
>>  #29 0x56330313bde3 in main_loop () at /home/elmarco/src/qq/vl.c:1862
>>  #30 0x563303143781 in main (argc=3, argv=0x7ffc6ab5f068, 
>> envp=0x7ffc6ab5f088) at /home/elmarco/src/qq/vl.c:4644
>>
>> Note that error 

Re: [Qemu-devel] [PATCH v2] monitor: temporary fix for dead-lock on event recursion

[Markus Armbruster]

Marc-André Lureau  writes:

> With a Spice port chardev, it is possible to reenter
> monitor_qapi_event_queue() (when the client disconnects for
> example). This will dead-lock on monitor_lock.
>
> Instead, use some TLS variables to check for recursion and queue the
> events.
>
> Fixes:
>  (gdb) bt
>  #0  0x7fa69e7217fd in __lll_lock_wait () at /lib64/libpthread.so.0
>  #1  0x7fa69e71acf4 in pthread_mutex_lock () at /lib64/libpthread.so.0
>  #2  0x563303567619 in qemu_mutex_lock_impl (mutex=0x563303d3e220 
> , file=0x5633036589a8 "/home/elmarco/src/qq/monitor.c", 
> line=645) at /home/elmarco/src/qq/util/qemu-thread-posix.c:66
>  #3  0x563302fa6c25 in monitor_qapi_event_queue 
> (event=QAPI_EVENT_SPICE_DISCONNECTED, qdict=0x56330602bde0, 
> errp=0x7ffc6ab5e728) at /home/elmarco/src/qq/monitor.c:645
>  #4  0x563303549aca in qapi_event_send_spice_disconnected 
> (server=0x563305afd630, client=0x563305745360, errp=0x563303d8d0f0 
> ) at qapi/qapi-events-ui.c:149
>  #5  0x5633033e600f in channel_event (event=3, info=0x5633061b0050) at 
> /home/elmarco/src/qq/ui/spice-core.c:235
>  #6  0x7fa69f6c86bb in reds_handle_channel_event (reds=, 
> event=3, info=0x5633061b0050) at reds.c:316
>  #7  0x7fa69f6b193b in main_dispatcher_self_handle_channel_event 
> (info=0x5633061b0050, event=3, self=0x563304e088c0) at main-dispatcher.c:197
>  #8  0x7fa69f6b193b in main_dispatcher_channel_event 
> (self=0x563304e088c0, event=event@entry=3, info=0x5633061b0050) at 
> main-dispatcher.c:197
>  #9  0x7fa69f6d0833 in red_stream_push_channel_event 
> (s=s@entry=0x563305ad8f50, event=event@entry=3) at red-stream.c:414
>  #10 0x7fa69f6d086b in red_stream_free (s=0x563305ad8f50) at 
> red-stream.c:388
>  #11 0x7fa69f6b7ddc in red_channel_client_finalize 
> (object=0x563304df2360) at red-channel-client.c:347
>  #12 0x7fa6a56b7fb9 in g_object_unref () at /lib64/libgobject-2.0.so.0
>  #13 0x7fa69f6ba212 in red_channel_client_push (rcc=0x563304df2360) at 
> red-channel-client.c:1341
>  #14 0x7fa69f68b259 in red_char_device_send_msg_to_client 
> (client=, msg=0x5633059b6310, dev=0x563304e08bc0) at 
> char-device.c:305
>  #15 0x7fa69f68b259 in red_char_device_send_msg_to_clients 
> (msg=0x5633059b6310, dev=0x563304e08bc0) at char-device.c:305
>  #16 0x7fa69f68b259 in red_char_device_read_from_device 
> (dev=0x563304e08bc0) at char-device.c:353
>  #17 0x56330317d01d in spice_chr_write (chr=0x563304cafe20, 
> buf=0x563304cc50b0 "{\"timestamp\": {\"seconds\": 1532944763, 
> \"microseconds\": 326636}, \"event\": \"SHUTDOWN\", \"data\": {\"guest\": 
> false}}\r\n", len=111) at /home/elmarco/src/qq/chardev/spice.c:199
>  #18 0x5633034deee7 in qemu_chr_write_buffer (s=0x563304cafe20, 
> buf=0x563304cc50b0 "{\"timestamp\": {\"seconds\": 1532944763, 
> \"microseconds\": 326636}, \"event\": \"SHUTDOWN\", \"data\": {\"guest\": 
> false}}\r\n", len=111, offset=0x7ffc6ab5ea70, write_all=false) at 
> /home/elmarco/src/qq/chardev/char.c:112
>  #19 0x5633034df054 in qemu_chr_write (s=0x563304cafe20, 
> buf=0x563304cc50b0 "{\"timestamp\": {\"seconds\": 1532944763, 
> \"microseconds\": 326636}, \"event\": \"SHUTDOWN\", \"data\": {\"guest\": 
> false}}\r\n", len=111, write_all=false) at 
> /home/elmarco/src/qq/chardev/char.c:147
>  #20 0x5633034e1e13 in qemu_chr_fe_write (be=0x563304dbb800, 
> buf=0x563304cc50b0 "{\"timestamp\": {\"seconds\": 1532944763, 
> \"microseconds\": 326636}, \"event\": \"SHUTDOWN\", \"data\": {\"guest\": 
> false}}\r\n", len=111) at /home/elmarco/src/qq/chardev/char-fe.c:42
>  #21 0x563302fa6334 in monitor_flush_locked (mon=0x563304dbb800) at 
> /home/elmarco/src/qq/monitor.c:425
>  #22 0x563302fa6520 in monitor_puts (mon=0x563304dbb800, 
> str=0x563305de7e9e "") at /home/elmarco/src/qq/monitor.c:468
>  #23 0x563302fa680c in qmp_send_response (mon=0x563304dbb800, 
> rsp=0x563304df5730) at /home/elmarco/src/qq/monitor.c:517
>  #24 0x563302fa6905 in qmp_queue_response (mon=0x563304dbb800, 
> rsp=0x563304df5730) at /home/elmarco/src/qq/monitor.c:538
>  #25 0x563302fa6b5b in monitor_qapi_event_emit 
> (event=QAPI_EVENT_SHUTDOWN, qdict=0x563304df5730) at 
> /home/elmarco/src/qq/monitor.c:624
>  #26 0x563302fa6c4b in monitor_qapi_event_queue 
> (event=QAPI_EVENT_SHUTDOWN, qdict=0x563304df5730, errp=0x7ffc6ab5ed00) at 
> /home/elmarco/src/qq/monitor.c:649
>  #27 0x563303548cce in qapi_event_send_shutdown (guest=false, 
> errp=0x563303d8d0f0 ) at qapi/qapi-events-run-state.c:58
>  #28 0x56330313bcd7 in main_loop_should_exit () at 
> /home/elmarco/src/qq/vl.c:1822
>  #29 0x56330313bde3 in main_loop () at /home/elmarco/src/qq/vl.c:1862
>  #30 0x563303143781 in main (argc=3, argv=0x7ffc6ab5f068, 
> envp=0x7ffc6ab5f088) at /home/elmarco/src/qq/vl.c:4644
>
> Note that error report is now moved to the first caller, which may
> receive an error for a recursed event. This is probably fine (95% of
> callers use _abort, the 

Re: [Qemu-devel] [PATCH RFC] monitor: temporary fix for dead-lock on event recursion

[Markus Armbruster]

Marc-André Lureau  writes:

> Hi
>
> On Tue, Jul 31, 2018 at 9:05 AM, Markus Armbruster  wrote:
>> Marc-André Lureau  writes:
>>
>>> With a Spice port chardev, it is possible to reenter
>>> monitor_qapi_event_queue() (when the client disconnects for
>>> example). This will dead-lock on monitor_lock.
>>>
>>> Instead, use some TLS variables to check for recursion and queue the
>>> events.
>> [...]
>>>
>>> Note that error report is now moved to the first caller, which may
>>> receive an error for a recursed event. This is probably fine (95% of
>>> callers use _abort, the rest have NULL error and ignore it)
>>
>> I'm not 100% sure I understand this paragraph, but it might be moot; see
>> [*] below.
>>
>>>
>>> Signed-off-by: Marc-André Lureau 
>>> ---
>>>  monitor.c | 51 ++-
>>>  1 file changed, 50 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/monitor.c b/monitor.c
>>> index d8d8211ae4..d580c5a79c 100644
>>> --- a/monitor.c
>>> +++ b/monitor.c
>>> @@ -633,7 +633,7 @@ static void monitor_qapi_event_handler(void *opaque);
>>>   * applying any rate limiting if required.
>>>   */
>>>  static void
>>> -monitor_qapi_event_queue(QAPIEvent event, QDict *qdict, Error **errp)
>>> +monitor_qapi_event_queue_no_recurse(QAPIEvent event, QDict *qdict, Error 
>>> **errp)
>>>  {
>>>  MonitorQAPIEventConf *evconf;
>>>  MonitorQAPIEventState *evstate;
>>> @@ -688,6 +688,55 @@ monitor_qapi_event_queue(QAPIEvent event, QDict 
>>> *qdict, Error **errp)
>>>  qemu_mutex_unlock(_lock);
>>>  }
>>>
>>> +static void
>>> +monitor_qapi_event_queue(QAPIEvent event, QDict *qdict, Error **errp)
>>> +{
>>> +Error *local_err = NULL;
>>> +/*
>>> + * If the function recurse, monitor_lock will dead-lock.
>>> + * Instead, queue pending events in TLS.
>>
>> recurses
>>
>> The claim is correct before the patch.  But the comment needs to explain
>> current code.  Perhaps:
>>
>> * monitor_qapi_event_queue_no_recurse() is not reentrant: it
>> * would deadlock on monitor_lock.  Work around by queueing
>> * events in thread-local storage.
>
> ok
>
>>
>>> + * TODO: remove this, make it re-enter safe.
>>> + */
>>> +static __thread bool recurse;
>>> +typedef struct MonitorQapiEvent {
>>> +QAPIEvent event;
>>> +QDict *qdict;
>>> +QSIMPLEQ_ENTRY(MonitorQapiEvent) entry;
>>> +} MonitorQapiEvent;
>>> +MonitorQapiEvent *ev;
>>> +static __thread QSIMPLEQ_HEAD(, MonitorQapiEvent) event_queue;
>>
>> Let's put the static variables first.
>
> ok
>
>>> +
>>> +if (!recurse) {
>>> +QSIMPLEQ_INIT(_queue);
>>> +}
>>> +
>>> +ev = g_new(MonitorQapiEvent, 1);
>>> +ev->qdict = qobject_ref(qdict);
>>> +ev->event = event;
>>> +QSIMPLEQ_INSERT_TAIL(_queue, ev, entry);
>>> +if (recurse) {
>>> +return;
>>> +}
>>> +
>>> +recurse = true;
>>> +
>>> +while ((ev = QSIMPLEQ_FIRST(_queue)) != NULL) {
>>> +QSIMPLEQ_REMOVE_HEAD(_queue, entry);
>>
>> Could we use QSIMPLEQ_FOREACH_SAFE()?
>
> I don't think so, the next variable could be NULL, while a recursive
> call could be adding an element.
>
>>
>>> +if (!local_err) {
>>> +monitor_qapi_event_queue_no_recurse(ev->event, ev->qdict,
>>> +_err);
>>> +}
>>
>> [*] This looks scary: we silently throw away events after event queuing
>> fails.
>>
>> Fortunately, monitor_qapi_event_queue_no_recurse() cannot fail.  It
>> takes an Error ** parameter only so you can put it into @qmp_emit.
>>
>
> right
>
>> Aside: I wish we'd get rid of the indirection through @qmp_emit.
>>
>
> for later: You would hard code a function name instead?

Yes.

@qmp_emit is always monitor_qapi_event_queue in qemu-system-FOO, and
event_test_emit in test-qmp-event.  That's a linker job, not an indirect
call job.

>> Let's pass _abort and drop @local_err.
>
> actually, we can just ignore errp, right?

Not sure what you mean, but you've since sent v2, so let me figure it
out there :)

> for later: What about dropping error from the emit function?

Fine with me.  Easy enough to bring back if we find a need.

>
>>
>>> +qobject_unref(ev->qdict);
>>> +g_free(ev);
>>> +}
>>> +
>>> +recurse = false;
>>
>> Aha: @recurse means we've reentered the function.
>>
>> "Recurse" is imperative mood.  Misleading, as it's not an order to
>> recurse.
>>
>> Rename to @recursed?  @reentered?
>>
>
> ok, let's rename it "reentered"
>
>>> +
>>> +if (local_err) {
>>> +error_propagate(errp, local_err);
>>> +}
>>> +}
>>> +
>>>  /*
>>>   * This function runs evconf->rate ns after sending a throttled
>>>   * event.
>>
>> monitor_lock clearly needs a rethink.  Your TODO comment suggests you
>> agree.  But that's something for 3.1.
>>
>> I hate messing with locks at -rc3, but I also hate shipping known
>> deadlocks.  Your patch isn't pretty, but probably as simple as we can
>> 

Re: [Qemu-devel] [PATCH v2 0/4] Balloon inhibit enhancements, vfio restriction

[Dr. David Alan Gilbert]

* Alex Williamson (alex.william...@redhat.com) wrote:
> On Tue, 31 Jul 2018 15:29:17 +0300
> "Michael S. Tsirkin"  wrote:
> 
> > On Mon, Jul 30, 2018 at 05:13:26PM -0600, Alex Williamson wrote:
> > > v2:
> > >  - Use atomic ops for balloon inhibit counter (Peter)
> > >  - Allow endpoint driver opt-in for ballooning, vfio-ccw opt-in by
> > >default, vfio-pci opt-in by device option, only allowed for mdev
> > >devices, no support added for platform as there are no platform
> > >mdev devices.
> > > 
> > > See patch 3/4 for detailed explanation why ballooning and device
> > > assignment typically don't mix.  If this eventually changes, flags
> > > on the iommu info struct or perhaps device info struct can inform
> > > us for automatic opt-in.  Thanks,
> > > 
> > > Alex  
> > 
> > So this patch seems to block ballooning when vfio is added.
> > But what if balloon is added and inflated first?
> 
> Good point.
>  
> > I'd suggest making qemu_balloon_inhibit fail in that case,
> > and then vfio realize will fail as well.
> 
> That might be the correct behavior for vfio, but I wonder about the
> existing postcopy use case.  Dave Gilbert, what do you think?  We might
> need a separate interface for callers that cannot tolerate existing
> ballooned pages.  Of course we'll also need another atomic counter to
> keep a tally of ballooned pages.  Thanks,

For postcopy, preinflation isn't a problem; our only issue is ballooning
during the postcopy phase itself.

Dave

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

Re: [Qemu-devel] [PATCH for-3.0 v2] pc: acpi: fix memory hotplug regression by reducing stub SRAT entry size

[Eduardo Habkost]

On Tue, Jul 31, 2018 at 11:53:40AM +0200, Igor Mammedov wrote:
> On Mon, 30 Jul 2018 17:26:24 -0300
> Eduardo Habkost  wrote:
> 
> > On Mon, Jul 30, 2018 at 11:41:41AM +0200, Igor Mammedov wrote:
> > > Commit 848a1cc1e (hw/acpi-build: build SRAT memory affinity structures 
> > > for DIMM devices)
> > > broke the first dimm hotplug in following cases:
> > > 
> > >  1: there is no coldplugged dimm in the last numa node
> > > but there is a coldplugged dimm in another node
> > > 
> > >   -m 4096,slots=4,maxmem=32G   \
> > >   -object memory-backend-ram,id=m0,size=2G \
> > >   -device pc-dimm,memdev=m0,node=0 \
> > >   -numa node,nodeid=0  \
> > >   -numa node,nodeid=1
> > > 
> > >  2: if order of dimms on CLI is:
> > >1st plugged dimm in node1
> > >2nd plugged dimm in node0
> > > 
> > >   -m 4096,slots=4,maxmem=32G   \
> > >   -object memory-backend-ram,size=2G,id=m0 \
> > >   -device pc-dimm,memdev=m0,node=1 \
> > >   -object memory-backend-ram,id=m1,size=2G \
> > >   -device pc-dimm,memdev=m1,node=0 \
> > >   -numa node,nodeid=0  \
> > >   -numa node,nodeid=1
> > > 
> > > (qemu) object_add memory-backend-ram,id=m2,size=1G
> > > (qemu) device_add pc-dimm,memdev=m2,node=0
> > > 
> > > the first DIMM hotplug to any node except the last one
> > > fails (Windows is unable to online it).
> > > 
> > > Length reduction of stub hotplug memory SRAT entry,
> > > fixes issue for some reason.
> > >   
> > 
> > I'm really bothered by the lack of automated testing for all
> > these NUMA/ACPI corner cases.
> > 
> > This looks like a good candidate for an avocado_qemu test case.
> > Can you show pseudo-code of how exactly the bug fix could be
> > verified, so we can try to write a test case?
> Sadly I do it manually every time I'm suspect a patch would
> affect the feature. On just has to check if a new memory device
> appeared in device manager and it is in working state (started successfully).
> One also need to run it against to test it against windows version
> that supports memory hot-add (DC ed.).
> 
> It's typically what RHEL QE does, and they just found
> a new case which wasn't on test list so proactive measures
> wouldn't work here in any case as we didn't know about
> this particular combination.
> 
> I'm not sure how it will work with upstream avocado though,
> windows testing implies tester would need access to MSDN
> subscription or multiple retail versions to test against.
> So with windows it becomes expensive and complicated
> hence I'd leave this job to QE which has resources and
> upstream would benefit from downstream when a bug is found
> (albeit it's a catch up game).

I don't mean functional testing of Windows guests.  I'm just
looking for a way we can ensure we won't reintroduce this
particular bug later.  We should be able to encode known
requirements of existing guest OSes in test code (especially the
undocumented requirements).

In other words, we need test code that will check if the entry
you are adding below is still present and contains the right
flags, so people won't remove it by mistake.


[...]
> > > @@ -2269,7 +2269,16 @@ static void build_srat_hotpluggable_memory(GArray 
> > > *table_data, uint64_t base,
> > >  numamem = acpi_data_push(table_data, sizeof *numamem);
> > >  
> > >  if (!info) {
> > > -build_srat_memory(numamem, cur, end - cur, default_node,
> > > +/*
> > > + * Entry is required for Windows to enable memory hotplug in 
> > > OS
> > > + * and for Linux to enable SWIOTLB when booted with less than
> > > + * 4G of RAM. Windows works better if the entry sets 
> > > proximity
> > > + * to the highest NUMA node in the machine at the end of the
> > > + * reserved space.
> > > + * Memory devices may override proximity set by this entry,
> > > + * providing _PXM method if necessary.
> > > + */
> > > +build_srat_memory(numamem, end - 1, 1, default_node,
> > >MEM_AFFINITY_HOTPLUGGABLE | 
> > > MEM_AFFINITY_ENABLED);
> > >  break;
> > >  }

-- 
Eduardo

[Qemu-devel] [PATCH v2] monitor: temporary fix for dead-lock on event recursion

[Marc-André Lureau]

With a Spice port chardev, it is possible to reenter
monitor_qapi_event_queue() (when the client disconnects for
example). This will dead-lock on monitor_lock.

Instead, use some TLS variables to check for recursion and queue the
events.

Fixes:
 (gdb) bt
 #0  0x7fa69e7217fd in __lll_lock_wait () at /lib64/libpthread.so.0
 #1  0x7fa69e71acf4 in pthread_mutex_lock () at /lib64/libpthread.so.0
 #2  0x563303567619 in qemu_mutex_lock_impl (mutex=0x563303d3e220 
, file=0x5633036589a8 "/home/elmarco/src/qq/monitor.c", line=645) 
at /home/elmarco/src/qq/util/qemu-thread-posix.c:66
 #3  0x563302fa6c25 in monitor_qapi_event_queue 
(event=QAPI_EVENT_SPICE_DISCONNECTED, qdict=0x56330602bde0, 
errp=0x7ffc6ab5e728) at /home/elmarco/src/qq/monitor.c:645
 #4  0x563303549aca in qapi_event_send_spice_disconnected 
(server=0x563305afd630, client=0x563305745360, errp=0x563303d8d0f0 
) at qapi/qapi-events-ui.c:149
 #5  0x5633033e600f in channel_event (event=3, info=0x5633061b0050) at 
/home/elmarco/src/qq/ui/spice-core.c:235
 #6  0x7fa69f6c86bb in reds_handle_channel_event (reds=, 
event=3, info=0x5633061b0050) at reds.c:316
 #7  0x7fa69f6b193b in main_dispatcher_self_handle_channel_event 
(info=0x5633061b0050, event=3, self=0x563304e088c0) at main-dispatcher.c:197
 #8  0x7fa69f6b193b in main_dispatcher_channel_event (self=0x563304e088c0, 
event=event@entry=3, info=0x5633061b0050) at main-dispatcher.c:197
 #9  0x7fa69f6d0833 in red_stream_push_channel_event 
(s=s@entry=0x563305ad8f50, event=event@entry=3) at red-stream.c:414
 #10 0x7fa69f6d086b in red_stream_free (s=0x563305ad8f50) at 
red-stream.c:388
 #11 0x7fa69f6b7ddc in red_channel_client_finalize (object=0x563304df2360) 
at red-channel-client.c:347
 #12 0x7fa6a56b7fb9 in g_object_unref () at /lib64/libgobject-2.0.so.0
 #13 0x7fa69f6ba212 in red_channel_client_push (rcc=0x563304df2360) at 
red-channel-client.c:1341
 #14 0x7fa69f68b259 in red_char_device_send_msg_to_client 
(client=, msg=0x5633059b6310, dev=0x563304e08bc0) at 
char-device.c:305
 #15 0x7fa69f68b259 in red_char_device_send_msg_to_clients 
(msg=0x5633059b6310, dev=0x563304e08bc0) at char-device.c:305
 #16 0x7fa69f68b259 in red_char_device_read_from_device 
(dev=0x563304e08bc0) at char-device.c:353
 #17 0x56330317d01d in spice_chr_write (chr=0x563304cafe20, 
buf=0x563304cc50b0 "{\"timestamp\": {\"seconds\": 1532944763, \"microseconds\": 
326636}, \"event\": \"SHUTDOWN\", \"data\": {\"guest\": false}}\r\n", len=111) 
at /home/elmarco/src/qq/chardev/spice.c:199
 #18 0x5633034deee7 in qemu_chr_write_buffer (s=0x563304cafe20, 
buf=0x563304cc50b0 "{\"timestamp\": {\"seconds\": 1532944763, \"microseconds\": 
326636}, \"event\": \"SHUTDOWN\", \"data\": {\"guest\": false}}\r\n", len=111, 
offset=0x7ffc6ab5ea70, write_all=false) at 
/home/elmarco/src/qq/chardev/char.c:112
 #19 0x5633034df054 in qemu_chr_write (s=0x563304cafe20, buf=0x563304cc50b0 
"{\"timestamp\": {\"seconds\": 1532944763, \"microseconds\": 326636}, 
\"event\": \"SHUTDOWN\", \"data\": {\"guest\": false}}\r\n", len=111, 
write_all=false) at /home/elmarco/src/qq/chardev/char.c:147
 #20 0x5633034e1e13 in qemu_chr_fe_write (be=0x563304dbb800, 
buf=0x563304cc50b0 "{\"timestamp\": {\"seconds\": 1532944763, \"microseconds\": 
326636}, \"event\": \"SHUTDOWN\", \"data\": {\"guest\": false}}\r\n", len=111) 
at /home/elmarco/src/qq/chardev/char-fe.c:42
 #21 0x563302fa6334 in monitor_flush_locked (mon=0x563304dbb800) at 
/home/elmarco/src/qq/monitor.c:425
 #22 0x563302fa6520 in monitor_puts (mon=0x563304dbb800, str=0x563305de7e9e 
"") at /home/elmarco/src/qq/monitor.c:468
 #23 0x563302fa680c in qmp_send_response (mon=0x563304dbb800, 
rsp=0x563304df5730) at /home/elmarco/src/qq/monitor.c:517
 #24 0x563302fa6905 in qmp_queue_response (mon=0x563304dbb800, 
rsp=0x563304df5730) at /home/elmarco/src/qq/monitor.c:538
 #25 0x563302fa6b5b in monitor_qapi_event_emit (event=QAPI_EVENT_SHUTDOWN, 
qdict=0x563304df5730) at /home/elmarco/src/qq/monitor.c:624
 #26 0x563302fa6c4b in monitor_qapi_event_queue (event=QAPI_EVENT_SHUTDOWN, 
qdict=0x563304df5730, errp=0x7ffc6ab5ed00) at /home/elmarco/src/qq/monitor.c:649
 #27 0x563303548cce in qapi_event_send_shutdown (guest=false, 
errp=0x563303d8d0f0 ) at qapi/qapi-events-run-state.c:58
 #28 0x56330313bcd7 in main_loop_should_exit () at 
/home/elmarco/src/qq/vl.c:1822
 #29 0x56330313bde3 in main_loop () at /home/elmarco/src/qq/vl.c:1862
 #30 0x563303143781 in main (argc=3, argv=0x7ffc6ab5f068, 
envp=0x7ffc6ab5f088) at /home/elmarco/src/qq/vl.c:4644

Note that error report is now moved to the first caller, which may
receive an error for a recursed event. This is probably fine (95% of
callers use _abort, the rest have NULL error and ignore it)

Signed-off-by: Marc-André Lureau 
---
 monitor.c | 44 +++-
 1 file changed, 43 insertions(+), 1 deletion(-)

diff --git a/monitor.c 

Re: [Qemu-devel] [PATCH 2/3] migration: Add qmp command for migrate_set_max_cpu_throttle

[Juan Quintela]

"Dr. David Alan Gilbert"  wrote:
> * Li Qiang (liq...@gmail.com) wrote:
>> The default max cpu throttle is 99, this is too big that may
>> influence the guest loads. Add a qmp to config it can make it
>> more flexible.
>> 
>> Signed-off-by: Li Qiang 
>
> This should be done as a migration parameter rather than a new command.
> For example, follow the cpu-throttle-increment parameter; and this
> should work just like it.

I was about to comment this one.

migrate_set_downtime, migrate_set_speed, migrate-set-cache-size,
query-migrate-cache-size are marked as deprecated.  Any way that we
could have make more clear that one should use
migrate_set/get_parameter/capability?

Thanks, Juan.

Re: [Qemu-devel] [PATCH RFC] monitor: temporary fix for dead-lock on event recursion

[Marc-André Lureau]

Hi

On Tue, Jul 31, 2018 at 9:05 AM, Markus Armbruster  wrote:
> Marc-André Lureau  writes:
>
>> With a Spice port chardev, it is possible to reenter
>> monitor_qapi_event_queue() (when the client disconnects for
>> example). This will dead-lock on monitor_lock.
>>
>> Instead, use some TLS variables to check for recursion and queue the
>> events.
> [...]
>>
>> Note that error report is now moved to the first caller, which may
>> receive an error for a recursed event. This is probably fine (95% of
>> callers use _abort, the rest have NULL error and ignore it)
>
> I'm not 100% sure I understand this paragraph, but it might be moot; see
> [*] below.
>
>>
>> Signed-off-by: Marc-André Lureau 
>> ---
>>  monitor.c | 51 ++-
>>  1 file changed, 50 insertions(+), 1 deletion(-)
>>
>> diff --git a/monitor.c b/monitor.c
>> index d8d8211ae4..d580c5a79c 100644
>> --- a/monitor.c
>> +++ b/monitor.c
>> @@ -633,7 +633,7 @@ static void monitor_qapi_event_handler(void *opaque);
>>   * applying any rate limiting if required.
>>   */
>>  static void
>> -monitor_qapi_event_queue(QAPIEvent event, QDict *qdict, Error **errp)
>> +monitor_qapi_event_queue_no_recurse(QAPIEvent event, QDict *qdict, Error 
>> **errp)
>>  {
>>  MonitorQAPIEventConf *evconf;
>>  MonitorQAPIEventState *evstate;
>> @@ -688,6 +688,55 @@ monitor_qapi_event_queue(QAPIEvent event, QDict *qdict, 
>> Error **errp)
>>  qemu_mutex_unlock(_lock);
>>  }
>>
>> +static void
>> +monitor_qapi_event_queue(QAPIEvent event, QDict *qdict, Error **errp)
>> +{
>> +Error *local_err = NULL;
>> +/*
>> + * If the function recurse, monitor_lock will dead-lock.
>> + * Instead, queue pending events in TLS.
>
> recurses
>
> The claim is correct before the patch.  But the comment needs to explain
> current code.  Perhaps:
>
> * monitor_qapi_event_queue_no_recurse() is not reentrant: it
> * would deadlock on monitor_lock.  Work around by queueing
> * events in thread-local storage.

ok

>
>> + * TODO: remove this, make it re-enter safe.
>> + */
>> +static __thread bool recurse;
>> +typedef struct MonitorQapiEvent {
>> +QAPIEvent event;
>> +QDict *qdict;
>> +QSIMPLEQ_ENTRY(MonitorQapiEvent) entry;
>> +} MonitorQapiEvent;
>> +MonitorQapiEvent *ev;
>> +static __thread QSIMPLEQ_HEAD(, MonitorQapiEvent) event_queue;
>
> Let's put the static variables first.

ok

>> +
>> +if (!recurse) {
>> +QSIMPLEQ_INIT(_queue);
>> +}
>> +
>> +ev = g_new(MonitorQapiEvent, 1);
>> +ev->qdict = qobject_ref(qdict);
>> +ev->event = event;
>> +QSIMPLEQ_INSERT_TAIL(_queue, ev, entry);
>> +if (recurse) {
>> +return;
>> +}
>> +
>> +recurse = true;
>> +
>> +while ((ev = QSIMPLEQ_FIRST(_queue)) != NULL) {
>> +QSIMPLEQ_REMOVE_HEAD(_queue, entry);
>
> Could we use QSIMPLEQ_FOREACH_SAFE()?

I don't think so, the next variable could be NULL, while a recursive
call could be adding an element.

>
>> +if (!local_err) {
>> +monitor_qapi_event_queue_no_recurse(ev->event, ev->qdict,
>> +_err);
>> +}
>
> [*] This looks scary: we silently throw away events after event queuing
> fails.
>
> Fortunately, monitor_qapi_event_queue_no_recurse() cannot fail.  It
> takes an Error ** parameter only so you can put it into @qmp_emit.
>

right

> Aside: I wish we'd get rid of the indirection through @qmp_emit.
>

for later: You would hard code a function name instead?

> Let's pass _abort and drop @local_err.

actually, we can just ignore errp, right?

for later: What about dropping error from the emit function?

>
>> +qobject_unref(ev->qdict);
>> +g_free(ev);
>> +}
>> +
>> +recurse = false;
>
> Aha: @recurse means we've reentered the function.
>
> "Recurse" is imperative mood.  Misleading, as it's not an order to
> recurse.
>
> Rename to @recursed?  @reentered?
>

ok, let's rename it "reentered"

>> +
>> +if (local_err) {
>> +error_propagate(errp, local_err);
>> +}
>> +}
>> +
>>  /*
>>   * This function runs evconf->rate ns after sending a throttled
>>   * event.
>
> monitor_lock clearly needs a rethink.  Your TODO comment suggests you
> agree.  But that's something for 3.1.
>
> I hate messing with locks at -rc3, but I also hate shipping known
> deadlocks.  Your patch isn't pretty, but probably as simple as we can
> make it for 3.0.  A few more review eyeballs would be nice.
>

thanks, I'll send v2

-- 
Marc-André Lureau

Re: [Qemu-devel] [PATCH v2 0/4] Balloon inhibit enhancements, vfio restriction

[Alex Williamson]

On Tue, 31 Jul 2018 15:29:17 +0300
"Michael S. Tsirkin"  wrote:

> On Mon, Jul 30, 2018 at 05:13:26PM -0600, Alex Williamson wrote:
> > v2:
> >  - Use atomic ops for balloon inhibit counter (Peter)
> >  - Allow endpoint driver opt-in for ballooning, vfio-ccw opt-in by
> >default, vfio-pci opt-in by device option, only allowed for mdev
> >devices, no support added for platform as there are no platform
> >mdev devices.
> > 
> > See patch 3/4 for detailed explanation why ballooning and device
> > assignment typically don't mix.  If this eventually changes, flags
> > on the iommu info struct or perhaps device info struct can inform
> > us for automatic opt-in.  Thanks,
> > 
> > Alex  
> 
> So this patch seems to block ballooning when vfio is added.
> But what if balloon is added and inflated first?

Good point.
 
> I'd suggest making qemu_balloon_inhibit fail in that case,
> and then vfio realize will fail as well.

That might be the correct behavior for vfio, but I wonder about the
existing postcopy use case.  Dave Gilbert, what do you think?  We might
need a separate interface for callers that cannot tolerate existing
ballooned pages.  Of course we'll also need another atomic counter to
keep a tally of ballooned pages.  Thanks,

Alex

Re: [Qemu-devel] [PATCH v2] bitmap: fix BITMAP_LAST_WORD_MASK

[Juan Quintela]

Wei Wang  wrote:
> When "nbits = 0", which means no bits to mask, this macro is expected to
> return 0, instead of 0x. This patch changes the macro to return
> 0 when there is no bit needs to be masked.
>
> Signed-off-by: Wei Wang 
> CC: Juan Quintela 
> CC: Dr. David Alan Gilbert 
> CC: Peter Xu 

Reviewed-by: Juan Quintela 

Re: [Qemu-devel] [PATCH v9 1/3] migration: Create socket-address parameter

[Juan Quintela]

Eric Blake  wrote:

>> +++ b/qapi/migration.json
>> @@ -6,6 +6,7 @@
>>   ##
>> { 'include': 'common.json' }
>> +{ 'include': 'sockets.json' }
>> ##
>>   # @MigrationStats:
>> @@ -169,6 +170,7 @@
>>   #   only present when the postcopy-blocktime migration capability
>>   #   is enabled. (Since 2.13)
>>   #
>> +# @socket-address: Only used for tcp, to know what the real port is (Since 
>> 2.13)
>
> Maybe s/real port is/real ports are/, since...

done.

>
>>   #
>>   # Since: 0.14.0
>>   ##
>> @@ -183,7 +185,8 @@
>>  '*cpu-throttle-percentage': 'int',
>>  '*error-desc': 'str',
>>  '*postcopy-blocktime' : 'uint32',
>> -   '*postcopy-vcpu-blocktime': ['uint32']} }
>> +   '*postcopy-vcpu-blocktime': ['uint32'],
>> +   '*socket-address': ['SocketAddress'] } }
>
> ...an array is potentially plural.

done. (didn't like to put so many s's, but ...)

>> ##
>>   # @query-migrate:
>> @@ -690,6 +693,7 @@
>>   # needs to be a multiple of the target page size
>>   # and a power of 2
>>   # (Since 2.11)
>> +#
>>   # Since: 2.4
>>   ##
>>   { 'struct': 'MigrationParameters',
>
> Spurious hunk?  Although it looks reasonable, it could be a separate
> trivial cleanup patch.

Fixed thanks.

>
>> diff --git a/qapi/sockets.json b/qapi/sockets.json
>> index fc81d8d5e8..f1ca09a927 100644
>> --- a/qapi/sockets.json
>> +++ b/qapi/sockets.json
>> @@ -152,3 +152,16 @@
>>   'unix': 'UnixSocketAddress',
>>   'vsock': 'VsockSocketAddress',
>>   'fd': 'String' } }
>> +
>> +##
>> +# @DummyStruct:
>> +#
>> +# Both block-core and migration needs SocketAddressList
>
> s/needs/need/

done

>> +# I am open to comments about how to share it
>
> Since this is two sentences, trailing '.' would help.

done

>> +#
>> +# @dummy-list: A dummy list
>> +#
>> +# Since: 2.13
>> +##
>> +{ 'struct': 'DummyStruct',
>> +  'data': { 'dummy-list': ['SocketAddress'] } }
>>
>
> We've used this idiom elsewhere; it might be better to amend
> DummyForceArrays in qapi/misc.json, except then misc.json might need
> to include sockets.json for the definition of SocketAddress.

Not done.  It needs more qapi magic that puttingthe socket.json include
there, so this one stays.  Putting it there means including
qapi/-misc.h in several places, I think it is easier this way
(famous last words).

Later, Juan.

Re: [Qemu-devel] [Qemu-ppc] [PATCH 1/2] ppc: Allow clients of the 440 pcix bus to specify the number of interrupts

[Cédric Le Goater]

On 07/31/2018 06:36 AM, Sebastian Bauer wrote:
> This can be done by using the newly introduced num_irqs property. In
> particular, this change introduces a special case if num_irqs is 1 in which
> case any interrupt pin will be connected to the single irq. The default
> case is untouched (but note that the only client is the Sam460ex board for
> which the special case was actually created).
> 
> Signed-off-by: Sebastian Bauer 
> ---
>  hw/ppc/ppc440_pcix.c | 28 +---
>  1 file changed, 25 insertions(+), 3 deletions(-)
> 
> diff --git a/hw/ppc/ppc440_pcix.c b/hw/ppc/ppc440_pcix.c
> index d8af04b70f..cb7d7cfd2b 100644
> --- a/hw/ppc/ppc440_pcix.c
> +++ b/hw/ppc/ppc440_pcix.c
> @@ -57,6 +57,7 @@ typedef struct PPC440PCIXState {
>  struct PLBOutMap pom[PPC440_PCIX_NR_POMS];
>  struct PLBInMap pim[PPC440_PCIX_NR_PIMS];
>  uint32_t sts;
> +uint16_t num_irqs;
>  qemu_irq irq[PCI_NUM_PINS];
>  AddressSpace bm_as;
>  MemoryRegion bm;
> @@ -423,6 +424,12 @@ static int ppc440_pcix_map_irq(PCIDevice *pci_dev, int 
> irq_num)
>  return slot - 1;
>  }
>  
> +/* All pins from each slot are tied the same and only board IRQ. */
> +static int ppc440_pcix_map_irq_single(PCIDevice *pci_dev, int irq_num)
> +{
> +return 0;
> +}
> +
>  static void ppc440_pcix_set_irq(void *opaque, int irq_num, int level)
>  {
>  qemu_irq *pci_irqs = opaque;
> @@ -469,6 +476,7 @@ const MemoryRegionOps ppc440_pcix_host_data_ops = {
>  
>  static int ppc440_pcix_initfn(SysBusDevice *dev)
>  {
> +pci_map_irq_fn map_irq;
>  PPC440PCIXState *s;
>  PCIHostState *h;
>  int i;
> @@ -476,14 +484,22 @@ static int ppc440_pcix_initfn(SysBusDevice *dev)
>  h = PCI_HOST_BRIDGE(dev);
>  s = PPC440_PCIX_HOST_BRIDGE(dev);
>  
> -for (i = 0; i < ARRAY_SIZE(s->irq); i++) {
> +if (s->num_irqs > 4) {

may be use PCI_NUM_PINS

> +fprintf(stderr, "%s: Number of irqs must not exceed 4\n", __func__);

error_report would be better.
 
> +return -1;
> +}

It might be the right time to QOM'ify a bit more "ppc440-pcix-host" and 
introduce a realize() operation to handle errors. 

> +for (i = 0; i < s->num_irqs; i++) {
>  sysbus_init_irq(dev, >irq[i]);
>  }
>  
>  memory_region_init(>busmem, OBJECT(dev), "pci bus memory", 
> UINT64_MAX);
> +
> +map_irq = s->num_irqs == 1 ?
> +ppc440_pcix_map_irq_single : ppc440_pcix_map_irq;
>  h->bus = pci_register_root_bus(DEVICE(dev), NULL, ppc440_pcix_set_irq,
> - ppc440_pcix_map_irq, s->irq, >busmem,
> - get_system_io(), PCI_DEVFN(0, 0), 4, TYPE_PCI_BUS);
> + map_irq, s->irq, >busmem, get_system_io(),
> + PCI_DEVFN(0, 0), s->num_irqs, TYPE_PCI_BUS);
>  
>  s->dev = pci_create_simple(h->bus, PCI_DEVFN(0, 0), 
> "ppc4xx-host-bridge");
>  
> @@ -507,6 +523,11 @@ static int ppc440_pcix_initfn(SysBusDevice *dev)
>  return 0;
>  }
>  
> +static Property ppc440_pcix_properties[] = {
> +DEFINE_PROP_UINT16("num-irqs", PPC440PCIXState, num_irqs, 4),

PCI_NUM_PINS ?

> +DEFINE_PROP_END_OF_LIST(),
> +};
> +
>  static void ppc440_pcix_class_init(ObjectClass *klass, void *data)
>  {
>  SysBusDeviceClass *k = SYS_BUS_DEVICE_CLASS(klass);
> @@ -514,6 +535,7 @@ static void ppc440_pcix_class_init(ObjectClass *klass, 
> void *data)
>  
>  k->init = ppc440_pcix_initfn;
>  dc->reset = ppc440_pcix_reset;
> +dc->props = ppc440_pcix_properties;
>  }
>  
>  static const TypeInfo ppc440_pcix_info = {
> 

Re: [Qemu-devel] [PATCH v5 30/76] target/mips: Add emulation of misc nanoMIPS instructions (pool32a0)

[Richard Henderson]

On 07/30/2018 12:12 PM, Aleksandar Markovic wrote:
> +case NM_SOV:
> +{
> +TCGv t0 = tcg_temp_local_new();

tcg_temp_new.

Otherwise,
Reviewed-by: Richard Henderson 


r~

Re: [Qemu-devel] [PULL 0/3] Linux user for 3.0 patches

[Peter Maydell]

On 31 July 2018 at 09:42, Laurent Vivier  wrote:
> The following changes since commit 6d9dd5fb9d0e9f4a174f53a0e20a39fbe809c71e:
>
>   Merge remote-tracking branch 
> 'remotes/armbru/tags/pull-qobject-2018-07-27-v2' into staging (2018-07-30 
> 09:55:47 +0100)
>
> are available in the Git repository at:
>
>   git://github.com/vivier/qemu.git tags/linux-user-for-3.0-pull-request
>
> for you to fetch changes up to 5d9f3ea0817215ad4baac5aa30414e9ebbaaf0d6:
>
>   linux-user: ppc64: don't use volatile register during safe_syscall 
> (2018-07-31 09:57:43 +0200)
>
> 
> Fix safe_syscall() on ppc64 host
> Fix mmap() 0 length error case
>
> 

Applied, thanks.

-- PMM

Re: [Qemu-devel] [PATCH v5 28/76] target/mips: Add emulation of nanoMIPS 48-bit instructions

[Richard Henderson]

On 07/30/2018 12:12 PM, Aleksandar Markovic wrote:
>  case NM_P48I:
> -return 6;
> +{
> +insn = cpu_lduw_code(env, ctx->base.pc_next + 4);
> +uint32_t offset = extract32(ctx->opcode, 0, 16) | insn << 16;

This value is supposed to be signed.


r~

Re: [Qemu-devel] [PATCH v5 27/76] target/mips: Add emulation of nanoMIPS instructions MOVE.P and MOVE.PREV

[Richard Henderson]

On 07/30/2018 12:12 PM, Aleksandar Markovic wrote:
> From: Yongbok Kim 
> 
> Add emulation of nanoMIPS instructions MOVE.P and MOVE.PREV.
> 
> Signed-off-by: Yongbok Kim 
> Signed-off-by: Aleksandar Markovic 
> Signed-off-by: Stefan Markovic 
> ---
>  target/mips/translate.c | 33 -
>  1 file changed, 32 insertions(+), 1 deletion(-)

Reviewed-by: Richard Henderson 


r~

[Qemu-devel] ARM: SVE while issue

[Laurent Desnogues]

Hello Richard,

according to SVE specification, whilels/whilele instructions have a
special case where if the second operand is the maximum (un)signed
integer then the result is an all-true predicate.  The current code in
trans_WHILE doesn't seem to capture that requirement.  I'm afraid the
fix won't be as trivial as for the other two bugs I reported :-)

Thanks,

Laurent

Re: [Qemu-devel] [PATCH v5 26/76] target/mips: Add emulation of some common nanoMIPS 32-bit instructions

[Richard Henderson]

On 07/30/2018 12:11 PM, Aleksandar Markovic wrote:
> +case NM_ADDIUPC:
> +if (rt != 0) {
> +int32_t offset = sextract32(ctx->opcode, 0, 1) << 21 |
> + extract32(ctx->opcode, 1, 20) << 1;
> +target_long addr = addr_add(ctx, ctx->base.pc_next + 4, offset);
> +tcg_gen_movi_tl(cpu_gpr[rt], (int32_t)addr);

addr_add has already done any required sign-extend.
Including an extra one is wrong for nanomips64.

> +case NM_SEQI:
> +{
> +TCGv t0 = tcg_temp_new();
> +TCGv t1 = tcg_temp_new();
> +TCGv t2 = tcg_temp_local_new();
> +
> +gen_load_gpr(t0, rs);
> +tcg_gen_movi_tl(t1, extract32(ctx->opcode, 0, 12));
> +tcg_gen_setcond_tl(TCG_COND_EQ, t2, t0, t1);
> +gen_store_gpr(t2, rt);
> +
> +tcg_temp_free(t0);
> +tcg_temp_free(t1);
> +tcg_temp_free(t2);
> +}
> +break;

You only need one temporary.

  TCGv t0 = tcg_temp_new();
  int imm = extract32(ctx->opcode, 0, 12);

  gen_load_gpr(t0, rs);
  tcg_gen_setcondi_tl(TCG_COND_EQ, t0, t0, imm);
  gen_store_gpr(t0, rt);

  tcg_temp_free(t0);

Do not use tcg_temp_local_new unless you really need it in order to cross a
basic block boundary (branch or label).

> +} else if (rt == 0 && shift == 6) {
> +/* SYNC */
> +check_insn(ctx, ISA_MIPS2);

Cut and paste error from elsewhere?
Why would you need this ISA check?


r~

Re: [Qemu-devel] [PATCH] throttle-groups: fix hang when group member leaves

[Alberto Garcia]

On Wed 04 Jul 2018 04:54:10 PM CEST, Stefan Hajnoczi wrote:
> Throttle groups consist of members sharing one throttling state
> (including bps/iops limits).  Round-robin scheduling is used to ensure
> fairness.  If a group member already has a timer pending then other
> groups members do not schedule their own timers.  The next group
> member will have its turn when the existing timer expires.
>
> A hang may occur when a group member leaves while it had a timer
> scheduled.

I haven't been able to reproduce this. When a member is removed from the
group the pending request queue must already be empty, so does this mean
that there's still a timer when the queue is already empty?

Berto

Re: [Qemu-devel] [PATCH] Add interactive mode to qemu-img command

[no-reply]

Hi,

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

Type: series
Message-id: 20180730192955.14291-1-programmingk...@gmail.com
Subject: [Qemu-devel] [PATCH] Add interactive mode to qemu-img command

=== TEST SCRIPT BEGIN ===
#!/bin/bash
time make docker-test-mingw@fedora SHOW_ENV=1 J=8
=== TEST SCRIPT END ===

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
Switched to a new branch 'test'
cabdf929f6 Add interactive mode to qemu-img command

=== OUTPUT BEGIN ===
  BUILD   fedora
make[1]: Entering directory '/var/tmp/patchew-tester-tmp-bxethase/src'
  GEN 
/var/tmp/patchew-tester-tmp-bxethase/src/docker-src.2018-07-30-21.50.01.3098/qemu.tar
Cloning into 
'/var/tmp/patchew-tester-tmp-bxethase/src/docker-src.2018-07-30-21.50.01.3098/qemu.tar.vroot'...
done.
Checking out files:  49% (3152/6319)   
Checking out files:  50% (3160/6319)   
Checking out files:  51% (3223/6319)   
Checking out files:  52% (3286/6319)   
Checking out files:  53% (3350/6319)   
Checking out files:  54% (3413/6319)   
Checking out files:  55% (3476/6319)   
Checking out files:  56% (3539/6319)   
Checking out files:  57% (3602/6319)   
Checking out files:  58% (3666/6319)   
Checking out files:  59% (3729/6319)   
Checking out files:  60% (3792/6319)   
Checking out files:  61% (3855/6319)   
Checking out files:  62% (3918/6319)   
Checking out files:  63% (3981/6319)   
Checking out files:  64% (4045/6319)   
Checking out files:  65% (4108/6319)   
Checking out files:  66% (4171/6319)   
Checking out files:  67% (4234/6319)   
Checking out files:  68% (4297/6319)   
Checking out files:  69% (4361/6319)   
Checking out files:  70% (4424/6319)   
Checking out files:  71% (4487/6319)   
Checking out files:  72% (4550/6319)   
Checking out files:  73% (4613/6319)   
Checking out files:  74% (4677/6319)   
Checking out files:  75% (4740/6319)   
Checking out files:  76% (4803/6319)   
Checking out files:  77% (4866/6319)   
Checking out files:  78% (4929/6319)   
Checking out files:  79% (4993/6319)   
Checking out files:  80% (5056/6319)   
Checking out files:  81% (5119/6319)   
Checking out files:  82% (5182/6319)   
Checking out files:  83% (5245/6319)   
Checking out files:  84% (5308/6319)   
Checking out files:  85% (5372/6319)   
Checking out files:  86% (5435/6319)   
Checking out files:  87% (5498/6319)   
Checking out files:  88% (5561/6319)   
Checking out files:  89% (5624/6319)   
Checking out files:  90% (5688/6319)   
Checking out files:  91% (5751/6319)   
Checking out files:  92% (5814/6319)   
Checking out files:  93% (5877/6319)   
Checking out files:  94% (5940/6319)   
Checking out files:  95% (6004/6319)   
Checking out files:  96% (6067/6319)   
Checking out files:  97% (6130/6319)   
Checking out files:  98% (6193/6319)   
Checking out files:  99% (6256/6319)   
Checking out files: 100% (6319/6319)   
Checking out files: 100% (6319/6319), done.
Your branch is up-to-date with 'origin/test'.
Submodule 'dtc' (git://git.qemu-project.org/dtc.git) registered for path 'dtc'
Cloning into 
'/var/tmp/patchew-tester-tmp-bxethase/src/docker-src.2018-07-30-21.50.01.3098/qemu.tar.vroot/dtc'...
Submodule path 'dtc': checked out 'e54388015af1fb4bf04d0bca99caba1074d9cc42'
Submodule 'ui/keycodemapdb' (git://git.qemu.org/keycodemapdb.git) registered 
for path 'ui/keycodemapdb'
Cloning into 
'/var/tmp/patchew-tester-tmp-bxethase/src/docker-src.2018-07-30-21.50.01.3098/qemu.tar.vroot/ui/keycodemapdb'...
Submodule path 'ui/keycodemapdb': checked out 
'6b3d716e2b6472eb7189d3220552280ef3d832ce'
  COPYRUNNER
RUN test-mingw in qemu:fedora 
Packages installed:
SDL2-devel-2.0.8-5.fc28.x86_64
bc-1.07.1-5.fc28.x86_64
bison-3.0.4-9.fc28.x86_64
bluez-libs-devel-5.49-3.fc28.x86_64
brlapi-devel-0.6.7-12.fc28.x86_64
bzip2-1.0.6-26.fc28.x86_64
bzip2-devel-1.0.6-26.fc28.x86_64
ccache-3.4.2-2.fc28.x86_64
clang-6.0.0-5.fc28.x86_64
device-mapper-multipath-devel-0.7.4-2.git07e7bd5.fc28.x86_64
findutils-4.6.0-19.fc28.x86_64
flex-2.6.1-7.fc28.x86_64
gcc-8.1.1-1.fc28.x86_64
gcc-c++-8.1.1-1.fc28.x86_64
gettext-0.19.8.1-14.fc28.x86_64
git-2.17.1-2.fc28.x86_64
glib2-devel-2.56.1-3.fc28.x86_64
glusterfs-api-devel-4.0.2-1.fc28.x86_64
gnutls-devel-3.6.2-1.fc28.x86_64
gtk3-devel-3.22.30-1.fc28.x86_64
hostname-3.20-3.fc28.x86_64
libaio-devel-0.3.110-11.fc28.x86_64
libasan-8.1.1-1.fc28.x86_64
libattr-devel-2.4.47-23.fc28.x86_64
libcap-devel-2.25-9.fc28.x86_64
libcap-ng-devel-0.7.9-1.fc28.x86_64
libcurl-devel-7.59.0-3.fc28.x86_64
libfdt-devel-1.4.6-4.fc28.x86_64
libpng-devel-1.6.34-3.fc28.x86_64
librbd-devel-12.2.5-1.fc28.x86_64
libssh2-devel-1.8.0-7.fc28.x86_64
libubsan-8.1.1-1.fc28.x86_64
libusbx-devel-1.0.21-6.fc28.x86_64
libxml2-devel-2.9.7-4.fc28.x86_64
llvm-6.0.0-11.fc28.x86_64
lzo-devel-2.08-12.fc28.x86_64
make-4.2.1-6.fc28.x86_64
mingw32-SDL2-2.0.5-3.fc27.noarch
mingw32-bzip2-1.0.6-9.fc27.noarch

Re: [Qemu-devel] [PATCH v5 25/76] target/mips: Add emulation of nanoMIPS 16-bit save and restore instructions

[Richard Henderson]

On 07/30/2018 12:11 PM, Aleksandar Markovic wrote:
> +static void gen_adjust_sp(DisasContext *ctx, int u)
> +{
> +TCGv tsp = tcg_temp_new();
> +gen_base_offset_addr(ctx, tsp, 29, u);
> +gen_store_gpr(tsp, 29);
> +tcg_temp_free(tsp);
> +}

This could now be just

  gen_op_addr_addi(cpu_gpr[29], cpu_gpr[29], u);


r~

Re: [Qemu-devel] [PULL 0/3] Linux user for 3.0 patches

[Alex Bennée]

Laurent Vivier  writes:

> Le 31/07/2018 à 14:24, no-re...@patchew.org a écrit:
>> Hi,
>>
>> This series seems to have some coding style problems. See output below for
>> more information:
>>
>> Type: series
>> Message-id: 20180731084203.29959-1-laur...@vivier.eu
>> Subject: [Qemu-devel] [PULL 0/3] Linux user for 3.0 patches
>>
>> === TEST SCRIPT BEGIN ===
>> #!/bin/bash
>>
>> BASE=base
>> n=1
>> total=$(git log --oneline $BASE.. | wc -l)
>> failed=0
>>
>> git config --local diff.renamelimit 0
>> git config --local diff.renames True
>> git config --local diff.algorithm histogram
>>
>> commits="$(git log --format=%H --reverse $BASE..)"
>> for c in $commits; do
>> echo "Checking PATCH $n/$total: $(git log -n 1 --format=%s $c)..."
>> if ! git show $c --format=email | ./scripts/checkpatch.pl --mailback -; 
>> then
>> failed=1
>> echo
>> fi
>> n=$((n+1))
>> done
>>
>> exit $failed
>> === TEST SCRIPT END ===
>>
>> Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
>> Switched to a new branch 'test'
>> 806398c875 linux-user: ppc64: don't use volatile register during safe_syscall
>> ba78346662 tests: add check_invalid_maps to test-mmap
>> 80fc1be868 linux-user/mmap.c: handle invalid len maps correctly
>>
>> === OUTPUT BEGIN ===
>> Checking PATCH 1/3: linux-user/mmap.c: handle invalid len maps correctly...
>> Checking PATCH 2/3: tests: add check_invalid_maps to test-mmap...
>> ERROR: code indent should never use tabs
>> #62: FILE: tests/tcg/multiarch/test-mmap.c:498:
>> +^Icheck_invalid_mmaps();$

Sorry I should of flagged this in the commit message. I left the touched
bits as is and used spaces for new functions.

>>
>> total: 1 errors, 0 warnings, 40 lines checked
>
> I'm going to resend a pull request without the tab.
>
> Thanks,
> Laurent


--
Alex Bennée

Re: [Qemu-devel] [PATCH v5 24/76] target/mips: Add emulation of nanoMIPS 16-bit logic instructions

[Richard Henderson]

On 07/30/2018 12:11 PM, Aleksandar Markovic wrote:
> From: Yongbok Kim 
> 
> Add emulation of NOT16, AND16, XOR16, OR16 instructions.
> 
> Reviewed-by: Richard Henderson 
> Signed-off-by: Yongbok Kim 
> Signed-off-by: Aleksandar Markovic 
> Signed-off-by: Stefan Markovic 
> ---
>  target/mips/translate.c | 28 
>  1 file changed, 28 insertions(+)

Reviewed-by: Richard Henderson 


r~

Re: [Qemu-devel] [PATCH v3 2/2] s390x: Enable KVM huge page backing support

[Janosch Frank]

On 31.07.2018 15:06, Cornelia Huck wrote:
> On Tue, 31 Jul 2018 13:09:08 +0100
> Janosch Frank  wrote:
> 
>> QEMU has had huge page support for a longer time already, but KVM
>> memory management under s390x needed some changes to work with huge
>> backings.
>>
>> Now that we have support, let's enable it if requested and
>> available. Otherwise we now properly tell the user if there is no
>> support and back out instead of failing to run the VM later on.
>>
>> Signed-off-by: Janosch Frank 
>> ---
>>  target/s390x/kvm.c | 15 +++
>>  1 file changed, 15 insertions(+)
>>
>> diff --git a/target/s390x/kvm.c b/target/s390x/kvm.c
>> index d923cf4240..f961c3b84a 100644
>> --- a/target/s390x/kvm.c
>> +++ b/target/s390x/kvm.c
>> @@ -34,6 +34,7 @@
>>  #include "qapi/error.h"
>>  #include "qemu/error-report.h"
>>  #include "qemu/timer.h"
>> +#include "qemu/mmap-alloc.h"
>>  #include "sysemu/sysemu.h"
>>  #include "sysemu/hw_accel.h"
>>  #include "hw/hw.h"
>> @@ -285,6 +286,20 @@ int kvm_arch_init(MachineState *ms, KVMState *s)
>>  {
>>  MachineClass *mc = MACHINE_GET_CLASS(ms);
>>  
>> +if (mem_path) {
>> +if (qemu_mempath_getpagesize(mem_path) != (1 << 20)) {
> 
> 1 * MiB ?

I only looked for page size definitions, didn't know it exists, yes 1 *
MiB is a bit nicer.

I'd wait for more comments before sending the v4 out, so consider this
fixed.

> 
>> +error_report("Huge page backing with pages > 1M was specified, "
>> + "but KVM does not support this memory backing");
>> +return -EINVAL;
>> +
>> +}
>> +if (kvm_vm_enable_cap(s, KVM_CAP_S390_HPAGE_1M, 0)) {
>> +error_report("Huge page backing with 1M pages was specified, "
>> + "but KVM does not support this memory backing");
>> +return -EINVAL;
>> +}
>> +}
>> +
>>  mc->default_cpu_type = S390_CPU_TYPE_NAME("host");
>>  cap_sync_regs = kvm_check_extension(s, KVM_CAP_SYNC_REGS);
>>  cap_async_pf = kvm_check_extension(s, KVM_CAP_ASYNC_PF);
> 
> 




signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH] qemu-img-cmds.hx: Add example usage for create command

[Programmingkid]

> On Jul 31, 2018, at 7:57 AM, Eric Blake  wrote:
> 
> On 07/30/2018 09:52 PM, John Arbuckle wrote:
>> Add an example on how to use the create command. I believe this will make 
>> qemu-img easier to use.
>> Signed-off-by: John Arbuckle 
>> ---
>>  qemu-img-cmds.hx | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>> diff --git a/qemu-img-cmds.hx b/qemu-img-cmds.hx
>> index 69758fb6e8..92f7437944 100644
>> --- a/qemu-img-cmds.hx
>> +++ b/qemu-img-cmds.hx
>> @@ -50,7 +50,7 @@ STEXI
>>  ETEXI
>>DEF("create", img_create,
>> -"create [--object objectdef] [-q] [-f fmt] [-b backing_file] [-F 
>> backing_fmt] [-u] [-o options] filename [size]")
>> +"create [--object objectdef] [-q] [-f fmt] [-b backing_file] [-F 
>> backing_fmt] [-u] [-o options] filename [size]\nExample: qemu-img create -f 
>> qcow2 WindowsXP.qcow2 10G")
> 
> Making a long line longer. It would be worth using C string concatenation and 
> splitting this over two lines, at the \n.

Sounds like a good idea.

> Using the name WindowsXP.qcow2 as the guest is somewhat misleading (that OS 
> is proprietary, and quickly reaching the point of obsolescence from its 
> vendor - furthermore, qemu-img doesn't actually install an OS, but rather 
> creates a blank image for a later install process to utilize); better would 
> be a generic name that won't go out of date, such 'image.qcow2'.

I always felt a concrete example was easier to understand rather than a generic 
example. What about this: 

Example: qemu-img create -f qcow2 .qcow2 10G

> 
>>  STEXI
>>  @item create [--object @var{objectdef}] [-q] [-f @var{fmt}] [-b 
>> @var{backing_file}] [-F @var{backing_fmt}] [-u] [-o @var{options}] 
>> @var{filename} [@var{size}]
>>  ETEXI
> 
> -- 
> Eric Blake, Principal Software Engineer
> Red Hat, Inc.   +1-919-301-3266
> Virtualization:  qemu.org | libvirt.org

Re: [Qemu-devel] [PATCH] qemu-img-cmds.hx: Add example usage for create command

[Programmingkid]

> On Jul 31, 2018, at 8:35 AM, Kevin Wolf  wrote:
> 
> Am 31.07.2018 um 13:57 hat Eric Blake geschrieben:
>> On 07/30/2018 09:52 PM, John Arbuckle wrote:
>>> Add an example on how to use the create command. I believe this will make 
>>> qemu-img easier to use.
>>> 
>>> Signed-off-by: John Arbuckle 
>>> ---
>>>  qemu-img-cmds.hx | 2 +-
>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>> 
>>> diff --git a/qemu-img-cmds.hx b/qemu-img-cmds.hx
>>> index 69758fb6e8..92f7437944 100644
>>> --- a/qemu-img-cmds.hx
>>> +++ b/qemu-img-cmds.hx
>>> @@ -50,7 +50,7 @@ STEXI
>>>  ETEXI
>>>  DEF("create", img_create,
>>> -"create [--object objectdef] [-q] [-f fmt] [-b backing_file] [-F 
>>> backing_fmt] [-u] [-o options] filename [size]")
>>> +"create [--object objectdef] [-q] [-f fmt] [-b backing_file] [-F 
>>> backing_fmt] [-u] [-o options] filename [size]\nExample: qemu-img create -f 
>>> qcow2 WindowsXP.qcow2 10G")
>> 
>> Making a long line longer. It would be worth using C string concatenation
>> and splitting this over two lines, at the \n.
>> 
>> Using the name WindowsXP.qcow2 as the guest is somewhat misleading (that OS
>> is proprietary, and quickly reaching the point of obsolescence from its
>> vendor - furthermore, qemu-img doesn't actually install an OS, but rather
>> creates a blank image for a later install process to utilize); better would
>> be a generic name that won't go out of date, such 'image.qcow2'.
> 
> Also, while I like the idea of adding examples to the man page, I don't
> think adding it here would give the right result. The example would
> appear in the middle of the subcommand syntax lines.

As it reads now I don't feel its easy for the user to decipher. Having an
example near by would help the user understand how to use it.

> I'd rather add a whole new section "EXAMPLES" in the man page.

That is a good idea. I would like to have examples in both documents. 

Re: [Qemu-devel] [PULL 0/3] Linux user for 3.0 patches

[no-reply]

Hi,

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

Type: series
Message-id: 20180731084203.29959-1-laur...@vivier.eu
Subject: [Qemu-devel] [PULL 0/3] Linux user for 3.0 patches

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

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

git config --local diff.renamelimit 0
git config --local diff.renames True
git config --local diff.algorithm histogram

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

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

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
Switched to a new branch 'test'
806398c875 linux-user: ppc64: don't use volatile register during safe_syscall
ba78346662 tests: add check_invalid_maps to test-mmap
80fc1be868 linux-user/mmap.c: handle invalid len maps correctly

=== OUTPUT BEGIN ===
Checking PATCH 1/3: linux-user/mmap.c: handle invalid len maps correctly...
Checking PATCH 2/3: tests: add check_invalid_maps to test-mmap...
ERROR: code indent should never use tabs
#62: FILE: tests/tcg/multiarch/test-mmap.c:498:
+^Icheck_invalid_mmaps();$

total: 1 errors, 0 warnings, 40 lines checked

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

Checking PATCH 3/3: linux-user: ppc64: don't use volatile register during 
safe_syscall...
=== OUTPUT END ===

Test command exited with code: 1


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