Re: [PATCH 2/5] hw/arm/aspeed: Do not sysbus-map mmio flash region directly, use alias

[Philippe Mathieu-Daudé]

On 3/17/21 7:46 PM, Cédric Le Goater wrote:
> On 3/13/21 1:05 PM, Philippe Mathieu-Daudé wrote:
>> Incorrect subject prefix, should be "hw/ssi/aspeed_smc"
> 
> Is this just good practice or something that was agreed upon ? 

Not sure, maybe "good practice"? Anyway here I only meant to
correct my own patch without respining the series just for this.

> We should update checkpatch if so.

Certainly a waste of time =)

> 
> Thanks,
> 
> C. 

Re: [PATCH v3] hw/block/nvme: align with existing style

[Klaus Jensen]

On Apr 16 09:22, Gollu Appalanaidu wrote:

Use lower case hexadecimal format for the constants and in
comments use the same format as used in Spec. ("h")

Signed-off-by: Gollu Appalanaidu 
---
-v3: Add Suggestions (Philippe)
Describe the NVMe subsystem style in nvme.c header

-v2: Address review comments (Klaus)
use lower case hexa format for the code and in comments
use the same format as used in Spec. ("h")

hw/block/nvme-ns.c   |  2 +-
hw/block/nvme.c  | 46 +---
include/block/nvme.h | 10 +-
3 files changed, 32 insertions(+), 26 deletions(-)

diff --git a/hw/block/nvme-ns.c b/hw/block/nvme-ns.c
index 7bb618f182..a0895614d9 100644
--- a/hw/block/nvme-ns.c
+++ b/hw/block/nvme-ns.c
@@ -303,7 +303,7 @@ static void nvme_ns_init_zoned(NvmeNamespace *ns)

id_ns_z = g_malloc0(sizeof(NvmeIdNsZoned));

-/* MAR/MOR are zeroes-based, 0x means no limit */
+/* MAR/MOR are zeroes-based, Fh means no limit */
id_ns_z->mar = cpu_to_le32(ns->params.max_active_zones - 1);
id_ns_z->mor = cpu_to_le32(ns->params.max_open_zones - 1);
id_ns_z->zoc = 0;
diff --git a/hw/block/nvme.c b/hw/block/nvme.c
index 624a1431d0..cbe7f33daa 100644
--- a/hw/block/nvme.c
+++ b/hw/block/nvme.c
@@ -134,6 +134,12 @@
 * Setting this property to true enables Read Across Zone Boundaries.
 */

+/**
+ * While QEMU coding style prefers lowercase hexadecimal in constants,
+ * the NVMe subsystem use the format from the NVMe specifications in
+ * the comments: no '0x' prefix, uppercase, 'h' hexadecimal suffix
+ */
+
#include "qemu/osdep.h"
#include "qemu/units.h"
#include "qemu/error-report.h"
@@ -3607,18 +3613,18 @@ static uint16_t nvme_io_cmd(NvmeCtrl *n, NvmeRequest 
*req)

/*
 * In the base NVM command set, Flush may apply to all namespaces
- * (indicated by NSID being set to 0x). But if that feature is used
+ * (indicated by NSID being set to h). But if that feature is used
 * along with TP 4056 (Namespace Types), it may be pretty screwed up.
 *
- * If NSID is indeed set to 0x, we simply cannot associate the
+ * If NSID is indeed set to h, we simply cannot associate the
 * opcode with a specific command since we cannot determine a unique I/O
 * command set. Opcode 0x0 could have any other meaning than something
 * equivalent to flushing and say it DOES have completely different
- * semantics in some other command set - does an NSID of 0x then
+ * semantics in some other command set - does an NSID of h then
 * mean "for all namespaces, apply whatever command set specific command
 * that uses the 0x0 opcode?" Or does it mean "for all namespaces, apply
 * whatever command that uses the 0x0 opcode if, and only if, it allows
- * NSID to be 0x"?
+ * NSID to be h"?
 *
 * Anyway (and luckily), for now, we do not care about this since the
 * device only supports namespace types that includes the NVM Flush command
@@ -3934,7 +3940,7 @@ static uint16_t nvme_changed_nslist(NvmeCtrl *n, uint8_t 
rae, uint32_t buf_len,
NVME_CHANGED_NSID_SIZE) {
/*
 * If more than 1024 namespaces, the first entry in the log page should
- * be set to 0x and the others to 0 as spec.
+ * be set to h and the others to 0 as spec.
 */
if (i == ARRAY_SIZE(nslist)) {
memset(nslist, 0x0, sizeof(nslist));
@@ -4332,7 +4338,7 @@ static uint16_t nvme_identify_nslist(NvmeCtrl *n, 
NvmeRequest *req,
trace_pci_nvme_identify_nslist(min_nsid);

/*
- * Both 0x (NVME_NSID_BROADCAST) and 0xfffe are invalid values
+ * Both h (NVME_NSID_BROADCAST) and Eh are invalid values
 * since the Active Namespace ID List should return namespaces with ids
 * *higher* than the NSID specified in the command. This is also specified
 * in the spec (NVM Express v1.3d, Section 5.15.4).
@@ -4379,7 +4385,7 @@ static uint16_t nvme_identify_nslist_csi(NvmeCtrl *n, 
NvmeRequest *req,
trace_pci_nvme_identify_nslist_csi(min_nsid, c->csi);

/*
- * Same as in nvme_identify_nslist(), 0x/0xfffe are invalid.
+ * Same as in nvme_identify_nslist(), h/Eh are invalid.
 */
if (min_nsid >= NVME_NSID_BROADCAST - 1) {
return NVME_INVALID_NSID | NVME_DNR;
@@ -4595,7 +4601,7 @@ static uint16_t nvme_get_feature(NvmeCtrl *n, NvmeRequest 
*req)
/*
 * The Reservation Notification Mask and Reservation Persistence
 * features require a status code of Invalid Field in Command when
- * NSID is 0x. Since the device does not support those
+ * NSID is h. Since the device does not support those
 * features we can always return Invalid Namespace or Format as we
 * should do for all other features.
 

Re: [PATCH 3/8] tests/acceptance/linux_ssh_mips_malta.py: drop identical setUp

[Philippe Mathieu-Daudé]

On 4/15/21 11:51 PM, Cleber Rosa wrote:
> These tests' setUp do not do anything beyong what their base class do.
> And while they do decorate the setUp() we can decorate the classes
> instead, so no functionality is lost here.

This is what I did first when adding this test, but it was not working,
so I had to duplicate it to each method. Did something change so now
this is possible?

> Signed-off-by: Cleber Rosa 
> ---
>  tests/acceptance/linux_ssh_mips_malta.py | 7 ++-
>  1 file changed, 2 insertions(+), 5 deletions(-)
> 
> diff --git a/tests/acceptance/linux_ssh_mips_malta.py 
> b/tests/acceptance/linux_ssh_mips_malta.py
> index 6dbd02d49d..e309a1105c 100644
> --- a/tests/acceptance/linux_ssh_mips_malta.py
> +++ b/tests/acceptance/linux_ssh_mips_malta.py
> @@ -19,6 +19,8 @@
>  from avocado.utils import ssh
>  
>  
> +@skipUnless(os.getenv('AVOCADO_TIMEOUT_EXPECTED'), 'Test might timeout')
> +@skipUnless(ssh.SSH_CLIENT_BINARY, 'No SSH client available')
>  class LinuxSSH(Test):
>  
>  timeout = 150 # Not for 'configure --enable-debug --enable-debug-tcg'
> @@ -65,11 +67,6 @@ def get_kernel_info(self, endianess, wordsize):
>  kernel_hash = self.IMAGE_INFO[endianess]['kernel_hash'][wordsize]
>  return kernel_url, kernel_hash
>  
> -@skipUnless(ssh.SSH_CLIENT_BINARY, 'No SSH client available')
> -@skipUnless(os.getenv('AVOCADO_TIMEOUT_EXPECTED'), 'Test might timeout')
> -def setUp(self):
> -super(LinuxSSH, self).setUp()
> -
>  def get_portfwd(self):
>  res = self.vm.command('human-monitor-command',
>command_line='info usernet')
> 

[PATCH] block/file-posix: Fix problem with fallocate(PUNCH_HOLE) on GPFS

[Thomas Huth]

A customer reported that running

 qemu-img convert -t none -O qcow2 -f qcow2 input.qcow2 output.qcow2

fails for them with the following error message when the images are
stored on a GPFS file system:

 qemu-img: error while writing sector 0: Invalid argument

After analyzing the strace output, it seems like the problem is in
handle_aiocb_write_zeroes(): The call to fallocate(FALLOC_FL_PUNCH_HOLE)
returns EINVAL, which can apparently happen if the file system has
a different idea of the granularity of the operation. It's arguably
a bug in GPFS, since the PUNCH_HOLE mode should not result in EINVAL
according to the man-page of fallocate(), but the file system is out
there in production and so we have to deal with it. In commit 294682cc3a
("block: workaround for unaligned byte range in fallocate()") we also
already applied the a work-around for the same problem to the earlier
fallocate(FALLOC_FL_ZERO_RANGE) call, so do it now similar with the
PUNCH_HOLE call.

Signed-off-by: Thomas Huth 
---
 block/file-posix.c | 7 +++
 1 file changed, 7 insertions(+)

diff --git a/block/file-posix.c b/block/file-posix.c
index 20e14f8e96..7a40428d52 100644
--- a/block/file-posix.c
+++ b/block/file-posix.c
@@ -1675,6 +1675,13 @@ static int handle_aiocb_write_zeroes(void *opaque)
 }
 s->has_fallocate = false;
 } else if (ret != -ENOTSUP) {
+if (ret == -EINVAL) {
+/*
+ * File systems like GPFS do not like unaligned byte ranges,
+ * treat it like unsupported (so caller falls back to pwrite)
+ */
+return -ENOTSUP;
+}
 return ret;
 } else {
 s->has_discard = false;
-- 
2.27.0

Re: [PATCH 8/8] Tests: add custom test jobs

[Philippe Mathieu-Daudé]

Hi Cleber,

On 4/15/21 11:51 PM, Cleber Rosa wrote:
> Different users (or even companies) have different interests, and
> may want to run a reduced set of tests during development, or a
> larger set of tests during QE.
> 
> To cover these use cases, some example (but functional) jobs are
> introduced here:
> 
> 1) acceptance-all-targets.py: runs all arch agnostic tests on all
>built targets, unless there are conditions that make them not work
>out of the box ATM, then run all tests that are specific to
>predefined targets.
> 
> 2) acceptance-kvm-only.py: runs only tests that require KVM and are
>specific to the host architecture.
> 
> 3) qtest-unit.py: runs a combination of qtest and unit tests (in
>parallel).
> 
> 4) qtest-unit-acceptance.py: runs a combineation of qtest, unit tests

Typo "combination".

>and acceptance tests (all of them in parallel)
> 
> To run the first two manually, follow the example bellow:
> 
>  $ cd build
>  $ make check-venv
>  $ ./tests/venv/bin/python3 tests/jobs/acceptance-all-targets.py
>  $ ./tests/venv/bin/python3 tests/jobs/acceptance-kvm-only.py
> 
> The third and fouth example depends on information coming from Meson,
> so the easiest way to run it is:
> 
>  $ cd build
>  $ make check-qtest-unit
>  $ make check-qtest-unit-acceptance
> 
> These are based on Avocado's Job API, a way to customize an Avocado
> job execution beyond the possibilities of command line arguments.
> For more Job API resources, please refer to:
> 
> a) Job API Examples:
>  - https://github.com/avocado-framework/avocado/tree/master/examples/jobs
> 
> b) Documentation about configurable features at the Job Level:
>  - https://avocado-framework.readthedocs.io/en/87.0/config/index.html
> 
> c) Documentation about the TestSuite class
>  - 
> https://avocado-framework.readthedocs.io/en/87.0/api/core/avocado.core.html#avocado.core.suite.TestSuite
> 
> d) Documentation about the Job class
>  - 
> https://avocado-framework.readthedocs.io/en/87.0/api/core/avocado.core.html#avocado.core.job.Job
> 
> Signed-off-by: Cleber Rosa 
> ---
>  configure|  2 +-
>  tests/Makefile.include   |  8 
>  tests/jobs/acceptance-all-targets.py | 67 
>  tests/jobs/acceptance-kvm-only.py| 35 +++
>  tests/jobs/qtest-unit-acceptance.py  | 31 +
>  tests/jobs/qtest-unit.py | 24 ++
>  tests/jobs/utils.py  | 22 +
>  7 files changed, 188 insertions(+), 1 deletion(-)
>  create mode 100644 tests/jobs/acceptance-all-targets.py
>  create mode 100644 tests/jobs/acceptance-kvm-only.py
>  create mode 100644 tests/jobs/qtest-unit-acceptance.py
>  create mode 100644 tests/jobs/qtest-unit.py
>  create mode 100644 tests/jobs/utils.py

> +if __name__ == '__main__':
> +sys.exit(main())
> diff --git a/tests/jobs/acceptance-kvm-only.py 
> b/tests/jobs/acceptance-kvm-only.py
> new file mode 100644
> index 00..acdcbbe087
> --- /dev/null
> +++ b/tests/jobs/acceptance-kvm-only.py
> @@ -0,0 +1,35 @@
> +#!/usr/bin/env python3
> +
> +import os
> +import sys
> +
> +# This comes from tests/acceptance/avocado_qemu/__init__.py and should
> +# not be duplicated here.  The solution is to have the "avocado_qemu"
> +# code and "python/qemu" available during build
> +BUILD_DIR = os.path.dirname(os.path.dirname(os.path.dirname(__file__)))
> +if os.path.islink(os.path.dirname(os.path.dirname(__file__))):
> +# The link to the acceptance tests dir in the source code directory
> +lnk = os.path.dirname(os.path.dirname(__file__))
> +#: The QEMU root source directory
> +SOURCE_DIR = os.path.dirname(os.path.dirname(os.readlink(lnk)))
> +else:
> +SOURCE_DIR = BUILD_DIR
> +sys.path.append(os.path.join(SOURCE_DIR, 'python'))
> +
> +from avocado.core.job import Job
> +
> +from qemu.accel import kvm_available
> +
> +
> +def main():
> +if not kvm_available():
> +sys.exit(0)
> +
> +config = {'run.references': ['tests/acceptance/'],
> +  'filter.by_tags.tags': ['accel:kvm,arch:%s' % os.uname()[4]]}

If we want forks to use their own set of tags, it would be better to
provide an uniform way, not adding new test entry point for each set
of fork tags. Could we consume a YAML config file instead? And provide
templates so forks could adapt to their needs?

Thanks,

Phil.

Re: [PATCH 5/8] tests/acceptance/cpu_queries.py: use the proper logging channels

[Philippe Mathieu-Daudé]

On 4/15/21 11:51 PM, Cleber Rosa wrote:
> The test contains methods for the proper log of test related

"The Test class ..."?

> information.  Let's use that and remove the print and the unused
> logging import.
> 
> Reference: 
> https://avocado-framework.readthedocs.io/en/87.0/api/test/avocado.html#avocado.Test.log

This test predates Avocado 87.0 :) Maybe mention this is an update
to the recent API?

Reviewed-by: Philippe Mathieu-Daudé 

> Signed-off-by: Cleber Rosa 
> ---
>  tests/acceptance/cpu_queries.py | 4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)
> 
> diff --git a/tests/acceptance/cpu_queries.py b/tests/acceptance/cpu_queries.py
> index 293dccb89a..cc9e380cc7 100644
> --- a/tests/acceptance/cpu_queries.py
> +++ b/tests/acceptance/cpu_queries.py
> @@ -8,8 +8,6 @@
>  # This work is licensed under the terms of the GNU GPL, version 2 or
>  # later.  See the COPYING file in the top-level directory.
>  
> -import logging
> -
>  from avocado_qemu import Test
>  
>  class QueryCPUModelExpansion(Test):
> @@ -27,7 +25,7 @@ def test(self):
>  
>  cpus = self.vm.command('query-cpu-definitions')
>  for c in cpus:
> -print(repr(c))
> +self.log.info("Checking CPU: %s", c)
>  self.assertNotIn('', c['unavailable-features'], c['name'])
>  
>  for c in cpus:
> 

Re: [PATCH 4/8] tests/acceptance/migration.py: cancel test if migration is not supported

[Philippe Mathieu-Daudé]

On 4/15/21 11:51 PM, Cleber Rosa wrote:
> FIXME: check if there's a way to query migration support before
> actually requesting migration.
> 
> Some targets/machines contain devices that do not support migration.
> Let's acknowledge that and cancel the test as early as possible.
> 
> Signed-off-by: Cleber Rosa 
> ---
>  tests/acceptance/migration.py | 6 +-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/tests/acceptance/migration.py b/tests/acceptance/migration.py
> index 792639cb69..25ee55f36a 100644
> --- a/tests/acceptance/migration.py
> +++ b/tests/acceptance/migration.py
> @@ -53,7 +53,11 @@ def do_migrate(self, dest_uri, src_uri=None):
>  source_vm = self.get_vm()
>  source_vm.add_args('-nodefaults')
>  source_vm.launch()
> -source_vm.qmp('migrate', uri=src_uri)
> +response = source_vm.qmp('migrate', uri=src_uri)
> +if 'error' in response:
> +if 'desc' in response['error']:
> +msg = response['error']['desc']
> +self.cancel('Migration does not seem to be supported: %s' % msg)
>  self.assert_migration(source_vm, dest_vm)

It would be better to have this done as a generic check_requisites()
method. First because we could reuse it (also at the class level),
second because we could account the time spent for checking separately
from the time spent for the actual testing.

Re: [PING^2] [PATCH] [NFC] Mark locally used symbols as static.

[Palmer Dabbelt]

On Thu, 15 Apr 2021 10:27:42 PDT (-0700), tetra2...@gmail.com wrote:

Hi all,

This patch makes locally used symbols static to enable more compiler
optimizations on them. Some of the symbols turned out to not be used
at all so I marked them with ATTRIBUTE_UNUSED (as I wasn't sure if
they were ok to delete).

The symbols have been identified with a pet project of mine:
https://github.com/yugr/Localizer

Link to patch: 
https://patchew.org/QEMU/cajotw+5ddmsr8qjqxaa1oht79rpmjcrwkybuartynr_ngux...@mail.gmail.com/

From 4e790fd06becfbbf6fb106ac52ae1e4515f1ac73 Mon Sep 17 00:00:00 2001
From: Yury Gribov 
Date: Sat, 20 Mar 2021 23:39:15 +0300
Subject: [PATCH] Mark locally used symbols as static.

Signed-off-by: Yury Gribov 
Acked-by: Max Filippov  (xtensa)
Acked-by: David Gibson  (ppc)
Reviewed-by: Stefan Hajnoczi  (tracetool)
Reviewed-by: Taylor Simpson  (hexagon)
---
 disas/alpha.c | 16 ++--
 disas/m68k.c  | 78 -
 disas/mips.c  | 14 ++--
 disas/nios2.c | 84 +--
 disas/ppc.c   | 26 +++---
 disas/riscv.c |  2 +-
 pc-bios/optionrom/linuxboot_dma.c |  4 +-
 scripts/tracetool/format/c.py |  2 +-
 target/hexagon/gen_dectree_import.c   |  2 +-
 target/hexagon/opcodes.c  |  2 +-
 target/i386/cpu.c |  2 +-
 target/s390x/cpu_models.c |  2 +-
 .../xtensa/core-dc232b/xtensa-modules.c.inc   |  2 +-
 .../xtensa/core-dc233c/xtensa-modules.c.inc   |  2 +-
 target/xtensa/core-de212/xtensa-modules.c.inc |  2 +-
 .../core-de233_fpu/xtensa-modules.c.inc   |  2 +-
 .../xtensa/core-dsp3400/xtensa-modules.c.inc  |  2 +-
 target/xtensa/core-fsf/xtensa-modules.c.inc   |  2 +-
 .../xtensa-modules.c.inc  |  2 +-
 .../core-test_kc705_be/xtensa-modules.c.inc   |  2 +-
 .../core-test_mmuhifi_c3/xtensa-modules.c.inc |  2 +-
 21 files changed, 125 insertions(+), 127 deletions(-)


You might have better luck splitting these sorts of things up: when 
there's a patch that touches a bunch of different trees it can be 
unclear who is going to merge it, which is frequently how these sorts of 
things get lost.



