date:20180221

Sometimes the same keysym can be created using different key
combinations.  Record them all in the reverse keymap, not only
the first one.

Signed-off-by: Gerd Hoffmann 
Reviewed-by: Daniel P. Berrangé 
---
 ui/keymaps.c | 14 +++---
 1 file changed, 11 insertions(+), 3 deletions(-)

diff --git a/ui/keymaps.c b/ui/keymaps.c
index cbdd65c767..1eba6d7057 100644
--- a/ui/keymaps.c
+++ b/ui/keymaps.c
@@ -29,7 +29,8 @@
 #include "qemu/error-report.h"
 
 struct keysym2code {
-uint16_t keycode;
+uint32_t count;
+uint16_t keycodes[4];
 };
 
 struct kbd_layout_t {
@@ -62,11 +63,18 @@ static void add_keysym(char *line, int keysym, int keycode, 
kbd_layout_t *k)
 
 keysym2code = g_hash_table_lookup(k->hash, GINT_TO_POINTER(keysym));
 if (keysym2code) {
+if (keysym2code->count < ARRAY_SIZE(keysym2code->keycodes)) {
+keysym2code->keycodes[keysym2code->count++] = keycode;
+} else {
+warn_report("more than %zd keycodes for keysym %d",
+ARRAY_SIZE(keysym2code->keycodes), keysym);
+}
 return;
 }
 
 keysym2code = g_new0(struct keysym2code, 1);
-keysym2code->keycode = keycode;
+keysym2code->keycodes[0] = keycode;
+keysym2code->count = 1;
 g_hash_table_replace(k->hash, GINT_TO_POINTER(keysym), keysym2code);
 trace_keymap_add(keysym, keycode, line);
 }
@@ -185,7 +193,7 @@ int keysym2scancode(kbd_layout_t *k, int keysym)
 return 0;
 }
 
-return keysym2code->keycode;
+return keysym2code->keycodes[0];
 }
 
 int keycode_is_keypad(kbd_layout_t *k, int keycode)
-- 
2.9.3

Re: [Qemu-devel] [PATCH 08/11] openpic: move OpenPIC state and related definitions to openpic.h

2018-02-21 Thread David Gibson

On Mon, Feb 19, 2018 at 06:19:19PM +, Mark Cave-Ayland wrote:
> This is to faciliate access to OpenPICState when wiring up the PIC to the 
> macio
> controller.
> 
> Signed-off-by: Mark Cave-Ayland 

Reviewed-by: David Gibson 

> ---
>  hw/intc/openpic.c| 157 --
>  include/hw/ppc/openpic.h | 160 
> ++-
>  2 files changed, 159 insertions(+), 158 deletions(-)
> 
> diff --git a/hw/intc/openpic.c b/hw/intc/openpic.c
> index 9159a06f07..811cee9b26 100644
> --- a/hw/intc/openpic.c
> +++ b/hw/intc/openpic.c
> @@ -63,10 +63,6 @@ static int get_current_cpu(void);
>  } \
>  } while (0)
>  
> -#define MAX_CPU 32
> -#define MAX_MSI 8
> -#define VID 0x03 /* MPIC version ID */
> -
>  /* OpenPIC capability flags */
>  #define OPENPIC_FLAG_IDR_CRIT (1 << 0)
>  #define OPENPIC_FLAG_ILR  (2 << 0)
> @@ -85,35 +81,6 @@ static int get_current_cpu(void);
>  #define OPENPIC_CPU_REG_START0x2
>  #define OPENPIC_CPU_REG_SIZE 0x100 + ((MAX_CPU - 1) * 0x1000)
>  
> -/* Raven */
> -#define RAVEN_MAX_CPU  2
> -#define RAVEN_MAX_EXT 48
> -#define RAVEN_MAX_IRQ 64
> -#define RAVEN_MAX_TMR  OPENPIC_MAX_TMR
> -#define RAVEN_MAX_IPI  OPENPIC_MAX_IPI
> -
> -/* KeyLargo */
> -#define KEYLARGO_MAX_CPU  4
> -#define KEYLARGO_MAX_EXT  64
> -#define KEYLARGO_MAX_IPI  4
> -#define KEYLARGO_MAX_IRQ  (64 + KEYLARGO_MAX_IPI)
> -#define KEYLARGO_MAX_TMR  0
> -#define KEYLARGO_IPI_IRQ  (KEYLARGO_MAX_EXT) /* First IPI IRQ */
> -/* Timers don't exist but this makes the code happy... */
> -#define KEYLARGO_TMR_IRQ  (KEYLARGO_IPI_IRQ + KEYLARGO_MAX_IPI)
> -
> -/* Interrupt definitions */
> -#define RAVEN_FE_IRQ (RAVEN_MAX_EXT) /* Internal functional IRQ */
> -#define RAVEN_ERR_IRQ(RAVEN_MAX_EXT + 1) /* Error IRQ */
> -#define RAVEN_TMR_IRQ(RAVEN_MAX_EXT + 2) /* First timer IRQ */
> -#define RAVEN_IPI_IRQ(RAVEN_TMR_IRQ + RAVEN_MAX_TMR) /* First IPI IRQ */
> -/* First doorbell IRQ */
> -#define RAVEN_DBL_IRQ(RAVEN_IPI_IRQ + (RAVEN_MAX_CPU * RAVEN_MAX_IPI))
> -
> -typedef struct FslMpicInfo {
> -int max_ext;
> -} FslMpicInfo;
> -
>  static FslMpicInfo fsl_mpic_20 = {
>  .max_ext = 12,
>  };
> @@ -211,55 +178,6 @@ static void openpic_cpu_write_internal(void *opaque, 
> hwaddr addr,
> uint32_t val, int idx);
>  static void openpic_reset(DeviceState *d);
>  
> -typedef enum IRQType {
> -IRQ_TYPE_NORMAL = 0,
> -IRQ_TYPE_FSLINT,/* FSL internal interrupt -- level only */
> -IRQ_TYPE_FSLSPECIAL,/* FSL timer/IPI interrupt, edge, no polarity */
> -} IRQType;
> -
> -/* Round up to the nearest 64 IRQs so that the queue length
> - * won't change when moving between 32 and 64 bit hosts.
> - */
> -#define IRQQUEUE_SIZE_BITS ((OPENPIC_MAX_IRQ + 63) & ~63)
> -
> -typedef struct IRQQueue {
> -unsigned long *queue;
> -int32_t queue_size; /* Only used for VMSTATE_BITMAP */
> -int next;
> -int priority;
> -} IRQQueue;
> -
> -typedef struct IRQSource {
> -uint32_t ivpr;  /* IRQ vector/priority register */
> -uint32_t idr;   /* IRQ destination register */
> -uint32_t destmask; /* bitmap of CPU destinations */
> -int last_cpu;
> -int output; /* IRQ level, e.g. OPENPIC_OUTPUT_INT */
> -int pending;/* TRUE if IRQ is pending */
> -IRQType type;
> -bool level:1;   /* level-triggered */
> -bool nomask:1;  /* critical interrupts ignore mask on some FSL MPICs */
> -} IRQSource;
> -
> -#define IVPR_MASK_SHIFT   31
> -#define IVPR_MASK_MASK(1U << IVPR_MASK_SHIFT)
> -#define IVPR_ACTIVITY_SHIFT   30
> -#define IVPR_ACTIVITY_MASK(1U << IVPR_ACTIVITY_SHIFT)
> -#define IVPR_MODE_SHIFT   29
> -#define IVPR_MODE_MASK(1U << IVPR_MODE_SHIFT)
> -#define IVPR_POLARITY_SHIFT   23
> -#define IVPR_POLARITY_MASK(1U << IVPR_POLARITY_SHIFT)
> -#define IVPR_SENSE_SHIFT  22
> -#define IVPR_SENSE_MASK   (1U << IVPR_SENSE_SHIFT)
> -
> -#define IVPR_PRIORITY_MASK (0xFU << 16)
> -#define IVPR_PRIORITY(_ivprr_) ((int)(((_ivprr_) & IVPR_PRIORITY_MASK) >> 
> 16))
> -#define IVPR_VECTOR(opp, _ivprr_) ((_ivprr_) & (opp)->vector_mask)
> -
> -/* IDR[EP/CI] are only for FSL MPIC prior to v4.0 */
> -#define IDR_EP  0x8000  /* external pin */
> -#define IDR_CI  0x4000  /* critical interrupt */
> -
>  /* Convert between openpic clock ticks and nanosecs.  In the hardware the 
> clock
> frequency is driven by board inputs to the PIC which the PIC would then
> divide by 4 or 8.  For now hard code to 25MZ.
> @@ -275,81 +193,6 @@ static inline uint64_t ticks_to_ns(uint64_t ticks)
>  return ticks * OPENPIC_TIMER_NS_PER_TICK;
>  }
>  
> -typedef struct OpenPICTimer {
> -uint32_t tccr;  /* Global timer current count register */
> -uint32_t tbcr;  /* Global timer

Re: [Qemu-devel] [PATCH 09/11] mac_newworld: use object link to pass OpenPIC object to macio

2018-02-21 Thread David Gibson

On Mon, Feb 19, 2018 at 06:19:20PM +, Mark Cave-Ayland wrote:
> Also switch macio_newworld_realize() over to use it rather than using the 
> pic_mem
> memory region directly.
> 
> Now that both Old World and New World macio devices no longer make use of the
> pic_mem memory region directly, we can remove it.
> 
> Signed-off-by: Mark Cave-Ayland 

Reviewed-by: David Gibson 

> ---
>  hw/misc/macio/macio.c | 14 +-
>  hw/ppc/mac_newworld.c | 20 +++-
>  include/hw/misc/macio/macio.h |  4 +++-
>  3 files changed, 23 insertions(+), 15 deletions(-)
> 
> diff --git a/hw/misc/macio/macio.c b/hw/misc/macio/macio.c
> index d4c1d190c4..e5288f1084 100644
> --- a/hw/misc/macio/macio.c
> +++ b/hw/misc/macio/macio.c
> @@ -279,10 +279,10 @@ static void macio_newworld_realize(PCIDevice *d, Error 
> **errp)
>  sysbus_connect_irq(sysbus_dev, 0, ns->irqs[cur_irq++]);
>  sysbus_connect_irq(sysbus_dev, 1, ns->irqs[cur_irq++]);
>  
> -if (s->pic_mem) {
> -/* OpenPIC */
> -memory_region_add_subregion(>bar, 0x4, s->pic_mem);
> -}
> +/* OpenPIC */
> +sysbus_dev = SYS_BUS_DEVICE(ns->pic);
> +memory_region_add_subregion(>bar, 0x4,
> +sysbus_mmio_get_region(sysbus_dev, 0));
>  
>  /* IDE buses */
>  for (i = 0; i < ARRAY_SIZE(ns->ide); i++) {
> @@ -311,6 +311,11 @@ static void macio_newworld_init(Object *obj)
>  
>  qdev_init_gpio_out(DEVICE(obj), ns->irqs, ARRAY_SIZE(ns->irqs));
>  
> +object_property_add_link(obj, "pic", TYPE_OPENPIC,
> + (Object **) >pic,
> + qdev_prop_allow_set_link_before_realize,
> + 0, NULL);
> +
>  for (i = 0; i < 2; i++) {
>  macio_init_ide(s, >ide[i], sizeof(ns->ide[i]), i);
>  }
> @@ -441,7 +446,6 @@ void macio_init(PCIDevice *d,
>  {
>  MacIOState *macio_state = MACIO(d);
>  
> -macio_state->pic_mem = pic_mem;
>  /* Note: this code is strongly inspirated from the corresponding code
> in PearPC */
>  qdev_prop_set_uint64(DEVICE(_state->cuda), "timebase-frequency",
> diff --git a/hw/ppc/mac_newworld.c b/hw/ppc/mac_newworld.c
> index 396216954e..c7960ab67a 100644
> --- a/hw/ppc/mac_newworld.c
> +++ b/hw/ppc/mac_newworld.c
> @@ -154,7 +154,7 @@ static void ppc_core99_init(MachineState *machine)
>  hwaddr kernel_base, initrd_base, cmdline_base = 0;
>  long kernel_size, initrd_size;
>  PCIBus *pci_bus;
> -PCIDevice *macio;
> +NewWorldMacIOState *macio;
>  MACIOIDEState *macio_ide;
>  BusState *adb_bus;
>  MacIONVRAMState *nvr;
> @@ -166,7 +166,7 @@ static void ppc_core99_init(MachineState *machine)
>  void *fw_cfg;
>  int machine_arch;
>  SysBusDevice *s;
> -DeviceState *dev;
> +DeviceState *dev, *pic_dev;
>  int *token = g_new(int, 1);
>  hwaddr nvram_addr = 0xFFF04000;
>  uint64_t tbfreq;
> @@ -333,10 +333,10 @@ static void ppc_core99_init(MachineState *machine)
>  
>  pic = g_new0(qemu_irq, 64);
>  
> -dev = qdev_create(NULL, TYPE_OPENPIC);
> -qdev_prop_set_uint32(dev, "model", OPENPIC_MODEL_KEYLARGO);
> -qdev_init_nofail(dev);
> -s = SYS_BUS_DEVICE(dev);
> +pic_dev = qdev_create(NULL, TYPE_OPENPIC);
> +qdev_prop_set_uint32(pic_dev, "model", OPENPIC_MODEL_KEYLARGO);
> +qdev_init_nofail(pic_dev);
> +s = SYS_BUS_DEVICE(pic_dev);
>  pic_mem = s->mmio[0].memory;
>  k = 0;
>  for (i = 0; i < smp_cpus; i++) {
> @@ -346,7 +346,7 @@ static void ppc_core99_init(MachineState *machine)
>  }
>  
>  for (i = 0; i < 64; i++) {
> -pic[i] = qdev_get_gpio_in(dev, i);
> +pic[i] = qdev_get_gpio_in(pic_dev, i);
>  }
>  
>  if (PPC_INPUT(env) == PPC_FLAGS_INPUT_970) {
> @@ -369,7 +369,7 @@ static void ppc_core99_init(MachineState *machine)
>  }
>  
>  /* MacIO */
> -macio = pci_create(pci_bus, -1, TYPE_NEWWORLD_MACIO);
> +macio = NEWWORLD_MACIO(pci_create(pci_bus, -1, TYPE_NEWWORLD_MACIO));
>  dev = DEVICE(macio);
>  qdev_connect_gpio_out(dev, 0, pic[0x19]); /* CUDA */
>  qdev_connect_gpio_out(dev, 1, pic[0x24]); /* ESCC-B */
> @@ -379,7 +379,9 @@ static void ppc_core99_init(MachineState *machine)
>  qdev_connect_gpio_out(dev, 5, pic[0x0e]); /* IDE */
>  qdev_connect_gpio_out(dev, 6, pic[0x03]); /* IDE DMA */
>  qdev_prop_set_uint64(dev, "frequency", tbfreq);
> -macio_init(macio, pic_mem);
> +object_property_set_link(OBJECT(macio), OBJECT(pic_dev), "pic",
> + _abort);
> +macio_init(PCI_DEVICE(macio), pic_mem);
>  
>  /* We only emulate 2 out of 3 IDE controllers for now */
>  ide_drive_get(hd, ARRAY_SIZE(hd));
> diff --git a/include/hw/misc/macio/macio.h b/include/hw/misc/macio/macio.h
> index 843c114c07..4528282b36 100644
> --- a/include/hw/misc/macio/macio.h
> +++

[Qemu-devel] [PATCH V4 3/3] tests: Add migration test for aarch64

2018-02-21 Thread Wei Huang

This patch adds migration test support for aarch64. The test code, which
implements the same functionality as x86, is booted as a kernel in qemu.
Here are the design choices we make for aarch64:

 * We choose this -kernel approach because aarch64 QEMU doesn't provide a
   built-in fw like x86 does. So instead of relying on a boot loader, we
   use -kernel approach for aarch64.
 * The serial output is sent to PL011 directly.
 * The physical memory base for mach-virt machine is 0x4000. We change
   the start_address and end_address for aarch64.

In addition to providing the binary, this patch also includes the source
code and the build script in tests/migration/. So users can change the
source and/or re-compile the binary as they wish.

Signed-off-by: Wei Huang 
---
 tests/Makefile.include   |  1 +
 tests/migration-test.c   | 47 +---
 tests/migration/Makefile | 12 +-
 tests/migration/aarch64-a-b-kernel.S | 71 
 tests/migration/aarch64-a-b-kernel.h | 19 ++
 tests/migration/migration-test.h |  5 +++
 6 files changed, 147 insertions(+), 8 deletions(-)
 create mode 100644 tests/migration/aarch64-a-b-kernel.S
 create mode 100644 tests/migration/aarch64-a-b-kernel.h

diff --git a/tests/Makefile.include b/tests/Makefile.include
index a1bcbffe12..df9f64438f 100644
--- a/tests/Makefile.include
+++ b/tests/Makefile.include
@@ -372,6 +372,7 @@ check-qtest-arm-y += tests/sdhci-test$(EXESUF)
 check-qtest-aarch64-y = tests/numa-test$(EXESUF)
 check-qtest-aarch64-y += tests/sdhci-test$(EXESUF)
 check-qtest-aarch64-y += tests/boot-serial-test$(EXESUF)
+check-qtest-aarch64-y += tests/migration-test$(EXESUF)
 
 check-qtest-microblazeel-y = $(check-qtest-microblaze-y)
 
diff --git a/tests/migration-test.c b/tests/migration-test.c
index e2e06ed337..a4f6732a59 100644
--- a/tests/migration-test.c
+++ b/tests/migration-test.c
@@ -11,6 +11,7 @@
  */
 
 #include "qemu/osdep.h"
+#include 
 
 #include "libqtest.h"
 #include "qapi/qmp/qdict.h"
@@ -23,8 +24,8 @@
 
 #include "migration/migration-test.h"
 
-const unsigned start_address = TEST_MEM_START;
-const unsigned end_address = TEST_MEM_END;
+unsigned start_address = TEST_MEM_START;
+unsigned end_address = TEST_MEM_END;
 bool got_stop;
 
 #if defined(__linux__)
@@ -81,12 +82,13 @@ static const char *tmpfs;
  * outputting a 'B' every so often if it's still running.
  */
 #include "tests/migration/x86-a-b-bootblock.h"
+#include "tests/migration/aarch64-a-b-kernel.h"
 
-static void init_bootfile_x86(const char *bootpath)
+static void init_bootfile(const char *bootpath, void *content)
 {
 FILE *bootfile = fopen(bootpath, "wb");
 
-g_assert_cmpint(fwrite(x86_bootsect, 512, 1, bootfile), ==, 1);
+g_assert_cmpint(fwrite(content, 512, 1, bootfile), ==, 1);
 fclose(bootfile);
 }
 
@@ -393,7 +395,7 @@ static void test_migrate_start(QTestState **from, 
QTestState **to,
 got_stop = false;
 
 if (strcmp(arch, "i386") == 0 || strcmp(arch, "x86_64") == 0) {
-init_bootfile_x86(bootpath);
+init_bootfile(bootpath, x86_bootsect);
 cmd_src = g_strdup_printf("-machine accel=%s -m 150M"
   " -name source,debug-threads=on"
   " -serial file:%s/src_serial"
@@ -422,6 +424,39 @@ static void test_migrate_start(QTestState **from, 
QTestState **to,
   " -serial file:%s/dest_serial"
   " -incoming %s",
   accel, tmpfs, uri);
+} else if (strcmp(arch, "aarch64") == 0) {
+const char *cpu;
+const char *gic_ver;
+struct utsname utsname;
+
+/* kvm and tcg need different cpu and gic-version configs */
+if (access("/dev/kvm", F_OK) == 0 && uname() == 0 &&
+strcmp(utsname.machine, "aarch64") == 0) {
+accel = "kvm";
+cpu = "host";
+gic_ver = "host";
+} else {
+accel = "tcg";
+cpu = "cortex-a57";
+gic_ver = "2";
+}
+
+init_bootfile(bootpath, aarch64_kernel);
+cmd_src = g_strdup_printf("-machine virt,accel=%s,gic-version=%s "
+  "-name vmsource,debug-threads=on -cpu %s "
+  "-m 150M -serial file:%s/src_serial "
+  "-kernel %s ",
+  accel, gic_ver, cpu, tmpfs, bootpath);
+cmd_dst = g_strdup_printf("-machine virt,accel=%s,gic-version=%s "
+  "-name vmdest,debug-threads=on -cpu %s "
+  "-m 150M -serial file:%s/dest_serial "
+  "-kernel %s "
+  "-incoming %s ",
+  accel, gic_ver, cpu, tmpfs, bootpath, uri);
+
+/* aarch64 virt machine physical

Re: [Qemu-devel] [qemu-s390x] [PATCH v8 06/13] s390-ccw: parse and set boot menu options

On 21.02.2018 20:35, Collin L. Walling wrote:
> Set boot menu options for an s390 guest and store them in
> the iplb. These options are set via the QEMU command line
> option:
> 
> -boot menu=on|off[,splash-time=X]
> 
> or via the libvirt domain xml:
> 
> 
>   
> 
> 
> Where X represents some positive integer representing
> milliseconds.
> 
> Any value set for loadparm will override all boot menu options.
> If loadparm=PROMPT, then the menu will be enabled without a
> timeout.
> 
> Signed-off-by: Collin L. Walling 
> Reviewed-by: Janosch Frank 
> ---
>  hw/s390x/ipl.c  | 44 
>  hw/s390x/ipl.h  |  9 +++--
>  pc-bios/s390-ccw/iplb.h |  9 +++--
>  3 files changed, 58 insertions(+), 4 deletions(-)

Reviewed-by: Thomas Huth

Re: [Qemu-devel] [RFC PATCH v0 1/2] pc-dimm: Make pc_dimm_built_list() global

2018-02-21 Thread Bharata B Rao

On Tue, Feb 20, 2018 at 03:35:10PM +0100, Igor Mammedov wrote:
> On Mon, 19 Feb 2018 12:12:53 +0530
> Bharata B Rao  wrote:
> 
> > Making pc_dimm_built_list() global allows other parts of QEMU code
> > to build and walk through the DIMM list in address-sorted order.
> > 
> > This is needed in the next patch for sPAPR code to create
> > ibm,dynamic-memory-v2 device tree property that will have entries
> > for populated DIMMs as well as available hotpluggable areas.
> > 
> > CHECK: List of DIMMs is already available via qmp_pc_dimm_device_list(),
> maybe make it sorted first and use it?
> 
> (i.e. use pc_dimm_built_list in qmp_pc_dimm_device_list) and hide
> recursive callback ugliness from external users.
> 
> MemoryDeviceInfoList *qmp_pc_dimm_device_list(void) {
> object_child_foreach(qdev_get_machine(), pc_dimm_built_list, );
> ...
> }

Thanks will attempt this in my next version.

Regards,
Bharata.

Re: [Qemu-devel] [qemu-s390x] [PATCH v8 13/13] s390-ccw: interactive boot menu for scsi

On 21.02.2018 20:35, Collin L. Walling wrote:
> Interactive boot menu for scsi. This follows a similar procedure
> as the interactive menu for eckd dasd. An example follows:
> 
> s390x Enumerated Boot Menu.
> 
> 3 entries detected. Select from index 0 to 2.
> 
> Signed-off-by: Collin L. Walling 
> Reviewed-by: Thomas Huth 
> ---
>  hw/s390x/ipl.c  |  1 +
>  pc-bios/s390-ccw/bootmap.c  |  4 
>  pc-bios/s390-ccw/main.c |  1 +
>  pc-bios/s390-ccw/menu.c | 14 ++
>  pc-bios/s390-ccw/s390-ccw.h |  1 +
>  5 files changed, 21 insertions(+)
> 
> diff --git a/hw/s390x/ipl.c b/hw/s390x/ipl.c
> index a0f4f40..566248e 100644
> --- a/hw/s390x/ipl.c
> +++ b/hw/s390x/ipl.c
> @@ -238,6 +238,7 @@ static void s390_ipl_set_boot_menu(S390IPLState *ipl)
>  *flags |= QIPL_FLAG_BM_OPTS_ZIPL;
>  return;
>  }
> +case S390_IPL_TYPE_QEMU_SCSI:
>  break;

It's not a real bug, but I'm pretty sure this will cause Coverity to
report an issue of unintended switch-case fall through. Could you please
add a break before the new case label?
(Since all other patches look fine to me, this could maybe also be fixed
when the patches are picked up, so no need to respin the whole patch
series just because of this)

 Thomas

[Qemu-devel] [PATCH v3 3/5] keymap: numpad keysyms and keycodes are fixed

No need to figure them at runtime from the keymap.

Signed-off-by: Gerd Hoffmann 
Reviewed-by: Daniel P. Berrangé 
---
 ui/keymaps.c | 61 +---
 1 file changed, 9 insertions(+), 52 deletions(-)

diff --git a/ui/keymaps.c b/ui/keymaps.c
index 449c3dec22..cbdd65c767 100644
--- a/ui/keymaps.c
+++ b/ui/keymaps.c
@@ -28,20 +28,12 @@
 #include "trace.h"
 #include "qemu/error-report.h"
 
-struct key_range {
-int start;
-int end;
-struct key_range *next;
-};
-
 struct keysym2code {
 uint16_t keycode;
 };
 
 struct kbd_layout_t {
 GHashTable *hash;
-struct key_range *keypad_range;
-struct key_range *numlock_range;
 };
 
 static int get_keysym(const name2keysym_t *table,
@@ -64,29 +56,6 @@ static int get_keysym(const name2keysym_t *table,
 }
 
 
-static void add_to_key_range(struct key_range **krp, int code) {
-struct key_range *kr;
-for (kr = *krp; kr; kr = kr->next) {
-if (code >= kr->start && code <= kr->end) {
-break;
-}
-if (code == kr->start - 1) {
-kr->start--;
-break;
-}
-if (code == kr->end + 1) {
-kr->end++;
-break;
-}
-}
-if (kr == NULL) {
-kr = g_malloc0(sizeof(*kr));
-kr->start = kr->end = code;
-kr->next = *krp;
-*krp = kr;
-}
-}
-
 static void add_keysym(char *line, int keysym, int keycode, kbd_layout_t *k)
 {
 struct keysym2code *keysym2code;
@@ -160,13 +129,6 @@ static kbd_layout_t *parse_keyboard_layout(const 
name2keysym_t *table,
 const char *rest = line + offset + 1;
 int keycode = strtol(rest, NULL, 0);
 
-if (strstr(rest, "numlock")) {
-add_to_key_range(>keypad_range, keycode);
-add_to_key_range(>numlock_range, keysym);
-/* fprintf(stderr, "keypad keysym %04x keycode %d\n",
-   keysym, keycode); */
-}
-
 if (strstr(rest, "shift")) {
 keycode |= SCANCODE_SHIFT;
 }
@@ -228,24 +190,19 @@ int keysym2scancode(kbd_layout_t *k, int keysym)
 
 int keycode_is_keypad(kbd_layout_t *k, int keycode)
 {
-struct key_range *kr;
-
-for (kr = k->keypad_range; kr; kr = kr->next) {
-if (keycode >= kr->start && keycode <= kr->end) {
-return 1;
-}
+if (keycode >= 0x47 && keycode <= 0x53) {
+return true;
 }
-return 0;
+return false;
 }
 
 int keysym_is_numlock(kbd_layout_t *k, int keysym)
 {
-struct key_range *kr;
-
-for (kr = k->numlock_range; kr; kr = kr->next) {
-if (keysym >= kr->start && keysym <= kr->end) {
-return 1;
-}
+switch (keysym) {
+case 0xffb0 ... 0xffb9:  /* KP_0 .. KP_9 */
+case 0xffac: /* KP_Separator */
+case 0xffae: /* KP_Decimal   */
+return true;
 }
-return 0;
+return false;
 }
-- 
2.9.3

[Qemu-devel] [PATCH v3 5/5] keymap: consider modifier state when picking a mapping

Pass the modifier state to the keymap lookup function.  In case multiple
keysym -> keycode mappings exist look at the modifier state and prefer
the mapping where the modifier state matches.

Signed-off-by: Gerd Hoffmann 
Reviewed-by: Daniel P. Berrangé 
---
 ui/keymaps.h |  3 ++-
 ui/curses.c  |  3 ++-
 ui/keymaps.c | 33 -
 ui/sdl.c |  6 +-
 ui/vnc.c |  9 +++--
 5 files changed, 48 insertions(+), 6 deletions(-)

diff --git a/ui/keymaps.h b/ui/keymaps.h
index 17ec03387a..0693588225 100644
--- a/ui/keymaps.h
+++ b/ui/keymaps.h
@@ -54,7 +54,8 @@ typedef struct kbd_layout_t kbd_layout_t;
 
 kbd_layout_t *init_keyboard_layout(const name2keysym_t *table,
const char *language);
-int keysym2scancode(kbd_layout_t *k, int keysym);
+int keysym2scancode(kbd_layout_t *k, int keysym,
+bool shift, bool altgr, bool ctrl);
 int keycode_is_keypad(kbd_layout_t *k, int keycode);
 int keysym_is_numlock(kbd_layout_t *k, int keysym);
 
diff --git a/ui/curses.c b/ui/curses.c
index 479b77bd03..597e47fd4a 100644
--- a/ui/curses.c
+++ b/ui/curses.c
@@ -271,7 +271,8 @@ static void curses_refresh(DisplayChangeListener *dcl)
 keysym = chr;
 }
 
-keycode = keysym2scancode(kbd_layout, keysym & KEYSYM_MASK);
+keycode = keysym2scancode(kbd_layout, keysym & KEYSYM_MASK,
+  false, false, false);
 if (keycode == 0)
 continue;
 
diff --git a/ui/keymaps.c b/ui/keymaps.c
index 1eba6d7057..43fe604724 100644
--- a/ui/keymaps.c
+++ b/ui/keymaps.c
@@ -176,8 +176,12 @@ kbd_layout_t *init_keyboard_layout(const name2keysym_t 
*table,
 }
 
 
-int keysym2scancode(kbd_layout_t *k, int keysym)
+int keysym2scancode(kbd_layout_t *k, int keysym,
+bool shift, bool altgr, bool ctrl)
 {
+static const uint32_t mask =
+SCANCODE_SHIFT | SCANCODE_ALTGR | SCANCODE_CTRL;
+uint32_t mods, i;
 struct keysym2code *keysym2code;
 
 #ifdef XK_ISO_Left_Tab
@@ -193,6 +197,33 @@ int keysym2scancode(kbd_layout_t *k, int keysym)
 return 0;
 }
 
+if (keysym2code->count == 1) {
+return keysym2code->keycodes[0];
+}
+
+/*
+ * We have multiple keysym -> keycode mappings.
+ *
+ * Check whenever we find one mapping where the modifier state of
+ * the mapping matches the current user interface modifier state.
+ * If so, prefer that one.
+ */
+mods = 0;
+if (shift) {
+mods |= SCANCODE_SHIFT;
+}
+if (altgr) {
+mods |= SCANCODE_ALTGR;
+}
+if (ctrl) {
+mods |= SCANCODE_CTRL;
+}
+
+for (i = 0; i < keysym2code->count; i++) {
+if ((keysym2code->keycodes[i] & mask) == mods) {
+return keysym2code->keycodes[i];
+}
+}
 return keysym2code->keycodes[0];
 }
 
diff --git a/ui/sdl.c b/ui/sdl.c
index 963cdf77a7..c4ae7ab05d 100644
--- a/ui/sdl.c
+++ b/ui/sdl.c
@@ -201,6 +201,9 @@ static kbd_layout_t *kbd_layout = NULL;
 
 static uint8_t sdl_keyevent_to_keycode_generic(const SDL_KeyboardEvent *ev)
 {
+bool shift = modifiers_state[0x2a] || modifiers_state[0x36];
+bool altgr = modifiers_state[0xb8];
+bool ctrl  = modifiers_state[0x1d] || modifiers_state[0x9d];
 int keysym;
 /* workaround for X11+SDL bug with AltGR */
 keysym = ev->keysym.sym;
@@ -210,7 +213,8 @@ static uint8_t sdl_keyevent_to_keycode_generic(const 
SDL_KeyboardEvent *ev)
 if (keysym == 92 && ev->keysym.scancode == 133) {
 keysym = 0xa5;
 }
-return keysym2scancode(kbd_layout, keysym) & SCANCODE_KEYMASK;
+return keysym2scancode(kbd_layout, keysym,
+   shift, altgr, ctrl) & SCANCODE_KEYMASK;
 }
 
 
diff --git a/ui/vnc.c b/ui/vnc.c
index a77b568b57..d19f86c7f4 100644
--- a/ui/vnc.c
+++ b/ui/vnc.c
@@ -1734,7 +1734,8 @@ static void reset_keys(VncState *vs)
 
 static void press_key(VncState *vs, int keysym)
 {
-int keycode = keysym2scancode(vs->vd->kbd_layout, keysym) & 
SCANCODE_KEYMASK;
+int keycode = keysym2scancode(vs->vd->kbd_layout, keysym,
+  false, false, false) & SCANCODE_KEYMASK;
 qemu_input_event_send_key_number(vs->vd->dcl.con, keycode, true);
 qemu_input_event_send_key_delay(vs->vd->key_delay_ms);
 qemu_input_event_send_key_number(vs->vd->dcl.con, keycode, false);
@@ -1993,6 +1994,9 @@ static const char *code2name(int keycode)
 
 static void key_event(VncState *vs, int down, uint32_t sym)
 {
+bool shift = vs->modifiers_state[0x2a] || vs->modifiers_state[0x36];
+bool altgr = vs->modifiers_state[0xb8];
+bool ctrl  = vs->modifiers_state[0x1d] || vs->modifiers_state[0x9d];
 int keycode;
 int lsym = sym;
 
@@ -2000,7 +2004,8 @@ static void key_event(VncState *vs, int down, uint32_t 
sym)
 lsym = lsym - 'A' + 'a';
 }
 
-keycode =

[Qemu-devel] [PATCH v3 0/5] keymap: support kbd layouts with multiple mappings for the same key.

The reverse keymap (-k $map) code has problems dealing with keymaps
where multiple key combinations can create the same keysym, because
it can store a single keycode per keysym only.  This series fixes it
and does some cleanups along the way.

v2: rebase, codestyle fixes.
v3: use GINT_TO_POINTER

Gerd Hoffmann (5):
  keymap: make struct kbd_layout_t private to ui/keymaps.c
  keymap: use glib hash for kbd_layout_t
  keymap: numpad keysyms and keycodes are fixed
  keymap: record multiple keysym -> keycode mappings
  keymap: consider modifier state when picking a mapping

 ui/keymaps.h|  30 +++
 ui/curses.c |   3 +-
 ui/keymaps.c| 163 +---
 ui/sdl.c|   6 ++-
 ui/vnc.c|   9 +++-
 ui/trace-events |   2 +-
 6 files changed, 105 insertions(+), 108 deletions(-)

-- 
2.9.3

Re: [Qemu-devel] [PATCH 0/2] Firmware blob and git submodule for Sam460ex

2018-02-21 Thread David Gibson

On Wed, Feb 21, 2018 at 06:33:42PM +, Peter Maydell wrote:
> On 21 February 2018 at 17:06, BALATON Zoltan  wrote:
> > It's not that upstream u-boot has abandoned board support (it only removed
> > support for the PPC440 CPU it once had). The board itself never had support
> > in upstream u-boot, it only exists in vendor's fork which is the reason we
> > need a separate source and cannot use upstream u-boot source we already
> > have.
> >
> > In my opinion we don't aim to take on support for this board in u-boot, we
> > only need to include the firmware binary for the emulation to be useful
> > which then requires us to also include the source for the GPL it's licensed
> > under. I've also found a few bugs in the firmware which I've fixed but apart
> > from such occasional bug fixes when needed I don't expect to take over
> > support for the board from the hardware vendor so this source is only so we
> > can include the firmware binary which is needed for the board emulation.
> > Does this answer your concerns?
> 
> We have lots of boards we don't ship firmware blobs for and
> which we expect the users to provide the guest code for
> if they're going to use them. If we had a git submodule
> for every random dev board model that needs some hardware
> vendor's BSP and bootloader we'd probably have 50 submodules...
> 
> Which isn't to say I'm definitely against this -- I'm just
> trying to figure out where we should draw the line of
> "these bits of guest code we build for you and ship with
> QEMU" versus "we provide the model of the hardware for you
> to run whatever guest code you happen to have".

So, I encouraged Zoltan to attempt this because I thought boards
without suitable firmware blobs were the exception.  If they're common
it's probably fine as is.

-- 
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] [qemu-s390x] [PATCH v8 05/13] s390-ccw: move auxiliary IPL data to separate location

On 21.02.2018 20:35, Collin L. Walling wrote:
> The s390-ccw firmware needs some information in support of the
> boot process which is not available on the native machine.
> Examples are the netboot firmware load address and now the
> boot menu parameters.
> 
> While storing that data in unused fields of the IPL parameter block
> works, that approach could create problems if the parameter block
> definition should change in the future. Because then a guest could
> overwrite these fields using the set IPLB diagnose.
> 
> In fact the data in question is of more global nature and not really
> tied to an IPL device, so separating it is rather logical.
> 
> This commit introduces a new structure to hold firmware relevant
> IPL parameters set by QEMU. The data is stored at location 204 (dec)
> and can contain up to 7 32-bit words. This area is available to
> programming in the z/Architecture Principles of Operation and
> can thus safely be used by the firmware until the IPL has completed.
> 
> Signed-off-by: Viktor Mihajlovski 
> Signed-off-by: Collin L. Walling 
> ---
>  hw/s390x/ipl.c  | 18 +-
>  hw/s390x/ipl.h  | 25 +++--
>  pc-bios/s390-ccw/iplb.h | 18 --
>  pc-bios/s390-ccw/main.c |  6 +-
>  4 files changed, 61 insertions(+), 6 deletions(-)
> 
> diff --git a/hw/s390x/ipl.c b/hw/s390x/ipl.c
> index 0d06fc1..79f5a58 100644
> --- a/hw/s390x/ipl.c
> +++ b/hw/s390x/ipl.c
> @@ -399,6 +399,21 @@ void s390_reipl_request(void)
>  qemu_system_reset_request(SHUTDOWN_CAUSE_GUEST_RESET);
>  }
>  
> +static void s390_ipl_prepare_qipl(S390CPU *cpu)
> +{
> +S390IPLState *ipl = get_ipl_device();
> +uint8_t *addr;
> +uint64_t len = 4096;
> +
> +addr = cpu_physical_memory_map(cpu->env.psa, , 1);
> +if (!addr || len < QIPL_ADDRESS + sizeof(QemuIplParameters)) {
> +error_report("Cannot set QEMU IPL parameters");
> +return;
> +}
> +memcpy(addr + QIPL_ADDRESS, >qipl, sizeof(QemuIplParameters));
> +cpu_physical_memory_unmap(addr, len, 1, len);
> +}
> +
>  void s390_ipl_prepare_cpu(S390CPU *cpu)
>  {
>  S390IPLState *ipl = get_ipl_device();
> @@ -418,8 +433,9 @@ void s390_ipl_prepare_cpu(S390CPU *cpu)
>  error_report_err(err);
>  vm_stop(RUN_STATE_INTERNAL_ERROR);
>  }
> -ipl->iplb.ccw.netboot_start_addr = cpu_to_be64(ipl->start_addr);
> +ipl->qipl.netboot_start_addr = cpu_to_be64(ipl->start_addr);
>  }
> +s390_ipl_prepare_qipl(cpu);
>  }
>  
>  static void s390_ipl_reset(DeviceState *dev)
> diff --git a/hw/s390x/ipl.h b/hw/s390x/ipl.h
> index 8a705e0..08926a3 100644
> --- a/hw/s390x/ipl.h
> +++ b/hw/s390x/ipl.h
> @@ -16,8 +16,7 @@
>  #include "cpu.h"
>  
>  struct IplBlockCcw {
> -uint64_t netboot_start_addr;
> -uint8_t  reserved0[77];
> +uint8_t  reserved0[85];
>  uint8_t  ssid;
>  uint16_t devno;
>  uint8_t  vm_flags;
> @@ -90,6 +89,27 @@ void s390_ipl_prepare_cpu(S390CPU *cpu);
>  IplParameterBlock *s390_ipl_get_iplb(void);
>  void s390_reipl_request(void);
>  
> +#define QIPL_ADDRESS  0xcc
> +
> +/*
> + * The QEMU IPL Parameters will be stored at absolute address
> + * 204 (0xcc) which means it is 32-bit word aligned but not
> + * double-word aligned.
> + * Placement of data fields in this area must account for
> + * their alignment needs. E.g., netboot_start_address must
> + * have an offset of n * 8 bytes within the struct in order
> + * to keep it double-word aligned.

Should that rather be "4 + n * 8" instead of "n * 8" ?

Apart from that, patch looks good to me now, so once you've fixed the
comment (if necessary):

Reviewed-by: Thomas Huth

Re: [Qemu-devel] [PATCH] intel-iommu: Accept 64-bit writes to FEADDR

2018-02-21 Thread Peter Xu

On Sat, Feb 17, 2018 at 12:26:19PM +0100, Jan Kiszka wrote:
> From: Jan Kiszka 
> 
> Xen is doing this [1] and currently triggers an abort.
> 
> [1] 
> http://xenbits.xenproject.org/gitweb/?p=xen.git;a=blob;f=xen/drivers/passthrough/vtd/iommu.c;h=daaed0abbdd06b6ba3d948ea103aadf02651e83c;hb=refs/heads/master#l1108
> 
> Reported-by: Luis Lloret 
> Signed-off-by: Jan Kiszka 
> ---
>  hw/i386/intel_iommu.c | 7 ++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c
> index 2e841cde27..b61d0da270 100644
> --- a/hw/i386/intel_iommu.c
> +++ b/hw/i386/intel_iommu.c
> @@ -2129,7 +2129,12 @@ static void vtd_mem_write(void *opaque, hwaddr addr,
>  
>  /* Fault Event Address Register, 32-bit */
>  case DMAR_FEADDR_REG:
> -assert(size == 4);
> +/*
> + * While the register is 32-bit only, some guests (Xen...) write to 
> it
> + * with 64-bit. Ignore the upper part, that's likely what the 
> hardware
> + * does as well (plus the upper part is not used by our model 
> anyway).
> + */
> +assert(size >= 4);
>  vtd_set_long(s, addr, val);
>  break;

(Sorry for the late response due to Chinese Spring Festival)

I agree with the problem there, but do we still better provide a
conditional vtd_set_quad()?  Since from the spec 10.4.13 the upper 32
bits may still be used when x2apic (Extended Interrupt Mode) is
enabled?

Thanks,

-- 
Peter Xu

Re: [Qemu-devel] [PATCH v4 0/7] vfio: add display support

2018-02-21 Thread Zhenyu Wang

On 2018.02.20 18:04:17 +0100, Gerd Hoffmann wrote:
>   Hi,
> 
> > Sneak preview at https://www.kraxel.org/cgit/qemu/log/?h=work/intel-vgpu
> > Note: branch is a moving target ;)
> > 
> > State:
> >   spice: Partly working (no mouse ptr yet).
> 
> Working now, in case anyone wants play.
> 
> Must turn on opengl and use a unix socket, i.e. this:
> 
>   qemu -spice disable-ticketing,gl=on,unix,addr=/tmp/spice-vgpu
> 
> and this:
> 
>   remote-viewer spice+unix:tmp/spice-vgpu
> 
> Host needs a 4.16-rc kernel.
> 
> Guest can be older, most of the time I'm testing with the Fedora 27 live
> iso (4.13) which works ok.
> 

Nice! Seems to be the last missing gap for local spice with cursor
dmabuf support, we'll do more testing on that for sure. Btw, another
method might be to add direct cursor dmabuf passing for spice as gl
output, is that correct way?

And really like to see dmabuf support could be in upstream soon. Do
you have any predict for which qemu version?

Thanks!

-- 
Open Source Technology Center, Intel ltd.

$gpg --keyserver wwwkeys.pgp.net --recv-keys 4D781827

signature.asc
Description: PGP signature

Re: [Qemu-devel] [PATCH v4 4/5] usb-mtp: Introduce write support for MTP objects

2018-02-21 Thread Bandan Das

Daniel P. Berrangé  writes:

>>  #define TYPE_USB_MTP "usb-mtp"
>> @@ -804,6 +815,7 @@ static MTPData *usb_mtp_get_device_info(MTPState *s, 
>> MTPControl *c)
>>  CMD_GET_OBJECT_HANDLES,
>>  CMD_GET_OBJECT_INFO,
>>  CMD_DELETE_OBJECT,
>> +CMD_SEND_OBJECT,
>
> Seems we should not advertize this for readonly devices.

Advertising CMD_SEND_OBJECT shows that it's supported but the device is
read-only probably due to a server side setting. It differentiates between
Operation_Not_Supported and Store_Read_Only.

I agree, we should not rely on the initiator to check for this.
I will add a check for readonly when accepting DELETE, OBJECT_INFO
and return the READ_ONLY response code.

Bandan


>>  CMD_GET_OBJECT,
>>  CMD_GET_PARTIAL_OBJECT,
>>  CMD_GET_OBJECT_PROPS_SUPPORTED,
>> @@ -1378,6 +1390,14 @@ static void usb_mtp_command(MTPState *s, MTPControl 
>> *c)
>>  nres = 1;
>>  res0 = data_in->length;
>>  break;
>> +case CMD_SEND_OBJECT:
>> +if (!s->write_pending) {
>> +usb_mtp_queue_result(s, RES_INVALID_OBJECTINFO,
>> + c->trans, 0, 0, 0, 0);
>> +return;
>> +}
>> +s->data_out = usb_mtp_data_alloc(c);
>> +return;
>>  case CMD_GET_OBJECT_PROPS_SUPPORTED:
>>  if (c->argv[0] != FMT_UNDEFINED_OBJECT &&
>>  c->argv[0] != FMT_ASSOCIATION) {
>> @@ -1472,12 +1492,126 @@ static void usb_mtp_cancel_packet(USBDevice *dev, 
>> USBPacket *p)
>>  fprintf(stderr, "%s\n", __func__);
>>  }
>>  
>> +static void usb_mtp_write_data(MTPState *s)
>> +{
>> +MTPData *d = s->data_out;
>> +MTPObject *parent =
>> +usb_mtp_object_lookup(s, s->dataset.parent_handle);
>> +char *path = NULL;
>> +int rc = -1;
>> +mode_t mask = 0644;
>> +
>> +assert(d != NULL);
>> +
>
>
> Somewhere in here should surely be validating the "readonly" flag.
>
>> +if (parent == NULL || !s->write_pending) {
>> +usb_mtp_queue_result(s, RES_INVALID_OBJECTINFO, d->trans,
>> + 0, 0, 0, 0);
>> +return;
>> +}
>> +
>
> Regards,
> Daniel

Re: [Qemu-devel] [PATCH v7 13/23] monitor: let suspend/resume work even with QMPs

On Wed, Jan 24, 2018 at 01:39:47PM +0800, Peter Xu wrote:
> diff --git a/monitor.c b/monitor.c
> index 60bcf67b3a..76137ba2a4 100644
> --- a/monitor.c
> +++ b/monitor.c
> @@ -246,6 +246,21 @@ static inline bool monitor_is_qmp(const Monitor *mon)
>  return (mon->flags & MONITOR_USE_CONTROL);
>  }
>  
> +/**
> + * Whether @mon is using readline?  Note: not all HMP monitors can are
> + * using readline, e.g., gdbserver has non-interactive HMP monitor, so

s/can are using/use/

s/has non-interactive HMP monitor/has a non-interactive HMP monitor/

> @@ -3994,19 +4009,43 @@ static void monitor_command_cb(void *opaque, const 
> char *cmdline,
>  
>  int monitor_suspend(Monitor *mon)
>  {
> -if (!mon->rs)
> +if (monitor_is_hmp_non_interactive(mon)) {
>  return -ENOTTY;
> +}
> +
> +if (monitor_is_qmp(mon)) {
> +/*
> + * Kick iothread to make sure this takes effect.  It'll be
> + * evaluated again in prepare() of the watch object.
> + */
> +aio_notify(iothread_get_aio_context(mon_global.mon_iothread));

This must be done after incrementing suspend_cnt to avoid the race
condition between the iothread and our thread.


signature.asc
Description: PGP signature

Re: [Qemu-devel] [PATCH v1 3/3] s390x/sclp: extend SCLP event masks to 64 bits

2018-02-21 Thread Claudio Imbrenda

On Wed, 21 Feb 2018 16:34:59 +0100
Cornelia Huck  wrote:

> On Tue, 20 Feb 2018 19:45:02 +0100
> Claudio Imbrenda  wrote:
> 
> > Extend the SCLP event masks to 64 bits. This will make us future
> > proof against future extensions of the architecture.
> > 
> > Notice that using any of the new bits results in a state that
> > cannot be migrated to an older version.
> > 
> > Signed-off-by: Claudio Imbrenda 
> > ---
> >  hw/s390x/event-facility.c | 43
> > +--
> > include/hw/s390x/event-facility.h |  2 +- 2 files changed, 38
> > insertions(+), 7 deletions(-)
> > 
> > diff --git a/hw/s390x/event-facility.c b/hw/s390x/event-facility.c
> > index f6f28fd..e71302a 100644
> > --- a/hw/s390x/event-facility.c
> > +++ b/hw/s390x/event-facility.c
> > @@ -30,7 +30,10 @@ struct SCLPEventFacility {
> >  SysBusDevice parent_obj;
> >  SCLPEventsBus sbus;
> >  /* guest' receive mask */
> > -sccb_mask_t receive_mask;
> > +union {
> > +uint32_t receive_mask_compat32;
> > +sccb_mask_t receive_mask;
> > +};
> >  /*
> >   * when false, we keep the same broken, backwards compatible
> > behaviour as
> >   * before; when true, we implement the architecture correctly.
> > Needed for @@ -261,7 +264,7 @@ static void
> > read_event_data(SCLPEventFacility *ef, SCCB *sccb) case
> > SCLP_SELECTIVE_READ: copy_mask((uint8_t
> > *)_active_selection_mask, (uint8_t *)>mask,
> > sizeof(sclp_active_selection_mask), ef->mask_length);
> > -sclp_active_selection_mask =
> > be32_to_cpu(sclp_active_selection_mask);
> > +sclp_active_selection_mask =
> > be64_to_cpu(sclp_active_selection_mask);  
> 
> Would it make sense to introduce a wrapper that combines copy_mask()
> and the endianness conversion (just in case you want to extend this
> again in the future?

maybe? but then we'd need to change the scalars into bitmasks, and the
whole thing would have to be rewritten anyway.

> >  if (!sclp_cp_receive_mask ||
> >  (sclp_active_selection_mask & ~sclp_cp_receive_mask)) {
> >  sccb->h.response_code =
> > @@ -301,13 +304,13 @@ static void
> > write_event_mask(SCLPEventFacility *ef, SCCB *sccb) /* keep track
> > of the guest's capability masks */ copy_mask((uint8_t *)_mask,
> > WEM_CP_RECEIVE_MASK(we_mask, mask_length), sizeof(tmp_mask),
> > mask_length);
> > -ef->receive_mask = be32_to_cpu(tmp_mask);
> > +ef->receive_mask = be64_to_cpu(tmp_mask);
> >  
> >  /* return the SCLP's capability masks to the guest */
> > -tmp_mask = cpu_to_be32(get_host_receive_mask(ef));
> > +tmp_mask = cpu_to_be64(get_host_receive_mask(ef));
> >  copy_mask(WEM_RECEIVE_MASK(we_mask, mask_length), (uint8_t
> > *)_mask, mask_length, sizeof(tmp_mask));
> > -tmp_mask = cpu_to_be32(get_host_send_mask(ef));
> > +tmp_mask = cpu_to_be64(get_host_send_mask(ef));
> >  copy_mask(WEM_SEND_MASK(we_mask, mask_length), (uint8_t
> > *)_mask, mask_length, sizeof(tmp_mask));
> >  
> > @@ -368,6 +371,21 @@ static void command_handler(SCLPEventFacility
> > *ef, SCCB *sccb, uint64_t code) }
> >  }
> >  
> > +static bool vmstate_event_facility_mask64_needed(void *opaque)
> > +{
> > +SCLPEventFacility *ef = opaque;
> > +
> > +return (ef->receive_mask & 0x) != 0;
> > +}
> > +
> > +static int vmstate_event_facility_mask64_pre_load(void *opaque)
> > +{
> > +SCLPEventFacility *ef = opaque;
> > +
> > +ef->receive_mask &= ~0xULL;
> > +return 0;
> > +}
> > +
> >  static bool vmstate_event_facility_mask_length_needed(void *opaque)
> >  {
> >  SCLPEventFacility *ef = opaque;
> > @@ -383,6 +401,18 @@ static int
> > vmstate_event_facility_mask_length_pre_load(void *opaque) return 0;
> >  }
> >  
> > +static const VMStateDescription vmstate_event_facility_mask64 = {
> > +.name = "vmstate-event-facility/mask64",
> > +.version_id = 0,
> > +.minimum_version_id = 0,
> > +.needed = vmstate_event_facility_mask64_needed,
> > +.pre_load = vmstate_event_facility_mask64_pre_load,
> > +.fields = (VMStateField[]) {
> > +VMSTATE_UINT64(receive_mask, SCLPEventFacility),
> > +VMSTATE_END_OF_LIST()
> > + }
> > +};
> > +  
> 
> Are there plans for extending this beyond 64 bits? Would it make sense

I don't know. I'm not even aware of anything above 32 bits, but since we
are already using all of the first 32 bits, it's only matter of time I
guess :)

> to use the maximum possible size for the mask here, so you don't need
> to introduce yet another vmstate in the future? (If it's unlikely that

That's true, but it requires changing simple scalars into bitmasks.
Surely doable, but I wanted to touch as little as possible.

> the mask will ever move beyond 64 bit, that might be overkill, of
> course.)
> 
> >  static const VMStateDescription vmstate_event_facility_mask_length
> > = { .name =

Re: [Qemu-devel] [PATCH 2/2] qcow2: Avoid memory over-allocation on compressed images

2018-02-21 Thread Alberto Garcia

On Wed 21 Feb 2018 04:00:54 PM CET, Eric Blake wrote:

>> - Solution b: the width of the 'compressed cluster size' field is
>>(cluster_bits - 8), that's (cluster_size / 256) sectors.
>
> Not true.  It is (cluster_bits - 9) or (cluster_size / 512).

It's not, it's (cluster_bits - 8), the documentation is wrong, see the
patch that I sent earlier.

Here's qcow2_do_open():

s->csize_mask = (1 << (s->cluster_bits - 8)) - 1;

For a 64k cluster (16 bits) the width is 16 - 8 = 8 bits.

That's 2^8 = 256 sectors, and 256 * 512 = 128k, exactly two clusters.

Coming back to your patch, since we know where the compressed data
starts and we know that it's not going to be larger than cluster_size,
we can read [coffset, coffset + cluster_size) and skip the final bytes
of the last sector because we know that we don't need that data.

Or we can read as much as the size field indicates (up to twice the
cluster size) and let the decompression code handle the rest.

Berto

Re: [Qemu-devel] [PATCH v1 3/3] s390x/sclp: extend SCLP event masks to 64 bits

On Tue, 20 Feb 2018 19:45:02 +0100
Claudio Imbrenda  wrote:

> Extend the SCLP event masks to 64 bits. This will make us future proof
> against future extensions of the architecture.
> 
> Notice that using any of the new bits results in a state that cannot be
> migrated to an older version.
> 
> Signed-off-by: Claudio Imbrenda 
> ---
>  hw/s390x/event-facility.c | 43 
> +--
>  include/hw/s390x/event-facility.h |  2 +-
>  2 files changed, 38 insertions(+), 7 deletions(-)
> 
> diff --git a/hw/s390x/event-facility.c b/hw/s390x/event-facility.c
> index f6f28fd..e71302a 100644
> --- a/hw/s390x/event-facility.c
> +++ b/hw/s390x/event-facility.c
> @@ -30,7 +30,10 @@ struct SCLPEventFacility {
>  SysBusDevice parent_obj;
>  SCLPEventsBus sbus;
>  /* guest' receive mask */
> -sccb_mask_t receive_mask;
> +union {
> +uint32_t receive_mask_compat32;
> +sccb_mask_t receive_mask;
> +};
>  /*
>   * when false, we keep the same broken, backwards compatible behaviour as
>   * before; when true, we implement the architecture correctly. Needed for
> @@ -261,7 +264,7 @@ static void read_event_data(SCLPEventFacility *ef, SCCB 
> *sccb)
>  case SCLP_SELECTIVE_READ:
>  copy_mask((uint8_t *)_active_selection_mask, (uint8_t 
> *)>mask,
>sizeof(sclp_active_selection_mask), ef->mask_length);
> -sclp_active_selection_mask = be32_to_cpu(sclp_active_selection_mask);
> +sclp_active_selection_mask = be64_to_cpu(sclp_active_selection_mask);

Would it make sense to introduce a wrapper that combines copy_mask()
and the endianness conversion (just in case you want to extend this
again in the future?

>  if (!sclp_cp_receive_mask ||
>  (sclp_active_selection_mask & ~sclp_cp_receive_mask)) {
>  sccb->h.response_code =
> @@ -301,13 +304,13 @@ static void write_event_mask(SCLPEventFacility *ef, 
> SCCB *sccb)
>  /* keep track of the guest's capability masks */
>  copy_mask((uint8_t *)_mask, WEM_CP_RECEIVE_MASK(we_mask, 
> mask_length),
>sizeof(tmp_mask), mask_length);
> -ef->receive_mask = be32_to_cpu(tmp_mask);
> +ef->receive_mask = be64_to_cpu(tmp_mask);
>  
>  /* return the SCLP's capability masks to the guest */
> -tmp_mask = cpu_to_be32(get_host_receive_mask(ef));
> +tmp_mask = cpu_to_be64(get_host_receive_mask(ef));
>  copy_mask(WEM_RECEIVE_MASK(we_mask, mask_length), (uint8_t *)_mask,
>mask_length, sizeof(tmp_mask));
> -tmp_mask = cpu_to_be32(get_host_send_mask(ef));
> +tmp_mask = cpu_to_be64(get_host_send_mask(ef));
>  copy_mask(WEM_SEND_MASK(we_mask, mask_length), (uint8_t *)_mask,
>mask_length, sizeof(tmp_mask));
>  
> @@ -368,6 +371,21 @@ static void command_handler(SCLPEventFacility *ef, SCCB 
> *sccb, uint64_t code)
>  }
>  }
>  
> +static bool vmstate_event_facility_mask64_needed(void *opaque)
> +{
> +SCLPEventFacility *ef = opaque;
> +
> +return (ef->receive_mask & 0x) != 0;
> +}
> +
> +static int vmstate_event_facility_mask64_pre_load(void *opaque)
> +{
> +SCLPEventFacility *ef = opaque;
> +
> +ef->receive_mask &= ~0xULL;
> +return 0;
> +}
> +
>  static bool vmstate_event_facility_mask_length_needed(void *opaque)
>  {
>  SCLPEventFacility *ef = opaque;
> @@ -383,6 +401,18 @@ static int 
> vmstate_event_facility_mask_length_pre_load(void *opaque)
>  return 0;
>  }
>  
> +static const VMStateDescription vmstate_event_facility_mask64 = {
> +.name = "vmstate-event-facility/mask64",
> +.version_id = 0,
> +.minimum_version_id = 0,
> +.needed = vmstate_event_facility_mask64_needed,
> +.pre_load = vmstate_event_facility_mask64_pre_load,
> +.fields = (VMStateField[]) {
> +VMSTATE_UINT64(receive_mask, SCLPEventFacility),
> +VMSTATE_END_OF_LIST()
> + }
> +};
> +

Are there plans for extending this beyond 64 bits? Would it make sense
to use the maximum possible size for the mask here, so you don't need
to introduce yet another vmstate in the future? (If it's unlikely that
the mask will ever move beyond 64 bit, that might be overkill, of
course.)

>  static const VMStateDescription vmstate_event_facility_mask_length = {
>  .name = "vmstate-event-facility/mask_length",
>  .version_id = 0,
> @@ -401,10 +431,11 @@ static const VMStateDescription vmstate_event_facility 
> = {
>  .version_id = 0,
>  .minimum_version_id = 0,
>  .fields = (VMStateField[]) {
> -VMSTATE_UINT32(receive_mask, SCLPEventFacility),
> +VMSTATE_UINT32(receive_mask_compat32, SCLPEventFacility),
>  VMSTATE_END_OF_LIST()
>   },
>  .subsections = (const VMStateDescription * []) {
> +_event_facility_mask64,
>  _event_facility_mask_length,
>  NULL
>   }

So what happens for compat machines?

Re: [Qemu-devel] [PATCH 2/2] qcow2: Avoid memory over-allocation on compressed images


On 02/21/2018 09:00 AM, Eric Blake wrote:

On 02/21/2018 04:04 AM, Alberto Garcia wrote:

On Tue 20 Feb 2018 11:24:59 PM CET, Eric Blake wrote:

I was also preparing a patch to change this, but you arrived first :-)


So, it's time to cut back on the waste.  A compressed cluster
will NEVER occupy more than an uncompressed cluster



And here, csize can only get smaller than the length picked by 
nb_csectors.  Therefore, csize is GUARANTEED to be <= c->sector_size.




- Solution a: check that csize < s->cluster_size and return an error if
   it's not. However! although QEMU won't produce an image with a
   compressed cluster that is larger than the uncompressed one, the qcow2
   on-disk format in principle allows for that, so arguably we should
   accept it.


No, the qcow2 on-disk format does not have enough bits reserved for 
that.  It is impossible to store an inflated cluster, because you don't 
have enough bits to record it.


Okay, the spec is WRONG, compared to our code base.



That said, we MAY have a bug, more likely to be visible in 512-byte 
clusters but possible on any size.  While the 'number sectors' field IS 
sufficient for any compressed cluster starting at offset 0 in the 
cluster, we run into issues if the starting offset is later in the 
cluster.  That is, even though the compressed length of a 512-byte 
cluster is <= 512 (or we wouldn't compress it), if we start at offset 
510, we NEED to read the next cluster to get the rest of the compressed 
stream - but at 512-byte clusters, there are 0 bits reserved for 'number 
sectors'.  Per the above calculations with the example offset of 510, 
nb_csectors would be 1 (it can't be anything else for a 512-byte cluster 
image), and csize would then be 2 bytes, which is insufficient for 
reading back enough data to reconstruct the cluster.


In fact, here's a demonstration of a discrepancy, where qemu-img and 
John's qcheck tool [1] disagree about the validity of an image created 
by qemu (although it may just be that qcheck hasn't yet learned about 
compressed clusters):


[1]https://github.com/jnsnow/qcheck

$ f=12345678
$ f=$f$f$f$f # 32
$ f=$f$f$f$f # 128
$ f=$f$f$f$f # 512
$ f=$f$f$f$f # 2k
$ f=$f$f$f$f # 8k
$ f=$f$f$f$f # 32k
$ f=$f$f$f$f # 128k
$ printf "$f" > data
$ qemu-img convert -c -f raw -O qcow2 -o cluster_size=512 \
  data data.qcow2
$ qemu-img check data.qcow2
No errors were found on the image.
256/256 = 100.00% allocated, 100.00% fragmented, 100.00% compressed clusters
Image end offset: 18944
$ ./qcheck data.qcow2
...
== L2 Tables ==

== L2 cluster l1[0] : 0x0800 ==
Corrupt entries: 63; Non-zero entries: 64; Corrupt:Non-zero ratio: 0.984375
== L2 cluster l1[1] : 0x0e00 ==
Corrupt entries: 63; Non-zero entries: 64; Corrupt:Non-zero ratio: 0.984375
== L2 cluster l1[2] : 0x1400 ==
Corrupt entries: 63; Non-zero entries: 64; Corrupt:Non-zero ratio: 0.984375
== L2 cluster l1[3] : 0x1a00 ==
Corrupt entries: 63; Non-zero entries: 64; Corrupt:Non-zero ratio: 0.984375
L2 tables: Could not complete analysis, 257 problems found


== Reference Count Analysis ==

Refcount analysis: 00 vacant clusters
Refcount analysis: 04 leaked clusters
Refcount analysis: 00 ghost clusters
Refcount analysis: 04 miscounted clusters
Refcount analysis: 8 problems found


== Cluster Counts ==

Metadata: 0x1000
Data: 0x800
Leaked: 0x800
Vacant: 0x0
total: 0x2000
qcheck: 73 problems found




Not true.  It is (cluster_bits - 9) or (cluster_size / 512).  Remember, 
x = 62 - (cluster_bits - 8); for a 512-byte cluster, x = 61.  The 
'number sectors' field is then bits x+1 - 61 (but you can't have a 
bitfield occupying bit 62 upto 61; especially since bit 62 is the bit 
for compressed cluster).


So instead of blindly reading the spec, I'm now going to single-stepping 
through the 'qemu-img convert' command line above, to see what REALLY 
happens:


Line numbers from commit a6e0344fa0
$ gdb --args ./qemu-img convert -c -f raw -O qcow2 -o cluster_size=512 
data data.qcow2

...
(gdb) b qcow2_alloc_bytes
Breakpoint 1 at 0x57610: file block/qcow2-refcount.c, line 1052.
(gdb) r
Thread 1 "qemu-img" hit Breakpoint 1, qcow2_alloc_bytes (
bs=bs@entry=0x55d87f50, size=size@entry=15)
at block/qcow2-refcount.c:1052
1052{
(gdb)

So we are compressing 512 bytes down to 15 every time, which means that 
after 34 clusters compressed, we should be at offset 510.  Let's resume 
debugging:


(gdb) c 34
Will ignore next 33 crossings of breakpoint 1.  Continuing.
[Thread 0x7fffe3cfe700 (LWP 32229) exited]
[New Thread 0x7fffe3cfe700 (LWP 32300)]
[New Thread 0x7fffe25ed700 (LWP 32301)]

Thread 1 "qemu-img" hit Breakpoint 1, qcow2_alloc_bytes (
bs=bs@entry=0x55d87f50, size=size@entry=15)
at block/qcow2-refcount.c:1052
1052{
(gdb) n
1053BDRVQcow2State *s = bs->opaque;
(gdb)
1058BLKDBG_EVENT(bs->file, BLKDBG_CLUSTER_ALLOC_BYTES);
(gdb)
1059assert(size > 0 && size <= s->cluster_size);
(gdb) p

Re: [Qemu-devel] [PATCH v2 03/32] target/arm/cpu64: allow fp16 to be disabled

Peter Maydell  writes:

> On 8 February 2018 at 17:31, Alex Bennée  wrote:
>> While for CONFIG_USER_ONLY it is policy for the "cpu" to be the most
>> capable is can be this does cause problems. For example legacy RISU
>> runs would fail as there are a bunch of implemented instructions which
>> would have caused failures that now trigger actual calculations.
>>
>> Signed-off-by: Alex Bennée 
>
>>  static void aarch64_cpu_initfn(Object *obj)
>>  {
>>  object_property_add_bool(obj, "aarch64", aarch64_cpu_get_aarch64,
>> @@ -283,6 +303,13 @@ static void aarch64_cpu_initfn(Object *obj)
>>  "Set on/off to enable/disable aarch64 "
>>  "execution state ",
>>  NULL);
>> +#ifdef CONFIG_USER_ONLY
>> +object_property_add_bool(obj, "fp16", aarch64_cpu_get_fp16,
>> + aarch64_cpu_set_fp16, NULL);
>> +object_property_set_description(obj, "fp16",
>> +"Set on/off to enable/disable FP16 
>> extensions ",
>> +NULL);
>> +#endif
>>  }
>
> Good news everybody -- this is an opportunity for a naming bikeshed
> discussion!

Everyone's favourite kind of discussion ;-)

> The property names we use here are effectively ABI because they'll
> be available to the user on the command line, so we want to get the
> right names. This is the first of these, but we can reasonably
> assume we'll have more feature switches in the future for various
> other optional instruction set extensions.
>
> There are two obvious choices here:
>  * use the architecture extension names from the Arm ARM A1.7.4
>(in this case that's "ARMv8.2-FP16", which we could reasonably
>abbreviate to fp16)

So since I last tested this stuff I noticed upstream broke my RISU
testing with the addition of the crypto instructions. The reason being
the RISU test set does exercise UNDEF's which get used in later revs.

However I realised I could use -cpu cortex-a57 to achieve the same thing
and avoid enabling features for later specs. Maybe it would be simpler
just to add cpu types for the baseline architecture profiles?

  -cpu armv8.0
  -cpu armv8.1
  -cpu armv8.2

Defaulting of course to the most capable CPU type for linux-user.

That said FP16 is an optional feature so it is perfectly legitimate to
have:

  -cpu armv8.2+fp16

In fact the manual goes further in allowing any v8.x+1 feature to be
snarfed into a v8.x confirming CPU.

That said *my* use case is turning features off, maybe that is enough to
expose a plain v8.0 on the command line for now until someone comes up
with a useful for case for building these franken-CPUs?

>  * use the hwcaps names that Linux defines and prints in /proc/cpuinfo
>(in this case that would be a combination of "fphp" and "asimdhp",
>since hwcaps reflects the ID register setup that allows a CPU
>to report support for one and not the other)

In naming I favour the Arm ARM over whatever Linux-ism /proc came up
with.

>
> Whatever we do, we should have a comment describing our naming
> conventions, so we can follow it next time we add one of these...
>
> thanks
> -- PMM

--
Alex Bennée

Re: [Qemu-devel] [PATCH v3] specs/qcow2: Fix documentation of the compressed cluster descriptor

2018-02-21 Thread Alberto Garcia

On Wed 21 Feb 2018 05:10:30 PM CET, Eric Blake wrote:
>>   Compressed Clusters Descriptor (x = 62 - (cluster_bits - 8)):
>>   
>> -Bit  0 -  x:Host cluster offset. This is usually _not_ aligned to a
>> -cluster boundary!
>> +Bit  0 - x-1:   Host cluster offset. This is usually _not_ aligned to a
>> +cluster or sector boundary!
>
>
> This breaks the nice alignment of the ':' character that the rest of the 
> section preserves.  You could also have done:
>
> Compressed Clusters Descriptor (x = 61 - (cluster_bits - 8)):
> Bit  0 -  x:  ...
>x+1 - 61:  ...
>
> to preserve the alignment, by shifting the adjustment into the 
> calculation of x.  But I'm fine with either approach.

Yeah I actually saw the same problem, in the end I chose this one
because the numbers match the implementation:

s->csize_shift = (62 - (s->cluster_bits - 8));

Thanks for reviewing!

Berto

Re: [Qemu-devel] [PATCH v1 2/3] s390x/sclp: clean up sclp masks

2018-02-21 Thread Claudio Imbrenda

On Wed, 21 Feb 2018 16:20:05 +0100
Cornelia Huck  wrote:

> On Tue, 20 Feb 2018 19:45:01 +0100
> Claudio Imbrenda  wrote:
> 
> > Clean up SCLP masks: introduce an sccb_mask_t to be used for SCLP
> > event masks instead of just unsigned int or uint32_t. This will
> > allow later to extend the mask with more ease.  
> 
> Looks mostly sane.
> 
> > 
> > Signed-off-by: Claudio Imbrenda 
> > ---
> >  hw/char/sclpconsole-lm.c  |  4 ++--
> >  hw/char/sclpconsole.c |  4 ++--
> >  hw/s390x/event-facility.c | 18 +-
> >  hw/s390x/sclpcpu.c|  4 ++--
> >  hw/s390x/sclpquiesce.c|  4 ++--
> >  include/hw/s390x/event-facility.h | 22 +-
> >  6 files changed, 30 insertions(+), 26 deletions(-)
> > 
> > diff --git a/hw/char/sclpconsole-lm.c b/hw/char/sclpconsole-lm.c
> > index c500bda..cc4d70a 100644
> > --- a/hw/char/sclpconsole-lm.c
> > +++ b/hw/char/sclpconsole-lm.c  
> 
> > diff --git a/hw/s390x/event-facility.c b/hw/s390x/event-facility.c
> > index 2414614..f6f28fd 100644
> > --- a/hw/s390x/event-facility.c
> > +++ b/hw/s390x/event-facility.c
> > @@ -30,7 +30,7 @@ struct SCLPEventFacility {
> >  SysBusDevice parent_obj;
> >  SCLPEventsBus sbus;
> >  /* guest' receive mask */  
> 
> Let's make this "guest's", as you're touching the line right below.

will do

> > -unsigned int receive_mask;
> > +sccb_mask_t receive_mask;
> >  /*
> >   * when false, we keep the same broken, backwards compatible
> > behaviour as
> >   * before; when true, we implement the architecture correctly.
> > Needed for  
> 
> (...)
> 
> > @@ -241,8 +241,8 @@ static void copy_mask(uint8_t *dst, uint8_t
> > *src, uint16_t dst_len, 
> >  static void read_event_data(SCLPEventFacility *ef, SCCB *sccb)
> >  {
> > -unsigned int sclp_active_selection_mask;
> > -unsigned int sclp_cp_receive_mask;
> > +sccb_mask_t sclp_active_selection_mask = 0;  
> 
> Why do you need to initialize this now?

right, copy_mask zeroes the unused space at the end of the destination,
it can be removed

> > +sccb_mask_t sclp_cp_receive_mask;
> >  
> >  ReadEventData *red = (ReadEventData *) sccb;
> >  
> > @@ -284,7 +284,7 @@ static void write_event_mask(SCLPEventFacility
> > *ef, SCCB *sccb) {
> >  WriteEventMask *we_mask = (WriteEventMask *) sccb;
> >  uint16_t mask_length = be16_to_cpu(we_mask->mask_length);
> > -uint32_t tmp_mask;
> > +sccb_mask_t tmp_mask = 0;  
> 
> Same here.

this is different, but it can still be avoided; I'll fix both.

> >  
> >  if (!mask_length || (mask_length > SCLP_EVENT_MASK_LEN_MAX) ||
> >  ((mask_length != 4) && !ef->allow_all_mask_sizes)) {  
> 
> (...)
> 
> > diff --git a/include/hw/s390x/event-facility.h
> > b/include/hw/s390x/event-facility.h index 5119b9b..0a8b47a 100644
> > --- a/include/hw/s390x/event-facility.h
> > +++ b/include/hw/s390x/event-facility.h
> > @@ -28,12 +28,14 @@
> >  #define SCLP_EVENT_SIGNAL_QUIESCE   0x1d
> >  
> >  /* SCLP event masks */
> > -#define SCLP_EVENT_MASK_SIGNAL_QUIESCE  0x0008
> > -#define SCLP_EVENT_MASK_MSG_ASCII   0x0040
> > -#define SCLP_EVENT_MASK_CONFIG_MGT_DATA 0x1000
> > -#define SCLP_EVENT_MASK_OP_CMD  0x8000
> > -#define SCLP_EVENT_MASK_MSG 0x4000
> > -#define SCLP_EVENT_MASK_PMSGCMD 0x0080
> > +#define SCLPEVMSK(T)  (1ULL << (sizeof(sccb_mask_t) * 8 - (T)))  
> 
> SCLP_EVMASK() would be a bit more readable, I think.

I know, but then it looks ugly when trying to fit everything in 80
columns. 

> > +
> > +#define SCLP_EVENT_MASK_OP_CMD
> > SCLPEVMSK(SCLP_EVENT_OPRTNS_COMMAND) +#define
> > SCLP_EVENT_MASK_MSG SCLPEVMSK(SCLP_EVENT_MESSAGE)
> > +#define SCLP_EVENT_MASK_CONFIG_MGT_DATA
> > SCLPEVMSK(SCLP_EVENT_CONFIG_MGT_DATA) +#define
> > SCLP_EVENT_MASK_PMSGCMD SCLPEVMSK(SCLP_EVENT_PMSGCMD)
> > +#define SCLP_EVENT_MASK_MSG_ASCII
> > SCLPEVMSK(SCLP_EVENT_ASCII_CONSOLE_DATA) +#define
> > SCLP_EVENT_MASK_SIGNAL_QUIESCE
> > SCLPEVMSK(SCLP_EVENT_SIGNAL_QUIESCE) #define
> > SCLP_UNCONDITIONAL_READ 0x00 #define
> > SCLP_SELECTIVE_READ 0x01  
>

Re: [Qemu-devel] [PATCH v3] specs/qcow2: Fix documentation of the compressed cluster descriptor


CC: qemu-sta...@nongnu.org

Incorrect docs make for difficult interoperability.

On 02/21/2018 08:08 AM, Alberto Garcia wrote:

This patch fixes several mistakes in the documentation of the
compressed cluster descriptor:

1) the documentation claims that the cluster descriptor contains the
number of sectors used to store the compressed data, but what it
actually contains is the number of sectors *minus one* or, in other
words, the number of additional sectors after the first one.

2) the width of the fields is incorrectly specified. The number of bits
used by each field is

   x = 62 - (cluster_bits - 8)   for the offset field
   y = (cluster_bits - 8)for the size field

So the offset field's location is [0, x-1], not [0, x] as stated.

3) the size field does not contain the size of the compressed data,
but rather the number of sectors where that data is stored. The
compressed data starts at the exact point specified in the offset
field and ends when there's enough data to produce a cluster of
decompressed data. Both points can be in the middle of a sector,
allowing several compressed clusters to be stored next to one
another, sharing sectors if necessary.

Signed-off-by: Alberto Garcia 
---

v3: Fix the specification of the width of the fields, and update the
 explanation of how the compressed data is stored [Eric].

v2: I realized that the documentation is not completely clear about
 the exact location and size of the compressed data, so I updated
 the patch to clarify this.

---
  docs/interop/qcow2.txt | 16 +---
  1 file changed, 13 insertions(+), 3 deletions(-)

diff --git a/docs/interop/qcow2.txt b/docs/interop/qcow2.txt
index d7fdb1fee3..feb711fb6a 100644
--- a/docs/interop/qcow2.txt
+++ b/docs/interop/qcow2.txt
@@ -426,10 +426,20 @@ Standard Cluster Descriptor:
  
  Compressed Clusters Descriptor (x = 62 - (cluster_bits - 8)):
  
-Bit  0 -  x:Host cluster offset. This is usually _not_ aligned to a

-cluster boundary!
+Bit  0 - x-1:   Host cluster offset. This is usually _not_ aligned to a
+cluster or sector boundary!
  
-   x+1 - 61:Compressed size of the images in sectors of 512 bytes

+ x - 61:Number of additional 512-byte sectors used for the
+compressed data, beyond the sector containing the offset
+in the previous field. Some of these sectors may reside
+in the next contiguous host cluster.
+
+Note that the compressed data does not necessarily occupy
+all of the bytes in the final sector; rather, decompression
+stops when it has produced a cluster of data.
+
+Another compressed cluster may map to the tail of the final
+sector used by this compressed cluster.
  
  If a cluster is unallocated, read requests shall read the data from the backing

  file (except if bit 0 in the Standard Cluster Descriptor is set). If there is



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

Re: [Qemu-devel] [PATCH v7 10/23] qmp: introduce QMPCapability

On Wed, Jan 24, 2018 at 01:39:44PM +0800, Peter Xu wrote:
> diff --git a/qapi-schema.json b/qapi-schema.json
> index 5c06745c79..2490d5188e 100644
> --- a/qapi-schema.json
> +++ b/qapi-schema.json
> @@ -102,21 +102,47 @@
>  #
>  # Enable QMP capabilities.
>  #
> -# Arguments: None.
> +# Arguments:
> +#
> +# @enable:List of QMPCapabilities to enable, which is optional.
> +# Client must not enable any capability that is not
> +# mentioned in QMP greeting message.

qapi-schema.json uses full sentences so I've added missing articles to
the text.  I also changed s/QMPCapabilities/QMPCapability values/ to
avoid confusion about the type name:

  An optional list of QMPCapability values to enable.  The client must
  not enable any capability that is not mentioned in the QMP greeting
  message.

>  #
>  # Example:
>  #
> -# -> { "execute": "qmp_capabilities" }
> +# -> { "execute": "qmp_capabilities",
> +#  "arguments": { "enable": [ "oob" ] } }
>  # <- { "return": {} }
>  #
>  # Notes: This command is valid exactly when first connecting: it must be
>  # issued before any other command will be accepted, and will fail once the
>  # monitor is accepting other commands. (see qemu docs/interop/qmp-spec.txt)
>  #
> +# QMP client needs to explicitly enable QMP capabilities, otherwise

s/QMP client/The QMP client/


signature.asc
Description: PGP signature

Re: [Qemu-devel] [PATCH 11/27] block: x-blockdev-create QMP command


On 02/21/2018 04:29 AM, Kevin Wolf wrote:


+++ b/include/block/block_int.h
@@ -130,6 +130,8 @@ struct BlockDriver {
   int (*bdrv_file_open)(BlockDriverState *bs, QDict *options, int flags,
 Error **errp);
   void (*bdrv_close)(BlockDriverState *bs);
+int coroutine_fn (*bdrv_co_create)(BlockdevCreateOptions *opts,
+   Error **errp);


I know we haven't been very good in the past, but can you add a comment here
on the contract that drivers are supposed to obey when implementing this
callback?


Anything specific you want to see here?

Essentially the meaning of BlockdevCreateOptions depends on the driver
and is documented in the QAPI schema, how Error works is common
knowledge, and I don't see much else to explain here.

I mean, I can add something like "Creates an image. See the QAPI
documentation for BlockdevCreateOptions for details." if you think this
is useful. But is it?


I guess my concern is whether this interface MUST overwrite any existing 
data in order to convert existing storage into a newly-created image of 
this driver's type (even if the overwritten data previously probed as a 
different image type), or if it is only called at a point when any 
existing data would be probed as raw, or any other useful tidbits that a 
driver might need to know in implementing it.  But if all you can think 
of is "See QAPI for BlockdevCreateOptions for details", then yeah, 
that's not worth a comment.





+/* Call callback if it exists */
+if (!drv->bdrv_co_create) {
+error_setg(errp, "Driver does not support blockdev-create");


Should this error message refer to 'x-blockdev-create' in the short term?


Hm, it would be more correct. On the other hand, I'm almost sure we'd
forget to rename it back when we remove the x- prefix...


Good point.  And being an x- prefix implies that inconsistency may be 
expected (not to mention short-lived, if we promote the interface); 
while being consistent now but risking long-term inconsistency down the 
road when it actually becomes a stable interface is indeed worse.  So 
keep this message as-is.


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

Re: [Qemu-devel] [PATCH v3 5/7] net: Remove the deprecated way of dumping network packets


On 02/21/2018 04:18 AM, Thomas Huth wrote:

"-net dump" has been marked as deprecated since QEMU v2.10, since it
only works with the deprecated 'vlan' parameter (or hubs). Network
dumping should be done with "-object filter-dump" nowadays instead.
Since nobody complained so far about the deprecation message, let's
finally get rid of "-net dump" now.

Signed-off-by: Thomas Huth 
---


Reviewed-by: Eric Blake 

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

Re: [Qemu-devel] [PATCH v7 12/23] monitor: let suspend_cnt be thread safe

On Wed, Jan 24, 2018 at 01:39:46PM +0800, Peter Xu wrote:
> Monitor code now can be run in more than one thread.  Let it be thread
> safe when accessing suspend_cnt counter.
> 
> Reviewed-by: Eric Blake 
> Signed-off-by: Peter Xu 
> ---
>  monitor.c | 15 ---
>  1 file changed, 8 insertions(+), 7 deletions(-)

Reviewed-by: Stefan Hajnoczi 


signature.asc
Description: PGP signature

Re: [Qemu-devel] [PATCH v7 12/23] monitor: let suspend_cnt be thread safe

Nevermind, I see the next patch adds QMP support.

Stefan


signature.asc
Description: PGP signature

[Qemu-devel] [PATCH v2 1/1] 390x/cpumodel: document S390FeatDef.bit not applicable

2018-02-21 Thread Halil Pasic

The 'bit' field of the 'S390FeatDef' structure is not applicable to all
its instances. Currently this field is not applicable, and remains
unused, iff the feature is of type S390_FEAT_TYPE_MISC. Having the value 0
specified for multiple such feature definition  was a little confusing,
as it's a perfectly legit bit value, and as the value of the bit
field is usually ought to be unique for each feature of a given
feature type.

Let us introduce a specialized macro for defining features of type
S390_FEAT_TYPE_MISC so, that one does not have to specify neither bit nor
type (as the later is implied).

Signed-off-by: Halil Pasic 
---

v1 -> v2
* Specialized feature initializer macro for type MISC that does not
  require a bit value instead of defining a 'not a bit number' (that
  is extremal) bit number.
---
 target/s390x/cpu_features.c | 8 ++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/target/s390x/cpu_features.c b/target/s390x/cpu_features.c
index a5619f2893..3b9e2745e9 100644
--- a/target/s390x/cpu_features.c
+++ b/target/s390x/cpu_features.c
@@ -23,6 +23,10 @@
 .desc = _desc,   \
 }
 
+/* S390FeatDef.bit is not applicable as there is no feature block. */
+#define FEAT_INIT_MISC(_name, _desc) \
+FEAT_INIT(_name, S390_FEAT_TYPE_MISC, 0, _desc)
+
 /* indexed by feature number for easy lookup */
 static const S390FeatDef s390_features[] = {
 FEAT_INIT("esan3", S390_FEAT_TYPE_STFL, 0, "Instructions marked as n3"),
@@ -123,8 +127,8 @@ static const S390FeatDef s390_features[] = {
 FEAT_INIT("ib", S390_FEAT_TYPE_SCLP_CPU, 42, "SIE: Intervention bypass 
facility"),
 FEAT_INIT("cei", S390_FEAT_TYPE_SCLP_CPU, 43, "SIE: 
Conditional-external-interception facility"),
 
-FEAT_INIT("dateh2", S390_FEAT_TYPE_MISC, 0, "DAT-enhancement facility 2"),
-FEAT_INIT("cmm", S390_FEAT_TYPE_MISC, 0, "Collaborative-memory-management 
facility"),
+FEAT_INIT_MISC("dateh2", "DAT-enhancement facility 2"),
+FEAT_INIT_MISC("cmm", "Collaborative-memory-management facility"),
 
 FEAT_INIT("plo-cl", S390_FEAT_TYPE_PLO, 0, "PLO Compare and load (32 bit 
in general registers)"),
 FEAT_INIT("plo-clg", S390_FEAT_TYPE_PLO, 1, "PLO Compare and load (64 bit 
in parameter list)"),
-- 
2.13.5

Re: [Qemu-devel] [PATCH v1 1/3] s390x/sclp: proper support of larger send and receive masks

On Tue, 20 Feb 2018 19:45:00 +0100
Claudio Imbrenda  wrote:

> The architecture allows the guests to ask for masks up to 1021 bytes in
> length. Part was fixed in 67915de9f0383ccf4ab8c42dd02aa18dcd79b411
> ("s390x/event-facility: variable-length event masks"), but some issues
> were still remaining, in particular regarding the handling of selective
> reads.
> 
> This patch fixes the handling of selective reads, whose size will now
> match the length of the event mask, as per architecture.
> 
> The default behaviour is to be compliant with the architecture, but when
> using older machine models the old behaviour is selected, in order to
> be able to migrate toward older versions.

I remember trying to do this back when I still had access to the
architecture, but somehow never finished this (don't remember why).

> 
> Fixes: 67915de9f0383ccf4a ("s390x/event-facility: variable-length event 
> masks")

Doesn't that rather fix the initial implementation? What am I missing?

> Signed-off-by: Claudio Imbrenda 
> ---
>  hw/s390x/event-facility.c  | 90 
> +++---
>  hw/s390x/s390-virtio-ccw.c |  8 -
>  2 files changed, 84 insertions(+), 14 deletions(-)
> 
> diff --git a/hw/s390x/event-facility.c b/hw/s390x/event-facility.c
> index 155a694..2414614 100644
> --- a/hw/s390x/event-facility.c
> +++ b/hw/s390x/event-facility.c
> @@ -31,6 +31,14 @@ struct SCLPEventFacility {
>  SCLPEventsBus sbus;
>  /* guest' receive mask */
>  unsigned int receive_mask;
> +/*
> + * when false, we keep the same broken, backwards compatible behaviour as
> + * before; when true, we implement the architecture correctly. Needed for
> + * migration toward older versions.
> + */
> +bool allow_all_mask_sizes;

The comment does not really tell us what the old behaviour is ;)

So, if this is about extending the mask size, call this
"allow_large_masks" or so?

> +/* length of the receive mask */
> +uint16_t mask_length;
>  };
>  
>  /* return true if any child has event pending set */
> @@ -220,6 +228,17 @@ static uint16_t 
> handle_sccb_read_events(SCLPEventFacility *ef, SCCB *sccb,
>  return rc;
>  }
>  
> +/* copy up to dst_len bytes and fill the rest of dst with zeroes */
> +static void copy_mask(uint8_t *dst, uint8_t *src, uint16_t dst_len,
> +  uint16_t src_len)
> +{
> +int i;
> +
> +for (i = 0; i < dst_len; i++) {
> +dst[i] = i < src_len ? src[i] : 0;
> +}
> +}
> +
>  static void read_event_data(SCLPEventFacility *ef, SCCB *sccb)
>  {
>  unsigned int sclp_active_selection_mask;
> @@ -240,7 +259,9 @@ static void read_event_data(SCLPEventFacility *ef, SCCB 
> *sccb)
>  sclp_active_selection_mask = sclp_cp_receive_mask;
>  break;
>  case SCLP_SELECTIVE_READ:
> -sclp_active_selection_mask = be32_to_cpu(red->mask);
> +copy_mask((uint8_t *)_active_selection_mask, (uint8_t 
> *)>mask,
> +  sizeof(sclp_active_selection_mask), ef->mask_length);
> +sclp_active_selection_mask = be32_to_cpu(sclp_active_selection_mask);

Hm, this looks like a real bug fix for the commit referenced above.
Split this out? We don't need compat support for that; maybe even
cc:stable?

(Not sure what the consequences are here, other than unwanted bits at
the end; can't say without doc.)

>  if (!sclp_cp_receive_mask ||
>  (sclp_active_selection_mask & ~sclp_cp_receive_mask)) {
>  sccb->h.response_code =
> @@ -259,24 +280,14 @@ out:
>  return;
>  }
>  
> -/* copy up to dst_len bytes and fill the rest of dst with zeroes */
> -static void copy_mask(uint8_t *dst, uint8_t *src, uint16_t dst_len,
> -  uint16_t src_len)
> -{
> -int i;
> -
> -for (i = 0; i < dst_len; i++) {
> -dst[i] = i < src_len ? src[i] : 0;
> -}
> -}
> -
>  static void write_event_mask(SCLPEventFacility *ef, SCCB *sccb)
>  {
>  WriteEventMask *we_mask = (WriteEventMask *) sccb;
>  uint16_t mask_length = be16_to_cpu(we_mask->mask_length);
>  uint32_t tmp_mask;
>  
> -if (!mask_length || (mask_length > SCLP_EVENT_MASK_LEN_MAX)) {
> +if (!mask_length || (mask_length > SCLP_EVENT_MASK_LEN_MAX) ||
> +((mask_length != 4) && !ef->allow_all_mask_sizes)) {

This is a behaviour change, isn't it? Previously, we allowed up to 4
bytes, now we allow exactly 4 bytes?

>  sccb->h.response_code = cpu_to_be16(SCLP_RC_INVALID_MASK_LENGTH);
>  goto out;
>  }

Re: [Qemu-devel] [PATCH v1 2/3] s390x/sclp: clean up sclp masks

On Tue, 20 Feb 2018 19:45:01 +0100
Claudio Imbrenda  wrote:

> Clean up SCLP masks: introduce an sccb_mask_t to be used for SCLP event
> masks instead of just unsigned int or uint32_t. This will allow later
> to extend the mask with more ease.

Looks mostly sane.

> 
> Signed-off-by: Claudio Imbrenda 
> ---
>  hw/char/sclpconsole-lm.c  |  4 ++--
>  hw/char/sclpconsole.c |  4 ++--
>  hw/s390x/event-facility.c | 18 +-
>  hw/s390x/sclpcpu.c|  4 ++--
>  hw/s390x/sclpquiesce.c|  4 ++--
>  include/hw/s390x/event-facility.h | 22 +-
>  6 files changed, 30 insertions(+), 26 deletions(-)
> 
> diff --git a/hw/char/sclpconsole-lm.c b/hw/char/sclpconsole-lm.c
> index c500bda..cc4d70a 100644
> --- a/hw/char/sclpconsole-lm.c
> +++ b/hw/char/sclpconsole-lm.c

> diff --git a/hw/s390x/event-facility.c b/hw/s390x/event-facility.c
> index 2414614..f6f28fd 100644
> --- a/hw/s390x/event-facility.c
> +++ b/hw/s390x/event-facility.c
> @@ -30,7 +30,7 @@ struct SCLPEventFacility {
>  SysBusDevice parent_obj;
>  SCLPEventsBus sbus;
>  /* guest' receive mask */

Let's make this "guest's", as you're touching the line right below.

> -unsigned int receive_mask;
> +sccb_mask_t receive_mask;
>  /*
>   * when false, we keep the same broken, backwards compatible behaviour as
>   * before; when true, we implement the architecture correctly. Needed for

(...)

> @@ -241,8 +241,8 @@ static void copy_mask(uint8_t *dst, uint8_t *src, 
> uint16_t dst_len,
>  
>  static void read_event_data(SCLPEventFacility *ef, SCCB *sccb)
>  {
> -unsigned int sclp_active_selection_mask;
> -unsigned int sclp_cp_receive_mask;
> +sccb_mask_t sclp_active_selection_mask = 0;

Why do you need to initialize this now?

> +sccb_mask_t sclp_cp_receive_mask;
>  
>  ReadEventData *red = (ReadEventData *) sccb;
>  
> @@ -284,7 +284,7 @@ static void write_event_mask(SCLPEventFacility *ef, SCCB 
> *sccb)
>  {
>  WriteEventMask *we_mask = (WriteEventMask *) sccb;
>  uint16_t mask_length = be16_to_cpu(we_mask->mask_length);
> -uint32_t tmp_mask;
> +sccb_mask_t tmp_mask = 0;

Same here.

>  
>  if (!mask_length || (mask_length > SCLP_EVENT_MASK_LEN_MAX) ||
>  ((mask_length != 4) && !ef->allow_all_mask_sizes)) {

(...)

> diff --git a/include/hw/s390x/event-facility.h 
> b/include/hw/s390x/event-facility.h
> index 5119b9b..0a8b47a 100644
> --- a/include/hw/s390x/event-facility.h
> +++ b/include/hw/s390x/event-facility.h
> @@ -28,12 +28,14 @@
>  #define SCLP_EVENT_SIGNAL_QUIESCE   0x1d
>  
>  /* SCLP event masks */
> -#define SCLP_EVENT_MASK_SIGNAL_QUIESCE  0x0008
> -#define SCLP_EVENT_MASK_MSG_ASCII   0x0040
> -#define SCLP_EVENT_MASK_CONFIG_MGT_DATA 0x1000
> -#define SCLP_EVENT_MASK_OP_CMD  0x8000
> -#define SCLP_EVENT_MASK_MSG 0x4000
> -#define SCLP_EVENT_MASK_PMSGCMD 0x0080
> +#define SCLPEVMSK(T)  (1ULL << (sizeof(sccb_mask_t) * 8 - (T)))

SCLP_EVMASK() would be a bit more readable, I think.

> +
> +#define SCLP_EVENT_MASK_OP_CMD  SCLPEVMSK(SCLP_EVENT_OPRTNS_COMMAND)
> +#define SCLP_EVENT_MASK_MSG SCLPEVMSK(SCLP_EVENT_MESSAGE)
> +#define SCLP_EVENT_MASK_CONFIG_MGT_DATA SCLPEVMSK(SCLP_EVENT_CONFIG_MGT_DATA)
> +#define SCLP_EVENT_MASK_PMSGCMD SCLPEVMSK(SCLP_EVENT_PMSGCMD)
> +#define SCLP_EVENT_MASK_MSG_ASCII   
> SCLPEVMSK(SCLP_EVENT_ASCII_CONSOLE_DATA)
> +#define SCLP_EVENT_MASK_SIGNAL_QUIESCE  SCLPEVMSK(SCLP_EVENT_SIGNAL_QUIESCE)
>  
>  #define SCLP_UNCONDITIONAL_READ 0x00
>  #define SCLP_SELECTIVE_READ 0x01

Re: [Qemu-devel] [PATCH v1 0/3] s390x/sclp: 64 bit event masks

On Tue, 20 Feb 2018 19:44:59 +0100
Claudio Imbrenda  wrote:

> Until 67915de9f0383ccf4a ("s390x/event-facility: variable-length event masks")
> we only supported 32bit sclp event masks, even though the archiecture
> allows the guests to set up sclp event masks up to 1021 bytes in length.
> With that patch the behaviour was almost compliant, but some issues were
> still remaining, in particular regarding the handling of selective reads
> and migration.
> 
> This patchset fixes migration and the handling of selective reads, and
> puts in place the support for 64-bit sclp event masks internally.
> 
> A new property of the sclp-event device switches between the 32bit masks
> and the compliant behaviour. The property is bound to the machine
> version, so older machines keep the old broken behaviour, allowing for
> migration, but the default is the compliant implementation.
> 
> Fixes: 67915de9f0383ccf4a ("s390x/event-facility: variable-length event 
> masks")
> 
> Claudio Imbrenda (3):
>   s390x/sclp: proper support of larger send and receive masks
>   s390x/sclp: clean up sclp masks
>   s390x/sclp: extend SCLP event masks to 64 bits
> 
>  hw/char/sclpconsole-lm.c  |   4 +-
>  hw/char/sclpconsole.c |   4 +-
>  hw/s390x/event-facility.c | 147 
> +++---
>  hw/s390x/s390-virtio-ccw.c|   8 ++-
>  hw/s390x/sclpcpu.c|   4 +-
>  hw/s390x/sclpquiesce.c|   4 +-
>  include/hw/s390x/event-facility.h |  22 +++---
>  7 files changed, 149 insertions(+), 44 deletions(-)
> 

I can't review the architecture details without documentation
obviously, so another question: Do you have a test you can share? :)

Re: [Qemu-devel] [PATCH v7 09/23] monitor: allow using IO thread for parsing

On Wed, Jan 24, 2018 at 01:39:43PM +0800, Peter Xu wrote:
> @@ -4034,12 +4044,29 @@ static void sortcmdlist(void)
>  qsort((void *)info_cmds, array_num, elem_size, compare_mon_cmd);
>  }
>  
> +static GMainContext *monitor_io_context_get(void)
> +{
> +return iothread_get_g_main_context(mon_global.mon_iothread);
> +}
> +
> +static AioContext *monitor_aio_context_get(void)
> +{
> +return iothread_get_aio_context(mon_global.mon_iothread);
> +}

Please follow the X_get_Y() naming convention instead of X_Y_get().  For
example, see qemu_get_aio_context() and iothread_get_aio_context().

> @@ -4082,11 +4109,41 @@ void error_vprintf_unless_qmp(const char *fmt, 
> va_list ap)
>  }
>  }
>  
> +static void monitor_list_append(Monitor *mon)
> +{
> +qemu_mutex_lock(_lock);
> +QTAILQ_INSERT_HEAD(_list, mon, entry);
> +qemu_mutex_unlock(_lock);
> +}
> +
> +static void monitor_qmp_setup_handlers(void *data)

BH functions are usually declared like this:

  static void X_bh(void *opaque)

This way it's immediately clear that this function is invoked as a BH.

I suggest renaming the function to monitor_qmp_setup_handlers_bh().
Using 'opaque' instead of 'data' is common, too.

> @@ -4099,24 +4156,55 @@ void monitor_init(Chardev *chr, int flags)
>  }
>  
>  if (monitor_is_qmp(mon)) {
> -qemu_chr_fe_set_handlers(>chr, monitor_can_read, 
> monitor_qmp_read,
> - monitor_qmp_event, NULL, mon, NULL, true);
>  qemu_chr_fe_set_echo(>chr, true);
>  json_message_parser_init(>qmp.parser, handle_qmp_command);
> +if (mon->use_io_thr) {
> +/*
> + * It's possible that we already have an IOWatchPoll
> + * registered for the Chardev during chardev_init_func().

When does this happen?

This seems like a hack that breaks when certain -chardev options are
used.  For example, what happens if the chardev is a TCP connection with
reconnect=5.  In that case the socket will be connecting asynchronously
and we cannot just remove the fd watch.

How does this interact with TCP listen chardevs?  It looks like the
listener socket uses the main loop (see tcp_chr_disconnect()).

I'm worried that the chardev layer isn't thread-safe and you haven't
added anything to protect it or at least refuse to run in unsafe
conditions.

signature.asc
Description: PGP signature

Re: [Qemu-devel] [PATCH v3] specs/qcow2: Fix documentation of the compressed cluster descriptor


On 02/21/2018 08:08 AM, Alberto Garcia wrote:

This patch fixes several mistakes in the documentation of the
compressed cluster descriptor:

1) the documentation claims that the cluster descriptor contains the
number of sectors used to store the compressed data, but what it
actually contains is the number of sectors *minus one* or, in other
words, the number of additional sectors after the first one.

2) the width of the fields is incorrectly specified. The number of bits
used by each field is

   x = 62 - (cluster_bits - 8)   for the offset field
   y = (cluster_bits - 8)for the size field

So the offset field's location is [0, x-1], not [0, x] as stated.


Yep, I figured that out independently, although our mails crossed, and I 
didn't notice your fix until after my other email calling out this bug 
in the spec.




3) the size field does not contain the size of the compressed data,
but rather the number of sectors where that data is stored. The
compressed data starts at the exact point specified in the offset
field and ends when there's enough data to produce a cluster of
decompressed data. Both points can be in the middle of a sector,
allowing several compressed clusters to be stored next to one
another, sharing sectors if necessary.

Signed-off-by: Alberto Garcia 
---

v3: Fix the specification of the width of the fields, and update the
 explanation of how the compressed data is stored [Eric].




+++ b/docs/interop/qcow2.txt
@@ -426,10 +426,20 @@ Standard Cluster Descriptor:
  
  Compressed Clusters Descriptor (x = 62 - (cluster_bits - 8)):
  
-Bit  0 -  x:Host cluster offset. This is usually _not_ aligned to a

-cluster boundary!
+Bit  0 - x-1:   Host cluster offset. This is usually _not_ aligned to a
+cluster or sector boundary!



This breaks the nice alignment of the ':' character that the rest of the 
section preserves.  You could also have done:


Compressed Clusters Descriptor (x = 61 - (cluster_bits - 8)):
   Bit  0 -  x:  ...
  x+1 - 61:  ...

to preserve the alignment, by shifting the adjustment into the 
calculation of x.  But I'm fine with either approach.


  
-   x+1 - 61:Compressed size of the images in sectors of 512 bytes

+ x - 61:Number of additional 512-byte sectors used for the
+compressed data, beyond the sector containing the offset
+in the previous field. Some of these sectors may reside
+in the next contiguous host cluster.
+
+Note that the compressed data does not necessarily occupy
+all of the bytes in the final sector; rather, decompression
+stops when it has produced a cluster of data.
+
+Another compressed cluster may map to the tail of the final
+sector used by this compressed cluster.


Yes, that wording is MUCH nicer.  Thanks for persisting, and for making 
me learn more about qcow2 compression in the process.  So either way on 
the colon alignment issue:


Reviewed-by: Eric Blake 

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

Re: [Qemu-devel] [PATCH v1 1/3] s390x/sclp: proper support of larger send and receive masks

2018-02-21 Thread Claudio Imbrenda

On Wed, 21 Feb 2018 16:12:59 +0100
Cornelia Huck  wrote:

> On Tue, 20 Feb 2018 19:45:00 +0100
> Claudio Imbrenda  wrote:
> 
> > The architecture allows the guests to ask for masks up to 1021
> > bytes in length. Part was fixed in
> > 67915de9f0383ccf4ab8c42dd02aa18dcd79b411 ("s390x/event-facility:
> > variable-length event masks"), but some issues were still
> > remaining, in particular regarding the handling of selective reads.
> > 
> > This patch fixes the handling of selective reads, whose size will
> > now match the length of the event mask, as per architecture.
> > 
> > The default behaviour is to be compliant with the architecture, but
> > when using older machine models the old behaviour is selected, in
> > order to be able to migrate toward older versions.  
> 
> I remember trying to do this back when I still had access to the
> architecture, but somehow never finished this (don't remember why).
> 
> > 
> > Fixes: 67915de9f0383ccf4a ("s390x/event-facility: variable-length
> > event masks")  
> 
> Doesn't that rather fix the initial implementation? What am I missing?

well that too. but I think this fixes the fix, since the fix was
incomplete?

> > Signed-off-by: Claudio Imbrenda 
> > ---
> >  hw/s390x/event-facility.c  | 90
> > +++---
> > hw/s390x/s390-virtio-ccw.c |  8 - 2 files changed, 84
> > insertions(+), 14 deletions(-)
> > 
> > diff --git a/hw/s390x/event-facility.c b/hw/s390x/event-facility.c
> > index 155a694..2414614 100644
> > --- a/hw/s390x/event-facility.c
> > +++ b/hw/s390x/event-facility.c
> > @@ -31,6 +31,14 @@ struct SCLPEventFacility {
> >  SCLPEventsBus sbus;
> >  /* guest' receive mask */
> >  unsigned int receive_mask;
> > +/*
> > + * when false, we keep the same broken, backwards compatible
> > behaviour as
> > + * before; when true, we implement the architecture correctly.
> > Needed for
> > + * migration toward older versions.
> > + */
> > +bool allow_all_mask_sizes;  
> 
> The comment does not really tell us what the old behaviour is ;)

hmm, I'll fix that

> So, if this is about extending the mask size, call this
> "allow_large_masks" or so?

no, the old broken behaviour allowed only _exactly_ 4 bytes:

if (be16_to_cpu(we_mask->mask_length) != 4) {
 sccb->h.response_code = cpu_to_be16(SCLP_RC_INVALID_MASK_LENGTH);
 goto out;
}

With the 67915de9f0383ccf4a patch mentioned above, we allow any size,
but we don't keep track of the size itself, only the bits. This is a
problem for selective reads (see below)

> > +/* length of the receive mask */
> > +uint16_t mask_length;
> >  };
> >  
> >  /* return true if any child has event pending set */
> > @@ -220,6 +228,17 @@ static uint16_t
> > handle_sccb_read_events(SCLPEventFacility *ef, SCCB *sccb, return
> > rc; }
> >  
> > +/* copy up to dst_len bytes and fill the rest of dst with zeroes */
> > +static void copy_mask(uint8_t *dst, uint8_t *src, uint16_t dst_len,
> > +  uint16_t src_len)
> > +{
> > +int i;
> > +
> > +for (i = 0; i < dst_len; i++) {
> > +dst[i] = i < src_len ? src[i] : 0;
> > +}
> > +}
> > +
> >  static void read_event_data(SCLPEventFacility *ef, SCCB *sccb)
> >  {
> >  unsigned int sclp_active_selection_mask;
> > @@ -240,7 +259,9 @@ static void read_event_data(SCLPEventFacility
> > *ef, SCCB *sccb) sclp_active_selection_mask = sclp_cp_receive_mask;
> >  break;
> >  case SCLP_SELECTIVE_READ:
> > -sclp_active_selection_mask = be32_to_cpu(red->mask);
> > +copy_mask((uint8_t *)_active_selection_mask, (uint8_t
> > *)>mask,
> > +  sizeof(sclp_active_selection_mask),
> > ef->mask_length);
> > +sclp_active_selection_mask =
> > be32_to_cpu(sclp_active_selection_mask);  
> 
> Hm, this looks like a real bug fix for the commit referenced above.
> Split this out? We don't need compat support for that; maybe even
> cc:stable?

I think we do. We can avoid keeping track of the mask size when setting
the mask size, because we can simply take the bits we need and ignore
the rest. But for selective reads we need the mask size, so we have to
keep track of it. (selective reads specify a mask, but not a mask
length, the mask length is the length of the last mask set)

And now we have additional state that we need to copy around when
migrating. And we don't want to break older machines! Moreover a
"new" guest started on a new qemu but with older machine version should
still be limited to 4 bytes, so we can migrate it to an actual older
version of qemu.

If a "new" guest uses a larger (or shorter!) mask then we can't migrate
it back to an older version of qemu. New guests that support masks of
size != 4 also still need to support the case where only size == 4 is
allowed, otherwise they will not work at all on actual old versions of
qemu.

So fixing selective reads needs

Re: [Qemu-devel] [PATCH v7 12/23] monitor: let suspend_cnt be thread safe

On Wed, Jan 24, 2018 at 01:39:46PM +0800, Peter Xu wrote:
> @@ -3996,7 +3996,7 @@ int monitor_suspend(Monitor *mon)
>  {
>  if (!mon->rs)
>  return -ENOTTY;

Does this mean that QMP monitors like -qmp unix:/tmp/foo,server,nowait
will not use suspend_cnt?

I thought the point was to use suspend_cnt for QMP monitors too.

signature.asc
Description: PGP signature

Re: [Qemu-devel] [PATCH 2/2] qcow2: Avoid memory over-allocation on compressed images

2018-02-21 Thread Kevin Wolf

Am 20.02.2018 um 23:24 hat Eric Blake geschrieben:
> When reading a compressed image, we were allocating s->cluster_data
> to 32*cluster_size + 512 (possibly over 64 megabytes, for an image
> with 2M clusters).  Let's check out the history:
> 
> Back when qcow2 was first written, we used s->cluster_data for
> everything, including copy_sectors() and encryption, where we want
> to operate on more than one cluster at once.  Obviously, at that
> point, the buffer had to be aligned for other users, even though
> compression itself doesn't require any alignment.
> 
> But commit 1b9f1491 (v1.1!) changed things to allocate parallel
> buffers on demand rather than sharing a single buffer, for encryption
> and COW, leaving compression as the final client of s->cluster_data.
> That use was still preserved, because if a single compressed cluster
> is read more than once, we reuse the cache instead of decompressing
> it a second time (I'm not sure how often this optimization actually
> fires, or if it penalizes us from being able to decompress multiple
> clusters in parallel even though we can now decrypt clusters in
> parallel; the XXX comment in qcow2_co_preadv for
> QCOW2_CLUSTER_COMPRESSED is telling).
> 
> Much later, in commit de82815d (v2.2), we noticed that a 64M
> allocation is prone to failure, so we switched over to a graceful
> memory allocation error message.  But note that elsewhere in the
> code, we do g_malloc(2 * cluster_size) without ever checking for
> failure.
> 
> Then even later, in 3e4c7052 (2.11), we realized that allocating
> a large buffer up front for every qcow2 image is expensive, and
> switched to lazy allocation only for images that actually had
> compressed clusters.  But in the process, we never even bothered
> to check whether what we were allocating still made sense in its
> new context!
> 
> So, it's time to cut back on the waste.  A compressed cluster
> will NEVER occupy more than an uncompressed cluster (okay, gzip
> DOES document that because the compression stream adds metadata,
> and because of the pigeonhole principle, there are worst case
> scenarios where attempts to compress will actually inflate an
> image - but in those cases, we would just write the cluster
> uncompressed instead of inflating it).  And as that is a smaller
> amount of memory, we can get by with the simpler g_malloc.
> 
> Signed-off-by: Eric Blake 

My comments feel a bit trivial compared to Berto's, but anyway:

> diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c
> index 85be7d5e340..8c4b26ceaf2 100644
> --- a/block/qcow2-cluster.c
> +++ b/block/qcow2-cluster.c
> @@ -1603,15 +1603,9 @@ int qcow2_decompress_cluster(BlockDriverState *bs, 
> uint64_t cluster_offset)
>   * are freed in .bdrv_close().
>   */
>  if (!s->cluster_data) {
> -/* one more sector for decompressed data alignment */
> -s->cluster_data = qemu_try_blockalign(bs->file->bs,
> -QCOW_MAX_CRYPT_CLUSTERS * s->cluster_size + 512);
> -if (!s->cluster_data) {
> -return -ENOMEM;
> -}
> -}
> -if (!s->cluster_cache) {
> -s->cluster_cache = g_malloc(s->cluster_size);
> +assert(!s->cluster_cache);

Wouldn't it be better to assert (!!s->cluster_cache ==
!!s->cluster_data) unconditionally?

> +s->cluster_data = g_try_malloc(s->cluster_size);

Why are you going from qemu_try_blockalign() to simple malloc here? This
buffer is used with bdrv_read() (or bdrv_pread() after patch 1), so we
should avoid unnecessary use of a bounce buffer.

> +s->cluster_cache = g_try_malloc(s->cluster_size);

As you already said, either g_malloc() or check the result. I actually
think that g_try_malloc() and checking the result is nicer, we still
allocate up to 2 MB here.

Kevin

[Qemu-devel] [Bug 1732959] Re: [regression] Clock jump on source VM after migration

2018-02-21 Thread Neil Skrypuch

As a further test, I disabled ntpd on the host and ran ntpdate via cron
every 12 hours, so that the clock would be relatively accurate, but no
clock skew would be involved. This also reproduced the failure as
initially described.

This is interesting as it means that a much simpler and faster
reproduction case is probably feasible, something like:

1) start guest
2) jump host clock by a few seconds
3) migrate + cont src guest
4) check clock in src guest

Though, I haven't tried this yet.

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

Title:
  [regression] Clock jump on source VM after migration

Status in QEMU:
  New

Bug description:
  We (ab)use migration + block mirroring to perform transparent zero
  downtime VM backups. Basically:

  1) do a block mirror of the source VM's disk
  2) migrate the source VM to a destination VM using the disk copy
  3) cancel the block mirroring
  4) resume the source VM
  5) shut down the destination VM gracefully and move the disk to backup

  Relatively recently, the source VM's clock started jumping after step
  #4. More specifically, the clock jumps an amount of time proportional
  to the time since it was last migrated. With a week between
  migrations, clock jumps between ~2.5s and ~12s have been observed. For
  a particular host, the amount of clock jump is fairly consistent, but
  there is a large variation from one host to the next (this is likely
  down to hardware variations and the amount of NTP adjusted clock drift
  on the host).

  This is caused by a kernel regression which I was able to bisect. The
  result of the bisect was:

  108b249c453dd7132599ab6dc7e435a7036c193f is the first bad commit
  commit 108b249c453dd7132599ab6dc7e435a7036c193f
  Author: Paolo Bonzini 
  Date:   Thu Sep 1 14:21:03 2016 +0200

  KVM: x86: introduce get_kvmclock_ns
  
  Introduce a function that reads the exact nanoseconds value that is
  provided to the guest in kvmclock.  This crystallizes the notion of
  kvmclock as a thin veneer over a stable TSC, that the guest will
  (hopefully) convert with NTP.  In other words, kvmclock is *not* a
  paravirtualized host-to-guest NTP.
  
  Drop the get_kernel_ns() function, that was used both to get the base
  value of the master clock and to get the current value of kvmclock.
  The former use is replaced by ktime_get_boot_ns(), the latter is
  the purpose of get_kernel_ns().
  
  This also allows KVM to provide a Hyper-V time reference counter that
  is synchronized with the time that is computed from the TSC page.
  
  Reviewed-by: Roman Kagan 
  Signed-off-by: Paolo Bonzini 

  I am able to reproduce the issue with much newer kernels as well,
  including 4.12.5 and 4.9.6.

  Reliably reproducing the problem in isolation is difficult, as one
  must run a VM for many hours before the clock jump from this bug is
  noticeable over the clock jump inherent with a pause and resume of the
  VM. The reproducer I am including is set to run the VM for 18 hours
  before migration and looks for >= 150 ms of clock jump. On different
  hardware, you may need to let the VM run for more than 18 hours to
  reliably reproduce the issue.

  To reproduce the issue, please see the attached reproducer. The host
  needs to have perl, screen and socat installed for the backup script
  to work. Both the host and guest need to be running NTP (and NTP must
  autostart at boot in the guest). The host needs to be able to SSH into
  the guest using SSH keys (to measure the clock jump), so you will need
  to configure the network and SSH keys appropriately, then change the
  hardcoded IP address in checktime.sh and test.sh. I have only tested
  with CentOS 7 guests.

  The qemu command that gets run is in .kvmscreen (the destination VM's
  command line is programmatically constructed from this command as
  well), you may need to tweak the bridge configuration. Also, although
  the reproducer is relatively self contained, it has several built in
  assumptions that will break if the image file is not in the
  /var/lib/kvm directory or if the monitor file is not in the
  /var/lib/kvm/monitor directory, or if the /backup directory does not
  exist. Finally, if you change the process name or socket name in
  .kvmscreen, you'll need to adjust the cleanup section in test.sh.

  With all of the above in place, run test.sh and check back in a little
  over 18 hours, part of the output should include something along these
  lines:

  Target not found (wanted 150, at 10)

  - or -

  Target found (wanted 150, found 340)

  If the target is reported as found, that means that we have probably
  reproduced the described issue.

  The version of QEMU in use does not appear to matter. At one point I
  tested every major version from 2.4 to

[Qemu-devel] [PATCH v3 4/7] net: Make net_client_init() static

The function is only used within net.c, so there's no need that
this is a global function.

While we're at it, also remove the unused prototype compute_mcast_idx()
(the function has been removed in commit d9caeb09b107e91122d10ba4a08a).

Reviewed-by: Paolo Bonzini 
Signed-off-by: Thomas Huth 
---
 include/net/net.h | 2 --
 net/net.c | 2 +-
 2 files changed, 1 insertion(+), 3 deletions(-)

diff --git a/include/net/net.h b/include/net/net.h
index bdd4d9f..cd1708c 100644
--- a/include/net/net.h
+++ b/include/net/net.h
@@ -204,7 +204,6 @@ extern const char *host_net_devices[];
 extern const char *legacy_tftp_prefix;
 extern const char *legacy_bootp_filename;
 
-int net_client_init(QemuOpts *opts, bool is_netdev, Error **errp);
 int net_client_parse(QemuOptsList *opts_list, const char *str);
 int net_init_clients(Error **errp);
 void net_check_clients(void);
@@ -228,7 +227,6 @@ void qdev_set_nic_properties(DeviceState *dev, NICInfo *nd);
 #define POLYNOMIAL_LE 0xedb88320
 uint32_t net_crc32(const uint8_t *p, int len);
 uint32_t net_crc32_le(const uint8_t *p, int len);
-unsigned compute_mcast_idx(const uint8_t *ep);
 
 #define vmstate_offset_macaddr(_state, _field)   \
 vmstate_offset_array(_state, _field.a, uint8_t,\
diff --git a/net/net.c b/net/net.c
index cf07e15..dd80f1b 100644
--- a/net/net.c
+++ b/net/net.c
@@ -1119,7 +1119,7 @@ static void show_netdevs(void)
 }
 }
 
-int net_client_init(QemuOpts *opts, bool is_netdev, Error **errp)
+static int net_client_init(QemuOpts *opts, bool is_netdev, Error **errp)
 {
 void *object = NULL;
 Error *err = NULL;
-- 
1.8.3.1

[Qemu-devel] [PATCH v3 7/7] net: Add a new convenience option "--nic" to configure default/on-board NICs

The legacy "-net" option can be quite confusing for the users since most
people do not expect to get a "vlan" hub between their emulated guest
hardware and the host backend. But so far, we are also not able to get
rid of "-net" completely, since it is the only way to configure on-board
NICs that can not be instantiated via "-device" yet. It's also a little
bit shorter to type "-net nic -net tap" instead of "-device xyz,netdev=n1
-netdev tap,id=n1".

So what we need is a new convenience option that is shorter to type than
the full -device + -netdev stuff, and which can be used to configure the
on-board NICs that can not be handled via -device yet. Thus this patch now
provides such a new option "--nic": It adds an entry in the nd_table to
configure a on-board / default NIC, creates a host backend and connects
the two directly, without a confusing "vlan" hub inbetween.

Reviewed-by: Paolo Bonzini 
Signed-off-by: Thomas Huth 
---
 include/sysemu/sysemu.h |  1 +
 net/net.c   | 78 +
 qemu-options.hx | 40 +
 vl.c|  7 +
 4 files changed, 120 insertions(+), 6 deletions(-)

diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h
index 77bb3da..66f0761 100644
--- a/include/sysemu/sysemu.h
+++ b/include/sysemu/sysemu.h
@@ -197,6 +197,7 @@ extern QemuOptsList bdrv_runtime_opts;
 extern QemuOptsList qemu_chardev_opts;
 extern QemuOptsList qemu_device_opts;
 extern QemuOptsList qemu_netdev_opts;
+extern QemuOptsList qemu_nic_opts;
 extern QemuOptsList qemu_net_opts;
 extern QemuOptsList qemu_global_opts;
 extern QemuOptsList qemu_mon_opts;
diff --git a/net/net.c b/net/net.c
index 2d05808..0bab269 100644
--- a/net/net.c
+++ b/net/net.c
@@ -1462,6 +1462,67 @@ static int net_init_netdev(void *dummy, QemuOpts *opts, 
Error **errp)
 return net_client_init(opts, true, errp);
 }
 
+/* For the convenience "--nic" parameter */
+static int net_param_nic(void *dummy, QemuOpts *opts, Error **errp)
+{
+char *mac, *nd_id;
+int idx, ret;
+NICInfo *ni;
+const char *type;
+
+type = qemu_opt_get(opts, "type");
+if (type && g_str_equal(type, "none")) {
+return 0;/* Nothing to do, default_net is cleared in vl.c */
+}
+
+idx = nic_get_free_idx();
+if (idx == -1 || nb_nics >= MAX_NICS) {
+error_setg(errp, "no more on-board/default NIC slots available");
+return -1;
+}
+
+if (!type) {
+qemu_opt_set(opts, "type", "user", _abort);
+}
+
+ni = _table[idx];
+memset(ni, 0, sizeof(*ni));
+ni->model = qemu_opt_get_del(opts, "model");
+
+/* Create an ID if the user did not specify one */
+nd_id = g_strdup(qemu_opts_id(opts));
+if (!nd_id) {
+nd_id = g_strdup_printf("__org.qemu.nic%i\n", idx);
+qemu_opts_set_id(opts, nd_id);
+}
+
+/* Handle MAC address */
+mac = qemu_opt_get_del(opts, "mac");
+if (mac) {
+ret = net_parse_macaddr(ni->macaddr.a, mac);
+g_free(mac);
+if (ret) {
+error_setg(errp, "invalid syntax for ethernet address");
+return -1;
+}
+if (is_multicast_ether_addr(ni->macaddr.a)) {
+error_setg(errp, "NIC cannot have multicast MAC address");
+return -1;
+}
+}
+qemu_macaddr_default_if_unset(>macaddr);
+
+ret = net_client_init(opts, true, errp);
+if (ret == 0) {
+ni->netdev = qemu_find_netdev(nd_id);
+ni->used = true;
+nb_nics++;
+}
+
+g_free(nd_id);
+return ret;
+}
+
 int net_init_clients(Error **errp)
 {
 net_change_state_entry =
@@ -1474,6 +1535,10 @@ int net_init_clients(Error **errp)
 return -1;
 }
 
+if (qemu_opts_foreach(qemu_find_opts("nic"), net_param_nic, NULL, errp)) {
+return -1;
+}
+
 if (qemu_opts_foreach(qemu_find_opts("net"), net_init_client, NULL, errp)) 
{
 return -1;
 }
@@ -1549,6 +1614,19 @@ QemuOptsList qemu_netdev_opts = {
 },
 };
 
+QemuOptsList qemu_nic_opts = {
+.name = "nic",
+.implied_opt_name = "type",
+.head = QTAILQ_HEAD_INITIALIZER(qemu_nic_opts.head),
+.desc = {
+/*
+ * no elements => accept any params
+ * validation will happen later
+ */
+{ /* end of list */ }
+},
+};
+
 QemuOptsList qemu_net_opts = {
 .name = "net",
 .implied_opt_name = "type",
diff --git a/qemu-options.hx b/qemu-options.hx
index a9249b6..399905e 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -2004,13 +2004,34 @@ DEF("netdev", HAS_ARG, QEMU_OPTION_netdev,
 #endif
 "-netdev hubport,id=str,hubid=n[,netdev=nd]\n"
 "configure a hub port on QEMU VLAN 'n'\n", QEMU_ARCH_ALL)
+DEF("nic", HAS_ARG, QEMU_OPTION_nic,
+"--nic [tap|bridge|"
+#ifdef CONFIG_SLIRP
+"user|"
+#endif
+#ifdef __linux__
+"l2tpv3|"
+#endif
+#ifdef CONFIG_VDE
+"vde|"
+#endif

Re: [Qemu-devel] [PATCH v3 0/7] Improvements and clean-ups related to -net

2018-02-21 Thread Mark Cave-Ayland

On 21/02/18 10:18, Thomas Huth wrote:

"-net" is a legacy option that often causes confusion and
misconfigurations for the users, since most people are not aware
of the underlying "vlan" (i.e. hub) concept that is used for this
parameter. The prefered way of configuring your network stack is
to use "--netdev" instead, which gives you a clean 1:1 connection
between your emulated guest hardware and the host network backend.

However, there are two reasons why we could not completely deprecate
"-net" yet:

1) Convenience:
In some cases, it's more convenient to use "-net" instead of "--netdev",
e.g. if you just want to have a "tap" network connection, it's faster
to type "-net nic -net tap" instead of "--device e1000,netdev=n1
--netdev tap,id=n1".

2) On-board NICs:
Currently the "-net nic" parameter is the only way to configure on-
board NICs on certain (embedded) machines via the nd_table[] array.

So beside some generic clean-ups and removal of code that has been
marked as deprecated since QEMU 2.10 already, this patch series intro-
duces a new parameter "--nic" (in patch 7) which should be able to re-
place "-net" in the long run completely: This new convenience parameter
can be used to configure the default/on-board guest HW together with a
host network backend in a very compact way. To configure a tap backend
for the default NIC, you just have to type "--nic tap" here for example.

Hi Thomas,

This is a great improvement for configuring on-board NICs! I do have a
couple of questions about your patchset based on personal experience:

1) Does the new -nic syntax support multiple on-board NICs? I remember
seeing this on some of the ARM boards I was studying when trying to
implement something similar for SPARC.

2) Is it possible to provide a convenient wrapper function to handle the
logic related to determining whether a specified NIC is on-board or not?

For example take a look at
https://git.qemu.org/?p=qemu.git;a=blob;f=hw/sparc64/sun4u.c;h=da28ab9413efdbe0bf0e1d3bf2b545577b83d88a;hb=a6e0344fa0e09413324835ae122c4cadd7890231#l596
where we have a nice dance to check whether the device specified is
on-board or not, and if it isn't to plug it in as an extra PCI device
instead. The complication here is that nd_tables[] is populated by
default (and also with -net nic -net user) but not when someone
specifies a single card via -netdev.

From memory this was derived from the code in
https://git.qemu.org/?p=qemu.git;a=blob;f=hw/arm/realview.c;h=87cd1e583cd20b6d8a2beeef1cba6977496d4477;hb=a6e0344fa0e09413324835ae122c4cadd7890231#l259
if having another example helps?

ATB,

Mark.

[Qemu-devel] [PULL 12/22] fpu/softfloat: re-factor add/sub

We can now add float16_add/sub and use the common decompose and
canonicalize functions to have a single implementation for
float16/32/64 add and sub functions.

Signed-off-by: Alex Bennée 
Signed-off-by: Richard Henderson 
Reviewed-by: Philippe Mathieu-Daudé 

diff --git a/fpu/softfloat.c b/fpu/softfloat.c
index 568d95..2190e7de56 100644
--- a/fpu/softfloat.c
+++ b/fpu/softfloat.c
@@ -83,6 +83,7 @@ this code that are retained.
  * target-dependent and needs the TARGET_* macros.
  */
 #include "qemu/osdep.h"
+#include "qemu/bitops.h"
 #include "fpu/softfloat.h"
 
 /* We only need stdlib for abort() */
@@ -270,6 +271,470 @@ static const FloatFmt float64_params = {
 FLOAT_PARAMS(11, 52)
 };
 
