Re: [Qemu-devel] [Qemu-trivial] [PATCH] misc: fix spelling

2018-11-06 Thread Laurent Vivier
On 05/11/2018 14:54, Marc-André Lureau wrote:
> Signed-off-by: Marc-André Lureau 
> ---
>  target/i386/cpu.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/target/i386/cpu.c b/target/i386/cpu.c
> index af7e9f09cc..4d2e2d9902 100644
> --- a/target/i386/cpu.c
> +++ b/target/i386/cpu.c
> @@ -4065,7 +4065,7 @@ arch_query_cpu_model_expansion(CpuModelExpansionType 
> type,
>  x86_cpu_to_dict_full(xc, props);
>  break;
>  default:
> -error_setg(&err, "Unsupportted expansion type");
> +error_setg(&err, "Unsupported expansion type");
>  goto out;
>  }
>  
> 

Applied to my trivial-patches branch.

Thanks,
Laurent



Re: [Qemu-devel] [PATCH 2/2] tpm: use loop iterator to set sts data field

2018-11-06 Thread Marc-André Lureau
Hi

On Tue, Nov 6, 2018 at 9:24 AM P J P  wrote:
>
> From: Prasad J Pandit 
>
> When TIS request is done, set 'sts' data field across all localities.

The code certainly meant to set the field across all localities.
However I don't see in the "TCG PC Client Specific TPM Interface
Specification (TIS)" where it states that the field should be set
across all localities. Could you quote the relevant part?

thanks

> Signed-off-by: Prasad J Pandit 
> ---
>  hw/tpm/tpm_tis.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/hw/tpm/tpm_tis.c b/hw/tpm/tpm_tis.c
> index 20126dd838..58d90645bc 100644
> --- a/hw/tpm/tpm_tis.c
> +++ b/hw/tpm/tpm_tis.c
> @@ -299,7 +299,7 @@ static void tpm_tis_request_completed(TPMIf *ti, int ret)
>
>  if (s->cmd.selftest_done) {
>  for (l = 0; l < TPM_TIS_NUM_LOCALITIES; l++) {
> -s->loc[locty].sts |= TPM_TIS_STS_SELFTEST_DONE;
> +s->loc[l].sts |= TPM_TIS_STS_SELFTEST_DONE;
>  }
>  }
>
> --
> 2.17.2
>
>


-- 
Marc-André Lureau



Re: [Qemu-devel] [PATCH v2 02/10] pci/shpc: rename hotplug handler callbacks

2018-11-06 Thread David Gibson
On Mon, Nov 05, 2018 at 11:20:36AM +0100, David Hildenbrand wrote:
> The callbacks are also called for cold plugged devices. Drop the "hot"
> to better match the actual callback names.
> 
> While at it, also rename shpc_device_hotplug_common() to
> shpc_device_plug_common().
> 
> Signed-off-by: David Hildenbrand 

Reviewed-by: David Gibson 

> ---
>  hw/pci-bridge/pci_bridge_dev.c  | 17 -
>  hw/pci-bridge/pcie_pci_bridge.c | 17 -
>  hw/pci/shpc.c   | 14 +++---
>  include/hw/pci/shpc.h   |  8 
>  4 files changed, 27 insertions(+), 29 deletions(-)
> 
> diff --git a/hw/pci-bridge/pci_bridge_dev.c b/hw/pci-bridge/pci_bridge_dev.c
> index 97a8e8b6a4..e1df9a52ac 100644
> --- a/hw/pci-bridge/pci_bridge_dev.c
> +++ b/hw/pci-bridge/pci_bridge_dev.c
> @@ -206,8 +206,8 @@ static const VMStateDescription pci_bridge_dev_vmstate = {
>  }
>  };
>  
> -static void pci_bridge_dev_hotplug_cb(HotplugHandler *hotplug_dev,
> -  DeviceState *dev, Error **errp)
> +static void pci_bridge_dev_plug_cb(HotplugHandler *hotplug_dev,
> +   DeviceState *dev, Error **errp)
>  {
>  PCIDevice *pci_hotplug_dev = PCI_DEVICE(hotplug_dev);
>  
> @@ -216,12 +216,11 @@ static void pci_bridge_dev_hotplug_cb(HotplugHandler 
> *hotplug_dev,
> "this %s", TYPE_PCI_BRIDGE_DEV);
>  return;
>  }
> -shpc_device_hotplug_cb(hotplug_dev, dev, errp);
> +shpc_device_plug_cb(hotplug_dev, dev, errp);
>  }
>  
> -static void pci_bridge_dev_hot_unplug_request_cb(HotplugHandler *hotplug_dev,
> - DeviceState *dev,
> - Error **errp)
> +static void pci_bridge_dev_unplug_request_cb(HotplugHandler *hotplug_dev,
> + DeviceState *dev, Error **errp)
>  {
>  PCIDevice *pci_hotplug_dev = PCI_DEVICE(hotplug_dev);
>  
> @@ -230,7 +229,7 @@ static void 
> pci_bridge_dev_hot_unplug_request_cb(HotplugHandler *hotplug_dev,
> "this %s", TYPE_PCI_BRIDGE_DEV);
>  return;
>  }
> -shpc_device_hot_unplug_request_cb(hotplug_dev, dev, errp);
> +shpc_device_unplug_request_cb(hotplug_dev, dev, errp);
>  }
>  
>  static void pci_bridge_dev_class_init(ObjectClass *klass, void *data)
> @@ -251,8 +250,8 @@ static void pci_bridge_dev_class_init(ObjectClass *klass, 
> void *data)
>  dc->props = pci_bridge_dev_properties;
>  dc->vmsd = &pci_bridge_dev_vmstate;
>  set_bit(DEVICE_CATEGORY_BRIDGE, dc->categories);
> -hc->plug = pci_bridge_dev_hotplug_cb;
> -hc->unplug_request = pci_bridge_dev_hot_unplug_request_cb;
> +hc->plug = pci_bridge_dev_plug_cb;
> +hc->unplug_request = pci_bridge_dev_unplug_request_cb;
>  }
>  
>  static const TypeInfo pci_bridge_dev_info = {
> diff --git a/hw/pci-bridge/pcie_pci_bridge.c b/hw/pci-bridge/pcie_pci_bridge.c
> index 04cf5a6a92..c634353b06 100644
> --- a/hw/pci-bridge/pcie_pci_bridge.c
> +++ b/hw/pci-bridge/pcie_pci_bridge.c
> @@ -137,8 +137,8 @@ static const VMStateDescription 
> pcie_pci_bridge_dev_vmstate = {
>  }
>  };
>  
> -static void pcie_pci_bridge_hotplug_cb(HotplugHandler *hotplug_dev,
> -  DeviceState *dev, Error **errp)
> +static void pcie_pci_bridge_plug_cb(HotplugHandler *hotplug_dev,
> +DeviceState *dev, Error **errp)
>  {
>  PCIDevice *pci_hotplug_dev = PCI_DEVICE(hotplug_dev);
>  
> @@ -147,12 +147,11 @@ static void pcie_pci_bridge_hotplug_cb(HotplugHandler 
> *hotplug_dev,
> "this %s", TYPE_PCIE_PCI_BRIDGE_DEV);
>  return;
>  }
> -shpc_device_hotplug_cb(hotplug_dev, dev, errp);
> +shpc_device_plug_cb(hotplug_dev, dev, errp);
>  }
>  
> -static void pcie_pci_bridge_hot_unplug_request_cb(HotplugHandler 
> *hotplug_dev,
> - DeviceState *dev,
> - Error **errp)
> +static void pcie_pci_bridge_unplug_request_cb(HotplugHandler *hotplug_dev,
> +  DeviceState *dev, Error **errp)
>  {
>  PCIDevice *pci_hotplug_dev = PCI_DEVICE(hotplug_dev);
>  
> @@ -161,7 +160,7 @@ static void 
> pcie_pci_bridge_hot_unplug_request_cb(HotplugHandler *hotplug_dev,
> "this %s", TYPE_PCIE_PCI_BRIDGE_DEV);
>  return;
>  }
> -shpc_device_hot_unplug_request_cb(hotplug_dev, dev, errp);
> +shpc_device_unplug_request_cb(hotplug_dev, dev, errp);
>  }
>  
>  static void pcie_pci_bridge_class_init(ObjectClass *klass, void *data)
> @@ -180,8 +179,8 @@ static void pcie_pci_bridge_class_init(ObjectClass 
> *klass, void *data)
>  dc->props = pcie_pci_bridge_dev_properties;
>  dc->reset = &pcie_pci_bridge_reset;
>  set_bit(DEVICE_CATEGORY_BRIDGE, dc->categories);
> -hc->plug = p

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

2018-11-06 Thread Thomas Huth
On 2018-11-05 19:14, Michael S. Tsirkin wrote:
> The following changes since commit b2f7a038bb4c4fc5ce6b8486e8513dfd97665e2a:
> 
>   Merge remote-tracking branch 'remotes/rth/tags/pull-softfloat-20181104' 
> into staging (2018-11-05 10:32:49 +)
> 
> are available in the Git repository at:
> 
>   git://git.kernel.org/pub/scm/virt/kvm/mst/qemu.git tags/for_upstream
> 
> for you to fetch changes up to 6196df5c8e6688c1c3f06f73442820066335337c:
> 
>   vhost-scsi: prevent using uninitialized vqs (2018-11-05 12:59:35 -0500)
> 
> 
> pci, pc, virtio: fixes, features
> 
> AMD IOMMU VAPIC support + fixes all over the place.
> 
> Signed-off-by: Michael S. Tsirkin 
> 
> 
> Gerd Hoffmann (1):
>   pci-testdev: add optional memory bar
> 
> Laszlo Ersek (4):
>   MAINTAINERS: list "tests/acpi-test-data" files in ACPI/SMBIOS section
[...]
>  tests/{acpi-test-data => data/acpi}/pc/APIC| Bin
>  tests/{acpi-test-data => data/acpi}/pc/APIC.cphp   | Bin
>  .../{acpi-test-data => data/acpi}/pc/APIC.dimmpxm  | Bin
>  tests/{acpi-test-data => data/acpi}/pc/DSDT| Bin

So patch 1 moves tests/acpi-test-data/ to tests/data/acpi/ and patch 20
adds an entry for tests/acpi-test-data/ ? Does not make much sense to me
... I think patch 20 needs to be adapted now.

 Thomas



Re: [Qemu-devel] [PATCH 1/2] tpm: check localities index

2018-11-06 Thread Marc-André Lureau
Hi

On Tue, Nov 6, 2018 at 9:25 AM P J P  wrote:
>
> From: Prasad J Pandit 
>
> While performing mmio device r/w operations, guest could set 'addr'
> parameter such that 'locty' index exceeds TPM_TIS_NUM_LOCALITIES=5.
> Add check to avoid OOB access.
>

Unless memory_region_init_io() is broken, and the region size is >
TPM_TIS_NUM_LOCALITIES << TPM_TIS_LOCALITY_SHIFT, I don't see how that
could happen. Do you have a reproducer?

Perhaps replace the conditions with an assert instead?

