Re: [Qemu-devel] [PATCH 02/10] tests/tcg/README: fix location for lm32 tests

2018-10-04 Thread Thomas Huth
On 2018-10-04 18:18, Cleber Rosa wrote:
> Point to the right and obvious location for lm32 tests.
> 
> Signed-off-by: Cleber Rosa 
> ---
>  tests/tcg/README | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/tests/tcg/README b/tests/tcg/README
> index a5643d33e7..2a58f9a058 100644
> --- a/tests/tcg/README
> +++ b/tests/tcg/README
> @@ -10,6 +10,6 @@ with "make test-cris".
>  
>  LM32
>  
> -The testsuite for LM32 is in tests/tcg/cris.  You can run it
> +The testsuite for LM32 is in tests/tcg/lm32.  You can run it
>  with "make test-lm32".

Reviewed-by: Thomas Huth 



Re: [Qemu-devel] [PATCH] hw/ppc/spapr_rng: Introduce CONFIG_SPAPR_RNG switch for spapr_rng.c

2018-10-04 Thread Thomas Huth
On 2018-10-05 06:25, David Gibson wrote:
> On Thu, Oct 04, 2018 at 12:07:01PM +0200, Thomas Huth wrote:
>> The spapr-rng device is suboptimal when compared to virtio-rng, so
>> users might want to disable it in their builds. Thus let's introduce
>> a proper CONFIG switch to allow us to compile QEMU without this device.
>>
>> Signed-off-by: Thomas Huth 
> 
> Uh, sure, I guess so.

Yes, we definitely want this for the QEMU in RHEL :-)

>> diff --git a/hw/ppc/spapr_rng.c b/hw/ppc/spapr_rng.c
>> index d2acd61..dec8434 100644
>> --- a/hw/ppc/spapr_rng.c
>> +++ b/hw/ppc/spapr_rng.c
>> @@ -27,6 +27,7 @@
>>  #include "sysemu/rng.h"
>>  #include "hw/ppc/spapr.h"
>>  #include "kvm_ppc.h"
>> +#include "spapr_rng.h"
>>  
>>  #define SPAPR_RNG(obj) \
>>  OBJECT_CHECK(sPAPRRngState, (obj), TYPE_SPAPR_RNG)
>> @@ -132,29 +133,6 @@ static void spapr_rng_realize(DeviceState *dev, Error 
>> **errp)
>>  }
>>  }
>>  
>> -int spapr_rng_populate_dt(void *fdt)
>> -{
>> -int node;
>> -int ret;
>> -
>> -node = qemu_fdt_add_subnode(fdt, "/ibm,platform-facilities");
>> -if (node <= 0) {
>> -return -1;
>> -}
>> -ret = fdt_setprop_string(fdt, node, "device_type",
>> - "ibm,platform-facilities");
>> -ret |= fdt_setprop_cell(fdt, node, "#address-cells", 0x1);
>> -ret |= fdt_setprop_cell(fdt, node, "#size-cells", 0x0);
>> -
>> -node = fdt_add_subnode(fdt, node, "ibm,random-v1");
>> -if (node <= 0) {
>> -return -1;
>> -}
>> -ret |= fdt_setprop_string(fdt, node, "compatible", "ibm,random");
>> -
>> -return ret ? -1 : 0;
>> -}
>> -
> 
> Moving this to an inline doesn't seem right to me though - it's a more
> complex function that we usually want in a .h inline, and I don't
> really see a good reason for it to be there (rather than an #ifdeffed
> stub).

An #ifdef is not possible here - the CONFIG switches for the targets are
*not* turned into pre-processor macros, only the CONFIG switches for the
host settings. So putting this function as static inline into a separate
header seems to be the best option to me right now. Alternatively, I
could also put it directly into spapr.c directly, but that file is
already very big... well, I don't mind, let me know what you prefer.

 Thomas



signature.asc
Description: OpenPGP digital signature


Re: [Qemu-devel] [PATCH] hw/ppc/spapr_rng: Introduce CONFIG_SPAPR_RNG switch for spapr_rng.c

2018-10-04 Thread David Gibson
On Thu, Oct 04, 2018 at 12:07:01PM +0200, Thomas Huth wrote:
> The spapr-rng device is suboptimal when compared to virtio-rng, so
> users might want to disable it in their builds. Thus let's introduce
> a proper CONFIG switch to allow us to compile QEMU without this device.
> 
> Signed-off-by: Thomas Huth 

Uh, sure, I guess so.



> ---
>  default-configs/ppc64-softmmu.mak |  1 +
>  hw/ppc/Makefile.objs  |  3 ++-
>  hw/ppc/spapr.c|  3 ++-
>  hw/ppc/spapr_rng.c| 24 +---
>  hw/ppc/spapr_rng.h| 48 
> +++
>  include/hw/ppc/spapr.h|  4 
>  6 files changed, 54 insertions(+), 29 deletions(-)
>  create mode 100644 hw/ppc/spapr_rng.h
> 
> diff --git a/default-configs/ppc64-softmmu.mak 
> b/default-configs/ppc64-softmmu.mak
> index b94af6c..24d4717 100644
> --- a/default-configs/ppc64-softmmu.mak
> +++ b/default-configs/ppc64-softmmu.mak
> @@ -17,3 +17,4 @@ CONFIG_XICS=$(CONFIG_PSERIES)
>  CONFIG_XICS_SPAPR=$(CONFIG_PSERIES)
>  CONFIG_XICS_KVM=$(call land,$(CONFIG_PSERIES),$(CONFIG_KVM))
>  CONFIG_MEM_HOTPLUG=y
> +CONFIG_SPAPR_RNG=y
> diff --git a/hw/ppc/Makefile.objs b/hw/ppc/Makefile.objs
> index 4ab5564..4e0c1c0 100644
> --- a/hw/ppc/Makefile.objs
> +++ b/hw/ppc/Makefile.objs
> @@ -3,8 +3,9 @@ obj-y += ppc.o ppc_booke.o fdt.o
>  # IBM pSeries (sPAPR)
>  obj-$(CONFIG_PSERIES) += spapr.o spapr_caps.o spapr_vio.o spapr_events.o
>  obj-$(CONFIG_PSERIES) += spapr_hcall.o spapr_iommu.o spapr_rtas.o
> -obj-$(CONFIG_PSERIES) += spapr_pci.o spapr_rtc.o spapr_drc.o spapr_rng.o
> +obj-$(CONFIG_PSERIES) += spapr_pci.o spapr_rtc.o spapr_drc.o
>  obj-$(CONFIG_PSERIES) += spapr_cpu_core.o spapr_ovec.o spapr_irq.o
> +obj-$(CONFIG_SPAPR_RNG) +=  spapr_rng.o
>  # IBM PowerNV
>  obj-$(CONFIG_POWERNV) += pnv.o pnv_xscom.o pnv_core.o pnv_lpc.o pnv_psi.o 
> pnv_occ.o pnv_bmc.o
>  ifeq ($(CONFIG_PCI)$(CONFIG_PSERIES)$(CONFIG_LINUX), yyy)
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index 98868d8..ba40c85 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -53,9 +53,10 @@
>  #include "hw/ppc/fdt.h"
>  #include "hw/ppc/spapr.h"
>  #include "hw/ppc/spapr_vio.h"
> +#include "spapr_rng.h"
> +
>  #include "hw/pci-host/spapr.h"
>  #include "hw/pci/msi.h"
> -
>  #include "hw/pci/pci.h"
>  #include "hw/scsi/scsi.h"
>  #include "hw/virtio/virtio-scsi.h"
> diff --git a/hw/ppc/spapr_rng.c b/hw/ppc/spapr_rng.c
> index d2acd61..dec8434 100644
> --- a/hw/ppc/spapr_rng.c
> +++ b/hw/ppc/spapr_rng.c
> @@ -27,6 +27,7 @@
>  #include "sysemu/rng.h"
>  #include "hw/ppc/spapr.h"
>  #include "kvm_ppc.h"
> +#include "spapr_rng.h"
>  
>  #define SPAPR_RNG(obj) \
>  OBJECT_CHECK(sPAPRRngState, (obj), TYPE_SPAPR_RNG)
> @@ -132,29 +133,6 @@ static void spapr_rng_realize(DeviceState *dev, Error 
> **errp)
>  }
>  }
>  
> -int spapr_rng_populate_dt(void *fdt)
> -{
> -int node;
> -int ret;
> -
> -node = qemu_fdt_add_subnode(fdt, "/ibm,platform-facilities");
> -if (node <= 0) {
> -return -1;
> -}
> -ret = fdt_setprop_string(fdt, node, "device_type",
> - "ibm,platform-facilities");
> -ret |= fdt_setprop_cell(fdt, node, "#address-cells", 0x1);
> -ret |= fdt_setprop_cell(fdt, node, "#size-cells", 0x0);
> -
> -node = fdt_add_subnode(fdt, node, "ibm,random-v1");
> -if (node <= 0) {
> -return -1;
> -}
> -ret |= fdt_setprop_string(fdt, node, "compatible", "ibm,random");
> -
> -return ret ? -1 : 0;
> -}
> -

Moving this to an inline doesn't seem right to me though - it's a more
complex function that we usually want in a .h inline, and I don't
really see a good reason for it to be there (rather than an #ifdeffed
stub).

>  static Property spapr_rng_properties[] = {
>  DEFINE_PROP_BOOL("use-kvm", sPAPRRngState, use_kvm, false),
>  DEFINE_PROP_LINK("rng", sPAPRRngState, backend, TYPE_RNG_BACKEND,
> diff --git a/hw/ppc/spapr_rng.h b/hw/ppc/spapr_rng.h
> new file mode 100644
> index 000..1d5fdcb
> --- /dev/null
> +++ b/hw/ppc/spapr_rng.h
> @@ -0,0 +1,48 @@
> +/*
> + * QEMU sPAPR random number generator "device" for H_RANDOM hypercall
> + *
> + * Copyright 2015 Thomas Huth, Red Hat Inc.
> + *
> + * 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 SPAPR_RNG_H
> +#define SPAPR_RNG_H
> +
> +#def

Re: [Qemu-devel] [PATCH v3] vmdk: align end of file to a sector boundary

2018-10-04 Thread yuchenlin via Qemu-devel

Ping?

On 2018-09-13 16:34, Fam Zheng wrote:

On Thu, 09/13 16:29, yuchen...@synology.com wrote:

From: yuchenlin 

There is a rare case which the size of last compressed cluster
is larger than the cluster size, which will cause the file is
not aligned at the sector boundary.

There are three reasons to do it. First, if vmdk doesn't align at
the sector boundary, there may be many undefined behaviors,
such as, in vbox it will show VMDK: Compressed image is corrupted
'syno-vm-disk1.vmdk' (VERR_ZIP_CORRUPTED) when we try to import an
ova with unaligned vmdk. Second, all the cluster_sector is aligned
to sector, the last one should be like this, too. Third, it ease
reading with sector based I/Os.

Signed-off-by: yuchenlin 


Reviewed-by: Fam Zheng 





Re: [Qemu-devel] [PATCH] linux-user: Suppress address-of-packed-member warnings in __get/put_user_e

2018-10-04 Thread Laurent Vivier
On 04/10/2018 18:55, Laurent Vivier wrote:
> Le 28/09/2018 à 16:25, Peter Maydell a écrit :
>> Our __get_user_e() and __put_user_e() macros cause newer versions
>> of clang to generate false-positive -Waddress-of-packed-member
>> warnings if they are passed the address of a member of a packed
>> struct (see https://bugs.llvm.org/show_bug.cgi?id=39113).
>> Suppress these using the _Pragma() operator.
>>
>> To put in the pragmas we need to convert the macros from
>> expressions to statements, but all the callsites effectively
>> treat them as statements already so this is OK.
>>
>> Signed-off-by: Peter Maydell 
>> ---
>>  linux-user/qemu.h | 57 +++
>>  1 file changed, 38 insertions(+), 19 deletions(-)
>>
> 
> Applied to my linux-user branch.

I have the following error when building on Fedora 28 and gcc (GCC)
8.1.1 20180712 (Red Hat 8.1.1-5)

  CC  aarch64_be-linux-user/target/arm/arm-semi.o
.../target/arm/arm-semi.c: In function ‘do_arm_semihosting’:
.../target/arm/arm-semi.c:270:1: error: unknown option after ‘#pragma
GCC diagnostic’ kind [-Werror=pragmas]

Perhaps you should use a "#if defined(__clang__)" to apply your fix only
to clang?

Thanks,
Laurent



Re: [Qemu-devel] [PATCH 2/7] Acceptance Tests: introduce arch parameter and attribute

2018-10-04 Thread Murilo Opsfelder Araujo
Hi, Cleber.

On Thu, Oct 04, 2018 at 11:14:24AM -0400, Cleber Rosa wrote:
> On a number of different scenarios, such as when choosing a QEMU
> binary to be used on tests (or a image to use to boot a test VM), it's
> useful to define the architecture that should be used.
>
> This introduces both a test parameter and a test instance attribute,
> that will contain such a value.
>
> The selection of the default QEMU binary, if one is not given
> explicitly, has also been changed to look at the arch attribute.
>
> Signed-off-by: Cleber Rosa 
> ---
>  docs/devel/testing.rst| 18 ++
>  tests/acceptance/avocado_qemu/__init__.py | 13 ++---
>  2 files changed, 28 insertions(+), 3 deletions(-)
>
> diff --git a/docs/devel/testing.rst b/docs/devel/testing.rst
> index 727c4019b5..4178ae5022 100644
> --- a/docs/devel/testing.rst
> +++ b/docs/devel/testing.rst
> @@ -659,6 +659,17 @@ vm
>  A QEMUMachine instance, initially configured according to the given
>  ``qemu_bin`` parameter.
>
> +arch
> +
> +
> +The architecture that will be used on a number of different
> +scenarios.  For instance, when a QEMU binary is not explicitly given,
> +the one selected will depend on this attribute.
> +
> +The ``arch`` attribute will be set to the test parameter of the same
> +name, and if one is not given explicitly, it will default to the
> +system architecture (as given by ``uname``).
> +
>  qemu_bin
>  
>
> @@ -681,6 +692,13 @@ like the following:
>
>PARAMS (key=qemu_bin, path=*, default=x86_64-softmmu/qemu-system-x86_64) 
> => 'x86_64-softmmu/qemu-system-x86_64
>
> +arch
> +
> +
> +The architecture that will be used on a number of different scenarios.
> +This parameter has a direct relation with the ``arch`` attribute.  If
> +not given, it will default to None.
> +
>  qemu_bin
>  
>
> diff --git a/tests/acceptance/avocado_qemu/__init__.py 
> b/tests/acceptance/avocado_qemu/__init__.py
> index d8d5b48dac..9329d9b9ec 100644
> --- a/tests/acceptance/avocado_qemu/__init__.py
> +++ b/tests/acceptance/avocado_qemu/__init__.py
> @@ -23,16 +23,22 @@ def is_readable_executable_file(path):
>  return os.path.isfile(path) and os.access(path, os.R_OK | os.X_OK)
>
>
> -def pick_default_qemu_bin():
> +def pick_default_qemu_bin(arch=None):
>  """
>  Picks the path of a QEMU binary, starting either in the current working
>  directory or in the source tree root directory.
>
> +:param arch: the arch to use when looking for a QEMU binary (the target
> + will match the arch given).  If None (the default) arch
> + will be the current host system arch (as given by
> + :func:`os.uname`).
> +:param arch: str

Do you mean

:type arch: str

?

>  :returns: the path to the default QEMU binary or None if one could not
>be found
>  :rtype: str or None
>  """
> -arch = os.uname()[4]
> +if arch is None:
> +arch = os.uname()[4]
>  qemu_bin_relative_path = os.path.join("%s-softmmu" % arch,
>"qemu-system-%s" % arch)

On ppc64le systems, this can lead to an unexisting path.

For instance, os.uname() returns:

>>> os.uname()
('Linux', 'localhost', '4.15.0-34-generic', '#37-Ubuntu SMP Mon Aug 27 
15:18:58 UTC 2018', 'ppc64le')

Then, qemu_bin_relative_path would be set to:

ppc64le-softmmu/qemu-system-ppc64le

However, the correct path is:

ppc64-softmmu/qemu-system-ppc64

I'm not sure if ppc64le is the only case where the target name is different from
the OS architecture.

If you decide not to handle this now, at least add a FIXME, if possible.

>  if is_readable_executable_file(qemu_bin_relative_path):
> @@ -47,8 +53,9 @@ def pick_default_qemu_bin():
>  class Test(avocado.Test):
>  def setUp(self):
>  self.vm = None
> +self.arch = self.params.get('arch', default=os.uname()[4])
>  self.qemu_bin = self.params.get('qemu_bin',
> -default=pick_default_qemu_bin())
> +
> default=pick_default_qemu_bin(self.arch))
>  if self.qemu_bin is None:
>  self.cancel("No QEMU binary defined or found in the source tree")
>  self.vm = QEMUMachine(self.qemu_bin)
> --
> 2.17.1
>
>

--
Murilo




Re: [Qemu-devel] [PATCH v1 08/12] net: cadence_gem: Announce 64bit addressing support

2018-10-04 Thread Alistair

On 10/03/2018 08:07 AM, Edgar E. Iglesias wrote:

From: "Edgar E. Iglesias" 

Announce 64bit addressing support.

Signed-off-by: Edgar E. Iglesias 


Reviewed-by: Alistair Francis 

Alistair


---
  hw/net/cadence_gem.c | 3 ++-
  1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/hw/net/cadence_gem.c b/hw/net/cadence_gem.c
index f93cd8e..fc81fb5 100644
--- a/hw/net/cadence_gem.c
+++ b/hw/net/cadence_gem.c
@@ -142,6 +142,7 @@
  #define GEM_DESCONF4  (0x028C/4)
  #define GEM_DESCONF5  (0x0290/4)
  #define GEM_DESCONF6  (0x0294/4)
+#define GEM_DESCONF6_64B_MASK (1U << 23)
  #define GEM_DESCONF7  (0x0298/4)
  
  #define GEM_INT_Q1_STATUS   (0x0400 / 4)

@@ -1300,7 +1301,7 @@ static void gem_reset(DeviceState *d)
  s->regs[GEM_DESCONF] = 0x02500111;
  s->regs[GEM_DESCONF2] = 0x2ab13fff;
  s->regs[GEM_DESCONF5] = 0x002f2045;
-s->regs[GEM_DESCONF6] = 0x0;
+s->regs[GEM_DESCONF6] = GEM_DESCONF6_64B_MASK;
  
  queues_mask = MAKE_64BIT_MASK(1, s->num_priority_queues - 1);

  s->regs[GEM_DESCONF6] |= queues_mask;





Re: [Qemu-devel] [PATCH v1 05/12] net: cadence_gem: Add support for extended descriptors

2018-10-04 Thread Alistair

On 10/03/2018 08:07 AM, Edgar E. Iglesias wrote:

From: "Edgar E. Iglesias" 

Add support for extended descriptors with optional 64bit
addressing and timestamping. QEMU will not yet provide
timestamps (always leaving the valid timestamp bit as zero).

Signed-off-by: Edgar E. Iglesias 


Reviewed-by: Alistair Francis 

Alistair


---
  hw/net/cadence_gem.c | 69 
  include/hw/net/cadence_gem.h |  2 +-
  2 files changed, 52 insertions(+), 19 deletions(-)

diff --git a/hw/net/cadence_gem.c b/hw/net/cadence_gem.c
index 4d769b0..759c1d7 100644
--- a/hw/net/cadence_gem.c
+++ b/hw/net/cadence_gem.c
@@ -207,6 +207,9 @@
  #define GEM_NWCFG_BCAST_REJ0x0020 /* Reject broadcast packets */
  #define GEM_NWCFG_PROMISC  0x0010 /* Accept all packets */
  
+#define GEM_DMACFG_ADDR_64B(1U << 30)

+#define GEM_DMACFG_TX_BD_EXT   (1U << 29)
+#define GEM_DMACFG_RX_BD_EXT   (1U << 28)
  #define GEM_DMACFG_RBUFSZ_M0x00FF /* DMA RX Buffer Size mask */
  #define GEM_DMACFG_RBUFSZ_S16 /* DMA RX Buffer Size shift */
  #define GEM_DMACFG_RBUFSZ_MUL  64 /* DMA RX Buffer Size multiplier */
@@ -302,9 +305,14 @@
  
  #define GEM_MODID_VALUE 0x00020118
  
-static inline unsigned tx_desc_get_buffer(uint32_t *desc)

+static inline uint64_t tx_desc_get_buffer(CadenceGEMState *s, uint32_t *desc)
  {
-return desc[0];
+uint64_t ret = desc[0];
+
+if (s->regs[GEM_DMACFG] & GEM_DMACFG_ADDR_64B) {
+ret |= (uint64_t)desc[2] << 32;
+}
+return ret;
  }
  
  static inline unsigned tx_desc_get_used(uint32_t *desc)

@@ -347,9 +355,30 @@ static inline void print_gem_tx_desc(uint32_t *desc, 
uint8_t queue)
  DB_PRINT("length:  %d\n", tx_desc_get_length(desc));
  }
  
-static inline unsigned rx_desc_get_buffer(uint32_t *desc)

+static inline uint64_t rx_desc_get_buffer(CadenceGEMState *s, uint32_t *desc)
  {
-return desc[0] & ~0x3UL;
+uint64_t ret = desc[0] & ~0x3UL;
+
+if (s->regs[GEM_DMACFG] & GEM_DMACFG_ADDR_64B) {
+ret |= (uint64_t)desc[2] << 32;
+}
+return ret;
+}
+
+static inline int gem_get_desc_len(CadenceGEMState *s, bool rx_n_tx)
+{
+int ret = 2;
+
+if (s->regs[GEM_DMACFG] & GEM_DMACFG_ADDR_64B) {
+ret += 2;
+}
+if (s->regs[GEM_DMACFG] & (rx_n_tx ? GEM_DMACFG_RX_BD_EXT
+   : GEM_DMACFG_TX_BD_EXT)) {
+ret += 2;
+}
+
+assert(ret <= DESC_MAX_NUM_WORDS);
+return ret;
  }
  
  static inline unsigned rx_desc_get_wrap(uint32_t *desc)

@@ -419,7 +448,7 @@ static void gem_init_register_masks(CadenceGEMState *s)
  memset(&s->regs_ro[0], 0, sizeof(s->regs_ro));
  s->regs_ro[GEM_NWCTRL]   = 0xFFF8;
  s->regs_ro[GEM_NWSTATUS] = 0x;
-s->regs_ro[GEM_DMACFG]   = 0xFE00F000;
+s->regs_ro[GEM_DMACFG]   = 0x8E00F000;
  s->regs_ro[GEM_TXSTATUS] = 0xFE08;
  s->regs_ro[GEM_RXQBASE]  = 0x0003;
  s->regs_ro[GEM_TXQBASE]  = 0x0003;
@@ -807,7 +836,8 @@ static void gem_get_rx_desc(CadenceGEMState *s, int q)
  DB_PRINT("read descriptor 0x%x\n", (unsigned)s->rx_desc_addr[q]);
  /* read current descriptor */
  cpu_physical_memory_read(s->rx_desc_addr[q],
- (uint8_t *)s->rx_desc[q], sizeof(s->rx_desc[q]));
+ (uint8_t *)s->rx_desc[q],
+ sizeof(uint32_t) * gem_get_desc_len(s, true));
  
  /* Descriptor owned by software ? */

  if (rx_desc_get_ownership(s->rx_desc[q]) == 1) {
@@ -926,9 +956,10 @@ static ssize_t gem_receive(NetClientState *nc, const 
uint8_t *buf, size_t size)
  rx_desc_get_buffer(s->rx_desc[q]));
  
  /* Copy packet data to emulated DMA buffer */

-cpu_physical_memory_write(rx_desc_get_buffer(s->rx_desc[q]) +
- rxbuf_offset,
-  rxbuf_ptr, MIN(bytes_to_copy, rxbufsize));
+cpu_physical_memory_write(rx_desc_get_buffer(s, s->rx_desc[q]) +
+  rxbuf_offset,
+  rxbuf_ptr,
+  MIN(bytes_to_copy, rxbufsize));
  rxbuf_ptr += MIN(bytes_to_copy, rxbufsize);
  bytes_to_copy -= MIN(bytes_to_copy, rxbufsize);
  
@@ -964,7 +995,7 @@ static ssize_t gem_receive(NetClientState *nc, const uint8_t *buf, size_t size)

  /* Descriptor write-back.  */
  cpu_physical_memory_write(s->rx_desc_addr[q],
(uint8_t *)s->rx_desc[q],
-  sizeof(s->rx_desc[q]));
+  sizeof(uint32_t) * gem_get_desc_len(s, 
true));
  
  /* Next descriptor */

  if (rx_desc_get_wrap(s->rx_desc[q])) {
@@ -972,7 +1003,7 @@ static ssize_t gem_receive(NetClientState *nc, const 
uint8_t *buf, size_t size)
  s->rx_desc

Re: [Qemu-devel] [PATCH v1 04/12] net: cadence_gem: Add macro with max number of descriptor words

2018-10-04 Thread Alistair

On 10/03/2018 08:07 AM, Edgar E. Iglesias wrote:

From: "Edgar E. Iglesias" 

Add macro with max number of DMA descriptor words.
No functional change.

Signed-off-by: Edgar E. Iglesias 


Reviewed-by: Alistair Francis 

Alistair


---
  hw/net/cadence_gem.c | 4 ++--
  include/hw/net/cadence_gem.h | 5 -
  2 files changed, 6 insertions(+), 3 deletions(-)

diff --git a/hw/net/cadence_gem.c b/hw/net/cadence_gem.c
index 31f3fe0..4d769b0 100644
--- a/hw/net/cadence_gem.c
+++ b/hw/net/cadence_gem.c
@@ -1042,7 +1042,7 @@ static void gem_transmit_updatestats(CadenceGEMState *s, 
const uint8_t *packet,
   */
  static void gem_transmit(CadenceGEMState *s)
  {
-uint32_t desc[2];
+uint32_t desc[DESC_MAX_NUM_WORDS];
  hwaddr packet_desc_addr;
  uint8_t tx_packet[2048];
  uint8_t *p;
@@ -1108,7 +1108,7 @@ static void gem_transmit(CadenceGEMState *s)
  
  /* Last descriptor for this packet; hand the whole thing off */

  if (tx_desc_get_last(desc)) {
-uint32_t desc_first[2];
+uint32_t desc_first[DESC_MAX_NUM_WORDS];
  
  /* Modify the 1st descriptor of this packet to be owned by

   * the processor.
diff --git a/include/hw/net/cadence_gem.h b/include/hw/net/cadence_gem.h
index 633d564..b33ef65 100644
--- a/include/hw/net/cadence_gem.h
+++ b/include/hw/net/cadence_gem.h
@@ -32,6 +32,9 @@
  
  #define CADENCE_GEM_MAXREG(0x0800 / 4) /* Last valid GEM address */
  
+/* Max number of words in a DMA descriptor.  */

+#define DESC_MAX_NUM_WORDS  2
+
  #define MAX_PRIORITY_QUEUES 8
  #define MAX_TYPE1_SCREENERS 16
  #define MAX_TYPE2_SCREENERS 16
@@ -74,7 +77,7 @@ typedef struct CadenceGEMState {
  
  uint8_t can_rx_state; /* Debug only */
  
-uint32_t rx_desc[MAX_PRIORITY_QUEUES][2];

+uint32_t rx_desc[MAX_PRIORITY_QUEUES][DESC_MAX_NUM_WORDS];
  
  bool sar_active[4];

  } CadenceGEMState;





Re: [Qemu-devel] [PATCH v1 03/12] net: cadence_gem: Use uint32_t for 32bit descriptor words

2018-10-04 Thread Alistair

On 10/03/2018 08:07 AM, Edgar E. Iglesias wrote:

From: "Edgar E. Iglesias" 

Use uint32_t instead of unsigned to describe 32bit descriptor words.

Signed-off-by: Edgar E. Iglesias 


Reviewed-by: Alistair Francis 

Alistair


---
  hw/net/cadence_gem.c | 42 +-
  include/hw/net/cadence_gem.h |  2 +-
  2 files changed, 22 insertions(+), 22 deletions(-)

diff --git a/hw/net/cadence_gem.c b/hw/net/cadence_gem.c
index 901c173..31f3fe0 100644
--- a/hw/net/cadence_gem.c
+++ b/hw/net/cadence_gem.c
@@ -302,42 +302,42 @@
  
  #define GEM_MODID_VALUE 0x00020118
  
-static inline unsigned tx_desc_get_buffer(unsigned *desc)

+static inline unsigned tx_desc_get_buffer(uint32_t *desc)
  {
  return desc[0];
  }
  
-static inline unsigned tx_desc_get_used(unsigned *desc)

+static inline unsigned tx_desc_get_used(uint32_t *desc)
  {
  return (desc[1] & DESC_1_USED) ? 1 : 0;
  }
  
-static inline void tx_desc_set_used(unsigned *desc)

+static inline void tx_desc_set_used(uint32_t *desc)
  {
  desc[1] |= DESC_1_USED;
  }
  
-static inline unsigned tx_desc_get_wrap(unsigned *desc)

+static inline unsigned tx_desc_get_wrap(uint32_t *desc)
  {
  return (desc[1] & DESC_1_TX_WRAP) ? 1 : 0;
  }
  
-static inline unsigned tx_desc_get_last(unsigned *desc)

+static inline unsigned tx_desc_get_last(uint32_t *desc)
  {
  return (desc[1] & DESC_1_TX_LAST) ? 1 : 0;
  }
  
-static inline void tx_desc_set_last(unsigned *desc)

+static inline void tx_desc_set_last(uint32_t *desc)
  {
  desc[1] |= DESC_1_TX_LAST;
  }
  
-static inline unsigned tx_desc_get_length(unsigned *desc)

+static inline unsigned tx_desc_get_length(uint32_t *desc)
  {
  return desc[1] & DESC_1_LENGTH;
  }
  
-static inline void print_gem_tx_desc(unsigned *desc, uint8_t queue)

+static inline void print_gem_tx_desc(uint32_t *desc, uint8_t queue)
  {
  DB_PRINT("TXDESC (queue %" PRId8 "):\n", queue);
  DB_PRINT("bufaddr: 0x%08x\n", *desc);
@@ -347,58 +347,58 @@ static inline void print_gem_tx_desc(unsigned *desc, 
uint8_t queue)
  DB_PRINT("length:  %d\n", tx_desc_get_length(desc));
  }
  
-static inline unsigned rx_desc_get_buffer(unsigned *desc)

+static inline unsigned rx_desc_get_buffer(uint32_t *desc)
  {
  return desc[0] & ~0x3UL;
  }
  
-static inline unsigned rx_desc_get_wrap(unsigned *desc)

+static inline unsigned rx_desc_get_wrap(uint32_t *desc)
  {
  return desc[0] & DESC_0_RX_WRAP ? 1 : 0;
  }
  
-static inline unsigned rx_desc_get_ownership(unsigned *desc)

+static inline unsigned rx_desc_get_ownership(uint32_t *desc)
  {
  return desc[0] & DESC_0_RX_OWNERSHIP ? 1 : 0;
  }
  
-static inline void rx_desc_set_ownership(unsigned *desc)

+static inline void rx_desc_set_ownership(uint32_t *desc)
  {
  desc[0] |= DESC_0_RX_OWNERSHIP;
  }
  
-static inline void rx_desc_set_sof(unsigned *desc)

+static inline void rx_desc_set_sof(uint32_t *desc)
  {
  desc[1] |= DESC_1_RX_SOF;
  }
  
-static inline void rx_desc_set_eof(unsigned *desc)

+static inline void rx_desc_set_eof(uint32_t *desc)
  {
  desc[1] |= DESC_1_RX_EOF;
  }
  
-static inline void rx_desc_set_length(unsigned *desc, unsigned len)

+static inline void rx_desc_set_length(uint32_t *desc, unsigned len)
  {
  desc[1] &= ~DESC_1_LENGTH;
  desc[1] |= len;
  }
  
-static inline void rx_desc_set_broadcast(unsigned *desc)

+static inline void rx_desc_set_broadcast(uint32_t *desc)
  {
  desc[1] |= R_DESC_1_RX_BROADCAST;
  }
  
-static inline void rx_desc_set_unicast_hash(unsigned *desc)

+static inline void rx_desc_set_unicast_hash(uint32_t *desc)
  {
  desc[1] |= R_DESC_1_RX_UNICAST_HASH;
  }
  
-static inline void rx_desc_set_multicast_hash(unsigned *desc)

+static inline void rx_desc_set_multicast_hash(uint32_t *desc)
  {
  desc[1] |= R_DESC_1_RX_MULTICAST_HASH;
  }
  
-static inline void rx_desc_set_sar(unsigned *desc, int sar_idx)

+static inline void rx_desc_set_sar(uint32_t *desc, int sar_idx)
  {
  desc[1] = deposit32(desc[1], R_DESC_1_RX_SAR_SHIFT, 
R_DESC_1_RX_SAR_LENGTH,
  sar_idx);
@@ -1042,7 +1042,7 @@ static void gem_transmit_updatestats(CadenceGEMState *s, 
const uint8_t *packet,
   */
  static void gem_transmit(CadenceGEMState *s)
  {
-unsigneddesc[2];
+uint32_t desc[2];
  hwaddr packet_desc_addr;
  uint8_t tx_packet[2048];
  uint8_t *p;
@@ -1108,7 +1108,7 @@ static void gem_transmit(CadenceGEMState *s)
  
  /* Last descriptor for this packet; hand the whole thing off */

  if (tx_desc_get_last(desc)) {
-unsigneddesc_first[2];
+uint32_t desc_first[2];
  
  /* Modify the 1st descriptor of this packet to be owned by

   * the processor.
diff --git a/include/hw/net/cadence_gem.h b/include/hw/net/cadence_gem.h
index 35de622..633d564 100644
--- a/include/hw/net/cadence_gem.h
+++ b/include/hw/net/cadence_gem.h
@@ -74,7 +74,

Re: [Qemu-devel] [PATCH v1 02/12] net: cadence_gem: Announce availability of priority queues

2018-10-04 Thread Alistair

On 10/03/2018 08:07 AM, Edgar E. Iglesias wrote:

From: "Edgar E. Iglesias" 

Announce the availability of the various priority queues.
This fixes an issue where guest kernels would miss to
configure secondary queues due to inproper feature bits.

Signed-off-by: Edgar E. Iglesias 


Reviewed-by: Alistair Francis 

Alistair


---
  hw/net/cadence_gem.c | 6 +-
  1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/hw/net/cadence_gem.c b/hw/net/cadence_gem.c
index e560b7a..901c173 100644
--- a/hw/net/cadence_gem.c
+++ b/hw/net/cadence_gem.c
@@ -1213,6 +1213,7 @@ static void gem_reset(DeviceState *d)
  int i;
  CadenceGEMState *s = CADENCE_GEM(d);
  const uint8_t *a;
+uint32_t queues_mask;
  
  DB_PRINT("\n");
  
@@ -1229,7 +1230,10 @@ static void gem_reset(DeviceState *d)

  s->regs[GEM_DESCONF] = 0x02500111;
  s->regs[GEM_DESCONF2] = 0x2ab13fff;
  s->regs[GEM_DESCONF5] = 0x002f2045;
-s->regs[GEM_DESCONF6] = 0x0200;
+s->regs[GEM_DESCONF6] = 0x0;
+
+queues_mask = MAKE_64BIT_MASK(1, s->num_priority_queues - 1);
+s->regs[GEM_DESCONF6] |= queues_mask;
  
  /* Set MAC address */

  a = &s->conf.macaddr.a[0];





Re: [Qemu-devel] [RFC] target/xtensa: rework zero overhead loops implementation

2018-10-04 Thread Max Filippov
On Thu, Oct 4, 2018 at 3:03 PM Max Filippov  wrote:
> But I guess always storing low 13 bits of LEND - PC should work?

...when they're within two pages from each other...

-- 
Thanks.
-- Max



Re: [Qemu-devel] [RFC] target/xtensa: rework zero overhead loops implementation

2018-10-04 Thread Max Filippov
Hi Richard,

thank you for taking a look.

On Thu, Oct 4, 2018 at 2:13 PM Richard Henderson
 wrote:
> Think first about how, if PC >= LEND or LEND - PC > PAGE_SIZE, that all of the
> loop stuff is irrelevant because we won't hit LEND within this TB.
>
> Think second about how to represent the common case -- how LOOP sets LBEG and
> LEND together.  That is, LEND - LBEG <= 256.  So, usually, we can also have an
> exact representation of LBEG and have a direct link rather than an indirect
> link.

I tried similar thing as a followup to my original patch, recording entire LBEG
in the cs_base -- surprisingly to me it made no measurable difference on my
tests.

>  But I presume that one can play games with special registers to create
> ranges that LOOP won't.  So we need some setting that will indicate that.
>
> Consider CS_BASE fields:
>
>   [12: 0]  EDIF = LEND - PC, if PC < LEND && LEND - PC < 2*PAGE_SIZE, or 0.

That has the same issue as my original patch: two TBs with PC > LEND
in both may be linked together, and then LEND may change, so that
PC1 > LEND and PC2 < LEND.
But I guess always storing low 13 bits of LEND - PC should work?
I'll play with it...

-- 
Thanks.
-- Max



Re: [Qemu-devel] [PATCH v2 15/15] arm/xlnx-zynqmp: put APUs and RPUs in separate GDB groups

2018-10-04 Thread Eduardo Habkost
On Thu, Oct 04, 2018 at 09:01:09PM +0100, Peter Maydell wrote:
> On 4 October 2018 at 20:52, Eduardo Habkost  wrote:
> > Changing the object hierarchy based on GDB groups doesn't seem
> > right, but I don't think it would be a big deal if we have the
> > board code explicitly telling the GDB code how to group the CPUs.
> >
> > If you really want to do it implicitly, would it work if you
> > simply group the CPUs based on object_get_canonical_path()?
> >
> > If a more explicit GDB grouping API is acceptable, what about
> > just adding a INTERFACE_GDB_GROUP interface name to (existing)
> > container objects that we expect to become GDB groups?
> >
> > I'm not sure which way is better. I'm a bit worried that making
> > things too implicit could easily break (e.g. if somebody changes
> > the CPU QOM hierarchy in the future for unrelated reasons).
> 
> I don't want things implicit. I just don't want the explicitness
> to be "this is all about GDB", because it isn't. I want us
> to explicitly say "these 4 CPUs are in one cluster" (or
> whatever term we use), because that affects more than merely GDB.

We already have a way to say "these 4 CPUs are in one cluster",
don't we?  That's the QOM hierarchy.

My question is if "the CPUs are in one cluster" should implicitly
mean "the CPUs are in one GDB group".

-- 
Eduardo



Re: [Qemu-devel] [PATCH v3 13/18] block: introduce new filter driver: fleecing-hook

2018-10-04 Thread Vladimir Sementsov-Ogievskiy

On 10/04/2018 05:52 PM, Kevin Wolf wrote:

Am 04.10.2018 um 15:59 hat Vladimir Sementsov-Ogievskiy geschrieben:

04.10.2018 15:44, Kevin Wolf wrote:

Am 01.10.2018 um 12:29 hat Vladimir Sementsov-Ogievskiy geschrieben:

Fleecing-hook filter does copy-before-write operation. It should be
inserted above active disk and has a target node for CBW, like the
following:

  +---+
  | Guest |
  +---+---+
  |r,w
  v
  +---+---+  target   +---+
  | Fleecing hook |-->| target(qcow2) |
  +---+---+   CBW +---+---+
  |   |
backing |r,w|
  v   |
  +---+-+  backing|
  | Active disk |<+
  +-+r

Target's backing may point to active disk (should be set up
separately), which gives fleecing-scheme.

Signed-off-by: Vladimir Sementsov-Ogievskiy 

This lacks an explanation why we need a specialised fleecing hook driver
rather than just a generic bdrv_backup_top block driver in analogy to
what commit and mirror are already doing.

In fact, if I'm reading the last patch of the series right, backup
doesn't even restrict the use of the fleecing-hook driver to actual
fleecing scenarios.

Maybe what doesn't feel right to me is just that it's a misnomer, and if
you rename it into bdrv_backup_top (and make it internal to the block
job), it is very close to what I actually have in mind?

Kevin

Hm.
1. assume we move to internal bdrv_backup_top
2. backup(mode=none) becomes just a wrapper for append/drop of the
bdrv_backup_top node

I think you mean sync=none?

Yes, this is true. There is no actual background job taking place there,
so the job infrastructure doesn't add much. As you say, it's just
inserting the node at the start and dropping it again at the end.


3. looks interesting to get rid of empty (doing nothing) job and use
bdrv_backup_top directly.

We could directly make the filter node available for the user, like this
series does. Should we do that? I'm not sure, but I'm not necessarily
opposed either.

But looking at the big picture, I have some more thoughts on this:

1. Is backup with sync=none only useful for fleecing? My understanding
was that "fleecing" specifically means a setup where the target of
the backup node is an overlay of the active layer of the guest
device.

I can imagine other use cases that would use sync=none (e.g. if you
don't access arbitrary blocks like from the NBD server in the
fleecing setup, but directly write to a backup file that can be
commited back later to revert things).

So I think 'fleecing-hook' is too narrow as a name. Maybe just
'backup' would be better?


may be copy-before-write?



2. mirror has a sync=none mode, too. And like backup, it doesn't
actually have any background job running then (at least in active
mirror mode), but only changes the graph at the end of the job.
Some consistency would be nice there, so is the goal to eventually
let the user create filter nodes for all jobs that don't have a
real background job?

3. We have been thinking about unifying backup, commit and mirror
into a single copy block job because they are doing quite similar
things. Of course, there are differences whether the old data or the
new data should be copied on a write, and which graph changes to make
at the end of the job, but many of the other differences are actually
features that would make sense in all of them, but are only
implemented in one job driver.

Maybe having a single 'copy' filter driver that provides options to
select backup-like behaviour or mirror-like behaviour, and that can
then internally be used by all three block jobs would be an
interesting first step towards this?



Isn't it a question about having several simple things against one 
complicated?)
All these jobs are similar only in the fact that they are copying blocks 
from one point to another.. So, instead of creating one big job with a 
lot of options, we can separate copying code to some kind of internal 
copying api, to then share it between jobs (and qemi-img convert). 
Didn't you considered this way? Intuitively, I'm not a fan of idea to 
create one job, but I don't insist. Of course, we can create one job 
carefully split the code to different objects files, with (again) 
separate copying api, shared with qemu-img, so, difference between 
one-job vs several-jobs will be mostly in qapi/ , not in the real code...




We can start with supporting only what backup needs, but design
everything with the idea that mirror and commit could use it, too.

I honestly feel that at first this wouldn't be very different from what
you have, so with a few renames and cleanups we might be good. But it
would give us a design in the grand scheme to work towards instead of
doing one-off things fo

Re: [Qemu-devel] [RFC] target/xtensa: rework zero overhead loops implementation

2018-10-04 Thread Richard Henderson
On 10/4/18 3:14 PM, Max Filippov wrote:
> I thought about it some more and it looks like this is not going to work
> in general case in the presence of TB linking: a block with a big (and thus
> not precise) LEND distance may be linked to a block with a small (and
> thus precise) LEND distance. Then LEND may change so that next time
> it still goes to the first TB. In that case it shouldn't go from the first TB
> to the second, but with this scheme it will.

Indeed.  Perhaps think of ways in which LBEG and LEND can be represented
relative to each other and PC and store than in the 32-bits you have available
in the CS_BASE field.

Think first about how, if PC >= LEND or LEND - PC > PAGE_SIZE, that all of the
loop stuff is irrelevant because we won't hit LEND within this TB.

Think second about how to represent the common case -- how LOOP sets LBEG and
LEND together.  That is, LEND - LBEG <= 256.  So, usually, we can also have an
exact representation of LBEG and have a direct link rather than an indirect
link.  But I presume that one can play games with special registers to create
ranges that LOOP won't.  So we need some setting that will indicate that.

Consider CS_BASE fields:

  [12: 0]  EDIF = LEND - PC, if PC < LEND && LEND - PC < 2*PAGE_SIZE, or 0.
  [20:13]  BDIF = LEND - LBEG, if LEND - LBEG < 256, or 0.

So you can tell if advancing PC within a TB will exactly match LEND.  You can
tell what LBEG should be, except if BDIF == 0.  In that, presumably rare case,
you load LBEG at runtime as you did in this patch.

Note that if CS_BASE == 0, and thus EDIF == 0, looping is disabled for the TB.

I'll note that this also makes XTENSA_TBFLAG_EXCM redundant.  Simply skip
setting CS_BASE to a non-zero value instead.


r~



Re: [Qemu-devel] [PATCH v2 0/4] per-TLB lock

2018-10-04 Thread Alex Bennée


Emilio G. Cota  writes:

> v1: https://lists.gnu.org/archive/html/qemu-devel/2018-10/msg00395.html
>
> Changes since v1:
>
> - Rebase on master
>
> - Expand lock usage to other tlb_table/tlb_v_table updates, which I
>   missed in v1
>
> - Fix assert_cpu_is_self macro
>
> - Add comment on why the owner thread doesn't need to use atomic_set
>   for updates
>
> - Add more calls to assert_cpu_is_self macro, which together with
>   the added comment should make the code simpler to understand
>
> - Include perf numbers in the last patch
>
> The series is checkpatch-clean. You can fetch the code from:
>   https://github.com/cota/qemu/tree/tlb-lock-v2

Weird build failure aside I gave it a good build test soak which usually
trips up TLB issues.

Tested-by: Alex Bennée 

--
Alex Bennée



Re: [Qemu-devel] [RFC] target/xtensa: rework zero overhead loops implementation

2018-10-04 Thread Max Filippov
On Wed, Oct 3, 2018 at 6:05 PM Max Filippov  wrote:
>
> Don't invalidate TB with the end of zero overhead loop when LBEG or LEND
> change. Instead encode the distance from the TB start to the LEND in the
> TB flags and generate loopback code when offset of the next PC from the
> TB start equals that distance. Distance not greater than the maximal
> instruction length is encoded literally, greater distances are capped at
> the target page size and encoded as the maximal instruction length plus
> the greatest power of 2 that is not bigger than the distance.
>
> Although this change adds dynamic TB search at the end of each zero
> overhead loop the resulting emulation speed is about 10% higher in
> uClibc-ng and LTP tests.

I thought about it some more and it looks like this is not going to work
in general case in the presence of TB linking: a block with a big (and thus
not precise) LEND distance may be linked to a block with a small (and
thus precise) LEND distance. Then LEND may change so that next time
it still goes to the first TB. In that case it shouldn't go from the first TB
to the second, but with this scheme it will.

-- 
Thanks.
-- Max



[Qemu-devel] [PATCH v5 4/5] hw/riscv/sifive_u: Connect the Xilinx PCIe

2018-10-04 Thread Alistair Francis
Connect the Xilinx PCIe device based on the information in the device
tree stored in the ROM of the HiFish Unleashed board.

Signed-off-by: Alistair Francis 
---
 hw/riscv/sifive_u.c | 64 +
 include/hw/riscv/sifive_u.h |  4 ++-
 2 files changed, 67 insertions(+), 1 deletion(-)

diff --git a/hw/riscv/sifive_u.c b/hw/riscv/sifive_u.c
index 862f8ff5f7..4e273119c3 100644
--- a/hw/riscv/sifive_u.c
+++ b/hw/riscv/sifive_u.c
@@ -45,6 +45,8 @@
 #include "sysemu/arch_init.h"
 #include "sysemu/device_tree.h"
 #include "exec/address-spaces.h"
+#include "hw/pci/pci.h"
+#include "hw/pci-host/xilinx-pcie.h"
 #include "elf.h"
 
 #include 
@@ -61,6 +63,7 @@ static const struct MemmapEntry {
 [SIFIVE_U_UART1] ={ 0x10023000, 0x1000 },
 [SIFIVE_U_DRAM] = { 0x8000,0x0 },
 [SIFIVE_U_GEM] =  { 0x100900FC, 0x2000 },
+[SIFIVE_U_PCIE] = { 0x20, 0x400 },
 };
 
 #define GEM_REVISION0x10070109
@@ -218,6 +221,32 @@ static void create_fdt(SiFiveUState *s, const struct 
MemmapEntry *memmap,
 qemu_fdt_setprop_cells(fdt, nodename, "reg", 0x0);
 g_free(nodename);
 
+nodename = g_strdup_printf("/pci@%lx",
+(long) memmap[SIFIVE_U_PCIE].base);
+qemu_fdt_add_subnode(fdt, nodename);
+qemu_fdt_setprop_cells(fdt, nodename, "#address-cells", 0x3);
+qemu_fdt_setprop_cells(fdt, nodename, "#interrupt-cells", 0x1);
+qemu_fdt_setprop_cells(fdt, nodename, "#size-cells", 0x2);
+qemu_fdt_setprop_string(fdt, nodename, "compatible",
+"xlnx,axi-pcie-host-1.00.a");
+qemu_fdt_setprop_string(fdt, nodename, "device_type", "pci");
+qemu_fdt_setprop_cells(fdt, nodename, "reg", 0x20, 0x0, 0x0,
+   memmap[SIFIVE_U_PCIE].size);
+qemu_fdt_setprop_string(fdt, nodename, "reg-names", "control");
+qemu_fdt_setprop_cells(fdt, nodename, "ranges", 0x200, 0x0,
+   0x4000, 0x0, 0x4000, 0x0, 0x2000);
+qemu_fdt_setprop_cells(fdt, nodename, "interrupt-parent", plic_phandle);
+qemu_fdt_setprop_cells(fdt, nodename, "interrupts", SIFIVE_U_PCIE_IRQ);
+g_free(nodename);
+
+nodename = g_strdup_printf("/pci@%lx/interrupt-controller",
+(long) memmap[SIFIVE_U_PCIE].base);
+qemu_fdt_add_subnode(fdt, nodename);
+qemu_fdt_setprop_cells(fdt, nodename, "#address-cells", 0x00);
+qemu_fdt_setprop_cells(fdt, nodename, "#interrupt-cells", 0x1);
+qemu_fdt_setprop(fdt, nodename, "interrupt-controller", NULL, 0);
+g_free(nodename);
+
 nodename = g_strdup_printf("/soc/uart@%lx",
 (long)memmap[SIFIVE_U_UART0].base);
 qemu_fdt_add_subnode(fdt, nodename);
@@ -234,6 +263,37 @@ static void create_fdt(SiFiveUState *s, const struct 
MemmapEntry *memmap,
 g_free(nodename);
 }
 
+static inline DeviceState *
+xilinx_pcie_init(MemoryRegion *sys_mem, uint32_t bus_nr,
+ hwaddr cfg_base, uint64_t cfg_size,
+ hwaddr mmio_base, uint64_t mmio_size,
+ qemu_irq irq, bool link_up)
+{
+DeviceState *dev;
+MemoryRegion *cfg, *mmio;
+
+dev = qdev_create(NULL, TYPE_XILINX_PCIE_HOST);
+
+qdev_prop_set_uint32(dev, "bus_nr", bus_nr);
+qdev_prop_set_uint64(dev, "cfg_base", cfg_base);
+qdev_prop_set_uint64(dev, "cfg_size", cfg_size);
+qdev_prop_set_uint64(dev, "mmio_base", mmio_base);
+qdev_prop_set_uint64(dev, "mmio_size", mmio_size);
+qdev_prop_set_bit(dev, "link_up", link_up);
+
+qdev_init_nofail(dev);
+
+cfg = sysbus_mmio_get_region(SYS_BUS_DEVICE(dev), 0);
+memory_region_add_subregion_overlap(sys_mem, cfg_base, cfg, 0);
+
+mmio = sysbus_mmio_get_region(SYS_BUS_DEVICE(dev), 1);
+memory_region_add_subregion_overlap(sys_mem, 0, mmio, 0);
+
+qdev_connect_gpio_out_named(dev, "interrupt_out", 0, irq);
+
+return dev;
+}
+
 static void riscv_sifive_u_init(MachineState *machine)
 {
 const struct MemmapEntry *memmap = sifive_u_memmap;
@@ -373,6 +433,10 @@ static void riscv_sifive_u_soc_realize(DeviceState *dev, 
Error **errp)
 sysbus_mmio_map(SYS_BUS_DEVICE(&s->gem), 0, memmap[SIFIVE_U_GEM].base);
 sysbus_connect_irq(SYS_BUS_DEVICE(&s->gem), 0,
plic_gpios[SIFIVE_U_GEM_IRQ]);
+
+xilinx_pcie_init(system_memory, 0, memmap[SIFIVE_U_PCIE].base,
+ memmap[SIFIVE_U_PCIE].size, 0x4000, 0x2000,
+ qdev_get_gpio_in(DEVICE(s->plic), SIFIVE_U_PCIE_IRQ), 
true);
 }
 
 static void riscv_sifive_u_machine_init(MachineClass *mc)
diff --git a/include/hw/riscv/sifive_u.h b/include/hw/riscv/sifive_u.h
index e8b4d9ffa3..e7292ea83b 100644
--- a/include/hw/riscv/sifive_u.h
+++ b/include/hw/riscv/sifive_u.h
@@ -53,12 +53,14 @@ enum {
 SIFIVE_U_UART0,
 SIFIVE_U_UART1,
 SIFIVE_U_DRAM,
-SIFIVE_U_GEM
+SIFIVE_U_GEM,
+SIFIVE_U_PCIE
 };
 
 enum {
 SIFIVE_U_UART0_IRQ = 3,
 SIFIVE_U_UART1_IRQ = 4,
+SIFIVE_U_PCIE_

[Qemu-devel] [PATCH v5 3/5] riscv: Enable VGA and PCIE_VGA

2018-10-04 Thread Alistair Francis
Enable compile support for VGA devices. This allows the user to conenct
a display by adding '-device bochs-display -display sdl' to their
command line argument.

Signed-off-by: Alistair Francis 
---
 default-configs/riscv32-softmmu.mak | 3 +++
 default-configs/riscv64-softmmu.mak | 3 +++
 hw/riscv/virt.c | 4 ++--
 3 files changed, 8 insertions(+), 2 deletions(-)

diff --git a/default-configs/riscv32-softmmu.mak 
b/default-configs/riscv32-softmmu.mak
index 3e3d195f37..05fae82f1b 100644
--- a/default-configs/riscv32-softmmu.mak
+++ b/default-configs/riscv32-softmmu.mak
@@ -9,3 +9,6 @@ CONFIG_CADENCE=y
 
 CONFIG_PCI_GENERIC=y
 CONFIG_PCI_XILINX=y
+
+CONFIG_VGA=y
+CONFIG_VGA_PCI=y
diff --git a/default-configs/riscv64-softmmu.mak 
b/default-configs/riscv64-softmmu.mak
index 3e3d195f37..05fae82f1b 100644
--- a/default-configs/riscv64-softmmu.mak
+++ b/default-configs/riscv64-softmmu.mak
@@ -9,3 +9,6 @@ CONFIG_CADENCE=y
 
 CONFIG_PCI_GENERIC=y
 CONFIG_PCI_XILINX=y
+
+CONFIG_VGA=y
+CONFIG_VGA_PCI=y
diff --git a/hw/riscv/virt.c b/hw/riscv/virt.c
index 9bd2c10581..62a953aa2b 100644
--- a/hw/riscv/virt.c
+++ b/hw/riscv/virt.c
@@ -437,8 +437,8 @@ static void riscv_virt_board_init(MachineState *machine)
 }
 
 gpex_pcie_init(system_memory, 0, memmap[VIRT_PCIE].base,
-   memmap[VIRT_PCIE].size, 0x4000, 0x2000,
-   qdev_get_gpio_in(DEVICE(s->plic), PCIE_IRQ), true);
+   memmap[VIRT_PCIE].size, 0x4000, 0x2000,
+   qdev_get_gpio_in(DEVICE(s->plic), PCIE_IRQ), true);
 
 serial_mm_init(system_memory, memmap[VIRT_UART0].base,
 0, qdev_get_gpio_in(DEVICE(s->plic), UART0_IRQ), 399193,
-- 
2.17.1




[Qemu-devel] [PATCH v5 2/5] hw/riscv/virt: Connect the gpex PCIe

2018-10-04 Thread Alistair Francis
Connect the gpex PCIe device based on the device tree included in the
HiFive Unleashed ROM.

Signed-off-by: Alistair Francis 
---
 default-configs/riscv32-softmmu.mak |  6 ++-
 default-configs/riscv64-softmmu.mak |  6 ++-
 hw/riscv/virt.c | 58 +
 include/hw/riscv/virt.h |  4 +-
 4 files changed, 71 insertions(+), 3 deletions(-)

diff --git a/default-configs/riscv32-softmmu.mak 
b/default-configs/riscv32-softmmu.mak
index 7937c69e22..3e3d195f37 100644
--- a/default-configs/riscv32-softmmu.mak
+++ b/default-configs/riscv32-softmmu.mak
@@ -1,7 +1,11 @@
 # Default configuration for riscv-softmmu
 
+include pci.mak
+
 CONFIG_SERIAL=y
 CONFIG_VIRTIO_MMIO=y
-include virtio.mak
 
 CONFIG_CADENCE=y
+
+CONFIG_PCI_GENERIC=y
+CONFIG_PCI_XILINX=y
diff --git a/default-configs/riscv64-softmmu.mak 
b/default-configs/riscv64-softmmu.mak
index 7937c69e22..3e3d195f37 100644
--- a/default-configs/riscv64-softmmu.mak
+++ b/default-configs/riscv64-softmmu.mak
@@ -1,7 +1,11 @@
 # Default configuration for riscv-softmmu
 
+include pci.mak
+
 CONFIG_SERIAL=y
 CONFIG_VIRTIO_MMIO=y
-include virtio.mak
 
 CONFIG_CADENCE=y
+
+CONFIG_PCI_GENERIC=y
+CONFIG_PCI_XILINX=y
diff --git a/hw/riscv/virt.c b/hw/riscv/virt.c
index 005169eabc..9bd2c10581 100644
--- a/hw/riscv/virt.c
+++ b/hw/riscv/virt.c
@@ -39,6 +39,8 @@
 #include "sysemu/arch_init.h"
 #include "sysemu/device_tree.h"
 #include "exec/address-spaces.h"
+#include "hw/pci/pci.h"
+#include "hw/pci-host/gpex.h"
 #include "elf.h"
 
 #include 
@@ -55,6 +57,7 @@ static const struct MemmapEntry {
 [VIRT_UART0] ={ 0x1000,  0x100 },
 [VIRT_VIRTIO] =   { 0x10001000, 0x1000 },
 [VIRT_DRAM] = { 0x8000,0x0 },
+[VIRT_PCIE] = { 0x20, 0x400 },
 };
 
 static uint64_t load_kernel(const char *kernel_filename)
@@ -233,6 +236,32 @@ static void *create_fdt(RISCVVirtState *s, const struct 
MemmapEntry *memmap,
 g_free(nodename);
 }
 
+nodename = g_strdup_printf("/pci@%lx",
+(long) memmap[VIRT_PCIE].base);
+qemu_fdt_add_subnode(fdt, nodename);
+qemu_fdt_setprop_cells(fdt, nodename, "#address-cells", 0x3);
+qemu_fdt_setprop_cells(fdt, nodename, "#interrupt-cells", 0x1);
+qemu_fdt_setprop_cells(fdt, nodename, "#size-cells", 0x2);
+qemu_fdt_setprop_string(fdt, nodename, "compatible",
+"pci-host-ecam-generic");
+qemu_fdt_setprop_string(fdt, nodename, "device_type", "pci");
+qemu_fdt_setprop_cells(fdt, nodename, "reg", 0x20, 0x0, 0x0,
+   memmap[VIRT_PCIE].size);
+qemu_fdt_setprop_string(fdt, nodename, "reg-names", "control");
+qemu_fdt_setprop_cells(fdt, nodename, "ranges", 0x200, 0x0,
+   0x4000, 0x0, 0x4000, 0x0, 0x2000);
+qemu_fdt_setprop_cells(fdt, nodename, "interrupt-parent", plic_phandle);
+qemu_fdt_setprop_cells(fdt, nodename, "interrupts", PCIE_IRQ);
+g_free(nodename);
+
+nodename = g_strdup_printf("/pci@%lx/interrupt-controller",
+(long) memmap[VIRT_PCIE].base);
+qemu_fdt_add_subnode(fdt, nodename);
+qemu_fdt_setprop_cells(fdt, nodename, "#address-cells", 0x00);
+qemu_fdt_setprop_cells(fdt, nodename, "#interrupt-cells", 0x1);
+qemu_fdt_setprop(fdt, nodename, "interrupt-controller", NULL, 0);
+g_free(nodename);
+
 nodename = g_strdup_printf("/test@%lx",
 (long)memmap[VIRT_TEST].base);
 qemu_fdt_add_subnode(fdt, nodename);
@@ -260,6 +289,31 @@ static void *create_fdt(RISCVVirtState *s, const struct 
MemmapEntry *memmap,
 return fdt;
 }
 
+
+static inline DeviceState *
+gpex_pcie_init(MemoryRegion *sys_mem, uint32_t bus_nr,
+ hwaddr cfg_base, uint64_t cfg_size,
+ hwaddr mmio_base, uint64_t mmio_size,
+ qemu_irq irq, bool link_up)
+{
+DeviceState *dev;
+MemoryRegion *cfg, *mmio;
+
+dev = qdev_create(NULL, TYPE_GPEX_HOST);
+
+qdev_init_nofail(dev);
+
+cfg = sysbus_mmio_get_region(SYS_BUS_DEVICE(dev), 0);
+memory_region_add_subregion_overlap(sys_mem, cfg_base, cfg, 0);
+
+mmio = sysbus_mmio_get_region(SYS_BUS_DEVICE(dev), 1);
+memory_region_add_subregion_overlap(sys_mem, 0, mmio, 0);
+
+sysbus_connect_irq(SYS_BUS_DEVICE(dev), 0, irq);
+
+return dev;
+}
+
 static void riscv_virt_board_init(MachineState *machine)
 {
 const struct MemmapEntry *memmap = virt_memmap;
@@ -382,6 +436,10 @@ static void riscv_virt_board_init(MachineState *machine)
 qdev_get_gpio_in(DEVICE(s->plic), VIRTIO_IRQ + i));
 }
 
+gpex_pcie_init(system_memory, 0, memmap[VIRT_PCIE].base,
+   memmap[VIRT_PCIE].size, 0x4000, 0x2000,
+   qdev_get_gpio_in(DEVICE(s->plic), PCIE_IRQ), true);
+
 serial_mm_init(system_memory, memmap[VIRT_UART0].base,
 0, qdev_get_gpio_in(DEVICE(s->plic), UART0_IRQ), 399193,
 serial_hd(0), DEVIC

[Qemu-devel] [PATCH v5 5/5] hw/riscv/virt: Connect a VirtIO net PCIe device

2018-10-04 Thread Alistair Francis
Signed-off-by: Alistair Francis 
---
 default-configs/riscv32-softmmu.mak |  1 +
 default-configs/riscv64-softmmu.mak |  1 +
 hw/riscv/virt.c | 20 +---
 3 files changed, 19 insertions(+), 3 deletions(-)

diff --git a/default-configs/riscv32-softmmu.mak 
b/default-configs/riscv32-softmmu.mak
index 05fae82f1b..bb3dd34606 100644
--- a/default-configs/riscv32-softmmu.mak
+++ b/default-configs/riscv32-softmmu.mak
@@ -9,6 +9,7 @@ CONFIG_CADENCE=y
 
 CONFIG_PCI_GENERIC=y
 CONFIG_PCI_XILINX=y
+CONFIG_VIRTIO_PCI=y
 
 CONFIG_VGA=y
 CONFIG_VGA_PCI=y
diff --git a/default-configs/riscv64-softmmu.mak 
b/default-configs/riscv64-softmmu.mak
index 05fae82f1b..bb3dd34606 100644
--- a/default-configs/riscv64-softmmu.mak
+++ b/default-configs/riscv64-softmmu.mak
@@ -9,6 +9,7 @@ CONFIG_CADENCE=y
 
 CONFIG_PCI_GENERIC=y
 CONFIG_PCI_XILINX=y
+CONFIG_VIRTIO_PCI=y
 
 CONFIG_VGA=y
 CONFIG_VGA_PCI=y
diff --git a/hw/riscv/virt.c b/hw/riscv/virt.c
index 62a953aa2b..6d91810bcf 100644
--- a/hw/riscv/virt.c
+++ b/hw/riscv/virt.c
@@ -36,6 +36,7 @@
 #include "hw/riscv/sifive_test.h"
 #include "hw/riscv/virt.h"
 #include "chardev/char.h"
+#include "net/net.h"
 #include "sysemu/arch_init.h"
 #include "sysemu/device_tree.h"
 #include "exec/address-spaces.h"
@@ -322,6 +323,8 @@ static void riscv_virt_board_init(MachineState *machine)
 MemoryRegion *system_memory = get_system_memory();
 MemoryRegion *main_mem = g_new(MemoryRegion, 1);
 MemoryRegion *mask_rom = g_new(MemoryRegion, 1);
+DeviceState *dev;
+PCIBus *pci_bus;
 char *plic_hart_config;
 size_t plic_hart_config_len;
 int i;
@@ -436,9 +439,20 @@ static void riscv_virt_board_init(MachineState *machine)
 qdev_get_gpio_in(DEVICE(s->plic), VIRTIO_IRQ + i));
 }
 
-gpex_pcie_init(system_memory, 0, memmap[VIRT_PCIE].base,
-   memmap[VIRT_PCIE].size, 0x4000, 0x2000,
-   qdev_get_gpio_in(DEVICE(s->plic), PCIE_IRQ), true);
+dev = gpex_pcie_init(system_memory, 0, memmap[VIRT_PCIE].base,
+ memmap[VIRT_PCIE].size, 0x4000, 0x2000,
+ qdev_get_gpio_in(DEVICE(s->plic), PCIE_IRQ), true);
+pci_bus = PCI_HOST_BRIDGE(dev)->bus;
+
+for (i = 0; i < nb_nics; i++) {
+NICInfo *nd = &nd_table[i];
+
+if (!nd->model) {
+nd->model = g_strdup("virtio");
+}
+
+pci_nic_init_nofail(nd, pci_bus, nd->model, NULL);
+}
 
 serial_mm_init(system_memory, memmap[VIRT_UART0].base,
 0, qdev_get_gpio_in(DEVICE(s->plic), UART0_IRQ), 399193,
-- 
2.17.1




[Qemu-devel] [PATCH v5 1/5] hw/riscv/virt: Increase the number of interrupts

2018-10-04 Thread Alistair Francis
Increase the number of interrupts to match the HiFive Unleashed board.

Signed-off-by: Alistair Francis 
---
 include/hw/riscv/virt.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/hw/riscv/virt.h b/include/hw/riscv/virt.h
index 91163d6cbf..7cb2742070 100644
--- a/include/hw/riscv/virt.h
+++ b/include/hw/riscv/virt.h
@@ -45,7 +45,7 @@ enum {
 UART0_IRQ = 10,
 VIRTIO_IRQ = 1, /* 1 to 8 */
 VIRTIO_COUNT = 8,
-VIRTIO_NDEV = 10
+VIRTIO_NDEV = 0x35
 };
 
 enum {
-- 
2.17.1




[Qemu-devel] [PATCH v5 0/5] Connect a PCIe host and graphics support to RISC-V

2018-10-04 Thread Alistair Francis
V5:
 - Rebase
 - Include pci.mak in the default configs
V4:
 - Fix the spike device tree
 - Don't use stdvga device
V3:
 - Remove Makefile config changes
 - Connect a network adapter to the virt device
V2:
 - Use the gpex PCIe host for virt
 - Add support for SiFive U PCIe


Alistair Francis (5):
  hw/riscv/virt: Increase the number of interrupts
  hw/riscv/virt: Connect the gpex PCIe
  riscv: Enable VGA and PCIE_VGA
  hw/riscv/sifive_u: Connect the Xilinx PCIe
  hw/riscv/virt: Connect a VirtIO net PCIe device

 default-configs/riscv32-softmmu.mak | 10 +++-
 default-configs/riscv64-softmmu.mak | 10 +++-
 hw/riscv/sifive_u.c | 64 +
 hw/riscv/virt.c | 72 +
 include/hw/riscv/sifive_u.h |  4 +-
 include/hw/riscv/virt.h |  6 ++-
 6 files changed, 161 insertions(+), 5 deletions(-)

-- 
2.17.1




Re: [Qemu-devel] [PATCH v2 15/15] arm/xlnx-zynqmp: put APUs and RPUs in separate GDB groups

2018-10-04 Thread Peter Maydell
On 4 October 2018 at 20:52, Eduardo Habkost  wrote:
> Changing the object hierarchy based on GDB groups doesn't seem
> right, but I don't think it would be a big deal if we have the
> board code explicitly telling the GDB code how to group the CPUs.
>
> If you really want to do it implicitly, would it work if you
> simply group the CPUs based on object_get_canonical_path()?
>
> If a more explicit GDB grouping API is acceptable, what about
> just adding a INTERFACE_GDB_GROUP interface name to (existing)
> container objects that we expect to become GDB groups?
>
> I'm not sure which way is better. I'm a bit worried that making
> things too implicit could easily break (e.g. if somebody changes
> the CPU QOM hierarchy in the future for unrelated reasons).

I don't want things implicit. I just don't want the explicitness
to be "this is all about GDB", because it isn't. I want us
to explicitly say "these 4 CPUs are in one cluster" (or
whatever term we use), because that affects more than merely GDB.

thanks
-- PMM



Re: [Qemu-devel] [PATCH v2 15/15] arm/xlnx-zynqmp: put APUs and RPUs in separate GDB groups

2018-10-04 Thread Eduardo Habkost
On Thu, Oct 04, 2018 at 06:07:45PM +0200, Philippe Mathieu-Daudé wrote:
> On 03/10/2018 13:44, Luc Michel wrote:
> > On 10/2/18 1:58 PM, Peter Maydell wrote:
> >> On 2 October 2018 at 12:33, Philippe Mathieu-Daudé  
> >> wrote:
> >>> Cc'ing more QOM involved people.
> >>>
> >>> On 01/10/2018 13:57, Luc Michel wrote:
>  Create two separate QOM containers for APUs and RPUs to indicate to the
>  GDB stub that those CPUs should be put in different processes.
> 
>  Signed-off-by: Luc Michel 
>  ---
>   hw/arm/xlnx-zynqmp.c | 7 +--
>   1 file changed, 5 insertions(+), 2 deletions(-)
> 
>  diff --git a/hw/arm/xlnx-zynqmp.c b/hw/arm/xlnx-zynqmp.c
>  index c195040350..5e92adbc71 100644
>  --- a/hw/arm/xlnx-zynqmp.c
>  +++ b/hw/arm/xlnx-zynqmp.c
>  @@ -22,10 +22,11 @@
>   #include "hw/arm/xlnx-zynqmp.h"
>   #include "hw/intc/arm_gic_common.h"
>   #include "exec/address-spaces.h"
>   #include "sysemu/kvm.h"
>   #include "kvm_arm.h"
>  +#include "exec/gdbstub.h"
> 
>   #define GIC_NUM_SPI_INTR 160
> 
>   #define ARM_PHYS_TIMER_PPI  30
>   #define ARM_VIRT_TIMER_PPI  27
>  @@ -175,17 +176,18 @@ static void xlnx_zynqmp_create_rpu(XlnxZynqMPState 
>  *s, const char *boot_cpu,
>  Error **errp)
>   {
>   Error *err = NULL;
>   int i;
>   int num_rpus = MIN(smp_cpus - XLNX_ZYNQMP_NUM_APU_CPUS, 
>  XLNX_ZYNQMP_NUM_RPU_CPUS);
>  +Object *rpu_group = gdb_cpu_group_container_get(OBJECT(s));
> >>>
> >>> I'd rather keep this generic: not involve 'gdb' container name.
> >>
> >> Yes, I agree. We should structure how we construct our
> >> model to follow what the hardware has (two CPU clusters
> >> with 4 cores each), and then the gdb code should introspect
> >> the system later to decide how it exposes things to the gdb
> >> user. GDB specifics should (as far as possible) be kept out
> >> of the board code.

Agreed.

> >>
> >> The fact that there are two clusters here also
> >> affects other things, like whether they have the
> >> same view of memory, whether they can share translated
> >> code (they shouldn't but do at the moment), and so on --
> >> it's not just a GDB-relevant distinction. So we should
> >> be modelling it somehow, definitely. I don't have a clear
> >> view how just yet.
> > 
> > So for now, maybe it's better to rely on an heuristic such as the one
> > suggested by Philippe in the gdb code to group the CPUs. Once QEMU gains
> > more supports for such heterogeneous architectures, we can remove the
> > heuristic and put the proper QOM introspection code instead.
> 
> I'm not sure this is the best approach, just suggested because using
> object_resolve_path_type("", TYPE_CPU, NULL) seemed to me the
> quicker/easiest approach.
> 
> Eduardo: Do you have other thoughts on how to resolve those generic
> containers, without involving any gdb-specific tag?

Changing the object hierarchy based on GDB groups doesn't seem
right, but I don't think it would be a big deal if we have the
board code explicitly telling the GDB code how to group the CPUs.

If you really want to do it implicitly, would it work if you
simply group the CPUs based on object_get_canonical_path()?

If a more explicit GDB grouping API is acceptable, what about
just adding a INTERFACE_GDB_GROUP interface name to (existing)
container objects that we expect to become GDB groups?

I'm not sure which way is better. I'm a bit worried that making
things too implicit could easily break (e.g. if somebody changes
the CPU QOM hierarchy in the future for unrelated reasons).


> 
> >> This probably ties into the stuff I have somewhere on
> >> my todo list about supporting multiple heterogenous
> >> systems. The problem with this xilinx board is that it
> >> is trying to model that kind of system but we don't actually
> >> properly support that in QEMU yet.
> >>
> >> thanks
> >> -- PMM
> >>

-- 
Eduardo



[Qemu-devel] [PATCH v3 3/4] softfloat: Specialize udiv_qrnnd for s390x

2018-10-04 Thread Richard Henderson
The ISA has a 128/64-bit division instruction.

Signed-off-by: Richard Henderson 
---
 include/fpu/softfloat-macros.h | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/include/fpu/softfloat-macros.h b/include/fpu/softfloat-macros.h
index 39eb08b4f1..eafc68932b 100644
--- a/include/fpu/softfloat-macros.h
+++ b/include/fpu/softfloat-macros.h
@@ -641,6 +641,12 @@ static inline uint64_t udiv_qrnnd(uint64_t *r, uint64_t n1,
 uint64_t q;
 asm("divq %4" : "=a"(q), "=d"(*r) : "0"(n0), "1"(n1), "rm"(d));
 return q;
+#elif defined(__s390x__)
+/* Need to use a TImode type to get an even register pair for DLGR.  */
+unsigned __int128 n = (unsigned __int128)n1 << 64 | n0;
+asm("dlgr %0, %1" : "+r"(n) : "r"(d));
+*r = n >> 64;
+return n;
 #else
 uint64_t d0, d1, q0, q1, r1, r0, m;
 
-- 
2.17.1




[Qemu-devel] [PATCH v3 0/4] softfloat: Fix division

2018-10-04 Thread Richard Henderson
Changes from v2:
  * Add shift128Left.  I had been using shortShift128Left,
with a shift of 64, which lead to undefined behaviour.
Which I suspect is exactly the Heisenbug Alex saw.

I did keep the R-b tags I had already applied.


r~


Richard Henderson (4):
  softfloat: Fix division
  softfloat: Specialize udiv_qrnnd for x86_64
  softfloat: Specialize udiv_qrnnd for s390x
  softfloat: Specialize udiv_qrnnd for ppc64

 include/fpu/softfloat-macros.h | 62 +-
 fpu/softfloat.c| 35 ++-
 2 files changed, 80 insertions(+), 17 deletions(-)

-- 
2.17.1




[Qemu-devel] [PATCH v3 2/4] softfloat: Specialize udiv_qrnnd for x86_64

2018-10-04 Thread Richard Henderson
The ISA has a 128/64-bit division instruction.

Tested-by: Emilio G. Cota 
Tested-by: Alex Bennée 
Reviewed-by: Alex Bennée 
Signed-off-by: Richard Henderson 
---
 include/fpu/softfloat-macros.h | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/include/fpu/softfloat-macros.h b/include/fpu/softfloat-macros.h
index a1d99c730d..39eb08b4f1 100644
--- a/include/fpu/softfloat-macros.h
+++ b/include/fpu/softfloat-macros.h
@@ -637,6 +637,11 @@ static inline uint64_t estimateDiv128To64(uint64_t a0, 
uint64_t a1, uint64_t b)
 static inline uint64_t udiv_qrnnd(uint64_t *r, uint64_t n1,
   uint64_t n0, uint64_t d)
 {
+#if defined(__x86_64__)
+uint64_t q;
+asm("divq %4" : "=a"(q), "=d"(*r) : "0"(n0), "1"(n1), "rm"(d));
+return q;
+#else
 uint64_t d0, d1, q0, q1, r1, r0, m;
 
 d0 = (uint32_t)d;
@@ -676,6 +681,7 @@ static inline uint64_t udiv_qrnnd(uint64_t *r, uint64_t n1,
 
 *r = r0;
 return (q1 << 32) | q0;
+#endif
 }
 
 /*
-- 
2.17.1




[Qemu-devel] [PATCH v3 4/4] softfloat: Specialize udiv_qrnnd for ppc64

2018-10-04 Thread Richard Henderson
The ISA has a 128/64-bit division instruction, though it assumes the
low 64-bits of the numerator are 0, and so requires a bit more fixup
than a full 128-bit division insn.

Reviewed-by: David Gibson 
Signed-off-by: Richard Henderson 
---
 include/fpu/softfloat-macros.h | 16 
 1 file changed, 16 insertions(+)

diff --git a/include/fpu/softfloat-macros.h b/include/fpu/softfloat-macros.h
index eafc68932b..c86687fa5e 100644
--- a/include/fpu/softfloat-macros.h
+++ b/include/fpu/softfloat-macros.h
@@ -647,6 +647,22 @@ static inline uint64_t udiv_qrnnd(uint64_t *r, uint64_t n1,
 asm("dlgr %0, %1" : "+r"(n) : "r"(d));
 *r = n >> 64;
 return n;
+#elif defined(_ARCH_PPC64)
+/* From Power ISA 3.0B, programming note for divdeu.  */
+uint64_t q1, q2, Q, r1, r2, R;
+asm("divdeu %0,%2,%4; divdu %1,%3,%4"
+: "=&r"(q1), "=r"(q2)
+: "r"(n1), "r"(n0), "r"(d));
+r1 = -(q1 * d); /* low part of (n1<<64) - (q1 * d) */
+r2 = n0 - (q2 * d);
+Q = q1 + q2;
+R = r1 + r2;
+if (R >= d || R < r2) { /* overflow implies R > d */
+Q += 1;
+R -= d;
+}
+*r = R;
+return Q;
 #else
 uint64_t d0, d1, q0, q1, r1, r0, m;
 
-- 
2.17.1




[Qemu-devel] [PATCH v3 1/4] softfloat: Fix division

2018-10-04 Thread Richard Henderson
The __udiv_qrnnd primitive that we nicked from gmp requires its
inputs to be normalized.  We were not doing that.  Because the
inputs are nearly normalized already, finishing that is trivial.

Replace div128to64 with a "proper" udiv_qrnnd, so that this
remains a reusable primitive.

Fixes: cf07323d494
Fixes: https://bugs.launchpad.net/qemu/+bug/1793119
Tested-by: Emilio G. Cota 
Tested-by: Alex Bennée 
Reviewed-by: Alex Bennée 
Signed-off-by: Richard Henderson 
---
 include/fpu/softfloat-macros.h | 34 -
 fpu/softfloat.c| 35 ++
 2 files changed, 52 insertions(+), 17 deletions(-)

diff --git a/include/fpu/softfloat-macros.h b/include/fpu/softfloat-macros.h
index edc682139e..a1d99c730d 100644
--- a/include/fpu/softfloat-macros.h
+++ b/include/fpu/softfloat-macros.h
@@ -329,15 +329,30 @@ static inline void
 | pieces which are stored at the locations pointed to by `z0Ptr' and `z1Ptr'.
 **/
 
-static inline void
- shortShift128Left(
- uint64_t a0, uint64_t a1, int count, uint64_t *z0Ptr, uint64_t *z1Ptr)
+static inline void shortShift128Left(uint64_t a0, uint64_t a1, int count,
+ uint64_t *z0Ptr, uint64_t *z1Ptr)
 {
+*z1Ptr = a1 << count;
+*z0Ptr = count == 0 ? a0 : (a0 << count) | (a1 >> (-count & 63));
+}
 
-*z1Ptr = a1<>( ( - count ) & 63 ) );
+/*
+| Shifts the 128-bit value formed by concatenating `a0' and `a1' left by the
+| number of bits given in `count'.  Any bits shifted off are lost.  The value
+| of `count' may be greater than 64.  The result is broken into two 64-bit
+| pieces which are stored at the locations pointed to by `z0Ptr' and `z1Ptr'.
+**/
 
+static inline void shift128Left(uint64_t a0, uint64_t a1, int count,
+uint64_t *z0Ptr, uint64_t *z1Ptr)
+{
+if (count < 64) {
+*z1Ptr = a1 << count;
+*z0Ptr = count == 0 ? a0 : (a0 << count) | (a1 >> (-count & 63));
+} else {
+*z1Ptr = 0;
+*z0Ptr = a1 << (count - 64);
+}
 }
 
 /*
@@ -619,7 +634,8 @@ static inline uint64_t estimateDiv128To64(uint64_t a0, 
uint64_t a1, uint64_t b)
  *
  * Licensed under the GPLv2/LGPLv3
  */
-static inline uint64_t div128To64(uint64_t n0, uint64_t n1, uint64_t d)
+static inline uint64_t udiv_qrnnd(uint64_t *r, uint64_t n1,
+  uint64_t n0, uint64_t d)
 {
 uint64_t d0, d1, q0, q1, r1, r0, m;
 
@@ -658,8 +674,8 @@ static inline uint64_t div128To64(uint64_t n0, uint64_t n1, 
uint64_t d)
 }
 r0 -= m;
 
-/* Return remainder in LSB */
-return (q1 << 32) | q0 | (r0 != 0);
+*r = r0;
+return (q1 << 32) | q0;
 }
 
 /*
diff --git a/fpu/softfloat.c b/fpu/softfloat.c
index 71da0f68bb..46ae206172 100644
--- a/fpu/softfloat.c
+++ b/fpu/softfloat.c
@@ -1112,19 +1112,38 @@ static FloatParts div_floats(FloatParts a, FloatParts 
b, float_status *s)
 bool sign = a.sign ^ b.sign;
 
 if (a.cls == float_class_normal && b.cls == float_class_normal) {
-uint64_t temp_lo, temp_hi;
+uint64_t n0, n1, q, r;
 int exp = a.exp - b.exp;
+
+/*
+ * We want a 2*N / N-bit division to produce exactly an N-bit
+ * result, so that we do not lose any precision and so that we
+ * do not have to renormalize afterward.  If A.frac < B.frac,
+ * then division would produce an (N-1)-bit result; shift A left
+ * by one to produce the an N-bit result, and decrement the
+ * exponent to match.
+ *
+ * The udiv_qrnnd algorithm that we're using requires normalization,
+ * i.e. the msb of the denominator must be set.  Since we know that
+ * DECOMPOSED_BINARY_POINT is msb-1, the inputs must be shifted left
+ * by one (more), and the remainder must be shifted right by one.
+ */
 if (a.frac < b.frac) {
 exp -= 1;
-shortShift128Left(0, a.frac, DECOMPOSED_BINARY_POINT + 1,
-  &temp_hi, &temp_lo);
+shift128Left(0, a.frac, DECOMPOSED_BINARY_POINT + 2, &n1, &n0);
 } else {
-shortShift128Left(0, a.frac, DECOMPOSED_BINARY_POINT,
-  &temp_hi, &temp_lo);
+shift128Left(0, a.frac, DECOMPOSED_BINARY_POINT + 1, &n1, &n0);
 }
-/* LSB of quot is set if inexact which roundandpack will use
- * to set flags. Yet again we re-use a for the result */
-a.frac = div128To64(temp_lo, temp_hi, b.frac);
+q = udiv_qrnnd(&r, n1, n0, b.frac << 1);
+
+/*
+

[Qemu-devel] [Bug 1795527] Re: Malformed audio and video output stuttering after upgrade to QEMU 3.0

2018-10-04 Thread tlloss
Sorry for the late response.

By switching to 2.12 no noticeable change happens in respect to the 2.11
machine type: distorted audio but correct timing and no overlapping.


By the way, the stuttering looks less consistent than I previously thought: 
today it appears even with the 2.11 setting, but it DOES NOT appear on the 
other Windows VM, even if that machine has less allocated resources and runs 
with the i440fx-3.0 (same audio issues are still there, tho).

Looks like it varies both with guest's and host's loads. Maybe it showed
after the update because of new CPU bug mitigations being introduced in
the kernel (the update to QEMU was carried out during a full system
upgrade; you know, rolling release OSes...), or maybe they are due to
differences in pagefault occurrencies (now I'm running on a freshly-
booted system, last time it was something like 34 days in uptime; and
this is strange on its own accord).

Therefore I think the stuttering problem requires a little bit more
investigation on my side and might be an entirely different problem,
although the nearly perfect sync between stutters and audio overlaps
seems suspicious.

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

Title:
  Malformed audio and video output stuttering after upgrade to QEMU 3.0

Status in QEMU:
  New

Bug description:
  My host is an x86_64 Arch Linux OS with a recompiled 4.18.10 hardened
  kernel, running a few KVM guests with varying OSes and configurations
  managed through a Libvirt stack.

  Among these guests I have two Windows 10 VMs with VGA passthrough and
  PulseAudio-backed virtual audio devices.

  After upgrading to QEMU 3.0.0, both of the Win10 guests started
  showing corrupted audio output in the form of unnatural reproduction
  speed and occasional but consistently misplaced audio fragments
  originating from what seems to be a circular buffer wrapping over
  itself (misbehaviour detected by starting some games with known OSTs
  and dialogues: soundtracks sound accelerated and past dialogue lines
  start replaying middle-sentence until the next line starts playing).

  In addition, the video output of the malfunctioning VMs regularly
  stutters roughly twice a second for a fraction of a second (sync'ed
  with the suspected buffer wrapping and especially pronounced during
  not-pre-rendered cutscenes), toghether with mouse freezes that look
  like actual input misses more than simple lack of screen refreshes.

  
  The issue was succesfully reproduced without the managing stack, directly 
with the following command line, on the most capable Windows guest:

   QEMU_AUDIO_DRV=pa
   QEMU_PA_SERVER=127.0.0.1
   /usr/bin/qemu-system-x86_64 -name guest=win10_gms,debug-threads=on \
   -machine pc-i440fx-3.0,accel=kvm,usb=off,vmport=off,dump-guest-core=off \

   
   -cpu 
host,hv_time,hv_relaxed,hv_vapic,hv_spinlocks=0x1fff,hv_vendor_id=123456789abc,kvm=off
 \  
   -drive 
file=/usr/share/ovmf/x64/OVMF_CODE.fd,if=pflash,format=raw,unit=0,readonly=on \ 
  
   -drive 
file=/var/lib/libvirt/qemu/nvram/win10_gms_VARS.fd,if=pflash,format=raw,unit=1 \
   -m 5120 \
  
   -realtime mlock=off \
   -smp 3,sockets=1,cores=3,threads=1 \
   -uuid 39b56ee2-6bae-4009-9108-7be26d5d63ac \
   -display none \ 
   -no-user-config \
   -nodefaults \
   -rtc base=localtime,driftfix=slew \  

   
   -global kvm-pit.lost_tick_policy=delay \ 
 
   -no-hpet \  
   -no-shutdown \
   -global PIIX4_PM.disable_s3=1 \
   -global PIIX4_PM.disable_s4=1 \
   -boot strict=on \  
   -device ich9-usb-ehci1,id=usb,bus=pci.0,addr=0x4.0x7 \
   -device 
ich9-usb-uhci1,masterbus=usb.0,firstport=0,bus=pci.0,multifunction=on,addr=0x4 \
   -device ich9-usb-uhci2,masterbus=usb.0,firstport=2,bus=pci.0,addr=0x4.0x1 \  
   
   -device ich9-usb-uhci3,masterbus=usb.0,firstport=4,bus=pci.0,addr=0x4.0x2 \
   -device ahci,id=sata0,bus=pci.0,addr=0x9 \ 
   -drive 
file=/dev/vms/win10_gaming,format=raw,if=none,id=drive-virtio-disk0,cache=none,aio=native
 \
   -device 
virtio-blk-pci,scsi=off,bus=pci.0,addr=0x5,drive=drive-virtio-disk0,id=virtio-disk0,bootindex=1,write-cache=on
 \
   -drive 
file=/dev/sr0,format=raw,if=none,id=drive-sata0-0-0,media=cdrom,readonly=on \   
 
   -device ide-cd,bus=sata0.0,drive=drive-sata0-0-0,id=sata0-0-0 \  
   
   -device intel-h

Re: [Qemu-devel] [PATCH v2 12/15] gdbstub: processes initialization on new peer connection

2018-10-04 Thread Alistair Francis
On Mon, Oct 1, 2018 at 5:07 AM Luc Michel  wrote:
>
> 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 

Alistair

> ---
>  gdbstub.c | 20 +++-
>  1 file changed, 15 insertions(+), 5 deletions(-)
>
> diff --git a/gdbstub.c b/gdbstub.c
> index b6de821025..c27a3edf1d 100644
> --- a/gdbstub.c
> +++ b/gdbstub.c
> @@ -2221,13 +2221,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;
> @@ -2309,12 +2310,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;
> @@ -2474,19 +2486,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.0
>
>



Re: [Qemu-devel] [PATCH v1 01/12] net: cadence_gem: Disable TSU feature bit

2018-10-04 Thread Alistair Francis
On Wed, Oct 3, 2018 at 8:11 AM Edgar E. Iglesias
 wrote:
>
> From: "Edgar E. Iglesias" 
>
> Disable the Timestamping Unit feature bit since QEMU does not
> yet support it. This allows guest SW to correctly probe for
> its existance.
>
> Signed-off-by: Edgar E. Iglesias 

Reviewed-by: Alistair Francis 

Alistair

> ---
>  hw/net/cadence_gem.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/hw/net/cadence_gem.c b/hw/net/cadence_gem.c
> index 0fa4b0d..e560b7a 100644
> --- a/hw/net/cadence_gem.c
> +++ b/hw/net/cadence_gem.c
> @@ -1228,7 +1228,7 @@ static void gem_reset(DeviceState *d)
>  s->regs[GEM_MODID] = s->revision;
>  s->regs[GEM_DESCONF] = 0x02500111;
>  s->regs[GEM_DESCONF2] = 0x2ab13fff;
> -s->regs[GEM_DESCONF5] = 0x002f2145;
> +s->regs[GEM_DESCONF5] = 0x002f2045;
>  s->regs[GEM_DESCONF6] = 0x0200;
>
>  /* Set MAC address */
> --
> 2.7.4
>
>



Re: [Qemu-devel] [PATCH v2 05/15] gdbstub: add multiprocess support to 'sC' packets

2018-10-04 Thread Alistair Francis
On Mon, Oct 1, 2018 at 4:57 AM Luc Michel  wrote:
>
> 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 

Alistair

> ---
>  gdbstub.c | 11 ---
>  1 file changed, 8 insertions(+), 3 deletions(-)
>
> diff --git a/gdbstub.c b/gdbstub.c
> index 779cc8b241..3242f0d261 100644
> --- a/gdbstub.c
> +++ b/gdbstub.c
> @@ -1530,13 +1530,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.0
>
>



Re: [Qemu-devel] [PATCH 04/15] hw/ssi/xilinx_spi: Use DeviceState::realize rather than SysBusDevice::init

2018-10-04 Thread Alistair Francis
On Mon, Oct 1, 2018 at 3:32 PM Philippe Mathieu-Daudé  wrote:
>
> Move from the legacy SysBusDevice::init method to using DeviceState::realize.
>
> Signed-off-by: Philippe Mathieu-Daudé 

Reviewed-by: Alistair Francis 

Alistair

> ---
>  hw/ssi/xilinx_spi.c | 9 +++--
>  1 file changed, 3 insertions(+), 6 deletions(-)
>
> diff --git a/hw/ssi/xilinx_spi.c b/hw/ssi/xilinx_spi.c
> index 83585bc8b2..3dae303d5b 100644
> --- a/hw/ssi/xilinx_spi.c
> +++ b/hw/ssi/xilinx_spi.c
> @@ -319,9 +319,9 @@ static const MemoryRegionOps spi_ops = {
>  }
>  };
>
> -static int xilinx_spi_init(SysBusDevice *sbd)
> +static void xilinx_spi_realize(DeviceState *dev, Error **errp)
>  {
> -DeviceState *dev = DEVICE(sbd);
> +SysBusDevice *sbd = SYS_BUS_DEVICE(dev);
>  XilinxSPI *s = XILINX_SPI(dev);
>  int i;
>
> @@ -344,8 +344,6 @@ static int xilinx_spi_init(SysBusDevice *sbd)
>
>  fifo8_create(&s->tx_fifo, FIFO_CAPACITY);
>  fifo8_create(&s->rx_fifo, FIFO_CAPACITY);
> -
> -return 0;
>  }
>
>  static const VMStateDescription vmstate_xilinx_spi = {
> @@ -368,9 +366,8 @@ static Property xilinx_spi_properties[] = {
>  static void xilinx_spi_class_init(ObjectClass *klass, void *data)
>  {
>  DeviceClass *dc = DEVICE_CLASS(klass);
> -SysBusDeviceClass *k = SYS_BUS_DEVICE_CLASS(klass);
>
> -k->init = xilinx_spi_init;
> +dc->realize = xilinx_spi_realize;
>  dc->reset = xlx_spi_reset;
>  dc->props = xilinx_spi_properties;
>  dc->vmsd = &vmstate_xilinx_spi;
> --
> 2.19.0
>
>



Re: [Qemu-devel] [PATCH v5 6/9] hw/misc/zynq_slcr: use standard register definition

2018-10-04 Thread Alistair Francis
On Tue, Oct 2, 2018 at 7:36 AM Damien Hedde  wrote:
>
> Replace the zynq_slcr registers enum and macros using the
> hw/registerfields.h macros.
>
> Signed-off-by: Damien Hedde 
> Reviewed-by: Philippe Mathieu-Daudé 

Reviewed-by: Alistair Francis 

Alistair

> ---
>  hw/misc/zynq_slcr.c | 468 ++--
>  1 file changed, 234 insertions(+), 234 deletions(-)
>
> diff --git a/hw/misc/zynq_slcr.c b/hw/misc/zynq_slcr.c
> index d6bdd027ef..baa13d1316 100644
> --- a/hw/misc/zynq_slcr.c
> +++ b/hw/misc/zynq_slcr.c
> @@ -20,6 +20,7 @@
>  #include "hw/sysbus.h"
>  #include "sysemu/sysemu.h"
>  #include "qemu/log.h"
> +#include "hw/registerfields.h"
>
>  #ifndef ZYNQ_SLCR_ERR_DEBUG
>  #define ZYNQ_SLCR_ERR_DEBUG 0
> @@ -35,138 +36,135 @@
>  #define XILINX_LOCK_KEY 0x767b
>  #define XILINX_UNLOCK_KEY 0xdf0d
>
> -#define R_PSS_RST_CTRL_SOFT_RST 0x1
> +REG32(SCL, 0x000)
> +REG32(LOCK, 0x004)
> +REG32(UNLOCK, 0x008)
> +REG32(LOCKSTA, 0x00c)
>
> -enum {
> -SCL = 0x000 / 4,
> -LOCK,
> -UNLOCK,
> -LOCKSTA,
> +REG32(ARM_PLL_CTRL, 0x100)
> +REG32(DDR_PLL_CTRL, 0x104)
> +REG32(IO_PLL_CTRL, 0x108)
> +REG32(PLL_STATUS, 0x10c)
> +REG32(ARM_PLL_CFG, 0x110)
> +REG32(DDR_PLL_CFG, 0x114)
> +REG32(IO_PLL_CFG, 0x118)
>
> -ARM_PLL_CTRL= 0x100 / 4,
> -DDR_PLL_CTRL,
> -IO_PLL_CTRL,
> -PLL_STATUS,
> -ARM_PLL_CFG,
> -DDR_PLL_CFG,
> -IO_PLL_CFG,
> -
> -ARM_CLK_CTRL= 0x120 / 4,
> -DDR_CLK_CTRL,
> -DCI_CLK_CTRL,
> -APER_CLK_CTRL,
> -USB0_CLK_CTRL,
> -USB1_CLK_CTRL,
> -GEM0_RCLK_CTRL,
> -GEM1_RCLK_CTRL,
> -GEM0_CLK_CTRL,
> -GEM1_CLK_CTRL,
> -SMC_CLK_CTRL,
> -LQSPI_CLK_CTRL,
> -SDIO_CLK_CTRL,
> -UART_CLK_CTRL,
> -SPI_CLK_CTRL,
> -CAN_CLK_CTRL,
> -CAN_MIOCLK_CTRL,
> -DBG_CLK_CTRL,
> -PCAP_CLK_CTRL,
> -TOPSW_CLK_CTRL,
> +REG32(ARM_CLK_CTRL, 0x120)
> +REG32(DDR_CLK_CTRL, 0x124)
> +REG32(DCI_CLK_CTRL, 0x128)
> +REG32(APER_CLK_CTRL, 0x12c)
> +REG32(USB0_CLK_CTRL, 0x130)
> +REG32(USB1_CLK_CTRL, 0x134)
> +REG32(GEM0_RCLK_CTRL, 0x138)
> +REG32(GEM1_RCLK_CTRL, 0x13c)
> +REG32(GEM0_CLK_CTRL, 0x140)
> +REG32(GEM1_CLK_CTRL, 0x144)
> +REG32(SMC_CLK_CTRL, 0x148)
> +REG32(LQSPI_CLK_CTRL, 0x14c)
> +REG32(SDIO_CLK_CTRL, 0x150)
> +REG32(UART_CLK_CTRL, 0x154)
> +REG32(SPI_CLK_CTRL, 0x158)
> +REG32(CAN_CLK_CTRL, 0x15c)
> +REG32(CAN_MIOCLK_CTRL, 0x160)
> +REG32(DBG_CLK_CTRL, 0x164)
> +REG32(PCAP_CLK_CTRL, 0x168)
> +REG32(TOPSW_CLK_CTRL, 0x16c)
>
>  #define FPGA_CTRL_REGS(n, start) \
> -FPGA ## n ## _CLK_CTRL = (start) / 4, \
> -FPGA ## n ## _THR_CTRL, \
> -FPGA ## n ## _THR_CNT, \
> -FPGA ## n ## _THR_STA,
> -FPGA_CTRL_REGS(0, 0x170)
> -FPGA_CTRL_REGS(1, 0x180)
> -FPGA_CTRL_REGS(2, 0x190)
> -FPGA_CTRL_REGS(3, 0x1a0)
> -
> -BANDGAP_TRIP= 0x1b8 / 4,
> -PLL_PREDIVISOR  = 0x1c0 / 4,
> -CLK_621_TRUE,
> -
> -PSS_RST_CTRL= 0x200 / 4,
> -DDR_RST_CTRL,
> -TOPSW_RESET_CTRL,
> -DMAC_RST_CTRL,
> -USB_RST_CTRL,
> -GEM_RST_CTRL,
> -SDIO_RST_CTRL,
> -SPI_RST_CTRL,
> -CAN_RST_CTRL,
> -I2C_RST_CTRL,
> -UART_RST_CTRL,
> -GPIO_RST_CTRL,
> -LQSPI_RST_CTRL,
> -SMC_RST_CTRL,
> -OCM_RST_CTRL,
> -FPGA_RST_CTRL   = 0x240 / 4,
> -A9_CPU_RST_CTRL,
> -
> -RS_AWDT_CTRL= 0x24c / 4,
> -RST_REASON,
> -
> -REBOOT_STATUS   = 0x258 / 4,
> -BOOT_MODE,
> -
> -APU_CTRL= 0x300 / 4,
> -WDT_CLK_SEL,
> -
> -TZ_DMA_NS   = 0x440 / 4,
> -TZ_DMA_IRQ_NS,
> -TZ_DMA_PERIPH_NS,
> -
> -PSS_IDCODE  = 0x530 / 4,
> -
> -DDR_URGENT  = 0x600 / 4,
> -DDR_CAL_START   = 0x60c / 4,
> -DDR_REF_START   = 0x614 / 4,
> -DDR_CMD_STA,
> -DDR_URGENT_SEL,
> -DDR_DFI_STATUS,
> -
> -MIO = 0x700 / 4,
> +REG32(FPGA ## n ## _CLK_CTRL, (start)) \
> +REG32(FPGA ## n ## _THR_CTRL, (start) + 0x4)\
> +REG32(FPGA ## n ## _THR_CNT,  (start) + 0x8)\
> +REG32(FPGA ## n ## _THR_STA,  (start) + 0xc)
> +FPGA_CTRL_REGS(0, 0x170)
> +FPGA_CTRL_REGS(1, 0x180)
> +FPGA_CTRL_REGS(2, 0x190)
> +FPGA_CTRL_REGS(3, 0x1a0)
> +
> +REG32(BANDGAP_TRIP, 0x1b8)
> +REG32(PLL_PREDIVISOR, 0x1c0)
> +REG32(CLK_621_TRUE, 0x1c4)
> +
> +REG32(PSS_RST_CTRL, 0x200)
> +FIELD(PSS_RST_CTRL, SOFT_RST, 0, 1)
> +REG32(DDR_RST_CTRL, 0x204)
> +REG32(TOPSW_RESET_CTRL, 0x208)
> +REG32(DMAC_RST_CTRL, 0x20c)
> +REG32(USB_RST_CTRL, 0x210)
> +REG32(GEM_RST_CTRL, 0x214)
> +REG32(SDIO_RST_CTRL, 0x218)
> +REG32(SPI_RST_CTRL, 0x21c)
> +REG32(CAN_RST_CTRL, 0x220)
> +REG32(I2C_RST_CTRL, 0x224)
> +REG32(UART_RST_CTRL, 0x228)
> +REG32(GPIO_RST_CTRL, 0x22c)
> +REG32(LQSPI_RST_CTRL, 0x230)
> +REG32(SMC_RST_CTRL, 0x234)
> +REG32(OCM_RST_CTRL, 0x238)
> +REG32(FPGA_RST_CTRL, 0x240)
> +REG32(A9_CPU_RST_CTRL, 0x244)
> +
> +REG32(RS_AWDT_CTRL, 0x24c)
> +REG32(RST_REASON, 0x250)
> +
> +REG32(REBOOT_STATUS, 0x258)
> +REG32(BOOT_MODE, 0x25c)
> +
> +REG32(APU_CTRL, 0x300)
> +REG32(WDT_CLK_SEL, 0

Re: [Qemu-devel] ACPI PCI hotplug table updates

2018-10-04 Thread Michael S. Tsirkin
On Thu, Oct 04, 2018 at 01:57:21PM +0200, Igor Mammedov wrote:
> On Wed, 3 Oct 2018 10:44:20 -0700
> open sorcerer <0p3n.s0rc3...@gmail.com> wrote:
> 
> > Hi,
> > 
> > I am digging into an issue where qmp_device_del does not actually delete
> > devices when a guest OS is in prelaunch. This seems to be due to the guest
> > OS not handling ACPI events because it is not currently running. If I
> > assume correctly, qmp should allow you to add/remove devices while the host
> > is down, or if not possible, publish an error message.
> may I ask why one would delete a device at -S pause point, isn't it easier
> to start QEMU without it, to begin with?
> 
> > I think fixing this issue is as simple as making sure that the VM is in a
> > safe state to ignore the hotplug ACPI dance but eject the disk, something
> > like:
> in prelaunch runstate where '-S' option pauses VM, it is practically paused
> at the first instruction to be executed. So device_add at that point is
> considered as hotplug with all actions already executed on hardware level
> (interrupts sent, devices responsible for hotplug handling has changed state).
> So if one wished to delete device at that point, one would have to rollback
> related state changes.
> If one would additionally use -incoming CLI option, it becomes more 
> complicated
> as we might endup in prelaunch runstate with VM in running state
> (see possible transitions in runstate_transitions_def[])
> I'd say prelauch runstate can't be used for removing devices that do not
> support surprise removal (in our case PCI isn't).

I'd say the point is this. In prelaunch guest did not observe any
device state yet, we could make device_add look just like
a non-hotplugged device. And we could make device_del pretend
there was a reset immediately afterwards.

Not sure why it matters to anyone, but it's doable I think.



> > prelaunch, preconfig, shutdown: ignore acpi and deal with cleaning devices
> > other non-running: bubble up error
> > running: default behavior
> > 
> > I was trying to validate that this change would be safe (keep in mind I am
> > learning ACPI in little pieces while digging) using GDB, and code
> > inspection. While stepping through with GDB i noticed that the PCI slots
> > are controlled by memory region and the opaque acpi pci hp state object. I
> > was unable this far to find any code executed that modifies the ACPI tables
> > beyond just the pci hotplug state.
> > 
> > I also tried to test using "while true; do acpidump | md5; sleep 1; done"
> > in the guest OS and then add/remove a virtio-blk-pci device (which
> > exercised the ACPI callbacks via piix4 callbacks). The output of the
> > acpidump -> md5 was consistent during each phase of the data collection
> > which I believe implied that the acpi tables were not modified by the PCI
> > hotplug.
> > 
> > Can someone help me understand:
> > 
> > 1. Are the ACPI tables not modified when doing PCI hotplug?
> > 2. Do the general changes proposed seem safe?
> > 3. Are there resources or documentation I can read to help me understand
> > this problem further? I have skimmed through alot of different documents
> > and watched some youtube videos, but the ACPI documentation is hard to read
> > and sift through and the youtube videos are generally too high level.
> Regarding ACPI based PCI hotplug you can look at
>   docs/specs/acpi_pci_hotplug.txt
>   hw/acpi/pcihp.c
>   ACPI AML part in build_append_pci_bus_devices()
> 
> > 
> > Thanks.



Re: [Qemu-devel] [PATCH v2 0/3] Linux usermode emulation user mode USB driver support.

2018-10-04 Thread Laurent Vivier
Le 25/09/2018 à 09:12, Cortland Tölva a écrit :
> This patch series aims to let programs running under QEMU Linux user mode
> emulation implement user-space USB drivers via the USBFS ioctl()s.  First
> I check for the necessary header files, then I define some types, and
> last I implement the submit, discard, and reap functions, which involve
> USB request buffers which live beyond a single ioctl() call.
> 
> Each patch in this series compiles for i386, ppc64, ppc64le, mips, mipsel,
> xtensa, and xtensaeb with an armv7l host and an x86_64 host.  The
> i386-linux-user target is tested working with a USB scanner driver on an
> armv7l host.  Additionally, a patched copy of strace was used to verify
> the conversion for reaping.  Additionally, a MIPS binary of lsusb was run
> on armv7l host to check reaping and other functionality across endianness.
> 
> Not tested on 64-bit hosts.
> 
> Changes from v1:
>   use check_include in configure
>   move struct definitions to later patch where possible
>   improve pointer cast to int compatibility
>   remove unimplemented types for usb streams
> 
> Cortland Tölva (3):
>   linux-user: Check for Linux USBFS in configure
>   linux-user: Define ordinary usbfs ioctls.
>   linux-user: implement special usbfs ioctls.
> 
>  configure  |  12 +++-
>  linux-user/ioctls.h|  46 
>  linux-user/syscall.c   | 171 
> +
>  linux-user/syscall_defs.h  |  30 
>  linux-user/syscall_types.h |  68 ++
>  5 files changed, 326 insertions(+), 1 deletion(-)
> 

I've applied patches 1 and 2 to my linux-user branch.

Thanks,
Laurent



Re: [Qemu-devel] [PATCH] linux-user: Suppress address-of-packed-member warnings in __get/put_user_e

2018-10-04 Thread Laurent Vivier
Le 28/09/2018 à 16:25, Peter Maydell a écrit :
> Our __get_user_e() and __put_user_e() macros cause newer versions
> of clang to generate false-positive -Waddress-of-packed-member
> warnings if they are passed the address of a member of a packed
> struct (see https://bugs.llvm.org/show_bug.cgi?id=39113).
> Suppress these using the _Pragma() operator.
> 
> To put in the pragmas we need to convert the macros from
> expressions to statements, but all the callsites effectively
> treat them as statements already so this is OK.
> 
> Signed-off-by: Peter Maydell 
> ---
>  linux-user/qemu.h | 57 +++
>  1 file changed, 38 insertions(+), 19 deletions(-)
> 

Applied to my linux-user branch.

Thanks,
Laurent




Re: [Qemu-devel] [PATCH 09/10] scripts/qemu.py: use a more consistent docstring style

2018-10-04 Thread Cleber Rosa



On 10/4/18 12:50 PM, Eric Blake wrote:
> On 10/4/18 11:18 AM, Cleber Rosa wrote:
>> Signed-off-by: Cleber Rosa 
>> ---
>>   dtc |  2 +-
>>   scripts/qemu.py | 65 +++--
>>   2 files changed, 42 insertions(+), 25 deletions(-)
>>
>> diff --git a/dtc b/dtc
>> index 88f18909db..e54388015a 16
>> --- a/dtc
>> +++ b/dtc
>> @@ -1 +1 @@
>> -Subproject commit 88f18909db731a627456f26d779445f84e449536
>> +Subproject commit e54388015af1fb4bf04d0bca99caba1074d9cc42
> 
> The submodule change was probably unintended.
> 

Yep, my bad.



Re: [Qemu-devel] [PATCH 01/10] qemu-img.c: comment typo fix

2018-10-04 Thread Cleber Rosa



On 10/4/18 12:48 PM, Eric Blake wrote:
> On 10/4/18 11:18 AM, Cleber Rosa wrote:
>> A trivial comment typo fix.
>>
>> Signed-off-by: Cleber Rosa 
>> ---
>>   qemu-img.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/qemu-img.c b/qemu-img.c
>> index b12f4cd19b..a96b76c187 100644
>> --- a/qemu-img.c
>> +++ b/qemu-img.c
>> @@ -1085,7 +1085,7 @@ static int64_t find_nonzero(const uint8_t *buf,
>> int64_t n)
>>   }
>>     /*
>> - * Returns true iff the first sector pointed to by 'buf' contains at
>> least
>> + * Returns true if the first sector pointed to by 'buf' contains at
>> least
> 
> NACK.  You're not the first person to propose this change.  However,
> "iff" is an English word (albeit archaic) which is shorthand for "if and
> only if", which has a distinct logical meaning separate from the weaker
> "if".  Spelling it out in longhand instead of calling it a typo is
> probably acceptable, though.
> 

Wow, thanks for the explanation.  I agree that spelling it out is a good
option, as it'll probably prevent other people from falling into the
same "language trap" as I did.

Thanks!
- Cleber.



Re: [Qemu-devel] [PATCH 03/10] qemu-iotests: make 218 executable

2018-10-04 Thread Eric Blake

On 10/4/18 11:18 AM, Cleber Rosa wrote:

Commit 990dc39c made all tests executable at the time, but 218 came in
later, and missing those permissions.

Signed-off-by: Cleber Rosa 
---
  tests/qemu-iotests/218 | 0
  1 file changed, 0 insertions(+), 0 deletions(-)
  mode change 100644 => 100755 tests/qemu-iotests/218

diff --git a/tests/qemu-iotests/218 b/tests/qemu-iotests/218
old mode 100644
new mode 100755


Reviewed-by: Eric Blake 

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



Re: [Qemu-devel] [PATCH 09/10] scripts/qemu.py: use a more consistent docstring style

2018-10-04 Thread Eric Blake

On 10/4/18 11:18 AM, Cleber Rosa wrote:

Signed-off-by: Cleber Rosa 
---
  dtc |  2 +-
  scripts/qemu.py | 65 +++--
  2 files changed, 42 insertions(+), 25 deletions(-)

diff --git a/dtc b/dtc
index 88f18909db..e54388015a 16
--- a/dtc
+++ b/dtc
@@ -1 +1 @@
-Subproject commit 88f18909db731a627456f26d779445f84e449536
+Subproject commit e54388015af1fb4bf04d0bca99caba1074d9cc42


The submodule change was probably unintended.

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



Re: [Qemu-devel] [PATCH 01/10] qemu-img.c: comment typo fix

2018-10-04 Thread Eric Blake

On 10/4/18 11:18 AM, Cleber Rosa wrote:

A trivial comment typo fix.

Signed-off-by: Cleber Rosa 
---
  qemu-img.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/qemu-img.c b/qemu-img.c
index b12f4cd19b..a96b76c187 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -1085,7 +1085,7 @@ static int64_t find_nonzero(const uint8_t *buf, int64_t n)
  }
  
  /*

- * Returns true iff the first sector pointed to by 'buf' contains at least
+ * Returns true if the first sector pointed to by 'buf' contains at least


NACK.  You're not the first person to propose this change.  However, 
"iff" is an English word (albeit archaic) which is shorthand for "if and 
only if", which has a distinct logical meaning separate from the weaker 
"if".  Spelling it out in longhand instead of calling it a typo is 
probably acceptable, though.



   * a non-NUL byte.
   *
   * 'pnum' is set to the number of sectors (including and immediately following



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



Re: [Qemu-devel] [PATCH 6/7] Acceptance Tests: add variants definition for architectures

2018-10-04 Thread Laszlo Ersek
On 10/04/18 17:14, Cleber Rosa wrote:
> One of the Avocado features relevant to virtualization testing is the
> ability to reuse tests in different scenarios, known as variants.
> This adds a JSON based variants file, that can be used to run most
> tests in a number of different architectures.  It can be run with:
> 
>$ avocado run \
>  --json-variants-load=tests/acceptance/variants/arch.json \
>  --filter-by-tags='-x86_64' -- tests/acceptance/
> 
> Currently this covers 5 architectures, resulting in the execution
> of 25 different combinations.
> 
> Signed-off-by: Cleber Rosa 
> ---
>  tests/acceptance/variants/arch.json | 1 +
>  1 file changed, 1 insertion(+)
>  create mode 100644 tests/acceptance/variants/arch.json
> 
> diff --git a/tests/acceptance/variants/arch.json 
> b/tests/acceptance/variants/arch.json
> new file mode 100644
> index 00..a7a2570553
> --- /dev/null
> +++ b/tests/acceptance/variants/arch.json
> @@ -0,0 +1 @@
> +[{"paths":["/run/*"],"variant":[["/run/aarch64",[["/run/aarch64", "arch", 
> "aarch64","variant_id": 
> "aarch64"},{"paths":["/run/*"],"variant":[["/run/ppc",[["/run/ppc", "arch", 
> "ppc","variant_id": 
> "ppc"},{"paths":["/run/*"],"variant":[["/run/ppc64",[["/run/ppc64", "arch", 
> "ppc64","variant_id": 
> "ppc64"},{"paths":["/run/*"],"variant":[["/run/s390x",[["/run/s390x", "arch", 
> "s390x","variant_id": 
> "s390x"},{"paths":["/run/*"],"variant":[["/run/x86_64",[["/run/x86_64", 
> "arch", "x86_64","variant_id": "x86_64"}]
> 

(Side comment: you can parse json? :) That's awesome. Then I *am*
tempted to suggest that Philippe's work parse the firmware metadata
format, in the long run.)

Laszlo



Re: [Qemu-devel] [PATCH 06/10] thunk.c: clean up commented out definition

2018-10-04 Thread Laurent Vivier
Le 04/10/2018 à 18:31, Cleber Rosa a écrit :
> 
> 
> On 10/4/18 12:24 PM, Laurent Vivier wrote:
>> Le 04/10/2018 à 18:18, Cleber Rosa a écrit :
>>> Signed-off-by: Cleber Rosa 
>>> ---
>>>  thunk.c | 2 --
>>>  1 file changed, 2 deletions(-)
>>>
>>> diff --git a/thunk.c b/thunk.c
>>> index d5d8645cd4..e351ae53af 100644
>>> --- a/thunk.c
>>> +++ b/thunk.c
>>> @@ -21,8 +21,6 @@
>>>  #include "qemu.h"
>>>  #include "exec/user/thunk.h"
>>>  
>>> -//#define DEBUG
>>> -
>>
>> Generally we use that to enable easily existing debug traces.
>>
> 
> Oh, I wasn't aware that was intentional (I clearly lack some background
> here).
> 
>> In this case, if you remove the "#define" I think you should also
>> replace the "#ifdef DEBUG"s by some trace_() functions.
>>
> 
> Is that desirable? I don't want to disrupt the status quo just because
> it looked odd to me.

Yes, it's better to have trace_XXX() functions than DEBUG macros. The
change is trivial (add the format strings in ./trace-events and call
them from thunk.c with the existing parameters)

Thanks,
Laurent






Re: [Qemu-devel] [PATCH 06/10] thunk.c: clean up commented out definition

2018-10-04 Thread Cleber Rosa



On 10/4/18 12:24 PM, Laurent Vivier wrote:
> Le 04/10/2018 à 18:18, Cleber Rosa a écrit :
>> Signed-off-by: Cleber Rosa 
>> ---
>>  thunk.c | 2 --
>>  1 file changed, 2 deletions(-)
>>
>> diff --git a/thunk.c b/thunk.c
>> index d5d8645cd4..e351ae53af 100644
>> --- a/thunk.c
>> +++ b/thunk.c
>> @@ -21,8 +21,6 @@
>>  #include "qemu.h"
>>  #include "exec/user/thunk.h"
>>  
>> -//#define DEBUG
>> -
> 
> Generally we use that to enable easily existing debug traces.
> 

Oh, I wasn't aware that was intentional (I clearly lack some background
here).

> In this case, if you remove the "#define" I think you should also
> replace the "#ifdef DEBUG"s by some trace_() functions.
> 

Is that desirable? I don't want to disrupt the status quo just because
it looked odd to me.

Thanks!
- Cleber.

> Thanks,
> Laurent
> 



[Qemu-devel] [PATCH 08/10] scripts/decodetree.py: fix reference to attributes

2018-10-04 Thread Cleber Rosa
Signed-off-by: Cleber Rosa 
---
 scripts/decodetree.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/scripts/decodetree.py b/scripts/decodetree.py
index 457cffea90..37c76b5507 100755
--- a/scripts/decodetree.py
+++ b/scripts/decodetree.py
@@ -298,7 +298,7 @@ class Field:
 s = 's'
 else:
 s = ''
-return str(pos) + ':' + s + str(len)
+return str(self.pos) + ':' + s + str(self.len)
 
 def str_extract(self):
 if self.sign:
-- 
2.17.1




[Qemu-devel] [PATCH 10/10] scripts/qemu.py: remove trailing quotes on docstring

2018-10-04 Thread Cleber Rosa
Signed-off-by: Cleber Rosa 
---
 scripts/qemu.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/scripts/qemu.py b/scripts/qemu.py
index 7abe26de69..676eb9709a 100644
--- a/scripts/qemu.py
+++ b/scripts/qemu.py
@@ -88,7 +88,7 @@ class QEMUMachine(object):
 @param name: prefix for socket and log file names (default: qemu-PID)
 @param test_dir: where to create socket and log file
 @param monitor_address: address for QMP monitor
-@param socket_scm_helper: helper program, required for send_fd_scm()"
+@param socket_scm_helper: helper program, required for send_fd_scm()
 @note: Qemu process is not started until launch() is used.
 '''
 if args is None:
-- 
2.17.1




[Qemu-devel] [PATCH 04/10] qemu-iotests: fix filename containing checks

2018-10-04 Thread Cleber Rosa
Commit cce293a2945 moved some functions from common.config to
common.rc, but the error messages still reference the old file
location.

Signed-off-by: Cleber Rosa 
---
 tests/qemu-iotests/common.rc | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tests/qemu-iotests/common.rc b/tests/qemu-iotests/common.rc
index 44bee16a5e..70ca65b49b 100644
--- a/tests/qemu-iotests/common.rc
+++ b/tests/qemu-iotests/common.rc
@@ -170,7 +170,7 @@ if [ ! -e "$TEST_DIR" ]; then
 fi
 
 if [ ! -d "$TEST_DIR" ]; then
-echo "common.config: Error: \$TEST_DIR ($TEST_DIR) is not a directory"
+echo "common.rc: Error: \$TEST_DIR ($TEST_DIR) is not a directory"
 exit 1
 fi
 
@@ -179,7 +179,7 @@ if [ -z "$REMOTE_TEST_DIR" ]; then
 fi
 
 if [ ! -d "$SAMPLE_IMG_DIR" ]; then
-echo "common.config: Error: \$SAMPLE_IMG_DIR ($SAMPLE_IMG_DIR) is not a 
directory"
+echo "common.rc: Error: \$SAMPLE_IMG_DIR ($SAMPLE_IMG_DIR) is not a 
directory"
 exit 1
 fi
 
-- 
2.17.1




Re: [Qemu-devel] [PATCH 06/10] thunk.c: clean up commented out definition

2018-10-04 Thread Laurent Vivier
Le 04/10/2018 à 18:18, Cleber Rosa a écrit :
> Signed-off-by: Cleber Rosa 
> ---
>  thunk.c | 2 --
>  1 file changed, 2 deletions(-)
> 
> diff --git a/thunk.c b/thunk.c
> index d5d8645cd4..e351ae53af 100644
> --- a/thunk.c
> +++ b/thunk.c
> @@ -21,8 +21,6 @@
>  #include "qemu.h"
>  #include "exec/user/thunk.h"
>  
> -//#define DEBUG
> -

Generally we use that to enable easily existing debug traces.

In this case, if you remove the "#define" I think you should also
replace the "#ifdef DEBUG"s by some trace_() functions.

Thanks,
Laurent




[Qemu-devel] [PATCH 09/10] scripts/qemu.py: use a more consistent docstring style

2018-10-04 Thread Cleber Rosa
Signed-off-by: Cleber Rosa 
---
 dtc |  2 +-
 scripts/qemu.py | 65 +++--
 2 files changed, 42 insertions(+), 25 deletions(-)

diff --git a/dtc b/dtc
index 88f18909db..e54388015a 16
--- a/dtc
+++ b/dtc
@@ -1 +1 @@
-Subproject commit 88f18909db731a627456f26d779445f84e449536
+Subproject commit e54388015af1fb4bf04d0bca99caba1074d9cc42
diff --git a/scripts/qemu.py b/scripts/qemu.py
index f099ce7278..7abe26de69 100644
--- a/scripts/qemu.py
+++ b/scripts/qemu.py
@@ -53,9 +53,9 @@ class QEMUMachineAddDeviceError(QEMUMachineError):
 """
 
 class MonitorResponseError(qmp.qmp.QMPError):
-'''
+"""
 Represents erroneous QMP monitor reply
-'''
+"""
 def __init__(self, reply):
 try:
 desc = reply["error"]["desc"]
@@ -66,14 +66,15 @@ class MonitorResponseError(qmp.qmp.QMPError):
 
 
 class QEMUMachine(object):
-'''A QEMU VM
+"""
+A QEMU VM
 
 Use this object as a context manager to ensure the QEMU process 
terminates::
 
 with VM(binary) as vm:
 ...
 # vm is guaranteed to be shut down here
-'''
+"""
 
 def __init__(self, binary, args=None, wrapper=None, name=None,
  test_dir="/var/tmp", monitor_address=None,
@@ -135,7 +136,9 @@ class QEMUMachine(object):
 self._args.append(args)
 
 def add_fd(self, fd, fdset, opaque, opts=''):
-'''Pass a file descriptor to the VM'''
+"""
+Pass a file descriptor to the VM
+"""
 options = ['fd=%d' % fd,
'set=%d' % fdset,
'opaque=%s' % opaque]
@@ -168,7 +171,9 @@ class QEMUMachine(object):
 
 @staticmethod
 def _remove_if_exists(path):
-'''Remove file object at path if it exists'''
+"""
+Remove file object at path if it exists
+"""
 try:
 os.remove(path)
 except OSError as exception:
@@ -271,7 +276,9 @@ class QEMUMachine(object):
 raise
 
 def _launch(self):
-'''Launch the VM and establish a QMP connection'''
+"""
+Launch the VM and establish a QMP connection
+"""
 devnull = open(os.path.devnull, 'rb')
 self._pre_launch()
 self._qemu_full_args = (self._wrapper + [self._binary] +
@@ -284,14 +291,18 @@ class QEMUMachine(object):
 self._post_launch()
 
 def wait(self):
-'''Wait for the VM to power off'''
+"""
+Wait for the VM to power off
+"""
 self._popen.wait()
 self._qmp.close()
 self._load_io_log()
 self._post_shutdown()
 
 def shutdown(self):
-'''Terminate the VM and clean up'''
+"""
+Terminate the VM and clean up
+"""
 if self.is_running():
 try:
 self._qmp.cmd('quit')
@@ -315,7 +326,9 @@ class QEMUMachine(object):
 self._launched = False
 
 def qmp(self, cmd, conv_keys=True, **args):
-'''Invoke a QMP command and return the response dict'''
+"""
+Invoke a QMP command and return the response dict
+"""
 qmp_args = dict()
 for key, value in args.items():
 if conv_keys:
@@ -326,11 +339,11 @@ class QEMUMachine(object):
 return self._qmp.cmd(cmd, args=qmp_args)
 
 def command(self, cmd, conv_keys=True, **args):
-'''
+"""
 Invoke a QMP command.
 On success return the response dict.
 On failure raise an exception.
-'''
+"""
 reply = self.qmp(cmd, conv_keys, **args)
 if reply is None:
 raise qmp.qmp.QMPError("Monitor is closed")
@@ -339,13 +352,17 @@ class QEMUMachine(object):
 return reply["return"]
 
 def get_qmp_event(self, wait=False):
-'''Poll for one queued QMP events and return it'''
+"""
+Poll for one queued QMP events and return it
+"""
 if len(self._events) > 0:
 return self._events.pop(0)
 return self._qmp.pull_event(wait=wait)
 
 def get_qmp_events(self, wait=False):
-'''Poll for queued QMP events and return a list of dicts'''
+"""
+Poll for queued QMP events and return a list of dicts
+"""
 events = self._qmp.get_events(wait=wait)
 events.extend(self._events)
 del self._events[:]
@@ -353,7 +370,7 @@ class QEMUMachine(object):
 return events
 
 def event_wait(self, name, timeout=60.0, match=None):
-'''
+"""
 Wait for specified timeout on named event in QMP; optionally filter
 results by match.
 
@@ -361,7 +378,7 @@ class QEMUMachine(object):
 branch processing on match's value None
{"foo": {"bar": 1}} matches {"foo": None}
{"foo": {"bar": 1}} does not matches {"foo": {"baz": None}}
-'''
+"""
 def event_match(event, match=None):
 if 

[Qemu-devel] [PATCH 05/10] docs/devel/testing.rst: add missing newlines after code block

2018-10-04 Thread Cleber Rosa
The line immediate following a ".. code::" block is considered
to contains arguments to the "code directive".  The lack of a
new line gives me during at parse time:

   testing.rst:63: (ERROR/3) Error in "code" directive:
   maximum 1 argument(s) allowed, 3 supplied.

   .. code::
 make check-unit V=1

   testing.rst:120: (ERROR/3) Error in "code" directive:
   maximum 1 argument(s) allowed, 3 supplied.

   .. code::
 make check-qtest V=1

Let's add the missing newlines, both for consistency and to
avoid the parsing errors.

Signed-off-by: Cleber Rosa 
---
 docs/devel/testing.rst | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/docs/devel/testing.rst b/docs/devel/testing.rst
index 727c4019b5..9355ad49f2 100644
--- a/docs/devel/testing.rst
+++ b/docs/devel/testing.rst
@@ -61,6 +61,7 @@ variable (which affects memory reclamation and catches 
invalid pointers better)
 and gtester options. If necessary, you can run
 
 .. code::
+
   make check-unit V=1
 
 and copy the actual command line which executes the unit test, then run
@@ -118,6 +119,7 @@ and using gdb on the test is still simple to do: find out 
the actual command
 from the output of
 
 .. code::
+
   make check-qtest V=1
 
 which you can run manually.
-- 
2.17.1




Re: [Qemu-devel] [RFC PATCH v2 3/3] acceptance tests: Add EDK2 ArmVirtQemu boot and console checking test

2018-10-04 Thread Laszlo Ersek
On 10/04/18 17:15, Philippe Mathieu-Daudé wrote:
> On 04/10/2018 17:07, Laszlo Ersek wrote:
>> On 10/03/18 20:30, Philippe Mathieu-Daudé wrote:
>>> This test boots EDK2 ArmVirtQemu and check the debug console (PL011) 
>>> reports enough
>>> information on the initialized devices.
>>>
>>> $ avocado run -p qemu_bin=aarch64-softmmu/qemu-system-aarch64 
>>> tests/acceptance/boot_firmware.py
>>> JOB ID : cb1c5bd9e0312483eabeffbb37885a5273ef23bf
>>> JOB LOG: 
>>> /home/phil/avocado/job-results/job-2018-10-03T19.39-cb1c5bd/job.log
>>>  (1/1) tests/acceptance/boot_firmware.py:BootFirmware.test_ovmf_virt: PASS 
>>> (5.02 s)
>>> RESULTS: PASS 1 | ERROR 0 | FAIL 0 | SKIP 0 | WARN 0 | INTERRUPT 0 | 
>>> CANCEL 0
>>> JOB TIME   : 5.30 s
>>>
>>> Signed-off-by: Philippe Mathieu-Daudé 
>>> ---
>>> The '-p' option requires Avocado >= 65.0
>>> ---
>>>  tests/acceptance/boot_firmware.py | 36 +++
>>>  1 file changed, 36 insertions(+)
>>>
>>> diff --git a/tests/acceptance/boot_firmware.py 
>>> b/tests/acceptance/boot_firmware.py
>>> index 2053b1a4b6..42f0672963 100644
>>> --- a/tests/acceptance/boot_firmware.py
>>> +++ b/tests/acceptance/boot_firmware.py
>>> @@ -121,3 +121,39 @@ class BootFirmware(Test):
>>>  debugcon_logger.debug(line.strip())
>>>  for exp in expected:
>>>  self.assertIn(exp + '\r\n', content)
>>> +
>>> +def test_ovmf_virt(self):
>>> +"""
>>> +Boots OVMF on the default virt machine, checks the debug console
>>> +
>>> +:avocado: tags=arch:aarch64
>>> +:avocado: tags=maxtime:20s
>>> +"""
>>> +image_url = ('http://snapshots.linaro.org/components/kernel/'
>>> +'leg-virt-tianocore-edk2-upstream/latest/'
>>> +'QEMU-AARCH64/DEBUG_GCC5/QEMU_EFI.img.gz')
>>> +image_path_gz = self.fetch_asset(image_url)
>>> +image_path = os.path.join(self.workdir, 'flash.img')
>>> +
>>> +# kludge until Avocado support gzip files
>>> +import gzip, shutil
>>> +with gzip.open(image_path_gz) as gz, open(image_path, 'wb') as img:
>>> +shutil.copyfileobj(gz, img)
>>> +
>>> +serial_path = os.path.join(self.workdir, 'serial.log')
>>> +self.vm.set_machine('virt')
>>> +self.vm.add_args('-nographic',
>>> + '-cpu', 'cortex-a57',
>>> + '-m', '1G', # 1GB min to boot fw?
>>> + '-drive', 'file=%s,format=raw,if=pflash' % 
>>> image_path,
>>> + '-chardev', 'file,path=%s,id=console' % 
>>> serial_path,
>>> + '-serial', 'chardev:console')
>>> +self.vm.launch()
>>> +serial_logger = logging.getLogger('serial')
>>> +
>>> +# serial console checks
>>> +if not wait_for(read_console_for_string, timeout=15, step=0,
>>> +args=(open(serial_path),
>>> +  'Start PXE over IPv4',
>>> +  serial_logger)):
>>> +self.fail("OVMF failed to boot")
>>>
>>
>> I suggest replacing "OVMF" with "ArmVirtQemu" in comments and strings as
>> well.
> 
> Oops OK.
> 
>>
>> I'd also suggest renaming the method test_ovmf_virt() to
>> test_armvirtqemu_virt().
>>
>> If that seems baroque, then I suggest "test_uefi_" in both
>> patches #2 and #3.
> 
> What about this?
> 
> - test_bios_seabios_x86_64_pc()
> - test_uefi_ovmf_x86_64_pc()
> - test_uefi_armvirtqemu_aarch64_virt()

Those work for me :)

Laszlo



[Qemu-devel] [PATCH 07/10] scripts/decodetree.py: remove unused imports

2018-10-04 Thread Cleber Rosa
Signed-off-by: Cleber Rosa 
---
 scripts/decodetree.py | 2 --
 1 file changed, 2 deletions(-)

diff --git a/scripts/decodetree.py b/scripts/decodetree.py
index 277f9a9bba..457cffea90 100755
--- a/scripts/decodetree.py
+++ b/scripts/decodetree.py
@@ -149,12 +149,10 @@
 #   trans_addl_i(ctx, &arg_opi, insn)
 #
 
-import io
 import os
 import re
 import sys
 import getopt
-import pdb
 
 insnwidth = 32
 insnmask = 0x
-- 
2.17.1




[Qemu-devel] [PATCH 01/10] qemu-img.c: comment typo fix

2018-10-04 Thread Cleber Rosa
A trivial comment typo fix.

Signed-off-by: Cleber Rosa 
---
 qemu-img.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/qemu-img.c b/qemu-img.c
index b12f4cd19b..a96b76c187 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -1085,7 +1085,7 @@ static int64_t find_nonzero(const uint8_t *buf, int64_t n)
 }
 
 /*
- * Returns true iff the first sector pointed to by 'buf' contains at least
+ * Returns true if the first sector pointed to by 'buf' contains at least
  * a non-NUL byte.
  *
  * 'pnum' is set to the number of sectors (including and immediately following
-- 
2.17.1




[Qemu-devel] [PATCH 00/10] Trivial fixes and clean ups

2018-10-04 Thread Cleber Rosa
Just a collection of trivial fixes and clean ups that have been lying
around here for some time.

Cleber Rosa (10):
  qemu-img.c: comment typo fix
  tests/tcg/README: fix location for lm32 tests
  qemu-iotests: make 218 executable
  qemu-iotests: fix filename containing checks
  docs/devel/testing.rst: add missing newlines after code block
  thunk.c: clean up commented out definition
  scripts/decodetree.py: remove unused imports
  scripts/decodetree.py: fix reference to attributes
  scripts/qemu.py: use a more consistent docstring style
  scripts/qemu.py: remove trailing quotes on docstring

 docs/devel/testing.rst   |  2 ++
 dtc  |  2 +-
 qemu-img.c   |  2 +-
 scripts/decodetree.py|  4 +--
 scripts/qemu.py  | 67 ++--
 tests/qemu-iotests/218   |  0
 tests/qemu-iotests/common.rc |  4 +--
 tests/tcg/README |  2 +-
 thunk.c  |  2 --
 9 files changed, 50 insertions(+), 35 deletions(-)
 mode change 100644 => 100755 tests/qemu-iotests/218

-- 
2.17.1




[Qemu-devel] [PATCH 02/10] tests/tcg/README: fix location for lm32 tests

2018-10-04 Thread Cleber Rosa
Point to the right and obvious location for lm32 tests.

Signed-off-by: Cleber Rosa 
---
 tests/tcg/README | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/tcg/README b/tests/tcg/README
index a5643d33e7..2a58f9a058 100644
--- a/tests/tcg/README
+++ b/tests/tcg/README
@@ -10,6 +10,6 @@ with "make test-cris".
 
 LM32
 
-The testsuite for LM32 is in tests/tcg/cris.  You can run it
+The testsuite for LM32 is in tests/tcg/lm32.  You can run it
 with "make test-lm32".
 
-- 
2.17.1




[Qemu-devel] [PATCH 06/10] thunk.c: clean up commented out definition

2018-10-04 Thread Cleber Rosa
Signed-off-by: Cleber Rosa 
---
 thunk.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/thunk.c b/thunk.c
index d5d8645cd4..e351ae53af 100644
--- a/thunk.c
+++ b/thunk.c
@@ -21,8 +21,6 @@
 #include "qemu.h"
 #include "exec/user/thunk.h"
 
-//#define DEBUG
-
 static unsigned int max_struct_entries;
 StructEntry *struct_entries;
 
-- 
2.17.1




[Qemu-devel] [PATCH 03/10] qemu-iotests: make 218 executable

2018-10-04 Thread Cleber Rosa
Commit 990dc39c made all tests executable at the time, but 218 came in
later, and missing those permissions.

Signed-off-by: Cleber Rosa 
---
 tests/qemu-iotests/218 | 0
 1 file changed, 0 insertions(+), 0 deletions(-)
 mode change 100644 => 100755 tests/qemu-iotests/218

diff --git a/tests/qemu-iotests/218 b/tests/qemu-iotests/218
old mode 100644
new mode 100755
-- 
2.17.1




Re: [Qemu-devel] [PATCH 1/1] tests: Add migration test for aarch64

2018-10-04 Thread Wei Huang



On 10/04/2018 09:17 AM, Thomas Huth wrote:
> On 2018-10-04 15:48, Andrew Jones wrote:
>> On Fri, Sep 28, 2018 at 03:47:35PM -0400, Wei Huang wrote:
> [...]
>>> diff --git a/tests/migration/aarch64/Makefile 
>>> b/tests/migration/aarch64/Makefile
>>> new file mode 100644
>>> index 000..d440fa8
>>> --- /dev/null
>>> +++ b/tests/migration/aarch64/Makefile
>>> @@ -0,0 +1,20 @@
>>> +# To specify cross compiler prefix, use CROSS_PREFIX=
>>> +#   $ make CROSS_PREFIX=aarch64-linux-gnu-
>>> +
>>> +.PHONY: all clean
>>> +all: a-b-kernel.h
>>> +
>>> +a-b-kernel.h: aarch64.kernel
>>> +   echo "$$__note" > header.tmp
>>> +   xxd -i $< | sed -e 's/.*int.*//' >> header.tmp
>>> +   mv header.tmp $@
>>> +
>>> +aarch64.kernel: aarch64.elf
>>> +   $(CROSS_PREFIX)objcopy -O binary $< $@
>>> +
>>> +aarch64.elf: a-b-kernel.S
>>> +   $(CROSS_PREFIX)gcc -o $@ -nostdlib -Wl,--build-id=none $<
>>> +
>>> +clean:
>>> +   @rm -rf *.kernel *.elf
>>
>> I don't think we need/want '-f'. Why not use $(RM)?
> 
> Does $(RM) work now in the QEMU Makefiles? AFAIK we are disabling the
> standard variables in rules.mak ("MAKEFLAGS += -rR"), but never set RM
> again...

This Makefile isn't part of QEMU regular build process (as Juan pointed
it out before: most people won't run/care it unless they want to change
the aarch64 migration test case themselves). So rules.mak isn't
included. As a result, $(RM) is still available.

I just sent in a V2 version to address Philippe's comments, in which I
fix @rm anyway.

PS: No cover letter to show V1->V2 difference as it is a very
straightforward.



> 
>  Thomas
> 



Re: [Qemu-devel] [PATCH v5 0/9] Clock framework API.

2018-10-04 Thread Philippe Mathieu-Daudé
Hi Damien,

On 02/10/2018 16:24, Damien Hedde wrote:
> This series aims to add a way to model clocks in qemu between devices.
> This allows to model the clock tree of a platform allowing us to inspect clock
> configuration and detect problems such as disabled clock or bad configured
> pll.
> 
> This series is a reroll of the v4 sent recently without the reset feature as
> requested by Peter. I also took into account the reviews about migration and
> other suggestions.
> 
> This framework was originally discussed in 2017, here:
> https://lists.gnu.org/archive/html/qemu-devel/2017-02/msg07218.html
> 
> For the user, the framework is now very similar to the device's gpio API.
> Clocks inputs and outputs can be added in devices during initialization phase.
> Then an input can be connected to an output: it means every time the output
> clock changes, a callback in the input is triggered allowing any action to be
> taken. A difference with gpios is that several inputs can be connected to a
> single output without doing any glue.
> 
> Behind the scene, there is 2 objects: a clock input which is a placeholder for
> a callback, and a clock output which is a list of inputs. The value 
> transferred
> between an output and an input is an integer representing the clock frequency.
> The input clock callback is called every time the clock frequency changes.
> The input side holds a cached value of the frequency (the output does not need
> one). This allows a device to fetch its input clock frequency at any time
> without caching it itself.
> 
> This internal state is added to handle migration in order not to update and
> propagate clocks during it (it adds cross-device and order-specific effects).
> Each device handles its input clock migration by adding the clock frequency in
> its own vmstate description.
> 
> Regarding the migration strategy, discussion started in the v4 patches.
> The problem is that we add some kind of wire between different devices and 
> we face propagation issue.
> The chosen solution allows migration compatibility from a platform version
> with no implemented clocks to a platform with clocks. A migrated device that
> have a new input clock is responsible to initialize its frequency during
> migration. Each input clock having its own state, such initialization will not
> have any side-effect on other input clock connected to the same output.
> Output clocks, having no state, are not set during migration: If a clock 
> output
> frequency changes due to migration (eg: clock computation bug-fix), the 
> effects
> will be delayed. Eventually the clock output will be updated after migration 
> if
> some (software) event trigger a clock update, but it can not be guaranteed.
> 
> There is also the problem of initialization which is very much like the
> migration. Currently, in the zynq example, clocks outputs are initialized in
> the clock controller device_reset. According to Peter, outputs should not be
> set in device_reset, so we need a better way.
> 
> It is not simple, since we can have complicated cases with several propagation
> step.
> We can't initialize outputs (without propagating) during device init and uses
> inputs value in device_reset to complete the initialization.
> Consider the case where we have a device generating a frequency, another 
> device
> doing a clock division, then a device using this clock.
> [DevClockGenerator] -> clk1 -> [DevClockDiv] -> clk2 -> [Dev]
> I don't see how we can handle such initialization without doing some
> propagation phase at some point.
> 
> Regarding clock gating. The 0 frequency value means the clock is gated.
> If need be, a gate device can be built taking an input gpio and clock and
> generating an output clock.
> 
> I've tested this patchset running Xilinx's Linux on the xilinx-zynq-a9 
> machine.
> Clocks are correctly updated and we ends up with a configured baudrate of 
> 115601
> on the console uart (for a theoretical 115200) which is nice. "cadence_uart*" 
> and
> "clock*" traces can be enabled to see what's going on in this platform.
> 
> We are considering switching to a generic payload evolution of this API.
> For example by specifying the qom carried type when adding an input/output to
> a device. This would allow us, for example, to add a power input port to 
> handle
> power gating or others ports without modifying the device state structure.
> 
> Any comments and suggestion are welcomed.

How would you instanciate devices and connect their clocks from the
command line (with the -device option)?

Should clocked devices have DeviceClass::user_creatable = false by default?

Thanks,

Phil.



Re: [Qemu-devel] [PATCH v4 18/24] qdev: hotplug: provide do_unplug handler

2018-10-04 Thread Igor Mammedov
On Wed, 3 Oct 2018 19:21:25 +0200
David Hildenbrand  wrote:

> On 03/10/2018 08:29, David Gibson wrote:
> > On Wed, Sep 26, 2018 at 11:42:13AM +0200, David Hildenbrand wrote:  
> >> The unplug and unplug_request handlers are special: They are not
> >> executed when unrealizing a device, but rather trigger the removal of a
> >> device from device_del() via object_unparent() - to effectively
> >> unrealize a device.
> >>
> >> If such a device has a child bus and another device attached to
> >> that bus (e.g. how virtio devices are created with their proxy device),
> >> we will not get a call to the unplug handler. As we want to support
> >> hotplug handlers (and especially also some unplug logic to undo resource
> >> assignment) for such devices, we cannot simply call the unplug handler
> >> when unrealizing - it has a different semantic ("trigger removal").
> >>
> >> To handle this scenario, we need a do_unplug handler, that will be
> >> executed for all devices with a hotplug handler.
> >>
> >> While at it, introduce hotplug_fn_nofail and fix a spelling mistake in
> >> a comment.
> >>
> >> Signed-off-by: David Hildenbrand 
> >> ---
> >>  hw/core/hotplug.c| 10 ++
> >>  hw/core/qdev.c   |  6 ++
> >>  include/hw/hotplug.h | 26 --
> >>  3 files changed, 40 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/hw/core/hotplug.c b/hw/core/hotplug.c
> >> index 2253072d0e..e7a68d5160 100644
> >> --- a/hw/core/hotplug.c
> >> +++ b/hw/core/hotplug.c
> >> @@ -45,6 +45,16 @@ void hotplug_handler_post_plug(HotplugHandler 
> >> *plug_handler,
> >>  }
> >>  }
> >>  
> >> +void hotplug_handler_do_unplug(HotplugHandler *plug_handler,
> >> + DeviceState *plugged_dev)  
> > 
> > Hrm.  I really dislike things named "do_X".  The "do" rarely adds any
> > useful meaning.  And when there's also something called just plain
> > "X", it's *always* unclear how they relate to each other.
> > 
> > That's doubly true when it's a general interface like this, rather
> > than just some local functions.
> >   
> 
> Yes, the naming is not the best, but I didn't want to rename all unplug
> handlers before we have an agreement on how to proceed. My concept would
> be that
> 
> 1. unplug() is renamed to trigger_unplug(). unplug_request() remains.
> 2. do_unplug() is renamed to pre_unplug() (just like pre_plug())
> 3. we might have in addition unplug() after realize(false) - just like
> plug()
> 
> trigger_unplug() would perform checks and result in object_unparent().
> From there, all cleanup/unplugging would be performed via the unrealize
> chain, just like we have for realize() now. That would allow to properly
> unplug complete device hierarchies.
> 
> But Igor rather wants one hotplug handler chain, and no dedicated
> hotplug handlers for other devices in the device hierarchy.

Because what we are dealing here with (virtio-pmem) is not separate
devices hierarchy, it's one complex device and if we are to avoid
layering violation, we should operate internal devices via that outer
device which would orchestrate given to it resources internally as it
sees it fit.

It's similar with be spapr cpu core, where internal threads do
not have their own handlers they are plugged as the integral part
of the core.

What I'm strongly opposed is using separate hotplug handlers for
internal devices of a composite one.
I'm more lenient in cases of where the hotplug handler of a composite
device access it's internals directly if creating interfaces to
manage internal devices is big over-engineering, since all
hotplug flow is explicitly described within one handler and
I don't need to hunt around to figure out how device is wired up.

It's still not right wrt not violating abstraction layers and
might break if internal details change, but usually hotplug
handler is target unique and tightly coupled code of manged
and managing pieces (like the case of spapr cpu core) so it
works so far. For some generic handler I'd vote for following
all the rules.

In this series approach, handlers are used if they are separate
devices without explicit connection which I find totally broken
(and tried to explain in this thread, probably not well enough).


> As long as
> there are no other opinions I guess I will have to go into the direction
> Igor proposes to get virtio-pmem and friends upstream.
> 




Re: [Qemu-devel] [PATCH v2 15/15] arm/xlnx-zynqmp: put APUs and RPUs in separate GDB groups

2018-10-04 Thread Philippe Mathieu-Daudé
On 03/10/2018 13:44, Luc Michel wrote:
> On 10/2/18 1:58 PM, Peter Maydell wrote:
>> On 2 October 2018 at 12:33, Philippe Mathieu-Daudé  wrote:
>>> Cc'ing more QOM involved people.
>>>
>>> On 01/10/2018 13:57, Luc Michel wrote:
 Create two separate QOM containers for APUs and RPUs to indicate to the
 GDB stub that those CPUs should be put in different processes.

 Signed-off-by: Luc Michel 
 ---
  hw/arm/xlnx-zynqmp.c | 7 +--
  1 file changed, 5 insertions(+), 2 deletions(-)

 diff --git a/hw/arm/xlnx-zynqmp.c b/hw/arm/xlnx-zynqmp.c
 index c195040350..5e92adbc71 100644
 --- a/hw/arm/xlnx-zynqmp.c
 +++ b/hw/arm/xlnx-zynqmp.c
 @@ -22,10 +22,11 @@
  #include "hw/arm/xlnx-zynqmp.h"
  #include "hw/intc/arm_gic_common.h"
  #include "exec/address-spaces.h"
  #include "sysemu/kvm.h"
  #include "kvm_arm.h"
 +#include "exec/gdbstub.h"

  #define GIC_NUM_SPI_INTR 160

  #define ARM_PHYS_TIMER_PPI  30
  #define ARM_VIRT_TIMER_PPI  27
 @@ -175,17 +176,18 @@ static void xlnx_zynqmp_create_rpu(XlnxZynqMPState 
 *s, const char *boot_cpu,
 Error **errp)
  {
  Error *err = NULL;
  int i;
  int num_rpus = MIN(smp_cpus - XLNX_ZYNQMP_NUM_APU_CPUS, 
 XLNX_ZYNQMP_NUM_RPU_CPUS);
 +Object *rpu_group = gdb_cpu_group_container_get(OBJECT(s));
>>>
>>> I'd rather keep this generic: not involve 'gdb' container name.
>>
>> Yes, I agree. We should structure how we construct our
>> model to follow what the hardware has (two CPU clusters
>> with 4 cores each), and then the gdb code should introspect
>> the system later to decide how it exposes things to the gdb
>> user. GDB specifics should (as far as possible) be kept out
>> of the board code.
>>
>> The fact that there are two clusters here also
>> affects other things, like whether they have the
>> same view of memory, whether they can share translated
>> code (they shouldn't but do at the moment), and so on --
>> it's not just a GDB-relevant distinction. So we should
>> be modelling it somehow, definitely. I don't have a clear
>> view how just yet.
> 
> So for now, maybe it's better to rely on an heuristic such as the one
> suggested by Philippe in the gdb code to group the CPUs. Once QEMU gains
> more supports for such heterogeneous architectures, we can remove the
> heuristic and put the proper QOM introspection code instead.

I'm not sure this is the best approach, just suggested because using
object_resolve_path_type("", TYPE_CPU, NULL) seemed to me the
quicker/easiest approach.

Eduardo: Do you have other thoughts on how to resolve those generic
containers, without involving any gdb-specific tag?

>> This probably ties into the stuff I have somewhere on
>> my todo list about supporting multiple heterogenous
>> systems. The problem with this xilinx board is that it
>> is trying to model that kind of system but we don't actually
>> properly support that in QEMU yet.
>>
>> thanks
>> -- PMM
>>



[Qemu-devel] [PATCH V2 1/1] tests: Add migration test for aarch64

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

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

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

Reviewed-by: Juan Quintela 
Reviewed-by: Andrew Jones 
Signed-off-by: Wei Huang 
---
 tests/Makefile.include   |  1 +
 tests/migration-test.c   | 27 +++--
 tests/migration/Makefile |  2 +-
 tests/migration/aarch64/Makefile | 19 +
 tests/migration/aarch64/a-b-kernel.S | 75 
 tests/migration/aarch64/a-b-kernel.h | 19 +
 tests/migration/migration-test.h |  9 +
 7 files changed, 147 insertions(+), 5 deletions(-)
 create mode 100644 tests/migration/aarch64/Makefile
 create mode 100644 tests/migration/aarch64/a-b-kernel.S
 create mode 100644 tests/migration/aarch64/a-b-kernel.h

diff --git a/tests/Makefile.include b/tests/Makefile.include
index 175d013..857e7cc 100644
--- a/tests/Makefile.include
+++ b/tests/Makefile.include
@@ -402,6 +402,7 @@ check-qtest-arm-y += tests/hexloader-test$(EXESUF)
 check-qtest-aarch64-y = tests/numa-test$(EXESUF)
 check-qtest-aarch64-$(CONFIG_SDHCI) += tests/sdhci-test$(EXESUF)
 check-qtest-aarch64-y += tests/boot-serial-test$(EXESUF)
+check-qtest-aarch64-y += tests/migration-test$(EXESUF)
 
 check-qtest-microblazeel-y = $(check-qtest-microblaze-y)
 
diff --git a/tests/migration-test.c b/tests/migration-test.c
index 20f38f1..5bdc0bd 100644
--- a/tests/migration-test.c
+++ b/tests/migration-test.c
@@ -86,12 +86,13 @@ static const char *tmpfs;
  * repeatedly. It outputs a 'B' at a fixed rate while it's still running.
  */
 #include "tests/migration/i386/a-b-bootblock.h"
+#include "tests/migration/aarch64/a-b-kernel.h"
 
-static void init_bootfile_x86(const char *bootpath)
+static void init_bootfile(const char *bootpath, void *content)
 {
 FILE *bootfile = fopen(bootpath, "wb");
 
-g_assert_cmpint(fwrite(x86_bootsect, 512, 1, bootfile), ==, 1);
+g_assert_cmpint(fwrite(content, 512, 1, bootfile), ==, 1);
 fclose(bootfile);
 }
 
@@ -428,7 +429,7 @@ static int test_migrate_start(QTestState **from, QTestState 
**to,
 got_stop = false;
 
 if (strcmp(arch, "i386") == 0 || strcmp(arch, "x86_64") == 0) {
-init_bootfile_x86(bootpath);
+init_bootfile(bootpath, x86_bootsect);
 cmd_src = g_strdup_printf("-machine accel=%s -m 150M"
   " -name source,debug-threads=on"
   " -serial file:%s/src_serial"
@@ -459,6 +460,24 @@ static int test_migrate_start(QTestState **from, 
QTestState **to,
 
 start_address = PPC_TEST_MEM_START;
 end_address = PPC_TEST_MEM_END;
+} else if (strcmp(arch, "aarch64") == 0) {
+init_bootfile(bootpath, aarch64_kernel);
+cmd_src = g_strdup_printf("-machine virt,accel=%s,gic-version=max "
+  "-name vmsource,debug-threads=on -cpu max "
+  "-m 150M -serial file:%s/src_serial "
+  "-kernel %s ",
+  accel, tmpfs, bootpath);
+cmd_dst = g_strdup_printf("-machine virt,accel=%s,gic-version=max "
+  "-name vmdest,debug-threads=on -cpu max "
+  "-m 150M -serial file:%s/dest_serial "
+  "-kernel %s "
+  "-incoming %s ",
+  accel, tmpfs, bootpath, uri);
+
+start_address = ARM_TEST_MEM_START;
+end_address = ARM_TEST_MEM_END;
+
+g_assert(sizeof(aarch64_kernel) <= ARM_TEST_MAX_KERNEL_SIZE);
 } else {
 g_assert_not_reached();
 }
@@ -545,7 +564,7 @@ static void test_deprecated(void)
 {
 QTestState *from;
 
-from = qtest_start("");
+from = qtest_start("-machine none");
 
 deprecated_set_downtime(from, 0.12345);
 deprecated_set_speed(from, 12345);
diff --git a/tests/migration/Makefile b/tests/migration/Makefile
index dc3b551..91237a8 100644
--- a/tests/migration/Makefile
+++ b/tests/migration/Makefile
@@ -5,7 +5,7 @@
 # See the COPYING file in the top-level directory.
 #
 
-TARGET_LIST = i386
+TARGET_LIST = i386 aarch64
 
 SRC_PATH = ../..
 
diff --git a/tests/migration/aarch64/Makefile b/tests/migration/aarch64/Makefile
new file mode 100644
index 000

Re: [Qemu-devel] [PATCH 01/13] target/arm: Define new TBFLAG for v8M stack checking

2018-10-04 Thread Philippe Mathieu-Daudé
Hi Peter,

On 02/10/2018 18:35, Peter Maydell wrote:
> The Arm v8M architecture includes hardware stack limit checking.
> When certain instructions update the stack pointer, if the new
> value of SP is below the limit set in the associated limit register
> then an exception is taken. Add a TB flag that tracks whether
> the limit-checking code needs to be emitted.
> 
> Signed-off-by: Peter Maydell 
> ---
>  target/arm/cpu.h   |  7 +++
>  target/arm/translate.h |  1 +
>  target/arm/helper.c| 10 ++
>  target/arm/translate.c |  1 +
>  4 files changed, 19 insertions(+)
> 
> diff --git a/target/arm/cpu.h b/target/arm/cpu.h
> index 65c0fa0a659..d2c1d005ed7 100644
> --- a/target/arm/cpu.h
> +++ b/target/arm/cpu.h
> @@ -1336,8 +1336,10 @@ FIELD(V7M_CCR, UNALIGN_TRP, 3, 1)
>  FIELD(V7M_CCR, DIV_0_TRP, 4, 1)
>  FIELD(V7M_CCR, BFHFNMIGN, 8, 1)
>  FIELD(V7M_CCR, STKALIGN, 9, 1)
> +FIELD(V7M_CCR, STKOFHFNMIGN, 10, 1)
>  FIELD(V7M_CCR, DC, 16, 1)
>  FIELD(V7M_CCR, IC, 17, 1)
> +FIELD(V7M_CCR, BP, 18, 1)
>  
>  /* V7M SCR bits */
>  FIELD(V7M_SCR, SLEEPONEXIT, 1, 1)
> @@ -2842,6 +2844,9 @@ static inline bool 
> arm_cpu_data_is_big_endian(CPUARMState *env)
>  /* For M profile only, Handler (ie not Thread) mode */
>  #define ARM_TBFLAG_HANDLER_SHIFT21
>  #define ARM_TBFLAG_HANDLER_MASK (1 << ARM_TBFLAG_HANDLER_SHIFT)
> +/* For M profile only, whether we should generate stack-limit checks */
> +#define ARM_TBFLAG_STACKCHECK_SHIFT 22
> +#define ARM_TBFLAG_STACKCHECK_MASK  (1 << ARM_TBFLAG_STACKCHECK_SHIFT)
>  
>  /* Bit usage when in AArch64 state */
>  #define ARM_TBFLAG_TBI0_SHIFT 0/* TBI0 for EL0/1 or TBI for EL2/3 */
> @@ -2884,6 +2889,8 @@ static inline bool 
> arm_cpu_data_is_big_endian(CPUARMState *env)
>  (((F) & ARM_TBFLAG_BE_DATA_MASK) >> ARM_TBFLAG_BE_DATA_SHIFT)
>  #define ARM_TBFLAG_HANDLER(F) \
>  (((F) & ARM_TBFLAG_HANDLER_MASK) >> ARM_TBFLAG_HANDLER_SHIFT)
> +#define ARM_TBFLAG_STACKCHECK(F) \
> +(((F) & ARM_TBFLAG_STACKCHECK_MASK) >> ARM_TBFLAG_STACKCHECK_SHIFT)
>  #define ARM_TBFLAG_TBI0(F) \
>  (((F) & ARM_TBFLAG_TBI0_MASK) >> ARM_TBFLAG_TBI0_SHIFT)
>  #define ARM_TBFLAG_TBI1(F) \
> diff --git a/target/arm/translate.h b/target/arm/translate.h
> index 45f04244be8..c1b65f3efb0 100644
> --- a/target/arm/translate.h
> +++ b/target/arm/translate.h
> @@ -38,6 +38,7 @@ typedef struct DisasContext {
>  int vec_stride;
>  bool v7m_handler_mode;
>  bool v8m_secure; /* true if v8M and we're in Secure mode */
> +bool v8m_stackcheck; /* true if we need to perform v8M stack limit 
> checks */

OK

>  /* Immediate value in AArch32 SVC insn; must be set if is_jmp == 
> DISAS_SWI
>   * so that top level loop can generate correct syndrome information.
>   */
> diff --git a/target/arm/helper.c b/target/arm/helper.c
> index 5e721a65272..6ed8631dbee 100644
> --- a/target/arm/helper.c
> +++ b/target/arm/helper.c
> @@ -12667,6 +12667,16 @@ void cpu_get_tb_cpu_state(CPUARMState *env, 
> target_ulong *pc,
>  flags |= ARM_TBFLAG_HANDLER_MASK;
>  }
>  
> +/* v8M always applies stack limit checks unless CCR.STKOFHFNMIGN is
> + * suppressing them because the requested execution priority is less 
> than 0.
> + */
> +if (arm_feature(env, ARM_FEATURE_V8) &&
> +arm_feature(env, ARM_FEATURE_M) &&
> +!((mmu_idx  & ARM_MMU_IDX_M_NEGPRI) &&
> +  (env->v7m.ccr[env->v7m.secure] & R_V7M_CCR_STKOFHFNMIGN_MASK))) {

Specs:

CCR.STKOFHFNMIGN controls whether stack limit violations
are IGNORED while executing at a requested execution priority
that is negative.

The possible values of this bit are:
0 Stack limit faults not ignored.
1 Stack limit faults at requested priorities of less than 0 ignored.

OK, the '!((' notation was hard to read, hopefully the 2 indent spaces
in the 2nd line helped...

Reviewed-by: Philippe Mathieu-Daudé 

> +flags |= ARM_TBFLAG_STACKCHECK_MASK;
> +}
> +
>  *pflags = flags;
>  *cs_base = 0;
>  }
> diff --git a/target/arm/translate.c b/target/arm/translate.c
> index c6a5d2ac444..751d5811cee 100644
> --- a/target/arm/translate.c
> +++ b/target/arm/translate.c
> @@ -12451,6 +12451,7 @@ static void 
> arm_tr_init_disas_context(DisasContextBase *dcbase, CPUState *cs)
>  dc->v7m_handler_mode = ARM_TBFLAG_HANDLER(dc->base.tb->flags);
>  dc->v8m_secure = arm_feature(env, ARM_FEATURE_M_SECURITY) &&
>  regime_is_secure(env, dc->mmu_idx);
> +dc->v8m_stackcheck = ARM_TBFLAG_STACKCHECK(dc->base.tb->flags);
>  dc->cp_regs = cpu->cp_regs;
>  dc->features = env->features;
>  
> 



Re: [Qemu-devel] [PATCH v4 7/8] file-posix: account discard operations

2018-10-04 Thread Kevin Wolf
Am 21.08.2018 um 11:46 hat Anton Nefedov geschrieben:
> This will help to identify how many of the user-issued discard operations
> (accounted on a device level) have actually suceeded down on the host file
> (even though the numbers will not be exactly the same if non-raw format
> driver is used (e.g. qcow2 sending metadata discards)).
> 
> Note that these numbers will not include discards triggered by
> write-zeroes + MAY_UNMAP calls.
> 
> Signed-off-by: Anton Nefedov 

Why not implement accounting at the BDS level for all drivers? Then we
can also reuse the existing BlockStats fields instead of duplicating
them into driver-specific new ones.

Kevin



Re: [Qemu-devel] [PATCH 1/4] target/mips: Add bit definitions for DSP R3 ASE

2018-10-04 Thread Philippe Mathieu-Daudé
Hi Aleksandar and Stefan,

On 04/10/2018 14:34, Aleksandar Markovic wrote:
> From: Stefan Markovic 
> 
> Add DSP R3 ASE related bit definition for insn_flags and hflags.
> 
> Signed-off-by: Aleksandar Markovic 
> ---
>  target/mips/cpu.h   | 1 +
>  target/mips/mips-defs.h | 1 +
>  2 files changed, 2 insertions(+)
> 
> diff --git a/target/mips/cpu.h b/target/mips/cpu.h
> index 28af4d1..4160699 100644
> --- a/target/mips/cpu.h
> +++ b/target/mips/cpu.h
> @@ -598,6 +598,7 @@ struct CPUMIPSState {
>  /* MIPS DSP resources access. */
>  #define MIPS_HFLAG_DSP   0x08  /* Enable access to MIPS DSP resources. */
>  #define MIPS_HFLAG_DSPR2 0x10  /* Enable access to MIPS DSPR2 resources. 
> */
> +#define MIPS_HFLAG_DSPR3 0x2000 /* Enable access to MIPS DSPR3 
> resources.*/

I find it confusing to add this in the middle (rather that at the end)
of this list. It looks also bug prone, if someone add another definition
at the end of the list he might use the same value.

>  /* Extra flag about HWREna register. */
>  #define MIPS_HFLAG_HWRENA_ULR 0x20 /* ULR bit from HWREna is set. */
>  #define MIPS_HFLAG_SBRI  0x40 /* R6 SDBBP causes RI excpt. in user mode 
> */
> diff --git a/target/mips/mips-defs.h b/target/mips/mips-defs.h
> index c8e9979..b27b7ae 100644
> --- a/target/mips/mips-defs.h
> +++ b/target/mips/mips-defs.h
> @@ -47,6 +47,7 @@
>  #define   ASE_MDMX  0x0004
>  #define   ASE_DSP   0x0008
>  #define   ASE_DSPR2 0x0010
> +#define   ASE_DSPR3 0x0200

Ditto.

>  #define   ASE_MT0x0020
>  #define   ASE_SMARTMIPS 0x0040
>  #define   ASE_MICROMIPS 0x0080

What about adding this patch on top of the "mips: Clean the 'insn_flags'
namespace" patch which use your suggestion and let available space for
this ASE flag?

https://lists.gnu.org/archive/html/qemu-devel/2018-09/msg04070.html

Thanks,

Phil.



Re: [Qemu-devel] [PATCH 1/1] tests: Add migration test for aarch64

2018-10-04 Thread Wei Huang



On 10/04/2018 10:30 AM, Philippe Mathieu-Daudé wrote:
> On 04/10/2018 17:27, Wei Huang wrote:
>> On 10/04/2018 10:07 AM, Philippe Mathieu-Daudé wrote:
>>> On 28/09/2018 21:47, Wei Huang wrote:
>>> [...]> +++ b/tests/migration/aarch64/Makefile
 @@ -0,0 +1,20 @@
 +# To specify cross compiler prefix, use CROSS_PREFIX=
 +#   $ make CROSS_PREFIX=aarch64-linux-gnu-
 +
 +.PHONY: all clean
 +all: a-b-kernel.h
 +
 +a-b-kernel.h: aarch64.kernel
 +  echo "$$__note" > header.tmp
>>>
>>> This won't work on a read-only fs.
>>
>> Under which setting? If tmp file can't be generated on a read-only fs,
>> wouldn't $@ have the same problem?
> 
> Yes you are right :)
> 
>>>
>>> Why don't you use $@ directly?
> 
> What about this?

Yes, I can address it with the following, along with $(RM) as Drew
pointed it out.

a-b-kernel.h: aarch64.kernel
   echo "$$__note" > $@
   xxd -i $< | sed -e 's/.*int.*//' >> $@


> 
>>>
 +  xxd -i $< | sed -e 's/.*int.*//' >> header.tmp
>>>
>>> Please use:
>>>
>>> xxd -g4 ...
>>
>> This option doesn't work with -i (the include file style output) which
>> is what we want. From xxd manual:
>>
>> "-g bytes | -groupsize bytes
>> separate the output of every  bytes (two hex characters or
>> eight bit-digits each) by a whitespace. Specify -g 0 to suppress
>> grouping.   defaults to 2 in normal mode and 1 in bits
>> mode. Grouping does not apply to postscript or include style."
> 
> Indeed, too bad.
> 
>>>
>>> xxd might not be installed on the host.
>>
>> xxd is provided by vim packages. So it should be available in most distros.
>>
>>>
>>> That said we should however install it on the docker cross images.
>>
>> Agreed.
>>
>>>
 +  mv header.tmp $@
 +
 +aarch64.kernel: aarch64.elf
 +  $(CROSS_PREFIX)objcopy -O binary $< $@
 +
 +aarch64.elf: a-b-kernel.S
 +  $(CROSS_PREFIX)gcc -o $@ -nostdlib -Wl,--build-id=none $<
 +
 +clean:
 +  @rm -rf *.kernel *.elf
>>>



Re: [Qemu-devel] [PATCH 7/7] Acceptance Tests: change the handling of tests for specific archs

2018-10-04 Thread Cleber Rosa



On 10/4/18 11:42 AM, Philippe Mathieu-Daudé wrote:
> On 04/10/2018 17:14, Cleber Rosa wrote:
>> With the introduction of a variants file that can run the same
>> tests on various architectures, it makes sense to make most tests
>> to be reusable on those environments.  The exception should be
>> when a test is really testing a specific architecture feature.
>>
>> With the change proposed here, on a command line such as:
>>
>>   $ avocado run \
>>  --json-variants-load=tests/acceptance/variants/arch.json \
>>  -- tests/acceptance/
>>
>> The boot_linux_console.py tests will appear as "CANCELED: Currently
>> specific to the x86_64 arch", which is as a good thing when compared
>> to being ignored by tags because:
>>
>>  * The architecture specific parts can be addressed
>>  * It will be run on the matching architecture (as opposed to always
>>being filtered out by the tags mechanism)
>>  * CANCELED tests do no influence negatively the overall job results,
>>they're not considered an error or failure
>>
>> Signed-off-by: Cleber Rosa 
>> ---
>>  tests/acceptance/boot_linux_console.py | 3 ++-
>>  1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/tests/acceptance/boot_linux_console.py 
>> b/tests/acceptance/boot_linux_console.py
>> index 58032f971c..2fb5da63cb 100644
>> --- a/tests/acceptance/boot_linux_console.py
>> +++ b/tests/acceptance/boot_linux_console.py
>> @@ -19,12 +19,13 @@ class BootLinuxConsole(Test):
>>  and the kernel command line is properly passed from QEMU to the kernel
>>  
>>  :avocado: enable
>> -:avocado: tags=x86_64
>>  """
>>  
>>  timeout = 60
>>  
>>  def test(self):
>> +if self.arch != 'x86_64':
>> +self.cancel('Currently specific to the x86_64 arch')
> 
> Watch out the difference between the HOST arch and the TARGET arch.
> 
> This test is specific to the x86_64 TARGET arch (the one emulated),
> _but_ it can be run on any HOST arch with TCG enabled:
> 

Right, do you think we should be more explicit about the terminology
here and use "Currently specific to a x86_64 target arch" ?

> $ uname -m
> aarch64
> 
> $ avocado \
>   --show=console \
> run \
>   -p qemu_bin=x86_64-softmmu/qemu-system-x86_64 \
>tests/acceptance/boot_linux_console.py

The arch will only be based on `uname -m` if not given explicitly.  What
I mean is that on your aarch64 host, you should be able to do:

 $ avocado run -p arch=x86_64 ...

And that test should run the same way.  So yes, arch is about the
target, but it defaults to the same arch as the host.

Again, should we be more explicit about it?

Thanks!
- Cleber.

> JOB ID : f6e023ac4ee0f999ab01bfb1b196f43fde8538b9
> 
> JOB LOG:
> /home/phil/avocado/job-results/job-2018-10-04T15.41-f6e023a/job.log
>  (1/1) tests/acceptance/boot_linux_console.py:BootLinuxConsole.test: /
> console: [0.00] Linux version 4.16.3-301.fc28.x86_64
> (mockbu...@bkernel02.phx2.fedoraproject.org) (gcc version 8.0.1 20180324
> (Red Hat 8.0.1-0.20) (GCC)) #1 SMP Mon Apr 23 21:59:58 8
> console: [0.00] Command line: console=ttyS0
> console: [0.00] x86/fpu: x87 FPU will use FXSAVE
> ...
> console: [0.00] NX (Execute Disable) protection: active
> console: [0.00] random: fast init done
> console: [0.00] SMBIOS 2.8 present.
> 
> console: [0.00] DMI: QEMU Standard PC (i440FX + PIIX, 1996),
> BIOS rel-1.11.2-0-gf9626ccb91-prebuilt.qemu-project.org 04/01/2014
> ...
> console: [0.00] Booting paravirtualized kernel on bare hardware
> ...
> console: [0.00] Kernel command line: console=ttyS0
> PASS (12.89 s)
> RESULTS: PASS 1 | ERROR 0 | FAIL 0 | SKIP 0 | WARN 0 | INTERRUPT 0 |
> CANCEL 0
> JOB TIME   : 13.12 s
> 
>>  kernel_url = ('https://mirrors.kernel.org/fedora/releases/28/'
>>'Everything/x86_64/os/images/pxeboot/vmlinuz')
>>  kernel_hash = '238e083e114c48200f80d889f7e32eeb2793e02a'
>>



Re: [Qemu-devel] [PATCH v4 6/8] scsi: account unmap operations

2018-10-04 Thread Kevin Wolf
Am 21.08.2018 um 11:46 hat Anton Nefedov geschrieben:
> Signed-off-by: Anton Nefedov 
> Reviewed-by: Vladimir Sementsov-Ogievskiy 
> Reviewed-by: Alberto Garcia 
> ---
>  hw/scsi/scsi-disk.c | 9 -
>  1 file changed, 8 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/scsi/scsi-disk.c b/hw/scsi/scsi-disk.c
> index 9d10daf..0aac137 100644
> --- a/hw/scsi/scsi-disk.c
> +++ b/hw/scsi/scsi-disk.c
> @@ -1664,6 +1664,10 @@ static void scsi_unmap_complete_noio(UnmapCBData 
> *data, int ret)
>  goto done;
>  }
>  
> +block_acct_start(blk_get_stats(s->qdev.conf.blk), &r->acct,
> + r->sector_count * s->qdev.blocksize,
> + BLOCK_ACCT_UNMAP);

If the check just above this (check_lba_range) fails, we should account
for an invalid request.

>  r->req.aiocb = blk_aio_pdiscard(s->qdev.conf.blk,
>  r->sector * s->qdev.blocksize,
>  r->sector_count * s->qdev.blocksize,
> @@ -1690,10 +1694,11 @@ static void scsi_unmap_complete(void *opaque, int ret)
>  r->req.aiocb = NULL;
>  
>  aio_context_acquire(blk_get_aio_context(s->qdev.conf.blk));
> -if (scsi_disk_req_check_error(r, ret, false)) {
> +if (scsi_disk_req_check_error(r, ret, true)) {
>  scsi_req_unref(&r->req);
>  g_free(data);
>  } else {
> +block_acct_done(blk_get_stats(s->qdev.conf.blk), &r->acct);
>  scsi_unmap_complete_noio(data, ret);
>  }
>  aio_context_release(blk_get_aio_context(s->qdev.conf.blk));
> @@ -1740,10 +1745,12 @@ static void scsi_disk_emulate_unmap(SCSIDiskReq *r, 
> uint8_t *inbuf)
>  return;
>  
>  invalid_param_len:
> +block_acct_invalid(blk_get_stats(s->qdev.conf.blk), BLOCK_ACCT_UNMAP);
>  scsi_check_condition(r, SENSE_CODE(INVALID_PARAM_LEN));
>  return;
>  
>  invalid_field:
> +block_acct_invalid(blk_get_stats(s->qdev.conf.blk), BLOCK_ACCT_UNMAP);
>  scsi_check_condition(r, SENSE_CODE(INVALID_FIELD));
>  }

What about the blk_is_read_only() case which directly returns without
jumping to one of the error labels?

Kevin



Re: [Qemu-devel] [PATCH v2 0/4] softfloat: Fix division

2018-10-04 Thread Alex Bennée


Emilio G. Cota  writes:

> On Thu, Oct 04, 2018 at 10:13:55 +0100, Alex Bennée wrote:
>>
>> Richard Henderson  writes:
>>
>> > Changes from v1:
>> >   * Preserve udiv_qrnnd as a separate division primitive that
>> > could be used as a building block for float128 division.
>> >   * Include asm fragments for x86_64, s390x, and ppc64.
>>
>> It passes my fops fdiv_double test but Emilio's test is reporting:
>>
>>   Errors found in f64_div, rounding near_even:
>>   +252.7FF80  +001.E
>>   => +64F.7FF82 x  expected +64F.7FF81 x
>
> Did you rebuild the test program? v2 passes all f64_div tests for me.
>
> Tested-by: Emilio G. Cota 
> for patches 1 and 2.

Hmm I did:

  make clean
  make

in the tests/fp dir multiple times while going through before/after fix
scenarios. And now of course it works

Reviewed-by: Alex Bennée 
Tested-by: Alex Bennée 


>
> Thanks,
>
>   E.


--
Alex Bennée



[Qemu-devel] [PULL 12/15] s390x/tcg: handle privileged instructions via flags

2018-10-04 Thread Cornelia Huck
From: David Hildenbrand 

Let's check this also at a central place.

Reviewed-by: Richard Henderson 
Signed-off-by: David Hildenbrand 
Message-Id: <20180927130303.12236-8-da...@redhat.com>
Acked-by: Thomas Huth 
Signed-off-by: Cornelia Huck 
---
 target/s390x/insn-data.def | 138 ++---
 target/s390x/translate.c   |  83 +++
 2 files changed, 76 insertions(+), 145 deletions(-)

diff --git a/target/s390x/insn-data.def b/target/s390x/insn-data.def
index 7be6e661fa..54e39df831 100644
--- a/target/s390x/insn-data.def
+++ b/target/s390x/insn-data.def
@@ -964,126 +964,126 @@
 
 #ifndef CONFIG_USER_ONLY
 /* COMPARE AND SWAP AND PURGE */
-D(0xb250, CSP, RRE,   Z,   r1_32u, ra2, r1_P, 0, csp, 0, MO_TEUL)
-D(0xb98a, CSPG,RRE, DAT_ENH, r1_o, ra2, r1_P, 0, csp, 0, MO_TEQ)
+E(0xb250, CSP, RRE,   Z,   r1_32u, ra2, r1_P, 0, csp, 0, MO_TEUL, 
IF_PRIV)
+E(0xb98a, CSPG,RRE, DAT_ENH, r1_o, ra2, r1_P, 0, csp, 0, MO_TEQ, 
IF_PRIV)
 /* DIAGNOSE (KVM hypercall) */
-C(0x8300, DIAG,RSI,   Z,   0, 0, 0, 0, diag, 0)
+F(0x8300, DIAG,RSI,   Z,   0, 0, 0, 0, diag, 0, IF_PRIV)
 /* INSERT STORAGE KEY EXTENDED */
-C(0xb229, ISKE,RRE,   Z,   0, r2_o, new, r1_8, iske, 0)
+F(0xb229, ISKE,RRE,   Z,   0, r2_o, new, r1_8, iske, 0, IF_PRIV)
 /* INVALIDATE DAT TABLE ENTRY */
-C(0xb98e, IPDE,RRF_b, Z,   r1_o, r2_o, 0, 0, idte, 0)
+F(0xb98e, IPDE,RRF_b, Z,   r1_o, r2_o, 0, 0, idte, 0, IF_PRIV)
 /* INVALIDATE PAGE TABLE ENTRY */
-C(0xb221, IPTE,RRF_a, Z,   r1_o, r2_o, 0, 0, ipte, 0)
+F(0xb221, IPTE,RRF_a, Z,   r1_o, r2_o, 0, 0, ipte, 0, IF_PRIV)
 /* LOAD CONTROL */
-C(0xb700, LCTL,RS_a,  Z,   0, a2, 0, 0, lctl, 0)
-C(0xeb2f, LCTLG,   RSY_a, Z,   0, a2, 0, 0, lctlg, 0)
+F(0xb700, LCTL,RS_a,  Z,   0, a2, 0, 0, lctl, 0, IF_PRIV)
+F(0xeb2f, LCTLG,   RSY_a, Z,   0, a2, 0, 0, lctlg, 0, IF_PRIV)
 /* LOAD PROGRAM PARAMETER */
-C(0xb280, LPP, S,   LPP,   0, m2_64, 0, 0, lpp, 0)
+F(0xb280, LPP, S,   LPP,   0, m2_64, 0, 0, lpp, 0, IF_PRIV)
 /* LOAD PSW */
-C(0x8200, LPSW,S, Z,   0, a2, 0, 0, lpsw, 0)
+F(0x8200, LPSW,S, Z,   0, a2, 0, 0, lpsw, 0, IF_PRIV)
 /* LOAD PSW EXTENDED */
-C(0xb2b2, LPSWE,   S, Z,   0, a2, 0, 0, lpswe, 0)
+F(0xb2b2, LPSWE,   S, Z,   0, a2, 0, 0, lpswe, 0, IF_PRIV)
 /* LOAD REAL ADDRESS */
-C(0xb100, LRA, RX_a,  Z,   0, a2, r1, 0, lra, 0)
-C(0xe313, LRAY,RXY_a, LD,  0, a2, r1, 0, lra, 0)
-C(0xe303, LRAG,RXY_a, Z,   0, a2, r1, 0, lra, 0)
+F(0xb100, LRA, RX_a,  Z,   0, a2, r1, 0, lra, 0, IF_PRIV)
+F(0xe313, LRAY,RXY_a, LD,  0, a2, r1, 0, lra, 0, IF_PRIV)
+F(0xe303, LRAG,RXY_a, Z,   0, a2, r1, 0, lra, 0, IF_PRIV)
 /* LOAD USING REAL ADDRESS */
-C(0xb24b, LURA,RRE,   Z,   0, r2, new, r1_32, lura, 0)
-C(0xb905, LURAG,   RRE,   Z,   0, r2, r1, 0, lurag, 0)
+F(0xb24b, LURA,RRE,   Z,   0, r2, new, r1_32, lura, 0, IF_PRIV)
+F(0xb905, LURAG,   RRE,   Z,   0, r2, r1, 0, lurag, 0, IF_PRIV)
 /* MOVE TO PRIMARY */
-C(0xda00, MVCP,SS_d,  Z,   la1, a2, 0, 0, mvcp, 0)
+F(0xda00, MVCP,SS_d,  Z,   la1, a2, 0, 0, mvcp, 0, IF_PRIV)
 /* MOVE TO SECONDARY */
-C(0xdb00, MVCS,SS_d,  Z,   la1, a2, 0, 0, mvcs, 0)
+F(0xdb00, MVCS,SS_d,  Z,   la1, a2, 0, 0, mvcs, 0, IF_PRIV)
 /* PURGE TLB */
-C(0xb20d, PTLB,S, Z,   0, 0, 0, 0, ptlb, 0)
+F(0xb20d, PTLB,S, Z,   0, 0, 0, 0, ptlb, 0, IF_PRIV)
 /* RESET REFERENCE BIT EXTENDED */
-C(0xb22a, RRBE,RRE,   Z,   0, r2_o, 0, 0, rrbe, 0)
+F(0xb22a, RRBE,RRE,   Z,   0, r2_o, 0, 0, rrbe, 0, IF_PRIV)
 /* SERVICE CALL LOGICAL PROCESSOR (PV hypercall) */
-C(0xb220, SERVC,   RRE,   Z,   r1_o, r2_o, 0, 0, servc, 0)
+F(0xb220, SERVC,   RRE,   Z,   r1_o, r2_o, 0, 0, servc, 0, IF_PRIV)
 /* SET ADDRESS SPACE CONTROL FAST */
-C(0xb279, SACF,S, Z,   0, a2, 0, 0, sacf, 0)
+F(0xb279, SACF,S, Z,   0, a2, 0, 0, sacf, 0, IF_PRIV)
 /* SET CLOCK */
-C(0xb204, SCK, S, Z,   la2, 0, 0, 0, sck, 0)
+F(0xb204, SCK, S, Z,   la2, 0, 0, 0, sck, 0, IF_PRIV)
 /* SET CLOCK COMPARATOR */
-C(0xb206, SCKC,S, Z,   0, m2_64a, 0, 0, sckc, 0)
+F(0xb206, SCKC,S, Z,   0, m2_64a, 0, 0, sckc, 0, IF_PRIV)
 /* SET CLOCK PROGRAMMABLE FIELD */
-C(0x0107, SCKPF,   E, Z,   0, 0, 0, 0, sckpf, 0)
+F(0x0107, SCKPF,   E, Z,   0, 0, 0, 0, sckpf, 0, IF_PRIV)
 /* SET CPU TIMER */
-C(0xb208, SPT, S, Z,   0, m2_64a, 0, 0, spt, 0)
+F(0xb208, SPT, S, Z,   0, m2_64a, 0, 0, spt, 0, IF_PRIV)
 /* SET PREFIX */
-C(0xb210, SPX, S, Z,   0, m2_32ua, 0, 0, spx, 0)
+F(0xb210, SPX, S, Z,   0, m2_32ua, 0, 0, spx, 0, IF_PRIV)
 /* SET PSW KEY FROM ADDRESS */
-C(0xb20a, SPKA,S, Z,   0, a2, 0, 0, spka, 0)
+F(0xb20a, SPKA,S, Z,   0, a2, 0, 0, spka, 0, IF_PRIV)
 /* S

Re: [Qemu-devel] [PATCH 7/7] Acceptance Tests: change the handling of tests for specific archs

2018-10-04 Thread Philippe Mathieu-Daudé
On 04/10/2018 17:14, Cleber Rosa wrote:
> With the introduction of a variants file that can run the same
> tests on various architectures, it makes sense to make most tests
> to be reusable on those environments.  The exception should be
> when a test is really testing a specific architecture feature.
> 
> With the change proposed here, on a command line such as:
> 
>   $ avocado run \
>  --json-variants-load=tests/acceptance/variants/arch.json \
>  -- tests/acceptance/
> 
> The boot_linux_console.py tests will appear as "CANCELED: Currently
> specific to the x86_64 arch", which is as a good thing when compared
> to being ignored by tags because:
> 
>  * The architecture specific parts can be addressed
>  * It will be run on the matching architecture (as opposed to always
>being filtered out by the tags mechanism)
>  * CANCELED tests do no influence negatively the overall job results,
>they're not considered an error or failure
> 
> Signed-off-by: Cleber Rosa 
> ---
>  tests/acceptance/boot_linux_console.py | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/tests/acceptance/boot_linux_console.py 
> b/tests/acceptance/boot_linux_console.py
> index 58032f971c..2fb5da63cb 100644
> --- a/tests/acceptance/boot_linux_console.py
> +++ b/tests/acceptance/boot_linux_console.py
> @@ -19,12 +19,13 @@ class BootLinuxConsole(Test):
>  and the kernel command line is properly passed from QEMU to the kernel
>  
>  :avocado: enable
> -:avocado: tags=x86_64
>  """
>  
>  timeout = 60
>  
>  def test(self):
> +if self.arch != 'x86_64':
> +self.cancel('Currently specific to the x86_64 arch')

Watch out the difference between the HOST arch and the TARGET arch.

This test is specific to the x86_64 TARGET arch (the one emulated),
_but_ it can be run on any HOST arch with TCG enabled:

$ uname -m
aarch64

$ avocado \
  --show=console \
run \
  -p qemu_bin=x86_64-softmmu/qemu-system-x86_64 \
   tests/acceptance/boot_linux_console.py
JOB ID : f6e023ac4ee0f999ab01bfb1b196f43fde8538b9

JOB LOG:
/home/phil/avocado/job-results/job-2018-10-04T15.41-f6e023a/job.log
 (1/1) tests/acceptance/boot_linux_console.py:BootLinuxConsole.test: /
console: [0.00] Linux version 4.16.3-301.fc28.x86_64
(mockbu...@bkernel02.phx2.fedoraproject.org) (gcc version 8.0.1 20180324
(Red Hat 8.0.1-0.20) (GCC)) #1 SMP Mon Apr 23 21:59:58 8
console: [0.00] Command line: console=ttyS0
console: [0.00] x86/fpu: x87 FPU will use FXSAVE
...
console: [0.00] NX (Execute Disable) protection: active
console: [0.00] random: fast init done
console: [0.00] SMBIOS 2.8 present.

console: [0.00] DMI: QEMU Standard PC (i440FX + PIIX, 1996),
BIOS rel-1.11.2-0-gf9626ccb91-prebuilt.qemu-project.org 04/01/2014
...
console: [0.00] Booting paravirtualized kernel on bare hardware
...
console: [0.00] Kernel command line: console=ttyS0
PASS (12.89 s)
RESULTS: PASS 1 | ERROR 0 | FAIL 0 | SKIP 0 | WARN 0 | INTERRUPT 0 |
CANCEL 0
JOB TIME   : 13.12 s

>  kernel_url = ('https://mirrors.kernel.org/fedora/releases/28/'
>'Everything/x86_64/os/images/pxeboot/vmlinuz')
>  kernel_hash = '238e083e114c48200f80d889f7e32eeb2793e02a'
> 



Re: [Qemu-devel] [PATCH v4 3/8] ide: account UNMAP (TRIM) operations

2018-10-04 Thread Kevin Wolf
Am 21.08.2018 um 11:46 hat Anton Nefedov geschrieben:
> Signed-off-by: Anton Nefedov 
> Reviewed-by: Alberto Garcia 
> ---
>  hw/ide/core.c | 12 
>  1 file changed, 12 insertions(+)
> 
> diff --git a/hw/ide/core.c b/hw/ide/core.c
> index 2c62efc..352429b 100644
> --- a/hw/ide/core.c
> +++ b/hw/ide/core.c
> @@ -440,6 +440,14 @@ static void ide_issue_trim_cb(void *opaque, int ret)
>  TrimAIOCB *iocb = opaque;
>  IDEState *s = iocb->s;
>  
> +if (iocb->i >= 0) {
> +if (ret >= 0) {
> +block_acct_done(blk_get_stats(s->blk), &s->acct);
> +} else {
> +block_acct_failed(blk_get_stats(s->blk), &s->acct);
> +}
> +}
> +
>  if (ret >= 0) {
>  while (iocb->j < iocb->qiov->niov) {
>  int j = iocb->j;
> @@ -461,6 +469,9 @@ static void ide_issue_trim_cb(void *opaque, int ret)
>  goto done;
>  }
>  
> +block_acct_start(blk_get_stats(s->blk), &s->acct,
> + count << BDRV_SECTOR_BITS, 
> BLOCK_ACCT_UNMAP);
> +
>  /* Got an entry! Submit and exit.  */
>  iocb->aiocb = blk_aio_pdiscard(s->blk,
> sector << BDRV_SECTOR_BITS,
> @@ -845,6 +856,7 @@ static void ide_dma_cb(void *opaque, int ret)
>  }
>  
>  if (ret == -EINVAL) {
> +block_acct_invalid(blk_get_stats(s->blk), BLOCK_ACCT_UNMAP);

This looks wrong to me, ide_dma_cb() is not only called for unmap, but
also for reads and writes, and each of them could return -EINVAL.

Also, -EINVAL doesn't necessarily mean that the guest driver did
something wrong, it could also be the result of a host problem.
Therefore, it isn't right to call block_acct_invalid() here - especially
since the request may already have been accounted for as either done or
failed in ide_issue_trim_cb().

Instead, I think it would be better to immediately account for invalid
requests in ide_issue_trim_cb() where iocb->ret = -EINVAL is set and we
know for sure that indeed !ide_sect_range_ok() is the cause for the
-EINVAL return code.

>  ide_dma_error(s);
>  return;
>  }

Kevin



[Qemu-devel] [PULL 10/15] s390x/tcg: add instruction flags for floating point instructions

2018-10-04 Thread Cornelia Huck
From: David Hildenbrand 

These flags allow us to later on detect if a DATA program interrupt
is to be injected, and which DXC (1,2,3) is to be used.

Interestingly, some support FP instructions are considered as HFP
instructions (I assume simply because they were available very early).

Reviewed-by: Richard Henderson 
Signed-off-by: David Hildenbrand 
Message-Id: <20180927130303.12236-6-da...@redhat.com>
Acked-by: Thomas Huth 
Signed-off-by: Cornelia Huck 
---
 target/s390x/insn-data.def | 254 ++---
 target/s390x/translate.c   |   8 ++
 2 files changed, 135 insertions(+), 127 deletions(-)

diff --git a/target/s390x/insn-data.def b/target/s390x/insn-data.def
index 7ab28b7f2d..7be6e661fa 100644
--- a/target/s390x/insn-data.def
+++ b/target/s390x/insn-data.def
@@ -32,11 +32,11 @@
 C(0xb9e8, AGRK,RRF_a, DO,  r2, r3, r1, 0, add, adds64)
 C(0xe308, AG,  RXY_a, Z,   r1, m2_64, r1, 0, add, adds64)
 C(0xe318, AGF, RXY_a, Z,   r1, m2_32s, r1, 0, add, adds64)
-C(0xb30a, AEBR,RRE,   Z,   e1, e2, new, e1, aeb, f32)
-C(0xb31a, ADBR,RRE,   Z,   f1_o, f2_o, f1, 0, adb, f64)
-C(0xb34a, AXBR,RRE,   Z,   0, x2_o, x1, 0, axb, f128)
-C(0xed0a, AEB, RXE,   Z,   e1, m2_32u, new, e1, aeb, f32)
-C(0xed1a, ADB, RXE,   Z,   f1_o, m2_64, f1, 0, adb, f64)
+F(0xb30a, AEBR,RRE,   Z,   e1, e2, new, e1, aeb, f32, IF_BFP)
+F(0xb31a, ADBR,RRE,   Z,   f1_o, f2_o, f1, 0, adb, f64, IF_BFP)
+F(0xb34a, AXBR,RRE,   Z,   0, x2_o, x1, 0, axb, f128, IF_BFP)
+F(0xed0a, AEB, RXE,   Z,   e1, m2_32u, new, e1, aeb, f32, IF_BFP)
+F(0xed1a, ADB, RXE,   Z,   f1_o, m2_64, f1, 0, adb, f64, IF_BFP)
 /* ADD HIGH */
 C(0xb9c8, AHHHR,   RRF_a, HW,  r2_sr32, r3_sr32, new, r1_32h, add, adds32)
 C(0xb9d8, AHHLR,   RRF_a, HW,  r2_sr32, r3, new, r1_32h, add, adds32)
@@ -154,7 +154,7 @@
 C(0xb241, CKSM,RRE,   Z,   r1_o, ra2, new, r1_32, cksm, 0)
 
 /* COPY SIGN */
-C(0xb372, CPSDR,   RRF_b, FPSSH, f3_o, f2_o, f1, 0, cps, 0)
+F(0xb372, CPSDR,   RRF_b, FPSSH, f3_o, f2_o, f1, 0, cps, 0, IF_AFP1 | 
IF_AFP2 | IF_AFP3)
 
 /* COMPARE */
 C(0x1900, CR,  RR_a,  Z,   r1_o, r2_o, 0, 0, 0, cmps32)
@@ -164,17 +164,17 @@
 C(0xb930, CGFR,RRE,   Z,   r1_o, r2_32s, 0, 0, 0, cmps64)
 C(0xe320, CG,  RXY_a, Z,   r1_o, m2_64, 0, 0, 0, cmps64)
 C(0xe330, CGF, RXY_a, Z,   r1_o, m2_32s, 0, 0, 0, cmps64)
-C(0xb309, CEBR,RRE,   Z,   e1, e2, 0, 0, ceb, 0)
-C(0xb319, CDBR,RRE,   Z,   f1_o, f2_o, 0, 0, cdb, 0)
-C(0xb349, CXBR,RRE,   Z,   x1_o, x2_o, 0, 0, cxb, 0)
-C(0xed09, CEB, RXE,   Z,   e1, m2_32u, 0, 0, ceb, 0)
-C(0xed19, CDB, RXE,   Z,   f1_o, m2_64, 0, 0, cdb, 0)
+F(0xb309, CEBR,RRE,   Z,   e1, e2, 0, 0, ceb, 0, IF_BFP)
+F(0xb319, CDBR,RRE,   Z,   f1_o, f2_o, 0, 0, cdb, 0, IF_BFP)
+F(0xb349, CXBR,RRE,   Z,   x1_o, x2_o, 0, 0, cxb, 0, IF_BFP)
+F(0xed09, CEB, RXE,   Z,   e1, m2_32u, 0, 0, ceb, 0, IF_BFP)
+F(0xed19, CDB, RXE,   Z,   f1_o, m2_64, 0, 0, cdb, 0, IF_BFP)
 /* COMPARE AND SIGNAL */
-C(0xb308, KEBR,RRE,   Z,   e1, e2, 0, 0, keb, 0)
-C(0xb318, KDBR,RRE,   Z,   f1_o, f2_o, 0, 0, kdb, 0)
-C(0xb348, KXBR,RRE,   Z,   x1_o, x2_o, 0, 0, kxb, 0)
-C(0xed08, KEB, RXE,   Z,   e1, m2_32u, 0, 0, keb, 0)
-C(0xed18, KDB, RXE,   Z,   f1_o, m2_64, 0, 0, kdb, 0)
+F(0xb308, KEBR,RRE,   Z,   e1, e2, 0, 0, keb, 0, IF_BFP)
+F(0xb318, KDBR,RRE,   Z,   f1_o, f2_o, 0, 0, kdb, 0, IF_BFP)
+F(0xb348, KXBR,RRE,   Z,   x1_o, x2_o, 0, 0, kxb, 0, IF_BFP)
+F(0xed08, KEB, RXE,   Z,   e1, m2_32u, 0, 0, keb, 0, IF_BFP)
+F(0xed18, KDB, RXE,   Z,   f1_o, m2_64, 0, 0, kdb, 0, IF_BFP)
 /* COMPARE IMMEDIATE */
 C(0xc20d, CFI, RIL_a, EI,  r1, i2, 0, 0, 0, cmps32)
 C(0xc20c, CGFI,RIL_a, EI,  r1, i2, 0, 0, 0, cmps64)
@@ -291,33 +291,33 @@
 C(0x4e00, CVD, RX_a,  Z,   r1_o, a2, 0, 0, cvd, 0)
 C(0xe326, CVDY,RXY_a, LD,  r1_o, a2, 0, 0, cvd, 0)
 /* CONVERT TO FIXED */
-C(0xb398, CFEBR,   RRF_e, Z,   0, e2, new, r1_32, cfeb, 0)
-C(0xb399, CFDBR,   RRF_e, Z,   0, f2_o, new, r1_32, cfdb, 0)
-C(0xb39a, CFXBR,   RRF_e, Z,   0, x2_o, new, r1_32, cfxb, 0)
-C(0xb3a8, CGEBR,   RRF_e, Z,   0, e2, r1, 0, cgeb, 0)
-C(0xb3a9, CGDBR,   RRF_e, Z,   0, f2_o, r1, 0, cgdb, 0)
-C(0xb3aa, CGXBR,   RRF_e, Z,   0, x2_o, r1, 0, cgxb, 0)
+F(0xb398, CFEBR,   RRF_e, Z,   0, e2, new, r1_32, cfeb, 0, IF_BFP)
+F(0xb399, CFDBR,   RRF_e, Z,   0, f2_o, new, r1_32, cfdb, 0, IF_BFP)
+F(0xb39a, CFXBR,   RRF_e, Z,   0, x2_o, new, r1_32, cfxb, 0, IF_BFP)
+F(0xb3a8, CGEBR,   RRF_e, Z,   0, e2, r1, 0, cgeb, 0, IF_BFP)
+F(0xb3a9, CGDBR,   RRF_e, Z,   0, f2_o, r1, 0, cgdb, 0, IF_BFP)
+F(0xb3aa, CGXBR,   RRF_e, Z,   0, x2_o, r1, 0, cgxb, 0, IF_BFP)
 /* CONVERT FROM FIXED */
-C(0xb394, CEFBR,   RRF_e, Z,   0, r2_32s, new, e1, cegb, 0)
-

[Qemu-devel] [PULL 11/15] s390x/tcg: check for AFP-register, BFP and DFP data exceptions

2018-10-04 Thread Cornelia Huck
From: David Hildenbrand 

With the annotated functions, we can now easily check this at a central
place.

DXC 1 is to be injected if an AFP register is used (for a HFP AND FPS
instruction) when AFP is disabled.
DXC 2 is to be injected if a BFP instruction is used when AFP is
disabled.
DXC 3 is to be injected if a DFP instruction is used when AFP is
disabled.

Reviewed-by: Richard Henderson 
Reviewed-by: Thomas Huth 
Signed-off-by: David Hildenbrand 
Message-Id: <20180927130303.12236-7-da...@redhat.com>
Signed-off-by: Cornelia Huck 
---
 target/s390x/translate.c | 33 +
 1 file changed, 33 insertions(+)

diff --git a/target/s390x/translate.c b/target/s390x/translate.c
index 49e5e2cc58..67049975fa 100644
--- a/target/s390x/translate.c
+++ b/target/s390x/translate.c
@@ -6094,6 +6094,11 @@ static const DisasInsn *extract_insn(CPUS390XState *env, 
DisasContext *s,
 return info;
 }
 
+static bool is_afp_reg(int reg)
+{
+return reg % 2 || reg > 6;
+}
+
 static DisasJumpType translate_one(CPUS390XState *env, DisasContext *s)
 {
 const DisasInsn *insn;
@@ -6120,6 +6125,34 @@ static DisasJumpType translate_one(CPUS390XState *env, 
DisasContext *s)
 }
 #endif
 
+/* process flags */
+if (insn->flags) {
+/* if AFP is not enabled, instructions and registers are forbidden */
+if (!(s->base.tb->flags & FLAG_MASK_AFP)) {
+uint8_t dxc = 0;
+
+if ((insn->flags & IF_AFP1) && is_afp_reg(get_field(&f, r1))) {
+dxc = 1;
+}
+if ((insn->flags & IF_AFP2) && is_afp_reg(get_field(&f, r2))) {
+dxc = 1;
+}
+if ((insn->flags & IF_AFP3) && is_afp_reg(get_field(&f, r3))) {
+dxc = 1;
+}
+if (insn->flags & IF_BFP) {
+dxc = 2;
+}
+if (insn->flags & IF_DFP) {
+dxc = 3;
+}
+if (dxc) {
+gen_data_exception(dxc);
+return DISAS_NORETURN;
+}
+}
+}
+
 /* Check for insn specification exceptions.  */
 if (insn->spec) {
 int spec = insn->spec, excp = 0, r;
-- 
2.14.4




[Qemu-devel] [PULL 15/15] hw/s390x/s390-pci-bus: Convert sysbus init function to realize function

2018-10-04 Thread Cornelia Huck
From: Thomas Huth 

The SysBusDeviceClass->init() interface is considered as a legacy interface
and there are currently some efforts going on to get rid of it. Thus let's
convert the init function in the s390x code to realize() instead.

Signed-off-by: Thomas Huth 
Message-Id: <1538466491-2073-1-git-send-email-th...@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé 
Reviewed-by: David Hildenbrand 
Signed-off-by: Cornelia Huck 
---
 hw/s390x/s390-pci-bus.c | 34 +-
 1 file changed, 21 insertions(+), 13 deletions(-)

diff --git a/hw/s390x/s390-pci-bus.c b/hw/s390x/s390-pci-bus.c
index e3e0ebb7f6..e42e1b80d6 100644
--- a/hw/s390x/s390-pci-bus.c
+++ b/hw/s390x/s390-pci-bus.c
@@ -692,27 +692,35 @@ static void s390_pci_iommu_free(S390pciState *s, PCIBus 
*bus, int32_t devfn)
 object_unref(OBJECT(iommu));
 }
 
-static int s390_pcihost_init(SysBusDevice *dev)
+static void s390_pcihost_realize(DeviceState *dev, Error **errp)
 {
 PCIBus *b;
 BusState *bus;
 PCIHostState *phb = PCI_HOST_BRIDGE(dev);
 S390pciState *s = S390_PCI_HOST_BRIDGE(dev);
+Error *local_err = NULL;
 
 DPRINTF("host_init\n");
 
-b = pci_register_root_bus(DEVICE(dev), NULL,
-  s390_pci_set_irq, s390_pci_map_irq, NULL,
-  get_system_memory(), get_system_io(), 0, 64,
-  TYPE_PCI_BUS);
+b = pci_register_root_bus(dev, NULL, s390_pci_set_irq, s390_pci_map_irq,
+  NULL, get_system_memory(), get_system_io(), 0,
+  64, TYPE_PCI_BUS);
 pci_setup_iommu(b, s390_pci_dma_iommu, s);
 
 bus = BUS(b);
-qbus_set_hotplug_handler(bus, DEVICE(dev), NULL);
+qbus_set_hotplug_handler(bus, dev, &local_err);
+if (local_err) {
+error_propagate(errp, local_err);
+return;
+}
 phb->bus = b;
 
-s->bus = S390_PCI_BUS(qbus_create(TYPE_S390_PCI_BUS, DEVICE(s), NULL));
-qbus_set_hotplug_handler(BUS(s->bus), DEVICE(s), NULL);
+s->bus = S390_PCI_BUS(qbus_create(TYPE_S390_PCI_BUS, dev, NULL));
+qbus_set_hotplug_handler(BUS(s->bus), dev, &local_err);
+if (local_err) {
+error_propagate(errp, local_err);
+return;
+}
 
 s->iommu_table = g_hash_table_new_full(g_int64_hash, g_int64_equal,
NULL, g_free);
@@ -722,9 +730,10 @@ static int s390_pcihost_init(SysBusDevice *dev)
 QTAILQ_INIT(&s->zpci_devs);
 
 css_register_io_adapters(CSS_IO_ADAPTER_PCI, true, false,
- S390_ADAPTER_SUPPRESSIBLE, &error_abort);
-
-return 0;
+ S390_ADAPTER_SUPPRESSIBLE, &local_err);
+if (local_err) {
+error_propagate(errp, local_err);
+}
 }
 
 static int s390_pci_msix_init(S390PCIBusDevice *pbdev)
@@ -1018,12 +1027,11 @@ static void s390_pcihost_reset(DeviceState *dev)
 
 static void s390_pcihost_class_init(ObjectClass *klass, void *data)
 {
-SysBusDeviceClass *k = SYS_BUS_DEVICE_CLASS(klass);
 DeviceClass *dc = DEVICE_CLASS(klass);
 HotplugHandlerClass *hc = HOTPLUG_HANDLER_CLASS(klass);
 
 dc->reset = s390_pcihost_reset;
-k->init = s390_pcihost_init;
+dc->realize = s390_pcihost_realize;
 hc->plug = s390_pcihost_hot_plug;
 hc->unplug = s390_pcihost_hot_unplug;
 msi_nonbroken = true;
-- 
2.14.4




[Qemu-devel] [PULL 08/15] s390x/tcg: store in the TB flags if AFP is enabled

2018-10-04 Thread Cornelia Huck
From: David Hildenbrand 

We exit the TB when changing the control registers, so just like PSW
bits, this should always be consistent for a TB.

Using the PSW bit semantic makes things a lot easier compared to
manually defining the spare, shifted bits.

Reviewed-by: Richard Henderson 
Signed-off-by: David Hildenbrand 
Message-Id: <20180927130303.12236-4-da...@redhat.com>
Signed-off-by: Cornelia Huck 
---
 target/s390x/cpu.h | 8 
 1 file changed, 8 insertions(+)

diff --git a/target/s390x/cpu.h b/target/s390x/cpu.h
index 5e50c3a303..8c2320e882 100644
--- a/target/s390x/cpu.h
+++ b/target/s390x/cpu.h
@@ -255,6 +255,7 @@ extern const struct VMStateDescription vmstate_s390_cpu;
 
 /* PSW defines */
 #undef PSW_MASK_PER
+#undef PSW_MASK_UNUSED_2
 #undef PSW_MASK_DAT
 #undef PSW_MASK_IO
 #undef PSW_MASK_EXT
@@ -273,6 +274,7 @@ extern const struct VMStateDescription vmstate_s390_cpu;
 #undef PSW_MASK_ESA_ADDR
 
 #define PSW_MASK_PER0x4000ULL
+#define PSW_MASK_UNUSED_2   0x2000ULL
 #define PSW_MASK_DAT0x0400ULL
 #define PSW_MASK_IO 0x0200ULL
 #define PSW_MASK_EXT0x0100ULL
@@ -318,6 +320,9 @@ extern const struct VMStateDescription vmstate_s390_cpu;
 #define FLAG_MASK_PSW   (FLAG_MASK_PER | FLAG_MASK_DAT | 
FLAG_MASK_PSTATE \
 | FLAG_MASK_ASC | FLAG_MASK_64 | FLAG_MASK_32)
 
+/* we'll use some unused PSW positions to store CR flags in tb flags */
+#define FLAG_MASK_AFP   (PSW_MASK_UNUSED_2 >> FLAG_MASK_PSW_SHIFT)
+
 /* Control register 0 bits */
 #define CR0_LOWPROT 0x1000ULL
 #define CR0_SECONDARY   0x0400ULL
@@ -364,6 +369,9 @@ static inline void cpu_get_tb_cpu_state(CPUS390XState* env, 
target_ulong *pc,
 *pc = env->psw.addr;
 *cs_base = env->ex_value;
 *flags = (env->psw.mask >> FLAG_MASK_PSW_SHIFT) & FLAG_MASK_PSW;
+if (env->cregs[0] & CR0_AFP) {
+*flags |= FLAG_MASK_AFP;
+}
 }
 
 /* PER bits from control register 9 */
-- 
2.14.4




Re: [Qemu-devel] [PATCH 1/1] tests: Add migration test for aarch64

2018-10-04 Thread Philippe Mathieu-Daudé
On 04/10/2018 17:27, Wei Huang wrote:
> On 10/04/2018 10:07 AM, Philippe Mathieu-Daudé wrote:
>> On 28/09/2018 21:47, Wei Huang wrote:
>> [...]> +++ b/tests/migration/aarch64/Makefile
>>> @@ -0,0 +1,20 @@
>>> +# To specify cross compiler prefix, use CROSS_PREFIX=
>>> +#   $ make CROSS_PREFIX=aarch64-linux-gnu-
>>> +
>>> +.PHONY: all clean
>>> +all: a-b-kernel.h
>>> +
>>> +a-b-kernel.h: aarch64.kernel
>>> +   echo "$$__note" > header.tmp
>>
>> This won't work on a read-only fs.
> 
> Under which setting? If tmp file can't be generated on a read-only fs,
> wouldn't $@ have the same problem?

Yes you are right :)

>>
>> Why don't you use $@ directly?

What about this?

>>
>>> +   xxd -i $< | sed -e 's/.*int.*//' >> header.tmp
>>
>> Please use:
>>
>> xxd -g4 ...
> 
> This option doesn't work with -i (the include file style output) which
> is what we want. From xxd manual:
> 
> "-g bytes | -groupsize bytes
> separate the output of every  bytes (two hex characters or
> eight bit-digits each) by a whitespace. Specify -g 0 to suppress
> grouping.   defaults to 2 in normal mode and 1 in bits
> mode. Grouping does not apply to postscript or include style."

Indeed, too bad.

>>
>> xxd might not be installed on the host.
> 
> xxd is provided by vim packages. So it should be available in most distros.
> 
>>
>> That said we should however install it on the docker cross images.
> 
> Agreed.
> 
>>
>>> +   mv header.tmp $@
>>> +
>>> +aarch64.kernel: aarch64.elf
>>> +   $(CROSS_PREFIX)objcopy -O binary $< $@
>>> +
>>> +aarch64.elf: a-b-kernel.S
>>> +   $(CROSS_PREFIX)gcc -o $@ -nostdlib -Wl,--build-id=none $<
>>> +
>>> +clean:
>>> +   @rm -rf *.kernel *.elf
>>



[Qemu-devel] [PULL 09/15] s390x/tcg: support flags for instructions

2018-10-04 Thread Cornelia Huck
From: David Hildenbrand 

Storing flags for instructions allows us to efficiently verify certain
properties at a central point. Examples might later be handling if
AFP is disabled in CR0, we are not in problem state, or if vector
instructions are disabled in CR0.

Reviewed-by: Richard Henderson 
Reviewed-by: Thomas Huth 
Signed-off-by: David Hildenbrand 
Message-Id: <20180927130303.12236-5-da...@redhat.com>
Signed-off-by: Cornelia Huck 
---
 target/s390x/insn-data.def |  3 +++
 target/s390x/translate.c   | 22 --
 2 files changed, 19 insertions(+), 6 deletions(-)

diff --git a/target/s390x/insn-data.def b/target/s390x/insn-data.def
index 9c7b434fca..7ab28b7f2d 100644
--- a/target/s390x/insn-data.def
+++ b/target/s390x/insn-data.def
@@ -3,6 +3,8 @@
  *
  *  C(OPC,NAME,FMT,   FAC, I1, I2, P, W, OP, CC)
  *  D(OPC,NAME,FMT,   FAC, I1, I2, P, W, OP, CC, DATA)
+ *  E(OPC,NAME,FMT,   FAC, I1, I2, P, W, OP, CC, DATA, FLAGS)
+ *  F(OPC,NAME,FMT,   FAC, I1, I2, P, W, OP, CC, FLAGS)
  *
  *  OPC  = (op << 8) | op2 where op is the major, op2 the minor opcode
  *  NAME = name of the opcode, used internally
@@ -15,6 +17,7 @@
  *  OP   = func op_xx does the bulk of the operation
  *  CC   = func cout_xx defines how cc should get set
  *  DATA = immediate argument to op_xx function
+ *  FLAGS = categorize the type of instruction (e.g. for advanced checks)
  *
  *  The helpers get called in order: I1, I2, P, OP, W, CC
  */
diff --git a/target/s390x/translate.c b/target/s390x/translate.c
index 43b736335f..146a817ce2 100644
--- a/target/s390x/translate.c
+++ b/target/s390x/translate.c
@@ -1121,6 +1121,7 @@ typedef struct {
 
 struct DisasInsn {
 unsigned opc:16;
+unsigned flags:16;
 DisasFormat fmt:8;
 unsigned fac:8;
 unsigned spec:8;
@@ -5835,17 +5836,24 @@ static void in2_insn(DisasContext *s, DisasFields *f, 
DisasOps *o)
search tree, rather than us having to post-process the table.  */
 
 #define C(OPC, NM, FT, FC, I1, I2, P, W, OP, CC) \
-D(OPC, NM, FT, FC, I1, I2, P, W, OP, CC, 0)
+E(OPC, NM, FT, FC, I1, I2, P, W, OP, CC, 0, 0)
 
-#define D(OPC, NM, FT, FC, I1, I2, P, W, OP, CC, D) insn_ ## NM,
+#define D(OPC, NM, FT, FC, I1, I2, P, W, OP, CC, D) \
+E(OPC, NM, FT, FC, I1, I2, P, W, OP, CC, D, 0)
+
+#define F(OPC, NM, FT, FC, I1, I2, P, W, OP, CC, FL) \
+E(OPC, NM, FT, FC, I1, I2, P, W, OP, CC, 0, FL)
+
+#define E(OPC, NM, FT, FC, I1, I2, P, W, OP, CC, D, FL) insn_ ## NM,
 
 enum DisasInsnEnum {
 #include "insn-data.def"
 };
 
-#undef D
-#define D(OPC, NM, FT, FC, I1, I2, P, W, OP, CC, D) {   \
+#undef E
+#define E(OPC, NM, FT, FC, I1, I2, P, W, OP, CC, D, FL) {   \
 .opc = OPC, \
+.flags = FL,\
 .fmt = FMT_##FT,\
 .fac = FAC_##FC,\
 .spec = SPEC_in1_##I1 | SPEC_in2_##I2 | SPEC_prep_##P | SPEC_wout_##W,  \
@@ -5916,8 +5924,8 @@ static const DisasInsn insn_info[] = {
 #include "insn-data.def"
 };
 
-#undef D
-#define D(OPC, NM, FT, FC, I1, I2, P, W, OP, CC, D) \
+#undef E
+#define E(OPC, NM, FT, FC, I1, I2, P, W, OP, CC, D, FL) \
 case OPC: return &insn_info[insn_ ## NM];
 
 static const DisasInsn *lookup_opc(uint16_t opc)
@@ -5929,6 +5937,8 @@ static const DisasInsn *lookup_opc(uint16_t opc)
 }
 }
 
+#undef F
+#undef E
 #undef D
 #undef C
 
-- 
2.14.4




[Qemu-devel] [PULL 07/15] s390x/tcg: factor out and fix DATA exception injection

2018-10-04 Thread Cornelia Huck
From: David Hildenbrand 

The DXC is to be stored in the low core, and only in the FPC in case AFP
is enabled in CR0. Stub is not required in current code, but this way
we never run into problems.

Reviewed-by: Richard Henderson 
Reviewed-by: Thomas Huth 
Signed-off-by: David Hildenbrand 
Message-Id: <20180927130303.12236-3-da...@redhat.com>
Signed-off-by: Cornelia Huck 
---
 target/s390x/cpu.h |  1 +
 target/s390x/excp_helper.c | 23 +++
 target/s390x/fpu_helper.c  | 13 +++--
 target/s390x/helper.h  |  1 +
 target/s390x/tcg-stub.c|  5 +
 target/s390x/tcg_s390x.h   |  2 ++
 target/s390x/translate.c   | 19 +--
 7 files changed, 44 insertions(+), 20 deletions(-)

diff --git a/target/s390x/cpu.h b/target/s390x/cpu.h
index 6f8861e554..5e50c3a303 100644
--- a/target/s390x/cpu.h
+++ b/target/s390x/cpu.h
@@ -322,6 +322,7 @@ extern const struct VMStateDescription vmstate_s390_cpu;
 #define CR0_LOWPROT 0x1000ULL
 #define CR0_SECONDARY   0x0400ULL
 #define CR0_EDAT0x0080ULL
+#define CR0_AFP 0x0004ULL
 #define CR0_EMERGENCY_SIGNAL_SC 0x4000ULL
 #define CR0_EXTERNAL_CALL_SC0x2000ULL
 #define CR0_CKC_SC  0x0800ULL
diff --git a/target/s390x/excp_helper.c b/target/s390x/excp_helper.c
index 5dab3387c3..cd76c3163a 100644
--- a/target/s390x/excp_helper.c
+++ b/target/s390x/excp_helper.c
@@ -21,6 +21,7 @@
 #include "qemu/osdep.h"
 #include "cpu.h"
 #include "internal.h"
+#include "exec/helper-proto.h"
 #include "qemu/timer.h"
 #include "exec/exec-all.h"
 #include "exec/cpu_ldst.h"
@@ -61,6 +62,28 @@ void QEMU_NORETURN tcg_s390_program_interrupt(CPUS390XState 
*env, uint32_t code,
 cpu_loop_exit(cs);
 }
 
+void QEMU_NORETURN tcg_s390_data_exception(CPUS390XState *env, uint32_t dxc,
+   uintptr_t ra)
+{
+g_assert(dxc <= 0xff);
+#if !defined(CONFIG_USER_ONLY)
+/* Store the DXC into the lowcore */
+stl_phys(CPU(s390_env_get_cpu(env))->as,
+ env->psa + offsetof(LowCore, data_exc_code), dxc);
+#endif
+
+/* Store the DXC into the FPC if AFP is enabled */
+if (env->cregs[0] & CR0_AFP) {
+env->fpc = deposit32(env->fpc, 8, 8, dxc);
+}
+tcg_s390_program_interrupt(env, PGM_DATA, ILEN_AUTO, ra);
+}
+
+void HELPER(data_exception)(CPUS390XState *env, uint32_t dxc)
+{
+tcg_s390_data_exception(env, dxc, GETPC());
+}
+
 #if defined(CONFIG_USER_ONLY)
 
 void s390_cpu_do_interrupt(CPUState *cs)
diff --git a/target/s390x/fpu_helper.c b/target/s390x/fpu_helper.c
index 5c5b451b3b..1b662d2520 100644
--- a/target/s390x/fpu_helper.c
+++ b/target/s390x/fpu_helper.c
@@ -21,6 +21,7 @@
 #include "qemu/osdep.h"
 #include "cpu.h"
 #include "internal.h"
+#include "tcg_s390x.h"
 #include "exec/exec-all.h"
 #include "exec/cpu_ldst.h"
 #include "exec/helper-proto.h"
@@ -40,14 +41,6 @@
  ? (mask / (from / to)) & to\
  : (mask & from) * (to / from))
 
-static void ieee_exception(CPUS390XState *env, uint32_t dxc, uintptr_t retaddr)
-{
-/* Install the DXC code.  */
-env->fpc = (env->fpc & ~0xff00) | (dxc << 8);
-/* Trap.  */
-s390_program_interrupt(env, PGM_DATA, ILEN_AUTO, retaddr);
-}
-
 /* Should be called after any operation that may raise IEEE exceptions.  */
 static void handle_exceptions(CPUS390XState *env, uintptr_t retaddr)
 {
@@ -75,7 +68,7 @@ static void handle_exceptions(CPUS390XState *env, uintptr_t 
retaddr)
 /* Send signals for enabled exceptions.  */
 s390_exc &= env->fpc >> 24;
 if (s390_exc) {
-ieee_exception(env, s390_exc, retaddr);
+tcg_s390_data_exception(env, s390_exc, retaddr);
 }
 }
 
@@ -773,6 +766,6 @@ void HELPER(sfas)(CPUS390XState *env, uint64_t val)
is also 1, a simulated-iee-exception trap occurs.  */
 s390_exc = (signalling >> 16) & (source >> 24);
 if (s390_exc) {
-ieee_exception(env, s390_exc | 3, GETPC());
+tcg_s390_data_exception(env, s390_exc | 3, GETPC());
 }
 }
diff --git a/target/s390x/helper.h b/target/s390x/helper.h
index 97c60ca7bc..018e9dd414 100644
--- a/target/s390x/helper.h
+++ b/target/s390x/helper.h
@@ -1,4 +1,5 @@
 DEF_HELPER_2(exception, noreturn, env, i32)
+DEF_HELPER_2(data_exception, noreturn, env, i32)
 DEF_HELPER_FLAGS_4(nc, TCG_CALL_NO_WG, i32, env, i32, i64, i64)
 DEF_HELPER_FLAGS_4(oc, TCG_CALL_NO_WG, i32, env, i32, i64, i64)
 DEF_HELPER_FLAGS_4(xc, TCG_CALL_NO_WG, i32, env, i32, i64, i64)
diff --git a/target/s390x/tcg-stub.c b/target/s390x/tcg-stub.c
index dc444fc867..32adb7276a 100644
--- a/target/s390x/tcg-stub.c
+++ b/target/s390x/tcg-stub.c
@@ -23,3 +23,8 @@ void QEMU_NORETURN tcg_s390_program_interrupt(CPUS390XState 
*env, uint32_t code,
 {
 g_assert_not_reached();
 }
+void QEMU_NORETURN tcg_s390_data_exception(CPUS390XState *env, uint32_t dxc,
+   uintptr_t

[Qemu-devel] [PULL 14/15] s390x/tcg: refactor specification checking

2018-10-04 Thread Cornelia Huck
From: David Hildenbrand 

We can fit this nicely into less LOC, without harming readability.

Reviewed-by: Richard Henderson 
Reviewed-by: Thomas Huth 
Signed-off-by: David Hildenbrand 
Message-Id: <20180927130303.12236-10-da...@redhat.com>
Signed-off-by: Cornelia Huck 
---
 target/s390x/translate.c | 34 ++
 1 file changed, 6 insertions(+), 28 deletions(-)

diff --git a/target/s390x/translate.c b/target/s390x/translate.c
index f6813d0674..18861cd186 100644
--- a/target/s390x/translate.c
+++ b/target/s390x/translate.c
@@ -6092,34 +6092,12 @@ static DisasJumpType translate_one(CPUS390XState *env, 
DisasContext *s)
 
 /* Check for insn specification exceptions.  */
 if (insn->spec) {
-int spec = insn->spec, excp = 0, r;
-
-if (spec & SPEC_r1_even) {
-r = get_field(&f, r1);
-if (r & 1) {
-excp = PGM_SPECIFICATION;
-}
-}
-if (spec & SPEC_r2_even) {
-r = get_field(&f, r2);
-if (r & 1) {
-excp = PGM_SPECIFICATION;
-}
-}
-if (spec & SPEC_r3_even) {
-r = get_field(&f, r3);
-if (r & 1) {
-excp = PGM_SPECIFICATION;
-}
-}
-if (spec & SPEC_r1_f128 && !is_fp_pair(get_field(&f, r1))) {
-excp = PGM_SPECIFICATION;
-}
-if (spec & SPEC_r2_f128 && !is_fp_pair(get_field(&f, r2))) {
-excp = PGM_SPECIFICATION;
-}
-if (excp) {
-gen_program_exception(s, excp);
+if ((insn->spec & SPEC_r1_even && get_field(&f, r1) & 1) ||
+(insn->spec & SPEC_r2_even && get_field(&f, r2) & 1) ||
+(insn->spec & SPEC_r3_even && get_field(&f, r3) & 1) ||
+(insn->spec & SPEC_r1_f128 && !is_fp_pair(get_field(&f, r1))) ||
+(insn->spec & SPEC_r2_f128 && !is_fp_pair(get_field(&f, r2 {
+gen_program_exception(s, PGM_SPECIFICATION);
 return DISAS_NORETURN;
 }
 }
-- 
2.14.4




[Qemu-devel] [PULL 13/15] s390x/tcg: fix FP register pair checks

2018-10-04 Thread Cornelia Huck
From: David Hildenbrand 

Valid register pairs are 0/2, 1/3, 4/6, 5/7, 8/10, 9/11, 12/14, 13/15.

R1/R2 always selects the lower number, so the current checks are not
correct as e.g. 2/4 could be selected as a pair.

Reviewed-by: Richard Henderson 
Reviewed-by: Thomas Huth 
Signed-off-by: David Hildenbrand 
Message-Id: <20180927130303.12236-9-da...@redhat.com>
Signed-off-by: Cornelia Huck 
---
 target/s390x/translate.c | 20 ++--
 1 file changed, 10 insertions(+), 10 deletions(-)

diff --git a/target/s390x/translate.c b/target/s390x/translate.c
index f93ad20951..f6813d0674 100644
--- a/target/s390x/translate.c
+++ b/target/s390x/translate.c
@@ -6024,6 +6024,12 @@ static bool is_afp_reg(int reg)
 return reg % 2 || reg > 6;
 }
 
+static bool is_fp_pair(int reg)
+{
+/* 0,1,4,5,8,9,12,13: to exclude the others, check for single bit */
+return !(reg & 0x2);
+}
+
 static DisasJumpType translate_one(CPUS390XState *env, DisasContext *s)
 {
 const DisasInsn *insn;
@@ -6106,17 +6112,11 @@ static DisasJumpType translate_one(CPUS390XState *env, 
DisasContext *s)
 excp = PGM_SPECIFICATION;
 }
 }
-if (spec & SPEC_r1_f128) {
-r = get_field(&f, r1);
-if (r > 13) {
-excp = PGM_SPECIFICATION;
-}
+if (spec & SPEC_r1_f128 && !is_fp_pair(get_field(&f, r1))) {
+excp = PGM_SPECIFICATION;
 }
-if (spec & SPEC_r2_f128) {
-r = get_field(&f, r2);
-if (r > 13) {
-excp = PGM_SPECIFICATION;
-}
+if (spec & SPEC_r2_f128 && !is_fp_pair(get_field(&f, r2))) {
+excp = PGM_SPECIFICATION;
 }
 if (excp) {
 gen_program_exception(s, excp);
-- 
2.14.4




Re: [Qemu-devel] [RFC PATCH v2 0/3] acceptance tests: Test firmware checking debug console output

2018-10-04 Thread Alex Bennée


Cleber Rosa  writes:

> On 10/4/18 8:58 AM, Alex Bennée wrote:
>>

>>
>> The error log: https://transfer.sh/lK71v/avocado-errors.log
>>
>
> The core reason is:
>
> 2018-10-04 12:53:16,339 qemu L0270 DEBUG| Output:
> 'qemu-system-aarch64: -machine pc: unsupported machine type\nUse
> -machine help to list supported machines\n'
>
> I'm minutes away from sending a series that adds support for arch and
> machine type defaults.

\o/

I see it now, I shall have a look.

>
> - Cleber.
>
>>>
>>> Since v1: 
>>> https://lists.gnu.org/archive/html/qemu-devel/2018-09/msg03780.html
>>> - more Pythonic context managers (Cleber)
>>> - use wait_for (Cleber)
>>> - use workdir (Cleber)
>>> - use Avocado 65.0 "-p" option for aarch64 (Cleber)
>>> - fixed Q35 incorrect description in cover (Laszlo)
>>> - use ArmVirtQemu name (Laszlo)
>>>
>>> Next:
>>> - address Laszlo comments about correct QEMU options and flash images
>>> - use FDDrainer
>>> - refactor common code in setUp()
>>> - use new avocado.utils.archive gzip features (Cleber)
>>> - test x86_64/aarch64 'arch' tags (Cleber)
>>> - build EDK2 flash images (Laszlo)
>>> - include variable store flash image (Laszlo)
>>> - use virtual disk with UEFI shell script (Laszlo)
>>> - improve OVMF binary selection (releases / snapshots) (Laszlo)
>>> - check ISO images (Laszlo -> Cleber)
>>> - add aexpect tests (Cleber?)
>>>
>>> Regards,
>>>
>>> Phil.
>>>
>>> Philippe Mathieu-Daudé (3):
>>>   acceptance tests: Add SeaBIOS boot and debug console checking test
>>>   acceptance tests: Add EDK2 OVMF boot and debug console checking test
>>>   acceptance tests: Add EDK2 ArmVirtQemu boot and console checking test
>>>
>>>  tests/acceptance/boot_firmware.py | 159 ++
>>>  1 file changed, 159 insertions(+)
>>>  create mode 100644 tests/acceptance/boot_firmware.py
>>
>>
>> --
>> Alex Bennée
>>


--
Alex Bennée



[Qemu-devel] [PULL 06/15] s390x: move tcg_s390_program_interrupt() into TCG code and mark it noreturn

2018-10-04 Thread Cornelia Huck
From: David Hildenbrand 

Move it into TCG-only code and provide a stub. Turn it into noreturn.

As Richard noted, we currently don't log the psw.addr before restoring
the state, fix that by moving (duplicating) the qemu_log_mask in the
tcg/kvm handlers.

Reviewed-by: Richard Henderson 
Reviewed-by: Thomas Huth 
Signed-off-by: David Hildenbrand 
Message-Id: <20180927130303.12236-2-da...@redhat.com>
Signed-off-by: Cornelia Huck 
---
 target/s390x/excp_helper.c | 13 +
 target/s390x/interrupt.c   | 15 +--
 target/s390x/kvm.c |  4 +++-
 target/s390x/tcg-stub.c|  5 +
 target/s390x/tcg_s390x.h   |  2 ++
 5 files changed, 24 insertions(+), 15 deletions(-)

diff --git a/target/s390x/excp_helper.c b/target/s390x/excp_helper.c
index f0ce60cff2..5dab3387c3 100644
--- a/target/s390x/excp_helper.c
+++ b/target/s390x/excp_helper.c
@@ -26,6 +26,7 @@
 #include "exec/cpu_ldst.h"
 #include "hw/s390x/ioinst.h"
 #include "exec/address-spaces.h"
+#include "tcg_s390x.h"
 #ifndef CONFIG_USER_ONLY
 #include "sysemu/sysemu.h"
 #include "hw/s390x/s390_flic.h"
@@ -48,6 +49,18 @@
 do { } while (0)
 #endif
 
+void QEMU_NORETURN tcg_s390_program_interrupt(CPUS390XState *env, uint32_t 
code,
+  int ilen, uintptr_t ra)
+{
+CPUState *cs = CPU(s390_env_get_cpu(env));
+
+cpu_restore_state(cs, ra, true);
+qemu_log_mask(CPU_LOG_INT, "program interrupt at %#" PRIx64 "\n",
+  env->psw.addr);
+trigger_pgm_exception(env, code, ilen);
+cpu_loop_exit(cs);
+}
+
 #if defined(CONFIG_USER_ONLY)
 
 void s390_cpu_do_interrupt(CPUState *cs)
diff --git a/target/s390x/interrupt.c b/target/s390x/interrupt.c
index 25cfb3eef8..a17eff5ebc 100644
--- a/target/s390x/interrupt.c
+++ b/target/s390x/interrupt.c
@@ -15,6 +15,7 @@
 #include "exec/exec-all.h"
 #include "sysemu/kvm.h"
 #include "hw/s390x/ioinst.h"
+#include "tcg_s390x.h"
 #if !defined(CONFIG_USER_ONLY)
 #include "hw/s390x/s390_flic.h"
 #endif
@@ -29,25 +30,11 @@ void trigger_pgm_exception(CPUS390XState *env, uint32_t 
code, uint32_t ilen)
 env->int_pgm_ilen = ilen;
 }
 
-static void tcg_s390_program_interrupt(CPUS390XState *env, uint32_t code,
-   int ilen, uintptr_t ra)
-{
-#ifdef CONFIG_TCG
-trigger_pgm_exception(env, code, ilen);
-cpu_loop_exit_restore(CPU(s390_env_get_cpu(env)), ra);
-#else
-g_assert_not_reached();
-#endif
-}
-
 void s390_program_interrupt(CPUS390XState *env, uint32_t code, int ilen,
 uintptr_t ra)
 {
 S390CPU *cpu = s390_env_get_cpu(env);
 
-qemu_log_mask(CPU_LOG_INT, "program interrupt at %#" PRIx64 "\n",
-  env->psw.addr);
-
 if (kvm_enabled()) {
 kvm_s390_program_interrupt(cpu, code);
 } else if (tcg_enabled()) {
diff --git a/target/s390x/kvm.c b/target/s390x/kvm.c
index 323cb00e6a..78d39b34d0 100644
--- a/target/s390x/kvm.c
+++ b/target/s390x/kvm.c
@@ -36,6 +36,7 @@
 #include "qemu/timer.h"
 #include "qemu/units.h"
 #include "qemu/mmap-alloc.h"
+#include "qemu/log.h"
 #include "sysemu/sysemu.h"
 #include "sysemu/hw_accel.h"
 #include "hw/hw.h"
@@ -1115,7 +1116,8 @@ void kvm_s390_program_interrupt(S390CPU *cpu, uint16_t 
code)
 .type = KVM_S390_PROGRAM_INT,
 .u.pgm.code = code,
 };
-
+qemu_log_mask(CPU_LOG_INT, "program interrupt at %#" PRIx64 "\n",
+  cpu->env.psw.addr);
 kvm_s390_vcpu_interrupt(cpu, &irq);
 }
 
diff --git a/target/s390x/tcg-stub.c b/target/s390x/tcg-stub.c
index c93501db0b..dc444fc867 100644
--- a/target/s390x/tcg-stub.c
+++ b/target/s390x/tcg-stub.c
@@ -18,3 +18,8 @@
 void tcg_s390_tod_updated(CPUState *cs, run_on_cpu_data opaque)
 {
 }
+void QEMU_NORETURN tcg_s390_program_interrupt(CPUS390XState *env, uint32_t 
code,
+  int ilen, uintptr_t ra)
+{
+g_assert_not_reached();
+}
diff --git a/target/s390x/tcg_s390x.h b/target/s390x/tcg_s390x.h
index 4e308aa0ce..d1fe01ef7e 100644
--- a/target/s390x/tcg_s390x.h
+++ b/target/s390x/tcg_s390x.h
@@ -14,5 +14,7 @@
 #define TCG_S390X_H
 
 void tcg_s390_tod_updated(CPUState *cs, run_on_cpu_data opaque);
+void QEMU_NORETURN tcg_s390_program_interrupt(CPUS390XState *env, uint32_t 
code,
+  int ilen, uintptr_t ra);
 
 #endif /* TCG_S390X_H */
-- 
2.14.4




[Qemu-devel] [PULL 02/15] hw/s390x/css: Remove QEMU_PACKED from struct SenseId

2018-10-04 Thread Cornelia Huck
From: Thomas Huth 

The uint16_t member cu_type of struct SenseId is not naturally aligned,
and since the struct is marked with QEMU_PACKED, this can lead to
unaligned memory accesses - which does not work on architectures like
Sparc. Thus remove the QEMU_PACKED here and rather copy the struct
byte by byte when we do copy_sense_id_to_guest().

Signed-off-by: Thomas Huth 
Message-Id: <1538036615-32542-3-git-send-email-th...@redhat.com>
Reviewed-by: David Hildenbrand 
Signed-off-by: Cornelia Huck 
---
 hw/s390x/css.c | 38 ++
 include/hw/s390x/css.h |  2 +-
 2 files changed, 23 insertions(+), 17 deletions(-)

diff --git a/hw/s390x/css.c b/hw/s390x/css.c
index 5a9fe45ce8..04ec5cc970 100644
--- a/hw/s390x/css.c
+++ b/hw/s390x/css.c
@@ -750,20 +750,25 @@ static void sch_handle_halt_func(SubchDev *sch)
 
 }
 
-static void copy_sense_id_to_guest(SenseId *dest, SenseId *src)
+/*
+ * As the SenseId struct cannot be packed (would cause unaligned accesses), we
+ * have to copy the individual fields to an unstructured area using the correct
+ * layout (see SA22-7204-01 "Common I/O-Device Commands").
+ */
+static void copy_sense_id_to_guest(uint8_t *dest, SenseId *src)
 {
 int i;
 
-dest->reserved = src->reserved;
-dest->cu_type = cpu_to_be16(src->cu_type);
-dest->cu_model = src->cu_model;
-dest->dev_type = cpu_to_be16(src->dev_type);
-dest->dev_model = src->dev_model;
-dest->unused = src->unused;
-for (i = 0; i < ARRAY_SIZE(dest->ciw); i++) {
-dest->ciw[i].type = src->ciw[i].type;
-dest->ciw[i].command = src->ciw[i].command;
-dest->ciw[i].count = cpu_to_be16(src->ciw[i].count);
+dest[0] = src->reserved;
+stw_be_p(dest + 1, src->cu_type);
+dest[3] = src->cu_model;
+stw_be_p(dest + 4, src->dev_type);
+dest[6] = src->dev_model;
+dest[7] = src->unused;
+for (i = 0; i < ARRAY_SIZE(src->ciw); i++) {
+dest[8 + i * 4] = src->ciw[i].type;
+dest[9 + i * 4] = src->ciw[i].command;
+stw_be_p(dest + 10 + i * 4, src->ciw[i].count);
 }
 }
 
@@ -1044,9 +1049,10 @@ static int css_interpret_ccw(SubchDev *sch, hwaddr 
ccw_addr,
 break;
 case CCW_CMD_SENSE_ID:
 {
-SenseId sense_id;
+/* According to SA22-7204-01, Sense-ID can store up to 256 bytes */
+uint8_t sense_id[256];
 
-copy_sense_id_to_guest(&sense_id, &sch->id);
+copy_sense_id_to_guest(sense_id, &sch->id);
 /* Sense ID information is device specific. */
 if (check_len) {
 if (ccw.count != sizeof(sense_id)) {
@@ -1060,11 +1066,11 @@ static int css_interpret_ccw(SubchDev *sch, hwaddr 
ccw_addr,
  * have enough place to store at least bytes 0-3.
  */
 if (len >= 4) {
-sense_id.reserved = 0xff;
+sense_id[0] = 0xff;
 } else {
-sense_id.reserved = 0;
+sense_id[0] = 0;
 }
-ccw_dstream_write_buf(&sch->cds, &sense_id, len);
+ccw_dstream_write_buf(&sch->cds, sense_id, len);
 sch->curr_status.scsw.count = ccw_dstream_residual_count(&sch->cds);
 ret = 0;
 break;
diff --git a/include/hw/s390x/css.h b/include/hw/s390x/css.h
index 9da5912921..bec82d0e5b 100644
--- a/include/hw/s390x/css.h
+++ b/include/hw/s390x/css.h
@@ -48,7 +48,7 @@ typedef struct SenseId {
 uint8_t unused;  /* padding byte */
 /* extended part */
 CIW ciw[MAX_CIWS];   /* variable # of CIWs */
-} QEMU_PACKED SenseId;
+} SenseId;   /* Note: No QEMU_PACKED due to unaligned members 
*/
 
 /* Channel measurements, from linux/drivers/s390/cio/cmf.c. */
 typedef struct CMB {
-- 
2.14.4




[Qemu-devel] [PULL 05/15] target/s390x: exception on non-aligned LPSW(E)

2018-10-04 Thread Cornelia Huck
From: Pavel Zbitskiy 

Both LPSW and LPSWE should raise a specification exception when their
operand is not doubleword aligned.

Signed-off-by: Pavel Zbitskiy 
Message-Id: <20180902003322.3428-3-pavel.zbits...@gmail.com>
Reviewed-by: David Hildenbrand 
Signed-off-by: Cornelia Huck 
---
 target/s390x/translate.c | 6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/target/s390x/translate.c b/target/s390x/translate.c
index 7363aabf3a..59b1e5893c 100644
--- a/target/s390x/translate.c
+++ b/target/s390x/translate.c
@@ -2835,7 +2835,8 @@ static DisasJumpType op_lpsw(DisasContext *s, DisasOps *o)
 
 t1 = tcg_temp_new_i64();
 t2 = tcg_temp_new_i64();
-tcg_gen_qemu_ld32u(t1, o->in2, get_mem_index(s));
+tcg_gen_qemu_ld_i64(t1, o->in2, get_mem_index(s),
+MO_TEUL | MO_ALIGN_8);
 tcg_gen_addi_i64(o->in2, o->in2, 4);
 tcg_gen_qemu_ld32u(t2, o->in2, get_mem_index(s));
 /* Convert the 32-bit PSW_MASK into the 64-bit PSW_MASK.  */
@@ -2855,7 +2856,8 @@ static DisasJumpType op_lpswe(DisasContext *s, DisasOps 
*o)
 
 t1 = tcg_temp_new_i64();
 t2 = tcg_temp_new_i64();
-tcg_gen_qemu_ld64(t1, o->in2, get_mem_index(s));
+tcg_gen_qemu_ld_i64(t1, o->in2, get_mem_index(s),
+MO_TEQ | MO_ALIGN_8);
 tcg_gen_addi_i64(o->in2, o->in2, 8);
 tcg_gen_qemu_ld64(t2, o->in2, get_mem_index(s));
 gen_helper_load_psw(cpu_env, t1, t2);
-- 
2.14.4




[Qemu-devel] [PULL 01/15] hw/s390x/ipl: Fix alignment problems of S390IPLState members

2018-10-04 Thread Cornelia Huck
From: Thomas Huth 

The IplParameterBlock and QemuIplParameters structures are declared with
QEMU_PACKED, so the compiler assumes that the structures do not need to
be aligned in memory. Since the are listed after a "bool" within the
S390IPLState, the IplParameterBlock and QemuIplParameters are also indeed
mis-aligned in memory. This causes problems on Sparc during migration, since
we use VMSTATE_UINT16 in vmstate_iplb to access the devno member for example,
and the corresponding migration functions (like qemu_get_be16s) then try to
access a 16-bit value from a misaligned memory address.
The easiest solution to fix this problem is to move the packed structures
to the beginning of the S390IPLState, right after the DeviceState of course
which has to stay first for QOM reasons. But since DeviceState is a non-packed
struct, we can be sure that it will be padded to the correct alignment at the
end. If not, the QEMU_BUILD_BUG_MSG in this patch will tell us.

Signed-off-by: Thomas Huth 
Message-Id: <1538036615-32542-2-git-send-email-th...@redhat.com>
Reviewed-by: David Hildenbrand 
Signed-off-by: Cornelia Huck 
---
 hw/s390x/ipl.h | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/hw/s390x/ipl.h b/hw/s390x/ipl.h
index 4e87b89418..b3a07a12d8 100644
--- a/hw/s390x/ipl.h
+++ b/hw/s390x/ipl.h
@@ -132,15 +132,15 @@ typedef struct QemuIplParameters QemuIplParameters;
 struct S390IPLState {
 /*< private >*/
 DeviceState parent_obj;
+IplParameterBlock iplb;
+QemuIplParameters qipl;
 uint64_t start_addr;
 uint64_t compat_start_addr;
 uint64_t bios_start_addr;
 uint64_t compat_bios_start_addr;
 bool enforce_bios;
-IplParameterBlock iplb;
 bool iplb_valid;
 bool netboot;
-QemuIplParameters qipl;
 /* reset related properties don't have to be migrated or reset */
 enum s390_reset reset_type;
 int reset_cpu_index;
@@ -157,6 +157,7 @@ struct S390IPLState {
 bool iplbext_migration;
 };
 typedef struct S390IPLState S390IPLState;
+QEMU_BUILD_BUG_MSG(offsetof(S390IPLState, iplb) & 3, "alignment of iplb 
wrong");
 
 #define S390_IPL_TYPE_FCP 0x00
 #define S390_IPL_TYPE_CCW 0x02
-- 
2.14.4




[Qemu-devel] [PULL 04/15] s390x: Fence huge pages prior to 3.1

2018-10-04 Thread Cornelia Huck
From: Janosch Frank 

As the kernel has no way of disallowing the start of a huge page
backed VM, we can migrate a running huge backed VM to a host that has
no huge page KVM support.

Let's glue huge page support support to the 3.1 machine, so we do not
migrate to a destination host that doesn't have QEMU huge page support
and can stop migration if KVM doesn't indicate support.

Signed-off-by: Janosch Frank 
Message-Id: <20180928093435.198573-1-fran...@linux.ibm.com>
Reviewed-by: David Hildenbrand 
Signed-off-by: Cornelia Huck 
---
 hw/s390x/s390-virtio-ccw.c | 10 ++
 include/hw/s390x/s390-virtio-ccw.h |  3 +++
 target/s390x/kvm.c |  6 ++
 3 files changed, 19 insertions(+)

diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c
index f0f7fdcadd..53fd7c975f 100644
--- a/hw/s390x/s390-virtio-ccw.c
+++ b/hw/s390x/s390-virtio-ccw.c
@@ -456,6 +456,7 @@ static void ccw_machine_class_init(ObjectClass *oc, void 
*data)
 s390mc->ri_allowed = true;
 s390mc->cpu_model_allowed = true;
 s390mc->css_migration_enabled = true;
+s390mc->hpage_1m_allowed = true;
 mc->init = ccw_init;
 mc->reset = s390_machine_reset;
 mc->hot_add_cpu = s390_hot_add_cpu;
@@ -535,6 +536,12 @@ bool cpu_model_allowed(void)
 return get_machine_class()->cpu_model_allowed;
 }
 
+bool hpage_1m_allowed(void)
+{
+/* for "none" machine this results in true */
+return get_machine_class()->hpage_1m_allowed;
+}
+
 static char *machine_get_loadparm(Object *obj, Error **errp)
 {
 S390CcwMachineState *ms = S390_CCW_MACHINE(obj);
@@ -747,6 +754,9 @@ static void ccw_machine_3_0_instance_options(MachineState 
*machine)
 
 static void ccw_machine_3_0_class_options(MachineClass *mc)
 {
+S390CcwMachineClass *s390mc = S390_MACHINE_CLASS(mc);
+
+s390mc->hpage_1m_allowed = false;
 ccw_machine_3_1_class_options(mc);
 SET_MACHINE_COMPAT(mc, CCW_COMPAT_3_0);
 }
diff --git a/include/hw/s390x/s390-virtio-ccw.h 
b/include/hw/s390x/s390-virtio-ccw.h
index e9c4f4182b..8aa27199c9 100644
--- a/include/hw/s390x/s390-virtio-ccw.h
+++ b/include/hw/s390x/s390-virtio-ccw.h
@@ -39,12 +39,15 @@ typedef struct S390CcwMachineClass {
 bool ri_allowed;
 bool cpu_model_allowed;
 bool css_migration_enabled;
+bool hpage_1m_allowed;
 } S390CcwMachineClass;
 
 /* runtime-instrumentation allowed by the machine */
 bool ri_allowed(void);
 /* cpu model allowed by the machine */
 bool cpu_model_allowed(void);
+/* 1M huge page mappings allowed by the machine */
+bool hpage_1m_allowed(void);
 
 /**
  * Returns true if (vmstate based) migration of the channel subsystem
diff --git a/target/s390x/kvm.c b/target/s390x/kvm.c
index 348e8cc546..323cb00e6a 100644
--- a/target/s390x/kvm.c
+++ b/target/s390x/kvm.c
@@ -292,6 +292,12 @@ static int kvm_s390_configure_mempath_backing(KVMState *s)
 return 0;
 }
 
+if (!hpage_1m_allowed()) {
+error_report("This QEMU machine does not support huge page "
+ "mappings");
+return -EINVAL;
+}
+
 if (path_psize != 1 * MiB) {
 error_report("Memory backing with 2G pages was specified, "
  "but KVM does not support this memory backing");
-- 
2.14.4




[Qemu-devel] [PULL 00/15] s390x updates

2018-10-04 Thread Cornelia Huck
The following changes since commit dafd95053611aa14dda40266857608d12ddce658:

  Merge remote-tracking branch 'remotes/bonzini/tags/for-upstream' into staging 
(2018-10-02 18:27:18 +0100)

are available in the Git repository at:

  git://github.com/cohuck/qemu tags/s390x-20181004

for you to fetch changes up to b576d582ea2b03f4eada186fff59308d22b40a6a:

  hw/s390x/s390-pci-bus: Convert sysbus init function to realize function 
(2018-10-04 12:10:40 +0200)


Various s390x updates:
- fix several struct definitions so that sparc hosts do not trip over
  unaligned accesses
- fence enabling huge pages for pre-3.1 machines
- sysbus init -> realize conversion
- fixes and improvements in tcg (instruction flags and AFP registers)



David Hildenbrand (9):
  s390x: move tcg_s390_program_interrupt() into TCG code and mark it
noreturn
  s390x/tcg: factor out and fix DATA exception injection
  s390x/tcg: store in the TB flags if AFP is enabled
  s390x/tcg: support flags for instructions
  s390x/tcg: add instruction flags for floating point instructions
  s390x/tcg: check for AFP-register, BFP and DFP data exceptions
  s390x/tcg: handle privileged instructions via flags
  s390x/tcg: fix FP register pair checks
  s390x/tcg: refactor specification checking

Janosch Frank (1):
  s390x: Fence huge pages prior to 3.1

Pavel Zbitskiy (1):
  target/s390x: exception on non-aligned LPSW(E)

Thomas Huth (4):
  hw/s390x/ipl: Fix alignment problems of S390IPLState members
  hw/s390x/css: Remove QEMU_PACKED from struct SenseId
  hw/s390x/ioinst: Fix alignment problem in struct SubchDev
  hw/s390x/s390-pci-bus: Convert sysbus init function to realize
function

 hw/s390x/css.c |  38 ++--
 hw/s390x/ipl.h |   5 +-
 hw/s390x/s390-pci-bus.c|  34 ++--
 hw/s390x/s390-virtio-ccw.c |  10 +
 include/hw/s390x/css.h |   6 +-
 include/hw/s390x/ioinst.h  |  21 +-
 include/hw/s390x/s390-virtio-ccw.h |   3 +
 target/s390x/cpu.h |   9 +
 target/s390x/excp_helper.c |  36 
 target/s390x/fpu_helper.c  |  13 +-
 target/s390x/helper.h  |   1 +
 target/s390x/insn-data.def | 395 +++--
 target/s390x/interrupt.c   |  15 +-
 target/s390x/kvm.c |  10 +-
 target/s390x/tcg-stub.c|  10 +
 target/s390x/tcg_s390x.h   |   4 +
 target/s390x/translate.c   | 203 ---
 17 files changed, 430 insertions(+), 383 deletions(-)

-- 
2.14.4




[Qemu-devel] [PULL 03/15] hw/s390x/ioinst: Fix alignment problem in struct SubchDev

2018-10-04 Thread Cornelia Huck
From: Thomas Huth 

struct SubchDev embeds several other structures which are marked with
QEMU_PACKED. This causes the compiler to not care for proper alignment
of these structures. When we later pass around pointers to the unaligned
struct members during migration, this causes problems on host architectures
like Sparc that can not do unaligned memory access.

Most of the structs in ioinst.h are naturally aligned, so we can fix
most of the problem by removing the QEMU_PACKED statements (and use
QEMU_BUILD_BUG_MSG() statements instead to make sure that there is no
padding). However, for the struct SCHIB, we have to keep the QEMU_PACKED
since the compiler adds some padding here otherwise. Move this struct
to the beginning of struct SubchDev instead to fix the alignment problem
here, too.

Signed-off-by: Thomas Huth 
Message-Id: <1538036615-32542-4-git-send-email-th...@redhat.com>
Reviewed-by: David Hildenbrand 
Signed-off-by: Cornelia Huck 
---
 include/hw/s390x/css.h|  4 ++--
 include/hw/s390x/ioinst.h | 21 ++---
 2 files changed, 16 insertions(+), 9 deletions(-)

diff --git a/include/hw/s390x/css.h b/include/hw/s390x/css.h
index bec82d0e5b..aae19c4272 100644
--- a/include/hw/s390x/css.h
+++ b/include/hw/s390x/css.h
@@ -118,11 +118,12 @@ typedef enum IOInstEnding {
 typedef struct SubchDev SubchDev;
 struct SubchDev {
 /* channel-subsystem related things: */
+SCHIB curr_status;   /* Needs alignment and thus must come first */
+ORB orb;
 uint8_t cssid;
 uint8_t ssid;
 uint16_t schid;
 uint16_t devno;
-SCHIB curr_status;
 uint8_t sense_data[32];
 hwaddr channel_prog;
 CCW1 last_cmd;
@@ -131,7 +132,6 @@ struct SubchDev {
 bool thinint_active;
 uint8_t ccw_no_data_cnt;
 uint16_t migrated_schid; /* used for missmatch detection */
-ORB orb;
 CcwDataStream cds;
 /* transport-provided data: */
 int (*ccw_cb) (SubchDev *, CCW1);
diff --git a/include/hw/s390x/ioinst.h b/include/hw/s390x/ioinst.h
index 5f2db6949d..c6737a30d4 100644
--- a/include/hw/s390x/ioinst.h
+++ b/include/hw/s390x/ioinst.h
@@ -25,7 +25,8 @@ typedef struct SCSW {
 uint8_t dstat;
 uint8_t cstat;
 uint16_t count;
-} QEMU_PACKED SCSW;
+} SCSW;
+QEMU_BUILD_BUG_MSG(sizeof(SCSW) != 12, "size of SCSW is wrong");
 
 #define SCSW_FLAGS_MASK_KEY 0xf000
 #define SCSW_FLAGS_MASK_SCTL 0x0800
@@ -94,7 +95,8 @@ typedef struct PMCW {
 uint8_t  pam;
 uint8_t  chpid[8];
 uint32_t chars;
-} QEMU_PACKED PMCW;
+} PMCW;
+QEMU_BUILD_BUG_MSG(sizeof(PMCW) != 28, "size of PMCW is wrong");
 
 #define PMCW_FLAGS_MASK_QF 0x8000
 #define PMCW_FLAGS_MASK_W 0x4000
@@ -127,7 +129,8 @@ typedef struct IRB {
 uint32_t esw[5];
 uint32_t ecw[8];
 uint32_t emw[8];
-} QEMU_PACKED IRB;
+} IRB;
+QEMU_BUILD_BUG_MSG(sizeof(IRB) != 96, "size of IRB is wrong");
 
 /* operation request block */
 typedef struct ORB {
@@ -136,7 +139,8 @@ typedef struct ORB {
 uint8_t lpm;
 uint8_t ctrl1;
 uint32_t cpa;
-} QEMU_PACKED ORB;
+} ORB;
+QEMU_BUILD_BUG_MSG(sizeof(ORB) != 12, "size of ORB is wrong");
 
 #define ORB_CTRL0_MASK_KEY 0xf000
 #define ORB_CTRL0_MASK_SPND 0x0800
@@ -165,7 +169,8 @@ typedef struct CCW0 {
 uint8_t flags;
 uint8_t reserved;
 uint16_t count;
-} QEMU_PACKED CCW0;
+} CCW0;
+QEMU_BUILD_BUG_MSG(sizeof(CCW0) != 8, "size of CCW0 is wrong");
 
 /* channel command word (type 1) */
 typedef struct CCW1 {
@@ -173,7 +178,8 @@ typedef struct CCW1 {
 uint8_t flags;
 uint16_t count;
 uint32_t cda;
-} QEMU_PACKED CCW1;
+} CCW1;
+QEMU_BUILD_BUG_MSG(sizeof(CCW1) != 8, "size of CCW1 is wrong");
 
 #define CCW_FLAG_DC  0x80
 #define CCW_FLAG_CC  0x40
@@ -192,7 +198,8 @@ typedef struct CCW1 {
 typedef struct CRW {
 uint16_t flags;
 uint16_t rsid;
-} QEMU_PACKED CRW;
+} CRW;
+QEMU_BUILD_BUG_MSG(sizeof(CRW) != 4, "size of CRW is wrong");
 
 #define CRW_FLAGS_MASK_S 0x4000
 #define CRW_FLAGS_MASK_R 0x2000
-- 
2.14.4




Re: [Qemu-devel] [PATCH 1/1] tests: Add migration test for aarch64

2018-10-04 Thread Wei Huang



On 10/04/2018 10:07 AM, Philippe Mathieu-Daudé wrote:
> On 28/09/2018 21:47, Wei Huang wrote:
> [...]> +++ b/tests/migration/aarch64/Makefile
>> @@ -0,0 +1,20 @@
>> +# To specify cross compiler prefix, use CROSS_PREFIX=
>> +#   $ make CROSS_PREFIX=aarch64-linux-gnu-
>> +
>> +.PHONY: all clean
>> +all: a-b-kernel.h
>> +
>> +a-b-kernel.h: aarch64.kernel
>> +echo "$$__note" > header.tmp
> 
> This won't work on a read-only fs.

Under which setting? If tmp file can't be generated on a read-only fs,
wouldn't $@ have the same problem?

> 
> Why don't you use $@ directly?
> 
>> +xxd -i $< | sed -e 's/.*int.*//' >> header.tmp
> 
> Please use:
> 
> xxd -g4 ...

This option doesn't work with -i (the include file style output) which
is what we want. From xxd manual:

"-g bytes | -groupsize bytes
separate the output of every  bytes (two hex characters or
eight bit-digits each) by a whitespace. Specify -g 0 to suppress
grouping.   defaults to 2 in normal mode and 1 in bits
mode. Grouping does not apply to postscript or include style."


> 
> xxd might not be installed on the host.

xxd is provided by vim packages. So it should be available in most distros.

> 
> That said we should however install it on the docker cross images.

Agreed.

> 
>> +mv header.tmp $@
>> +
>> +aarch64.kernel: aarch64.elf
>> +$(CROSS_PREFIX)objcopy -O binary $< $@
>> +
>> +aarch64.elf: a-b-kernel.S
>> +$(CROSS_PREFIX)gcc -o $@ -nostdlib -Wl,--build-id=none $<
>> +
>> +clean:
>> +@rm -rf *.kernel *.elf
> 



Re: [Qemu-devel] [RFC] [PATCH 0/3] qemu: arm: Migration between machines with different MIDR values

2018-10-04 Thread Peter Maydell
On 4 October 2018 at 16:05, Andrew Jones  wrote:
> On Tue, Oct 02, 2018 at 02:07:38PM +0100, Peter Maydell wrote:
>> Hi; thanks for this patch. The issue I see with this patch
>> is that the KVM/ARM QEMU approach to system registers so far
>> has been "the kernel knows about these and it is in control".
>> So we ask the kernel for the list of registers, and just save
>> and restore those. That would suggest that if there are sysregs
>> where it's OK in fact to ignore a difference between two constant
>> register values, it should be the kernel doing the "actually, this
>> mismatch is OK" behaviour...
>
> I don't think the kernel should have to maintain all that logic. If a user
> wants to load guest registers, then the kernel should do it, as long as
> it's safe from the host's integrity/security PoV, and the hardware would
> actually support it. Anything that can only break the guest, but not the
> host, should be allowed. The KVM userspace can certainly ask the kernel
> what it recommends first (i.e. read the invariant registers first, before
> deciding what to write), but the decision of what to write should be left
> up to the user.

I can see the logic of that approach. But right now QEMU
userspace knows basically nothing about the system registers
when we're using KVM: all that knowledge is in the kernel.
So we don't have a place really to put policy info (and
not really anywhere to put policy info that depends on the
host CPU type when mostly we let the kernel care about that).

>> For instance, it's probably OK to ignore a MIDR_EL1 difference
>> that just indicates a minor revision bump; but not to ignore
>> one that indicates you just tried to migrate a Cortex-A53
>> over to a Cavium CPU.
>
> If the user does that, then the guest will break - oh well. That's not the
> host kernel's problem.

Patching QEMU to ignore all attempts to write wrong values
to read-only registers would be easy. It just wouldn't
be very helpful to the user...

thanks
-- PMM



Re: [Qemu-devel] [RFC PATCH v2 3/3] acceptance tests: Add EDK2 ArmVirtQemu boot and console checking test

2018-10-04 Thread Philippe Mathieu-Daudé
On 04/10/2018 17:07, Laszlo Ersek wrote:
> On 10/03/18 20:30, Philippe Mathieu-Daudé wrote:
>> This test boots EDK2 ArmVirtQemu and check the debug console (PL011) reports 
>> enough
>> information on the initialized devices.
>>
>> $ avocado run -p qemu_bin=aarch64-softmmu/qemu-system-aarch64 
>> tests/acceptance/boot_firmware.py
>> JOB ID : cb1c5bd9e0312483eabeffbb37885a5273ef23bf
>> JOB LOG: 
>> /home/phil/avocado/job-results/job-2018-10-03T19.39-cb1c5bd/job.log
>>  (1/1) tests/acceptance/boot_firmware.py:BootFirmware.test_ovmf_virt: PASS 
>> (5.02 s)
>> RESULTS: PASS 1 | ERROR 0 | FAIL 0 | SKIP 0 | WARN 0 | INTERRUPT 0 | 
>> CANCEL 0
>> JOB TIME   : 5.30 s
>>
>> Signed-off-by: Philippe Mathieu-Daudé 
>> ---
>> The '-p' option requires Avocado >= 65.0
>> ---
>>  tests/acceptance/boot_firmware.py | 36 +++
>>  1 file changed, 36 insertions(+)
>>
>> diff --git a/tests/acceptance/boot_firmware.py 
>> b/tests/acceptance/boot_firmware.py
>> index 2053b1a4b6..42f0672963 100644
>> --- a/tests/acceptance/boot_firmware.py
>> +++ b/tests/acceptance/boot_firmware.py
>> @@ -121,3 +121,39 @@ class BootFirmware(Test):
>>  debugcon_logger.debug(line.strip())
>>  for exp in expected:
>>  self.assertIn(exp + '\r\n', content)
>> +
>> +def test_ovmf_virt(self):
>> +"""
>> +Boots OVMF on the default virt machine, checks the debug console
>> +
>> +:avocado: tags=arch:aarch64
>> +:avocado: tags=maxtime:20s
>> +"""
>> +image_url = ('http://snapshots.linaro.org/components/kernel/'
>> +'leg-virt-tianocore-edk2-upstream/latest/'
>> +'QEMU-AARCH64/DEBUG_GCC5/QEMU_EFI.img.gz')
>> +image_path_gz = self.fetch_asset(image_url)
>> +image_path = os.path.join(self.workdir, 'flash.img')
>> +
>> +# kludge until Avocado support gzip files
>> +import gzip, shutil
>> +with gzip.open(image_path_gz) as gz, open(image_path, 'wb') as img:
>> +shutil.copyfileobj(gz, img)
>> +
>> +serial_path = os.path.join(self.workdir, 'serial.log')
>> +self.vm.set_machine('virt')
>> +self.vm.add_args('-nographic',
>> + '-cpu', 'cortex-a57',
>> + '-m', '1G', # 1GB min to boot fw?
>> + '-drive', 'file=%s,format=raw,if=pflash' % 
>> image_path,
>> + '-chardev', 'file,path=%s,id=console' % 
>> serial_path,
>> + '-serial', 'chardev:console')
>> +self.vm.launch()
>> +serial_logger = logging.getLogger('serial')
>> +
>> +# serial console checks
>> +if not wait_for(read_console_for_string, timeout=15, step=0,
>> +args=(open(serial_path),
>> +  'Start PXE over IPv4',
>> +  serial_logger)):
>> +self.fail("OVMF failed to boot")
>>
> 
> I suggest replacing "OVMF" with "ArmVirtQemu" in comments and strings as
> well.

Oops OK.

> 
> I'd also suggest renaming the method test_ovmf_virt() to
> test_armvirtqemu_virt().
> 
> If that seems baroque, then I suggest "test_uefi_" in both
> patches #2 and #3.

What about this?

- test_bios_seabios_x86_64_pc()
- test_uefi_ovmf_x86_64_pc()
- test_uefi_armvirtqemu_aarch64_virt()

> 
> (Sorry about my obsession with these names; just trying to capture the
> existing "vernacular".)
> 
> Thanks
> Laszlo
> 



[Qemu-devel] [PATCH 7/7] Acceptance Tests: change the handling of tests for specific archs

2018-10-04 Thread Cleber Rosa
With the introduction of a variants file that can run the same
tests on various architectures, it makes sense to make most tests
to be reusable on those environments.  The exception should be
when a test is really testing a specific architecture feature.

With the change proposed here, on a command line such as:

  $ avocado run \
 --json-variants-load=tests/acceptance/variants/arch.json \
 -- tests/acceptance/

The boot_linux_console.py tests will appear as "CANCELED: Currently
specific to the x86_64 arch", which is as a good thing when compared
to being ignored by tags because:

 * The architecture specific parts can be addressed
 * It will be run on the matching architecture (as opposed to always
   being filtered out by the tags mechanism)
 * CANCELED tests do no influence negatively the overall job results,
   they're not considered an error or failure

Signed-off-by: Cleber Rosa 
---
 tests/acceptance/boot_linux_console.py | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/tests/acceptance/boot_linux_console.py 
b/tests/acceptance/boot_linux_console.py
index 58032f971c..2fb5da63cb 100644
--- a/tests/acceptance/boot_linux_console.py
+++ b/tests/acceptance/boot_linux_console.py
@@ -19,12 +19,13 @@ class BootLinuxConsole(Test):
 and the kernel command line is properly passed from QEMU to the kernel
 
 :avocado: enable
-:avocado: tags=x86_64
 """
 
 timeout = 60
 
 def test(self):
+if self.arch != 'x86_64':
+self.cancel('Currently specific to the x86_64 arch')
 kernel_url = ('https://mirrors.kernel.org/fedora/releases/28/'
   'Everything/x86_64/os/images/pxeboot/vmlinuz')
 kernel_hash = '238e083e114c48200f80d889f7e32eeb2793e02a'
-- 
2.17.1




Re: [Qemu-devel] [RFC] [PATCH 3/3] arm: Skip invariant register restore

2018-10-04 Thread Andrew Jones
On Thu, Sep 27, 2018 at 01:13:54AM +, mja...@caviumnetworks.com wrote:
> From: Manish Jaggi 
> 
> Invariant registers will be skipped from being restored from
> guests' context on migrated host.
> 
> Signed-off-by: Manish Jaggi 
> 
> diff --git a/target/arm/kvm.c b/target/arm/kvm.c
> index 65f867d..2d89600 100644
> --- a/target/arm/kvm.c
> +++ b/target/arm/kvm.c
> @@ -451,6 +451,9 @@ bool write_list_to_kvmstate(ARMCPU *cpu, int level)
>  default:
>  abort();
>  }
> +if (skip_invariant && kvm_arm_is_invariant(&r)) {
> +continue;
> +}
>  ret = kvm_vcpu_ioctl(cs, KVM_SET_ONE_REG, &r);
>  if (ret) {
>  /* We might fail for "unknown register" and also for
> -- 
> 1.8.3.1
> 
>

I think we should compare the invariants we're going to skip restoring
with their saved state and output messages when they don't match to the
migration log. That way when things go wrong we have a clue as to why.

Thanks,
drew



[Qemu-devel] [PATCH 5/7] Acceptance Tests: set machine type

2018-10-04 Thread Cleber Rosa
By setting the machine type, even if it's the one that will be picked
based on the arch, it's possible to run the same tests with targets
that require a machine type (in addition to those that don't).

Given that only boot_linux_console.py contains code specific to x86_64
(an explicit reference to the kernel image that will be used) the
other tests can be used to test different targets:

  $ avocado run -p arch=aarch64 --filter-by-tags='-x86_64' -- tests/acceptance/

Eventually, to reduce boiler plate code, the idea is to concentrate
the basic configuration (arch, machine, console) in either another
utility method, or make that happen by default.  This is of course the
subject of a future discussion.

Signed-off-by: Cleber Rosa 
---
 tests/acceptance/boot_linux_console.py | 3 ++-
 tests/acceptance/version.py| 2 ++
 tests/acceptance/vnc.py| 5 +
 3 files changed, 9 insertions(+), 1 deletion(-)

diff --git a/tests/acceptance/boot_linux_console.py 
b/tests/acceptance/boot_linux_console.py
index 98324f7591..58032f971c 100644
--- a/tests/acceptance/boot_linux_console.py
+++ b/tests/acceptance/boot_linux_console.py
@@ -30,7 +30,8 @@ class BootLinuxConsole(Test):
 kernel_hash = '238e083e114c48200f80d889f7e32eeb2793e02a'
 kernel_path = self.fetch_asset(kernel_url, asset_hash=kernel_hash)
 
-self.vm.set_machine('pc')
+self.vm.set_arch(self.arch)
+self.vm.set_machine()
 self.vm.set_console()
 kernel_command_line = 'console=ttyS0'
 self.vm.add_args('-kernel', kernel_path,
diff --git a/tests/acceptance/version.py b/tests/acceptance/version.py
index 13b0a7440d..7a3a20945f 100644
--- a/tests/acceptance/version.py
+++ b/tests/acceptance/version.py
@@ -18,6 +18,8 @@ class Version(Test):
 :avocado: tags=quick
 """
 def test_qmp_human_info_version(self):
+self.vm.set_arch(self.arch)
+self.vm.set_machine()
 self.vm.launch()
 res = self.vm.command('human-monitor-command',
   command_line='info version')
diff --git a/tests/acceptance/vnc.py b/tests/acceptance/vnc.py
index b1ef9d71b1..4a8a83025f 100644
--- a/tests/acceptance/vnc.py
+++ b/tests/acceptance/vnc.py
@@ -16,6 +16,11 @@ class Vnc(Test):
 :avocado: enable
 :avocado: tags=vnc,quick
 """
+def setUp(self):
+super(Vnc, self).setUp()
+self.vm.set_arch(self.arch)
+self.vm.set_machine()
+
 def test_no_vnc(self):
 self.vm.add_args('-nodefaults', '-S')
 self.vm.launch()
-- 
2.17.1




[Qemu-devel] [PATCH 4/7] scripts/qemu.py: set predefined machine type based on arch

2018-10-04 Thread Cleber Rosa
Some targets require a machine type to be set, as there's no default
(aarch64 is one example).  To give a consistent interface to users of
this API, this changes set_machine() so that a predefined default can
be used, if one is not given.  The approach used is exactly the same
with the console device type.

Also, even when there's a default machine type, for some purposes,
testing included, it's better if outside code is explicit about the
machine type, instead of relying on whatever is set internally.

Signed-off-by: Cleber Rosa 
---
 scripts/qemu.py | 22 +-
 1 file changed, 21 insertions(+), 1 deletion(-)

diff --git a/scripts/qemu.py b/scripts/qemu.py
index d9e24a0c1a..fca9b76990 100644
--- a/scripts/qemu.py
+++ b/scripts/qemu.py
@@ -36,6 +36,15 @@ CONSOLE_DEV_TYPES = {
 r'^s390-ccw-virtio.*': 'sclpconsole',
 }
 
+#: Maps archictures to the preferred machine type
+MACHINE_TYPES = {
+r'^aarch64$': 'virt',
+r'^ppc$': 'g3beige',
+r'^ppc64$': 'pseries',
+r'^s390x$': 's390-ccw-virtio',
+r'^x86_64$': 'q35',
+}
+
 
 class QEMUMachineError(Exception):
 """
@@ -413,13 +422,24 @@ class QEMUMachine(object):
 """
 self._arch = arch
 
-def set_machine(self, machine_type):
+def set_machine(self, machine_type=None):
 '''
 Sets the machine type
 
 If set, the machine type will be added to the base arguments
 of the resulting QEMU command line.
 '''
+if machine_type is None:
+if self._arch is None:
+raise QEMUMachineError("Can not set a default machine type: "
+   "QEMU instance without a defined arch")
+for regex, machine in MACHINE_TYPES.items():
+if re.match(regex, self._arch):
+machine_type = machine
+break
+if machine_type is None:
+raise QEMUMachineError("Can not set a machine type: no "
+   "matching machine type definition")
 self._machine = machine_type
 
 def set_console(self, device_type=None):
-- 
2.17.1




  1   2   3   >