+/* Unpack a float to parts, but do not canonicalize.  */
+static inline FloatParts unpack_raw(FloatFmt fmt, uint64_t raw)
+{
+const int sign_pos = fmt.frac_size + fmt.exp_size;
+
+return (FloatParts) {
+.cls = float_class_unclassified,
+.sign = extract64(raw, sign_pos, 1),
+.exp = extract64(raw, fmt.frac_size, fmt.exp_size),
+.frac = extract64(raw, 0, fmt.frac_size),
+};
+}
+
+static inline FloatParts float16_unpack_raw(float16 f)
+{
+return unpack_raw(float16_params, f);
+}
+
+static inline FloatParts float32_unpack_raw(float32 f)
+{
+return unpack_raw(float32_params, f);
+}
+
+static inline FloatParts float64_unpack_raw(float64 f)
+{
+return unpack_raw(float64_params, f);
+}
+
+/* Pack a float from parts, but do not canonicalize.  */
+static inline uint64_t pack_raw(FloatFmt fmt, FloatParts p)
+{
+const int sign_pos = fmt.frac_size + fmt.exp_size;
+uint64_t ret = deposit64(p.frac, fmt.frac_size, fmt.exp_size, p.exp);
+return deposit64(ret, sign_pos, 1, p.sign);
+}
+
+static inline float16 float16_pack_raw(FloatParts p)
+{
+return make_float16(pack_raw(float16_params, p));
+}
+
+static inline float32 float32_pack_raw(FloatParts p)
+{
+return make_float32(pack_raw(float32_params, p));
+}
+
+static inline float64 float64_pack_raw(FloatParts p)
+{
+return make_float64(pack_raw(float64_params, p));
+}
+
+/* Canonicalize EXP and FRAC, setting CLS.  */
+static FloatParts canonicalize(FloatParts part, const FloatFmt *parm,
+   float_status *status)
+{
+if (part.exp == parm->exp_max) {
+if (part.frac == 0) {
+part.cls = float_class_inf;
+} else {
+#ifdef NO_SIGNALING_NANS
+part.cls = float_class_qnan;
+#else
+int64_t msb = part.frac << (parm->frac_shift + 2);
+if ((msb < 0) == status->snan_bit_is_one) {
+part.cls = float_class_snan;
+} else {
+part.cls = float_class_qnan;
+}
+#endif
+}
+} else if (part.exp == 0) {
+if (likely(part.frac == 0)) {
+part.cls = float_class_zero;
+} else if (status->flush_inputs_to_zero) {
+float_raise(float_flag_input_denormal, status);
+part.cls = float_class_zero;
+part.frac = 0;
+} else {
+int shift = clz64(part.frac) - 1;
+part.cls = float_class_normal;
+part.exp = parm->frac_shift - parm->exp_bias - shift + 1;
+part.frac <<= shift;
+}
+} else {
+part.cls = float_class_normal;
+part.exp -= parm->exp_bias;
+part.frac = DECOMPOSED_IMPLICIT_BIT + (part.frac << parm->frac_shift);
+}
+return part;
+}
+
+/* Round and uncanonicalize a floating-point number by parts. There
+ * are FRAC_SHIFT bits that may require rounding at the bottom of the
+ * fraction; these bits will be removed. The exponent will be biased
+ * by EXP_BIAS and must be bounded by [EXP_MAX-1, 0].
+ */
+
+static FloatParts round_canonical(FloatParts p, float_status *s,
+  const FloatFmt *parm)
+{
+const uint64_t frac_lsbm1 = parm->frac_lsbm1;
+const uint64_t round_mask = parm->round_mask;
+const uint64_t roundeven_mask = parm->roundeven_mask;
+const int exp_max = parm->exp_max;
+const int frac_shift = parm->frac_shift;
+uint64_t frac, inc;
+int exp, flags = 0;
+bool overflow_norm;
+
+frac = p.frac;
+exp = p.exp;
+
+switch (p.cls) {
+case float_class_normal:
+switch (s->float_rounding_mode) {
+case float_round_nearest_even:
+overflow_norm = false;
+inc = ((frac & roundeven_mask) != frac_lsbm1 ? frac_lsbm1 : 0);
+break;
+case float_round_ties_away:
+overflow_norm = false;
+inc = frac_lsbm1;
+break;
+case float_round_to_zero:
+overflow_norm = true;
+inc = 0;
+break;
+case float_round_up:
+inc = p.sign ? 0 : round_mask;
+overflow_norm = p.sign;
+break;
+case float_round_down:
+inc

[Qemu-devel] [PULL 13/22] fpu/softfloat: re-factor mul

We can now add float16_mul and use the common decompose and
canonicalize functions to have a single implementation for
float16/32/64 versions.

Signed-off-by: Alex Bennée 
Signed-off-by: Richard Henderson 
Reviewed-by: Peter Maydell 

diff --git a/fpu/softfloat.c b/fpu/softfloat.c
index 2190e7de56..6d29e1a103 100644
--- a/fpu/softfloat.c
+++ b/fpu/softfloat.c
@@ -735,6 +735,87 @@ float64 __attribute__((flatten)) float64_sub(float64 a, 
float64 b,
 return float64_round_pack_canonical(pr, status);
 }
 
+/*
+ * Returns the result of multiplying the floating-point values `a' and
+ * `b'. The operation is performed according to the IEC/IEEE Standard
+ * for Binary Floating-Point Arithmetic.
+ */
+
+static FloatParts mul_floats(FloatParts a, FloatParts b, float_status *s)
+{
+bool sign = a.sign ^ b.sign;
+
+if (a.cls == float_class_normal && b.cls == float_class_normal) {
+uint64_t hi, lo;
+int exp = a.exp + b.exp;
+
+mul64To128(a.frac, b.frac, , );
+shift128RightJamming(hi, lo, DECOMPOSED_BINARY_POINT, , );
+if (lo & DECOMPOSED_OVERFLOW_BIT) {
+shift64RightJamming(lo, 1, );
+exp += 1;
+}
+
+/* Re-use a */
+a.exp = exp;
+a.sign = sign;
+a.frac = lo;
+return a;
+}
+/* handle all the NaN cases */
+if (is_nan(a.cls) || is_nan(b.cls)) {
+return pick_nan(a, b, s);
+}
+/* Inf * Zero == NaN */
+if ((a.cls == float_class_inf && b.cls == float_class_zero) ||
+(a.cls == float_class_zero && b.cls == float_class_inf)) {
+s->float_exception_flags |= float_flag_invalid;
+a.cls = float_class_dnan;
+a.sign = sign;
+return a;
+}
+/* Multiply by 0 or Inf */
+if (a.cls == float_class_inf || a.cls == float_class_zero) {
+a.sign = sign;
+return a;
+}
+if (b.cls == float_class_inf || b.cls == float_class_zero) {
+b.sign = sign;
+return b;
+}
+g_assert_not_reached();
+}
+
+float16 __attribute__((flatten)) float16_mul(float16 a, float16 b,
+ float_status *status)
+{
+FloatParts pa = float16_unpack_canonical(a, status);
+FloatParts pb = float16_unpack_canonical(b, status);
+FloatParts pr = mul_floats(pa, pb, status);
+
+return float16_round_pack_canonical(pr, status);
+}
+
+float32 __attribute__((flatten)) float32_mul(float32 a, float32 b,
+ float_status *status)
+{
+FloatParts pa = float32_unpack_canonical(a, status);
+FloatParts pb = float32_unpack_canonical(b, status);
+FloatParts pr = mul_floats(pa, pb, status);
+
+return float32_round_pack_canonical(pr, status);
+}
+
+float64 __attribute__((flatten)) float64_mul(float64 a, float64 b,
+ float_status *status)
+{
+FloatParts pa = float64_unpack_canonical(a, status);
+FloatParts pb = float64_unpack_canonical(b, status);
+FloatParts pr = mul_floats(pa, pb, status);
+
+return float64_round_pack_canonical(pr, status);
+}
+
 /*
 | Takes a 64-bit fixed-point value `absZ' with binary point between bits 6
 | and 7, and returns the properly rounded 32-bit integer corresponding to the
@@ -2546,70 +2627,6 @@ float32 float32_round_to_int(float32 a, float_status 
*status)
 
 }
 
-/*
-| Returns the result of multiplying the single-precision floating-point values
-| `a' and `b'.  The operation is performed according to the IEC/IEEE Standard
-| for Binary Floating-Point Arithmetic.
-**/
-
-float32 float32_mul(float32 a, float32 b, float_status *status)
-{
-flag aSign, bSign, zSign;
-int aExp, bExp, zExp;
-uint32_t aSig, bSig;
-uint64_t zSig64;
-uint32_t zSig;
-
-a = float32_squash_input_denormal(a, status);
-b = float32_squash_input_denormal(b, status);
-
-aSig = extractFloat32Frac( a );
-aExp = extractFloat32Exp( a );
-aSign = extractFloat32Sign( a );
-bSig = extractFloat32Frac( b );
-bExp = extractFloat32Exp( b );
-bSign = extractFloat32Sign( b );
-zSign = aSign ^ bSign;
-if ( aExp == 0xFF ) {
-if ( aSig || ( ( bExp == 0xFF ) && bSig ) ) {
-return propagateFloat32NaN(a, b, status);
-}
-if ( ( bExp | bSig ) == 0 ) {
-float_raise(float_flag_invalid, status);
-return float32_default_nan(status);
-}
-return packFloat32( zSign, 0xFF, 0 );
-}
-if ( bExp == 0xFF ) {
-if (bSig) {
-return propagateFloat32NaN(a, b, status);
-}
-if ( ( aExp | aSig ) == 0 ) {
-float_raise(float_flag_invalid,

[Qemu-devel] [PATCH 05/12] egl-headless: switch over to new display registry

Signed-off-by: Gerd Hoffmann 
---
 include/ui/console.h |  3 ---
 ui/egl-headless.c| 20 +++-
 vl.c | 12 
 3 files changed, 19 insertions(+), 16 deletions(-)

diff --git a/include/ui/console.h b/include/ui/console.h
index 4794c98c9a..1832c7eccf 100644
--- a/include/ui/console.h
+++ b/include/ui/console.h
@@ -453,7 +453,4 @@ int vnc_init_func(void *opaque, QemuOpts *opts, Error 
**errp);
 /* input.c */
 int index_from_key(const char *key, size_t key_length);
 
-/* egl-headless.c */
-void egl_headless_init(DisplayOptions *opts);
-
 #endif
diff --git a/ui/egl-headless.c b/ui/egl-headless.c
index 38b3766548..655ef4eecb 100644
--- a/ui/egl-headless.c
+++ b/ui/egl-headless.c
@@ -154,7 +154,12 @@ static const DisplayChangeListenerOps egl_ops = {
 .dpy_gl_update   = egl_scanout_flush,
 };
 
-void egl_headless_init(DisplayOptions *opts)
+static void early_egl_headless_init(DisplayOptions *opts)
+{
+display_opengl = 1;
+}
+
+static void egl_headless_init(DisplayState *ds, DisplayOptions *opts)
 {
 QemuConsole *con;
 egl_dpy *edpy;
@@ -178,3 +183,16 @@ void egl_headless_init(DisplayOptions *opts)
 register_displaychangelistener(>dcl);
 }
 }
+
+static QemuDisplay qemu_display_egl = {
+.type   = DISPLAY_TYPE_EGL_HEADLESS,
+.early_init = early_egl_headless_init,
+.init   = egl_headless_init,
+};
+
+static void register_egl(void)
+{
+qemu_display_register(_display_egl);
+}
+
+type_init(register_egl);
diff --git a/vl.c b/vl.c
index 2b4af34fbb..47c953f8dc 100644
--- a/vl.c
+++ b/vl.c
@@ -2153,13 +2153,7 @@ static void parse_display(const char *p)
 exit(1);
 }
 } else if (strstart(p, "egl-headless", )) {
-#ifdef CONFIG_OPENGL_DMABUF
-display_opengl = 1;
 dpy.type = DISPLAY_TYPE_EGL_HEADLESS;
-#else
-error_report("egl support is disabled");
-exit(1);
-#endif
 } else if (strstart(p, "curses", )) {
 dpy.type = DISPLAY_TYPE_CURSES;
 } else if (strstart(p, "gtk", )) {
@@ -4659,12 +4653,6 @@ int main(int argc, char **argv, char **envp)
 qemu_spice_display_init();
 }
 
-#ifdef CONFIG_OPENGL_DMABUF
-if (dpy.type == DISPLAY_TYPE_EGL_HEADLESS) {
-egl_headless_init();
-}
-#endif
-
 if (foreach_device_config(DEV_GDB, gdbserver_start) < 0) {
 exit(1);
 }
-- 
2.9.3

[Qemu-devel] [PATCH v3 2/7] net: List available netdevs with "-netdev help"

Other options like "-chardev" or "-device" feature a nice help text
with the available devices when being called with "help" or "?".
Since it is quite useful, especially if you want to see which network
backends have been compiled into the QEMU binary, let's provide such
a help text for "-netdev", too.

Reviewed-by: Paolo Bonzini 
Reviewed-by: Eric Blake 
Signed-off-by: Thomas Huth 
---
 net/net.c | 37 -
 1 file changed, 36 insertions(+), 1 deletion(-)

diff --git a/net/net.c b/net/net.c
index e213a61..cf07e15 100644
--- a/net/net.c
+++ b/net/net.c
@@ -1086,6 +1086,38 @@ static int net_client_init1(const void *object, bool 
is_netdev, Error **errp)
 return 0;
 }
 
+static void show_netdevs(void)
+{
+int idx;
+const char *available_netdevs[] = {
+"socket",
+"hubport",
+"tap",
+#ifdef CONFIG_SLIRP
+"user",
+#endif
+#ifdef CONFIG_L2TPV3
+"l2tpv3",
+#endif
+#ifdef CONFIG_VDE
+"vde",
+#endif
+#ifdef CONFIG_NET_BRIDGE
+"bridge",
+#endif
+#ifdef CONFIG_NETMAP
+"netmap",
+#endif
+#ifdef CONFIG_POSIX
+"vhost-user",
+#endif
+};
+
+printf("Available netdev backend types:\n");
+for (idx = 0; idx < ARRAY_SIZE(available_netdevs); idx++) {
+puts(available_netdevs[idx]);
+}
+}
 
 int net_client_init(QemuOpts *opts, bool is_netdev, Error **errp)
 {
@@ -1094,7 +1126,10 @@ int net_client_init(QemuOpts *opts, bool is_netdev, 
Error **errp)
 int ret = -1;
 Visitor *v = opts_visitor_new(opts);
 
-{
+if (is_netdev && is_help_option(qemu_opt_get(opts, "type"))) {
+show_netdevs();
+exit(0);
+} else {
 /* Parse convenience option format ip6-net=fec0::0[/64] */
 const char *ip6_net = qemu_opt_get(opts, "ipv6-net");
 
-- 
1.8.3.1

Re: [Qemu-devel] [PATCH v4 4/5] usb-mtp: Introduce write support for MTP objects

> > +static void usb_mtp_write_data(MTPState *s)
> > +{
> > +MTPData *d = s->data_out;
> > +MTPObject *parent =
> > +usb_mtp_object_lookup(s, s->dataset.parent_handle);
> > +char *path = NULL;
> > +int rc = -1;
> > +mode_t mask = 0644;
> > +
> > +assert(d != NULL);
> > +
> 
> 
> Somewhere in here should surely be validating the "readonly" flag.
> 
> > +if (parent == NULL || !s->write_pending) {

Does happens here.  With a readonly device write_pending should
never be true.

> > +usb_mtp_queue_result(s, RES_INVALID_OBJECTINFO, d->trans,
> > + 0, 0, 0, 0);
> > +return;
> > +}

But adding an "assert(!readonly)" here as double-check surely doesn't hurt.

cheers,
  Gerd

[Qemu-devel] [PULL 09/22] fpu/softfloat: improve comments on ARM NaN propagation

Mention the pseudo-code fragment from which this is based.

Signed-off-by: Alex Bennée 
Reviewed-by: Richard Henderson 

diff --git a/fpu/softfloat-specialize.h b/fpu/softfloat-specialize.h
index de2c5d5702..4be0fb21ba 100644
--- a/fpu/softfloat-specialize.h
+++ b/fpu/softfloat-specialize.h
@@ -445,9 +445,10 @@ static float32 commonNaNToFloat32(commonNaNT a, 
float_status *status)
 
 #if defined(TARGET_ARM)
 static int pickNaN(flag aIsQNaN, flag aIsSNaN, flag bIsQNaN, flag bIsSNaN,
-flag aIsLargerSignificand)
+   flag aIsLargerSignificand)
 {
-/* ARM mandated NaN propagation rules: take the first of:
+/* ARM mandated NaN propagation rules (see FPProcessNaNs()), take
+ * the first of:
  *  1. A if it is signaling
  *  2. B if it is signaling
  *  3. A (quiet)
-- 
2.15.1

[Qemu-devel] [PULL 08/22] include/fpu/softfloat: add some float16 constants

This defines the same set of common constants for float 16 as defined
for 32 and 64 bit floats. These are often used by target helper
functions. I've also removed constants that are not used by anybody.

Signed-off-by: Alex Bennée 
Reviewed-by: Philippe Mathieu-Daudé 
Reviewed-by: Richard Henderson 

diff --git a/include/fpu/softfloat.h b/include/fpu/softfloat.h
index 59c06ef192..23824a3000 100644
--- a/include/fpu/softfloat.h
+++ b/include/fpu/softfloat.h
@@ -286,6 +286,11 @@ static inline float16 float16_set_sign(float16 a, int sign)
 return make_float16((float16_val(a) & 0x7fff) | (sign << 15));
 }
 
+#define float16_zero make_float16(0)
+#define float16_one make_float16(0x3c00)
+#define float16_half make_float16(0x3800)
+#define float16_infinity make_float16(0x7c00)
+
 /*
 | The pattern for a default generated half-precision NaN.
 **/
@@ -392,8 +397,6 @@ static inline float32 float32_set_sign(float32 a, int sign)
 
 #define float32_zero make_float32(0)
 #define float32_one make_float32(0x3f80)
-#define float32_ln2 make_float32(0x3f317218)
-#define float32_pi make_float32(0x40490fdb)
 #define float32_half make_float32(0x3f00)
 #define float32_infinity make_float32(0x7f80)
 
@@ -506,7 +509,6 @@ static inline float64 float64_set_sign(float64 a, int sign)
 #define float64_zero make_float64(0)
 #define float64_one make_float64(0x3ff0LL)
 #define float64_ln2 make_float64(0x3fe62e42fefa39efLL)
-#define float64_pi make_float64(0x400921fb54442d18LL)
 #define float64_half make_float64(0x3fe0LL)
 #define float64_infinity make_float64(0x7ff0LL)
 
-- 
2.15.1

[Qemu-devel] [PULL 17/22] fpu/softfloat: re-factor float to int/uint

We share the common int64/uint64_pack_decomposed function across all
the helpers and simply limit the final result depending on the final
size.

Signed-off-by: Alex Bennée 
Reviewed-by: Richard Henderson 

diff --git a/fpu/softfloat.c b/fpu/softfloat.c
index 616c6cef07..da0c43c0e7 100644
--- a/fpu/softfloat.c
+++ b/fpu/softfloat.c
@@ -1320,6 +1320,186 @@ float64 float64_trunc_to_int(float64 a, float_status *s)
 return float64_round_pack_canonical(pr, s);
 }
 
+/*
+ * Returns the result of converting the floating-point value `a' to
+ * the two's complement integer format. The conversion is performed
+ * according to the IEC/IEEE Standard for Binary Floating-Point
+ * Arithmetic---which means in particular that the conversion is
+ * rounded according to the current rounding mode. If `a' is a NaN,
+ * the largest positive integer is returned. Otherwise, if the
+ * conversion overflows, the largest integer with the same sign as `a'
+ * is returned.
+*/
+
+static int64_t round_to_int_and_pack(FloatParts in, int rmode,
+ int64_t min, int64_t max,
+ float_status *s)
+{
+uint64_t r;
+int orig_flags = get_float_exception_flags(s);
+FloatParts p = round_to_int(in, rmode, s);
+
+switch (p.cls) {
+case float_class_snan:
+case float_class_qnan:
+return max;
+case float_class_inf:
+return p.sign ? min : max;
+case float_class_zero:
+return 0;
+case float_class_normal:
+if (p.exp < DECOMPOSED_BINARY_POINT) {
+r = p.frac >> (DECOMPOSED_BINARY_POINT - p.exp);
+} else if (p.exp - DECOMPOSED_BINARY_POINT < 2) {
+r = p.frac << (p.exp - DECOMPOSED_BINARY_POINT);
+} else {
+r = UINT64_MAX;
+}
+if (p.sign) {
+if (r < -(uint64_t) min) {
+return -r;
+} else {
+s->float_exception_flags = orig_flags | float_flag_invalid;
+return min;
+}
+} else {
+if (r < max) {
+return r;
+} else {
+s->float_exception_flags = orig_flags | float_flag_invalid;
+return max;
+}
+}
+default:
+g_assert_not_reached();
+}
+}
+
+#define FLOAT_TO_INT(fsz, isz)  \
+int ## isz ## _t float ## fsz ## _to_int ## isz(float ## fsz a, \
+float_status *s)\
+{   \
+FloatParts p = float ## fsz ## _unpack_canonical(a, s); \
+return round_to_int_and_pack(p, s->float_rounding_mode, \
+ INT ## isz ## _MIN, INT ## isz ## _MAX,\
+ s);\
+}   \
+\
+int ## isz ## _t float ## fsz ## _to_int ## isz ## _round_to_zero   \
+ (float ## fsz a, float_status *s)  \
+{   \
+FloatParts p = float ## fsz ## _unpack_canonical(a, s); \
+return round_to_int_and_pack(p, float_round_to_zero,\
+ INT ## isz ## _MIN, INT ## isz ## _MAX,\
+ s);\
+}
+
+FLOAT_TO_INT(16, 16)
+FLOAT_TO_INT(16, 32)
+FLOAT_TO_INT(16, 64)
+
+FLOAT_TO_INT(32, 16)
+FLOAT_TO_INT(32, 32)
+FLOAT_TO_INT(32, 64)
+
+FLOAT_TO_INT(64, 16)
+FLOAT_TO_INT(64, 32)
+FLOAT_TO_INT(64, 64)
+
+#undef FLOAT_TO_INT
+
+/*
+ *  Returns the result of converting the floating-point value `a' to
+ *  the unsigned integer format. The conversion is performed according
+ *  to the IEC/IEEE Standard for Binary Floating-Point
+ *  Arithmetic---which means in particular that the conversion is
+ *  rounded according to the current rounding mode. If `a' is a NaN,
+ *  the largest unsigned integer is returned. Otherwise, if the
+ *  conversion overflows, the largest unsigned integer is returned. If
+ *  the 'a' is negative, the result is rounded and zero is returned;
+ *  values that do not round to zero will raise the inexact exception
+ *  flag.
+ */
+
+static uint64_t round_to_uint_and_pack(FloatParts in, int rmode, uint64_t max,
+   float_status *s)
+{
+int orig_flags = get_float_exception_flags(s);
+FloatParts p = round_to_int(in, rmode, s);
+
+switch (p.cls) {
+case float_class_snan:
+case float_class_qnan:
+s->float_exception_flags = orig_flags | float_flag_invalid;
+return max;
+case float_class_inf:
+return p.sign ? 0 : max;
+case

[Qemu-devel] QEMU & KVM are participating in Outreachy May-August 2018!

QEMU & KVM are participating in the Outreachy open source internship
program that offers 12-week, paid, remote work internships for
individuals from underrepresented groups in open source.

The application period is from February 12 to March 22 2018.  The
internships themselves run May-August.

Our project ideas page is currently being populated and will have more
projects over the coming days:
https://www.outreachy.org/2018-may-august/communities/qemu/

Let me know if you have questions or drop by #qemu-outreachy on
irc.oftc.net to chat with mentors!

Stefan

Re: [Qemu-devel] [qemu-s390x] [PATCH 1/1] 390x/cpumodel: document S390FeatDef.bit not applicable

2018-02-21 Thread Halil Pasic



On 02/20/2018 05:25 PM, Cornelia Huck wrote:
> On Tue, 20 Feb 2018 17:08:52 +0100
> David Hildenbrand  wrote:
> 
>> On 20.02.2018 17:07, Cornelia Huck wrote:
>>> On Tue, 20 Feb 2018 17:04:19 +0100
>>> Christian Borntraeger  wrote:
>>>   
 On 02/20/2018 04:55 PM, David Hildenbrand wrote:  
> On 20.02.2018 16:53, Cornelia Huck wrote:
>> On Tue, 20 Feb 2018 16:07:13 +0100
>> Halil Pasic  wrote:
>>
>>> The 'bit' field of the 'S390FeatDef' structure is not applicable to all
>>> it's instances. Currently a this field is not applicable, and remains   
>>>  
>>
>> s/it's/its/
>>
>> s/a this/this/
>>
>>> unused, iff the feature is of type S390_FEAT_TYPE_MISC. Having the 
>>> value 0
>>> specified for multiple such feature definition  was a little confusing,
>>> as it's a perfectly legit bit value, and as usually the value of the bit
>>> field is ought to be unique for each feature.
>>>
>>> Let's document this, and hopefully reduce the potential for confusion.
>>>
>>> Signed-off-by: Halil Pasic 
>>> ---
>>>
>>> Hi!
>>>
>>> This may be an overkill. A comment where the misc features
>>> are defined would do to, but I think this is nicer. So
>>> I decided to try it with this approach first.
>>
>> Is there likely to be anything else than FEAT_MISC _not_ using .bit? If
>> not, would it be better to at a comment to the FEAT_MISC definition?
>
> Doubt it right now. I would sign the "overkill" part :)

 I can cconfirm that this code caused some questions and it took me some
 minutes to remember why 0 and 0 was ok. So I certainly want to have a 
 comment
 of some form.
  
>>>
>>> I'd prefer a comment about FEAT_MISC usage rather than a magic value.
>>>   
>>
>> We can also add FEAT_INIT_MISC. And add a comment in the initializer.
>>
> 
> That's what I like best.
> 

OK, seems we have a winner: I will redo this with
#define FEAT_INIT_MISC(_name, _desc) \  
FEAT_INIT(_name, S390_FEAT_TYPE_MISC, 0, _desc)

Everybody thanks for the comments.

Regards,
Halil

[Qemu-devel] [PATCH 11/12] gtk: build as ui module

Also drop gtk and vte libs from libs_softmmu, so the libs are not
pulled in unless the gtk module actually gets loaded.

Shared library dependencies dropped from qemu-system-*:

libEGL.so.1 => /lib64/libEGL.so.1
libGL.so.1 => /lib64/libGL.so.1
libXcomposite.so.1 => /lib64/libXcomposite.so.1
libXcursor.so.1 => /lib64/libXcursor.so.1
libXdamage.so.1 => /lib64/libXdamage.so.1
libXfixes.so.3 => /lib64/libXfixes.so.3
libXinerama.so.1 => /lib64/libXinerama.so.1
libXrandr.so.2 => /lib64/libXrandr.so.2
libXrender.so.1 => /lib64/libXrender.so.1
libXxf86vm.so.1 => /lib64/libXxf86vm.so.1
libatk-1.0.so.0 => /lib64/libatk-1.0.so.0
libatk-bridge-2.0.so.0 => /lib64/libatk-bridge-2.0.so.0
libatspi.so.0 => /lib64/libatspi.so.0
libcairo-gobject.so.2 => /lib64/libcairo-gobject.so.2
libcairo.so.2 => /lib64/libcairo.so.2
libfontconfig.so.1 => /lib64/libfontconfig.so.1
libfreetype.so.6 => /lib64/libfreetype.so.6
libgdk-3.so.0 => /lib64/libgdk-3.so.0
libgdk_pixbuf-2.0.so.0 => /lib64/libgdk_pixbuf-2.0.so.0
libglapi.so.0 => /lib64/libglapi.so.0
libgraphite2.so.3 => /lib64/libgraphite2.so.3
libgtk-3.so.0 => /lib64/libgtk-3.so.0
libharfbuzz.so.0 => /lib64/libharfbuzz.so.0
libpango-1.0.so.0 => /lib64/libpango-1.0.so.0
libpangocairo-1.0.so.0 => /lib64/libpangocairo-1.0.so.0
libpangoft2-1.0.so.0 => /lib64/libpangoft2-1.0.so.0
libpcre2-8.so.0 => /lib64/libpcre2-8.so.0
libthai.so.0 => /lib64/libthai.so.0
libvte-2.91.so.0 => /lib64/libvte-2.91.so.0
libwayland-cursor.so.0 => /lib64/libwayland-cursor.so.0
libwayland-egl.so.1 => /lib64/libwayland-egl.so.1
libxcb-dri2.so.0 => /lib64/libxcb-dri2.so.0
libxcb-dri3.so.0 => /lib64/libxcb-dri3.so.0
libxcb-glx.so.0 => /lib64/libxcb-glx.so.0
libxcb-present.so.0 => /lib64/libxcb-present.so.0
libxcb-render.so.0 => /lib64/libxcb-render.so.0
libxcb-shm.so.0 => /lib64/libxcb-shm.so.0
libxcb-sync.so.1 => /lib64/libxcb-sync.so.1
libxcb-xfixes.so.0 => /lib64/libxcb-xfixes.so.0
libxkbcommon.so.0 => /lib64/libxkbcommon.so.0
libxshmfence.so.1 => /lib64/libxshmfence.so.1

Signed-off-by: Gerd Hoffmann 
---
 configure|  5 ++---
 ui/Makefile.objs | 17 +
 2 files changed, 11 insertions(+), 11 deletions(-)

diff --git a/configure b/configure
index fe80940bdc..cfab17275f 100755
--- a/configure
+++ b/configure
@@ -2546,7 +2546,6 @@ if test "$gtk" != "no"; then
 gtk_cflags="$gtk_cflags $x11_cflags"
 gtk_libs="$gtk_libs $x11_libs"
 fi
-libs_softmmu="$gtk_libs $libs_softmmu"
 gtk="yes"
 elif test "$gtk" = "yes"; then
 feature_not_found "gtk" "Install gtk3-devel"
@@ -2796,7 +2795,6 @@ if test "$vte" != "no"; then
 vte_cflags=$($pkg_config --cflags $vtepackage)
 vte_libs=$($pkg_config --libs $vtepackage)
 vteversion=$($pkg_config --modversion $vtepackage)
-libs_softmmu="$vte_libs $libs_softmmu"
 vte="yes"
 elif test "$vte" = "yes"; then
 if test "$gtkabi" = "3.0"; then
@@ -6136,7 +6134,7 @@ if test "$glib_subprocess" = "yes" ; then
   echo "CONFIG_HAS_GLIB_SUBPROCESS_TESTS=y" >> $config_host_mak
 fi
 if test "$gtk" = "yes" ; then
-  echo "CONFIG_GTK=y" >> $config_host_mak
+  echo "CONFIG_GTK=m" >> $config_host_mak
   echo "CONFIG_GTKABI=$gtkabi" >> $config_host_mak
   echo "GTK_CFLAGS=$gtk_cflags" >> $config_host_mak
   echo "GTK_LIBS=$gtk_libs" >> $config_host_mak
@@ -6187,6 +6185,7 @@ fi
 if test "$vte" = "yes" ; then
   echo "CONFIG_VTE=y" >> $config_host_mak
   echo "VTE_CFLAGS=$vte_cflags" >> $config_host_mak
+  echo "VTE_LIBS=$vte_libs" >> $config_host_mak
 fi
 if test "$virglrenderer" = "yes" ; then
   echo "CONFIG_VIRGL=y" >> $config_host_mak
diff --git a/ui/Makefile.objs b/ui/Makefile.objs
index ef4bd83fde..9a0e8a94f1 100644
--- a/ui/Makefile.objs
+++ b/ui/Makefile.objs
@@ -15,7 +15,6 @@ common-obj-$(CONFIG_COCOA) += cocoa.o
 common-obj-$(CONFIG_CURSES) += curses.o
 common-obj-$(CONFIG_VNC) += $(vnc-obj-y)
 common-obj-$(call lnot,$(CONFIG_VNC)) += vnc-stubs.o
-common-obj-$(CONFIG_GTK) += gtk.o
 
 common-obj-$(CONFIG_X11) += x_keymap.o
 x_keymap.o-cflags := $(X11_CFLAGS)
@@ -35,6 +34,12 @@ endif
 sdl.mo-cflags := $(SDL_CFLAGS)
 sdl.mo-libs := $(SDL_LIBS)
 
+# ui-gtk module
+common-obj-$(CONFIG_GTK) += gtk.mo
+gtk.mo-objs := gtk.o
+gtk.mo-cflags := $(GTK_CFLAGS) $(VTE_CFLAGS)
+gtk.mo-libs := $(GTK_LIBS) $(VTE_LIBS)
+
 ifeq ($(CONFIG_OPENGL),y)
 common-obj-y += shader.o
 common-obj-y += console-gl.o
@@ -42,17 +47,13 @@ common-obj-y += egl-helpers.o
 common-obj-y += egl-context.o
 common-obj-$(CONFIG_OPENGL_DMABUF) += egl-headless.o
 ifeq ($(CONFIG_GTK_GL),y)
-common-obj-$(CONFIG_GTK) += gtk-gl-area.o
+gtk.mo-objs += gtk-gl-area.o
 else
-common-obj-$(CONFIG_GTK) += gtk-egl.o
+gtk.mo-objs += gtk-egl.o
+gtk.mo-libs += $(OPENGL_LIBS)
 endif
 endif
 
-gtk.o-cflags := $(GTK_CFLAGS) $(VTE_CFLAGS)
-gtk-egl.o-cflags := $(GTK_CFLAGS) $(VTE_CFLAGS)
-gtk-gl-area.o-cflags := $(GTK_CFLAGS) $(VTE_CFLAGS)
-
-gtk-egl.o-libs += $(OPENGL_LIBS)
 shader.o-libs += $(OPENGL_LIBS)

[Qemu-devel] [PATCH 07/12] console: add ui module loading support

If a requested user interface is not available, try loading it as
module, simliar to block layer modules.  Needed to keep things working
when followup patches start to build user interfaces as modules.

Signed-off-by: Gerd Hoffmann 
---
 include/qemu/module.h | 1 +
 ui/console.c  | 6 ++
 2 files changed, 7 insertions(+)

diff --git a/include/qemu/module.h b/include/qemu/module.h
index 56dd218205..9fea75aaeb 100644
--- a/include/qemu/module.h
+++ b/include/qemu/module.h
@@ -53,6 +53,7 @@ typedef enum {
 #define trace_init(function) module_init(function, MODULE_INIT_TRACE)
 
 #define block_module_load_one(lib) module_load_one("block-", lib)
+#define ui_module_load_one(lib) module_load_one("ui-", lib)
 
 void register_module_init(void (*fn)(void), module_init_type type);
 void register_dso_module_init(void (*fn)(void), module_init_type type);
diff --git a/ui/console.c b/ui/console.c
index 5a63e9dfa2..dd663b9127 100644
--- a/ui/console.c
+++ b/ui/console.c
@@ -2189,6 +2189,9 @@ bool qemu_display_find_default(DisplayOptions *opts)
 
 for (i = 0; i < ARRAY_SIZE(prio); i++) {
 if (dpys[prio[i]] == NULL) {
+ui_module_load_one(DisplayType_lookup.array[prio[i]]);
+}
+if (dpys[prio[i]] == NULL) {
 continue;
 }
 opts->type = prio[i];
@@ -2204,6 +2207,9 @@ void qemu_display_early_init(DisplayOptions *opts)
 return;
 }
 if (dpys[opts->type] == NULL) {
+ui_module_load_one(DisplayType_lookup.array[opts->type]);
+}
+if (dpys[opts->type] == NULL) {
 error_report("Display '%s' is not available.",
  DisplayType_lookup.array[opts->type]);
 exit(1);
-- 
2.9.3

Re: [Qemu-devel] [PATCH 19/19] mps2-an505: New board model: MPS2 with AN505 Cortex-M33 FPGA image

2018-02-21 Thread Igor Mammedov

On Tue, 20 Feb 2018 18:03:25 +
Peter Maydell  wrote:

> Define a new board model for the MPS2 with an AN505 FPGA image
> containing a Cortex-M33. Since the FPGA images for TrustZone
> cores (AN505, and the similar AN519 for Cortex-M23) have a
> significantly different layout of devices to the non-TrustZone
> images, we use a new source file rather than shoehorning them
> into the existing mps2.c.
> 
> Signed-off-by: Peter Maydell 
> ---
>  hw/arm/Makefile.objs |   1 +
>  hw/arm/mps2-tz.c | 504 
> +++
>  2 files changed, 505 insertions(+)
>  create mode 100644 hw/arm/mps2-tz.c
> 
> diff --git a/hw/arm/Makefile.objs b/hw/arm/Makefile.objs
> index 79cd30bb92..232258160a 100644
> --- a/hw/arm/Makefile.objs
> +++ b/hw/arm/Makefile.objs
> @@ -19,5 +19,6 @@ obj-$(CONFIG_FSL_IMX31) += fsl-imx31.o kzm.o
>  obj-$(CONFIG_FSL_IMX6) += fsl-imx6.o sabrelite.o
>  obj-$(CONFIG_ASPEED_SOC) += aspeed_soc.o aspeed.o
>  obj-$(CONFIG_MPS2) += mps2.o
> +obj-$(CONFIG_MPS2) += mps2-tz.o
>  obj-$(CONFIG_MSF2) += msf2-soc.o msf2-som.o
>  obj-$(CONFIG_IOTKIT) += iotkit.o
> diff --git a/hw/arm/mps2-tz.c b/hw/arm/mps2-tz.c
> new file mode 100644
> index 00..ff414c649c
> --- /dev/null
> +++ b/hw/arm/mps2-tz.c
> @@ -0,0 +1,504 @@
> +/*
> + * ARM V2M MPS2 board emulation, trustzone aware FPGA images
> + *
> + * Copyright (c) 2017 Linaro Limited
> + * Written by Peter Maydell
> + *
> + *  This program is free software; you can redistribute it and/or modify
> + *  it under the terms of the GNU General Public License version 2 or
> + *  (at your option) any later version.
> + */
> +
> +/* The MPS2 and MPS2+ dev boards are FPGA based (the 2+ has a bigger
> + * FPGA but is otherwise the same as the 2). Since the CPU itself
> + * and most of the devices are in the FPGA, the details of the board
> + * as seen by the guest depend significantly on the FPGA image.
> + * This source file covers the following FPGA images, for TrustZone cores:
> + *  "mps2-an505" -- Cortex-M33 as documented in ARM Application Note AN505
> + *
> + * Links to the TRM for the board itself and to the various Application
> + * Notes which document the FPGA images can be found here:
> + * 
> https://developer.arm.com/products/system-design/development-boards/fpga-prototyping-boards/mps2
> + *
> + * Board TRM:
> + * 
> http://infocenter.arm.com/help/topic/com.arm.doc.100112_0200_06_en/versatile_express_cortex_m_prototyping_systems_v2m_mps2_and_v2m_mps2plus_technical_reference_100112_0200_06_en.pdf
> + * Application Note AN505:
> + * http://infocenter.arm.com/help/topic/com.arm.doc.dai0505b/index.html
> + *
> + * The AN505 defers to the Cortex-M33 processor ARMv8M IoT Kit FVP User Guide
> + * (ARM ECM0601256) for the details of some of the device layout:
> + *   
> http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.ecm0601256/index.html
> + */
> +
> +#include "qemu/osdep.h"
> +#include "qapi/error.h"
> +#include "qemu/error-report.h"
> +#include "hw/arm/arm.h"
> +#include "hw/arm/armv7m.h"
> +#include "hw/or-irq.h"
> +#include "hw/boards.h"
> +#include "exec/address-spaces.h"
> +#include "sysemu/sysemu.h"
> +#include "hw/misc/unimp.h"
> +#include "hw/char/cmsdk-apb-uart.h"
> +#include "hw/timer/cmsdk-apb-timer.h"
> +#include "hw/misc/mps2-scc.h"
> +#include "hw/misc/mps2-fpgaio.h"
> +#include "hw/arm/iotkit.h"
> +#include "hw/devices.h"
> +#include "net/net.h"
> +#include "hw/core/split-irq.h"
> +
> +typedef enum MPS2TZFPGAType {
> +FPGA_AN505,
> +} MPS2TZFPGAType;
> +
> +typedef struct {
> +MachineClass parent;
> +MPS2TZFPGAType fpga_type;
> +const char *cpu_model;
I don't see it used in this patch, should it be removed?


> +uint32_t scc_id;
> +} MPS2TZMachineClass;
> +
[...]

[Qemu-devel] [PULL 16/22] fpu/softfloat: re-factor round_to_int

We can now add float16_round_to_int and use the common round_decomposed and
canonicalize functions to have a single implementation for
float16/32/64 round_to_int functions.

Signed-off-by: Alex Bennée 
Signed-off-by: Richard Henderson 
Reviewed-by: Peter Maydell 

diff --git a/fpu/softfloat.c b/fpu/softfloat.c
index ae4ba6de51..616c6cef07 100644
--- a/fpu/softfloat.c
+++ b/fpu/softfloat.c
@@ -560,6 +560,25 @@ static bool is_qnan(FloatClass c)
 return c == float_class_qnan;
 }
 
+static FloatParts return_nan(FloatParts a, float_status *s)
+{
+switch (a.cls) {
+case float_class_snan:
+s->float_exception_flags |= float_flag_invalid;
+a.cls = float_class_msnan;
+/* fall through */
+case float_class_qnan:
+if (s->default_nan_mode) {
+a.cls = float_class_dnan;
+}
+break;
+
+default:
+g_assert_not_reached();
+}
+return a;
+}
+
 static FloatParts pick_nan(FloatParts a, FloatParts b, float_status *s)
 {
 if (is_snan(a.cls) || is_snan(b.cls)) {
@@ -1175,6 +1194,132 @@ float64 float64_div(float64 a, float64 b, float_status 
*status)
 return float64_round_pack_canonical(pr, status);
 }
 
+/*
+ * Rounds the floating-point value `a' to an integer, and returns the
+ * result as a floating-point value. The operation is performed
+ * according to the IEC/IEEE Standard for Binary Floating-Point
+ * Arithmetic.
+ */
+
+static FloatParts round_to_int(FloatParts a, int rounding_mode, float_status 
*s)
+{
+if (is_nan(a.cls)) {
+return return_nan(a, s);
+}
+
+switch (a.cls) {
+case float_class_zero:
+case float_class_inf:
+case float_class_qnan:
+/* already "integral" */
+break;
+case float_class_normal:
+if (a.exp >= DECOMPOSED_BINARY_POINT) {
+/* already integral */
+break;
+}
+if (a.exp < 0) {
+bool one;
+/* all fractional */
+s->float_exception_flags |= float_flag_inexact;
+switch (rounding_mode) {
+case float_round_nearest_even:
+one = a.exp == -1 && a.frac > DECOMPOSED_IMPLICIT_BIT;
+break;
+case float_round_ties_away:
+one = a.exp == -1 && a.frac >= DECOMPOSED_IMPLICIT_BIT;
+break;
+case float_round_to_zero:
+one = false;
+break;
+case float_round_up:
+one = !a.sign;
+break;
+case float_round_down:
+one = a.sign;
+break;
+default:
+g_assert_not_reached();
+}
+
+if (one) {
+a.frac = DECOMPOSED_IMPLICIT_BIT;
+a.exp = 0;
+} else {
+a.cls = float_class_zero;
+}
+} else {
+uint64_t frac_lsb = DECOMPOSED_IMPLICIT_BIT >> a.exp;
+uint64_t frac_lsbm1 = frac_lsb >> 1;
+uint64_t rnd_even_mask = (frac_lsb - 1) | frac_lsb;
+uint64_t rnd_mask = rnd_even_mask >> 1;
+uint64_t inc;
+
+switch (rounding_mode) {
+case float_round_nearest_even:
+inc = ((a.frac & rnd_even_mask) != frac_lsbm1 ? frac_lsbm1 : 
0);
+break;
+case float_round_ties_away:
+inc = frac_lsbm1;
+break;
+case float_round_to_zero:
+inc = 0;
+break;
+case float_round_up:
+inc = a.sign ? 0 : rnd_mask;
+break;
+case float_round_down:
+inc = a.sign ? rnd_mask : 0;
+break;
+default:
+g_assert_not_reached();
+}
+
+if (a.frac & rnd_mask) {
+s->float_exception_flags |= float_flag_inexact;
+a.frac += inc;
+a.frac &= ~rnd_mask;
+if (a.frac & DECOMPOSED_OVERFLOW_BIT) {
+a.frac >>= 1;
+a.exp++;
+}
+}
+}
+break;
+default:
+g_assert_not_reached();
+}
+return a;
+}
+
+float16 float16_round_to_int(float16 a, float_status *s)
+{
+FloatParts pa = float16_unpack_canonical(a, s);
+FloatParts pr = round_to_int(pa, s->float_rounding_mode, s);
+return float16_round_pack_canonical(pr, s);
+}
+
+float32 float32_round_to_int(float32 a, float_status *s)
+{
+FloatParts pa = float32_unpack_canonical(a, s);
+FloatParts pr = round_to_int(pa, s->float_rounding_mode, s);
+return float32_round_pack_canonical(pr, s);
+}
+
+float64 float64_round_to_int(float64 a, float_status *s)
+{
+FloatParts pa = float64_unpack_canonical(a, s);
+FloatParts pr = round_to_int(pa, s->float_rounding_mode, s);
+return

[Qemu-devel] [PULL 15/22] fpu/softfloat: re-factor muladd

We can now add float16_muladd and use the common decompose and
canonicalize functions to have a single implementation for
float16/32/64 muladd functions.

Signed-off-by: Alex Bennée 
Signed-off-by: Richard Henderson 
Reviewed-by: Peter Maydell 

diff --git a/fpu/softfloat-specialize.h b/fpu/softfloat-specialize.h
index 4be0fb21ba..e81ca001e1 100644
--- a/fpu/softfloat-specialize.h
+++ b/fpu/softfloat-specialize.h
@@ -729,58 +729,6 @@ static float32 propagateFloat32NaN(float32 a, float32 b, 
float_status *status)
 }
 }
 
-/*
-| Takes three single-precision floating-point values `a', `b' and `c', one of
-| which is a NaN, and returns the appropriate NaN result.  If any of  `a',
-| `b' or `c' is a signaling NaN, the invalid exception is raised.
-| The input infzero indicates whether a*b was 0*inf or inf*0 (in which case
-| obviously c is a NaN, and whether to propagate c or some other NaN is
-| implementation defined).
-**/
-
-static float32 propagateFloat32MulAddNaN(float32 a, float32 b,
- float32 c, flag infzero,
- float_status *status)
-{
-flag aIsQuietNaN, aIsSignalingNaN, bIsQuietNaN, bIsSignalingNaN,
-cIsQuietNaN, cIsSignalingNaN;
-int which;
-
-aIsQuietNaN = float32_is_quiet_nan(a, status);
-aIsSignalingNaN = float32_is_signaling_nan(a, status);
-bIsQuietNaN = float32_is_quiet_nan(b, status);
-bIsSignalingNaN = float32_is_signaling_nan(b, status);
-cIsQuietNaN = float32_is_quiet_nan(c, status);
-cIsSignalingNaN = float32_is_signaling_nan(c, status);
-
-if (aIsSignalingNaN | bIsSignalingNaN | cIsSignalingNaN) {
-float_raise(float_flag_invalid, status);
-}
-
-which = pickNaNMulAdd(aIsQuietNaN, aIsSignalingNaN,
-  bIsQuietNaN, bIsSignalingNaN,
-  cIsQuietNaN, cIsSignalingNaN, infzero, status);
-
-if (status->default_nan_mode) {
-/* Note that this check is after pickNaNMulAdd so that function
- * has an opportunity to set the Invalid flag.
- */
-return float32_default_nan(status);
-}
-
-switch (which) {
-case 0:
-return float32_maybe_silence_nan(a, status);
-case 1:
-return float32_maybe_silence_nan(b, status);
-case 2:
-return float32_maybe_silence_nan(c, status);
-case 3:
-default:
-return float32_default_nan(status);
-}
-}
-
 #ifdef NO_SIGNALING_NANS
 int float64_is_quiet_nan(float64 a_, float_status *status)
 {
@@ -936,58 +884,6 @@ static float64 propagateFloat64NaN(float64 a, float64 b, 
float_status *status)
 }
 }
 
-/*
-| Takes three double-precision floating-point values `a', `b' and `c', one of
-| which is a NaN, and returns the appropriate NaN result.  If any of  `a',
-| `b' or `c' is a signaling NaN, the invalid exception is raised.
-| The input infzero indicates whether a*b was 0*inf or inf*0 (in which case
-| obviously c is a NaN, and whether to propagate c or some other NaN is
-| implementation defined).
-**/
-
-static float64 propagateFloat64MulAddNaN(float64 a, float64 b,
- float64 c, flag infzero,
- float_status *status)
-{
-flag aIsQuietNaN, aIsSignalingNaN, bIsQuietNaN, bIsSignalingNaN,
-cIsQuietNaN, cIsSignalingNaN;
-int which;
-
-aIsQuietNaN = float64_is_quiet_nan(a, status);
-aIsSignalingNaN = float64_is_signaling_nan(a, status);
-bIsQuietNaN = float64_is_quiet_nan(b, status);
-bIsSignalingNaN = float64_is_signaling_nan(b, status);
-cIsQuietNaN = float64_is_quiet_nan(c, status);
-cIsSignalingNaN = float64_is_signaling_nan(c, status);
-
-if (aIsSignalingNaN | bIsSignalingNaN | cIsSignalingNaN) {
-float_raise(float_flag_invalid, status);
-}
-
-which = pickNaNMulAdd(aIsQuietNaN, aIsSignalingNaN,
-  bIsQuietNaN, bIsSignalingNaN,
-  cIsQuietNaN, cIsSignalingNaN, infzero, status);
-
-if (status->default_nan_mode) {
-/* Note that this check is after pickNaNMulAdd so that function
- * has an opportunity to set the Invalid flag.
- */
-return float64_default_nan(status);
-}
-
-switch (which) {
-case 0:
-return float64_maybe_silence_nan(a, status);
-case 1:
-return float64_maybe_silence_nan(b, status);
-case 2:
-return float64_maybe_silence_nan(c, status);
-case 3:
-default:
-return float64_default_nan(status);
-}
-}
-
 #ifdef NO_SIGNALING_NANS

[Qemu-devel] [PATCH 12/12] curses: build as ui module

Also drop curses libs from libs_softmmu.  Add CURSES_{CFLAGS,LIBS}
variables so we can use them for linking the curses module.

Shared library dependencies dropped from qemu-system-*:

libncursesw.so.5 => /lib64/libncursesw.so.5
libtinfo.so.5 => /lib64/libtinfo.so.5

Signed-off-by: Gerd Hoffmann 
---
 configure| 6 +++---
 ui/Makefile.objs | 6 +-
 2 files changed, 8 insertions(+), 4 deletions(-)

diff --git a/configure b/configure
index cfab17275f..4d0100ec7e 100755
--- a/configure
+++ b/configure
@@ -3268,8 +3268,6 @@ EOF
   unset IFS
   if compile_prog "$curses_inc" "$curses_lib" ; then
 curses_found=yes
-QEMU_CFLAGS="$curses_inc $QEMU_CFLAGS"
-libs_softmmu="$curses_lib $libs_softmmu"
 break
   fi
 done
@@ -6037,7 +6035,9 @@ if test "$cocoa" = "yes" ; then
   echo "CONFIG_COCOA=y" >> $config_host_mak
 fi
 if test "$curses" = "yes" ; then
-  echo "CONFIG_CURSES=y" >> $config_host_mak
+  echo "CONFIG_CURSES=m" >> $config_host_mak
+  echo "CURSES_CFLAGS=$curses_inc" >> $config_host_mak
+  echo "CURSES_LIBS=$curses_lib" >> $config_host_mak
 fi
 if test "$pipe2" = "yes" ; then
   echo "CONFIG_PIPE2=y" >> $config_host_mak
diff --git a/ui/Makefile.objs b/ui/Makefile.objs
index 9a0e8a94f1..dcd54a5287 100644
--- a/ui/Makefile.objs
+++ b/ui/Makefile.objs
@@ -12,7 +12,6 @@ common-obj-y += input.o input-keymap.o input-legacy.o
 common-obj-$(CONFIG_LINUX) += input-linux.o
 common-obj-$(CONFIG_SPICE) += spice-core.o spice-input.o spice-display.o
 common-obj-$(CONFIG_COCOA) += cocoa.o
-common-obj-$(CONFIG_CURSES) += curses.o
 common-obj-$(CONFIG_VNC) += $(vnc-obj-y)
 common-obj-$(call lnot,$(CONFIG_VNC)) += vnc-stubs.o
 
@@ -40,6 +39,11 @@ gtk.mo-objs := gtk.o
 gtk.mo-cflags := $(GTK_CFLAGS) $(VTE_CFLAGS)
 gtk.mo-libs := $(GTK_LIBS) $(VTE_LIBS)
 
+common-obj-$(CONFIG_CURSES) += curses.mo
+curses.mo-objs := curses.o
+curses.mo-cflags := $(CURSES_CFLAGS)
+curses.mo-libs := $(CURSES_LIBS)
+
 ifeq ($(CONFIG_OPENGL),y)
 common-obj-y += shader.o
 common-obj-y += console-gl.o
-- 
2.9.3

[Qemu-devel] [PATCH 04/12] curses: switch over to new display registry

Signed-off-by: Gerd Hoffmann 
---
 include/ui/console.h | 12 
 ui/curses.c  | 14 +-
 vl.c | 17 ++---
 3 files changed, 15 insertions(+), 28 deletions(-)

diff --git a/include/ui/console.h b/include/ui/console.h
index b97d9ccae4..4794c98c9a 100644
--- a/include/ui/console.h
+++ b/include/ui/console.h
@@ -450,18 +450,6 @@ int vnc_display_pw_expire(const char *id, time_t expires);
 QemuOpts *vnc_parse(const char *str, Error **errp);
 int vnc_init_func(void *opaque, QemuOpts *opts, Error **errp);
 
-/* curses.c */
-#ifdef CONFIG_CURSES
-void curses_display_init(DisplayState *ds, DisplayOptions *opts);
-#else
-static inline void curses_display_init(DisplayState *ds, DisplayOptions *opts)
-{
-/* This must never be called if CONFIG_CURSES is disabled */
-error_report("curses support is disabled");
-abort();
-}
-#endif
-
 /* input.c */
 int index_from_key(const char *key, size_t key_length);
 
diff --git a/ui/curses.c b/ui/curses.c
index 479b77bd03..d55e6d74a1 100644
--- a/ui/curses.c
+++ b/ui/curses.c
@@ -434,7 +434,7 @@ static const DisplayChangeListenerOps dcl_ops = {
 .dpy_text_cursor = curses_cursor_position,
 };
 
-void curses_display_init(DisplayState *ds, DisplayOptions *opts)
+static void curses_display_init(DisplayState *ds, DisplayOptions *opts)
 {
 #ifndef _WIN32
 if (!isatty(1)) {
@@ -455,3 +455,15 @@ void curses_display_init(DisplayState *ds, DisplayOptions 
*opts)
 
 invalidate = 1;
 }
+
+static QemuDisplay qemu_display_curses = {
+.type   = DISPLAY_TYPE_CURSES,
+.init   = curses_display_init,
+};
+
+static void register_curses(void)
+{
+qemu_display_register(_display_curses);
+}
+
+type_init(register_curses);
diff --git a/vl.c b/vl.c
index 2c3cb4651c..2b4af34fbb 100644
--- a/vl.c
+++ b/vl.c
@@ -2161,12 +2161,7 @@ static void parse_display(const char *p)
 exit(1);
 #endif
 } else if (strstart(p, "curses", )) {
-#ifdef CONFIG_CURSES
 dpy.type = DISPLAY_TYPE_CURSES;
-#else
-error_report("curses support is disabled");
-exit(1);
-#endif
 } else if (strstart(p, "gtk", )) {
 dpy.type = DISPLAY_TYPE_GTK;
 while (*opts) {
@@ -4647,17 +4642,9 @@ int main(int argc, char **argv, char **envp)
 qemu_register_reset(restore_boot_order, g_strdup(boot_order));
 }
 
-ds = init_displaystate();
-
 /* init local displays */
-switch (dpy.type) {
-case DISPLAY_TYPE_CURSES:
-curses_display_init(ds, );
-break;
-default:
-qemu_display_init(ds, );
-break;
-}
+ds = init_displaystate();
+qemu_display_init(ds, );
 
 /* must be after terminal init, SDL library changes signal handlers */
 os_setup_signal_handling();
-- 
2.9.3

[Qemu-devel] [PATCH v3 1/7] net: Move error reporting from net_init_client/netdev to the calling site

It looks strange that net_init_client() and net_init_netdev() both
take an "Error **errp" parameter, but then do the error reporting
with "error_report_err(local_err)" on their own. Let's move the
error reporting to the calling site instead to simplify this code
a little bit.

Reviewed-by: Eric Blake 
Reviewed-by: Paolo Bonzini 
Signed-off-by: Thomas Huth 
---
 include/net/net.h |  2 +-
 net/net.c | 29 +
 vl.c  |  3 ++-
 3 files changed, 8 insertions(+), 26 deletions(-)

diff --git a/include/net/net.h b/include/net/net.h
index 3fc48e4..bdd4d9f 100644
--- a/include/net/net.h
+++ b/include/net/net.h
@@ -206,7 +206,7 @@ extern const char *legacy_bootp_filename;
 
 int net_client_init(QemuOpts *opts, bool is_netdev, Error **errp);
 int net_client_parse(QemuOptsList *opts_list, const char *str);
-int net_init_clients(void);
+int net_init_clients(Error **errp);
 void net_check_clients(void);
 void net_cleanup(void);
 void hmp_host_net_add(Monitor *mon, const QDict *qdict);
diff --git a/net/net.c b/net/net.c
index 7d42925..e213a61 100644
--- a/net/net.c
+++ b/net/net.c
@@ -1520,46 +1520,27 @@ void net_check_clients(void)
 
 static int net_init_client(void *dummy, QemuOpts *opts, Error **errp)
 {
-Error *local_err = NULL;
-
-net_client_init(opts, false, _err);
-if (local_err) {
-error_report_err(local_err);
-return -1;
-}
-
-return 0;
+return net_client_init(opts, false, errp);
 }
 
 static int net_init_netdev(void *dummy, QemuOpts *opts, Error **errp)
 {
-Error *local_err = NULL;
-int ret;
-
-ret = net_client_init(opts, true, _err);
-if (local_err) {
-error_report_err(local_err);
-return -1;
-}
-
-return ret;
+return net_client_init(opts, true, errp);
 }
 
-int net_init_clients(void)
+int net_init_clients(Error **errp)
 {
-QemuOptsList *net = qemu_find_opts("net");
-
 net_change_state_entry =
 qemu_add_vm_change_state_handler(net_vm_change_state_handler, NULL);
 
 QTAILQ_INIT(_clients);
 
 if (qemu_opts_foreach(qemu_find_opts("netdev"),
-  net_init_netdev, NULL, NULL)) {
+  net_init_netdev, NULL, errp)) {
 return -1;
 }
 
-if (qemu_opts_foreach(net, net_init_client, NULL, NULL)) {
+if (qemu_opts_foreach(qemu_find_opts("net"), net_init_client, NULL, errp)) 
{
 return -1;
 }
 
diff --git a/vl.c b/vl.c
index 9e7235d..698b681 100644
--- a/vl.c
+++ b/vl.c
@@ -4476,7 +4476,8 @@ int main(int argc, char **argv, char **envp)
 
 colo_info_init();
 
-if (net_init_clients() < 0) {
+if (net_init_clients() < 0) {
+error_report_err(err);
 exit(1);
 }
 
-- 
1.8.3.1

[Qemu-devel] [PATCH v3 5/7] net: Remove the deprecated way of dumping network packets

"-net dump" has been marked as deprecated since QEMU v2.10, since it
only works with the deprecated 'vlan' parameter (or hubs). Network
dumping should be done with "-object filter-dump" nowadays instead.
Since nobody complained so far about the deprecation message, let's
finally get rid of "-net dump" now.

Signed-off-by: Thomas Huth 
---
 net/dump.c  | 102 ++--
 net/net.c   |   9 +
 qapi/net.json   |  29 
 qemu-doc.texi   |   6 
 qemu-options.hx |   8 -
 5 files changed, 9 insertions(+), 145 deletions(-)

diff --git a/net/dump.c b/net/dump.c
index 15df9a4..f16c354 100644
--- a/net/dump.c
+++ b/net/dump.c
@@ -109,7 +109,7 @@ static int net_dump_state_init(DumpState *s, const char 
*filename,
 
 fd = open(filename, O_CREAT | O_TRUNC | O_WRONLY | O_BINARY, 0644);
 if (fd < 0) {
-error_setg_errno(errp, errno, "-net dump: can't open %s", filename);
+error_setg_errno(errp, errno, "net dump: can't open %s", filename);
 return -1;
 }
 
@@ -122,7 +122,7 @@ static int net_dump_state_init(DumpState *s, const char 
*filename,
 hdr.linktype = 1;
 
 if (write(fd, , sizeof(hdr)) < sizeof(hdr)) {
-error_setg_errno(errp, errno, "-net dump write error");
+error_setg_errno(errp, errno, "net dump write error");
 close(fd);
 return -1;
 }
@@ -136,104 +136,6 @@ static int net_dump_state_init(DumpState *s, const char 
*filename,
 return 0;
 }
 
-/* Dumping via VLAN netclient */
-
-struct DumpNetClient {
-NetClientState nc;
-DumpState ds;
-};
-typedef struct DumpNetClient DumpNetClient;
-
-static ssize_t dumpclient_receive(NetClientState *nc, const uint8_t *buf,
-  size_t size)
-{
-DumpNetClient *dc = DO_UPCAST(DumpNetClient, nc, nc);
-struct iovec iov = {
-.iov_base = (void *)buf,
-.iov_len = size
-};
-
-return dump_receive_iov(>ds, , 1);
-}
-
-static ssize_t dumpclient_receive_iov(NetClientState *nc,
-  const struct iovec *iov, int cnt)
-{
-DumpNetClient *dc = DO_UPCAST(DumpNetClient, nc, nc);
-
-return dump_receive_iov(>ds, iov, cnt);
-}
-
-static void dumpclient_cleanup(NetClientState *nc)
-{
-DumpNetClient *dc = DO_UPCAST(DumpNetClient, nc, nc);
-
-dump_cleanup(>ds);
-}
-
-static NetClientInfo net_dump_info = {
-.type = NET_CLIENT_DRIVER_DUMP,
-.size = sizeof(DumpNetClient),
-.receive = dumpclient_receive,
-.receive_iov = dumpclient_receive_iov,
-.cleanup = dumpclient_cleanup,
-};
-
-int net_init_dump(const Netdev *netdev, const char *name,
-  NetClientState *peer, Error **errp)
-{
-int len, rc;
-const char *file;
-char def_file[128];
-const NetdevDumpOptions *dump;
-NetClientState *nc;
-DumpNetClient *dnc;
-
-assert(netdev->type == NET_CLIENT_DRIVER_DUMP);
-dump = >u.dump;
-
-assert(peer);
-
-error_report("'-net dump' is deprecated. "
- "Please use '-object filter-dump' instead.");
-
-if (dump->has_file) {
-file = dump->file;
-} else {
-int id;
-int ret;
-
-ret = net_hub_id_for_client(peer, );
-assert(ret == 0); /* peer must be on a hub */
-
-snprintf(def_file, sizeof(def_file), "qemu-vlan%d.pcap", id);
-file = def_file;
-}
-
-if (dump->has_len) {
-if (dump->len > INT_MAX) {
-error_setg(errp, "invalid length: %"PRIu64, dump->len);
-return -1;
-}
-len = dump->len;
-} else {
-len = 65536;
-}
-
-nc = qemu_new_net_client(_dump_info, peer, "dump", name);
-snprintf(nc->info_str, sizeof(nc->info_str),
- "dump to %s (len=%d)", file, len);
-
-dnc = DO_UPCAST(DumpNetClient, nc, nc);
-rc = net_dump_state_init(>ds, file, len, errp);
-if (rc) {
-qemu_del_net_client(nc);
-}
-return rc;
-}
-
-/* Dumping via filter */
-
 #define TYPE_FILTER_DUMP "filter-dump"
 
 #define FILTER_DUMP(obj) \
diff --git a/net/net.c b/net/net.c
index dd80f1b..cbd553d 100644
--- a/net/net.c
+++ b/net/net.c
@@ -63,7 +63,6 @@ static QTAILQ_HEAD(, NetClientState) net_clients;
 const char *host_net_devices[] = {
 "tap",
 "socket",
-"dump",
 #ifdef CONFIG_NET_BRIDGE
 "bridge",
 #endif
@@ -967,7 +966,6 @@ static int (* const 
net_client_init_fun[NET_CLIENT_DRIVER__MAX])(
 #ifdef CONFIG_NETMAP
 [NET_CLIENT_DRIVER_NETMAP]= net_init_netmap,
 #endif
-[NET_CLIENT_DRIVER_DUMP]  = net_init_dump,
 #ifdef CONFIG_NET_BRIDGE
 [NET_CLIENT_DRIVER_BRIDGE]= net_init_bridge,
 #endif
@@ -993,8 +991,7 @@ static int net_client_init1(const void *object, bool 
is_netdev, Error **errp)
 netdev = object;
 name = netdev->id;
 
-if (netdev->type == NET_CLIENT_DRIVER_DUMP ||
-netdev->type == NET_CLIENT_DRIVER_NIC ||
+

Re: [Qemu-devel] [PATCH 11/27] block: x-blockdev-create QMP command

2018-02-21 Thread Kevin Wolf

Am 15.02.2018 um 20:58 hat Eric Blake geschrieben:
> On 02/08/2018 01:23 PM, Kevin Wolf wrote:
> > This adds a synchronous x-blockdev-create QMP command that can create
> > qcow2 images on a given node name.
> > 
> > We don't want to block while creating an image, so this is not the final
> > interface in all aspects, but BlockdevCreateOptionsQcow2 and
> > .bdrv_co_create() are what they actually might look like in the end. In
> > any case, this should be good enough to test whether we interpret
> > BlockdevCreateOptions as we should.
> > 
> > Signed-off-by: Kevin Wolf 
> > ---
> 
> > @@ -3442,6 +3442,18 @@
> > } }
> >   ##
> > +# @x-blockdev-create:
> > +#
> > +# Create an image format on a given node.
> > +# TODO Replace with something asynchronous (block job?)
> 
> We've learned our lesson - don't commit to the final name on an interface
> that we have not yet experimented with.
> 
> > +#
> > +# Since: 2.12
> > +##
> > +{ 'command': 'x-blockdev-create',
> > +  'data': 'BlockdevCreateOptions',
> > +  'boxed': true }
> > +
> 
> Lots of code packed in that little description ;)
> 
> 
> > +++ b/include/block/block_int.h
> > @@ -130,6 +130,8 @@ struct BlockDriver {
> >   int (*bdrv_file_open)(BlockDriverState *bs, QDict *options, int flags,
> > Error **errp);
> >   void (*bdrv_close)(BlockDriverState *bs);
> > +int coroutine_fn (*bdrv_co_create)(BlockdevCreateOptions *opts,
> > +   Error **errp);
> 
> I know we haven't been very good in the past, but can you add a comment here
> on the contract that drivers are supposed to obey when implementing this
> callback?

Anything specific you want to see here?

Essentially the meaning of BlockdevCreateOptions depends on the driver
and is documented in the QAPI schema, how Error works is common
knowledge, and I don't see much else to explain here.

I mean, I can add something like "Creates an image. See the QAPI
documentation for BlockdevCreateOptions for details." if you think this
is useful. But is it?

> > +/* Call callback if it exists */
> > +if (!drv->bdrv_co_create) {
> > +error_setg(errp, "Driver does not support blockdev-create");
> 
> Should this error message refer to 'x-blockdev-create' in the short term?

Hm, it would be more correct. On the other hand, I'm almost sure we'd
forget to rename it back when we remove the x- prefix...

Kevin

[Qemu-devel] [PULL 02/22] include/fpu/softfloat: remove USE_SOFTFLOAT_STRUCT_TYPES

It's not actively built and when enabled things fail to compile. I'm
not sure the type-checking is really helping here. Seeing as we "own"
our softfloat now lets remove the cruft.

Signed-off-by: Alex Bennée 
Reviewed-by: Richard Henderson 

diff --git a/include/fpu/softfloat.h b/include/fpu/softfloat.h
index d5e99667b6..52af1412de 100644
--- a/include/fpu/softfloat.h
+++ b/include/fpu/softfloat.h
@@ -103,32 +103,6 @@ enum {
 /*
 | Software IEC/IEEE floating-point types.
 **/
-/* Use structures for soft-float types.  This prevents accidentally mixing
-   them with native int/float types.  A sufficiently clever compiler and
-   sane ABI should be able to see though these structs.  However
-   x86/gcc 3.x seems to struggle a bit, so leave them disabled by default.  */
-//#define USE_SOFTFLOAT_STRUCT_TYPES
-#ifdef USE_SOFTFLOAT_STRUCT_TYPES
-typedef struct {
-uint16_t v;
-} float16;
-#define float16_val(x) (((float16)(x)).v)
-#define make_float16(x) __extension__ ({ float16 f16_val = {x}; f16_val; })
-#define const_float16(x) { x }
-typedef struct {
-uint32_t v;
-} float32;
-/* The cast ensures an error if the wrong type is passed.  */
-#define float32_val(x) (((float32)(x)).v)
-#define make_float32(x) __extension__ ({ float32 f32_val = {x}; f32_val; })
-#define const_float32(x) { x }
-typedef struct {
-uint64_t v;
-} float64;
-#define float64_val(x) (((float64)(x)).v)
-#define make_float64(x) __extension__ ({ float64 f64_val = {x}; f64_val; })
-#define const_float64(x) { x }
-#else
 typedef uint16_t float16;
 typedef uint32_t float32;
 typedef uint64_t float64;
@@ -141,7 +115,6 @@ typedef uint64_t float64;
 #define const_float16(x) (x)
 #define const_float32(x) (x)
 #define const_float64(x) (x)
-#endif
 typedef struct {
 uint64_t low;
 uint16_t high;
-- 
2.15.1

[Qemu-devel] [PULL 10/22] fpu/softfloat: move the extract functions to the top of the file

This is pure code-motion during re-factoring as the helpers will be
needed earlier.

Signed-off-by: Alex Bennée 
Reviewed-by: Richard Henderson 
Reviewed-by: Peter Maydell 

diff --git a/fpu/softfloat.c b/fpu/softfloat.c
index 3a4ab1355f..297e48f5c9 100644
--- a/fpu/softfloat.c
+++ b/fpu/softfloat.c
@@ -132,6 +132,60 @@ static inline flag extractFloat16Sign(float16 a)
 return float16_val(a)>>15;
 }
 
+/*
+| Returns the fraction bits of the single-precision floating-point value `a'.
+**/
+
+static inline uint32_t extractFloat32Frac(float32 a)
+{
+return float32_val(a) & 0x007F;
+}
+
+/*
+| Returns the exponent bits of the single-precision floating-point value `a'.
+**/
+
+static inline int extractFloat32Exp(float32 a)
+{
+return (float32_val(a) >> 23) & 0xFF;
+}
+
+/*
+| Returns the sign bit of the single-precision floating-point value `a'.
+**/
+
+static inline flag extractFloat32Sign(float32 a)
+{
+return float32_val(a) >> 31;
+}
+
+/*
+| Returns the fraction bits of the double-precision floating-point value `a'.
+**/
+
+static inline uint64_t extractFloat64Frac(float64 a)
+{
+return float64_val(a) & LIT64(0x000F);
+}
+
+/*
+| Returns the exponent bits of the double-precision floating-point value `a'.
+**/
+
+static inline int extractFloat64Exp(float64 a)
+{
+return (float64_val(a) >> 52) & 0x7FF;
+}
+
+/*
+| Returns the sign bit of the double-precision floating-point value `a'.
+**/
+
+static inline flag extractFloat64Sign(float64 a)
+{
+return float64_val(a) >> 63;
+}
+
 /*
 | Takes a 64-bit fixed-point value `absZ' with binary point between bits 6
 | and 7, and returns the properly rounded 32-bit integer corresponding to the
@@ -299,39 +353,6 @@ static int64_t roundAndPackUint64(flag zSign, uint64_t 
absZ0,
 return absZ0;
 }
 
-/*
-| Returns the fraction bits of the single-precision floating-point value `a'.
-**/
-
-static inline uint32_t extractFloat32Frac( float32 a )
-{
-
-return float32_val(a) & 0x007F;
-
-}
-
-/*
-| Returns the exponent bits of the single-precision floating-point value `a'.
-**/
-
-static inline int extractFloat32Exp(float32 a)
-{
-
-return ( float32_val(a)>>23 ) & 0xFF;
-
-}
-
-/*
-| Returns the sign bit of the single-precision floating-point value `a'.
-**/
-
-static inline flag extractFloat32Sign( float32 a )
-{
-
-return float32_val(a)>>31;
-
-}
-
 /*
 | If `a' is denormal and we are in flush-to-zero mode then set the
 | input-denormal exception and return zero. Otherwise just return the value.
@@ -492,39 +513,6 @@ static float32
 
 }
 
-/*
-| Returns the fraction bits of the double-precision floating-point value `a'.
-**/
-
-static inline uint64_t extractFloat64Frac( float64 a )
-{
-
-return float64_val(a) & LIT64( 0x000F );
-
-}
-
-/*
-| Returns the exponent bits of the double-precision floating-point value `a'.
-**/
-
-static inline int extractFloat64Exp(float64 a)
-{
-
-return ( float64_val(a)>>52 ) & 0x7FF;
-
-}
-
-/*
-| Returns the sign bit of

[Qemu-devel] [PULL 01/22] fpu/softfloat: implement float16_squash_input_denormal

This will be required when expanding the MINMAX() macro for 16
bit/half-precision operations.

Signed-off-by: Alex Bennée 
Reviewed-by: Richard Henderson 
Reviewed-by: Peter Maydell 

diff --git a/fpu/softfloat.c b/fpu/softfloat.c
index 433c5dad2d..3a4ab1355f 100644
--- a/fpu/softfloat.c
+++ b/fpu/softfloat.c
@@ -3488,6 +3488,21 @@ static float16 roundAndPackFloat16(flag zSign, int zExp,
 return packFloat16(zSign, zExp, zSig >> 13);
 }
 
+/*
+| If `a' is denormal and we are in flush-to-zero mode then set the
+| input-denormal exception and return zero. Otherwise just return the value.
+**/
+float16 float16_squash_input_denormal(float16 a, float_status *status)
+{
+if (status->flush_inputs_to_zero) {
+if (extractFloat16Exp(a) == 0 && extractFloat16Frac(a) != 0) {
+float_raise(float_flag_input_denormal, status);
+return make_float16(float16_val(a) & 0x8000);
+}
+}
+return a;
+}
+
 static void normalizeFloat16Subnormal(uint32_t aSig, int *zExpPtr,
   uint32_t *zSigPtr)
 {
diff --git a/include/fpu/softfloat.h b/include/fpu/softfloat.h
index 0f96a0edd1..d5e99667b6 100644
--- a/include/fpu/softfloat.h
+++ b/include/fpu/softfloat.h
@@ -277,6 +277,7 @@ void float_raise(uint8_t flags, float_status *status);
 | If `a' is denormal and we are in flush-to-zero mode then set the
 | input-denormal exception and return zero. Otherwise just return the value.
 **/
+float16 float16_squash_input_denormal(float16 a, float_status *status);
 float32 float32_squash_input_denormal(float32 a, float_status *status);
 float64 float64_squash_input_denormal(float64 a, float_status *status);
 
-- 
2.15.1

[Qemu-devel] [PULL 05/22] include/fpu/softfloat: implement float16_abs helper

This will be required when expanding the MINMAX() macro for 16
bit/half-precision operations.

Signed-off-by: Alex Bennée 
Reviewed-by: Richard Henderson 
Reviewed-by: Philippe Mathieu-Daudé 
Reviewed-by: Peter Maydell 

diff --git a/include/fpu/softfloat.h b/include/fpu/softfloat.h
index f3b9008f78..1d34f2c3eb 100644
--- a/include/fpu/softfloat.h
+++ b/include/fpu/softfloat.h
@@ -265,6 +265,13 @@ static inline int float16_is_zero_or_denormal(float16 a)
 return (float16_val(a) & 0x7c00) == 0;
 }
 
+static inline float16 float16_abs(float16 a)
+{
+/* Note that abs does *not* handle NaN specially, nor does
+ * it flush denormal inputs to zero.
+ */
+return make_float16(float16_val(a) & 0x7fff);
+}
 /*
 | The pattern for a default generated half-precision NaN.
 **/
-- 
2.15.1

[Qemu-devel] [PULL 11/22] fpu/softfloat: define decompose structures

These structures pave the way for generic softfloat helper routines
that will operate on fully decomposed numbers.

Signed-off-by: Alex Bennée 
Signed-off-by: Richard Henderson 
Reviewed-by: Philippe Mathieu-Daudé 

diff --git a/fpu/softfloat.c b/fpu/softfloat.c
index 297e48f5c9..568d95 100644
--- a/fpu/softfloat.c
+++ b/fpu/softfloat.c
@@ -83,7 +83,6 @@ this code that are retained.
  * target-dependent and needs the TARGET_* macros.
  */
 #include "qemu/osdep.h"
-
 #include "fpu/softfloat.h"
 
 /* We only need stdlib for abort() */
@@ -186,6 +185,91 @@ static inline flag extractFloat64Sign(float64 a)
 return float64_val(a) >> 63;
 }
 
+/*
+ * Classify a floating point number. Everything above float_class_qnan
+ * is a NaN so cls >= float_class_qnan is any NaN.
+ */
+
+typedef enum __attribute__ ((__packed__)) {
+float_class_unclassified,
+float_class_zero,
+float_class_normal,
+float_class_inf,
+float_class_qnan,  /* all NaNs from here */
+float_class_snan,
+float_class_dnan,
+float_class_msnan, /* maybe silenced */
+} FloatClass;
+
+/*
+ * Structure holding all of the decomposed parts of a float. The
+ * exponent is unbiased and the fraction is normalized. All
+ * calculations are done with a 64 bit fraction and then rounded as
+ * appropriate for the final format.
+ *
+ * Thanks to the packed FloatClass a decent compiler should be able to
+ * fit the whole structure into registers and avoid using the stack
+ * for parameter passing.
+ */
+
+typedef struct {
+uint64_t frac;
+int32_t  exp;
+FloatClass cls;
+bool sign;
+} FloatParts;
+
+#define DECOMPOSED_BINARY_POINT(64 - 2)
+#define DECOMPOSED_IMPLICIT_BIT(1ull << DECOMPOSED_BINARY_POINT)
+#define DECOMPOSED_OVERFLOW_BIT(DECOMPOSED_IMPLICIT_BIT << 1)
+
+/* Structure holding all of the relevant parameters for a format.
+ *   exp_size: the size of the exponent field
+ *   exp_bias: the offset applied to the exponent field
+ *   exp_max: the maximum normalised exponent
+ *   frac_size: the size of the fraction field
+ *   frac_shift: shift to normalise the fraction with DECOMPOSED_BINARY_POINT
+ * The following are computed based the size of fraction
+ *   frac_lsb: least significant bit of fraction
+ *   fram_lsbm1: the bit bellow the least significant bit (for rounding)
+ *   round_mask/roundeven_mask: masks used for rounding
+ */
+typedef struct {
+int exp_size;
+int exp_bias;
+int exp_max;
+int frac_size;
+int frac_shift;
+uint64_t frac_lsb;
+uint64_t frac_lsbm1;
+uint64_t round_mask;
+uint64_t roundeven_mask;
+} FloatFmt;
+
+/* Expand fields based on the size of exponent and fraction */
+#define FLOAT_PARAMS(E, F)   \
+.exp_size   = E, \
+.exp_bias   = ((1 << E) - 1) >> 1,   \
+.exp_max= (1 << E) - 1,  \
+.frac_size  = F, \
+.frac_shift = DECOMPOSED_BINARY_POINT - F,   \
+.frac_lsb   = 1ull << (DECOMPOSED_BINARY_POINT - F), \
+.frac_lsbm1 = 1ull << ((DECOMPOSED_BINARY_POINT - F) - 1),   \
+.round_mask = (1ull << (DECOMPOSED_BINARY_POINT - F)) - 1,   \
+.roundeven_mask = (2ull << (DECOMPOSED_BINARY_POINT - F)) - 1
+
+static const FloatFmt float16_params = {
+FLOAT_PARAMS(5, 10)
+};
+
+static const FloatFmt float32_params = {
+FLOAT_PARAMS(8, 23)
+};
+
+static const FloatFmt float64_params = {
+FLOAT_PARAMS(11, 52)
+};
+
 /*
 | Takes a 64-bit fixed-point value `absZ' with binary point between bits 6
 | and 7, and returns the properly rounded 32-bit integer corresponding to the
-- 
2.15.1

[Qemu-devel] [PULL 21/22] fpu/softfloat: re-factor compare

The compare function was already expanded from a macro. I keep the
macro expansion but move most of the logic into a compare_decomposed.

Signed-off-by: Alex Bennée 
Reviewed-by: Richard Henderson 

diff --git a/fpu/softfloat.c b/fpu/softfloat.c
index 7a53c36da4..4bc425d7e4 100644
--- a/fpu/softfloat.c
+++ b/fpu/softfloat.c
@@ -1783,6 +1783,86 @@ MINMAX(64, maxnummag, false, true, true)
 
 #undef MINMAX
 
+/* Floating point compare */
+static int compare_floats(FloatParts a, FloatParts b, bool is_quiet,
+  float_status *s)
+{
+if (is_nan(a.cls) || is_nan(b.cls)) {
+if (!is_quiet ||
+a.cls == float_class_snan ||
+b.cls == float_class_snan) {
+s->float_exception_flags |= float_flag_invalid;
+}
+return float_relation_unordered;
+}
+
+if (a.cls == float_class_zero) {
+if (b.cls == float_class_zero) {
+return float_relation_equal;
+}
+return b.sign ? float_relation_greater : float_relation_less;
+} else if (b.cls == float_class_zero) {
+return a.sign ? float_relation_less : float_relation_greater;
+}
+
+/* The only really important thing about infinity is its sign. If
+ * both are infinities the sign marks the smallest of the two.
+ */
+if (a.cls == float_class_inf) {
+if ((b.cls == float_class_inf) && (a.sign == b.sign)) {
+return float_relation_equal;
+}
+return a.sign ? float_relation_less : float_relation_greater;
+} else if (b.cls == float_class_inf) {
+return b.sign ? float_relation_greater : float_relation_less;
+}
+
+if (a.sign != b.sign) {
+return a.sign ? float_relation_less : float_relation_greater;
+}
+
+if (a.exp == b.exp) {
+if (a.frac == b.frac) {
+return float_relation_equal;
+}
+if (a.sign) {
+return a.frac > b.frac ?
+float_relation_less : float_relation_greater;
+} else {
+return a.frac > b.frac ?
+float_relation_greater : float_relation_less;
+}
+} else {
+if (a.sign) {
+return a.exp > b.exp ? float_relation_less : 
float_relation_greater;
+} else {
+return a.exp > b.exp ? float_relation_greater : 
float_relation_less;
+}
+}
+}
+
+#define COMPARE(sz) \
+int float ## sz ## _compare(float ## sz a, float ## sz b,   \
+float_status *s)\
+{   \
+FloatParts pa = float ## sz ## _unpack_canonical(a, s); \
+FloatParts pb = float ## sz ## _unpack_canonical(b, s); \
+return compare_floats(pa, pb, false, s);\
+}   \
+int float ## sz ## _compare_quiet(float ## sz a, float ## sz b, \
+  float_status *s)  \
+{   \
+FloatParts pa = float ## sz ## _unpack_canonical(a, s); \
+FloatParts pb = float ## sz ## _unpack_canonical(b, s); \
+return compare_floats(pa, pb, true, s); \
+}
+
+COMPARE(16)
+COMPARE(32)
+COMPARE(64)
+
+#undef COMPARE
+
 /* Multiply A by 2 raised to the power N.  */
 static FloatParts scalbn_decomposed(FloatParts a, int n, float_status *s)
 {
@@ -6884,60 +6964,6 @@ int float128_unordered_quiet(float128 a, float128 b, 
float_status *status)
 return 0;
 }
 
-#define COMPARE(s, nan_exp)  \
-static inline int float ## s ## _compare_internal(float ## s a, float ## s b,\
-  int is_quiet, float_status *status)\
-{\
-flag aSign, bSign;   \
-uint ## s ## _t av, bv;  \
-a = float ## s ## _squash_input_denormal(a, status); \
-b = float ## s ## _squash_input_denormal(b, status); \
- \
-if (( ( extractFloat ## s ## Exp( a ) == nan_exp ) &&\
- extractFloat ## s ## Frac( a ) ) || \
-( ( extractFloat ## s ## Exp( b ) == nan_exp ) &&\
-  extractFloat ## s ## Frac( b ) )) {\
-if (!is_quiet || \
-float ## s ## _is_signaling_nan(a, status) ||  \
-

Re: [Qemu-devel] [PATCH v3 0/7] Improvements and clean-ups related to -net

On 21.02.2018 11:41, Mark Cave-Ayland wrote:
> On 21/02/18 10:18, Thomas Huth wrote:
> 
>> "-net" is a legacy option that often causes confusion and
>> misconfigurations for the users, since most people are not aware
>> of the underlying "vlan" (i.e. hub) concept that is used for this
>> parameter. The prefered way of configuring your network stack is
>> to use "--netdev" instead, which gives you a clean 1:1 connection
>> between your emulated guest hardware and the host network backend.
>>
>> However, there are two reasons why we could not completely deprecate
>> "-net" yet:
>>
>> 1) Convenience:
>> In some cases, it's more convenient to use "-net" instead of "--netdev",
>> e.g. if you just want to have a "tap" network connection, it's faster
>> to type "-net nic -net tap" instead of "--device e1000,netdev=n1
>> --netdev tap,id=n1".
>>
>> 2) On-board NICs:
>> Currently the "-net nic" parameter is the only way to configure on-
>> board NICs on certain (embedded) machines via the nd_table[] array.
>>
>> So beside some generic clean-ups and removal of code that has been
>> marked as deprecated since QEMU 2.10 already, this patch series intro-
>> duces a new parameter "--nic" (in patch 7) which should be able to re-
>> place "-net" in the long run completely: This new convenience parameter
>> can be used to configure the default/on-board guest HW together with a
>> host network backend in a very compact way. To configure a tap backend
>> for the default NIC, you just have to type "--nic tap" here for example.
> 
> Hi Thomas,
> 
> This is a great improvement for configuring on-board NICs! I do have a
> couple of questions about your patchset based on personal experience:
> 
> 1) Does the new -nic syntax support multiple on-board NICs? I remember
> seeing this on some of the ARM boards I was studying when trying to
> implement something similar for SPARC.

Yes, that's possible. If you specify the "--nic" parameter multiple
times, it populates multiple entries in the nd_table[] array, so boards
can pick up multiple NICs from there. For example hw/arm/xlnx-zynqmp.c
initializes up to 4 (XLNX_ZYNQMP_NUM_GEMS) on-board NICs this way.

> 2) Is it possible to provide a convenient wrapper function to handle the
> logic related to determining whether a specified NIC is on-board or not?
> 
> For example take a look at
> https://git.qemu.org/?p=qemu.git;a=blob;f=hw/sparc64/sun4u.c;h=da28ab9413efdbe0bf0e1d3bf2b545577b83d88a;hb=a6e0344fa0e09413324835ae122c4cadd7890231#l596
> where we have a nice dance to check whether the device specified is
> on-board or not, and if it isn't to plug it in as an extra PCI device
> instead. The complication here is that nd_tables[] is populated by
> default (and also with -net nic -net user) but not when someone
> specifies a single card via -netdev.
> 
> From memory this was derived from the code in
> https://git.qemu.org/?p=qemu.git;a=blob;f=hw/arm/realview.c;h=87cd1e583cd20b6d8a2beeef1cba6977496d4477;hb=a6e0344fa0e09413324835ae122c4cadd7890231#l259
> if having another example helps?

Do you have something in mind how the interface to such a function
should look like?
Not sure whether there is a easy, magic solution for all boards ...
there are multiple problems, e.g. that you can normally not specify
on-board NICs via --device yet...
Anyway, that would be subject to another clean-up patch series - this
series here only focuses on the net/* files, not on the files in the hw/
folder.

 Thomas

[Qemu-devel] [PULL 19/22] fpu/softfloat: re-factor scalbn

This is one of the simpler manipulations you could make to a floating
point number.

Signed-off-by: Alex Bennée 
Reviewed-by: Richard Henderson 

diff --git a/fpu/softfloat.c b/fpu/softfloat.c
index 4313d3a602..a07b8556b0 100644
--- a/fpu/softfloat.c
+++ b/fpu/softfloat.c
@@ -1663,6 +1663,39 @@ float64 uint16_to_float64(uint16_t a, float_status 
*status)
 return uint64_to_float64(a, status);
 }
 
+/* Multiply A by 2 raised to the power N.  */
+static FloatParts scalbn_decomposed(FloatParts a, int n, float_status *s)
+{
+if (unlikely(is_nan(a.cls))) {
+return return_nan(a, s);
+}
+if (a.cls == float_class_normal) {
+a.exp += n;
+}
+return a;
+}
+
+float16 float16_scalbn(float16 a, int n, float_status *status)
+{
+FloatParts pa = float16_unpack_canonical(a, status);
+FloatParts pr = scalbn_decomposed(pa, n, status);
+return float16_round_pack_canonical(pr, status);
+}
+
+float32 float32_scalbn(float32 a, int n, float_status *status)
+{
+FloatParts pa = float32_unpack_canonical(a, status);
+FloatParts pr = scalbn_decomposed(pa, n, status);
+return float32_round_pack_canonical(pr, status);
+}
+
+float64 float64_scalbn(float64 a, int n, float_status *status)
+{
+FloatParts pa = float64_unpack_canonical(a, status);
+FloatParts pr = scalbn_decomposed(pa, n, status);
+return float64_round_pack_canonical(pr, status);
+}
+
 /*
 | Takes a 64-bit fixed-point value `absZ' with binary point between bits 6
 | and 7, and returns the properly rounded 32-bit integer corresponding to the
@@ -6986,79 +7019,6 @@ MINMAX(32)
 MINMAX(64)
 
 
-/* Multiply A by 2 raised to the power N.  */
-float32 float32_scalbn(float32 a, int n, float_status *status)
-{
-flag aSign;
-int16_t aExp;
-uint32_t aSig;
-
-a = float32_squash_input_denormal(a, status);
-aSig = extractFloat32Frac( a );
-aExp = extractFloat32Exp( a );
-aSign = extractFloat32Sign( a );
-
-if ( aExp == 0xFF ) {
-if ( aSig ) {
-return propagateFloat32NaN(a, a, status);
-}
-return a;
-}
-if (aExp != 0) {
-aSig |= 0x0080;
-} else if (aSig == 0) {
-return a;
-} else {
-aExp++;
-}
-
-if (n > 0x200) {
-n = 0x200;
-} else if (n < -0x200) {
-n = -0x200;
-}
-
-aExp += n - 1;
-aSig <<= 7;
-return normalizeRoundAndPackFloat32(aSign, aExp, aSig, status);
-}
-
-float64 float64_scalbn(float64 a, int n, float_status *status)
-{
-flag aSign;
-int16_t aExp;
-uint64_t aSig;
-
-a = float64_squash_input_denormal(a, status);
-aSig = extractFloat64Frac( a );
-aExp = extractFloat64Exp( a );
-aSign = extractFloat64Sign( a );
-
-if ( aExp == 0x7FF ) {
-if ( aSig ) {
-return propagateFloat64NaN(a, a, status);
-}
-return a;
-}
-if (aExp != 0) {
-aSig |= LIT64( 0x0010 );
-} else if (aSig == 0) {
-return a;
-} else {
-aExp++;
-}
-
-if (n > 0x1000) {
-n = 0x1000;
-} else if (n < -0x1000) {
-n = -0x1000;
-}
-
-aExp += n - 1;
-aSig <<= 10;
-return normalizeRoundAndPackFloat64(aSign, aExp, aSig, status);
-}
-
 floatx80 floatx80_scalbn(floatx80 a, int n, float_status *status)
 {
 flag aSign;
diff --git a/include/fpu/softfloat.h b/include/fpu/softfloat.h
index 3e6fdd756a..52621e0b79 100644
--- a/include/fpu/softfloat.h
+++ b/include/fpu/softfloat.h
@@ -244,6 +244,7 @@ float16 float16_sub(float16, float16, float_status *status);
 float16 float16_mul(float16, float16, float_status *status);
 float16 float16_muladd(float16, float16, float16, int, float_status *status);
 float16 float16_div(float16, float16, float_status *status);
+float16 float16_scalbn(float16, int, float_status *status);
 
 int float16_is_quiet_nan(float16, float_status *status);
 int float16_is_signaling_nan(float16, float_status *status);
-- 
2.15.1

[Qemu-devel] [PULL 04/22] target/*/cpu.h: remove softfloat.h

As cpu.h is another typically widely included file which doesn't need
full access to the softfloat API we can remove the includes from here
as well. Where they do need types it's typically for float_status and
the rounding modes so we move that to softfloat-types.h as well.

As a result of not having softfloat in every cpu.h call we now need to
add it to various helpers that do need the full softfloat.h
definitions.

Signed-off-by: Alex Bennée 
Reviewed-by: Philippe Mathieu-Daudé 
Reviewed-by: Richard Henderson 
[For PPC parts]
Acked-by: David Gibson 

diff --git a/include/fpu/softfloat-types.h b/include/fpu/softfloat-types.h
index 8210a94ea1..4e378cb612 100644
--- a/include/fpu/softfloat-types.h
+++ b/include/fpu/softfloat-types.h
@@ -80,6 +80,12 @@ this code that are retained.
 #ifndef SOFTFLOAT_TYPES_H
 #define SOFTFLOAT_TYPES_H
 
+/* This 'flag' type must be able to hold at least 0 and 1. It should
+ * probably be replaced with 'bool' but the uses would need to be audited
+ * to check that they weren't accidentally relying on it being a larger type.
+ */
+typedef uint8_t flag;
+
 /*
  * Software IEC/IEEE floating-point types.
  */
@@ -112,4 +118,62 @@ typedef struct {
 #define make_float128(high_, low_) ((float128) { .high = high_, .low = low_ })
 #define make_float128_init(high_, low_) { .high = high_, .low = low_ }
 
+/*
+ * Software IEC/IEEE floating-point underflow tininess-detection mode.
+ */
+
+enum {
+float_tininess_after_rounding  = 0,
+float_tininess_before_rounding = 1
+};
+
+/*
+ *Software IEC/IEEE floating-point rounding mode.
+ */
+
+enum {
+float_round_nearest_even = 0,
+float_round_down = 1,
+float_round_up   = 2,
+float_round_to_zero  = 3,
+float_round_ties_away= 4,
+/* Not an IEEE rounding mode: round to the closest odd mantissa value */
+float_round_to_odd   = 5,
+};
+
+/*
+ * Software IEC/IEEE floating-point exception flags.
+ */
+
+enum {
+float_flag_invalid   =  1,
+float_flag_divbyzero =  4,
+float_flag_overflow  =  8,
+float_flag_underflow = 16,
+float_flag_inexact   = 32,
+float_flag_input_denormal = 64,
+float_flag_output_denormal = 128
+};
+
+
+/*
+ * Floating Point Status. Individual architectures may maintain
+ * several versions of float_status for different functions. The
+ * correct status for the operation is then passed by reference to
+ * most of the softfloat functions.
+ */
+
+typedef struct float_status {
+signed char float_detect_tininess;
+signed char float_rounding_mode;
+uint8_t float_exception_flags;
+signed char floatx80_rounding_precision;
+/* should denormalised results go to zero and set the inexact flag? */
+flag flush_to_zero;
+/* should denormalised inputs go to zero and set the input_denormal flag? 
*/
+flag flush_inputs_to_zero;
+flag default_nan_mode;
+flag snan_bit_is_one;
+} float_status;
+
 #endif /* SOFTFLOAT_TYPES_H */
diff --git a/include/fpu/softfloat.h b/include/fpu/softfloat.h
index 4e16e22e58..f3b9008f78 100644
--- a/include/fpu/softfloat.h
+++ b/include/fpu/softfloat.h
@@ -82,12 +82,6 @@ this code that are retained.
 #ifndef SOFTFLOAT_H
 #define SOFTFLOAT_H
 
-/* This 'flag' type must be able to hold at least 0 and 1. It should
- * probably be replaced with 'bool' but the uses would need to be audited
- * to check that they weren't accidentally relying on it being a larger type.
- */
-typedef uint8_t flag;
-
 #define LIT64( a ) a##LL
 
 /*
@@ -102,53 +96,6 @@ enum {
 
 #include "fpu/softfloat-types.h"
 
-/*
-| Software IEC/IEEE floating-point underflow tininess-detection mode.
-**/
-enum {
-float_tininess_after_rounding  = 0,
-float_tininess_before_rounding = 1
-};
-
-/*
-| Software IEC/IEEE floating-point rounding mode.
-**/
-enum {
-float_round_nearest_even = 0,
-float_round_down = 1,
-float_round_up   = 2,
-float_round_to_zero  = 3,
-float_round_ties_away= 4,
-/* Not an IEEE rounding mode: round to the closest odd mantissa value */
-float_round_to_odd   = 5,
-};
-
-/*
-| Software IEC/IEEE floating-point exception flags.
-**/
-enum {
-float_flag_invalid   =  1,
-float_flag_divbyzero =  4,
-float_flag_overflow  =  8,
-float_flag_underflow = 16,
-float_flag_inexact   = 32,
-float_flag_input_denormal

Re: [Qemu-devel] Call for GSoC & Outreachy 2018 mentors & project ideas

On Tue, Feb 20, 2018 at 11:13 AM, Paolo Bonzini  wrote:
> On 20/02/2018 11:36, Stefan Hajnoczi wrote:
>> === Multi-CPU cluster support for GDB server in QEMU ===
>>
>> There are many examples in modern computing where multiple CPU
>> clusters are grouped together in a single SoC. This is common in the
>> ARM world especially. There are numerous examples such as ARM's
>> big.LITTLE implementations and Xilinx's 4xA53s and 2xR5s on the ZynqMP
>> SoC. The goal of this task is to add support to the GDB server to
>> allow users to debug across these clusters.
>>
>> This is another step towards single binary QEMU as well.
>>
>>  Detailed description of the project.
>>
>> Xilinx has an out of tree implementation that can be used as a
>> starting point. Work will need to be done on top of this to prepare it
>> for upstream submission and to ensure the implementation is more
>> generic.
>>
>> This will mostly involve extending GDB server to tell GDB about
>> different architectures and then allow the user to swap between them.
>>
>> The Xilinx implementation can be seen here:
>> https://github.com/Xilinx/qemu/blob/master/gdbstub.c
>> There has been some steps in preparing the work to go upstream, which
>> can be seen here:
>> https://github.com/Xilinx/qemu/tree/mainline/alistair/gdb
>
> I agree this is interesting.

Thanks!  I have added it to the wiki:

https://wiki.qemu.org/Google_Summer_of_Code_2018#Multi-CPU_cluster_support_for_GDB_server_in_QEMU

Stefan

[Qemu-devel] [PULL 22/22] fpu/softfloat: re-factor sqrt

This is a little bit of a departure from softfloat's original approach
as we skip the estimate step in favour of a straight iteration. There
is a minor optimisation to avoid calculating more bits of precision
than we need however this still brings a performance drop, especially
for float64 operations.

Suggested-by: Richard Henderson 
Signed-off-by: Alex Bennée 
Reviewed-by: Peter Maydell 
Reviewed-by: Richard Henderson 

diff --git a/fpu/softfloat.c b/fpu/softfloat.c
index 4bc425d7e4..e7fb0d357a 100644
--- a/fpu/softfloat.c
+++ b/fpu/softfloat.c
@@ -1896,6 +1896,102 @@ float64 float64_scalbn(float64 a, int n, float_status 
*status)
 return float64_round_pack_canonical(pr, status);
 }
 
+/*
+ * Square Root
+ *
+ * The old softfloat code did an approximation step before zeroing in
+ * on the final result. However for simpleness we just compute the
+ * square root by iterating down from the implicit bit to enough extra
+ * bits to ensure we get a correctly rounded result.
+ *
+ * This does mean however the calculation is slower than before,
+ * especially for 64 bit floats.
+ */
+
+static FloatParts sqrt_float(FloatParts a, float_status *s, const FloatFmt *p)
+{
+uint64_t a_frac, r_frac, s_frac;
+int bit, last_bit;
+
+if (is_nan(a.cls)) {
+return return_nan(a, s);
+}
+if (a.cls == float_class_zero) {
+return a;  /* sqrt(+-0) = +-0 */
+}
+if (a.sign) {
+s->float_exception_flags |= float_flag_invalid;
+a.cls = float_class_dnan;
+return a;
+}
+if (a.cls == float_class_inf) {
+return a;  /* sqrt(+inf) = +inf */
+}
+
+assert(a.cls == float_class_normal);
+
+/* We need two overflow bits at the top. Adding room for that is a
+ * right shift. If the exponent is odd, we can discard the low bit
+ * by multiplying the fraction by 2; that's a left shift. Combine
+ * those and we shift right if the exponent is even.
+ */
+a_frac = a.frac;
+if (!(a.exp & 1)) {
+a_frac >>= 1;
+}
+a.exp >>= 1;
+
+/* Bit-by-bit computation of sqrt.  */
+r_frac = 0;
+s_frac = 0;
+
+/* Iterate from implicit bit down to the 3 extra bits to compute a
+ * properly rounded result. Remember we've inserted one more bit
+ * at the top, so these positions are one less.
+ */
+bit = DECOMPOSED_BINARY_POINT - 1;
+last_bit = MAX(p->frac_shift - 4, 0);
+do {
+uint64_t q = 1ULL << bit;
+uint64_t t_frac = s_frac + q;
+if (t_frac <= a_frac) {
+s_frac = t_frac + q;
+a_frac -= t_frac;
+r_frac += q;
+}
+a_frac <<= 1;
+} while (--bit >= last_bit);
+
+/* Undo the right shift done above. If there is any remaining
+ * fraction, the result is inexact. Set the sticky bit.
+ */
+a.frac = (r_frac << 1) + (a_frac != 0);
+
+return a;
+}
+
+float16 __attribute__((flatten)) float16_sqrt(float16 a, float_status *status)
+{
+FloatParts pa = float16_unpack_canonical(a, status);
+FloatParts pr = sqrt_float(pa, status, _params);
+return float16_round_pack_canonical(pr, status);
+}
+
+float32 __attribute__((flatten)) float32_sqrt(float32 a, float_status *status)
+{
+FloatParts pa = float32_unpack_canonical(a, status);
+FloatParts pr = sqrt_float(pa, status, _params);
+return float32_round_pack_canonical(pr, status);
+}
+
+float64 __attribute__((flatten)) float64_sqrt(float64 a, float_status *status)
+{
+FloatParts pa = float64_unpack_canonical(a, status);
+FloatParts pr = sqrt_float(pa, status, _params);
+return float64_round_pack_canonical(pr, status);
+}
+
+
 /*
 | Takes a 64-bit fixed-point value `absZ' with binary point between bits 6
 | and 7, and returns the properly rounded 32-bit integer corresponding to the
@@ -3303,62 +3399,6 @@ float32 float32_rem(float32 a, float32 b, float_status 
*status)
 }
 
 
-/*
-| Returns the square root of the single-precision floating-point value `a'.
-| The operation is performed according to the IEC/IEEE Standard for Binary
-| Floating-Point Arithmetic.
-**/
-
-float32 float32_sqrt(float32 a, float_status *status)
-{
-flag aSign;
-int aExp, zExp;
-uint32_t aSig, zSig;
-uint64_t rem, term;
-a = float32_squash_input_denormal(a, status);
-
-aSig = extractFloat32Frac( a );
-aExp = extractFloat32Exp( a );
-aSign = extractFloat32Sign( a );
-if ( aExp == 0xFF ) {
-if (aSig) {
-return propagateFloat32NaN(a, float32_zero, status);
-}
-if ( ! aSign ) return a;
-float_raise(float_flag_invalid, status);
-return

[Qemu-devel] [PULL 14/22] fpu/softfloat: re-factor div

We can now add float16_div and use the common decompose and
canonicalize functions to have a single implementation for
float16/32/64 versions.

Signed-off-by: Alex Bennée 
Signed-off-by: Richard Henderson 
Reviewed-by: Peter Maydell 

diff --git a/fpu/softfloat-macros.h b/fpu/softfloat-macros.h
index 9cc6158cb4..c45a23193e 100644
--- a/fpu/softfloat-macros.h
+++ b/fpu/softfloat-macros.h
@@ -625,6 +625,54 @@ static uint64_t estimateDiv128To64( uint64_t a0, uint64_t 
a1, uint64_t b )
 
 }
 
+/* From the GNU Multi Precision Library - longlong.h __udiv_qrnnd
+ * (https://gmplib.org/repo/gmp/file/tip/longlong.h)
+ *
+ * Licensed under the GPLv2/LGPLv3
+ */
+static uint64_t div128To64(uint64_t n0, uint64_t n1, uint64_t d)
+{
+uint64_t d0, d1, q0, q1, r1, r0, m;
+
+d0 = (uint32_t)d;
+d1 = d >> 32;
+
+r1 = n1 % d1;
+q1 = n1 / d1;
+m = q1 * d0;
+r1 = (r1 << 32) | (n0 >> 32);
+if (r1 < m) {
+q1 -= 1;
+r1 += d;
+if (r1 >= d) {
+if (r1 < m) {
+q1 -= 1;
+r1 += d;
+}
+}
+}
+r1 -= m;
+
+r0 = r1 % d1;
+q0 = r1 / d1;
+m = q0 * d0;
+r0 = (r0 << 32) | (uint32_t)n0;
+if (r0 < m) {
+q0 -= 1;
+r0 += d;
+if (r0 >= d) {
+if (r0 < m) {
+q0 -= 1;
+r0 += d;
+}
+}
+}
+r0 -= m;
+
+/* Return remainder in LSB */
+return (q1 << 32) | q0 | (r0 != 0);
+}
+
 /*
 | Returns an approximation to the square root of the 32-bit significand given
 | by `a'.  Considered as an integer, `a' must be at least 2^31.  If bit 0 of
diff --git a/fpu/softfloat.c b/fpu/softfloat.c
index 6d29e1a103..4a859b2721 100644
--- a/fpu/softfloat.c
+++ b/fpu/softfloat.c
@@ -816,6 +816,94 @@ float64 __attribute__((flatten)) float64_mul(float64 a, 
float64 b,
 return float64_round_pack_canonical(pr, status);
 }
 
+/*
+ * Returns the result of dividing the floating-point value `a' by the
+ * corresponding value `b'. The operation is performed according to
+ * the IEC/IEEE Standard for Binary Floating-Point Arithmetic.
+ */
+
+static FloatParts div_floats(FloatParts a, FloatParts b, float_status *s)
+{
+bool sign = a.sign ^ b.sign;
+
+if (a.cls == float_class_normal && b.cls == float_class_normal) {
+uint64_t temp_lo, temp_hi;
+int exp = a.exp - b.exp;
+if (a.frac < b.frac) {
+exp -= 1;
+shortShift128Left(0, a.frac, DECOMPOSED_BINARY_POINT + 1,
+  _hi, _lo);
+} else {
+shortShift128Left(0, a.frac, DECOMPOSED_BINARY_POINT,
+  _hi, _lo);
+}
+/* LSB of quot is set if inexact which roundandpack will use
+ * to set flags. Yet again we re-use a for the result */
+a.frac = div128To64(temp_lo, temp_hi, b.frac);
+a.sign = sign;
+a.exp = exp;
+return a;
+}
+/* handle all the NaN cases */
+if (is_nan(a.cls) || is_nan(b.cls)) {
+return pick_nan(a, b, s);
+}
+/* 0/0 or Inf/Inf */
+if (a.cls == b.cls
+&&
+(a.cls == float_class_inf || a.cls == float_class_zero)) {
+s->float_exception_flags |= float_flag_invalid;
+a.cls = float_class_dnan;
+return a;
+}
+/* Div 0 => Inf */
+if (b.cls == float_class_zero) {
+s->float_exception_flags |= float_flag_divbyzero;
+a.cls = float_class_inf;
+a.sign = sign;
+return a;
+}
+/* Inf / x or 0 / x */
+if (a.cls == float_class_inf || a.cls == float_class_zero) {
+a.sign = sign;
+return a;
+}
+/* Div by Inf */
+if (b.cls == float_class_inf) {
+a.cls = float_class_zero;
+a.sign = sign;
+return a;
+}
+g_assert_not_reached();
+}
+
+float16 float16_div(float16 a, float16 b, float_status *status)
+{
+FloatParts pa = float16_unpack_canonical(a, status);
+FloatParts pb = float16_unpack_canonical(b, status);
+FloatParts pr = div_floats(pa, pb, status);
+
+return float16_round_pack_canonical(pr, status);
+}
+
+float32 float32_div(float32 a, float32 b, float_status *status)
+{
+FloatParts pa = float32_unpack_canonical(a, status);
+FloatParts pb = float32_unpack_canonical(b, status);
+FloatParts pr = div_floats(pa, pb, status);
+
+return float32_round_pack_canonical(pr, status);
+}
+
+float64 float64_div(float64 a, float64 b, float_status *status)
+{
+FloatParts pa = float64_unpack_canonical(a, status);
+FloatParts pb = float64_unpack_canonical(b, status);
+FloatParts pr = div_floats(pa, pb, status);
+
+return float64_round_pack_canonical(pr, status);
+}
+
 /*
 | Takes a

[Qemu-devel] [PULL 20/22] fpu/softfloat: re-factor minmax

Let's do the same re-factor treatment for minmax functions. I still
use the MACRO trick to expand but now all the checking code is common.

Signed-off-by: Alex Bennée 
Reviewed-by: Richard Henderson 

diff --git a/fpu/softfloat.c b/fpu/softfloat.c
index a07b8556b0..7a53c36da4 100644
--- a/fpu/softfloat.c
+++ b/fpu/softfloat.c
@@ -1663,6 +1663,126 @@ float64 uint16_to_float64(uint16_t a, float_status 
*status)
 return uint64_to_float64(a, status);
 }
 
+/* Float Min/Max */
+/* min() and max() functions. These can't be implemented as
+ * 'compare and pick one input' because that would mishandle
+ * NaNs and +0 vs -0.
+ *
+ * minnum() and maxnum() functions. These are similar to the min()
+ * and max() functions but if one of the arguments is a QNaN and
+ * the other is numerical then the numerical argument is returned.
+ * SNaNs will get quietened before being returned.
+ * minnum() and maxnum correspond to the IEEE 754-2008 minNum()
+ * and maxNum() operations. min() and max() are the typical min/max
+ * semantics provided by many CPUs which predate that specification.
+ *
+ * minnummag() and maxnummag() functions correspond to minNumMag()
+ * and minNumMag() from the IEEE-754 2008.
+ */
+static FloatParts minmax_floats(FloatParts a, FloatParts b, bool ismin,
+bool ieee, bool ismag, float_status *s)
+{
+if (unlikely(is_nan(a.cls) || is_nan(b.cls))) {
+if (ieee) {
+/* Takes two floating-point values `a' and `b', one of
+ * which is a NaN, and returns the appropriate NaN
+ * result. If either `a' or `b' is a signaling NaN,
+ * the invalid exception is raised.
+ */
+if (is_snan(a.cls) || is_snan(b.cls)) {
+return pick_nan(a, b, s);
+} else if (is_nan(a.cls) && !is_nan(b.cls)) {
+return b;
+} else if (is_nan(b.cls) && !is_nan(a.cls)) {
+return a;
+}
+}
+return pick_nan(a, b, s);
+} else {
+int a_exp, b_exp;
+bool a_sign, b_sign;
+
+switch (a.cls) {
+case float_class_normal:
+a_exp = a.exp;
+break;
+case float_class_inf:
+a_exp = INT_MAX;
+break;
+case float_class_zero:
+a_exp = INT_MIN;
+break;
+default:
+g_assert_not_reached();
+break;
+}
+switch (b.cls) {
+case float_class_normal:
+b_exp = b.exp;
+break;
+case float_class_inf:
+b_exp = INT_MAX;
+break;
+case float_class_zero:
+b_exp = INT_MIN;
+break;
+default:
+g_assert_not_reached();
+break;
+}
+
+a_sign = a.sign;
+b_sign = b.sign;
+if (ismag) {
+a_sign = b_sign = 0;
+}
+
+if (a_sign == b_sign) {
+bool a_less = a_exp < b_exp;
+if (a_exp == b_exp) {
+a_less = a.frac < b.frac;
+}
+return a_sign ^ a_less ^ ismin ? b : a;
+} else {
+return a_sign ^ ismin ? b : a;
+}
+}
+}
+
+#define MINMAX(sz, name, ismin, isiee, ismag)   \
+float ## sz float ## sz ## _ ## name(float ## sz a, float ## sz b,  \
+ float_status *s)   \
+{   \
+FloatParts pa = float ## sz ## _unpack_canonical(a, s); \
+FloatParts pb = float ## sz ## _unpack_canonical(b, s); \
+FloatParts pr = minmax_floats(pa, pb, ismin, isiee, ismag, s);  \
+\
+return float ## sz ## _round_pack_canonical(pr, s); \
+}
+
+MINMAX(16, min, true, false, false)
+MINMAX(16, minnum, true, true, false)
+MINMAX(16, minnummag, true, true, true)
+MINMAX(16, max, false, false, false)
+MINMAX(16, maxnum, false, true, false)
+MINMAX(16, maxnummag, false, true, true)
+
+MINMAX(32, min, true, false, false)
+MINMAX(32, minnum, true, true, false)
+MINMAX(32, minnummag, true, true, true)
+MINMAX(32, max, false, false, false)
+MINMAX(32, maxnum, false, true, false)
+MINMAX(32, maxnummag, false, true, true)
+
+MINMAX(64, min, true, false, false)
+MINMAX(64, minnum, true, true, false)
+MINMAX(64, minnummag, true, true, true)
+MINMAX(64, max, false, false, false)
+MINMAX(64, maxnum, false, true, false)
+MINMAX(64, maxnummag, false, true, true)
+
+#undef MINMAX
+
 /* Multiply A by 2 raised to the power N.  */
 static FloatParts scalbn_decomposed(FloatParts a, int n, float_status *s)
 {
@@ -6912,113 +7032,6 @@ int float128_compare_quiet(float128 a, float128 b, 
float_status *status)
 return float128_compare_internal(a, b, 1, status);
 }
 
-/*

Re: [Qemu-devel] [PATCH v3 0/7] Improvements and clean-ups related to -net

2018-02-21 Thread Paolo Bonzini

On 21/02/2018 11:41, Mark Cave-Ayland wrote:
> 1) Does the new -nic syntax support multiple on-board NICs? I remember
> seeing this on some of the ARM boards I was studying when trying to
> implement something similar for SPARC.

Yes, but they will be in different subnets if you do "-nic user -nic
user".  If you want to put them on the same trunk, what you want is
(cut-and-pasted from an offlist email from Thomas):

 -netdev user,id=slirp \
 -netdev hubport,id=port,netdev=slirp,hubid=0 \
 -nic hubport,hubid=0 \
 -nic hubport,hubid=0

We could make id and hubid optional (id was already optional in -net so
the logic is there already, see assign_name in net/net.c), giving the
much nicer:

 -netdev user,id=slirp \
 -netdev hubport,netdev=slirp \
 -nic hubport \
 -nic hubport

Thanks,

Paolo

> 2) Is it possible to provide a convenient wrapper function to handle the
> logic related to determining whether a specified NIC is on-board or not?

[Qemu-devel] [PATCH 00/12] ui: build sdl, gtk and curses as modules

This patch series adds a registry for user interfaces (aka displays),
adds support for user interface modules and allows to build sdl, gtk
and curses as modules.  Especially gtk cuts down the number of shared
libraries qemu links against by a significant amount.

Note one: Modules are disabled by default, so configure with
  --enable-modules to test this.

Note two: Qemu build system doesn't rebuild object files when
  the compiler flags change.  You might see build failures
  when enabling modules without "make clean" because of this,
  due to non-modular object files being built without -fPIC.

Gerd Hoffmann (12):
  console: add qemu display registry, add gtk
  sdl: switch over to new display registry
  cocoa: switch over to new display registry
  curses: switch over to new display registry
  egl-headless: switch over to new display registry
  console: add and use qemu_display_find_default
  console: add ui module loading support
  configure: add X11 vars to config-host.mak
  configure: opengl doesn't depend on x11
  sdl: build as ui module
  gtk: build as ui module
  curses: build as ui module

 configure | 29 +++-
 Makefile.objs |  1 +
 include/qemu/module.h |  1 +
 include/ui/console.h  | 75 ---
 ui/console.c  | 59 
 ui/curses.c   | 14 +-
 ui/egl-headless.c | 20 +-
 ui/gtk.c  | 17 ++--
 ui/sdl.c  | 24 +
 ui/sdl2.c | 17 ++--
 vl.c  | 74 --
 ui/Makefile.objs  | 31 +
 ui/cocoa.m| 14 +-
 13 files changed, 208 insertions(+), 168 deletions(-)

-- 
2.9.3

[Qemu-devel] [PATCH 01/12] console: add qemu display registry, add gtk

Add a registry for user interfaces.  Add qemu_display_init and
qemu_display_early_init helper functions for display initialization.

Hook up gtk ui as first user.

Signed-off-by: Gerd Hoffmann 
---
 include/ui/console.h | 32 
 ui/console.c | 34 ++
 ui/gtk.c | 17 +++--
 vl.c | 18 ++
 4 files changed, 67 insertions(+), 34 deletions(-)

diff --git a/include/ui/console.h b/include/ui/console.h
index e6b1227bef..ce3589aadd 100644
--- a/include/ui/console.h
+++ b/include/ui/console.h
@@ -429,6 +429,18 @@ void surface_gl_setup_viewport(QemuGLShader *gls,
int ww, int wh);
 #endif
 
+typedef struct QemuDisplay QemuDisplay;
+
+struct QemuDisplay {
+DisplayType type;
+void (*early_init)(DisplayOptions *opts);
+void (*init)(DisplayState *ds, DisplayOptions *opts);
+};
+
+void qemu_display_register(QemuDisplay *ui);
+void qemu_display_early_init(DisplayOptions *opts);
+void qemu_display_init(DisplayState *ds, DisplayOptions *opts);
+
 /* sdl.c */
 #ifdef CONFIG_SDL
 void sdl_display_early_init(DisplayOptions *opts);
@@ -484,26 +496,6 @@ static inline void curses_display_init(DisplayState *ds, 
DisplayOptions *opts)
 /* input.c */
 int index_from_key(const char *key, size_t key_length);
 
-/* gtk.c */
-#ifdef CONFIG_GTK
-void early_gtk_display_init(DisplayOptions *opts);
-void gtk_display_init(DisplayState *ds, DisplayOptions *opts);
-#else
-static inline void gtk_display_init(DisplayState *ds, DisplayOptions *opts)
-{
-/* This must never be called if CONFIG_GTK is disabled */
-error_report("GTK support is disabled");
-abort();
-}
-
-static inline void early_gtk_display_init(DisplayOptions *opts)
-{
-/* This must never be called if CONFIG_GTK is disabled */
-error_report("GTK support is disabled");
-abort();
-}
-#endif
-
 /* egl-headless.c */
 void egl_headless_init(DisplayOptions *opts);
 
diff --git a/ui/console.c b/ui/console.c
index 36584d039e..8e55a05108 100644
--- a/ui/console.c
+++ b/ui/console.c
@@ -2170,6 +2170,40 @@ PixelFormat qemu_default_pixelformat(int bpp)
 return pf;
 }
 
+static QemuDisplay *dpys[DISPLAY_TYPE__MAX];
+
+void qemu_display_register(QemuDisplay *ui)
+{
+assert(ui->type < DISPLAY_TYPE__MAX);
+dpys[ui->type] = ui;
+}
+
+void qemu_display_early_init(DisplayOptions *opts)
+{
+assert(opts->type < DISPLAY_TYPE__MAX);
+if (opts->type == DISPLAY_TYPE_NONE) {
+return;
+}
+if (dpys[opts->type] == NULL) {
+error_report("Display '%s' is not available.",
+ DisplayType_lookup.array[opts->type]);
+exit(1);
+}
+if (dpys[opts->type]->early_init) {
+dpys[opts->type]->early_init(opts);
+}
+}
+
+void qemu_display_init(DisplayState *ds, DisplayOptions *opts)
+{
+assert(opts->type < DISPLAY_TYPE__MAX);
+if (opts->type == DISPLAY_TYPE_NONE) {
+return;
+}
+assert(dpys[opts->type] != NULL);
+dpys[opts->type]->init(ds, opts);
+}
+
 void qemu_chr_parse_vc(QemuOpts *opts, ChardevBackend *backend, Error **errp)
 {
 int val;
diff --git a/ui/gtk.c b/ui/gtk.c
index ab646b70e1..c63408e036 100644
--- a/ui/gtk.c
+++ b/ui/gtk.c
@@ -2297,7 +2297,7 @@ static void gd_create_menus(GtkDisplayState *s)
 
 static gboolean gtkinit;
 
-void gtk_display_init(DisplayState *ds, DisplayOptions *opts)
+static void gtk_display_init(DisplayState *ds, DisplayOptions *opts)
 {
 VirtualConsole *vc;
 
@@ -2407,7 +2407,7 @@ void gtk_display_init(DisplayState *ds, DisplayOptions 
*opts)
 }
 }
 
-void early_gtk_display_init(DisplayOptions *opts)
+static void early_gtk_display_init(DisplayOptions *opts)
 {
 /* The QEMU code relies on the assumption that it's always run in
  * the C locale. Therefore it is not prepared to deal with
@@ -2450,3 +2450,16 @@ void early_gtk_display_init(DisplayOptions *opts)
 type_register(_gd_vc_type_info);
 #endif
 }
+
+static QemuDisplay qemu_display_gtk = {
+.type   = DISPLAY_TYPE_GTK,
+.early_init = early_gtk_display_init,
+.init   = gtk_display_init,
+};
+
+static void register_gtk(void)
+{
+qemu_display_register(_display_gtk);
+}
+
+type_init(register_gtk);
diff --git a/vl.c b/vl.c
index 9e7235df6d..70458ba708 100644
--- a/vl.c
+++ b/vl.c
@@ -2173,7 +2173,6 @@ static void parse_display(const char *p)
 exit(1);
 #endif
 } else if (strstart(p, "gtk", )) {
-#ifdef CONFIG_GTK
 dpy.type = DISPLAY_TYPE_GTK;
 while (*opts) {
 const char *nextopt;
@@ -2205,10 +2204,6 @@ static void parse_display(const char *p)
 }
 opts = nextopt;
 }
-#else
-error_report("GTK support is disabled");
-exit(1);
-#endif
 } else if (strstart(p, "none", )) {
 dpy.type = DISPLAY_TYPE_NONE;
 } else {
@@ -4318,6 +4313,9 @@ int main(int argc, char **argv, char **envp)

[Qemu-devel] [PATCH 09/12] configure: opengl doesn't depend on x11

So remove x11 from pkg-config check and don't
add x11 cflags/libs to opengl cflags/libs.

Signed-off-by: Gerd Hoffmann 
---
 configure | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/configure b/configure
index 2934a4bcff..66c210026a 100755
--- a/configure
+++ b/configure
@@ -3767,9 +3767,9 @@ libs_softmmu="$libs_softmmu $fdt_libs"
 
 if test "$opengl" != "no" ; then
   opengl_pkgs="epoxy libdrm gbm"
-  if $pkg_config $opengl_pkgs x11; then
-opengl_cflags="$($pkg_config --cflags $opengl_pkgs) $x11_cflags"
-opengl_libs="$($pkg_config --libs $opengl_pkgs) $x11_libs"
+  if $pkg_config $opengl_pkgs; then
+opengl_cflags="$($pkg_config --cflags $opengl_pkgs)"
+opengl_libs="$($pkg_config --libs $opengl_pkgs)"
 opengl=yes
 if test "$gtk" = "yes" && $pkg_config --exists "$gtkpackage >= 3.16"; then
 gtk_gl="yes"
-- 
2.9.3

[Qemu-devel] [PATCH 10/12] sdl: build as ui module

Shared library dependencies dropped from qemu-system-*:

libSDL2-2.0.so.0 => /lib64/libSDL2-2.0.so.0

Signed-off-by: Gerd Hoffmann 
---
 configure| 2 +-
 Makefile.objs| 1 +
 ui/Makefile.objs | 3 ++-
 3 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/configure b/configure
index 66c210026a..fe80940bdc 100755
--- a/configure
+++ b/configure
@@ -6030,7 +6030,7 @@ if test "$have_x11" = "yes" -a "$need_x11" = "yes"; then
   echo "X11_LIBS=$x11_libs" >> $config_host_mak
 fi
 if test "$sdl" = "yes" ; then
-  echo "CONFIG_SDL=y" >> $config_host_mak
+  echo "CONFIG_SDL=m" >> $config_host_mak
   echo "CONFIG_SDLABI=$sdlabi" >> $config_host_mak
   echo "SDL_CFLAGS=$sdl_cflags" >> $config_host_mak
   echo "SDL_LIBS=$sdl_libs" >> $config_host_mak
diff --git a/Makefile.objs b/Makefile.objs
index 5dc134818c..57ca6d908b 100644
--- a/Makefile.objs
+++ b/Makefile.objs
@@ -56,6 +56,7 @@ common-obj-y += hw/
 common-obj-y += replay/
 
 common-obj-y += ui/
+common-obj-m += ui/
 common-obj-y += bt-host.o bt-vhci.o
 bt-host.o-cflags := $(BLUEZ_CFLAGS)
 
diff --git a/ui/Makefile.objs b/ui/Makefile.objs
index 9b725b07aa..ef4bd83fde 100644
--- a/ui/Makefile.objs
+++ b/ui/Makefile.objs
@@ -11,7 +11,6 @@ common-obj-y += keymaps.o console.o cursor.o qemu-pixman.o
 common-obj-y += input.o input-keymap.o input-legacy.o
 common-obj-$(CONFIG_LINUX) += input-linux.o
 common-obj-$(CONFIG_SPICE) += spice-core.o spice-input.o spice-display.o
-common-obj-$(CONFIG_SDL) += sdl.mo
 common-obj-$(CONFIG_COCOA) += cocoa.o
 common-obj-$(CONFIG_CURSES) += curses.o
 common-obj-$(CONFIG_VNC) += $(vnc-obj-y)
@@ -22,6 +21,8 @@ common-obj-$(CONFIG_X11) += x_keymap.o
 x_keymap.o-cflags := $(X11_CFLAGS)
 x_keymap.o-libs := $(X11_LIBS)
 
+# ui-sdl module
+common-obj-$(CONFIG_SDL) += sdl.mo
 ifeq ($(CONFIG_SDLABI),1.2)
 sdl.mo-objs := sdl.o sdl_zoom.o
 endif
-- 
2.9.3

[Qemu-devel] [PATCH 08/12] configure: add X11 vars to config-host.mak

Simplifies handling the X11 dependency,
also makes ui/Makefile.objs more readable.

Signed-off-by: Gerd Hoffmann 
---
 configure| 10 --
 ui/Makefile.objs |  5 -
 2 files changed, 12 insertions(+), 3 deletions(-)

diff --git a/configure b/configure
index ed45a3c4dd..2934a4bcff 100755
--- a/configure
+++ b/configure
@@ -2508,9 +2508,8 @@ fi
 
 ##
 # X11 probe
-x11_cflags=
-x11_libs=-lX11
 if $pkg_config --exists "x11"; then
+have_x11=yes
 x11_cflags=$($pkg_config --cflags x11)
 x11_libs=$($pkg_config --libs x11)
 fi
@@ -2543,6 +2542,7 @@ if test "$gtk" != "no"; then
 gtk_libs=$($pkg_config --libs $gtkpackage)
 gtk_version=$($pkg_config --modversion $gtkpackage)
 if $pkg_config --exists "$gtkx11package >= $gtkversion"; then
+need_x11=yes
 gtk_cflags="$gtk_cflags $x11_cflags"
 gtk_libs="$gtk_libs $x11_libs"
 fi
@@ -2911,6 +2911,7 @@ if test "$sdl" = "yes" ; then
 int main(void) { return 0; }
 EOF
   if compile_prog "$sdl_cflags $x11_cflags" "$sdl_libs $x11_libs" ; then
+need_x11=yes
 sdl_cflags="$sdl_cflags $x11_cflags"
 sdl_libs="$sdl_libs $x11_libs"
   fi
@@ -6023,6 +6024,11 @@ if test "$modules" = "yes"; then
   echo "CONFIG_STAMP=_$( (echo $qemu_version; echo $pkgversion; cat $0) | 
$shacmd - | cut -f1 -d\ )" >> $config_host_mak
   echo "CONFIG_MODULES=y" >> $config_host_mak
 fi
+if test "$have_x11" = "yes" -a "$need_x11" = "yes"; then
+  echo "CONFIG_X11=y" >> $config_host_mak
+  echo "X11_CFLAGS=$x11_cflags" >> $config_host_mak
+  echo "X11_LIBS=$x11_libs" >> $config_host_mak
+fi
 if test "$sdl" = "yes" ; then
   echo "CONFIG_SDL=y" >> $config_host_mak
   echo "CONFIG_SDLABI=$sdlabi" >> $config_host_mak
diff --git a/ui/Makefile.objs b/ui/Makefile.objs
index ced7d91a63..9b725b07aa 100644
--- a/ui/Makefile.objs
+++ b/ui/Makefile.objs
@@ -17,7 +17,10 @@ common-obj-$(CONFIG_CURSES) += curses.o
 common-obj-$(CONFIG_VNC) += $(vnc-obj-y)
 common-obj-$(call lnot,$(CONFIG_VNC)) += vnc-stubs.o
 common-obj-$(CONFIG_GTK) += gtk.o
-common-obj-$(if $(CONFIG_WIN32),n,$(if $(CONFIG_SDL),y,$(CONFIG_GTK))) += 
x_keymap.o
+
+common-obj-$(CONFIG_X11) += x_keymap.o
+x_keymap.o-cflags := $(X11_CFLAGS)
+x_keymap.o-libs := $(X11_LIBS)
 
 ifeq ($(CONFIG_SDLABI),1.2)
 sdl.mo-objs := sdl.o sdl_zoom.o
-- 
2.9.3

Re: [Qemu-devel] [PATCH v3] specs/qcow2: Fix documentation of the compressed cluster descriptor

2018-02-21 Thread Kevin Wolf

Am 21.02.2018 um 15:08 hat Alberto Garcia geschrieben:
> This patch fixes several mistakes in the documentation of the
> compressed cluster descriptor:
> 
> 1) the documentation claims that the cluster descriptor contains the
>number of sectors used to store the compressed data, but what it
>actually contains is the number of sectors *minus one* or, in other
>words, the number of additional sectors after the first one.
> 
> 2) the width of the fields is incorrectly specified. The number of bits
>used by each field is
> 
>   x = 62 - (cluster_bits - 8)   for the offset field
>   y = (cluster_bits - 8)for the size field
> 
>So the offset field's location is [0, x-1], not [0, x] as stated.
> 
> 3) the size field does not contain the size of the compressed data,
>but rather the number of sectors where that data is stored. The
>compressed data starts at the exact point specified in the offset
>field and ends when there's enough data to produce a cluster of
>decompressed data. Both points can be in the middle of a sector,
>allowing several compressed clusters to be stored next to one
>another, sharing sectors if necessary.
> 
> Signed-off-by: Alberto Garcia 

Thanks, applied to the block branch.

Kevin

Re: [Qemu-devel] [PATCH v7 18/23] qmp: support out-of-band (oob) execution

On Wed, Jan 24, 2018 at 01:39:52PM +0800, Peter Xu wrote:
> Having "allow-oob" to true for a command does not mean that this command
> will always be run in out-of-band mode.  The out-of-band quick path will
> only be executed if we specify the extra "run-oob" flag when sending the
> QMP request:
> 
> { "execute":   "command-that-allows-oob",
>   "arguments": { ... },
>   "control":   { "run-oob": true } }
> 
> The "control" key is introduced to store this extra flag.  "control"
> field is used to store arguments that are shared by all the commands,
> rather than command specific arguments.  Let "run-oob" be the first.
> 
> Note that in the patch I exported qmp_dispatch_check_obj() to be used to
> check the request earlier, and at the same time allowed "id" field to be
> there since actually we always allow that.
> 
> Signed-off-by: Peter Xu 
> ---
>  include/qapi/qmp/dispatch.h |  2 ++
>  monitor.c   | 84 
> -
>  qapi/qmp-dispatch.c | 32 -
>  trace-events|  2 ++
>  4 files changed, 110 insertions(+), 10 deletions(-)

Reviewed-by: Stefan Hajnoczi 


signature.asc
Description: PGP signature

Re: [Qemu-devel] [PATCH 2/2] qcow2: Avoid memory over-allocation on compressed images


On 02/21/2018 10:51 AM, Kevin Wolf wrote:

Am 20.02.2018 um 23:24 hat Eric Blake geschrieben:

When reading a compressed image, we were allocating s->cluster_data
to 32*cluster_size + 512 (possibly over 64 megabytes, for an image
with 2M clusters).  Let's check out the history:




Much later, in commit de82815d (v2.2), we noticed that a 64M
allocation is prone to failure, so we switched over to a graceful
memory allocation error message.  But note that elsewhere in the
code, we do g_malloc(2 * cluster_size) without ever checking for
failure.




-}
-if (!s->cluster_cache) {
-s->cluster_cache = g_malloc(s->cluster_size);
+assert(!s->cluster_cache);


Wouldn't it be better to assert (!!s->cluster_cache ==
!!s->cluster_data) unconditionally?



Sure.


+s->cluster_data = g_try_malloc(s->cluster_size);


Why are you going from qemu_try_blockalign() to simple malloc here? This
buffer is used with bdrv_read() (or bdrv_pread() after patch 1), so we
should avoid unnecessary use of a bounce buffer.


But does bdrv_pread() actually need to use a bounce buffer if we don't 
have an aligned buffer to read into?  Either the underlying protocol 
already supports byte-aligned reads (direct into our buffer, regardless 
of alignment, no bouncing required), or it already has do to a full 
sector read into a bounce buffer anyways (and it doesn't matter whether 
we aligned our buffer).  blockalign() made sense when we had multiple 
clients for the buffer, but ever since v1.1, when we have only a single 
client, and that single client is most likely not going to read 
sector-aligned data in the first place, aligning the buffer doesn't buy 
us anything.





+s->cluster_cache = g_try_malloc(s->cluster_size);


As you already said, either g_malloc() or check the result. I actually
think that g_try_malloc() and checking the result is nicer, we still
allocate up to 2 MB here.


See my commit message comment - we have other spots in the code base 
that blindly g_malloc(2 * s->cluster_size).  And I intended (but sent 
the email without amending my commit) to use g_malloc().  But as Berto 
has convinced me that an externally produced image can convince us to 
read up to 4M (even though we don't need that much to decompress), I 
suppose that the try_ variant plus checking is reasonable (and care in 
NULL'ing out if one but not both allocations succeed).


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

Re: [Qemu-devel] [PATCH v1 1/3] s390x/sclp: proper support of larger send and receive masks

On Wed, 21 Feb 2018 17:28:49 +0100
Claudio Imbrenda  wrote:

> On Wed, 21 Feb 2018 16:12:59 +0100
> Cornelia Huck  wrote:
> 
> > On Tue, 20 Feb 2018 19:45:00 +0100
> > Claudio Imbrenda  wrote:
> >   
> > > The architecture allows the guests to ask for masks up to 1021
> > > bytes in length. Part was fixed in
> > > 67915de9f0383ccf4ab8c42dd02aa18dcd79b411 ("s390x/event-facility:
> > > variable-length event masks"), but some issues were still
> > > remaining, in particular regarding the handling of selective reads.
> > > 
> > > This patch fixes the handling of selective reads, whose size will
> > > now match the length of the event mask, as per architecture.
> > > 
> > > The default behaviour is to be compliant with the architecture, but
> > > when using older machine models the old behaviour is selected, in
> > > order to be able to migrate toward older versions.
> > 
> > I remember trying to do this back when I still had access to the
> > architecture, but somehow never finished this (don't remember why).
> >   
> > > 
> > > Fixes: 67915de9f0383ccf4a ("s390x/event-facility: variable-length
> > > event masks")
> > 
> > Doesn't that rather fix the initial implementation? What am I missing?  
> 
> well that too. but I think this fixes the fix, since the fix was
> incomplete?
> 
> > > Signed-off-by: Claudio Imbrenda 
> > > ---
> > >  hw/s390x/event-facility.c  | 90
> > > +++---
> > > hw/s390x/s390-virtio-ccw.c |  8 - 2 files changed, 84
> > > insertions(+), 14 deletions(-)
> > > 
> > > diff --git a/hw/s390x/event-facility.c b/hw/s390x/event-facility.c
> > > index 155a694..2414614 100644
> > > --- a/hw/s390x/event-facility.c
> > > +++ b/hw/s390x/event-facility.c
> > > @@ -31,6 +31,14 @@ struct SCLPEventFacility {
> > >  SCLPEventsBus sbus;
> > >  /* guest' receive mask */
> > >  unsigned int receive_mask;
> > > +/*
> > > + * when false, we keep the same broken, backwards compatible
> > > behaviour as
> > > + * before; when true, we implement the architecture correctly.
> > > Needed for
> > > + * migration toward older versions.
> > > + */
> > > +bool allow_all_mask_sizes;
> > 
> > The comment does not really tell us what the old behaviour is ;)  
> 
> hmm, I'll fix that
> 
> > So, if this is about extending the mask size, call this
> > "allow_large_masks" or so?  
> 
> no, the old broken behaviour allowed only _exactly_ 4 bytes:
> 
> if (be16_to_cpu(we_mask->mask_length) != 4) {
>  sccb->h.response_code = cpu_to_be16(SCLP_RC_INVALID_MASK_LENGTH);
>  goto out;
> }

Hm, I can't seem to find this in the code?

> 
> With the 67915de9f0383ccf4a patch mentioned above, we allow any size,
> but we don't keep track of the size itself, only the bits. This is a
> problem for selective reads (see below)

Oh, you meant before that patch...

> 
> > > +/* length of the receive mask */
> > > +uint16_t mask_length;
> > >  };
> > >  
> > >  /* return true if any child has event pending set */
> > > @@ -220,6 +228,17 @@ static uint16_t
> > > handle_sccb_read_events(SCLPEventFacility *ef, SCCB *sccb, return
> > > rc; }
> > >  
> > > +/* copy up to dst_len bytes and fill the rest of dst with zeroes */
> > > +static void copy_mask(uint8_t *dst, uint8_t *src, uint16_t dst_len,
> > > +  uint16_t src_len)
> > > +{
> > > +int i;
> > > +
> > > +for (i = 0; i < dst_len; i++) {
> > > +dst[i] = i < src_len ? src[i] : 0;
> > > +}
> > > +}
> > > +
> > >  static void read_event_data(SCLPEventFacility *ef, SCCB *sccb)
> > >  {
> > >  unsigned int sclp_active_selection_mask;
> > > @@ -240,7 +259,9 @@ static void read_event_data(SCLPEventFacility
> > > *ef, SCCB *sccb) sclp_active_selection_mask = sclp_cp_receive_mask;
> > >  break;
> > >  case SCLP_SELECTIVE_READ:
> > > -sclp_active_selection_mask = be32_to_cpu(red->mask);
> > > +copy_mask((uint8_t *)_active_selection_mask, (uint8_t
> > > *)>mask,
> > > +  sizeof(sclp_active_selection_mask),
> > > ef->mask_length);
> > > +sclp_active_selection_mask =
> > > be32_to_cpu(sclp_active_selection_mask);
> > 
> > Hm, this looks like a real bug fix for the commit referenced above.
> > Split this out? We don't need compat support for that; maybe even
> > cc:stable?  
> 
> I think we do. We can avoid keeping track of the mask size when setting
> the mask size, because we can simply take the bits we need and ignore
> the rest. But for selective reads we need the mask size, so we have to
> keep track of it. (selective reads specify a mask, but not a mask
> length, the mask length is the length of the last mask set)

OK, that's non-obvious without documentation :/

> 
> And now we have additional state that we need to copy around when
> migrating. And we don't want to break older machines! Moreover a

Re: [Qemu-devel] [PATCH 0/2] Firmware blob and git submodule for Sam460ex

2018-02-21 Thread BALATON Zoltan


On Wed, 21 Feb 2018, Peter Maydell wrote:

On 20 February 2018 at 20:44, Emilio G. Cota  wrote:

On Tue, Feb 20, 2018 at 18:31:17 +, Peter Maydell wrote:

On 20 February 2018 at 18:10, BALATON Zoltan  wrote:

I've created a git repo for the Sam460ex u-boot sources and this adds
that as a submodule and a separate patch to add the binary built from
these sources. Feel free to keep this as two patches, squash them into
one patch or take the git repo and commit the content under the QEMU
repo and use that as a submodule as you see fit (or let me know if any
changes are needed for these patches).

BALATON Zoltan (2):
  roms: Added git submodule for u-boot-sam460 (firmware for sam460ex)
  pc-bios: Added u-boot-sam460 firmware binary


We already have a submodule for u-boot. Is it not possible to
build this bios blob from those upstream u-boot sources?


This is discussed in the following thread:
  Re: [Qemu-ppc] [PATCH v3 2/2] ppc: Add aCube Sam460ex board
  http://lists.gnu.org/archive/html/qemu-ppc/2018-02/msg00268.html


If upstream u-boot have abandoned the board support I'm not very
enthusiastic about our taking it on :-(


It's not that upstream u-boot has abandoned board support (it only removed 
support for the PPC440 CPU it once had). The board itself never had 
support in upstream u-boot, it only exists in vendor's fork which is the 
reason we need a separate source and cannot use upstream u-boot source we 
already have.


In my opinion we don't aim to take on support for this board in u-boot, we 
only need to include the firmware binary for the emulation to be useful 
which then requires us to also include the source for the GPL it's 
licensed under. I've also found a few bugs in the firmware which I've 
fixed but apart from such occasional bug fixes when needed I don't expect 
to take over support for the board from the hardware vendor so this source 
is only so we can include the firmware binary which is needed for the 
board emulation. Does this answer your concerns?


Regards,
BALATON Zoltan

[Qemu-devel] [RFC PATCH 2/5] kbd-state: add hotkey registry

Add support to register hotkeys and to check whenever a given QKeyCode
combined with the current modifier state is a hotkey.  A hotkey can be
any key combined with up to three modifier keys.

Signed-off-by: Gerd Hoffmann 
---
 include/ui/kbd-state.h | 27 +++
 ui/kbd-state.c | 41 +
 2 files changed, 64 insertions(+), 4 deletions(-)

diff --git a/include/ui/kbd-state.h b/include/ui/kbd-state.h
index c961da45b2..3f13649b63 100644
--- a/include/ui/kbd-state.h
+++ b/include/ui/kbd-state.h
@@ -20,3 +20,30 @@ bool kbd_state_key_get(KbdState *kbd, QKeyCode qcode);
 void kbd_state_key_event(KbdState *kbd, QKeyCode qcode, bool down);
 void kbd_state_lift_all_keys(KbdState *kbd);
 KbdState *kbd_state_init(QemuConsole *con);
+
+/* -- */
+
+typedef enum KbdHotkey KbdHotkey;
+
+enum KbdHotkey {
+KBD_HOTKEY_NONE = 0,
+
+KBD_HOTKEY_GRAB,
+KBD_HOTKEY_FULLSCREEN,
+KBD_HOTKEY_REDRAW,
+
+KBD_HOTKEY_CONSOLE_1,
+KBD_HOTKEY_CONSOLE_2,
+KBD_HOTKEY_CONSOLE_3,
+KBD_HOTKEY_CONSOLE_4,
+KBD_HOTKEY_CONSOLE_5,
+KBD_HOTKEY_CONSOLE_6,
+KBD_HOTKEY_CONSOLE_7,
+KBD_HOTKEY_CONSOLE_8,
+KBD_HOTKEY_CONSOLE_9,
+};
+
+void kbd_state_hotkey_register(KbdState *kbd, KbdHotkey, QKeyCode qcode,
+   KbdModifier mod1, KbdModifier mod2,
+   KbdModifier mod3);
+KbdHotkey kbd_state_hotkey_get(KbdState *kbd, QKeyCode qcode);
diff --git a/ui/kbd-state.c b/ui/kbd-state.c
index 7a9fe268c2..812cb368e3 100644
--- a/ui/kbd-state.c
+++ b/ui/kbd-state.c
@@ -6,20 +6,20 @@
 #include "ui/input.h"
 #include "ui/kbd-state.h"
 
-typedef struct KbdHotkey KbdHotkey;
+typedef struct KbdHotkeyEntry KbdHotkeyEntry;
 
-struct KbdHotkey {
+struct KbdHotkeyEntry {
 uint32_t id;
 QKeyCode qcode;
 DECLARE_BITMAP(mods, KBD_MOD__MAX);
-QTAILQ_ENTRY(KbdHotkey) next;
+QTAILQ_ENTRY(KbdHotkeyEntry) next;
 };
 
 struct KbdState {
 QemuConsole *con;
 DECLARE_BITMAP(keys, Q_KEY_CODE__MAX);
 DECLARE_BITMAP(mods, KBD_MOD__MAX);
-QTAILQ_HEAD(,KbdHotkey) hotkeys;
+QTAILQ_HEAD(, KbdHotkeyEntry) hotkeys;
 };
 
 static void kbd_state_modifier_update(KbdState *kbd,
@@ -117,3 +117,36 @@ KbdState *kbd_state_init(QemuConsole *con)
 
 return kbd;
 }
+
+void kbd_state_hotkey_register(KbdState *kbd, KbdHotkey id, QKeyCode qcode,
+   KbdModifier mod1, KbdModifier mod2,
+   KbdModifier mod3)
+{
+KbdHotkeyEntry *hotkey = g_new0(KbdHotkeyEntry, 1);
+
+hotkey->id= id;
+hotkey->qcode = qcode;
+if (mod1 != KBD_MOD_NONE) {
+set_bit(mod1, hotkey->mods);
+}
+if (mod2 != KBD_MOD_NONE) {
+set_bit(mod2, hotkey->mods);
+}
+if (mod3 != KBD_MOD_NONE) {
+set_bit(mod3, hotkey->mods);
+}
+QTAILQ_INSERT_TAIL(>hotkeys, hotkey, next);
+}
+
+KbdHotkey kbd_state_hotkey_get(KbdState *kbd, QKeyCode qcode)
+{
+KbdHotkeyEntry *hotkey;
+
+QTAILQ_FOREACH(hotkey, >hotkeys, next) {
+if (qcode == hotkey->qcode &&
+bitmap_equal(kbd->mods, hotkey->mods, KBD_MOD__MAX)) {
+return hotkey->id;
+}
+}
+return KBD_HOTKEY_NONE;
+}
-- 
2.9.3

[Qemu-devel] [RFC PATCH 1/5] kbd-state: add keyboard state tracker

Now that most user interfaces are using QKeyCodes it is easier to have
common keyboard code useable by all user interfaces.

This patch adds helper code to track the state of all keyboard keys,
using a bitmap indexed by QKeyCode.  Modifier state is tracked too,
as separate bitmap.  That makes checking modifier state easier.
Likewise we can easily apply special handling for capslock & numlock
(toggles on keypress) and ctrl + shift (we have two keys for that).

Signed-off-by: Gerd Hoffmann 
---
 include/ui/kbd-state.h |  22 +
 ui/kbd-state.c | 119 +
 ui/Makefile.objs   |   2 +-
 3 files changed, 142 insertions(+), 1 deletion(-)
 create mode 100644 include/ui/kbd-state.h
 create mode 100644 ui/kbd-state.c

diff --git a/include/ui/kbd-state.h b/include/ui/kbd-state.h
new file mode 100644
index 00..c961da45b2
--- /dev/null
+++ b/include/ui/kbd-state.h
@@ -0,0 +1,22 @@
+typedef enum KbdModifier KbdModifier;
+
+enum KbdModifier {
+KBD_MOD_NONE = 0,
+
+KBD_MOD_SHIFT,
+KBD_MOD_CTRL,
+KBD_MOD_ALT,
+
+KBD_MOD_NUMLOCK,
+KBD_MOD_CAPSLOCK,
+
+KBD_MOD__MAX
+};
+
+typedef struct KbdState KbdState;
+
+bool kbd_state_modifier_get(KbdState *kbd, KbdModifier mod);
+bool kbd_state_key_get(KbdState *kbd, QKeyCode qcode);
+void kbd_state_key_event(KbdState *kbd, QKeyCode qcode, bool down);
+void kbd_state_lift_all_keys(KbdState *kbd);
+KbdState *kbd_state_init(QemuConsole *con);
diff --git a/ui/kbd-state.c b/ui/kbd-state.c
new file mode 100644
index 00..7a9fe268c2
--- /dev/null
+++ b/ui/kbd-state.c
@@ -0,0 +1,119 @@
+#include "qemu/osdep.h"
+#include "qemu/bitmap.h"
+#include "qemu/queue.h"
+#include "qapi-types.h"
+#include "ui/console.h"
+#include "ui/input.h"
+#include "ui/kbd-state.h"
+
+typedef struct KbdHotkey KbdHotkey;
+
+struct KbdHotkey {
+uint32_t id;
+QKeyCode qcode;
+DECLARE_BITMAP(mods, KBD_MOD__MAX);
+QTAILQ_ENTRY(KbdHotkey) next;
+};
+
+struct KbdState {
+QemuConsole *con;
+DECLARE_BITMAP(keys, Q_KEY_CODE__MAX);
+DECLARE_BITMAP(mods, KBD_MOD__MAX);
+QTAILQ_HEAD(,KbdHotkey) hotkeys;
+};
+
+static void kbd_state_modifier_update(KbdState *kbd,
+  QKeyCode qcode1, QKeyCode qcode2,
+  KbdModifier mod)
+{
+if (test_bit(qcode1, kbd->keys) || test_bit(qcode2, kbd->keys)) {
+set_bit(mod, kbd->mods);
+} else {
+clear_bit(mod, kbd->mods);
+}
+}
+
+bool kbd_state_modifier_get(KbdState *kbd, KbdModifier mod)
+{
+return test_bit(mod, kbd->mods);
+}
+
+bool kbd_state_key_get(KbdState *kbd, QKeyCode qcode)
+{
+return test_bit(qcode, kbd->keys);
+}
+
+void kbd_state_key_event(KbdState *kbd, QKeyCode qcode, bool down)
+{
+bool state = test_bit(qcode, kbd->keys);
+
+if (state == down) {
+/*
+ * Filter out events which don't change the keyboard state.
+ *
+ * Most notably this allows to simply send along all key-up
+ * events, and this function will filter out everything where
+ * the corresponding key-down event wasn't send to the guest,
+ * for example due to being a host hotkey.
+ */
+return;
+}
+
+/* update key and modifier state */
+change_bit(qcode, kbd->keys);
+switch (qcode) {
+case Q_KEY_CODE_SHIFT:
+case Q_KEY_CODE_SHIFT_R:
+kbd_state_modifier_update(kbd, Q_KEY_CODE_SHIFT, Q_KEY_CODE_SHIFT_R,
+  KBD_MOD_SHIFT);
+break;
+case Q_KEY_CODE_CTRL:
+case Q_KEY_CODE_CTRL_R:
+kbd_state_modifier_update(kbd, Q_KEY_CODE_CTRL, Q_KEY_CODE_CTRL_R,
+  KBD_MOD_CTRL);
+break;
+case Q_KEY_CODE_ALT:
+kbd_state_modifier_update(kbd, Q_KEY_CODE_ALT, Q_KEY_CODE_ALT,
+  KBD_MOD_ALT);
+break;
+case Q_KEY_CODE_CAPS_LOCK:
+if (down) {
+change_bit(KBD_MOD_CAPSLOCK, kbd->mods);
+}
+break;
+case Q_KEY_CODE_NUM_LOCK:
+if (down) {
+change_bit(KBD_MOD_NUMLOCK, kbd->mods);
+}
+break;
+default:
+/* keep gcc happy */
+break;
+}
+
+/* send to guest */
+if (qemu_console_is_graphic(kbd->con)) {
+qemu_input_event_send_key_qcode(kbd->con, qcode, down);
+}
+}
+
+void kbd_state_lift_all_keys(KbdState *kbd)
+{
+int qcode;
+
+for (qcode = 0; qcode < Q_KEY_CODE__MAX; qcode++) {
+if (test_bit(qcode, kbd->keys)) {
+kbd_state_key_event(kbd, qcode, false);
+}
+}
+}
+
+KbdState *kbd_state_init(QemuConsole *con)
+{
+KbdState *kbd = g_new0(KbdState, 1);
+
+kbd->con = con;
+QTAILQ_INIT(>hotkeys);
+
+return kbd;
+}
diff --git a/ui/Makefile.objs b/ui/Makefile.objs
index ced7d91a63..aa81ce4c47 100644
--- a/ui/Makefile.objs
+++ b/ui/Makefile.objs
@@ -8,7 +8,7 @@ vnc-obj-y +=

Re: [Qemu-devel] [PATCH v7 14/23] monitor: separate QMP parser and dispatcher

On Wed, Jan 24, 2018 at 01:39:48PM +0800, Peter Xu wrote:
> Originally QMP goes through these steps:
> 
>   JSON Parser --> QMP Dispatcher --> Respond
>   /|\(2)(3) |
>(1) |   \|/ (4)
>+-  main thread  +
> 
> This patch does this:
> 
>   JSON Parser QMP Dispatcher --> Respond
>   /|\ |   /|\   (4) |
>|  | (2)| (3)|  (5)
>(1) |  +->  |   \|/
>+-  main thread  <---+
> 
> So the parsing job and the dispatching job is isolated now.  It gives us
> a chance in following up patches to totally move the parser outside.
> 
> The isolation is done using one QEMUBH. Only one dispatcher QEMUBH is
> used for all the monitors.
> 
> Signed-off-by: Peter Xu 
> ---
>  monitor.c | 201 
> +++---
>  1 file changed, 178 insertions(+), 23 deletions(-)

Reviewed-by: Stefan Hajnoczi 


signature.asc
Description: PGP signature

Re: [Qemu-devel] [PATCH v7 16/23] monitor: send event when command queue full

On Wed, Jan 24, 2018 at 01:39:50PM +0800, Peter Xu wrote:
> Set maximum QMP command queue length to 8.  If queue full, instead of
> queue the command, we directly return a "command-dropped" event, telling
> client that specific command is dropped.
> 
> Note that this flow control mechanism is only valid if OOB is enabled.
> If it's not, the effective queue length will always be 1, which strictly
> follows original behavior of QMP command handling (which never drop
> messages).
> 
> Signed-off-by: Peter Xu 
> ---
>  monitor.c | 18 +-
>  1 file changed, 17 insertions(+), 1 deletion(-)

Reviewed-by: Stefan Hajnoczi 


signature.asc
Description: PGP signature

Re: [Qemu-devel] [PATCH v1 2/3] s390x/sclp: clean up sclp masks

On Wed, 21 Feb 2018 17:42:57 +0100
Claudio Imbrenda  wrote:

> On Wed, 21 Feb 2018 16:20:05 +0100
> Cornelia Huck  wrote:
> 
> > On Tue, 20 Feb 2018 19:45:01 +0100
> > Claudio Imbrenda  wrote:

> > > diff --git a/include/hw/s390x/event-facility.h
> > > b/include/hw/s390x/event-facility.h index 5119b9b..0a8b47a 100644
> > > --- a/include/hw/s390x/event-facility.h
> > > +++ b/include/hw/s390x/event-facility.h
> > > @@ -28,12 +28,14 @@
> > >  #define SCLP_EVENT_SIGNAL_QUIESCE   0x1d
> > >  
> > >  /* SCLP event masks */
> > > -#define SCLP_EVENT_MASK_SIGNAL_QUIESCE  0x0008
> > > -#define SCLP_EVENT_MASK_MSG_ASCII   0x0040
> > > -#define SCLP_EVENT_MASK_CONFIG_MGT_DATA 0x1000
> > > -#define SCLP_EVENT_MASK_OP_CMD  0x8000
> > > -#define SCLP_EVENT_MASK_MSG 0x4000
> > > -#define SCLP_EVENT_MASK_PMSGCMD 0x0080
> > > +#define SCLPEVMSK(T)  (1ULL << (sizeof(sccb_mask_t) * 8 - (T)))
> > 
> > SCLP_EVMASK() would be a bit more readable, I think.  
> 
> I know, but then it looks ugly when trying to fit everything in 80
> columns. 

I'd rather go slightly over 80 in that case (as long as you don't cross
90).

> 
> > > +
> > > +#define SCLP_EVENT_MASK_OP_CMD
> > > SCLPEVMSK(SCLP_EVENT_OPRTNS_COMMAND) +#define
> > > SCLP_EVENT_MASK_MSG SCLPEVMSK(SCLP_EVENT_MESSAGE)
> > > +#define SCLP_EVENT_MASK_CONFIG_MGT_DATA
> > > SCLPEVMSK(SCLP_EVENT_CONFIG_MGT_DATA) +#define
> > > SCLP_EVENT_MASK_PMSGCMD SCLPEVMSK(SCLP_EVENT_PMSGCMD)
> > > +#define SCLP_EVENT_MASK_MSG_ASCII
> > > SCLPEVMSK(SCLP_EVENT_ASCII_CONSOLE_DATA) +#define
> > > SCLP_EVENT_MASK_SIGNAL_QUIESCE
> > > SCLPEVMSK(SCLP_EVENT_SIGNAL_QUIESCE) #define
> > > SCLP_UNCONDITIONAL_READ 0x00 #define
> > > SCLP_SELECTIVE_READ 0x01
> >   
>

Re: [Qemu-devel] [PATCH v2 1/1] 390x/cpumodel: document S390FeatDef.bit not applicable

2018-02-21 Thread David Hildenbrand

On 21.02.2018 17:56, Halil Pasic wrote:
> The 'bit' field of the 'S390FeatDef' structure is not applicable to all
> its instances. Currently this field is not applicable, and remains
> unused, iff the feature is of type S390_FEAT_TYPE_MISC. Having the value 0
> specified for multiple such feature definition  was a little confusing,

s/  / /

> as it's a perfectly legit bit value, and as the value of the bit
> field is usually ought to be unique for each feature of a given
> feature type.
> 
> Let us introduce a specialized macro for defining features of type
> S390_FEAT_TYPE_MISC so, that one does not have to specify neither bit nor
> type (as the later is implied).

s/later is implied/latter is implicit/

> 
> Signed-off-by: Halil Pasic 
> ---
> 
> v1 -> v2
> * Specialized feature initializer macro for type MISC that does not
>   require a bit value instead of defining a 'not a bit number' (that
>   is extremal) bit number.
> ---
>  target/s390x/cpu_features.c | 8 ++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/target/s390x/cpu_features.c b/target/s390x/cpu_features.c
> index a5619f2893..3b9e2745e9 100644
> --- a/target/s390x/cpu_features.c
> +++ b/target/s390x/cpu_features.c
> @@ -23,6 +23,10 @@
>  .desc = _desc,   \
>  }
>  
> +/* S390FeatDef.bit is not applicable as there is no feature block. */
> +#define FEAT_INIT_MISC(_name, _desc) \
> +FEAT_INIT(_name, S390_FEAT_TYPE_MISC, 0, _desc)
> +
>  /* indexed by feature number for easy lookup */
>  static const S390FeatDef s390_features[] = {
>  FEAT_INIT("esan3", S390_FEAT_TYPE_STFL, 0, "Instructions marked as n3"),
> @@ -123,8 +127,8 @@ static const S390FeatDef s390_features[] = {
>  FEAT_INIT("ib", S390_FEAT_TYPE_SCLP_CPU, 42, "SIE: Intervention bypass 
> facility"),
>  FEAT_INIT("cei", S390_FEAT_TYPE_SCLP_CPU, 43, "SIE: 
> Conditional-external-interception facility"),
>  
> -FEAT_INIT("dateh2", S390_FEAT_TYPE_MISC, 0, "DAT-enhancement facility 
> 2"),
> -FEAT_INIT("cmm", S390_FEAT_TYPE_MISC, 0, 
> "Collaborative-memory-management facility"),
> +FEAT_INIT_MISC("dateh2", "DAT-enhancement facility 2"),
> +FEAT_INIT_MISC("cmm", "Collaborative-memory-management facility"),
>  
>  FEAT_INIT("plo-cl", S390_FEAT_TYPE_PLO, 0, "PLO Compare and load (32 bit 
> in general registers)"),
>  FEAT_INIT("plo-clg", S390_FEAT_TYPE_PLO, 1, "PLO Compare and load (64 
> bit in parameter list)"),
> 

Reviewed-by: David Hildenbrand 

-- 

Thanks,

David / dhildenb

Re: [Qemu-devel] [PATCH v7 00/23] QMP: out-of-band (OOB) execution support

On Wed, Jan 24, 2018 at 01:39:34PM +0800, Peter Xu wrote:
> This version should have addressed all comments in previous one, also
> fixed another race condition after I addressed all the comments (a new
> race condition introduced by addressing the comments...).  For some
> more details of the race condition, please see the last entry of
> change log, and please refer to patch 9 for the code change.
> 
> I removed RFC tag from this version.  Please review.  Thanks.

I have posted comments on a few patches.

Stefan


signature.asc
Description: PGP signature

Re: [Qemu-devel] [PATCH v2 03/32] target/arm/cpu64: allow fp16 to be disabled

2018-02-21 Thread Richard Henderson

On 02/21/2018 08:35 AM, Alex Bennée wrote:
>> Good news everybody -- this is an opportunity for a naming bikeshed
>> discussion!
...
>>  * use the hwcaps names that Linux defines and prints in /proc/cpuinfo
>>(in this case that would be a combination of "fphp" and "asimdhp",
>>since hwcaps reflects the ID register setup that allows a CPU
>>to report support for one and not the other)
> 
> In naming I favour the Arm ARM over whatever Linux-ism /proc came up
> with.

Likewise.


r~

Re: [Qemu-devel] [PATCH 2/2] qcow2: Avoid memory over-allocation on compressed images


On 02/21/2018 11:39 AM, Kevin Wolf wrote:

See my commit message comment - we have other spots in the code base that
blindly g_malloc(2 * s->cluster_size).


Though is that a reason to do the same in new code or to phase out such
allocations whenever you touch them?


Touché.




And I intended (but sent the email without amending my commit) to use
g_malloc().  But as Berto has convinced me that an externally produced
image can convince us to read up to 4M (even though we don't need that
much to decompress), I suppose that the try_ variant plus checking is
reasonable (and care in NULL'ing out if one but not both allocations
succeed).


Sounds good.

Another thought I had is whether we should do per-request allocation for
compressed clusters, too, instead of having per-BDS buffers.


The only benefit of a per-BDS buffer is that we cache things - multiple 
sub-cluster reads in a row all from the same compressed cluster benefit 
from decompressing only once.  The drawbacks of a per-BDS buffer: we 
can't do things in parallel (everything else in qcow2 drops the lock 
around bdrv_co_pread[v]), so the initial read prevents anything else in 
the qcow2 layer from progressing.


I also wonder - since we ARE allowing multiple parallel readers in other 
parts of qcow2 (without a patch, decompression is not in this boat, but 
decryption and even bounce buffers due to lower-layer alignment 
constraints are), what sort of mechanisms do we have for using a pool of 
reusable buffers, rather than having each cluster access that requires a 
buffer malloc and free the buffer on a per-access basis?  I don't know 
how much time the malloc/free per-transaction overhead adds, or if it is 
already much smaller than the actual I/O time.


But note that while reusable buffers from a pool would cut down on the 
per-I/O malloc/free overhead if we switch decompression away from 
per-BDS buffer, it would still not solve the fact that we only get the 
caching ability where multiple sub-cluster requests from the same 
compressed cluster require only one decompression, since that's only 
possible on a per-BDS caching level.


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

[Qemu-devel] [RFC PATCH 5/5] sdl2: use only QKeyCode in sdl2_process_key()

Small cleanup.  Also drop the special backspace handling,
kbd_put_qcode_console() learned to handle that meanwhile.
And sdl2_process_key is never called with scon == NULL.

Signed-off-by: Gerd Hoffmann 
---
 ui/sdl2-input.c | 10 +++---
 1 file changed, 3 insertions(+), 7 deletions(-)

diff --git a/ui/sdl2-input.c b/ui/sdl2-input.c
index 82ec375ea2..f0847dd745 100644
--- a/ui/sdl2-input.c
+++ b/ui/sdl2-input.c
@@ -39,23 +39,19 @@ void sdl2_process_key(struct sdl2_console *scon,
   SDL_KeyboardEvent *ev)
 {
 int qcode;
-QemuConsole *con = scon ? scon->dcl.con : NULL;
+QemuConsole *con = scon->dcl.con;
 
 if (ev->keysym.scancode >= qemu_input_map_usb_to_qcode_len) {
 return;
 }
-
 qcode = qemu_input_map_usb_to_qcode[ev->keysym.scancode];
 
 if (!qemu_console_is_graphic(con)) {
 if (ev->type == SDL_KEYDOWN) {
-switch (ev->keysym.scancode) {
-case SDL_SCANCODE_RETURN:
+switch (qcode) {
+case Q_KEY_CODE_RET:
 kbd_put_keysym_console(con, '\n');
 break;
-case SDL_SCANCODE_BACKSPACE:
-kbd_put_keysym_console(con, QEMU_KEY_BACKSPACE);
-break;
 default:
 kbd_put_qcode_console(con, qcode);
 break;
-- 
2.9.3

Re: [Qemu-devel] [PATCH v7 15/23] qmp: add new event "command-dropped"

On Wed, Jan 24, 2018 at 01:39:49PM +0800, Peter Xu wrote:
> This event will be emitted if one QMP command is dropped.  Along,
> declare an enum for the reasons.
> 
> Reviewed-by: Fam Zheng 
> Signed-off-by: Peter Xu 
> ---
>  qapi-schema.json | 37 +
>  1 file changed, 37 insertions(+)

Reviewed-by: Stefan Hajnoczi 


signature.asc
Description: PGP signature

Re: [Qemu-devel] [PATCH v1 3/3] s390x/sclp: extend SCLP event masks to 64 bits