diff --git a/disas/riscv.c b/disas/riscv.c
index 278d9be924..0d124d8b61 100644
--- a/disas/riscv.c
+++ b/disas/riscv.c
@@ -789,7 +789,7 @@ static const rv_comp_data rvcp_fsgnjx_q[] = {

 /* instruction metadata */

-const rv_opcode_data opcode_data[] = {
+static const rv_opcode_data opcode_data[] = {
 { "illegal", rv_codec_illegal, rv_fmt_none, NULL, 0, 0, 0 },
 { "lui", rv_codec_u, rv_fmt_rd_imm, NULL, 0, 0, 0 },
 { "auipc", rv_codec_u, rv_fmt_rd_offset, NULL, 0, 0, 0 },


Reviewed-by: Palmer Dabbelt  (RISC-V)

Thanks!

Re: [PATCH 2/8] Acceptance tests: do not try to reuse packages from the system

[Philippe Mathieu-Daudé]

On 4/15/21 11:51 PM, Cleber Rosa wrote:
> The premise behind the original behavior is that it would save people
> from downloading Avocado (and other dependencies) if already installed
> on the system.  To be honest, I think it's extremely rare that the
> same versions described as dependencies will be available on most
> systems.  But, the biggest motivations here are that:
> 
>  1) Hacking on QEMU in the same system used to develop Avocado leads
> to confusion with regards to the exact bits that are being used;
> 
>  2) Not reusing Python packages from system wide installations gives
> extra assurance that the same behavior will be seen from tests run
> on different machines;
> 
> With regards to downloads, pip already caches the downloaded wheels
> and tarballs under ~/.cache/pip, so there should not be more than
> one download even if the venv is destroyed and recreated.
> 
> Signed-off-by: Cleber Rosa 
> ---
>  tests/Makefile.include | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Reviewed-by: Philippe Mathieu-Daudé 

Re: [PING^2] [PATCH] [NFC] Mark locally used symbols as static.

[Philippe Mathieu-Daudé]

Hi Yuri,

On 4/15/21 7:27 PM, Yuri Gribov wrote:
> Hi all,
> 
> This patch makes locally used symbols static to enable more compiler
> optimizations on them. Some of the symbols turned out to not be used
> at all so I marked them with ATTRIBUTE_UNUSED (as I wasn't sure if
> they were ok to delete).

It would be easier to integrate your work if your split it
in multiple patches, and send them as a series, and add the
Reviewed-by/Acked-by tags to the corresponding ones (these
could be queued via the qemu-trival tree already). See below.

> The symbols have been identified with a pet project of mine:
> https://github.com/yugr/Localizer
> 
> Link to patch: 
> https://patchew.org/QEMU/cajotw+5ddmsr8qjqxaa1oht79rpmjcrwkybuartynr_ngux...@mail.gmail.com/
> 
> From 4e790fd06becfbbf6fb106ac52ae1e4515f1ac73 Mon Sep 17 00:00:00 2001
> From: Yury Gribov 
> Date: Sat, 20 Mar 2021 23:39:15 +0300
> Subject: [PATCH] Mark locally used symbols as static.
> 
> Signed-off-by: Yury Gribov 
> Acked-by: Max Filippov  (xtensa)
> Acked-by: David Gibson  (ppc)
> Reviewed-by: Stefan Hajnoczi  (tracetool)
> Reviewed-by: Taylor Simpson  (hexagon)
> ---
>  disas/alpha.c | 16 ++--
>  disas/m68k.c  | 78 -
>  disas/mips.c  | 14 ++--
>  disas/nios2.c | 84 +--
>  disas/ppc.c   | 26 +++---
>  disas/riscv.c |  2 +-

patch #1, "disas: Mark locally used symbols as static"

>  pc-bios/optionrom/linuxboot_dma.c |  4 +-

patch #2, "pc-bios/optionrom: Mark locally used symbols as static"

>  scripts/tracetool/format/c.py |  2 +-

patch #3, "scripts/tracetool: Mark locally used symbols as static"

>  target/hexagon/gen_dectree_import.c   |  2 +-
>  target/hexagon/opcodes.c  |  2 +-

patch #4, "target/hexagon: Mark locally used symbols as static"

>  target/i386/cpu.c |  2 +-

patch #5, "target/i386: Mark locally used symbols as static"

>  target/s390x/cpu_models.c |  2 +-

patch #6, "target/s390x: Mark locally used symbols as static"

>  .../xtensa/core-dc232b/xtensa-modules.c.inc   |  2 +-
>  .../xtensa/core-dc233c/xtensa-modules.c.inc   |  2 +-
>  target/xtensa/core-de212/xtensa-modules.c.inc |  2 +-
>  .../core-de233_fpu/xtensa-modules.c.inc   |  2 +-
>  .../xtensa/core-dsp3400/xtensa-modules.c.inc  |  2 +-
>  target/xtensa/core-fsf/xtensa-modules.c.inc   |  2 +-
>  .../xtensa-modules.c.inc  |  2 +-
>  .../core-test_kc705_be/xtensa-modules.c.inc   |  2 +-
>  .../core-test_mmuhifi_c3/xtensa-modules.c.inc |  2 +-

patch #7, "target/xtensa: Mark locally used symbols as static"

>  21 files changed, 125 insertions(+), 127 deletions(-)

Regards,

Phil.

Re: [PATCH] target/riscv: fix wfi exception behavior

[Alistair Francis]

On Sun, Apr 11, 2021 at 5:41 AM Jose Martins  wrote:
>
> The wfi exception trigger behavior was not taking into account the fact
> that user mode is not allowed to execute wfi instructions or the effect
> of the hstatus.vtw bit. It was also always generating virtual instruction
> exceptions when this should only happen when the wfi instruction is
> executed when the virtualization mode is enabled:
>
> - when a wfi instruction is executed, an illegal exception should be triggered
> if either the current mode is user or the mode is supervisor and mstatus.tw is
> set.
>
> - a virtual instruction exception should be raised when a wfi is executed from
> virtual-user or virtual-supervisor and hstatus.vtw is set.
>
> Signed-off-by: Jose Martins 
> ---
>  target/riscv/cpu_bits.h  | 1 +
>  target/riscv/op_helper.c | 8 +---
>  2 files changed, 6 insertions(+), 3 deletions(-)
>
> diff --git a/target/riscv/cpu_bits.h b/target/riscv/cpu_bits.h
> index 24b24c69c5..ed8b97c788 100644
> --- a/target/riscv/cpu_bits.h
> +++ b/target/riscv/cpu_bits.h
> @@ -436,6 +436,7 @@
>  #define HSTATUS_HU   0x0200
>  #define HSTATUS_VGEIN0x0003F000
>  #define HSTATUS_VTVM 0x0010
> +#define HSTATUS_VTW  0x0020
>  #define HSTATUS_VTSR 0x0040
>  #if defined(TARGET_RISCV64)
>  #define HSTATUS_VSXL0x3
> diff --git a/target/riscv/op_helper.c b/target/riscv/op_helper.c
> index d55def76cf..354e39ec26 100644
> --- a/target/riscv/op_helper.c
> +++ b/target/riscv/op_helper.c
> @@ -174,9 +174,11 @@ void helper_wfi(CPURISCVState *env)
>  {
>  CPUState *cs = env_cpu(env);
>
> -if ((env->priv == PRV_S &&
> -get_field(env->mstatus, MSTATUS_TW)) ||
> -riscv_cpu_virt_enabled(env)) {
> +if ((!riscv_cpu_virt_enabled(env) && env->priv == PRV_U) ||

> +(env->priv == PRV_S && get_field(env->mstatus, MSTATUS_TW))) {

Shouldn't we check here that we aren't virtualised?

Alistair

> +riscv_raise_exception(env, RISCV_EXCP_ILLEGAL_INST, GETPC());
> +} else if (riscv_cpu_virt_enabled(env) && (env->priv == PRV_U ||
> +(env->priv == PRV_S && get_field(env->hstatus, HSTATUS_VTW {
>  riscv_raise_exception(env, RISCV_EXCP_VIRT_INSTRUCTION_FAULT, 
> GETPC());
>  } else {
>  cs->halted = 1;
> --
> 2.25.1
>
>

Re: [RFC PATCH v2 1/6] device_tree: Add qemu_fdt_add_path

[David Gibson]

On Tue, Apr 13, 2021 at 04:07:40PM +0800, Yanan Wang wrote:
> From: Andrew Jones 
> 
> qemu_fdt_add_path() works like qemu_fdt_add_subnode(), except
> it also adds any missing subnodes in the path. We also tweak
> an error message of qemu_fdt_add_subnode().
> 
> We'll make use of this new function in a coming patch.
> 
> Signed-off-by: Andrew Jones 
> Signed-off-by: Yanan Wang 
> ---
>  include/sysemu/device_tree.h |  1 +
>  softmmu/device_tree.c| 45 ++--
>  2 files changed, 44 insertions(+), 2 deletions(-)
> 
> diff --git a/include/sysemu/device_tree.h b/include/sysemu/device_tree.h
> index 8a2fe55622..ef060a9759 100644
> --- a/include/sysemu/device_tree.h
> +++ b/include/sysemu/device_tree.h
> @@ -121,6 +121,7 @@ uint32_t qemu_fdt_get_phandle(void *fdt, const char 
> *path);
>  uint32_t qemu_fdt_alloc_phandle(void *fdt);
>  int qemu_fdt_nop_node(void *fdt, const char *node_path);
>  int qemu_fdt_add_subnode(void *fdt, const char *name);
> +int qemu_fdt_add_path(void *fdt, const char *path);
>  
>  #define qemu_fdt_setprop_cells(fdt, node_path, property, ...)
>  \
>  do { 
>  \
> diff --git a/softmmu/device_tree.c b/softmmu/device_tree.c
> index 2691c58cf6..8592c7aa1b 100644
> --- a/softmmu/device_tree.c
> +++ b/softmmu/device_tree.c
> @@ -541,8 +541,8 @@ int qemu_fdt_add_subnode(void *fdt, const char *name)
>  
>  retval = fdt_add_subnode(fdt, parent, basename);
>  if (retval < 0) {
> -error_report("FDT: Failed to create subnode %s: %s", name,
> - fdt_strerror(retval));
> +error_report("%s: Failed to create subnode %s: %s",
> + __func__, name, fdt_strerror(retval));
>  exit(1);
>  }
>  
> @@ -550,6 +550,47 @@ int qemu_fdt_add_subnode(void *fdt, const char *name)
>  return retval;
>  }
>  
> +/*
> + * Like qemu_fdt_add_subnode(), but will add all missing
> + * subnodes in the path.
> + */
> +int qemu_fdt_add_path(void *fdt, const char *path)
> +{
> +char *dupname, *basename, *p;
> +int parent, retval = -1;
> +
> +if (path[0] != '/') {
> +return retval;
> +}
> +
> +parent = fdt_path_offset(fdt, "/");

Getting the offset for "/" is never needed - it's always 0.

> +p = dupname = g_strdup(path);

You shouldn't need the strdup(), see below.

> +
> +while (p) {
> +*p = '/';
> +basename = p + 1;
> +p = strchr(p + 1, '/');
> +if (p) {
> +*p = '\0';
> +}
> +retval = fdt_path_offset(fdt, dupname);

The fdt_path_offset_namelen() function exists *exactly* so that you
can look up partial parths without having to mangle your input
string.  Just set the namelen right, and it will ignore anything to
the right of that.

> +if (retval < 0 && retval != -FDT_ERR_NOTFOUND) {
> +error_report("%s: Invalid path %s: %s",
> + __func__, path, fdt_strerror(retval));

If you're getting an error other than FDT_ERR_NOTFOUND here, chances
are it's not an invalid path, but a corrupted fdt blob or something
else.

> +exit(1);
> +} else if (retval == -FDT_ERR_NOTFOUND) {
> +retval = fdt_add_subnode(fdt, parent, basename);
> +if (retval < 0) {
> +break;
> +}
> +}
> +parent = retval;
> +}
> +
> +g_free(dupname);
> +return retval;
> +}
> +
>  void qemu_fdt_dumpdtb(void *fdt, int size)
>  {
>  const char *dumpdtb = current_machine->dumpdtb;

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


signature.asc
Description: PGP signature

Re: [PATCH v4 09/12] qtest/migration-test: Skip tests if KVM not builtin on s390x/ppc64

[David Gibson]

On Thu, Apr 15, 2021 at 06:33:01PM +0200, Philippe Mathieu-Daudé wrote:
> We might have a s390x/ppc64 QEMU binary built without the KVM
> accelerator (configured with --disable-kvm).
> Checking for /dev/kvm accessibility isn't enough, also check for the
> accelerator in the binary.
> 
> Signed-off-by: Philippe Mathieu-Daudé 

Reviewed-by: David Gibson 

> ---
> Cc: David Gibson 
> Cc: Greg Kurz 
> Cc: Halil Pasic 
> Cc: Cornelia Huck 
> Cc: Christian Borntraeger 
> Cc: qemu-...@nongnu.org
> Cc: qemu-s3...@nongnu.org
> ---
>  tests/qtest/migration-test.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/tests/qtest/migration-test.c b/tests/qtest/migration-test.c
> index 3a711bb4929..c32a2aa30a2 100644
> --- a/tests/qtest/migration-test.c
> +++ b/tests/qtest/migration-test.c
> @@ -1408,7 +1408,7 @@ int main(int argc, char **argv)
>   */
>  if (g_str_equal(qtest_get_arch(), "ppc64") &&
>  (access("/sys/module/kvm_hv", F_OK) ||
> - access("/dev/kvm", R_OK | W_OK))) {
> + access("/dev/kvm", R_OK | W_OK) || !qtest_has_accel("kvm"))) {
>  g_test_message("Skipping test: kvm_hv not available");
>  return g_test_run();
>  }
> @@ -1419,7 +1419,7 @@ int main(int argc, char **argv)
>   */
>  if (g_str_equal(qtest_get_arch(), "s390x")) {
>  #if defined(HOST_S390X)
> -if (access("/dev/kvm", R_OK | W_OK)) {
> +if (access("/dev/kvm", R_OK | W_OK) || !qtest_has_accel("kvm")) {
>  g_test_message("Skipping test: kvm not available");
>  return g_test_run();
>  }

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


signature.asc
Description: PGP signature

Re: [EXTERNAL] [PATCH v2 2/4] target/ppc: POWER10 supports scv

[David Gibson]

On Thu, Apr 15, 2021 at 09:43:23AM +0200, Cédric Le Goater wrote:
> On 4/15/21 7:42 AM, Nicholas Piggin wrote:
> > This must have slipped through the cracks between adding POWER10 support
> > and scv support.
> > 
> > Signed-off-by: Nicholas Piggin 
> 
> Reviewed-by: Cédric Le Goater 

Applied to ppc-for-6.1, thanks.

> 
> 
> > ---
> >  target/ppc/translate_init.c.inc | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/target/ppc/translate_init.c.inc 
> > b/target/ppc/translate_init.c.inc
> > index c03a7c4f52..70f9b9b150 100644
> > --- a/target/ppc/translate_init.c.inc
> > +++ b/target/ppc/translate_init.c.inc
> > @@ -9323,7 +9323,7 @@ POWERPC_FAMILY(POWER10)(ObjectClass *oc, void *data)
> >  pcc->flags = POWERPC_FLAG_VRE | POWERPC_FLAG_SE |
> >   POWERPC_FLAG_BE | POWERPC_FLAG_PMM |
> >   POWERPC_FLAG_BUS_CLK | POWERPC_FLAG_CFAR |
> > - POWERPC_FLAG_VSX | POWERPC_FLAG_TM;
> > + POWERPC_FLAG_VSX | POWERPC_FLAG_TM | POWERPC_FLAG_SCV;
> >  pcc->l1_dcache_size = 0x8000;
> >  pcc->l1_icache_size = 0x8000;
> >  pcc->interrupts_big_endian = ppc_cpu_interrupts_big_endian_lpcr;
> > 
> 

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


signature.asc
Description: PGP signature

Re: [PATCH v2 4/4] target/ppc: Add POWER10 exception model

[David Gibson]

On Thu, Apr 15, 2021 at 03:42:27PM +1000, Nicholas Piggin wrote:
> POWER10 adds a new bit that modifies interrupt behaviour, LPCR[HAIL],
> and it removes support for the LPCR[AIL]=0b10 mode.
> 
> Reviewed-by: Cédric Le Goater 
> Tested-by: Cédric Le Goater 
> Signed-off-by: Nicholas Piggin 
> ---
>  hw/ppc/spapr_hcall.c|  7 -
>  target/ppc/cpu-qom.h|  2 ++
>  target/ppc/cpu.h|  5 ++--
>  target/ppc/excp_helper.c| 51 +++--
>  target/ppc/translate.c  |  3 +-
>  target/ppc/translate_init.c.inc |  2 +-
>  6 files changed, 62 insertions(+), 8 deletions(-)
> 
> diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c
> index 2fbe04a689..6802cd4dc8 100644
> --- a/hw/ppc/spapr_hcall.c
> +++ b/hw/ppc/spapr_hcall.c
> @@ -1396,7 +1396,12 @@ static target_ulong 
> h_set_mode_resource_addr_trans_mode(PowerPCCPU *cpu,
>  }
>  
>  if (mflags == 1) {
> -/* AIL=1 is reserved */
> +/* AIL=1 is reserved in POWER8/POWER9 */
> +return H_UNSUPPORTED_FLAG;
> +}
> +
> +if (mflags == 2 && (pcc->insns_flags2 & PPC2_ISA310)) {
> +/* AIL=2 is also reserved in POWER10 (ISA v3.1) */
>  return H_UNSUPPORTED_FLAG;
>  }
>  
> diff --git a/target/ppc/cpu-qom.h b/target/ppc/cpu-qom.h
> index 118baf8d41..06b6571bc9 100644
> --- a/target/ppc/cpu-qom.h
> +++ b/target/ppc/cpu-qom.h
> @@ -116,6 +116,8 @@ enum powerpc_excp_t {
>  POWERPC_EXCP_POWER8,
>  /* POWER9 exception model   */
>  POWERPC_EXCP_POWER9,
> +/* POWER10 exception model   */
> +POWERPC_EXCP_POWER10,
>  };
>  
>  
> /*/
> diff --git a/target/ppc/cpu.h b/target/ppc/cpu.h
> index 5200a16d23..9d35cdfa92 100644
> --- a/target/ppc/cpu.h
> +++ b/target/ppc/cpu.h
> @@ -354,10 +354,11 @@ typedef struct ppc_v3_pate_t {
>  #define LPCR_PECE_U_SHIFT (63 - 19)
>  #define LPCR_PECE_U_MASK  (0x7ull << LPCR_PECE_U_SHIFT)
>  #define LPCR_HVEE PPC_BIT(17) /* Hypervisor Virt Exit Enable */
> -#define LPCR_RMLS_SHIFT   (63 - 37)
> +#define LPCR_RMLS_SHIFT   (63 - 37)   /* RMLS (removed in ISA v3.0) */
>  #define LPCR_RMLS (0xfull << LPCR_RMLS_SHIFT)
> +#define LPCR_HAIL PPC_BIT(37) /* ISA v3.1 HV AIL=3 equivalent */
>  #define LPCR_ILE  PPC_BIT(38)
> -#define LPCR_AIL_SHIFT(63 - 40)  /* Alternate interrupt location */
> +#define LPCR_AIL_SHIFT(63 - 40)   /* Alternate interrupt location */
>  #define LPCR_AIL  (3ull << LPCR_AIL_SHIFT)
>  #define LPCR_UPRT PPC_BIT(41) /* Use Process Table */
>  #define LPCR_EVIRTPPC_BIT(42) /* Enhanced Virtualisation */
> diff --git a/target/ppc/excp_helper.c b/target/ppc/excp_helper.c
> index 964a58cfdc..38a1482519 100644
> --- a/target/ppc/excp_helper.c
> +++ b/target/ppc/excp_helper.c
> @@ -170,7 +170,27 @@ static int powerpc_reset_wakeup(CPUState *cs, 
> CPUPPCState *env, int excp,
>   * +---+
>   *
>   * The difference with POWER9 being that MSR[HV] 0->1 interrupts can be sent 
> to
> - * the hypervisor in AIL mode if the guest is radix.
> + * the hypervisor in AIL mode if the guest is radix. This is good for
> + * performance but allows the guest to influence the AIL of hypervisor
> + * interrupts using its MSR, and also the hypervisor must disallow guest
> + * interrupts (MSR[HV] 0->0) from using AIL if the hypervisor does not want 
> to
> + * use AIL for its MSR[HV] 0->1 interrupts.
> + *
> + * POWER10 addresses those issues with a new LPCR[HAIL] bit that is applied 
> to
> + * interrupts that begin execution with MSR[HV]=1 (so both MSR[HV] 0->1 and
> + * MSR[HV] 1->1).
> + *
> + * HAIL=1 is equivalent to AIL=3, for interrupts delivered with MSR[HV]=1.
> + *
> + * POWER10 behaviour is
> + * | LPCR[AIL] | LPCR[HAIL] | MSR[IR||DR] | MSR[HV] | new MSR[HV] | AIL |
> + * +---++-+-+-+-+
> + * | a | h  | 00/01/10| 0   | 0   | 0   |
> + * | a | h  | 11  | 0   | 0   | a   |
> + * | a | h  | x   | 0   | 1   | h   |
> + * | a | h  | 00/01/10| 1   | 1   | 0   |
> + * | a | h  | 11  | 1   | 1   | h   |
> + * ++
>   */
>  static inline void ppc_excp_apply_ail(PowerPCCPU *cpu, int excp_model, int 
> excp,
>target_ulong msr,
> @@ -210,6 +230,29 @@ static inline void ppc_excp_apply_ail(PowerPCCPU *cpu, 
> int excp_model, int excp,
>  /* AIL=1 is reserved */
>  return;
>  }
> +
> +} else if (excp_model == POWERPC_EXCP_POWER10) {
> +if (!mmu_all_on && !hv_escalation) {
> +/*
> + * AIL works for HV interrupts 

Re: [PATCH v2 3/4] target/ppc: Rework AIL logic in interrupt delivery

[David Gibson]

On Thu, Apr 15, 2021 at 03:42:26PM +1000, Nicholas Piggin wrote:
> The AIL logic is becoming unmanageable spread all over powerpc_excp(),
> and it is slated to get even worse with POWER10 support.
> 
> Move it all to a new helper function.
> 
> Reviewed-by: Cédric Le Goater 
> Tested-by: Cédric Le Goater 
> Signed-off-by: Nicholas Piggin 

Looks like a nice cleanup overall, just a few minor comments.

> ---
>  hw/ppc/spapr_hcall.c|   3 +-
>  target/ppc/cpu.h|   8 --
>  target/ppc/excp_helper.c| 159 
>  target/ppc/translate_init.c.inc |   2 +-
>  4 files changed, 102 insertions(+), 70 deletions(-)
> 
> diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c
> index 7b5cd3553c..2fbe04a689 100644
> --- a/hw/ppc/spapr_hcall.c
> +++ b/hw/ppc/spapr_hcall.c
> @@ -1395,7 +1395,8 @@ static target_ulong 
> h_set_mode_resource_addr_trans_mode(PowerPCCPU *cpu,
>  return H_P4;
>  }
>  
> -if (mflags == AIL_RESERVED) {
> +if (mflags == 1) {
> +/* AIL=1 is reserved */
>  return H_UNSUPPORTED_FLAG;
>  }
>  
> diff --git a/target/ppc/cpu.h b/target/ppc/cpu.h
> index e73416da68..5200a16d23 100644
> --- a/target/ppc/cpu.h
> +++ b/target/ppc/cpu.h
> @@ -2375,14 +2375,6 @@ enum {
>  HMER_XSCOM_STATUS_MASK  = PPC_BITMASK(21, 23),
>  };
>  
> -/* Alternate Interrupt Location (AIL) */
> -enum {
> -AIL_NONE= 0,
> -AIL_RESERVED= 1,
> -AIL_0001_8000   = 2,
> -AIL_C000___4000 = 3,
> -};

Yeah, I always thought these particular constants were a but
pointless.

> -
>  
> /*/
>  
>  #define is_isa300(ctx) (!!(ctx->insns_flags2 & PPC2_ISA300))
> diff --git a/target/ppc/excp_helper.c b/target/ppc/excp_helper.c
> index b8881c0f85..964a58cfdc 100644
> --- a/target/ppc/excp_helper.c
> +++ b/target/ppc/excp_helper.c
> @@ -136,25 +136,105 @@ static int powerpc_reset_wakeup(CPUState *cs, 
> CPUPPCState *env, int excp,
>  return POWERPC_EXCP_RESET;
>  }
>  
> -static uint64_t ppc_excp_vector_offset(CPUState *cs, int ail)
> +/*
> + * AIL - Alternate Interrupt Location, a mode that allows interrupts to be
> + * taken with the MMU on, and which uses an alternate location (e.g., so the
> + * kernel/hv can map the vectors there with an effective address).
> + *
> + * An interrupt is considered to be taken "with AIL" or "AIL applies" if they
> + * are delivered in this way. AIL requires the LPCR to be set to enable this
> + * mode, and then a number of conditions have to be true for AIL to apply.
> + *
> + * First of all, SRESET, MCE, and HMI are always delivered without AIL, 
> because
> + * they specifically want to be in real mode (e.g., the MCE might be 
> signaling
> + * a SLB multi-hit which requires SLB flush before the MMU can be enabled).
> + *
> + * After that, behaviour depends on the current MSR[IR], MSR[DR], MSR[HV],
> + * whether or not the interrupt changes MSR[HV] from 0 to 1, and the current
> + * radix mode (LPCR[HR]).
> + *
> + * POWER8, POWER9 with LPCR[HR]=0
> + * | LPCR[AIL] | MSR[IR||DR] | MSR[HV] | new MSR[HV] | AIL |
> + * +---+-+-+-+-+
> + * | a | 00/01/10| x   | x   | 0   |
> + * | a | 11  | 0   | 1   | 0   |
> + * | a | 11  | 1   | 1   | a   |
> + * | a | 11  | 0   | 0   | a   |
> + * +---+
> + *
> + * POWER9 with LPCR[HR]=1
> + * | LPCR[AIL] | MSR[IR||DR] | MSR[HV] | new MSR[HV] | AIL |
> + * +---+-+-+-+-+
> + * | a | 00/01/10| x   | x   | 0   |
> + * | a | 11  | x   | x   | a   |
> + * +---+
> + *
> + * The difference with POWER9 being that MSR[HV] 0->1 interrupts can be sent 
> to
> + * the hypervisor in AIL mode if the guest is radix.
> + */
> +static inline void ppc_excp_apply_ail(PowerPCCPU *cpu, int excp_model, int 
> excp,
> +  target_ulong msr,
> +  target_ulong *new_msr,
> +  target_ulong *vector)
>  {
> -uint64_t offset = 0;
> +#if defined(TARGET_PPC64)
> +CPUPPCState *env = >env;
> +bool mmu_all_on = ((msr >> MSR_IR) & 1) && ((msr >> MSR_DR) & 1);
> +bool hv_escalation = !(msr & MSR_HVB) && (*new_msr & MSR_HVB);
> +int ail = 0;
> +
> +if (excp == POWERPC_EXCP_MCHECK ||
> +excp == POWERPC_EXCP_RESET ||
> +excp == POWERPC_EXCP_HV_MAINT) {
> +/* SRESET, MCE, HMI never apply AIL */
> +return;
> +}
>  
> -switch (ail) {
> -case AIL_NONE:
> -break;
> -case AIL_0001_8000:
> -offset = 0x18000;
> -break;
> -case 

Re: [PATCH v2 1/4] target/ppc: Fix POWER9 radix guest HV interrupt AIL behaviour

[David Gibson]

On Thu, Apr 15, 2021 at 09:12:21AM -0300, Fabiano Rosas wrote:
> Nicholas Piggin  writes:
> 
> > ISA v3.0 radix guest execution has a quirk in AIL behaviour such that
> > the LPCR[AIL] value can apply to hypervisor interrupts.
> >
> > This affects machines that emulate HV=1 mode (i.e., powernv9).
> >
> > Signed-off-by: Nicholas Piggin 
> 
> Reviewed-by: Fabiano Rosas 

Applied to ppc-for-6.1.

> 
> > ---
> >  target/ppc/excp_helper.c | 17 +
> >  1 file changed, 13 insertions(+), 4 deletions(-)
> >
> > diff --git a/target/ppc/excp_helper.c b/target/ppc/excp_helper.c
> > index 85de7e6c90..b8881c0f85 100644
> > --- a/target/ppc/excp_helper.c
> > +++ b/target/ppc/excp_helper.c
> > @@ -791,14 +791,23 @@ static inline void powerpc_excp(PowerPCCPU *cpu, int 
> > excp_model, int excp)
> >  #endif
> >  
> >  /*
> > - * AIL only works if there is no HV transition and we are running
> > - * with translations enabled
> > + * AIL only works if MSR[IR] and MSR[DR] are both enabled.
> >   */
> > -if (!((msr >> MSR_IR) & 1) || !((msr >> MSR_DR) & 1) ||
> > -((new_msr & MSR_HVB) && !(msr & MSR_HVB))) {
> > +if (!((msr >> MSR_IR) & 1) || !((msr >> MSR_DR) & 1)) {
> >  ail = 0;
> >  }
> >  
> > +/*
> > + * AIL does not work if there is a MSR[HV] 0->1 transition and the
> > + * partition is in HPT mode. For radix guests, such interrupts are
> > + * allowed to be delivered to the hypervisor in ail mode.
> > + */
> > +if ((new_msr & MSR_HVB) && !(msr & MSR_HVB)) {
> > +if (!(env->spr[SPR_LPCR] & LPCR_HR)) {
> > +ail = 0;
> > +}
> > +}
> > +
> >  vector = env->excp_vectors[excp];
> >  if (vector == (target_ulong)-1ULL) {
> >  cpu_abort(cs, "Raised an exception without defined vector %d\n",
> 

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


signature.asc
Description: PGP signature

[PATCH v3] hw/block/nvme: align with existing style

[Gollu Appalanaidu]

Use lower case hexadecimal format for the constants and in
comments use the same format as used in Spec. ("h")

Signed-off-by: Gollu Appalanaidu 
---
-v3: Add Suggestions (Philippe)
 Describe the NVMe subsystem style in nvme.c header

-v2: Address review comments (Klaus)
 use lower case hexa format for the code and in comments
 use the same format as used in Spec. ("h")

 hw/block/nvme-ns.c   |  2 +-
 hw/block/nvme.c  | 46 +---
 include/block/nvme.h | 10 +-
 3 files changed, 32 insertions(+), 26 deletions(-)

diff --git a/hw/block/nvme-ns.c b/hw/block/nvme-ns.c
index 7bb618f182..a0895614d9 100644
--- a/hw/block/nvme-ns.c
+++ b/hw/block/nvme-ns.c
@@ -303,7 +303,7 @@ static void nvme_ns_init_zoned(NvmeNamespace *ns)
 
 id_ns_z = g_malloc0(sizeof(NvmeIdNsZoned));
 
-/* MAR/MOR are zeroes-based, 0x means no limit */
+/* MAR/MOR are zeroes-based, Fh means no limit */
 id_ns_z->mar = cpu_to_le32(ns->params.max_active_zones - 1);
 id_ns_z->mor = cpu_to_le32(ns->params.max_open_zones - 1);
 id_ns_z->zoc = 0;
diff --git a/hw/block/nvme.c b/hw/block/nvme.c
index 624a1431d0..cbe7f33daa 100644
--- a/hw/block/nvme.c
+++ b/hw/block/nvme.c
@@ -134,6 +134,12 @@
  * Setting this property to true enables Read Across Zone Boundaries.
  */
 
+/**
+ * While QEMU coding style prefers lowercase hexadecimal in constants,
+ * the NVMe subsystem use the format from the NVMe specifications in
+ * the comments: no '0x' prefix, uppercase, 'h' hexadecimal suffix
+ */
+
 #include "qemu/osdep.h"
 #include "qemu/units.h"
 #include "qemu/error-report.h"
@@ -3607,18 +3613,18 @@ static uint16_t nvme_io_cmd(NvmeCtrl *n, NvmeRequest 
*req)
 
 /*
  * In the base NVM command set, Flush may apply to all namespaces
- * (indicated by NSID being set to 0x). But if that feature is used
+ * (indicated by NSID being set to h). But if that feature is used
  * along with TP 4056 (Namespace Types), it may be pretty screwed up.
  *
- * If NSID is indeed set to 0x, we simply cannot associate the
+ * If NSID is indeed set to h, we simply cannot associate the
  * opcode with a specific command since we cannot determine a unique I/O
  * command set. Opcode 0x0 could have any other meaning than something
  * equivalent to flushing and say it DOES have completely different
- * semantics in some other command set - does an NSID of 0x then
+ * semantics in some other command set - does an NSID of h then
  * mean "for all namespaces, apply whatever command set specific command
  * that uses the 0x0 opcode?" Or does it mean "for all namespaces, apply
  * whatever command that uses the 0x0 opcode if, and only if, it allows
- * NSID to be 0x"?
+ * NSID to be h"?
  *
  * Anyway (and luckily), for now, we do not care about this since the
  * device only supports namespace types that includes the NVM Flush command
@@ -3934,7 +3940,7 @@ static uint16_t nvme_changed_nslist(NvmeCtrl *n, uint8_t 
rae, uint32_t buf_len,
 NVME_CHANGED_NSID_SIZE) {
 /*
  * If more than 1024 namespaces, the first entry in the log page should
- * be set to 0x and the others to 0 as spec.
+ * be set to h and the others to 0 as spec.
  */
 if (i == ARRAY_SIZE(nslist)) {
 memset(nslist, 0x0, sizeof(nslist));
@@ -4332,7 +4338,7 @@ static uint16_t nvme_identify_nslist(NvmeCtrl *n, 
NvmeRequest *req,
 trace_pci_nvme_identify_nslist(min_nsid);
 
 /*
- * Both 0x (NVME_NSID_BROADCAST) and 0xfffe are invalid values
+ * Both h (NVME_NSID_BROADCAST) and Eh are invalid values
  * since the Active Namespace ID List should return namespaces with ids
  * *higher* than the NSID specified in the command. This is also specified
  * in the spec (NVM Express v1.3d, Section 5.15.4).
@@ -4379,7 +4385,7 @@ static uint16_t nvme_identify_nslist_csi(NvmeCtrl *n, 
NvmeRequest *req,
 trace_pci_nvme_identify_nslist_csi(min_nsid, c->csi);
 
 /*
- * Same as in nvme_identify_nslist(), 0x/0xfffe are invalid.
+ * Same as in nvme_identify_nslist(), h/Eh are invalid.
  */
 if (min_nsid >= NVME_NSID_BROADCAST - 1) {
 return NVME_INVALID_NSID | NVME_DNR;
@@ -4595,7 +4601,7 @@ static uint16_t nvme_get_feature(NvmeCtrl *n, NvmeRequest 
*req)
 /*
  * The Reservation Notification Mask and Reservation Persistence
  * features require a status code of Invalid Field in Command when
- * NSID is 0x. Since the device does not support those
+ * NSID is h. Since the device does not support those
  * features we can always return Invalid Namespace or Format as we
  * should do for all 

Re: [RFC PATCH 1/3] tests/docker: gcc-10 based images for ppc64{, le} tests

[David Gibson]

On Thu, Apr 15, 2021 at 06:41:36PM -0300, matheus.fe...@eldorado.org.br wrote:
> From: Matheus Ferst 
> 
> A newer compiler is needed to build tests for Power10 instructions. As
> done for arm64 on c729a99d2701, new '-test-cross' images are created for
> ppc64 and ppc64le. As done on 936fda4d771f, a test for compiler support
> is added to verify that the toolchain in use has '-mpower10'.
> 
> Signed-off-by: Matheus Ferst 
> ---
>  tests/docker/Makefile.include   |  4 
>  .../dockerfiles/debian-ppc64-test-cross.docker  | 13 +
>  .../debian-ppc64el-test-cross.docker| 17 +
>  tests/tcg/configure.sh  | 12 
>  4 files changed, 42 insertions(+), 4 deletions(-)
>  create mode 100644 tests/docker/dockerfiles/debian-ppc64-test-cross.docker
>  create mode 100644 tests/docker/dockerfiles/debian-ppc64el-test-cross.docker
> 
> diff --git a/tests/docker/Makefile.include b/tests/docker/Makefile.include
> index 9f464cb92c..1f8941d290 100644
> --- a/tests/docker/Makefile.include
> +++ b/tests/docker/Makefile.include
> @@ -152,10 +152,14 @@ docker-image-debian-sparc64-cross: docker-image-debian10
>  docker-image-debian-tricore-cross: docker-image-debian10
>  docker-image-debian-all-test-cross: docker-image-debian10
>  docker-image-debian-arm64-test-cross: docker-image-debian11
> +docker-image-debian-ppc64-test-cross: docker-image-debian11
> +docker-image-debian-ppc64el-test-cross: docker-image-debian11
>  
>  # These images may be good enough for building tests but not for test builds
>  DOCKER_PARTIAL_IMAGES += debian-alpha-cross
>  DOCKER_PARTIAL_IMAGES += debian-arm64-test-cross
> +DOCKER_PARTIAL_IMAGES += debian-ppc64-test-cross
> +DOCKER_PARTIAL_IMAGES += debian-ppc64el-test-cross
>  DOCKER_PARTIAL_IMAGES += debian-hppa-cross
>  DOCKER_PARTIAL_IMAGES += debian-m68k-cross debian-mips64-cross
>  DOCKER_PARTIAL_IMAGES += debian-powerpc-cross debian-ppc64-cross

Why are you adding new images, rather than updating the existing
debian-powerpc-cross image?  I don't think you should need separate
ppc64 and ppc64el images, a single image with a gcc that can target
both should suffice.  (Also, it's typically ppc64le, not ppc64el,
which, yes, is different from what the mips and arm people do for no
particularly good reason).

> diff --git a/tests/docker/dockerfiles/debian-ppc64-test-cross.docker 
> b/tests/docker/dockerfiles/debian-ppc64-test-cross.docker
> new file mode 100644
> index 00..66abfdeb47
> --- /dev/null
> +++ b/tests/docker/dockerfiles/debian-ppc64-test-cross.docker
> @@ -0,0 +1,13 @@
> +#
> +# Docker ppc64 cross-compiler target (tests only)
> +#
> +# This docker target builds on the debian Bullseye base image.
> +#
> +FROM qemu/debian11
> +
> +# Add the foreign architecture we want and install dependencies
> +RUN dpkg --add-architecture ppc64
> +RUN apt update && \
> +DEBIAN_FRONTEND=noninteractive eatmydata \
> +apt install -y --no-install-recommends \
> +libc6-dev-ppc64-cross gcc-10-powerpc64-linux-gnu
> diff --git a/tests/docker/dockerfiles/debian-ppc64el-test-cross.docker 
> b/tests/docker/dockerfiles/debian-ppc64el-test-cross.docker
> new file mode 100644
> index 00..7582508467
> --- /dev/null
> +++ b/tests/docker/dockerfiles/debian-ppc64el-test-cross.docker
> @@ -0,0 +1,17 @@
> +#
> +# Docker ppc64el cross-compiler target (tests only)
> +#
> +# This docker target builds on the debian Bullseye base image.
> +#
> +FROM qemu/debian11
> +
> +# Add the foreign architecture we want and install dependencies
> +RUN dpkg --add-architecture ppc64el
> +RUN apt update && \
> +DEBIAN_FRONTEND=noninteractive eatmydata \
> +apt install -y --no-install-recommends \
> +crossbuild-essential-ppc64el gcc-10-powerpc64le-linux-gnu
> +
> +# Specify the cross prefix for this image (see tests/docker/common.rc)
> +#ENV QEMU_CONFIGURE_OPTS --cross-prefix=powerpc64le-linux-gnu-
> +#ENV DEF_TARGET_LIST ppc64-softmmu,ppc64-linux-user,ppc64abi32-linux-user
> diff --git a/tests/tcg/configure.sh b/tests/tcg/configure.sh
> index fa1a4261a4..5f5db91a01 100755
> --- a/tests/tcg/configure.sh
> +++ b/tests/tcg/configure.sh
> @@ -170,13 +170,13 @@ for target in $target_list; do
>;;
>  ppc64-*)
>container_hosts=x86_64
> -  container_image=debian-ppc64-cross
> -  container_cross_cc=powerpc64-linux-gnu-gcc
> +  container_image=debian-ppc64-test-cross
> +  container_cross_cc=powerpc64-linux-gnu-gcc-10
>;;
>  ppc64le-*)
>container_hosts=x86_64
> -  container_image=debian-ppc64el-cross
> -  container_cross_cc=powerpc64le-linux-gnu-gcc
> +  container_image=debian-ppc64el-test-cross
> +  container_cross_cc=powerpc64le-linux-gnu-gcc-10
>;;
>  riscv64-*)
>container_hosts=x86_64
> @@ -280,6 +280,10 @@ for target in $target_list; do
> -mpower8-vector -o $TMPE $TMPC; then
>  echo 

Re: [PATCH v3] target/ppc: code motion from translate_init.c.inc to gdbstub.c

[David Gibson]

On Wed, Apr 14, 2021 at 01:09:19PM -0700, Richard Henderson wrote:
> On 4/14/21 7:59 AM, Bruno Larsen (billionai) wrote:
> > All the code related to gdb has been moved from translate_init.c.inc
> > file to the gdbstub.c file, where it makes more sense.
> > 
> > This new version puts the prototypes in internal.h, to not expose
> > them unnecessarily.
> > 
> > Signed-off-by: Bruno Larsen (billionai) 
> > Suggested-by: Fabiano Rosas 
> > ---
> >   target/ppc/gdbstub.c| 258 
> >   target/ppc/internal.h   |   5 +
> >   target/ppc/translate_init.c.inc | 254 +--
> >   3 files changed, 264 insertions(+), 253 deletions(-)
> 
> Reviewed-by: Richard Henderson 

Applied to ppc-for-6.1, thanks.

> 
> > +void ppc_gdb_init(CPUState *cs, PowerPCCPUClass *pcc)
> > +{
> > +
> > +if (pcc->insns_flags & PPC_FLOAT) {
> 
> Watch the extra blank lines.

Fixed in my tree.

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


signature.asc
Description: PGP signature

Re: [RFC PATCH 0/3] tests/tcg/ppc64le: paddi tests

[David Gibson]

On Thu, Apr 15, 2021 at 06:41:35PM -0300, matheus.fe...@eldorado.org.br wrote:
> From: Matheus Ferst 
> 
> Based-on: <20210413211129.457272-1-luis.pi...@eldorado.org.br>

First things first: it's unclear to me if this is testing stuff that's
already merged, or it's speculative tests for the in-progress prefixed
instruction stuff.  i.e. If these tests are applied right now, will
they pass?

> This series adds gcc-10 based images to enable the build of tests with Power10
> instructions. Then two tests for paddi are added:
> - The first one checks a weird behavior observed on POWER10 Functional 
> Simulator
>   1.1.0, where the 34-bit immediate is treated as a 32-bits one;
> - The second one exercises the R=1 path of paddi, where CIA is used instead 
> of RA.
>   The test is failing with the current implementation because we use cpu_nip,
>   which is not updated all the time. Luis already has the fix, it should be
>   applied on the next version of his patch series.
> 
> The main reason to submit this patch as an RFC first is the docker part. I 
> would
> lie if I tell you that I understand half of what is going on there.
>  - 'make docker-test-tcg' fails, but apparently on unrelated things;
>  - 'make docker-run-test-tcg@debian-ppc64el-cross' passes, but it looks
>like the test is skipped?
>  - 'make check-tcg' runs the test and passes (with the fix in place for the
>second).

What sort of host was that on?  Unfortunately 'make check-tcg' has
been broken on a POWER host for some time, and I've never had time to
look into it.

> 
> Finally, get_maintainer.pl found no maintainers for
> tests/tcg/ppc64{,le}/Makefile.target. Would it be Mr. Gibson?

Uh... sorta?  I also don't know much about what's going on here, but
I'm probably maintainer by default.


> 
> Thanks,
> Matheus K. Ferst
> 
> Matheus Ferst (3):
>   tests/docker: gcc-10 based images for ppc64{,le} tests
>   tests/tcg/ppc64le: load 33-bits constant with paddi
>   tests/tcg/ppc64le: R=1 test for paddi
> 
>  tests/docker/Makefile.include |  4 +++
>  .../debian-ppc64-test-cross.docker| 13 ++
>  .../debian-ppc64el-test-cross.docker  | 17 
>  tests/tcg/configure.sh| 12 ++---
>  tests/tcg/ppc64/Makefile.target   |  6 +
>  tests/tcg/ppc64le/Makefile.target |  6 +
>  tests/tcg/ppc64le/pla.c   | 26 +++
>  tests/tcg/ppc64le/pli_33bits.c| 22 
>  8 files changed, 102 insertions(+), 4 deletions(-)
>  create mode 100644 tests/docker/dockerfiles/debian-ppc64-test-cross.docker
>  create mode 100644 tests/docker/dockerfiles/debian-ppc64el-test-cross.docker
>  create mode 100644 tests/tcg/ppc64le/pla.c
>  create mode 100644 tests/tcg/ppc64le/pli_33bits.c
> 

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


signature.asc
Description: PGP signature

Re: [PATCH 1/8] Acceptance Jobs: preserve the cache for pip on GitLab CI

[Thomas Huth]

On 15/04/2021 23.51, Cleber Rosa wrote:

The acceptance jobs (via `make check-venv`) will setup a virtual
environment, and after that install packages defined in
tests/requirements.txt via pip.

Let's enable pip's default cache directory, so that we can save
a bit on time/bandwidth.

Signed-off-by: Cleber Rosa 
---
  .gitlab-ci.yml | 1 +
  1 file changed, 1 insertion(+)

diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml
index 52d65d6c04..9cc4676912 100644
--- a/.gitlab-ci.yml
+++ b/.gitlab-ci.yml
@@ -53,6 +53,7 @@ include:
  key: "${CI_JOB_NAME}-cache"
  paths:
- ${CI_PROJECT_DIR}/avocado-cache
+  - ~/.cache/pip


Did you check whether this works? AFAIK the cache directories have to be 
part of the project directory, see:


 https://docs.gitlab.com/ee/ci/yaml/README.html#cache

We already tried to cache ~/avocado/data/cache in the past, but it did not 
work and we had to move the cache manually to the current working directory 
(see commit 5896c539547).


 Thomas

[Bug 1924669] [NEW] VFP code cannot see CPACR write in the same TB

[Hansni Bu]

Public bug reported:

If FPU is enabled by writing to CPACR, and the code is in the same
translation block as the following VFP code, qemu generates "v7M NOCP
UsageFault".

This can be reproduced with git HEAD (commit
8fe9f1f891eff4e37f82622b7480ee748bf4af74).

The target binary is attached. The qemu command is:
qemu-system-arm -nographic -monitor null -serial null -semihosting -machine 
mps2-an505 -cpu cortex-m33 -kernel cpacr_vfp.elf -d 
in_asm,int,exec,cpu,cpu_reset,unimp,guest_errors,nochain -D log

If the code is changed a little, so that they are not in the same block,
VFP code can see the effect of CPACR, or -singlestep of qemu has the
same result.

** Affects: qemu
 Importance: Undecided
 Status: New


** Tags: arm vfp

** Attachment added: "cpacr_vfp.elf"
   
https://bugs.launchpad.net/bugs/1924669/+attachment/5488612/+files/cpacr_vfp.elf

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

Title:
  VFP code cannot see CPACR write in the same TB

Status in QEMU:
  New

Bug description:
  If FPU is enabled by writing to CPACR, and the code is in the same
  translation block as the following VFP code, qemu generates "v7M NOCP
  UsageFault".

  This can be reproduced with git HEAD (commit
  8fe9f1f891eff4e37f82622b7480ee748bf4af74).

  The target binary is attached. The qemu command is:
  qemu-system-arm -nographic -monitor null -serial null -semihosting -machine 
mps2-an505 -cpu cortex-m33 -kernel cpacr_vfp.elf -d 
in_asm,int,exec,cpu,cpu_reset,unimp,guest_errors,nochain -D log

  If the code is changed a little, so that they are not in the same
  block, VFP code can see the effect of CPACR, or -singlestep of qemu
  has the same result.

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

[PATCH v5] i386/cpu_dump: support AVX512 ZMM regs dump

[Robert Hoo]

Since commit fa4518741e (target-i386: Rename struct XMMReg to ZMMReg),
CPUX86State.xmm_regs[] has already been extended to 512bit to support
AVX512.
Also, other qemu level supports for AVX512 registers are there for
years.
But in x86_cpu_dump_state(), still only dump XMM registers no matter
YMM/ZMM is enabled.
This patch is to complement this, let it dump XMM/YMM/ZMM accordingly.

Signed-off-by: Robert Hoo 
---
Changelog:
v5: fix a minor issue. rebase to latest master.
v4: stringent AVX512 case and AVX case judgement criteria
v3: fix some coding style issue.
v2: dump XMM/YMM/ZMM according to XSAVE state-components enablement.
 target/i386/cpu-dump.c | 62 ++
 1 file changed, 47 insertions(+), 15 deletions(-)

diff --git a/target/i386/cpu-dump.c b/target/i386/cpu-dump.c
index aac21f1..ece64e1 100644
--- a/target/i386/cpu-dump.c
+++ b/target/i386/cpu-dump.c
@@ -478,6 +478,11 @@ void x86_cpu_dump_state(CPUState *cs, FILE *f, int flags)
 qemu_fprintf(f, "EFER=%016" PRIx64 "\n", env->efer);
 if (flags & CPU_DUMP_FPU) {
 int fptag;
+const uint64_t avx512_mask = XSTATE_OPMASK_MASK | \
+ XSTATE_ZMM_Hi256_MASK | \
+ XSTATE_Hi16_ZMM_MASK | \
+ XSTATE_YMM_MASK | XSTATE_SSE_MASK,
+   avx_mask = XSTATE_YMM_MASK | XSTATE_SSE_MASK;
 fptag = 0;
 for(i = 0; i < 8; i++) {
 fptag |= ((!env->fptags[i]) << i);
@@ -499,21 +504,48 @@ void x86_cpu_dump_state(CPUState *cs, FILE *f, int flags)
 else
 qemu_fprintf(f, " ");
 }
-if (env->hflags & HF_CS64_MASK)
-nb = 16;
-else
-nb = 8;
-for(i=0;ixmm_regs[i].ZMM_L(3),
- env->xmm_regs[i].ZMM_L(2),
- env->xmm_regs[i].ZMM_L(1),
- env->xmm_regs[i].ZMM_L(0));
-if ((i & 1) == 1)
-qemu_fprintf(f, "\n");
-else
-qemu_fprintf(f, " ");
+
+if ((env->xcr0 & avx512_mask) == avx512_mask) {
+/* XSAVE enabled AVX512 */
+for (i = 0; i < NB_OPMASK_REGS; i++) {
+qemu_fprintf(f, "Opmask%02d=%016lx%s", i, env->opmask_regs[i],
+((i & 3) == 3) ? "\n" : " ");
+}
+
+nb = (env->hflags & HF_CS64_MASK) ? 32 : 8;
+for (i = 0; i < nb; i++) {
+qemu_fprintf(f, "ZMM%02d=%016lx %016lx %016lx %016lx %016lx "
+"%016lx %016lx %016lx\n",
+ i,
+ env->xmm_regs[i].ZMM_Q(7),
+ env->xmm_regs[i].ZMM_Q(6),
+ env->xmm_regs[i].ZMM_Q(5),
+ env->xmm_regs[i].ZMM_Q(4),
+ env->xmm_regs[i].ZMM_Q(3),
+ env->xmm_regs[i].ZMM_Q(2),
+ env->xmm_regs[i].ZMM_Q(1),
+ env->xmm_regs[i].ZMM_Q(0));
+}
+} else if ((env->xcr0 & avx_mask)  == avx_mask) {
+/* XSAVE enabled AVX */
+nb = env->hflags & HF_CS64_MASK ? 16 : 8;
+for (i = 0; i < nb; i++) {
+qemu_fprintf(f, "YMM%02d=%016lx %016lx %016lx %016lx\n",
+ i,
+ env->xmm_regs[i].ZMM_Q(3),
+ env->xmm_regs[i].ZMM_Q(2),
+ env->xmm_regs[i].ZMM_Q(1),
+ env->xmm_regs[i].ZMM_Q(0));
+}
+} else { /* SSE and below cases */
+nb = env->hflags & HF_CS64_MASK ? 16 : 8;
+for (i = 0; i < nb; i++) {
+qemu_fprintf(f, "XMM%02d=%016lx %016lx%s",
+ i,
+ env->xmm_regs[i].ZMM_Q(1),
+ env->xmm_regs[i].ZMM_Q(0),
+ (i & 1) ? "\n" : " ");
+}
 }
 }
 if (flags & CPU_DUMP_CODE) {
-- 
1.8.3.1

Re: [PATCH v2] hw/block/nvme: align with existing style

[Gollu Appalanaidu]

On Thu, Apr 15, 2021 at 07:18:50PM +0200, Klaus Jensen wrote:

On Apr 15 15:13, Philippe Mathieu-Daudé wrote:

On 4/15/21 2:00 PM, Gollu Appalanaidu wrote:

Make uniform hexadecimal numbers format.

Signed-off-by: Gollu Appalanaidu 
---
-v2: Address review comments (Klaus)
use lower case hexa format for the code and in comments
use the same format as used in Spec. ("h")


^ This comment is relevant to the commit message.

Also it would be nice if the subsystem could describe somewhere
what is its style. Not sure where... The file header is probably
the simplest place.

Something like:

"While QEMU coding style prefers lowercase hexadecimal in constants,
the NVMe subsystem use the format from the NVMe specifications in
the comments: no '0x' prefix, uppercase, 'h' hexadecimal suffix."



+1 for that suggestion.



Sure, will add it and send v3.


hw/block/nvme-ns.c   |  2 +-
hw/block/nvme.c  | 40 
include/block/nvme.h | 10 +-
3 files changed, 26 insertions(+), 26 deletions(-)

RE: [PATCH V4 4/7] hmp-commands: Add new HMP command for COLO passthrough

[Zhang, Chen]

> -Original Message-
> From: Dr. David Alan Gilbert 
> Sent: Wednesday, March 24, 2021 6:40 PM
> To: Zhang, Chen 
> Cc: Jason Wang ; qemu-dev  de...@nongnu.org>; Eric Blake ; Markus Armbruster
> ; Li Zhijian ; Zhang Chen
> ; Lukas Straub 
> Subject: Re: [PATCH V4 4/7] hmp-commands: Add new HMP command for
> COLO passthrough
> 
> * Zhang Chen (chen.zh...@intel.com) wrote:
> > Add hmp_colo_passthrough_add and hmp_colo_passthrough_del make
> user
> > can maintain COLO network passthrough list in human monitor.
> >
> > Signed-off-by: Zhang Chen 
> > ---
> >  hmp-commands.hx   | 26 ++
> >  include/monitor/hmp.h |  2 ++
> >  monitor/hmp-cmds.c| 34 ++
> >  3 files changed, 62 insertions(+)
> >
> > diff --git a/hmp-commands.hx b/hmp-commands.hx index
> > d4001f9c5d..b67a5a04cb 100644
> > --- a/hmp-commands.hx
> > +++ b/hmp-commands.hx
> > @@ -1335,6 +1335,32 @@ SRST
> >Remove host network device.
> >  ERST
> >
> > +{
> > +.name   = "colo_passthrough_add",
> > +.args_type  =
> "protocol:s,id:s?,src_ip:s?,dst_ip:s?,src_port:i?,dst_port:i?",
> > +.params = "protocol [id] [src_ip] [dst_ip] [src_port] 
> > [dst_port]",
> 
> That ordering is a bit unnatural; it's normal to keep ip and port together;
> maybe this would be better as:
> 
>protocol:s,id:s,src:s?,dst:s?
> 
> then pass src and dst through inet_parse to get your hostname and port ?

OK, already update to V5, please review it.

Thanks
Chen

> 
> Dave
> 
> > +.help   = "Add network stream to colo passthrough list",
> > +.cmd= hmp_colo_passthrough_add,
> > +},
> > +
> > +SRST
> > +``colo_passthrough_add``
> > +  Add network stream to colo passthrough list.
> > +ERST
> > +
> > +{
> > +.name   = "colo_passthrough_del",
> > +.args_type  =
> "protocol:s,id:s?,src_ip:s?,dst_ip:s?,src_port:i?,dst_port:i?",
> > +.params = "protocol [id] [src_ip] [dst_ip] [src_port] 
> > [dst_port]",
> > +.help   = "Delete network stream from colo passthrough list",
> > +.cmd= hmp_colo_passthrough_del,
> > +},
> > +
> > +SRST
> > +``colo_passthrough_del``
> > +  Delete network stream from colo passthrough list.
> > +ERST
> > +
> >  {
> >  .name   = "object_add",
> >  .args_type  = "object:O",
> > diff --git a/include/monitor/hmp.h b/include/monitor/hmp.h index
> > ed2913fd18..3c4943b09f 100644
> > --- a/include/monitor/hmp.h
> > +++ b/include/monitor/hmp.h
> > @@ -81,6 +81,8 @@ void hmp_device_del(Monitor *mon, const QDict
> > *qdict);  void hmp_dump_guest_memory(Monitor *mon, const QDict
> > *qdict);  void hmp_netdev_add(Monitor *mon, const QDict *qdict);  void
> > hmp_netdev_del(Monitor *mon, const QDict *qdict);
> > +void hmp_colo_passthrough_add(Monitor *mon, const QDict *qdict);
> void
> > +hmp_colo_passthrough_del(Monitor *mon, const QDict *qdict);
> >  void hmp_getfd(Monitor *mon, const QDict *qdict);  void
> > hmp_closefd(Monitor *mon, const QDict *qdict);  void
> > hmp_sendkey(Monitor *mon, const QDict *qdict); diff --git
> > a/monitor/hmp-cmds.c b/monitor/hmp-cmds.c index
> 3c88a4faef..b57e3430ab
> > 100644
> > --- a/monitor/hmp-cmds.c
> > +++ b/monitor/hmp-cmds.c
> > @@ -1668,6 +1668,40 @@ void hmp_netdev_del(Monitor *mon, const
> QDict *qdict)
> >  hmp_handle_error(mon, err);
> >  }
> >
> > +void hmp_colo_passthrough_add(Monitor *mon, const QDict *qdict) {
> > +const char *prot = qdict_get_str(qdict, "protocol");
> > +L4_Connection *l4_conn = g_new0(L4_Connection, 1);
> > +Error *err = NULL;
> > +
> > +l4_conn->id = g_strdup(qdict_get_try_str(qdict, "id"));
> > +l4_conn->protocol = qapi_enum_parse(_PROTOCOL_lookup, prot, -
> 1, );
> > +l4_conn->src_ip = g_strdup(qdict_get_try_str(qdict, "src_ip"));
> > +l4_conn->dst_ip = g_strdup(qdict_get_try_str(qdict, "dst_ip"));
> > +l4_conn->src_port = qdict_get_try_int(qdict, "src_port", 0);
> > +l4_conn->dst_port = qdict_get_try_int(qdict, "dst_port", 0);
> > +
> > +qmp_colo_passthrough_add(l4_conn, );
> > +hmp_handle_error(mon, err);
> > +}
> > +
> > +void hmp_colo_passthrough_del(Monitor *mon, const QDict *qdict) {
> > +const char *prot = qdict_get_str(qdict, "protocol");
> > +L4_Connection *l4_conn = g_new0(L4_Connection, 1);
> > +Error *err = NULL;
> > +
> > +l4_conn->id = g_strdup(qdict_get_try_str(qdict, "id"));
> > +l4_conn->protocol = qapi_enum_parse(_PROTOCOL_lookup, prot, -
> 1, );
> > +l4_conn->src_ip = g_strdup(qdict_get_try_str(qdict, "src_ip"));
> > +l4_conn->dst_ip = g_strdup(qdict_get_try_str(qdict, "dst_ip"));
> > +l4_conn->src_port = qdict_get_try_int(qdict, "src_port", 0);
> > +l4_conn->dst_port = qdict_get_try_int(qdict, "dst_port", 0);
> > +
> > +qmp_colo_passthrough_del(l4_conn, );
> > +hmp_handle_error(mon, err);
> > +}
> > +
> >  void hmp_object_add(Monitor *mon, const QDict *qdict) 

Re: [RFC PATCH 0/9] migration/snap-tool: External snapshot utility

[Peter Xu]

On Wed, Mar 17, 2021 at 07:32:13PM +0300, Andrey Gruzdev wrote:
> This series is a kind of PoC for asynchronous snapshot reverting. This is
> about external snapshots only and doesn't involve block devices. Thus, it's
> mainly intended to be used with the new 'background-snapshot' migration
> capability and otherwise standard QEMU migration mechanism.
> 
> The major ideas behind this first version were:
>   * Make it compatible with 'exec:'-style migration - options can be create
> some separate tool or integrate into qemu-system.
>   * Support asynchronous revert stage by using unaltered postcopy logic
> at destination. To do this, we should be capable of saving RAM pages
> so that any particular page can be directly addressed by it's block ID
> and page offset. Possible solutions here seem to be:
>   use separate index (and storing it somewhere)
>   create sparse file on host FS and address pages with file offset
>   use QCOW2 (or other) image container with inherent sparsity support
>   * Make snapshot image file dense on the host FS so we don't depend on
> copy/backup tools and how they deal with sparse files. Off course,
> there's some performance cost for this choice.
>   * Make the code which is parsing unstructered format of migration stream,
> at least, not very sophisticated. Also, try to have minimum dependencies
> on QEMU migration code, both RAM and device.
>   * Try to keep page save latencies small while not degrading migration
> bandwidth too much.
> 
> For this first version I decided not to integrate into main QEMU code but
> create a separate tool. The main reason is that there's not too much migration
> code that is target-specific and can be used in it's unmodified form. Also,
> it's still not very clear how to make 'qemu-system' integration in terms of
> command-line (or monitor/QMP?) interface extension.
> 
> For the storage format, QCOW2 as a container and large (1MB) cluster size seem
> to be an optimal choice. Larger cluster is beneficial for performance 
> particularly
> in the case when image preallocation is disabled. Such cluster size does not 
> result
> in too high internal fragmentation level (~10% of space waste in most cases) 
> yet
> allows to reduce significantly the number of expensive cluster allocations.
> 
> A bit tricky part is dispatching QEMU migration stream cause it is mostly
> unstructered and depends on configuration parameters like 'send-configuration'
> and 'send-section-footer'. But, for the case with default values in migration
> globals it seems that implemented dispatching code works well and won't have
> compatibility issues in a reasonably long time frame.
> 
> I decided to keep RAM save path synchronous, anyhow it's better to use 
> writeback
> cache mode for the live snapshots cause of it's interleaving page address 
> pattern.
> Page coalescing buffer is used to merge contiguous pages to optimize block 
> layer
> writes.
> 
> Since for snapshot loading opening image file in cached mode would not do any 
> good,
> it implies that Linux native AIO and O_DIRECT mode is used in a common 
> scenario.
> AIO support in RAM loading path is implemented by using a ring of preallocated
> fixed-sized buffers in such a way that there's always a number of outstanding 
> block
> requests anytime. It also ensures in-order request completion.
> 
> How to use:
> 
> **Save:**
> * qemu> migrate_set_capability background-snapshot on
> * qemu> migrate "exec:/qemu-snap -s 
>--cache=writeback --aio=threads save "
> 
> **Load:**
> * Use 'qemu-system-* -incoming defer'
> * qemu> migrate_incoming "exec:/qemu-snap
>   --cache=none --aio=native load "
> 
> **Load with postcopy:**
> * Use 'qemu-system-* -incoming defer'
> * qemu> migrate_set_capability postcopy-ram on
> * qemu> migrate_incoming "exec:/qemu-snap --postcopy=60
>   --cache=none --aio=native load "
> 
> And yes, asynchronous revert works well only with SSD, not with rotational 
> disk..
> 
> Some performance stats:
> * SATA SSD drive with ~500/450 MB/s sequantial read/write and ~60K IOPS max.
> * 220 MB/s average save rate (depends on workload)
> * 440 MB/s average load rate in precopy
> * 260 MB/s average load rate in postcopy

Andrey,

Before I try to read it (since I'm probably not the best person to review
it..).. Would you remind me on the major difference of external snapshots
comparing to the existing one, and problems to solve?

Thanks,

-- 
Peter Xu

Re: [PATCH v2 1/2] exec/memory: Extract address_space_set() from dma_memory_set()

[Richard Henderson]

On 4/15/21 3:04 AM, Philippe Mathieu-Daudé wrote:

dma_memory_set() does a DMA barrier, set the address space with
a constant value. The constant value filling code is not specific
to DMA and can be used for AddressSpace. Extract it as a new
helper: address_space_set().

Signed-off-by: Philippe Mathieu-Daudé
---
  include/exec/memory.h | 16 
  softmmu/dma-helpers.c | 16 +---
  softmmu/physmem.c | 19 +++
  3 files changed, 36 insertions(+), 15 deletions(-)


Reviewed-by: Richard Henderson 

r~

Re: [PATCH] docs: Add documentation for shakti_c machine

[Alistair Francis]

On Tue, Apr 13, 2021 at 3:44 AM Vijai Kumar K  wrote:
>
> Add documentation for Shakti C reference platform.
>
> Signed-off-by: Vijai Kumar K 

Thanks!

Applied to riscv-to-apply.next

Alistair

> ---
>  docs/system/riscv/shakti-c.rst | 82 ++
>  1 file changed, 82 insertions(+)
>  create mode 100644 docs/system/riscv/shakti-c.rst
>
> diff --git a/docs/system/riscv/shakti-c.rst b/docs/system/riscv/shakti-c.rst
> new file mode 100644
> index 00..a6035d42b0
> --- /dev/null
> +++ b/docs/system/riscv/shakti-c.rst
> @@ -0,0 +1,82 @@
> +Shakti C Reference Platform (``shakti_c``)
> +==
> +
> +Shakti C Reference Platform is a reference platform based on arty a7 100t
> +for the Shakti SoC.
> +
> +Shakti SoC is a SoC based on the Shakti C-class processor core. Shakti C
> +is a 64bit RV64GCSUN processor core.
> +
> +For more details on Shakti SoC, please see:
> +https://gitlab.com/shaktiproject/cores/shakti-soc/-/blob/master/fpga/boards/artya7-100t/c-class/README.rst
> +
> +For more info on the Shakti C-class core, please see:
> +https://c-class.readthedocs.io/en/latest/
> +
> +Supported devices
> +-
> +
> +The ``shakti_c`` machine supports the following devices:
> +
> + * 1 C-class core
> + * Core Level Interruptor (CLINT)
> + * Platform-Level Interrupt Controller (PLIC)
> + * 1 UART
> +
> +Boot options
> +
> +
> +The ``shakti_c`` machine can start using the standard -bios
> +functionality for loading the baremetal application or opensbi.
> +
> +Boot the machine
> +
> +
> +Shakti SDK
> +~~
> +Shakti SDK can be used to generate the baremetal example UART applications.
> +
> +.. code-block:: bash
> +
> +   $ git clone https://gitlab.com/behindbytes/shakti-sdk.git
> +   $ cd shakti-sdk
> +   $ make software PROGRAM=loopback TARGET=artix7_100t
> +
> +Binary would be generated in:
> +  software/examples/uart_applns/loopback/output/loopback.shakti
> +
> +You could also download the precompiled example applicatons using below
> +commands.
> +
> +.. code-block:: bash
> +
> +   $ wget -c 
> https://gitlab.com/behindbytes/shakti-binaries/-/raw/master/sdk/shakti_sdk_qemu.zip
> +   $ unzip shakti_sdk_qemu.zip
> +
> +Then we can run the UART example using:
> +
> +.. code-block:: bash
> +
> +   $ qemu-system-riscv64 -M shakti_c -nographic \
> +  -bios path/to/shakti_sdk_qemu/loopback.shakti
> +
> +OpenSBI
> +~~~
> +We can also run OpenSBI with Test Payload.
> +
> +.. code-block:: bash
> +
> +   $ git clone https://github.com/riscv/opensbi.git -b v0.9
> +   $ cd opensbi
> +   $ wget -c 
> https://gitlab.com/behindbytes/shakti-binaries/-/raw/master/dts/shakti.dtb
> +   $ export CROSS_COMPILE=riscv64-unknown-elf-
> +   $ export FW_FDT_PATH=./shakti.dtb
> +   $ make PLATFORM=generic
> +
> +fw_payload.elf would be generated in 
> build/platform/generic/firmware/fw_payload.elf.
> +Boot it using the below qemu command.
> +
> +.. code-block:: bash
> +
> +   $ qemu-system-riscv64 -M shakti_c -nographic \
> +  -bios path/to/fw_payload.elf
> --
> 2.25.1
>
>
>

Re: [PATCH] target/xtensa: don't generate extra EXCP_DEBUG on exception

[Richard Henderson]

On 4/15/21 1:55 PM, Max Filippov wrote:

target/xtensa used to generate an extra EXCP_DEBUG exception before the
first instruction executed after an interrupt or an exception is taken
to allow single-stepping that instruction in the debugger.
This is no longer needed after the following commits:
a7ba744f4082 ("tcg/cpu-exec: precise single-stepping after an exception")
ba3c35d9c402 ("tcg/cpu-exec: precise single-stepping after an interrupt")
Drop exception state tracking/extra EXCP_DEBUG generation code.

Signed-off-by: Max Filippov
---
This patch depends on the "target/xtensa: Make sure that tb->size != 0"
patch.

  target/xtensa/cpu.c| 1 -
  target/xtensa/cpu.h| 7 ---
  target/xtensa/exc_helper.c | 5 -
  target/xtensa/translate.c  | 6 --
  4 files changed, 19 deletions(-)


Reviewed-by: Richard Henderson 

r~

Re: [PATCH v2 2/2] hw/elf_ops: clear uninitialized segment space

[Richard Henderson]

On 4/15/21 3:04 AM, Philippe Mathieu-Daudé wrote:

From: Laurent Vivier

When the mem_size of the segment is bigger than the file_size,
and if this space doesn't overlap another segment, it needs
to be cleared.

This bug is very similar to the one we had for linux-user,
22d113b52f41 ("linux-user: Fix loading of BSS segments"),
where .bss section is encoded as an extension of the the data
one by setting the segment p_memsz > p_filesz.

Signed-off-by: Laurent Vivier
Message-Id:<20210414105838.205019-1-laur...@vivier.eu>
[PMD: Use recently added address_space_set()]
Signed-off-by: Philippe Mathieu-Daudé
---
  include/hw/elf_ops.h | 13 +
  1 file changed, 13 insertions(+)


Reviewed-by: Richard Henderson 

r~

[RFC PATCH 1/3] tests/docker: gcc-10 based images for ppc64{, le} tests

[matheus . ferst]

From: Matheus Ferst 

A newer compiler is needed to build tests for Power10 instructions. As
done for arm64 on c729a99d2701, new '-test-cross' images are created for
ppc64 and ppc64le. As done on 936fda4d771f, a test for compiler support
is added to verify that the toolchain in use has '-mpower10'.

Signed-off-by: Matheus Ferst 
---
 tests/docker/Makefile.include   |  4 
 .../dockerfiles/debian-ppc64-test-cross.docker  | 13 +
 .../debian-ppc64el-test-cross.docker| 17 +
 tests/tcg/configure.sh  | 12 
 4 files changed, 42 insertions(+), 4 deletions(-)
 create mode 100644 tests/docker/dockerfiles/debian-ppc64-test-cross.docker
 create mode 100644 tests/docker/dockerfiles/debian-ppc64el-test-cross.docker

diff --git a/tests/docker/Makefile.include b/tests/docker/Makefile.include
index 9f464cb92c..1f8941d290 100644
--- a/tests/docker/Makefile.include
+++ b/tests/docker/Makefile.include
@@ -152,10 +152,14 @@ docker-image-debian-sparc64-cross: docker-image-debian10
 docker-image-debian-tricore-cross: docker-image-debian10
 docker-image-debian-all-test-cross: docker-image-debian10
 docker-image-debian-arm64-test-cross: docker-image-debian11
+docker-image-debian-ppc64-test-cross: docker-image-debian11
+docker-image-debian-ppc64el-test-cross: docker-image-debian11
 
 # These images may be good enough for building tests but not for test builds
 DOCKER_PARTIAL_IMAGES += debian-alpha-cross
 DOCKER_PARTIAL_IMAGES += debian-arm64-test-cross
+DOCKER_PARTIAL_IMAGES += debian-ppc64-test-cross
+DOCKER_PARTIAL_IMAGES += debian-ppc64el-test-cross
 DOCKER_PARTIAL_IMAGES += debian-hppa-cross
 DOCKER_PARTIAL_IMAGES += debian-m68k-cross debian-mips64-cross
 DOCKER_PARTIAL_IMAGES += debian-powerpc-cross debian-ppc64-cross
diff --git a/tests/docker/dockerfiles/debian-ppc64-test-cross.docker 
b/tests/docker/dockerfiles/debian-ppc64-test-cross.docker
new file mode 100644
index 00..66abfdeb47
--- /dev/null
+++ b/tests/docker/dockerfiles/debian-ppc64-test-cross.docker
@@ -0,0 +1,13 @@
+#
+# Docker ppc64 cross-compiler target (tests only)
+#
+# This docker target builds on the debian Bullseye base image.
+#
+FROM qemu/debian11
+
+# Add the foreign architecture we want and install dependencies
+RUN dpkg --add-architecture ppc64
+RUN apt update && \
+DEBIAN_FRONTEND=noninteractive eatmydata \
+apt install -y --no-install-recommends \
+libc6-dev-ppc64-cross gcc-10-powerpc64-linux-gnu
diff --git a/tests/docker/dockerfiles/debian-ppc64el-test-cross.docker 
b/tests/docker/dockerfiles/debian-ppc64el-test-cross.docker
new file mode 100644
index 00..7582508467
--- /dev/null
+++ b/tests/docker/dockerfiles/debian-ppc64el-test-cross.docker
@@ -0,0 +1,17 @@
+#
+# Docker ppc64el cross-compiler target (tests only)
+#
+# This docker target builds on the debian Bullseye base image.
+#
+FROM qemu/debian11
+
+# Add the foreign architecture we want and install dependencies
+RUN dpkg --add-architecture ppc64el
+RUN apt update && \
+DEBIAN_FRONTEND=noninteractive eatmydata \
+apt install -y --no-install-recommends \
+crossbuild-essential-ppc64el gcc-10-powerpc64le-linux-gnu
+
+# Specify the cross prefix for this image (see tests/docker/common.rc)
+#ENV QEMU_CONFIGURE_OPTS --cross-prefix=powerpc64le-linux-gnu-
+#ENV DEF_TARGET_LIST ppc64-softmmu,ppc64-linux-user,ppc64abi32-linux-user
diff --git a/tests/tcg/configure.sh b/tests/tcg/configure.sh
index fa1a4261a4..5f5db91a01 100755
--- a/tests/tcg/configure.sh
+++ b/tests/tcg/configure.sh
@@ -170,13 +170,13 @@ for target in $target_list; do
   ;;
 ppc64-*)
   container_hosts=x86_64
-  container_image=debian-ppc64-cross
-  container_cross_cc=powerpc64-linux-gnu-gcc
+  container_image=debian-ppc64-test-cross
+  container_cross_cc=powerpc64-linux-gnu-gcc-10
   ;;
 ppc64le-*)
   container_hosts=x86_64
-  container_image=debian-ppc64el-cross
-  container_cross_cc=powerpc64le-linux-gnu-gcc
+  container_image=debian-ppc64el-test-cross
+  container_cross_cc=powerpc64le-linux-gnu-gcc-10
   ;;
 riscv64-*)
   container_hosts=x86_64
@@ -280,6 +280,10 @@ for target in $target_list; do
-mpower8-vector -o $TMPE $TMPC; then
 echo "CROSS_CC_HAS_POWER8_VECTOR=y" >> $config_target_mak
 fi
+if do_compiler "$target_compiler" $target_compiler_cflags \
+   -mpower10 -o $TMPE $TMPC; then
+echo "CROSS_CC_HAS_POWER10=y" >> $config_target_mak
+fi
 ;;
 i386-linux-user)
 if do_compiler "$target_compiler" $target_compiler_cflags \
-- 
2.25.1

[RFC PATCH 0/3] tests/tcg/ppc64le: paddi tests

[matheus . ferst]

From: Matheus Ferst 

Based-on: <20210413211129.457272-1-luis.pi...@eldorado.org.br>

This series adds gcc-10 based images to enable the build of tests with Power10
instructions. Then two tests for paddi are added:
- The first one checks a weird behavior observed on POWER10 Functional Simulator
  1.1.0, where the 34-bit immediate is treated as a 32-bits one;
- The second one exercises the R=1 path of paddi, where CIA is used instead of 
RA.
  The test is failing with the current implementation because we use cpu_nip,
  which is not updated all the time. Luis already has the fix, it should be
  applied on the next version of his patch series.

The main reason to submit this patch as an RFC first is the docker part. I would
lie if I tell you that I understand half of what is going on there.
 - 'make docker-test-tcg' fails, but apparently on unrelated things;
 - 'make docker-run-test-tcg@debian-ppc64el-cross' passes, but it looks
   like the test is skipped?
 - 'make check-tcg' runs the test and passes (with the fix in place for the
   second).

Finally, get_maintainer.pl found no maintainers for
tests/tcg/ppc64{,le}/Makefile.target. Would it be Mr. Gibson?

Thanks,
Matheus K. Ferst

Matheus Ferst (3):
  tests/docker: gcc-10 based images for ppc64{,le} tests
  tests/tcg/ppc64le: load 33-bits constant with paddi
  tests/tcg/ppc64le: R=1 test for paddi

 tests/docker/Makefile.include |  4 +++
 .../debian-ppc64-test-cross.docker| 13 ++
 .../debian-ppc64el-test-cross.docker  | 17 
 tests/tcg/configure.sh| 12 ++---
 tests/tcg/ppc64/Makefile.target   |  6 +
 tests/tcg/ppc64le/Makefile.target |  6 +
 tests/tcg/ppc64le/pla.c   | 26 +++
 tests/tcg/ppc64le/pli_33bits.c| 22 
 8 files changed, 102 insertions(+), 4 deletions(-)
 create mode 100644 tests/docker/dockerfiles/debian-ppc64-test-cross.docker
 create mode 100644 tests/docker/dockerfiles/debian-ppc64el-test-cross.docker
 create mode 100644 tests/tcg/ppc64le/pla.c
 create mode 100644 tests/tcg/ppc64le/pli_33bits.c

-- 
2.25.1

[RFC PATCH 2/3] tests/tcg/ppc64le: load 33-bits constant with paddi

[matheus . ferst]

From: Matheus Ferst 

This test checks that we can correctly load a 33-bit constant and its
two's complement. At least until version 1.1-0, POWER10 Functional
Simulation fails this test, processing the immediate as if it were
32-bits instead of 34, so it's probably something to keep an eye on.

Signed-off-by: Matheus Ferst 
---
 tests/tcg/ppc64/Makefile.target   |  5 +
 tests/tcg/ppc64le/Makefile.target |  5 +
 tests/tcg/ppc64le/pli_33bits.c| 22 ++
 3 files changed, 32 insertions(+)
 create mode 100644 tests/tcg/ppc64le/pli_33bits.c

diff --git a/tests/tcg/ppc64/Makefile.target b/tests/tcg/ppc64/Makefile.target
index 0c6a4585fc..6eccd2c06f 100644
--- a/tests/tcg/ppc64/Makefile.target
+++ b/tests/tcg/ppc64/Makefile.target
@@ -10,4 +10,9 @@ PPC64_TESTS=bcdsub
 endif
 bcdsub: CFLAGS += -mpower8-vector
 
+ifneq ($(DOCKER_IMAGE)$(CROSS_CC_HAS_POWER10),)
+PPC64LE_TESTS += pli_33bits
+endif
+pli_33bits: CFLAGS += -mpower10
+
 TESTS += $(PPC64_TESTS)
diff --git a/tests/tcg/ppc64le/Makefile.target 
b/tests/tcg/ppc64le/Makefile.target
index 1acfcff94a..2003eab2df 100644
--- a/tests/tcg/ppc64le/Makefile.target
+++ b/tests/tcg/ppc64le/Makefile.target
@@ -9,4 +9,9 @@ PPC64LE_TESTS=bcdsub
 endif
 bcdsub: CFLAGS += -mpower8-vector
 
+ifneq ($(DOCKER_IMAGE)$(CROSS_CC_HAS_POWER10),)
+PPC64LE_TESTS += pli_33bits
+endif
+pli_33bits: CFLAGS += -mpower10
+
 TESTS += $(PPC64LE_TESTS)
diff --git a/tests/tcg/ppc64le/pli_33bits.c b/tests/tcg/ppc64le/pli_33bits.c
new file mode 100644
index 00..848cbce165
--- /dev/null
+++ b/tests/tcg/ppc64le/pli_33bits.c
@@ -0,0 +1,22 @@
+#include 
+#include 
+#include 
+
+int main(void)
+{
+long int var;
+struct sigaction action;
+
+action.sa_handler = _exit;
+sigaction(SIGABRT, , NULL);
+
+asm(" pli %0,0x1\n"
+: "=r"(var));
+assert(var == 0x1);
+
+asm(" pli %0,-0x1\n"
+   : "=r"(var));
+assert(var == -0x1);
+
+return 0;
+}
-- 
2.25.1

[RFC PATCH 3/3] tests/tcg/ppc64le: R=1 test for paddi

[matheus . ferst]

From: Matheus Ferst 

This test exercise the R=1 path of paddi implementation using the
extended mnemonic "pla".

Signed-off-by: Matheus Ferst 
---
 tests/tcg/ppc64/Makefile.target   |  3 ++-
 tests/tcg/ppc64le/Makefile.target |  3 ++-
 tests/tcg/ppc64le/pla.c   | 26 ++
 3 files changed, 30 insertions(+), 2 deletions(-)
 create mode 100644 tests/tcg/ppc64le/pla.c

diff --git a/tests/tcg/ppc64/Makefile.target b/tests/tcg/ppc64/Makefile.target
index 6eccd2c06f..a6cd7a21b2 100644
--- a/tests/tcg/ppc64/Makefile.target
+++ b/tests/tcg/ppc64/Makefile.target
@@ -11,8 +11,9 @@ endif
 bcdsub: CFLAGS += -mpower8-vector
 
 ifneq ($(DOCKER_IMAGE)$(CROSS_CC_HAS_POWER10),)
-PPC64LE_TESTS += pli_33bits
+PPC64LE_TESTS += pli_33bits pla
 endif
 pli_33bits: CFLAGS += -mpower10
+pla: CFLAGS += -mpower10
 
 TESTS += $(PPC64_TESTS)
diff --git a/tests/tcg/ppc64le/Makefile.target 
b/tests/tcg/ppc64le/Makefile.target
index 2003eab2df..db92b2ec99 100644
--- a/tests/tcg/ppc64le/Makefile.target
+++ b/tests/tcg/ppc64le/Makefile.target
@@ -10,8 +10,9 @@ endif
 bcdsub: CFLAGS += -mpower8-vector
 
 ifneq ($(DOCKER_IMAGE)$(CROSS_CC_HAS_POWER10),)
-PPC64LE_TESTS += pli_33bits
+PPC64LE_TESTS += pli_33bits pla
 endif
 pli_33bits: CFLAGS += -mpower10
+pla: CFLAGS += -mpower10
 
 TESTS += $(PPC64LE_TESTS)
diff --git a/tests/tcg/ppc64le/pla.c b/tests/tcg/ppc64le/pla.c
new file mode 100644
index 00..4826579216
--- /dev/null
+++ b/tests/tcg/ppc64le/pla.c
@@ -0,0 +1,26 @@
+#include 
+#include 
+#include 
+
+int main(void)
+{
+long unsigned int label, addr;
+struct sigaction action;
+
+action.sa_handler = _exit;
+sigaction(SIGABRT, , NULL);
+
+asm("insn:\n"
+" lis%0, insn@highest\n"
+" addi   %0, %0, insn@higher\n"
+" rldicr %0, %0, 32, 31\n"
+" oris   %0, %0, insn@h\n"
+" ori%0, %0, insn@l\n"
+" pla%1, %2\n"
+: "=r" (label), "=r" (addr)
+: "i" (-5 * 4)); /* number of instruction between label and pla */
+assert(addr == label);
+
+return 0;
+}
+
-- 
2.25.1

Re: [PATCH for-6.0?] hw/arm/armsse: Give SSE-300 its own Property array

[Richard Henderson]

On 4/15/21 11:23 AM, Peter Maydell wrote:

SSE-300 currently shares the SSE-200 Property array. This is
bad principally because the default values of the CPU0_FPU
and CPU0_DSP properties disable the FPU and DSP on the CPU.
That is correct for the SSE-300 but not the SSE-200.
Give the SSE-300 its own Property array with the correct
SSE-300 specific settings:
  * SSE-300 has only one CPU, so no CPU1* properties
  * SSE-300 CPU has FPU and DSP

Buglink:https://bugs.launchpad.net/qemu/+bug/1923861
Signed-off-by: Peter Maydell
---
This is a simple and pretty safe fix, but I don't think it quite
merits doing an rc4 by itself. I think if we do an rc4 for some
other reason it ought to go in, though.


Reviewed-by: Richard Henderson 

r~

[PATCH 8/8] Tests: add custom test jobs

[Cleber Rosa]

Different users (or even companies) have different interests, and
may want to run a reduced set of tests during development, or a
larger set of tests during QE.

To cover these use cases, some example (but functional) jobs are
introduced here:

1) acceptance-all-targets.py: runs all arch agnostic tests on all
   built targets, unless there are conditions that make them not work
   out of the box ATM, then run all tests that are specific to
   predefined targets.

2) acceptance-kvm-only.py: runs only tests that require KVM and are
   specific to the host architecture.

3) qtest-unit.py: runs a combination of qtest and unit tests (in
   parallel).

4) qtest-unit-acceptance.py: runs a combineation of qtest, unit tests
   and acceptance tests (all of them in parallel)

To run the first two manually, follow the example bellow:

 $ cd build
 $ make check-venv
 $ ./tests/venv/bin/python3 tests/jobs/acceptance-all-targets.py
 $ ./tests/venv/bin/python3 tests/jobs/acceptance-kvm-only.py

The third and fouth example depends on information coming from Meson,
so the easiest way to run it is:

 $ cd build
 $ make check-qtest-unit
 $ make check-qtest-unit-acceptance

These are based on Avocado's Job API, a way to customize an Avocado
job execution beyond the possibilities of command line arguments.
For more Job API resources, please refer to:

a) Job API Examples:
 - https://github.com/avocado-framework/avocado/tree/master/examples/jobs

b) Documentation about configurable features at the Job Level:
 - https://avocado-framework.readthedocs.io/en/87.0/config/index.html

c) Documentation about the TestSuite class
 - 
https://avocado-framework.readthedocs.io/en/87.0/api/core/avocado.core.html#avocado.core.suite.TestSuite

d) Documentation about the Job class
 - 
https://avocado-framework.readthedocs.io/en/87.0/api/core/avocado.core.html#avocado.core.job.Job

Signed-off-by: Cleber Rosa 
---
 configure|  2 +-
 tests/Makefile.include   |  8 
 tests/jobs/acceptance-all-targets.py | 67 
 tests/jobs/acceptance-kvm-only.py| 35 +++
 tests/jobs/qtest-unit-acceptance.py  | 31 +
 tests/jobs/qtest-unit.py | 24 ++
 tests/jobs/utils.py  | 22 +
 7 files changed, 188 insertions(+), 1 deletion(-)
 create mode 100644 tests/jobs/acceptance-all-targets.py
 create mode 100644 tests/jobs/acceptance-kvm-only.py
 create mode 100644 tests/jobs/qtest-unit-acceptance.py
 create mode 100644 tests/jobs/qtest-unit.py
 create mode 100644 tests/jobs/utils.py

diff --git a/configure b/configure
index 4f374b4889..23273262e5 100755
--- a/configure
+++ b/configure
@@ -6265,7 +6265,7 @@ LINKS="$LINKS pc-bios/s390-ccw/Makefile"
 LINKS="$LINKS roms/seabios/Makefile"
 LINKS="$LINKS pc-bios/qemu-icon.bmp"
 LINKS="$LINKS .gdbinit scripts" # scripts needed by relative path in .gdbinit
-LINKS="$LINKS tests/acceptance tests/data"
+LINKS="$LINKS tests/acceptance tests/data tests/jobs"
 LINKS="$LINKS tests/qemu-iotests/check"
 LINKS="$LINKS python"
 LINKS="$LINKS contrib/plugins/Makefile "
diff --git a/tests/Makefile.include b/tests/Makefile.include
index 63477c8b4b..03443dd0e8 100644
--- a/tests/Makefile.include
+++ b/tests/Makefile.include
@@ -133,6 +133,14 @@ check-acceptance: check-venv $(TESTS_RESULTS_DIR) 
get-vm-images
 $(if $(GITLAB_CI),,--failfast) tests/acceptance, \
 "AVOCADO", "tests/acceptance")
 
+# Runs qtest and unit tests on a custom Avocado job
+check-qtest-unit: check-venv $(TESTS_RESULTS_DIR)
+   $(MESON) introspect --tests | $(TESTS_VENV_DIR)/bin/python 
$(SRC_PATH)/tests/jobs/qtest-unit.py $(TESTS_RESULTS_DIR)
+
+# Runs qtest and unit and accpetance tests on a custom Avocado job
+check-qtest-unit-acceptance: check-venv $(TESTS_RESULTS_DIR)
+   $(MESON) introspect --tests | $(TESTS_VENV_DIR)/bin/python 
$(SRC_PATH)/tests/jobs/qtest-unit-acceptance.py $(TESTS_RESULTS_DIR)
+
 # Consolidated targets
 
 .PHONY: check-block check check-clean get-vm-images
diff --git a/tests/jobs/acceptance-all-targets.py 
b/tests/jobs/acceptance-all-targets.py
new file mode 100644
index 00..96703824e6
--- /dev/null
+++ b/tests/jobs/acceptance-all-targets.py
@@ -0,0 +1,67 @@
+#!/usr/bin/env python3
+
+import glob
+import os
+import sys
+
+from avocado.core.job import Job
+from avocado.core.suite import TestSuite
+
+
+def filter_out_currently_broken(variants):
+"""Filter out targets that can not be currently used transparently."""
+result = []
+for variant in variants:
+if (# qemu-system-m68k: Kernel image must be specified
+variant['qemu_bin'].endswith('qemu-system-m68k') or
+# qemu-system-sh4: Could not load SHIX bios 'shix_bios.bin'
+variant['qemu_bin'].endswith('qemu-system-sh4')):
+continue
+result.append(variant)
+return result
+
+
+def add_machine_type(variants):
+"""Add default machine type  

[PATCH 6/8] Acceptance tests: prevent shutdown on non-specific target tests

[Cleber Rosa]

When running tests that are not target specific with various target
binaries, some specific behavior appears.  For s390x, when there's no
guest code running, it will produce GUEST_PANICKED events as the
firmware will shutdown the machine.

With this change, no GUEST_PANICKED *event* will be generated.

For some QMP commands, such as "query-migrate", a proper response
("guest-panicked" for the s390x target) will still be given.

Signed-off-by: Cleber Rosa 
---
 tests/acceptance/migration.py | 4 ++--
 tests/acceptance/version.py   | 2 +-
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/tests/acceptance/migration.py b/tests/acceptance/migration.py
index 25ee55f36a..b4d46becc6 100644
--- a/tests/acceptance/migration.py
+++ b/tests/acceptance/migration.py
@@ -46,12 +46,12 @@ def assert_migration(self, src_vm, dst_vm):
 
 def do_migrate(self, dest_uri, src_uri=None):
 dest_vm = self.get_vm('-incoming', dest_uri)
-dest_vm.add_args('-nodefaults')
+dest_vm.add_args('-nodefaults', '-no-shutdown')
 dest_vm.launch()
 if src_uri is None:
 src_uri = dest_uri
 source_vm = self.get_vm()
-source_vm.add_args('-nodefaults')
+source_vm.add_args('-nodefaults', '-no-shutdown')
 source_vm.launch()
 response = source_vm.qmp('migrate', uri=src_uri)
 if 'error' in response:
diff --git a/tests/acceptance/version.py b/tests/acceptance/version.py
index 79b923d4fc..3cf18c9878 100644
--- a/tests/acceptance/version.py
+++ b/tests/acceptance/version.py
@@ -17,7 +17,7 @@ class Version(Test):
 :avocado: tags=quick
 """
 def test_qmp_human_info_version(self):
-self.vm.add_args('-nodefaults')
+self.vm.add_args('-nodefaults', '-no-shutdown')
 self.vm.launch()
 res = self.vm.command('human-monitor-command',
   command_line='info version')
-- 
2.25.4

[PATCH 7/8] tests/acceptance/migration.py: cancel test on s390x

[Cleber Rosa]

Because s390x targets it can not currently migrate without a guest
running.

Future work may provide a proper guest, but for now, it's safer to
cancel the test.

Signed-off-by: Cleber Rosa 
---
 tests/acceptance/migration.py | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/tests/acceptance/migration.py b/tests/acceptance/migration.py
index b4d46becc6..4174d55c81 100644
--- a/tests/acceptance/migration.py
+++ b/tests/acceptance/migration.py
@@ -48,6 +48,12 @@ def do_migrate(self, dest_uri, src_uri=None):
 dest_vm = self.get_vm('-incoming', dest_uri)
 dest_vm.add_args('-nodefaults', '-no-shutdown')
 dest_vm.launch()
+
+cpus = dest_vm.command('query-cpus-fast')
+if cpus:
+if cpus[0].get('target') == 's390x':
+self.cancel('Migration without a guest not possible on s390')
+
 if src_uri is None:
 src_uri = dest_uri
 source_vm = self.get_vm()
-- 
2.25.4

[PATCH 4/8] tests/acceptance/migration.py: cancel test if migration is not supported

[Cleber Rosa]

FIXME: check if there's a way to query migration support before
actually requesting migration.

Some targets/machines contain devices that do not support migration.
Let's acknowledge that and cancel the test as early as possible.

Signed-off-by: Cleber Rosa 
---
 tests/acceptance/migration.py | 6 +-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/tests/acceptance/migration.py b/tests/acceptance/migration.py
index 792639cb69..25ee55f36a 100644
--- a/tests/acceptance/migration.py
+++ b/tests/acceptance/migration.py
@@ -53,7 +53,11 @@ def do_migrate(self, dest_uri, src_uri=None):
 source_vm = self.get_vm()
 source_vm.add_args('-nodefaults')
 source_vm.launch()
-source_vm.qmp('migrate', uri=src_uri)
+response = source_vm.qmp('migrate', uri=src_uri)
+if 'error' in response:
+if 'desc' in response['error']:
+msg = response['error']['desc']
+self.cancel('Migration does not seem to be supported: %s' % msg)
 self.assert_migration(source_vm, dest_vm)
 
 def _get_free_port(self):
-- 
2.25.4

[PATCH 5/8] tests/acceptance/cpu_queries.py: use the proper logging channels

[Cleber Rosa]

The test contains methods for the proper log of test related
information.  Let's use that and remove the print and the unused
logging import.

Reference: 
https://avocado-framework.readthedocs.io/en/87.0/api/test/avocado.html#avocado.Test.log
Signed-off-by: Cleber Rosa 
---
 tests/acceptance/cpu_queries.py | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/tests/acceptance/cpu_queries.py b/tests/acceptance/cpu_queries.py
index 293dccb89a..cc9e380cc7 100644
--- a/tests/acceptance/cpu_queries.py
+++ b/tests/acceptance/cpu_queries.py
@@ -8,8 +8,6 @@
 # This work is licensed under the terms of the GNU GPL, version 2 or
 # later.  See the COPYING file in the top-level directory.
 
-import logging
-
 from avocado_qemu import Test
 
 class QueryCPUModelExpansion(Test):
@@ -27,7 +25,7 @@ def test(self):
 
 cpus = self.vm.command('query-cpu-definitions')
 for c in cpus:
-print(repr(c))
+self.log.info("Checking CPU: %s", c)
 self.assertNotIn('', c['unavailable-features'], c['name'])
 
 for c in cpus:
-- 
2.25.4

[PATCH 1/8] Acceptance Jobs: preserve the cache for pip on GitLab CI

[Cleber Rosa]

The acceptance jobs (via `make check-venv`) will setup a virtual
environment, and after that install packages defined in
tests/requirements.txt via pip.

Let's enable pip's default cache directory, so that we can save
a bit on time/bandwidth.

Signed-off-by: Cleber Rosa 
---
 .gitlab-ci.yml | 1 +
 1 file changed, 1 insertion(+)

diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml
index 52d65d6c04..9cc4676912 100644
--- a/.gitlab-ci.yml
+++ b/.gitlab-ci.yml
@@ -53,6 +53,7 @@ include:
 key: "${CI_JOB_NAME}-cache"
 paths:
   - ${CI_PROJECT_DIR}/avocado-cache
+  - ~/.cache/pip
 policy: pull-push
   artifacts:
 name: "$CI_JOB_NAME-$CI_COMMIT_REF_SLUG"
-- 
2.25.4

[PATCH 0/8] Tests: introduce custom jobs

[Cleber Rosa]

Different users (or even companies) have different interests, and
may want to run a reduced set of tests during development, or a
larger set of tests during QE.

To cover these use cases, this introduces some example (but
functional) jobs.

It's expected that some common jobs will come up from common
requirements for different users (and maybe be added to a common
location such as tests/jobs), and that very specific jobs will be
added to directories specific to certain groups, say
"contrib/com.redhat/jobs" or the like.

This series does *not* add new jobs to GitLab CI pipeline, but this is
expected to be done later on custom runners.  That is, custom runners
could be used for custom jobs.  Anyway, a GitLab CI pipeline can be
seen here:

 https://gitlab.com/cleber.gnu/qemu/-/pipelines/287210066

This is based on the Avocado version bump patch:

 https://lists.gnu.org/archive/html/qemu-devel/2021-04/msg02391.html

Based-On: <20210414161144.1598980-1-cr...@redhat.com>

Cleber Rosa (8):
  Acceptance Jobs: preserve the cache for pip on GitLab CI
  Acceptance tests: do not try to reuse packages from the system
  tests/acceptance/linux_ssh_mips_malta.py: drop identical setUp
  tests/acceptance/migration.py: cancel test if migration is not
supported
  tests/acceptance/cpu_queries.py: use the proper logging channels
  Acceptance tests: prevent shutdown on non-specific target tests
  tests/acceptance/migration.py: cancel test on s390x
  Tests: add custom test jobs

 .gitlab-ci.yml   |  1 +
 configure|  2 +-
 tests/Makefile.include   | 10 +++-
 tests/acceptance/cpu_queries.py  |  4 +-
 tests/acceptance/linux_ssh_mips_malta.py |  7 +--
 tests/acceptance/migration.py| 16 --
 tests/acceptance/version.py  |  2 +-
 tests/jobs/acceptance-all-targets.py | 67 
 tests/jobs/acceptance-kvm-only.py| 35 +
 tests/jobs/qtest-unit-acceptance.py  | 31 +++
 tests/jobs/qtest-unit.py | 24 +
 tests/jobs/utils.py  | 22 
 12 files changed, 207 insertions(+), 14 deletions(-)
 create mode 100644 tests/jobs/acceptance-all-targets.py
 create mode 100644 tests/jobs/acceptance-kvm-only.py
 create mode 100644 tests/jobs/qtest-unit-acceptance.py
 create mode 100644 tests/jobs/qtest-unit.py
 create mode 100644 tests/jobs/utils.py

-- 
2.25.4

[PATCH 3/8] tests/acceptance/linux_ssh_mips_malta.py: drop identical setUp

[Cleber Rosa]

These tests' setUp do not do anything beyong what their base class do.
And while they do decorate the setUp() we can decorate the classes
instead, so no functionality is lost here.

Signed-off-by: Cleber Rosa 
---
 tests/acceptance/linux_ssh_mips_malta.py | 7 ++-
 1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/tests/acceptance/linux_ssh_mips_malta.py 
b/tests/acceptance/linux_ssh_mips_malta.py
index 6dbd02d49d..e309a1105c 100644
--- a/tests/acceptance/linux_ssh_mips_malta.py
+++ b/tests/acceptance/linux_ssh_mips_malta.py
@@ -19,6 +19,8 @@
 from avocado.utils import ssh
 
 
+@skipUnless(os.getenv('AVOCADO_TIMEOUT_EXPECTED'), 'Test might timeout')
+@skipUnless(ssh.SSH_CLIENT_BINARY, 'No SSH client available')
 class LinuxSSH(Test):
 
 timeout = 150 # Not for 'configure --enable-debug --enable-debug-tcg'
@@ -65,11 +67,6 @@ def get_kernel_info(self, endianess, wordsize):
 kernel_hash = self.IMAGE_INFO[endianess]['kernel_hash'][wordsize]
 return kernel_url, kernel_hash
 
-@skipUnless(ssh.SSH_CLIENT_BINARY, 'No SSH client available')
-@skipUnless(os.getenv('AVOCADO_TIMEOUT_EXPECTED'), 'Test might timeout')
-def setUp(self):
-super(LinuxSSH, self).setUp()
-
 def get_portfwd(self):
 res = self.vm.command('human-monitor-command',
   command_line='info usernet')
-- 
2.25.4

[PATCH 2/8] Acceptance tests: do not try to reuse packages from the system

[Cleber Rosa]

The premise behind the original behavior is that it would save people
from downloading Avocado (and other dependencies) if already installed
on the system.  To be honest, I think it's extremely rare that the
same versions described as dependencies will be available on most
systems.  But, the biggest motivations here are that:

 1) Hacking on QEMU in the same system used to develop Avocado leads
to confusion with regards to the exact bits that are being used;

 2) Not reusing Python packages from system wide installations gives
extra assurance that the same behavior will be seen from tests run
on different machines;

With regards to downloads, pip already caches the downloaded wheels
and tarballs under ~/.cache/pip, so there should not be more than
one download even if the venv is destroyed and recreated.

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

diff --git a/tests/Makefile.include b/tests/Makefile.include
index 8f220e15d1..63477c8b4b 100644
--- a/tests/Makefile.include
+++ b/tests/Makefile.include
@@ -96,7 +96,7 @@ AVOCADO_TAGS=$(patsubst %-softmmu,-t arch:%, $(filter 
%-softmmu,$(TARGETS)))
 
 $(TESTS_VENV_DIR): $(TESTS_VENV_REQ)
$(call quiet-command, \
-$(PYTHON) -m venv --system-site-packages $@, \
+$(PYTHON) -m venv $@, \
 VENV, $@)
$(call quiet-command, \
 $(TESTS_VENV_DIR)/bin/python -m pip -q install -r 
$(TESTS_VENV_REQ), \
-- 
2.25.4

[PATCH] target/xtensa: don't generate extra EXCP_DEBUG on exception

[Max Filippov]

target/xtensa used to generate an extra EXCP_DEBUG exception before the
first instruction executed after an interrupt or an exception is taken
to allow single-stepping that instruction in the debugger.
This is no longer needed after the following commits:
a7ba744f4082 ("tcg/cpu-exec: precise single-stepping after an exception")
ba3c35d9c402 ("tcg/cpu-exec: precise single-stepping after an interrupt")
Drop exception state tracking/extra EXCP_DEBUG generation code.

Signed-off-by: Max Filippov 
---
This patch depends on the "target/xtensa: Make sure that tb->size != 0"
patch.

 target/xtensa/cpu.c| 1 -
 target/xtensa/cpu.h| 7 ---
 target/xtensa/exc_helper.c | 5 -
 target/xtensa/translate.c  | 6 --
 4 files changed, 19 deletions(-)

diff --git a/target/xtensa/cpu.c b/target/xtensa/cpu.c
index e2b2c7a71c1d..210ef800923b 100644
--- a/target/xtensa/cpu.c
+++ b/target/xtensa/cpu.c
@@ -79,7 +79,6 @@ static void xtensa_cpu_reset(DeviceState *dev)
 
 xcc->parent_reset(dev);
 
-env->exception_taken = 0;
 env->pc = env->config->exception_vector[EXC_RESET0 + env->static_vectors];
 env->sregs[LITBASE] &= ~1;
 #ifndef CONFIG_USER_ONLY
diff --git a/target/xtensa/cpu.h b/target/xtensa/cpu.h
index 3bd4f691c1a0..2345cb59c790 100644
--- a/target/xtensa/cpu.h
+++ b/target/xtensa/cpu.h
@@ -540,7 +540,6 @@ typedef struct CPUXtensaState {
 uint32_t ccount_base;
 #endif
 
-int exception_taken;
 int yield_needed;
 unsigned static_vectors;
 
@@ -711,7 +710,6 @@ static inline int cpu_mmu_index(CPUXtensaState *env, bool 
ifetch)
 #define XTENSA_TBFLAG_ICOUNT 0x20
 #define XTENSA_TBFLAG_CPENABLE_MASK 0x3fc0
 #define XTENSA_TBFLAG_CPENABLE_SHIFT 6
-#define XTENSA_TBFLAG_EXCEPTION 0x4000
 #define XTENSA_TBFLAG_WINDOW_MASK 0x18000
 #define XTENSA_TBFLAG_WINDOW_SHIFT 15
 #define XTENSA_TBFLAG_YIELD 0x2
@@ -732,8 +730,6 @@ typedef XtensaCPU ArchCPU;
 static inline void cpu_get_tb_cpu_state(CPUXtensaState *env, target_ulong *pc,
 target_ulong *cs_base, uint32_t *flags)
 {
-CPUState *cs = env_cpu(env);
-
 *pc = env->pc;
 *cs_base = 0;
 *flags = 0;
@@ -782,9 +778,6 @@ static inline void cpu_get_tb_cpu_state(CPUXtensaState 
*env, target_ulong *pc,
 if (xtensa_option_enabled(env->config, XTENSA_OPTION_COPROCESSOR)) {
 *flags |= env->sregs[CPENABLE] << XTENSA_TBFLAG_CPENABLE_SHIFT;
 }
-if (cs->singlestep_enabled && env->exception_taken) {
-*flags |= XTENSA_TBFLAG_EXCEPTION;
-}
 if (xtensa_option_enabled(env->config, XTENSA_OPTION_WINDOWED_REGISTER) &&
 (env->sregs[PS] & (PS_WOE | PS_EXCM)) == PS_WOE) {
 uint32_t windowstart = xtensa_replicate_windowstart(env) >>
diff --git a/target/xtensa/exc_helper.c b/target/xtensa/exc_helper.c
index 2f032bc05333..10e75ab070d7 100644
--- a/target/xtensa/exc_helper.c
+++ b/target/xtensa/exc_helper.c
@@ -40,9 +40,6 @@ void HELPER(exception)(CPUXtensaState *env, uint32_t excp)
 if (excp == EXCP_YIELD) {
 env->yield_needed = 0;
 }
-if (excp == EXCP_DEBUG) {
-env->exception_taken = 0;
-}
 cpu_loop_exit(cs);
 }
 
@@ -197,7 +194,6 @@ static void handle_interrupt(CPUXtensaState *env)
 }
 env->sregs[PS] |= PS_EXCM;
 }
-env->exception_taken = 1;
 }
 }
 
@@ -242,7 +238,6 @@ void xtensa_cpu_do_interrupt(CPUState *cs)
 
 vector = env->config->exception_vector[cs->exception_index];
 env->pc = relocated_vector(env, vector);
-env->exception_taken = 1;
 } else {
 qemu_log_mask(CPU_LOG_INT,
   "%s(pc = %08x) bad exception_index: %d\n",
diff --git a/target/xtensa/translate.c b/target/xtensa/translate.c
index 73584d9d605b..f93df87ec490 100644
--- a/target/xtensa/translate.c
+++ b/target/xtensa/translate.c
@@ -1279,12 +1279,6 @@ static void xtensa_tr_translate_insn(DisasContextBase 
*dcbase, CPUState *cpu)
 dc->base.is_jmp = DISAS_NORETURN;
 return;
 }
-if (dc->base.tb->flags & XTENSA_TBFLAG_EXCEPTION) {
-gen_exception(dc, EXCP_DEBUG);
-dc->base.pc_next = dc->pc + 1;
-dc->base.is_jmp = DISAS_NORETURN;
-return;
-}
 
 if (dc->icount) {
 TCGLabel *label = gen_new_label();
-- 
2.20.1

Re: [PATCH v3 3/3] accel/tcg: Assert that tb->size != 0 after translation

[Max Filippov]

On Thu, Apr 15, 2021 at 8:03 AM Peter Maydell  wrote:
>
> On Thu, 15 Apr 2021 at 02:24, Max Filippov  wrote:
> > I see a few places where target/xtensa may do that. E.g. it does that on 
> > entry
> > to an exception handler to allow for debugging its first instruction.
>
> That should now be handled by the common code, I think (see commits
> a7ba744f4082ab and ba3c35d9c402636) -- does xtensa still need to
> special case this ?

Thanks for letting me know. It now stops there twice, so no, special casing
is no longer needed. I'll send a patch.

-- 
Thanks.
-- Max

Re: [PATCH v4 3/4] target/xtensa: Make sure that tb->size != 0

[Max Filippov]

On Thu, Apr 15, 2021 at 6:03 AM Ilya Leoshkevich  wrote:
>
> tb_gen_code() assumes that tb->size must never be zero, otherwise it
> may produce spurious exceptions. For xtensa this may happen when
> decoding an unknown instruction, when handling a write into the
> CCOUNT or CCOMPARE special register and when single-stepping the first
> instruction of an exception handler.
>
> Fix by pretending that the size of the respective translation block is
> 1 in all these cases.
>
> Signed-off-by: Ilya Leoshkevich 
> ---
>  target/xtensa/translate.c | 3 +++
>  1 file changed, 3 insertions(+)

Tested-by: Max Filippov 
Acked-by: Max Filippov 

-- 
Thanks.
-- Max

Re: [PATCH for-6.0?] hw/arm/armsse: Give SSE-300 its own Property array

[Philippe Mathieu-Daudé]

On 4/15/21 8:23 PM, Peter Maydell wrote:
> SSE-300 currently shares the SSE-200 Property array. This is
> bad principally because the default values of the CPU0_FPU
> and CPU0_DSP properties disable the FPU and DSP on the CPU.
> That is correct for the SSE-300 but not the SSE-200.
> Give the SSE-300 its own Property array with the correct
> SSE-300 specific settings:
>  * SSE-300 has only one CPU, so no CPU1* properties
>  * SSE-300 CPU has FPU and DSP
> 
> Buglink: https://bugs.launchpad.net/qemu/+bug/1923861
> Signed-off-by: Peter Maydell 
> ---
> This is a simple and pretty safe fix, but I don't think it quite
> merits doing an rc4 by itself. I think if we do an rc4 for some
> other reason it ought to go in, though.

Sounds good.

Reviewed-by: Philippe Mathieu-Daudé 

Re: [PATCH 0/5] block, migration: improve debugging of migration bdrv_flush failure

[Connor Kuehl]

On 4/15/21 8:58 AM, Daniel P. Berrangé wrote:

I spent a while debugging a tricky migration failure today which was
ultimately caused by fdatasync() getting EACCESS. The existing probes
were not sufficient to diagnose this, so I had to resort to GDB. This
improves probes and block error reporting to make future diagnosis
possible without GDB.

Daniel P. Berrangé (5):
   migration: add trace point when vm_stop_force_state fails
   softmmu: add trace point when bdrv_flush_all fails
   block: preserve errno from fdatasync failures
   block: add trace point when fdatasync fails
   block: remove duplicate trace.h include

  block/file-posix.c | 10 +-
  block/trace-events |  1 +
  migration/migration.c  |  1 +
  migration/trace-events |  1 +
  softmmu/cpus.c |  7 ++-
  softmmu/trace-events   |  3 +++
  6 files changed, 17 insertions(+), 6 deletions(-)



For the series:

Reviewed-by: Connor Kuehl 

Re: [PATCH 0/2] i386: Fix interrupt based Async PF enablement

[Dr. David Alan Gilbert]

* Paolo Bonzini (pbonz...@redhat.com) wrote:
> On 06/04/21 13:42, Vitaly Kuznetsov wrote:
> > older machine types are still available (I disable it for <= 5.1 but we
> > can consider disabling it for 5.2 too). The feature is upstream since
> > Linux 5.8, I know that QEMU supports much older kernels but this doesn't
> > probably mean that we can't enable new KVM PV features unless all
> > supported kernels have it, we'd have to wait many years otherwise.
> 
> Yes, this is a known problem in fact. :(  In 6.0 we even support RHEL 7,
> though that will go away in 6.1.
> 
> We should take the occasion of dropping RHEL7 to be clearer about which
> kernels are supported.

It would be nice to be able to define sets of KVM functonality that we
can either start given machine types with, or provide a separate switch
to limit kvm functionality back to some defined point.  We do trip over
the same things pretty regularly when accidentally turning on new
features.

Dave

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

Re: [PATCH 1/2] qdev: Separate implementations of qdev_get_machine() for user and system

[Philippe Mathieu-Daudé]

On 4/15/21 6:56 PM, Greg Kurz wrote:
> On Thu, 15 Apr 2021 18:45:45 +0200
> Philippe Mathieu-Daudé  wrote:
> 
>> On 4/15/21 3:30 PM, Greg Kurz wrote:
>>> On Thu, 15 Apr 2021 14:39:55 +0200
>>> Philippe Mathieu-Daudé  wrote:
>>>
 On 4/9/21 6:03 PM, Greg Kurz wrote:
> Despite its simple name and common usage of "getting a pointer to
> the machine" in system-mode emulation, qdev_get_machine() has some
> subtilities.
>
> First, it can be called when running user-mode emulation : this is
> because user-mode partly relies on qdev to instantiate its CPU
> model.
>
> Second, but not least, it has a side-effect : if it cannot find an
> object at "/machine" in the QOM tree, it creates a dummy "container"
> object and put it there. A simple check on the type returned by
> qdev_get_machine() allows user-mode to run the common qdev code,
> skipping the parts that only make sense for system-mode.
>
> This side-effect turns out to complicate the use of qdev_get_machine()
> for the system-mode case though. Most notably, qdev_get_machine() must
> not be called before the machine object is added to the QOM tree by
> qemu_create_machine(), otherwise the existing dummy "container" object
> would cause qemu_create_machine() to fail with something like :
>
> Unexpected error in object_property_try_add() at ../../qom/object.c:1223:
> qemu-system-ppc64: attempt to add duplicate property 'machine' to
>  object (type 'container')
> Aborted (core dumped)
>
> This situation doesn't exist in the current code base, mostly because
> of preventive fixing of some "latent bugs" in QEMU 4.0 (see 1a3ec8c1564
> and e2fb3fbbf9c for details).
>
> A new kind of breakage was spotted very recently though :
>
> $ ./qemu-system-ppc64 -device power8_v2.0-spapr-cpu-core,help
> /home/thuth/devel/qemu/include/hw/boards.h:24:
>  MACHINE: Object 0x5635bd53af10 is not an instance of type machine
> Aborted (core dumped)
>
> This comes from the change 3df261b6676b in QEMU 5.0. It unwillingly
> added a new condition for qdev_get_machine() to be called too early,
> breaking MACHINE(qdev_get_machine()) in generic cpu-core code this
> time.
>
> In order to avoid further subtle breakages like this, change the
> implentation of qdev_get_machine() to:
> - keep the existing behaviour of creating the dummy "container"
>   object for the user-mode case only ;
> - abort() if the machine doesn't exist yet in the QOM tree for
>   the system-mode case. This gives a precise hint to developpers
>   that calling qdev_get_machine() too early is a programming bug.
>
> This is achieved with a new do_qdev_get_machine() function called
> from qdev_get_machine(), with different implementations for system
> and user mode.
>
> $ ./qemu-system-ppc64 -device power8_v2.0-spapr-cpu-core,help
> qemu-system-ppc64: ../../hw/core/machine.c:1290:
>  qdev_get_machine: Assertion `machine != NULL' failed.
> Aborted (core dumped)
>
> Reported-by: Thomas Huth 
> Signed-off-by: Greg Kurz 
> ---
>  hw/core/machine.c| 14 ++
>  hw/core/qdev.c   |  2 +-
>  include/hw/qdev-core.h   |  1 +
>  stubs/meson.build|  1 +
>  stubs/qdev-get-machine.c | 11 +++
>  5 files changed, 28 insertions(+), 1 deletion(-)
>  create mode 100644 stubs/qdev-get-machine.c
 ...

> diff --git a/stubs/meson.build b/stubs/meson.build
> index be6f6d609e58..b99ee2b33e94 100644
> --- a/stubs/meson.build
> +++ b/stubs/meson.build
> @@ -54,3 +54,4 @@ if have_system
>  else
>stub_ss.add(files('qdev.c'))
>  endif
> +stub_ss.add(files('qdev-get-machine.c'))

 Adding this as a stub looks suspicious...
 Why not add it in to user_ss in hw/core/meson.build?
 Maybe name the new file hw/core/qdev-user.c?

>>>
>>> It turns out that this isn't specific to user-mode but rather
>>> to any non-qemu-system-FOO binary built with qdev, e.g.
>>> test-qdev-global-props :
>>>
>>> #0  do_qdev_get_machine () at ../../stubs/qdev-get-machine.c:10
>>> #1  0x000100017938 in qdev_get_machine () at ../../hw/core/qdev.c:1134
>>> #2  0x00010001855c in device_set_realized (obj=0x100128b60, 
>>> value=, errp=0x7fffd4e0) at ../../hw/core/qdev.c:745
>>> #3  0x00010001cc5c in property_set_bool (obj=0x100128b60, v=>> out>, name=, opaque=0x1000f33f0, errp=0x7fffd4e0) at 
>>> ../../qom/object.c:2257
>>> #4  0x000100020a9c in object_property_set (obj=0x100128b60, 
>>> name=0x100093f78 "realized", v=0x100136d30, errp=0x1000e3af8 ) 
>>> at ../../qom/object.c:1402
>>> #5  0x00010001c38c in object_property_set_qobject (obj=0x100128b60, 
>>> name=0x100093f78 "realized", value=, errp=0x1000e3af8 
>>> ) at ../../qom/qom-qobject.c:28
>>> #6  0x000100020e20 in 

about mirror cancel

[Vladimir Sementsov-Ogievskiy]

Hi all!

Recently I've implemented fast-cancelling of mirror job: do 
bdrv_cancel_in_flight() in mirror_cancel().

Now I'm in doubt: is it a correct thing? I heard, that mirror-cancel is a kind 
of valid mirror completion..

Looking at documentation:

# Note that if you issue 'block-job-cancel' after 'drive-mirror' has indicated
# (via the event BLOCK_JOB_READY) that the source and destination are
# synchronized, then the event triggered by this command changes to
# BLOCK_JOB_COMPLETED, to indicate that the mirroring has ended and the
# destination now has a point-in-time copy tied to the time of the cancellation.

So, in other words, do we guarantee something to the user, if it calls 
mirror-cancel in ready state? Does this abuse exist in libvirt?



Note, that if cancelling all in-flight requests on target is wrong on mirror 
cancel, we still don't have real bug, as the only implementation of 
.bdrv_cancel_in_flight is stopping reconnect waiting in nbd driver. So, we'll 
cancel requests only if connection is already lost anyway.

But that probably means, that correct name of the handler would be 
.bdrv_cancel_in_fligth_requests_that_will_most_probably_fail_anyway()..

And it also means, that abuse of mirror-cancel as valid completion is a bad 
idea, as we can't distinguish the cases when user calls job-cancel to have a 
kind of point-in-time copy, or just to cancel job (and being not interested in 
the final state of target).

So, probably we need an option boolean argument for blockjob-cancel, like 
"hard", that will cancel in-flight writes on target node..

--
Best regards,
Vladimir

[Bug 1923861] Re: Hardfault when accessing FPSCR register

[Peter Maydell]

The bug fix for the QEMU part of this is
https://patchew.org/QEMU/20210415182353.8173-1-peter.mayd...@linaro.org/

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

Title:
  Hardfault when accessing FPSCR register

Status in QEMU:
  New

Bug description:
  QEMU release version: v6.0.0-rc2

  command line:
  qemu-system-arm -machine mps3-an547 -nographic -kernel .elf 
-semihosting -semihosting-config enable=on,target=native

  host operating system: Linux ISCNR90TMR1S 5.4.72-microsoft-standard-
  WSL2 #1 SMP Wed Oct 28 23:40:43 UTC 2020 x86_64 x86_64 x86_64
  GNU/Linux

  guest operating system: none (bare metal)

  Observation:
  I am simulating embedded firmware for a Cortex-M55 device, using MPS3-AN547 
machine. In the startup code I am accessing the FPSCR core register:

  unsigned int fpscr =__get_FPSCR();
  fpscr = fpscr & (~FPU_FPDSCR_AHP_Msk);
  __set_FPSCR(fpscr);

  where the register access functions __get_FPSCR() and
  __set_FPSCR(fpscr) are taken from CMSIS_5 at
  ./CMSIS/Core/include/cmsis_gcc.h

  I observe hardfaults upon __get_FPSCR() and __set_FPSCR(fpscr). The
  same startup code works fine on the Arm Corstone-300 FVP (MPS3-AN547).

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

Re: [PATCH for-6.0?] hw/arm/armsse: Give SSE-300 its own Property array

[Peter Maydell]

On Thu, 15 Apr 2021 at 19:23, Peter Maydell  wrote:
>
> SSE-300 currently shares the SSE-200 Property array. This is
> bad principally because the default values of the CPU0_FPU
> and CPU0_DSP properties disable the FPU and DSP on the CPU.
> That is correct for the SSE-300 but not the SSE-200.

Should read "correct for the SSE-200 but not the SSE-300",
of course.

> Give the SSE-300 its own Property array with the correct
> SSE-300 specific settings:
>  * SSE-300 has only one CPU, so no CPU1* properties
>  * SSE-300 CPU has FPU and DSP
>
> Buglink: https://bugs.launchpad.net/qemu/+bug/1923861
> Signed-off-by: Peter Maydell 
> ---

-- PMM

[Bug 1923861] Re: Hardfault when accessing FPSCR register

[Peter Maydell]

Thanks. This is a bug in the AN547 model -- we were accidentally turning
off the FPU. I'll write a patch.

NB that with that bug fixed your code then hits an UNDEF trying to do:
  0x0996:  eef7 1a10  vmrs r1, mvfr0

Only A-profile CPUs have MVFR0 accessible via the vmrs instruction. For
M-profile this register is memory-mapped, at 0xE000EF40.

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

Title:
  Hardfault when accessing FPSCR register

Status in QEMU:
  New

Bug description:
  QEMU release version: v6.0.0-rc2

  command line:
  qemu-system-arm -machine mps3-an547 -nographic -kernel .elf 
-semihosting -semihosting-config enable=on,target=native

  host operating system: Linux ISCNR90TMR1S 5.4.72-microsoft-standard-
  WSL2 #1 SMP Wed Oct 28 23:40:43 UTC 2020 x86_64 x86_64 x86_64
  GNU/Linux

  guest operating system: none (bare metal)

  Observation:
  I am simulating embedded firmware for a Cortex-M55 device, using MPS3-AN547 
machine. In the startup code I am accessing the FPSCR core register:

  unsigned int fpscr =__get_FPSCR();
  fpscr = fpscr & (~FPU_FPDSCR_AHP_Msk);
  __set_FPSCR(fpscr);

  where the register access functions __get_FPSCR() and
  __set_FPSCR(fpscr) are taken from CMSIS_5 at
  ./CMSIS/Core/include/cmsis_gcc.h

  I observe hardfaults upon __get_FPSCR() and __set_FPSCR(fpscr). The
  same startup code works fine on the Arm Corstone-300 FVP (MPS3-AN547).

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

[Bug 1924603] Re: Incorrect feature negotiation for vhost-vdpa netdevice

[Gautam Dawar]

QEMU version: 5.1.0

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

Title:
  Incorrect feature negotiation for vhost-vdpa netdevice

Status in QEMU:
  New

Bug description:
  QEMU cmdline:
  =
  ./x86_64-softmmu/qemu-system-x86_64 -machine accel=kvm -m 2G -hda  
/gautam/centos75_1.qcow2 -name gautam,process=gautam -enable-kvm -netdev 
vhost-vdpa,id=mynet0,vhostdev=/dev/vhost-vdpa-0 -device 
virtio-net-pci,netdev=mynet0,mac=02:AA:BB:DD:00:20,disable-modern=off,page-per-vq=on
 -cpu host --nographic

  Host OS:
  
  Linux kernel 5.11 running on x86 host

  Guest OS:
  ==
  CentOS 7.5

  Root cause analysis:
  =

  For vhost-vdpa netdevice, the feature negotiation results in sending
  the superset of features received from device in call to get_features
  vdpa ops callback.

  During the feature-negotiation phase, the acknowledged feature bits
  are initialized with backend_features  and then checked for supported
  feature bits in vhost_ack_features():

  void vhost_net_ack_features(struct vhost_net *net, uint64_t features)
  {
net->dev.acked_features = net->dev.backend_features;
vhost_ack_features(>dev, vhost_net_get_feature_bits(net), features);
  }

   
  The vhost_ack_features() function just builds up on the dev.acked_features 
and never trims it down:

  void vhost_ack_features(struct vhost_dev *hdev, const int *feature_bits, 
uint64_t features)
  { const int *bit = feature_bits;

while (*bit != VHOST_INVALID_FEATURE_BIT) {
 uint64_t bit_mask = (1ULL << *bit);  

  if (features & bit_mask)
   hdev->acked_features |= bit_mask;

  bit++;
 }
  }

  Because of this hdev->acked_features is always minimally equal to the
  value of device features and this is the value that is passed to the
  device in set_features callback:

  static int vhost_dev_set_features(struct vhost_dev *dev, bool enable_log)
  {
 uint64_t *features = dev->acked_features;
 .
 r = dev->vhost_ops->*vhost_set_features*(dev, features);
  }

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

Re: [RFC PATCH] accel/tcg: avoid re-translating one-shot instructions

[Peter Maydell]

On Thu, 15 Apr 2021 at 19:13, Alex Bennée  wrote:
>
>
> Peter Maydell  writes:
>
> > On Thu, 15 Apr 2021 at 17:25, Alex Bennée  wrote:
> >>
> >> By definition a single instruction is capable of being an IO
> >> instruction. This avoids a problem of triggering a cpu_io_recompile on
> >> a non-recorded translation which then fails because it expects
> >> tcg_tb_lookup() to succeed unconditionally. The normal use case
> >> requires a TB to be able to resolve machine state.
> >>
> >> The other users of tcg_tb_lookup() are able to tolerate a missing TB
> >> if the machine state has been resolved by other means - which in the
> >> single-shot case is always true because machine state is synced at the
> >> start of a block.
> >>
> >> Reported-by: Peter Maydell 
> >> Signed-off-by: Alex Bennée 
> >> ---
> >>  accel/tcg/translate-all.c | 2 +-
> >>  1 file changed, 1 insertion(+), 1 deletion(-)
> >>
> >> diff --git a/accel/tcg/translate-all.c b/accel/tcg/translate-all.c
> >> index ba6ab09790..b12d0898d0 100644
> >> --- a/accel/tcg/translate-all.c
> >> +++ b/accel/tcg/translate-all.c
> >> @@ -1863,7 +1863,7 @@ TranslationBlock *tb_gen_code(CPUState *cpu,
> >>
> >>  if (phys_pc == -1) {
> >>  /* Generate a one-shot TB with 1 insn in it */
> >> -cflags = (cflags & ~CF_COUNT_MASK) | 1;
> >> +cflags = (cflags & ~CF_COUNT_MASK) | CF_LAST_IO | 1;
> >>  }
> >>
> >>  max_insns = cflags & CF_COUNT_MASK;
> >> --
> >
> > Reviewed-by: Peter Maydell 
>
> Are you going to apply this directly or do you want it through a tree?

For the moment I've just added it to the list of "not fixed in 6.0 but
not quite meriting an rc4" items on the Planning page. If we need an rc4
I can apply it directly. (Though if you did whatever testing you'd do on
a pullreq that's beyond just "run it through gitlab" that would be
useful I think.)

thanks
-- PMM

[Bug 1924603] [NEW] Incorrect feature negotiation for vhost-vdpa netdevice

[Gautam Dawar]

Public bug reported:

QEMU cmdline:
=
./x86_64-softmmu/qemu-system-x86_64 -machine accel=kvm -m 2G -hda  
/gautam/centos75_1.qcow2 -name gautam,process=gautam -enable-kvm -netdev 
vhost-vdpa,id=mynet0,vhostdev=/dev/vhost-vdpa-0 -device 
virtio-net-pci,netdev=mynet0,mac=02:AA:BB:DD:00:20,disable-modern=off,page-per-vq=on
 -cpu host --nographic

Host OS:

Linux kernel 5.11 running on x86 host

Guest OS:
==
CentOS 7.5

Root cause analysis:
=

For vhost-vdpa netdevice, the feature negotiation results in sending the
superset of features received from device in call to get_features vdpa
ops callback.

During the feature-negotiation phase, the acknowledged feature bits are
initialized with backend_features  and then checked for supported
feature bits in vhost_ack_features():

void vhost_net_ack_features(struct vhost_net *net, uint64_t features)
{
  net->dev.acked_features = net->dev.backend_features;
  vhost_ack_features(>dev, vhost_net_get_feature_bits(net), features);
}

 
The vhost_ack_features() function just builds up on the dev.acked_features and 
never trims it down:

void vhost_ack_features(struct vhost_dev *hdev, const int *feature_bits, 
uint64_t features)
{ const int *bit = feature_bits;

  while (*bit != VHOST_INVALID_FEATURE_BIT) {
   uint64_t bit_mask = (1ULL << *bit);  

if (features & bit_mask)
 hdev->acked_features |= bit_mask;

bit++;
   }
}

Because of this hdev->acked_features is always minimally equal to the
value of device features and this is the value that is passed to the
device in set_features callback:

static int vhost_dev_set_features(struct vhost_dev *dev, bool enable_log)
{
   uint64_t *features = dev->acked_features;
   .
   r = dev->vhost_ops->*vhost_set_features*(dev, features);
}

** Affects: qemu
 Importance: Undecided
 Status: New


** Tags: v5.1.0

** Tags added: v5.1.0

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

Title:
  Incorrect feature negotiation for vhost-vdpa netdevice

Status in QEMU:
  New

Bug description:
  QEMU cmdline:
  =
  ./x86_64-softmmu/qemu-system-x86_64 -machine accel=kvm -m 2G -hda  
/gautam/centos75_1.qcow2 -name gautam,process=gautam -enable-kvm -netdev 
vhost-vdpa,id=mynet0,vhostdev=/dev/vhost-vdpa-0 -device 
virtio-net-pci,netdev=mynet0,mac=02:AA:BB:DD:00:20,disable-modern=off,page-per-vq=on
 -cpu host --nographic

  Host OS:
  
  Linux kernel 5.11 running on x86 host

  Guest OS:
  ==
  CentOS 7.5

  Root cause analysis:
  =

  For vhost-vdpa netdevice, the feature negotiation results in sending
  the superset of features received from device in call to get_features
  vdpa ops callback.

  During the feature-negotiation phase, the acknowledged feature bits
  are initialized with backend_features  and then checked for supported
  feature bits in vhost_ack_features():

  void vhost_net_ack_features(struct vhost_net *net, uint64_t features)
  {
net->dev.acked_features = net->dev.backend_features;
vhost_ack_features(>dev, vhost_net_get_feature_bits(net), features);
  }

   
  The vhost_ack_features() function just builds up on the dev.acked_features 
and never trims it down:

  void vhost_ack_features(struct vhost_dev *hdev, const int *feature_bits, 
uint64_t features)
  { const int *bit = feature_bits;

while (*bit != VHOST_INVALID_FEATURE_BIT) {
 uint64_t bit_mask = (1ULL << *bit);  

  if (features & bit_mask)
   hdev->acked_features |= bit_mask;

  bit++;
 }
  }

  Because of this hdev->acked_features is always minimally equal to the
  value of device features and this is the value that is passed to the
  device in set_features callback:

  static int vhost_dev_set_features(struct vhost_dev *dev, bool enable_log)
  {
 uint64_t *features = dev->acked_features;
 .
 r = dev->vhost_ops->*vhost_set_features*(dev, features);
  }

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

[PATCH for-6.0?] hw/arm/armsse: Give SSE-300 its own Property array

[Peter Maydell]

SSE-300 currently shares the SSE-200 Property array. This is
bad principally because the default values of the CPU0_FPU
and CPU0_DSP properties disable the FPU and DSP on the CPU.
That is correct for the SSE-300 but not the SSE-200.
Give the SSE-300 its own Property array with the correct
SSE-300 specific settings:
 * SSE-300 has only one CPU, so no CPU1* properties
 * SSE-300 CPU has FPU and DSP

Buglink: https://bugs.launchpad.net/qemu/+bug/1923861
Signed-off-by: Peter Maydell 
---
This is a simple and pretty safe fix, but I don't think it quite
merits doing an rc4 by itself. I think if we do an rc4 for some
other reason it ought to go in, though.

 hw/arm/armsse.c | 17 ++---
 1 file changed, 14 insertions(+), 3 deletions(-)

diff --git a/hw/arm/armsse.c b/hw/arm/armsse.c
index e5aeb9e485f..170dea8632d 100644
--- a/hw/arm/armsse.c
+++ b/hw/arm/armsse.c
@@ -84,7 +84,7 @@ static Property iotkit_properties[] = {
 DEFINE_PROP_END_OF_LIST()
 };
 
-static Property armsse_properties[] = {
+static Property sse200_properties[] = {
 DEFINE_PROP_LINK("memory", ARMSSE, board_memory, TYPE_MEMORY_REGION,
  MemoryRegion *),
 DEFINE_PROP_UINT32("EXP_NUMIRQ", ARMSSE, exp_numirq, 64),
@@ -97,6 +97,17 @@ static Property armsse_properties[] = {
 DEFINE_PROP_END_OF_LIST()
 };
 
+static Property sse300_properties[] = {
+DEFINE_PROP_LINK("memory", ARMSSE, board_memory, TYPE_MEMORY_REGION,
+ MemoryRegion *),
+DEFINE_PROP_UINT32("EXP_NUMIRQ", ARMSSE, exp_numirq, 64),
+DEFINE_PROP_UINT32("SRAM_ADDR_WIDTH", ARMSSE, sram_addr_width, 15),
+DEFINE_PROP_UINT32("init-svtor", ARMSSE, init_svtor, 0x1000),
+DEFINE_PROP_BOOL("CPU0_FPU", ARMSSE, cpu_fpu[0], true),
+DEFINE_PROP_BOOL("CPU0_DSP", ARMSSE, cpu_dsp[0], true),
+DEFINE_PROP_END_OF_LIST()
+};
+
 static const ARMSSEDeviceInfo iotkit_devices[] = {
 {
 .name = "timer0",
@@ -519,7 +530,7 @@ static const ARMSSEInfo armsse_variants[] = {
 .has_cpuid = true,
 .has_cpu_pwrctrl = false,
 .has_sse_counter = false,
-.props = armsse_properties,
+.props = sse200_properties,
 .devinfo = sse200_devices,
 .irq_is_common = sse200_irq_is_common,
 },
@@ -537,7 +548,7 @@ static const ARMSSEInfo armsse_variants[] = {
 .has_cpuid = true,
 .has_cpu_pwrctrl = true,
 .has_sse_counter = true,
-.props = armsse_properties,
+.props = sse300_properties,
 .devinfo = sse300_devices,
 .irq_is_common = sse300_irq_is_common,
 },
-- 
2.20.1

Re: [RFC PATCH] accel/tcg: avoid re-translating one-shot instructions

[Alex Bennée]

Peter Maydell  writes:

> On Thu, 15 Apr 2021 at 17:25, Alex Bennée  wrote:
>>
>> By definition a single instruction is capable of being an IO
>> instruction. This avoids a problem of triggering a cpu_io_recompile on
>> a non-recorded translation which then fails because it expects
>> tcg_tb_lookup() to succeed unconditionally. The normal use case
>> requires a TB to be able to resolve machine state.
>>
>> The other users of tcg_tb_lookup() are able to tolerate a missing TB
>> if the machine state has been resolved by other means - which in the
>> single-shot case is always true because machine state is synced at the
>> start of a block.
>>
>> Reported-by: Peter Maydell 
>> Signed-off-by: Alex Bennée 
>> ---
>>  accel/tcg/translate-all.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/accel/tcg/translate-all.c b/accel/tcg/translate-all.c
>> index ba6ab09790..b12d0898d0 100644
>> --- a/accel/tcg/translate-all.c
>> +++ b/accel/tcg/translate-all.c
>> @@ -1863,7 +1863,7 @@ TranslationBlock *tb_gen_code(CPUState *cpu,
>>
>>  if (phys_pc == -1) {
>>  /* Generate a one-shot TB with 1 insn in it */
>> -cflags = (cflags & ~CF_COUNT_MASK) | 1;
>> +cflags = (cflags & ~CF_COUNT_MASK) | CF_LAST_IO | 1;
>>  }
>>
>>  max_insns = cflags & CF_COUNT_MASK;
>> --
>
> Reviewed-by: Peter Maydell 

Are you going to apply this directly or do you want it through a tree?

>
> thanks
> -- PMM


-- 
Alex Bennée

Re: [RFC PATCH] accel/tcg: avoid re-translating one-shot instructions

[Richard Henderson]

On 4/15/21 9:24 AM, Alex Bennée wrote:

By definition a single instruction is capable of being an IO
instruction. This avoids a problem of triggering a cpu_io_recompile on
a non-recorded translation which then fails because it expects
tcg_tb_lookup() to succeed unconditionally. The normal use case
requires a TB to be able to resolve machine state.

The other users of tcg_tb_lookup() are able to tolerate a missing TB
if the machine state has been resolved by other means - which in the
single-shot case is always true because machine state is synced at the
start of a block.

Reported-by: Peter Maydell
Signed-off-by: Alex Bennée
---
  accel/tcg/translate-all.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)


Reviewed-by: Richard Henderson 

Re: [RFC PATCH] accel/tcg: avoid re-translating one-shot instructions

[Peter Maydell]

On Thu, 15 Apr 2021 at 17:25, Alex Bennée  wrote:
>
> By definition a single instruction is capable of being an IO
> instruction. This avoids a problem of triggering a cpu_io_recompile on
> a non-recorded translation which then fails because it expects
> tcg_tb_lookup() to succeed unconditionally. The normal use case
> requires a TB to be able to resolve machine state.
>
> The other users of tcg_tb_lookup() are able to tolerate a missing TB
> if the machine state has been resolved by other means - which in the
> single-shot case is always true because machine state is synced at the
> start of a block.
>
> Reported-by: Peter Maydell 
> Signed-off-by: Alex Bennée 
> ---
>  accel/tcg/translate-all.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/accel/tcg/translate-all.c b/accel/tcg/translate-all.c
> index ba6ab09790..b12d0898d0 100644
> --- a/accel/tcg/translate-all.c
> +++ b/accel/tcg/translate-all.c
> @@ -1863,7 +1863,7 @@ TranslationBlock *tb_gen_code(CPUState *cpu,
>
>  if (phys_pc == -1) {
>  /* Generate a one-shot TB with 1 insn in it */
> -cflags = (cflags & ~CF_COUNT_MASK) | 1;
> +cflags = (cflags & ~CF_COUNT_MASK) | CF_LAST_IO | 1;
>  }
>
>  max_insns = cflags & CF_COUNT_MASK;
> --

Reviewed-by: Peter Maydell 

thanks
-- PMM

[PING^2] [PATCH] [NFC] Mark locally used symbols as static.

[Yuri Gribov]

Hi all,

This patch makes locally used symbols static to enable more compiler
optimizations on them. Some of the symbols turned out to not be used
at all so I marked them with ATTRIBUTE_UNUSED (as I wasn't sure if
they were ok to delete).

The symbols have been identified with a pet project of mine:
https://github.com/yugr/Localizer

Link to patch: 
https://patchew.org/QEMU/cajotw+5ddmsr8qjqxaa1oht79rpmjcrwkybuartynr_ngux...@mail.gmail.com/

>From 4e790fd06becfbbf6fb106ac52ae1e4515f1ac73 Mon Sep 17 00:00:00 2001
From: Yury Gribov 
Date: Sat, 20 Mar 2021 23:39:15 +0300
Subject: [PATCH] Mark locally used symbols as static.

Signed-off-by: Yury Gribov 
Acked-by: Max Filippov  (xtensa)
Acked-by: David Gibson  (ppc)
Reviewed-by: Stefan Hajnoczi  (tracetool)
Reviewed-by: Taylor Simpson  (hexagon)
---
 disas/alpha.c | 16 ++--
 disas/m68k.c  | 78 -
 disas/mips.c  | 14 ++--
 disas/nios2.c | 84 +--
 disas/ppc.c   | 26 +++---
 disas/riscv.c |  2 +-
 pc-bios/optionrom/linuxboot_dma.c |  4 +-
 scripts/tracetool/format/c.py |  2 +-
 target/hexagon/gen_dectree_import.c   |  2 +-
 target/hexagon/opcodes.c  |  2 +-
 target/i386/cpu.c |  2 +-
 target/s390x/cpu_models.c |  2 +-
 .../xtensa/core-dc232b/xtensa-modules.c.inc   |  2 +-
 .../xtensa/core-dc233c/xtensa-modules.c.inc   |  2 +-
 target/xtensa/core-de212/xtensa-modules.c.inc |  2 +-
 .../core-de233_fpu/xtensa-modules.c.inc   |  2 +-
 .../xtensa/core-dsp3400/xtensa-modules.c.inc  |  2 +-
 target/xtensa/core-fsf/xtensa-modules.c.inc   |  2 +-
 .../xtensa-modules.c.inc  |  2 +-
 .../core-test_kc705_be/xtensa-modules.c.inc   |  2 +-
 .../core-test_mmuhifi_c3/xtensa-modules.c.inc |  2 +-
 21 files changed, 125 insertions(+), 127 deletions(-)

diff --git a/disas/alpha.c b/disas/alpha.c
index 3db90fa665..361a4ed101 100644
--- a/disas/alpha.c
+++ b/disas/alpha.c
@@ -56,8 +56,8 @@ struct alpha_opcode
 /* The table itself is sorted by major opcode number, and is otherwise
in the order in which the disassembler should consider
instructions.  */
-extern const struct alpha_opcode alpha_opcodes[];
-extern const unsigned alpha_num_opcodes;
+static const struct alpha_opcode alpha_opcodes[];
+static const unsigned alpha_num_opcodes;

 /* Values defined for the flags field of a struct alpha_opcode.  */

@@ -137,8 +137,8 @@ struct alpha_operand
 /* Elements in the table are retrieved by indexing with values from
the operands field of the alpha_opcodes table.  */

-extern const struct alpha_operand alpha_operands[];
-extern const unsigned alpha_num_operands;
+static const struct alpha_operand alpha_operands[];
+static const unsigned alpha_num_operands;

 /* Values defined for the flags field of a struct alpha_operand.  */

@@ -293,7 +293,7 @@ static int extract_ev6hwjhint (unsigned, int *);

 /* The operands table  */

-const struct alpha_operand alpha_operands[] =
+static const struct alpha_operand alpha_operands[] =
 {
   /* The fields are bits, shift, insert, extract, flags */
   /* The zero index is used to indicate end-of-list */
@@ -424,7 +424,7 @@ const struct alpha_operand alpha_operands[] =
 insert_ev6hwjhint, extract_ev6hwjhint }
 };

-const unsigned alpha_num_operands =
sizeof(alpha_operands)/sizeof(*alpha_operands);
+static ATTRIBUTE_UNUSED const unsigned alpha_num_operands =
sizeof(alpha_operands)/sizeof(*alpha_operands);

 /* The RB field when it is the same as the RA field in the same insn.
This operand is marked fake.  The insertion function just copies
@@ -706,7 +706,7 @@ extract_ev6hwjhint(unsigned insn, int *invalid
ATTRIBUTE_UNUSED)
that were not assigned to a particular extension.
 */

-const struct alpha_opcode alpha_opcodes[] = {
+static const struct alpha_opcode alpha_opcodes[] = {
   { "halt",SPCD(0x00,0x), BASE, ARG_NONE },
   { "draina",  SPCD(0x00,0x0002), BASE, ARG_NONE },
   { "bpt", SPCD(0x00,0x0080), BASE, ARG_NONE },
@@ -1732,7 +1732,7 @@ const struct alpha_opcode alpha_opcodes[] = {
   { "bgt", BRA(0x3F), BASE, ARG_BRA },
 };

-const unsigned alpha_num_opcodes =
sizeof(alpha_opcodes)/sizeof(*alpha_opcodes);
+static ATTRIBUTE_UNUSED const unsigned alpha_num_opcodes =
sizeof(alpha_opcodes)/sizeof(*alpha_opcodes);

 /* OSF register names.  */

diff --git a/disas/m68k.c b/disas/m68k.c
index aefaecfbd6..903d5cfec4 100644
--- a/disas/m68k.c
+++ b/disas/m68k.c
@@ -95,29 +95,29 @@ struct floatformat

 /* floatformats for IEEE single and double, big and little endian.  */

-extern const struct floatformat floatformat_ieee_single_big;
-extern const struct floatformat floatformat_ieee_single_little;
-extern const struct floatformat 

Re: [EXTERNAL] Re: [PULL 18/23] accel/tcg: re-factor non-RAM execution code

[Cédric Le Goater]

On 4/15/21 5:55 PM, Philippe Mathieu-Daudé wrote:
> On 4/15/21 4:54 PM, Peter Maydell wrote:
>> On Thu, 15 Apr 2021 at 15:32, Alex Bennée  wrote:
>>> --8<---cut here---start->8---
>>> accel/tcg: avoid re-translating one-shot instructions
>>>
>>> By definition a single instruction is capable of being an IO
>>> instruction. This avoids a problem of triggering a cpu_io_recompile on
>>> a non-cached translation which would only do exactly this anyway.
>>>
>>> Signed-off-by: Alex Bennée 
>>>
>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>> accel/tcg/translate-all.c | 2 +-
>>>
>>> modified   accel/tcg/translate-all.c
>>> @@ -1863,7 +1863,7 @@ TranslationBlock *tb_gen_code(CPUState *cpu,
>>>
>>>  if (phys_pc == -1) {
>>>  /* Generate a one-shot TB with 1 insn in it */
>>> -cflags = (cflags & ~CF_COUNT_MASK) | 1;
>>> +cflags = (cflags & ~CF_COUNT_MASK) | CF_LAST_IO | 1;
>>>  }
>>>
>>>  max_insns = cflags & CF_COUNT_MASK;
>>> --8<---cut here---end--->8---
>>
>> Yes, this fixes the problem. Do we want to put this in for 6.0? My
>> feeling is that executing from non-RAM is pretty niche, so maybe
>> if we need an rc4 anyway, but this isn't important enough to cause an
>> rc4 itself.
> 
> Isn't it the default for Aspeed machines (with U-Boot)? (Cc'ing Cédric).

You need to set the 'execute-in-place' machine option to load/execute the
instructions from the AHB window of CE0. It's not on by default because 
boot can be really slow with some recent u-boot which heavily trash the TBs.

But this seems to work fine with -rc3. 

C. 

Re: [PATCH v4 0/4] hw/i2c: Adds pca954x i2c mux switch device

[Patrick Venture]

On Thu, Apr 15, 2021 at 5:13 AM Corey Minyard  wrote:
>
> On Mon, Apr 12, 2021 at 12:45:18PM -0700, Patrick Venture wrote:
> > The i2c mux device pca954x implements two devices:
> >  - the pca9546 and pca9548.
>
> This looks good, I have pulled it into my queue.  6.0 is about to be
> released, I'll try to remember to request a pull after that.
>
> Thanks,
>
> -corey

Thanks, after I see the pull I'll send out a small stack I have of bmc
boards that can now use this device.

Patrick

>
> >
> > v4:
> >  - Fixed up bug where the i2c_scan_bus wasn't parameterizing the
> >  current_devs list.
> >  - Minor consistency changes in the i2c mux pca954x.
> >
> > v3:
> >  - fixup comment with missing end parenthesis.
> >  - removed superfluous object cast.
> >
> > v2:
> >  - the core i2c bus now calls a match method on each i2c child, which
> >  by default will only check for a match against itself.
> >  - the pca954x device overrides the i2c device match method to search
> >  the children for each of its buses that are active.
> >  - the pca954x device now owns an i2c bus for each channel, allowing
> >  the normal device model to attach devices to the channels.
> >
> > Patrick Venture (4):
> >   hw/i2c: name I2CNode list in I2CBus
> >   hw/i2c: add match method for device search
> >   hw/i2c: move search to i2c_scan_bus method
> >   hw/i2c: add pca954x i2c-mux switch
> >
> >  MAINTAINERS  |   6 +
> >  hw/i2c/Kconfig   |   4 +
> >  hw/i2c/core.c|  55 --
> >  hw/i2c/i2c_mux_pca954x.c | 290 +++
> >  hw/i2c/meson.build   |   1 +
> >  hw/i2c/trace-events  |   5 +
> >  include/hw/i2c/i2c.h |  17 +-
> >  include/hw/i2c/i2c_mux_pca954x.h |  19 ++
> >  8 files changed, 383 insertions(+), 14 deletions(-)
> >  create mode 100644 hw/i2c/i2c_mux_pca954x.c
> >  create mode 100644 include/hw/i2c/i2c_mux_pca954x.h
> >
> > --
> > 2.31.1.295.g9ea45b61b8-goog
> >
> >

Re: [PATCH RFC] migration: warn about non-migratable configurations unless '--no-migration' was specified

[Daniel P . Berrangé]

On Thu, Apr 15, 2021 at 05:40:40PM +0100, Daniel P. Berrangé wrote:
> On Thu, Apr 15, 2021 at 12:30:11PM -0400, Eduardo Habkost wrote:
> > On Thu, Apr 15, 2021 at 12:04 PM Daniel P. Berrangé  
> > wrote:
> > >
> > > Is it possible to query the migration blockers via QMP ?
> > 
> > I don't think it is, but we can add that if it's useful for libvirt.
> 
> I think it would be useful.

To expand on how it could be used by a libvirt client if we could wire
this up in our message reporting API.

A client app using libvirt python API would do:

msgs = dom.getMessages(libvirt.VIR_DOMAIN_MESSAGES_MIGBLOCKERS)

if len(msgs) > 0:
print("Domain %s uses features that block migration" % dom.getName())
for msg in msgs:
   print("   >> %s" % msg)


Migration blockers change over time, as hardware is hotplugged/unplugged,
so this isn't a one-off thing. The list of blockers is only valid at the
point in time that it is queried.

Regards,
Daniel
-- 
|: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|

Re: [EXTERNAL] Re: [PULL 18/23] accel/tcg: re-factor non-RAM execution code

[Peter Maydell]

On Thu, 15 Apr 2021 at 18:18, Cédric Le Goater  wrote:
>
> On 4/15/21 5:55 PM, Philippe Mathieu-Daudé wrote:
> > On 4/15/21 4:54 PM, Peter Maydell wrote:
> >> On Thu, 15 Apr 2021 at 15:32, Alex Bennée  wrote:
> >>> --8<---cut here---start->8---
> >>> accel/tcg: avoid re-translating one-shot instructions
> >>>
> >>> By definition a single instruction is capable of being an IO
> >>> instruction. This avoids a problem of triggering a cpu_io_recompile on
> >>> a non-cached translation which would only do exactly this anyway.
> >>>
> >>> Signed-off-by: Alex Bennée 
> >>>
> >>> 1 file changed, 1 insertion(+), 1 deletion(-)
> >>> accel/tcg/translate-all.c | 2 +-
> >>>
> >>> modified   accel/tcg/translate-all.c
> >>> @@ -1863,7 +1863,7 @@ TranslationBlock *tb_gen_code(CPUState *cpu,
> >>>
> >>>  if (phys_pc == -1) {
> >>>  /* Generate a one-shot TB with 1 insn in it */
> >>> -cflags = (cflags & ~CF_COUNT_MASK) | 1;
> >>> +cflags = (cflags & ~CF_COUNT_MASK) | CF_LAST_IO | 1;
> >>>  }
> >>>
> >>>  max_insns = cflags & CF_COUNT_MASK;
> >>> --8<---cut here---end--->8---
> >>
> >> Yes, this fixes the problem. Do we want to put this in for 6.0? My
> >> feeling is that executing from non-RAM is pretty niche, so maybe
> >> if we need an rc4 anyway, but this isn't important enough to cause an
> >> rc4 itself.
> >
> > Isn't it the default for Aspeed machines (with U-Boot)? (Cc'ing Cédric).
>
> You need to set the 'execute-in-place' machine option to load/execute the
> instructions from the AHB window of CE0. It's not on by default because
> boot can be really slow with some recent u-boot which heavily trash the TBs.
>
> But this seems to work fine with -rc3.

Triggering the bug requires both execute-in-place and -icount -- did
you test with -icount enabled?

thanks
-- PMM

Re: [PATCH v4 07/12] qtest/arm-cpu-features: Use generic qtest_has_accel() to check for TCG

[Andrew Jones]

On Thu, Apr 15, 2021 at 06:32:59PM +0200, Philippe Mathieu-Daudé wrote:
> Now than we can probe if the TCG accelerator is available
> at runtime with a QMP command, only run these tests if TCG
> is built into the QEMU binary.
> 
> Suggested-by: Andrew Jones 
> Signed-off-by: Philippe Mathieu-Daudé 
> ---
>  tests/qtest/arm-cpu-features.c | 16 +---
>  1 file changed, 9 insertions(+), 7 deletions(-)
>

Reviewed-by: Andrew Jones 

Re: [PATCH RFC] migration: warn about non-migratable configurations unless '--no-migration' was specified

[Dr. David Alan Gilbert]

* Daniel P. Berrangé (berra...@redhat.com) wrote:
> On Thu, Apr 15, 2021 at 05:44:02PM +0200, Vitaly Kuznetsov wrote:
> > When a migration blocker is added nothing is reported to the user,
> > inability to migrate such guest may come as a late surprise. As a bare
> > minimum, we can print a warning. To not pollute the output for those, who
> > have no intention to migrate their guests, introduce '--no-migration'
> > option which both block the migration and eliminates warning from
> 
> I wonder how this is actually going to work in practice ?
> 
> At the time libvirt starts a guest, it has no idea whether the guest
> is likely to need migration 3, 6, 12, 24 months in to the future.
> 
> IOW, we can't use a --no-migration flag and will be stuck with these
> warnings no mtter what.
> 
> Is it possible to query the migration blockers via QMP ?

It's possible to query the currently active ones, as of 6.0; from my
commit  3af8554bd068576b0399087583df48518a2a98f6 it appears in the
output of query-migrate in the 'blocked-reasons' list.

The HMP equivalent is a64aec725ea0b26fa4e44f8b8b8c72be9aaa4230 showing:

(qemu) info migrate
globals:
store-global-state: on
only-migratable: off
send-configuration: on
send-section-footer: on
decompress-error-check: on
clear-bitmap-shift: 18
Outgoing migration blocked:
  Migration is disabled when VirtFS export path '/home' is mounted in the 
guest using mount_tag 'fs'
  non-migratable device: :00:01.2/1/usb-serial

Dave

> Libvirt recently introduced a new API 'virDomainGetMessages' which
> lets us report a list of human targetted message strings against
> a guest. We use it for reporting when an operation has tainted
> a guest, and also use it for reporting when a deprecated QEMU
> feature is used.  We could use it to report any migration
> blockers that exist.
> 
> These are visible from 'virsh dominfo $guestname' and could also
> be displayed by a mgmt application.
> 
> NB, the messages are intentionally declared opaque strings, so
> mgmt apps shouldn't try to parse them. They merely know whether
> the count is non-zero for any given message class.
> 
> 
> Regards,
> Daniel
> -- 
> |: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
> |: https://libvirt.org -o-https://fstop138.berrange.com :|
> |: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|
-- 
Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK

Re: [PATCH 0/2] osdep: allow including qemu/osdep.h outside extern "C"

[Aleksandar Rikalo]

Hi Paolo,

Can you specify how to reproduce the issue ? We need more details about 
environment.

In my case, everything seems to work fine for the newest version of glib (2.68).

Thank you,
Aleksandar

> qemu/osdep.h is quite special in that, despite being part of QEMU sources,
> it is included by C++ source files as well.
>
> disas/nanomips.cpp is doing so within an 'extern "C"' block, which breaks
> with latest glib due to the inclusion of templates in glib.h.
>
> These patches implement Daniel Berrangé's idea of pushing the 'extern "C"'
> block within glib.h and including system headers (including glib.h,
> and in fact QEMU's own glib-compat.h too) *outside* the block.
>
> (CI has not finished running yet, but it seems encouraging).
>
> Paolo
>
> Paolo Bonzini (2):
>   osdep: include glib-compat.h before other QEMU headers
>   osdep: protect qemu/osdep.h with extern "C"
>
>  disas/nanomips.cpp  |  2 +-
>  include/qemu/compiler.h |  6 ++
>  include/qemu/osdep.h| 13 +++--
>  3 files changed, 18 insertions(+), 3 deletions(-)
>
> --
> 2.30.1

From: Philippe Mathieu-Daudé  on behalf of 
Philippe Mathieu-Daudé 
Sent: Tuesday, April 13, 2021 5:58 PM
To: Paolo Bonzini ; qemu-devel@nongnu.org 

Cc: peter.mayd...@linaro.org ; berra...@redhat.com 
; Aleksandar Rikalo ; 
vince.delvecc...@mediatek.com ; Petar Jovanovic 
; Filip Vidojevic 
Subject: Re: [PATCH 0/2] osdep: allow including qemu/osdep.h outside extern "C"

Cc'ing MediaTek reviewers.

On 4/13/21 1:37 PM, Paolo Bonzini wrote:
> qemu/osdep.h is quite special in that, despite being part of QEMU sources,
> it is included by C++ source files as well.
>
> disas/nanomips.cpp is doing so within an 'extern "C"' block, which breaks
> with latest glib due to the inclusion of templates in glib.h.
>
> These patches implement Daniel Berrangé's idea of pushing the 'extern "C"'
> block within glib.h and including system headers (including glib.h,
> and in fact QEMU's own glib-compat.h too) *outside* the block.
>
> (CI has not finished running yet, but it seems encouraging).
>
> Paolo
>
> Paolo Bonzini (2):
>   osdep: include glib-compat.h before other QEMU headers
>   osdep: protect qemu/osdep.h with extern "C"
>
>  disas/nanomips.cpp  |  2 +-
>  include/qemu/compiler.h |  6 ++
>  include/qemu/osdep.h| 13 +++--
>  3 files changed, 18 insertions(+), 3 deletions(-)
>

[PATCH v4 07/12] target/hexagon: expose next PC in DisasContext

[Alessandro Di Federico via]

From: Paolo Montesel 

Signed-off-by: Alessandro Di Federico 
Signed-off-by: Paolo Montesel 
---
 target/hexagon/translate.c | 3 ++-
 target/hexagon/translate.h | 1 +
 2 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/target/hexagon/translate.c b/target/hexagon/translate.c
index eeaad5f8ba..30ff3c5d51 100644
--- a/target/hexagon/translate.c
+++ b/target/hexagon/translate.c
@@ -503,11 +503,12 @@ static void decode_and_translate_packet(CPUHexagonState 
*env, DisasContext *ctx)
 if (decode_packet(nwords, words, , false) > 0) {
 HEX_DEBUG_PRINT_PKT();
 gen_start_packet(ctx, );
+ctx->npc = ctx->base.pc_next + pkt.encod_pkt_size_in_bytes;
 for (i = 0; i < pkt.num_insns; i++) {
 gen_insn(env, ctx, [i], );
 }
 gen_commit_packet(ctx, );
-ctx->base.pc_next += pkt.encod_pkt_size_in_bytes;
+ctx->base.pc_next = ctx->npc;
 } else {
 gen_exception(HEX_EXCP_INVALID_PACKET);
 ctx->base.is_jmp = DISAS_NORETURN;
diff --git a/target/hexagon/translate.h b/target/hexagon/translate.h
index 938f7fbb9f..2195e20f4b 100644
--- a/target/hexagon/translate.h
+++ b/target/hexagon/translate.h
@@ -36,6 +36,7 @@ typedef struct DisasContext {
 int preg_log_idx;
 uint8_t store_width[STORES_MAX];
 uint8_t s1_store_processed;
+uint32_t npc;
 } DisasContext;
 
 static inline void ctx_log_reg_write(DisasContext *ctx, int rnum)
-- 
2.31.1

Re: [PATCH v2] hw/block/nvme: align with existing style

[Klaus Jensen]

On Apr 15 15:13, Philippe Mathieu-Daudé wrote:

On 4/15/21 2:00 PM, Gollu Appalanaidu wrote:

Make uniform hexadecimal numbers format.

Signed-off-by: Gollu Appalanaidu 
---
-v2: Address review comments (Klaus)
use lower case hexa format for the code and in comments
use the same format as used in Spec. ("h")


^ This comment is relevant to the commit message.

Also it would be nice if the subsystem could describe somewhere
what is its style. Not sure where... The file header is probably
the simplest place.

Something like:

"While QEMU coding style prefers lowercase hexadecimal in constants,
the NVMe subsystem use the format from the NVMe specifications in
the comments: no '0x' prefix, uppercase, 'h' hexadecimal suffix."



+1 for that suggestion.


 hw/block/nvme-ns.c   |  2 +-
 hw/block/nvme.c  | 40 
 include/block/nvme.h | 10 +-
 3 files changed, 26 insertions(+), 26 deletions(-)





signature.asc
Description: PGP signature

Re: [PATCH 5/5] target/ppc: Implement paddi and replace addi insns

[Richard Henderson]

On 4/14/21 12:11 PM, Richard Henderson wrote:
This approach seems like it will work fine for MLS and MMIR prefixes.  For 8LS, 
8RR, and MRR prefixes, we'll need some extra help within ppc_tr_translate_insn. 
  E.g.


     insn = translator_ldl_swap(env, ctx->base.pc_next,
    need_byteswap(ctx));
     switch (ctx->prefix_type) {
     case PREFIX_NONE:
     ok = decode_opcode_space_0(ctx, insn) ||
  decode_legacy(ctx, insn);
     break;
     case PREFIX_MLS:
     case PREFIX_MMIRR:
     ok = decode_opcode_space_0(ctx, insn);
     break;


I played about with this last night, and there's an interesting trade-off:

(1) The thousands of 32-bit insns which do not allow prefixes
now each require 3 lines to assert that no prefix is present,

(2) There are only 12 MLS and 29 MMIRR prefixed insns.

I think it may well be that eliminating boiler-plate from thousands of insns it 
a good trade-off to special-casing 41 insns.


At which point, considering the multiple variations on 8RR and MMIRR prefixes, 
seems to indicate that we should decode all 64 bits all at once.



r~

[PATCH v4 01/12] tcg: expose TCGCond manipulation routines

[Alessandro Di Federico via]

From: Alessandro Di Federico 

This commit moves into a separate file routines used to manipulate
TCGCond. These will be employed by the idef-parser.

Signed-off-by: Alessandro Di Federico 
Signed-off-by: Paolo Montesel 
---
 include/tcg/tcg-cond.h | 101 +
 include/tcg/tcg.h  |  70 +---
 2 files changed, 102 insertions(+), 69 deletions(-)
 create mode 100644 include/tcg/tcg-cond.h

diff --git a/include/tcg/tcg-cond.h b/include/tcg/tcg-cond.h
new file mode 100644
index 00..2a38a386d4
--- /dev/null
+++ b/include/tcg/tcg-cond.h
@@ -0,0 +1,101 @@
+/*
+ * Tiny Code Generator for QEMU
+ *
+ * Copyright (c) 2008 Fabrice Bellard
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to 
deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING 
FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ */
+
+#ifndef TCG_COND_H
+#define TCG_COND_H
+
+/*
+ * Conditions.  Note that these are laid out for easy manipulation by
+ * the functions below:
+ *bit 0 is used for inverting;
+ *bit 1 is signed,
+ *bit 2 is unsigned,
+ *bit 3 is used with bit 0 for swapping signed/unsigned.
+ */
+typedef enum {
+/* non-signed */
+TCG_COND_NEVER  = 0 | 0 | 0 | 0,
+TCG_COND_ALWAYS = 0 | 0 | 0 | 1,
+TCG_COND_EQ = 8 | 0 | 0 | 0,
+TCG_COND_NE = 8 | 0 | 0 | 1,
+/* signed */
+TCG_COND_LT = 0 | 0 | 2 | 0,
+TCG_COND_GE = 0 | 0 | 2 | 1,
+TCG_COND_LE = 8 | 0 | 2 | 0,
+TCG_COND_GT = 8 | 0 | 2 | 1,
+/* unsigned */
+TCG_COND_LTU= 0 | 4 | 0 | 0,
+TCG_COND_GEU= 0 | 4 | 0 | 1,
+TCG_COND_LEU= 8 | 4 | 0 | 0,
+TCG_COND_GTU= 8 | 4 | 0 | 1,
+} TCGCond;
+
+/* Invert the sense of the comparison.  */
+static inline TCGCond tcg_invert_cond(TCGCond c)
+{
+return (TCGCond)(c ^ 1);
+}
+
+/* Swap the operands in a comparison.  */
+static inline TCGCond tcg_swap_cond(TCGCond c)
+{
+return c & 6 ? (TCGCond)(c ^ 9) : c;
+}
+
+/* Create an "unsigned" version of a "signed" comparison.  */
+static inline TCGCond tcg_unsigned_cond(TCGCond c)
+{
+return c & 2 ? (TCGCond)(c ^ 6) : c;
+}
+
+/* Create a "signed" version of an "unsigned" comparison.  */
+static inline TCGCond tcg_signed_cond(TCGCond c)
+{
+return c & 4 ? (TCGCond)(c ^ 6) : c;
+}
+
+/* Must a comparison be considered unsigned?  */
+static inline bool is_unsigned_cond(TCGCond c)
+{
+return (c & 4) != 0;
+}
+
+/*
+ * Create a "high" version of a double-word comparison.
+ * This removes equality from a LTE or GTE comparison.
+ */
+static inline TCGCond tcg_high_cond(TCGCond c)
+{
+switch (c) {
+case TCG_COND_GE:
+case TCG_COND_LE:
+case TCG_COND_GEU:
+case TCG_COND_LEU:
+return (TCGCond)(c ^ 8);
+default:
+return c;
+}
+}
+
+#endif /* TCG_COND_H */
diff --git a/include/tcg/tcg.h b/include/tcg/tcg.h
index 0f0695e90d..3dc468ebd8 100644
--- a/include/tcg/tcg.h
+++ b/include/tcg/tcg.h
@@ -34,6 +34,7 @@
 #include "tcg/tcg-mo.h"
 #include "tcg-target.h"
 #include "qemu/int128.h"
+#include "tcg/tcg-cond.h"
 
 /* XXX: make safe guess about sizes */
 #define MAX_OP_PER_INSTR 266
@@ -407,75 +408,6 @@ typedef TCGv_ptr TCGv_env;
 /* Used to align parameters.  See the comment before tcgv_i32_temp.  */
 #define TCG_CALL_DUMMY_ARG  ((TCGArg)0)
 
-/* Conditions.  Note that these are laid out for easy manipulation by
-   the functions below:
- bit 0 is used for inverting;
- bit 1 is signed,
- bit 2 is unsigned,
- bit 3 is used with bit 0 for swapping signed/unsigned.  */
-typedef enum {
-/* non-signed */
-TCG_COND_NEVER  = 0 | 0 | 0 | 0,
-TCG_COND_ALWAYS = 0 | 0 | 0 | 1,
-TCG_COND_EQ = 8 | 0 | 0 | 0,
-TCG_COND_NE = 8 | 0 | 0 | 1,
-/* signed */
-TCG_COND_LT = 0 | 0 | 2 | 0,
-TCG_COND_GE = 0 | 0 | 2 | 1,
-TCG_COND_LE = 8 | 0 | 2 | 0,
-TCG_COND_GT = 8 | 0 | 2 | 1,
-/* unsigned */
-TCG_COND_LTU= 0 | 4 | 0 | 0,
-TCG_COND_GEU= 0 | 4 | 0 | 1,
-TCG_COND_LEU= 8 | 4 | 0 

[Bug 1923861] Re: Hardfault when accessing FPSCR register

[ml-0]

Command line is
qemu-system-arm -machine mps3-an547 -nographic -kernel test.elf -semihosting 
-semihosting-config enable=on,target=native

Binary is attached. It does

int main(int argc, char* argv[])
{
SCB->NSACR |= (3U << 10U);/* enable Non-secure access to 
CP10 and CP11 coprocessors */
__DSB();
__ISB();

SCB->CPACR |= ((3U << 10U*2U) |   /* enable CP10 Full Access */
   (3U << 11U*2U)  ); /* enable CP11 Full Access */
__DSB();
__ISB();

//   enable DL branch cache
#define CCR  (*((volatile unsigned int *)0xE000ED14))
#define CCR_DL   (1 << 19)
  CCR |= CCR_DL;
__ISB();

   uint32_t result;
__asm volatile ("VMRS %0, fpscr" : "=r" (result) );   // <-- NOCP 
hardfault
printf("fpscr = 0x%08lx\r\n", result);
__asm volatile ("VMRS %0, mvfr0" : "=r" (result) );
printf("mvfr0 = 0x%08lx\r\n", result);
__asm volatile ("VMRS %0, mvfr1" : "=r" (result) );
printf("mvfr1 = 0x%08lx\r\n", result);
__asm volatile ("VMRS %0, mvfr2" : "=r" (result) );
printf("mvfr2 = 0x%08lx\r\n", result);

exit(0);
}

Thank you for your help!


** Attachment added: "test.elf"
   
https://bugs.launchpad.net/qemu/+bug/1923861/+attachment/5488449/+files/test.elf

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

Title:
  Hardfault when accessing FPSCR register

Status in QEMU:
  New

Bug description:
  QEMU release version: v6.0.0-rc2

  command line:
  qemu-system-arm -machine mps3-an547 -nographic -kernel .elf 
-semihosting -semihosting-config enable=on,target=native

  host operating system: Linux ISCNR90TMR1S 5.4.72-microsoft-standard-
  WSL2 #1 SMP Wed Oct 28 23:40:43 UTC 2020 x86_64 x86_64 x86_64
  GNU/Linux

  guest operating system: none (bare metal)

  Observation:
  I am simulating embedded firmware for a Cortex-M55 device, using MPS3-AN547 
machine. In the startup code I am accessing the FPSCR core register:

  unsigned int fpscr =__get_FPSCR();
  fpscr = fpscr & (~FPU_FPDSCR_AHP_Msk);
  __set_FPSCR(fpscr);

  where the register access functions __get_FPSCR() and
  __set_FPSCR(fpscr) are taken from CMSIS_5 at
  ./CMSIS/Core/include/cmsis_gcc.h

  I observe hardfaults upon __get_FPSCR() and __set_FPSCR(fpscr). The
  same startup code works fine on the Arm Corstone-300 FVP (MPS3-AN547).

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

Re: [PATCH v4 06/12] qtest/arm-cpu-features: Remove TCG fallback to KVM specific tests

[Andrew Jones]

On Thu, Apr 15, 2021 at 06:32:58PM +0200, Philippe Mathieu-Daudé wrote:
> sve_tests_sve_off_kvm() and test_query_cpu_model_expansion_kvm()
> tests are now only being run if KVM is available. Drop the TCG
> fallback.
> 
> Suggested-by: Andrew Jones 
> Signed-off-by: Philippe Mathieu-Daudé 
> ---
>  tests/qtest/arm-cpu-features.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/tests/qtest/arm-cpu-features.c b/tests/qtest/arm-cpu-features.c
> index 66300c3bc20..b1d406542f7 100644
> --- a/tests/qtest/arm-cpu-features.c
> +++ b/tests/qtest/arm-cpu-features.c
> @@ -21,7 +21,7 @@
>  #define SVE_MAX_VQ 16
>  
>  #define MACHINE "-machine virt,gic-version=max -accel tcg "
> -#define MACHINE_KVM "-machine virt,gic-version=max -accel kvm -accel tcg "
> +#define MACHINE_KVM "-machine virt,gic-version=max -accel kvm "
>  #define QUERY_HEAD  "{ 'execute': 'query-cpu-model-expansion', " \
>  "  'arguments': { 'type': 'full', "
>  #define QUERY_TAIL  "}}"
> -- 
> 2.26.3
>

Reviewed-by: Andrew Jones 

[PATCH v4 04/12] target/hexagon: make slot number an unsigned

[Alessandro Di Federico via]

From: Paolo Montesel 

Signed-off-by: Alessandro Di Federico 
Signed-off-by: Paolo Montesel 
---
 target/hexagon/genptr.c | 6 --
 target/hexagon/macros.h | 2 +-
 2 files changed, 5 insertions(+), 3 deletions(-)

diff --git a/target/hexagon/genptr.c b/target/hexagon/genptr.c
index 7481f4c1dd..fd18aabe8d 100644
--- a/target/hexagon/genptr.c
+++ b/target/hexagon/genptr.c
@@ -33,7 +33,8 @@ static inline TCGv gen_read_preg(TCGv pred, uint8_t num)
 return pred;
 }
 
-static inline void gen_log_predicated_reg_write(int rnum, TCGv val, int slot)
+static inline void gen_log_predicated_reg_write(int rnum, TCGv val,
+unsigned slot)
 {
 TCGv one = tcg_const_tl(1);
 TCGv zero = tcg_const_tl(0);
@@ -62,7 +63,8 @@ static inline void gen_log_reg_write(int rnum, TCGv val)
 #endif
 }
 
-static void gen_log_predicated_reg_write_pair(int rnum, TCGv_i64 val, int slot)
+static void gen_log_predicated_reg_write_pair(int rnum, TCGv_i64 val,
+  unsigned slot)
 {
 TCGv val32 = tcg_temp_new();
 TCGv one = tcg_const_tl(1);
diff --git a/target/hexagon/macros.h b/target/hexagon/macros.h
index cfcb8173ba..d9473c8823 100644
--- a/target/hexagon/macros.h
+++ b/target/hexagon/macros.h
@@ -154,7 +154,7 @@
 #define LOAD_CANCEL(EA) do { CANCEL; } while (0)
 
 #ifdef QEMU_GENERATE
-static inline void gen_pred_cancel(TCGv pred, int slot_num)
+static inline void gen_pred_cancel(TCGv pred, unsigned slot_num)
  {
 TCGv slot_mask = tcg_const_tl(1 << slot_num);
 TCGv tmp = tcg_temp_new();
-- 
2.31.1

Re: [PATCH 1/2] qdev: Separate implementations of qdev_get_machine() for user and system

[Greg Kurz]

On Thu, 15 Apr 2021 18:45:45 +0200
Philippe Mathieu-Daudé  wrote:

> On 4/15/21 3:30 PM, Greg Kurz wrote:
> > On Thu, 15 Apr 2021 14:39:55 +0200
> > Philippe Mathieu-Daudé  wrote:
> > 
> >> On 4/9/21 6:03 PM, Greg Kurz wrote:
> >>> Despite its simple name and common usage of "getting a pointer to
> >>> the machine" in system-mode emulation, qdev_get_machine() has some
> >>> subtilities.
> >>>
> >>> First, it can be called when running user-mode emulation : this is
> >>> because user-mode partly relies on qdev to instantiate its CPU
> >>> model.
> >>>
> >>> Second, but not least, it has a side-effect : if it cannot find an
> >>> object at "/machine" in the QOM tree, it creates a dummy "container"
> >>> object and put it there. A simple check on the type returned by
> >>> qdev_get_machine() allows user-mode to run the common qdev code,
> >>> skipping the parts that only make sense for system-mode.
> >>>
> >>> This side-effect turns out to complicate the use of qdev_get_machine()
> >>> for the system-mode case though. Most notably, qdev_get_machine() must
> >>> not be called before the machine object is added to the QOM tree by
> >>> qemu_create_machine(), otherwise the existing dummy "container" object
> >>> would cause qemu_create_machine() to fail with something like :
> >>>
> >>> Unexpected error in object_property_try_add() at ../../qom/object.c:1223:
> >>> qemu-system-ppc64: attempt to add duplicate property 'machine' to
> >>>  object (type 'container')
> >>> Aborted (core dumped)
> >>>
> >>> This situation doesn't exist in the current code base, mostly because
> >>> of preventive fixing of some "latent bugs" in QEMU 4.0 (see 1a3ec8c1564
> >>> and e2fb3fbbf9c for details).
> >>>
> >>> A new kind of breakage was spotted very recently though :
> >>>
> >>> $ ./qemu-system-ppc64 -device power8_v2.0-spapr-cpu-core,help
> >>> /home/thuth/devel/qemu/include/hw/boards.h:24:
> >>>  MACHINE: Object 0x5635bd53af10 is not an instance of type machine
> >>> Aborted (core dumped)
> >>>
> >>> This comes from the change 3df261b6676b in QEMU 5.0. It unwillingly
> >>> added a new condition for qdev_get_machine() to be called too early,
> >>> breaking MACHINE(qdev_get_machine()) in generic cpu-core code this
> >>> time.
> >>>
> >>> In order to avoid further subtle breakages like this, change the
> >>> implentation of qdev_get_machine() to:
> >>> - keep the existing behaviour of creating the dummy "container"
> >>>   object for the user-mode case only ;
> >>> - abort() if the machine doesn't exist yet in the QOM tree for
> >>>   the system-mode case. This gives a precise hint to developpers
> >>>   that calling qdev_get_machine() too early is a programming bug.
> >>>
> >>> This is achieved with a new do_qdev_get_machine() function called
> >>> from qdev_get_machine(), with different implementations for system
> >>> and user mode.
> >>>
> >>> $ ./qemu-system-ppc64 -device power8_v2.0-spapr-cpu-core,help
> >>> qemu-system-ppc64: ../../hw/core/machine.c:1290:
> >>>  qdev_get_machine: Assertion `machine != NULL' failed.
> >>> Aborted (core dumped)
> >>>
> >>> Reported-by: Thomas Huth 
> >>> Signed-off-by: Greg Kurz 
> >>> ---
> >>>  hw/core/machine.c| 14 ++
> >>>  hw/core/qdev.c   |  2 +-
> >>>  include/hw/qdev-core.h   |  1 +
> >>>  stubs/meson.build|  1 +
> >>>  stubs/qdev-get-machine.c | 11 +++
> >>>  5 files changed, 28 insertions(+), 1 deletion(-)
> >>>  create mode 100644 stubs/qdev-get-machine.c
> >> ...
> >>
> >>> diff --git a/stubs/meson.build b/stubs/meson.build
> >>> index be6f6d609e58..b99ee2b33e94 100644
> >>> --- a/stubs/meson.build
> >>> +++ b/stubs/meson.build
> >>> @@ -54,3 +54,4 @@ if have_system
> >>>  else
> >>>stub_ss.add(files('qdev.c'))
> >>>  endif
> >>> +stub_ss.add(files('qdev-get-machine.c'))
> >>
> >> Adding this as a stub looks suspicious...
> >> Why not add it in to user_ss in hw/core/meson.build?
> >> Maybe name the new file hw/core/qdev-user.c?
> >>
> > 
> > It turns out that this isn't specific to user-mode but rather
> > to any non-qemu-system-FOO binary built with qdev, e.g.
> > test-qdev-global-props :
> > 
> > #0  do_qdev_get_machine () at ../../stubs/qdev-get-machine.c:10
> > #1  0x000100017938 in qdev_get_machine () at ../../hw/core/qdev.c:1134
> > #2  0x00010001855c in device_set_realized (obj=0x100128b60, 
> > value=, errp=0x7fffd4e0) at ../../hw/core/qdev.c:745
> > #3  0x00010001cc5c in property_set_bool (obj=0x100128b60, v= > out>, name=, opaque=0x1000f33f0, errp=0x7fffd4e0) at 
> > ../../qom/object.c:2257
> > #4  0x000100020a9c in object_property_set (obj=0x100128b60, 
> > name=0x100093f78 "realized", v=0x100136d30, errp=0x1000e3af8 ) 
> > at ../../qom/object.c:1402
> > #5  0x00010001c38c in object_property_set_qobject (obj=0x100128b60, 
> > name=0x100093f78 "realized", value=, errp=0x1000e3af8 
> > ) at ../../qom/qom-qobject.c:28
> > #6  0x000100020e20 in object_property_set_bool (obj=0x100128b60, 
> > 

Re: [PATCH 1/2] qdev: Separate implementations of qdev_get_machine() for user and system

[Philippe Mathieu-Daudé]

On 4/15/21 3:30 PM, Greg Kurz wrote:
> On Thu, 15 Apr 2021 14:39:55 +0200
> Philippe Mathieu-Daudé  wrote:
> 
>> On 4/9/21 6:03 PM, Greg Kurz wrote:
>>> Despite its simple name and common usage of "getting a pointer to
>>> the machine" in system-mode emulation, qdev_get_machine() has some
>>> subtilities.
>>>
>>> First, it can be called when running user-mode emulation : this is
>>> because user-mode partly relies on qdev to instantiate its CPU
>>> model.
>>>
>>> Second, but not least, it has a side-effect : if it cannot find an
>>> object at "/machine" in the QOM tree, it creates a dummy "container"
>>> object and put it there. A simple check on the type returned by
>>> qdev_get_machine() allows user-mode to run the common qdev code,
>>> skipping the parts that only make sense for system-mode.
>>>
>>> This side-effect turns out to complicate the use of qdev_get_machine()
>>> for the system-mode case though. Most notably, qdev_get_machine() must
>>> not be called before the machine object is added to the QOM tree by
>>> qemu_create_machine(), otherwise the existing dummy "container" object
>>> would cause qemu_create_machine() to fail with something like :
>>>
>>> Unexpected error in object_property_try_add() at ../../qom/object.c:1223:
>>> qemu-system-ppc64: attempt to add duplicate property 'machine' to
>>>  object (type 'container')
>>> Aborted (core dumped)
>>>
>>> This situation doesn't exist in the current code base, mostly because
>>> of preventive fixing of some "latent bugs" in QEMU 4.0 (see 1a3ec8c1564
>>> and e2fb3fbbf9c for details).
>>>
>>> A new kind of breakage was spotted very recently though :
>>>
>>> $ ./qemu-system-ppc64 -device power8_v2.0-spapr-cpu-core,help
>>> /home/thuth/devel/qemu/include/hw/boards.h:24:
>>>  MACHINE: Object 0x5635bd53af10 is not an instance of type machine
>>> Aborted (core dumped)
>>>
>>> This comes from the change 3df261b6676b in QEMU 5.0. It unwillingly
>>> added a new condition for qdev_get_machine() to be called too early,
>>> breaking MACHINE(qdev_get_machine()) in generic cpu-core code this
>>> time.
>>>
>>> In order to avoid further subtle breakages like this, change the
>>> implentation of qdev_get_machine() to:
>>> - keep the existing behaviour of creating the dummy "container"
>>>   object for the user-mode case only ;
>>> - abort() if the machine doesn't exist yet in the QOM tree for
>>>   the system-mode case. This gives a precise hint to developpers
>>>   that calling qdev_get_machine() too early is a programming bug.
>>>
>>> This is achieved with a new do_qdev_get_machine() function called
>>> from qdev_get_machine(), with different implementations for system
>>> and user mode.
>>>
>>> $ ./qemu-system-ppc64 -device power8_v2.0-spapr-cpu-core,help
>>> qemu-system-ppc64: ../../hw/core/machine.c:1290:
>>>  qdev_get_machine: Assertion `machine != NULL' failed.
>>> Aborted (core dumped)
>>>
>>> Reported-by: Thomas Huth 
>>> Signed-off-by: Greg Kurz 
>>> ---
>>>  hw/core/machine.c| 14 ++
>>>  hw/core/qdev.c   |  2 +-
>>>  include/hw/qdev-core.h   |  1 +
>>>  stubs/meson.build|  1 +
>>>  stubs/qdev-get-machine.c | 11 +++
>>>  5 files changed, 28 insertions(+), 1 deletion(-)
>>>  create mode 100644 stubs/qdev-get-machine.c
>> ...
>>
>>> diff --git a/stubs/meson.build b/stubs/meson.build
>>> index be6f6d609e58..b99ee2b33e94 100644
>>> --- a/stubs/meson.build
>>> +++ b/stubs/meson.build
>>> @@ -54,3 +54,4 @@ if have_system
>>>  else
>>>stub_ss.add(files('qdev.c'))
>>>  endif
>>> +stub_ss.add(files('qdev-get-machine.c'))
>>
>> Adding this as a stub looks suspicious...
>> Why not add it in to user_ss in hw/core/meson.build?
>> Maybe name the new file hw/core/qdev-user.c?
>>
> 
> It turns out that this isn't specific to user-mode but rather
> to any non-qemu-system-FOO binary built with qdev, e.g.
> test-qdev-global-props :
> 
> #0  do_qdev_get_machine () at ../../stubs/qdev-get-machine.c:10
> #1  0x000100017938 in qdev_get_machine () at ../../hw/core/qdev.c:1134
> #2  0x00010001855c in device_set_realized (obj=0x100128b60, 
> value=, errp=0x7fffd4e0) at ../../hw/core/qdev.c:745
> #3  0x00010001cc5c in property_set_bool (obj=0x100128b60, v= out>, name=, opaque=0x1000f33f0, errp=0x7fffd4e0) at 
> ../../qom/object.c:2257
> #4  0x000100020a9c in object_property_set (obj=0x100128b60, 
> name=0x100093f78 "realized", v=0x100136d30, errp=0x1000e3af8 ) 
> at ../../qom/object.c:1402
> #5  0x00010001c38c in object_property_set_qobject (obj=0x100128b60, 
> name=0x100093f78 "realized", value=, errp=0x1000e3af8 
> ) at ../../qom/qom-qobject.c:28
> #6  0x000100020e20 in object_property_set_bool (obj=0x100128b60, 
> name=0x100093f78 "realized", value=, errp=0x1000e3af8 
> ) at ../../qom/object.c:1472
> #7  0x00010001612c in qdev_realize (dev=0x100128b60, bus=, 
> errp=0x1000e3af8 ) at ../../hw/core/qdev.c:389
> #8  0x0001fb10 in test_static_prop_subprocess () at 
> 

[PATCH v4 02/12] target/hexagon: update MAINTAINERS for idef-parser

[Alessandro Di Federico via]

From: Alessandro Di Federico 

Signed-off-by: Alessandro Di Federico 
---
 MAINTAINERS | 8 
 1 file changed, 8 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 36055f14c5..158badbd18 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -193,11 +193,19 @@ Hexagon TCG CPUs
 M: Taylor Simpson 
 S: Supported
 F: target/hexagon/
+X: target/hexagon/idef-parser/
+X: target/hexagon/gen_idef_parser_funcs.py
 F: linux-user/hexagon/
 F: tests/tcg/hexagon/
 F: disas/hexagon.c
 F: default-configs/targets/hexagon-linux-user.mak
 
+Hexagon idef-parser
+M: Alessandro Di Federico 
+S: Supported
+F: target/hexagon/idef-parser/
+F: target/hexagon/gen_idef_parser_funcs.py
+
 HPPA (PA-RISC) TCG CPUs
 M: Richard Henderson 
 S: Maintained
-- 
2.31.1

Re: [PATCH v4 04/12] qtest/arm-cpu-features: Use generic qtest_has_accel() to check for KVM

[Andrew Jones]

On Thu, Apr 15, 2021 at 06:32:56PM +0200, Philippe Mathieu-Daudé wrote:
> Use the recently added generic qtest_has_accel() method to
> check if KVM is available.
> 
> Suggested-by: Claudio Fontana 
> Signed-off-by: Philippe Mathieu-Daudé 
> ---
>  tests/qtest/arm-cpu-features.c | 25 +
>  1 file changed, 1 insertion(+), 24 deletions(-)
>

Reviewed-by: Andrew Jones 

Re: [PATCH v4 05/12] qtest/arm-cpu-features: Restrict sve_tests_sve_off_kvm test to KVM

[Andrew Jones]

On Thu, Apr 15, 2021 at 06:32:57PM +0200, Philippe Mathieu-Daudé wrote:
> The sve_tests_sve_off_kvm() test is KVM specific.
> Only run it if KVM is available.
> 
> Suggested-by: Andrew Jones 
> Signed-off-by: Philippe Mathieu-Daudé 
> ---
>  tests/qtest/arm-cpu-features.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/tests/qtest/arm-cpu-features.c b/tests/qtest/arm-cpu-features.c
> index 7f4b2521277..66300c3bc20 100644
> --- a/tests/qtest/arm-cpu-features.c
> +++ b/tests/qtest/arm-cpu-features.c
> @@ -604,6 +604,8 @@ int main(int argc, char **argv)
>  if (g_str_equal(qtest_get_arch(), "aarch64") && qtest_has_accel("kvm")) {
>  qtest_add_data_func("/arm/kvm/query-cpu-model-expansion",
>  NULL, test_query_cpu_model_expansion_kvm);
> +qtest_add_data_func("/arm/kvm/query-cpu-model-expansion/sve-off",
> +NULL, sve_tests_sve_off_kvm);
>  }
>  
>  if (g_str_equal(qtest_get_arch(), "aarch64")) {
> @@ -611,8 +613,6 @@ int main(int argc, char **argv)
>  NULL, sve_tests_sve_max_vq_8);
>  qtest_add_data_func("/arm/max/query-cpu-model-expansion/sve-off",
>  NULL, sve_tests_sve_off);
> -qtest_add_data_func("/arm/kvm/query-cpu-model-expansion/sve-off",
> -NULL, sve_tests_sve_off_kvm);
>  }
>  
>  return g_test_run();
> -- 
> 2.26.3
>

Reviewed-by: Andrew Jones 

[PATCH v4 11/12] target/hexagon: call idef-parser functions

[Alessandro Di Federico via]

From: Alessandro Di Federico 

Extend gen_tcg_funcs.py in order to emit calls to the functions emitted
by the idef-parser, if available.

Signed-off-by: Alessandro Di Federico 
---
 target/hexagon/gen_tcg_funcs.py | 28 ++--
 target/hexagon/hex_common.py| 10 ++
 target/hexagon/meson.build  | 22 +-
 3 files changed, 49 insertions(+), 11 deletions(-)

diff --git a/target/hexagon/gen_tcg_funcs.py b/target/hexagon/gen_tcg_funcs.py
index db9f663a77..5980dab8ee 100755
--- a/target/hexagon/gen_tcg_funcs.py
+++ b/target/hexagon/gen_tcg_funcs.py
@@ -394,7 +394,29 @@ def gen_tcg_func(f, tag, regs, imms):
 if (hex_common.is_read(regid)):
 genptr_src_read_opn(f,regtype,regid,tag)
 
-if ( hex_common.skip_qemu_helper(tag) ):
+if hex_common.is_idef_parser_enabled(tag):
+declared = []
+## Handle registers
+for regtype,regid,toss,numregs in regs:
+if (hex_common.is_pair(regid)
+or (hex_common.is_single(regid)
+and hex_common.is_old_val(regtype, regid, tag))):
+declared.append("%s%sV" % (regtype, regid))
+if regtype == "M":
+declared.append("%s%sN" % (regtype, regid))
+elif hex_common.is_new_val(regtype, regid, tag):
+declared.append("%s%sN" % (regtype,regid))
+else:
+print("Bad register parse: ",regtype,regid,toss,numregs)
+
+## Handle immediates
+for immlett,bits,immshift in imms:
+declared.append(hex_common.imm_name(immlett))
+
+arguments = ", ".join(["ctx", "insn", "pkt"] + declared)
+f.write("emit_%s(%s);\n" % (tag, arguments))
+
+elif ( hex_common.skip_qemu_helper(tag) ):
 f.write("fGEN_TCG_%s(%s);\n" % (tag, hex_common.semdict[tag]))
 else:
 ## Generate the call to the helper
@@ -455,12 +477,14 @@ def main():
 hex_common.read_attribs_file(sys.argv[2])
 hex_common.read_overrides_file(sys.argv[3])
 hex_common.calculate_attribs()
+hex_common.read_idef_parser_enabled_file(sys.argv[4])
 tagregs = hex_common.get_tagregs()
 tagimms = hex_common.get_tagimms()
 
-with open(sys.argv[4], 'w') as f:
+with open(sys.argv[5], 'w') as f:
 f.write("#ifndef HEXAGON_TCG_FUNCS_H\n")
 f.write("#define HEXAGON_TCG_FUNCS_H\n\n")
+f.write("#include \"idef-generated-emitter.h.inc\"\n\n")
 
 for tag in hex_common.tags:
 ## Skip the priv instructions
diff --git a/target/hexagon/hex_common.py b/target/hexagon/hex_common.py
index b3b534057d..648ad29e94 100755
--- a/target/hexagon/hex_common.py
+++ b/target/hexagon/hex_common.py
@@ -28,6 +28,7 @@
 attribinfo = {}   # Register information and misc
 tags = [] # list of all tags
 overrides = {}# tags with helper overrides
+idef_parser_enabled = {} # tags enabled for idef-parser
 
 # We should do this as a hash for performance,
 # but to keep order let's keep it as a list.
@@ -201,6 +202,9 @@ def need_ea(tag):
 def skip_qemu_helper(tag):
 return tag in overrides.keys()
 
+def is_idef_parser_enabled(tag):
+return tag in idef_parser_enabled
+
 def imm_name(immlett):
 return "%siV" % immlett
 
@@ -232,3 +236,9 @@ def read_overrides_file(name):
 continue
 tag = overridere.findall(line)[0]
 overrides[tag] = True
+
+def read_idef_parser_enabled_file(name):
+global idef_parser_enabled
+with open(name, "r") as idef_parser_enabled_file:
+lines = idef_parser_enabled_file.read().strip().split("\n")
+idef_parser_enabled = set(lines)
diff --git a/target/hexagon/meson.build b/target/hexagon/meson.build
index abd931d636..7f30237e59 100644
--- a/target/hexagon/meson.build
+++ b/target/hexagon/meson.build
@@ -69,15 +69,6 @@ helper_protos_generated = custom_target(
 )
 hexagon_ss.add(helper_protos_generated)
 
-tcg_funcs_generated = custom_target(
-'tcg_funcs_generated.c.inc',
-output: 'tcg_funcs_generated.c.inc',
-depends: [semantics_generated],
-depend_files: [hex_common_py, attribs_def, gen_tcg_h],
-command: [python, files('gen_tcg_funcs.py'), semantics_generated, 
attribs_def, gen_tcg_h, '@OUTPUT@'],
-)
-hexagon_ss.add(tcg_funcs_generated)
-
 tcg_func_table_generated = custom_target(
 'tcg_func_table_generated.c.inc',
 output: 'tcg_func_table_generated.c.inc',
@@ -221,4 +212,17 @@ idef_generated_tcg = custom_target(
 command: [idef_parser, '@INPUT@', '@OUTPUT0@', '@OUTPUT1@', '@OUTPUT2@'],
 )
 
+idef_generated_list = idef_generated_tcg[2].full_path()
+
+hexagon_ss.add(idef_generated_tcg)
+
+tcg_funcs_generated = custom_target(
+'tcg_funcs_generated.c.inc',
+output: 'tcg_funcs_generated.c.inc',
+depends: [semantics_generated, idef_generated_tcg],
+depend_files: [hex_common_py, attribs_def, gen_tcg_h],
+command: [python, files('gen_tcg_funcs.py'), 

Re: [PATCH 5/5] block: remove duplicate trace.h include

[Dr. David Alan Gilbert]

* Daniel P. Berrangé (berra...@redhat.com) wrote:
> Signed-off-by: Daniel P. Berrangé 

Reviewed-by: Dr. David Alan Gilbert 

> ---
>  block/file-posix.c | 2 --
>  1 file changed, 2 deletions(-)
> 
> diff --git a/block/file-posix.c b/block/file-posix.c
> index 6aafeda44f..2538e43299 100644
> --- a/block/file-posix.c
> +++ b/block/file-posix.c
> @@ -106,8 +106,6 @@
>  #include 
>  #endif
>  
> -#include "trace.h"
> -
>  /* OS X does not have O_DSYNC */
>  #ifndef O_DSYNC
>  #ifdef O_SYNC
> -- 
> 2.30.2
> 
-- 
Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK

[PATCH v4 11/12] tests/qtest: Do not restrict bios-tables-test to Aarch64 hosts anymore

[Philippe Mathieu-Daudé]

Since commit 82bf7ae84ce ("target/arm: Remove KVM support for
32-bit Arm hosts") we can remove the comment / check added in
commit ab6b6a4 and directly run the bios-tables-test.

Reviewed-by: Eric Blake 
Signed-off-by: Philippe Mathieu-Daudé 
---
 tests/qtest/meson.build | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/tests/qtest/meson.build b/tests/qtest/meson.build
index 0c767389217..46de073d155 100644
--- a/tests/qtest/meson.build
+++ b/tests/qtest/meson.build
@@ -175,14 +175,13 @@
'boot-serial-test',
'hexloader-test']
 
-# TODO: once aarch64 TCG is fixed on ARM 32 bit host, make bios-tables-test 
unconditional
 qtests_aarch64 = \
-  (cpu != 'arm' ? ['bios-tables-test'] : []) + 
 \
   (config_all_devices.has_key('CONFIG_TPM_TIS_SYSBUS') ? 
['tpm-tis-device-test'] : []) +\
   (config_all_devices.has_key('CONFIG_TPM_TIS_SYSBUS') ? 
['tpm-tis-device-swtpm-test'] : []) +  \
   ['arm-cpu-features',
'numa-test',
'boot-serial-test',
+   'bios-tables-test',
'xlnx-can-test',
'migration-test']
 
-- 
2.26.3

Re: [PATCH RFC] migration: warn about non-migratable configurations unless '--no-migration' was specified

[Daniel P . Berrangé]

On Thu, Apr 15, 2021 at 12:30:11PM -0400, Eduardo Habkost wrote:
> On Thu, Apr 15, 2021 at 12:04 PM Daniel P. Berrangé  
> wrote:
> >
> > On Thu, Apr 15, 2021 at 05:44:02PM +0200, Vitaly Kuznetsov wrote:
> > > When a migration blocker is added nothing is reported to the user,
> > > inability to migrate such guest may come as a late surprise. As a bare
> > > minimum, we can print a warning. To not pollute the output for those, who
> > > have no intention to migrate their guests, introduce '--no-migration'
> > > option which both block the migration and eliminates warning from
> >
> > I wonder how this is actually going to work in practice ?
> >
> > At the time libvirt starts a guest, it has no idea whether the guest
> > is likely to need migration 3, 6, 12, 24 months in to the future.
> 
> I believe the libvirt API could be extended so applications can
> indicate that migration is not a required feature. This would let QEMU
> automatically enable useful but non-migration-friendly features. For
> example, I would expect "-cpu host" to become the default CPU model if
> --no-migration is specified.
> 
> >
> > IOW, we can't use a --no-migration flag and will be stuck with these
> > warnings no mtter what.
> 
> The expected behavior of the libvirt API is to create migratable VMs
> by default, isn't it? This means it would be valid for libvirt to use
> "--only-migratable" by default.

Yes & no.  Libvirt attempts to explicitly configure the guest so
that it has a stable ABI, and thus can theoretically be migrated
We doesn't try to secondguess which QEMU features may or may not
be migratable though.

> If libvirt can't use "--only-migratable" by default, I would say it's
> a deficiency of the libvirt API that needs to be addressed.

It is valid to boot a guest with an attached PCI device, which will
make it non-migratable, but later hot-unplug the PCI device before
starting the migration.

> > Is it possible to query the migration blockers via QMP ?
> 
> I don't think it is, but we can add that if it's useful for libvirt.

I think it would be useful.

> > Libvirt recently introduced a new API 'virDomainGetMessages' which
> > lets us report a list of human targetted message strings against
> > a guest. We use it for reporting when an operation has tainted
> > a guest, and also use it for reporting when a deprecated QEMU
> > feature is used.  We could use it to report any migration
> > blockers that exist.
> >
> > These are visible from 'virsh dominfo $guestname' and could also
> > be displayed by a mgmt application.
> 
> Cool!
> 
> Will the API include all warnings printed by QEMU to stderr?

No, we don't look at stderr - that's just going into a logfile.

When I refer to deprecated featurs here, I'm talking specifically
about stuff we see from the QAPI schema. Right now that means
CPU models and machine types with the deprecated flag. It can be
extended to other devices later.

Regards,
Daniel
-- 
|: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|

[PATCH v4 10/12] target/hexagon: import parser for idef-parser

[Alessandro Di Federico via]

From: Paolo Montesel 

Signed-off-by: Alessandro Di Federico 
Signed-off-by: Paolo Montesel 
---
 target/hexagon/idef-parser/idef-parser.y  |  947 +++
 target/hexagon/idef-parser/parser-helpers.c   | 2374 +
 target/hexagon/idef-parser/parser-helpers.h   |  347 +++
 target/hexagon/meson.build|   26 +-
 tests/docker/dockerfiles/alpine.docker|1 +
 tests/docker/dockerfiles/centos7.docker   |1 +
 tests/docker/dockerfiles/centos8.docker   |1 +
 tests/docker/dockerfiles/debian10.docker  |2 +
 .../dockerfiles/fedora-i386-cross.docker  |2 +
 .../dockerfiles/fedora-win32-cross.docker |2 +
 .../dockerfiles/fedora-win64-cross.docker |2 +
 tests/docker/dockerfiles/fedora.docker|1 +
 tests/docker/dockerfiles/opensuse-leap.docker |1 +
 tests/docker/dockerfiles/ubuntu.docker|2 +
 tests/docker/dockerfiles/ubuntu1804.docker|2 +
 tests/docker/dockerfiles/ubuntu2004.docker|4 +-
 16 files changed, 3713 insertions(+), 2 deletions(-)
 create mode 100644 target/hexagon/idef-parser/idef-parser.y
 create mode 100644 target/hexagon/idef-parser/parser-helpers.c
 create mode 100644 target/hexagon/idef-parser/parser-helpers.h

diff --git a/target/hexagon/idef-parser/idef-parser.y 
b/target/hexagon/idef-parser/idef-parser.y
new file mode 100644
index 00..dbfc9572f9
--- /dev/null
+++ b/target/hexagon/idef-parser/idef-parser.y
@@ -0,0 +1,947 @@
+%{
+/*
+ * Copyright(c) 2019-2021 rev.ng Srls. All Rights Reserved.
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2 of the License, or (at your option) any later version.
+ *
+ * This library 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
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, see .
+ */
+
+#include "idef-parser.h"
+#include "parser-helpers.h"
+#include "idef-parser.tab.h"
+#include "idef-parser.yy.h"
+
+/* Uncomment this to disable yyasserts */
+/* #define NDEBUG */
+
+#define ERR_LINE_CONTEXT 40
+
+%}
+
+%lex-param {void *scanner}
+%parse-param {void *scanner}
+%parse-param {Context *c}
+
+%define parse.error verbose
+%define parse.lac full
+%define api.pure full
+
+%locations
+
+%union {
+GString *string;
+HexValue rvalue;
+HexSat sat;
+HexCast cast;
+HexExtract extract;
+HexMpy mpy;
+bool is_unsigned;
+int index;
+}
+
+/* Tokens */
+%start input
+
+%expect 1
+
+%token INAME DREG DIMM DPRE DEA RREG WREG FREG FIMM RPRE WPRE FPRE FWRAP FEA 
VAR
+%token POW ABS CROUND ROUND CIRCADD COUNTONES INC DEC ANDA ORA XORA PLUSPLUS 
ASL
+%token ASR LSR EQ NEQ LTE GTE MIN MAX ANDL ORL FOR ICIRC IF MUN FSCR FCHK SXT
+%token ZXT CONSTEXT LOCNT BREV SIGN LOAD STORE CONSTLL CONSTULL PC NPC LPCFG
+%token CANCEL IDENTITY PART1 BREV_4 BREV_8 ROTL INSBITS SETBITS EXTBITS 
EXTRANGE
+%token CAST4_8U SETOVF FAIL DEINTERLEAVE INTERLEAVE CARRY_FROM_ADD
+
+%token  REG IMM PRE
+%token  ELSE
+%token  MPY
+%token  SAT
+%token  CAST DEPOSIT SETHALF
+%token  EXTRACT
+%type  INAME
+%type  rvalue lvalue VAR assign_statement var
+%type  DREG DIMM DPRE RREG RPRE FAIL
+%type  if_stmt IF
+%type  SIGN
+
+/* Operator Precedences */
+%left MIN MAX
+%left '('
+%left ','
+%left '='
+%right CIRCADD
+%right INC DEC ANDA ORA XORA
+%left '?' ':'
+%left ORL
+%left ANDL
+%left '|'
+%left '^' ANDOR
+%left '&'
+%left EQ NEQ
+%left '<' '>' LTE GTE
+%left ASL ASR LSR
+%right ABS
+%left '-' '+'
+%left POW
+%left '*' '/' '%' MPY
+%right '~' '!'
+%left '['
+%right CAST
+%right LOCNT BREV
+
+/* Bison Grammar */
+%%
+
+/* Input file containing the description of each hexagon instruction */
+input : instructions
+  {
+  YYACCEPT;
+  }
+  ;
+
+instructions : instruction instructions
+ | %empty
+ ;
+
+instruction : INAME
+  {
+  gen_inst(c, $1);
+  }
+  arguments
+  {
+  gen_inst_args(c, &@1);
+  }
+  code
+  {
+  gen_inst_code(c, &@1);
+  }
+| error /* Recover gracefully after instruction compilation error 
*/
+  {
+  free_instruction(c);
+  }
+;
+
+arguments : '(' ')'
+  | '(' argument_list ')';
+
+argument_list : decl ',' argument_list
+  | decl
+  ;
+
+var : VAR
+  {
+  track_string(c, $1.var.name);
+  $$ = $1;
+  }
+;
+
+/* Return the modified registers list */
+code : '{' statements '}'
+   {
+

[PATCH v4 09/12] target/hexagon: import lexer for idef-parser

[Alessandro Di Federico via]

From: Paolo Montesel 

Signed-off-by: Alessandro Di Federico 
Signed-off-by: Paolo Montesel 
---
 target/hexagon/idef-parser/idef-parser.h  | 254 
 target/hexagon/idef-parser/idef-parser.lex| 611 ++
 target/hexagon/meson.build|   4 +
 tests/docker/dockerfiles/alpine.docker|   1 +
 tests/docker/dockerfiles/centos7.docker   |   1 +
 tests/docker/dockerfiles/centos8.docker   |   1 +
 tests/docker/dockerfiles/debian10.docker  |   1 +
 .../dockerfiles/fedora-i386-cross.docker  |   1 +
 .../dockerfiles/fedora-win32-cross.docker |   1 +
 .../dockerfiles/fedora-win64-cross.docker |   1 +
 tests/docker/dockerfiles/fedora.docker|   1 +
 tests/docker/dockerfiles/opensuse-leap.docker |   1 +
 tests/docker/dockerfiles/ubuntu.docker|   1 +
 tests/docker/dockerfiles/ubuntu1804.docker|   1 +
 tests/docker/dockerfiles/ubuntu2004.docker|   3 +-
 15 files changed, 882 insertions(+), 1 deletion(-)
 create mode 100644 target/hexagon/idef-parser/idef-parser.h
 create mode 100644 target/hexagon/idef-parser/idef-parser.lex

diff --git a/target/hexagon/idef-parser/idef-parser.h 
b/target/hexagon/idef-parser/idef-parser.h
new file mode 100644
index 00..f9aaff71f6
--- /dev/null
+++ b/target/hexagon/idef-parser/idef-parser.h
@@ -0,0 +1,254 @@
+/*
+ * Copyright(c) 2019-2021 rev.ng Srls. All Rights Reserved.
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2 of the License, or (at your option) any later version.
+ *
+ * This library 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 Lesser General Public
+ * License along with this library; if not, see .
+ */
+
+#ifndef IDEF_PARSER_H
+#define IDEF_PARSER_H
+
+#include 
+#include 
+#include 
+#include 
+
+#define TCGV_NAME_SIZE 7
+#define MAX_WRITTEN_REGS 32
+#define OFFSET_STR_LEN 32
+#define ALLOC_LIST_LEN 32
+#define ALLOC_NAME_SIZE 32
+#define INIT_LIST_LEN 32
+#define OUT_BUF_LEN (1024 * 1024)
+#define SIGNATURE_BUF_LEN (128 * 1024)
+#define HEADER_BUF_LEN (128 * 1024)
+
+/* Variadic macros to wrap the buffer printing functions */
+#define EMIT(c, ...) \
+do { \
+g_string_append_printf((c)->out_str, __VA_ARGS__);   \
+} while (0)
+
+#define EMIT_SIG(c, ...)   
\
+do {   
\
+g_string_append_printf((c)->signature_str, __VA_ARGS__);   
\
+} while (0)
+
+#define EMIT_HEAD(c, ...)  
\
+do {   
\
+g_string_append_printf((c)->header_str, __VA_ARGS__);  
\
+} while (0)
+
+/**
+ * Type of register, assigned to the HexReg.type field
+ */
+typedef enum {GENERAL_PURPOSE, CONTROL, MODIFIER, DOTNEW} RegType;
+
+/**
+ * Types of control registers, assigned to the HexReg.id field
+ */
+typedef enum {SP, FP, LR, GP, LC0, LC1, SA0, SA1} CregType;
+
+/**
+ * Identifier string of the control registers, indexed by the CregType enum
+ */
+extern const char *creg_str[];
+
+/**
+ * Semantic record of the REG tokens, identifying registers
+ */
+typedef struct HexReg {
+CregType id;/**< Identifier of the register  */
+RegType type;   /**< Type of the register*/
+unsigned bit_width; /**< Bit width of the reg, 32 or 64 bits */
+} HexReg;
+
+/**
+ * Data structure, identifying a TCGv temporary value
+ */
+typedef struct HexTmp {
+int index;  /**< Index of the TCGv temporary value*/
+} HexTmp;
+
+/**
+ * Enum of the possible immediated, an immediate is a value which is known
+ * at tinycode generation time, e.g. an integer value, not a TCGv
+ */
+enum ImmUnionTag {I, VARIABLE, VALUE, QEMU_TMP, IMM_PC, IMM_CONSTEXT};
+
+/**
+ * Semantic record of the IMM token, identifying an immediate constant
+ */
+typedef struct HexImm {
+union {
+char id;/**< Identifier of the immediate */
+uint64_t value; /**< Immediate value (for VALUE type immediates) */
+uint64_t index; /**< Index of the immediate (for int temp vars)  */
+};
+enum ImmUnionTag type;  /**< Type of the immediate  */
+} HexImm;
+
+/**
+ * Semantic record of the PRE token, identifying a predicate
+ */
+typedef 

[PATCH v4 09/12] qtest/migration-test: Skip tests if KVM not builtin on s390x/ppc64

[Philippe Mathieu-Daudé]

We might have a s390x/ppc64 QEMU binary built without the KVM
accelerator (configured with --disable-kvm).
Checking for /dev/kvm accessibility isn't enough, also check for the
accelerator in the binary.

Signed-off-by: Philippe Mathieu-Daudé 
---
Cc: David Gibson 
Cc: Greg Kurz 
Cc: Halil Pasic 
Cc: Cornelia Huck 
Cc: Christian Borntraeger 
Cc: qemu-...@nongnu.org
Cc: qemu-s3...@nongnu.org
---
 tests/qtest/migration-test.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tests/qtest/migration-test.c b/tests/qtest/migration-test.c
index 3a711bb4929..c32a2aa30a2 100644
--- a/tests/qtest/migration-test.c
+++ b/tests/qtest/migration-test.c
@@ -1408,7 +1408,7 @@ int main(int argc, char **argv)
  */
 if (g_str_equal(qtest_get_arch(), "ppc64") &&
 (access("/sys/module/kvm_hv", F_OK) ||
- access("/dev/kvm", R_OK | W_OK))) {
+ access("/dev/kvm", R_OK | W_OK) || !qtest_has_accel("kvm"))) {
 g_test_message("Skipping test: kvm_hv not available");
 return g_test_run();
 }
@@ -1419,7 +1419,7 @@ int main(int argc, char **argv)
  */
 if (g_str_equal(qtest_get_arch(), "s390x")) {
 #if defined(HOST_S390X)
-if (access("/dev/kvm", R_OK | W_OK)) {
+if (access("/dev/kvm", R_OK | W_OK) || !qtest_has_accel("kvm")) {
 g_test_message("Skipping test: kvm not available");
 return g_test_run();
 }
-- 
2.26.3

[PATCH v4 12/12] target/hexagon: import additional tests

[Alessandro Di Federico via]

From: Niccolò Izzo 

Signed-off-by: Alessandro Di Federico 
Signed-off-by: Niccolò Izzo 
---
 tests/tcg/hexagon/Makefile.target  | 36 -
 tests/tcg/hexagon/crt.S| 28 +
 tests/tcg/hexagon/test_abs.S   | 20 ++
 tests/tcg/hexagon/test_add.S   | 20 ++
 tests/tcg/hexagon/test_andp.S  | 23 +++
 tests/tcg/hexagon/test_bitcnt.S| 42 
 tests/tcg/hexagon/test_bitsplit.S  | 25 
 tests/tcg/hexagon/test_call.S  | 63 ++
 tests/tcg/hexagon/test_clobber.S   | 35 +
 tests/tcg/hexagon/test_cmp.S   | 34 
 tests/tcg/hexagon/test_cmpy.S  | 31 +++
 tests/tcg/hexagon/test_djump.S | 24 
 tests/tcg/hexagon/test_dotnew.S| 39 ++
 tests/tcg/hexagon/test_dstore.S| 29 ++
 tests/tcg/hexagon/test_ext.S   | 18 +
 tests/tcg/hexagon/test_fibonacci.S | 33 
 tests/tcg/hexagon/test_hello.S | 21 ++
 tests/tcg/hexagon/test_hl.S| 19 +
 tests/tcg/hexagon/test_hwloops.S   | 25 
 tests/tcg/hexagon/test_jmp.S   | 25 
 tests/tcg/hexagon/test_lsr.S   | 39 ++
 tests/tcg/hexagon/test_mpyi.S  | 20 ++
 tests/tcg/hexagon/test_overflow.S  | 63 ++
 tests/tcg/hexagon/test_packet.S| 26 
 tests/tcg/hexagon/test_reorder.S   | 31 +++
 tests/tcg/hexagon/test_round.S | 31 +++
 tests/tcg/hexagon/test_vavgw.S | 33 
 tests/tcg/hexagon/test_vcmpb.S | 32 +++
 tests/tcg/hexagon/test_vcmpw.S | 29 ++
 tests/tcg/hexagon/test_vcmpy.S | 50 
 tests/tcg/hexagon/test_vlsrw.S | 23 +++
 tests/tcg/hexagon/test_vmaxh.S | 37 ++
 tests/tcg/hexagon/test_vminh.S | 37 ++
 tests/tcg/hexagon/test_vpmpyh.S| 30 ++
 tests/tcg/hexagon/test_vspliceb.S  | 33 
 35 files changed, 1103 insertions(+), 1 deletion(-)
 create mode 100644 tests/tcg/hexagon/crt.S
 create mode 100644 tests/tcg/hexagon/test_abs.S
 create mode 100644 tests/tcg/hexagon/test_add.S
 create mode 100644 tests/tcg/hexagon/test_andp.S
 create mode 100644 tests/tcg/hexagon/test_bitcnt.S
 create mode 100644 tests/tcg/hexagon/test_bitsplit.S
 create mode 100644 tests/tcg/hexagon/test_call.S
 create mode 100644 tests/tcg/hexagon/test_clobber.S
 create mode 100644 tests/tcg/hexagon/test_cmp.S
 create mode 100644 tests/tcg/hexagon/test_cmpy.S
 create mode 100644 tests/tcg/hexagon/test_djump.S
 create mode 100644 tests/tcg/hexagon/test_dotnew.S
 create mode 100644 tests/tcg/hexagon/test_dstore.S
 create mode 100644 tests/tcg/hexagon/test_ext.S
 create mode 100644 tests/tcg/hexagon/test_fibonacci.S
 create mode 100644 tests/tcg/hexagon/test_hello.S
 create mode 100644 tests/tcg/hexagon/test_hl.S
 create mode 100644 tests/tcg/hexagon/test_hwloops.S
 create mode 100644 tests/tcg/hexagon/test_jmp.S
 create mode 100644 tests/tcg/hexagon/test_lsr.S
 create mode 100644 tests/tcg/hexagon/test_mpyi.S
 create mode 100644 tests/tcg/hexagon/test_overflow.S
 create mode 100644 tests/tcg/hexagon/test_packet.S
 create mode 100644 tests/tcg/hexagon/test_reorder.S
 create mode 100644 tests/tcg/hexagon/test_round.S
 create mode 100644 tests/tcg/hexagon/test_vavgw.S
 create mode 100644 tests/tcg/hexagon/test_vcmpb.S
 create mode 100644 tests/tcg/hexagon/test_vcmpw.S
 create mode 100644 tests/tcg/hexagon/test_vcmpy.S
 create mode 100644 tests/tcg/hexagon/test_vlsrw.S
 create mode 100644 tests/tcg/hexagon/test_vmaxh.S
 create mode 100644 tests/tcg/hexagon/test_vminh.S
 create mode 100644 tests/tcg/hexagon/test_vpmpyh.S
 create mode 100644 tests/tcg/hexagon/test_vspliceb.S

diff --git a/tests/tcg/hexagon/Makefile.target 
b/tests/tcg/hexagon/Makefile.target
index 616af697fe..b5323cba05 100644
--- a/tests/tcg/hexagon/Makefile.target
+++ b/tests/tcg/hexagon/Makefile.target
@@ -32,7 +32,7 @@ CFLAGS += -Wno-incompatible-pointer-types 
-Wno-undefined-internal
 HEX_SRC=$(SRC_PATH)/tests/tcg/hexagon
 VPATH += $(HEX_SRC)
 
-first: $(HEX_SRC)/first.S
+%: $(HEX_SRC)/%.S $(HEX_SRC)/crt.S
$(CC) -static -mv67 -nostdlib $^ -o $@
 
 HEX_TESTS = first
@@ -43,4 +43,38 @@ HEX_TESTS += mem_noshuf
 HEX_TESTS += atomics
 HEX_TESTS += fpstuff
 
+HEX_TESTS += test_abs
+HEX_TESTS += test_add
+HEX_TESTS += test_andp
+HEX_TESTS += test_bitcnt
+HEX_TESTS += test_bitsplit
+HEX_TESTS += test_call
+HEX_TESTS += test_clobber
+HEX_TESTS += test_cmp
+HEX_TESTS += test_cmpy
+HEX_TESTS += test_djump
+HEX_TESTS += test_dotnew
+HEX_TESTS += test_dstore
+HEX_TESTS += test_ext
+HEX_TESTS += test_fibonacci
+HEX_TESTS += test_hello
+HEX_TESTS += test_hl
+HEX_TESTS += test_hwloops
+HEX_TESTS += test_jmp
+HEX_TESTS += test_lsr
+HEX_TESTS += test_mpyi
+HEX_TESTS += test_overflow
+HEX_TESTS += test_packet

[PATCH v4 06/12] target/hexagon: introduce new helper functions

[Alessandro Di Federico via]

From: Niccolò Izzo 

These helpers will be employed by the idef-parser generated code.

Signed-off-by: Alessandro Di Federico 
Signed-off-by: Niccolò Izzo 
---
 target/hexagon/genptr.c | 188 
 target/hexagon/genptr.h |  22 +
 target/hexagon/macros.h |   9 ++
 3 files changed, 219 insertions(+)

diff --git a/target/hexagon/genptr.c b/target/hexagon/genptr.c
index 1ddb4360f1..024419bf39 100644
--- a/target/hexagon/genptr.c
+++ b/target/hexagon/genptr.c
@@ -28,6 +28,12 @@
 #include "gen_tcg.h"
 #include "genptr.h"
 
+TCGv gen_read_reg(TCGv result, int num)
+{
+tcg_gen_mov_tl(result, hex_gpr[num]);
+return result;
+}
+
 TCGv gen_read_preg(TCGv pred, uint8_t num)
 {
 tcg_gen_mov_tl(pred, hex_pred[num]);
@@ -330,5 +336,187 @@ static inline void gen_store_conditional8(CPUHexagonState 
*env,
 tcg_gen_movi_tl(hex_llsc_addr, ~0);
 }
 
+void gen_store32(TCGv vaddr, TCGv src, tcg_target_long width, unsigned slot)
+{
+tcg_gen_mov_tl(hex_store_addr[slot], vaddr);
+tcg_gen_movi_tl(hex_store_width[slot], width);
+tcg_gen_mov_tl(hex_store_val32[slot], src);
+}
+
+void gen_store1(TCGv_env cpu_env, TCGv vaddr, TCGv src, DisasContext *ctx,
+unsigned slot)
+{
+gen_store32(vaddr, src, 1, slot);
+ctx->store_width[slot] = 1;
+}
+
+void gen_store2(TCGv_env cpu_env, TCGv vaddr, TCGv src, DisasContext *ctx,
+unsigned slot)
+{
+gen_store32(vaddr, src, 2, slot);
+ctx->store_width[slot] = 2;
+}
+
+void gen_store4(TCGv_env cpu_env, TCGv vaddr, TCGv src, DisasContext *ctx,
+unsigned slot)
+{
+gen_store32(vaddr, src, 4, slot);
+ctx->store_width[slot] = 4;
+}
+
+
+void gen_store8(TCGv_env cpu_env, TCGv vaddr, TCGv_i64 src, DisasContext *ctx,
+unsigned slot)
+{
+tcg_gen_mov_tl(hex_store_addr[slot], vaddr);
+tcg_gen_movi_tl(hex_store_width[slot], 8);
+tcg_gen_mov_i64(hex_store_val64[slot], src);
+ctx->store_width[slot] = 8;
+}
+
+void gen_set_usr_field(int field, TCGv val)
+{
+tcg_gen_deposit_tl(hex_gpr[HEX_REG_USR], hex_gpr[HEX_REG_USR], val,
+   reg_field_info[field].offset,
+   reg_field_info[field].width);
+}
+
+void gen_set_usr_fieldi(int field, int x)
+{
+TCGv val = tcg_const_tl(x);
+gen_set_usr_field(field, val);
+tcg_temp_free(val);
+}
+
+void gen_write_new_pc(TCGv addr)
+{
+/* If there are multiple branches in a packet, ignore the second one */
+TCGv zero = tcg_const_tl(0);
+tcg_gen_movcond_tl(TCG_COND_NE, hex_next_PC, hex_branch_taken, zero,
+   hex_next_PC, addr);
+tcg_gen_movi_tl(hex_branch_taken, 1);
+tcg_temp_free(zero);
+}
+
+void gen_sat_i32(TCGv dest, TCGv source, int width)
+{
+TCGv max_val = tcg_const_i32((1 << (width - 1)) - 1);
+TCGv min_val = tcg_const_i32(-(1 << (width - 1)));
+tcg_gen_smin_tl(dest, source, max_val);
+tcg_gen_smax_tl(dest, dest, min_val);
+tcg_temp_free_i32(max_val);
+tcg_temp_free_i32(min_val);
+}
+
+void gen_sat_i32_ext(TCGv ovfl, TCGv dest, TCGv source, int width)
+{
+gen_sat_i32(dest, source, width);
+TCGv zero = tcg_const_i32(0);
+TCGv one = tcg_const_i32(1);
+tcg_gen_movcond_i32(TCG_COND_NE, ovfl, source, dest, one, zero);
+tcg_temp_free_i32(zero);
+tcg_temp_free_i32(one);
+}
+
+void gen_satu_i32(TCGv dest, TCGv source, int width)
+{
+TCGv max_val = tcg_const_i32((1 << width) - 1);
+tcg_gen_movcond_i32(TCG_COND_GTU, dest, source, max_val, max_val, source);
+TCGv_i32 zero = tcg_const_i32(0);
+tcg_gen_movcond_i32(TCG_COND_LT, dest, source, zero, zero, dest);
+tcg_temp_free_i32(max_val);
+tcg_temp_free_i32(zero);
+}
+
+void gen_satu_i32_ext(TCGv ovfl, TCGv dest, TCGv source, int width)
+{
+gen_satu_i32(dest, source, width);
+TCGv zero = tcg_const_i32(0);
+TCGv one = tcg_const_i32(1);
+tcg_gen_movcond_i32(TCG_COND_NE, ovfl, dest, source, one, zero);
+tcg_temp_free_i32(zero);
+tcg_temp_free_i32(one);
+}
+
+void gen_sat_i64(TCGv_i64 dest, TCGv_i64 source, int width)
+{
+TCGv_i64 max_val = tcg_const_i64((1 << (width - 1)) - 1);
+TCGv_i64 min_val = tcg_const_i64(-(1 << (width - 1)));
+tcg_gen_smin_i64(dest, source, max_val);
+tcg_gen_smax_i64(dest, dest, min_val);
+tcg_temp_free_i64(max_val);
+tcg_temp_free_i64(min_val);
+}
+
+void gen_sat_i64_ext(TCGv ovfl, TCGv_i64 dest, TCGv_i64 source, int width)
+{
+gen_sat_i64(dest, source, width);
+TCGv_i64 ovfl_64 = tcg_temp_new_i64();
+TCGv_i64 zero = tcg_const_i64(0);
+TCGv_i64 one = tcg_const_i64(1);
+tcg_gen_movcond_i64(TCG_COND_NE, ovfl_64, dest, source, one, zero);
+tcg_gen_trunc_i64_tl(ovfl, ovfl_64);
+tcg_temp_free_i64(ovfl_64);
+tcg_temp_free_i64(zero);
+tcg_temp_free_i64(one);
+}
+
+void gen_satu_i64(TCGv_i64 dest, TCGv_i64 source, int width)
+{
+TCGv_i64 max_val = tcg_const_i64((1 << width) - 1);
+

[PATCH v4 08/12] target/hexagon: prepare input for the idef-parser

[Alessandro Di Federico via]

From: Alessandro Di Federico 

Introduce infrastructure necessary to produce a file suitable for being
parsed by the idef-parser.

Signed-off-by: Alessandro Di Federico 
---
 target/hexagon/gen_idef_parser_funcs.py | 114 ++
 target/hexagon/idef-parser/macros.inc   | 150 
 target/hexagon/idef-parser/prepare  |  24 
 target/hexagon/meson.build  |  17 +++
 4 files changed, 305 insertions(+)
 create mode 100644 target/hexagon/gen_idef_parser_funcs.py
 create mode 100644 target/hexagon/idef-parser/macros.inc
 create mode 100755 target/hexagon/idef-parser/prepare

diff --git a/target/hexagon/gen_idef_parser_funcs.py 
b/target/hexagon/gen_idef_parser_funcs.py
new file mode 100644
index 00..7b8e0f6981
--- /dev/null
+++ b/target/hexagon/gen_idef_parser_funcs.py
@@ -0,0 +1,114 @@
+#!/usr/bin/env python3
+
+##
+##  Copyright(c) 2019-2021 rev.ng Srls. All Rights Reserved.
+##
+##  This program is free software; you can redistribute it and/or modify
+##  it under the terms of the GNU General Public License as published by
+##  the Free Software Foundation; either version 2 of the License, or
+##  (at your option) any later version.
+##
+##  This program is distributed in the hope that it will be useful,
+##  but WITHOUT ANY WARRANTY; without even the implied warranty of
+##  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+##  GNU General Public License for more details.
+##
+##  You should have received a copy of the GNU General Public License
+##  along with this program; if not, see .
+##
+
+import sys
+import re
+import string
+from io import StringIO
+
+import hex_common
+
+##
+## Generate code to be fed to the idef_parser
+##
+## Consider A2_add:
+##
+## Rd32=add(Rs32,Rt32), { RdV=RsV+RtV;}
+##
+## We produce:
+##
+## A2_add(RdV, in RsV, in RtV) {
+##   { RdV=RsV+RtV;}
+## }
+##
+## A2_add represents the instruction tag. Then we have a list of TCGv
+## that the code generated by the parser can expect in input. Some of
+## them are inputs ("in" prefix), while some others are outputs.
+##
+def main():
+hex_common.read_semantics_file(sys.argv[1])
+hex_common.read_attribs_file(sys.argv[2])
+hex_common.read_overrides_file(sys.argv[3])
+hex_common.calculate_attribs()
+tagregs = hex_common.get_tagregs()
+tagimms = hex_common.get_tagimms()
+
+with open(sys.argv[4], 'w') as f:
+f.write('#include "macros.inc"\n\n')
+
+for tag in hex_common.tags:
+## Skip the priv instructions
+if ( "A_PRIV" in hex_common.attribdict[tag] ) :
+continue
+## Skip the guest instructions
+if ( "A_GUEST" in hex_common.attribdict[tag] ) :
+continue
+## Skip instructions using switch
+if ( tag in {'S4_vrcrotate_acc', 'S4_vrcrotate'} ) :
+continue
+## Skip trap instructions
+if ( tag in {'J2_trap0', 'J2_trap1'} ) :
+continue
+## Skip 128-bit instructions
+if ( tag in {'A7_croundd_ri', 'A7_croundd_rr'} ) :
+continue
+## Skip other unsupported instructions
+if ( tag.startswith('S2_cabacdecbin') ) :
+continue
+if ( tag.startswith('Y') ) :
+continue
+if ( tag.startswith('V6_') ) :
+continue
+if ( tag.startswith('F') ) :
+continue
+if ( tag.endswith('_locked') ) :
+continue
+
+regs = tagregs[tag]
+imms = tagimms[tag]
+
+arguments = []
+if hex_common.need_ea(tag):
+arguments.append("EA")
+
+for regtype,regid,toss,numregs in regs:
+prefix = "in " if hex_common.is_read(regid) else ""
+
+is_pair = hex_common.is_pair(regid)
+is_single_old = (hex_common.is_single(regid)
+ and hex_common.is_old_val(regtype, regid, 
tag))
+is_single_new = (hex_common.is_single(regid)
+ and hex_common.is_new_val(regtype, regid, 
tag))
+
+if is_pair or is_single_old:
+arguments.append("%s%s%sV" % (prefix, regtype, regid))
+elif is_single_new:
+arguments.append("%s%s%sN" % (prefix, regtype, regid))
+else:
+print("Bad register parse: ",regtype,regid,toss,numregs)
+
+for immlett,bits,immshift in imms:
+arguments.append(hex_common.imm_name(immlett))
+
+f.write("%s(%s) {\n" % (tag, ", ".join(arguments)))
+f.write("%s\n" % hex_common.semdict[tag])
+f.write("}\n\n")
+
+if __name__ == "__main__":
+main()
diff --git a/target/hexagon/idef-parser/macros.inc 
b/target/hexagon/idef-parser/macros.inc

[PATCH v4 05/12] qtest/arm-cpu-features: Restrict sve_tests_sve_off_kvm test to KVM

[Philippe Mathieu-Daudé]

The sve_tests_sve_off_kvm() test is KVM specific.
Only run it if KVM is available.

Suggested-by: Andrew Jones 
Signed-off-by: Philippe Mathieu-Daudé 
---
 tests/qtest/arm-cpu-features.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tests/qtest/arm-cpu-features.c b/tests/qtest/arm-cpu-features.c
index 7f4b2521277..66300c3bc20 100644
--- a/tests/qtest/arm-cpu-features.c
+++ b/tests/qtest/arm-cpu-features.c
@@ -604,6 +604,8 @@ int main(int argc, char **argv)
 if (g_str_equal(qtest_get_arch(), "aarch64") && qtest_has_accel("kvm")) {
 qtest_add_data_func("/arm/kvm/query-cpu-model-expansion",
 NULL, test_query_cpu_model_expansion_kvm);
+qtest_add_data_func("/arm/kvm/query-cpu-model-expansion/sve-off",
+NULL, sve_tests_sve_off_kvm);
 }
 
 if (g_str_equal(qtest_get_arch(), "aarch64")) {
@@ -611,8 +613,6 @@ int main(int argc, char **argv)
 NULL, sve_tests_sve_max_vq_8);
 qtest_add_data_func("/arm/max/query-cpu-model-expansion/sve-off",
 NULL, sve_tests_sve_off);
-qtest_add_data_func("/arm/kvm/query-cpu-model-expansion/sve-off",
-NULL, sve_tests_sve_off_kvm);
 }
 
 return g_test_run();
-- 
2.26.3

[PATCH v4 05/12] target/hexagon: make helper functions non-static

[Alessandro Di Federico via]

From: Paolo Montesel 

Make certain helper functions non-static, making them available outside
genptr.c. These functions are required by code generated by the
idef-parser.

Signed-off-by: Alessandro Di Federico 
Signed-off-by: Paolo Montesel 
---
 target/hexagon/genptr.c | 7 ---
 target/hexagon/genptr.h | 6 ++
 2 files changed, 10 insertions(+), 3 deletions(-)

diff --git a/target/hexagon/genptr.c b/target/hexagon/genptr.c
index fd18aabe8d..1ddb4360f1 100644
--- a/target/hexagon/genptr.c
+++ b/target/hexagon/genptr.c
@@ -26,8 +26,9 @@
 #include "translate.h"
 #include "macros.h"
 #include "gen_tcg.h"
+#include "genptr.h"
 
-static inline TCGv gen_read_preg(TCGv pred, uint8_t num)
+TCGv gen_read_preg(TCGv pred, uint8_t num)
 {
 tcg_gen_mov_tl(pred, hex_pred[num]);
 return pred;
@@ -54,7 +55,7 @@ static inline void gen_log_predicated_reg_write(int rnum, 
TCGv val,
 tcg_temp_free(slot_mask);
 }
 
-static inline void gen_log_reg_write(int rnum, TCGv val)
+void gen_log_reg_write(int rnum, TCGv val)
 {
 tcg_gen_mov_tl(hex_new_value[rnum], val);
 #if HEX_DEBUG
@@ -118,7 +119,7 @@ static void gen_log_reg_write_pair(int rnum, TCGv_i64 val)
 #endif
 }
 
-static inline void gen_log_pred_write(int pnum, TCGv val)
+void gen_log_pred_write(int pnum, TCGv val)
 {
 TCGv zero = tcg_const_tl(0);
 TCGv base_val = tcg_temp_new();
diff --git a/target/hexagon/genptr.h b/target/hexagon/genptr.h
index c158005d2a..e1dcd24b0e 100644
--- a/target/hexagon/genptr.h
+++ b/target/hexagon/genptr.h
@@ -19,7 +19,13 @@
 #define HEXAGON_GENPTR_H
 
 #include "insn.h"
+#include "tcg/tcg.h"
+#include "translate.h"
 
 extern const SemanticInsn opcode_genptr[];
 
+TCGv gen_read_preg(TCGv pred, uint8_t num);
+void gen_log_reg_write(int rnum, TCGv val);
+void gen_log_pred_write(int pnum, TCGv val);
+
 #endif
-- 
2.31.1

[PATCH v4 00/12] target/hexagon: introduce idef-parser

[Alessandro Di Federico via]

From: Alessandro Di Federico 

This patchset introduces the idef-parser for target/hexagon.

It's the fourth iteration of the patchset and includes fixes suggested
in previous iterations.

`idef-parser` is a build-time tool built using flex and bison. Its aim
is to generate a large part of the tiny code generator frontend for
Hexagon. The prototype of idef-parser has been presented at KVM Forum
2019 ("QEMU-Hexagon: Automatic Translation of the ISA Manual Pseudcode
to Tiny Code Instructions"):

https://www.youtube.com/watch?v=3EpnTYBOXCI

`target/hexagon/idef-parser/README.rst` provides an overview of the
parser and its inner working.

A couple of notes:

* `idef-parser` also supports certain things that are not used in the
  most recently submitted version of the "Hexagon patch
  series". However, they will be needed and stripping them out of the
  parser is quite a bit of work.
* The build on the CI fails for the cross-i386-* jobs. This seems to be
  due to the fact that, despite we specified glib as a native
  dependency, meson still chooses the i386 bit version of the library,
  as opposed to the x86-64.
  Help on this matter is appreciated.
  The `clang-user` build fails too, but seems to be due reasons not
  concerning this patchset.
* Unlike the last iteration of this patchset, we no longer use local
  temps, which were detrimental for performance. Now, in order to
  handle side effects in ternary operators, the parser keeps track of
  nested ternary operators and their conditions. We then exploit that
  information to turn the side effect itself into a `movcond`. This is
  done by emitting a series of `and` instructions on the nested
  ternary operator conditions.

Alessandro Di Federico (5):
  tcg: expose TCGCond manipulation routines
  target/hexagon: update MAINTAINERS for idef-parser
  target/hexagon: import README for idef-parser
  target/hexagon: prepare input for the idef-parser
  target/hexagon: call idef-parser functions

Niccolò Izzo (2):
  target/hexagon: introduce new helper functions
  target/hexagon: import additional tests

Paolo Montesel (5):
  target/hexagon: make slot number an unsigned
  target/hexagon: make helper functions non-static
  target/hexagon: expose next PC in DisasContext
  target/hexagon: import lexer for idef-parser
  target/hexagon: import parser for idef-parser

 MAINTAINERS   |8 +
 include/tcg/tcg-cond.h|  101 +
 include/tcg/tcg.h |   70 +-
 target/hexagon/README |5 +
 target/hexagon/gen_idef_parser_funcs.py   |  114 +
 target/hexagon/gen_tcg_funcs.py   |   28 +-
 target/hexagon/genptr.c   |  201 +-
 target/hexagon/genptr.h   |   28 +
 target/hexagon/hex_common.py  |   10 +
 target/hexagon/idef-parser/README.rst |  447 
 target/hexagon/idef-parser/idef-parser.h  |  254 ++
 target/hexagon/idef-parser/idef-parser.lex|  611 +
 target/hexagon/idef-parser/idef-parser.y  |  947 +++
 target/hexagon/idef-parser/macros.inc |  150 ++
 target/hexagon/idef-parser/parser-helpers.c   | 2374 +
 target/hexagon/idef-parser/parser-helpers.h   |  347 +++
 target/hexagon/idef-parser/prepare|   24 +
 target/hexagon/macros.h   |   11 +-
 target/hexagon/meson.build|   67 +-
 target/hexagon/translate.c|3 +-
 target/hexagon/translate.h|1 +
 tests/docker/dockerfiles/alpine.docker|2 +
 tests/docker/dockerfiles/centos7.docker   |2 +
 tests/docker/dockerfiles/centos8.docker   |2 +
 tests/docker/dockerfiles/debian10.docker  |3 +
 .../dockerfiles/fedora-i386-cross.docker  |3 +
 .../dockerfiles/fedora-win32-cross.docker |3 +
 .../dockerfiles/fedora-win64-cross.docker |3 +
 tests/docker/dockerfiles/fedora.docker|2 +
 tests/docker/dockerfiles/opensuse-leap.docker |2 +
 tests/docker/dockerfiles/ubuntu.docker|3 +
 tests/docker/dockerfiles/ubuntu1804.docker|3 +
 tests/docker/dockerfiles/ubuntu2004.docker|5 +-
 tests/tcg/hexagon/Makefile.target |   36 +-
 tests/tcg/hexagon/crt.S   |   28 +
 tests/tcg/hexagon/test_abs.S  |   20 +
 tests/tcg/hexagon/test_add.S  |   20 +
 tests/tcg/hexagon/test_andp.S |   23 +
 tests/tcg/hexagon/test_bitcnt.S   |   42 +
 tests/tcg/hexagon/test_bitsplit.S |   25 +
 tests/tcg/hexagon/test_call.S |   63 +
 tests/tcg/hexagon/test_clobber.S  |   35 +
 tests/tcg/hexagon/test_cmp.S  |   34 +
 tests/tcg/hexagon/test_cmpy.S |   31 +
 tests/tcg/hexagon/test_djump.S|   24 +
 tests/tcg/hexagon/test_dotnew.S   |   39 +
 tests/tcg/hexagon/test_dstore.S 

[PATCH v4 03/12] target/hexagon: import README for idef-parser

[Alessandro Di Federico via]

From: Alessandro Di Federico 

Signed-off-by: Alessandro Di Federico 
---
 target/hexagon/README |   5 +
 target/hexagon/idef-parser/README.rst | 447 ++
 2 files changed, 452 insertions(+)
 create mode 100644 target/hexagon/idef-parser/README.rst

diff --git a/target/hexagon/README b/target/hexagon/README
index b0b2435070..2f2814380c 100644
--- a/target/hexagon/README
+++ b/target/hexagon/README
@@ -23,6 +23,10 @@ Hexagon-specific code are
 encode*.def Encoding patterns for each instruction
 iclass.def  Instruction class definitions used to determine
 legal VLIW slots for each instruction
+qemu/target/hexagon/idef-parser
+Parser that, given the high-level definitions of an instruction,
+produces a C function generating equivalent tiny code instructions.
+See README.rst.
 qemu/linux-user/hexagon
 Helpers for loading the ELF file and making Linux system calls,
 signals, etc
@@ -43,6 +47,7 @@ header files in /target/hexagon
 gen_tcg_funcs.py-> tcg_funcs_generated.c.inc
 gen_tcg_func_table.py   -> tcg_func_table_generated.c.inc
 gen_helper_funcs.py -> helper_funcs_generated.c.inc
+gen_idef_parser_funcs.py-> idef_parser_input.h
 
 Qemu helper functions have 3 parts
 DEF_HELPER declaration indicates the signature of the helper
diff --git a/target/hexagon/idef-parser/README.rst 
b/target/hexagon/idef-parser/README.rst
new file mode 100644
index 00..f4cb416e8b
--- /dev/null
+++ b/target/hexagon/idef-parser/README.rst
@@ -0,0 +1,447 @@
+Hexagon ISA instruction definitions to tinycode generator compiler
+--
+
+idef-parser is a small compiler able to translate the Hexagon ISA description
+language into tinycode generator code, that can be easily integrated into QEMU.
+
+Compilation Example
+---
+
+To better understand the scope of the idef-parser, we'll explore an applicative
+example. Let's start by one of the simplest Hexagon instruction: the ``add``.
+
+The ISA description language represents the ``add`` instruction as
+follows:
+
+.. code:: c
+
+   A2_add(RdV, in RsV, in RtV) {
+   { RdV=RsV+RtV;}
+   }
+
+idef-parser will compile the above code into the following code:
+
+.. code:: c
+
+   /* A2_add */
+   void emit_A2_add(DisasContext *ctx, Insn *insn, Packet *pkt, TCGv_i32 RdV,
+TCGv_i32 RsV, TCGv_i32 RtV)
+   /*  { RdV=RsV+RtV;} */
+   {
+   tcg_gen_movi_i32(RdV, 0);
+   TCGv_i32 tmp_0 = tcg_temp_new_i32();
+   tcg_gen_add_i32(tmp_0, RsV, RtV);
+   tcg_gen_mov_i32(RdV, tmp_0);
+   tcg_temp_free_i32(tmp_0);
+   }
+
+The output of the compilation process will be a function, containing the
+tinycode generator code, implementing the correct semantics. That function will
+not access any global variable, because all the accessed data structures will 
be
+passed explicitly as function parameters. Among the passed parameters we will
+have TCGv (tinycode variables) representing the input and output registers of
+the architecture, integers representing the immediates that come from the code,
+and other data structures which hold information about the disassemblation
+context (``DisasContext`` struct).
+
+Let's begin by describing the input code. The ``add`` instruction is associated
+with a unique identifier, in this case ``A2_add``, which allows to distinguish
+variants of the same instruction, and expresses the class to which the
+instruction belongs, in this case ``A2`` corresponds to the Hexagon
+``ALU32/ALU`` instruction subclass.
+
+After the instruction identifier, we have a series of parameters that 
represents
+TCG variables that will be passed to the generated function. Parameters marked
+with ``in`` are already initialized, while the others are output parameters.
+
+We will leverage this information to infer several information:
+
+-  Fill in the output function signature with the correct TCGv registers
+-  Fill in the output function signature with the immediate integers
+-  Keep track of which registers, among the declared one, have been
+   initialized
+
+Let's now observe the actual instruction description code, in this case:
+
+.. code:: c
+
+   { RdV=RsV+RtV;}
+
+This code is composed by a subset of the C syntax, and is the result of the
+application of some macro definitions contained in the ``macros.h`` file.
+
+This file is used to reduce the complexity of the input language where complex
+variants of similar constructs can be mapped to a unique primitive, so that the
+idef-parser has to handle a lower number of computation primitives.
+
+As you may notice, the description code modifies the registers which have been
+declared by the declaration statements. In this case all the three registers
+will be declared, ``RsV`` and ``RtV`` 

[PATCH v4 04/12] qtest/arm-cpu-features: Use generic qtest_has_accel() to check for KVM

[Philippe Mathieu-Daudé]

Use the recently added generic qtest_has_accel() method to
check if KVM is available.

Suggested-by: Claudio Fontana 
Signed-off-by: Philippe Mathieu-Daudé 
---
 tests/qtest/arm-cpu-features.c | 25 +
 1 file changed, 1 insertion(+), 24 deletions(-)

diff --git a/tests/qtest/arm-cpu-features.c b/tests/qtest/arm-cpu-features.c
index 8252b85bb85..7f4b2521277 100644
--- a/tests/qtest/arm-cpu-features.c
+++ b/tests/qtest/arm-cpu-features.c
@@ -26,21 +26,6 @@
 "  'arguments': { 'type': 'full', "
 #define QUERY_TAIL  "}}"
 
-static bool kvm_enabled(QTestState *qts)
-{
-QDict *resp, *qdict;
-bool enabled;
-
-resp = qtest_qmp(qts, "{ 'execute': 'query-kvm' }");
-g_assert(qdict_haskey(resp, "return"));
-qdict = qdict_get_qdict(resp, "return");
-g_assert(qdict_haskey(qdict, "enabled"));
-enabled = qdict_get_bool(qdict, "enabled");
-qobject_unref(resp);
-
-return enabled;
-}
-
 static QDict *do_query_no_props(QTestState *qts, const char *cpu_type)
 {
 return qtest_qmp(qts, QUERY_HEAD "'model': { 'name': %s }"
@@ -493,14 +478,6 @@ static void test_query_cpu_model_expansion_kvm(const void 
*data)
 
 qts = qtest_init(MACHINE_KVM "-cpu max");
 
-/*
- * These tests target the 'host' CPU type, so KVM must be enabled.
- */
-if (!kvm_enabled(qts)) {
-qtest_quit(qts);
-return;
-}
-
 /* Enabling and disabling kvm-no-adjvtime should always work. */
 assert_has_feature_disabled(qts, "host", "kvm-no-adjvtime");
 assert_set_feature(qts, "host", "kvm-no-adjvtime", true);
@@ -624,7 +601,7 @@ int main(int argc, char **argv)
  * order avoid attempting to run an AArch32 QEMU with KVM on
  * AArch64 hosts. That won't work and isn't easy to detect.
  */
-if (g_str_equal(qtest_get_arch(), "aarch64")) {
+if (g_str_equal(qtest_get_arch(), "aarch64") && qtest_has_accel("kvm")) {
 qtest_add_data_func("/arm/kvm/query-cpu-model-expansion",
 NULL, test_query_cpu_model_expansion_kvm);
 }
-- 
2.26.3

[PATCH v4 12/12] tests/meson: Only build softfloat objects if TCG is selected

[Philippe Mathieu-Daudé]

From: Philippe Mathieu-Daudé 

The previous attempt (commit f77147cd4de) doesn't work as
expected, as we still have CONFIG_TCG=1 when using:

  configure --disable-system --disable-user

Now than we have removed the use of CONFIG_TCG from target-dependent
files in tests/qtest/, we can remove the unconditional definition of
CONFIG_TCG in config_host.

This avoid to build a bunch of unrequired objects when building with
--disable-tcg (in particular the softfloat tests):

Before:

  $ make
  [1/812] Generating trace-qom.h with a custom command
  ...

After:

  $ make
  [1/349] Generating trace-qom.h with a custom command
  ...

A difference of 463 objects...

Reported-by: Claudio Fontana 
Suggested-by: Paolo Bonzini 
Signed-off-by: Philippe Mathieu-Daudé 
---
v3: Include Paolo's feedback:
https://www.mail-archive.com/qemu-devel@nongnu.org/msg793872.html
therefore o not include Alex's R-b tag.

Cc: Richard Henderson 
Cc: Alex Bennée 
Cc: Emilio G. Cota 
---
 meson.build | 1 -
 1 file changed, 1 deletion(-)

diff --git a/meson.build b/meson.build
index c6f4b0cf5e8..623cbe50685 100644
--- a/meson.build
+++ b/meson.build
@@ -262,7 +262,6 @@
 language: ['c', 'cpp', 'objc'])
 
   accelerators += 'CONFIG_TCG'
-  config_host += { 'CONFIG_TCG': 'y' }
 endif
 
 if 'CONFIG_KVM' not in accelerators and get_option('kvm').enabled()
-- 
2.26.3

Re: [PATCH 4/5] block: add trace point when fdatasync fails

[Dr. David Alan Gilbert]

* Daniel P. Berrangé (berra...@redhat.com) wrote:
> A flush failure is a critical failure scenario for some operations.
> For example, it will prevent migration from completing, as it will
> make vm_stop() report an error. Thus it is important to have a
> trace point present for debugging.
> 
> Signed-off-by: Daniel P. Berrangé 

I'd have had to admit to not having thought that would fail; the fact
we're debugging something where it does, suggests it's a good idea!


Reviewed-by: Dr. David Alan Gilbert 

> ---
>  block/file-posix.c | 2 ++
>  block/trace-events | 1 +
>  2 files changed, 3 insertions(+)
> 
> diff --git a/block/file-posix.c b/block/file-posix.c
> index 99cf452f84..6aafeda44f 100644
> --- a/block/file-posix.c
> +++ b/block/file-posix.c
> @@ -1362,6 +1362,8 @@ static int handle_aiocb_flush(void *opaque)
>  
>  ret = qemu_fdatasync(aiocb->aio_fildes);
>  if (ret == -1) {
> +trace_file_flush_fdatasync_failed(errno);
> +
>  /* There is no clear definition of the semantics of a failing 
> fsync(),
>   * so we may have to assume the worst. The sad truth is that this
>   * assumption is correct for Linux. Some pages are now probably 
> marked
> diff --git a/block/trace-events b/block/trace-events
> index 1a12d634e2..c8a943e992 100644
> --- a/block/trace-events
> +++ b/block/trace-events
> @@ -206,6 +206,7 @@ file_copy_file_range(void *bs, int src, int64_t src_off, 
> int dst, int64_t dst_of
>  file_FindEjectableOpticalMedia(const char *media) "Matching using %s"
>  file_setup_cdrom(const char *partition) "Using %s as optical disc"
>  file_hdev_is_sg(int type, int version) "SG device found: type=%d, version=%d"
> +file_flush_fdatasync_failed(int err) "errno %d"
>  
>  # sheepdog.c
>  sheepdog_reconnect_to_sdog(void) "Wait for connection to be established"
> -- 
> 2.30.2
> 
-- 
Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK