Re: [Qemu-devel] [PATCH 2/3] net: Deprecate tap backend's parameter "helper"

2019-06-17 Thread Jason Wang



On 2019/6/18 下午1:32, Markus Armbruster wrote:

Jason Wang  writes:


On 2019/6/4 下午7:52, Markus Armbruster wrote:

-netdev tap,helper=... is a useless duplicate of -netdev bridge.
Deprecate and de-document.

Signed-off-by: Markus Armbruster 


This requires more thought as TAP could be used independently. Force
using a "bridge" backend may lead some confusion.

Can you explain your qualms in a bit more detail?

The thoughts that led to this patch:
https://lists.gnu.org/archive/html/qemu-devel/2019-05/msg03164.html

Consensus back then:

1. Add qemu-bridge-helper.c to Jason's "Network device backends"

2. Deprecate -netdev tap parameter "helper"



The problem comes from this point. The main reason is TAP could be used 
without bridge e.g:


- you can simply assign an IP and and properly configure route table on 
host to make it work


- or setup tc actions or using XDP to transfer packets between TAP and 
another interfaces


- using AF_PACKET or other socket to capture the traffic and do the 
forwarding in userspace


So it looks to me switching to use -netdev bridge is inappropriate.

Thanks





3. Improve documentation of -netdev bridge

4. Create a manual page for qemu-bridge-helper that also covers
/etc/qemu/bridge.conf.

5. Fix the nutty error handling in parse_acl_file()

This series covers the first two [PATCH 1+2], and records the remaining
three more permanently [PATCH 3].





Re: [Qemu-devel] [PATCH 2/3] net: Deprecate tap backend's parameter "helper"

2019-06-17 Thread Markus Armbruster
Jason Wang  writes:

> On 2019/6/4 下午7:52, Markus Armbruster wrote:
>> -netdev tap,helper=... is a useless duplicate of -netdev bridge.
>> Deprecate and de-document.
>>
>> Signed-off-by: Markus Armbruster 
>
>
> This requires more thought as TAP could be used independently. Force
> using a "bridge" backend may lead some confusion.

Can you explain your qualms in a bit more detail?

The thoughts that led to this patch:
https://lists.gnu.org/archive/html/qemu-devel/2019-05/msg03164.html

Consensus back then:

1. Add qemu-bridge-helper.c to Jason's "Network device backends"

2. Deprecate -netdev tap parameter "helper"

3. Improve documentation of -netdev bridge

4. Create a manual page for qemu-bridge-helper that also covers
   /etc/qemu/bridge.conf.

5. Fix the nutty error handling in parse_acl_file()

This series covers the first two [PATCH 1+2], and records the remaining
three more permanently [PATCH 3].



Re: [Qemu-devel] [PATCH v1 1/9] target/riscv: Restructure deprecatd CPUs

2019-06-17 Thread Philippe Mathieu-Daudé
On 6/18/19 3:31 AM, Alistair Francis wrote:
> Restructure the deprecated CPUs to make it clear in the code that these
> are depreated. They are already marked as deprecated in
> qemu-deprecated.texi. There are no functional changes.
> 
> Signed-off-by: Alistair Francis 
> ---
>  target/riscv/cpu.c | 18 ++
>  target/riscv/cpu.h | 13 +++--
>  2 files changed, 17 insertions(+), 14 deletions(-)
> 
> diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
> index 0632ac08cf..a4dd7ae6fc 100644
> --- a/target/riscv/cpu.c
> +++ b/target/riscv/cpu.c
> @@ -558,18 +558,20 @@ static const TypeInfo riscv_cpu_type_infos[] = {
>  DEFINE_CPU(TYPE_RISCV_CPU_ANY,  riscv_any_cpu_init),
>  #if defined(TARGET_RISCV32)
>  DEFINE_CPU(TYPE_RISCV_CPU_BASE32,   riscv_base32_cpu_init),
> -DEFINE_CPU(TYPE_RISCV_CPU_RV32GCSU_V1_09_1, 
> rv32gcsu_priv1_09_1_cpu_init),
> -DEFINE_CPU(TYPE_RISCV_CPU_RV32GCSU_V1_10_0, 
> rv32gcsu_priv1_10_0_cpu_init),
> -DEFINE_CPU(TYPE_RISCV_CPU_RV32IMACU_NOMMU,  rv32imacu_nommu_cpu_init),
>  DEFINE_CPU(TYPE_RISCV_CPU_SIFIVE_E31,   rv32imacu_nommu_cpu_init),
> -DEFINE_CPU(TYPE_RISCV_CPU_SIFIVE_U34,   rv32gcsu_priv1_10_0_cpu_init)
> +DEFINE_CPU(TYPE_RISCV_CPU_SIFIVE_U34,   
> rv32gcsu_priv1_10_0_cpu_init),
> +/* Depreacted */

"Deprecated" in patch subject and here ;)

> +DEFINE_CPU(TYPE_RISCV_CPU_RV32IMACU_NOMMU,  rv32imacu_nommu_cpu_init),
> +DEFINE_CPU(TYPE_RISCV_CPU_RV32GCSU_V1_09_1, 
> rv32gcsu_priv1_09_1_cpu_init),
> +DEFINE_CPU(TYPE_RISCV_CPU_RV32GCSU_V1_10_0, rv32gcsu_priv1_10_0_cpu_init)
>  #elif defined(TARGET_RISCV64)
>  DEFINE_CPU(TYPE_RISCV_CPU_BASE64,   riscv_base64_cpu_init),
> -DEFINE_CPU(TYPE_RISCV_CPU_RV64GCSU_V1_09_1, 
> rv64gcsu_priv1_09_1_cpu_init),
> -DEFINE_CPU(TYPE_RISCV_CPU_RV64GCSU_V1_10_0, 
> rv64gcsu_priv1_10_0_cpu_init),
> -DEFINE_CPU(TYPE_RISCV_CPU_RV64IMACU_NOMMU,  rv64imacu_nommu_cpu_init),
>  DEFINE_CPU(TYPE_RISCV_CPU_SIFIVE_E51,   rv64imacu_nommu_cpu_init),
> -DEFINE_CPU(TYPE_RISCV_CPU_SIFIVE_U54,   rv64gcsu_priv1_10_0_cpu_init)
> +DEFINE_CPU(TYPE_RISCV_CPU_SIFIVE_U54,   
> rv64gcsu_priv1_10_0_cpu_init),
> +/* Deprecated */
> +DEFINE_CPU(TYPE_RISCV_CPU_RV64IMACU_NOMMU,  rv64imacu_nommu_cpu_init),
> +DEFINE_CPU(TYPE_RISCV_CPU_RV64GCSU_V1_09_1, 
> rv64gcsu_priv1_09_1_cpu_init),
> +DEFINE_CPU(TYPE_RISCV_CPU_RV64GCSU_V1_10_0, rv64gcsu_priv1_10_0_cpu_init)
>  #endif
>  };
>  
> diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
> index b47cde5017..1668d12018 100644
> --- a/target/riscv/cpu.h
> +++ b/target/riscv/cpu.h
> @@ -35,16 +35,17 @@
>  #define TYPE_RISCV_CPU_ANY  RISCV_CPU_TYPE_NAME("any")
>  #define TYPE_RISCV_CPU_BASE32   RISCV_CPU_TYPE_NAME("rv32")
>  #define TYPE_RISCV_CPU_BASE64   RISCV_CPU_TYPE_NAME("rv64")
> -#define TYPE_RISCV_CPU_RV32GCSU_V1_09_1 
> RISCV_CPU_TYPE_NAME("rv32gcsu-v1.9.1")
> -#define TYPE_RISCV_CPU_RV32GCSU_V1_10_0 
> RISCV_CPU_TYPE_NAME("rv32gcsu-v1.10.0")
> -#define TYPE_RISCV_CPU_RV32IMACU_NOMMU  
> RISCV_CPU_TYPE_NAME("rv32imacu-nommu")
> -#define TYPE_RISCV_CPU_RV64GCSU_V1_09_1 
> RISCV_CPU_TYPE_NAME("rv64gcsu-v1.9.1")
> -#define TYPE_RISCV_CPU_RV64GCSU_V1_10_0 
> RISCV_CPU_TYPE_NAME("rv64gcsu-v1.10.0")
> -#define TYPE_RISCV_CPU_RV64IMACU_NOMMU  
> RISCV_CPU_TYPE_NAME("rv64imacu-nommu")
>  #define TYPE_RISCV_CPU_SIFIVE_E31   RISCV_CPU_TYPE_NAME("sifive-e31")
>  #define TYPE_RISCV_CPU_SIFIVE_E51   RISCV_CPU_TYPE_NAME("sifive-e51")
>  #define TYPE_RISCV_CPU_SIFIVE_U34   RISCV_CPU_TYPE_NAME("sifive-u34")
>  #define TYPE_RISCV_CPU_SIFIVE_U54   RISCV_CPU_TYPE_NAME("sifive-u54")
> +/* Deprecated */
> +#define TYPE_RISCV_CPU_RV32IMACU_NOMMU  
> RISCV_CPU_TYPE_NAME("rv32imacu-nommu")
> +#define TYPE_RISCV_CPU_RV32GCSU_V1_09_1 
> RISCV_CPU_TYPE_NAME("rv32gcsu-v1.9.1")
> +#define TYPE_RISCV_CPU_RV32GCSU_V1_10_0 
> RISCV_CPU_TYPE_NAME("rv32gcsu-v1.10.0")
> +#define TYPE_RISCV_CPU_RV64IMACU_NOMMU  
> RISCV_CPU_TYPE_NAME("rv64imacu-nommu")
> +#define TYPE_RISCV_CPU_RV64GCSU_V1_09_1 
> RISCV_CPU_TYPE_NAME("rv64gcsu-v1.9.1")
> +#define TYPE_RISCV_CPU_RV64GCSU_V1_10_0 
> RISCV_CPU_TYPE_NAME("rv64gcsu-v1.10.0")
>  
>  #define RV32 ((target_ulong)1 << (TARGET_LONG_BITS - 2))
>  #define RV64 ((target_ulong)2 << (TARGET_LONG_BITS - 2))
> 



Re: [Qemu-devel] [PATCH 2/5] i.mx7d: Add no-op/unimplemented PCIE PHY IP block

2019-06-17 Thread Philippe Mathieu-Daudé
On 4/16/19 3:38 AM, Andrey Smirnov wrote:
> Add no-op/unimplemented PCIE PHY IP block. Needed by new kernels to
> use PCIE.
> 
> Signed-off-by: Andrey Smirnov 
> Cc: Peter Maydell 
> Cc: Michael S. Tsirkin 
> Cc: qemu-devel@nongnu.org
> Cc: qemu-...@nongnu.org
> ---
>  include/hw/arm/fsl-imx7.h | 3 +++
>  hw/arm/fsl-imx7.c | 5 +
>  2 files changed, 8 insertions(+)
> 
> diff --git a/include/hw/arm/fsl-imx7.h b/include/hw/arm/fsl-imx7.h
> index aae4f860fc..3efa697adc 100644
> --- a/include/hw/arm/fsl-imx7.h
> +++ b/include/hw/arm/fsl-imx7.h
> @@ -125,6 +125,9 @@ enum FslIMX7MemoryMap {
>  FSL_IMX7_ADC2_ADDR= 0x3062,
>  FSL_IMX7_ADCn_SIZE= 0x1000,
>  
> +FSL_IMX7_PCIE_PHY_ADDR= 0x306D,
> +FSL_IMX7_PCIE_PHY_SIZE= 0x1,
> +
>  FSL_IMX7_GPC_ADDR = 0x303A,
>  
>  FSL_IMX7_I2C1_ADDR= 0x30A2,
> diff --git a/hw/arm/fsl-imx7.c b/hw/arm/fsl-imx7.c
> index 1abfa5910c..813fb55ca9 100644
> --- a/hw/arm/fsl-imx7.c
> +++ b/hw/arm/fsl-imx7.c
> @@ -532,6 +532,11 @@ static void fsl_imx7_realize(DeviceState *dev, Error 
> **errp)
>   */
>  create_unimplemented_device("dma-apbh", FSL_IMX7_DMA_APBH_ADDR,
>  FSL_IMX7_DMA_APBH_SIZE);
> +/*
> + * PCIe PHY
> + */
> +create_unimplemented_device("pcie-phy", FSL_IMX7_PCIE_PHY_ADDR,
> +FSL_IMX7_PCIE_PHY_SIZE);
>  }
>  
>  static void fsl_imx7_class_init(ObjectClass *oc, void *data)
> 

Reviewed-by: Philippe Mathieu-Daudé 



[Qemu-devel] [PATCH v2] vhost-user: fix reconnection support for host notifier

2019-06-17 Thread Tiwei Bie
We need to destroy the host notifiers when cleaning up
the backend. Otherwise, some resources are not released
after the connection is closed, and it may prevent the
external backend from reopening them (e.g. VFIO files)
during restart.

Fixes: 44866521bd6e ("vhost-user: support registering external host notifiers")
Cc: qemu-sta...@nongnu.org

Signed-off-by: Tiwei Bie 
---
v2:
- Drop superfluous memset() (Marc-André);
- Factor the notifier code in a separate function (Marc-André);

 hw/virtio/vhost-user.c | 28 ++--
 1 file changed, 18 insertions(+), 10 deletions(-)

diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c
index 4ca5b2551e..e27a2a4647 100644
--- a/hw/virtio/vhost-user.c
+++ b/hw/virtio/vhost-user.c
@@ -226,6 +226,20 @@ static bool ioeventfd_enabled(void)
 return !kvm_enabled() || kvm_eventfds_enabled();
 }
 
+static void
+vhost_user_host_notifiers_cleanup(VhostUserState *user)
+{
+int i;
+
+for (i = 0; i < VIRTIO_QUEUE_MAX; i++) {
+if (user->notifier[i].addr) {
+object_unparent(OBJECT(>notifier[i].mr));
+munmap(user->notifier[i].addr, qemu_real_host_page_size);
+user->notifier[i].addr = NULL;
+}
+}
+}
+
 static int vhost_user_read_header(struct vhost_dev *dev, VhostUserMsg *msg)
 {
 struct vhost_user *u = dev->opaque;
@@ -1469,6 +1483,9 @@ static int vhost_user_backend_cleanup(struct vhost_dev 
*dev)
 assert(dev->vhost_ops->backend_type == VHOST_BACKEND_TYPE_USER);
 
 u = dev->opaque;
+if (dev->vq_index == 0) {
+vhost_user_host_notifiers_cleanup(u->user);
+}
 if (u->postcopy_notifier.notify) {
 postcopy_remove_notifier(>postcopy_notifier);
 u->postcopy_notifier.notify = NULL;
@@ -1898,19 +1915,10 @@ bool vhost_user_init(VhostUserState *user, CharBackend 
*chr, Error **errp)
 
 void vhost_user_cleanup(VhostUserState *user)
 {
-int i;
-
 if (!user->chr) {
 return;
 }
-
-for (i = 0; i < VIRTIO_QUEUE_MAX; i++) {
-if (user->notifier[i].addr) {
-object_unparent(OBJECT(>notifier[i].mr));
-munmap(user->notifier[i].addr, qemu_real_host_page_size);
-user->notifier[i].addr = NULL;
-}
-}
+vhost_user_host_notifiers_cleanup(user);
 user->chr = NULL;
 }
 
-- 
2.17.1




Re: [Qemu-devel] [PATCH 1/5] i.mx7d: Add no-op/unimplemented APBH DMA module

2019-06-17 Thread Philippe Mathieu-Daudé
On 4/16/19 3:38 AM, Andrey Smirnov wrote:
> Instantiate no-op APBH DMA module. Needed to boot latest Linux kernel.
> 
> Signed-off-by: Andrey Smirnov 
> Cc: Peter Maydell 
> Cc: Michael S. Tsirkin 
> Cc: qemu-devel@nongnu.org
> Cc: qemu-...@nongnu.org
> ---
>  include/hw/arm/fsl-imx7.h | 3 +++
>  hw/arm/fsl-imx7.c | 6 ++
>  2 files changed, 9 insertions(+)
> 
> diff --git a/include/hw/arm/fsl-imx7.h b/include/hw/arm/fsl-imx7.h
> index d848262bfd..aae4f860fc 100644
> --- a/include/hw/arm/fsl-imx7.h
> +++ b/include/hw/arm/fsl-imx7.h
> @@ -179,6 +179,9 @@ enum FslIMX7MemoryMap {
>  FSL_IMX7_PCIE_REG_SIZE= 16 * 1024,
>  
>  FSL_IMX7_GPR_ADDR = 0x3034,
> +
> +FSL_IMX7_DMA_APBH_ADDR= 0x3300,
> +FSL_IMX7_DMA_APBH_SIZE= 0x2000,

0x8000?

>  };
>  
>  enum FslIMX7IRQs {
> diff --git a/hw/arm/fsl-imx7.c b/hw/arm/fsl-imx7.c
> index 7663ad6861..1abfa5910c 100644
> --- a/hw/arm/fsl-imx7.c
> +++ b/hw/arm/fsl-imx7.c
> @@ -526,6 +526,12 @@ static void fsl_imx7_realize(DeviceState *dev, Error 
> **errp)
>   */
>  create_unimplemented_device("lcdif", FSL_IMX7_LCDIF_ADDR,
>  FSL_IMX7_LCDIF_SIZE);
> +
> +/*
> + * DMA APBH
> + */
> +create_unimplemented_device("dma-apbh", FSL_IMX7_DMA_APBH_ADDR,
> +FSL_IMX7_DMA_APBH_SIZE);
>  }
>  
>  static void fsl_imx7_class_init(ObjectClass *oc, void *data)
> 



Re: [Qemu-devel] [RFC PATCH v2 01/35] multi-process: memory: alloc RAM from file at offset

2019-06-17 Thread Gerd Hoffmann
On Mon, Jun 17, 2019 at 11:14:59AM -0700, elena.ufimts...@oracle.com wrote:
> From: Jagannathan Raman 
> 
> Allow RAM MemoryRegion to be created from an offset in a file, instead
> of allocating at offset of 0 by default. This is needed to synchronize
> RAM between QEMU & remote process.
> This will be needed for the following patches.

Details please.   vhost-user works fine without this ...

cheers,
  Gerd




Re: [Qemu-devel] [PATCH 5/6] hw/timer/armv7m_systick: Forbid non-privileged accesses

2019-06-17 Thread Philippe Mathieu-Daudé
On 6/17/19 7:53 PM, Peter Maydell wrote:
> Like most of the v7M memory mapped system registers, the systick
> registers are accessible to privileged code only and user accesses
> must generate a BusFault. We implement that for registers in
> the NVIC proper already, but missed it for systick since we
> implement it as a separate device. Correct the omission.
> 
> Signed-off-by: Peter Maydell 
> ---
>  hw/timer/armv7m_systick.c | 26 --
>  1 file changed, 20 insertions(+), 6 deletions(-)
> 
> diff --git a/hw/timer/armv7m_systick.c b/hw/timer/armv7m_systick.c
> index a17317ce2fe..94640743b5d 100644
> --- a/hw/timer/armv7m_systick.c
> +++ b/hw/timer/armv7m_systick.c
> @@ -75,11 +75,17 @@ static void systick_timer_tick(void *opaque)
>  }
>  }
>  
> -static uint64_t systick_read(void *opaque, hwaddr addr, unsigned size)
> +static MemTxResult systick_read(void *opaque, hwaddr addr, uint64_t *data,
> +unsigned size, MemTxAttrs attrs)
>  {
>  SysTickState *s = opaque;
>  uint32_t val;
>  
> +if (attrs.user) {
> +/* Generate BusFault for unprivileged accesses */
> +return MEMTX_ERROR;
> +}
> +
>  switch (addr) {
>  case 0x0: /* SysTick Control and Status.  */
>  val = s->control;
> @@ -121,14 +127,21 @@ static uint64_t systick_read(void *opaque, hwaddr addr, 
> unsigned size)
>  }
>  
>  trace_systick_read(addr, val, size);
> -return val;
> +*data = val;
> +return MEMTX_OK;
>  }
>  
> -static void systick_write(void *opaque, hwaddr addr,
> -  uint64_t value, unsigned size)
> +static MemTxResult systick_write(void *opaque, hwaddr addr,
> + uint64_t value, unsigned size,
> + MemTxAttrs attrs)
>  {
>  SysTickState *s = opaque;
>  
> +if (attrs.user) {
> +/* Generate BusFault for unprivileged accesses */
> +return MEMTX_ERROR;
> +}
> +
>  trace_systick_write(addr, value, size);
>  
>  switch (addr) {
> @@ -172,11 +185,12 @@ static void systick_write(void *opaque, hwaddr addr,
>  qemu_log_mask(LOG_GUEST_ERROR,
>"SysTick: Bad write offset 0x%" HWADDR_PRIx "\n", 
> addr);
>  }
> +return MEMTX_OK;
>  }
>  
>  static const MemoryRegionOps systick_ops = {
> -.read = systick_read,
> -.write = systick_write,
> +.read_with_attrs = systick_read,
> +.write_with_attrs = systick_write,
>  .endianness = DEVICE_NATIVE_ENDIAN,
>  .valid.min_access_size = 4,
>  .valid.max_access_size = 4,
> 

Reviewed-by: Philippe Mathieu-Daudé 



Re: [Qemu-devel] [PATCH v4 0/7] tcg/ppc: Add vector opcodes

2019-06-17 Thread Richard Henderson
Ping.  Otherwise I'll include it in my next tcg pull.


r~

On 5/18/19 9:15 PM, Richard Henderson wrote:
> Based-on: <20190518190157.21255-1-richard.hender...@linaro.org>
> Aka "tcg: misc gvec improvements".
> 
> Version 3 was last posted in March,
> https://lists.gnu.org/archive/html/qemu-devel/2019-03/msg05859.html
> 
> Changes since v3:
>   * Add support for bitsel, with the vsx xxsel insn.
>   * Rely on the new relocation overflow handling, so
> we don't require 3 insns for a vector load.
> 
> Changes since v2:
>   * Several generic tcg patches to improve dup vs dupi vs dupm.
> In particular, if a global temp (like guest r10) is not in
> a host register, we should duplicate from memory instead of
> loading to an integer register, spilling to stack, loading
> to a vector register, and then duplicating.
>   * I have more confidence that 32-bit ppc host should work
> this time around.  No testing on that front yet, but I've
> unified some code sequences with 64-bit ppc host.
>   * Base altivec now supports V128 only.  Moved V64 support to
> Power7 (v2.06), which has 64-bit load/store.
>   * Dropped support for 64-bit vector multiply using Power8.
> The expansion was too large compared to using integer regs.
> 
> 
> r~
> 
> 
> Richard Henderson (7):
>   tcg/ppc: Initial backend support for Altivec
>   tcg/ppc: Support vector shift by immediate
>   tcg/ppc: Support vector multiply
>   tcg/ppc: Support vector dup2
>   tcg/ppc: Update vector support to v2.06
>   tcg/ppc: Update vector support to v2.07
>   tcg/ppc: Update vector support to v3.00
> 
>  tcg/ppc/tcg-target.h |   39 +-
>  tcg/ppc/tcg-target.opc.h |   11 +
>  tcg/ppc/tcg-target.inc.c | 1077 +++---
>  3 files changed, 1063 insertions(+), 64 deletions(-)
>  create mode 100644 tcg/ppc/tcg-target.opc.h
> 




[Qemu-devel] [Bug 639651] Re: DRIVER_IRQL_NOT_LESS_OR_EQUAL booting WIndows XP with Synaptics driver installed

2019-06-17 Thread Launchpad Bug Tracker
[Expired for QEMU because there has been no activity for 60 days.]

** Changed in: qemu
   Status: Incomplete => Expired

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

Title:
  DRIVER_IRQL_NOT_LESS_OR_EQUAL booting WIndows XP with Synaptics driver
  installed

Status in QEMU:
  Expired
Status in qemu package in Debian:
  Incomplete

Bug description:
  Positng the issue here since I did not get any reply on the ML.

  I was trying to update some windows XP (SP3) images in kvm.

  It worked fine several times but last time I added mass storage
  drivers to sysprep and now on the second boot after reseal (the first
  is mini-setup) I get a BSOD with message
  DRIVER_IRQL_NOT_LESS_OR_EQUAL. 

  It turns out that the error is unrelated to storage drivers. It is
  triggered by Synaptics driver installing for the PS2 mouse in kvm
  (which does not happen in VirtualBox or on real hardware).

  The image is originally created on hardware with MP acpi (not
  virtualization).

  qemu-kvm  0.12.5+dfsg-2

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



Re: [Qemu-devel] [PATCH 0/3] Some qemu-bridge-helper work

2019-06-17 Thread Jason Wang



On 2019/6/4 下午7:52, Markus Armbruster wrote:

Markus Armbruster (3):
   MAINTAINERS: Add qemu-bridge-helper.c to "Network device backends"
   net: Deprecate tap backend's parameter "helper"
   qemu-bridge-helper: Document known shortcomings

  MAINTAINERS  |  1 +
  qapi/net.json|  3 ++-
  qemu-bridge-helper.c | 12 +++-
  qemu-deprecated.texi |  4 
  qemu-options.hx  | 18 ++
  5 files changed, 20 insertions(+), 18 deletions(-)



I've queued patch 1 and 3. For patch 2, it still require more thought 
since tap is not tied to bridge in fact, it could be used independently.


Thanks




Re: [Qemu-devel] [PATCH 2/3] net: Deprecate tap backend's parameter "helper"

2019-06-17 Thread Jason Wang



On 2019/6/4 下午7:52, Markus Armbruster wrote:

-netdev tap,helper=... is a useless duplicate of -netdev bridge.
Deprecate and de-document.

Signed-off-by: Markus Armbruster 



This requires more thought as TAP could be used independently. Force 
using a "bridge" backend may lead some confusion.


Thanks



---
  qapi/net.json|  3 ++-
  qemu-deprecated.texi |  4 
  qemu-options.hx  | 18 ++
  3 files changed, 8 insertions(+), 17 deletions(-)

diff --git a/qapi/net.json b/qapi/net.json
index 5f7bff1637..59d79a1ae1 100644
--- a/qapi/net.json
+++ b/qapi/net.json
@@ -221,7 +221,8 @@
  #
  # @br: bridge name (since 2.8)
  #
-# @helper: command to execute to configure bridge
+# @helper: command to execute to configure bridge (deprecated, use
+# type 'bridge' instead)
  #
  # @sndbuf: send buffer limit. Understands [TGMKkb] suffixes.
  #
diff --git a/qemu-deprecated.texi b/qemu-deprecated.texi
index 50292d820b..52e7600ebc 100644
--- a/qemu-deprecated.texi
+++ b/qemu-deprecated.texi
@@ -48,6 +48,10 @@ for these file types is 'host_cdrom' or 'host_device' as 
appropriate.
  The @option{name} parameter of the @option{-net} option is a synonym
  for the @option{id} parameter, which should now be used instead.
  
+@subsection -netdev tap,helper=... (since 4.1)

+
+Use -netdev bridge instead.
+
  @subsection -smp (invalid topologies) (since 3.1)
  
  CPU topology properties should describe whole machine topology including

diff --git a/qemu-options.hx b/qemu-options.hx
index 39dc170429..3324203b51 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -2121,7 +2121,7 @@ DEF("netdev", HAS_ARG, QEMU_OPTION_netdev,
  "configure a host TAP network backend with ID 'str'\n"
  #else
  "-netdev 
tap,id=str[,fd=h][,fds=x:y:...:z][,ifname=name][,script=file][,downscript=dfile]\n"
-" 
[,br=bridge][,helper=helper][,sndbuf=nbytes][,vnet_hdr=on|off][,vhost=on|off]\n"
+" [,br=bridge][,sndbuf=nbytes][,vnet_hdr=on|off][,vhost=on|off]\n"
  " 
[,vhostfd=h][,vhostfds=x:y:...:z][,vhostforce=on|off][,queues=n]\n"
  " [,poll-us=n]\n"
  "configure a host TAP network backend with ID 'str'\n"
@@ -2130,8 +2130,6 @@ DEF("netdev", HAS_ARG, QEMU_OPTION_netdev,
  "to configure it and 'dfile' (default=" DEFAULT_NETWORK_DOWN_SCRIPT 
")\n"
  "to deconfigure it\n"
  "use '[down]script=no' to disable script execution\n"
-"use network helper 'helper' (default=" DEFAULT_BRIDGE_HELPER 
") to\n"
-"configure it\n"
  "use 'fd=h' to connect to an already opened TAP 
interface\n"
  "use 'fds=x:y:...:z' to connect to already opened multiqueue 
capable TAP interfaces\n"
  "use 'sndbuf=nbytes' to limit the size of the send buffer 
(the\n"
@@ -2435,7 +2433,7 @@ qemu-system-i386 -nic  
'user,id=n1,guestfwd=tcp:10.0.2.100:1234-cmd:netcat 10.10
  
  @end table
  
-@item -netdev tap,id=@var{id}[,fd=@var{h}][,ifname=@var{name}][,script=@var{file}][,downscript=@var{dfile}][,br=@var{bridge}][,helper=@var{helper}]

+@item -netdev 
tap,id=@var{id}[,fd=@var{h}][,ifname=@var{name}][,script=@var{file}][,downscript=@var{dfile}][,br=@var{bridge}]
  Configure a host TAP network backend with ID @var{id}.
  
  Use the network script @var{file} to configure it and the network script

@@ -2445,11 +2443,6 @@ automatically provides one. The default network 
configure script is
  @file{/etc/qemu-ifdown}. Use @option{script=no} or @option{downscript=no}
  to disable script execution.
  
-If running QEMU as an unprivileged user, use the network helper

-@var{helper} to configure the TAP interface and attach it to the bridge.
-The default network helper executable is @file{/path/to/qemu-bridge-helper}
-and the default bridge device is @file{br0}.
-
  @option{fd}=@var{h} can be used to specify the handle of an already
  opened host TAP interface.
  
@@ -2468,13 +2461,6 @@ qemu-system-i386 linux.img \

  -netdev tap,id=nd1,ifname=tap1 -device rtl8139,netdev=nd1
  @end example
  
-@example

-#launch a QEMU instance with the default network helper to
-#connect a TAP device to bridge br0
-qemu-system-i386 linux.img -device virtio-net-pci,netdev=n1 \
--netdev tap,id=n1,"helper=/path/to/qemu-bridge-helper"
-@end example
-
  @item -netdev bridge,id=@var{id}[,br=@var{bridge}][,helper=@var{helper}]
  Connect a host TAP network interface to a host bridge device.
  




Re: [Qemu-devel] [PATCH v4 5/5] net/announce: Expand test for stopping self announce

2019-06-17 Thread Jason Wang



On 2019/6/13 下午5:59, Dr. David Alan Gilbert (git) wrote:

From: "Dr. David Alan Gilbert" 

Expand self-announce test to check we can stop an announce timer.
We set it up to send 300 packets, but after we receive
the first one we tell it to stop.

We error if:
a) We receive more than 30 of the packets
b) We're still receiving packets after a lot longer than the
   30 seconds should have arrived

Signed-off-by: Dr. David Alan Gilbert 
---
  tests/virtio-net-test.c | 57 ++---
  1 file changed, 54 insertions(+), 3 deletions(-)

diff --git a/tests/virtio-net-test.c b/tests/virtio-net-test.c
index 663cf7ea7e..3b49b431dc 100644
--- a/tests/virtio-net-test.c
+++ b/tests/virtio-net-test.c
@@ -184,21 +184,72 @@ static void announce_self(void *obj, void *data, 
QGuestAllocator *t_alloc)
  QDict *rsp;
  int ret;
  uint16_t *proto = (uint16_t *)[12];
+size_t total_received = 0;
+uint64_t start, now, last_rxt, deadline;
  
+/* Send a set of packets over a few second period */

  rsp = qmp("{ 'execute' : 'announce-self', "
" 'arguments': {"
-  " 'initial': 50, 'max': 550,"
-  " 'rounds': 10, 'step': 50 } }");
+  " 'initial': 20, 'max': 100,"
+  " 'rounds': 300, 'step': 10, 'id': 'bob' } }");
  assert(!qdict_haskey(rsp, "error"));
  qobject_unref(rsp);
  
-/* Catch the packet and make sure it's a RARP */

+/* Catch the first packet and make sure it's a RARP */
  ret = qemu_recv(sv[0], , sizeof(len), 0);
  g_assert_cmpint(ret, ==,  sizeof(len));
  len = ntohl(len);
  
  ret = qemu_recv(sv[0], buffer, len, 0);

  g_assert_cmpint(*proto, ==, htons(ETH_P_RARP));
+
+/*
+ * Stop the announcment by settings rounds to 0 on the
+ * existing timer.
+ */
+rsp = qmp("{ 'execute' : 'announce-self', "
+  " 'arguments': {"
+  " 'initial': 20, 'max': 100,"
+  " 'rounds': 0, 'step': 10, 'id': 'bob' } }");
+assert(!qdict_haskey(rsp, "error"));
+qobject_unref(rsp);
+
+/* Now make sure the packets stop */
+
+/* Times are in us */
+start = g_get_monotonic_time();
+/* 30 packets, max gap 100ms, * 2 for wiggle */
+deadline = start + 1000 * (100 * 30 * 2);
+last_rxt = start;
+
+do {



while (ture) looks better here.



+int saved_err;
+ret = qemu_recv(sv[0], buffer, 60, MSG_DONTWAIT);
+saved_err = errno;
+now = g_get_monotonic_time();
+g_assert_cmpint(now, <, deadline);


The maximum gap allowed is 1000 * 100 * 4, and we allow at most 30 
packets that's 30 * 1000 * 100 * 4 which is 120.


But the deadline is 1000 * 100 * 30 * 2 which is 600.

Does this mean deadline is conflict with the assumption above?

Thanks



+
+if (ret >= 0) {
+if (ret) {
+last_rxt = now;
+}
+total_received += ret;
+
+/* Check it's not spewing loads */
+g_assert_cmpint(total_received, <, 60 * 30 * 2);
+} else {
+g_assert_cmpint(saved_err, ==, EAGAIN);
+
+/* 400ms, i.e. 4 worst case gaps */
+if ((now - last_rxt) > (1000 * 100 * 4)) {
+/* Nothings arrived for a while - must have stopped */
+break;
+};
+
+/* 100ms */
+g_usleep(1000 * 100);
+}
+} while (true);
  }
  
  static void virtio_net_test_cleanup(void *sockets)




Re: [Qemu-devel] [PATCH v4 3/5] net/announce: Add optional ID

2019-06-17 Thread Jason Wang



On 2019/6/13 下午5:59, Dr. David Alan Gilbert (git) wrote:

From: "Dr. David Alan Gilbert" 

Previously there was a single instance of the timer used by
monitor triggered announces, that's OK, but when combined with the
previous change that lets you have announces for subsets of interfaces
it's a bit restrictive if you want to do different things to different
interfaces.

Add an 'id' field to the announce, and maintain a list of the
timers based on id.

This allows you to for example:
 a) Start an announce going on interface eth0 for a long time
 b) Start an announce going on interface eth1 for a long time
 c) Kill the announce on eth0 while leaving eth1 going.

Signed-off-by: Dr. David Alan Gilbert 
---
  hw/net/virtio-net.c|  4 ++--
  include/net/announce.h |  8 ++--
  net/announce.c | 46 +++---
  net/trace-events   |  3 ++-
  qapi/net.json  |  9 +++--
  5 files changed, 56 insertions(+), 14 deletions(-)

diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
index c3f5fccfd1..b9e1cd71cf 100644
--- a/hw/net/virtio-net.c
+++ b/hw/net/virtio-net.c
@@ -2360,7 +2360,7 @@ static int virtio_net_post_load_device(void *opaque, int 
version_id)
  timer_mod(n->announce_timer.tm,
qemu_clock_get_ms(n->announce_timer.type));
  } else {
-qemu_announce_timer_del(>announce_timer);
+qemu_announce_timer_del(>announce_timer, false);
  }
  }
  
@@ -2784,7 +2784,7 @@ static void virtio_net_device_unrealize(DeviceState *dev, Error **errp)

  virtio_net_del_queue(n, i);
  }
  
-qemu_announce_timer_del(>announce_timer);

+qemu_announce_timer_del(>announce_timer, false);
  g_free(n->vqs);
  qemu_del_nic(n->nic);
  virtio_net_rsc_cleanup(n);
diff --git a/include/net/announce.h b/include/net/announce.h
index 773470428b..3d90c83c23 100644
--- a/include/net/announce.h
+++ b/include/net/announce.h
@@ -22,8 +22,12 @@ struct AnnounceTimer {
  /* Returns: update the timer to the next time point */
  int64_t qemu_announce_timer_step(AnnounceTimer *timer);
  
-/* Delete the underlying timer and other data */

-void qemu_announce_timer_del(AnnounceTimer *timer);
+/*
+ * Delete the underlying timer and other data
+ * If 'free_named' true and the timer is a named timer, then remove
+ * it from the list of named timers and free the AnnounceTimer itself.
+ */
+void qemu_announce_timer_del(AnnounceTimer *timer, bool free_named);
  
  /*

   * Under BQL/main thread
diff --git a/net/announce.c b/net/announce.c
index 1ce42b571d..4d4e2c22a1 100644
--- a/net/announce.c
+++ b/net/announce.c
@@ -15,6 +15,8 @@
  #include "qapi/qapi-commands-net.h"
  #include "trace.h"
  
+static GData *named_timers;

+
  int64_t qemu_announce_timer_step(AnnounceTimer *timer)
  {
  int64_t step;
@@ -31,8 +33,13 @@ int64_t qemu_announce_timer_step(AnnounceTimer *timer)
  return step;
  }
  
-void qemu_announce_timer_del(AnnounceTimer *timer)

+/*
+ * If 'free_named' is true, then remove the timer from the list
+ * and free the timer itself.
+ */
+void qemu_announce_timer_del(AnnounceTimer *timer, bool free_named)
  {
+bool free_timer = false;
  if (timer->tm) {
  timer_del(timer->tm);
  timer_free(timer->tm);
@@ -40,6 +47,18 @@ void qemu_announce_timer_del(AnnounceTimer *timer)
  }
  qapi_free_strList(timer->params.interfaces);
  timer->params.interfaces = NULL;
+if (free_named && timer->params.has_id) {
+free_timer = timer ==
+ g_datalist_get_data(_timers, timer->params.id);



Any chance that the timer get from datalist is different from the one 
that caller passed to us?


Thanks



+g_datalist_remove_data(_timers, timer->params.id);
+}
+trace_qemu_announce_timer_del(free_named, free_timer, timer->params.id);
+g_free(timer->params.id);
+timer->params.id = NULL;
+
+if (free_timer) {
+g_free(timer);
+}
  }
  
  /*

@@ -56,7 +75,7 @@ void qemu_announce_timer_reset(AnnounceTimer *timer,
   * We're under the BQL, so the current timer can't
   * be firing, so we should be able to delete it.
   */
-qemu_announce_timer_del(timer);
+qemu_announce_timer_del(timer, false);
  
  QAPI_CLONE_MEMBERS(AnnounceParameters, >params, params);

  timer->round = params->rounds;
@@ -120,7 +139,8 @@ static void qemu_announce_self_iter(NICState *nic, void 
*opaque)
  skip = false;
  }
  