> Reported-by: Cheng Feng 
> Signed-off-by: Prasad J Pandit 
> ---
>  hw/tpm/tpm_tis.c | 10 --
>  1 file changed, 8 insertions(+), 2 deletions(-)
>
> diff --git a/hw/tpm/tpm_tis.c b/hw/tpm/tpm_tis.c
> index 12f5c9a759..20126dd838 100644
> --- a/hw/tpm/tpm_tis.c
> +++ b/hw/tpm/tpm_tis.c
> @@ -293,6 +293,10 @@ static void tpm_tis_request_completed(TPMIf *ti, int ret)
>  uint8_t locty = s->cmd.locty;
>  uint8_t l;
>
> +if (locty >= TPM_TIS_NUM_LOCALITIES) {
> +return;
> +}
> +
>  if (s->cmd.selftest_done) {
>  for (l = 0; l < TPM_TIS_NUM_LOCALITIES; l++) {
>  s->loc[locty].sts |= TPM_TIS_STS_SELFTEST_DONE;
> @@ -401,7 +405,8 @@ static uint64_t tpm_tis_mmio_read(void *opaque, hwaddr 
> addr,
>  uint32_t avail;
>  uint8_t v;
>
> -if (tpm_backend_had_startup_error(s->be_driver)) {
> +if (tpm_backend_had_startup_error(s->be_driver)
> +|| locty >= TPM_TIS_NUM_LOCALITIES) {
>  return 0;
>  }
>
> @@ -530,7 +535,8 @@ static void tpm_tis_mmio_write(void *opaque, hwaddr addr,
>  return;
>  }
>
> -if (tpm_backend_had_startup_error(s->be_driver)) {
> +if (tpm_backend_had_startup_error(s->be_driver)
> +|| locty >= TPM_TIS_NUM_LOCALITIES) {
>  return;
>  }
>
> --
> 2.17.2
>
>


-- 
Marc-André Lureau



Re: [Qemu-devel] [PATCH v2 01/10] pci/pcie: rename hotplug handler callbacks

2018-11-06 Thread David Hildenbrand
On 06.11.18 07:03, David Gibson wrote:
> On Mon, Nov 05, 2018 at 11:20:35AM +0100, David Hildenbrand wrote:
>> The callbacks are also called for cold plugged devices. Drop the "hot"
>> to better match the actual callback names.
>>
>> While at it, also rename  pcie_cap_slot_hotplug_common() to
>> pcie_cap_slot_check_common().
> 
> Uh.. this part of the message doesn't appear to be accurate any more.
> 

Indeed, after renaming it 10 times I think I forgot to update the
description :)

Thanks!

>>
>> Signed-off-by: David Hildenbrand 
> 
> Apart from that,
> 
> Reviewed-by: David Gibson 
> 


-- 

Thanks,

David / dhildenb



Re: [Qemu-devel] [PATCH v2 03/10] s390x/pci: rename hotplug handler callbacks

2018-11-06 Thread David Gibson
On Mon, Nov 05, 2018 at 11:20:37AM +0100, David Hildenbrand wrote:
> The callbacks are also called for cold plugged devices. Drop the "hot"
> to better match the actual callback names.
> 
> Signed-off-by: David Hildenbrand 

Reviewed-by: David Gibson 

> ---
>  hw/s390x/s390-pci-bus.c | 12 ++--
>  1 file changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/hw/s390x/s390-pci-bus.c b/hw/s390x/s390-pci-bus.c
> index e42e1b80d6..e1b14b131b 100644
> --- a/hw/s390x/s390-pci-bus.c
> +++ b/hw/s390x/s390-pci-bus.c
> @@ -813,8 +813,8 @@ static bool s390_pci_alloc_idx(S390pciState *s, 
> S390PCIBusDevice *pbdev)
>  return true;
>  }
>  
> -static void s390_pcihost_hot_plug(HotplugHandler *hotplug_dev,
> -  DeviceState *dev, Error **errp)
> +static void s390_pcihost_plug(HotplugHandler *hotplug_dev, DeviceState *dev,
> +  Error **errp)
>  {
>  PCIDevice *pdev = NULL;
>  S390PCIBusDevice *pbdev = NULL;
> @@ -923,8 +923,8 @@ static void s390_pcihost_timer_cb(void *opaque)
>  qdev_unplug(DEVICE(pbdev), NULL);
>  }
>  
> -static void s390_pcihost_hot_unplug(HotplugHandler *hotplug_dev,
> -DeviceState *dev, Error **errp)
> +static void s390_pcihost_unplug(HotplugHandler *hotplug_dev, DeviceState 
> *dev,
> +Error **errp)
>  {
>  PCIDevice *pci_dev = NULL;
>  PCIBus *bus;
> @@ -1032,8 +1032,8 @@ static void s390_pcihost_class_init(ObjectClass *klass, 
> void *data)
>  
>  dc->reset = s390_pcihost_reset;
>  dc->realize = s390_pcihost_realize;
> -hc->plug = s390_pcihost_hot_plug;
> -hc->unplug = s390_pcihost_hot_unplug;
> +hc->plug = s390_pcihost_plug;
> +hc->unplug = s390_pcihost_unplug;
>  msi_nonbroken = true;
>  }
>  

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


signature.asc
Description: PGP signature


[Qemu-devel] [Bug 1793859] Re: GTK display and mouse input area scaling fails when using vfio-pci device

2018-11-06 Thread Chen Zhang
Hi there,

Would anyone be so kind to review my patch?

Thanks.


https://lists.nongnu.org/archive/html/qemu-devel/2018-10/msg06806.html

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

Title:
  GTK display and mouse input area scaling fails when using vfio-pci
  device

Status in QEMU:
  New

Bug description:
  Version qemu 3.0.0-1 running on Arch. Found on Windows 8.1 and Windows
  10 VM's when using Intel gvt-g device.

  While in fullscreen the GTK display is scaled larger than the x11
  screen or virtual machine resolution. Without choosing zoom-to-fit
  portions of the VM display are not shown on x11 screen regardless of
  the VM resolution. When zoom-to-fit is done the mouse that's shown on
  screen and actual input are off sync. The mouse can wander off screen
  when going left and down.

  This message is shown when changing from gxl-vga to vfio-pci in view menu. 
  (qemu-system-x86_64:6472): Gtk-WARNING **: 09:50:06.663: drawing failure for 
widget 'GtkDrawingArea': NULL pointer
  (qemu-system-x86_64:6472): Gtk-WARNING **: 09:50:06.664: drawing failure for 
widget 'GtkNotebook': NULL pointer
  (qemu-system-x86_64:6472): Gtk-WARNING **: 09:50:06.664: drawing failure for 
widget 'GtkBox': NULL pointer
  (qemu-system-x86_64:6472): Gtk-WARNING **: 09:50:06.664: drawing failure for 
widget 'GtkWindow': NULL pointer

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



[Qemu-devel] [Bug 1017793] Re: S3 Trio64V+ support

2018-11-06 Thread Thomas Huth
Sorry, seems like nobody got interested in adding S3 emulation to QEMU
within the last 6 years, so this is very unlikely going to happen. Thus
I'm closing this bug ticket now.

** Changed in: qemu
   Status: New => Won't Fix

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

Title:
  S3 Trio64V+ support

Status in QEMU:
  Won't Fix

Bug description:
  Is it possible to add S3 Trio emulation to QEMU at all? Since 0.12.3
  the Cirrus Logic seems no longer working properly (bad font
  render/corrupted video). Also, S3 is a widely supported device on many
  OSes and architectures, which will give more compatibility for QEMU.

  Thanks!

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



Re: [Qemu-devel] [PATCH v2 1/1] include: Add a comment to explain the origin of sizes' lookup table

2018-11-06 Thread Leonid Bloch
Hi Phil, Hi Eric,

(Eric, for some reason you weren't CC'd to this thread - sorry.)

On 11/5/18 5:58 PM, Philippe Mathieu-Daudé wrote:
> Hi Leonid,
> 
> On 4/11/18 19:07, Leonid Bloch wrote:
>> The lookup table for power-of-two sizes was added in commit 540b8492618eb
>> for the purpose of having convenient shortcuts for these sizes in cases
>> when the literal number has to be present at compile time, and
>> expressions as '(1 * KiB)' can not be used. One such case is the
>> stringification of sizes. Beyond that, it is convenient to use these
>> shortcuts for all power-of-two sizes, even if they don't have to be
>> literal numbers.
>>
>> Despite its convenience, this table introduced 55 lines of "dumb" code,
>> the purpose and origin of which are obscure without reading the message
>> of the commit which introduced it. This patch fixes that by adding a
>> comment to the code itself with a brief explanation for the reasoning
>> behind this table. This comment includes the short AWK script that
>> generated the table, so that anyone who's interested could make sure
>> that the values in it are correct (otherwise these values look as if
>> they were typed manually).
>>
>> Signed-off-by: Leonid Bloch 
>> ---
>>   include/qemu/units.h | 18 ++
>>   1 file changed, 18 insertions(+)
>>
>> diff --git a/include/qemu/units.h b/include/qemu/units.h
>> index 68a7758650..1c959d182e 100644
>> --- a/include/qemu/units.h
>> +++ b/include/qemu/units.h
>> @@ -17,6 +17,24 @@
>>   #define PiB (INT64_C(1) << 50)
>>   #define EiB (INT64_C(1) << 60)
>> +/*
>> + * The following lookup table is intended to be used when a literal 
>> string of
>> + * the number of bytes is required (for example if it needs to be 
>> stringified).
>> + * It can also be used for generic shortcuts of power-of-two sizes.
> 
> ^ ok
> 
> v this part is not useful in the tree IMHO.
> 
>> + * This table is generated using the AWK script below:
>> + *
>> + *  BEGIN {
>> + *  suffix="KMGTPE";
>> + *  for(i=10; i<64; i++) {
>> + *  val=2**i;
>> + *  s=substr(suffix, int(i/10), 1);
>> + *  n=2**(i%10);
>> + *  pad=21-int(log(n)/log(10));
>> + *  printf("#define S_%d%siB %*d\n", n, s, pad, val);
>> + *  }
>> + *  }
>> + */
> 
> If it is generated and the stringified are also generated, why not 
> generate this once via the ./configure, since it is used by machines and 
> not by humans?

You beat me to it! I was just thinking about that last evening. Indeed 
it's not so elegant to put generated code in a file that is intended for 
human handling. Generating by ./configure would be prettier.
A runtime solution that interprets hard-coded expression strings, like 
Eric suggested, can also be a possibility, but it seems more complicated 
than just generating these at the configuration stage. Plus one gets the 
S_* constants that can be used in many places, not only where an 
explicit number is needed. What do you think?

A question though: if it to be generated by ./configure, where do you 
suggest to put the generated values? And also: is it OK to assume 
there's AWK (or another scripting language) on the system for generating 
these?

Thanks,

Leonid.


Re: [Qemu-devel] [PATCH v7 00/12] More fully implement ARM PMUv3

2018-11-06 Thread no-reply
Hi,

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

Type: series
Message-id: 20181105185046.2802-1-aa...@os.amperecomputing.com
Subject: [Qemu-devel] [PATCH v7 00/12] More fully implement ARM PMUv3

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

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
Switched to a new branch 'test'
5d7e95a226 target/arm: Send interrupts on PMU counter overflow
7c803f850e target/arm: Implement PMSWINC
0b85f12afc target/arm: PMU: Set PMCR.N to 4
9723933103 target/arm: PMU: Add instruction and cycle events
db26e0c7bf target/arm: Finish implementation of PM[X]EVCNTR and PM[X]EVTYPER
1a1b5fa486 target/arm: Add array for supported PMU events, generate PMCEID[01]
e19bb543b8 target/arm: Implement PMOVSSET
07b7bf48b1 target/arm: Allow AArch32 access for PMCCFILTR
8eefc04a65 target/arm: Filter cycle counter based on PMCCFILTR_EL0
5a3876bb4d target/arm: Swap PMU values before/after migrations
cfba1f5577 target/arm: Reorganize PMCCNTR accesses
9803bffbee migration: Add post_save function to VMStateDescription

=== OUTPUT BEGIN ===
  BUILD   centos7
make[1]: Entering directory '/var/tmp/patchew-tester-tmp-_16qprbf/src'
  GEN 
/var/tmp/patchew-tester-tmp-_16qprbf/src/docker-src.2018-11-06-03.47.10.29427/qemu.tar
Cloning into 
'/var/tmp/patchew-tester-tmp-_16qprbf/src/docker-src.2018-11-06-03.47.10.29427/qemu.tar.vroot'...
done.
Checking out files:  45% (2954/6453)   
Checking out files:  46% (2969/6453)   
Checking out files:  47% (3033/6453)   
Checking out files:  48% (3098/6453)   
Checking out files:  49% (3162/6453)   
Checking out files:  50% (3227/6453)   
Checking out files:  51% (3292/6453)   
Checking out files:  52% (3356/6453)   
Checking out files:  53% (3421/6453)   
Checking out files:  54% (3485/6453)   
Checking out files:  55% (3550/6453)   
Checking out files:  56% (3614/6453)   
Checking out files:  57% (3679/6453)   
Checking out files:  58% (3743/6453)   
Checking out files:  59% (3808/6453)   
Checking out files:  60% (3872/6453)   
Checking out files:  61% (3937/6453)   
Checking out files:  62% (4001/6453)   
Checking out files:  63% (4066/6453)   
Checking out files:  64% (4130/6453)   
Checking out files:  65% (4195/6453)   
Checking out files:  66% (4259/6453)   
Checking out files:  67% (4324/6453)   
Checking out files:  68% (4389/6453)   
Checking out files:  69% (4453/6453)   
Checking out files:  70% (4518/6453)   
Checking out files:  71% (4582/6453)   
Checking out files:  72% (4647/6453)   
Checking out files:  73% (4711/6453)   
Checking out files:  74% (4776/6453)   
Checking out files:  75% (4840/6453)   
Checking out files:  76% (4905/6453)   
Checking out files:  77% (4969/6453)   
Checking out files:  78% (5034/6453)   
Checking out files:  79% (5098/6453)   
Checking out files:  80% (5163/6453)   
Checking out files:  81% (5227/6453)   
Checking out files:  82% (5292/6453)   
Checking out files:  83% (5356/6453)   
Checking out files:  84% (5421/6453)   
Checking out files:  85% (5486/6453)   
Checking out files:  86% (5550/6453)   
Checking out files:  87% (5615/6453)   
Checking out files:  88% (5679/6453)   
Checking out files:  89% (5744/6453)   
Checking out files:  90% (5808/6453)   
Checking out files:  91% (5873/6453)   
Checking out files:  92% (5937/6453)   
Checking out files:  93% (6002/6453)   
Checking out files:  94% (6066/6453)   
Checking out files:  95% (6131/6453)   
Checking out files:  96% (6195/6453)   
Checking out files:  97% (6260/6453)   
Checking out files:  98% (6324/6453)   
Checking out files:  99% (6389/6453)   
Checking out files: 100% (6453/6453)   
Checking out files: 100% (6453/6453), done.
Your branch is up-to-date with 'origin/test'.
Submodule 'dtc' (git://git.qemu-project.org/dtc.git) registered for path 'dtc'
Cloning into 
'/var/tmp/patchew-tester-tmp-_16qprbf/src/docker-src.2018-11-06-03.47.10.29427/qemu.tar.vroot/dtc'...
Submodule path 'dtc': checked out '88f18909db731a627456f26d779445f84e449536'
Submodule 'ui/keycodemapdb' (git://git.qemu.org/keycodemapdb.git) registered 
for path 'ui/keycodemapdb'
Cloning into 
'/var/tmp/patchew-tester-tmp-_16qprbf/src/docker-src.2018-11-06-03.47.10.29427/qemu.tar.vroot/ui/keycodemapdb'...
Submodule path 'ui/keycodemapdb': checked out 
'6b3d716e2b6472eb7189d3220552280ef3d832ce'
  COPYRUNNER
RUN test-quick in qemu:centos7 
Packages installed:
SDL-devel-1.2.15-14.el7.x86_64
bison-3.0.4-1.el7.x86_64
bzip2-1.0.6-13.el7.x86_64
bzip2-devel-1.0.6-13.el7.x86_64
ccache-3.3.4-1.el7.x86_64
csnappy-devel-0-6.20150729gitd7bc683.el7.x86_64
flex-2.5.37-3.el7.x86_64
gcc-4.8.5-28.el7_5.1.x86_64
gettext-0.19.8.1-2.el7.x86_64
git-1.8.3.1-14.el7_5.x86_64
glib2-devel-2.54.2-2.el7.x86_64
libaio-devel-0.3.109-13.el7.x86_64
libepoxy-devel-1.3.1-2.el7_5.x86_64
libfdt-devel-1.4.6-1.el7.x86_64

Re: [Qemu-devel] [PATCH] qapi: misc: change the 'pc' to unsinged 64 in CpuInfo

2018-11-06 Thread Peter Maydell
On 2 November 2018 at 11:01, Li Qiang  wrote:
> When trigger a 'query-cpus' qmp, the pc is an signed value like
> following:
> {"arch": "x86", ...  "pc": -1732653994, "halted": true,...}
> It is strange. Change it to uint64_t.
>
> Signed-off-by: Li Qiang 
> ---

NB: typo in subject line: should be "unsigned".

thanks
-- PMM



Re: [Qemu-devel] [Libguestfs] How to emulate block I/O timeout on qemu side?

2018-11-06 Thread Richard W.M. Jones
On Tue, Nov 06, 2018 at 02:17:46PM +0800, Dongli Zhang wrote:
> On 11/06/2018 01:49 AM, Eric Blake wrote:
> > On 11/2/18 3:11 AM, Dongli Zhang wrote:
> >> Hi,
> >>
> >> Is there any way to emulate I/O timeout on qemu side (not fault
> >> injection in VM kernel) without modifying qemu source code?
> >
> > You may be interested in Rich's work on nbdkit.  If you don't mind
> > the overhead of the host connecting through NBD, then you can use
> > nbdkit's delay and fault-injection filters for inserting delays or
> > even run-time-controllable failures to investigate how the guest
> > reacts to those situations
> >
> Thank you all very much for the suggestions. I will take a look on nbdkit.

These links should help:

  
https://rwmj.wordpress.com/2018/09/04/nbdkit-for-loopback-pt-2-injecting-errors/
  https://rwmj.wordpress.com/2018/09/06/nbdkit-for-loopback-pt-7-a-slow-disk/

This link shows how to combine delay and error filters together:

  https://rwmj.wordpress.com/2018/11/04/nbd-graphical-viewer/

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
Fedora Windows cross-compiler. Compile Windows programs, test, and
build Windows installers. Over 100 libraries supported.
http://fedoraproject.org/wiki/MinGW



Re: [Qemu-devel] [PATCH v3 1/7] qapi: use qemu_strtoi64() in parse_str

2018-11-06 Thread David Hildenbrand
On 05.11.18 21:43, Markus Armbruster wrote:
> David Hildenbrand  writes:
> 
>> On 05.11.18 16:37, Markus Armbruster wrote:
>>> David Hildenbrand  writes:
>>>
 On 31.10.18 18:55, Markus Armbruster wrote:
> David Hildenbrand  writes:
>
>> On 31.10.18 15:40, Markus Armbruster wrote:
>>> David Hildenbrand  writes:
>>>
 The qemu api claims to be easier to use, and the resulting code seems 
 to
 agree.
> [...]
 @@ -60,9 +61,7 @@ static int parse_str(StringInputVisitor *siv, const 
 char *name, Error **errp)
  }
  
  do {
 -errno = 0;
 -start = strtoll(str, &endptr, 0);
 -if (errno == 0 && endptr > str) {
 +if (!qemu_strtoi64(str, &endptr, 0, &start)) {
  if (*endptr == '\0') {
  cur = g_malloc0(sizeof(*cur));
  range_set_bounds(cur, start, start);
 @@ -71,11 +70,7 @@ static int parse_str(StringInputVisitor *siv, const 
 char *name, Error **errp)
  str = NULL;
  } else if (*endptr == '-') {
  str = endptr + 1;
 -errno = 0;
 -end = strtoll(str, &endptr, 0);
 -if (errno == 0 && endptr > str && start <= end &&
 -(start > INT64_MAX - 65536 ||
 - end < start + 65536)) {
 +if (!qemu_strtoi64(str, &endptr, 0, &end) && start < 
 end) {
>>>
>>> You deleted (start > INT64_MAX - 65536 || end < start + 65536).  Can you
>>> explain that to me?  I'm feeling particularly dense today...
>>
>> qemu_strtoi64 performs all different kinds of error handling completely
>> internally. This old code here was an attempt to filter out -EWHATEVER
>> from the response. No longer needed as errors and the actual value are
>> reported via different ways.
>
> I understand why errno == 0 && endptr > str go away.  They also do in
> the previous hunk.
>
> The deletion of (start > INT64_MAX - 65536 || end < start + 65536) is
> unobvious.  What does it do before the patch?
>
> The condition goes back to commit 659268ffbff, which predates my watch
> as maintainer.  Its commit message is of no particular help.  Its code
> is... allright, the less I say about that, the better.
>
> We're parsing a range here.  We already parsed its lower bound into
> @start (and guarded against errors), and its upper bound into @end (and
> guarded against errors).
>
> If the condition you delete is false, we goto error.  So the condition
> is about range validity.  I figure it's an attempt to require valid
> ranges to be no "wider" than 65535.  The second part end < start + 65536
> checks exactly that, except shit happens when start + 65536 overflows.
> The first part attempts to guard against that, but
>
> (1) INT64_MAX is *wrong*, because we compute in long long, and
>
> (2) it rejects even small ranges like INT64_MAX - 2 .. INT64_MAX - 1.
>
> WTF?!?
>
> Unless I'm mistaken, the condition is not about handling any of the
> errors that qemu_strtoi64() handles for us.
>
> The easiest way for you out of this morass is probably to keep the
> condition exactly as it was, then use the "my patch doesn't make things
> any worse" get-out-of-jail-free card.
>

 Looking at the code in qapi/string-output-visitor.c related to range and
 list handling I feel like using the get-out-of-jail-free card to get out
 of qapi code now :) Too much magic in that code and too little time for
 me to understand it all.

 Thanks for your time and review anyway. My time is better invested in
 other parts of QEMU. I will drop both patches from this series.
>>>
>>> Understand.
>>>
>>> When I first looked at the ranges stuff in the string input visitor, I
>>> felt the urge to clean it up, then sat on my hands until it passed.
>>>
>>> The rest is reasonable once you understand how it works.  The learning
>>> curve is less than pleasant, though.
>>>
>>
>> Maybe I'll pick this up again when I have more time to invest.
>>
>> The general concept
>>
>> 1. of having an input visitor that is able to parse different types
>> (expected by e.g. a property) sounds sane to me.
>>
>> 2. of having a list of *something*, assuming it is int64_t, and assuming
>> it is to be parsed into a list of ranges sounds completely broken to me.
> 
> Starting point: the string visitors can only do scalars.  We have a need
> for lists of integers (see below).  The general solution would be
> generalizing these visitors to lists (and maybe objects while we're at
> it).  YAGNI.  So we put in a quick hack that can do just lists of
> integers.
> 
> Except applying YAGNI 

Re: [Qemu-devel] [PATCH v3 1/7] qapi: use qemu_strtoi64() in parse_str

2018-11-06 Thread David Hildenbrand
On 05.11.18 17:48, Eric Blake wrote:
> On 11/5/18 9:53 AM, David Hildenbrand wrote:
> 
>>> When I first looked at the ranges stuff in the string input visitor, I
>>> felt the urge to clean it up, then sat on my hands until it passed.
>>>
>>> The rest is reasonable once you understand how it works.  The learning
>>> curve is less than pleasant, though.
>>>
>>
>> Maybe I'll pick this up again when I have more time to invest.
>>
>> The general concept
>>
>> 1. of having an input visitor that is able to parse different types
>> (expected by e.g. a property) sounds sane to me.
>>
>> 2. of having a list of *something*, assuming it is int64_t, and assuming
>> it is to be parsed into a list of ranges sounds completely broken to me.
>>
>> I was not even able to find an example QEMU comand line for 2. Is this
>> maybe some very old code that nobody actually uses anymore? (who uses
>> list of ranges?)
> 
> I believe that the -numa node,cpus=first-last code is an example use of 
> a list of ranges.  At any rate, tests/test-string-input-visitor.c has 
> test_visitor_in_intList() to ensure we don't break back-compat with the 
> existing clients that use ranges, as awkward as they may be.
> 

Indeed, thanks for the pointer. Ranges should indeed be supported. But
maybe we can refactor this ugly "pre parse all uint64_t ranges" thingy.

-- 

Thanks,

David / dhildenb



Re: [Qemu-devel] [PATCH for-3.1 0/2] Fix qemu_thread_atexit* for OSX

2018-11-06 Thread Paolo Bonzini
On 05/11/2018 14:55, Peter Maydell wrote:
> Our current implementation of qemu_thread_atexit* is broken on OSX.
> This is because it works by cerating a piece of thread-specific
> data with pthread_key_create() and using the destructor function
> for that data to run the notifier function passed to it by
> the caller of qemu_thread_atexit_add(). The expected use case
> is that the caller uses a __thread variable as the notifier,
> and uses the callback to clean up information that it is
> keeping per-thread in __thread variables.
> 
> Unfortunately, on OSX this does not work, because on OSX
> a __thread variable may be destroyed (freed) before the
> pthread_key_create() destructor runs. (POSIX imposes no
> ordering constraint here; the OSX implementation happens
> to implement __thread variables in terms of pthread_key_create((),
> whereas Linux uses different mechanisms that mean the __thread
> variables will still be present when the pthread_key_create()
> destructor is run.)
> 
> Fix this by switching to a scheme similar to the one qemu-thread-win32
> uses for qemu_thread_atexit: keep the thread's notifiers on a
> __thread variable, and run the notifiers on calls to
> qemu_thread_exit() and on return from the start routine passed
> to qemu_thread_start(). We do this with the pthread_cleanup_push()
> API. (This approach was suggested by Paolo.)
> 
> There is a slightly awkward corner here for the main thread,
> which isn't started via qemu_thread_start() and so can exit
> without passing through the cleanup handler. qemu-thread-win32.c
> tries to handle the main thread using an atexit() handler to
> run its notifiers. I don't think this will work because the atexit
> handler may not run on the main thread, in which case notify
> callback functions which refer to __thread variables will get
> the wrong ones. So instead I have documented that the API
> leaves it unspecified whether notifiers are run for thread
> exits that happen when the entire process is exiting. This
> is OK for our current users (who only use the callbacks to
> clean up memory or win32 Fibers which are deleted on process
> exit anyway), and means we don't need to worry about running
> callbacks on main thread exit (which always causes the whole
> process to exit).
> 
> In particular, this fixes the ASAN errors and lockups we
> currently see on OSX hosts when running test-bdrv-drain.

Queued, thanks.  As an extra cleanup we can remove
qemu_thread_atexit_remove (it is unused, and a simple notifier_remove
now works), and also remove the atexit from Win32 so we can document
that the notifier does _not_ run for the main thread.

Paolo



Re: [Qemu-devel] [qemu-web PATCH] Use 'https://' instead of 'git://'

2018-11-06 Thread Stefan Hajnoczi
On Sun, Nov 04, 2018 at 11:23:12PM +0100, Paolo Bonzini wrote:
> Jeff Cody has enabled git smart HTTP support on qemu.org.  From now on HTTPS 
> is
> the preferred protocol because it adds some protection against
> man-in-the-middle when cloning a repo.
> 
> This patch series updates git:// URLs and changes them to https://.  The 
> https:// URL format is:
> 
>   https://git.qemu.org/git/.git
> 
> The old git:// URL format was:
> 
>   git://git.qemu.org/.git
> 
> Signed-off-by: Paolo Bonzini 
> ---
>  _download/source.html | 2 +-
>  contribute.md | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)

Reviewed-by: Stefan Hajnoczi 


signature.asc
Description: PGP signature


Re: [Qemu-devel] [Libguestfs] How to emulate block I/O timeout on qemu side?

2018-11-06 Thread Richard W.M. Jones
On Tue, Nov 06, 2018 at 09:14:57AM +, Richard W.M. Jones wrote:
> This link shows how to combine delay and error filters together:
> 
>   https://rwmj.wordpress.com/2018/11/04/nbd-graphical-viewer/

Oops, that's in a forthcoming blog post not this one.  Not enough
caffeine this morning.

Combining the filters is easy however:

  nbdkit --filter=error --filter=delay \
 memory size=$size \
 rdelay=$delay wdelay=$delay \
 error-rate=100% error-file=/tmp/error

Then touching /tmp/error will inject errors, and removing /tmp/error
will stop injecting errors.

The documentation says you should be able to write error-rate=1
instead of error-rate=100%, but in fact that was broken until
recently, and fixed in:

  
https://github.com/libguestfs/nbdkit/commit/ee2d3b4fea6d4b7618262f85f882374c23674b4a

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-p2v converts physical machines to virtual machines.  Boot with a
live CD or over the network (PXE) and turn machines into KVM guests.
http://libguestfs.org/virt-v2v



Re: [Qemu-devel] [PATCH 3/4] scsi-generic: avoid invalid access to struct when emulating block limits

2018-11-06 Thread Paolo Bonzini
On 06/11/2018 03:16, Max Reitz wrote:
>> diff --git a/hw/scsi/scsi-generic.c b/hw/scsi/scsi-generic.c
>> index c5497bbea8..8fc74ef0bd 100644
>> --- a/hw/scsi/scsi-generic.c
>> +++ b/hw/scsi/scsi-generic.c
>> @@ -16,6 +16,7 @@
>>  #include "qemu-common.h"
>>  #include "qemu/error-report.h"
>>  #include "hw/scsi/scsi.h"
>> +#include "hw/scsi/emulation.h"
>>  #include "sysemu/block-backend.h"
>>  
>>  #ifdef __linux__
>> @@ -209,9 +210,24 @@ static void scsi_handle_inquiry_reply(SCSIGenericReq 
>> *r, SCSIDevice *s)
>>  }
>>  }
>>  
>> -static int scsi_emulate_block_limits(SCSIGenericReq *r)
>> +static int scsi_generic_emulate_block_limits(SCSIGenericReq *r, SCSIDevice 
>> *s)
>>  {
>> -r->buflen = scsi_disk_emulate_vpd_page(&r->req, r->buf);
>> +int len, buflen;
> 
> buflen is unused, so this does not compile for me.

Yeah, I forgot to commit it.

>> +uint8_t buf[64];
>> +
>> +SCSIBlockLimits bl = {
>> +.max_io_sectors = blk_get_max_transfer(s->conf.blk) / s->blocksize
>> +};
>> +
>> +memset(r->buf, 0, r->buflen);
>> +stb_p(buf, s->type);
>> +stb_p(buf + 1, 0xb0);
>> +len = scsi_emulate_block_limits(buf + 4, &bl);
>> +assert(len <= sizeof(buf) - 4);
> 
> Let's hope our stack grows downwards, otherwise we'll never get back
> here if there was an overflow.  Maybe it would be better to pass the
> buffer length to scsi_emulate_block_limits() and then move the assertion
> there.

Isn't that a problem always with stacks that grow upwards?  A buffer
overflow on an argument that points to a local variable will always
corrupt the stack frame (on the other hand, a stack that grows downwards
will corrupt the stack frame if the buffer overflow occurs directly in
the function with no call in between).

> With buflen dropped, and you taking full responsibility for any future
> bugs on ABIs with upwards stacks when someone extended
> scsi_emulate_block_limits(), forgetting to adjust the buffer size here
> (:-)):

I think it's quite likely that the bug would be caught first on a
downwards-stack architecture. :)

The complete delta between v1 and v2 will be

diff --git a/hw/scsi/emulation.c b/hw/scsi/emulation.c
index 94c2254bb4..06d62f3c38 100644
--- a/hw/scsi/emulation.c
+++ b/hw/scsi/emulation.c
@@ -3,10 +3,10 @@
 #include "qemu/bswap.h"
 #include "hw/scsi/emulation.h"

-int scsi_emulate_block_limits(uint8_t *outbuf, SCSIBlockLimits *bl)
+int scsi_emulate_block_limits(uint8_t *outbuf, const SCSIBlockLimits *bl)
 {
 /* required VPD size with unmap support */
-memset(outbuf, 0, 0x3C);
+memset(outbuf, 0, 0x3c);

 outbuf[0] = bl->wsnz; /* wsnz */

diff --git a/hw/scsi/scsi-generic.c b/hw/scsi/scsi-generic.c
index b8fa0a0f7e..7237b4162e 100644
--- a/hw/scsi/scsi-generic.c
+++ b/hw/scsi/scsi-generic.c
@@ -182,7 +182,7 @@ static void scsi_handle_inquiry_reply(SCSIGenericReq
*r, SCSIDevice *s)
 /* Also take care of the opt xfer len. */
 stl_be_p(&r->buf[12],
 MIN_NON_ZERO(max_transfer, ldl_be_p(&r->buf[12])));
-} else if (page == 0x00 && s->needs_vpd_bl_emulation) {
+} else if (s->needs_vpd_bl_emulation && page == 0x00) {
 /*
  * Now we're capable of supplying the VPD Block Limits
  * response if the hardware can't. Add it in the INQUIRY
@@ -268,15 +268,14 @@ static void scsi_read_complete(void * opaque, int ret)
  * resulted in sense error but would need emulation.
  * In this case, emulate a valid VPD response.
  */
-if (ret == 0 &&
+if (s->needs_vpd_bl_emulation && ret == 0 &&
 (r->io_header.driver_status & SG_ERR_DRIVER_SENSE) &&
-scsi_parse_sense_buf(r->req.sense, r->io_header.sb_len_wr).key
== ILLEGAL_REQUEST &&
-s->needs_vpd_bl_emulation) {
-int is_vpd_bl = r->req.cmd.buf[0] == INQUIRY &&
- r->req.cmd.buf[1] & 0x01 &&
- r->req.cmd.buf[2] == 0xb0;
-
-if (is_vpd_bl) {
+r->req.cmd.buf[0] == INQUIRY &&
+(r->req.cmd.buf[1] & 0x01) &&
+r->req.cmd.buf[2] == 0xb0) {
+SCSISense sense =
+scsi_parse_sense_buf(r->req.sense, r->io_header.sb_len_wr);
+if (sense.key == ILLEGAL_REQUEST) {
 len = scsi_generic_emulate_block_limits(r, s);
 /*
  * No need to let scsi_read_complete go on and handle an
diff --git a/include/hw/scsi/emulation.h b/include/hw/scsi/emulation.h
index 42de7c30c2..09fba1ff39 100644
--- a/include/hw/scsi/emulation.h
+++ b/include/hw/scsi/emulation.h
@@ -11,6 +11,6 @@ typedef struct SCSIBlockLimits {
 uint32_t max_io_sectors;
 } SCSIBlockLimits;

-int scsi_emulate_block_limits(uint8_t *outbuf, SCSIBlockLimits *bl);
+int scsi_emulate_block_limits(uint8_t *outbuf, const SCSIBlockLimits *bl);

 #endif

with most of the changes being just cosmetic to avoid the long line
without reindenting everything.

Paolo

> Reviewed-by: Max Reitz 
> 
>> +stw_be_p(buf + 2, len)

Re: [Qemu-devel] [PATCH 10/18] json: Define new QMP message for pvrdma

2018-11-06 Thread Yuval Shaia
On Mon, Nov 05, 2018 at 09:58:56AM -0600, Eric Blake wrote:
> On 11/5/18 6:45 AM, Yuval Shaia wrote:
> > pvrdma requires that the same GID attached to it will be attached to the
> > backend device in the host.
> > 
> > A new QMP messages is defined so pvrdma device can broadcast any change
> > made to its GID table. This event is captured by libvirt which in turn
> > will update the GID table in the backend device.
> > 
> > Signed-off-by: Yuval Shaia 
> > ---
> 
> > +++ b/qapi/rdma.json
> > @@ -0,0 +1,34 @@
> > +# -*- Mode: Python -*-
> > +#
> > +
> > +##
> > +# = RDMA device
> > +##
> > +
> > +##
> > +# @RDMA_GID_STATUS_CHANGED:
> > +#
> > +# Emitted when guest driver adds/deletes GID to/from device
> > +#
> > +# @netdev: RoCE Network Device name - char *
> > +#
> > +# @gid-status: Add or delete indication - bool
> > +#
> > +# @subnet-prefix: Subnet Prefix - uint64
> > +#
> > +# @interface-id : Interface ID - uint64
> > +#
> > +# Since: 2.12.1
> 
> You've missed 2.12.1 by a long shot. Since we are in soft freeze and this is
> a new feature, the soonest this can be released is in 3.2.

Sure, will do the change.

Thanks.

> 
> > +#
> > +# Example:
> > +#
> > +# <- { {"timestamp": {"seconds": 1540819325, "microseconds": 504544},
> > +#   "event": "ADDGID", "data": {"netdev": "bridge0",
> > +#   "interface-id": 7052258031502978997, "subnet-prefix": 33022}}
> > +#
> > +##
> > +{ 'event': 'RDMA_GID_STATUS_CHANGED',
> > +  'data': { 'netdev': 'str',
> > +'gid-status': 'bool',
> > +'subnet-prefix' : 'uint64',
> > +'interface-id'  : 'uint64' } }
> > 
> 
> -- 
> Eric Blake, Principal Software Engineer
> Red Hat, Inc.   +1-919-301-3266
> Virtualization:  qemu.org | libvirt.org



[Qemu-devel] How to use the 'canon-a1100' machine?

2018-11-06 Thread Thomas Huth


 Hi,

does anybody know whether the "canon-a1100" machine in QEMU is still
usable, or rather how it can be used at all?

According to
http://lists.infradead.org/pipermail/barebox/2014-July/020327.html there
should be support for "-serial stdio" and on
https://lists.gnu.org/archive/html/qemu-devel/2013-10/msg02969.html
there is a link to a firmware image ... but when I try to use it, there
is simply no output on stdio...

Is there a way this machine can still be tested in QEMU?

 Thomas



Re: [Qemu-devel] [PULL 03/48] qemu-timer: introduce timer attributes

2018-11-06 Thread Paolo Bonzini
On 06/11/2018 00:16, Eric Blake wrote:
> On 10/18/18 3:31 PM, Paolo Bonzini wrote:
>> From: Artem Pisarenko 
>>
>> Attributes are simple flags, associated with individual timers for their
>> whole lifetime.  They intended to be used to mark individual timers for
>> special handling when they fire.
>>
>> New/init functions family in timer interface updated and refactored (new
>> 'attribute' argument added, timer_list replaced with
>> timer_list_group+type
>> combinations, comments improved to avoid info duplication).  Also
>> existing
>> aio interface extended with attribute-enabled variants of functions,
>> which create/initialize timers.
>>
>> Signed-off-by: Artem Pisarenko 
>> Message-Id:
>> 
>>
>> Signed-off-by: Paolo Bonzini 
>> ---
> 
> git bisect points to this patch as the reason that 'make check' is
> failing for me with:

I think you have a stale file somewhere, because "git grep timer_init_tl
89a603a0c80ae3d6a8711571550b2ae9a01ea909" shows no occurrences.

Paolo



Re: [Qemu-devel] [PATCH 0/2] target/arm: fix some ATS* bugs

2018-11-06 Thread Fam Zheng
On Tue, 11/06 01:50, no-re...@patchew.org wrote:
> ERROR: unknown option --with-gtkabi=3.0

Patchew is testing old series branches of which are heavily lagging behind. This
configure option in the mingw docker testing is recently dropped, so it's a
false positive.

Fam


> Try '/tmp/qemu-test/src/configure --help' for more information
> # QEMU configure log Tue Nov  6 09:50:16 UTC 2018
> # Configured with: '/tmp/qemu-test/src/configure' '--enable-werror' 
> '--target-list=x86_64-softmmu,aarch64-softmmu' 
> '--prefix=/tmp/qemu-test/install' '--python=/usr/bin/python3' 
> '--cross-prefix=x86_64-w64-mingw32-' '--enable-trace-backends=simple' 
> '--enable-gnutls' '--enable-nettle' '--enable-curl' '--enable-vnc' 
> '--enable-bzip2' '--enable-guest-agent' '--with-sdlabi=2.0' 
> '--with-gtkabi=3.0'
> #
> 
> funcs: do_compiler do_cc compile_object check_define main
> lines: 92 119 608 625 0
> x86_64-w64-mingw32-gcc -D_GNU_SOURCE -D_FILE_OFFSET_BITS=64 
> -D_LARGEFILE_SOURCE -Wstrict-prototypes -Wredundant-decls -Wall -Wundef 
> -Wwrite-strings -Wmissing-prototypes -fno-strict-aliasing -fno-common -fwrapv 
> -c -o config-temp/qemu-conf.o config-temp/qemu-conf.c
> config-temp/qemu-conf.c:2:2: error: #error __linux__ not defined
>  #error __linux__ not defined
>   ^
> 
> funcs: do_compiler do_cc compile_object check_define main
> lines: 92 119 608 627 0
> x86_64-w64-mingw32-gcc -D_GNU_SOURCE -D_FILE_OFFSET_BITS=64 
> -D_LARGEFILE_SOURCE -Wstrict-prototypes -Wredundant-decls -Wall -Wundef 
> -Wwrite-strings -Wmissing-prototypes -fno-strict-aliasing -fno-common -fwrapv 
> -c -o config-temp/qemu-conf.o config-temp/qemu-conf.c
> 
> funcs: do_compiler do_cc compile_object check_define main
> lines: 92 119 608 677 0
> x86_64-w64-mingw32-gcc -D_GNU_SOURCE -D_FILE_OFFSET_BITS=64 
> -D_LARGEFILE_SOURCE -Wstrict-prototypes -Wredundant-decls -Wall -Wundef 
> -Wwrite-strings -Wmissing-prototypes -fno-strict-aliasing -fno-common -fwrapv 
> -c -o config-temp/qemu-conf.o config-temp/qemu-conf.c
> config-temp/qemu-conf.c:2:2: error: #error __i386__ not defined
>  #error __i386__ not defined
>   ^
> 
> funcs: do_compiler do_cc compile_object check_define main
> lines: 92 119 608 679 0
> x86_64-w64-mingw32-gcc -D_GNU_SOURCE -D_FILE_OFFSET_BITS=64 
> -D_LARGEFILE_SOURCE -Wstrict-prototypes -Wredundant-decls -Wall -Wundef 
> -Wwrite-strings -Wmissing-prototypes -fno-strict-aliasing -fno-common -fwrapv 
> -c -o config-temp/qemu-conf.o config-temp/qemu-conf.c
> 
> funcs: do_compiler do_cc compile_object check_define main
> lines: 92 119 608 680 0
> x86_64-w64-mingw32-gcc -D_GNU_SOURCE -D_FILE_OFFSET_BITS=64 
> -D_LARGEFILE_SOURCE -Wstrict-prototypes -Wredundant-decls -Wall -Wundef 
> -Wwrite-strings -Wmissing-prototypes -fno-strict-aliasing -fno-common -fwrapv 
> -c -o config-temp/qemu-conf.o config-temp/qemu-conf.c
> config-temp/qemu-conf.c:2:2: error: #error __ILP32__ not defined
>  #error __ILP32__ not defined
>   ^
> 
> funcs: do_compiler do_cc compile_object check_include main
> lines: 92 119 616 771 0
> x86_64-w64-mingw32-gcc -D_GNU_SOURCE -D_FILE_OFFSET_BITS=64 
> -D_LARGEFILE_SOURCE -Wstrict-prototypes -Wredundant-decls -Wall -Wundef 
> -Wwrite-strings -Wmissing-prototypes -fno-strict-aliasing -fno-common -fwrapv 
> -c -o config-temp/qemu-conf.o config-temp/qemu-conf.c
> 
> funcs: do_compiler do_cc compile_prog main
> lines: 92 125 907 0
> x86_64-w64-mingw32-gcc -mthreads -D__USE_MINGW_ANSI_STDIO=1 
> -DWIN32_LEAN_AND_MEAN -DWINVER=0x501 -D_GNU_SOURCE -D_FILE_OFFSET_BITS=64 
> -D_LARGEFILE_SOURCE -Wstrict-prototypes -Wredundant-decls -Wall -Wundef 
> -Wwrite-strings -Wmissing-prototypes -fno-strict-aliasing -fno-common -fwrapv 
> -o config-temp/qemu-conf.exe config-temp/qemu-conf.c -g -liberty
> /usr/lib/gcc/x86_64-w64-mingw32/7.3.0/../../../../x86_64-w64-mingw32/bin/ld: 
> cannot find -liberty
> collect2: error: ld returned 1 exit status
> Failed to run 'configure'
> Traceback (most recent call last):
>   File "./tests/docker/docker.py", line 563, in 
> sys.exit(main())
>   File "./tests/docker/docker.py", line 560, in main
> return args.cmdobj.run(args, argv)
>   File "./tests/docker/docker.py", line 306, in run
> return Docker().run(argv, args.keep, quiet=args.quiet)
>   File "./tests/docker/docker.py", line 274, in run
> quiet=quiet)
>   File "./tests/docker/docker.py", line 181, in _do_check
> return subprocess.check_call(self._command + cmd, **kwargs)
>   File "/usr/lib64/python2.7/subprocess.py", line 186, in check_call
> raise CalledProcessError(retcode, cmd)
> subprocess.CalledProcessError: Command '['sudo', '-n', 'docker', 'run', 
> '--label', 'com.qemu.instance.uuid=5bcf7f1ce1a911e88b3152540069c830', '-u', 
> '1000', '--security-opt', 'seccomp=unconfined', '--rm', '--net=none', '-e', 
> 'TARGET_LIST=', '-e', 'EXTRA_CONFIGURE_OPTS=', '-e', 'V=', '-e', 'J=8', '-e', 
> 'DEBUG=', '-e', 'SHOW_ENV=1', '-e', 'CCACHE_DIR=/var/tmp/ccache', '-v', 
> '/home/patchew/.cache/qemu-docker-ccache:/var

Re: [Qemu-devel] [PULL 08/10] memory-mapping: skip non-volatile memory regions in GuestPhysBlockList

2018-11-06 Thread Paolo Bonzini
On 05/11/2018 16:37, Laszlo Ersek wrote:
> On 10/30/18 20:50, Paolo Bonzini wrote:
>> From: Marc-André Lureau 
>>
>> GuestPhysBlockList is currently used to produce dumps. Given the size
>> and the typical usage of NVDIMM for storage, they are not a good idea
>> to have in the dumps. We may want to have an extra dump option to
>> include them. For now, skip non-volatile regions.
>>
>> The TCG memory clear function is going to use the GuestPhysBlockList
>> as well, and will thus skip NVDIMM for similar reasons.
>>
>> Cc: ler...@redhat.com
>> Signed-off-by: Marc-André Lureau 
>> Message-Id: <20181003114454.5662-4-marcandre.lur...@redhat.com>
>> Signed-off-by: Paolo Bonzini 
>> ---
>>  memory_mapping.c | 3 ++-
>>  1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/memory_mapping.c b/memory_mapping.c
>> index 775466f..724dd0b 100644
>> --- a/memory_mapping.c
>> +++ b/memory_mapping.c
>> @@ -206,7 +206,8 @@ static void guest_phys_blocks_region_add(MemoryListener 
>> *listener,
>>  
>>  /* we only care about RAM */
>>  if (!memory_region_is_ram(section->mr) ||
>> -memory_region_is_ram_device(section->mr)) {
>> +memory_region_is_ram_device(section->mr) ||
>> +memory_region_is_nonvolatile(section->mr)) {
>>  return;
>>  }
>>  
>>
> 
> This patch misses my R-b, and (in chronological order) DavidH's, from:
> 
> http://mid.mail-archive.com/9fa8a684-8d5d-1644-3aee-86a196d31f8d@redhat.com
> http://mid.mail-archive.com/79e58e5c-4d78-e93d-ebe8-4b1bb65752fe@redhat.com

Fixed, thanks.

Paolo



Re: [Qemu-devel] [PATCH 0/2] target/arm: fix some ATS* bugs

2018-11-06 Thread no-reply
Hi,

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

Type: series
Message-id: 20181016093703.10637-1-peter.mayd...@linaro.org
Subject: [Qemu-devel] [PATCH 0/2] target/arm: fix some ATS* bugs

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

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
Switched to a new branch 'test'
7118b6bc32 target/arm: Fix ATS1Hx instructions
2e6d932650 target/arm: Set S and PTW in 64-bit PAR format

=== OUTPUT BEGIN ===
  BUILD   fedora
make[1]: Entering directory '/var/tmp/patchew-tester-tmp-j_cwgv70/src'
  GEN 
/var/tmp/patchew-tester-tmp-j_cwgv70/src/docker-src.2018-11-06-04.49.59.28947/qemu.tar
Cloning into 
'/var/tmp/patchew-tester-tmp-j_cwgv70/src/docker-src.2018-11-06-04.49.59.28947/qemu.tar.vroot'...
done.
Your branch is up-to-date with 'origin/test'.
Submodule 'dtc' (git://git.qemu-project.org/dtc.git) registered for path 'dtc'
Cloning into 
'/var/tmp/patchew-tester-tmp-j_cwgv70/src/docker-src.2018-11-06-04.49.59.28947/qemu.tar.vroot/dtc'...
Submodule path 'dtc': checked out '88f18909db731a627456f26d779445f84e449536'
Submodule 'ui/keycodemapdb' (git://git.qemu.org/keycodemapdb.git) registered 
for path 'ui/keycodemapdb'
Cloning into 
'/var/tmp/patchew-tester-tmp-j_cwgv70/src/docker-src.2018-11-06-04.49.59.28947/qemu.tar.vroot/ui/keycodemapdb'...
Submodule path 'ui/keycodemapdb': checked out 
'6b3d716e2b6472eb7189d3220552280ef3d832ce'
  COPYRUNNER
RUN test-mingw in qemu:fedora 
Packages installed:
SDL2-devel-2.0.8-5.fc28.x86_64
bc-1.07.1-5.fc28.x86_64
bison-3.0.4-9.fc28.x86_64
bluez-libs-devel-5.50-1.fc28.x86_64
brlapi-devel-0.6.7-19.fc28.x86_64
bzip2-1.0.6-26.fc28.x86_64
bzip2-devel-1.0.6-26.fc28.x86_64
ccache-3.4.2-2.fc28.x86_64
clang-6.0.1-1.fc28.x86_64
device-mapper-multipath-devel-0.7.4-3.git07e7bd5.fc28.x86_64
findutils-4.6.0-19.fc28.x86_64
flex-2.6.1-7.fc28.x86_64
gcc-8.1.1-5.fc28.x86_64
gcc-c++-8.1.1-5.fc28.x86_64
gettext-0.19.8.1-14.fc28.x86_64
git-2.17.1-3.fc28.x86_64
glib2-devel-2.56.1-4.fc28.x86_64
glusterfs-api-devel-4.1.2-2.fc28.x86_64
gnutls-devel-3.6.3-3.fc28.x86_64
gtk3-devel-3.22.30-1.fc28.x86_64
hostname-3.20-3.fc28.x86_64
libaio-devel-0.3.110-11.fc28.x86_64
libasan-8.1.1-5.fc28.x86_64
libattr-devel-2.4.48-3.fc28.x86_64
libcap-devel-2.25-9.fc28.x86_64
libcap-ng-devel-0.7.9-4.fc28.x86_64
libcurl-devel-7.59.0-6.fc28.x86_64
libfdt-devel-1.4.6-5.fc28.x86_64
libpng-devel-1.6.34-6.fc28.x86_64
librbd-devel-12.2.7-1.fc28.x86_64
libssh2-devel-1.8.0-7.fc28.x86_64
libubsan-8.1.1-5.fc28.x86_64
libusbx-devel-1.0.22-1.fc28.x86_64
libxml2-devel-2.9.8-4.fc28.x86_64
llvm-6.0.1-6.fc28.x86_64
lzo-devel-2.08-12.fc28.x86_64
make-4.2.1-6.fc28.x86_64
mingw32-SDL2-2.0.5-3.fc27.noarch
mingw32-bzip2-1.0.6-9.fc27.noarch
mingw32-curl-7.57.0-1.fc28.noarch
mingw32-glib2-2.56.1-1.fc28.noarch
mingw32-gmp-6.1.2-2.fc27.noarch
mingw32-gnutls-3.6.2-1.fc28.noarch
mingw32-gtk3-3.22.30-1.fc28.noarch
mingw32-libjpeg-turbo-1.5.1-3.fc27.noarch
mingw32-libpng-1.6.29-2.fc27.noarch
mingw32-libssh2-1.8.0-3.fc27.noarch
mingw32-libtasn1-4.13-1.fc28.noarch
mingw32-nettle-3.4-1.fc28.noarch
mingw32-pixman-0.34.0-3.fc27.noarch
mingw32-pkg-config-0.28-9.fc27.x86_64
mingw64-SDL2-2.0.5-3.fc27.noarch
mingw64-bzip2-1.0.6-9.fc27.noarch
mingw64-curl-7.57.0-1.fc28.noarch
mingw64-glib2-2.56.1-1.fc28.noarch
mingw64-gmp-6.1.2-2.fc27.noarch
mingw64-gnutls-3.6.2-1.fc28.noarch
mingw64-gtk3-3.22.30-1.fc28.noarch
mingw64-libjpeg-turbo-1.5.1-3.fc27.noarch
mingw64-libpng-1.6.29-2.fc27.noarch
mingw64-libssh2-1.8.0-3.fc27.noarch
mingw64-libtasn1-4.13-1.fc28.noarch
mingw64-nettle-3.4-1.fc28.noarch
mingw64-pixman-0.34.0-3.fc27.noarch
mingw64-pkg-config-0.28-9.fc27.x86_64
ncurses-devel-6.1-5.20180224.fc28.x86_64
nettle-devel-3.4-2.fc28.x86_64
nss-devel-3.38.0-1.0.fc28.x86_64
numactl-devel-2.0.11-8.fc28.x86_64
package PyYAML is not installed
package libjpeg-devel is not installed
perl-5.26.2-413.fc28.x86_64
pixman-devel-0.34.0-8.fc28.x86_64
python3-3.6.5-1.fc28.x86_64
snappy-devel-1.1.7-5.fc28.x86_64
sparse-0.5.2-1.fc28.x86_64
spice-server-devel-0.14.0-4.fc28.x86_64
systemtap-sdt-devel-3.3-1.fc28.x86_64
tar-1.30-3.fc28.x86_64
usbredir-devel-0.8.0-1.fc28.x86_64
virglrenderer-devel-0.6.0-4.20170210git76b3da97b.fc28.x86_64
vte3-devel-0.36.5-6.fc28.x86_64
which-2.21-8.fc28.x86_64
xen-devel-4.10.1-5.fc28.x86_64
zlib-devel-1.2.11-8.fc28.x86_64

Environment variables:
TARGET_LIST=
PACKAGES=bc bison bluez-libs-devel brlapi-devel bzip2 
bzip2-devel ccache clang device-mapper-multipath-devel 
findutils flex gcc gcc-c++ gettext git glib2-devel 
glusterfs-api-devel gnutls-devel gtk3-devel hostname 
libaio-devel libasan libattr-devel libcap-devel libcap-ng-devel 
libcurl-devel libfdt-devel libjpeg-devel libpng-devel 
librbd-

Re: [Qemu-devel] [PULL 09/10] scripts/dump-guest-memory: Synchronize with guest_phys_blocks_region_add

2018-11-06 Thread Paolo Bonzini
On 05/11/2018 16:46, Laszlo Ersek wrote:
> On 10/30/18 20:50, Paolo Bonzini wrote:
>> Recent patches have removed ram_device and nonvolatile RAM
>> from dump-guest-memory's output.  Do the same for dumps
>> that are extracted from a QEMU core file.
>>
>> Reviewed-by: Marc-André Lureau 
>> Signed-off-by: Paolo Bonzini 
>> ---
>>  scripts/dump-guest-memory.py | 4 +++-
>>  1 file changed, 3 insertions(+), 1 deletion(-)
>>
>> diff --git a/scripts/dump-guest-memory.py b/scripts/dump-guest-memory.py
>> index 5a857ce..f04697b 100644
>> --- a/scripts/dump-guest-memory.py
>> +++ b/scripts/dump-guest-memory.py
>> @@ -417,7 +417,9 @@ def get_guest_phys_blocks():
>>  memory_region = flat_range["mr"].dereference()
>>  
>>  # we only care about RAM
>> -if not memory_region["ram"]:
>> +if not memory_region["ram"] \
>> +   or memory_region["ram_device"] \
>> +   or memory_region["nonvolatile"]:
>>  continue
>>  
>>  section_size = int128_get64(flat_range["addr"]["size"])
>>
> 
> Sorry about the late comment, I've been away.
> 
> The line continuation style in the python script is inconsistent. When I
> wrote the original version, my understanding was that the "Pythonic" way
> to break up lines was to open a new parenthesized subexpression. This
> way the logical "or" operator could be left at the end of the line. See
> e.g. in the "get_guest_phys_blocks" method.
> 
> https://www.python.org/dev/peps/pep-0008/#maximum-line-length
> 
>> The preferred way of wrapping long lines is by using Python's implied
>> line continuation inside parentheses, brackets and braces. Long lines
>> can be broken over multiple lines by wrapping expressions in
>> parentheses. These should be used in preference to using a backslash
>> for line continuation.
> 
> However, several trailing backslashes have been added since, and I've
> totally failed to catch them. I guess at this point either style should
> be acceptable, in this script.
> 
> Reviewed-by: Laszlo Ersek 

Fixed this one, thanks.

Paolo



Re: [Qemu-devel] [PATCH v4] Add arm SBSA reference machine

2018-11-06 Thread Hongbo Zhang
On 6 November 2018 at 00:31, Peter Maydell  wrote:
> On 19 October 2018 at 09:55, Hongbo Zhang  wrote:
>> For the Aarch64, there is one machine 'virt', it is primarily meant to
>> run on KVM and execute virtualization workloads, but we need an
>> environment as faithful as possible to physical hardware,  to support
>> firmware and OS development for pysical Aarch64 machines.
>>
>> See the patch commit comments for the features description.
>>
>> V4 changes:
>>  - rebased to v3.0.0
>>  - removed timer, uart, rtc, *hci device tree nodes
>>(others were removerd in v3)
>>  - other minore codes clean up, mainly unsed header files, comments etc.
>>
>> V3 changes:
>>  - rename the platform 'sbsa-ref'
>>  - move all the codes to a separate file sbsa-ref.c
>>  - remove paravirtualized fw_cfg device
>>  - do not supply ACPI tables, since firmware will do it
>>  - supply only necessary DT nodes
>>  - and other minor code clean up
>
> Hi; just a note that this series isn't going to make it into
> the 3.1 release. It is still on my to-review queue for 4.0,
> but I may not get to reviewing it for a little bit while I
> deal with for-3.1 work.
>
Get it, thanks.

> thanks
> -- PMM



Re: [Qemu-devel] [PATCH v4 03/23] hw: acpi: Export the RSDP build API

2018-11-06 Thread Paolo Bonzini
On 02/11/2018 10:56, Philippe Mathieu-Daudé wrote:
>>>
>>>   }
>>>   /* RSDP */
>>> -static GArray *
>>> +static void
>>>   build_rsdp(GArray *rsdp_table, BIOSLinker *linker, unsigned
>>> xsdt_tbl_offset)
>>>   {
>>>   AcpiRsdpDescriptor *rsdp = acpi_data_push(rsdp_table, sizeof
>>> *rsdp);
>>
>> Why change this? It's not related to your patch purpose.
> 
> This patch updates include/hw/acpi/aml-build.h to export the
> build_rsdp() function.
> Since this file includes this header, the orototype needs to match.

But then it should also remove the "return".  I think something is
broken for bisectability between this patch and patch 4.

Perhaps this patch should already rename the function to
build_rsdp_rsdt, and patch 4 should move build_rsdp from hw/arm to hw/acpi?

Paolo



Re: [Qemu-devel] [PATCH] hw/arm/virt: remove unused header files

2018-11-06 Thread Hongbo Zhang
On 5 November 2018 at 21:26, Auger Eric  wrote:
> Hi Hongbo,
>
> On 10/23/18 12:21 PM, Hongbo Zhang wrote:
>> Well, after checking manually, the header files
>> "hw/arm/primecell.h"
>> "qapi/visitor.h"
>> "hw/arm/smmuv3.h"
>> are really not used in virt.c, still can be removed.
>>
>> On 22 October 2018 at 18:17, Hongbo Zhang  wrote:
>>> On 22 October 2018 at 09:37, Hongbo Zhang  wrote:
 On 19 October 2018 at 19:59, Peter Maydell  
 wrote:
> On 19 October 2018 at 11:18, Hongbo Zhang  wrote:
>> Remove the unused herder files, 'virt' can be compiled and run without
>> including them.
>>
>> Signed-off-by: Hongbo Zhang 
>> ---
>>  hw/arm/virt.c | 8 
>>  1 file changed, 8 deletions(-)
>>
>> diff --git a/hw/arm/virt.c b/hw/arm/virt.c
>> index 9f67782..f0066cb 100644
>> --- a/hw/arm/virt.c
>> +++ b/hw/arm/virt.c
>> @@ -30,9 +30,6 @@
>>
>>  #include "qemu/osdep.h"
>>  #include "qapi/error.h"
>> -#include "hw/sysbus.h"
>> -#include "hw/arm/arm.h"
>> -#include "hw/arm/primecell.h"
>
> Dropping primecell.h makes sense, we don't use what it provides.
> But I suspect the others are "unused" only because some other
> header we include drags them in implicitly. I think it's better
> to explicitly include what we need, in case that other header
> changes in future.
>
>>> I think at least another "hw/arm/smmuv3.h" can be removed too, even if
>>> we want to explicit including.
> Yes I think you can safely remove this one.
>
Thanks.

As my summary, I think these three can be removed,
"hw/arm/primecell.h"
"qapi/visitor.h"
"hw/arm/smmuv3.h"
Peter confirmed the first, Auger confirmed the third, anybody has
comments for the second? so that I can send out a v2 for this.

> Thanks
>
> Eric
>>> Will check others one by one.
>>>
> How did you determine which #includes to drop here?
>
 Well, while I was working on the 'sbsa-ref' machine, I believed I
 could remove some header files because I deleted some functions for
 that platform, so I did.
 And later, I thought I can try to test to remove part of what I did
 for 'sbsa-ref' for 'virt' too, tests showed me that 'virt'  can be
 compiled and run without them.

> thanks
> -- PMM
>>



Re: [Qemu-devel] How to use the 'canon-a1100' machine?

2018-11-06 Thread Peter Maydell
On 6 November 2018 at 09:40, Thomas Huth  wrote:
>
>  Hi,
>
> does anybody know whether the "canon-a1100" machine in QEMU is still
> usable, or rather how it can be used at all?
>
> According to
> http://lists.infradead.org/pipermail/barebox/2014-July/020327.html there
> should be support for "-serial stdio" and on
> https://lists.gnu.org/archive/html/qemu-devel/2013-10/msg02969.html
> there is a link to a firmware image ... but when I try to use it, there
> is simply no output on stdio...
>
> Is there a way this machine can still be tested in QEMU?

I have an image in my test set:
$ ~/test-images/digic/runme ./build/x86/arm-softmmu/qemu-system-arm
Switch to console [cs0]


barebox 2013.12.0-00160-g2e82c4a #507 Thu Dec 12 12:42:43 MSK 2013


Board: Canon PowerShot A1100 IS
digic-gpio c022.gpio: probed gpiochip-1 with base 0
cfi_flash f800.flash: found cfi flash at f800, size 4194304
malloc space: 0x0010 -> 0x002f (size 2 MiB)
Open /dev/env0 No such file or directory
no valid environment found on /dev/env0. Using default environment
running /env/bin/init...
canon> /

Command line is:

${QEMU} \
  -M canon-a1100 -serial stdio -display none \
  --machine firmware="$TESTDIR"/canon-a1100-rom1.bin

filename suggests that's the same firmware image you link to?

$ md5sum ~/test-images/digic/canon-a1100-rom1.bin
66e76691cbd8b140549c878ae3849147
/home/petmay01/test-images/digic/canon-a1100-rom1.bin

thanks
-- PMM



[Qemu-devel] [PATCH v2 1/6] move ObjectClass to typedefs.h

2018-11-06 Thread Gerd Hoffmann
Signed-off-by: Gerd Hoffmann 
---
 include/qemu/typedefs.h | 1 +
 include/qom/object.h| 1 -
 2 files changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/qemu/typedefs.h b/include/qemu/typedefs.h
index 3ec0e13a96..fed53f6de2 100644
--- a/include/qemu/typedefs.h
+++ b/include/qemu/typedefs.h
@@ -62,6 +62,7 @@ typedef struct NetClientState NetClientState;
 typedef struct NetFilterState NetFilterState;
 typedef struct NICInfo NICInfo;
 typedef struct NumaNodeMem NumaNodeMem;
+typedef struct ObjectClass ObjectClass;
 typedef struct PCIBridge PCIBridge;
 typedef struct PCIBus PCIBus;
 typedef struct PCIDevice PCIDevice;
diff --git a/include/qom/object.h b/include/qom/object.h
index f0b0bf39cc..499e1fd8b7 100644
--- a/include/qom/object.h
+++ b/include/qom/object.h
@@ -20,7 +20,6 @@
 struct TypeImpl;
 typedef struct TypeImpl *Type;
 
-typedef struct ObjectClass ObjectClass;
 typedef struct Object Object;
 
 typedef struct TypeInfo TypeInfo;
-- 
2.9.3




[Qemu-devel] [PATCH v2 2/6] add QemuSupportState

2018-11-06 Thread Gerd Hoffmann
Indicates support state for something (device, backend, subsystem, ...)
in qemu.  Add QemuSupportState field to ObjectClass.  Add some support
code.

TODO: wire up to qom-list-types

Signed-off-by: Gerd Hoffmann 
---
 include/qemu/support-state.h | 17 +
 include/qom/object.h |  3 +++
 util/support-state.c | 26 ++
 qapi/common.json | 32 
 util/Makefile.objs   |  1 +
 5 files changed, 79 insertions(+)
 create mode 100644 include/qemu/support-state.h
 create mode 100644 util/support-state.c

diff --git a/include/qemu/support-state.h b/include/qemu/support-state.h
new file mode 100644
index 00..6567d8702b
--- /dev/null
+++ b/include/qemu/support-state.h
@@ -0,0 +1,17 @@
+#ifndef QEMU_SUPPORT_STATE_H
+#define QEMU_SUPPORT_STATE_H
+
+#include "qapi/qapi-types-common.h"
+
+typedef struct QemuSupportState {
+SupportState state;
+const char   *help;
+} QemuSupportState;
+
+void qemu_warn_support_state(const char *type, const char *name,
+ ObjectClass *oc);
+
+bool qemu_is_deprecated(ObjectClass *oc);
+bool qemu_is_obsolete(ObjectClass *oc);
+
+#endif /* QEMU_SUPPORT_STATE_H */
diff --git a/include/qom/object.h b/include/qom/object.h
index 499e1fd8b7..2b5f27bf85 100644
--- a/include/qom/object.h
+++ b/include/qom/object.h
@@ -15,6 +15,7 @@
 #define QEMU_OBJECT_H
 
 #include "qapi/qapi-builtin-types.h"
+#include "qemu/support-state.h"
 #include "qemu/queue.h"
 
 struct TypeImpl;
@@ -399,6 +400,8 @@ struct ObjectClass
 ObjectUnparent *unparent;
 
 GHashTable *properties;
+
+QemuSupportState supported;
 };
 
 /**
diff --git a/util/support-state.c b/util/support-state.c
new file mode 100644
index 00..3170dbd15d
--- /dev/null
+++ b/util/support-state.c
@@ -0,0 +1,26 @@
+#include "qemu/osdep.h"
+#include "qemu/error-report.h"
+#include "qemu/support-state.h"
+#include "qom/object.h"
+
+void qemu_warn_support_state(const char *type, const char *name,
+ ObjectClass *oc)
+{
+const char *help = oc->supported.help;
+
+warn_report("%s %s is %s%s%s%s", type, name,
+SupportState_str(oc->supported.state),
+help ? " (" : "",
+help ? help : "",
+help ? ")"  : "");
+}
+
+bool qemu_is_deprecated(ObjectClass *oc)
+{
+return oc->supported.state == SUPPORT_STATE_DEPRECATED;
+}
+
+bool qemu_is_obsolete(ObjectClass *oc)
+{
+return oc->supported.state == SUPPORT_STATE_OBSOLETE;
+}
diff --git a/qapi/common.json b/qapi/common.json
index 021174f04e..00374127b8 100644
--- a/qapi/common.json
+++ b/qapi/common.json
@@ -151,3 +151,35 @@
  'ppc64', 'riscv32', 'riscv64', 's390x', 'sh4',
  'sh4eb', 'sparc', 'sparc64', 'tricore', 'unicore32',
  'x86_64', 'xtensa', 'xtensaeb' ] }
+
+##
+# @SupportState:
+#
+# Indicate Support level of qemu devices, backends, subsystems, ...
+#
+# @unspecified: not specified (zero-initialized).
+#
+# @experimental: in development, can be unstable or incomplete.
+#
+# @supported: works stable and is fully supported.
+# (supported + maintained in MAINTAINERS).
+#
+# @unsupported: should work, support is weak or not present.
+#   (odd-fixes + orphan in MAINTAINERS).
+#
+# @obsolete: is obsolete, still present for compatibility reasons,
+#will likely be removed at some point in the future.
+#Not deprecated (yet).
+#(obsolete in MAINTAINERS).
+#
+# @deprecated: is deprecated, according to qemu deprecation policy.
+#
+# Since: 3.2
+##
+{ 'enum': 'SupportState',
+  'data': [ 'unspecified',
+'experimental',
+'supported',
+'unsupported',
+'obsolete',
+'deprecated' ] }
diff --git a/util/Makefile.objs b/util/Makefile.objs
index 0820923c18..6e5f8faf82 100644
--- a/util/Makefile.objs
+++ b/util/Makefile.objs
@@ -50,5 +50,6 @@ util-obj-y += range.o
 util-obj-y += stats64.o
 util-obj-y += systemd.o
 util-obj-y += iova-tree.o
+util-obj-y += support-state.o
 util-obj-$(CONFIG_LINUX) += vfio-helpers.o
 util-obj-$(CONFIG_OPENGL) += drm.o
-- 
2.9.3




[Qemu-devel] [PATCH v2 0/6] Introducing QemuSupportState

2018-11-06 Thread Gerd Hoffmann
Trying to fill the need to be more finegrained on support status.

v2:
 - reduce the number of support states, add documentation for them.
 - move QemuSupportState to ObjectClass, to simplify introspection
   integration (not done yet).
 - add UsageHints.

Gerd Hoffmann (6):
  move ObjectClass to typedefs.h
  add QemuSupportState
  Use QemuSupportState for machine types.
  Warn on obsolete and deprecated devices.
  tag cirrus as obsolete
  add UsageHints to QemuSupportState

 include/hw/boards.h  |  3 ---
 include/qemu/support-state.h | 18 +
 include/qemu/typedefs.h  |  1 +
 include/qom/object.h |  4 ++-
 hw/core/qdev.c   |  8 +-
 hw/display/cirrus_vga.c  |  3 +++
 hw/display/cirrus_vga_isa.c  |  3 +++
 hw/i386/pc_piix.c|  5 +++-
 hw/ppc/prep.c|  5 +++-
 qdev-monitor.c   |  9 +++
 util/support-state.c | 26 +++
 vl.c |  7 ++---
 qapi/common.json | 61 
 util/Makefile.objs   |  1 +
 14 files changed, 144 insertions(+), 10 deletions(-)
 create mode 100644 include/qemu/support-state.h
 create mode 100644 util/support-state.c

-- 
2.9.3




Re: [Qemu-devel] [PATCH v5 03/24] hw: acpi: The RSDP build API can return void

2018-11-06 Thread Paolo Bonzini
On 05/11/2018 02:40, Samuel Ortiz wrote:
>  /* RSDP */
> -static GArray *
> +static void
>  build_rsdp(GArray *rsdp_table, BIOSLinker *linker, unsigned xsdt_tbl_offset)
>  {
>  AcpiRsdpDescriptor *rsdp = acpi_data_push(rsdp_table, sizeof *rsdp);
> @@ -392,8 +392,6 @@ build_rsdp(GArray *rsdp_table, BIOSLinker *linker, 
> unsigned xsdt_tbl_offset)
>  bios_linker_loader_add_checksum(linker, ACPI_BUILD_RSDP_FILE,
>  (char *)rsdp - rsdp_table->data, sizeof *rsdp,
>  (char *)&rsdp->checksum - rsdp_table->data);
> -
> -return rsdp_table;
>  }
>  

Better than v4. :)

Paolo



[Qemu-devel] [PATCH v2 6/6] add UsageHints to QemuSupportState

2018-11-06 Thread Gerd Hoffmann
So we can add device usage recommendations to devices,
independant from support state.

Signed-off-by: Gerd Hoffmann 
---
 include/qemu/support-state.h |  1 +
 qapi/common.json | 29 +
 2 files changed, 30 insertions(+)

diff --git a/include/qemu/support-state.h b/include/qemu/support-state.h
index 6567d8702b..6ea0d03bd5 100644
--- a/include/qemu/support-state.h
+++ b/include/qemu/support-state.h
@@ -5,6 +5,7 @@
 
 typedef struct QemuSupportState {
 SupportState state;
+UsageHints   hints;
 const char   *help;
 } QemuSupportState;
 
diff --git a/qapi/common.json b/qapi/common.json
index 00374127b8..6952ea2074 100644
--- a/qapi/common.json
+++ b/qapi/common.json
@@ -183,3 +183,32 @@
 'unsupported',
 'obsolete',
 'deprecated' ] }
+
+##
+# @UsageHints:
+#
+# Usage recommendations.
+#
+# @unspecified: not specified (zero-initialized).
+#
+# @green: Best choice.  Typically paravirtual devices go into this
+# category.
+# Example (nic): virtio-net.
+# Example (usb); xhci.
+#
+# @yellow: Reasonable choice.  Typically emulated devices with
+#  good performance go into this category.
+#  Example (nic): e1000, e1000e
+#
+# @red: Bad choice.  Avoid this unless you run an old guest which
+#   lacks support for something better.
+#   Example (nic): rtl8139, pcnet, ne2k, ...
+#   Example (usb): ehci, uhci, ohci
+#
+# Since: 3.2
+##
+{ 'enum': 'UsageHints',
+  'data': [ 'unspecified',
+'green',
+'yellow',
+'red' ] }
-- 
2.9.3




[Qemu-devel] [PATCH v2 5/6] tag cirrus as obsolete

2018-11-06 Thread Gerd Hoffmann
Standard VGA should be used instead.

Signed-off-by: Gerd Hoffmann 
---
 hw/display/cirrus_vga.c | 3 +++
 hw/display/cirrus_vga_isa.c | 3 +++
 2 files changed, 6 insertions(+)

diff --git a/hw/display/cirrus_vga.c b/hw/display/cirrus_vga.c
index d9b854d74d..8474415687 100644
--- a/hw/display/cirrus_vga.c
+++ b/hw/display/cirrus_vga.c
@@ -3024,6 +3024,9 @@ static void cirrus_vga_class_init(ObjectClass *klass, 
void *data)
 dc->vmsd = &vmstate_pci_cirrus_vga;
 dc->props = pci_vga_cirrus_properties;
 dc->hotpluggable = false;
+klass->supported.state = SUPPORT_STATE_OBSOLETE;
+klass->supported.help = "use \"-vga std\" instead, see "
+
"https://www.kraxel.org/blog/2014/10/qemu-using-cirrus-considered-harmful/";;
 }
 
 static const TypeInfo cirrus_vga_info = {
diff --git a/hw/display/cirrus_vga_isa.c b/hw/display/cirrus_vga_isa.c
index fa10b74230..6bc83f7509 100644
--- a/hw/display/cirrus_vga_isa.c
+++ b/hw/display/cirrus_vga_isa.c
@@ -81,6 +81,9 @@ static void isa_cirrus_vga_class_init(ObjectClass *klass, 
void *data)
 dc->realize = isa_cirrus_vga_realizefn;
 dc->props = isa_cirrus_vga_properties;
 set_bit(DEVICE_CATEGORY_DISPLAY, dc->categories);
+klass->supported.state = SUPPORT_STATE_OBSOLETE;
+klass->supported.help = "use \"-vga std\" instead, see "
+
"https://www.kraxel.org/blog/2014/10/qemu-using-cirrus-considered-harmful/";;
 }
 
 static const TypeInfo isa_cirrus_vga_info = {
-- 
2.9.3




[Qemu-devel] [PATCH v2 4/6] Warn on obsolete and deprecated devices.

2018-11-06 Thread Gerd Hoffmann
Print a warning for deprecated and obsolete devices.
Also add support state to device listing.

Signed-off-by: Gerd Hoffmann 
---
 hw/core/qdev.c | 8 +++-
 qdev-monitor.c | 9 +
 2 files changed, 16 insertions(+), 1 deletion(-)

diff --git a/hw/core/qdev.c b/hw/core/qdev.c
index 6b3cc55b27..6205522c3e 100644
--- a/hw/core/qdev.c
+++ b/hw/core/qdev.c
@@ -133,11 +133,17 @@ DeviceState *qdev_create(BusState *bus, const char *name)
 
 DeviceState *qdev_try_create(BusState *bus, const char *type)
 {
+ObjectClass *oc;
 DeviceState *dev;
 
-if (object_class_by_name(type) == NULL) {
+oc = object_class_by_name(type);
+if (oc == NULL) {
 return NULL;
 }
+if (qemu_is_deprecated(oc) ||
+qemu_is_obsolete(oc)) {
+qemu_warn_support_state("device", type, oc);
+}
 dev = DEVICE(object_new(type));
 if (!dev) {
 return NULL;
diff --git a/qdev-monitor.c b/qdev-monitor.c
index 802c18a74e..80370372f9 100644
--- a/qdev-monitor.c
+++ b/qdev-monitor.c
@@ -115,6 +115,8 @@ static void out_printf(const char *fmt, ...)
 
 static void qdev_print_devinfo(DeviceClass *dc)
 {
+ObjectClass *oc = OBJECT_CLASS(dc);
+
 out_printf("name \"%s\"", object_class_get_name(OBJECT_CLASS(dc)));
 if (dc->bus_type) {
 out_printf(", bus %s", dc->bus_type);
@@ -128,6 +130,9 @@ static void qdev_print_devinfo(DeviceClass *dc)
 if (!dc->user_creatable) {
 out_printf(", no-user");
 }
+if (oc->supported.state != SUPPORT_STATE_UNSPECIFIED) {
+out_printf(", %s", SupportState_str(oc->supported.state));
+}
 out_printf("\n");
 }
 
@@ -579,6 +584,10 @@ DeviceState *qdev_device_add(QemuOpts *opts, Error **errp)
 if (!dc) {
 return NULL;
 }
+if (qemu_is_deprecated(OBJECT_CLASS(dc)) ||
+qemu_is_obsolete(OBJECT_CLASS(dc))) {
+qemu_warn_support_state("device", driver, OBJECT_CLASS(dc));
+}
 
 /* find bus */
 path = qemu_opt_get(opts, "bus");
-- 
2.9.3




[Qemu-devel] [PATCH v2 3/6] Use QemuSupportState for machine types.

2018-11-06 Thread Gerd Hoffmann
Switch over the current deprecation_reason users to use
the QemuSupportState field in ObjectClass instead.

Signed-off-by: Gerd Hoffmann 
---
 include/hw/boards.h | 3 ---
 hw/i386/pc_piix.c   | 5 -
 hw/ppc/prep.c   | 5 -
 vl.c| 7 ---
 4 files changed, 12 insertions(+), 8 deletions(-)

diff --git a/include/hw/boards.h b/include/hw/boards.h
index f82f28468b..b7bb181fc8 100644
--- a/include/hw/boards.h
+++ b/include/hw/boards.h
@@ -106,8 +106,6 @@ typedef struct {
 
 /**
  * MachineClass:
- * @deprecation_reason: If set, the machine is marked as deprecated. The
- *string should provide some clear information about what to use instead.
  * @max_cpus: maximum number of CPUs supported. Default: 1
  * @min_cpus: minimum number of CPUs supported. Default: 1
  * @default_cpus: number of CPUs instantiated if none are specified. Default: 1
@@ -167,7 +165,6 @@ struct MachineClass {
 char *name;
 const char *alias;
 const char *desc;
-const char *deprecation_reason;
 
 void (*init)(MachineState *state);
 void (*reset)(void);
diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index dc09466b3e..407f4503a3 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -955,9 +955,12 @@ DEFINE_I440FX_MACHINE(v0_12, "pc-0.12", pc_compat_0_13,
 
 static void pc_i440fx_0_11_machine_options(MachineClass *m)
 {
+ObjectClass *oc = OBJECT_CLASS(m);
+
 pc_i440fx_0_12_machine_options(m);
 m->hw_version = "0.11";
-m->deprecation_reason = "use a newer machine type instead";
+oc->supported.state = SUPPORT_STATE_DEPRECATED;
+oc->supported.help = "use a newer machine type instead";
 SET_MACHINE_COMPAT(m, PC_COMPAT_0_11);
 }
 
diff --git a/hw/ppc/prep.c b/hw/ppc/prep.c
index 2afb7f437e..260700847b 100644
--- a/hw/ppc/prep.c
+++ b/hw/ppc/prep.c
@@ -588,7 +588,10 @@ static void ppc_prep_init(MachineState *machine)
 
 static void prep_machine_init(MachineClass *mc)
 {
-mc->deprecation_reason = "use 40p machine type instead";
+ObjectClass *oc = OBJECT_CLASS(mc);
+
+oc->supported.state = SUPPORT_STATE_DEPRECATED;
+oc->supported.help = "use 40p machine type instead";
 mc->desc = "PowerPC PREP platform";
 mc->init = ppc_prep_init;
 mc->block_default_type = IF_IDE;
diff --git a/vl.c b/vl.c
index 1fcacc5caa..ba81fefb66 100644
--- a/vl.c
+++ b/vl.c
@@ -2573,7 +2573,7 @@ static gint machine_class_cmp(gconstpointer a, 
gconstpointer b)
 }
 printf("%-20s %s%s%s\n", mc->name, mc->desc,
mc->is_default ? " (default)" : "",
-   mc->deprecation_reason ? " (deprecated)" : "");
+   qemu_is_deprecated(OBJECT_CLASS(mc)) ? " (deprecated)" : 
"");
 }
 }
 
@@ -4290,9 +4290,10 @@ int main(int argc, char **argv, char **envp)
 
 configure_accelerator(current_machine);
 
-if (!qtest_enabled() && machine_class->deprecation_reason) {
+if (!qtest_enabled() && qemu_is_deprecated(OBJECT_CLASS(machine_class))) {
 error_report("Machine type '%s' is deprecated: %s",
- machine_class->name, machine_class->deprecation_reason);
+ machine_class->name,
+ OBJECT_CLASS(machine_class)->supported.help);
 }
 
 /*
-- 
2.9.3




Re: [Qemu-devel] [PATCH v5 03/24] hw: acpi: The RSDP build API can return void

2018-11-06 Thread Samuel Ortiz
On Tue, Nov 06, 2018 at 11:23:39AM +0100, Paolo Bonzini wrote:
> On 05/11/2018 02:40, Samuel Ortiz wrote:
> >  /* RSDP */
> > -static GArray *
> > +static void
> >  build_rsdp(GArray *rsdp_table, BIOSLinker *linker, unsigned 
> > xsdt_tbl_offset)
> >  {
> >  AcpiRsdpDescriptor *rsdp = acpi_data_push(rsdp_table, sizeof *rsdp);
> > @@ -392,8 +392,6 @@ build_rsdp(GArray *rsdp_table, BIOSLinker *linker, 
> > unsigned xsdt_tbl_offset)
> >  bios_linker_loader_add_checksum(linker, ACPI_BUILD_RSDP_FILE,
> >  (char *)rsdp - rsdp_table->data, sizeof *rsdp,
> >  (char *)&rsdp->checksum - rsdp_table->data);
> > -
> > -return rsdp_table;
> >  }
> >  
> 
> Better than v4. :)
Right, I followed Philippe's advice and it does make things clearer :)

Cheers,
Samuel.



Re: [Qemu-devel] [Qemu-block] [PATCH 4/7] qcow2: refactor decompress_buffer

2018-11-06 Thread Alberto Garcia
On Thu 01 Nov 2018 07:27:35 PM CET, Vladimir Sementsov-Ogievskiy wrote:
> - make it look more like a pair of qcow2_compress - rename the function
>   and its parameters
> - drop extra out_len variable, check filling of output buffer by strm
>   structure itself
> - fix code style
> - add some documentation
>
> Signed-off-by: Vladimir Sementsov-Ogievskiy 

Reviewed-by: Alberto Garcia 

Berto



Re: [Qemu-devel] [PULL 0/6] final s390x patches for -rc0

2018-11-06 Thread Peter Maydell
On 5 November 2018 at 16:43, Cornelia Huck  wrote:
> The following changes since commit 7d56239f159afc2e7bd42623947e56ba48f37836:
>
>   Merge remote-tracking branch 
> 'remotes/pmaydell/tags/pull-target-arm-20181102' into staging (2018-11-02 
> 17:17:12 +)
>
> are available in the Git repository at:
>
>   https://github.com/cohuck/qemu tags/s390x-20181105
>
> for you to fetch changes up to 8e4eb4279fce9a736131b94b6f5f09f0503e4ab3:
>
>   MAINTAINERS: s390/boot: the ipl code and the bios belong together 
> (2018-11-05 09:55:29 +0100)
>
> 
> - some changes in s390x maintainership
> - bugfix in vfio-ap
>
> 
Applied, thanks.

-- PMM



Re: [Qemu-devel] How to use the 'canon-a1100' machine?

2018-11-06 Thread Thomas Huth
On 2018-11-06 11:32, Peter Maydell wrote:
> On 6 November 2018 at 09:40, Thomas Huth  wrote:
>>
>>  Hi,
>>
>> does anybody know whether the "canon-a1100" machine in QEMU is still
>> usable, or rather how it can be used at all?
>>
>> According to
>> http://lists.infradead.org/pipermail/barebox/2014-July/020327.html there
>> should be support for "-serial stdio" and on
>> https://lists.gnu.org/archive/html/qemu-devel/2013-10/msg02969.html
>> there is a link to a firmware image ... but when I try to use it, there
>> is simply no output on stdio...
>>
>> Is there a way this machine can still be tested in QEMU?
> 
> I have an image in my test set:
> $ ~/test-images/digic/runme ./build/x86/arm-softmmu/qemu-system-arm
> Switch to console [cs0]
> 
> 
> barebox 2013.12.0-00160-g2e82c4a #507 Thu Dec 12 12:42:43 MSK 2013
> 
> 
> Board: Canon PowerShot A1100 IS
> digic-gpio c022.gpio: probed gpiochip-1 with base 0
> cfi_flash f800.flash: found cfi flash at f800, size 4194304
> malloc space: 0x0010 -> 0x002f (size 2 MiB)
> Open /dev/env0 No such file or directory
> no valid environment found on /dev/env0. Using default environment
> running /env/bin/init...
> canon> /
> 
> Command line is:
> 
> ${QEMU} \
>   -M canon-a1100 -serial stdio -display none \
>   --machine firmware="$TESTDIR"/canon-a1100-rom1.bin
> 
> filename suggests that's the same firmware image you link to?
> 
> $ md5sum ~/test-images/digic/canon-a1100-rom1.bin
> 66e76691cbd8b140549c878ae3849147

Thanks for the hint! My image had a different md5sum, so I think
it's just a bad version that I've downloaded. I've finally also
managed to compile barebox on my own with these steps:

tar -xaf barebox-tar.bz2
cd barebox-...
cp ./arch/arm/configs/canon-a1100_defconfig .config
make CROSS_PREFIX=arm-linux-gnu- ARCH=arm CC=arm-linux-gnu-gcc \
 LD=arm-linux-gnu-ld OBJCOPY=arm-linux-gnu-objcopy olddefconfig
make CROSS_PREFIX=arm-linux-gnu- ARCH=arm CC=arm-linux-gnu-gcc \
 LD=arm-linux-gnu-ld OBJCOPY=arm-linux-gnu-objcopy -j8

And that version indeed prints some output via its emulated serial
console to stdio :-)

CC-ing some "tests/docker" people in case somebody wants to turn
that into a docker-based test, too...

 Thomas



[Qemu-devel] [PATCH v4 02/16] gdbstub: introduce GDB processes

2018-11-06 Thread Luc Michel
Add a structure GDBProcess that represent processes from the GDB
semantic point of view.

CPUs can be split into different processes, by grouping them under
different cpu-cluster objects.  Each occurrence of a cpu-cluster object
implies the existence of the corresponding process in the GDB stub. The
GDB process ID is derived from the corresponding cluster ID as follows:

  GDB PID = cluster ID + 1

This is because PIDs -1 and 0 are reserved in GDB and cannot be used by
processes.

When no such container are found, all the CPUs are put in a unique GDB
process (create_unique_process()). This is also the case when compiled
in user mode, where multi-processes do not make much sense for now.

Signed-off-by: Luc Michel 
Acked-by: Alistair Francis 
---
 gdbstub.c | 76 +++
 1 file changed, 76 insertions(+)

diff --git a/gdbstub.c b/gdbstub.c
index c4e4f9f082..0d70b89598 100644
--- a/gdbstub.c
+++ b/gdbstub.c
@@ -27,10 +27,11 @@
 #include "monitor/monitor.h"
 #include "chardev/char.h"
 #include "chardev/char-fe.h"
 #include "sysemu/sysemu.h"
 #include "exec/gdbstub.h"
+#include "hw/cpu/cluster.h"
 #endif
 
 #define MAX_PACKET_LENGTH 4096
 
 #include "qemu/sockets.h"
@@ -294,10 +295,15 @@ typedef struct GDBRegisterState {
 gdb_reg_cb set_reg;
 const char *xml;
 struct GDBRegisterState *next;
 } GDBRegisterState;
 
+typedef struct GDBProcess {
+uint32_t pid;
+bool attached;
+} GDBProcess;
+
 enum RSState {
 RS_INACTIVE,
 RS_IDLE,
 RS_GETLINE,
 RS_GETLINE_ESC,
@@ -322,10 +328,13 @@ typedef struct GDBState {
 int running_state;
 #else
 CharBackend chr;
 Chardev *mon_chr;
 #endif
+bool multiprocess;
+GDBProcess *processes;
+int process_num;
 char syscall_buf[256];
 gdb_syscall_complete_cb current_syscall_cb;
 } GDBState;
 
 /* By default use no IRQs and no timers while single stepping so as to
@@ -1749,10 +1758,24 @@ void gdb_exit(CPUArchState *env, int code)
 #ifndef CONFIG_USER_ONLY
   qemu_chr_fe_deinit(&s->chr, true);
 #endif
 }
 
+/*
+ * Create a unique process containing all the CPUs.
+ */
+static void create_unique_process(GDBState *s)
+{
+GDBProcess *process;
+
+s->processes = g_malloc0(sizeof(GDBProcess));
+s->process_num = 1;
+process = &s->processes[0];
+
+process->pid = 1;
+}
+
 #ifdef CONFIG_USER_ONLY
 int
 gdb_handlesig(CPUState *cpu, int sig)
 {
 GDBState *s;
@@ -1846,10 +1869,11 @@ static bool gdb_accept(void)
 }
 
 s = g_malloc0(sizeof(GDBState));
 s->c_cpu = first_cpu;
 s->g_cpu = first_cpu;
+create_unique_process(s);
 s->fd = fd;
 gdb_has_xml = false;
 
 gdbserver_state = s;
 return true;
@@ -2002,10 +2026,58 @@ static const TypeInfo char_gdb_type_info = {
 .name = TYPE_CHARDEV_GDB,
 .parent = TYPE_CHARDEV,
 .class_init = char_gdb_class_init,
 };
 
+static int find_cpu_clusters(Object *child, void *opaque)
+{
+if (object_dynamic_cast(child, TYPE_CPU_CLUSTER)) {
+GDBState *s = (GDBState *) opaque;
+CPUClusterState *cluster = CPU_CLUSTER(child);
+GDBProcess *process;
+
+s->processes = g_renew(GDBProcess, s->processes, ++s->process_num);
+
+process = &s->processes[s->process_num - 1];
+
+/* GDB process IDs -1 and 0 are reserved */
+process->pid = cluster->cluster_id + 1;
+process->attached = false;
+return 0;
+}
+
+return object_child_foreach(child, find_cpu_clusters, opaque);
+}
+
+static int pid_order(const void *a, const void *b)
+{
+GDBProcess *pa = (GDBProcess *) a;
+GDBProcess *pb = (GDBProcess *) b;
+
+return pa->pid - pb->pid;
+}
+
+static void create_processes(GDBState *s)
+{
+object_child_foreach(object_get_root(), find_cpu_clusters, s);
+
+if (!s->processes) {
+/* No CPU cluster specified by the machine */
+create_unique_process(s);
+} else {
+/* Sort by PID */
+qsort(s->processes, s->process_num, sizeof(s->processes[0]), 
pid_order);
+}
+}
+
+static void cleanup_processes(GDBState *s)
+{
+g_free(s->processes);
+s->process_num = 0;
+s->processes = NULL;
+}
+
 int gdbserver_start(const char *device)
 {
 trace_gdbstub_op_start(device);
 
 GDBState *s;
@@ -2058,15 +2130,19 @@ int gdbserver_start(const char *device)
NULL, &error_abort);
 monitor_init(mon_chr, 0);
 } else {
 qemu_chr_fe_deinit(&s->chr, true);
 mon_chr = s->mon_chr;
+cleanup_processes(s);
 memset(s, 0, sizeof(GDBState));
 s->mon_chr = mon_chr;
 }
 s->c_cpu = first_cpu;
 s->g_cpu = first_cpu;
+
+create_processes(s);
+
 if (chr) {
 qemu_chr_fe_init(&s->chr, chr, &error_abort);
 qemu_chr_fe_set_handlers(&s->chr, gdb_chr_can_receive, gdb_chr_receive,
  gdb_chr_event, NULL, NULL, NULL, true);
 }
-- 
2.19.1




[Qemu-devel] [PATCH v4 10/16] gdbstub: add multiprocess support to 'D' packets

2018-11-06 Thread Luc Michel
'D' packets are used by GDB to detach from a process. In multiprocess
mode, the PID to detach from is sent in the request.

Signed-off-by: Luc Michel 
Reviewed-by: Philippe Mathieu-Daudé 
---
 gdbstub.c | 60 ---
 1 file changed, 53 insertions(+), 7 deletions(-)

diff --git a/gdbstub.c b/gdbstub.c
index decf56c610..bd4895ac0a 100644
--- a/gdbstub.c
+++ b/gdbstub.c
@@ -1039,24 +1039,39 @@ static int gdb_breakpoint_remove(target_ulong addr, 
target_ulong len, int type)
 default:
 return -ENOSYS;
 }
 }
 
+static inline void gdb_cpu_breakpoint_remove_all(CPUState *cpu)
+{
+cpu_breakpoint_remove_all(cpu, BP_GDB);
+#ifndef CONFIG_USER_ONLY
+cpu_watchpoint_remove_all(cpu, BP_GDB);
+#endif
+}
+
+static void gdb_process_breakpoint_remove_all(const GDBState *s, GDBProcess *p)
+{
+CPUState *cpu = get_first_cpu_in_process(s, p);
+
+while (cpu) {
+gdb_cpu_breakpoint_remove_all(cpu);
+cpu = gdb_next_cpu_in_process(s, cpu);
+}
+}
+
 static void gdb_breakpoint_remove_all(void)
 {
 CPUState *cpu;
 
 if (kvm_enabled()) {
 kvm_remove_all_breakpoints(gdbserver_state->c_cpu);
 return;
 }
 
 CPU_FOREACH(cpu) {
-cpu_breakpoint_remove_all(cpu, BP_GDB);
-#ifndef CONFIG_USER_ONLY
-cpu_watchpoint_remove_all(cpu, BP_GDB);
-#endif
+gdb_cpu_breakpoint_remove_all(cpu);
 }
 }
 
 static void gdb_set_cpu_pc(GDBState *s, target_ulong pc)
 {
@@ -1331,13 +1346,44 @@ static int gdb_handle_packet(GDBState *s, const char 
*line_buf)
 /* Kill the target */
 error_report("QEMU: Terminated via GDBstub");
 exit(0);
 case 'D':
 /* Detach packet */
-gdb_breakpoint_remove_all();
-gdb_syscall_mode = GDB_SYS_DISABLED;
-gdb_continue(s);
+pid = 1;
+
+if (s->multiprocess) {
+unsigned long lpid;
+if (*p != ';') {
+put_packet(s, "E22");
+break;
+}
+
+if (qemu_strtoul(p + 1, &p, 16, &lpid)) {
+put_packet(s, "E22");
+break;
+}
+
+pid = lpid;
+}
+
+process = gdb_get_process(s, pid);
+gdb_process_breakpoint_remove_all(s, process);
+process->attached = false;
+
+if (pid == gdb_get_cpu_pid(s, s->c_cpu)) {
+s->c_cpu = gdb_first_cpu(s);
+}
+
+if (pid == gdb_get_cpu_pid(s, s->g_cpu)) {
+s->g_cpu = gdb_first_cpu(s);
+}
+
+if (s->c_cpu == NULL) {
+/* No more process attached */
+gdb_syscall_mode = GDB_SYS_DISABLED;
+gdb_continue(s);
+}
 put_packet(s, "OK");
 break;
 case 's':
 if (*p != '\0') {
 addr = strtoull(p, (char **)&p, 16);
-- 
2.19.1




[Qemu-devel] [PATCH v4 07/16] gdbstub: add multiprocess support to (f|s)ThreadInfo and ThreadExtraInfo

2018-11-06 Thread Luc Michel
Change the thread info related packets handling to support multiprocess
extension.

Add the CPUs class name in the extra info to help differentiate
them in multiprocess mode.

Signed-off-by: Luc Michel 
Reviewed-by: Philippe Mathieu-Daudé 
---
 gdbstub.c | 35 +--
 1 file changed, 25 insertions(+), 10 deletions(-)

diff --git a/gdbstub.c b/gdbstub.c
index 9be83486f5..43c9211bfa 100644
--- a/gdbstub.c
+++ b/gdbstub.c
@@ -1260,11 +1260,10 @@ out:
 static int gdb_handle_packet(GDBState *s, const char *line_buf)
 {
 CPUState *cpu;
 CPUClass *cc;
 const char *p;
-uint32_t thread;
 uint32_t pid, tid;
 int ch, reg_size, type, res;
 uint8_t mem_buf[MAX_PACKET_LENGTH];
 char buf[sizeof(mem_buf) + 1 /* trailing NUL */];
 char thread_id[16];
@@ -1556,30 +1555,46 @@ static int gdb_handle_packet(GDBState *s, const char 
*line_buf)
 snprintf(buf, sizeof(buf), "QC%s",
  gdb_fmt_thread_id(s, cpu, thread_id, sizeof(thread_id)));
 put_packet(s, buf);
 break;
 } else if (strcmp(p,"fThreadInfo") == 0) {
-s->query_cpu = first_cpu;
+s->query_cpu = gdb_first_cpu(s);
 goto report_cpuinfo;
 } else if (strcmp(p,"sThreadInfo") == 0) {
 report_cpuinfo:
 if (s->query_cpu) {
-snprintf(buf, sizeof(buf), "m%x", cpu_gdb_index(s->query_cpu));
+snprintf(buf, sizeof(buf), "m%s",
+ gdb_fmt_thread_id(s, s->query_cpu,
+   thread_id, sizeof(thread_id)));
 put_packet(s, buf);
-s->query_cpu = CPU_NEXT(s->query_cpu);
+s->query_cpu = gdb_next_cpu(s, s->query_cpu);
 } else
 put_packet(s, "l");
 break;
 } else if (strncmp(p,"ThreadExtraInfo,", 16) == 0) {
-thread = strtoull(p+16, (char **)&p, 16);
-cpu = find_cpu(thread);
+if (read_thread_id(p + 16, &p, &pid, &tid) == GDB_READ_THREAD_ERR) 
{
+put_packet(s, "E22");
+break;
+}
+cpu = gdb_get_cpu(s, pid, tid);
 if (cpu != NULL) {
 cpu_synchronize_state(cpu);
-/* memtohex() doubles the required space */
-len = snprintf((char *)mem_buf, sizeof(buf) / 2,
-   "CPU#%d [%s]", cpu->cpu_index,
-   cpu->halted ? "halted " : "running");
+
+if (s->multiprocess && (s->process_num > 1)) {
+/* Print the CPU model in multiprocess mode */
+ObjectClass *oc = object_get_class(OBJECT(cpu));
+const char *cpu_model = object_class_get_name(oc);
+len = snprintf((char *)mem_buf, sizeof(buf) / 2,
+   "CPU#%d %s [%s]", cpu->cpu_index,
+   cpu_model,
+   cpu->halted ? "halted " : "running");
+} else {
+/* memtohex() doubles the required space */
+len = snprintf((char *)mem_buf, sizeof(buf) / 2,
+   "CPU#%d [%s]", cpu->cpu_index,
+   cpu->halted ? "halted " : "running");
+}
 trace_gdbstub_op_extra_info((char *)mem_buf);
 memtohex(buf, mem_buf, len);
 put_packet(s, buf);
 }
 break;
-- 
2.19.1




[Qemu-devel] [PATCH v4 09/16] gdbstub: add multiprocess support to gdb_vm_state_change()

2018-11-06 Thread Luc Michel
Add support for multiprocess extension in gdb_vm_state_change()
function.

Signed-off-by: Luc Michel 
Reviewed-by: Philippe Mathieu-Daudé 
---
 gdbstub.c | 15 ---
 1 file changed, 12 insertions(+), 3 deletions(-)

diff --git a/gdbstub.c b/gdbstub.c
index aae3cce01a..decf56c610 100644
--- a/gdbstub.c
+++ b/gdbstub.c
@@ -1713,10 +1713,11 @@ void gdb_set_stop_cpu(CPUState *cpu)
 static void gdb_vm_state_change(void *opaque, int running, RunState state)
 {
 GDBState *s = gdbserver_state;
 CPUState *cpu = s->c_cpu;
 char buf[256];
+char thread_id[16];
 const char *type;
 int ret;
 
 if (running || s->state == RS_INACTIVE) {
 return;
@@ -1724,10 +1725,18 @@ static void gdb_vm_state_change(void *opaque, int 
running, RunState state)
 /* Is there a GDB syscall waiting to be sent?  */
 if (s->current_syscall_cb) {
 put_packet(s, s->syscall_buf);
 return;
 }
+
+if (cpu == NULL) {
+/* No process attached */
+return;
+}
+
+gdb_fmt_thread_id(s, cpu, thread_id, sizeof(thread_id));
+
 switch (state) {
 case RUN_STATE_DEBUG:
 if (cpu->watchpoint_hit) {
 switch (cpu->watchpoint_hit->flags & BP_MEM_ACCESS) {
 case BP_MEM_READ:
@@ -1741,12 +1750,12 @@ static void gdb_vm_state_change(void *opaque, int 
running, RunState state)
 break;
 }
 trace_gdbstub_hit_watchpoint(type, cpu_gdb_index(cpu),
 (target_ulong)cpu->watchpoint_hit->vaddr);
 snprintf(buf, sizeof(buf),
- "T%02xthread:%02x;%swatch:" TARGET_FMT_lx ";",
- GDB_SIGNAL_TRAP, cpu_gdb_index(cpu), type,
+ "T%02xthread:%s;%swatch:" TARGET_FMT_lx ";",
+ GDB_SIGNAL_TRAP, thread_id, type,
  (target_ulong)cpu->watchpoint_hit->vaddr);
 cpu->watchpoint_hit = NULL;
 goto send_packet;
 } else {
 trace_gdbstub_hit_break();
@@ -1784,11 +1793,11 @@ static void gdb_vm_state_change(void *opaque, int 
running, RunState state)
 trace_gdbstub_hit_unknown(state);
 ret = GDB_SIGNAL_UNKNOWN;
 break;
 }
 gdb_set_stop_cpu(cpu);
-snprintf(buf, sizeof(buf), "T%02xthread:%02x;", ret, cpu_gdb_index(cpu));
+snprintf(buf, sizeof(buf), "T%02xthread:%s;", ret, thread_id);
 
 send_packet:
 put_packet(s, buf);
 
 /* disable single step if it was enabled */
-- 
2.19.1




[Qemu-devel] [PATCH v4 06/16] gdbstub: add multiprocess support to 'sC' packets

2018-11-06 Thread Luc Michel
Change the sC packet handling to support the multiprocess extension.
Instead of returning the first thread, we return the first thread of the
current process.

Signed-off-by: Luc Michel 
Reviewed-by: Philippe Mathieu-Daudé 
Reviewed-by: Alistair Francis 
---
 gdbstub.c | 11 ---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/gdbstub.c b/gdbstub.c
index ba8b1a3413..9be83486f5 100644
--- a/gdbstub.c
+++ b/gdbstub.c
@@ -1546,13 +1546,18 @@ static int gdb_handle_packet(GDBState *s, const char 
*line_buf)
 type = strtoul(p, (char **)&p, 16);
 sstep_flags = type;
 put_packet(s, "OK");
 break;
 } else if (strcmp(p,"C") == 0) {
-/* "Current thread" remains vague in the spec, so always return
- *  the first CPU (gdb returns the first thread). */
-put_packet(s, "QC1");
+/* "Current thread" remains vague in the spec, so always return the
+ * first thread of the current process (gdb returns the first
+ * thread).
+ */
+cpu = get_first_cpu_in_process(s, gdb_get_cpu_process(s, 
s->g_cpu));
+snprintf(buf, sizeof(buf), "QC%s",
+ gdb_fmt_thread_id(s, cpu, thread_id, sizeof(thread_id)));
+put_packet(s, buf);
 break;
 } else if (strcmp(p,"fThreadInfo") == 0) {
 s->query_cpu = first_cpu;
 goto report_cpuinfo;
 } else if (strcmp(p,"sThreadInfo") == 0) {
-- 
2.19.1




[Qemu-devel] [PATCH v4 01/16] hw/cpu: introduce CPU clusters

2018-11-06 Thread Luc Michel
This commit adds the cpu-cluster type. It aims at gathering CPUs from
the same cluster in a machine.

For now it only has a `cluster-id` property.

Signed-off-by: Luc Michel 
Reviewed-by: Alistair Francis 
---
 include/hw/cpu/cluster.h | 38 ++
 hw/cpu/cluster.c | 59 
 hw/cpu/Makefile.objs |  2 +-
 3 files changed, 98 insertions(+), 1 deletion(-)
 create mode 100644 include/hw/cpu/cluster.h
 create mode 100644 hw/cpu/cluster.c

diff --git a/include/hw/cpu/cluster.h b/include/hw/cpu/cluster.h
new file mode 100644
index 00..0e5f121ec2
--- /dev/null
+++ b/include/hw/cpu/cluster.h
@@ -0,0 +1,38 @@
+/*
+ * QEMU CPU cluster
+ *
+ * Copyright (c) 2018 GreenSocs SAS
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; either version 2
+ * of the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, see
+ * 
+ */
+#ifndef HW_CPU_CLUSTER_H
+#define HW_CPU_CLUSTER_H
+
+#include "qemu/osdep.h"
+#include "hw/qdev.h"
+
+#define TYPE_CPU_CLUSTER "cpu-cluster"
+#define CPU_CLUSTER(obj) \
+OBJECT_CHECK(CPUClusterState, (obj), TYPE_CPU_CLUSTER)
+
+typedef struct CPUClusterState {
+/*< private >*/
+DeviceState parent_obj;
+
+/*< public >*/
+uint64_t cluster_id;
+} CPUClusterState;
+
+#endif
diff --git a/hw/cpu/cluster.c b/hw/cpu/cluster.c
new file mode 100644
index 00..a23d55054a
--- /dev/null
+++ b/hw/cpu/cluster.c
@@ -0,0 +1,59 @@
+/*
+ * QEMU CPU cluster
+ *
+ * Copyright (c) 2018 GreenSocs SAS
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; either version 2
+ * of the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, see
+ * 
+ */
+
+#include "qemu/osdep.h"
+#include "hw/cpu/cluster.h"
+#include "qapi/error.h"
+#include "qemu/module.h"
+
+static void cpu_cluster_init(Object *obj)
+{
+static uint64_t cluster_id_auto_increment = 0;
+CPUClusterState *cluster = CPU_CLUSTER(obj);
+
+cluster->cluster_id = cluster_id_auto_increment++;
+}
+
+static Property cpu_cluster_properties[] = {
+DEFINE_PROP_UINT64("cluster-id", CPUClusterState, cluster_id, 0),
+DEFINE_PROP_END_OF_LIST()
+};
+
+static void cpu_cluster_class_init(ObjectClass *klass, void *data)
+{
+DeviceClass *dc = DEVICE_CLASS(klass);
+
+dc->props = cpu_cluster_properties;
+}
+
+static const TypeInfo cpu_cluster_type_info = {
+.name = TYPE_CPU_CLUSTER,
+.parent = TYPE_DEVICE,
+.instance_size = sizeof(CPUClusterState),
+.instance_init = cpu_cluster_init,
+.class_init = cpu_cluster_class_init,
+};
+
+static void cpu_cluster_register_types(void)
+{
+type_register_static(&cpu_cluster_type_info);
+}
+
+type_init(cpu_cluster_register_types)
diff --git a/hw/cpu/Makefile.objs b/hw/cpu/Makefile.objs
index cd52d20b65..8db9e8a7b3 100644
--- a/hw/cpu/Makefile.objs
+++ b/hw/cpu/Makefile.objs
@@ -1,5 +1,5 @@
 obj-$(CONFIG_ARM11MPCORE) += arm11mpcore.o
 obj-$(CONFIG_REALVIEW) += realview_mpcore.o
 obj-$(CONFIG_A9MPCORE) += a9mpcore.o
 obj-$(CONFIG_A15MPCORE) += a15mpcore.o
-common-obj-y += core.o
+common-obj-y += core.o cluster.o
-- 
2.19.1




[Qemu-devel] [PATCH v4 04/16] gdbstub: add multiprocess support to 'H' and 'T' packets

2018-11-06 Thread Luc Michel
Add a couple of helper functions to cope with GDB threads and processes.

The gdb_get_process() function looks for a process given a pid.

The gdb_get_cpu() function returns the CPU corresponding to the (pid,
tid) pair given as parameters.

The read_thread_id() function parses the thread-id sent by the peer.
This function supports the multiprocess extension thread-id syntax.  The
return value specifies if the parsing failed, or if a special case was
encountered (all processes or all threads).

Use them in 'H' and 'T' packets handling to support the multiprocess
extension.

Signed-off-by: Luc Michel 
Reviewed-by: Philippe Mathieu-Daudé 
---
 gdbstub.c | 152 +++---
 1 file changed, 134 insertions(+), 18 deletions(-)

diff --git a/gdbstub.c b/gdbstub.c
index d26bad4b67..598a89b3ba 100644
--- a/gdbstub.c
+++ b/gdbstub.c
@@ -680,10 +680,73 @@ out:
 #else
 return s->processes[0].pid;
 #endif
 }
 
+static GDBProcess *gdb_get_process(const GDBState *s, uint32_t pid)
+{
+int i;
+
+if (!pid) {
+/* 0 means any process, we take the first one */
+return &s->processes[0];
+}
+
+for (i = 0; i < s->process_num; i++) {
+if (s->processes[i].pid == pid) {
+return &s->processes[i];
+}
+}
+
+return NULL;
+}
+
+static GDBProcess *gdb_get_cpu_process(const GDBState *s, CPUState *cpu)
+{
+return gdb_get_process(s, gdb_get_cpu_pid(s, cpu));
+}
+
+static CPUState *find_cpu(uint32_t thread_id)
+{
+CPUState *cpu;
+
+CPU_FOREACH(cpu) {
+if (cpu_gdb_index(cpu) == thread_id) {
+return cpu;
+}
+}
+
+return NULL;
+}
+
+static CPUState *gdb_get_cpu(const GDBState *s, uint32_t pid, uint32_t tid)
+{
+GDBProcess *process;
+CPUState *cpu = find_cpu(tid);
+
+if (!tid) {
+/* 0 means any thread, we take the first one */
+tid = 1;
+}
+
+if (cpu == NULL) {
+return NULL;
+}
+
+process = gdb_get_cpu_process(s, cpu);
+
+if (process->pid != pid) {
+return NULL;
+}
+
+if (!process->attached) {
+return NULL;
+}
+
+return cpu;
+}
+
 static const char *get_feature_xml(const char *p, const char **newp,
CPUClass *cc)
 {
 size_t len;
 int i;
@@ -936,23 +999,10 @@ static void gdb_set_cpu_pc(GDBState *s, target_ulong pc)
 
 cpu_synchronize_state(cpu);
 cpu_set_pc(cpu, pc);
 }
 
-static CPUState *find_cpu(uint32_t thread_id)
-{
-CPUState *cpu;
-
-CPU_FOREACH(cpu) {
-if (cpu_gdb_index(cpu) == thread_id) {
-return cpu;
-}
-}
-
-return NULL;
-}
-
 static char *gdb_fmt_thread_id(const GDBState *s, CPUState *cpu,
char *buf, size_t buf_size)
 {
 if (s->multiprocess) {
 snprintf(buf, buf_size, "p%02x.%02x",
@@ -962,10 +1012,64 @@ static char *gdb_fmt_thread_id(const GDBState *s, 
CPUState *cpu,
 }
 
 return buf;
 }
 
+typedef enum GDBThreadIdKind {
+GDB_ONE_THREAD = 0,
+GDB_ALL_THREADS, /* One process, all threads */
+GDB_ALL_PROCESSES,
+GDB_READ_THREAD_ERR
+} GDBThreadIdKind;
+
+static GDBThreadIdKind read_thread_id(const char *buf, const char **end_buf,
+  uint32_t *pid, uint32_t *tid)
+{
+unsigned long p, t;
+int ret;
+
+if (*buf == 'p') {
+buf++;
+ret = qemu_strtoul(buf, &buf, 16, &p);
+
+if (ret) {
+return GDB_READ_THREAD_ERR;
+}
+
+/* Skip '.' */
+buf++;
+} else {
+p = 1;
+}
+
+ret = qemu_strtoul(buf, &buf, 16, &t);
+
+if (ret) {
+return GDB_READ_THREAD_ERR;
+}
+
+*end_buf = buf;
+
+if (p == -1) {
+return GDB_ALL_PROCESSES;
+}
+
+if (pid) {
+*pid = p;
+}
+
+if (t == -1) {
+return GDB_ALL_THREADS;
+}
+
+if (tid) {
+*tid = t;
+}
+
+return GDB_ONE_THREAD;
+}
+
 static int is_query_packet(const char *p, const char *query, char separator)
 {
 unsigned int query_len = strlen(query);
 
 return strncmp(p, query, query_len) == 0 &&
@@ -1070,16 +1174,18 @@ static int gdb_handle_packet(GDBState *s, const char 
*line_buf)
 {
 CPUState *cpu;
 CPUClass *cc;
 const char *p;
 uint32_t thread;
+uint32_t pid, tid;
 int ch, reg_size, type, res;
 uint8_t mem_buf[MAX_PACKET_LENGTH];
 char buf[sizeof(mem_buf) + 1 /* trailing NUL */];
 char thread_id[16];
 uint8_t *registers;
 target_ulong addr, len;
+GDBThreadIdKind thread_kind;
 
 trace_gdbstub_io_command(line_buf);
 
 p = line_buf;
 ch = *p++;
@@ -1283,16 +1389,22 @@ static int gdb_handle_packet(GDBState *s, const char 
*line_buf)
 else
 put_packet(s, "E22");
 break;
 case 'H':
 type = *p++;
-thread = strtoull(p, (char **)&p, 16);
-if (thread == -1 || thread == 0) {
+
+thread_kin

[Qemu-devel] [PATCH v4 05/16] gdbstub: add multiprocess support to vCont packets

2018-11-06 Thread Luc Michel
Add the gdb_first_cpu() and gdb_next_cpu() to iterate over all
the CPUs in currently attached processes.

Add the gdb_first_cpu_in_process() and gdb_next_cpu_in_process() to
iterate over CPUs of a given process.

Use them to add multiprocess extension support to vCont packets.

Signed-off-by: Luc Michel 
Reviewed-by: Philippe Mathieu-Daudé 
---
 gdbstub.c | 117 +++---
 1 file changed, 102 insertions(+), 15 deletions(-)

diff --git a/gdbstub.c b/gdbstub.c
index 598a89b3ba..ba8b1a3413 100644
--- a/gdbstub.c
+++ b/gdbstub.c
@@ -716,10 +716,40 @@ static CPUState *find_cpu(uint32_t thread_id)
 }
 
 return NULL;
 }
 
+static CPUState *get_first_cpu_in_process(const GDBState *s,
+  GDBProcess *process)
+{
+CPUState *cpu;
+
+CPU_FOREACH(cpu) {
+if (gdb_get_cpu_pid(s, cpu) == process->pid) {
+return cpu;
+}
+}
+
+return NULL;
+}
+
+static CPUState *gdb_next_cpu_in_process(const GDBState *s, CPUState *cpu)
+{
+uint32_t pid = gdb_get_cpu_pid(s, cpu);
+cpu = CPU_NEXT(cpu);
+
+while (cpu) {
+if (gdb_get_cpu_pid(s, cpu) == pid) {
+break;
+}
+
+cpu = CPU_NEXT(cpu);
+}
+
+return cpu;
+}
+
 static CPUState *gdb_get_cpu(const GDBState *s, uint32_t pid, uint32_t tid)
 {
 GDBProcess *process;
 CPUState *cpu = find_cpu(tid);
 
@@ -743,10 +773,41 @@ static CPUState *gdb_get_cpu(const GDBState *s, uint32_t 
pid, uint32_t tid)
 }
 
 return cpu;
 }
 
+/* Return the cpu following @cpu, while ignoring
+ * unattached processes.
+ */
+static CPUState *gdb_next_cpu(const GDBState *s, CPUState *cpu)
+{
+cpu = CPU_NEXT(cpu);
+
+while (cpu) {
+if (gdb_get_cpu_process(s, cpu)->attached) {
+break;
+}
+
+cpu = CPU_NEXT(cpu);
+}
+
+return cpu;
+}
+
+/* Return the first attached cpu */
+static CPUState *gdb_first_cpu(const GDBState *s)
+{
+CPUState *cpu = first_cpu;
+GDBProcess *process = gdb_get_cpu_process(s, cpu);
+
+if (!process->attached) {
+return gdb_next_cpu(s, cpu);
+}
+
+return cpu;
+}
+
 static const char *get_feature_xml(const char *p, const char **newp,
CPUClass *cc)
 {
 size_t len;
 int i;
@@ -1081,14 +1142,16 @@ static int is_query_packet(const char *p, const char 
*query, char separator)
  * returns -ENOTSUP if a command is unsupported, -EINVAL or -ERANGE if there is
  * a format error, 0 on success.
  */
 static int gdb_handle_vcont(GDBState *s, const char *p)
 {
-int res, idx, signal = 0;
+int res, signal = 0;
 char cur_action;
 char *newstates;
 unsigned long tmp;
+uint32_t pid, tid;
+GDBProcess *process;
 CPUState *cpu;
 #ifdef CONFIG_USER_ONLY
 int max_cpus = 1; /* global variable max_cpus exists only in system mode */
 
 CPU_FOREACH(cpu) {
@@ -1127,29 +1190,52 @@ static int gdb_handle_vcont(GDBState *s, const char *p)
 } else if (cur_action != 'c' && cur_action != 's') {
 /* unknown/invalid/unsupported command */
 res = -ENOTSUP;
 goto out;
 }
-/* thread specification. special values: (none), -1 = all; 0 = any */
-if ((p[0] == ':' && p[1] == '-' && p[2] == '1') || (p[0] != ':')) {
-if (*p == ':') {
-p += 3;
-}
-for (idx = 0; idx < max_cpus; idx++) {
-if (newstates[idx] == 1) {
-newstates[idx] = cur_action;
+
+if (*p++ != ':') {
+res = -ENOTSUP;
+goto out;
+}
+
+switch (read_thread_id(p, &p, &pid, &tid)) {
+case GDB_READ_THREAD_ERR:
+res = -EINVAL;
+goto out;
+
+case GDB_ALL_PROCESSES:
+cpu = gdb_first_cpu(s);
+while (cpu) {
+if (newstates[cpu->cpu_index] == 1) {
+newstates[cpu->cpu_index] = cur_action;
 }
+
+cpu = gdb_next_cpu(s, cpu);
 }
-} else if (*p == ':') {
-p++;
-res = qemu_strtoul(p, &p, 16, &tmp);
-if (res) {
+break;
+
+case GDB_ALL_THREADS:
+process = gdb_get_process(s, pid);
+
+if (!process->attached) {
+res = -EINVAL;
 goto out;
 }
 
-/* 0 means any thread, so we pick the first valid CPU */
-cpu = tmp ? find_cpu(tmp) : first_cpu;
+cpu = get_first_cpu_in_process(s, process);
+while (cpu) {
+if (newstates[cpu->cpu_index] == 1) {
+newstates[cpu->cpu_index] = cur_action;
+}
+
+cpu = gdb_next_cpu_in_process(s, cpu);
+}
+break;
+
+case GDB_ONE_THREAD:
+cpu = gdb_get_cpu(s, pid, tid);
 
 /* invalid CP

[Qemu-devel] [PATCH v4 03/16] gdbstub: add multiprocess support to '?' packets

2018-11-06 Thread Luc Michel
The gdb_get_cpu_pid() function does the PID lookup for the given CPU. It
checks if the CPU is a direct child of a CPU cluster. If it is, the
returned PID is the cluster ID plus one (cluster IDs start at 0, GDB
PIDs at 1). When the CPU is not a child of such a container, the PID of
the first process is returned.

The gdb_fmt_thread_id() function generates the string to be used to identify
a given thread, in a response packet for the peer. This function
supports generating thread IDs when multiprocess mode is enabled (in the
form `p.').

Use them in the reply to a '?' request.

Signed-off-by: Luc Michel 
Acked-by: Alistair Francis 
---
 gdbstub.c | 60 +--
 1 file changed, 58 insertions(+), 2 deletions(-)

diff --git a/gdbstub.c b/gdbstub.c
index 0d70b89598..d26bad4b67 100644
--- a/gdbstub.c
+++ b/gdbstub.c
@@ -638,10 +638,52 @@ static int memtox(char *buf, const char *mem, int len)
 }
 }
 return p - buf;
 }
 
+static uint32_t gdb_get_cpu_pid(const GDBState *s, CPUState *cpu)
+{
+#ifndef CONFIG_USER_ONLY
+gchar *path, *name;
+Object *obj;
+CPUClusterState *cluster;
+uint32_t ret;
+
+path = object_get_canonical_path(OBJECT(cpu));
+name = object_get_canonical_path_component(OBJECT(cpu));
+
+if (path == NULL) {
+ret = s->processes[0].pid;
+goto out;
+}
+
+/*
+ * Retrieve the CPU parent path by removing the last '/' and the CPU name
+ * from the CPU canonical path. */
+path[strlen(path) - strlen(name) - 1] = '\0';
+
+obj = object_resolve_path_type(path, TYPE_CPU_CLUSTER, NULL);
+
+if (obj == NULL) {
+ret = s->processes[0].pid;
+goto out;
+}
+
+cluster = CPU_CLUSTER(obj);
+ret = cluster->cluster_id + 1;
+
+out:
+g_free(name);
+g_free(path);
+
+return ret;
+
+#else
+return s->processes[0].pid;
+#endif
+}
+
 static const char *get_feature_xml(const char *p, const char **newp,
CPUClass *cc)
 {
 size_t len;
 int i;
@@ -907,10 +949,23 @@ static CPUState *find_cpu(uint32_t thread_id)
 }
 
 return NULL;
 }
 
+static char *gdb_fmt_thread_id(const GDBState *s, CPUState *cpu,
+   char *buf, size_t buf_size)
+{
+if (s->multiprocess) {
+snprintf(buf, buf_size, "p%02x.%02x",
+ gdb_get_cpu_pid(s, cpu), cpu_gdb_index(cpu));
+} else {
+snprintf(buf, buf_size, "%02x", cpu_gdb_index(cpu));
+}
+
+return buf;
+}
+
 static int is_query_packet(const char *p, const char *query, char separator)
 {
 unsigned int query_len = strlen(query);
 
 return strncmp(p, query, query_len) == 0 &&
@@ -1018,22 +1073,23 @@ static int gdb_handle_packet(GDBState *s, const char 
*line_buf)
 const char *p;
 uint32_t thread;
 int ch, reg_size, type, res;
 uint8_t mem_buf[MAX_PACKET_LENGTH];
 char buf[sizeof(mem_buf) + 1 /* trailing NUL */];
+char thread_id[16];
 uint8_t *registers;
 target_ulong addr, len;
 
 trace_gdbstub_io_command(line_buf);
 
 p = line_buf;
 ch = *p++;
 switch(ch) {
 case '?':
 /* TODO: Make this return the correct value for user-mode.  */
-snprintf(buf, sizeof(buf), "T%02xthread:%02x;", GDB_SIGNAL_TRAP,
- cpu_gdb_index(s->c_cpu));
+snprintf(buf, sizeof(buf), "T%02xthread:%s;", GDB_SIGNAL_TRAP,
+ gdb_fmt_thread_id(s, s->c_cpu, thread_id, sizeof(thread_id)));
 put_packet(s, buf);
 /* Remove all the breakpoints when this query is issued,
  * because gdb is doing and initial connect and the state
  * should be cleaned up.
  */
-- 
2.19.1




[Qemu-devel] [PATCH v4 00/16] gdbstub: support for the multiprocess extension

2018-11-06 Thread Luc Michel
changes since v3:
  - patch 1cpu_cluster.h: remove QEMU_ from the multiple includes
   guard #ifdef/#define [Alistair]

  - patch 1cpu_cluster.c: include osdep.h first [Alistair]

  - patch 1use uint64_t for cluster ID for prosperity :) [Philippe]

  - patch 1auto-assign a cluster ID to newly created clusters [Philippe]

  - patch 2fix mid-code variable declaration [Alistair]

  - patch 3add a comment in gdb_get_cpu_pid() when retrieving CPU
   parent canonical path [Alistair]

  - patch 14   fix a typo in the commit message [Alistair]

changes since v2:
  - patch 1introducing the cpu-cluster type. I didn't opt for an
   Interface, but I can add one if you think it's necessary.
   For now this class inherits from Device and has a
   cluster-id property, used by the GDB stub to compute a
   PID.

  - patch 2removed GDB group related code as it has been replaced
   with CPU clusters

  - patch 2/8  moved GDBProcess target_xml field introduction into patch
   8 [Philippe]

  - patch 3gdb_get_cpu_pid() now search for CPU being a child of a
   cpu-cluster object. Use the cluster-id to compute the PID.

  - patch 4gdb_get_process() does not rely on s->processes array
   indices anymore as PIDs can now be sparse. Instead, iterate
   over the array to find the process.

  - patch 3/4  removed Reviewed-by tags because of substantial changes.

  - patch 4/7  read_thread_id() hardening [Philippe]

  - patch 12   safer vAttach packet parsing [Phillipe]

  - patch 16   put APUs and RPUs in different clusters instead of GDB
   groups

changes since v1:
  - rename qemu_get_thread_id() to gdb_fmt_thread_id() [Philippe]
  - check qemu_strtoul() return value for 'D' packets [Philippe]


This series adds support for the multiprocess extension of the GDB
remote protocol in the QEMU GDB stub.

This extension is useful to split QEMU emulated CPUs in different
processes from the point of view of the GDB client. It adds the
possibility to debug different kind of processors (e.g. an AArch64 and
an ARMv7 CPU) at the same time (it is not possible otherwise since GDB
expects an SMP view at the thread granularity.

CPUs are grouped using specially named QOM containers. CPUs that are
children of such a container are grouped under the same GDB process.

The last patch groups the CPUs of different model in the zynqmp machines
into separate processes.

To test this patchset, you can use the following commands:

(Note that this requires a recent enough GDB, I think GDB 7.2 is OK.
Also, it must be compiled to support both ARM and AArch64 architectures)

Run QEMU: (-smp 6 in xlnx-zcu102 enables both cortex-a53 and cortex-r5
CPUs)

qemu-system-aarch64 -M xlnx-zcu102 -gdb tcp::1234 -S -smp 6

Run the following commands in GDB:

target extended :1234
add-inferior
inferior 2
attach 2
info threads

I want to thanks the Xilinx's QEMU team who sponsored this work for
their collaboration and their prototype implementation.

Luc Michel (16):
  hw/cpu: introduce CPU clusters
  gdbstub: introduce GDB processes
  gdbstub: add multiprocess support to '?' packets
  gdbstub: add multiprocess support to 'H' and 'T' packets
  gdbstub: add multiprocess support to vCont packets
  gdbstub: add multiprocess support to 'sC' packets
  gdbstub: add multiprocess support to (f|s)ThreadInfo and
ThreadExtraInfo
  gdbstub: add multiprocess support to Xfer:features:read:
  gdbstub: add multiprocess support to gdb_vm_state_change()
  gdbstub: add multiprocess support to 'D' packets
  gdbstub: add support for extended mode packet
  gdbstub: add support for vAttach packets
  gdbstub: processes initialization on new peer connection
  gdbstub: gdb_set_stop_cpu: ignore request when process is not attached
  gdbstub: add multiprocess extension support
  arm/xlnx-zynqmp: put APUs and RPUs in separate CPU clusters

 include/hw/arm/xlnx-zynqmp.h |   3 +
 include/hw/cpu/cluster.h |  38 +++
 gdbstub.c| 632 ++-
 hw/arm/xlnx-zynqmp.c |  21 +-
 hw/cpu/cluster.c |  59 
 hw/cpu/Makefile.objs |   2 +-
 6 files changed, 674 insertions(+), 81 deletions(-)
 create mode 100644 include/hw/cpu/cluster.h
 create mode 100644 hw/cpu/cluster.c

-- 
2.19.1




[Qemu-devel] [PATCH v4 15/16] gdbstub: add multiprocess extension support

2018-11-06 Thread Luc Michel
Add multiprocess extension support by enabling multiprocess mode when
the peer requests it, and by replying that we actually support it in the
qSupported reply packet.

Signed-off-by: Luc Michel 
Reviewed-by: Philippe Mathieu-Daudé 
Acked-by: Alistair Francis 
---
 gdbstub.c | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/gdbstub.c b/gdbstub.c
index c662ada6ff..93a7c956c2 100644
--- a/gdbstub.c
+++ b/gdbstub.c
@@ -1721,10 +1721,16 @@ static int gdb_handle_packet(GDBState *s, const char 
*line_buf)
 snprintf(buf, sizeof(buf), "PacketSize=%x", MAX_PACKET_LENGTH);
 cc = CPU_GET_CLASS(first_cpu);
 if (cc->gdb_core_xml_file != NULL) {
 pstrcat(buf, sizeof(buf), ";qXfer:features:read+");
 }
+
+if (strstr(p, "multiprocess+")) {
+s->multiprocess = true;
+}
+pstrcat(buf, sizeof(buf), ";multiprocess+");
+
 put_packet(s, buf);
 break;
 }
 if (strncmp(p, "Xfer:features:read:", 19) == 0) {
 const char *xml;
-- 
2.19.1




[Qemu-devel] [PATCH v4 16/16] arm/xlnx-zynqmp: put APUs and RPUs in separate CPU clusters

2018-11-06 Thread Luc Michel
Create two separate CPU clusters for APUs and RPUs.

Signed-off-by: Luc Michel 
---
 include/hw/arm/xlnx-zynqmp.h |  3 +++
 hw/arm/xlnx-zynqmp.c | 21 +
 2 files changed, 20 insertions(+), 4 deletions(-)

diff --git a/include/hw/arm/xlnx-zynqmp.h b/include/hw/arm/xlnx-zynqmp.h
index 98f925ab84..591515c760 100644
--- a/include/hw/arm/xlnx-zynqmp.h
+++ b/include/hw/arm/xlnx-zynqmp.h
@@ -29,10 +29,11 @@
 #include "hw/dma/xlnx_dpdma.h"
 #include "hw/dma/xlnx-zdma.h"
 #include "hw/display/xlnx_dp.h"
 #include "hw/intc/xlnx-zynqmp-ipi.h"
 #include "hw/timer/xlnx-zynqmp-rtc.h"
+#include "hw/cpu/cluster.h"
 
 #define TYPE_XLNX_ZYNQMP "xlnx,zynqmp"
 #define XLNX_ZYNQMP(obj) OBJECT_CHECK(XlnxZynqMPState, (obj), \
TYPE_XLNX_ZYNQMP)
 
@@ -75,10 +76,12 @@
 typedef struct XlnxZynqMPState {
 /*< private >*/
 DeviceState parent_obj;
 
 /*< public >*/
+CPUClusterState apu_cluster;
+CPUClusterState rpu_cluster;
 ARMCPU apu_cpu[XLNX_ZYNQMP_NUM_APU_CPUS];
 ARMCPU rpu_cpu[XLNX_ZYNQMP_NUM_RPU_CPUS];
 GICState gic;
 MemoryRegion gic_mr[XLNX_ZYNQMP_GIC_REGIONS][XLNX_ZYNQMP_GIC_ALIASES];
 
diff --git a/hw/arm/xlnx-zynqmp.c b/hw/arm/xlnx-zynqmp.c
index c195040350..42a138074c 100644
--- a/hw/arm/xlnx-zynqmp.c
+++ b/hw/arm/xlnx-zynqmp.c
@@ -176,16 +176,22 @@ static void xlnx_zynqmp_create_rpu(XlnxZynqMPState *s, 
const char *boot_cpu,
 {
 Error *err = NULL;
 int i;
 int num_rpus = MIN(smp_cpus - XLNX_ZYNQMP_NUM_APU_CPUS, 
XLNX_ZYNQMP_NUM_RPU_CPUS);
 
+object_initialize_child(OBJECT(s), "rpu-cluster", &s->rpu_cluster,
+sizeof(s->rpu_cluster), TYPE_CPU_CLUSTER,
+&error_abort, NULL);
+
+qdev_init_nofail(DEVICE(&s->rpu_cluster));
+
 for (i = 0; i < num_rpus; i++) {
 char *name;
 
 object_initialize(&s->rpu_cpu[i], sizeof(s->rpu_cpu[i]),
   "cortex-r5f-" TYPE_ARM_CPU);
-object_property_add_child(OBJECT(s), "rpu-cpu[*]",
+object_property_add_child(OBJECT(&s->rpu_cluster), "rpu-cpu[*]",
   OBJECT(&s->rpu_cpu[i]), &error_abort);
 
 name = object_get_canonical_path_component(OBJECT(&s->rpu_cpu[i]));
 if (strcmp(name, boot_cpu)) {
 /* Secondary CPUs start in PSCI powered-down state */
@@ -211,14 +217,19 @@ static void xlnx_zynqmp_init(Object *obj)
 {
 XlnxZynqMPState *s = XLNX_ZYNQMP(obj);
 int i;
 int num_apus = MIN(smp_cpus, XLNX_ZYNQMP_NUM_APU_CPUS);
 
+object_initialize_child(obj, "apu-cluster", &s->apu_cluster,
+sizeof(s->apu_cluster), TYPE_CPU_CLUSTER,
+&error_abort, NULL);
+
 for (i = 0; i < num_apus; i++) {
-object_initialize_child(obj, "apu-cpu[*]", &s->apu_cpu[i],
-sizeof(s->apu_cpu[i]),
-"cortex-a53-" TYPE_ARM_CPU, &error_abort, 
NULL);
+object_initialize_child(OBJECT(&s->apu_cluster), "apu-cpu[*]",
+&s->apu_cpu[i], sizeof(s->apu_cpu[i]),
+"cortex-a53-" TYPE_ARM_CPU, &error_abort,
+NULL);
 }
 
 sysbus_init_child_obj(obj, "gic", &s->gic, sizeof(s->gic),
   gic_class_name());
 
@@ -331,10 +342,12 @@ static void xlnx_zynqmp_realize(DeviceState *dev, Error 
**errp)
 qdev_prop_set_uint32(DEVICE(&s->gic), "num-cpu", num_apus);
 qdev_prop_set_bit(DEVICE(&s->gic), "has-security-extensions", s->secure);
 qdev_prop_set_bit(DEVICE(&s->gic),
   "has-virtualization-extensions", s->virt);
 
+qdev_init_nofail(DEVICE(&s->apu_cluster));
+
 /* Realize APUs before realizing the GIC. KVM requires this.  */
 for (i = 0; i < num_apus; i++) {
 char *name;
 
 object_property_set_int(OBJECT(&s->apu_cpu[i]), QEMU_PSCI_CONDUIT_SMC,
-- 
2.19.1




[Qemu-devel] [PATCH v4 08/16] gdbstub: add multiprocess support to Xfer:features:read:

2018-11-06 Thread Luc Michel
Change the Xfer:features:read: packet handling to support the
multiprocess extension. This packet is used to request the XML
description of the CPU. In multiprocess mode, different descriptions can
be sent for different processes.

This function now takes the process to send the description for as a
parameter, and use a buffer in the process structure to store the
generated description.

It takes the first CPU of the process to generate the description.

Signed-off-by: Luc Michel 
Reviewed-by: Philippe Mathieu-Daudé 
---
 gdbstub.c | 47 +++
 1 file changed, 27 insertions(+), 20 deletions(-)

diff --git a/gdbstub.c b/gdbstub.c
index 43c9211bfa..aae3cce01a 100644
--- a/gdbstub.c
+++ b/gdbstub.c
@@ -298,10 +298,12 @@ typedef struct GDBRegisterState {
 } GDBRegisterState;
 
 typedef struct GDBProcess {
 uint32_t pid;
 bool attached;
+
+char target_xml[1024];
 } GDBProcess;
 
 enum RSState {
 RS_INACTIVE,
 RS_IDLE,
@@ -804,55 +806,57 @@ static CPUState *gdb_first_cpu(const GDBState *s)
 }
 
 return cpu;
 }
 
-static const char *get_feature_xml(const char *p, const char **newp,
-   CPUClass *cc)
+static const char *get_feature_xml(const GDBState *s, const char *p,
+   const char **newp, GDBProcess *process)
 {
 size_t len;
 int i;
 const char *name;
-static char target_xml[1024];
+CPUState *cpu = get_first_cpu_in_process(s, process);
+CPUClass *cc = CPU_GET_CLASS(cpu);
 
 len = 0;
 while (p[len] && p[len] != ':')
 len++;
 *newp = p + len;
 
 name = NULL;
 if (strncmp(p, "target.xml", len) == 0) {
+char *buf = process->target_xml;
+const size_t buf_sz = sizeof(process->target_xml);
+
 /* Generate the XML description for this CPU.  */
-if (!target_xml[0]) {
+if (!buf[0]) {
 GDBRegisterState *r;
-CPUState *cpu = first_cpu;
 
-pstrcat(target_xml, sizeof(target_xml),
+pstrcat(buf, buf_sz,
 ""
 ""
 "");
 if (cc->gdb_arch_name) {
 gchar *arch = cc->gdb_arch_name(cpu);
-pstrcat(target_xml, sizeof(target_xml), "");
-pstrcat(target_xml, sizeof(target_xml), arch);
-pstrcat(target_xml, sizeof(target_xml), "");
+pstrcat(buf, buf_sz, "");
+pstrcat(buf, buf_sz, arch);
+pstrcat(buf, buf_sz, "");
 g_free(arch);
 }
-pstrcat(target_xml, sizeof(target_xml), "gdb_core_xml_file);
-pstrcat(target_xml, sizeof(target_xml), "\"/>");
+pstrcat(buf, buf_sz, "gdb_core_xml_file);
+pstrcat(buf, buf_sz, "\"/>");
 for (r = cpu->gdb_regs; r; r = r->next) {
-pstrcat(target_xml, sizeof(target_xml), "xml);
-pstrcat(target_xml, sizeof(target_xml), "\"/>");
+pstrcat(buf, buf_sz, "xml);
+pstrcat(buf, buf_sz, "\"/>");
 }
-pstrcat(target_xml, sizeof(target_xml), "");
+pstrcat(buf, buf_sz, "");
 }
-return target_xml;
+return buf;
 }
 if (cc->gdb_get_dynamic_xml) {
-CPUState *cpu = first_cpu;
 char *xmlname = g_strndup(p, len);
 const char *xml = cc->gdb_get_dynamic_xml(cpu, xmlname);
 
 g_free(xmlname);
 if (xml) {
@@ -1258,10 +1262,11 @@ out:
 }
 
 static int gdb_handle_packet(GDBState *s, const char *line_buf)
 {
 CPUState *cpu;
+GDBProcess *process;
 CPUClass *cc;
 const char *p;
 uint32_t pid, tid;
 int ch, reg_size, type, res;
 uint8_t mem_buf[MAX_PACKET_LENGTH];
@@ -1639,18 +1644,19 @@ static int gdb_handle_packet(GDBState *s, const char 
*line_buf)
 }
 if (strncmp(p, "Xfer:features:read:", 19) == 0) {
 const char *xml;
 target_ulong total_len;
 
-cc = CPU_GET_CLASS(first_cpu);
+process = gdb_get_cpu_process(s, s->g_cpu);
+cc = CPU_GET_CLASS(s->g_cpu);
 if (cc->gdb_core_xml_file == NULL) {
 goto unknown_command;
 }
 
 gdb_has_xml = true;
 p += 19;
-xml = get_feature_xml(p, &p, cc);
+xml = get_feature_xml(s, p, &p, process);
 if (!xml) {
 snprintf(buf, sizeof(buf), "E00");
 put_packet(s, buf);
 break;
 }
@@ -2319,10 +2325,11 @@ static int find_cpu_clusters(Object *child, void 
*opaque)
 process = &s->processes[s->process_num - 1];
 
 /* GDB process IDs -1 and 0 are reserved */
 process->pid = cluster->cluster_id + 1;
 process->attached = false;
+process->target_xml[0] = '\0';
 return 0;
 }
 
 return object_child_foreach(child, fin

[Qemu-devel] [PATCH v4 12/16] gdbstub: add support for vAttach packets

2018-11-06 Thread Luc Michel
Add support for the vAttach packets. In multiprocess mode, GDB sends
them to attach to additional processes.

Signed-off-by: Luc Michel 
---
 gdbstub.c | 35 +++
 1 file changed, 35 insertions(+)

diff --git a/gdbstub.c b/gdbstub.c
index 4132227092..ba365808db 100644
--- a/gdbstub.c
+++ b/gdbstub.c
@@ -1340,10 +1340,45 @@ static int gdb_handle_packet(GDBState *s, const char 
*line_buf)
 break;
 }
 goto unknown_command;
 }
 break;
+} else if (strncmp(p, "Attach;", 7) == 0) {
+unsigned long pid;
+
+p += 7;
+
+if (qemu_strtoul(p, &p, 16, &pid)) {
+put_packet(s, "E22");
+break;
+}
+
+process = gdb_get_process(s, pid);
+
+if (process == NULL) {
+put_packet(s, "E22");
+break;
+}
+
+cpu = get_first_cpu_in_process(s, process);
+
+if (cpu == NULL) {
+/* Refuse to attach an empty process */
+put_packet(s, "E22");
+break;
+}
+
+process->attached = true;
+
+s->g_cpu = cpu;
+s->c_cpu = cpu;
+
+snprintf(buf, sizeof(buf), "T%02xthread:%s;", GDB_SIGNAL_TRAP,
+ gdb_fmt_thread_id(s, cpu, thread_id, sizeof(thread_id)));
+
+put_packet(s, buf);
+break;
 } else {
 goto unknown_command;
 }
 case 'k':
 /* Kill the target */
-- 
2.19.1




[Qemu-devel] [PATCH v4 13/16] gdbstub: processes initialization on new peer connection

2018-11-06 Thread Luc Michel
When a new connection is established, we set the first process to be
attached, and the others detached. The first CPU of the first process
is selected as the current CPU.

Signed-off-by: Luc Michel 
Reviewed-by: Alistair Francis 
---
 gdbstub.c | 20 +++-
 1 file changed, 15 insertions(+), 5 deletions(-)

diff --git a/gdbstub.c b/gdbstub.c
index ba365808db..1a3353ff3c 100644
--- a/gdbstub.c
+++ b/gdbstub.c
@@ -2245,13 +2245,14 @@ static bool gdb_accept(void)
 close(fd);
 return false;
 }
 
 s = g_malloc0(sizeof(GDBState));
-s->c_cpu = first_cpu;
-s->g_cpu = first_cpu;
 create_unique_process(s);
+s->processes[0].attached = true;
+s->c_cpu = gdb_first_cpu(s);
+s->g_cpu = s->c_cpu;
 s->fd = fd;
 gdb_has_xml = false;
 
 gdbserver_state = s;
 return true;
@@ -2333,12 +2334,23 @@ static void gdb_chr_receive(void *opaque, const uint8_t 
*buf, int size)
 }
 }
 
 static void gdb_chr_event(void *opaque, int event)
 {
+int i;
+GDBState *s = (GDBState *) opaque;
+
 switch (event) {
 case CHR_EVENT_OPENED:
+/* Start with first process attached, others detached */
+for (i = 0; i < s->process_num; i++) {
+s->processes[i].attached = !i;
+}
+
+s->c_cpu = gdb_first_cpu(s);
+s->g_cpu = s->c_cpu;
+
 vm_stop(RUN_STATE_PAUSED);
 gdb_has_xml = false;
 break;
 default:
 break;
@@ -2513,19 +2525,17 @@ int gdbserver_start(const char *device)
 mon_chr = s->mon_chr;
 cleanup_processes(s);
 memset(s, 0, sizeof(GDBState));
 s->mon_chr = mon_chr;
 }
-s->c_cpu = first_cpu;
-s->g_cpu = first_cpu;
 
 create_processes(s);
 
 if (chr) {
 qemu_chr_fe_init(&s->chr, chr, &error_abort);
 qemu_chr_fe_set_handlers(&s->chr, gdb_chr_can_receive, gdb_chr_receive,
- gdb_chr_event, NULL, NULL, NULL, true);
+ gdb_chr_event, NULL, s, NULL, true);
 }
 s->state = chr ? RS_IDLE : RS_INACTIVE;
 s->mon_chr = mon_chr;
 s->current_syscall_cb = NULL;
 
-- 
2.19.1




[Qemu-devel] [PATCH v4 11/16] gdbstub: add support for extended mode packet

2018-11-06 Thread Luc Michel
Add support for the '!' extended mode packet. This is required for the
multiprocess extension.

Signed-off-by: Luc Michel 
---
 gdbstub.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/gdbstub.c b/gdbstub.c
index bd4895ac0a..4132227092 100644
--- a/gdbstub.c
+++ b/gdbstub.c
@@ -1294,10 +1294,13 @@ static int gdb_handle_packet(GDBState *s, const char 
*line_buf)
 trace_gdbstub_io_command(line_buf);
 
 p = line_buf;
 ch = *p++;
 switch(ch) {
+case '!':
+put_packet(s, "OK");
+break;
 case '?':
 /* TODO: Make this return the correct value for user-mode.  */
 snprintf(buf, sizeof(buf), "T%02xthread:%s;", GDB_SIGNAL_TRAP,
  gdb_fmt_thread_id(s, s->c_cpu, thread_id, sizeof(thread_id)));
 put_packet(s, buf);
-- 
2.19.1




[Qemu-devel] [PATCH v4 14/16] gdbstub: gdb_set_stop_cpu: ignore request when process is not attached

2018-11-06 Thread Luc Michel
When gdb_set_stop_cpu() is called with a CPU associated to a process
currently not attached by the GDB client, return without modifying the
stop CPU. Otherwise, GDB gets confused if it receives packets with a
thread-id it does not know about.

Signed-off-by: Luc Michel 
Acked-by: Alistair Francis 
---
 gdbstub.c | 9 +
 1 file changed, 9 insertions(+)

diff --git a/gdbstub.c b/gdbstub.c
index 1a3353ff3c..c662ada6ff 100644
--- a/gdbstub.c
+++ b/gdbstub.c
@@ -1787,10 +1787,19 @@ static int gdb_handle_packet(GDBState *s, const char 
*line_buf)
 return RS_IDLE;
 }
 
 void gdb_set_stop_cpu(CPUState *cpu)
 {
+GDBProcess *p = gdb_get_cpu_process(gdbserver_state, cpu);
+
+if (!p->attached) {
+/* Having a stop CPU corresponding to a process that is not attached
+ * confuses GDB. So we ignore the request.
+ */
+return;
+}
+
 gdbserver_state->c_cpu = cpu;
 gdbserver_state->g_cpu = cpu;
 }
 
 #ifndef CONFIG_USER_ONLY
-- 
2.19.1




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

2018-11-06 Thread Michael S. Tsirkin
On Tue, Nov 06, 2018 at 09:18:49AM +0100, Thomas Huth wrote:
> On 2018-11-05 19:14, Michael S. Tsirkin wrote:
> > The following changes since commit b2f7a038bb4c4fc5ce6b8486e8513dfd97665e2a:
> > 
> >   Merge remote-tracking branch 'remotes/rth/tags/pull-softfloat-20181104' 
> > into staging (2018-11-05 10:32:49 +)
> > 
> > are available in the Git repository at:
> > 
> >   git://git.kernel.org/pub/scm/virt/kvm/mst/qemu.git tags/for_upstream
> > 
> > for you to fetch changes up to 6196df5c8e6688c1c3f06f73442820066335337c:
> > 
> >   vhost-scsi: prevent using uninitialized vqs (2018-11-05 12:59:35 -0500)
> > 
> > 
> > pci, pc, virtio: fixes, features
> > 
> > AMD IOMMU VAPIC support + fixes all over the place.
> > 
> > Signed-off-by: Michael S. Tsirkin 
> > 
> > 
> > Gerd Hoffmann (1):
> >   pci-testdev: add optional memory bar
> > 
> > Laszlo Ersek (4):
> >   MAINTAINERS: list "tests/acpi-test-data" files in ACPI/SMBIOS section
> [...]
> >  tests/{acpi-test-data => data/acpi}/pc/APIC| Bin
> >  tests/{acpi-test-data => data/acpi}/pc/APIC.cphp   | Bin
> >  .../{acpi-test-data => data/acpi}/pc/APIC.dimmpxm  | Bin
> >  tests/{acpi-test-data => data/acpi}/pc/DSDT| Bin
> 
> So patch 1 moves tests/acpi-test-data/ to tests/data/acpi/ and patch 20
> adds an entry for tests/acpi-test-data/ ? Does not make much sense to me
> ... I think patch 20 needs to be adapted now.
> 
>  Thomas

Oh right, MAINTAINERS needs to be fixed. Can be done with a patch on top
though.

-- 
MST



Re: [Qemu-devel] [PATCH v2 1/1] include: Add a comment to explain the origin of sizes' lookup table

2018-11-06 Thread Kevin Wolf
Am 06.11.2018 um 09:56 hat Leonid Bloch geschrieben:
> Hi Phil, Hi Eric,
> 
> (Eric, for some reason you weren't CC'd to this thread - sorry.)
> 
> On 11/5/18 5:58 PM, Philippe Mathieu-Daudé wrote:
> > Hi Leonid,
> > 
> > On 4/11/18 19:07, Leonid Bloch wrote:
> >> The lookup table for power-of-two sizes was added in commit 540b8492618eb
> >> for the purpose of having convenient shortcuts for these sizes in cases
> >> when the literal number has to be present at compile time, and
> >> expressions as '(1 * KiB)' can not be used. One such case is the
> >> stringification of sizes. Beyond that, it is convenient to use these
> >> shortcuts for all power-of-two sizes, even if they don't have to be
> >> literal numbers.
> >>
> >> Despite its convenience, this table introduced 55 lines of "dumb" code,
> >> the purpose and origin of which are obscure without reading the message
> >> of the commit which introduced it. This patch fixes that by adding a
> >> comment to the code itself with a brief explanation for the reasoning
> >> behind this table. This comment includes the short AWK script that
> >> generated the table, so that anyone who's interested could make sure
> >> that the values in it are correct (otherwise these values look as if
> >> they were typed manually).
> >>
> >> Signed-off-by: Leonid Bloch 
> >> ---
> >>   include/qemu/units.h | 18 ++
> >>   1 file changed, 18 insertions(+)
> >>
> >> diff --git a/include/qemu/units.h b/include/qemu/units.h
> >> index 68a7758650..1c959d182e 100644
> >> --- a/include/qemu/units.h
> >> +++ b/include/qemu/units.h
> >> @@ -17,6 +17,24 @@
> >>   #define PiB (INT64_C(1) << 50)
> >>   #define EiB (INT64_C(1) << 60)
> >> +/*
> >> + * The following lookup table is intended to be used when a literal 
> >> string of
> >> + * the number of bytes is required (for example if it needs to be 
> >> stringified).
> >> + * It can also be used for generic shortcuts of power-of-two sizes.
> > 
> > ^ ok
> > 
> > v this part is not useful in the tree IMHO.
> > 
> >> + * This table is generated using the AWK script below:
> >> + *
> >> + *  BEGIN {
> >> + *  suffix="KMGTPE";
> >> + *  for(i=10; i<64; i++) {
> >> + *  val=2**i;
> >> + *  s=substr(suffix, int(i/10), 1);
> >> + *  n=2**(i%10);
> >> + *  pad=21-int(log(n)/log(10));
> >> + *  printf("#define S_%d%siB %*d\n", n, s, pad, val);
> >> + *  }
> >> + *  }
> >> + */
> > 
> > If it is generated and the stringified are also generated, why not 
> > generate this once via the ./configure, since it is used by machines and 
> > not by humans?
> 
> You beat me to it! I was just thinking about that last evening. Indeed 
> it's not so elegant to put generated code in a file that is intended for 
> human handling. Generating by ./configure would be prettier.

We can still do this on top (and probably only for 3.2 as we're in
freeze now and it's neither a bug fix nor a documentation improvement or
test).

> A runtime solution that interprets hard-coded expression strings, like 
> Eric suggested, can also be a possibility, but it seems more complicated 
> than just generating these at the configuration stage. Plus one gets the 
> S_* constants that can be used in many places, not only where an 
> explicit number is needed. What do you think?

I think this is not a good use of our time when we want to use QAPI in
the long run anyway.

> A question though: if it to be generated by ./configure, where do you 
> suggest to put the generated values?

We already generate a lot of files, you could check where these end up.
But I suppose it might just be the root of the build directory (unless
an include/ exists there?)

> And also: is it OK to assume there's AWK (or another scripting
> language) on the system for generating these?

git grep says that configure already calls awk, so it's probably okay.
Of course, the existing test would just disable libgcrypt instead of
completely failing, so it's still a bit different.

As for other scripting languages, configure itself is written in shell,
and we also assume that Python is available (used e.g. by the QAPI
generators). At least these two are safe, too.

Kevin



Re: [Qemu-devel] [PATCH v2 2/2] tests/test-char: add muxed chardev testing for open/close

2018-11-06 Thread Marc-André Lureau
Hi

On Mon, Nov 5, 2018 at 4:47 PM Artem Pisarenko
 wrote:
>
> Validate that frontend callbacks for CHR_EVENT_OPENED/CHR_EVENT_CLOSED
> events are being issued when expected and in strictly pairing order.
>
> Signed-off-by: Artem Pisarenko 
> ---
>  tests/test-char.c | 80 
> +--
>  1 file changed, 78 insertions(+), 2 deletions(-)
>
> diff --git a/tests/test-char.c b/tests/test-char.c
> index 831e37f..657cb4c 100644
> --- a/tests/test-char.c
> +++ b/tests/test-char.c
> @@ -16,6 +16,9 @@ static bool quit;
>
>  typedef struct FeHandler {
>  int read_count;
> +bool is_open;
> +int openclose_count;
> +bool openclose_mismatch;
>  int last_event;
>  char read_buf[128];
>  } FeHandler;
> @@ -49,10 +52,24 @@ static void fe_read(void *opaque, const uint8_t *buf, int 
> size)
>  static void fe_event(void *opaque, int event)
>  {
>  FeHandler *h = opaque;
> +bool new_open_state;
>
>  h->last_event = event;
> -if (event != CHR_EVENT_BREAK) {
> +switch (event) {
> +case CHR_EVENT_BREAK:
> +break;
> +case CHR_EVENT_OPENED:
> +case CHR_EVENT_CLOSED:
> +h->openclose_count++;
> +new_open_state = (event == CHR_EVENT_OPENED);
> +if (h->is_open == new_open_state) {
> +h->openclose_mismatch = true;
> +}
> +h->is_open = new_open_state;
> +/* no break */
> +default:
>  quit = true;
> +break;
>  }
>  }
>
> @@ -161,7 +178,7 @@ static void char_mux_test(void)
>  QemuOpts *opts;
>  Chardev *chr, *base;
>  char *data;
> -FeHandler h1 = { 0, }, h2 = { 0, };
> +FeHandler h1 = { 0, false, 0, false, }, h2 = { 0, false, 0, false, };

this is unnecessary change, I can drop on commit

>  CharBackend chr_be1, chr_be2;
>
>  opts = qemu_opts_create(qemu_find_opts("chardev"), "mux-label",
> @@ -233,6 +250,65 @@ static void char_mux_test(void)
>  g_assert_cmpint(h1.last_event, ==, CHR_EVENT_BREAK);
>  g_assert_cmpint(h2.last_event, ==, CHR_EVENT_MUX_OUT);
>
> +/* open/close state and corresponding events */
> +g_assert_true(qemu_chr_fe_backend_open(&chr_be1));
> +g_assert_true(qemu_chr_fe_backend_open(&chr_be2));
> +g_assert_true(h1.is_open);
> +g_assert_false(h1.openclose_mismatch);
> +g_assert_true(h2.is_open);
> +g_assert_false(h2.openclose_mismatch);
> +
> +h1.openclose_count = h2.openclose_count = 0;
> +
> +qemu_chr_fe_set_handlers(&chr_be1, NULL, NULL, NULL, NULL,
> + NULL, NULL, false);
> +qemu_chr_fe_set_handlers(&chr_be2, NULL, NULL, NULL, NULL,
> + NULL, NULL, false);
> +g_assert_cmpint(h1.openclose_count, ==, 0);
> +g_assert_cmpint(h2.openclose_count, ==, 0);
> +
> +h1.is_open = h2.is_open = false;
> +qemu_chr_fe_set_handlers(&chr_be1,
> + NULL,
> + NULL,
> + fe_event,
> + NULL,
> + &h1,
> + NULL, false);
> +qemu_chr_fe_set_handlers(&chr_be2,
> + NULL,
> + NULL,
> + fe_event,
> + NULL,
> + &h2,
> + NULL, false);
> +g_assert_cmpint(h1.openclose_count, ==, 1);
> +g_assert_false(h1.openclose_mismatch);
> +g_assert_cmpint(h2.openclose_count, ==, 1);
> +g_assert_false(h2.openclose_mismatch);
> +
> +qemu_chr_be_event(base, CHR_EVENT_CLOSED);
> +qemu_chr_be_event(base, CHR_EVENT_OPENED);
> +g_assert_cmpint(h1.openclose_count, ==, 3);
> +g_assert_false(h1.openclose_mismatch);
> +g_assert_cmpint(h2.openclose_count, ==, 3);
> +g_assert_false(h2.openclose_mismatch);
> +
> +qemu_chr_fe_set_handlers(&chr_be2,
> + fe_can_read,
> + fe_read,
> + fe_event,
> + NULL,
> + &h2,
> + NULL, false);
> +qemu_chr_fe_set_handlers(&chr_be1,
> + fe_can_read,
> + fe_read,
> + fe_event,
> + NULL,
> + &h1,
> + NULL, false);
> +

otherwise, looks good to me
Reviewed-by: Marc-André Lureau 


>  /* remove first handler */
>  qemu_chr_fe_set_handlers(&chr_be1, NULL, NULL, NULL, NULL,
>   NULL, NULL, true);
> --
> 2.7.4
>
>


-- 
Marc-André Lureau



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

2018-11-06 Thread Peter Maydell
On 6 November 2018 at 11:07, Michael S. Tsirkin  wrote:
> On Tue, Nov 06, 2018 at 09:18:49AM +0100, Thomas Huth wrote:
>> On 2018-11-05 19:14, Michael S. Tsirkin wrote:
>> > The following changes since commit 
>> > b2f7a038bb4c4fc5ce6b8486e8513dfd97665e2a:
>> >
>> >   Merge remote-tracking branch 'remotes/rth/tags/pull-softfloat-20181104' 
>> > into staging (2018-11-05 10:32:49 +)
>> >
>> > are available in the Git repository at:
>> >
>> >   git://git.kernel.org/pub/scm/virt/kvm/mst/qemu.git tags/for_upstream
>> >
>> > for you to fetch changes up to 6196df5c8e6688c1c3f06f73442820066335337c:
>> >
>> >   vhost-scsi: prevent using uninitialized vqs (2018-11-05 12:59:35 -0500)
>> >
>> > 
>> > pci, pc, virtio: fixes, features
>> >
>> > AMD IOMMU VAPIC support + fixes all over the place.
>> >
>> > Signed-off-by: Michael S. Tsirkin 
>> >
>> > 
>> > Gerd Hoffmann (1):
>> >   pci-testdev: add optional memory bar
>> >
>> > Laszlo Ersek (4):
>> >   MAINTAINERS: list "tests/acpi-test-data" files in ACPI/SMBIOS section
>> [...]
>> >  tests/{acpi-test-data => data/acpi}/pc/APIC| Bin
>> >  tests/{acpi-test-data => data/acpi}/pc/APIC.cphp   | Bin
>> >  .../{acpi-test-data => data/acpi}/pc/APIC.dimmpxm  | Bin
>> >  tests/{acpi-test-data => data/acpi}/pc/DSDT| Bin
>>
>> So patch 1 moves tests/acpi-test-data/ to tests/data/acpi/ and patch 20
>> adds an entry for tests/acpi-test-data/ ? Does not make much sense to me
>> ... I think patch 20 needs to be adapted now.
>>
>>  Thomas
>
> Oh right, MAINTAINERS needs to be fixed. Can be done with a patch on top
> though.

Yeah, given the timing for rc0 I'll just apply this version of
the pullreq, and we can fix up MAINTAINERS afterwards.

thanks
-- PMM



[Qemu-devel] [PULL 0/1] Seabios 20181106 patches

2018-11-06 Thread Gerd Hoffmann
The following changes since commit 7d56239f159afc2e7bd42623947e56ba48f37836:

  Merge remote-tracking branch 'remotes/pmaydell/tags/pull-target-arm-20181102' 
into staging (2018-11-02 17:17:12 +)

are available in the git repository at:

  git://git.kraxel.org/qemu tags/seabios-20181106-pull-request

for you to fetch changes up to 0b8f74488e50f98b04e63157f85fde8a13f8d6aa:

  update seabios to master snapshot (2018-11-06 12:23:58 +0100)


seabios update for 3.1



Gerd Hoffmann (1):
  update seabios to master snapshot

 pc-bios/bios-256k.bin | Bin 262144 -> 262144 bytes
 pc-bios/bios.bin  | Bin 131072 -> 131072 bytes
 pc-bios/vgabios-bochs-display.bin | Bin 27648 -> 27648 bytes
 pc-bios/vgabios-cirrus.bin| Bin 38400 -> 38400 bytes
 pc-bios/vgabios-qxl.bin   | Bin 38912 -> 38912 bytes
 pc-bios/vgabios-ramfb.bin | Bin 28160 -> 28160 bytes
 pc-bios/vgabios-stdvga.bin| Bin 38912 -> 38912 bytes
 pc-bios/vgabios-virtio.bin| Bin 38912 -> 38912 bytes
 pc-bios/vgabios-vmware.bin| Bin 38912 -> 38912 bytes
 pc-bios/vgabios.bin   | Bin 38400 -> 38400 bytes
 roms/seabios  |   2 +-
 11 files changed, 1 insertion(+), 1 deletion(-)

-- 
2.9.3




Re: [Qemu-devel] [PATCH 1/2] The discard flag for block stream operation

2018-11-06 Thread Andrey Shinkevich
OK, David,

I will implement that with the next series.

Kindly,

Andrey Shinkevich


On 31.10.2018 20:38, Dr. David Alan Gilbert wrote:
> * Andrey Shinkevich (andrey.shinkev...@virtuozzo.com) wrote:
>> Adding a parameter to QMP block-stream command to allow discarding
>> blocks in the backing chain while blocks are being copied to the
>> active layer.
>>
>> Signed-off-by: Andrey Shinkevich 
>> ---
>>   block/stream.c| 2 +-
>>   blockdev.c| 8 +++-
>>   hmp-commands.hx   | 4 ++--
>>   hmp.c | 4 +++-
>>   include/block/block_int.h | 2 +-
>>   qapi/block-core.json  | 5 -
>>   6 files changed, 18 insertions(+), 7 deletions(-)
>>
>> diff --git a/block/stream.c b/block/stream.c
>> index 81a7ec8..db81df4 100644
>> --- a/block/stream.c
>> +++ b/block/stream.c
>> @@ -221,7 +221,7 @@ static const BlockJobDriver stream_job_driver = {
>>   
>>   void stream_start(const char *job_id, BlockDriverState *bs,
>> BlockDriverState *base, const char *backing_file_str,
>> -  int creation_flags, int64_t speed,
>> +  int creation_flags, int64_t speed, bool discard,
>> BlockdevOnError on_error, Error **errp)
>>   {
>>   StreamBlockJob *s;
>> diff --git a/blockdev.c b/blockdev.c
>> index 574adbc..04aecf5 100644
>> --- a/blockdev.c
>> +++ b/blockdev.c
>> @@ -3122,6 +3122,7 @@ void qmp_block_stream(bool has_job_id, const char 
>> *job_id, const char *device,
>> bool has_base_node, const char *base_node,
>> bool has_backing_file, const char *backing_file,
>> bool has_speed, int64_t speed,
>> +  bool has_discard, bool discard,
>> bool has_on_error, BlockdevOnError on_error,
>> bool has_auto_finalize, bool auto_finalize,
>> bool has_auto_dismiss, bool auto_dismiss,
>> @@ -3138,6 +3139,10 @@ void qmp_block_stream(bool has_job_id, const char 
>> *job_id, const char *device,
>>   on_error = BLOCKDEV_ON_ERROR_REPORT;
>>   }
>>   
>> +if (!has_discard) {
>> +discard = false;
>> +}
>> +
>>   bs = bdrv_lookup_bs(device, device, errp);
>>   if (!bs) {
>>   return;
>> @@ -3202,7 +3207,8 @@ void qmp_block_stream(bool has_job_id, const char 
>> *job_id, const char *device,
>>   }
>>   
>>   stream_start(has_job_id ? job_id : NULL, bs, base_bs, base_name,
>> - job_flags, has_speed ? speed : 0, on_error, &local_err);
>> + job_flags, has_speed ? speed : 0,
>> + discard, on_error, &local_err);
>>   if (local_err) {
>>   error_propagate(errp, local_err);
>>   goto out;
>> diff --git a/hmp-commands.hx b/hmp-commands.hx
>> index db0c681..b455e0d 100644
>> --- a/hmp-commands.hx
>> +++ b/hmp-commands.hx
>> @@ -95,8 +95,8 @@ ETEXI
>>   
>>   {
>>   .name   = "block_stream",
>> -.args_type  = "device:B,speed:o?,base:s?",
>> -.params = "device [speed [base]]",
>> +.args_type  = "device:B,speed:o?,base:s?,discard:o?",
> I think that 'o?' is wrong - see the table at the top of monitor.c, 'o'
> is octets, ? is optional - so speed here is an optional byte count, I
> think your 'discard' is just an optional flag.
> So I think you'd be better with a flag, like the -f on block_job_cancel.
>
>> +.params = "device [speed [base]] [discard]",
>>   .help   = "copy data from a backing file into a block device",
>>   .cmd= hmp_block_stream,
>>   },
>> diff --git a/hmp.c b/hmp.c
>> index 7828f93..c63e806 100644
>> --- a/hmp.c
>> +++ b/hmp.c
>> @@ -1920,9 +1920,11 @@ void hmp_block_stream(Monitor *mon, const QDict 
>> *qdict)
>>   const char *device = qdict_get_str(qdict, "device");
>>   const char *base = qdict_get_try_str(qdict, "base");
>>   int64_t speed = qdict_get_try_int(qdict, "speed", 0);
>> +bool discard = qdict_get_try_bool(qdict, "discard", false);
>>   
>>   qmp_block_stream(true, device, device, base != NULL, base, false, NULL,
>> - false, NULL, qdict_haskey(qdict, "speed"), speed, true,
>> + false, NULL, qdict_haskey(qdict, "speed"), speed,
>> + qdict_haskey(qdict, "discard"), discard, true,
> Since you've got the default 'false' on the bool discard =   above, I
> wonder if that can just be   true, discard, true?
>
> Dave
>
>>BLOCKDEV_ON_ERROR_REPORT, false, false, false, false,
>>&error);
>>   
>> diff --git a/include/block/block_int.h b/include/block/block_int.h
>> index 92ecbd8..e531d03 100644
>> --- a/include/block/block_int.h
>> +++ b/include/block/block_int.h
>> @@ -970,7 +970,7 @@ int is_windows_drive(const char *filename);
>>*/
>>   void stream_start(const char *job_id, BlockDriverState *bs,
>>  

Re: [Qemu-devel] [Qemu-block] [PATCH 1/2] The discard flag for block stream operation

2018-11-06 Thread Andrey Shinkevich
Berto,

Well noted about the "after implementation".

Kindly,

Andrey Shinkevich


On 05.11.2018 19:08, Alberto Garcia wrote:
> On Wed 31 Oct 2018 05:47:19 PM CET, Andrey Shinkevich 
>  wrote:
>> Adding a parameter to QMP block-stream command to allow discarding
>> blocks in the backing chain while blocks are being copied to the
>> active layer.
>>
>> Signed-off-by: Andrey Shinkevich 
> I haven't checked the other patch yet, but if you're going to add new
> API (this new 'discard' parameter) I suggest you to do it _after_ the
> implementation.
>
>> --- a/qapi/block-core.json
>> +++ b/qapi/block-core.json
>> @@ -2329,6 +2329,9 @@
>>   #
>>   # @speed:  the maximum speed, in bytes per second
>>   #
>> +# @discard: true to delete blocks duplicated in old backing files.
>> +#   (default: false). Since 3.1.
>> +#
>>   # @on-error: the action to take on an error (default report).
>>   #'stop' and 'enospc' can only be used if the block device
>>   #supports io-status (see BlockInfo).  Since 1.3.
>> @@ -2361,7 +2364,7 @@
>>   { 'command': 'block-stream',
>> 'data': { '*job-id': 'str', 'device': 'str', '*base': 'str',
>>   '*base-node': 'str', '*backing-file': 'str', '*speed': 'int',
>> -'*on-error': 'BlockdevOnError',
>> +'*discard': 'bool', '*on-error': 'BlockdevOnError',
>>   '*auto-finalize': 'bool', '*auto-dismiss': 'bool' } }
> Berto



Re: [Qemu-devel] [PATCH v2 1/2] chardev: fix mess in OPENED/CLOSED events when muxed

2018-11-06 Thread Marc-André Lureau
Hi

On Mon, Nov 5, 2018 at 4:46 PM Artem Pisarenko
 wrote:
>
> When chardev is multiplexed (mux=on) there are a lot of cases, when
> CHR_EVENT_OPENED/CHR_EVENT_CLOSED events pairing (expected from
> frontend side) is broken. There are either generation of multiple
> repeated or extra CHR_EVENT_OPENED events, or CHR_EVENT_CLOSED just
> isn't generated at all (when it does with mux=off).
> Fix that.
>
> Signed-off-by: Artem Pisarenko 

I can see that this fixes the mismatch you describe, and the changes
look reasonably simple that I can understand how it works. But if you
could also describe with your own words the "why" it happened and the
"how" you fixed it in the commit message, that would be really helpful
for the future readers (including ourself).

otherwise, patch looks good to me.

Since it's not for 3.1, after you send v3, I will queue those patches
for merge early in 3.2, so we get enough testing.

thanks!

> ---
>  chardev/char-fe.c | 33 -
>  chardev/char-mux.c| 16 
>  include/chardev/char-fe.h | 18 +-
>  3 files changed, 49 insertions(+), 18 deletions(-)
>
> diff --git a/chardev/char-fe.c b/chardev/char-fe.c
> index a8931f7..b7bcbd5 100644
> --- a/chardev/char-fe.c
> +++ b/chardev/char-fe.c
> @@ -246,14 +246,15 @@ void qemu_chr_fe_deinit(CharBackend *b, bool del)
>  }
>  }
>
> -void qemu_chr_fe_set_handlers(CharBackend *b,
> -  IOCanReadHandler *fd_can_read,
> -  IOReadHandler *fd_read,
> -  IOEventHandler *fd_event,
> -  BackendChangeHandler *be_change,
> -  void *opaque,
> -  GMainContext *context,
> -  bool set_open)
> +void qemu_chr_fe_set_handlers_full(CharBackend *b,
> +   IOCanReadHandler *fd_can_read,
> +   IOReadHandler *fd_read,
> +   IOEventHandler *fd_event,
> +   BackendChangeHandler *be_change,
> +   void *opaque,
> +   GMainContext *context,
> +   bool set_open,
> +   bool sync_state)
>  {
>  Chardev *s;
>  int fe_open;
> @@ -285,7 +286,7 @@ void qemu_chr_fe_set_handlers(CharBackend *b,
>  qemu_chr_fe_take_focus(b);
>  /* We're connecting to an already opened device, so let's make sure 
> we
> also get the open event */
> -if (s->be_open) {
> +if (sync_state && s->be_open) {
>  qemu_chr_be_event(s, CHR_EVENT_OPENED);
>  }
>  }
> @@ -295,6 +296,20 @@ void qemu_chr_fe_set_handlers(CharBackend *b,
>  }
>  }
>
> +void qemu_chr_fe_set_handlers(CharBackend *b,
> +  IOCanReadHandler *fd_can_read,
> +  IOReadHandler *fd_read,
> +  IOEventHandler *fd_event,
> +  BackendChangeHandler *be_change,
> +  void *opaque,
> +  GMainContext *context,
> +  bool set_open)
> +{
> +qemu_chr_fe_set_handlers_full(b, fd_can_read, fd_read, fd_event, 
> be_change,
> +  opaque, context, set_open,
> +  true);
> +}
> +
>  void qemu_chr_fe_take_focus(CharBackend *b)
>  {
>  if (!b->chr) {
> diff --git a/chardev/char-mux.c b/chardev/char-mux.c
> index 6055e76..1199d32 100644
> --- a/chardev/char-mux.c
> +++ b/chardev/char-mux.c
> @@ -283,13 +283,13 @@ void mux_chr_set_handlers(Chardev *chr, GMainContext 
> *context)
>  MuxChardev *d = MUX_CHARDEV(chr);
>
>  /* Fix up the real driver with mux routines */
> -qemu_chr_fe_set_handlers(&d->chr,
> - mux_chr_can_read,
> - mux_chr_read,
> - mux_chr_event,
> - NULL,
> - chr,
> - context, true);
> +qemu_chr_fe_set_handlers_full(&d->chr,
> +  mux_chr_can_read,
> +  mux_chr_read,
> +  mux_chr_event,
> +  NULL,
> +  chr,
> +  context, true, false);
>  }
>
>  void mux_set_focus(Chardev *chr, int focus)
> @@ -367,7 +367,7 @@ static int open_muxes(Chardev *chr)
>   * mark mux as OPENED so any new FEs will immediately receive
>   * OPENED event
>   */
> -qemu_chr_be_event(chr, CHR_EVENT_OPENED);
> +chr->be_open = 1;
>
>  return 0;
>  }
> diff --git a/include/chardev/char-fe.h b/include/chardev/char-fe.h
> index 46c997d..c

[Qemu-devel] [PULL 5/5] target/arm: Fix ATS1Hx instructions

2018-11-06 Thread Peter Maydell
ATS1HR and ATS1HW (which allow AArch32 EL2 to do address translations
on the EL2 translation regime) were implemented in commit 14db7fe09a2c8.
However, we got them wrong: these should do stage 1 address translations
as defined for NS-EL2, which is ARMMMUIdx_S1E2. We were incorrectly
making them perform stage 2 translations.

A few years later in commit 1313e2d7e2cd we forgot entirely that
we'd implemented ATS1Hx, and added a comment that ATS1Hx were
"not supported yet". Remove the comment; there is no extra code
needed to handle these operations in do_ats_write(), because
arm_s1_regime_using_lpae_format() returns true for ARMMMUIdx_S1E2,
which forces 64-bit PAR format.

Signed-off-by: Peter Maydell 
Reviewed-by: Alex Bennée 
Message-id: 20181016093703.10637-3-peter.mayd...@linaro.org
Reviewed-by: Edgar E. Iglesias 
---
 target/arm/helper.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/target/arm/helper.c b/target/arm/helper.c
index 69f684abd89..96301930cc8 100644
--- a/target/arm/helper.c
+++ b/target/arm/helper.c
@@ -2319,7 +2319,7 @@ static uint64_t do_ats_write(CPUARMState *env, uint64_t 
value,
  *
  * (Note that HCR.DC makes HCR.VM behave as if it is 1.)
  *
- * ATS1Hx always uses the 64bit format (not supported yet).
+ * ATS1Hx always uses the 64bit format.
  */
 format64 = arm_s1_regime_using_lpae_format(env, mmu_idx);
 
@@ -2444,7 +2444,7 @@ static void ats1h_write(CPUARMState *env, const 
ARMCPRegInfo *ri,
 MMUAccessType access_type = ri->opc2 & 1 ? MMU_DATA_STORE : MMU_DATA_LOAD;
 uint64_t par64;
 
-par64 = do_ats_write(env, value, access_type, ARMMMUIdx_S2NS);
+par64 = do_ats_write(env, value, access_type, ARMMMUIdx_S1E2);
 
 A32_BANKED_CURRENT_REG_SET(env, par, par64);
 }
-- 
2.19.1




[Qemu-devel] [PULL 2/5] milkymist: Check for failure trying to load BIOS image

2018-11-06 Thread Peter Maydell
Check the return value from load_image_targphys(), which tells us
whether our attempt to load the BIOS image into RAM failed.
(Spotted by Coverity, CID 1190305.)

Signed-off-by: Peter Maydell 
Reviewed-by: Philippe Mathieu-Daudé 
Acked-by: Michael Walle 
Message-id: 20181030170032.1844-1-peter.mayd...@linaro.org
---
 hw/lm32/milkymist.c | 5 -
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/hw/lm32/milkymist.c b/hw/lm32/milkymist.c
index 321f184595e..63c6894c955 100644
--- a/hw/lm32/milkymist.c
+++ b/hw/lm32/milkymist.c
@@ -138,7 +138,10 @@ milkymist_init(MachineState *machine)
 bios_filename = qemu_find_file(QEMU_FILE_TYPE_BIOS, bios_name);
 
 if (bios_filename) {
-load_image_targphys(bios_filename, BIOS_OFFSET, BIOS_SIZE);
+if (load_image_targphys(bios_filename, BIOS_OFFSET, BIOS_SIZE) < 0) {
+error_report("could not load bios '%s'", bios_filename);
+exit(1);
+}
 }
 
 reset_info->bootstrap_pc = BIOS_OFFSET;
-- 
2.19.1




[Qemu-devel] [PULL 0/5] target-arm queue

2018-11-06 Thread Peter Maydell
Handful of bugfix patches for arm for rc0; also
one milkymist patch, thrown in since I was putting
the pullreq together anyway.

thanks
-- PMM

The following changes since commit 03c1ca1c51783603d42eb0f91d35961f0f4b4947:

  Merge remote-tracking branch 'remotes/cohuck/tags/s390x-20181105' into 
staging (2018-11-06 09:10:46 +)

are available in the Git repository at:

  https://git.linaro.org/people/pmaydell/qemu-arm.git 
tags/pull-target-arm-20181106

for you to fetch changes up to 23463e0e4aeb2f0a9c60549a2c163f4adc0b8512:

  target/arm: Fix ATS1Hx instructions (2018-11-06 11:32:14 +)


target-arm queue:
 * Remove can't-happen if() from handle_vec_simd_shli()
 * hw/arm/exynos4210: Zero memory allocated for Exynos4210State
 * Set S and PTW in 64-bit PAR format
 * Fix ATS1Hx instructions
 * milkymist: Check for failure trying to load BIOS image


Peter Maydell (5):
  target/arm: Remove can't-happen if() from handle_vec_simd_shli()
  milkymist: Check for failure trying to load BIOS image
  hw/arm/exynos4210: Zero memory allocated for Exynos4210State
  target/arm: Set S and PTW in 64-bit PAR format
  target/arm: Fix ATS1Hx instructions

 hw/arm/exynos4210.c|  2 +-
 hw/lm32/milkymist.c|  5 -
 target/arm/helper.c| 14 --
 target/arm/translate-a64.c |  8 +++-
 4 files changed, 16 insertions(+), 13 deletions(-)



[Qemu-devel] [PULL 1/5] target/arm: Remove can't-happen if() from handle_vec_simd_shli()

2018-11-06 Thread Peter Maydell
In handle_vec_simd_shli() we have a check:
 if (size > 3 && !is_q) {
 unallocated_encoding(s);
 return;
 }
However this can never be true, because we calculate
int size = 32 - clz32(immh) - 1;
where immh is a 4 bit field which we know cannot be all-zeroes.
So the clz32() return must be in {28,29,30,31} and the resulting
size is in {0,1,2,3}, and "size > 3" is never true.

This unnecessary code confuses Coverity's analysis:
in CID 1396476 it thinks we might later index off the
end of an array because the condition implies that we
might have a size > 3.

Remove the code, and instead assert that the size is in [0..3],
since the decode that enforces that is somewhat distant from
this function.

Signed-off-by: Peter Maydell 
Reviewed-by: Philippe Mathieu-Daudé 
Reviewed-by: Alex Bennée 
Tested-by: Alex Bennée 
Message-id: 20181030162517.21816-1-peter.mayd...@linaro.org
---
 target/arm/translate-a64.c | 8 +++-
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/target/arm/translate-a64.c b/target/arm/translate-a64.c
index 88195ab9490..fd36425f1ae 100644
--- a/target/arm/translate-a64.c
+++ b/target/arm/translate-a64.c
@@ -9483,12 +9483,10 @@ static void handle_vec_simd_shli(DisasContext *s, bool 
is_q, bool insert,
 int immhb = immh << 3 | immb;
 int shift = immhb - (8 << size);
 
-if (extract32(immh, 3, 1) && !is_q) {
-unallocated_encoding(s);
-return;
-}
+/* Range of size is limited by decode: immh is a non-zero 4 bit field */
+assert(size >= 0 && size <= 3);
 
-if (size > 3 && !is_q) {
+if (extract32(immh, 3, 1) && !is_q) {
 unallocated_encoding(s);
 return;
 }
-- 
2.19.1




[Qemu-devel] [PULL 4/5] target/arm: Set S and PTW in 64-bit PAR format

2018-11-06 Thread Peter Maydell
In do_ats_write() we construct a PAR value based on the result
of the translation.  A comment says "S2WLK and FSTAGE are always
zero, because we don't implement virtualization".
Since we do in fact now implement virtualization, add the missing
code that sets these bits based on the reported ARMMMUFaultInfo.

(These bits are named PTW and S in ARMv8, so we follow that
convention in the new comments in this patch.)

Signed-off-by: Peter Maydell 
Reviewed-by: Edgar E. Iglesias 
Reviewed-by: Alex Bennée 
Message-id: 20181016093703.10637-2-peter.mayd...@linaro.org
---
 target/arm/helper.c | 10 ++
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/target/arm/helper.c b/target/arm/helper.c
index 0ea95b08151..69f684abd89 100644
--- a/target/arm/helper.c
+++ b/target/arm/helper.c
@@ -2347,10 +2347,12 @@ static uint64_t do_ats_write(CPUARMState *env, uint64_t 
value,
 
 par64 |= 1; /* F */
 par64 |= (fsr & 0x3f) << 1; /* FS */
-/* Note that S2WLK and FSTAGE are always zero, because we don't
- * implement virtualization and therefore there can't be a stage 2
- * fault.
- */
+if (fi.stage2) {
+par64 |= (1 << 9); /* S */
+}
+if (fi.s1ptw) {
+par64 |= (1 << 8); /* PTW */
+}
 }
 } else {
 /* fsr is a DFSR/IFSR value for the short descriptor
-- 
2.19.1




[Qemu-devel] [PULL 3/5] hw/arm/exynos4210: Zero memory allocated for Exynos4210State

2018-11-06 Thread Peter Maydell
In exynos4210_init() we allocate memory for an Exynos4210State
struct. Generally devices can assume that the memory allocated
for their state struct is zero-initialized; we broke that
assumption here by using g_new(). Use g_new0() instead.
(In particular, some code assumes that the various irq arrays
in the Exynos4210Irq sub-struct are zero-initialized.)

In the longer term, this code should be QOMified, and then
the struct memory will be allocated elsewhere and by functions
which always zero-initalize it; but for 3.1 this is a
simple fix.

Signed-off-by: Peter Maydell 
Reviewed-by: Philippe Mathieu-Daudé 
Tested-by: Philippe Mathieu-Daudé 
Message-id: 20181105151132.13884-1-peter.mayd...@linaro.org
---
 hw/arm/exynos4210.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/arm/exynos4210.c b/hw/arm/exynos4210.c
index 827318a0036..af82e955421 100644
--- a/hw/arm/exynos4210.c
+++ b/hw/arm/exynos4210.c
@@ -162,7 +162,7 @@ static uint64_t exynos4210_calc_affinity(int cpu)
 
 Exynos4210State *exynos4210_init(MemoryRegion *system_mem)
 {
-Exynos4210State *s = g_new(Exynos4210State, 1);
+Exynos4210State *s = g_new0(Exynos4210State, 1);
 qemu_irq gate_irq[EXYNOS4210_NCPUS][EXYNOS4210_IRQ_GATE_NINPUTS];
 SysBusDevice *busdev;
 DeviceState *dev;
-- 
2.19.1




Re: [Qemu-devel] [PATCH v3] oslib-posix: Use MAP_STACK in qemu_alloc_stack() on OpenBSD

2018-11-06 Thread Peter Maydell
On 5 November 2018 at 22:44, Brad Smith  wrote:
> ping.
>

Thanks for the ping, applied to master for rc0.

-- PMM



Re: [Qemu-devel] [PULL v2 2/5] tests/tpm: Display if swtpm is not found or --tpm2 not supported

2018-11-06 Thread Thomas Huth
On 2018-10-30 22:47, Stefan Berger wrote:
> From: Marc-André Lureau 
> 
> If swtpm is not found in $PATH or --tpm2 isn't supported, we display
> this in the test log. We cannot mark the test as skipped due to a bug in
> certain versions of the gtester environment that interprets a skipped test
> as failure.
> 
> Signed-off-by: Marc-André Lureau 
> Reviewed-by: Stefan Berger 
> Signed-off-by: Stefan Berger 
> ---
>  tests/tpm-tests.c | 33 +
>  tests/tpm-util.c  |  8 +---
>  tests/tpm-util.h  |  2 ++
>  3 files changed, 24 insertions(+), 19 deletions(-)
> 
> diff --git a/tests/tpm-tests.c b/tests/tpm-tests.c
> index 10c6592aac..93a5beba01 100644
> --- a/tests/tpm-tests.c
> +++ b/tests/tpm-tests.c
> @@ -18,6 +18,17 @@
>  #include "libqtest.h"
>  #include "tpm-tests.h"
>  
> +static bool
> +tpm_test_swtpm_skip(void)
> +{
> +if (!tpm_util_swtpm_has_tpm2()) {
> +fprintf(stderr, "swtpm not in PATH or missing --tpm2 support; ");
> +return true;
> +}

I now get this ugly (4-times duplicated) output each time I run "make check":

swtpm not in PATH or missing --tpm2 support; swtpm not in PATH or missing 
--tpm2 support; swtpm not in PATH or missing --tpm2 support; swtpm not in PATH 
or missing --tpm2 support;

Could you please use g_test_message() here instead like we're doing
it in most of the other tests already?

 Thanks,
  Thomas



[Qemu-devel] aarch32 acquire/release vs mttcg

2018-11-06 Thread Peter Maydell
Looking through code I noticed that in target/arm/translate.c there
is the following comment in the decode for the load-acquire/store-release
instructions:

  /* Since the emulation does not have barriers,
 the acquire/release semantics need no special
 handling */

This is out of date now we have MTTCG, isn't it?
The equivalent code in translate-a64.c has some calls to tcg_gen_mb()
to emit barriers...presumably we need to fix the a32 code to do
the same.

(We haven't noticed this because I think in practice nobody
much is compiling Armv8-specific AArch32 code.)

thanks
-- PMM



[Qemu-devel] [PATCH] lsi53c895a: check script ram address value

2018-11-06 Thread P J P
From: Prasad J Pandit 

While accessing script ram[2048] via 'lsi_ram_read/write' routines,
'addr' could exceed the ram range. Mask high order bits to avoid
OOB access.

Reported-by: Mark Kanda 
Signed-off-by: Prasad J Pandit 
---
 hw/scsi/lsi53c895a.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/hw/scsi/lsi53c895a.c b/hw/scsi/lsi53c895a.c
index 3f207f607c..0800df416e 100644
--- a/hw/scsi/lsi53c895a.c
+++ b/hw/scsi/lsi53c895a.c
@@ -2035,6 +2035,7 @@ static void lsi_ram_write(void *opaque, hwaddr addr,
 uint32_t mask;
 int shift;
 
+addr &= 0x01FFF;
 newval = s->script_ram[addr >> 2];
 shift = (addr & 3) * 8;
 mask = ((uint64_t)1 << (size * 8)) - 1;
@@ -2050,6 +2051,7 @@ static uint64_t lsi_ram_read(void *opaque, hwaddr addr,
 uint32_t val;
 uint32_t mask;
 
+addr &= 0x01FFF;
 val = s->script_ram[addr >> 2];
 mask = ((uint64_t)1 << (size * 8)) - 1;
 val >>= (addr & 3) * 8;
-- 
2.17.2




Re: [Qemu-devel] [PATCH] lsi53c895a: check script ram address value

2018-11-06 Thread Peter Maydell
On 6 November 2018 at 11:53, P J P  wrote:
> From: Prasad J Pandit 
>
> While accessing script ram[2048] via 'lsi_ram_read/write' routines,
> 'addr' could exceed the ram range. Mask high order bits to avoid
> OOB access.
>
> Reported-by: Mark Kanda 
> Signed-off-by: Prasad J Pandit 
> ---
>  hw/scsi/lsi53c895a.c | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/hw/scsi/lsi53c895a.c b/hw/scsi/lsi53c895a.c
> index 3f207f607c..0800df416e 100644
> --- a/hw/scsi/lsi53c895a.c
> +++ b/hw/scsi/lsi53c895a.c
> @@ -2035,6 +2035,7 @@ static void lsi_ram_write(void *opaque, hwaddr addr,
>  uint32_t mask;
>  int shift;
>
> +addr &= 0x01FFF;
>  newval = s->script_ram[addr >> 2];
>  shift = (addr & 3) * 8;
>  mask = ((uint64_t)1 << (size * 8)) - 1;
> @@ -2050,6 +2051,7 @@ static uint64_t lsi_ram_read(void *opaque, hwaddr addr,
>  uint32_t val;
>  uint32_t mask;
>
> +addr &= 0x01FFF;
>  val = s->script_ram[addr >> 2];
>  mask = ((uint64_t)1 << (size * 8)) - 1;
>  val >>= (addr & 3) * 8;
> --

When can this masking have any effect? These functions are
the read and write ops for lsi_ram_ops, which we register with
memory_region_init_io(&s->ram_io, OBJECT(s), &lsi_ram_ops, s,
  "lsi-ram", 0x2000);
which specifies a memory region size of 0x2000. So the input
addr must be in the 0..0x1fff range already -- or have I missed
something ?

It would probably be helpful (for readers and static analysers)
to assert() that addr is < 0x2000, though.

thanks
-- PMM



[Qemu-devel] [PATCH] block/nvme: call blk_drain in NVMe reset code to avoid lockups

2018-11-06 Thread Igor Druzhinin
When blk_flush called in NVMe reset path S/C queues are already freed
which means that re-entering AIO handling loop having some IO requests
unfinished will lockup or crash as their SG structures being potentially
reused. Call blk_drain before freeing the queues to avoid this nasty
scenario.

Signed-off-by: Igor Druzhinin 
---
 hw/block/nvme.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/hw/block/nvme.c b/hw/block/nvme.c
index fc7dacb..cdf836e 100644
--- a/hw/block/nvme.c
+++ b/hw/block/nvme.c
@@ -797,6 +797,8 @@ static void nvme_clear_ctrl(NvmeCtrl *n)
 {
 int i;
 
+blk_drain(n->conf.blk);
+
 for (i = 0; i < n->num_queues; i++) {
 if (n->sq[i] != NULL) {
 nvme_free_sq(n->sq[i], n);
-- 
2.7.4




[Qemu-devel] [PATCH v2 0/5] migration: improve multithreads

2018-11-06 Thread guangrong . xiao
From: Xiao Guangrong 

Changelog in v2:
These changes are based on Paolo's suggestion:
1) rename the lockless multithreads model to threaded workqueue
2) hugely improve the internal design, that make all the request be
   a large array, properly partition it, assign requests to threads
   respectively and use bitmaps to sync up threads and the submitter,
   after that ptr_ring and spinlock are dropped
3) introduce event wait for the submitter

These changes are based on Emilio's review:
4) make more detailed description for threaded workqueue
5) add a benchmark for threaded workqueue

The previous version can be found at
https://marc.info/?l=kvm&m=153968821910007&w=2

There's the simple performance measurement comparing these two versions,
the environment is the same as we listed in the previous version.

Use 8 threads to compress the data in the source QEMU
- with compress-wait-thread = off


  total timebusy-ratio
--
v11250660.38
v21204440.35

- with compress-wait-thread = on
 total timebusy-ratio
--
v11644260
v21426090

The v2 win slightly.

Xiao Guangrong (5):
  bitops: introduce change_bit_atomic
  util: introduce threaded workqueue
  migration: use threaded workqueue for compression
  migration: use threaded workqueue for decompression
  tests: add threaded-workqueue-bench

 include/qemu/bitops.h |  13 +
 include/qemu/threaded-workqueue.h |  94 +++
 migration/ram.c   | 538 ++
 tests/Makefile.include|   5 +-
 tests/threaded-workqueue-bench.c  | 256 ++
 util/Makefile.objs|   1 +
 util/threaded-workqueue.c | 466 +
 7 files changed, 1030 insertions(+), 343 deletions(-)
 create mode 100644 include/qemu/threaded-workqueue.h
 create mode 100644 tests/threaded-workqueue-bench.c
 create mode 100644 util/threaded-workqueue.c

-- 
2.14.5




[Qemu-devel] [PATCH v2 2/5] util: introduce threaded workqueue

2018-11-06 Thread guangrong . xiao
From: Xiao Guangrong 

This modules implements the lockless and efficient threaded workqueue.

Three abstracted objects are used in this module:
- Request.
 It not only contains the data that the workqueue fetches out
to finish the request but also offers the space to save the result
after the workqueue handles the request.

It's flowed between user and workqueue. The user fills the request
data into it when it is owned by user. After it is submitted to the
workqueue, the workqueue fetched data out and save the result into
it after the request is handled.

All the requests are pre-allocated and carefully partitioned between
threads so there is no contention on the request, that make threads
be parallel as much as possible.

  - User, i.e, the submitter
It's the one fills the request and submits it to the workqueue,
the result will be collected after it is handled by the work queue.

The user can consecutively submit requests without waiting the previous
requests been handled.
It only supports one submitter, you should do serial submission by
yourself if you want more, e.g, use lock on you side.

  - Workqueue, i.e, thread
Each workqueue is represented by a running thread that fetches
the request submitted by the user, do the specified work and save
the result to the request.

Signed-off-by: Xiao Guangrong 
---
 include/qemu/threaded-workqueue.h |  94 
 util/Makefile.objs|   1 +
 util/threaded-workqueue.c | 466 ++
 3 files changed, 561 insertions(+)
 create mode 100644 include/qemu/threaded-workqueue.h
 create mode 100644 util/threaded-workqueue.c

diff --git a/include/qemu/threaded-workqueue.h 
b/include/qemu/threaded-workqueue.h
new file mode 100644
index 00..d7eb66c8d2
--- /dev/null
+++ b/include/qemu/threaded-workqueue.h
@@ -0,0 +1,94 @@
+/*
+ * Lockless and Efficient Threaded Workqueue Abstraction
+ *
+ * Author:
+ *   Xiao Guangrong 
+ *
+ * Copyright(C) 2018 Tencent Corporation.
+ *
+ * This work is licensed under the terms of the GNU LGPL, version 2.1 or later.
+ * See the COPYING.LIB file in the top-level directory.
+ */
+
+#ifndef QEMU_THREADED_WORKQUEUE_H
+#define QEMU_THREADED_WORKQUEUE_H
+
+#include "qemu/queue.h"
+#include "qemu/thread.h"
+
+/*
+ * This modules implements the lockless and efficient threaded workqueue.
+ *
+ * Three abstracted objects are used in this module:
+ * - Request.
+ *   It not only contains the data that the workqueue fetches out
+ *   to finish the request but also offers the space to save the result
+ *   after the workqueue handles the request.
+ *
+ *   It's flowed between user and workqueue. The user fills the request
+ *   data into it when it is owned by user. After it is submitted to the
+ *   workqueue, the workqueue fetched data out and save the result into
+ *   it after the request is handled.
+ *
+ *   All the requests are pre-allocated and carefully partitioned between
+ *   threads so there is no contention on the request, that make threads
+ *   be parallel as much as possible.
+ *
+ * - User, i.e, the submitter
+ *   It's the one fills the request and submits it to the workqueue,
+ *   the result will be collected after it is handled by the work queue.
+ *
+ *   The user can consecutively submit requests without waiting the previous
+ *   requests been handled.
+ *   It only supports one submitter, you should do serial submission by
+ *   yourself if you want more, e.g, use lock on you side.
+ *
+ * - Workqueue, i.e, thread
+ *   Each workqueue is represented by a running thread that fetches
+ *   the request submitted by the user, do the specified work and save
+ *   the result to the request.
+ */
+
+typedef struct Threads Threads;
+
+struct ThreadedWorkqueueOps {
+/* return the size of each request */
+int (*thread_get_request_size)(void);
+
+/* constructor of the request */
+int (*thread_request_init)(void *request);
+/*  destructor of the request */
+void (*thread_request_uninit)(void *request);
+
+/* the handler of the request that is called by the thread */
+void (*thread_request_handler)(void *request);
+/* called by the user after the request has been handled */
+void (*thread_request_done)(void *request);
+};
+typedef struct ThreadedWorkqueueOps ThreadedWorkqueueOps;
+
+/* the default number of requests that thread need handle */
+#define DEFAULT_THREAD_REQUEST_NR 4
+
+Threads *threaded_workqueue_create(const char *name, unsigned int threads_nr,
+   int thread_request_nr,
+   ThreadedWorkqueueOps *ops);
+
+void threaded_workqueue_destroy(Threads *threads);
+
+/*
+ * find a free request where the user can store the data that is needed to
+ * finish the request
+ *
+ * If all requests are used up, return NULL
+ */
+void *threaded_workqueue_get_request(Threads *threads);
+/* submit the request

[Qemu-devel] [PATCH v2 5/5] tests: add threaded-workqueue-bench

2018-11-06 Thread guangrong . xiao
From: Xiao Guangrong 

It's the benhcmark of threaded-workqueue, also it's a good
example to show how threaded-workqueue is used

Signed-off-by: Xiao Guangrong 
---
 tests/Makefile.include   |   5 +-
 tests/threaded-workqueue-bench.c | 256 +++
 2 files changed, 260 insertions(+), 1 deletion(-)
 create mode 100644 tests/threaded-workqueue-bench.c

diff --git a/tests/Makefile.include b/tests/Makefile.include
index d2e577eabb..a4deb210ab 100644
--- a/tests/Makefile.include
+++ b/tests/Makefile.include
@@ -499,7 +499,8 @@ test-obj-y = tests/check-qnum.o tests/check-qstring.o 
tests/check-qdict.o \
tests/test-rcu-tailq.o \
tests/test-qdist.o tests/test-shift128.o \
tests/test-qht.o tests/qht-bench.o tests/test-qht-par.o \
-   tests/atomic_add-bench.o tests/atomic64-bench.o
+   tests/atomic_add-bench.o tests/atomic64-bench.o \
+   tests/threaded-workqueue-bench.o
 
 $(test-obj-y): QEMU_INCLUDES += -Itests
 QEMU_CFLAGS += -I$(SRC_PATH)/tests
@@ -555,6 +556,8 @@ tests/qht-bench$(EXESUF): tests/qht-bench.o 
$(test-util-obj-y)
 tests/test-bufferiszero$(EXESUF): tests/test-bufferiszero.o $(test-util-obj-y)
 tests/atomic_add-bench$(EXESUF): tests/atomic_add-bench.o $(test-util-obj-y)
 tests/atomic64-bench$(EXESUF): tests/atomic64-bench.o $(test-util-obj-y)
+tests/threaded-workqueue-bench$(EXESUF): tests/threaded-workqueue-bench.o 
migration/qemu-file.o \
+   $(test-util-obj-y)
 
 tests/fp/%:
$(MAKE) -C $(dir $@) $(notdir $@)
diff --git a/tests/threaded-workqueue-bench.c b/tests/threaded-workqueue-bench.c
new file mode 100644
index 00..88026f1a8f
--- /dev/null
+++ b/tests/threaded-workqueue-bench.c
@@ -0,0 +1,256 @@
+/*
+ * Threaded Workqueue Benchmark
+ *
+ * Author:
+ *   Xiao Guangrong 
+ *
+ * Copyright(C) 2018 Tencent Corporation.
+ *
+ * This work is licensed under the terms of the GNU LGPL, version 2.1 or later.
+ * See the COPYING.LIB file in the top-level directory.
+ */
+#include 
+
+#include "qemu/osdep.h"
+#include "exec/cpu-common.h"
+#include "qemu/error-report.h"
+#include "migration/qemu-file.h"
+#include "qemu/threaded-workqueue.h"
+
+#define PAGE_SHIFT  12
+#define PAGE_SIZE   (1 << PAGE_SHIFT)
+#define DEFAULT_THREAD_NR   2
+#define DEFAULT_MEM_SIZE1
+#define DEFAULT_REPEATED_COUNT  3
+
+static ssize_t test_writev_buffer(void *opaque, struct iovec *iov, int iovcnt,
+   int64_t pos)
+{
+int i, size = 0;
+
+for (i = 0; i < iovcnt; i++) {
+size += iov[i].iov_len;
+}
+return size;
+}
+
+static int test_fclose(void *opaque)
+{
+return 0;
+}
+
+static const QEMUFileOps test_write_ops = {
+.writev_buffer  = test_writev_buffer,
+.close  = test_fclose
+};
+
+static QEMUFile *dest_file;
+
+static const QEMUFileOps empty_ops = { };
+
+struct CompressData {
+uint8_t *ram_addr;
+QEMUFile *file;
+z_stream stream;
+};
+typedef struct CompressData CompressData;
+
+static int compress_request_size(void)
+{
+return sizeof(CompressData);
+}
+
+static int compress_request_init(void *request)
+{
+CompressData *cd = request;
+
+if (deflateInit(&cd->stream, 1) != Z_OK) {
+return -1;
+}
+cd->file = qemu_fopen_ops(NULL, &empty_ops);
+return 0;
+}
+
+static void compress_request_uninit(void *request)
+{
+CompressData *cd = request;
+
+qemu_fclose(cd->file);
+deflateEnd(&cd->stream);
+}
+
+static void compress_thread_data_handler(void *request)
+{
+CompressData *cd = request;
+int blen;
+
+blen = qemu_put_compression_data(cd->file, &cd->stream, cd->ram_addr,
+ PAGE_SIZE);
+if (blen < 0) {
+error_report("compressed data failed!");
+qemu_file_set_error(dest_file, blen);
+}
+}
+
+struct CompressStats {
+unsigned long pages;
+unsigned long compressed_size;
+};
+typedef struct CompressStats CompressStats;
+
+static CompressStats comp_stats;
+
+static void compress_thread_data_done(void *request)
+{
+CompressData *cd = request;
+int bytes_xmit;
+
+bytes_xmit = qemu_put_qemu_file(dest_file, cd->file);
+
+comp_stats.pages++;
+comp_stats.compressed_size += bytes_xmit;
+}
+
+static ThreadedWorkqueueOps ops = {
+.thread_get_request_size = compress_request_size,
+.thread_request_init = compress_request_init,
+.thread_request_uninit = compress_request_uninit,
+.thread_request_handler = compress_thread_data_handler,
+.thread_request_done = compress_thread_data_done,
+};
+
+static void compress_threads_save_cleanup(Threads *threads)
+{
+threaded_workqueue_destroy(threads);
+qemu_fclose(dest_file);
+}
+
+static Threads *compress_threads_save_setup(int threads_nr, int requests_nr)
+{
+Threads *compress_threads;
+
+dest_file = qemu_fopen_ops(NULL, &test_write_ops);
+compress_threads = threaded_workqueue_create("compress", threads_nr,
+ 

[Qemu-devel] [PATCH v2 4/5] migration: use threaded workqueue for decompression

2018-11-06 Thread guangrong . xiao
From: Xiao Guangrong 

Adapt the compression code to the threaded workqueue

Signed-off-by: Xiao Guangrong 
---
 migration/ram.c | 225 
 1 file changed, 81 insertions(+), 144 deletions(-)

diff --git a/migration/ram.c b/migration/ram.c
index acca842aff..834198f11c 100644
--- a/migration/ram.c
+++ b/migration/ram.c
@@ -350,25 +350,9 @@ typedef struct PageSearchStatus PageSearchStatus;
 
 CompressionStats compression_counters;
 
-struct DecompressParam {
-bool done;
-bool quit;
-QemuMutex mutex;
-QemuCond cond;
-void *des;
-uint8_t *compbuf;
-int len;
-z_stream stream;
-};
-typedef struct DecompressParam DecompressParam;
-
 static const QEMUFileOps empty_ops = { };
 
 static QEMUFile *decomp_file;
-static DecompressParam *decomp_param;
-static QemuThread *decompress_threads;
-static QemuMutex decomp_done_lock;
-static QemuCond decomp_done_cond;
 
 /* Multiple fd's */
 
@@ -3404,6 +3388,7 @@ void ram_handle_compressed(void *host, uint8_t ch, 
uint64_t size)
 }
 }
 
+
 /* return the size after decompression, or negative value on error */
 static int
 qemu_uncompress_data(z_stream *stream, uint8_t *dest, size_t dest_len,
@@ -3429,166 +3414,118 @@ qemu_uncompress_data(z_stream *stream, uint8_t *dest, 
size_t dest_len,
 return stream->total_out;
 }
 
-static void *do_data_decompress(void *opaque)
+struct DecompressData {
+/* filled by migration thread.*/
+void *des;
+uint8_t *compbuf;
+size_t len;
+
+z_stream stream;
+};
+typedef struct DecompressData DecompressData;
+
+static Threads *decompress_threads;
+
+static int decompress_thread_get_data_size(void)
 {
-DecompressParam *param = opaque;
-unsigned long pagesize;
-uint8_t *des;
-int len, ret;
-
-qemu_mutex_lock(¶m->mutex);
-while (!param->quit) {
-if (param->des) {
-des = param->des;
-len = param->len;
-param->des = 0;
-qemu_mutex_unlock(¶m->mutex);
-
-pagesize = TARGET_PAGE_SIZE;
-
-ret = qemu_uncompress_data(¶m->stream, des, pagesize,
-   param->compbuf, len);
-if (ret < 0 && migrate_get_current()->decompress_error_check) {
-error_report("decompress data failed");
-qemu_file_set_error(decomp_file, ret);
-}
+return sizeof(DecompressData);
+}
 
-qemu_mutex_lock(&decomp_done_lock);
-param->done = true;
-qemu_cond_signal(&decomp_done_cond);
-qemu_mutex_unlock(&decomp_done_lock);
+static int decompress_thread_data_init(void *request)
+{
+DecompressData *dd = request;
 
-qemu_mutex_lock(¶m->mutex);
-} else {
-qemu_cond_wait(¶m->cond, ¶m->mutex);
-}
+if (inflateInit(&dd->stream) != Z_OK) {
+return -1;
 }
-qemu_mutex_unlock(¶m->mutex);
 
-return NULL;
+dd->compbuf = g_malloc0(compressBound(TARGET_PAGE_SIZE));
+return 0;
 }
 
-static int wait_for_decompress_done(void)
+static void decompress_thread_data_fini(void *request)
 {
-int idx, thread_count;
+DecompressData *dd = request;
 
-if (!migrate_use_compression()) {
-return 0;
-}
+inflateEnd(&dd->stream);
+g_free(dd->compbuf);
+}
 
-thread_count = migrate_decompress_threads();
-qemu_mutex_lock(&decomp_done_lock);
-for (idx = 0; idx < thread_count; idx++) {
-while (!decomp_param[idx].done) {
-qemu_cond_wait(&decomp_done_cond, &decomp_done_lock);
-}
+static void decompress_thread_data_handler(void *request)
+{
+DecompressData *dd = request;
+unsigned long pagesize = TARGET_PAGE_SIZE;
+int ret;
+
+ret = qemu_uncompress_data(&dd->stream, dd->des, pagesize,
+   dd->compbuf, dd->len);
+if (ret < 0 && migrate_get_current()->decompress_error_check) {
+error_report("decompress data failed");
+qemu_file_set_error(decomp_file, ret);
 }
-qemu_mutex_unlock(&decomp_done_lock);
-return qemu_file_get_error(decomp_file);
 }
 
-static void compress_threads_load_cleanup(void)
+static void decompress_thread_data_done(void *request)
 {
-int i, thread_count;
+}
+
+static ThreadedWorkqueueOps decompress_ops = {
+.thread_get_request_size = decompress_thread_get_data_size,
+.thread_request_init = decompress_thread_data_init,
+.thread_request_uninit = decompress_thread_data_fini,
+.thread_request_handler = decompress_thread_data_handler,
+.thread_request_done = decompress_thread_data_done,
+};
 
+static int decompress_init(QEMUFile *f)
+{
 if (!migrate_use_compression()) {
-return;
+return 0;
 }
-thread_count = migrate_decompress_threads();
-for (i = 0; i < thread_count; i++) {
-/*
- * we use it as a indicator which shows if the thread is
- * properly init'd or not
- */
-   

[Qemu-devel] [PATCH v2 1/5] bitops: introduce change_bit_atomic

2018-11-06 Thread guangrong . xiao
From: Xiao Guangrong 

It will be used by threaded workqueue

Signed-off-by: Xiao Guangrong 
---
 include/qemu/bitops.h | 13 +
 1 file changed, 13 insertions(+)

diff --git a/include/qemu/bitops.h b/include/qemu/bitops.h
index 3f0926cf40..c522958852 100644
--- a/include/qemu/bitops.h
+++ b/include/qemu/bitops.h
@@ -79,6 +79,19 @@ static inline void change_bit(long nr, unsigned long *addr)
 *p ^= mask;
 }
 
+/**
+ * change_bit_atomic - Toggle a bit in memory atomically
+ * @nr: Bit to change
+ * @addr: Address to start counting from
+ */
+static inline void change_bit_atomic(long nr, unsigned long *addr)
+{
+unsigned long mask = BIT_MASK(nr);
+unsigned long *p = addr + BIT_WORD(nr);
+
+atomic_xor(p, mask);
+}
+
 /**
  * test_and_set_bit - Set a bit and return its old value
  * @nr: Bit to set
-- 
2.14.5




Re: [Qemu-devel] [PATCH] gtk: Fix mouse offset in scaled gtk-gl display for VFIO/iGVT-g DMA Buf mode

2018-11-06 Thread Gerd Hoffmann
On Wed, Oct 31, 2018 at 06:24:56AM +, Chen Zhang wrote:
> The issue was reported as in https://bugs.launchpad.net/qemu/+bug/1793859
> 
> When an OpenGL accelerated GTK window is used for iGVT-g DMA Buf device,
> window scaling would cause guest cursor to move in undesirable velocity.
> 
> To fix this usability issue, the gtk mouse motion events was modified to
> scale the position of event to match the coordinates of the scaled GL
> surface.

Hmm, we already have logic in place to deal with that.  I suspect the
root cause is simply that the scale_{x,y} variables are not set properly
in egl mode.  Can you try the patch below?

thanks,
  Gerd

== [ cut here ] ===
>From ebecaab0102aca37a3101131c20f45576835b6b3 Mon Sep 17 00:00:00 2001
From: Gerd Hoffmann 
Date: Tue, 6 Nov 2018 13:18:39 +0100
Subject: [PATCH] try fix gtk egl cursor

---
 ui/gtk-egl.c | 17 -
 1 file changed, 12 insertions(+), 5 deletions(-)

diff --git a/ui/gtk-egl.c b/ui/gtk-egl.c
index a77c25b490..5420c2362b 100644
--- a/ui/gtk-egl.c
+++ b/ui/gtk-egl.c
@@ -68,8 +68,15 @@ void gd_egl_draw(VirtualConsole *vc)
 return;
 }
 
+window = gtk_widget_get_window(vc->gfx.drawing_area);
+ww = gdk_window_get_width(window);
+wh = gdk_window_get_height(window);
+
 if (vc->gfx.scanout_mode) {
 gd_egl_scanout_flush(&vc->gfx.dcl, 0, 0, vc->gfx.w, vc->gfx.h);
+
+vc->gfx.scale_x = (double)ww / vc->gfx.w;
+vc->gfx.scale_y = (double)wh / vc->gfx.h;
 } else {
 if (!vc->gfx.ds) {
 return;
@@ -77,13 +84,13 @@ void gd_egl_draw(VirtualConsole *vc)
 eglMakeCurrent(qemu_egl_display, vc->gfx.esurface,
vc->gfx.esurface, vc->gfx.ectx);
 
-window = gtk_widget_get_window(vc->gfx.drawing_area);
-ww = gdk_window_get_width(window);
-wh = gdk_window_get_height(window);
 surface_gl_setup_viewport(vc->gfx.gls, vc->gfx.ds, ww, wh);
 surface_gl_render_texture(vc->gfx.gls, vc->gfx.ds);
 
 eglSwapBuffers(qemu_egl_display, vc->gfx.esurface);
+
+vc->gfx.scale_x = (double)ww / surface_width(vc->gfx.ds);
+vc->gfx.scale_y = (double)wh / surface_height(vc->gfx.ds);
 }
 }
 
@@ -232,8 +239,8 @@ void gd_egl_cursor_position(DisplayChangeListener *dcl,
 {
 VirtualConsole *vc = container_of(dcl, VirtualConsole, gfx.dcl);
 
-vc->gfx.cursor_x = pos_x;
-vc->gfx.cursor_y = pos_y;
+vc->gfx.cursor_x = pos_x * vc->gfx.scale_x;
+vc->gfx.cursor_y = pos_y * vc->gfx.scale_y;
 }
 
 void gd_egl_release_dmabuf(DisplayChangeListener *dcl,
-- 
2.9.3



[Qemu-devel] [PATCH v2 3/5] migration: use threaded workqueue for compression

2018-11-06 Thread guangrong . xiao
From: Xiao Guangrong 

Adapt the compression code to the threaded workqueue

Signed-off-by: Xiao Guangrong 
---
 migration/ram.c | 313 +---
 1 file changed, 115 insertions(+), 198 deletions(-)

diff --git a/migration/ram.c b/migration/ram.c
index 7e7deec4d8..acca842aff 100644
--- a/migration/ram.c
+++ b/migration/ram.c
@@ -57,6 +57,7 @@
 #include "qemu/uuid.h"
 #include "savevm.h"
 #include "qemu/iov.h"
+#include "qemu/threaded-workqueue.h"
 
 /***/
 /* ram save/restore */
@@ -349,22 +350,6 @@ typedef struct PageSearchStatus PageSearchStatus;
 
 CompressionStats compression_counters;
 
-struct CompressParam {
-bool done;
-bool quit;
-bool zero_page;
-QEMUFile *file;
-QemuMutex mutex;
-QemuCond cond;
-RAMBlock *block;
-ram_addr_t offset;
-
-/* internally used fields */
-z_stream stream;
-uint8_t *originbuf;
-};
-typedef struct CompressParam CompressParam;
-
 struct DecompressParam {
 bool done;
 bool quit;
@@ -377,15 +362,6 @@ struct DecompressParam {
 };
 typedef struct DecompressParam DecompressParam;
 
-static CompressParam *comp_param;
-static QemuThread *compress_threads;
-/* comp_done_cond is used to wake up the migration thread when
- * one of the compression threads has finished the compression.
- * comp_done_lock is used to co-work with comp_done_cond.
- */
-static QemuMutex comp_done_lock;
-static QemuCond comp_done_cond;
-/* The empty QEMUFileOps will be used by file in CompressParam */
 static const QEMUFileOps empty_ops = { };
 
 static QEMUFile *decomp_file;
@@ -394,125 +370,6 @@ static QemuThread *decompress_threads;
 static QemuMutex decomp_done_lock;
 static QemuCond decomp_done_cond;
 
-static bool do_compress_ram_page(QEMUFile *f, z_stream *stream, RAMBlock 
*block,
- ram_addr_t offset, uint8_t *source_buf);
-
-static void *do_data_compress(void *opaque)
-{
-CompressParam *param = opaque;
-RAMBlock *block;
-ram_addr_t offset;
-bool zero_page;
-
-qemu_mutex_lock(¶m->mutex);
-while (!param->quit) {
-if (param->block) {
-block = param->block;
-offset = param->offset;
-param->block = NULL;
-qemu_mutex_unlock(¶m->mutex);
-
-zero_page = do_compress_ram_page(param->file, ¶m->stream,
- block, offset, param->originbuf);
-
-qemu_mutex_lock(&comp_done_lock);
-param->done = true;
-param->zero_page = zero_page;
-qemu_cond_signal(&comp_done_cond);
-qemu_mutex_unlock(&comp_done_lock);
-
-qemu_mutex_lock(¶m->mutex);
-} else {
-qemu_cond_wait(¶m->cond, ¶m->mutex);
-}
-}
-qemu_mutex_unlock(¶m->mutex);
-
-return NULL;
-}
-
-static void compress_threads_save_cleanup(void)
-{
-int i, thread_count;
-
-if (!migrate_use_compression() || !comp_param) {
-return;
-}
-
-thread_count = migrate_compress_threads();
-for (i = 0; i < thread_count; i++) {
-/*
- * we use it as a indicator which shows if the thread is
- * properly init'd or not
- */
-if (!comp_param[i].file) {
-break;
-}
-
-qemu_mutex_lock(&comp_param[i].mutex);
-comp_param[i].quit = true;
-qemu_cond_signal(&comp_param[i].cond);
-qemu_mutex_unlock(&comp_param[i].mutex);
-
-qemu_thread_join(compress_threads + i);
-qemu_mutex_destroy(&comp_param[i].mutex);
-qemu_cond_destroy(&comp_param[i].cond);
-deflateEnd(&comp_param[i].stream);
-g_free(comp_param[i].originbuf);
-qemu_fclose(comp_param[i].file);
-comp_param[i].file = NULL;
-}
-qemu_mutex_destroy(&comp_done_lock);
-qemu_cond_destroy(&comp_done_cond);
-g_free(compress_threads);
-g_free(comp_param);
-compress_threads = NULL;
-comp_param = NULL;
-}
-
-static int compress_threads_save_setup(void)
-{
-int i, thread_count;
-
-if (!migrate_use_compression()) {
-return 0;
-}
-thread_count = migrate_compress_threads();
-compress_threads = g_new0(QemuThread, thread_count);
-comp_param = g_new0(CompressParam, thread_count);
-qemu_cond_init(&comp_done_cond);
-qemu_mutex_init(&comp_done_lock);
-for (i = 0; i < thread_count; i++) {
-comp_param[i].originbuf = g_try_malloc(TARGET_PAGE_SIZE);
-if (!comp_param[i].originbuf) {
-goto exit;
-}
-
-if (deflateInit(&comp_param[i].stream,
-migrate_compress_level()) != Z_OK) {
-g_free(comp_param[i].originbuf);
-goto exit;
-}
-
-/* comp_param[i].file is just used as a dummy buffer to save data,
- * set its ops to empty.
- */
-comp_param[i].file = qemu_fopen_ops(NULL, &empty_ops);

Re: [Qemu-devel] [PATCH] lsi53c895a: check script ram address value

2018-11-06 Thread Paolo Bonzini
On 06/11/2018 13:03, Peter Maydell wrote:
> When can this masking have any effect? These functions are
> the read and write ops for lsi_ram_ops, which we register with
> memory_region_init_io(&s->ram_io, OBJECT(s), &lsi_ram_ops, s,
>   "lsi-ram", 0x2000);
> which specifies a memory region size of 0x2000. So the input
> addr must be in the 0..0x1fff range already -- or have I missed
> something ?
> 
> It would probably be helpful (for readers and static analysers)
> to assert() that addr is < 0x2000, though.

Indeed, there are cases where the address is used blindly in a memcpy
with size>1, but this is not one of them.

Paolo



Re: [Qemu-devel] [PATCH] lsi53c895a: check script ram address value

2018-11-06 Thread li qiang

在 2018/11/6 20:03, Peter Maydell 写道:
> On 6 November 2018 at 11:53, P J P  wrote:
>> From: Prasad J Pandit 
>>
>> While accessing script ram[2048] via 'lsi_ram_read/write' routines,
>> 'addr' could exceed the ram range. Mask high order bits to avoid
>> OOB access.
>>
>> Reported-by: Mark Kanda 
>> Signed-off-by: Prasad J Pandit 
>> ---
>>   hw/scsi/lsi53c895a.c | 2 ++
>>   1 file changed, 2 insertions(+)
>>
>> diff --git a/hw/scsi/lsi53c895a.c b/hw/scsi/lsi53c895a.c
>> index 3f207f607c..0800df416e 100644
>> --- a/hw/scsi/lsi53c895a.c
>> +++ b/hw/scsi/lsi53c895a.c
>> @@ -2035,6 +2035,7 @@ static void lsi_ram_write(void *opaque, hwaddr addr,
>>   uint32_t mask;
>>   int shift;
>>
>> +addr &= 0x01FFF;
>>   newval = s->script_ram[addr >> 2];
>>   shift = (addr & 3) * 8;
>>   mask = ((uint64_t)1 << (size * 8)) - 1;
>> @@ -2050,6 +2051,7 @@ static uint64_t lsi_ram_read(void *opaque, hwaddr addr,
>>   uint32_t val;
>>   uint32_t mask;
>>
>> +addr &= 0x01FFF;
>>   val = s->script_ram[addr >> 2];
>>   mask = ((uint64_t)1 << (size * 8)) - 1;
>>   val >>= (addr & 3) * 8;
>> --
> When can this masking have any effect? These functions are
> the read and write ops for lsi_ram_ops, which we register with
>  memory_region_init_io(&s->ram_io, OBJECT(s), &lsi_ram_ops, s,
>"lsi-ram", 0x2000);
> which specifies a memory region size of 0x2000. So the input
> addr must be in the 0..0x1fff range already -- or have I missed
> something ?


Hello Peter,

There is a oob access indeed,

The addr is 0~0x1fff, but when addr is at the near the end ,for example 
0x1fffe, the add>>2 can be 2047

and as script_ram is a uint32_t and so s->script_ram[addr >> 2] can read 
out of the script_ram.

Some of the debug data.

Thread 5 "qemu-system-x86" hit Breakpoint 1, lsi_ram_read 
(opaque=0x6260f100, addr=8188, size=4) at hw/scsi/lsi53c895a.c:2046
2046        LSIState *s = opaque;
(gdb) p /x addr
$12 = 0x1ffc
(gdb) p addr
$13 = 8188
(gdb) n
...
2050        val = s->script_ram[addr >> 2];
(gdb) p addr
$14 = 8188
(gdb) p addr >>2
$15 = 2047
(gdb) n
2051        mask = ((uint64_t)1 << (size * 8)) - 1;
(gdb) p /x val
$16 = 0x0
(gdb) n
2052        val >>= (addr & 3) * 8;
(gdb) n
2053        return val & mask;
(gdb) p /x mask
$17 = 0x
(gdb) p /s size
$18 = 4

But as you point Prasad's patch does nothing.

Hello Prasad,

I think you should check the addr with size to ensure the access

doesn't exceed script_ram.


Thanks,

Li Qiang


> It would probably be helpful (for readers and static analysers)
> to assert() that addr is < 0x2000, though.
>
> thanks
> -- PMM
>


Re: [Qemu-devel] [PATCH] lsi53c895a: check script ram address value

2018-11-06 Thread Paolo Bonzini
On 06/11/2018 13:27, li qiang wrote:
> The addr is 0~0x1fff, but when addr is at the near the end ,for example 
> 0x1fffe, the add>>2 can be 2047
> 
> and as script_ram is a uint32_t and so s->script_ram[addr >> 2] can read 
> out of the script_ram.

How so?  s->script_ram has size 2048, it's okay to access it at 2047.

Paolo



[Qemu-devel] [PATCH v3 0/2] chardev: fix mess in OPENED/CLOSED events when muxed

2018-11-06 Thread Artem Pisarenko
This issue actually more complex. Idea of generating events from inside 
function called '*_set_handlers' isn't good, at least its implicit nature, and 
especially a fact, that function decides about open state (see 'fe_open' 
variable), but generates event only in one direction. Combined with 
'mux_chr_set_handlers()' hack this makes things even worse.
Better solution is to change fe interface and rewrite all frontends code (a lot 
of stuff in hw/char/* and somewhere else).
Although first patch doesn't fix any bug (known to me), its main effect is 
optimization of emulation performance by avoiding extra activity.
Added testing demonstrates issue and prevents potential bugs in future.

v3 changes:
 - extended commit message explaining fix (as supposed by Marc-André Lureau)

v2 changes:
 - fix failed unit test
 - 'mux_chr_set_handlers()' hack rewritten (as supposed by Marc-André Lureau)
 - added testing of issue to unit test (new patch)

Artem Pisarenko (2):
  chardev: fix mess in OPENED/CLOSED events when muxed
  tests/test-char: add muxed chardev testing for open/close

 chardev/char-fe.c | 33 +--
 chardev/char-mux.c| 16 +-
 include/chardev/char-fe.h | 18 ++-
 tests/test-char.c | 80 +--
 4 files changed, 127 insertions(+), 20 deletions(-)

-- 
2.7.4




Re: [Qemu-devel] [PATCH v2 2/2] tests/test-char: add muxed chardev testing for open/close

2018-11-06 Thread Artem Pisarenko
> this is unnecessary change, I can drop on commit

Oops, didn't noticed your message before sent v3.


[Qemu-devel] [PATCH v3 1/2] chardev: fix mess in OPENED/CLOSED events when muxed

2018-11-06 Thread Artem Pisarenko
When chardev is multiplexed (mux=on) there are a lot of cases where
CHR_EVENT_OPENED/CHR_EVENT_CLOSED events pairing (expected from
frontend side) is broken. There are either generation of multiple
repeated or extra CHR_EVENT_OPENED events, or CHR_EVENT_CLOSED just
isn't generated at all.
This is mostly because 'qemu_chr_fe_set_handlers()' function makes its
own (and often wrong) implicit decision on updated frontend state and
invokes 'fd_event' callback with 'CHR_EVENT_OPENED'. And even worse,
it doesn't do symmetric action in opposite direction, as someone may
expect (i.e. it doesn't invoke previously set 'fd_event' with
'CHR_EVENT_CLOSED'). Muxed chardev uses trick by calling this function
again to replace callback handlers with its own ones, but it doesn't
account for such side effect.
Fix that using extended version of this function with added argument
for disabling side effect and keep original function for compatibility
with lots of frontends already using this interface and being
"tolerant" to its side effects.
One more source of event duplication is just line of code in
char-mux.c, which does far more than comment above says (obvious fix).

Signed-off-by: Artem Pisarenko 
---

Notes:
v3:
 - extended and improved commit message with 'why' and 'how' explanation

 chardev/char-fe.c | 33 -
 chardev/char-mux.c| 16 
 include/chardev/char-fe.h | 18 +-
 3 files changed, 49 insertions(+), 18 deletions(-)

diff --git a/chardev/char-fe.c b/chardev/char-fe.c
index a8931f7..b7bcbd5 100644
--- a/chardev/char-fe.c
+++ b/chardev/char-fe.c
@@ -246,14 +246,15 @@ void qemu_chr_fe_deinit(CharBackend *b, bool del)
 }
 }
 
-void qemu_chr_fe_set_handlers(CharBackend *b,
-  IOCanReadHandler *fd_can_read,
-  IOReadHandler *fd_read,
-  IOEventHandler *fd_event,
-  BackendChangeHandler *be_change,
-  void *opaque,
-  GMainContext *context,
-  bool set_open)
+void qemu_chr_fe_set_handlers_full(CharBackend *b,
+   IOCanReadHandler *fd_can_read,
+   IOReadHandler *fd_read,
+   IOEventHandler *fd_event,
+   BackendChangeHandler *be_change,
+   void *opaque,
+   GMainContext *context,
+   bool set_open,
+   bool sync_state)
 {
 Chardev *s;
 int fe_open;
@@ -285,7 +286,7 @@ void qemu_chr_fe_set_handlers(CharBackend *b,
 qemu_chr_fe_take_focus(b);
 /* We're connecting to an already opened device, so let's make sure we
also get the open event */
-if (s->be_open) {
+if (sync_state && s->be_open) {
 qemu_chr_be_event(s, CHR_EVENT_OPENED);
 }
 }
@@ -295,6 +296,20 @@ void qemu_chr_fe_set_handlers(CharBackend *b,
 }
 }
 
+void qemu_chr_fe_set_handlers(CharBackend *b,
+  IOCanReadHandler *fd_can_read,
+  IOReadHandler *fd_read,
+  IOEventHandler *fd_event,
+  BackendChangeHandler *be_change,
+  void *opaque,
+  GMainContext *context,
+  bool set_open)
+{
+qemu_chr_fe_set_handlers_full(b, fd_can_read, fd_read, fd_event, be_change,
+  opaque, context, set_open,
+  true);
+}
+
 void qemu_chr_fe_take_focus(CharBackend *b)
 {
 if (!b->chr) {
diff --git a/chardev/char-mux.c b/chardev/char-mux.c
index 6055e76..1199d32 100644
--- a/chardev/char-mux.c
+++ b/chardev/char-mux.c
@@ -283,13 +283,13 @@ void mux_chr_set_handlers(Chardev *chr, GMainContext 
*context)
 MuxChardev *d = MUX_CHARDEV(chr);
 
 /* Fix up the real driver with mux routines */
-qemu_chr_fe_set_handlers(&d->chr,
- mux_chr_can_read,
- mux_chr_read,
- mux_chr_event,
- NULL,
- chr,
- context, true);
+qemu_chr_fe_set_handlers_full(&d->chr,
+  mux_chr_can_read,
+  mux_chr_read,
+  mux_chr_event,
+  NULL,
+  chr,
+  context, true, false);
 }
 
 void mux_set_focus(Chardev *chr, int focus)
@@ -367,7 +367,7 @@ static int open_muxes(Chardev *chr)
  * mark mux as OPENED so any new FEs will immediately receive
  * OPENED event
  */
-

Re: [Qemu-devel] [PATCH] lsi53c895a: check script ram address value

2018-11-06 Thread Peter Maydell
On 6 November 2018 at 12:38, li qiang  wrote:
>
> 在 2018/11/6 20:28, Paolo Bonzini 写道:
>> On 06/11/2018 13:27, li qiang wrote:
>>> The addr is 0~0x1fff, but when addr is at the near the end ,for example
>>> 0x1fffe, the add>>2 can be 2047
>>>
>>> and as script_ram is a uint32_t and so s->script_ram[addr >> 2] can read
>>> out of the script_ram.
>> How so?  s->script_ram has size 2048, it's okay to access it at 2047.
>
> Oh, right.
>
>   I'm confused by the script_ram, it's not byte array.

Incidentally, I think the read and write functions here
would be somewhat clearer written as

static void lsi_ram_write(void *opaque, hwaddr addr,
  uint64_t val, unsigned size)
{
LSIState *s = opaque;
void *p = ((void *)s->script_ram) + addr;

assert(addr + size <= sizeof(s->script_ram));
stn_p(p, size, val);
}

static uint64_t lsi_ram_read(void *opaque, hwaddr addr,
 unsigned size)
{
LSIState *s = opaque;
void *p = ((void *)s->script_ram) + addr;

assert(addr + size <= sizeof(s->script_ram));
return ldn_p(p, size);
}

thanks
-- PMM



[Qemu-devel] [PATCH v3 2/2] tests/test-char: add muxed chardev testing for open/close

2018-11-06 Thread Artem Pisarenko
Validate that frontend callbacks for CHR_EVENT_OPENED/CHR_EVENT_CLOSED
events are being issued when expected and in strictly pairing order.

Signed-off-by: Artem Pisarenko 
---
 tests/test-char.c | 80 +--
 1 file changed, 78 insertions(+), 2 deletions(-)

diff --git a/tests/test-char.c b/tests/test-char.c
index 831e37f..657cb4c 100644
--- a/tests/test-char.c
+++ b/tests/test-char.c
@@ -16,6 +16,9 @@ static bool quit;
 
 typedef struct FeHandler {
 int read_count;
+bool is_open;
+int openclose_count;
+bool openclose_mismatch;
 int last_event;
 char read_buf[128];
 } FeHandler;
@@ -49,10 +52,24 @@ static void fe_read(void *opaque, const uint8_t *buf, int 
size)
 static void fe_event(void *opaque, int event)
 {
 FeHandler *h = opaque;
+bool new_open_state;
 
 h->last_event = event;
-if (event != CHR_EVENT_BREAK) {
+switch (event) {
+case CHR_EVENT_BREAK:
+break;
+case CHR_EVENT_OPENED:
+case CHR_EVENT_CLOSED:
+h->openclose_count++;
+new_open_state = (event == CHR_EVENT_OPENED);
+if (h->is_open == new_open_state) {
+h->openclose_mismatch = true;
+}
+h->is_open = new_open_state;
+/* no break */
+default:
 quit = true;
+break;
 }
 }
 
@@ -161,7 +178,7 @@ static void char_mux_test(void)
 QemuOpts *opts;
 Chardev *chr, *base;
 char *data;
-FeHandler h1 = { 0, }, h2 = { 0, };
+FeHandler h1 = { 0, false, 0, false, }, h2 = { 0, false, 0, false, };
 CharBackend chr_be1, chr_be2;
 
 opts = qemu_opts_create(qemu_find_opts("chardev"), "mux-label",
@@ -233,6 +250,65 @@ static void char_mux_test(void)
 g_assert_cmpint(h1.last_event, ==, CHR_EVENT_BREAK);
 g_assert_cmpint(h2.last_event, ==, CHR_EVENT_MUX_OUT);
 
+/* open/close state and corresponding events */
+g_assert_true(qemu_chr_fe_backend_open(&chr_be1));
+g_assert_true(qemu_chr_fe_backend_open(&chr_be2));
+g_assert_true(h1.is_open);
+g_assert_false(h1.openclose_mismatch);
+g_assert_true(h2.is_open);
+g_assert_false(h2.openclose_mismatch);
+
+h1.openclose_count = h2.openclose_count = 0;
+
+qemu_chr_fe_set_handlers(&chr_be1, NULL, NULL, NULL, NULL,
+ NULL, NULL, false);
+qemu_chr_fe_set_handlers(&chr_be2, NULL, NULL, NULL, NULL,
+ NULL, NULL, false);
+g_assert_cmpint(h1.openclose_count, ==, 0);
+g_assert_cmpint(h2.openclose_count, ==, 0);
+
+h1.is_open = h2.is_open = false;
+qemu_chr_fe_set_handlers(&chr_be1,
+ NULL,
+ NULL,
+ fe_event,
+ NULL,
+ &h1,
+ NULL, false);
+qemu_chr_fe_set_handlers(&chr_be2,
+ NULL,
+ NULL,
+ fe_event,
+ NULL,
+ &h2,
+ NULL, false);
+g_assert_cmpint(h1.openclose_count, ==, 1);
+g_assert_false(h1.openclose_mismatch);
+g_assert_cmpint(h2.openclose_count, ==, 1);
+g_assert_false(h2.openclose_mismatch);
+
+qemu_chr_be_event(base, CHR_EVENT_CLOSED);
+qemu_chr_be_event(base, CHR_EVENT_OPENED);
+g_assert_cmpint(h1.openclose_count, ==, 3);
+g_assert_false(h1.openclose_mismatch);
+g_assert_cmpint(h2.openclose_count, ==, 3);
+g_assert_false(h2.openclose_mismatch);
+
+qemu_chr_fe_set_handlers(&chr_be2,
+ fe_can_read,
+ fe_read,
+ fe_event,
+ NULL,
+ &h2,
+ NULL, false);
+qemu_chr_fe_set_handlers(&chr_be1,
+ fe_can_read,
+ fe_read,
+ fe_event,
+ NULL,
+ &h1,
+ NULL, false);
+
 /* remove first handler */
 qemu_chr_fe_set_handlers(&chr_be1, NULL, NULL, NULL, NULL,
  NULL, NULL, true);
-- 
2.7.4




Re: [Qemu-devel] [PATCH] lsi53c895a: check script ram address value

2018-11-06 Thread Peter Maydell
On 6 November 2018 at 12:27, li qiang  wrote:
> The addr is 0~0x1fff, but when addr is at the near the end ,for example
> 0x1fffe, the add>>2 can be 2047
>
> and as script_ram is a uint32_t and so s->script_ram[addr >> 2] can read
> out of the script_ram.

But script_ram is declared as
  uint32_t script_ram[2048];
so if addr >> 2 == 2047, that's still in-bounds, isn't it?

thanks
-- PMM



Re: [Qemu-devel] [PATCH] lsi53c895a: check script ram address value

2018-11-06 Thread li qiang

在 2018/11/6 20:28, Paolo Bonzini 写道:
> On 06/11/2018 13:27, li qiang wrote:
>> The addr is 0~0x1fff, but when addr is at the near the end ,for example
>> 0x1fffe, the add>>2 can be 2047
>>
>> and as script_ram is a uint32_t and so s->script_ram[addr >> 2] can read
>> out of the script_ram.
> How so?  s->script_ram has size 2048, it's okay to access it at 2047.

Oh, right.

  I'm confused by the script_ram, it's not byte array.




> Paolo


Re: [Qemu-devel] [PATCH] target/ppc: fix mtmsr instruction for icount

2018-11-06 Thread 'David Gibson'
On Tue, Nov 06, 2018 at 09:10:45AM +0300, Pavel Dovgalyuk wrote:
> > From: David Gibson [mailto:da...@gibson.dropbear.id.au]
> > On Tue, Oct 30, 2018 at 12:30:31PM +0300, Pavel Dovgalyuk wrote:
> > > This patch fixes processing of mtmsr instructions in icount mode.
> > > In this mode writing to interrupt/peripheral state is controlled
> > > by can_do_io flag. This flag must be set explicitly before helper
> > > function invocation.
> > >
> > > Signed-off-by: Maria Klimushenkova 
> > > Signed-off-by: Pavel Dovgalyuk 
> > 
> > Applied to ppc-for-3.1, thanks.
> 
> Thanks. What about this one
> https://patchew.org/QEMU/20181030122134.11055.15711.stgit@pasha-VirtualBox/
> There is a mess with the subject, but the code is ok :)

I've been procrastinating on that because I don't understand icount
well enough to review it easily, and no-one has replied with
Reviewed-by or Tested-by.

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


signature.asc
Description: PGP signature


Re: [Qemu-devel] [PATCH] ivshmem: fix memory backend leak

2018-11-06 Thread Paolo Bonzini
On 01/11/2018 11:44, Igor Mammedov wrote:
> object_new() returns a new backend with refcount == 1 and
> then later object_property_add_child() increases refcount to 2
> So when ivshmem is desroyed, the backend it has created isn't
> destroyed along with it as children cleanup will bring
> backend's refcount only to 1, which leaks backend including
> resources it is using.
> 
> Drop the original reference from object_new() once backend
> is attached to its parent.
> 
> Signed-off-by: Igor Mammedov 
> ---
>  hw/misc/ivshmem.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/hw/misc/ivshmem.c b/hw/misc/ivshmem.c
> index f88910e..ecfd10a 100644
> --- a/hw/misc/ivshmem.c
> +++ b/hw/misc/ivshmem.c
> @@ -1279,6 +1279,7 @@ static void desugar_shm(IVShmemState *s)
>  object_property_set_bool(obj, true, "share", &error_abort);
>  object_property_add_child(OBJECT(s), "internal-shm-backend", obj,
>&error_abort);
> +object_unref(obj);
>  user_creatable_complete(obj, &error_abort);
>  s->hostmem = MEMORY_BACKEND(obj);
>  }
> 

Queued, thanks.

Paolo



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

2018-11-06 Thread Peter Maydell
On 6 November 2018 at 11:20, Peter Maydell  wrote:
> On 6 November 2018 at 11:07, Michael S. Tsirkin  wrote:
>> On Tue, Nov 06, 2018 at 09:18:49AM +0100, Thomas Huth wrote:
>>> On 2018-11-05 19:14, Michael S. Tsirkin wrote:
>>> > The following changes since commit 
>>> > b2f7a038bb4c4fc5ce6b8486e8513dfd97665e2a:
>>> >
>>> >   Merge remote-tracking branch 'remotes/rth/tags/pull-softfloat-20181104' 
>>> > into staging (2018-11-05 10:32:49 +)
>>> >
>>> > are available in the Git repository at:
>>> >
>>> >   git://git.kernel.org/pub/scm/virt/kvm/mst/qemu.git tags/for_upstream
>>> >
>>> > for you to fetch changes up to 6196df5c8e6688c1c3f06f73442820066335337c:
>>> >
>>> >   vhost-scsi: prevent using uninitialized vqs (2018-11-05 12:59:35 -0500)
>>> >
>>> > 
>>> > pci, pc, virtio: fixes, features
>>> >
>>> > AMD IOMMU VAPIC support + fixes all over the place.
>>> >
>>> > Signed-off-by: Michael S. Tsirkin 
>>> >
>>> > 
>>> > Gerd Hoffmann (1):
>>> >   pci-testdev: add optional memory bar
>>> >
>>> > Laszlo Ersek (4):
>>> >   MAINTAINERS: list "tests/acpi-test-data" files in ACPI/SMBIOS 
>>> > section
>>> [...]
>>> >  tests/{acpi-test-data => data/acpi}/pc/APIC| Bin
>>> >  tests/{acpi-test-data => data/acpi}/pc/APIC.cphp   | Bin
>>> >  .../{acpi-test-data => data/acpi}/pc/APIC.dimmpxm  | Bin
>>> >  tests/{acpi-test-data => data/acpi}/pc/DSDT| Bin
>>>
>>> So patch 1 moves tests/acpi-test-data/ to tests/data/acpi/ and patch 20
>>> adds an entry for tests/acpi-test-data/ ? Does not make much sense to me
>>> ... I think patch 20 needs to be adapted now.
>>>
>>>  Thomas
>>
>> Oh right, MAINTAINERS needs to be fixed. Can be done with a patch on top
>> though.
>
> Yeah, given the timing for rc0 I'll just apply this version of
> the pullreq, and we can fix up MAINTAINERS afterwards.

...applied.

thanks
-- PMM



Re: [Qemu-devel] [PATCH v3] file-posix: Use error API properly

2018-11-06 Thread Kevin Wolf
Am 01.11.2018 um 07:29 hat Fam Zheng geschrieben:
> Use error_report for situations that affect user operation (i.e.  we're
> actually returning error), and warn_report/warn_report_err when some
> less critical error happened but the user operation can still carry on.
> 
> For raw_normalize_devicepath, add Error parameter to propagate to
> its callers.
> 
> Suggested-by: Markus Armbruster 
> Signed-off-by: Fam Zheng 

Thanks, applied to the block branch.

Kevin



  1   2   3   >