-trace_qemu_announce_self_iter(nic->ncs->name,

+trace_qemu_announce_self_iter(timer->params.has_id ? timer->params.id : 
"_",
+  nic->ncs->name,
qemu_ether_ntoa(>conf->macaddr), skip);
  
  if (!skip) {

@@ -143,7 +163,7 @@ static void qemu_announce_self_once(void *opaque)
  if (--timer->round) {
  qemu_announce_timer_step(timer);
  } else {
-

Re: [Qemu-devel] [PATCH v4 1/5] net/announce: Allow optional list of interfaces

2019-06-17 Thread Jason Wang



On 2019/6/13 下午5:59, Dr. David Alan Gilbert (git) wrote:

From: "Dr. David Alan Gilbert" 

Allow the caller to restrict the set of interfaces that announces are
sent on.  The default is still to send on all interfaces.

e.g.

   { "execute": "announce-self", "arguments": { "initial": 50, "max": 550, "rounds": 5, "step": 50, 
"interfaces": ["vn2", "vn1"] } }

This doesn't affect the behaviour of migraiton announcments.

Note: There's still only one timer for the qmp command, so that
performing an 'announce-self' on one list of interfaces followed
by another 'announce-self' on another list will stop the announces
on the existing set.

Signed-off-by: Dr. David Alan Gilbert 
---
  include/net/announce.h |  2 +-
  net/announce.c | 39 ---
  net/trace-events   |  2 +-
  qapi/net.json  | 11 ---
  4 files changed, 42 insertions(+), 12 deletions(-)

diff --git a/include/net/announce.h b/include/net/announce.h
index 04a035f679..773470428b 100644
--- a/include/net/announce.h
+++ b/include/net/announce.h
@@ -22,7 +22,7 @@ struct AnnounceTimer {
  /* Returns: update the timer to the next time point */
  int64_t qemu_announce_timer_step(AnnounceTimer *timer);
  
-/* Delete the underlying timer */

+/* Delete the underlying timer and other data */
  void qemu_announce_timer_del(AnnounceTimer *timer);
  
  /*

diff --git a/net/announce.c b/net/announce.c
index 91e9a6e267..1ce42b571d 100644
--- a/net/announce.c
+++ b/net/announce.c
@@ -38,6 +38,8 @@ void qemu_announce_timer_del(AnnounceTimer *timer)
  timer_free(timer->tm);
  timer->tm = NULL;
  }
+qapi_free_strList(timer->params.interfaces);
+timer->params.interfaces = NULL;
  }
  
  /*

@@ -96,24 +98,47 @@ static int announce_self_create(uint8_t *buf,
  
  static void qemu_announce_self_iter(NICState *nic, void *opaque)

  {
+AnnounceTimer *timer = opaque;
  uint8_t buf[60];
  int len;
+bool skip;
+
+if (timer->params.has_interfaces) {
+strList *entry = timer->params.interfaces;
+/* Skip unless we find our name in the requested list */
+skip = true;
+
+while (entry) {
+if (!strcmp(entry->value, nic->ncs->name)) {
+/* Found us */
+skip = false;
+break;
+}
+entry = entry->next;
+}
+} else {
+skip = false;
+}



I wonder whether or not it's better to filter the name on the caller.

Thanks



+
+trace_qemu_announce_self_iter(nic->ncs->name,
+  qemu_ether_ntoa(>conf->macaddr), skip);
  
-trace_qemu_announce_self_iter(qemu_ether_ntoa(>conf->macaddr));

-len = announce_self_create(buf, nic->conf->macaddr.a);
+if (!skip) {
+len = announce_self_create(buf, nic->conf->macaddr.a);
  
-qemu_send_packet_raw(qemu_get_queue(nic), buf, len);

+qemu_send_packet_raw(qemu_get_queue(nic), buf, len);
  
-/* if the NIC provides it's own announcement support, use it as well */

-if (nic->ncs->info->announce) {
-nic->ncs->info->announce(nic->ncs);
+/* if the NIC provides it's own announcement support, use it as well */
+if (nic->ncs->info->announce) {
+nic->ncs->info->announce(nic->ncs);
+}
  }
  }
  static void qemu_announce_self_once(void *opaque)
  {
  AnnounceTimer *timer = (AnnounceTimer *)opaque;
  
-qemu_foreach_nic(qemu_announce_self_iter, NULL);

+qemu_foreach_nic(qemu_announce_self_iter, timer);
  
  if (--timer->round) {

  qemu_announce_timer_step(timer);
diff --git a/net/trace-events b/net/trace-events
index a7937f3f3a..875ef2a0f3 100644
--- a/net/trace-events
+++ b/net/trace-events
@@ -1,7 +1,7 @@
  # See docs/devel/tracing.txt for syntax documentation.
  
  # announce.c

-qemu_announce_self_iter(const char *mac) "%s"
+qemu_announce_self_iter(const char *name, const char *mac, int skip) "%s:%s skip: 
%d"
  
  # vhost-user.c

  vhost_user_event(const char *chr, int event) "chr: %s got event: %d"
diff --git a/qapi/net.json b/qapi/net.json
index 5f7bff1637..6f2cd4f530 100644
--- a/qapi/net.json
+++ b/qapi/net.json
@@ -699,6 +699,9 @@
  #
  # @step: Delay increase (in ms) after each self-announcement attempt
  #
+# @interfaces: An optional list of interface names, which restricts the
+#announcement to the listed interfaces. (Since 4.1)
+#
  # Since: 4.0
  ##
  
@@ -706,7 +709,8 @@

'data': { 'initial': 'int',
  'max': 'int',
  'rounds': 'int',
-'step': 'int' } }
+'step': 'int',
+'*interfaces': ['str'] } }
  
  ##

  # @announce-self:
@@ -718,9 +722,10 @@
  #
  # Example:
  #
-# -> { "execute": "announce-self"
+# -> { "execute": "announce-self",
  #  "arguments": {
-#  "initial": 50, "max": 550, "rounds": 10, "step": 50 } }
+#  "initial": 50, "max": 550, "rounds": 10, "step": 50,
+#  

Re: [Qemu-devel] [PATCH v4 0/5] network announce; interface selection & IDs

2019-06-17 Thread Jason Wang



On 2019/6/13 下午5:59, Dr. David Alan Gilbert (git) wrote:

From: "Dr. David Alan Gilbert" 

Laine asked for some extra features on the network announce support;



It's better to explain why this feature is needed.  Is this because 
libvirt can change the host network topology on the fly?


Thanks




The first allows the announce timer to announce on a subset of the
interfaces.

The second allows there to be multiple timers, each with their own
parameters (including the interface list).

Signed-off-by: Dr. David Alan Gilbert 

v4
   Minor typo fixes
   Expanded the test to check we can stop a running announce

Dr. David Alan Gilbert (5):
   net/announce: Allow optional list of interfaces
   net/announce: Add HMP optional interface list
   net/announce: Add optional ID
   net/announce: Add HMP optional ID
   net/announce: Expand test for stopping self announce

  hmp-commands.hx |  7 +++-
  hmp.c   | 41 +++-
  hw/net/virtio-net.c |  4 +-
  include/net/announce.h  |  8 +++-
  net/announce.c  | 83 ++---
  net/trace-events|  3 +-
  qapi/net.json   | 16 ++--
  tests/virtio-net-test.c | 57 ++--
  8 files changed, 192 insertions(+), 27 deletions(-)





[Qemu-devel] [PATCH v1 7/9] target/riscv: Remove user version information

2019-06-17 Thread Alistair Francis
Remove the user version information. This was never used and never
publically exposed in a release of QEMU, so let's just remove it. In
future to manage versions we can extend the extension properties to
specify version.

Signed-off-by: Alistair Francis 
---
 target/riscv/cpu.c | 32 +---
 target/riscv/cpu.h |  2 --
 2 files changed, 9 insertions(+), 25 deletions(-)

diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index 21bfaa9722..ddbe922958 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -89,9 +89,8 @@ static void set_misa(CPURISCVState *env, target_ulong misa)
 env->misa_mask = env->misa = misa;
 }
 
-static void set_versions(CPURISCVState *env, int user_ver, int priv_ver)
+static void set_priv_version(CPURISCVState *env, int priv_ver)
 {
-env->user_ver = user_ver;
 env->priv_ver = priv_ver;
 }
 
@@ -111,7 +110,7 @@ static void riscv_any_cpu_init(Object *obj)
 {
 CPURISCVState *env = _CPU(obj)->env;
 set_misa(env, RVXLEN | RVI | RVM | RVA | RVF | RVD | RVC | RVU);
-set_versions(env, USER_VERSION_2_02_0, PRIV_VERSION_1_11_0);
+set_priv_version(env, PRIV_VERSION_1_11_0);
 set_resetvec(env, DEFAULT_RSTVEC);
 }
 
@@ -128,7 +127,7 @@ static void rv32gcsu_priv1_09_1_cpu_init(Object *obj)
 {
 CPURISCVState *env = _CPU(obj)->env;
 set_misa(env, RV32 | RVI | RVM | RVA | RVF | RVD | RVC | RVS | RVU);
-set_versions(env, USER_VERSION_2_02_0, PRIV_VERSION_1_09_1);
+set_priv_version(env, PRIV_VERSION_1_09_1);
 set_resetvec(env, DEFAULT_RSTVEC);
 set_feature(env, RISCV_FEATURE_MMU);
 set_feature(env, RISCV_FEATURE_PMP);
@@ -138,7 +137,7 @@ static void rv32gcsu_priv1_10_0_cpu_init(Object *obj)
 {
 CPURISCVState *env = _CPU(obj)->env;
 set_misa(env, RV32 | RVI | RVM | RVA | RVF | RVD | RVC | RVS | RVU);
-set_versions(env, USER_VERSION_2_02_0, PRIV_VERSION_1_10_0);
+set_priv_version(env, PRIV_VERSION_1_10_0);
 set_resetvec(env, DEFAULT_RSTVEC);
 set_feature(env, RISCV_FEATURE_MMU);
 set_feature(env, RISCV_FEATURE_PMP);
@@ -148,7 +147,7 @@ static void rv32imacu_nommu_cpu_init(Object *obj)
 {
 CPURISCVState *env = _CPU(obj)->env;
 set_misa(env, RV32 | RVI | RVM | RVA | RVC | RVU);
-set_versions(env, USER_VERSION_2_02_0, PRIV_VERSION_1_10_0);
+set_priv_version(env, PRIV_VERSION_1_10_0);
 set_resetvec(env, DEFAULT_RSTVEC);
 set_feature(env, RISCV_FEATURE_PMP);
 }
@@ -166,7 +165,7 @@ static void rv64gcsu_priv1_09_1_cpu_init(Object *obj)
 {
 CPURISCVState *env = _CPU(obj)->env;
 set_misa(env, RV64 | RVI | RVM | RVA | RVF | RVD | RVC | RVS | RVU);
-set_versions(env, USER_VERSION_2_02_0, PRIV_VERSION_1_09_1);
+set_priv_version(env, PRIV_VERSION_1_09_1);
 set_resetvec(env, DEFAULT_RSTVEC);
 set_feature(env, RISCV_FEATURE_MMU);
 set_feature(env, RISCV_FEATURE_PMP);
@@ -176,7 +175,7 @@ static void rv64gcsu_priv1_10_0_cpu_init(Object *obj)
 {
 CPURISCVState *env = _CPU(obj)->env;
 set_misa(env, RV64 | RVI | RVM | RVA | RVF | RVD | RVC | RVS | RVU);
-set_versions(env, USER_VERSION_2_02_0, PRIV_VERSION_1_10_0);
+set_priv_version(env, PRIV_VERSION_1_10_0);
 set_resetvec(env, DEFAULT_RSTVEC);
 set_feature(env, RISCV_FEATURE_MMU);
 set_feature(env, RISCV_FEATURE_PMP);
@@ -186,7 +185,7 @@ static void rv64imacu_nommu_cpu_init(Object *obj)
 {
 CPURISCVState *env = _CPU(obj)->env;
 set_misa(env, RV64 | RVI | RVM | RVA | RVC | RVU);
-set_versions(env, USER_VERSION_2_02_0, PRIV_VERSION_1_10_0);
+set_priv_version(env, PRIV_VERSION_1_10_0);
 set_resetvec(env, DEFAULT_RSTVEC);
 set_feature(env, RISCV_FEATURE_PMP);
 }
@@ -317,7 +316,6 @@ static void riscv_cpu_realize(DeviceState *dev, Error 
**errp)
 CPURISCVState *env = >env;
 RISCVCPUClass *mcc = RISCV_CPU_GET_CLASS(dev);
 int priv_version = PRIV_VERSION_1_11_0;
-int user_version = USER_VERSION_2_02_0;
 target_ulong target_misa = 0;
 Error *local_err = NULL;
 
@@ -342,18 +340,7 @@ static void riscv_cpu_realize(DeviceState *dev, Error 
**errp)
 }
 }
 
-if (cpu->cfg.user_spec) {
-if (!g_strcmp0(cpu->cfg.user_spec, "v2.02.0")) {
-user_version = USER_VERSION_2_02_0;
-} else {
-error_setg(errp,
-   "Unsupported user spec version '%s'",
-   cpu->cfg.user_spec);
-return;
-}
-}
-
-set_versions(env, user_version, priv_version);
+set_priv_version(env, priv_version);
 set_resetvec(env, DEFAULT_RSTVEC);
 
 if (cpu->cfg.mmu) {
@@ -454,7 +441,6 @@ static Property riscv_cpu_properties[] = {
 DEFINE_PROP_BOOL("s", RISCVCPU, cfg.ext_s, true),
 DEFINE_PROP_BOOL("u", RISCVCPU, cfg.ext_u, true),
 DEFINE_PROP_STRING("priv_spec", RISCVCPU, cfg.priv_spec),
-DEFINE_PROP_STRING("user_spec", RISCVCPU, cfg.user_spec),
 DEFINE_PROP_BOOL("mmu", RISCVCPU, cfg.mmu, true),
 DEFINE_PROP_BOOL("pmp", RISCVCPU, 

[Qemu-devel] [PATCH v1 8/9] target/riscv: Add support for disabling/enabling Counters

2019-06-17 Thread Alistair Francis
Add support for disabling/enabling the "Counters" extension.

Signed-off-by: Alistair Francis 
---
 target/riscv/cpu.c | 1 +
 target/riscv/cpu.h | 1 +
 target/riscv/csr.c | 7 +++
 3 files changed, 9 insertions(+)

diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index ddbe922958..5af1c9b38c 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -440,6 +440,7 @@ static Property riscv_cpu_properties[] = {
 DEFINE_PROP_BOOL("c", RISCVCPU, cfg.ext_c, true),
 DEFINE_PROP_BOOL("s", RISCVCPU, cfg.ext_s, true),
 DEFINE_PROP_BOOL("u", RISCVCPU, cfg.ext_u, true),
+DEFINE_PROP_BOOL("Counters", RISCVCPU, cfg.ext_counters, true),
 DEFINE_PROP_STRING("priv_spec", RISCVCPU, cfg.priv_spec),
 DEFINE_PROP_BOOL("mmu", RISCVCPU, cfg.mmu, true),
 DEFINE_PROP_BOOL("pmp", RISCVCPU, cfg.pmp, true),
diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
index a558c353f0..786f620564 100644
--- a/target/riscv/cpu.h
+++ b/target/riscv/cpu.h
@@ -221,6 +221,7 @@ typedef struct RISCVCPU {
 bool ext_c;
 bool ext_s;
 bool ext_u;
+bool ext_counters;
 
 char *priv_spec;
 char *user_spec;
diff --git a/target/riscv/csr.c b/target/riscv/csr.c
index 437387fd28..a9aa8ab1b5 100644
--- a/target/riscv/csr.c
+++ b/target/riscv/csr.c
@@ -56,8 +56,15 @@ static int fs(CPURISCVState *env, int csrno)
 static int ctr(CPURISCVState *env, int csrno)
 {
 #if !defined(CONFIG_USER_ONLY)
+CPUState *cs = env_cpu(env);
+RISCVCPU *cpu = RISCV_CPU(cs);
 uint32_t ctr_en = ~0u;
 
+if (!cpu->cfg.ext_counters) {
+/* The Counters extensions is not enabled */
+return -1;
+}
+
 if (env->priv < PRV_M) {
 ctr_en &= env->mcounteren;
 }
-- 
2.22.0




[Qemu-devel] [PATCH v1 6/9] target/riscv: Require either I or E base extension

2019-06-17 Thread Alistair Francis
Signed-off-by: Alistair Francis 
---
 target/riscv/cpu.c | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index a23d83921a..21bfaa9722 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -373,6 +373,12 @@ static void riscv_cpu_realize(DeviceState *dev, Error 
**errp)
return;
}
 
+if (!cpu->cfg.ext_i && !cpu->cfg.ext_e) {
+error_setg(errp,
+   "Either I or E extension must be set");
+   return;
+   }
+
if (cpu->cfg.ext_g && !(cpu->cfg.ext_i & cpu->cfg.ext_m &
cpu->cfg.ext_a & cpu->cfg.ext_f &
cpu->cfg.ext_d)) {
-- 
2.22.0




[Qemu-devel] [PATCH v1 3/9] target/riscv: Comment in the mcountinhibit CSR

2019-06-17 Thread Alistair Francis
Add a comment for the new mcountinhibit which conflicts with the
CSR_MUCOUNTEREN from version 1.09.1. This can be updated when we remove
1.09.1.

Signed-off-by: Alistair Francis 
---
 target/riscv/cpu_bits.h | 1 +
 target/riscv/csr.c  | 6 --
 2 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/target/riscv/cpu_bits.h b/target/riscv/cpu_bits.h
index 47450a3cdb..11f971ad5d 100644
--- a/target/riscv/cpu_bits.h
+++ b/target/riscv/cpu_bits.h
@@ -136,6 +136,7 @@
 #define CSR_MCOUNTEREN  0x306
 
 /* Legacy Counter Setup (priv v1.9.1) */
+/* Update to #define CSR_MCOUNTINHIBIT 0x320 for 1.11.0 */
 #define CSR_MUCOUNTEREN 0x320
 #define CSR_MSCOUNTEREN 0x321
 #define CSR_MHCOUNTEREN 0x322
diff --git a/target/riscv/csr.c b/target/riscv/csr.c
index c67d29e206..437387fd28 100644
--- a/target/riscv/csr.c
+++ b/target/riscv/csr.c
@@ -461,18 +461,20 @@ static int write_mcounteren(CPURISCVState *env, int 
csrno, target_ulong val)
 return 0;
 }
 
+/* This regiser is replaced with CSR_MCOUNTINHIBIT in 1.11.0 */
 static int read_mscounteren(CPURISCVState *env, int csrno, target_ulong *val)
 {
-if (env->priv_ver > PRIV_VERSION_1_09_1) {
+if (env->priv_ver > PRIV_VERSION_1_09_1 && env->priv_ver < 
PRIV_VERSION_1_11_0) {
 return -1;
 }
 *val = env->mcounteren;
 return 0;
 }
 
+/* This regiser is replaced with CSR_MCOUNTINHIBIT in 1.11.0 */
 static int write_mscounteren(CPURISCVState *env, int csrno, target_ulong val)
 {
-if (env->priv_ver > PRIV_VERSION_1_09_1) {
+if (env->priv_ver > PRIV_VERSION_1_09_1 && env->priv_ver < 
PRIV_VERSION_1_11_0) {
 return -1;
 }
 env->mcounteren = val;
-- 
2.22.0




[Qemu-devel] [PATCH v1 9/9] target/riscv: Add Zifencei and Zicsr as command line options

2019-06-17 Thread Alistair Francis
For completeness let's add Zifencei and Zicsr as command line options,
even though they can't be disabled at the moment.

Signed-off-by: Alistair Francis 
---
 target/riscv/cpu.c | 9 +
 target/riscv/cpu.h | 2 ++
 2 files changed, 11 insertions(+)

diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index 5af1c9b38c..53cf8607f7 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -409,6 +409,13 @@ static void riscv_cpu_realize(DeviceState *dev, Error 
**errp)
 set_misa(env, RVXLEN | target_misa);
 }
 
+if (!cpu->cfg.ext_ifencei) {
+warn_report("QEMU does not support disabling Zifencei");
+}
+if (!cpu->cfg.ext_icsr) {
+warn_report("QEMU does not support disabling Zicsr");
+}
+
 riscv_cpu_register_gdb_regs_for_features(cs);
 
 qemu_init_vcpu(cs);
@@ -441,6 +448,8 @@ static Property riscv_cpu_properties[] = {
 DEFINE_PROP_BOOL("s", RISCVCPU, cfg.ext_s, true),
 DEFINE_PROP_BOOL("u", RISCVCPU, cfg.ext_u, true),
 DEFINE_PROP_BOOL("Counters", RISCVCPU, cfg.ext_counters, true),
+DEFINE_PROP_BOOL("Zifencei", RISCVCPU, cfg.ext_ifencei, true),
+DEFINE_PROP_BOOL("Zicsr", RISCVCPU, cfg.ext_icsr, true),
 DEFINE_PROP_STRING("priv_spec", RISCVCPU, cfg.priv_spec),
 DEFINE_PROP_BOOL("mmu", RISCVCPU, cfg.mmu, true),
 DEFINE_PROP_BOOL("pmp", RISCVCPU, cfg.pmp, true),
diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
index 786f620564..b4c212dfcf 100644
--- a/target/riscv/cpu.h
+++ b/target/riscv/cpu.h
@@ -222,6 +222,8 @@ typedef struct RISCVCPU {
 bool ext_s;
 bool ext_u;
 bool ext_counters;
+bool ext_ifencei;
+bool ext_icsr;
 
 char *priv_spec;
 char *user_spec;
-- 
2.22.0




[Qemu-devel] [PATCH v1 1/9] target/riscv: Restructure deprecatd CPUs

2019-06-17 Thread Alistair Francis
Restructure the deprecated CPUs to make it clear in the code that these
are depreated. They are already marked as deprecated in
qemu-deprecated.texi. There are no functional changes.

Signed-off-by: Alistair Francis 
---
 target/riscv/cpu.c | 18 ++
 target/riscv/cpu.h | 13 +++--
 2 files changed, 17 insertions(+), 14 deletions(-)

diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index 0632ac08cf..a4dd7ae6fc 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -558,18 +558,20 @@ static const TypeInfo riscv_cpu_type_infos[] = {
 DEFINE_CPU(TYPE_RISCV_CPU_ANY,  riscv_any_cpu_init),
 #if defined(TARGET_RISCV32)
 DEFINE_CPU(TYPE_RISCV_CPU_BASE32,   riscv_base32_cpu_init),
-DEFINE_CPU(TYPE_RISCV_CPU_RV32GCSU_V1_09_1, rv32gcsu_priv1_09_1_cpu_init),
-DEFINE_CPU(TYPE_RISCV_CPU_RV32GCSU_V1_10_0, rv32gcsu_priv1_10_0_cpu_init),
-DEFINE_CPU(TYPE_RISCV_CPU_RV32IMACU_NOMMU,  rv32imacu_nommu_cpu_init),
 DEFINE_CPU(TYPE_RISCV_CPU_SIFIVE_E31,   rv32imacu_nommu_cpu_init),
-DEFINE_CPU(TYPE_RISCV_CPU_SIFIVE_U34,   rv32gcsu_priv1_10_0_cpu_init)
+DEFINE_CPU(TYPE_RISCV_CPU_SIFIVE_U34,   rv32gcsu_priv1_10_0_cpu_init),
+/* Depreacted */
+DEFINE_CPU(TYPE_RISCV_CPU_RV32IMACU_NOMMU,  rv32imacu_nommu_cpu_init),
+DEFINE_CPU(TYPE_RISCV_CPU_RV32GCSU_V1_09_1, rv32gcsu_priv1_09_1_cpu_init),
+DEFINE_CPU(TYPE_RISCV_CPU_RV32GCSU_V1_10_0, rv32gcsu_priv1_10_0_cpu_init)
 #elif defined(TARGET_RISCV64)
 DEFINE_CPU(TYPE_RISCV_CPU_BASE64,   riscv_base64_cpu_init),
-DEFINE_CPU(TYPE_RISCV_CPU_RV64GCSU_V1_09_1, rv64gcsu_priv1_09_1_cpu_init),
-DEFINE_CPU(TYPE_RISCV_CPU_RV64GCSU_V1_10_0, rv64gcsu_priv1_10_0_cpu_init),
-DEFINE_CPU(TYPE_RISCV_CPU_RV64IMACU_NOMMU,  rv64imacu_nommu_cpu_init),
 DEFINE_CPU(TYPE_RISCV_CPU_SIFIVE_E51,   rv64imacu_nommu_cpu_init),
-DEFINE_CPU(TYPE_RISCV_CPU_SIFIVE_U54,   rv64gcsu_priv1_10_0_cpu_init)
+DEFINE_CPU(TYPE_RISCV_CPU_SIFIVE_U54,   rv64gcsu_priv1_10_0_cpu_init),
+/* Deprecated */
+DEFINE_CPU(TYPE_RISCV_CPU_RV64IMACU_NOMMU,  rv64imacu_nommu_cpu_init),
+DEFINE_CPU(TYPE_RISCV_CPU_RV64GCSU_V1_09_1, rv64gcsu_priv1_09_1_cpu_init),
+DEFINE_CPU(TYPE_RISCV_CPU_RV64GCSU_V1_10_0, rv64gcsu_priv1_10_0_cpu_init)
 #endif
 };
 
diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
index b47cde5017..1668d12018 100644
--- a/target/riscv/cpu.h
+++ b/target/riscv/cpu.h
@@ -35,16 +35,17 @@
 #define TYPE_RISCV_CPU_ANY  RISCV_CPU_TYPE_NAME("any")
 #define TYPE_RISCV_CPU_BASE32   RISCV_CPU_TYPE_NAME("rv32")
 #define TYPE_RISCV_CPU_BASE64   RISCV_CPU_TYPE_NAME("rv64")
-#define TYPE_RISCV_CPU_RV32GCSU_V1_09_1 RISCV_CPU_TYPE_NAME("rv32gcsu-v1.9.1")
-#define TYPE_RISCV_CPU_RV32GCSU_V1_10_0 RISCV_CPU_TYPE_NAME("rv32gcsu-v1.10.0")
-#define TYPE_RISCV_CPU_RV32IMACU_NOMMU  RISCV_CPU_TYPE_NAME("rv32imacu-nommu")
-#define TYPE_RISCV_CPU_RV64GCSU_V1_09_1 RISCV_CPU_TYPE_NAME("rv64gcsu-v1.9.1")
-#define TYPE_RISCV_CPU_RV64GCSU_V1_10_0 RISCV_CPU_TYPE_NAME("rv64gcsu-v1.10.0")
-#define TYPE_RISCV_CPU_RV64IMACU_NOMMU  RISCV_CPU_TYPE_NAME("rv64imacu-nommu")
 #define TYPE_RISCV_CPU_SIFIVE_E31   RISCV_CPU_TYPE_NAME("sifive-e31")
 #define TYPE_RISCV_CPU_SIFIVE_E51   RISCV_CPU_TYPE_NAME("sifive-e51")
 #define TYPE_RISCV_CPU_SIFIVE_U34   RISCV_CPU_TYPE_NAME("sifive-u34")
 #define TYPE_RISCV_CPU_SIFIVE_U54   RISCV_CPU_TYPE_NAME("sifive-u54")
+/* Deprecated */
+#define TYPE_RISCV_CPU_RV32IMACU_NOMMU  RISCV_CPU_TYPE_NAME("rv32imacu-nommu")
+#define TYPE_RISCV_CPU_RV32GCSU_V1_09_1 RISCV_CPU_TYPE_NAME("rv32gcsu-v1.9.1")
+#define TYPE_RISCV_CPU_RV32GCSU_V1_10_0 RISCV_CPU_TYPE_NAME("rv32gcsu-v1.10.0")
+#define TYPE_RISCV_CPU_RV64IMACU_NOMMU  RISCV_CPU_TYPE_NAME("rv64imacu-nommu")
+#define TYPE_RISCV_CPU_RV64GCSU_V1_09_1 RISCV_CPU_TYPE_NAME("rv64gcsu-v1.9.1")
+#define TYPE_RISCV_CPU_RV64GCSU_V1_10_0 RISCV_CPU_TYPE_NAME("rv64gcsu-v1.10.0")
 
 #define RV32 ((target_ulong)1 << (TARGET_LONG_BITS - 2))
 #define RV64 ((target_ulong)2 << (TARGET_LONG_BITS - 2))
-- 
2.22.0




[Qemu-devel] [PATCH v1 5/9] qemu-deprecated.texi: Deprecate the RISC-V privledge spec 1.09.1

2019-06-17 Thread Alistair Francis
Deprecate the RISC-V privledge spec version 1.09.1 in favour of the new
1.10.0 and the ratified 1.11.0.

Signed-off-by: Alistair Francis 
---
 qemu-deprecated.texi | 8 
 1 file changed, 8 insertions(+)

diff --git a/qemu-deprecated.texi b/qemu-deprecated.texi
index 50292d820b..bdebf7 100644
--- a/qemu-deprecated.texi
+++ b/qemu-deprecated.texi
@@ -138,6 +138,14 @@ The ``acl_show'', ``acl_reset'', ``acl_policy'', 
``acl_add'', and
 ``acl_remove'' commands are deprecated with no replacement. Authorization
 for VNC should be performed using the pluggable QAuthZ objects.
 
+@section Guest Emulator ISAs
+
+@subsection RISC-V ISA privledge specification version 1.09.1 (since 4.1)
+
+The RISC-V ISA privledge specification version 1.09.1 has been deprecated.
+QEMU supports both the newer version 1.10.0 and the ratified version 1.11.0, 
these
+should be used instead of the 1.09.1 version.
+
 @section System emulator CPUS
 
 @subsection RISC-V ISA CPUs (since 4.1)
-- 
2.22.0




[Qemu-devel] [PATCH v1 2/9] target/riscv: Add the privledge spec version 1.11.0

2019-06-17 Thread Alistair Francis
Add support for the ratified RISC-V privledge spec.

Signed-off-by: Alistair Francis 
---
 target/riscv/cpu.h | 1 +
 target/riscv/insn_trans/trans_privileged.inc.c | 2 +-
 2 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
index 1668d12018..4e58c3b856 100644
--- a/target/riscv/cpu.h
+++ b/target/riscv/cpu.h
@@ -81,6 +81,7 @@ enum {
 #define USER_VERSION_2_02_0 0x00020200
 #define PRIV_VERSION_1_09_1 0x00010901
 #define PRIV_VERSION_1_10_0 0x00011000
+#define PRIV_VERSION_1_11_0 0x00011100
 
 #define TRANSLATE_FAIL 1
 #define TRANSLATE_SUCCESS 0
diff --git a/target/riscv/insn_trans/trans_privileged.inc.c 
b/target/riscv/insn_trans/trans_privileged.inc.c
index 664d6ba3f2..c5e4b3e49a 100644
--- a/target/riscv/insn_trans/trans_privileged.inc.c
+++ b/target/riscv/insn_trans/trans_privileged.inc.c
@@ -90,7 +90,7 @@ static bool trans_wfi(DisasContext *ctx, arg_wfi *a)
 static bool trans_sfence_vma(DisasContext *ctx, arg_sfence_vma *a)
 {
 #ifndef CONFIG_USER_ONLY
-if (ctx->priv_ver == PRIV_VERSION_1_10_0) {
+if (ctx->priv_ver >= PRIV_VERSION_1_10_0) {
 gen_helper_tlb_flush(cpu_env);
 return true;
 }
-- 
2.22.0




[Qemu-devel] [PATCH v1 0/9] Update the RISC-V specification versions

2019-06-17 Thread Alistair Francis
Based-on: 

Now that the RISC-V spec has started to be ratified let's update our
QEMU implementation. There are a few things going on here:
 - Add priv version 1.11.0 to QEMU
- This is the ratified version of the Privledge spec
- There are almost no changes to 1.10
 - Mark the 1.09.1 privledge spec as depreated
 - Let's aim to remove it in two releases
 - Set priv version 1.11.0 as the default
 - Remove the user_spec version
 - This doesn't really mean anything so let's remove it
 - Add support for the "Counters" extension
 - Add command line options for Zifencei and Zicsr

We can remove the spec version as it's unused and has never been exposed
to users. The idea is to match the specs in specifying the version. To
handle versions in the future we can extend the extension props to
handle version information.

For example something like this: -cpu rv64,i=2.2,c=2.0,h=0.4,priv_spec=1.11

NOTE: This isn't supported today as we only have one of each version.

This will be a future change if we decide to support multiple versions
of extensions.

The "priv_spec" string doesn't really match, but I don't have a better
way to say "Machine ISA" and "Supervisor ISA" which is what is included
in "priv_spec".

For completeness I have also added the Counters, Zifencei and Zicsr
extensions.

Everything else seems to match the spec names/style.

Please let me know if I'm missing something. QEMU 4.1 is the first
release to support the extensions from the command line, so we can
easily change it until then. After that it'll take more work to change
the command line interface.

Alistair Francis (9):
  target/riscv: Restructure deprecatd CPUs
  target/riscv: Add the privledge spec version 1.11.0
  target/riscv: Comment in the mcountinhibit CSR
  target/riscv: Set privledge spec 1.11.0 as default
  qemu-deprecated.texi: Deprecate the RISC-V privledge spec 1.09.1
  target/riscv: Require either I or E base extension
  target/riscv: Remove user version information
  target/riscv: Add support for disabling/enabling Counters
  target/riscv: Add Zifencei and Zicsr as command line options

 qemu-deprecated.texi  |  8 +++
 target/riscv/cpu.c| 72 ++-
 target/riscv/cpu.h| 19 ++---
 target/riscv/cpu_bits.h   |  1 +
 target/riscv/csr.c| 13 +++-
 .../riscv/insn_trans/trans_privileged.inc.c   |  2 +-
 6 files changed, 71 insertions(+), 44 deletions(-)

-- 
2.22.0




[Qemu-devel] [PATCH v1 4/9] target/riscv: Set privledge spec 1.11.0 as default

2019-06-17 Thread Alistair Francis
Set the priv spec version 1.11.0 as the default and allow selecting it
via the command line.

Signed-off-by: Alistair Francis 
---
 target/riscv/cpu.c | 8 +---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index a4dd7ae6fc..a23d83921a 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -111,7 +111,7 @@ static void riscv_any_cpu_init(Object *obj)
 {
 CPURISCVState *env = _CPU(obj)->env;
 set_misa(env, RVXLEN | RVI | RVM | RVA | RVF | RVD | RVC | RVU);
-set_versions(env, USER_VERSION_2_02_0, PRIV_VERSION_1_10_0);
+set_versions(env, USER_VERSION_2_02_0, PRIV_VERSION_1_11_0);
 set_resetvec(env, DEFAULT_RSTVEC);
 }
 
@@ -316,7 +316,7 @@ static void riscv_cpu_realize(DeviceState *dev, Error 
**errp)
 RISCVCPU *cpu = RISCV_CPU(dev);
 CPURISCVState *env = >env;
 RISCVCPUClass *mcc = RISCV_CPU_GET_CLASS(dev);
-int priv_version = PRIV_VERSION_1_10_0;
+int priv_version = PRIV_VERSION_1_11_0;
 int user_version = USER_VERSION_2_02_0;
 target_ulong target_misa = 0;
 Error *local_err = NULL;
@@ -328,7 +328,9 @@ static void riscv_cpu_realize(DeviceState *dev, Error 
**errp)
 }
 
 if (cpu->cfg.priv_spec) {
-if (!g_strcmp0(cpu->cfg.priv_spec, "v1.10.0")) {
+if (!g_strcmp0(cpu->cfg.priv_spec, "v1.11.0")) {
+priv_version = PRIV_VERSION_1_11_0;
+} else if (!g_strcmp0(cpu->cfg.priv_spec, "v1.10.0")) {
 priv_version = PRIV_VERSION_1_10_0;
 } else if (!g_strcmp0(cpu->cfg.priv_spec, "v1.9.1")) {
 priv_version = PRIV_VERSION_1_09_1;
-- 
2.22.0




Re: [Qemu-devel] [PATCH 3/5] pci: designware: Update MSI mapping unconditionally

2019-06-17 Thread Michael S. Tsirkin
On Mon, Apr 15, 2019 at 06:39:00PM -0700, Andrey Smirnov wrote:
> Expression to calculate update_msi_mapping in code handling writes to
> DESIGNWARE_PCIE_MSI_INTR0_ENABLE is missing an ! operator and should
> be:
> 
> !!root->msi.intr[0].enable ^ !!val;
> 
> so that MSI mapping is updated when enabled transitions from either
> "none" -> "any" or "any" -> "none". Since that register shouldn't be
> written to very often, change the code to update MSI mapping
> unconditionally instead of trying to fix the update_msi_mapping logic.
> 
> Signed-off-by: Andrey Smirnov 
> Cc: Peter Maydell 
> Cc: Michael S. Tsirkin 
> Cc: qemu-devel@nongnu.org
> Cc: qemu-...@nongnu.org

Acked-by: Michael S. Tsirkin 

> ---
>  hw/pci-host/designware.c | 10 ++
>  1 file changed, 2 insertions(+), 8 deletions(-)
> 
> diff --git a/hw/pci-host/designware.c b/hw/pci-host/designware.c
> index 29ea313798..6affe823c0 100644
> --- a/hw/pci-host/designware.c
> +++ b/hw/pci-host/designware.c
> @@ -296,16 +296,10 @@ static void designware_pcie_root_config_write(PCIDevice 
> *d, uint32_t address,
>  root->msi.base |= (uint64_t)val << 32;
>  break;
>  
> -case DESIGNWARE_PCIE_MSI_INTR0_ENABLE: {
> -const bool update_msi_mapping = !root->msi.intr[0].enable ^ !!val;
> -
> +case DESIGNWARE_PCIE_MSI_INTR0_ENABLE:
>  root->msi.intr[0].enable = val;
> -
> -if (update_msi_mapping) {
> -designware_pcie_root_update_msi_mapping(root);
> -}
> +designware_pcie_root_update_msi_mapping(root);
>  break;
> -}
>  
>  case DESIGNWARE_PCIE_MSI_INTR0_MASK:
>  root->msi.intr[0].mask = val;
> -- 
> 2.20.1



Re: [Qemu-devel] [PATCH 4/5] pci: designware: Update MSI mapping when MSI address changes

2019-06-17 Thread Michael S. Tsirkin
On Mon, Apr 15, 2019 at 06:39:01PM -0700, Andrey Smirnov wrote:
> MSI mapping needs to be update when MSI address changes, so add the
> code to do so.
> 
> Signed-off-by: Andrey Smirnov 
> Cc: Peter Maydell 
> Cc: Michael S. Tsirkin 
> Cc: qemu-devel@nongnu.org
> Cc: qemu-...@nongnu.org

Acked-by: Michael S. Tsirkin 

> ---
>  hw/pci-host/designware.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/hw/pci-host/designware.c b/hw/pci-host/designware.c
> index 6affe823c0..e80facc4a0 100644
> --- a/hw/pci-host/designware.c
> +++ b/hw/pci-host/designware.c
> @@ -289,11 +289,13 @@ static void designware_pcie_root_config_write(PCIDevice 
> *d, uint32_t address,
>  case DESIGNWARE_PCIE_MSI_ADDR_LO:
>  root->msi.base &= 0xULL;
>  root->msi.base |= val;
> +designware_pcie_root_update_msi_mapping(root);
>  break;
>  
>  case DESIGNWARE_PCIE_MSI_ADDR_HI:
>  root->msi.base &= 0xULL;
>  root->msi.base |= (uint64_t)val << 32;
> +designware_pcie_root_update_msi_mapping(root);
>  break;
>  
>  case DESIGNWARE_PCIE_MSI_INTR0_ENABLE:
> -- 
> 2.20.1



Re: [Qemu-devel] [PATCH] i386: Fix signedness of hyperv_spinlock_attempts

2019-06-17 Thread Vadim Rozenfeld
On Mon, 2019-06-17 at 14:49 -0300, Eduardo Habkost wrote:
> On Mon, Jun 17, 2019 at 05:32:13PM +, Roman Kagan wrote:
> > On Mon, Jun 17, 2019 at 11:23:01AM -0300, Eduardo Habkost wrote:
> > > On Mon, Jun 17, 2019 at 01:48:59PM +, Roman Kagan wrote:
> > > > On Sat, Jun 15, 2019 at 05:05:05PM -0300, Eduardo Habkost
> > > > wrote:
> > > > > The current default value for hv-spinlocks is 0x
> > > > > (meaning
> > > > > "never retry").  However, the value is stored as a signed
> > > > > integer, making the getter of the hv-spinlocks QOM property
> > > > > return -1 instead of 0x.
> > > > > 
> > > > > Fix this by changing the type of
> > > > > X86CPU::hyperv_spinlock_attempts
> > > > > to uint32_t.  This has no visible effect to guest operating
> > > > > systems, affecting just the behavior of the QOM getter.
> > > > > 
> > > > > Signed-off-by: Eduardo Habkost 
> > > > > ---
> > > > >  target/i386/cpu.h | 2 +-
> > > > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > > 
> > > > Reviewed-by: Roman Kagan 
> > > > 
> > > > That said, it's tempting to just nuke qdev_prop_spinlocks and
> > > > make
> > > > hv-spinlocks a regular DEFINE_PROP_UINT32...
> > > 
> > > Agreed.  The only difference is that we would validate the
> > > property at realize time instead of object_property_set().
> > 
> > Right.  But currently it's validated to be no less than 0xfff and
> > no
> > bigger than 0x.  The latter check would become unnecessary,
> > and
> > I'm unable to find any reason to do the former (neither spec
> > references
> > nor the log messages of the commits that introduced it).
> 
> The 0xFFF lower limit was originally introduced by commit
> 28f52cc04d34 ("hyper-v: introduce Hyper-V support infrastructure").
> 
> Vadim, do you know where the 0xFFF limit comes from?

I simply took this value from Windows Server 2008 R2 that 
I used as a reference while working on Hyper-V support for KVM.
I also remember some paper (probably published by AMD ???) mentioned
that 0x2fff seemed to have the best balance for PLE logic.


Best,
Vadim.



Re: [Qemu-devel] [PATCH 0/5] Various i.MX7 fixes

2019-06-17 Thread Andrey Smirnov
On Mon, Apr 15, 2019 at 6:39 PM Andrey Smirnov  wrote:
>
> Everyone:
>
> I recently revisited my i.MX7 work and this series contains all of the
> fixes I had to make to get it to work with latest (5.1-rc1) Linux
> kernel again as well as fixes for genuine bugs that I somehow missed
> during original submission ("pci: designware" patches). Hopefully each
> patch is self-explanatory.
>
> Feedback is welcome!
>

Is there any changes necessary for this to go in?

Thanks,
Andrey Smirnov



Re: [Qemu-devel] [PATCH] atomic failures on qemu-system-riscv64

2019-06-17 Thread Richard Henderson
On 6/16/19 12:19 PM, Joel Sing wrote:
> +/*
> + * Clear the load reservation, since an SC must fail if there is
> + * an SC to any address, in between an LR and SC pair.
> + */
> +tcg_gen_movi_tl(load_res, 0);
> +
>  gen_set_label(l2);

This clear needs to be moved down below label l2.
Otherwise, with lr / sc / sc, the second sc could succeed in error.

FWIW, other targets have used -1 as the "invalid" load reservation, since the
architecture does not require address 0 to be unmapped.  This should be quite
visible in M-mode with paging disabled and ram at offset 0.  Often, other
targets require alignment for the lr/sc address, though I don't see that for 
riscv.


r~



Re: [Qemu-devel] [PATCH v3 01/50] trace: expand mem_info:size_shift to 3 bits

2019-06-17 Thread Richard Henderson
On 6/17/19 1:22 AM, Alex Bennée wrote:
> 
> Richard Henderson  writes:
> 
>> On 6/14/19 10:11 AM, Alex Bennée wrote:
>>> From: "Emilio G. Cota" 
>>>
>>> This will allow us to trace 16B-long memory accesses.
>>>
>>> Reviewed-by: Alex Bennée 
>>> Signed-off-by: Emilio G. Cota 
>>> ---
>>>  trace-events | 2 +-
>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/trace-events b/trace-events
>>> index 844ee58dd9..037169aab3 100644
>>> --- a/trace-events
>>> +++ b/trace-events
>>> @@ -159,7 +159,7 @@ vcpu guest_cpu_reset(void)
>>>  # Access information can be parsed as:
>>>  #
>>>  # struct mem_info {
>>> -# uint8_t size_shift : 2; /* interpreted as "1 << size_shift" bytes */
>>> +# uint8_t size_shift : 3; /* interpreted as "1 << size_shift" bytes */
>>>  # boolsign_extend: 1; /* sign-extended */
>>>  # uint8_t endianness : 1; /* 0: little, 1: big */
>>>  # boolstore  : 1; /* wheter it's a store operation */
>>>
>>
>> Well, 128B-long memory accesses.  But SVE supports 256B memory accesses
>> already.  So why not add one more bit now.
> 
> Good point.
> 
> Do we have any architectures that do load/stores that are not power of
> 2? I guess the SVE non-faulting accesses are treated as a series of elem
> size accesses.

Yes, non-faults are in addition predicated, so each element is considered
individually.

Even the SVE non-predicated load/stores can technically be considered a
sequence of byte operations.  Which, I suppose could be helpful, because SVE
can otherwise be configured to do non-power-of-2 operations.


r~



Re: [Qemu-devel] [PATCH v3 49/50] include/exec/cpu-defs.h: fix typo

2019-06-17 Thread Richard Henderson
On 6/14/19 10:11 AM, Alex Bennée wrote:
> - * This structure must be placed in ArchCPU immedately
> + * This structure must be placed in ArchCPU immediately

Reviewed-by: Richard Henderson 


r~




Re: [Qemu-devel] [PATCH v3 47/50] accel/stubs: reduce headers from tcg-stub

2019-06-17 Thread Richard Henderson
On 6/14/19 10:11 AM, Alex Bennée wrote:
> We don't need much for these. However I do wonder why these aren't
> just null inlines in exec-all.h
> 
> Signed-off-by: Alex Bennée 
> ---
>  accel/stubs/tcg-stub.c | 3 ---
>  1 file changed, 3 deletions(-)

Reviewed-by: Richard Henderson 


r~




Re: [Qemu-devel] [PATCH v3 44/50] plugin: add qemu_plugin_insn_disas helper

2019-06-17 Thread Richard Henderson
On 6/14/19 10:11 AM, Alex Bennée wrote:
> Give the plugins access to the QEMU dissasembler so they don't have to
> re-invent the wheel.
> 
> Signed-off-by: Alex Bennée 
> ---
>  disas.c  | 103 +++
>  include/disas/disas.h|   2 +
>  include/qemu/qemu-plugin.h   |   9 +++
>  plugins/api.c|   7 +++
>  plugins/qemu-plugins.symbols |   1 +
>  5 files changed, 122 insertions(+)

There are a couple of checkpatch errors, otherwise
Reviewed-by: Richard Henderson 


r~




Re: [Qemu-devel] [PATCH v3 40/50] linux-user: support -plugin option

2019-06-17 Thread Richard Henderson
On 6/14/19 10:11 AM, Alex Bennée wrote:
> From: Lluís Vilanova 
> 
> Signed-off-by: Lluís Vilanova 
> [ cota: s/instrument/plugin ]
> Signed-off-by: Emilio G. Cota 
> ---
>  linux-user/main.c | 18 ++
>  1 file changed, 18 insertions(+)

Reviewed-by: Richard Henderson 


r~




Re: [Qemu-devel] [PATCH v3 39/50] vl: support -plugin option

2019-06-17 Thread Richard Henderson
On 6/14/19 10:11 AM, Alex Bennée wrote:
> From: Lluís Vilanova 
> 
> Signed-off-by: Lluís Vilanova 
> [ cota: s/instrument/plugin ]
> Signed-off-by: Emilio G. Cota 
> ---
>  qemu-options.hx | 17 +
>  vl.c| 11 +++
>  2 files changed, 28 insertions(+)

Reviewed-by: Richard Henderson 


r~



Re: [Qemu-devel] [PULL 00/16] Monitor patches for 2019-06-17

2019-06-17 Thread no-reply
Patchew URL: https://patchew.org/QEMU/20190617184903.19436-1-arm...@redhat.com/



Hi,

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

=== TEST SCRIPT BEGIN ===
#!/bin/bash
# Testing script will be invoked under the git checkout with
# HEAD pointing to a commit that has the patches applied on top of "base"
# branch
set -e
CC=$HOME/bin/cc
INSTALL=$PWD/install
BUILD=$PWD/build
mkdir -p $BUILD $INSTALL
SRC=$PWD
cd $BUILD
$SRC/configure --cc=$CC --prefix=$INSTALL
make -j4
# XXX: we need reliable clean up
# make check -j4 V=1
make install

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

echo
echo "=== PACKAGES ==="
rpm -qa
=== TEST SCRIPT END ===

  CC  arm-softmmu/hw/block/virtio-blk.o
  CC  alpha-softmmu/hw/char/virtio-serial-bus.o
/var/tmp/patchew-tester-tmp-0lxd7r14/src/monitor/misc.c: In function 
‘netdev_del_completion’:
/var/tmp/patchew-tester-tmp-0lxd7r14/src/monitor/misc.c:2165:31: error: 
implicit declaration of function ‘qemu_find_opts_err’; did you mean 
‘qemu_find_netdev’? [-Werror=implicit-function-declaration]
 2165 | opts = qemu_opts_find(qemu_find_opts_err("netdev", NULL), name);
  |   ^~
  |   qemu_find_netdev
/var/tmp/patchew-tester-tmp-0lxd7r14/src/monitor/misc.c:2165:31: error: nested 
extern declaration of ‘qemu_find_opts_err’ [-Werror=nested-externs]
/var/tmp/patchew-tester-tmp-0lxd7r14/src/monitor/misc.c:2165:31: error: passing 
argument 1 of ‘qemu_opts_find’ makes pointer from integer without a cast 
[-Werror=int-conversion]
 2165 | opts = qemu_opts_find(qemu_find_opts_err("netdev", NULL), name);
  |   ^~
  |   |
---
  CC  aarch64-softmmu/hw/intc/arm_gicv3_cpuif.o
  CC  alpha-softmmu/qapi/qapi-introspect.o
/var/tmp/patchew-tester-tmp-0lxd7r14/src/monitor/misc.c: In function 
‘netdev_del_completion’:
/var/tmp/patchew-tester-tmp-0lxd7r14/src/monitor/misc.c:2165:31: error: 
implicit declaration of function ‘qemu_find_opts_err’; did you mean 
‘qemu_find_netdev’? [-Werror=implicit-function-declaration]
 2165 | opts = qemu_opts_find(qemu_find_opts_err("netdev", NULL), name);
  |   ^~
  |   qemu_find_netdev
/var/tmp/patchew-tester-tmp-0lxd7r14/src/monitor/misc.c:2165:31: error: nested 
extern declaration of ‘qemu_find_opts_err’ [-Werror=nested-externs]
/var/tmp/patchew-tester-tmp-0lxd7r14/src/monitor/misc.c:2165:31: error: passing 
argument 1 of ‘qemu_opts_find’ makes pointer from integer without a cast 
[-Werror=int-conversion]
 2165 | opts = qemu_opts_find(qemu_find_opts_err("netdev", NULL), name);
  |   ^~
  |   |
---
  CC  arm-softmmu/hw/arm/msf2-soc.o
  CC  aarch64-softmmu/qapi/qapi-types.o
/var/tmp/patchew-tester-tmp-0lxd7r14/src/monitor/misc.c: In function 
‘netdev_del_completion’:
/var/tmp/patchew-tester-tmp-0lxd7r14/src/monitor/misc.c:2165:31: error: 
implicit declaration of function ‘qemu_find_opts_err’; did you mean 
‘qemu_find_netdev’? [-Werror=implicit-function-declaration]
 2165 | opts = qemu_opts_find(qemu_find_opts_err("netdev", NULL), name);
  |   ^~
  |   qemu_find_netdev
/var/tmp/patchew-tester-tmp-0lxd7r14/src/monitor/misc.c:2165:31: error: nested 
extern declaration of ‘qemu_find_opts_err’ [-Werror=nested-externs]
/var/tmp/patchew-tester-tmp-0lxd7r14/src/monitor/misc.c:2165:31: error: passing 
argument 1 of ‘qemu_opts_find’ makes pointer from integer without a cast 
[-Werror=int-conversion]
 2165 | opts = qemu_opts_find(qemu_find_opts_err("netdev", NULL), name);
  |   ^~
  |   |
---
  CC  arm-softmmu/qapi/qapi-events.o
  CC  arm-softmmu/qapi/qapi-commands-target.o
/var/tmp/patchew-tester-tmp-0lxd7r14/src/monitor/misc.c: In function 
‘netdev_del_completion’:
/var/tmp/patchew-tester-tmp-0lxd7r14/src/monitor/misc.c:2165:31: error: 
implicit declaration of function ‘qemu_find_opts_err’; did you mean 
‘qemu_find_netdev’? [-Werror=implicit-function-declaration]
 2165 | opts = qemu_opts_find(qemu_find_opts_err("netdev", NULL), name);
  |   ^~
  |   qemu_find_netdev
/var/tmp/patchew-tester-tmp-0lxd7r14/src/monitor/misc.c:2165:31: error: nested 
extern declaration of ‘qemu_find_opts_err’ [-Werror=nested-externs]
/var/tmp/patchew-tester-tmp-0lxd7r14/src/monitor/misc.c:2165:31: error: passing 
argument 1 of ‘qemu_opts_find’ makes pointer from integer without a cast 
[-Werror=int-conversion]
 2165 | opts = qemu_opts_find(qemu_find_opts_err("netdev", NULL), name);
  | 

Re: [Qemu-devel] [PATCH v3 36/50] target/openrisc: fetch code with translator_ld

2019-06-17 Thread Richard Henderson
On 6/14/19 10:11 AM, Alex Bennée wrote:
> From: "Emilio G. Cota" 
> 
> Signed-off-by: Emilio G. Cota 
> ---
>  target/openrisc/translate.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Reviewed-by: Richard Henderson 


r~




Re: [Qemu-devel] [PATCH v3 37/50] translator: inject instrumentation from plugins

2019-06-17 Thread Richard Henderson
On 6/14/19 10:11 AM, Alex Bennée wrote:
> @@ -95,6 +103,10 @@ void translator_loop(const TranslatorOps *ops, 
> DisasContextBase *db,
>  ops->translate_insn(db, cpu);
>  }
>  
> +if (plugin_enabled) {
> +plugin_gen_insn_end();
> +}
> +
>  /* Stop translation if translate_insn so indicated.  */
>  if (db->is_jmp != DISAS_NEXT) {

This will of course not be reachable if db->is_jmp == DISAS_NORETURN.
Do we want to not bother calling the plugin for this case?


r~



Re: [Qemu-devel] [PATCH v3 34/50] target/sparc: fetch code with translator_ld

2019-06-17 Thread Richard Henderson
On 6/14/19 10:11 AM, Alex Bennée wrote:
> From: "Emilio G. Cota" 
> 
> Signed-off-by: Emilio G. Cota 
> ---
>  target/sparc/translate.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Reviewed-by: Richard Henderson 


r~




Re: [Qemu-devel] [PATCH v3 35/50] target/xtensa: fetch code with translator_ld

2019-06-17 Thread Richard Henderson
On 6/14/19 10:11 AM, Alex Bennée wrote:
> From: "Emilio G. Cota" 
> 
> Signed-off-by: Emilio G. Cota 
> ---
>  target/xtensa/translate.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)

Reviewed-by: Richard Henderson 


r~




Re: [Qemu-devel] [PATCH v3 33/50] target/riscv: fetch code with translator_ld

2019-06-17 Thread Richard Henderson
On 6/14/19 10:11 AM, Alex Bennée wrote:
> +++ b/target/riscv/translate.c
> @@ -793,7 +793,7 @@ static void riscv_tr_translate_insn(DisasContextBase 
> *dcbase, CPUState *cpu)
>  DisasContext *ctx = container_of(dcbase, DisasContext, base);
>  CPURISCVState *env = cpu->env_ptr;
>  
> -ctx->opcode = cpu_ldl_code(env, ctx->base.pc_next);
> +ctx->opcode = translator_ldl(env, ctx->base.pc_next);

I'll note for the riscv folks that this is an existing bug, reading too much in
the case of an RVC instruction.  This could well matter for the last 2-byte
instruction at the end of a page.

Reviewed-by: Richard Henderson 


r~



Re: [Qemu-devel] [PATCH v3 32/50] target/alpha: fetch code with translator_ld

2019-06-17 Thread Richard Henderson
On 6/14/19 10:11 AM, Alex Bennée wrote:
> From: "Emilio G. Cota" 
> 
> Signed-off-by: Emilio G. Cota 
> ---
>  target/alpha/translate.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Reviewed-by: Richard Henderson 


r~




Re: [Qemu-devel] [PATCH v3 28/50] target/sh4: fetch code with translator_ld

2019-06-17 Thread Richard Henderson
On 6/14/19 10:11 AM, Alex Bennée wrote:
> From: "Emilio G. Cota" 
> 
> There is a small wrinkle with the gUSA instruction. The translator
> effectively treats a (known) gUSA sequence as a single instruction.
> For the purposes of the plugin we end up with a long multi-instruction
> qemu_plugin_insn.
> 
> If the known sequence isn't detected we shall never run this
> translation anyway.
> 
> Signed-off-by: Emilio G. Cota 
> Signed-off-by: Alex Bennée 
> ---
>  target/sh4/translate.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)

Reviewed-by: Richard Henderson 


r~




Re: [Qemu-devel] [PATCH v3 31/50] target/m68k: fetch code with translator_ld

2019-06-17 Thread Richard Henderson
On 6/14/19 10:11 AM, Alex Bennée wrote:
> From: "Emilio G. Cota" 
> 
> Signed-off-by: Emilio G. Cota 
> ---
>  target/m68k/translate.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Reviewed-by: Richard Henderson 


r~




Re: [Qemu-devel] [PATCH v3 29/50] target/i386: fetch code with translator_ld

2019-06-17 Thread Richard Henderson
On 6/14/19 10:11 AM, Alex Bennée wrote:
> From: "Emilio G. Cota" 
> 
> Signed-off-by: Emilio G. Cota 
> ---
>  target/i386/translate.c | 10 +-
>  1 file changed, 5 insertions(+), 5 deletions(-)

Reviewed-by: Richard Henderson 


r~




Re: [Qemu-devel] [PATCH v3 30/50] target/hppa: fetch code with translator_ld

2019-06-17 Thread Richard Henderson
On 6/14/19 10:11 AM, Alex Bennée wrote:
> From: "Emilio G. Cota" 
> 
> Signed-off-by: Emilio G. Cota 
> ---
>  target/hppa/translate.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Reviewed-by: Richard Henderson 


r~




Re: [Qemu-devel] [PATCH v3 27/50] target/ppc: fetch code with translator_ld

2019-06-17 Thread Richard Henderson
On 6/14/19 10:11 AM, Alex Bennée wrote:
> -if (unlikely(need_byteswap(ctx))) {
> -ctx->opcode = bswap32(cpu_ldl_code(env, ctx->base.pc_next));
> -} else {
> -ctx->opcode = cpu_ldl_code(env, ctx->base.pc_next);
> -}
> +ctx->opcode = translator_ldl_swap(env, ctx->base.pc_next,
> +  need_byteswap(ctx));
> +

Reviewed-by: Richard Henderson 


r~



Re: [Qemu-devel] [PATCH v3 26/50] target/arm: call qemu_plugin_insn_append

2019-06-17 Thread Richard Henderson
On 6/14/19 10:11 AM, Alex Bennée wrote:
> From: "Emilio G. Cota" 
> 
> I considered using translator_ld* from arm_ldl_code
> et al. However, note that there's a helper that also calls
> arm_ldl_code, so we'd have to change that caller.

We should in fact change that caller.

Unfortunately, the SVC immediate copied into env->exception.syndrome is
truncated to 16 bits, which means that the aa32 0x123456 won't match.

However, it would be easy enough to create new EXCP_SWI_SEMIHOST, generate it
within translate*.c when the svc immediate matches.  Everywhere except
check_for_semihosting(), we'd treat the two exceptions the same.

The BKPT instruction is only used for semihosting by thumb (and maybe only
v7m?).  The exception syndrome does contain the entire 8-bit immediate, however
for consistency it might be convenient to create an EXCP_BKPT_SEMIHOST so that
all of the checks are always done at translation time.


r~



Re: [Qemu-devel] [PATCH v3 25/50] translator: add translator_ld{ub, sw, uw, l, q}

2019-06-17 Thread Richard Henderson
On 6/14/19 10:11 AM, Alex Bennée wrote:
> +#define GEN_TRANSLATOR_LD(fullname, name, type, swap_fn)\
> +static inline type  \
> +fullname ## _swap(CPUArchState *env, abi_ptr pc, bool do_swap)  \
> +{   \
> +type ret = cpu_ ## name ## _code(env, pc);  \
> +\
> +if (do_swap) {  \
> +ret = swap_fn(ret); \
> +}   \

This feels like we should have done this at a different level.  We already have
lower-level functions that read from memory with the proper endianness.

It seems that we don't have them for *_code, but that could be fixed.  Or,
indeed, bypassed, since these could be the new official interface, deprecating
the *_code functions.


r~



Re: [Qemu-devel] [PATCH v3 24/50] plugin-gen: add plugin_insn_append

2019-06-17 Thread Richard Henderson
On 6/14/19 10:11 AM, Alex Bennée wrote:
> From: "Emilio G. Cota" 
> 
> By adding it to plugin-gen's header file, we can export is as
> an inline, since tcg.h is included in the header (we need tcg_ctx).
> 
> Signed-off-by: Emilio G. Cota 
> 
> ---
> v3
>   - use g_byte_array
> ---
>  accel/tcg/plugin-gen.c| 10 +-
>  include/exec/plugin-gen.h | 23 ++-
>  2 files changed, 23 insertions(+), 10 deletions(-)
> 
> diff --git a/accel/tcg/plugin-gen.c b/accel/tcg/plugin-gen.c
> index 9d9ec29765..758fc5d099 100644
> --- a/accel/tcg/plugin-gen.c
> +++ b/accel/tcg/plugin-gen.c
> @@ -60,9 +60,17 @@
>  /*
>   * plugin_cb_start TCG op args[]:
>   * 0: enum plugin_gen_from
> - * 1: enum plugin_gen_cb (defined below)
> + * 1: enum plugin_gen_cb
>   * 2: set to 1 if it's a mem callback and it's a write, 0 otherwise.
>   */
> +enum plugin_gen_from {
> +PLUGIN_GEN_FROM_TB,
> +PLUGIN_GEN_FROM_INSN,
> +PLUGIN_GEN_FROM_MEM,
> +PLUGIN_GEN_AFTER_INSN,
> +PLUGIN_GEN_N_FROMS,
> +};
> +
>  enum plugin_gen_cb {
>  PLUGIN_GEN_CB_UDATA,
>  PLUGIN_GEN_CB_INLINE,
> diff --git a/include/exec/plugin-gen.h b/include/exec/plugin-gen.h
> index 449ea16034..316638c736 100644
> --- a/include/exec/plugin-gen.h
> +++ b/include/exec/plugin-gen.h
> @@ -15,15 +15,6 @@
>  #include "qemu/plugin.h"
>  #include "tcg/tcg.h"
>  
> -/* used by plugin_callback_start and plugin_callback_end TCG ops */
> -enum plugin_gen_from {
> -PLUGIN_GEN_FROM_TB,
> -PLUGIN_GEN_FROM_INSN,
> -PLUGIN_GEN_FROM_MEM,
> -PLUGIN_GEN_AFTER_INSN,
> -PLUGIN_GEN_N_FROMS,
> -};

Why is this movement in here, and can it be folded back?
It doesn't seem to be used from ...


> -
>  struct DisasContextBase;
>  
>  #ifdef CONFIG_PLUGIN
> @@ -36,6 +27,17 @@ void plugin_gen_insn_end(void);
>  void plugin_gen_disable_mem_helpers(void);
>  void plugin_gen_empty_mem_callback(TCGv addr, uint8_t info);
>  
> +static inline void plugin_insn_append(const void *from, size_t size)
> +{
> +struct qemu_plugin_insn *insn = tcg_ctx->plugin_insn;
> +
> +if (insn == NULL) {
> +return;
> +}
> +
> +insn->data = g_byte_array_append(insn->data, from, size);
> +}
> +
>  #else /* !CONFIG_PLUGIN */
>  
>  static inline
> @@ -60,6 +62,9 @@ static inline void plugin_gen_disable_mem_helpers(void)
>  static inline void plugin_gen_empty_mem_callback(TCGv addr, uint8_t info)
>  { }
>  
> +static inline void plugin_insn_append(const void *from, size_t size)
> +{ }
> +

... here.


r~



Re: [Qemu-devel] [PATCH v3 23/50] cpu: hook plugin vcpu events

2019-06-17 Thread Richard Henderson
On 6/14/19 10:11 AM, Alex Bennée wrote:
>  static void qemu_wait_io_event(CPUState *cpu)
>  {
> +bool slept = false;
> +
>  while (cpu_thread_is_idle(cpu)) {
> +if (!slept) {
> +slept = true;
> +qemu_plugin_vcpu_idle_cb(cpu);
> +}
>  qemu_cond_wait(cpu->halt_cond, _global_mutex);
>  }
> +if (slept) {
> +qemu_plugin_vcpu_resume_cb(cpu);
> +}

Maybe better without the variable.

if (cpu_thread_is_idle(cpu)) {
qemu_plugin_vcpu_idle_cb(cpu);
do {
qemu_cond_wait(cpu->halt_cond, _global_mutex);
} while (cpu_thread_is_idle(cpu);
qemu_plugin_vcpu_resume_cb(cpu);
}

Otherwise,
Reviewed-by: Richard Henderson 


r~



Re: [Qemu-devel] [PATCH v3 22/50] *-user: plugin syscalls

2019-06-17 Thread Richard Henderson
On 6/14/19 10:11 AM, Alex Bennée wrote:
> From: "Emilio G. Cota" 
> 
> Signed-off-by: Emilio G. Cota 
> ---
>  bsd-user/syscall.c   | 9 +
>  linux-user/syscall.c | 3 +++
>  2 files changed, 12 insertions(+)

Reviewed-by: Richard Henderson 


r~



Re: [Qemu-devel] [PATCH v3 21/50] *-user: notify plugin of exit

2019-06-17 Thread Richard Henderson
On 6/14/19 10:11 AM, Alex Bennée wrote:
> From: "Emilio G. Cota" 
> 
> Reviewed-by: Alex Bennée 
> Signed-off-by: Emilio G. Cota 
> ---
>  bsd-user/syscall.c | 3 +++
>  linux-user/exit.c  | 1 +
>  2 files changed, 4 insertions(+)

Reviewed-by: Richard Henderson 


r~



Re: [Qemu-devel] [PATCH v3 20/50] translate-all: notify plugin code of tb_flush

2019-06-17 Thread Richard Henderson
On 6/14/19 10:11 AM, Alex Bennée wrote:
> From: "Emilio G. Cota" 
> 
> Plugins might allocate per-TB data that then they get passed each
> time a TB is executed (via the *userdata pointer).
> 
> Notify plugin code every time a code cache flush occurs, so
> that plugins can then reclaim the memory of the per-TB data.
> 
> Reviewed-by: Alex Bennée 
> Signed-off-by: Emilio G. Cota 
> ---
>  accel/tcg/translate-all.c | 6 ++
>  1 file changed, 6 insertions(+)

Reviewed-by: Richard Henderson 


r~



Re: [Qemu-devel] [PATCH v3 19/50] tcg: let plugins instrument memory accesses

2019-06-17 Thread Richard Henderson
On 6/14/19 10:11 AM, Alex Bennée wrote:
> +static inline void set_hostaddr(CPUArchState *env, TCGMemOp mo, void *haddr)
> +{
> +#ifdef CONFIG_PLUGIN
> +if (mo & MO_HADDR) {
> +env_tlb(env)->c.hostaddr = haddr;
> +}
> +#endif
> +}
> +

Even if we weren't talking about recomputing this in the helper, would an
unconditional store be cheaper?


r~



Re: [Qemu-devel] [PATCH v3 18/50] cpu_ldst_useronly_template: remove redundant #ifndef CODE_ACCESS

2019-06-17 Thread Richard Henderson
On 6/14/19 10:11 AM, Alex Bennée wrote:
> From: "Emilio G. Cota" 
> 
> This function is already under #ifndef CODE_ACCESS.
> 
> Signed-off-by: Emilio G. Cota 
> ---
>  include/exec/cpu_ldst_useronly_template.h | 2 --
>  1 file changed, 2 deletions(-)

Reviewed-by: Richard Henderson 


r~



Re: [Qemu-devel] [PATCH v3 17/50] atomic_template: add inline trace/plugin helpers

2019-06-17 Thread Richard Henderson
On 6/14/19 10:11 AM, Alex Bennée wrote:
> +#ifndef ATOMIC_TEMPLATE_COMMON
> +#define ATOMIC_TEMPLATE_COMMON
> +static inline
> +void atomic_trace_rmw_pre(CPUArchState *env, target_ulong addr, uint8_t info)
> +{
> +CPUState *cpu = env_cpu(env);
> +
> +trace_guest_mem_before_exec(cpu, addr, info);
> +trace_guest_mem_before_exec(cpu, addr, info | TRACE_MEM_ST);
> +}
> +
> +static inline void atomic_trace_rmw_post(CPUArchState *env, target_ulong 
> addr,
> + void *haddr, uint8_t info)
> +{
> +}
> +
> +static inline
> +void atomic_trace_ld_pre(CPUArchState *env, target_ulong addr, uint8_t info)
> +{
> +trace_guest_mem_before_exec(env_cpu(env), addr, info);
> +}
> +
> +static inline void atomic_trace_ld_post(CPUArchState *env, target_ulong addr,
> +void *haddr, uint8_t info)
> +{
> +}
> +
> +static inline
> +void atomic_trace_st_pre(CPUArchState *env, target_ulong addr, uint8_t info)
> +{
> +trace_guest_mem_before_exec(env_cpu(env), addr, info);
> +}
> +
> +static inline void atomic_trace_st_post(CPUArchState *env, target_ulong addr,
> +void *haddr, uint8_t info)
> +{
> +}
> +#endif /* ATOMIC_TEMPLATE_COMMON */
>  

All of this should just go into atomic_common.inc.c.


r~



Re: [Qemu-devel] [PATCH v3 15/50] tcg: add MO_HADDR to TCGMemOp

2019-06-17 Thread Richard Henderson
On 6/14/19 10:11 AM, Alex Bennée wrote:
> +/*
> + * SoftMMU-only: if set, the TCG backend puts the corresponding host 
> address
> + * in CPUArchState.hostaddr.
> + */
> +MO_HSHIFT = MO_ASHIFT + 3,
> +MO_HADDR = 1 << MO_HSHIFT,

FYI, Alex and I talked about recomputing the host address within the helper.

For at least a few of the hosts, we currently never compute the full host
address into a single register -- we use reg+reg addressing when possible.
It's only a couple of instructions to re-compute, given that we know that the
tlb lookup succeeded, and importantly they are all out of line and not bloating
the inline code further.


r~



Re: [Qemu-devel] [PATCH v3 16/50] atomic_template: fix indentation in GEN_ATOMIC_HELPER

2019-06-17 Thread Richard Henderson
On 6/14/19 10:11 AM, Alex Bennée wrote:
> From: "Emilio G. Cota" 
> 
> Reviewed-by: Alex Bennée 
> Signed-off-by: Emilio G. Cota 
> ---
>  accel/tcg/atomic_template.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Reviewed-by: Richard Henderson 


r~



Re: [Qemu-devel] [PATCH v3 14/50] tcg: add tcg_gen_st_ptr

2019-06-17 Thread Richard Henderson
On 6/14/19 10:11 AM, Alex Bennée wrote:
> From: "Emilio G. Cota" 
> 
> Will gain a user soon.
> 
> Signed-off-by: Emilio G. Cota 
> ---
>  tcg/tcg-op.h | 5 +
>  1 file changed, 5 insertions(+)

Reviewed-by: Richard Henderson 


r~



Re: [Qemu-devel] [PULL 00/16] Monitor patches for 2019-06-17

2019-06-17 Thread no-reply
Patchew URL: https://patchew.org/QEMU/20190617184903.19436-1-arm...@redhat.com/



Hi,

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

Subject: [Qemu-devel] [PULL 00/16] Monitor patches for 2019-06-17
Type: series
Message-id: 20190617184903.19436-1-arm...@redhat.com

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

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
From https://github.com/patchew-project/qemu
 - [tag update]  patchew/20190617175317.27557-1-peter.mayd...@linaro.org -> 
patchew/20190617175317.27557-1-peter.mayd...@linaro.org
Switched to a new branch 'test'
10cbb82 vl: Deprecate -mon pretty=... for HMP monitors
c8d6357 monitor: Replace monitor_init() with monitor_init_{hmp, qmp}()
add28d5 monitor: Split Monitor.flags into separate bools
61f535a monitor: Split out monitor/monitor.c
1f84df4 monitor: Split out monitor/hmp.c
7b0f436 monitor: Split out monitor/qmp.c
62fff9a monitor: Create monitor-internal.h with common definitions
0be87c8 monitor: Move {hmp, qmp}.c to monitor/{hmp, qmp}-cmds.c
9dc5b85 Move monitor.c to monitor/misc.c
32b2352 monitor: Rename HMP command type and tables
6259cf7 monitor: Remove Monitor.cmd_table indirection
4cc8716 monitor: Create MonitorHMP with readline state
e28b9ba monitor: Make MonitorQMP a child class of Monitor
0032c2c monitor: Split monitor_init in HMP and QMP function
8ef4253 monitor: Remove unused password prompting fields
f21701f monitor: Fix return type of monitor_fdset_dup_fd_find

=== OUTPUT BEGIN ===
1/16 Checking commit f21701f2afc2 (monitor: Fix return type of 
monitor_fdset_dup_fd_find)
2/16 Checking commit 8ef4253a4478 (monitor: Remove unused password prompting 
fields)
3/16 Checking commit 0032c2cf455d (monitor: Split monitor_init in HMP and QMP 
function)
4/16 Checking commit e28b9ba8e716 (monitor: Make MonitorQMP a child class of 
Monitor)
5/16 Checking commit 4cc8716de54a (monitor: Create MonitorHMP with readline 
state)
6/16 Checking commit 6259cf7f6c52 (monitor: Remove Monitor.cmd_table 
indirection)
7/16 Checking commit 32b235292bf6 (monitor: Rename HMP command type and tables)
8/16 Checking commit 9dc5b85526e4 (Move monitor.c to monitor/misc.c)
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#82: 
new file mode 100644

total: 0 errors, 1 warnings, 78 lines checked

Patch 8/16 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
9/16 Checking commit 0be87c866744 (monitor: Move {hmp, qmp}.c to monitor/{hmp, 
qmp}-cmds.c)
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#106: 
rename from hmp.c

total: 0 errors, 1 warnings, 73 lines checked

Patch 9/16 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
10/16 Checking commit 62fff9aeab3c (monitor: Create monitor-internal.h with 
common definitions)
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#227: 
new file mode 100644

total: 0 errors, 1 warnings, 326 lines checked

Patch 10/16 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
11/16 Checking commit 7b0f43601c81 (monitor: Split out monitor/qmp.c)
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#628: 
new file mode 100644

total: 0 errors, 1 warnings, 984 lines checked

Patch 11/16 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
12/16 Checking commit 1f84df448bda (monitor: Split out monitor/hmp.c)
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#33: 
new file mode 100644

ERROR: consider using qemu_strtoull in preference to strtoull
#436: FILE: monitor/hmp.c:399:
+n = strtoull(pch, , 0);

total: 1 errors, 1 warnings, 2937 lines checked

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

13/16 Checking commit 61f535a02231 (monitor: Split out monitor/monitor.c)
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#735: 
new file mode 100644

WARNING: Block comments use a leading /* on a separate line
#1370: FILE: monitor/monitor.c:631:
+{ /* end of list */ }

total: 0 errors, 2 warnings, 1317 lines checked

Patch 13/16 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
14/16 Checking commit add28d51ab77 (monitor: Split Monitor.flags into separate 
bools)
15/16 Checking 

Re: [Qemu-devel] [PATCH 6/6] target/arm: Execute Thumb instructions when their condbits are 0xf

2019-06-17 Thread Richard Henderson
On 6/17/19 10:53 AM, Peter Maydell wrote:
> Thumb instructions in an IT block are set up to be conditionally
> executed depending on a set of condition bits encoded into the IT
> bits of the CPSR/XPSR.  The architecture specifies that if the
> condition bits are 0b this means "always execute" (like 0b1110),
> not "never execute"; we were treating it as "never execute".  (See
> the ConditionHolds() pseudocode in both the A-profile and M-profile
> Arm ARM.)
> 
> This is a bit of an obscure corner case, because the only legal
> way to get to an 0b set of condbits is to do an exception
> return which sets the XPSR/CPSR up that way. An IT instruction
> which encodes a condition sequence that would include an 0b is
> UNPREDICTABLE, and for v8A the CONSTRAINED UNPREDICTABLE choices
> for such an IT insn are to NOP, UNDEF, or treat 0b like 0b1110.
> Add a comment noting that we take the latter option.
> 
> Signed-off-by: Peter Maydell 
> ---
>  target/arm/translate.c | 15 +--
>  1 file changed, 13 insertions(+), 2 deletions(-)

Reviewed-by: Richard Henderson 


r~



Re: [Qemu-devel] [PATCH 5/6] hw/timer/armv7m_systick: Forbid non-privileged accesses

2019-06-17 Thread Richard Henderson
On 6/17/19 10:53 AM, Peter Maydell wrote:
> Like most of the v7M memory mapped system registers, the systick
> registers are accessible to privileged code only and user accesses
> must generate a BusFault. We implement that for registers in
> the NVIC proper already, but missed it for systick since we
> implement it as a separate device. Correct the omission.
> 
> Signed-off-by: Peter Maydell 
> ---
>  hw/timer/armv7m_systick.c | 26 --
>  1 file changed, 20 insertions(+), 6 deletions(-)

Reviewed-by: Richard Henderson 


r~



Re: [Qemu-devel] [PATCH 4/6] target/arm: Use _ra versions of cpu_stl_data() in v7M helpers

2019-06-17 Thread Richard Henderson
On 6/17/19 10:53 AM, Peter Maydell wrote:
> In the various helper functions for v7M/v8M instructions, use
> the _ra versions of cpu_stl_data() and friends. Otherwise we
> may get wrong behaviour or an assert() due to not being able
> to locate the TB if there is an exception on the memory access
> or if it performs an IO operation when in icount mode.
> 
> Signed-off-by: Peter Maydell 
> ---
>  target/arm/helper.c | 21 -
>  1 file changed, 12 insertions(+), 9 deletions(-)

Reviewed-by: Richard Henderson 


r~



Re: [Qemu-devel] [PATCH 3/6] target/arm: v8M: Check state of exception being returned from

2019-06-17 Thread Richard Henderson
On 6/17/19 10:53 AM, Peter Maydell wrote:
> In v8M, an attempt to return from an exception which is not
> active is an illegal exception return. For this purpose,
> exceptions which can configurably target either Secure or
> NonSecure are not considered to be active if they are
> configured for the opposite security state for the one
> we're trying to return from (eg attempt to return from
> an NS NMI but NMI targets Secure). In the pseudocode this
> is handled by IsActiveForState().
> 
> Detect this case rather than counting an active exception
> possibly of the wrong security state as being sufficient.
> 
> Signed-off-by: Peter Maydell 
> ---
>  hw/intc/armv7m_nvic.c | 14 +-
>  1 file changed, 13 insertions(+), 1 deletion(-)

Reviewed-by: Richard Henderson 


r~



Re: [Qemu-devel] [RFC PATCH v2 24/35] multi-process: add qdev_proxy_add to create proxy devices

2019-06-17 Thread Elena Ufimtseva
On Mon, Jun 17, 2019 at 02:22:08PM -0500, Eric Blake wrote:
> On 6/17/19 1:16 PM, elena.ufimts...@oracle.com wrote:
> > From: Elena Ufimtseva 
> > 
> > This is handled while parsing the command line options.
> > The parsed options are being sent to remote process
> > as the messgaes containing JSON strings.
> 
> s/messgaes/messages/
> 
> > 
> > Changes in v2:
> >  - parse socket and command suboptions of drive/device commands;
> 
> The changelog paragraph belongs...
> 
> > 
> > Signed-off-by: Jagannathan Raman 
> > Signed-off-by: John G Johnson 
> > Signed-off-by: Elena Ufimtseva 
> > ---
> 
> ...here, after the --- divider. It is useful to reviewers, but gets
> stripped by 'git am' when the maintainer applies the series. A year from
> now, we won't care how many iterations the patches went through on list,
> only the one version that got committed to git.
>

That one slipped trough with this, will fix.

> >  include/hw/proxy/qemu-proxy.h |   7 ++
> >  include/monitor/qdev.h|   5 +
> >  qdev-monitor.c| 166 ++
> >  3 files changed, 178 insertions(+)
> > 
> 
> 
> -- 
> Eric Blake, Principal Software Engineer
> Red Hat, Inc.   +1-919-301-3226
> Virtualization:  qemu.org | libvirt.org
> 






Re: [Qemu-devel] [RFC PATCH v2 01/35] multi-process: memory: alloc RAM from file at offset

2019-06-17 Thread Elena Ufimtseva
On Mon, Jun 17, 2019 at 02:14:49PM -0500, Eric Blake wrote:
> On 6/17/19 1:14 PM, elena.ufimts...@oracle.com wrote:
> > From: Jagannathan Raman 
> > 
> > Allow RAM MemoryRegion to be created from an offset in a file, instead
> > of allocating at offset of 0 by default. This is needed to synchronize
> > RAM between QEMU & remote process.
> > This will be needed for the following patches.
> 
> This message and the rest of the series was sent unthreaded (no
> References: or In-Reply-To: headers), which makes it very difficult to
> track. You'll want to fix your sending environment to ensure that
> threading is preserved correctly.
>

Hi Eric

Yes, my bad. I have adjusted my scripts.

Elena
> > 
> > Signed-off-by: Jagannathan Raman 
> > Signed-off-by: John G Johnson 
> > Signed-off-by: Elena Ufimtseva 
> > ---
> 
> 
> -- 
> Eric Blake, Principal Software Engineer
> Red Hat, Inc.   +1-919-301-3226
> Virtualization:  qemu.org | libvirt.org
> 






Re: [Qemu-devel] [PATCH 2/6] arm v8M: Forcibly clear negative-priority exceptions on deactivate

2019-06-17 Thread Richard Henderson
On 6/17/19 10:53 AM, Peter Maydell wrote:
> To prevent execution priority remaining negative if the guest
> returns from an NMI or HardFault with a corrupted IPSR, the
> v8M interrupt deactivation process forces the HardFault and NMI
> to inactive based on the current raw execution priority,
> even if the interrupt the guest is trying to deactivate
> is something else. In the pseudocode this is done in the
> Deactivate() function.
> 
> Signed-off-by: Peter Maydell 
> ---
>  hw/intc/armv7m_nvic.c | 40 +++-
>  1 file changed, 35 insertions(+), 5 deletions(-)

Reviewed-by: Richard Henderson 


r~



Re: [Qemu-devel] [PATCH 1/6] target/arm: NS BusFault on vector table fetch escalates to NS HardFault

2019-06-17 Thread Richard Henderson
On 6/17/19 10:53 AM, Peter Maydell wrote:
> + * The HardFault is Secure if BFHFNMINS is 0 (meaning that all HFs are
> + * secure); otherwise it targets the same security state as the
> + * underlying exception.
>   */
> -exc_secure = targets_secure ||
> -!(cpu->env.v7m.aircr & R_V7M_AIRCR_BFHFNMINS_MASK);
> +if (!(cpu->env.v7m.aircr & R_V7M_AIRCR_BFHFNMINS_MASK)) {
> +exc_secure = false;
> +}

exc_secure = true, surely?


r~



Re: [Qemu-devel] [RFC PATCH v2 01/35] multi-process: memory: alloc RAM from file at offset

2019-06-17 Thread Eric Blake
On 6/17/19 1:14 PM, elena.ufimts...@oracle.com wrote:
> From: Jagannathan Raman 
> 
> Allow RAM MemoryRegion to be created from an offset in a file, instead
> of allocating at offset of 0 by default. This is needed to synchronize
> RAM between QEMU & remote process.
> This will be needed for the following patches.

This message and the rest of the series was sent unthreaded (no
References: or In-Reply-To: headers), which makes it very difficult to
track. You'll want to fix your sending environment to ensure that
threading is preserved correctly.

> 
> Signed-off-by: Jagannathan Raman 
> Signed-off-by: John G Johnson 
> Signed-off-by: Elena Ufimtseva 
> ---


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



signature.asc
Description: OpenPGP digital signature


Re: [Qemu-devel] [PATCH 0/3] tricore: Convert to translate_loop

2019-06-17 Thread no-reply
Patchew URL: 
https://patchew.org/QEMU/20190617143533.15013-1-kbast...@mail.uni-paderborn.de/



Hi,

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

Message-id: 20190617143533.15013-1-kbast...@mail.uni-paderborn.de
Type: series
Subject: [Qemu-devel] [PATCH 0/3] tricore: Convert to translate_loop

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

From https://github.com/patchew-project/qemu
 * [new tag] 
patchew/20190617143533.15013-1-kbast...@mail.uni-paderborn.de -> 
patchew/20190617143533.15013-1-kbast...@mail.uni-paderborn.de
Switched to a new branch 'test'
3795cef target/tricore: Use translate_loop
d50dce9 target-tricore: Make env a member of DisasContext
6d30fd1 target/tricore: Use DisasContextBase API

=== OUTPUT BEGIN ===
1/3 Checking commit 6d30fd14d6c8 (target/tricore: Use DisasContextBase API)
2/3 Checking commit d50dce928c26 (target-tricore: Make env a member of 
DisasContext)
ERROR: spaces required around that '+' (ctx:VxV)
#660: FILE: target/tricore/translate.c:6586:
+gen_dvinit_b(ctx, cpu_gpr_d[r3], cpu_gpr_d[r3+1], cpu_gpr_d[r1],
  ^

ERROR: spaces required around that '+' (ctx:VxV)
#678: FILE: target/tricore/translate.c:6619:
+gen_dvinit_h(ctx, cpu_gpr_d[r3], cpu_gpr_d[r3+1], cpu_gpr_d[r1],
  ^

total: 2 errors, 0 warnings, 1154 lines checked

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

3/3 Checking commit 3795cefb9ed0 (target/tricore: Use translate_loop)
=== OUTPUT END ===

Test command exited with code: 1


The full log is available at
http://patchew.org/logs/20190617143533.15013-1-kbast...@mail.uni-paderborn.de/testing.checkpatch/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-de...@redhat.com

Re: [Qemu-devel] [PATCH] target/arm: Check for dp support for dp VFM, not sp

2019-06-17 Thread Richard Henderson
On 6/17/19 9:01 AM, Peter Maydell wrote:
> In commit 1120827fa182f0e7622 we accidentally put the
> "UNDEF unless FPU has double-precision support" check in
> the single-precision VFM function. Put it in the dp
> function where it belongs.
> 
> Signed-off-by: Peter Maydell 
> ---
> I just merged the pullreq with 1120827fa182f0e7622 an hour
> ago and then I spotted this bug in it...
> 
>  target/arm/translate-vfp.inc.c | 8 
>  1 file changed, 4 insertions(+), 4 deletions(-)

Oops.

Reviewed-by: Richard Henderson 


r~



Re: [Qemu-devel] [RFC PATCH v2 24/35] multi-process: add qdev_proxy_add to create proxy devices

2019-06-17 Thread Eric Blake
On 6/17/19 1:16 PM, elena.ufimts...@oracle.com wrote:
> From: Elena Ufimtseva 
> 
> This is handled while parsing the command line options.
> The parsed options are being sent to remote process
> as the messgaes containing JSON strings.

s/messgaes/messages/

> 
> Changes in v2:
>  - parse socket and command suboptions of drive/device commands;

The changelog paragraph belongs...

> 
> Signed-off-by: Jagannathan Raman 
> Signed-off-by: John G Johnson 
> Signed-off-by: Elena Ufimtseva 
> ---

...here, after the --- divider. It is useful to reviewers, but gets
stripped by 'git am' when the maintainer applies the series. A year from
now, we won't care how many iterations the patches went through on list,
only the one version that got committed to git.

>  include/hw/proxy/qemu-proxy.h |   7 ++
>  include/monitor/qdev.h|   5 +
>  qdev-monitor.c| 166 ++
>  3 files changed, 178 insertions(+)
> 


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



signature.asc
Description: OpenPGP digital signature


[Qemu-devel] [PULL 16/16] vl: Deprecate -mon pretty=... for HMP monitors

2019-06-17 Thread Markus Armbruster
From: Kevin Wolf 

The -mon pretty=on|off switch of the -mon option applies only to QMP
monitors. It's silently ignored for HMP. Deprecate this combination so
that we can make it an error in future versions.

Signed-off-by: Kevin Wolf 
Message-Id: <20190613153405.24769-16-kw...@redhat.com>
Reviewed-by: Markus Armbruster 
[Commit message tweaked]
Signed-off-by: Markus Armbruster 
---
 qemu-deprecated.texi |  6 ++
 vl.c | 10 +-
 2 files changed, 15 insertions(+), 1 deletion(-)

diff --git a/qemu-deprecated.texi b/qemu-deprecated.texi
index 50292d820b..df04f2840b 100644
--- a/qemu-deprecated.texi
+++ b/qemu-deprecated.texi
@@ -72,6 +72,12 @@ backend settings instead of environment variables.  To ease 
migration to
 the new format, the ``-audiodev-help'' option can be used to convert
 the current values of the environment variables to ``-audiodev'' options.
 
+@subsection -mon ...,control=readline,pretty=on|off (since 4.1)
+
+The @code{pretty=on|off} switch has no effect for HMP monitors, but is
+silently ignored. Using the switch with HMP monitors will become an
+error in the future.
+
 @subsection -realtime (since 4.1)
 
 The @code{-realtime mlock=on|off} argument has been replaced by the
diff --git a/vl.c b/vl.c
index 32daa434eb..99a56b5556 100644
--- a/vl.c
+++ b/vl.c
@@ -2317,6 +2317,10 @@ static int mon_init_func(void *opaque, QemuOpts *opts, 
Error **errp)
 return -1;
 }
 
+if (!qmp && qemu_opt_get(opts, "pretty")) {
+warn_report("'pretty' is deprecated for HMP monitors, it has no effect 
"
+"and will be removed in future versions");
+}
 if (qemu_opt_get_bool(opts, "pretty", 0)) {
 pretty = true;
 }
@@ -2362,7 +2366,11 @@ static void monitor_parse(const char *optarg, const char 
*mode, bool pretty)
 opts = qemu_opts_create(qemu_find_opts("mon"), label, 1, _fatal);
 qemu_opt_set(opts, "mode", mode, _abort);
 qemu_opt_set(opts, "chardev", label, _abort);
-qemu_opt_set_bool(opts, "pretty", pretty, _abort);
+if (!strcmp(mode, "control")) {
+qemu_opt_set_bool(opts, "pretty", pretty, _abort);
+} else {
+assert(pretty == false);
+}
 monitor_device_index++;
 }
 
-- 
2.21.0




[Qemu-devel] [PULL 15/16] monitor: Replace monitor_init() with monitor_init_{hmp, qmp}()

2019-06-17 Thread Markus Armbruster
From: Kevin Wolf 

Most callers know which monitor type they want to have. Instead of
calling monitor_init() with flags that can describe both types of
monitors, make monitor_init_{hmp,qmp}() public interfaces that take
specific bools instead of flags and call these functions directly.

Signed-off-by: Kevin Wolf 
Message-Id: <20190613153405.24769-15-kw...@redhat.com>
Reviewed-by: Markus Armbruster 
Signed-off-by: Markus Armbruster 
---
 chardev/char.c |  2 +-
 gdbstub.c  |  2 +-
 include/monitor/monitor.h  |  9 ++---
 monitor/hmp.c  |  4 ++--
 monitor/monitor-internal.h |  3 ---
 monitor/monitor.c  |  9 -
 monitor/qmp.c  |  7 ++-
 stubs/monitor.c|  6 +-
 tests/test-util-sockets.c  |  3 ++-
 vl.c   | 18 --
 10 files changed, 27 insertions(+), 36 deletions(-)

diff --git a/chardev/char.c b/chardev/char.c
index e4887bcc82..7b6b2cb123 100644
--- a/chardev/char.c
+++ b/chardev/char.c
@@ -731,7 +731,7 @@ Chardev *qemu_chr_new_noreplay(const char *label, const 
char *filename,
 
 if (qemu_opt_get_bool(opts, "mux", 0)) {
 assert(permit_mux_mon);
-monitor_init(chr, MONITOR_USE_READLINE);
+monitor_init_hmp(chr, true);
 }
 
 out:
diff --git a/gdbstub.c b/gdbstub.c
index d614a1f3c0..8618e34311 100644
--- a/gdbstub.c
+++ b/gdbstub.c
@@ -3344,7 +3344,7 @@ int gdbserver_start(const char *device)
 /* Initialize a monitor terminal for gdb */
 mon_chr = qemu_chardev_new(NULL, TYPE_CHARDEV_GDB,
NULL, NULL, _abort);
-monitor_init(mon_chr, 0);
+monitor_init_hmp(mon_chr, false);
 } else {
 qemu_chr_fe_deinit(>chr, true);
 mon_chr = s->mon_chr;
diff --git a/include/monitor/monitor.h b/include/monitor/monitor.h
index 44ac43df34..a81eeff5f8 100644
--- a/include/monitor/monitor.h
+++ b/include/monitor/monitor.h
@@ -8,19 +8,14 @@
 extern __thread Monitor *cur_mon;
 typedef struct MonitorHMP MonitorHMP;
 
-/* flags for monitor_init */
-/* 0x01 unused */
-#define MONITOR_USE_READLINE  0x02
-#define MONITOR_USE_CONTROL   0x04
-#define MONITOR_USE_PRETTY0x08
-
 #define QMP_REQ_QUEUE_LEN_MAX 8
 
 bool monitor_cur_is_qmp(void);
 
 void monitor_init_globals(void);
 void monitor_init_globals_core(void);
-void monitor_init(Chardev *chr, int flags);
+void monitor_init_qmp(Chardev *chr, bool pretty);
+void monitor_init_hmp(Chardev *chr, bool use_readline);
 void monitor_cleanup(void);
 
 int monitor_suspend(Monitor *mon);
diff --git a/monitor/hmp.c b/monitor/hmp.c
index 07f827c90c..5349a81307 100644
--- a/monitor/hmp.c
+++ b/monitor/hmp.c
@@ -1394,14 +1394,14 @@ static void monitor_readline_flush(void *opaque)
 monitor_flush(>common);
 }
 
-void monitor_init_hmp(Chardev *chr, int flags)
+void monitor_init_hmp(Chardev *chr, bool use_readline)
 {
 MonitorHMP *mon = g_new0(MonitorHMP, 1);
 
 monitor_data_init(>common, false, false, false);
 qemu_chr_fe_init(>common.chr, chr, _abort);
 
-mon->use_readline = flags & MONITOR_USE_READLINE;
+mon->use_readline = use_readline;
 if (mon->use_readline) {
 mon->rs = readline_init(monitor_readline_printf,
 monitor_readline_flush,
diff --git a/monitor/monitor-internal.h b/monitor/monitor-internal.h
index 03ea0239ef..7760b22ba3 100644
--- a/monitor/monitor-internal.h
+++ b/monitor/monitor-internal.h
@@ -163,9 +163,6 @@ extern int mon_refcount;
 
 extern HMPCommand hmp_cmds[];
 
-void monitor_init_qmp(Chardev *chr, int flags);
-void monitor_init_hmp(Chardev *chr, int flags);
-
 int monitor_puts(Monitor *mon, const char *str);
 void monitor_data_init(Monitor *mon, bool is_qmp, bool skip_flush,
bool use_io_thread);
diff --git a/monitor/monitor.c b/monitor/monitor.c
index 3f4808240a..3ef28171c0 100644
--- a/monitor/monitor.c
+++ b/monitor/monitor.c
@@ -551,15 +551,6 @@ void monitor_data_destroy(Monitor *mon)
 qemu_mutex_destroy(>mon_lock);
 }
 
-void monitor_init(Chardev *chr, int flags)
-{
-if (flags & MONITOR_USE_CONTROL) {
-monitor_init_qmp(chr, flags);
-} else {
-monitor_init_hmp(chr, flags);
-}
-}
-
 void monitor_cleanup(void)
 {
 /*
diff --git a/monitor/qmp.c b/monitor/qmp.c
index 940649f688..e1b196217d 100644
--- a/monitor/qmp.c
+++ b/monitor/qmp.c
@@ -364,18 +364,15 @@ static void monitor_qmp_setup_handlers_bh(void *opaque)
 monitor_list_append(>common);
 }
 
-void monitor_init_qmp(Chardev *chr, int flags)
+void monitor_init_qmp(Chardev *chr, bool pretty)
 {
 MonitorQMP *mon = g_new0(MonitorQMP, 1);
 
-/* Only HMP supports readline */
-assert(!(flags & MONITOR_USE_READLINE));
-
 /* Note: we run QMP monitor in I/O thread when @chr supports that */
 monitor_data_init(>common, true, false,
   qemu_chr_has_feature(chr, QEMU_CHAR_FEATURE_GCONTEXT));
 
-mon->pretty = flags & 

Re: [Qemu-devel] [RFC PATCH v2 20/35] multi-process: Add QMP & extend HMP commands to list remote info

2019-06-17 Thread Eric Blake
On 6/17/19 1:16 PM, elena.ufimts...@oracle.com wrote:
> From: Jagannathan Raman 
> 
> Add query-remote QMP command and extend "info" HMP command, to list
> the remote objects used by QEMU.
> 
> Signed-off-by: Jagannathan Raman 
> Signed-off-by: John G Johnson 
> Signed-off-by: Elena Ufimtseva 
> ---

> +++ b/qapi/block-core.json
> @@ -673,6 +673,23 @@
> '*tray_open': 'bool', '*io-status': 'BlockDeviceIoStatus',
> '*dirty-bitmaps': ['BlockDirtyInfo'] } }
>  
> +##
> +# @RemoteProc:

Unless there's a compelling reason to abbreviate, naming this
'RemoteProcess' is just fine.

> +#
> +# Remote process information.
> +#
> +# @id: Device ID
> +#
> +# @pid: Linux Process ID

Is this information only available for Linux, or is it a generic pid
appropriate to any operating system?  I'm wondering if you can just
s/Linux//.

> +#
> +# @proc: Process name

Again, no need to abbreviate, if @process or @name would be easier to
document.

> +#
> +# Since:  3.0.93

No such release. The next release will be 4.1.

> +##
> +{ 'struct': 'RemoteProc',
> +  'data': {'id': 'str', 'pid': 'int32', 'proc': 'str' },
> +  'if': 'defined(CONFIG_MPQEMU)' }
> +
>  ##
>  # @BlockMeasureInfo:
>  #
> @@ -795,6 +812,18 @@
>  ##
>  { 'command': 'query-block', 'returns': ['BlockInfo'] }
>  
> +##
> +# @query-remote:
> +#
> +# Get a list of all the remote processes spawned by QEMU.
> +#
> +# Returns: a list of @RemoteProc describing each remote process.
> +#
> +# Since: 3.0.93

4.1

> +#
> +##
> +{ 'command': 'query-remote', 'returns': ['RemoteProc'],
> +  'if': 'defined(CONFIG_MPQEMU)' }
>  
>  ##
>  # @BlockDeviceTimedStats:
> 

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



signature.asc
Description: OpenPGP digital signature


[Qemu-devel] [PULL 11/16] monitor: Split out monitor/qmp.c

2019-06-17 Thread Markus Armbruster
From: Kevin Wolf 

Move QMP infrastructure from monitor/misc.c to monitor/qmp.c. This is
code that can be shared for all targets, so compile it only once.

The amount of function and particularly extern variables in
monitor_int.h is probably a bit larger than it needs to be, but this way
no non-trivial code modifications are needed. The interfaces between QMP
and the monitor core can be cleaned up later.

Signed-off-by: Kevin Wolf 
Reviewed-by: Dr. David Alan Gilbert 
Message-Id: <20190613153405.24769-11-kw...@redhat.com>
Reviewed-by: Markus Armbruster 
[monitor_is_qmp() tidied up to make checkpatch.pl happy,
superfluous #include dropped]
Signed-off-by: Markus Armbruster 
---
 Makefile.objs  |   1 +
 monitor/Makefile.objs  |   1 +
 monitor/misc.c | 401 +---
 monitor/monitor-internal.h |  30 +++
 monitor/qmp.c  | 406 +
 monitor/trace-events   |   4 +-
 6 files changed, 450 insertions(+), 393 deletions(-)
 create mode 100644 monitor/qmp.c

diff --git a/Makefile.objs b/Makefile.objs
index 9495fcbc7e..658cfc9d9f 100644
--- a/Makefile.objs
+++ b/Makefile.objs
@@ -46,6 +46,7 @@ ifeq ($(CONFIG_SOFTMMU),y)
 common-obj-y = blockdev.o blockdev-nbd.o block/
 common-obj-y += bootdevice.o iothread.o
 common-obj-y += job-qmp.o
+common-obj-y += monitor/
 common-obj-y += net/
 common-obj-y += qdev-monitor.o device-hotplug.o
 common-obj-$(CONFIG_WIN32) += os-win32.o
diff --git a/monitor/Makefile.objs b/monitor/Makefile.objs
index a7170af6e1..1037c09ff8 100644
--- a/monitor/Makefile.objs
+++ b/monitor/Makefile.objs
@@ -1,2 +1,3 @@
 obj-y += misc.o
+common-obj-y += qmp.o
 common-obj-y += qmp-cmds.o hmp-cmds.o
diff --git a/monitor/misc.c b/monitor/misc.c
index 3cdbb681fb..824d28d717 100644
--- a/monitor/misc.c
+++ b/monitor/misc.c
@@ -36,7 +36,6 @@
 #include "exec/gdbstub.h"
 #include "net/net.h"
 #include "net/slirp.h"
-#include "chardev/char-io.h"
 #include "chardev/char-mux.h"
 #include "ui/qemu-spice.h"
 #include "sysemu/numa.h"
@@ -58,8 +57,6 @@
 #include "qapi/qmp/qerror.h"
 #include "qapi/qmp/qnum.h"
 #include "qapi/qmp/qstring.h"
-#include "qapi/qmp/qjson.h"
-#include "qapi/qmp/qlist.h"
 #include "qom/object_interfaces.h"
 #include "trace.h"
 #include "trace/control.h"
@@ -81,7 +78,6 @@
 #include "qapi/qapi-introspect.h"
 #include "sysemu/qtest.h"
 #include "sysemu/cpus.h"
-#include "sysemu/iothread.h"
 #include "qemu/cutils.h"
 #include "tcg/tcg.h"
 
@@ -138,51 +134,29 @@ IOThread *mon_iothread;
 /* Bottom half to dispatch the requests received from I/O thread */
 QEMUBH *qmp_dispatcher_bh;
 
-struct QMPRequest {
-/* Owner of the request */
-MonitorQMP *mon;
-/*
- * Request object to be handled or Error to be reported
- * (exactly one of them is non-null)
- */
-QObject *req;
-Error *err;
-};
-typedef struct QMPRequest QMPRequest;
-
 /* QMP checker flags */
 #define QMP_ACCEPT_UNKNOWNS 1
 
 /* Protects mon_list, monitor_qapi_event_state, monitor_destroyed.  */
-static QemuMutex monitor_lock;
+QemuMutex monitor_lock;
 static GHashTable *monitor_qapi_event_state;
-static QTAILQ_HEAD(, Monitor) mon_list;
+MonitorList mon_list;
 static bool monitor_destroyed;
 
 /* Protects mon_fdsets */
 static QemuMutex mon_fdsets_lock;
 static QLIST_HEAD(, MonFdset) mon_fdsets;
 
-static int mon_refcount;
+int mon_refcount;
 
 static HMPCommand hmp_cmds[];
 static HMPCommand hmp_info_cmds[];
 
-QmpCommandList qmp_commands, qmp_cap_negotiation_commands;
-
 __thread Monitor *cur_mon;
 
 static void monitor_command_cb(void *opaque, const char *cmdline,
void *readline_opaque);
 
-/**
- * Is @mon a QMP monitor?
- */
-static inline bool monitor_is_qmp(const Monitor *mon)
-{
-return (mon->flags & MONITOR_USE_CONTROL);
-}
-
 /**
  * Is @mon is using readline?
  * Note: not all HMP monitors use readline, e.g., gdbserver has a
@@ -241,28 +215,6 @@ int monitor_read_password(MonitorHMP *mon, ReadLineFunc 
*readline_func,
 }
 }
 
-static void qmp_request_free(QMPRequest *req)
-{
-qobject_unref(req->req);
-error_free(req->err);
-g_free(req);
-}
-
-/* Caller must hold mon->qmp.qmp_queue_lock */
-static void monitor_qmp_cleanup_req_queue_locked(MonitorQMP *mon)
-{
-while (!g_queue_is_empty(mon->qmp_requests)) {
-qmp_request_free(g_queue_pop_head(mon->qmp_requests));
-}
-}
-
-static void monitor_qmp_cleanup_queues(MonitorQMP *mon)
-{
-qemu_mutex_lock(>qmp_queue_lock);
-monitor_qmp_cleanup_req_queue_locked(mon);
-qemu_mutex_unlock(>qmp_queue_lock);
-}
-
 
 static void monitor_flush_locked(Monitor *mon);
 
@@ -322,7 +274,7 @@ void monitor_flush(Monitor *mon)
 }
 
 /* flush at every end of line */
-static int monitor_puts(Monitor *mon, const char *str)
+int monitor_puts(Monitor *mon, const char *str)
 {
 int i;
 char c;
@@ -372,21 +324,6 @@ int monitor_printf(Monitor *mon, const char *fmt, ...)
 return ret;
 }
 
-static 

[Qemu-devel] [PULL 06/16] monitor: Remove Monitor.cmd_table indirection

2019-06-17 Thread Markus Armbruster
From: Kevin Wolf 

Monitor.cmd_table is initialised to point to mon_cmds and never changed
afterwards. We can remove the indirection and just reference mon_cmds
directly instead.

Signed-off-by: Kevin Wolf 
Message-Id: <20190613153405.24769-6-kw...@redhat.com>
Reviewed-by: Markus Armbruster 
Signed-off-by: Markus Armbruster 
---
 monitor.c | 9 +++--
 1 file changed, 3 insertions(+), 6 deletions(-)

diff --git a/monitor.c b/monitor.c
index 7c57308e2a..6fb9fa285c 100644
--- a/monitor.c
+++ b/monitor.c
@@ -193,7 +193,6 @@ struct Monitor {
 bool use_io_thread;
 
 gchar *mon_cpu_path;
-mon_cmd_t *cmd_table;
 QTAILQ_ENTRY(Monitor) entry;
 
 /*
@@ -722,8 +721,6 @@ static void monitor_data_init(Monitor *mon, int flags, bool 
skip_flush,
 }
 qemu_mutex_init(>mon_lock);
 mon->outbuf = qstring_new();
-/* Use *mon_cmds by default. */
-mon->cmd_table = mon_cmds;
 mon->skip_flush = skip_flush;
 mon->use_io_thread = use_io_thread;
 mon->flags = flags;
@@ -1024,7 +1021,7 @@ static void help_cmd(Monitor *mon, const char *name)
 }
 
 /* 2. dump the contents according to parsed args */
-help_cmd_dump(mon, mon->cmd_table, args, nb_args, 0);
+help_cmd_dump(mon, mon_cmds, args, nb_args, 0);
 
 free_cmdline_args(args, nb_args);
 }
@@ -3485,7 +3482,7 @@ static void handle_hmp_command(MonitorHMP *mon, const 
char *cmdline)
 
 trace_handle_hmp_command(mon, cmdline);
 
-cmd = monitor_parse_command(mon, cmdline, , mon->common.cmd_table);
+cmd = monitor_parse_command(mon, cmdline, , mon_cmds);
 if (!cmd) {
 return;
 }
@@ -4132,7 +4129,7 @@ static void monitor_find_completion(void *opaque,
 }
 
 /* 2. auto complete according to args */
-monitor_find_completion_by_table(mon, mon->common.cmd_table, args, 
nb_args);
+monitor_find_completion_by_table(mon, mon_cmds, args, nb_args);
 
 cleanup:
 free_cmdline_args(args, nb_args);
-- 
2.21.0




[Qemu-devel] [PULL 14/16] monitor: Split Monitor.flags into separate bools

2019-06-17 Thread Markus Armbruster
From: Kevin Wolf 

Monitor.flags contains three different flags: One to distinguish HMP
from QMP; one specific to HMP (MONITOR_USE_READLINE) that is ignored
with QMP; and another one specific to QMP (MONITOR_USE_PRETTY) that is
ignored with HMP.

Split the flags field into three bools and move them to the right
subclass. Flags are still in use for the monitor_init() interface.

Signed-off-by: Kevin Wolf 
Message-Id: <20190613153405.24769-14-kw...@redhat.com>
Reviewed-by: Markus Armbruster 
Signed-off-by: Markus Armbruster 
---
 monitor/hmp.c  |  6 +++---
 monitor/misc.c |  2 +-
 monitor/monitor-internal.h |  8 +---
 monitor/monitor.c  | 14 +-
 monitor/qmp.c  |  7 ---
 5 files changed, 22 insertions(+), 15 deletions(-)

diff --git a/monitor/hmp.c b/monitor/hmp.c
index 86e86c1cf1..07f827c90c 100644
--- a/monitor/hmp.c
+++ b/monitor/hmp.c
@@ -1397,12 +1397,12 @@ static void monitor_readline_flush(void *opaque)
 void monitor_init_hmp(Chardev *chr, int flags)
 {
 MonitorHMP *mon = g_new0(MonitorHMP, 1);
-bool use_readline = flags & MONITOR_USE_READLINE;
 
-monitor_data_init(>common, flags, false, false);
+monitor_data_init(>common, false, false, false);
 qemu_chr_fe_init(>common.chr, chr, _abort);
 
-if (use_readline) {
+mon->use_readline = flags & MONITOR_USE_READLINE;
+if (mon->use_readline) {
 mon->rs = readline_init(monitor_readline_printf,
 monitor_readline_flush,
 mon,
diff --git a/monitor/misc.c b/monitor/misc.c
index 002aba1030..10f24673f8 100644
--- a/monitor/misc.c
+++ b/monitor/misc.c
@@ -121,7 +121,7 @@ char *qmp_human_monitor_command(const char *command_line, 
bool has_cpu_index,
 Monitor *old_mon;
 MonitorHMP hmp = {};
 
-monitor_data_init(, 0, true, false);
+monitor_data_init(, false, true, false);
 
 old_mon = cur_mon;
 cur_mon = 
diff --git a/monitor/monitor-internal.h b/monitor/monitor-internal.h
index b4996c14ac..03ea0239ef 100644
--- a/monitor/monitor-internal.h
+++ b/monitor/monitor-internal.h
@@ -86,8 +86,8 @@ typedef struct HMPCommand {
 struct Monitor {
 CharBackend chr;
 int reset_seen;
-int flags;
 int suspend_cnt;/* Needs to be accessed atomically */
+bool is_qmp;
 bool skip_flush;
 bool use_io_thread;
 
@@ -112,6 +112,7 @@ struct Monitor {
 
 struct MonitorHMP {
 Monitor common;
+bool use_readline;
 /*
  * State used only in the thread "owning" the monitor.
  * If @use_io_thread, this is @mon_iothread. (This does not actually happen
@@ -125,6 +126,7 @@ struct MonitorHMP {
 typedef struct {
 Monitor common;
 JSONMessageParser parser;
+bool pretty;
 /*
  * When a client connects, we're in capabilities negotiation mode.
  * @commands is _cap_negotiation_commands then.  When command
@@ -148,7 +150,7 @@ typedef struct {
  */
 static inline bool monitor_is_qmp(const Monitor *mon)
 {
-return mon->flags & MONITOR_USE_CONTROL;
+return mon->is_qmp;
 }
 
 typedef QTAILQ_HEAD(MonitorList, Monitor) MonitorList;
@@ -165,7 +167,7 @@ void monitor_init_qmp(Chardev *chr, int flags);
 void monitor_init_hmp(Chardev *chr, int flags);
 
 int monitor_puts(Monitor *mon, const char *str);
-void monitor_data_init(Monitor *mon, int flags, bool skip_flush,
+void monitor_data_init(Monitor *mon, bool is_qmp, bool skip_flush,
bool use_io_thread);
 void monitor_data_destroy(Monitor *mon);
 int monitor_can_read(void *opaque);
diff --git a/monitor/monitor.c b/monitor/monitor.c
index db3d5ece99..3f4808240a 100644
--- a/monitor/monitor.c
+++ b/monitor/monitor.c
@@ -78,14 +78,18 @@ bool monitor_cur_is_qmp(void)
  * Note: not all HMP monitors use readline, e.g., gdbserver has a
  * non-interactive HMP monitor, so readline is not used there.
  */
-static inline bool monitor_uses_readline(const Monitor *mon)
+static inline bool monitor_uses_readline(const MonitorHMP *mon)
 {
-return mon->flags & MONITOR_USE_READLINE;
+return mon->use_readline;
 }
 
 static inline bool monitor_is_hmp_non_interactive(const Monitor *mon)
 {
-return !monitor_is_qmp(mon) && !monitor_uses_readline(mon);
+if (monitor_is_qmp(mon)) {
+return false;
+}
+
+return !monitor_uses_readline(container_of(mon, MonitorHMP, common));
 }
 
 static void monitor_flush_locked(Monitor *mon);
@@ -521,17 +525,17 @@ static void monitor_iothread_init(void)
 mon_iothread = iothread_create("mon_iothread", _abort);
 }
 
-void monitor_data_init(Monitor *mon, int flags, bool skip_flush,
+void monitor_data_init(Monitor *mon, bool is_qmp, bool skip_flush,
bool use_io_thread)
 {
 if (use_io_thread && !mon_iothread) {
 monitor_iothread_init();
 }
 qemu_mutex_init(>mon_lock);
+mon->is_qmp = is_qmp;
 mon->outbuf = qstring_new();
 mon->skip_flush = skip_flush;
 mon->use_io_thread = 

[Qemu-devel] [PULL 05/16] monitor: Create MonitorHMP with readline state

2019-06-17 Thread Markus Armbruster
From: Kevin Wolf 

The ReadLineState in Monitor is only used for HMP monitors. Create
MonitorHMP and move it there.

Can't use container_of() in hmp_change().  Cast instead, and mark
FIXME.  Will be cleaned up shortly.

Signed-off-by: Kevin Wolf 
Reviewed-by: Dr. David Alan Gilbert 
Message-Id: <20190613153405.24769-5-kw...@redhat.com>
Reviewed-by: Markus Armbruster 
[Superfluous variable in monitor_data_destroy() eliminated, whitespace
tweaked in hmp_change(), commit message improved]
Signed-off-by: Markus Armbruster 
---
 hmp.c |   4 +-
 include/monitor/monitor.h |   5 +-
 monitor.c | 126 +-
 3 files changed, 77 insertions(+), 58 deletions(-)

diff --git a/hmp.c b/hmp.c
index be5e345c6f..e6ea7cb9c2 100644
--- a/hmp.c
+++ b/hmp.c
@@ -1943,6 +1943,8 @@ static void hmp_change_read_arg(void *opaque, const char 
*password,
 
 void hmp_change(Monitor *mon, const QDict *qdict)
 {
+/* FIXME Make MonitorHMP public and use container_of */
+MonitorHMP *hmp_mon = (MonitorHMP *)mon;
 const char *device = qdict_get_str(qdict, "device");
 const char *target = qdict_get_str(qdict, "target");
 const char *arg = qdict_get_try_str(qdict, "arg");
@@ -1960,7 +1962,7 @@ void hmp_change(Monitor *mon, const QDict *qdict)
 if (strcmp(target, "passwd") == 0 ||
 strcmp(target, "password") == 0) {
 if (!arg) {
-monitor_read_password(mon, hmp_change_read_arg, NULL);
+monitor_read_password(hmp_mon, hmp_change_read_arg, NULL);
 return;
 }
 }
diff --git a/include/monitor/monitor.h b/include/monitor/monitor.h
index 1e1d6d2269..f9d30e1d78 100644
--- a/include/monitor/monitor.h
+++ b/include/monitor/monitor.h
@@ -6,6 +6,7 @@
 #include "qemu/readline.h"
 
 extern __thread Monitor *cur_mon;
+typedef struct MonitorHMP MonitorHMP;
 
 /* flags for monitor_init */
 /* 0x01 unused */
@@ -34,8 +35,8 @@ void monitor_flush(Monitor *mon);
 int monitor_set_cpu(int cpu_index);
 int monitor_get_cpu_index(void);
 
-void monitor_read_command(Monitor *mon, int show_prompt);
-int monitor_read_password(Monitor *mon, ReadLineFunc *readline_func,
+void monitor_read_command(MonitorHMP *mon, int show_prompt);
+int monitor_read_password(MonitorHMP *mon, ReadLineFunc *readline_func,
   void *opaque);
 
 AddfdInfo *monitor_fdset_add_fd(int fd, bool has_fdset_id, int64_t fdset_id,
diff --git a/monitor.c b/monitor.c
index 15f94fc41f..7c57308e2a 100644
--- a/monitor.c
+++ b/monitor.c
@@ -192,14 +192,6 @@ struct Monitor {
 bool skip_flush;
 bool use_io_thread;
 
-/*
- * State used only in the thread "owning" the monitor.
- * If @use_io_thread, this is @mon_iothread.
- * Else, it's the main thread.
- * These members can be safely accessed without locks.
- */
-ReadLineState *rs;
-
 gchar *mon_cpu_path;
 mon_cmd_t *cmd_table;
 QTAILQ_ENTRY(Monitor) entry;
@@ -220,6 +212,18 @@ struct Monitor {
 int mux_out;
 };
 
+struct MonitorHMP {
+Monitor common;
+/*
+ * State used only in the thread "owning" the monitor.
+ * If @use_io_thread, this is @mon_iothread. (This does not actually happen
+ * in the current state of the code.)
+ * Else, it's the main thread.
+ * These members can be safely accessed without locks.
+ */
+ReadLineState *rs;
+};
+
 typedef struct {
 Monitor common;
 JSONMessageParser parser;
@@ -326,7 +330,7 @@ bool monitor_cur_is_qmp(void)
 return cur_mon && monitor_is_qmp(cur_mon);
 }
 
-void monitor_read_command(Monitor *mon, int show_prompt)
+void monitor_read_command(MonitorHMP *mon, int show_prompt)
 {
 if (!mon->rs)
 return;
@@ -336,7 +340,7 @@ void monitor_read_command(Monitor *mon, int show_prompt)
 readline_show_prompt(mon->rs);
 }
 
-int monitor_read_password(Monitor *mon, ReadLineFunc *readline_func,
+int monitor_read_password(MonitorHMP *mon, ReadLineFunc *readline_func,
   void *opaque)
 {
 if (mon->rs) {
@@ -344,7 +348,8 @@ int monitor_read_password(Monitor *mon, ReadLineFunc 
*readline_func,
 /* prompt is printed on return from the command handler */
 return 0;
 } else {
-monitor_printf(mon, "terminal does not support password prompting\n");
+monitor_printf(>common,
+   "terminal does not support password prompting\n");
 return -ENOTTY;
 }
 }
@@ -705,7 +710,7 @@ static void monitor_qapi_event_init(void)
 qapi_event_throttle_equal);
 }
 
-static void handle_hmp_command(Monitor *mon, const char *cmdline);
+static void handle_hmp_command(MonitorHMP *mon, const char *cmdline);
 
 static void monitor_iothread_init(void);
 
@@ -738,8 +743,9 @@ static void monitor_data_destroy(Monitor *mon)
 qemu_chr_fe_deinit(>chr, false);
 if (monitor_is_qmp(mon)) {
 

[Qemu-devel] [PULL 07/16] monitor: Rename HMP command type and tables

2019-06-17 Thread Markus Armbruster
From: Kevin Wolf 

This renames the type for HMP monitor commands and the tables holding
the commands to make clear that they are related to HMP and to allow
making them public later:

* mon_cmd_t -> HMPCommand (fixing use of a reserved name, too)
* mon_cmds -> hmp_cmds
* info_cmds -> hmp_info_cmds

Signed-off-by: Kevin Wolf 
Message-Id: <20190613153405.24769-7-kw...@redhat.com>
Reviewed-by: Markus Armbruster 
[sortcmdlist() cleaned up to make checkpatch.pl happy]
Signed-off-by: Markus Armbruster 
---
 hmp-commands.hx |  2 +-
 monitor.c   | 72 -
 2 files changed, 36 insertions(+), 38 deletions(-)

diff --git a/hmp-commands.hx b/hmp-commands.hx
index a2c3ffc218..8b7aec3e8d 100644
--- a/hmp-commands.hx
+++ b/hmp-commands.hx
@@ -1934,7 +1934,7 @@ ETEXI
 .params = "[subcommand]",
 .help   = "show various information about the system state",
 .cmd= hmp_info_help,
-.sub_table  = info_cmds,
+.sub_table  = hmp_info_cmds,
 .flags  = "p",
 },
 
diff --git a/monitor.c b/monitor.c
index 6fb9fa285c..3015b6e9c7 100644
--- a/monitor.c
+++ b/monitor.c
@@ -127,7 +127,7 @@
  *
  */
 
-typedef struct mon_cmd_t {
+typedef struct HMPCommand {
 const char *name;
 const char *args_type;
 const char *params;
@@ -138,9 +138,9 @@ typedef struct mon_cmd_t {
  * cmd should be used. If it exists, sub_table[?].cmd should be
  * used, and cmd of 1st level plays the role of help function.
  */
-struct mon_cmd_t *sub_table;
+struct HMPCommand *sub_table;
 void (*command_completion)(ReadLineState *rs, int nb_args, const char 
*str);
-} mon_cmd_t;
+} HMPCommand;
 
 /* file descriptors passed via SCM_RIGHTS */
 typedef struct mon_fd_t mon_fd_t;
@@ -277,8 +277,8 @@ static QLIST_HEAD(, MonFdset) mon_fdsets;
 
 static int mon_refcount;
 
-static mon_cmd_t mon_cmds[];
-static mon_cmd_t info_cmds[];
+static HMPCommand hmp_cmds[];
+static HMPCommand hmp_info_cmds[];
 
 QmpCommandList qmp_commands, qmp_cap_negotiation_commands;
 
@@ -933,7 +933,7 @@ static int parse_cmdline(const char *cmdline,
 /*
  * Can command @cmd be executed in preconfig state?
  */
-static bool cmd_can_preconfig(const mon_cmd_t *cmd)
+static bool cmd_can_preconfig(const HMPCommand *cmd)
 {
 if (!cmd->flags) {
 return false;
@@ -943,7 +943,7 @@ static bool cmd_can_preconfig(const mon_cmd_t *cmd)
 }
 
 static void help_cmd_dump_one(Monitor *mon,
-  const mon_cmd_t *cmd,
+  const HMPCommand *cmd,
   char **prefix_args,
   int prefix_args_nb)
 {
@@ -960,10 +960,10 @@ static void help_cmd_dump_one(Monitor *mon,
 }
 
 /* @args[@arg_index] is the valid command need to find in @cmds */
-static void help_cmd_dump(Monitor *mon, const mon_cmd_t *cmds,
+static void help_cmd_dump(Monitor *mon, const HMPCommand *cmds,
   char **args, int nb_args, int arg_index)
 {
-const mon_cmd_t *cmd;
+const HMPCommand *cmd;
 size_t i;
 
 /* No valid arg need to compare with, dump all in *cmds */
@@ -1021,7 +1021,7 @@ static void help_cmd(Monitor *mon, const char *name)
 }
 
 /* 2. dump the contents according to parsed args */
-help_cmd_dump(mon, mon_cmds, args, nb_args, 0);
+help_cmd_dump(mon, hmp_cmds, args, nb_args, 0);
 
 free_cmdline_args(args, nb_args);
 }
@@ -2689,13 +2689,13 @@ int monitor_fd_param(Monitor *mon, const char *fdname, 
Error **errp)
 }
 
 /* Please update hmp-commands.hx when adding or changing commands */
-static mon_cmd_t info_cmds[] = {
+static HMPCommand hmp_info_cmds[] = {
 #include "hmp-commands-info.h"
 { NULL, NULL, },
 };
 
-/* mon_cmds and info_cmds would be sorted at runtime */
-static mon_cmd_t mon_cmds[] = {
+/* hmp_cmds and hmp_info_cmds would be sorted at runtime */
+static HMPCommand hmp_cmds[] = {
 #include "hmp-commands.h"
 { NULL, NULL, },
 };
@@ -3037,10 +3037,10 @@ static int is_valid_option(const char *c, const char 
*typestr)
 return (typestr != NULL);
 }
 
-static const mon_cmd_t *search_dispatch_table(const mon_cmd_t *disp_table,
-  const char *cmdname)
+static const HMPCommand *search_dispatch_table(const HMPCommand *disp_table,
+   const char *cmdname)
 {
-const mon_cmd_t *cmd;
+const HMPCommand *cmd;
 
 for (cmd = disp_table; cmd->name != NULL; cmd++) {
 if (compare_cmd(cmdname, cmd->name)) {
@@ -3061,14 +3061,14 @@ static const mon_cmd_t *search_dispatch_table(const 
mon_cmd_t *disp_table,
  * Do not assume the return value points into @table!  It doesn't when
  * the command is found in a sub-command table.
  */
-static const mon_cmd_t *monitor_parse_command(MonitorHMP *hmp_mon,
-  const char *cmdp_start,
-  

[Qemu-devel] [PULL 08/16] Move monitor.c to monitor/misc.c

2019-06-17 Thread Markus Armbruster
From: Kevin Wolf 

Create a new monitor/ subdirectory and move monitor.c there. As the plan
is to move the monitor core into separate files, use the chance to
rename it to misc.c.

Signed-off-by: Kevin Wolf 
Reviewed-by: Dr. David Alan Gilbert 
Reviewed-by: Markus Armbruster 
Message-Id: <20190613153405.24769-8-kw...@redhat.com>
Signed-off-by: Markus Armbruster 
---
 MAINTAINERS |  4 ++--
 Makefile.objs   |  1 +
 Makefile.target |  3 ++-
 docs/devel/writing-qmp-commands.txt |  2 +-
 monitor/Makefile.objs   |  1 +
 monitor.c => monitor/misc.c |  2 +-
 monitor/trace-events| 11 +++
 trace-events| 10 --
 8 files changed, 19 insertions(+), 15 deletions(-)
 create mode 100644 monitor/Makefile.objs
 rename monitor.c => monitor/misc.c (99%)
 create mode 100644 monitor/trace-events

diff --git a/MAINTAINERS b/MAINTAINERS
index acbad134ec..575ea6e68d 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1918,7 +1918,7 @@ F: qapi/run-state.json
 Human Monitor (HMP)
 M: Dr. David Alan Gilbert 
 S: Maintained
-F: monitor.c
+F: monitor/misc.c
 F: hmp.[ch]
 F: hmp-commands*.hx
 F: include/monitor/hmp-target.h
@@ -2040,7 +2040,7 @@ QMP
 M: Markus Armbruster 
 S: Supported
 F: qmp.c
-F: monitor.c
+F: monitor/misc.c
 F: docs/devel/*qmp-*
 F: docs/interop/*qmp-*
 F: scripts/qmp/
diff --git a/Makefile.objs b/Makefile.objs
index c8337fa34b..dd39a70b48 100644
--- a/Makefile.objs
+++ b/Makefile.objs
@@ -130,6 +130,7 @@ trace-events-subdirs =
 trace-events-subdirs += accel/kvm
 trace-events-subdirs += accel/tcg
 trace-events-subdirs += crypto
+trace-events-subdirs += monitor
 ifeq ($(CONFIG_USER_ONLY),y)
 trace-events-subdirs += linux-user
 endif
diff --git a/Makefile.target b/Makefile.target
index ecd856e3a3..72c267f7dc 100644
--- a/Makefile.target
+++ b/Makefile.target
@@ -148,9 +148,10 @@ endif #CONFIG_BSD_USER
 #
 # System emulator target
 ifdef CONFIG_SOFTMMU
-obj-y += arch_init.o cpus.o monitor.o gdbstub.o balloon.o ioport.o numa.o
+obj-y += arch_init.o cpus.o gdbstub.o balloon.o ioport.o numa.o
 obj-y += qtest.o
 obj-y += hw/
+obj-y += monitor/
 obj-y += qapi/
 obj-y += memory.o
 obj-y += memory_mapping.o
diff --git a/docs/devel/writing-qmp-commands.txt 
b/docs/devel/writing-qmp-commands.txt
index 9dfc62bf5a..cc6ecd6d5d 100644
--- a/docs/devel/writing-qmp-commands.txt
+++ b/docs/devel/writing-qmp-commands.txt
@@ -470,7 +470,7 @@ it's good practice to always check for errors.
 
 Another important detail is that HMP's "info" commands don't go into the
 hmp-commands.hx. Instead, they go into the info_cmds[] table, which is defined
-in the monitor.c file. The entry for the "info alarmclock" follows:
+in the monitor/misc.c file. The entry for the "info alarmclock" follows:
 
 {
 .name   = "alarmclock",
diff --git a/monitor/Makefile.objs b/monitor/Makefile.objs
new file mode 100644
index 00..e783b0616b
--- /dev/null
+++ b/monitor/Makefile.objs
@@ -0,0 +1 @@
+obj-y += misc.o
diff --git a/monitor.c b/monitor/misc.c
similarity index 99%
rename from monitor.c
rename to monitor/misc.c
index 3015b6e9c7..c5f8483b7e 100644
--- a/monitor.c
+++ b/monitor/misc.c
@@ -64,7 +64,7 @@
 #include "qapi/qmp/json-parser.h"
 #include "qapi/qmp/qlist.h"
 #include "qom/object_interfaces.h"
-#include "trace-root.h"
+#include "trace.h"
 #include "trace/control.h"
 #include "monitor/hmp-target.h"
 #ifdef CONFIG_TRACE_SIMPLE
diff --git a/monitor/trace-events b/monitor/trace-events
new file mode 100644
index 00..abfdf20b14
--- /dev/null
+++ b/monitor/trace-events
@@ -0,0 +1,11 @@
+# See docs/devel/tracing.txt for syntax documentation.
+
+# misc.c
+monitor_protocol_event_handler(uint32_t event, void *qdict) "event=%d data=%p"
+monitor_protocol_event_emit(uint32_t event, void *data) "event=%d data=%p"
+monitor_protocol_event_queue(uint32_t event, void *qdict, uint64_t rate) 
"event=%d data=%p rate=%" PRId64
+handle_hmp_command(void *mon, const char *cmdline) "mon %p cmdline: %s"
+handle_qmp_command(void *mon, const char *req) "mon %p req: %s"
+monitor_suspend(void *ptr, int cnt) "mon %p: %d"
+monitor_qmp_cmd_in_band(const char *id) "%s"
+monitor_qmp_cmd_out_of_band(const char *id) "%s"
diff --git a/trace-events b/trace-events
index 844ee58dd9..aeea3c2bdb 100644
--- a/trace-events
+++ b/trace-events
@@ -41,16 +41,6 @@ system_wakeup_request(int reason) "reason=%d"
 qemu_system_shutdown_request(int reason) "reason=%d"
 qemu_system_powerdown_request(void) ""
 
-# monitor.c
-monitor_protocol_event_handler(uint32_t event, void *qdict) "event=%d data=%p"
-monitor_protocol_event_emit(uint32_t event, void *data) "event=%d data=%p"
-monitor_protocol_event_queue(uint32_t event, void *qdict, uint64_t rate) 
"event=%d data=%p rate=%" PRId64
-handle_hmp_command(void *mon, const char *cmdline) "mon %p cmdline: %s"
-handle_qmp_command(void *mon, const char 

Re: [Qemu-devel] [PULL 00/16] Monitor patches for 2019-06-17

2019-06-17 Thread Peter Maydell
On Mon, 17 Jun 2019 at 19:51, Markus Armbruster  wrote:
>
> The following changes since commit 076243ffe6c1b687e9e6d98348c3bf3398df78f3:
>
>   Merge remote-tracking branch 'remotes/pmaydell/tags/pull-docs-20190617' 
> into staging (2019-06-17 16:41:25 +0100)
>
> are available in the Git repository at:
>
>   git://repo.or.cz/qemu/armbru.git tags/pull-monitor-2019-06-17
>
> for you to fetch changes up to 092b8737c5e7695c4b9caa3b4eedc66763632755:
>
>   vl: Deprecate -mon pretty=... for HMP monitors (2019-06-17 20:36:56 +0200)
>
> 
> Monitor patches for 2019-06-17
>

Hi; I'm afraid this doesn't compile:

Most hosts (ppc64, s390, aarch64, aarch32, osx, and possibly
the others too though they haven't failed immediately) fail
something like this:

/home/pm215/qemu/monitor/misc.c: In function ‘netdev_del_completion’:
/home/pm215/qemu/monitor/misc.c:2165:9: error: implicit declaration of
function ‘qemu_find_opts_err’ [-Werror=implicit-function-declaration]
 opts = qemu_opts_find(qemu_find_opts_err("netdev", NULL), name);
 ^
/home/pm215/qemu/monitor/misc.c:2165:9: error: nested extern
declaration of ‘qemu_find_opts_err’ [-Werror=nested-externs]
/home/pm215/qemu/monitor/misc.c:2165:9: error: passing argument 1 of
‘qemu_opts_find’ makes pointer from integer without a cast [-Werror]
In file included from /home/pm215/qemu/monitor/misc.c:64:0:
/home/pm215/qemu/include/qemu/option.h:105:11: note: expected ‘struct
QemuOptsList *’ but argument is of type ‘int’
 QemuOpts *qemu_opts_find(QemuOptsList *list, const char *id);
   ^
cc1: all warnings being treated as errors


windows is a bit different:

/home/petmay01/qemu-for-merges/monitor/hmp.c: In function 'file_completion':
/home/petmay01/qemu-for-merges/monitor/hmp.c:1113:5: error: unknown
type name 'DIR'
 DIR *ffs;
 ^
/home/petmay01/qemu-for-merges/monitor/hmp.c:1135:11: error: implicit
declaration of function 'opendir'
[-Werror=implicit-function-declaration]
 ffs = opendir(path);
   ^
/home/petmay01/qemu-for-merges/monitor/hmp.c:1135:5: error: nested
extern declaration of 'opendir' [-Werror=nested-externs]
 ffs = opendir(path);
 ^
/home/petmay01/qemu-for-merges/monitor/hmp.c:1135:9: error: assignment
makes pointer from integer without a cast [-Werror=int-conversion]
 ffs = opendir(path);
 ^
/home/petmay01/qemu-for-merges/monitor/hmp.c:1141:13: error: implicit
declaration of function 'readdir'
[-Werror=implicit-function-declaration]
 d = readdir(ffs);
 ^
/home/petmay01/qemu-for-merges/monitor/hmp.c:1141:9: error: nested
extern declaration of 'readdir' [-Werror=nested-externs]
 d = readdir(ffs);
 ^
/home/petmay01/qemu-for-merges/monitor/hmp.c:1141:11: error:
assignment makes pointer from integer without a cast
[-Werror=int-conversion]
 d = readdir(ffs);
   ^
/home/petmay01/qemu-for-merges/monitor/hmp.c:1146:21: error:
dereferencing pointer to incomplete type 'struct dirent'
 if (strcmp(d->d_name, ".") == 0 || strcmp(d->d_name, "..") == 0) {
 ^
/home/petmay01/qemu-for-merges/monitor/hmp.c:1166:5: error: implicit
declaration of function 'closedir'
[-Werror=implicit-function-declaration]
 closedir(ffs);
 ^
/home/petmay01/qemu-for-merges/monitor/hmp.c:1166:5: error: nested
extern declaration of 'closedir' [-Werror=nested-externs]

thanks
-- PMM



[Qemu-devel] [PULL 09/16] monitor: Move {hmp, qmp}.c to monitor/{hmp, qmp}-cmds.c

2019-06-17 Thread Markus Armbruster
From: Kevin Wolf 

Now that we have a monitor/ subdirectory, let's move hmp.c and qmp.c
from the root directory there. As they contain implementations of
monitor commands, rename them to {hmp,qmp}-cmds.c, so that {hmp,qmp}.c
are free for the HMP and QMP infrastructure.

Signed-off-by: Kevin Wolf 
Reviewed-by: Dr. David Alan Gilbert 
Reviewed-by: Markus Armbruster 
Message-Id: <20190613153405.24769-9-kw...@redhat.com>
Signed-off-by: Markus Armbruster 
---
 MAINTAINERS | 5 +++--
 Makefile.objs   | 2 +-
 docs/devel/writing-qmp-commands.txt | 9 +
 monitor/Makefile.objs   | 1 +
 hmp.c => monitor/hmp-cmds.c | 2 +-
 qmp.c => monitor/qmp-cmds.c | 2 +-
 6 files changed, 12 insertions(+), 9 deletions(-)
 rename hmp.c => monitor/hmp-cmds.c (99%)
 rename qmp.c => monitor/qmp-cmds.c (99%)

diff --git a/MAINTAINERS b/MAINTAINERS
index 575ea6e68d..3c7d366727 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1919,7 +1919,8 @@ Human Monitor (HMP)
 M: Dr. David Alan Gilbert 
 S: Maintained
 F: monitor/misc.c
-F: hmp.[ch]
+F: monitor/hmp*
+F: hmp.h
 F: hmp-commands*.hx
 F: include/monitor/hmp-target.h
 F: tests/test-hmp.c
@@ -2039,7 +2040,7 @@ F: tests/check-qom-proplist.c
 QMP
 M: Markus Armbruster 
 S: Supported
-F: qmp.c
+F: monitor/qmp*
 F: monitor/misc.c
 F: docs/devel/*qmp-*
 F: docs/interop/*qmp-*
diff --git a/Makefile.objs b/Makefile.objs
index dd39a70b48..9495fcbc7e 100644
--- a/Makefile.objs
+++ b/Makefile.objs
@@ -83,8 +83,8 @@ common-obj-$(CONFIG_FDT) += device_tree.o
 ##
 # qapi
 
-common-obj-y += qmp.o hmp.o
 common-obj-y += qapi/
+common-obj-y += monitor/
 endif
 
 ###
diff --git a/docs/devel/writing-qmp-commands.txt 
b/docs/devel/writing-qmp-commands.txt
index cc6ecd6d5d..46a6c48683 100644
--- a/docs/devel/writing-qmp-commands.txt
+++ b/docs/devel/writing-qmp-commands.txt
@@ -20,7 +20,7 @@ new QMP command.
 
 2. Write the QMP command itself, which is a regular C function. Preferably,
the command should be exported by some QEMU subsystem. But it can also be
-   added to the qmp.c file
+   added to the monitor/qmp-cmds.c file
 
 3. At this point the command can be tested under the QMP protocol
 
@@ -101,7 +101,8 @@ protocol data.
 
 The next step is to write the "hello-world" implementation. As explained
 earlier, it's preferable for commands to live in QEMU subsystems. But
-"hello-world" doesn't pertain to any, so we put its implementation in qmp.c:
+"hello-world" doesn't pertain to any, so we put its implementation in
+monitor/qmp-cmds.c:
 
 void qmp_hello_world(Error **errp)
 {
@@ -146,7 +147,7 @@ for mandatory arguments). Finally, 'str' is the argument's 
type, which
 stands for "string". The QAPI also supports integers, booleans, enumerations
 and user defined types.
 
-Now, let's update our C implementation in qmp.c:
+Now, let's update our C implementation in monitor/qmp-cmds.c:
 
 void qmp_hello_world(bool has_message, const char *message, Error **errp)
 {
@@ -267,7 +268,7 @@ monitor (HMP).
 
 With the introduction of the QAPI, HMP commands make QMP calls. Most of the
 time HMP commands are simple wrappers. All HMP commands implementation exist in
-the hmp.c file.
+the monitor/hmp-cmds.c file.
 
 Here's the implementation of the "hello-world" HMP command:
 
diff --git a/monitor/Makefile.objs b/monitor/Makefile.objs
index e783b0616b..a7170af6e1 100644
--- a/monitor/Makefile.objs
+++ b/monitor/Makefile.objs
@@ -1 +1,2 @@
 obj-y += misc.o
+common-obj-y += qmp-cmds.o hmp-cmds.o
diff --git a/hmp.c b/monitor/hmp-cmds.c
similarity index 99%
rename from hmp.c
rename to monitor/hmp-cmds.c
index e6ea7cb9c2..c917e24d9c 100644
--- a/hmp.c
+++ b/monitor/hmp-cmds.c
@@ -1,5 +1,5 @@
 /*
- * Human Monitor Interface
+ * Human Monitor Interface commands
  *
  * Copyright IBM, Corp. 2011
  *
diff --git a/qmp.c b/monitor/qmp-cmds.c
similarity index 99%
rename from qmp.c
rename to monitor/qmp-cmds.c
index 6797568444..f1b1e4f08b 100644
--- a/qmp.c
+++ b/monitor/qmp-cmds.c
@@ -1,5 +1,5 @@
 /*
- * QEMU Management Protocol
+ * QEMU Management Protocol commands
  *
  * Copyright IBM, Corp. 2011
  *
-- 
2.21.0




[Qemu-devel] [PULL 01/16] monitor: Fix return type of monitor_fdset_dup_fd_find

2019-06-17 Thread Markus Armbruster
From: Yury Kotov 

monitor_fdset_dup_fd_find_remove() and monitor_fdset_dup_fd_find()
return mon_fdset->id which is int64_t. Downcasting from int64_t to int
leads to a bug with removing fd from fdset with id >= 2^32.
So, fix return types for these function.

Signed-off-by: Yury Kotov 
Reviewed-by: Markus Armbruster 
Message-Id: <20190523094433.30297-1-yury-ko...@yandex-team.ru>
Signed-off-by: Markus Armbruster 
---
 include/monitor/monitor.h | 2 +-
 monitor.c | 4 ++--
 stubs/fdset.c | 2 +-
 3 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/include/monitor/monitor.h b/include/monitor/monitor.h
index 06cfcd8f36..1e1d6d2269 100644
--- a/include/monitor/monitor.h
+++ b/include/monitor/monitor.h
@@ -44,6 +44,6 @@ AddfdInfo *monitor_fdset_add_fd(int fd, bool has_fdset_id, 
int64_t fdset_id,
 int monitor_fdset_get_fd(int64_t fdset_id, int flags);
 int monitor_fdset_dup_fd_add(int64_t fdset_id, int dup_fd);
 void monitor_fdset_dup_fd_remove(int dup_fd);
-int monitor_fdset_dup_fd_find(int dup_fd);
+int64_t monitor_fdset_dup_fd_find(int dup_fd);
 
 #endif /* MONITOR_H */
diff --git a/monitor.c b/monitor.c
index 5c5cbe254a..dce3496920 100644
--- a/monitor.c
+++ b/monitor.c
@@ -2604,7 +2604,7 @@ err:
 return -1;
 }
 
-static int monitor_fdset_dup_fd_find_remove(int dup_fd, bool remove)
+static int64_t monitor_fdset_dup_fd_find_remove(int dup_fd, bool remove)
 {
 MonFdset *mon_fdset;
 MonFdsetFd *mon_fdset_fd_dup;
@@ -2632,7 +2632,7 @@ err:
 return -1;
 }
 
-int monitor_fdset_dup_fd_find(int dup_fd)
+int64_t monitor_fdset_dup_fd_find(int dup_fd)
 {
 return monitor_fdset_dup_fd_find_remove(dup_fd, false);
 }
diff --git a/stubs/fdset.c b/stubs/fdset.c
index f3d9980b7e..67dd5e1d34 100644
--- a/stubs/fdset.c
+++ b/stubs/fdset.c
@@ -6,7 +6,7 @@ int monitor_fdset_dup_fd_add(int64_t fdset_id, int dup_fd)
 return -1;
 }
 
-int monitor_fdset_dup_fd_find(int dup_fd)
+int64_t monitor_fdset_dup_fd_find(int dup_fd)
 {
 return -1;
 }
-- 
2.21.0




[Qemu-devel] [PULL 10/16] monitor: Create monitor-internal.h with common definitions

2019-06-17 Thread Markus Armbruster
From: Kevin Wolf 

Before we can split monitor/misc.c, we need to create a header file that
contains the common definitions that will be used by multiple source
files.

For a start, add the type definitions for Monitor, MonitorHMP and
MonitorQMP and their dependencies. We'll add functions as needed when
splitting monitor/misc.c.

Signed-off-by: Kevin Wolf 
Reviewed-by: Dr. David Alan Gilbert 
Message-Id: <20190613153405.24769-10-kw...@redhat.com>
Reviewed-by: Markus Armbruster 
[Header guard symbol tidied up, superfluous #include dropped, FIXME in
hmp_change() resolved]
Signed-off-by: Markus Armbruster 
---
 MAINTAINERS|   2 +
 monitor/hmp-cmds.c |   5 +-
 monitor/misc.c | 114 +
 monitor/monitor-internal.h | 145 +
 4 files changed, 150 insertions(+), 116 deletions(-)
 create mode 100644 monitor/monitor-internal.h

diff --git a/MAINTAINERS b/MAINTAINERS
index 3c7d366727..15f21c35e0 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1918,6 +1918,7 @@ F: qapi/run-state.json
 Human Monitor (HMP)
 M: Dr. David Alan Gilbert 
 S: Maintained
+F: monitor/monitor-internal.h
 F: monitor/misc.c
 F: monitor/hmp*
 F: hmp.h
@@ -2040,6 +2041,7 @@ F: tests/check-qom-proplist.c
 QMP
 M: Markus Armbruster 
 S: Supported
+F: monitor/monitor-internal.h
 F: monitor/qmp*
 F: monitor/misc.c
 F: docs/devel/*qmp-*
diff --git a/monitor/hmp-cmds.c b/monitor/hmp-cmds.c
index c917e24d9c..c283dde0e9 100644
--- a/monitor/hmp-cmds.c
+++ b/monitor/hmp-cmds.c
@@ -24,7 +24,7 @@
 #include "qemu/option.h"
 #include "qemu/timer.h"
 #include "qemu/sockets.h"
-#include "monitor/monitor.h"
+#include "monitor/monitor-internal.h"
 #include "monitor/qdev.h"
 #include "qapi/error.h"
 #include "qapi/opts-visitor.h"
@@ -1943,8 +1943,7 @@ static void hmp_change_read_arg(void *opaque, const char 
*password,
 
 void hmp_change(Monitor *mon, const QDict *qdict)
 {
-/* FIXME Make MonitorHMP public and use container_of */
-MonitorHMP *hmp_mon = (MonitorHMP *)mon;
+MonitorHMP *hmp_mon = container_of(mon, MonitorHMP, common);
 const char *device = qdict_get_str(qdict, "device");
 const char *target = qdict_get_str(qdict, "target");
 const char *arg = qdict_get_try_str(qdict, "arg");
diff --git a/monitor/misc.c b/monitor/misc.c
index c5f8483b7e..3cdbb681fb 100644
--- a/monitor/misc.c
+++ b/monitor/misc.c
@@ -23,6 +23,7 @@
  */
 
 #include "qemu/osdep.h"
+#include "monitor-internal.h"
 #include "qemu/units.h"
 #include 
 #include "cpu.h"
@@ -35,15 +36,12 @@
 #include "exec/gdbstub.h"
 #include "net/net.h"
 #include "net/slirp.h"
-#include "chardev/char-fe.h"
 #include "chardev/char-io.h"
 #include "chardev/char-mux.h"
 #include "ui/qemu-spice.h"
 #include "sysemu/numa.h"
-#include "monitor/monitor.h"
 #include "qemu/config-file.h"
 #include "qemu/ctype.h"
-#include "qemu/readline.h"
 #include "ui/console.h"
 #include "ui/input.h"
 #include "sysemu/block-backend.h"
@@ -61,7 +59,6 @@
 #include "qapi/qmp/qnum.h"
 #include "qapi/qmp/qstring.h"
 #include "qapi/qmp/qjson.h"
-#include "qapi/qmp/json-parser.h"
 #include "qapi/qmp/qlist.h"
 #include "qom/object_interfaces.h"
 #include "trace.h"
@@ -93,55 +90,6 @@
 #include "hw/s390x/storage-attributes.h"
 #endif
 
-/*
- * Supported types:
- *
- * 'F'  filename
- * 'B'  block device name
- * 's'  string (accept optional quote)
- * 'S'  it just appends the rest of the string (accept optional quote)
- * 'O'  option string of the form NAME=VALUE,...
- *  parsed according to QemuOptsList given by its name
- *  Example: 'device:O' uses qemu_device_opts.
- *  Restriction: only lists with empty desc are supported
- *  TODO lift the restriction
- * 'i'  32 bit integer
- * 'l'  target long (32 or 64 bit)
- * 'M'  Non-negative target long (32 or 64 bit), in user mode the
- *  value is multiplied by 2^20 (think Mebibyte)
- * 'o'  octets (aka bytes)
- *  user mode accepts an optional E, e, P, p, T, t, G, g, M, m,
- *  K, k suffix, which multiplies the value by 2^60 for suffixes E
- *  and e, 2^50 for suffixes P and p, 2^40 for suffixes T and t,
- *  2^30 for suffixes G and g, 2^20 for M and m, 2^10 for K and k
- * 'T'  double
- *  user mode accepts an optional ms, us, ns suffix,
- *  which divides the value by 1e3, 1e6, 1e9, respectively
- * '/'  optional gdb-like print format (like "/10x")
- *
- * '?'  optional type (for all types, except '/')
- * '.'  other form of optional type (for 'i' and 'l')
- * 'b'  boolean
- *  user mode accepts "on" or "off"
- * '-'  optional parameter (eg. '-f')
- *
- */
-
-typedef struct HMPCommand {
-const char *name;
-const char *args_type;
-const char *params;
-const char *help;
-const char *flags; /* 

[Qemu-devel] [PULL 04/16] monitor: Make MonitorQMP a child class of Monitor

2019-06-17 Thread Markus Armbruster
From: Kevin Wolf 

Currently, struct Monitor mixes state that is only relevant for HMP,
state that is only relevant for QMP, and some actually shared state.
In particular, a MonitorQMP field is present in the state of any
monitor, even if it's not a QMP monitor and therefore doesn't use the
state.

As a first step towards a clean separation between QMP and HMP, let
MonitorQMP extend Monitor and create a MonitorQMP object only when the
monitor is actually a QMP monitor.

Some places accessed Monitor.qmp unconditionally, even for HMP monitors.
They can't keep doing this now, so during the conversion, they are
either changed to become conditional on monitor_is_qmp() or to assert()
that they always get a QMP monitor.

Signed-off-by: Kevin Wolf 
Reviewed-by: Dr. David Alan Gilbert 
Message-Id: <20190613153405.24769-4-kw...@redhat.com>
Reviewed-by: Markus Armbruster 
[Superfluous variable in monitor_data_destroy() eliminated]
Signed-off-by: Markus Armbruster 
---
 monitor.c | 219 ++
 1 file changed, 123 insertions(+), 96 deletions(-)

diff --git a/monitor.c b/monitor.c
index 261342a0f6..15f94fc41f 100644
--- a/monitor.c
+++ b/monitor.c
@@ -168,26 +168,6 @@ struct MonFdset {
 QLIST_ENTRY(MonFdset) next;
 };
 
-typedef struct {
-JSONMessageParser parser;
-/*
- * When a client connects, we're in capabilities negotiation mode.
- * @commands is _cap_negotiation_commands then.  When command
- * qmp_capabilities succeeds, we go into command mode, and
- * @command becomes _commands.
- */
-QmpCommandList *commands;
-bool capab_offered[QMP_CAPABILITY__MAX]; /* capabilities offered */
-bool capab[QMP_CAPABILITY__MAX]; /* offered and accepted */
-/*
- * Protects qmp request/response queue.
- * Take monitor_lock first when you need both.
- */
-QemuMutex qmp_queue_lock;
-/* Input queue that holds all the parsed QMP requests */
-GQueue *qmp_requests;
-} MonitorQMP;
-
 /*
  * To prevent flooding clients, events can be throttled. The
  * throttling is calculated globally, rather than per-Monitor
@@ -220,7 +200,6 @@ struct Monitor {
  */
 ReadLineState *rs;
 
-MonitorQMP qmp;
 gchar *mon_cpu_path;
 mon_cmd_t *cmd_table;
 QTAILQ_ENTRY(Monitor) entry;
@@ -241,6 +220,27 @@ struct Monitor {
 int mux_out;
 };
 
+typedef struct {
+Monitor common;
+JSONMessageParser parser;
+/*
+ * When a client connects, we're in capabilities negotiation mode.
+ * @commands is _cap_negotiation_commands then.  When command
+ * qmp_capabilities succeeds, we go into command mode, and
+ * @command becomes _commands.
+ */
+QmpCommandList *commands;
+bool capab_offered[QMP_CAPABILITY__MAX]; /* capabilities offered */
+bool capab[QMP_CAPABILITY__MAX]; /* offered and accepted */
+/*
+ * Protects qmp request/response queue.
+ * Take monitor_lock first when you need both.
+ */
+QemuMutex qmp_queue_lock;
+/* Input queue that holds all the parsed QMP requests */
+GQueue *qmp_requests;
+} MonitorQMP;
+
 /* Shared monitor I/O thread */
 IOThread *mon_iothread;
 
@@ -249,7 +249,7 @@ QEMUBH *qmp_dispatcher_bh;
 
 struct QMPRequest {
 /* Owner of the request */
-Monitor *mon;
+MonitorQMP *mon;
 /*
  * Request object to be handled or Error to be reported
  * (exactly one of them is non-null)
@@ -357,18 +357,18 @@ static void qmp_request_free(QMPRequest *req)
 }
 
 /* Caller must hold mon->qmp.qmp_queue_lock */
-static void monitor_qmp_cleanup_req_queue_locked(Monitor *mon)
+static void monitor_qmp_cleanup_req_queue_locked(MonitorQMP *mon)
 {
-while (!g_queue_is_empty(mon->qmp.qmp_requests)) {
-qmp_request_free(g_queue_pop_head(mon->qmp.qmp_requests));
+while (!g_queue_is_empty(mon->qmp_requests)) {
+qmp_request_free(g_queue_pop_head(mon->qmp_requests));
 }
 }
 
-static void monitor_qmp_cleanup_queues(Monitor *mon)
+static void monitor_qmp_cleanup_queues(MonitorQMP *mon)
 {
-qemu_mutex_lock(>qmp.qmp_queue_lock);
+qemu_mutex_lock(>qmp_queue_lock);
 monitor_qmp_cleanup_req_queue_locked(mon);
-qemu_mutex_unlock(>qmp.qmp_queue_lock);
+qemu_mutex_unlock(>qmp_queue_lock);
 }
 
 
@@ -480,17 +480,17 @@ int monitor_printf(Monitor *mon, const char *fmt, ...)
 return ret;
 }
 
-static void qmp_send_response(Monitor *mon, const QDict *rsp)
+static void qmp_send_response(MonitorQMP *mon, const QDict *rsp)
 {
 const QObject *data = QOBJECT(rsp);
 QString *json;
 
-json = mon->flags & MONITOR_USE_PRETTY ? qobject_to_json_pretty(data) :
- qobject_to_json(data);
+json = mon->common.flags & MONITOR_USE_PRETTY ?
+   qobject_to_json_pretty(data) : qobject_to_json(data);
 assert(json != NULL);
 
 qstring_append_chr(json, '\n');
-monitor_puts(mon, qstring_get_str(json));
+monitor_puts(>common, 

[Qemu-devel] [RFC PATCH v2 30/35] multi-process: handle heartbit messages in remote process

2019-06-17 Thread elena . ufimtseva
From: Elena Ufimtseva 

and reply back to proxy object.

Signed-off-by: Jagannathan Raman 
Signed-off-by: John G Johnson 
Signed-off-by: Elena Ufimtseva 
---
 remote/remote-main.c | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/remote/remote-main.c b/remote/remote-main.c
index c1362be93e..43134762bc 100644
--- a/remote/remote-main.c
+++ b/remote/remote-main.c
@@ -353,6 +353,7 @@ static void process_msg(GIOCondition cond)
 {
 ProcMsg *msg = NULL;
 Error *err = NULL;
+int wait;
 
 if ((cond & G_IO_HUP) || (cond & G_IO_ERR)) {
 error_setg(, "socket closed, cond is %d", cond);
@@ -432,6 +433,11 @@ static void process_msg(GIOCondition cond)
 case DEVICE_DEL:
 process_device_del_msg(msg);
 break;
+case PROXY_PING:
+wait = msg->fds[0];
+notify_proxy(wait, (uint32_t)getpid());
+PUT_REMOTE_WAIT(wait);
+break;
 default:
 error_setg(, "Unknown command");
 goto finalize_loop;
-- 
2.17.1




Re: [Qemu-devel] [PATCH v4] virtio-scsi: restart DMA after iothread

2019-06-17 Thread Paolo Bonzini
On 17/06/19 19:58, Kevin Wolf wrote:
> Of course, this makes me think that maybe for the actual proper
> solution, VM state change handlers should really be a feature that qdev
> provides for devices. Then if a HBA doesn't have anything else to do,
> the qdev core would just recurse into the children right away; if it has
> something to do, it would disable the device after its children, and
> re-enable it before the children.

This was more or less my reply to this version of the patch. :)

Paolo



[Qemu-devel] [PULL 12/16] monitor: Split out monitor/hmp.c

2019-06-17 Thread Markus Armbruster
From: Kevin Wolf 

Move HMP infrastructure from monitor/misc.c to monitor/hmp.c. This is
code that can be shared for all targets, so compile it only once.

The amount of function and particularly extern variables in
monitor_int.h is probably a bit larger than it needs to be, but this way
no non-trivial code modifications are needed. The interfaces between HMP
and the monitor core can be cleaned up later.

Signed-off-by: Kevin Wolf 
Message-Id: <20190613153405.24769-12-kw...@redhat.com>
Reviewed-by: Markus Armbruster 
[Comment reformatted to make checkpatch.pl happy, superfluous #include
dropped]
Signed-off-by: Markus Armbruster 
---
 monitor/Makefile.objs  |2 +-
 monitor/hmp.c  | 1416 
 monitor/misc.c | 1368 +-
 monitor/monitor-internal.h |8 +
 monitor/trace-events   |4 +-
 5 files changed, 1444 insertions(+), 1354 deletions(-)
 create mode 100644 monitor/hmp.c

diff --git a/monitor/Makefile.objs b/monitor/Makefile.objs
index 1037c09ff8..bea8838acc 100644
--- a/monitor/Makefile.objs
+++ b/monitor/Makefile.objs
@@ -1,3 +1,3 @@
 obj-y += misc.o
-common-obj-y += qmp.o
+common-obj-y += qmp.o hmp.o
 common-obj-y += qmp-cmds.o hmp-cmds.o
diff --git a/monitor/hmp.c b/monitor/hmp.c
new file mode 100644
index 00..86e86c1cf1
--- /dev/null
+++ b/monitor/hmp.c
@@ -0,0 +1,1416 @@
+/*
+ * QEMU monitor
+ *
+ * Copyright (c) 2003-2004 Fabrice Bellard
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to 
deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING 
FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ */
+
+#include "qemu/osdep.h"
+#include "monitor-internal.h"
+#include "qapi/error.h"
+#include "qapi/qmp/qdict.h"
+#include "qapi/qmp/qnum.h"
+#include "qemu/config-file.h"
+#include "qemu/ctype.h"
+#include "qemu/cutils.h"
+#include "qemu/log.h"
+#include "qemu/option.h"
+#include "qemu/units.h"
+#include "sysemu/block-backend.h"
+#include "sysemu/sysemu.h"
+#include "trace.h"
+
+static void monitor_command_cb(void *opaque, const char *cmdline,
+   void *readline_opaque)
+{
+MonitorHMP *mon = opaque;
+
+monitor_suspend(>common);
+handle_hmp_command(mon, cmdline);
+monitor_resume(>common);
+}
+
+void monitor_read_command(MonitorHMP *mon, int show_prompt)
+{
+if (!mon->rs) {
+return;
+}
+
+readline_start(mon->rs, "(qemu) ", 0, monitor_command_cb, NULL);
+if (show_prompt) {
+readline_show_prompt(mon->rs);
+}
+}
+
+int monitor_read_password(MonitorHMP *mon, ReadLineFunc *readline_func,
+  void *opaque)
+{
+if (mon->rs) {
+readline_start(mon->rs, "Password: ", 1, readline_func, opaque);
+/* prompt is printed on return from the command handler */
+return 0;
+} else {
+monitor_printf(>common,
+   "terminal does not support password prompting\n");
+return -ENOTTY;
+}
+}
+
+static int get_str(char *buf, int buf_size, const char **pp)
+{
+const char *p;
+char *q;
+int c;
+
+q = buf;
+p = *pp;
+while (qemu_isspace(*p)) {
+p++;
+}
+if (*p == '\0') {
+fail:
+*q = '\0';
+*pp = p;
+return -1;
+}
+if (*p == '\"') {
+p++;
+while (*p != '\0' && *p != '\"') {
+if (*p == '\\') {
+p++;
+c = *p++;
+switch (c) {
+case 'n':
+c = '\n';
+break;
+case 'r':
+c = '\r';
+break;
+case '\\':
+case '\'':
+case '\"':
+break;
+default:
+printf("unsupported escape code: '\\%c'\n", c);
+goto fail;
+}
+if ((q - buf) < buf_size - 1) {
+*q++ = c;
+}
+} else {
+if ((q - 

[Qemu-devel] [PULL 03/16] monitor: Split monitor_init in HMP and QMP function

2019-06-17 Thread Markus Armbruster
From: Kevin Wolf 

Instead of mixing HMP and QMP monitors in the same function, separate
the monitor creation function for both.

While in theory, one could pass both MONITOR_USE_CONTROL and
MONITOR_USE_READLINE before this patch and both flags would do
something, readline support is tightly coupled with HMP: QMP never feeds
its input to readline, and the tab completion function treats the input
as an HMP command. Therefore, this configuration is useless.

After this patch, the QMP path asserts that MONITOR_USE_READLINE is not
set. The HMP path can be used with or without MONITOR_USE_READLINE, like
before.

Signed-off-by: Kevin Wolf 
Reviewed-by: Dr. David Alan Gilbert 
Reviewed-by: Markus Armbruster 
Message-Id: <20190613153405.24769-3-kw...@redhat.com>
[Zero initialization of Monitor moved from monitor_data_init() to
callers]
Signed-off-by: Markus Armbruster 
---
 monitor.c | 95 ---
 1 file changed, 55 insertions(+), 40 deletions(-)

diff --git a/monitor.c b/monitor.c
index 8e9851ae15..261342a0f6 100644
--- a/monitor.c
+++ b/monitor.c
@@ -704,13 +704,12 @@ static void handle_hmp_command(Monitor *mon, const char 
*cmdline);
 
 static void monitor_iothread_init(void);
 
-static void monitor_data_init(Monitor *mon, bool skip_flush,
+static void monitor_data_init(Monitor *mon, int flags, bool skip_flush,
   bool use_io_thread)
 {
 if (use_io_thread && !mon_iothread) {
 monitor_iothread_init();
 }
-memset(mon, 0, sizeof(Monitor));
 qemu_mutex_init(>mon_lock);
 qemu_mutex_init(>qmp.qmp_queue_lock);
 mon->outbuf = qstring_new();
@@ -719,6 +718,7 @@ static void monitor_data_init(Monitor *mon, bool skip_flush,
 mon->skip_flush = skip_flush;
 mon->use_io_thread = use_io_thread;
 mon->qmp.qmp_requests = g_queue_new();
+mon->flags = flags;
 }
 
 static void monitor_data_destroy(Monitor *mon)
@@ -740,9 +740,10 @@ char *qmp_human_monitor_command(const char *command_line, 
bool has_cpu_index,
 int64_t cpu_index, Error **errp)
 {
 char *output = NULL;
-Monitor *old_mon, hmp;
+Monitor *old_mon;
+Monitor hmp = {};
 
-monitor_data_init(, true, false);
+monitor_data_init(, 0, true, false);
 
 old_mon = cur_mon;
 cur_mon = 
@@ -4605,19 +4606,51 @@ static void monitor_qmp_setup_handlers_bh(void *opaque)
 monitor_list_append(mon);
 }
 
-void monitor_init(Chardev *chr, int flags)
+static void monitor_init_qmp(Chardev *chr, int flags)
 {
-Monitor *mon = g_malloc(sizeof(*mon));
-bool use_readline = flags & MONITOR_USE_READLINE;
+Monitor *mon = g_new0(Monitor, 1);
+
+/* Only HMP supports readline */
+assert(!(flags & MONITOR_USE_READLINE));
 
 /* Note: we run QMP monitor in I/O thread when @chr supports that */
-monitor_data_init(mon, false,
-  (flags & MONITOR_USE_CONTROL)
-  && qemu_chr_has_feature(chr,
-  QEMU_CHAR_FEATURE_GCONTEXT));
+monitor_data_init(mon, flags, false,
+  qemu_chr_has_feature(chr, QEMU_CHAR_FEATURE_GCONTEXT));
 
 qemu_chr_fe_init(>chr, chr, _abort);
-mon->flags = flags;
+qemu_chr_fe_set_echo(>chr, true);
+
+json_message_parser_init(>qmp.parser, handle_qmp_command, mon, NULL);
+if (mon->use_io_thread) {
+/*
+ * Make sure the old iowatch is gone.  It's possible when
+ * e.g. the chardev is in client mode, with wait=on.
+ */
+remove_fd_in_watch(chr);
+/*
+ * We can't call qemu_chr_fe_set_handlers() directly here
+ * since chardev might be running in the monitor I/O
+ * thread.  Schedule a bottom half.
+ */
+aio_bh_schedule_oneshot(iothread_get_aio_context(mon_iothread),
+monitor_qmp_setup_handlers_bh, mon);
+/* The bottom half will add @mon to @mon_list */
+} else {
+qemu_chr_fe_set_handlers(>chr, monitor_can_read,
+ monitor_qmp_read, monitor_qmp_event,
+ NULL, mon, NULL, true);
+monitor_list_append(mon);
+}
+}
+
+static void monitor_init_hmp(Chardev *chr, int flags)
+{
+Monitor *mon = g_new0(Monitor, 1);
+bool use_readline = flags & MONITOR_USE_READLINE;
+
+monitor_data_init(mon, flags, false, false);
+qemu_chr_fe_init(>chr, chr, _abort);
+
 if (use_readline) {
 mon->rs = readline_init(monitor_readline_printf,
 monitor_readline_flush,
@@ -4626,36 +4659,18 @@ void monitor_init(Chardev *chr, int flags)
 monitor_read_command(mon, 0);
 }
 
-if (monitor_is_qmp(mon)) {
-qemu_chr_fe_set_echo(>chr, true);
-json_message_parser_init(>qmp.parser, handle_qmp_command,
- mon, NULL);
-if (mon->use_io_thread) {
-/*
-   

[Qemu-devel] [RFC PATCH v2 09/35] multi-process: setup PCI host bridge for remote device

2019-06-17 Thread elena . ufimtseva
From: Jagannathan Raman 

PCI host bridge is setup for the remote device process. It is
implemented using remote-pcihost object. It is an extension of the PCI
host bridge setup by QEMU.
Remote-pcihost configures a PCI bus which could be used by the remote
 PCI device to latch on to.

Signed-off-by: Jagannathan Raman 
Signed-off-by: John G Johnson 
Signed-off-by: Elena Ufimtseva 
---
 hw/pci/Makefile.objs |  2 +-
 include/remote/pcihost.h | 58 +++
 remote/Makefile.objs |  1 +
 remote/pcihost.c | 84 
 4 files changed, 144 insertions(+), 1 deletion(-)
 create mode 100644 include/remote/pcihost.h
 create mode 100644 remote/pcihost.c

diff --git a/hw/pci/Makefile.objs b/hw/pci/Makefile.objs
index 955be54472..90693a7695 100644
--- a/hw/pci/Makefile.objs
+++ b/hw/pci/Makefile.objs
@@ -13,6 +13,6 @@ common-obj-$(CONFIG_PCI_EXPRESS) += pcie_port.o pcie_host.o
 common-obj-$(call lnot,$(CONFIG_PCI)) += pci-stub.o
 common-obj-$(CONFIG_ALL) += pci-stub.o
 
-remote-pci-obj-$(CONFIG_MPQEMU) += pci.o pci_bridge.o
+remote-pci-obj-$(CONFIG_MPQEMU) += pci.o pci_bridge.o pci_host.o pcie_host.o
 remote-pci-obj-$(CONFIG_MPQEMU) += msi.o msix.o
 remote-pci-obj-$(CONFIG_MPQEMU) += pcie.o
diff --git a/include/remote/pcihost.h b/include/remote/pcihost.h
new file mode 100644
index 00..daf39c1291
--- /dev/null
+++ b/include/remote/pcihost.h
@@ -0,0 +1,58 @@
+/*
+ * PCI Host for remote device
+ *
+ * Copyright 2019, Oracle and/or its affiliates.
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to 
deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING 
FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ */
+
+#ifndef REMOTE_PCIHOST_H
+#define REMOTE_PCIHOST_H
+
+#include 
+#include 
+
+#include "exec/memory.h"
+#include "hw/pci/pcie_host.h"
+
+#define TYPE_REMOTE_HOST_DEVICE "remote-pcihost"
+#define REMOTE_HOST_DEVICE(obj) \
+OBJECT_CHECK(RemPCIHost, (obj), TYPE_REMOTE_HOST_DEVICE)
+
+typedef struct RemPCIHost {
+/*< private >*/
+PCIExpressHost parent_obj;
+/*< public >*/
+
+/* Memory Controller Hub (MCH) may not be necessary for the emulation
+ * program. The two important reasons for implementing a PCI host in the
+ * emulation program are:
+ * - Provide a PCI bus for IO devices
+ * - Enable translation of guest PA to the PCI bar regions
+ *
+ * For both the above mentioned purposes, it doesn't look like we would
+ * need the MCH
+ */
+
+MemoryRegion *mr_pci_mem;
+MemoryRegion *mr_sys_mem;
+MemoryRegion *mr_sys_io;
+} RemPCIHost;
+
+#endif
diff --git a/remote/Makefile.objs b/remote/Makefile.objs
index a9b2256b2a..2757f5a265 100644
--- a/remote/Makefile.objs
+++ b/remote/Makefile.objs
@@ -1 +1,2 @@
 remote-pci-obj-$(CONFIG_MPQEMU) += remote-main.o
+remote-pci-obj-$(CONFIG_MPQEMU) += pcihost.o
diff --git a/remote/pcihost.c b/remote/pcihost.c
new file mode 100644
index 00..bcdd214950
--- /dev/null
+++ b/remote/pcihost.c
@@ -0,0 +1,84 @@
+/*
+ * Remote PCI host device
+ *
+ * Copyright 2019, Oracle and/or its affiliates.
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to 
deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF 

[Qemu-devel] [Bug 1832916] Re: linux-user does not check PROT_EXEC

2019-06-17 Thread Alex Bennée
** Tags added: testcase

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

Title:
  linux-user does not check PROT_EXEC

Status in QEMU:
  Confirmed

Bug description:
  At no point do we actually verify that a page is PROT_EXEC before
  translating.  All we end up verifying is that the page is readable.
  Not the same thing, obviously.

  The following test case should work for any architecture, though I've
  only validated it for x86_64 and aarch64.

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



[Qemu-devel] [PULL 13/16] monitor: Split out monitor/monitor.c

2019-06-17 Thread Markus Armbruster
From: Kevin Wolf 

Move the monitor core infrastructure from monitor/misc.c to
monitor/monitor.c. This is code that can be shared for all targets, so
compile it only once.

What remains in monitor/misc.c after this patch is mostly monitor
command implementations (which could move to hmp-cmds.c or qmp-cmds.c
later) and code that requires a system emulator or is even
target-dependent (including HMP command completion code).

The amount of function and particularly extern variables in
monitor_int.h is probably a bit larger than it needs to be, but this way
no non-trivial code modifications are needed. The interfaces between all
monitor parts can be cleaned up later.

Signed-off-by: Kevin Wolf 
Message-Id: <20190613153405.24769-13-kw...@redhat.com>
Reviewed-by: Markus Armbruster 
[Superfluous #include dropped]
Signed-off-by: Markus Armbruster 
---
 MAINTAINERS|   2 +
 include/monitor/monitor.h  |   1 +
 monitor/Makefile.objs  |   2 +-
 monitor/misc.c | 598 +--
 monitor/monitor-internal.h |   1 +
 monitor/monitor.c  | 633 +
 monitor/trace-events   |   2 +-
 7 files changed, 640 insertions(+), 599 deletions(-)
 create mode 100644 monitor/monitor.c

diff --git a/MAINTAINERS b/MAINTAINERS
index 15f21c35e0..d32c5c2313 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1920,6 +1920,7 @@ M: Dr. David Alan Gilbert 
 S: Maintained
 F: monitor/monitor-internal.h
 F: monitor/misc.c
+F: monitor/monitor.c
 F: monitor/hmp*
 F: hmp.h
 F: hmp-commands*.hx
@@ -2044,6 +2045,7 @@ S: Supported
 F: monitor/monitor-internal.h
 F: monitor/qmp*
 F: monitor/misc.c
+F: monitor/monitor.c
 F: docs/devel/*qmp-*
 F: docs/interop/*qmp-*
 F: scripts/qmp/
diff --git a/include/monitor/monitor.h b/include/monitor/monitor.h
index f9d30e1d78..44ac43df34 100644
--- a/include/monitor/monitor.h
+++ b/include/monitor/monitor.h
@@ -19,6 +19,7 @@ typedef struct MonitorHMP MonitorHMP;
 bool monitor_cur_is_qmp(void);
 
 void monitor_init_globals(void);
+void monitor_init_globals_core(void);
 void monitor_init(Chardev *chr, int flags);
 void monitor_cleanup(void);
 
diff --git a/monitor/Makefile.objs b/monitor/Makefile.objs
index bea8838acc..e91a8581cd 100644
--- a/monitor/Makefile.objs
+++ b/monitor/Makefile.objs
@@ -1,3 +1,3 @@
 obj-y += misc.o
-common-obj-y += qmp.o hmp.o
+common-obj-y += monitor.o qmp.o hmp.o
 common-obj-y += qmp-cmds.o hmp-cmds.o
diff --git a/monitor/misc.c b/monitor/misc.c
index 5756d25590..002aba1030 100644
--- a/monitor/misc.c
+++ b/monitor/misc.c
@@ -54,7 +54,6 @@
 #include "qapi/qmp/qerror.h"
 #include "qapi/qmp/qstring.h"
 #include "qom/object_interfaces.h"
-#include "trace.h"
 #include "trace/control.h"
 #include "monitor/hmp-target.h"
 #ifdef CONFIG_TRACE_SIMPLE
@@ -71,7 +70,6 @@
 #include "qapi/error.h"
 #include "qapi/qmp-event.h"
 #include "qapi/qapi-introspect.h"
-#include "sysemu/qtest.h"
 #include "sysemu/cpus.h"
 #include "qemu/cutils.h"
 #include "tcg/tcg.h"
@@ -107,427 +105,15 @@ struct MonFdset {
 QLIST_ENTRY(MonFdset) next;
 };
 
-/*
- * To prevent flooding clients, events can be throttled. The
- * throttling is calculated globally, rather than per-Monitor
- * instance.
- */
-typedef struct MonitorQAPIEventState {
-QAPIEvent event;/* Throttling state for this event type and... */
-QDict *data;/* ... data, see qapi_event_throttle_equal() */
-QEMUTimer *timer;   /* Timer for handling delayed events */
-QDict *qdict;   /* Delayed event (if any) */
-} MonitorQAPIEventState;
-
-typedef struct {
-int64_t rate;   /* Minimum time (in ns) between two events */
-} MonitorQAPIEventConf;
-
-/* Shared monitor I/O thread */
-IOThread *mon_iothread;
-
-/* Bottom half to dispatch the requests received from I/O thread */
-QEMUBH *qmp_dispatcher_bh;
-
 /* QMP checker flags */
 #define QMP_ACCEPT_UNKNOWNS 1
 
-/* Protects mon_list, monitor_qapi_event_state, monitor_destroyed.  */
-QemuMutex monitor_lock;
-static GHashTable *monitor_qapi_event_state;
-MonitorList mon_list;
-static bool monitor_destroyed;
-
 /* Protects mon_fdsets */
 static QemuMutex mon_fdsets_lock;
 static QLIST_HEAD(, MonFdset) mon_fdsets;
 
-int mon_refcount;
-
 static HMPCommand hmp_info_cmds[];
 
-__thread Monitor *cur_mon;
-
-/**
- * Is @mon is using readline?
- * Note: not all HMP monitors use readline, e.g., gdbserver has a
- * non-interactive HMP monitor, so readline is not used there.
- */
-static inline bool monitor_uses_readline(const Monitor *mon)
-{
-return mon->flags & MONITOR_USE_READLINE;
-}
-
-static inline bool monitor_is_hmp_non_interactive(const Monitor *mon)
-{
-return !monitor_is_qmp(mon) && !monitor_uses_readline(mon);
-}
-
-/*
- * Return the clock to use for recording an event's time.
- * It's QEMU_CLOCK_REALTIME, except for qtests it's
- * QEMU_CLOCK_VIRTUAL, to support testing rate limits.
- * Beware: result is invalid before configure_accelerator().
- */
-static inline 

[Qemu-devel] [Bug 1830872] Re: AARCH64 to ARMv7 mistranslation in TCG

2019-06-17 Thread Alex Bennée
** Changed in: qemu
   Status: New => Fix Committed

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

Title:
  AARCH64 to ARMv7 mistranslation in TCG

Status in QEMU:
  Fix Committed

Bug description:
  The following guest code:

  
https://github.com/tianocore/edk2/blob/3604174718e2afc950c3cc64c64ba5165c8692bd/MdePkg/Library/BaseMemoryLibOptDxe/AArch64/CopyMem.S

  implements, in hand-optimized aarch64 assembly, the CopyMem() edk2 (EFI
  Development Kit II) library function. (CopyMem() basically has memmove()
  semantics, to provide a standard C analog here.) The relevant functions
  are InternalMemCopyMem() and __memcpy().

  When TCG translates this aarch64 code to x86_64, everything works
  fine.

  When TCG translates this aarch64 code to ARMv7, the destination area of
  the translated CopyMem() function becomes corrupted -- it differs from
  the intended source contents. Namely, in every 4096 byte block, the
  8-byte word at offset 4032 (0xFC0) is zeroed out in the destination,
  instead of receiving the intended source value.

  I'm attaching two hexdumps of the same destination area:

  - "good.txt" is a hexdump of the destination area when CopyMem() was
translated to x86_64,

  - "bad.txt" is a hexdump of the destination area when CopyMem() was
translated to ARMv7.

  In order to assist with the analysis of this issue, I disassembled the
  aarch64 binary with "objdump". Please find the listing in
  "DxeCore.objdump", attached. The InternalMemCopyMem() function starts at
  hex offset 2b2ec. The __memcpy() function starts at hex offset 2b180.

  And, I ran the guest on the ARMv7 host with "-d
  in_asm,op,op_opt,op_ind,out_asm". Please find the log in
  "tcg.in_asm.op.op_opt.op_ind.out_asm.log", attached.

  The TBs that correspond to (parts of) the InternalMemCopyMem() and
  __memcpy() functions are scattered over the TCG log file, but the offset
  between the "nice" disassembly from "DxeCore.objdump", and the in-RAM
  TBs in the TCG log, can be determined from the fact that there is a
  single prfm instruction in the entire binary. The instruction's offset
  is 0x2b180 in "DxeCore.objdump" -- at the beginning of the __memcpy()
  function --, and its RAM address is 0x472d2180 in the TCG log. Thus the
  difference (= the load address of DxeCore.efi) is 0x472a7000.

  QEMU was built at commit a4f667b67149 ("Merge remote-tracking branch
  'remotes/cohuck/tags/s390x-20190521-3' into staging", 2019-05-21).

  The reproducer command line is (on an ARMv7 host):

qemu-system-aarch64 \
  -display none \
  -machine virt,accel=tcg \
  -nodefaults \
  -nographic \
  -drive 
if=pflash,format=raw,file=$prefix/share/qemu/edk2-aarch64-code.fd,readonly \
  -drive 
if=pflash,format=raw,file=$prefix/share/qemu/edk2-arm-vars.fd,snapshot=on \
  -cpu cortex-a57 \
  -chardev stdio,signal=off,mux=on,id=char0 \
  -mon chardev=char0,mode=readline \
  -serial chardev:char0

  The apparent symptom is an assertion failure *in the guest*, such as

  > ASSERT [DxeCore]
  > 
/home/lacos/src/upstream/qemu/roms/edk2/MdePkg/Library/BaseLib/String.c(1090):
  > Length < _gPcd_FixedAtBuild_PcdMaximumAsciiStringLength

  but that is only a (distant) consequence of the CopyMem()
  mistranslation, and resultant destination area corruption.

  Originally reported in the following two mailing list messages:
  - http://mid.mail-archive.com/9d2e260c-c491-03d2-9b8b-b57b72083f77@redhat.com
  - http://mid.mail-archive.com/f1cec8c0-1a9b-f5bb-f951-ea0ba9d276ee@redhat.com

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



[Qemu-devel] [RFC PATCH v2 34/35] multi-process: add the concept description to docs/devel/qemu-multiprocess

2019-06-17 Thread elena . ufimtseva
From: Elena Ufimtseva 

Signed-off-by: John G Johnson 
Signed-off-by: Elena Ufimtseva 
Signed-off-by: Jagannathan Raman 
---
 Changes in v2:
 - changed the command line options descriptions;
 - added section about communication with remote process for MMIO and QMP 
commands
   using different sockets;
---
 docs/devel/qemu-multiprocess.txt | 1137 ++
 1 file changed, 1137 insertions(+)
 create mode 100644 docs/devel/qemu-multiprocess.txt

diff --git a/docs/devel/qemu-multiprocess.txt b/docs/devel/qemu-multiprocess.txt
new file mode 100644
index 00..f814522178
--- /dev/null
+++ b/docs/devel/qemu-multiprocess.txt
@@ -0,0 +1,1137 @@
+Disaggregating QEMU
+
+This document describes implementation details of multi-process
+qemu.
+
+1. QEMU services
+
+QEMU can be broadly described as providing three main
+services.  One is a VM control point, where VMs can be created,
+migrated, re-configured, and destroyed.  A second is to emulate the
+CPU instructions within the VM, often accelerated by HW virtualization
+features such as Intel's VT extensions.  Finally, it provides IO
+services to the VM by emulating HW IO devices, such as disk and
+network devices.
+
+1.1 A disaggregated QEMU
+
+A disaggregated QEMU involves separating QEMU services into
+separate host processes.  Each of these processes can be given only
+the privileges it needs to provide its service, e.g., a disk service
+could be given access only the the disk images it provides, and not be
+allowed to access other files, or any network devices.  An attacker
+who compromised this service would not be able to use this exploit to
+access files or devices beyond what the disk service was given access
+to.
+
+A control QEMU process would remain, but in disaggregated
+mode, it would be a control point that exec()s the processes needed to
+support the VM being created, but have no direct interface to the VM.
+During VM execution, it would still provide the user interface to
+hot-plug devices or live migrate the VM.
+
+A first step in creating a disaggregated QEMU is to separate
+IO services from the main QEMU program, which would continue to
+provide CPU emulation. i.e., the control process would also be the CPU
+emulation process.  In a later phase, CPU emulation could be separated
+from the QEMU control process.
+
+
+2. Disaggregating IO services
+
+Disaggregating IO services is a good place to begin QEMU
+disaggregating for a couple of reasons.  One is the sheer number of IO
+devices QEMU can emulate provides a large surface of interfaces which
+could potentially be exploited, and, indeed, have been a source of
+exploits in the past.  Another is the modular nature of QEMU device
+emulation code provides interface points where the QEMU functions that
+perform device emulation can be separated from the QEMU functions that
+manage the emulation of guest CPU instructions.
+
+2.1 QEMU device emulation
+
+QEMU uses a object oriented SW architecture for device
+emulation code.  Configured objects are all compiled into the QEMU
+binary, then objects are instantiated by name when used by the guest
+VM.  For example, the code to emulate a device named "foo" is always
+present in QEMU, but its instantiation code is only run when a device
+named "foo" is included in the target VM (such as via the QEMU command
+line as -device "foo".)
+
+The object model is hierarchical, so device emulation code can
+name its parent object (such as "pci-device" for a PCI device) and
+QEMU will instantiate a parent object before calling the device's
+instantiation code.
+
+2.2 Current separation models
+
+In order to separate the device emulation code from the CPU
+emulation code, the device object code must run in a different
+process.  There are a couple of existing QEMU features that can run
+emulation code separately from the main QEMU process.  These are
+examined below.
+
+2.2.1 vhost user model
+
+Virtio guest device drivers can be connected to vhost user
+applications in order to perform their IO operations.  This model uses
+special virtio device drivers in the guest and vhost user device
+objects in QEMU, but once the QEMU vhost user code has configured the
+vhost user application, mission-mode IO is performed by the
+application.  The vhost user application is a daemon process that can
+be contacted via a known UNIX domain socket.
+
+2.2.1.1 vhost socket
+
+As mentioned above, one of the tasks of the vhost device
+object within QEMU is to contact the vhost application and send it
+configuration information about this device instance.  As part of the
+configuration process, the application can also be sent other file
+descriptors over the socket, which then can be used by the vhost user
+application in various ways, some of which are described below.
+
+2.2.1.2 vhost MMIO store acceleration
+
+VMs are often run using HW virtualization features via the KVM

[Qemu-devel] [PULL 02/16] monitor: Remove unused password prompting fields

2019-06-17 Thread Markus Armbruster
From: Kevin Wolf 

Commit 788cf9f8c removed the code for password prompting from the
monitor. Since then, the Monitor fields password_completion_cb and
password_opaque have been unused. Remove them.

Signed-off-by: Kevin Wolf 
Reviewed-by: Dr. David Alan Gilbert 
Reviewed-by: Markus Armbruster 
Message-Id: <20190613153405.24769-2-kw...@redhat.com>
Signed-off-by: Markus Armbruster 
---
 monitor.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/monitor.c b/monitor.c
index dce3496920..8e9851ae15 100644
--- a/monitor.c
+++ b/monitor.c
@@ -222,8 +222,6 @@ struct Monitor {
 
 MonitorQMP qmp;
 gchar *mon_cpu_path;
-BlockCompletionFunc *password_completion_cb;
-void *password_opaque;
 mon_cmd_t *cmd_table;
 QTAILQ_ENTRY(Monitor) entry;
 
-- 
2.21.0




[Qemu-devel] [RFC PATCH v2 35/35] multi-process: add configure and usage information

2019-06-17 Thread elena . ufimtseva
From: Elena Ufimtseva 

Signed-off-by: Elena Ufimtseva 
Signed-off-by: Jagannathan Raman 
Signed-off-by: John G Johnson 
---
 Changes in v2:
 - since the changes were made to use existing device/drive options,
 the document was modified to reflect this.
---
 docs/qemu-multiprocess.txt | 59 ++
 1 file changed, 59 insertions(+)
 create mode 100644 docs/qemu-multiprocess.txt

diff --git a/docs/qemu-multiprocess.txt b/docs/qemu-multiprocess.txt
new file mode 100644
index 00..3b11ca9cc2
--- /dev/null
+++ b/docs/qemu-multiprocess.txt
@@ -0,0 +1,59 @@
+Multi-process QEMU
+
+This document describes how to configure and use multi-process qemu.
+For the design document refer to docs/devel/qemu-multiprocess.
+
+Configure
+
+To enable support for multi-process add --enable-mpqemu
+to the list of options for configure.
+
+Usage
+
+Command line
+
+To start qemu with devices intended to run in the separate emulation
+process, the following suboptions are used:
+
+ remote, [socket=[socket_number],|command="command to run",] rid="remote 
process id" 
+For exmaple, for non multi-process qemu:
+-device lsi53c895a,id=scsi0 device
+-device scsi-hd,drive=drive0,bus=scsi0.0,scsi-id=0
+-drive id=drive0,file=data-disk.img
+
+and for multi-process qemu and no libvirt support (i.e. QEMU forks child 
processes):
+-device lsi53c895a,id=scsi0,remote,rid=0,command="qemu-scsi-dev"
+-device 
scsi-hd,drive=drive0,bus=scsi0.0,scsi-id=0,remote,rid="0",command="qemu-scsi-dev"
+-drive id=drive0,file=data-disk.img,remote,rid=0,command="qemu-scsi-dev"
+
+
+The memorybackend object has to be specified on the command line:
+-object memory-backend-file,id=mem,mem-path=/dev/shm/,size=4096M,share=on
+
+
+Example of running scsi drive in the guest in separate qemu
+process:
+
+qemu-system-x86_64 -enable-kvm -machine q35 -smp 4 -m 4096M -vnc :0 \
+-net nic -net user,hostfwd=tcp::5022-:22 -hda os.qcow2 \
+-device lsi53c895a,id=scsi0,remote,rid=0,command="qemu-scsi-dev" \
+-device 
scsi-hd,drive=drive0,bus=scsi0.0,scsi-id=0,remote,rid="0",command="qemu-scsi-dev"
 \
+-drive id=drive0,file=data-disk.img,remote,rid=0,command="qemu-scsi-dev" \
+-object memory-backend-file,id=mem,mem-path=/dev/shm/,size=4096M,share=on 
-numa node,memdev=mem
+
+HMP commands
+
+For hotplugging in multi-process qemu the following commands
+can be used:
+
+- rdevice_add;
+- rdevice_del;
+- rdrive_add;
+- rdrive_del;
+- remote_proc_list
+
+After running rescan_scsi_bus.sh -a, guest will be able to identify newly
+added devices.
+
+
+
-- 
2.17.1




  1   2   3   4   >