Re: [PATCH 0/7] python: create installable package
For patches 05-07: Reviewing such patch is a strange thing: Pipfile changes are obvious enough, just select some version (I can't be sure about correct version choice, just believe in your commit messages). But what for Pipfile.lock? I can state that it's about package set selecting, Pipfile.lock looks like what it should be, but I have idea about all these packages, their versions and hashes (and even, does it correspond really to Pipfile or not) :) Ha, what I can check, is: does pipenv create almost same Pipfile.lock in my environment (with 3.7.5 python) OK, I've tried (pipenv lock --dev --python /usr/bin/python3), and yes, result is almost the same. For mypy and importlib-metadata packages I have newer version and different hashes (of course). Other packages are the same. Set of packages is the same. Hmm, but in may generated Pipfile.lock there no "markers". They are about python version and looks like "markers": "python_version >= '3.5'".. Does pipenv follow them? Then why they are not generated for me, did you use some additional command/option to create them? Anyway, they don't look dangerous, so for last three patches: Reviewed-by: Vladimir Sementsov-Ogievskiy 03.06.2020 03:15, John Snow wrote: Based-on: 20200602214528.12107-1-js...@redhat.com This series factors the python/qemu directory as an installable module. As a developer, you can install this to your virtual environment and then always have access to the classes contained within without needing to wrangle python import path problems. When developing, you could go to qemu/python/ and invoke `pipenv shell` to activate a virtual environment within which you could type `pip install -e .` to install a special development version of this package to your virtual environment. This package will always reflect the most recent version of the source files in the tree. When not developing, you could install a version of this package to your environment outright to gain access to the QMP and QEMUMachine classes for lightweight scripting and testing. This package is formatted in such a way that it COULD be uploaded to https://pypi.org/project/qemu and installed independently of qemu.git with `pip install qemu`, but of course that button remains unpushed. There are a few major questions to answer first: - What versioning scheme should we use? See patch 2. - Should we use a namespace package or not? - Namespaced: 'qemu.machine', 'qemu.monitor' etc may be separately versioned, packaged and distributed packages. Third party authors may register 'qemu.xxx' to create plugins within the namespace as they see fit. - Non-namespaced: 'qemu' is one giant glob package, packaged and versioned in unison. We control this package exclusively. - How do we eliminate sys.path hacks from the rest of the QEMU tree? (Background: sys.path hacks generally impede the function of static code quality analysis tools like mypy and pylint.) - Simplest: parent scripts (or developer) needs to set PYTHONPATH. - Harder: Python scripts should all be written to assume package form, all tests and CI that use Python should execute within a VENV. In either case, we lose the ability (for many scripts) to "just run" a script out of the source tree if it depends on other QEMU Python files. This is annoying, but as the complexity of the Python lib grows, it is unavoidable. In the VENV case, we at least establish a simple paradigm: the scripts should work in their "installed" forms; and the rest of the build and test infrastructure should use this VENV to automatically handle dependencies and path requirements. This should allow us to move many of our existing python scripts with "hidden" dependencies into a proper python module hierarchy and test for regressions with mypy, flake8, pylint, etc. (We could even establish e.g. Sphinx versions as a dependency for our build kit here and make sure it's installed to the VENV.) Pros: Almost all scripts can be moved under python/qemu/* and checked with CQA tools. imports are written the same no matter where you are (Use the fully qualified names, e.g. qemu.core.qmp.QMPMessage). Regressions in scripts are caught *much* faster. Downsides: Kind of annoying; most scripts now require you to install a devkit forwarder (pip3 install --user .) or be inside of an activated venv. Not too bad if you know python at all, but it's certainly less plug-n-play. - What's our backwards compatibility policy if we start shipping this? Proposed: Attempt to maintain API compatibility (after stabilizing the library). Incompatible changes should probably cause a semver bump. Each published release makes no attempt to support any version of QEMU other than the one it was released against. We publish this on the tin in big red letters. TESTING THIS PACKAGE OUT: 1. You can install to your local user's environment normally by
Re: [PATCH 4/7] python/qemu: Add pipenv support
05.06.2020 20:11, John Snow wrote: On 6/5/20 12:21 PM, Vladimir Sementsov-Ogievskiy wrote: 03.06.2020 03:15, John Snow wrote: pipenv is a tool used for managing virtual environments with precisely specified dependencies. It is separate from the dependencies listed in setup.py, which are (by 'best practices') not supposed to be pinned. Note that pipenv is not required to install or use this module; this is just a convenience for in-tree developing. Here, a "blank" pipfile is added with no dependencies, but specifies Python 3.6 for the virtual environment. Pipfile will specify our version minimums, while Pipfile.lock specifies an exact loudout of packages that were known to operate correctly. This latter file provides the real value for easy setup of container images and CI environments. Signed-off-by: John Snow --- python/Pipfile | 11 +++ 1 file changed, 11 insertions(+) create mode 100644 python/Pipfile diff --git a/python/Pipfile b/python/Pipfile new file mode 100644 index 000..9534830b5eb --- /dev/null +++ b/python/Pipfile @@ -0,0 +1,11 @@ +[[source]] +name = "pypi" +url = "https://pypi.org/simple; +verify_ssl = true + +[dev-packages] + +[packages] + +[requires] +python_version = "3.6" Should it be >= or something like this? I think logistically that makes sense, but I'm not sure if the tool supports it. I decided to target the oldest version of Python we support (for non-build infrastructure) to ensure a minimum viability. And, how should I use this all? My failed attempt: [root@kvm python]# pipenv install --python /usr/bin/python3 Virtualenv already exists! Removing existing virtualenv… Creating a virtualenv for this project… Pipfile: /work/src/qemu/john-python-installable/python/Pipfile Using /usr/bin/python3 (3.7.5) to create virtualenv… ⠹ Creating virtual environment...created virtual environment CPython3.7.5.final.0-64 in 112ms creator CPython3Posix(dest=/root/.local/share/virtualenvs/python-4FwBBPCc, clear=False, global=False) seeder FromAppData(download=False, pip=latest, setuptools=latest, wheel=latest, via=copy, app_data_dir=/root/.local/share/virtualenv/seed-app-data/v1.0.1) activators BashActivator,CShellActivator,FishActivator,PowerShellActivator,PythonActivator,XonshActivator ✔ Successfully created virtual environment! Virtualenv location: /root/.local/share/virtualenvs/python-4FwBBPCc Warning: Your Pipfile requires python_version 3.6, but you are using 3.7.5 (/root/.local/share/v/p/bin/python). $ pipenv --rm and rebuilding the virtual environment may resolve the issue. $ pipenv check will surely fail. Installing dependencies from Pipfile.lock (44d7bd)… 0/0 — 00:00:00 To activate this project's virtualenv, run pipenv shell. Alternatively, run a command inside the virtualenv with pipenv run. [root@kvm python]# pipenv shell Warning: Your Pipfile requires python_version 3.6, but you are using 3.7.5 (/root/.local/share/v/p/bin/python). $ pipenv --rm and rebuilding the virtual environment may resolve the issue. $ pipenv check will surely fail. Launching subshell in virtual environment… . /root/.local/share/virtualenvs/python-4FwBBPCc/bin/activate [root@kvm work]# . /root/.local/share/virtualenvs/python-4FwBBPCc/bin/activate (python) [root@kvm work]# python Python 3.7.5 (default, Oct 17 2019, 12:09:47) [GCC 9.2.1 20190827 (Red Hat 9.2.1-1)] on linux Type "help", "copyright", "credits" or "license" for more information. import pylint Traceback (most recent call last): File "", line 1, in ModuleNotFoundError: No module named 'pylint' and iotest 297 says: "pylint-3 not found" Ah! that's a bug in iotest 297. It's expecting the fedora package there. I'll have to fix that. (honestly, I'm new to using python virtual environment) Good questions. I'll document this in the README.rst for this folder, actually... 1. Create a virtual environment pipenv sync --dev jsnow@probe ~/s/q/python (python-package-refactor)> pipenv sync --dev Creating a virtualenv for this project… Pipfile: /home/jsnow/src/qemu/python/Pipfile Using /usr/bin/python3.6 (3.6.10) to create virtualenv… ⠏ Creating virtual environment...Using base prefix '/usr' New python executable in /home/jsnow/.local/share/virtualenvs/python-QepCANQl/bin/python3.6 Also creating executable in /home/jsnow/.local/share/virtualenvs/python-QepCANQl/bin/python Installing setuptools, pip, wheel...done. Running virtualenv with interpreter /usr/bin/python3.6 ✔ Successfully created virtual environment! Virtualenv location: /home/jsnow/.local/share/virtualenvs/python-QepCANQl Installing dependencies from Pipfile.lock (44d7bd)… 17/17 — 00:00:07 To activate this project's virtualenv, run pipenv shell. Alternatively, run a command inside the virtualenv with pipenv run. All dependencies are now up-to-date! This command can both create and synchronize the venv's packages with those listed in
Re: [PATCH 1/2] Introduce (x86) CPU model deprecation API
On Fri, 2020-06-05 at 08:47 -0500, Eric Blake wrote: > On 6/4/20 9:47 PM, Robert Hoo wrote: > > On Thu, 2020-06-04 at 06:59 -0500, Eric Blake wrote: > > > On 6/4/20 3:07 AM, Robert Hoo wrote: > > > > > > > > > +++ b/qapi/machine-target.json > > > > > > @@ -309,7 +309,8 @@ > > > > > > 'static': 'bool', > > > > > > '*unavailable-features': [ 'str' ], > > > > > > 'typename': 'str', > > > > > > -'*alias-of' : 'str' }, > > > > > > +'*alias-of' : 'str', > > > > > > +'deprecated' : 'bool' }, > > > > > > > > > > Missing documentation of the new member. Should it be > > > > > optional > > > > > (present > > > > > only when true)? > > > > > > > > Which document do you mean? > > > > Thanks Eric:) > > > > > > > > A few lines earlier is '@alias-of: ...'; you'll need to add a > > > similar > > > line for '@deprecated', mentioning it is '(since 5.1)'. > > > > > > > How to make it optional? > > > > How about not making it optional? refer to Machineinfo::deprecated. > > Always providing it doesn't hurt. If there is precedence for not > making > it optional, mentioning that precedence in the commit message can't > hurt. No specific precedence. Just feel a little weird that adding an additional boolean, just for judging another boolean should present or not. esp. given that Machineinfo::deprecated is not optional. >
[PATCH v5 3/3] tests/acpi: update expected SRAT files
Update expected SRAT files for the change to account for NVDIMM NUMA nodes in the SRAT. AML diffs: tests/data/acpi/pc/SRAT.dimmpxm: --- /tmp/asl-3P2IL0.dsl 2020-05-28 15:11:02.326439263 -0600 +++ /tmp/asl-1N4IL0.dsl 2020-05-28 15:11:02.325439280 -0600 @@ -3,7 +3,7 @@ * AML/ASL+ Disassembler version 20190509 (64-bit version) * Copyright (c) 2000 - 2019 Intel Corporation * - * Disassembly of tests/data/acpi/pc/SRAT.dimmpxm, Thu May 28 15:11:02 2020 + * Disassembly of /tmp/aml-4D4IL0, Thu May 28 15:11:02 2020 * * ACPI Data Table [SRAT] * @@ -13,7 +13,7 @@ [000h 4]Signature : "SRAT"[System Resource Affinity Table] [004h 0004 4] Table Length : 0188 [008h 0008 1] Revision : 01 -[009h 0009 1] Checksum : 80 +[009h 0009 1] Checksum : 68 [00Ah 0010 6] Oem ID : "BOCHS " [010h 0016 8] Oem Table ID : "BXPCSRAT" [018h 0024 4] Oem Revision : 0001 @@ -140,15 +140,15 @@ [138h 0312 1]Subtable Type : 01 [Memory Affinity] [139h 0313 1] Length : 28 -[13Ah 0314 4] Proximity Domain : +[13Ah 0314 4] Proximity Domain : 0002 [13Eh 0318 2]Reserved1 : -[140h 0320 8] Base Address : -[148h 0328 8] Address Length : +[140h 0320 8] Base Address : 00010800 +[148h 0328 8] Address Length : 0800 [150h 0336 4]Reserved2 : -[154h 0340 4]Flags (decoded below) : - Enabled : 0 +[154h 0340 4]Flags (decoded below) : 0005 + Enabled : 1 Hot Pluggable : 0 -Non-Volatile : 0 +Non-Volatile : 1 [158h 0344 8]Reserved3 : [160h 0352 1]Subtable Type : 01 [Memory Affinity] tests/data/acpi/q35/SRAT.dimmpxm: --- /tmp/asl-HW2LL0.dsl 2020-05-28 15:11:05.446384514 -0600 +++ /tmp/asl-8MYLL0.dsl 2020-05-28 15:11:05.445384532 -0600 @@ -3,7 +3,7 @@ * AML/ASL+ Disassembler version 20190509 (64-bit version) * Copyright (c) 2000 - 2019 Intel Corporation * - * Disassembly of tests/data/acpi/q35/SRAT.dimmpxm, Thu May 28 15:11:05 2020 + * Disassembly of /tmp/aml-2CYLL0, Thu May 28 15:11:05 2020 * * ACPI Data Table [SRAT] * @@ -13,7 +13,7 @@ [000h 4]Signature : "SRAT"[System Resource Affinity Table] [004h 0004 4] Table Length : 0188 [008h 0008 1] Revision : 01 -[009h 0009 1] Checksum : 80 +[009h 0009 1] Checksum : 68 [00Ah 0010 6] Oem ID : "BOCHS " [010h 0016 8] Oem Table ID : "BXPCSRAT" [018h 0024 4] Oem Revision : 0001 @@ -140,15 +140,15 @@ [138h 0312 1]Subtable Type : 01 [Memory Affinity] [139h 0313 1] Length : 28 -[13Ah 0314 4] Proximity Domain : +[13Ah 0314 4] Proximity Domain : 0002 [13Eh 0318 2]Reserved1 : -[140h 0320 8] Base Address : -[148h 0328 8] Address Length : +[140h 0320 8] Base Address : 00010800 +[148h 0328 8] Address Length : 0800 [150h 0336 4]Reserved2 : -[154h 0340 4]Flags (decoded below) : - Enabled : 0 +[154h 0340 4]Flags (decoded below) : 0005 + Enabled : 1 Hot Pluggable : 0 -Non-Volatile : 0 +Non-Volatile : 1 [158h 0344 8]Reserved3 : [160h 0352 1]Subtable Type : 01 [Memory Affinity] tests/data/acpi/virt/SRAT.memhp: --- /tmp/asl-E32WL0.dsl 2020-05-28 15:19:56.976095582 -0600 +++ /tmp/asl-Y69WL0.dsl 2020-05-28 15:19:56.974095617 -0600 @@ -3,7 +3,7 @@ * AML/ASL+ Disassembler version 20190509 (64-bit version) * Copyright (c) 2000 - 2019 Intel Corporation * - * Disassembly of tests/data/acpi/virt/SRAT.memhp, Thu May 28 15:19:56 2020 + * Disassembly of /tmp/aml-2CCXL0, Thu May 28 15:19:56 2020 * * ACPI Data Table [SRAT] * @@ -11,9 +11,9 @@ */ [000h 4]Signature : "SRAT"[System Resource Affinity Table] -[004h 0004 4] Table Length : 00BA +[004h 0004 4] Table Length : 00E2 [008h 0008 1] Revision : 03 -[009h 0009
[PATCH v5 0/3] account for NVDIMM nodes during SRAT generation
Changes since v4 (no code changes): - Change the commit message in patch 2 to use memdev= instead of mem= which is deprecated (Igor) - Add Igor's Reviewed-by Changes since v3: - Add the SRAT augmentation for ARM's virt-acpi-build as well (Igor) - Update patches 1 and 3 for the test binaries to include ARM tests. Changes since v2: - Change a repetitive OBJECT(dev) to a stored 'Object' (Igor) - No need to return 'numamem' back to build_srat (Igor) Changes since v1: - Use error_abort for getters (Igor) - Free the device list (Igor) - Refactor the NVDIMM related portion into hw/acpi/nvdimm.c (Igor) - Rebase onto latest master - Add Jingqi's Reviewed-by On the command line, one can specify a NUMA node for NVDIMM devices. If we set up the topology to give NVDIMMs their own nodes, i.e. not containing any CPUs or regular memory, qemu doesn't populate SRAT memory affinity structures for these nodes. However the NFIT does reference those proximity domains. As a result, Linux, while parsing the SRAT, fails to initialize node related structures for these nodes, and they never end up in the nodes_possible map. When these are onlined at a later point (via hotplug), this causes problems. I've followed the instructions in bios-tables-test.c to update the expected SRAT binary, and the tests (make check) pass. Patches 1 and 3 are the relevant ones for the binary update. Patch 2 is the main patch which changes SRAT generation. Vishal Verma (3): diffs-allowed: add the SRAT AML to diffs-allowed hw/acpi/nvdimm: add a helper to augment SRAT generation tests/acpi: update expected SRAT files hw/acpi/nvdimm.c | 23 +++ hw/arm/virt-acpi-build.c | 4 hw/i386/acpi-build.c | 5 + include/hw/mem/nvdimm.h | 1 + tests/data/acpi/pc/SRAT.dimmpxm | Bin 392 -> 392 bytes tests/data/acpi/q35/SRAT.dimmpxm | Bin 392 -> 392 bytes tests/data/acpi/virt/SRAT.memhp | Bin 186 -> 226 bytes 7 files changed, 33 insertions(+) -- 2.26.2
[PATCH v5 2/3] hw/acpi/nvdimm: add a helper to augment SRAT generation
NVDIMMs can belong to their own proximity domains, as described by the
NFIT. In such cases, the SRAT needs to have Memory Affinity structures
in the SRAT for these NVDIMMs, otherwise Linux doesn't populate node
data structures properly during NUMA initialization. See the following
for an example failure case.
https://lore.kernel.org/linux-nvdimm/20200416225438.15208-1-vishal.l.ve...@intel.com/
Introduce a new helper, nvdimm_build_srat(), and call it for both the
i386 and arm versions of 'build_srat()' to augment the SRAT with
memory affinity information for NVDIMMs.
The relevant command line options to exercise this are below. Nodes 0-1
contain CPUs and regular memory, and nodes 2-3 are the NVDIMM address
space.
-object memory-backend-ram,id=mem0,size=2048M
-numa node,nodeid=0,memdev=mem0,
-numa cpu,node-id=0,socket-id=0
-object memory-backend-ram,id=mem1,size=2048M
-numa node,nodeid=1,memdev=mem1,
-numa cpu,node-id=1,socket-id=1
-numa node,nodeid=2,
-object
memory-backend-file,id=nvmem0,share,mem-path=nvdimm-0,size=16384M,align=1G
-device nvdimm,memdev=nvmem0,id=nv0,label-size=2M,node=2
-numa node,nodeid=3,
-object
memory-backend-file,id=nvmem1,share,mem-path=nvdimm-1,size=16384M,align=1G
-device nvdimm,memdev=nvmem1,id=nv1,label-size=2M,node=3
Cc: Jingqi Liu
Cc: Michael S. Tsirkin
Reviewed-by: Jingqi Liu
Reviewed-by: Igor Mammedov
Signed-off-by: Vishal Verma
---
hw/acpi/nvdimm.c | 23 +++
hw/arm/virt-acpi-build.c | 4
hw/i386/acpi-build.c | 5 +
include/hw/mem/nvdimm.h | 1 +
4 files changed, 33 insertions(+)
diff --git a/hw/acpi/nvdimm.c b/hw/acpi/nvdimm.c
index 9316d12b70..8f7cc16add 100644
--- a/hw/acpi/nvdimm.c
+++ b/hw/acpi/nvdimm.c
@@ -28,6 +28,7 @@
#include "qemu/osdep.h"
#include "qemu/uuid.h"
+#include "qapi/error.h"
#include "hw/acpi/acpi.h"
#include "hw/acpi/aml-build.h"
#include "hw/acpi/bios-linker-loader.h"
@@ -1334,6 +1335,28 @@ static void nvdimm_build_ssdt(GArray *table_offsets,
GArray *table_data,
free_aml_allocator();
}
+void nvdimm_build_srat(GArray *table_data)
+{
+GSList *device_list = nvdimm_get_device_list();
+
+for (; device_list; device_list = device_list->next) {
+AcpiSratMemoryAffinity *numamem = NULL;
+DeviceState *dev = device_list->data;
+Object *obj = OBJECT(dev);
+uint64_t addr, size;
+int node;
+
+node = object_property_get_int(obj, PC_DIMM_NODE_PROP, _abort);
+addr = object_property_get_uint(obj, PC_DIMM_ADDR_PROP, _abort);
+size = object_property_get_uint(obj, PC_DIMM_SIZE_PROP, _abort);
+
+numamem = acpi_data_push(table_data, sizeof *numamem);
+build_srat_memory(numamem, addr, size, node,
+ MEM_AFFINITY_ENABLED | MEM_AFFINITY_NON_VOLATILE);
+}
+g_slist_free(device_list);
+}
+
void nvdimm_build_acpi(GArray *table_offsets, GArray *table_data,
BIOSLinker *linker, NVDIMMState *state,
uint32_t ram_slots)
diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
index 1b0a584c7b..2cbccd5fe2 100644
--- a/hw/arm/virt-acpi-build.c
+++ b/hw/arm/virt-acpi-build.c
@@ -539,6 +539,10 @@ build_srat(GArray *table_data, BIOSLinker *linker,
VirtMachineState *vms)
}
}
+if (ms->nvdimms_state->is_enabled) {
+nvdimm_build_srat(table_data);
+}
+
if (ms->device_memory) {
numamem = acpi_data_push(table_data, sizeof *numamem);
build_srat_memory(numamem, ms->device_memory->base,
diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
index 2e15f6848e..d996525e2c 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -2428,6 +2428,11 @@ build_srat(GArray *table_data, BIOSLinker *linker,
MachineState *machine)
MEM_AFFINITY_ENABLED);
}
}
+
+if (machine->nvdimms_state->is_enabled) {
+nvdimm_build_srat(table_data);
+}
+
slots = (table_data->len - numa_start) / sizeof *numamem;
for (; slots < pcms->numa_nodes + 2; slots++) {
numamem = acpi_data_push(table_data, sizeof *numamem);
diff --git a/include/hw/mem/nvdimm.h b/include/hw/mem/nvdimm.h
index a3c08955e8..b67a1aedf6 100644
--- a/include/hw/mem/nvdimm.h
+++ b/include/hw/mem/nvdimm.h
@@ -155,6 +155,7 @@ typedef struct NVDIMMState NVDIMMState;
void nvdimm_init_acpi_state(NVDIMMState *state, MemoryRegion *io,
struct AcpiGenericAddress dsm_io,
FWCfgState *fw_cfg, Object *owner);
+void nvdimm_build_srat(GArray *table_data);
void nvdimm_build_acpi(GArray *table_offsets, GArray *table_data,
BIOSLinker *linker, NVDIMMState *state,
uint32_t ram_slots);
--
2.26.2
[PATCH v5 1/3] diffs-allowed: add the SRAT AML to diffs-allowed
In anticipation of a change to the SRAT generation in qemu, add the AML file to diffs-allowed. Signed-off-by: Vishal Verma --- tests/qtest/bios-tables-test-allowed-diff.h | 3 +++ 1 file changed, 3 insertions(+) diff --git a/tests/qtest/bios-tables-test-allowed-diff.h b/tests/qtest/bios-tables-test-allowed-diff.h index dfb8523c8b..e8f2766a63 100644 --- a/tests/qtest/bios-tables-test-allowed-diff.h +++ b/tests/qtest/bios-tables-test-allowed-diff.h @@ -1 +1,4 @@ /* List of comma-separated changed AML files to ignore */ +"tests/data/acpi/pc/SRAT.dimmpxm", +"tests/data/acpi/q35/SRAT.dimmpxm", +"tests/data/acpi/virt/SRAT.memhp", -- 2.26.2
Re: [PATCH v4 2/3] hw/acpi/nvdimm: add a helper to augment SRAT generation
On Fri, 2020-06-05 at 10:23 +0200, Igor Mammedov wrote: > > > > The relevant command line options to exercise this are below. Nodes 0-1 > > > > contain CPUs and regular memory, and nodes 2-3 are the NVDIMM address > > > > space. > > > > > > > > -numa node,nodeid=0,mem=2048M, > > > > -numa node,nodeid=1,mem=2048M, > > > > > > pls note that 'mem' is about to be disabled for new machine types in > > > favor of memdev > > > so this CLI won't work. > > > It would be nice to update commit message with memdev variant of CLI > > > > I saw the warnings printed - I did try to use memdevs, but it didn't > > quite work with my use case. I'm supplying mem=0 for the pmem/nvdimm > > devices that I want to give a specific numa node, but not give them any > > more regular memory aside from the nvdimm itself (see nodes 4 and 5 > > below). And for some reason I couldn't do that with memdevs. > it should work since 4.1 > > here is example > qemu-system-x86_64 -object memory-backend-ram,id=mem0,size=1G -m 1G \ > -numa node,memdev=mem0 -numa node -monitor stdio > > QEMU 5.0.50 monitor - type 'help' for more information > (qemu) VNC server running on ::1:5900 > info numa > 2 nodes > node 0 cpus: 0 > node 0 size: 1024 MB > node 0 plugged: 0 MB > node 1 cpus: > node 1 size: 0 MB > node 1 plugged: 0 MB > (qemu) > Perfect got it working, Thanks Igor! I'll send a v5 with the updated commit message and add your reviewed-by. -Vishal
Re: [PATCH v2 1/1] virtio-ccw: auto-manage VIRTIO_F_IOMMU_PLATFORM if PV
On Wed, 20 May 2020 12:23:24 -0400 "Michael S. Tsirkin" wrote: > On Fri, May 15, 2020 at 12:11:55AM +0200, Halil Pasic wrote: > > The virtio specification tells that the device is to present > > VIRTIO_F_ACCESS_PLATFORM (a.k.a. VIRTIO_F_IOMMU_PLATFORM) when the > > device "can only access certain memory addresses with said access > > specified and/or granted by the platform". This is the case for a > > protected VMs, as the device can access only memory addresses that are > > in pages that are currently shared (only the guest can share/unsare its > > pages). > > > > No VM, however, starts out as a protected VM, but some VMs may be > > converted to protected VMs if the guest decides so. > > > > Making the end user explicitly manage the VIRTIO_F_ACCESS_PLATFORM via > > the property iommu_on is a minor disaster. Since the correctness of the > > paravirtualized virtio devices depends (and thus in a sense the > > correctness of the hypervisor) it, then the hypervisor should have the > > last word about whether VIRTIO_F_ACCESS_PLATFORM is to be presented or > > not. > > So, how about this: switch iommu to on/off/auto. Add a property with a > reasonable name "allow protected"? If set allow switch to protected > memory and also set iommu auto to on by default. If not set then don't. > > This will come handy for other things like migrating to hosts without > protected memory support. > > > Also, virtio now calls this PLATFORM_ACCESS, maybe we should rename > the property (keeping old one around for compat)? > I feel this will address lots of complaints ... > I'm not sure I've entirely understood your proposal, but I tried to do something in that direction. Short summary of the changes: * added new property "access_platform" as on/off/auto (default auto) * added alias "iommu_platform" for compatibility * rewrote this patch to on/off/auto so that we only do the override when user specified auto Let me list some pros and cons (compared to the previous patch): PRO: * Thanks to on/off/auto we don't override what the user specified. From user interface perspective preferable. I usually hate software that thinks its than me and can do the opposite I tell it. CON: * It is more code: "4 files changed, 37 insertions(+), 2 deletions(-)" against "3 files changed, 17 insertions(+)" * Unlike the previous one, this one is not fool-proof! The user can still specify access_platform=off to lets say a hotplug device, and bring down the guest. We could however fence such stuff with an error message. Would be even more code though. * As far as I can tell 'auto' was used to pick a value on initialization time. This is a novel, and possibly dodgy use in a sense that the value of the property may change during the lifetime of the VM. * We may need to do something about libvirt. Further improvements are possible and probably necessary if we want to go down this path. But I would like to verify that first. 8<- From: Halil Pasic Date: Wed, 26 Feb 2020 16:48:21 +0100 Subject: [PATCH v2.5 1/1] virtio-ccw: auto-manage VIRTIO_F_IOMMU_PLATFORM if PV The virtio specification tells that the device is to present VIRTIO_F_ACCESS_PLATFORM (a.k.a. VIRTIO_F_IOMMU_PLATFORM) when the device "can only access certain memory addresses with said access specified and/or granted by the platform". This is the case for a protected VMs, as the device can access only memory addresses that are in pages that are currently shared (only the guest can share/unsare its pages). No VM, however, starts out as a protected VM, but some VMs may be converted to protected VMs if the guest decides so. Making the end user explicitly manage the VIRTIO_F_ACCESS_PLATFORM via the property iommu_on is a minor disaster. Since the correctness of the paravirtualized virtio devices depends (and thus in a sense the correctness of the hypervisor) it, then the hypervisor should have the last word about whether VIRTIO_F_ACCESS_PLATFORM is to be presented or not. Currently presenting a PV guest with a (paravirtualized) virtio-ccw device has catastrophic consequences for the VM (after the hypervisors access to protected memory). This is especially grave in case of device hotplug (because in this case the guest is more likely to be in the middle of something important). Let us add the ability to manage the VIRTIO_F_ACCESS_PLATFORM virtio feature automatically. This is accomplished by turning the property into an 'on/off/auto' property, and for virtio-ccw devices if auto was specified forcing its value before we start the protected VM. If the VM should cease to be protected, the original value is restored. Signed-off-by: Halil Pasic --- hw/s390x/s390-virtio-ccw.c | 2 ++ hw/s390x/virtio-ccw.c | 14 ++ hw/virtio/virtio.c | 19 +++ include/hw/virtio/virtio.h | 4 ++-- 4 files changed, 37 insertions(+), 2 deletions(-) diff --git
Re: [PATCH v6 0/7] dwc-hsotg (aka dwc2) USB host controller emulation
On Fri, 5 Jun 2020 at 23:14, Paul Zimmerman wrote: > > > > On Fri, Jun 5, 2020 at 7:28 AM Peter Maydell wrote: >> >> On Thu, 21 May 2020 at 00:54, Paul Zimmerman wrote: >> > >> > This verion fixes a few things pointed out by Peter, and one by >> > Felippe. >> > >> > This patch series adds emulation for the dwc-hsotg USB controller, >> > which is used on the Raspberry Pi 3 and earlier, as well as a number >> > of other development boards. The main benefit for Raspberry Pi is that >> > this enables networking on these boards, since the network adapter is >> > attached via USB. >> >> Applied to target-arm.next, thanks. >> >> (Gerd, I'm assuming from the fact you put a previous version >> of this series into a usb pullreq that you're happy with the >> "usb: add short-packet handling to usb-storage driver" patch >> even though you didn't provide a reviewed-by for it.) >> > > Thanks Peter! Does this require an update to Maintainers and/or the release > notes? I'll add the oneliner to the release notes. I had assumed that you'd have put in a patch in the series updating MAINTAINERS if it was needed -- send a followup patch if you want. thanks -- PMM
Re: [PULL 00/10] s390x update
On Fri, 5 Jun 2020 at 16:38, Cornelia Huck wrote: > > The following changes since commit b489f015fbe2bd59d409211f79ea0a8ac5d2a66d: > > Merge remote-tracking branch 'remotes/stefanha/tags/block-pull-request' > into staging (2020-06-05 11:53:37 +0100) > > are available in the Git repository at: > > https://github.com/cohuck/qemu tags/s390x-20200605 > > for you to fetch changes up to c44d26a2347177f9bcd558a7c429396b373bb68e: > > target/s390x: Restrict system-mode declarations (2020-06-05 17:13:11 +0200) > > > s390x update: > - enhance s390x documentation > - allow ORBs without prefetch specified for vfio-ccw > - various cleanups and enhancements > > Applied, thanks. Please update the changelog at https://wiki.qemu.org/ChangeLog/5.1 for any user-visible changes. -- PMM
Re: [PATCH v6 0/7] dwc-hsotg (aka dwc2) USB host controller emulation
On Fri, Jun 5, 2020 at 7:28 AM Peter Maydell wrote: > On Thu, 21 May 2020 at 00:54, Paul Zimmerman wrote: > > > > This verion fixes a few things pointed out by Peter, and one by > > Felippe. > > > > This patch series adds emulation for the dwc-hsotg USB controller, > > which is used on the Raspberry Pi 3 and earlier, as well as a number > > of other development boards. The main benefit for Raspberry Pi is that > > this enables networking on these boards, since the network adapter is > > attached via USB. > > Applied to target-arm.next, thanks. > > (Gerd, I'm assuming from the fact you put a previous version > of this series into a usb pullreq that you're happy with the > "usb: add short-packet handling to usb-storage driver" patch > even though you didn't provide a reviewed-by for it.) > > Thanks Peter! Does this require an update to Maintainers and/or the release notes? Thanks, Paul
Re: [PATCH v2] fuzz: add oss-fuzz build.sh script
Patchew URL: https://patchew.org/QEMU/20200605175028.5626-1-alx...@bu.edu/ Hi, This series seems to have some coding style problems. See output below for more information: Message-id: 20200605175028.5626-1-alx...@bu.edu Subject: [PATCH v2] fuzz: add oss-fuzz build.sh script Type: series === TEST SCRIPT BEGIN === #!/bin/bash git rev-parse base > /dev/null || exit 0 git config --local diff.renamelimit 0 git config --local diff.renames True git config --local diff.algorithm histogram ./scripts/checkpatch.pl --mailback base.. === TEST SCRIPT END === Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384 Switched to a new branch 'test' c9ada67 fuzz: add oss-fuzz build.sh script === OUTPUT BEGIN === WARNING: added, moved or deleted file(s), does MAINTAINERS need updating? #16: new file mode 100755 ERROR: trailing whitespace #49: FILE: scripts/oss-fuzz/build.sh:29: +for i in $(ldd ./i386-softmmu/qemu-fuzz-i386 | cut -f3 -d' '); do $ total: 1 errors, 1 warnings, 50 lines checked Commit c9ada6738325 (fuzz: add oss-fuzz build.sh script) has style problems, please review. If any of these errors are false positives report them to the maintainer, see CHECKPATCH in MAINTAINERS. === OUTPUT END === Test command exited with code: 1 The full log is available at http://patchew.org/logs/20200605175028.5626-1-alx...@bu.edu/testing.checkpatch/?type=message. --- Email generated automatically by Patchew [https://patchew.org/]. Please send your feedback to patchew-de...@redhat.com
Re: [PATCH v7 06/11] Makefile: Remove dangerous EOL trailing backslash
On 6/5/20 10:58 AM, Philippe Mathieu-Daudé wrote: > One might get caught trying to understand unexpected Makefile > behavior. Trailing backslash can help to split very long lines, > but are rather dangerous when nothing follow. Preserve other > developers debugging time by removing this one. > > Reviewed-by: Thomas Huth > Reviewed-by: Alistair Francis > Signed-off-by: Philippe Mathieu-Daudé > --- > Makefile | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) Reviewed-by: Richard Henderson r~
Re: [PATCH v7 05/11] rules.mak: Add base-arch() rule
On 6/5/20 10:58 AM, Philippe Mathieu-Daudé wrote: > Add a rule to return the base architecture for a QEMU target. > > The current list of TARGET_BASE_ARCH is: > > $ git grep TARGET_BASE_ARCH configure > configure:7785:TARGET_BASE_ARCH="" > configure:7795:TARGET_BASE_ARCH=i386 > configure:7813:TARGET_BASE_ARCH=arm > configure:7846:TARGET_BASE_ARCH=mips > configure:7854:TARGET_BASE_ARCH=mips > configure:7864:TARGET_BASE_ARCH=openrisc > configure:7871:TARGET_BASE_ARCH=ppc > configure:7879:TARGET_BASE_ARCH=ppc > configure:7887:TARGET_BASE_ARCH=ppc > configure:7894:TARGET_BASE_ARCH=riscv > configure:7900:TARGET_BASE_ARCH=riscv > configure:7920:TARGET_BASE_ARCH=sparc > configure:7925:TARGET_BASE_ARCH=sparc > > The rule can be tested calling 'print-base-arch-$TARGET': > > $ make \ > print-base-arch-openrisc \ > print-base-arch-aarch64_be \ > print-base-arch-x86_64 \ > print-base-arch-mips64el \ > print-base-arch-ppc64 \ > print-base-arch-riscv64 > openrisc=openrisc > aarch64_be=arm > x86_64=i386 > mips64el=mips > ppc64=ppc > riscv64=riscv > > Signed-off-by: Philippe Mathieu-Daudé > --- > rules.mak | 35 +++ > 1 file changed, 35 insertions(+) Reviewed-by: Richard Henderson r~
Re: [PATCH RFC v2 0/5] block: add block-dirty-bitmap-populate job
On 5/13/20 10:49 PM, John Snow wrote: Hi, This is a new (very small) block job that writes a pattern into a bitmap. The only pattern implemented is the top allocation information. This can be used to "recover" an incremental bitmap chain if an external snapshot was taken without creating a new bitmap first: any writes made to the image will be reflected by the allocation status and can be written back into a bitmap. This is useful for e.g. libvirt managing backup chains if a user creates an external snapshot outside of libvirt. v2: - Addressed some, but not all feedback - Rebased on latest 'job-runner' series; but it's not clear if it should be kept. Message-id for that series? I'm not finding a message with a subject containing a literal 'job-runner', but am not sure which subject to look for instead. I also couldn't find an obvious tag or branch at https://github.com/jnsnow/qemu/branches where you might have stashed this including prerequisites. - This version doesn't address all of the feedback from v1, but I am posting it to the list as an RFC. I'm happy to try and take over these patches to prepare a v3, but only if I can get them to build by finding the prerequisites :) -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3226 Virtualization: qemu.org | libvirt.org
Re: [PATCH v7 04/11] rules.mak: Add strequal() and startswith() rules
On 6/5/20 10:58 AM, Philippe Mathieu-Daudé wrote: > Add a rule to test if two strings are equal, > and another to test if a string starts with a substring. > > Signed-off-by: Philippe Mathieu-Daudé > --- > rules.mak | 14 ++ > 1 file changed, 14 insertions(+) Reviewed-by: Richard Henderson r~
Re: [PULL 00/19] Linux user for 5.1 patches
On 6/5/20 1:32 PM, Peter Maydell wrote: > On Fri, 5 Jun 2020 at 20:20, Laurent Vivier wrote: >> I was thinking this kind of problem would be detected by the travis-ci >> builds, but in fact ppc64 and s390 builds don't build other architecture >> linux-user targets. > > That's an unfortunate gap in the CI -- we should ideally cover > the whole tree with at least one big-endian platform. Indeed. Hopefully we can do this with our pending gitlab setup. For travis, IIRC we only build this restricted set because we had problems with timeouts on those machines. r~
Re: [PATCH v2 00/13] Add Thread Sanitizer support to QEMU
Patchew URL:
https://patchew.org/QEMU/20200605173422.1490-1-robert.fo...@linaro.org/
Hi,
This series failed the asan build test. Please find the testing commands and
their output below. If you have Docker installed, you can probably reproduce it
locally.
=== TEST SCRIPT BEGIN ===
#!/bin/bash
export ARCH=x86_64
make docker-image-fedora V=1 NETWORK=1
time make docker-test-debug@fedora TARGET_LIST=x86_64-softmmu J=14 NETWORK=1
=== TEST SCRIPT END ===
PASS 1 fdc-test /x86_64/fdc/cmos
PASS 2 fdc-test /x86_64/fdc/no_media_on_start
PASS 3 fdc-test /x86_64/fdc/read_without_media
==9160==WARNING: ASan doesn't fully support makecontext/swapcontext functions
and may produce false positives in some cases!
==9160==ERROR: finishing a fiber switch that has not started
PASS 8 test-string-input-visitor /string-visitor/input/fuzz
Broken pipe
/tmp/qemu-test/src/tests/qtest/libqtest.c:166: kill_qemu() tried to terminate
QEMU process but encountered exit status 1 (expected 0)
ERROR - too few tests run (expected 13, got 3)
MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))}
tests/test-string-output-visitor -m=quick -k --tap < /dev/null |
./scripts/tap-driver.pl --test-name="test-string-output-visitor"
make: *** [/tmp/qemu-test/src/tests/Makefile.include:642: check-qtest-x86_64]
Error 1
make: *** Waiting for unfinished jobs
PASS 1 test-string-output-visitor /string-visitor/output/int
PASS 2 test-string-output-visitor /string-visitor/output/int-human
---
PASS 32 test-opts-visitor /visitor/opts/range/beyond
PASS 33 test-opts-visitor /visitor/opts/dict/unvisited
MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))}
tests/test-coroutine -m=quick -k --tap < /dev/null | ./scripts/tap-driver.pl
--test-name="test-coroutine"
==9190==WARNING: ASan doesn't fully support makecontext/swapcontext functions
and may produce false positives in some cases!
==9190==ERROR: finishing a fiber switch that has not started
ERROR - too few tests run (expected 10, got 0)
make: *** [/tmp/qemu-test/src/tests/Makefile.include:647: check-unit] Error 1
Traceback (most recent call last):
File "./tests/docker/docker.py", line 665, in
sys.exit(main())
---
raise CalledProcessError(retcode, cmd)
subprocess.CalledProcessError: Command '['sudo', '-n', 'docker', 'run',
'--label', 'com.qemu.instance.uuid=41c29776cd2c40bf8d43e98f3001772d', '-u',
'1003', '--security-opt', 'seccomp=unconfined', '--rm', '-e',
'TARGET_LIST=x86_64-softmmu', '-e', 'EXTRA_CONFIGURE_OPTS=', '-e', 'V=', '-e',
'J=14', '-e', 'DEBUG=', '-e', 'SHOW_ENV=', '-e', 'CCACHE_DIR=/var/tmp/ccache',
'-v', '/home/patchew2/.cache/qemu-docker-ccache:/var/tmp/ccache:z', '-v',
'/var/tmp/patchew-tester-tmp-s0ok4i_z/src/docker-src.2020-06-05-17.15.46.2022:/var/tmp/qemu:z,ro',
'qemu:fedora', '/var/tmp/qemu/run', 'test-debug']' returned non-zero exit
status 2.
filter=--filter=label=com.qemu.instance.uuid=41c29776cd2c40bf8d43e98f3001772d
make[1]: *** [docker-run] Error 1
make[1]: Leaving directory `/var/tmp/patchew-tester-tmp-s0ok4i_z/src'
make: *** [docker-run-test-debug@fedora] Error 2
real28m54.118s
user0m8.736s
The full log is available at
http://patchew.org/logs/20200605173422.1490-1-robert.fo...@linaro.org/testing.asan/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-de...@redhat.com
Re: [PATCH v4 00/12] iotests: Dump QCOW2 dirty bitmaps metadata
On 6/4/20 12:41 PM, Vladimir Sementsov-Ogievskiy wrote: Hi all! Here is my suggestion to substitute only first three patches :) of Andrey's [PATCH v3 0/6] iotests: Dump QCOW2 dirty bitmaps metadata so, I called it v4 for convenience. What is here: 1. First, update code style 2. Next, try to refactor in a manner which will make adding new data structures simple (look at Qcow2BitmapExt class in last patch) I think, next step is to add type hints. Then add more structures. And, anyway, at some point we should move it into python/ directory (at least qcow2_format.py lib) My python reviewing skills are weak, but in general this series makes sense. I had enough comments that you're probably better off spinning a v5, but it looks like it is probably close enough that I can include this in my next bitmaps pull request, and we can work on rebasing the remainder of Andrey's patches on top of it. -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3226 Virtualization: qemu.org | libvirt.org
Re: [PATCH] fuzz: add oss-fuzz build.sh script
Patchew URL: https://patchew.org/QEMU/20200605174036.4527-1-alx...@bu.edu/ Hi, This series seems to have some coding style problems. See output below for more information: Message-id: 20200605174036.4527-1-alx...@bu.edu Subject: [PATCH] fuzz: add oss-fuzz build.sh script Type: series === TEST SCRIPT BEGIN === #!/bin/bash git rev-parse base > /dev/null || exit 0 git config --local diff.renamelimit 0 git config --local diff.renames True git config --local diff.algorithm histogram ./scripts/checkpatch.pl --mailback base.. === TEST SCRIPT END === From https://github.com/patchew-project/qemu * [new tag] patchew/20200605174036.4527-1-alx...@bu.edu -> patchew/20200605174036.4527-1-alx...@bu.edu Switched to a new branch 'test' 8109a86 fuzz: add oss-fuzz build.sh script === OUTPUT BEGIN === WARNING: added, moved or deleted file(s), does MAINTAINERS need updating? #16: new file mode 100755 ERROR: trailing whitespace #48: FILE: scripts/oss-fuzz/build.sh:28: +for i in $(ldd ./i386-softmmu/qemu-fuzz-i386 | cut -f3 -d' '); do $ total: 1 errors, 1 warnings, 47 lines checked Commit 8109a8627d68 (fuzz: add oss-fuzz build.sh script) has style problems, please review. If any of these errors are false positives report them to the maintainer, see CHECKPATCH in MAINTAINERS. === OUTPUT END === Test command exited with code: 1 The full log is available at http://patchew.org/logs/20200605174036.4527-1-alx...@bu.edu/testing.checkpatch/?type=message. --- Email generated automatically by Patchew [https://patchew.org/]. Please send your feedback to patchew-de...@redhat.com
Re: [PULL 00/19] Linux user for 5.1 patches
On Fri, 5 Jun 2020 at 20:20, Laurent Vivier wrote: > I was thinking this kind of problem would be detected by the travis-ci > builds, but in fact ppc64 and s390 builds don't build other architecture > linux-user targets. That's an unfortunate gap in the CI -- we should ideally cover the whole tree with at least one big-endian platform. thanks -- PMM
Re: [PATCH v2 00/24] Fixes around device realization
Yes please, my next pull request is already at 120 patches... Paolo Il ven 5 giu 2020, 17:01 Markus Armbruster ha scritto: > Needs a rebase now. > > Paolo, would you like me to post the pull requests for my recent QOM > work myself? > >
Re: another tst-arm-mte bug: qemu-system segfaults
On 6/3/20 10:17 AM, Szabolcs Nagy wrote: > The 06/03/2020 09:21, Richard Henderson wrote: >> On 6/3/20 6:50 AM, Szabolcs Nagy wrote: >>> thanks my tests now get further but later i run into >>> the previous assert failure: >>> >>> target/arm/mte_helper.c:97:allocation_tag_mem: assertion failed: (tag_size >>> <= in_page) >>> >>> i might be able to reduce it to a small reproducer >>> this time. i assume that will help. >> >> Dang, I had hoped that the one fix would cover both -- it's definitely in the >> same area. Yes, a small reproducer will help, but I will also try again with >> your larger reproducer. > > reproducer .c and static exe attached. > > the referenced __memcmp_aarch64 is again > from the arm optimized-routines repo. That assert is just wrong -- it's attempting to sanity check a virtual address against a property associated with the physical address, and even doing that incorrectly. I've pushed a fixup to the branch to remove it, and I'll look into adding a correct assertion later. r~
Re: [PATCH v4 06/12] qcow2_format.py: use strings to specify c-type of struct fields
On 6/4/20 12:41 PM, Vladimir Sementsov-Ogievskiy wrote: We are going to move field-parsing to super-class, this will be simpler with simple string specifiers instead of variables. For some reason python doesn't allow to define ctypes in class too, as well as fields: it's not available than in 'for' operator. Don't worry: ctypes will be moved to metaclass soon. Grammar suggestion: For some reason, python doesn't allow the definition of ctypes in the class alongside fields: it would not be available then for use by the 'for' operator. Signed-off-by: Vladimir Sementsov-Ogievskiy --- tests/qemu-iotests/qcow2_format.py | 50 +- 1 file changed, 28 insertions(+), 22 deletions(-) -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3226 Virtualization: qemu.org | libvirt.org
Re: [PATCH v4 04/12] qcow2_format.py: use tuples instead of lists for fields
On 6/4/20 12:41 PM, Vladimir Sementsov-Ogievskiy wrote: No need in lists: it's a constant variable. Signed-off-by: Vladimir Sementsov-Ogievskiy --- tests/qemu-iotests/qcow2_format.py | 40 +++--- 1 file changed, 20 insertions(+), 20 deletions(-) # Version 3 header fields -[uint64_t, 'mask', 'incompatible_features'], -[uint64_t, 'mask', 'compatible_features'], -[uint64_t, 'mask', 'autoclear_features'], -[uint32_t, '%d', 'refcount_order'], -[uint32_t, '%d', 'header_length'], -] +(uint64_t, 'mask', 'incompatible_features'), +(uint64_t, 'mask', 'compatible_features'), +(uint64_t, 'mask', 'autoclear_features'), +(uint32_t, '%d', 'refcount_order'), +(uint32_t, '%d', 'header_length'), +) Not for this patch, but noticing it since we're here: should this be taught to list the optional compression_type field that we recently added after header_length? -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3226 Virtualization: qemu.org | libvirt.org
Re: [PATCH v4 02/12] qcow2.py: move qcow2 format classes to separate module
On 6/4/20 12:41 PM, Vladimir Sementsov-Ogievskiy wrote: We are going to enhance qcow2 format parsing by adding more structure classes. Let's split format parsing from utility code. Signed-off-by: Vladimir Sementsov-Ogievskiy --- tests/qemu-iotests/qcow2.py| 161 + tests/qemu-iotests/qcow2_format.py | 157 2 files changed, 161 insertions(+), 157 deletions(-) create mode 100644 tests/qemu-iotests/qcow2_format.py diff --git a/tests/qemu-iotests/qcow2.py b/tests/qemu-iotests/qcow2.py index 539f5a186b..d9c41668fd 100755 --- a/tests/qemu-iotests/qcow2.py +++ b/tests/qemu-iotests/qcow2.py @@ -1,163 +1,10 @@ #!/usr/bin/env python3 - import sys Pre-existing: no copyright blurb on the old file... +++ b/tests/qemu-iotests/qcow2_format.py @@ -0,0 +1,157 @@ +import struct +import string + It would be nice to fix that, and have one on the new file as well. -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3226 Virtualization: qemu.org | libvirt.org
Re: [PATCH 14/16] arm/aspeed: Drop aspeed_board_init_flashes() parameter @errp
On 6/5/20 4:56 PM, Markus Armbruster wrote:
> We always pass _abort. Drop the parameter, use _abort
> directly.
>
> Cc: Cédric Le Goater
> Cc: Peter Maydell
> Cc: Andrew Jeffery
> Cc: Joel Stanley
> Cc: qemu-...@nongnu.org
> Signed-off-by: Markus Armbruster
Reviewed-by: Cédric Le Goater
Thanks,
C.
> ---
> hw/arm/aspeed.c | 12 ++--
> 1 file changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/hw/arm/aspeed.c b/hw/arm/aspeed.c
> index 5ffaf86b86..4ce6ca0ef5 100644
> --- a/hw/arm/aspeed.c
> +++ b/hw/arm/aspeed.c
> @@ -215,8 +215,8 @@ static void write_boot_rom(DriveInfo *dinfo, hwaddr addr,
> size_t rom_size,
> g_free(storage);
> }
>
> -static void aspeed_board_init_flashes(AspeedSMCState *s, const char
> *flashtype,
> - Error **errp)
> +static void aspeed_board_init_flashes(AspeedSMCState *s,
> + const char *flashtype)
> {
> int i ;
>
> @@ -227,8 +227,8 @@ static void aspeed_board_init_flashes(AspeedSMCState *s,
> const char *flashtype,
>
> fl->flash = qdev_new(flashtype);
> if (dinfo) {
> -qdev_prop_set_drive_err(fl->flash, "drive",
> -blk_by_legacy_dinfo(dinfo), errp);
> +qdev_prop_set_drive(fl->flash, "drive",
> +blk_by_legacy_dinfo(dinfo));
> }
> qdev_realize_and_unref(fl->flash, BUS(s->spi), _fatal);
>
> @@ -314,8 +314,8 @@ static void aspeed_machine_init(MachineState *machine)
>"max_ram", max_ram_size - ram_size);
> memory_region_add_subregion(>ram_container, ram_size,
> >max_ram);
>
> -aspeed_board_init_flashes(>soc.fmc, amc->fmc_model, _abort);
> -aspeed_board_init_flashes(>soc.spi[0], amc->spi_model,
> _abort);
> +aspeed_board_init_flashes(>soc.fmc, amc->fmc_model);
> +aspeed_board_init_flashes(>soc.spi[0], amc->spi_model);
>
> /* Install first FMC flash content as a boot rom. */
> if (drive0) {
>
Re: [PULL 00/29] target-arm queue
Patchew URL: https://patchew.org/QEMU/20200605165007.12095-1-peter.mayd...@linaro.org/ Hi, This series seems to have some coding style problems. See output below for more information: Message-id: 20200605165007.12095-1-peter.mayd...@linaro.org Subject: [PULL 00/29] target-arm queue Type: series === TEST SCRIPT BEGIN === #!/bin/bash git rev-parse base > /dev/null || exit 0 git config --local diff.renamelimit 0 git config --local diff.renames True git config --local diff.algorithm histogram ./scripts/checkpatch.pl --mailback base.. === TEST SCRIPT END === Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384 From https://github.com/patchew-project/qemu 5d2f557..175198a master -> master Switched to a new branch 'test' 8d19bb1 target/arm: Convert Neon one-register-and-immediate insns to decodetree c07ebcb target/arm: Convert VCVT fixed-point ops to decodetree 595e77e target/arm: Convert Neon VSHLL, VMOVL to decodetree 0f42979 target/arm: Convert Neon narrowing shifts with op==9 to decodetree 93549e8 target/arm: Convert Neon narrowing shifts with op==8 to decodetree f0efe71 target/arm: Convert VQSHLU, VQSHL 2-reg-shift insns to decodetree c0457fa target/arm: Convert Neon VSRA, VSRI, VRSHR, VRSRA 2-reg-shift insns to decodetree 7e946c5 target/arm: Convert Neon VSHR 2-reg-shift insns to decodetree 3daf164 target/arm: Convert Neon VSHL and VSLI 2-reg-shift insn to decodetree 725f9e4 raspi2 acceptance test: add test for dwc-hsotg (dwc2) USB host 2a56c0c wire in the dwc-hsotg (dwc2) USB host controller emulation f9720e8 usb: add short-packet handling to usb-storage driver d6df0bc dwc-hsotg (dwc2) USB host controller emulation 26ec413 dwc-hsotg (dwc2) USB host controller state definitions 27c3b07 dwc-hsotg (dwc2) USB host controller register definitions 3f982e5 raspi: add BCM2835 SOC MPHI emulation 21a59c9 docs/system: Document Aspeed boards 65b3e83 tests/acceptance: Add a boot test for the xlnx-versal-virt machine a494cb0 hw/adc/stm32f2xx_adc: Correct memory region size and access size abbdeb5 target/arm: Split helper_crypto_sm3tt 0db4fd4 target/arm: Split helper_crypto_sha1_3reg 00ad42c target/arm: Convert sha1 and sha256 to gvec helpers 96fc084 target/arm: Convert sha512 and sm3 to gvec helpers 5199605 target/arm: Convert rax1 to gvec helpers 7cd9082 target/arm: Convert aes and sm4 to gvec helpers 188335a hw/arm/pxa2xx: Replace printf() call by qemu_log_mask() 240dce6 hw/input/pxa2xx_keypad: Replace hw_error() by qemu_log_mask() b09557e hw/ssi/imx_spi: Removed unnecessary cast of rx data received from slave f979463 hw/ssi/imx_spi: changed while statement to prevent underflow === OUTPUT BEGIN === 1/29 Checking commit f979463e1fdd (hw/ssi/imx_spi: changed while statement to prevent underflow) 2/29 Checking commit b09557efe185 (hw/ssi/imx_spi: Removed unnecessary cast of rx data received from slave) 3/29 Checking commit 240dce66be50 (hw/input/pxa2xx_keypad: Replace hw_error() by qemu_log_mask()) 4/29 Checking commit 188335aa83c3 (hw/arm/pxa2xx: Replace printf() call by qemu_log_mask()) 5/29 Checking commit 7cd9082434bd (target/arm: Convert aes and sm4 to gvec helpers) WARNING: added, moved or deleted file(s), does MAINTAINERS need updating? #396: new file mode 100644 total: 0 errors, 1 warnings, 364 lines checked Patch 5/29 has style problems, please review. If any of these errors are false positives report them to the maintainer, see CHECKPATCH in MAINTAINERS. 6/29 Checking commit 519960531b00 (target/arm: Convert rax1 to gvec helpers) 7/29 Checking commit 96fc08401629 (target/arm: Convert sha512 and sm3 to gvec helpers) 8/29 Checking commit 00ad42c753c1 (target/arm: Convert sha1 and sha256 to gvec helpers) ERROR: spaces required around that '*' (ctx:WxV) #270: FILE: target/arm/translate-neon.inc.c:735: +static bool trans_##NAME##_3s(DisasContext *s, arg_3same *a)\ ^ total: 1 errors, 0 warnings, 366 lines checked Patch 8/29 has style problems, please review. If any of these errors are false positives report them to the maintainer, see CHECKPATCH in MAINTAINERS. 9/29 Checking commit 0db4fd4a19b7 (target/arm: Split helper_crypto_sha1_3reg) ERROR: spaces required around that '*' (ctx:WxV) #246: FILE: target/arm/translate-neon.inc.c:698: +static bool trans_##NAME##_3s(DisasContext *s, arg_3same *a)\ ^ total: 1 errors, 0 warnings, 243 lines checked Patch 9/29 has style problems, please review. If any of these errors are false positives report them to the maintainer, see CHECKPATCH in MAINTAINERS. 10/29 Checking commit abbdeb5eb100 (target/arm: Split helper_crypto_sm3tt) 11/29 Checking commit a494cb00d665 (hw/adc/stm32f2xx_adc: Correct memory region size and access size) 12/29 Checking commit 65b3e832526b (tests/acceptance: Add a boot test for the xlnx-versal-virt machine) 13/29 Checking commit 21a59c942494 (docs/system: Document Aspeed
Re: [RFC v2 00/18] Refactor configuration of guest memory protection
Paolo Bonzini writes: > On 05/06/20 01:30, Thiago Jung Bauermann wrote: >> Paolo Bonzini writes: >>> On 04/06/20 23:54, Thiago Jung Bauermann wrote: QEMU could always create a PEF object, and if the command line defines one, it will correspond to it. And if the command line doesn't define one, then it would also work because the PEF object is already there. >>> >>> How would you start a non-protected VM? >>> Currently it's the "-machine" >>> property that decides that, and the argument requires an id >>> corresponding to "-object". >> >> If there's only one object, there's no need to specify its id. > > This answers my question. However, the property is defined for all > machines (it's in the "machine" class), so if it takes the id for one > machine it does so for all of them. I don't understand much about QEMU internals, so perhaps it's not practical to implement but from an end-user perspective I think this logic can apply to all architectures (since my understanding is that all of them use only one object): make the id optional. If it's not specified, then there must be only one object, and the property will implicitly refer to it. Then, if an architecture doesn't need to specify parameters at object creation time, it can be implicitly created and the user doesn't have to worry about this detail. -- Thiago Jung Bauermann IBM Linux Technology Center
Re: [PATCH v4 01/12] qcow2.py: python style fixes
On 6/4/20 12:41 PM, Vladimir Sementsov-Ogievskiy wrote: Fix flake8 complains. Leave the only chunk of lines over 79 characters: complaints initialization of cmds variable. Leave it for another day, when it should be refactored to utilize argparse instead of hand-written parsing. Signed-off-by: Vladimir Sementsov-Ogievskiy --- tests/qemu-iotests/qcow2.py | 92 + 1 file changed, 53 insertions(+), 39 deletions(-) cmds = [ -[ 'dump-header', cmd_dump_header, 0, 'Dump image header and header extensions' ], -[ 'dump-header-exts', cmd_dump_header_exts, 0, 'Dump image header extensions' ], -[ 'set-header', cmd_set_header, 2, 'Set a field in the header'], -[ 'add-header-ext', cmd_add_header_ext, 2, 'Add a header extension' ], -[ 'add-header-ext-stdio', cmd_add_header_ext_stdio, 1, 'Add a header extension, data from stdin' ], -[ 'del-header-ext', cmd_del_header_ext, 1, 'Delete a header extension' ], -[ 'set-feature-bit', cmd_set_feature_bit, 2, 'Set a feature bit'], +['dump-header', cmd_dump_header, 0, 'Dump image header and header extensions'], I know you mentioned argparse as a later refactoring, but is it worth reflowing the table in the meantime? ['dump-header', cmd_dump_header, 0, 'Dump image header and header extensions'], [... '...'], -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3226 Virtualization: qemu.org | libvirt.org
Re: [PATCH 2/7] python/qemu: formalize as package
On 6/5/20 10:40 AM, Vladimir Sementsov-Ogievskiy wrote: > > Hmm, documentation says: > > Warning Using setup_requires is discouraged in favor of PEP-518 > > did you consider this thing? I guess the difference here is we start using a pyproject.toml and then we declare setuptools, wheel and setuptools_scm dependencies there. In turn, pip will no longer install those projects to the user's environment, but will create its own virtual environment for the purposes of the build/installation process. (It looks like pip re-downloads and re-installs these files every time you run the install? That seems ... bad?) It looks like this support was added in pip 10. Not all the platforms we support at the moment appear to have pip 10 in their distribution repository, though technically ... you could use pip 9 to install a more modern pip to your userspace. I think maybe I'd prefer to just leave this alone for now. We can update it if there's some compelling reason to upgrade, but it's still a pretty new feature that doesn't seem to be in widespread usage yet. setuptools is widely regarded to be "the default", and I believe pip will assume and support this for a while longer yet. No hurry to upgrade, I think, but I could be wrong. Python packaging is convoluted. --js
Re: [PULL 00/19] Linux user for 5.1 patches
Le 05/06/2020 à 18:45, Peter Maydell a écrit :
> On Fri, 5 Jun 2020 at 12:48, Laurent Vivier wrote:
>>
>> The following changes since commit ddc760832fa8cf5e93b9d9e6e854a5114ac63510:
>>
>> Merge remote-tracking branch 'remotes/gkurz/tags/9p-next-2020-05-26' into
>> s=
>> taging (2020-05-26 14:05:53 +0100)
>>
>> are available in the Git repository at:
>>
>> git://github.com/vivier/qemu.git tags/linux-user-for-5.1-pull-request
>>
>> for you to fetch changes up to aa3d2045d4ca760bd8c22935a2d73ee4f3480bd5:
>>
>> stubs: Restrict ui/win32-kbd-hook to system-mode (2020-06-05 11:36:00
>> +0200)
>>
>>
>> linux-user pull request 20200605
>>
>> Implement F_OFD_ fcntl() command, /proc/cpuinfo for hppa
>> Fix socket(), prnctl() error codes, underflow in target_mremap,
>> epoll_create() strace, oldumount for alpha
>> User-mode build dependencies improvement
>>
>>
>
> Hi; this failed to compile on s390 and ppc when building hppa-linux-user:
>
> /home/ubuntu/qemu/linux-user/syscall.c: In function ‘do_openat’:
> /home/ubuntu/qemu/linux-user/syscall.c:7484:42: error: ‘is_proc’
> undeclared (first use in this function); did you mean ‘
> is_error’?
> { "/proc/cpuinfo", open_cpuinfo, is_proc },
> ^~~
> is_error
> /home/ubuntu/qemu/linux-user/syscall.c:7484:42: note: each undeclared
> identifier is reported only once for each function
> it appears in
> /home/ubuntu/qemu/rules.mak:69: recipe for target 'linux-user/syscall.o'
> failed
>
> Looks like this is because the #if condition guarding the
> is_proc() definition doesn't line up with the uses (missing
> a check on TARGET_HPPA).
You're right.
I was thinking this kind of problem would be detected by the travis-ci
builds, but in fact ppc64 and s390 builds don't build other architecture
linux-user targets.
I update my PR.
Thanks,
Laurent
[PATCH 7/7] target/i386: reimplement fprem using floatx80 operations
The x87 fprem emulation is currently based around conversion to
double, which is inherently unsuitable for a good emulation of any
floatx80 operation. Reimplement using the soft-float floatx80
remainder operations.
Signed-off-by: Joseph Myers
---
target/i386/fpu_helper.c | 58 +---
1 file changed, 1 insertion(+), 57 deletions(-)
diff --git a/target/i386/fpu_helper.c b/target/i386/fpu_helper.c
index bab35e00a0..d2fc2c1dde 100644
--- a/target/i386/fpu_helper.c
+++ b/target/i386/fpu_helper.c
@@ -989,63 +989,7 @@ void helper_fprem1(CPUX86State *env)
void helper_fprem(CPUX86State *env)
{
-double st0, st1, dblq, fpsrcop, fptemp;
-CPU_LDoubleU fpsrcop1, fptemp1;
-int expdif;
-signed long long int q;
-
-st0 = floatx80_to_double(env, ST0);
-st1 = floatx80_to_double(env, ST1);
-
-if (isinf(st0) || isnan(st0) || isnan(st1) || (st1 == 0.0)) {
-ST0 = double_to_floatx80(env, 0.0 / 0.0); /* NaN */
-env->fpus &= ~0x4700; /* (C3,C2,C1,C0) <-- */
-return;
-}
-
-fpsrcop = st0;
-fptemp = st1;
-fpsrcop1.d = ST0;
-fptemp1.d = ST1;
-expdif = EXPD(fpsrcop1) - EXPD(fptemp1);
-
-if (expdif < 0) {
-/* optimisation? taken from the AMD docs */
-env->fpus &= ~0x4700; /* (C3,C2,C1,C0) <-- */
-/* ST0 is unchanged */
-return;
-}
-
-if (expdif < 53) {
-dblq = fpsrcop / fptemp; /* ST0 / ST1 */
-/* round dblq towards zero */
-dblq = (dblq < 0.0) ? ceil(dblq) : floor(dblq);
-st0 = fpsrcop - fptemp * dblq; /* fpsrcop is ST0 */
-
-/* convert dblq to q by truncating towards zero */
-if (dblq < 0.0) {
-q = (signed long long int)(-dblq);
-} else {
-q = (signed long long int)dblq;
-}
-
-env->fpus &= ~0x4700; /* (C3,C2,C1,C0) <-- */
-/* (C0,C3,C1) <-- (q2,q1,q0) */
-env->fpus |= (q & 0x4) << (8 - 2); /* (C0) <-- q2 */
-env->fpus |= (q & 0x2) << (14 - 1); /* (C3) <-- q1 */
-env->fpus |= (q & 0x1) << (9 - 0); /* (C1) <-- q0 */
-} else {
-int N = 32 + (expdif % 32); /* as per AMD docs */
-
-env->fpus |= 0x400; /* C2 <-- 1 */
-fptemp = pow(2.0, (double)(expdif - N));
-fpsrcop = (st0 / st1) / fptemp;
-/* fpsrcop = integer obtained by chopping */
-fpsrcop = (fpsrcop < 0.0) ?
- -(floor(fabs(fpsrcop))) : floor(fpsrcop);
-st0 -= (st1 * fpsrcop * fptemp);
-}
-ST0 = double_to_floatx80(env, st0);
+helper_fprem_common(env, true);
}
void helper_fyl2xp1(CPUX86State *env)
--
2.17.1
--
Joseph S. Myers
jos...@codesourcery.com
[PATCH 5/7] softfloat: return low bits of quotient from floatx80_modrem
Both x87 and m68k need the low parts of the quotient for their
remainder operations. Arrange for floatx80_modrem to track those bits
and return them via a pointer.
The architectures using float32_rem and float64_rem do not appear to
need this information, so the *_rem interface is left unchanged and
the information returned only from floatx80_modrem. The logic used to
determine the low 7 bits of the quotient for m68k
(target/m68k/fpu_helper.c:make_quotient) appears completely bogus (it
looks at the result of converting the remainder to integer, the
quotient having been discarded by that point); this patch does not
change that, but the m68k maintainers may wish to do so.
Signed-off-by: Joseph Myers
---
fpu/softfloat.c | 23 ++-
include/fpu/softfloat.h | 3 ++-
2 files changed, 20 insertions(+), 6 deletions(-)
diff --git a/fpu/softfloat.c b/fpu/softfloat.c
index 423a815196..c3c3f382af 100644
--- a/fpu/softfloat.c
+++ b/fpu/softfloat.c
@@ -5684,10 +5684,11 @@ floatx80 floatx80_div(floatx80 a, floatx80 b,
float_status *status)
| `a' with respect to the corresponding value `b'. The operation is performed
| according to the IEC/IEEE Standard for Binary Floating-Point Arithmetic,
| if 'mod' is false; if 'mod' is true, return the remainder based on truncating
-| the quotient toward zero instead.
+| the quotient toward zero instead. '*quotient' is set to the low 64 bits of
+| the absolute value of the integer quotient.
**/
-floatx80 floatx80_modrem(floatx80 a, floatx80 b, bool mod,
+floatx80 floatx80_modrem(floatx80 a, floatx80 b, bool mod, uint64_t *quotient,
float_status *status)
{
bool aSign, zSign;
@@ -5695,6 +5696,7 @@ floatx80 floatx80_modrem(floatx80 a, floatx80 b, bool mod,
uint64_t aSig0, aSig1, bSig;
uint64_t q, term0, term1, alternateASig0, alternateASig1;
+*quotient = 0;
if (floatx80_invalid_encoding(a) || floatx80_invalid_encoding(b)) {
float_raise(float_flag_invalid, status);
return floatx80_default_nan(status);
@@ -5749,7 +5751,7 @@ floatx80 floatx80_modrem(floatx80 a, floatx80 b, bool mod,
shift128Right( aSig0, 0, 1, , );
expDiff = 0;
}
-q = ( bSig <= aSig0 );
+*quotient = q = ( bSig <= aSig0 );
if ( q ) aSig0 -= bSig;
expDiff -= 64;
while ( 0 < expDiff ) {
@@ -5759,6 +5761,8 @@ floatx80 floatx80_modrem(floatx80 a, floatx80 b, bool mod,
sub128( aSig0, aSig1, term0, term1, , );
shortShift128Left( aSig0, aSig1, 62, , );
expDiff -= 62;
+*quotient <<= 62;
+*quotient += q;
}
expDiff += 64;
if ( 0 < expDiff ) {
@@ -5772,6 +5776,12 @@ floatx80 floatx80_modrem(floatx80 a, floatx80 b, bool
mod,
++q;
sub128( aSig0, aSig1, term0, term1, , );
}
+if (expDiff < 64) {
+*quotient <<= expDiff;
+} else {
+*quotient = 0;
+}
+*quotient += q;
}
else {
term1 = 0;
@@ -5786,6 +5796,7 @@ floatx80 floatx80_modrem(floatx80 a, floatx80 b, bool mod,
aSig0 = alternateASig0;
aSig1 = alternateASig1;
zSign = ! zSign;
+++*quotient;
}
}
return
@@ -5802,7 +5813,8 @@ floatx80 floatx80_modrem(floatx80 a, floatx80 b, bool mod,
floatx80 floatx80_rem(floatx80 a, floatx80 b, float_status *status)
{
-return floatx80_modrem(a, b, false, status);
+uint64_t quotient;
+return floatx80_modrem(a, b, false, , status);
}
/*
@@ -5813,7 +5825,8 @@ floatx80 floatx80_rem(floatx80 a, floatx80 b,
float_status *status)
floatx80 floatx80_mod(floatx80 a, floatx80 b, float_status *status)
{
-return floatx80_modrem(a, b, true, status);
+uint64_t quotient;
+return floatx80_modrem(a, b, true, , status);
}
/*
diff --git a/include/fpu/softfloat.h b/include/fpu/softfloat.h
index bff6934d09..ff4e2605b1 100644
--- a/include/fpu/softfloat.h
+++ b/include/fpu/softfloat.h
@@ -687,7 +687,8 @@ floatx80 floatx80_add(floatx80, floatx80, float_status
*status);
floatx80 floatx80_sub(floatx80, floatx80, float_status *status);
floatx80 floatx80_mul(floatx80, floatx80, float_status *status);
floatx80 floatx80_div(floatx80, floatx80, float_status *status);
-floatx80 floatx80_modrem(floatx80, floatx80, bool, float_status *status);
+floatx80 floatx80_modrem(floatx80, floatx80, bool, uint64_t *,
+ float_status *status);
floatx80 floatx80_mod(floatx80, floatx80, float_status *status);
floatx80 floatx80_rem(floatx80, floatx80, float_status *status);
floatx80 floatx80_sqrt(floatx80, float_status *status);
--
2.17.1
--
Joseph S. Myers
jos...@codesourcery.com
[PATCH 6/7] target/i386: reimplement fprem1 using floatx80 operations
The x87 fprem1 emulation is currently based around conversion to
double, which is inherently unsuitable for a good emulation of any
floatx80 operation. Reimplement using the soft-float floatx80
remainder operations.
Signed-off-by: Joseph Myers
---
target/i386/fpu_helper.c | 96 +++-
1 file changed, 45 insertions(+), 51 deletions(-)
diff --git a/target/i386/fpu_helper.c b/target/i386/fpu_helper.c
index 8ef5b463ea..bab35e00a0 100644
--- a/target/i386/fpu_helper.c
+++ b/target/i386/fpu_helper.c
@@ -934,63 +934,57 @@ void helper_fxtract(CPUX86State *env)
merge_exception_flags(env, old_flags);
}
-void helper_fprem1(CPUX86State *env)
+static void helper_fprem_common(CPUX86State *env, bool mod)
{
-double st0, st1, dblq, fpsrcop, fptemp;
-CPU_LDoubleU fpsrcop1, fptemp1;
-int expdif;
-signed long long int q;
-
-st0 = floatx80_to_double(env, ST0);
-st1 = floatx80_to_double(env, ST1);
-
-if (isinf(st0) || isnan(st0) || isnan(st1) || (st1 == 0.0)) {
-ST0 = double_to_floatx80(env, 0.0 / 0.0); /* NaN */
-env->fpus &= ~0x4700; /* (C3,C2,C1,C0) <-- */
-return;
-}
-
-fpsrcop = st0;
-fptemp = st1;
-fpsrcop1.d = ST0;
-fptemp1.d = ST1;
-expdif = EXPD(fpsrcop1) - EXPD(fptemp1);
-
-if (expdif < 0) {
-/* optimisation? taken from the AMD docs */
-env->fpus &= ~0x4700; /* (C3,C2,C1,C0) <-- */
-/* ST0 is unchanged */
-return;
-}
+uint8_t old_flags = save_exception_flags(env);
+uint64_t quotient;
+CPU_LDoubleU temp0, temp1;
+int exp0, exp1, expdiff;
-if (expdif < 53) {
-dblq = fpsrcop / fptemp;
-/* round dblq towards nearest integer */
-dblq = rint(dblq);
-st0 = fpsrcop - fptemp * dblq;
+temp0.d = ST0;
+temp1.d = ST1;
+exp0 = EXPD(temp0);
+exp1 = EXPD(temp1);
-/* convert dblq to q by truncating towards zero */
-if (dblq < 0.0) {
-q = (signed long long int)(-dblq);
+env->fpus &= ~0x4700; /* (C3,C2,C1,C0) <-- */
+if (floatx80_is_zero(ST0) || floatx80_is_zero(ST1) ||
+exp0 == 0x7fff || exp1 == 0x7fff ||
+floatx80_invalid_encoding(ST0) || floatx80_invalid_encoding(ST1)) {
+ST0 = floatx80_modrem(ST0, ST1, mod, , >fp_status);
+} else {
+if (exp0 == 0) {
+exp0 = 1 - clz64(temp0.l.lower);
+}
+if (exp1 == 0) {
+exp1 = 1 - clz64(temp1.l.lower);
+}
+expdiff = exp0 - exp1;
+if (expdiff < 64) {
+ST0 = floatx80_modrem(ST0, ST1, mod, , >fp_status);
+env->fpus |= (quotient & 0x4) << (8 - 2); /* (C0) <-- q2 */
+env->fpus |= (quotient & 0x2) << (14 - 1); /* (C3) <-- q1 */
+env->fpus |= (quotient & 0x1) << (9 - 0); /* (C1) <-- q0 */
} else {
-q = (signed long long int)dblq;
+/* Partial remainder. This choice of how many bits to
+ * process at once is specified in AMD instruction set
+ * manuals, and empirically is followed by Intel
+ * processors as well; it ensures that the final remainder
+ * operation in a loop does produce the correct low three
+ * bits of the quotient. AMD manuals specify that the
+ * flags other than C2 are cleared, and empirically Intel
+ * processors clear them as well. */
+int n = 32 + (expdiff % 32);
+temp1.d = floatx80_scalbn(temp1.d, expdiff - n, >fp_status);
+ST0 = floatx80_mod(ST0, temp1.d, >fp_status);
+env->fpus |= 0x400; /* C2 <-- 1 */
}
-
-env->fpus &= ~0x4700; /* (C3,C2,C1,C0) <-- */
-/* (C0,C3,C1) <-- (q2,q1,q0) */
-env->fpus |= (q & 0x4) << (8 - 2); /* (C0) <-- q2 */
-env->fpus |= (q & 0x2) << (14 - 1); /* (C3) <-- q1 */
-env->fpus |= (q & 0x1) << (9 - 0); /* (C1) <-- q0 */
-} else {
-env->fpus |= 0x400; /* C2 <-- 1 */
-fptemp = pow(2.0, expdif - 50);
-fpsrcop = (st0 / st1) / fptemp;
-/* fpsrcop = integer obtained by chopping */
-fpsrcop = (fpsrcop < 0.0) ?
- -(floor(fabs(fpsrcop))) : floor(fpsrcop);
-st0 -= (st1 * fpsrcop * fptemp);
}
-ST0 = double_to_floatx80(env, st0);
+merge_exception_flags(env, old_flags);
+}
+
+void helper_fprem1(CPUX86State *env)
+{
+helper_fprem_common(env, false);
}
void helper_fprem(CPUX86State *env)
--
2.17.1
--
Joseph S. Myers
jos...@codesourcery.com
[PATCH 4/7] softfloat: do not set denominator high bit for floatx80 remainder
The floatx80 remainder implementation unnecessarily sets the high bit of bSig explicitly. By that point in the function, arguments that are invalid, zero, infinity or NaN have already been handled and subnormals have been through normalizeFloatx80Subnormal, so the high bit will already be set. Remove the unnecessary code. Signed-off-by: Joseph Myers --- fpu/softfloat.c | 1 - 1 file changed, 1 deletion(-) diff --git a/fpu/softfloat.c b/fpu/softfloat.c index 00f362af23..423a815196 100644 --- a/fpu/softfloat.c +++ b/fpu/softfloat.c @@ -5734,7 +5734,6 @@ floatx80 floatx80_modrem(floatx80 a, floatx80 b, bool mod, if ( aSig0 == 0 ) return a; normalizeFloatx80Subnormal( aSig0, , ); } -bSig |= UINT64_C(0x8000); zSign = aSign; expDiff = aExp - bExp; aSig1 = 0; -- 2.17.1 -- Joseph S. Myers jos...@codesourcery.com
[PATCH 3/7] softfloat: do not return pseudo-denormal from floatx80 remainder
The floatx80 remainder implementation sometimes returns the numerator
unchanged when the denominator is sufficiently larger than the
numerator. But if the value to be returned unchanged is a
pseudo-denormal, that is incorrect. Fix it to normalize the numerator
in that case.
Signed-off-by: Joseph Myers
---
fpu/softfloat.c | 18 +++---
1 file changed, 15 insertions(+), 3 deletions(-)
diff --git a/fpu/softfloat.c b/fpu/softfloat.c
index 091847beb9..00f362af23 100644
--- a/fpu/softfloat.c
+++ b/fpu/softfloat.c
@@ -5691,7 +5691,7 @@ floatx80 floatx80_modrem(floatx80 a, floatx80 b, bool mod,
float_status *status)
{
bool aSign, zSign;
-int32_t aExp, bExp, expDiff;
+int32_t aExp, bExp, expDiff, aExpOrig;
uint64_t aSig0, aSig1, bSig;
uint64_t q, term0, term1, alternateASig0, alternateASig1;
@@ -5700,7 +5700,7 @@ floatx80 floatx80_modrem(floatx80 a, floatx80 b, bool mod,
return floatx80_default_nan(status);
}
aSig0 = extractFloatx80Frac( a );
-aExp = extractFloatx80Exp( a );
+aExpOrig = aExp = extractFloatx80Exp( a );
aSign = extractFloatx80Sign( a );
bSig = extractFloatx80Frac( b );
bExp = extractFloatx80Exp( b );
@@ -5715,6 +5715,11 @@ floatx80 floatx80_modrem(floatx80 a, floatx80 b, bool
mod,
if ((uint64_t)(bSig << 1)) {
return propagateFloatx80NaN(a, b, status);
}
+if (aExp == 0 && aSig0 >> 63) {
+/* Pseudo-denormal argument must be returned in normalized
+ * form. */
+return packFloatx80(aSign, 1, aSig0);
+}
return a;
}
if ( bExp == 0 ) {
@@ -5734,7 +5739,14 @@ floatx80 floatx80_modrem(floatx80 a, floatx80 b, bool
mod,
expDiff = aExp - bExp;
aSig1 = 0;
if ( expDiff < 0 ) {
-if ( mod || expDiff < -1 ) return a;
+if ( mod || expDiff < -1 ) {
+if (aExp == 1 && aExpOrig == 0) {
+/* Pseudo-denormal argument must be returned in
+ * normalized form. */
+return packFloatx80(aSign, aExp, aSig0);
+}
+return a;
+}
shift128Right( aSig0, 0, 1, , );
expDiff = 0;
}
--
2.17.1
--
Joseph S. Myers
jos...@codesourcery.com
[PATCH 1/7] softfloat: merge floatx80_mod and floatx80_rem
The m68k-specific softfloat code includes a function floatx80_mod that
is extremely similar to floatx80_rem, but computing the remainder
based on truncating the quotient toward zero rather than rounding it
to nearest integer. This is also useful for emulating the x87 fprem
and fprem1 instructions. Change the floatx80_rem implementation into
floatx80_modrem that can perform either operation, with both
floatx80_rem and floatx80_mod as thin wrappers available for all
targets.
There does not appear to be any use for the _mod operation for other
floating-point formats in QEMU (the only other architectures using
_rem at all are linux-user/arm/nwfpe, for FPA emulation, and openrisc,
for instructions that have been removed in the latest version of the
architecture), so no change is made to the code for other formats.
Signed-off-by: Joseph Myers
---
fpu/softfloat.c | 49 ++--
include/fpu/softfloat.h | 2 +
target/m68k/softfloat.c | 83 -
target/m68k/softfloat.h | 1 -
4 files changed, 40 insertions(+), 95 deletions(-)
diff --git a/fpu/softfloat.c b/fpu/softfloat.c
index 6c8f2d597a..7b1ce7664f 100644
--- a/fpu/softfloat.c
+++ b/fpu/softfloat.c
@@ -5682,10 +5682,13 @@ floatx80 floatx80_div(floatx80 a, floatx80 b,
float_status *status)
/*
| Returns the remainder of the extended double-precision floating-point value
| `a' with respect to the corresponding value `b'. The operation is performed
-| according to the IEC/IEEE Standard for Binary Floating-Point Arithmetic.
+| according to the IEC/IEEE Standard for Binary Floating-Point Arithmetic,
+| if 'mod' is false; if 'mod' is true, return the remainder based on truncating
+| the quotient toward zero instead.
**/
-floatx80 floatx80_rem(floatx80 a, floatx80 b, float_status *status)
+floatx80 floatx80_modrem(floatx80 a, floatx80 b, bool mod,
+ float_status *status)
{
bool aSign, zSign;
int32_t aExp, bExp, expDiff;
@@ -5731,7 +5734,7 @@ floatx80 floatx80_rem(floatx80 a, floatx80 b,
float_status *status)
expDiff = aExp - bExp;
aSig1 = 0;
if ( expDiff < 0 ) {
-if ( expDiff < -1 ) return a;
+if ( mod || expDiff < -1 ) return a;
shift128Right( aSig0, 0, 1, , );
expDiff = 0;
}
@@ -5763,14 +5766,16 @@ floatx80 floatx80_rem(floatx80 a, floatx80 b,
float_status *status)
term1 = 0;
term0 = bSig;
}
-sub128( term0, term1, aSig0, aSig1, , );
-if (lt128( alternateASig0, alternateASig1, aSig0, aSig1 )
- || (eq128( alternateASig0, alternateASig1, aSig0, aSig1 )
- && ( q & 1 ) )
- ) {
-aSig0 = alternateASig0;
-aSig1 = alternateASig1;
-zSign = ! zSign;
+if (!mod) {
+sub128( term0, term1, aSig0, aSig1, , );
+if (lt128( alternateASig0, alternateASig1, aSig0, aSig1 )
+|| (eq128( alternateASig0, alternateASig1, aSig0, aSig1 )
+&& ( q & 1 ) )
+) {
+aSig0 = alternateASig0;
+aSig1 = alternateASig1;
+zSign = ! zSign;
+}
}
return
normalizeRoundAndPackFloatx80(
@@ -5778,6 +5783,28 @@ floatx80 floatx80_rem(floatx80 a, floatx80 b,
float_status *status)
}
+/*
+| Returns the remainder of the extended double-precision floating-point value
+| `a' with respect to the corresponding value `b'. The operation is performed
+| according to the IEC/IEEE Standard for Binary Floating-Point Arithmetic.
+**/
+
+floatx80 floatx80_rem(floatx80 a, floatx80 b, float_status *status)
+{
+return floatx80_modrem(a, b, false, status);
+}
+
+/*
+| Returns the remainder of the extended double-precision floating-point value
+| `a' with respect to the corresponding value `b', with the quotient truncated
+| toward zero.
+**/
+
+floatx80 floatx80_mod(floatx80 a, floatx80 b, float_status *status)
+{
+return floatx80_modrem(a, b, true, status);
+}
+
/*
| Returns the square root of the extended double-precision floating-point
| value `a'. The operation is performed according to the IEC/IEEE Standard
diff --git a/include/fpu/softfloat.h b/include/fpu/softfloat.h
index 16ca697a73..bff6934d09 100644
--- a/include/fpu/softfloat.h
+++ b/include/fpu/softfloat.h
@@ -687,6 +687,8 @@ floatx80 floatx80_add(floatx80, floatx80, float_status
*status);
floatx80 floatx80_sub(floatx80, floatx80,
[PATCH 2/7] softfloat: fix floatx80 remainder pseudo-denormal check for zero
The floatx80 remainder implementation ignores the high bit of the
significand when checking whether an operand (numerator) with zero
exponent is zero. This means it mishandles a pseudo-denormal
representation of 0x1p-16382L by treating it as zero. Fix this by
checking the whole significand instead.
Signed-off-by: Joseph Myers
---
fpu/softfloat.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/fpu/softfloat.c b/fpu/softfloat.c
index 7b1ce7664f..091847beb9 100644
--- a/fpu/softfloat.c
+++ b/fpu/softfloat.c
@@ -5726,7 +5726,7 @@ floatx80 floatx80_modrem(floatx80 a, floatx80 b, bool mod,
normalizeFloatx80Subnormal( bSig, , );
}
if ( aExp == 0 ) {
-if ( (uint64_t) ( aSig0<<1 ) == 0 ) return a;
+if ( aSig0 == 0 ) return a;
normalizeFloatx80Subnormal( aSig0, , );
}
bSig |= UINT64_C(0x8000);
--
2.17.1
--
Joseph S. Myers
jos...@codesourcery.com
Re: [PATCH v2] fuzz: add oss-fuzz build.sh script
On Friday, 2020-06-05 at 14:24:59 -04, Alexander Bulekov wrote: > Hi Darren, > > On 200605 1858, Darren Kenny wrote: >> Hi Alex, >> >> From looking at another OSS Fuzz project recently (a coincidence) I >> wonder if we could make this script work so that it can be run outside >> of the OSS-Fuzz environment? >> >> Specifically, for example, if $OUT is not set, then creating a subdir in >> the build directory, and setting it to be that. >> > For $OUT, do you think it would be better to require it as > a user-configurable environment variable? My concern is that making it > a subdirectory of the build dir would mean that the pc-bios files exist > located in $OUT/../pc-bios. This doesn't reflect OSS-Fuzz, where we > specifically have to copy them to $OUT/pc-bios/ > The script is copying them in to $OUT near the end still, isn't it? That should be fine if it is, shouldn't it? Or am I missing something? Thanks, Darren.
[PATCH 0/7] softfloat, target/i386: fprem, fprem1 fixes
The x87 floating-point emulation of the fprem and fprem1 instructions works via conversion to and from double. This is inherently unsuitable for a good emulation of any floatx80 operation. This patch series adapts the softfloat floatx80_rem implementation to be suitable for these instructions and uses it to reimplement them. There is an existing test for these instructions, test-i386-fprem.c, based on comparison of output. It produces 1679695 lines of output, and before this patch series 415422 of those lines are different on hardware from the output produced by QEMU. Some of those differences are because QEMU's x87 emulation does not yet produce the "denormal operand" exception; ignoring such differences (modifying the output from a native run not to report that exception), there are still 398833 different lines. This patch series reduces that latter number to 1 (that one difference being because of missing checks for floating-point stack underflow, another global issue with the x87 emulation), or 35517 different lines without the correction for lack of denormal operand exception support. Several fixes to and new features in the softfloat support for this operation are needed; floatx80_mod, previously present in the m68k code only, is made generic and unified with floatx80_rem in a new floatx80_modrem of which floatx80_mod and floatx80_rem are thin wrappers. The only architectures using float*_rem for other formats are arm (FPA emulation) and openrisc (instructions that have been removed in the latest architecture version); they do not appear to need any of the new features, and all the bugs fixed are specific to floatx80, so no changes are made to the remainder implementation for those formats. A new feature added is returning the low bits of the quotient from floatx80_modrem, as needed for both x87 and m68k. The logic used to determine the low 7 bits of the quotient for m68k (target/m68k/fpu_helper.c:make_quotient) appears completely bogus (it looks at the result of converting the remainder to integer, the quotient having been discarded by that point); this patch series does not change that to use the new interface, but the m68k maintainers may wish to do so. The Intel instruction set documentation leaves unspecified the exact number of bits by which the remainder instructions reduce the operand each time. The AMD documentation gives a specific formula, which empirically Intel processors follow as well, and that formula is implemented in the code. The AMD documentation also specifies that flags other than C2 are cleared in the partial remainder case, whereas the Intel manual is silent on that (but the processors do appear to clear those flags); this patch implements that flag clearing, and keeps the existing flag clearing in cases where the instructions raise "invalid" (although it seems hardware in fact only clears some but not all flags in that case, leaving other flags unchanged). The Intel manuals include an inaccurate table asserting that (finite REM 0) should raise "divide by zero"; actually, in accordance with IEEE semantics, it raises "invalid". The AMD manuals inaccurately say for both fprem and fprem1 that if the exponent difference is negative, the numerator is returned unchanged, which is correct (apart from normalizing pseudo-denormals) for fprem but not for fprem1 (and the old QEMU code had an incorrect optimization following the AMD manuals for fprem1). Joseph Myers (7): softfloat: merge floatx80_mod and floatx80_rem softfloat: fix floatx80 remainder pseudo-denormal check for zero softfloat: do not return pseudo-denormal from floatx80 remainder softfloat: do not set denominator high bit for floatx80 remainder softfloat: return low bits of quotient from floatx80_modrem target/i386: reimplement fprem1 using floatx80 operations target/i386: reimplement fprem using floatx80 operations fpu/softfloat.c | 83 + include/fpu/softfloat.h | 3 + target/i386/fpu_helper.c | 154 --- target/m68k/softfloat.c | 83 - target/m68k/softfloat.h | 1 - 5 files changed, 116 insertions(+), 208 deletions(-) -- 2.17.1 -- Joseph S. Myers jos...@codesourcery.com
[Bug 1815263] Re: hvf accelerator crashes on quest boot
** Tags added: hvf -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/1815263 Title: hvf accelerator crashes on quest boot Status in QEMU: New Bug description: Host OS: macOS High Sierra (10.13.6) MacBook Pro (Retina, Mid 2015) Processor: 2.8GHz Intel Core i7 Guest OS: OpenBSD 6.4 install media (install64.iso) Qemu 3.1.0 release, built with: ./configure --prefix=/usr/local/Cellar/qemu/3.1.0_1 --cc=clang --host-cc=clang --disable-bsd-user --disable-guest-agent --enable-curses --enable-libssh2 --enable-vde --extra-cflags=-DNCURSES_WIDECHAR=1 --enable-cocoa --disable-sdl --disable-gtk --enable-hvf --target-list=x86_64-softmmu --enable-debug I invoke qemu like this: Last command had exit code: 0 at 22:58 nwallace@nwallace-ltm3:~ $ sudo qemu-system-x86_64 -M accel=hvf -boot d -cdrom ~/Downloads/install64.iso Password: qemu-system-x86_64: warning: host doesn't support requested feature: CPUID.8001H:ECX.svm [bit 2] bad size Abort trap: 6 Last command had exit code: 134 at 22:58 nwallace@nwallace-ltm3:~ $ I ran qemu in lldb to get a stack trace and I get: Last command had exit code: 0 at 22:54 nwallace@nwallace-ltm3:~/Downloads $ sudo lldb -- qemu-system-x86_64 -M accel=hvf -boot d -cdrom /Users/nwallace/Downloads/install64.iso Password: (lldb) target create "qemu-system-x86_64" Current executable set to 'qemu-system-x86_64' (x86_64). (lldb) settings set -- target.run-args "-M" "accel=hvf" "-boot" "d" "-cdrom" "/Users/nwallace/Downloads/install64.i so" (lldb) run Process 96474 launched: '/usr/local/bin/qemu-system-x86_64' (x86_64) Process 96474 stopped * thread #1, queue = 'com.apple.main-thread', stop reason = signal SIGUSR2 frame #0: 0x7fff5ef0c00a libsystem_kernel.dylib`__sigsuspend + 10 libsystem_kernel.dylib`__sigsuspend: -> 0x7fff5ef0c00a <+10>: jae0x7fff5ef0c014; <+20> 0x7fff5ef0c00c <+12>: movq %rax, %rdi 0x7fff5ef0c00f <+15>: jmp0x7fff5ef02b0e; cerror 0x7fff5ef0c014 <+20>: retq Target 0: (qemu-system-x86_64) stopped. (lldb) process handle SIGUSR1 -n true -p true -s false NAME PASS STOP NOTIFY === = = == SIGUSR1 true false true (lldb) process handle SIGUSR2 -n true -p true -s false NAME PASS STOP NOTIFY === = = == SIGUSR2 true false true (lldb) c Process 96474 resuming qemu-system-x86_64: warning: host doesn't support requested feature: CPUID.8001H:ECX.svm [bit 2] Process 96474 stopped and restarted: thread 9 received signal: SIGUSR2 Process 96474 stopped and restarted: thread 9 received signal: SIGUSR2 bad size Process 96474 stopped * thread #9, stop reason = signal SIGABRT frame #0: 0x7fff5ef0bb66 libsystem_kernel.dylib`__pthread_kill + 10 libsystem_kernel.dylib`__pthread_kill: -> 0x7fff5ef0bb66 <+10>: jae0x7fff5ef0bb70; <+20> 0x7fff5ef0bb68 <+12>: movq %rax, %rdi 0x7fff5ef0bb6b <+15>: jmp0x7fff5ef02ae9; cerror_nocancel 0x7fff5ef0bb70 <+20>: retq Target 0: (qemu-system-x86_64) stopped. (lldb) bt * thread #9, stop reason = signal SIGABRT * frame #0: 0x7fff5ef0bb66 libsystem_kernel.dylib`__pthread_kill + 10 frame #1: 0x7fff5f0d6080 libsystem_pthread.dylib`pthread_kill + 333 frame #2: 0x7fff5ee671ae libsystem_c.dylib`abort + 127 frame #3: 0x00010016b6ec qemu-system-x86_64`exec_cmps_single + 400 frame #4: 0x00010016ada4 qemu-system-x86_64`exec_cmps + 65 frame #5: 0x000100169aaa qemu-system-x86_64`exec_instruction + 48 frame #6: 0x000100164eb2 qemu-system-x86_64`hvf_vcpu_exec + 2658 frame #7: 0x00010005bed6 qemu-system-x86_64`qemu_hvf_cpu_thread_fn + 200 frame #8: 0x0001003ee531 qemu-system-x86_64`qemu_thread_start + 107 frame #9: 0x7fff5f0d3661 libsystem_pthread.dylib`_pthread_body + 340 frame #10: 0x7fff5f0d350d libsystem_pthread.dylib`_pthread_start + 377 frame #11: 0x7fff5f0d2bf9 libsystem_pthread.dylib`thread_start + 13 (lldb) quit Quitting LLDB will kill one or more processes. Do you really want to proceed: [Y/n] Y Last command had exit code: 0 at 23:01 nwallace@nwallace-ltm3:~/Downloads $ I'm happy to work with someone more knowledgeable to reproduce this issue and provide debugging assistance as I'm able. To manage notifications about this bug go to: https://bugs.launchpad.net/qemu/+bug/1815263/+subscriptions
[Bug 1827005] Re: hvf: ubuntu iso boot menu issue
** Tags added: hvf -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/1827005 Title: hvf: ubuntu iso boot menu issue Status in QEMU: New Bug description: With hvf acceleration on macOS, ubuntu server installation ISO boot language menu shows fractured images. To reproduce the issue: ./x86_64-softmmu/qemu-system-x86_64 -m 800 -accel hvf -cdrom ~/ubuntu-16.04.4-server-amd64.iso Control: ./x86_64-softmmu/qemu-system-x86_64 -m 800 -accel tcg -cdrom ~/ubuntu-16.04.4-server-amd64.iso Host: macOS Mojave 10.14.3 Guest: Ubuntu Server 16.04.4 ISO QEMU: version 3.1.94 (v4.0.0-rc4) To manage notifications about this bug go to: https://bugs.launchpad.net/qemu/+bug/1827005/+subscriptions
Re: [PATCH] fuzz: add oss-fuzz build.sh script
On 200605 1956, Philippe Mathieu-Daudé wrote:
> On 6/5/20 7:40 PM, Alexander Bulekov wrote:
> > It is neater to keep this in the QEMU repo, since any change that
> > requires an update to the oss-fuzz build configuration, can make the
> > necessary changes in the same series.
> >
> > Suggested-by: Philippe Mathieu-Daude
>
> 'Philippe Mathieu-Daudé' ;)
Oops - Sorry.
>
> > Signed-off-by: Alexander Bulekov
> > ---
> > scripts/oss-fuzz/build.sh | 47 +++
> > 1 file changed, 47 insertions(+)
> > create mode 100755 scripts/oss-fuzz/build.sh
> >
> > diff --git a/scripts/oss-fuzz/build.sh b/scripts/oss-fuzz/build.sh
> > new file mode 100755
> > index 00..7be6dcce4c
> > --- /dev/null
> > +++ b/scripts/oss-fuzz/build.sh
> > @@ -0,0 +1,47 @@
> > +#!/bin/sh
> > +#
> > +# Update syscall_nr.h files from linux headers asm-generic/unistd.h
>
> Hmmm?
Fixed in v2.
>
> > +#
> > +# This code is licensed under the GPL version 2 or later. See
> > +# the COPYING file in the top-level directory.
> > +#
> > +
> > +# build project
>
> Please mention this file use consumed by
> https://github.com/google/oss-fuzz/.../projects/qemu/Dockerfile
>
> > +# e.g.
> > +# ./autogen.sh
> > +# ./configure
> > +# make -j$(nproc) all
> > +
> > +# build fuzzers
> > +# e.g.
> > +# $CXX $CXXFLAGS -std=c++11 -Iinclude \
> > +# /path/to/name_of_fuzzer.cc -o $OUT/name_of_fuzzer \
> > +# $LIB_FUZZING_ENGINE /path/to/library.a
> > +
> > +mkdir -p $OUT/lib/ # Shared libraries
>
> Maybe rename OUT -> DEST_DIR?
$OUT is something specified by OSS-Fuzz, when it runs the script in
docker. If its better, I can do DEST_DIR=$OUT
> > +
> > +# Build once to get the list of dynamic lib paths, and copy them over
> > +./configure --datadir="./data/" --disable-werror --cc="$CC" --cxx="$CXX" \
> > +--extra-cflags="$CFLAGS -U __OPTIMIZE__ "
>
> So we use an in-tree build.
>
> Still we could set some SRCDIR=./
I can change it to build in ./build/ or even an out-of-tree build, if
thats neater..
>
> > +make CONFIG_FUZZ=y CFLAGS="$LIB_FUZZING_ENGINE" -j$(nproc)
> > i386-softmmu/fuzz
> > +
> > +for i in $(ldd ./i386-softmmu/qemu-fuzz-i386 | cut -f3 -d' '); do
> > +cp $i $OUT/lib/
> > +done
> > +rm ./i386-softmmu/qemu-fuzz-i386
> > +
> > +# Build a second time to build the final binary with correct rpath
> > +./configure --datadir="./data/" --disable-werror --cc="$CC" --cxx="$CXX" \
> > +--extra-cflags="$CFLAGS -U __OPTIMIZE__" \
> > +--extra-ldflags="-Wl,-rpath,'\$\$ORIGIN/lib'"
> > +make CONFIG_FUZZ=y CFLAGS="$LIB_FUZZING_ENGINE" -j$(nproc)
> > i386-softmmu/fuzz
> > +
> > +# Copy over the datadir
> > +cp -r ./pc-bios/ $OUT/pc-bios
>
> "make install-datadir"?
With something like: ./configure --datadir="$OUT/pc-bios/"
Ok.
>
> > +
> > +# Copy over the qemu-fuzz-i386, naming it according to each available fuzz
> > +# target (See 05509c8e6d fuzz: select fuzz target using executable name)
> > +for target in $(./i386-softmmu/qemu-fuzz-i386 | awk '$1 ~ /\*/ {print
> > $2}');
> > +do
> > +cp ./i386-softmmu/qemu-fuzz-i386 $OUT/qemu-fuzz-i386-target-$target
>
> There seems to be an extra 'target'.
I don't think so, unless i'm missing something.
We do a strstr(argv[0], "-target-") in fuzz.c The
targets need to be named:
qemu-fuzz-i386-target-virtio-net-socket
qemu-fuzz-i386-target-i440fx-qos-fork-fuzz
etc..
Thanks
-Alex
> > +done
> >
>
> Or "make install", not sure.
Re: [PATCH] configure: Disable -Wtautological-type-limit-compare
On 6/5/20 1:09 PM, Peter Maydell wrote:
If there's an ordering requirement here we should make it clearer,
or somebody is going to do the obvious "tidying up" at some point
in the future.
Perhaps this whole set of lines should be rearranged, something like:
# Enable these warnings if the compiler supports them:
warn_flags="-Wold-style-declaration -Wold-style-definition -Wtype-limits"
warn_flags="-Wformat-security -Wformat-y2k -Winit-self
-Wignored-qualifiers $warn_flags"
warn_flags="-Wno-missing-include-dirs -Wempty-body -Wnested-externs $warn_flags"
warn_flags="-Wendif-labels -Wexpansion-to-defined $warn_flags"
# Now disable sub-types of warning we don't want but which are
# enabled by some of the warning flags we do want; these must come
# later in the compiler command line than the enabling warning options.
nowarn_flags="-Wno-missing-include-dirs -Wno-shift-negative-value"
nowarn_flags="-Wno-initializer-overrides $nowarn_flags"
nowarn_flags="-Wno-string-plus-int -Wno-typedef-redefinition $nowarn_flags"
warn_flags="$warn_flags $nowarn_flags"
(is there a nicer shell idiom for creating a variable that's
a long string of stuff without having over-long lines?)
You could always do:
# Append $2 into the variable named $1, with space separation
add_to () {
eval $1=\${$1:+\"\$$1 \"}\$2
}
add_to warn_flags -Wold-style-declaration
add_to warn_flags -Wold-style-definition
add_to warn_flags -Wtype-limits
...
add_to nowarn_flags -Wno-string-plus-int
add_to nowarn_flags -Wno-typedef-redefinition
warn_flags="$warn_flags $nowarn_flags"
It's also tempting to pull the handful of warning related
options currently set directly in QEMU_CFLAGS (-Wall, etc) into
this same set of variables.
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3226
Virtualization: qemu.org | libvirt.org
Re: [PULL v2 05/13] accel/tcg: Relax va restrictions on 64-bit guests
On 6/5/20 10:46 AM, Alex Bennée wrote:
>
> Richard Henderson writes:
>
>> On 6/5/20 7:11 AM, Alex Bennée wrote:
>>> @@ -467,7 +467,7 @@ abi_long target_mmap(abi_ulong start, abi_ulong len,
>>> int prot,
>>> * It can fail only on 64-bit host with 32-bit target.
>>> * On any other target/host host mmap() handles this error
>>> correctly.
>>> */
>>> -if (!guest_range_valid(start, len)) {
>>> +if (end < start || !guest_range_valid(start, len)) {
>>> errno = ENOMEM;
>>> goto fail;
>>> }
>>
>> Interesting. I was adjusting guest_range_valid tagged pointers yesterday,
>> and
>> thought that it looked buggy.
>
> Should be picking this up in guest_range_valid?
I think so. How can a range really be considered valid if it wraps?
r~
Re: [PATCH v2] fuzz: add oss-fuzz build.sh script
On 6/5/20 7:58 PM, Darren Kenny wrote: > Hi Alex, > > From looking at another OSS Fuzz project recently (a coincidence) I > wonder if we could make this script work so that it can be run outside > of the OSS-Fuzz environment? Yes, we want to have this feature to reproduce/debug what oss-fuzz does. > > Specifically, for example, if $OUT is not set, then creating a subdir in > the build directory, and setting it to be that. > > Similarly for some other things like $LIB_FUZZING_ENGINE? > > I'm just thinking that it might help someone that is not familiar with > OSS-Fuzz to validate that the script still works without having to go > through setting up the containers, etc that would be required to > validate it. Exactly. > > Also, I would definitely recommend running ShellCheck against any script > to ensure that you're catching any mistakes that can so easily be put in > to shell scripts - speaking from experience here ;) > > Thanks, > > Darren. [...]
Re: [PATCH v2] fuzz: add oss-fuzz build.sh script
Hi Darren,
On 200605 1858, Darren Kenny wrote:
> Hi Alex,
>
> From looking at another OSS Fuzz project recently (a coincidence) I
> wonder if we could make this script work so that it can be run outside
> of the OSS-Fuzz environment?
>
> Specifically, for example, if $OUT is not set, then creating a subdir in
> the build directory, and setting it to be that.
>
For $OUT, do you think it would be better to require it as
a user-configurable environment variable? My concern is that making it
a subdirectory of the build dir would mean that the pc-bios files exist
located in $OUT/../pc-bios. This doesn't reflect OSS-Fuzz, where we
specifically have to copy them to $OUT/pc-bios/
> Similarly for some other things like $LIB_FUZZING_ENGINE?
Will do.
> I'm just thinking that it might help someone that is not familiar with
> OSS-Fuzz to validate that the script still works without having to go
> through setting up the containers, etc that would be required to
> validate it.
>
> Also, I would definitely recommend running ShellCheck against any script
> to ensure that you're catching any mistakes that can so easily be put in
> to shell scripts - speaking from experience here ;)
I will :)
> Thanks,
>
> Darren.
Thanks for bringing these up!
-Alex
>
> On Friday, 2020-06-05 at 13:50:28 -04, Alexander Bulekov wrote:
> > It is neater to keep this in the QEMU repo, since any change that
> > requires an update to the oss-fuzz build configuration, can make the
> > necessary changes in the same series.
> >
> > Suggested-by: Philippe Mathieu-Daude
> > Signed-off-by: Alexander Bulekov
> > ---
> >
> > v2 updates the script header comment.
> >
> > scripts/oss-fuzz/build.sh | 50 +++
> > 1 file changed, 50 insertions(+)
> > create mode 100755 scripts/oss-fuzz/build.sh
> >
> > diff --git a/scripts/oss-fuzz/build.sh b/scripts/oss-fuzz/build.sh
> > new file mode 100755
> > index 00..e93d6f2e03
> > --- /dev/null
> > +++ b/scripts/oss-fuzz/build.sh
> > @@ -0,0 +1,50 @@
> > +#!/bin/sh
> > +#
> > +# OSS-Fuzz build script. See:
> > +#
> > https://google.github.io/oss-fuzz/getting-started/new-project-guide/#buildsh
> > +#
> > +# This code is licensed under the GPL version 2 or later. See
> > +# the COPYING file in the top-level directory.
> > +#
> > +
> > +# build project
> > +# e.g.
> > +# ./autogen.sh
> > +# ./configure
> > +# make -j$(nproc) all
> > +
> > +# build fuzzers
> > +# e.g.
> > +# $CXX $CXXFLAGS -std=c++11 -Iinclude \
> > +# /path/to/name_of_fuzzer.cc -o $OUT/name_of_fuzzer \
> > +# $LIB_FUZZING_ENGINE /path/to/library.a
> > +
> > +mkdir -p $OUT/lib/ # Shared libraries
> > +
> > +# Build once to get the list of dynamic lib paths, and copy them over
> > +./configure --datadir="./data/" --disable-werror --cc="$CC" --cxx="$CXX" \
> > +--extra-cflags="$CFLAGS -U __OPTIMIZE__ "
> > +make CONFIG_FUZZ=y CFLAGS="$LIB_FUZZING_ENGINE" -j$(nproc)
> > i386-softmmu/fuzz
> > +
> > +for i in $(ldd ./i386-softmmu/qemu-fuzz-i386 | cut -f3 -d' '); do
> > +cp $i $OUT/lib/
> > +done
> > +rm ./i386-softmmu/qemu-fuzz-i386
> > +
> > +# Build a second time to build the final binary with correct rpath
> > +./configure --datadir="./data/" --disable-werror --cc="$CC" --cxx="$CXX" \
> > +--extra-cflags="$CFLAGS -U __OPTIMIZE__" \
> > +--extra-ldflags="-Wl,-rpath,'\$\$ORIGIN/lib'"
> > +make CONFIG_FUZZ=y CFLAGS="$LIB_FUZZING_ENGINE" -j$(nproc)
> > i386-softmmu/fuzz
> > +
> > +# Copy over the datadir
> > +cp -r ./pc-bios/ $OUT/pc-bios
> > +
> > +# Run the fuzzer with no arguments, to print the help-string and get the
> > list
> > +# of available fuzz-targets. Copy over the qemu-fuzz-i386, naming it
> > according
> > +# to each available fuzz target (See 05509c8e6d fuzz: select fuzz target
> > using
> > +# executable name)
> > +for target in $(./i386-softmmu/qemu-fuzz-i386 | awk '$1 ~ /\*/ {print
> > $2}');
> > +do
> > +cp ./i386-softmmu/qemu-fuzz-i386 $OUT/qemu-fuzz-i386-target-$target
> > +done
> > --
> > 2.26.2
[PATCH v1 2/2] nvme: allow cmb and pmr to be enabled on same device
So far it was not possible to have CMB and PMR emulated on the same
device, because BAR2 was used exclusively either of PMR or CMB. This
patch places CMB at BAR4 offset so it not conflicts with MSI-X vectors.
Signed-off-by: Andrzej Jakowski
---
hw/block/nvme.c | 127 +--
hw/block/nvme.h | 3 +-
include/block/nvme.h | 4 +-
3 files changed, 91 insertions(+), 43 deletions(-)
diff --git a/hw/block/nvme.c b/hw/block/nvme.c
index f0b45704be..353cf20e0a 100644
--- a/hw/block/nvme.c
+++ b/hw/block/nvme.c
@@ -22,12 +22,12 @@
* [pmrdev=,] \
* num_queues=
*
- * Note cmb_size_mb denotes size of CMB in MB. CMB is assumed to be at
- * offset 0 in BAR2 and supports only WDS, RDS and SQS for now.
+ * Note cmb_size_mb denotes size of CMB in MB. CMB when configured is assumed
+ * to be resident in BAR4 at certain offset - this is because BAR4 is also
+ * used for storing MSI-X table that is available at offset 0 in BAR4.
*
- * cmb_size_mb= and pmrdev= options are mutually exclusive due to limitation
- * in available BAR's. cmb_size_mb= will take precedence over pmrdev= when
- * both provided.
+ * pmrdev is assumed to be resident in BAR2. When configured it consumes whole
+ * BAR2 exclusively.
* Enabling pmr emulation can be achieved by pointing to memory-backend-file.
* For example:
* -object memory-backend-file,id=,share=on,mem-path=, \
@@ -64,9 +64,10 @@ static void nvme_process_sq(void *opaque);
static void nvme_addr_read(NvmeCtrl *n, hwaddr addr, void *buf, int size)
{
-if (n->cmbsz && addr >= n->ctrl_mem.addr &&
-addr < (n->ctrl_mem.addr + int128_get64(n->ctrl_mem.size))) {
-memcpy(buf, (void *)>cmbuf[addr - n->ctrl_mem.addr], size);
+hwaddr cmb_addr = n->bar4.addr + n->cmb_offset;
+if (n->cmbsz && addr >= cmb_addr &&
+(addr + size) <= (cmb_addr + NVME_CMBSZ_GETSIZE(n->bar.cmbsz))) {
+memcpy(buf, (void *)>cmbuf[addr - cmb_addr], size);
} else {
pci_dma_read(>parent_obj, addr, buf, size);
}
@@ -152,17 +153,18 @@ static uint16_t nvme_map_prp(QEMUSGList *qsg,
QEMUIOVector *iov, uint64_t prp1,
uint64_t prp2, uint32_t len, NvmeCtrl *n)
{
hwaddr trans_len = n->page_size - (prp1 % n->page_size);
+hwaddr cmb_addr = n->bar4.addr + n->cmb_offset;
trans_len = MIN(len, trans_len);
int num_prps = (len >> n->page_bits) + 1;
if (unlikely(!prp1)) {
trace_nvme_err_invalid_prp();
return NVME_INVALID_FIELD | NVME_DNR;
-} else if (n->cmbsz && prp1 >= n->ctrl_mem.addr &&
- prp1 < n->ctrl_mem.addr + int128_get64(n->ctrl_mem.size)) {
+} else if (n->cmbsz && prp1 >= cmb_addr &&
+ prp1 < cmb_addr + int128_get64(n->bar4.size)) {
qsg->nsg = 0;
qemu_iovec_init(iov, num_prps);
-qemu_iovec_add(iov, (void *)>cmbuf[prp1 - n->ctrl_mem.addr],
trans_len);
+qemu_iovec_add(iov, (void *)>cmbuf[prp1 - cmb_addr], trans_len);
} else {
pci_dma_sglist_init(qsg, >parent_obj, num_prps);
qemu_sglist_add(qsg, prp1, trans_len);
@@ -207,7 +209,8 @@ static uint16_t nvme_map_prp(QEMUSGList *qsg, QEMUIOVector
*iov, uint64_t prp1,
if (qsg->nsg){
qemu_sglist_add(qsg, prp_ent, trans_len);
} else {
-qemu_iovec_add(iov, (void *)>cmbuf[prp_ent -
n->ctrl_mem.addr], trans_len);
+qemu_iovec_add(iov, (void *)>cmbuf[prp_ent - cmb_addr],
+ trans_len);
}
len -= trans_len;
i++;
@@ -220,7 +223,8 @@ static uint16_t nvme_map_prp(QEMUSGList *qsg, QEMUIOVector
*iov, uint64_t prp1,
if (qsg->nsg) {
qemu_sglist_add(qsg, prp2, len);
} else {
-qemu_iovec_add(iov, (void *)>cmbuf[prp2 -
n->ctrl_mem.addr], trans_len);
+qemu_iovec_add(iov, (void *)>cmbuf[prp2 - cmb_addr],
+ trans_len);
}
}
}
@@ -1342,6 +1346,71 @@ static const MemoryRegionOps nvme_cmb_ops = {
},
};
+#define NVME_MSIX_BIR (4)
+static void nvme_bar4_init(PCIDevice *pci_dev)
+{
+NvmeCtrl *n = NVME(pci_dev);
+int status;
+uint64_t bar_size = 4096;
+uint32_t nvme_pba_offset = bar_size / 2;
+uint32_t nvme_pba_size = QEMU_ALIGN_UP(n->num_queues, 64) / 8;
+uint32_t cmb_size_units;
+
+if (n->num_queues * PCI_MSIX_ENTRY_SIZE > nvme_pba_offset) {
+nvme_pba_offset = n->num_queues * PCI_MSIX_ENTRY_SIZE;
+}
+
+if (nvme_pba_offset + nvme_pba_size > 4096) {
+bar_size = nvme_pba_offset + nvme_pba_size;
+}
+
+if (n->cmb_size_mb) {
+/* Contoller capabilities */
+NVME_CAP_SET_CMBS(n->bar.cap, 1);
+
+NVME_CMBSZ_SET_SQS(n->bar.cmbsz, 1);
+NVME_CMBSZ_SET_CQS(n->bar.cmbsz, 0);
+
[PATCH v1] nvme: allow cmb and pmr emulation on same device
This patch series does following: - Fixes problem where CMBS bit was not set in controller capabilities register, so support for CMB was not correctly advertised to guest. This is resend of patch that has been submitted and reviewed by Klaus [1] - Introduces BAR4 sharing between MSI-X vectors and CMB. This allows to have CMB and PMR emulated on the same device. This extension was indicated by Keith [2] [1]: https://lore.kernel.org/qemu-devel/20200408055607.g2ii7gwqbnv6cd3w@apples.localdomain/ [2]: https://lore.kernel.org/qemu-devel/20200330165518.ga8...@redsun51.ssa.fujisawa.hgst.com/
[PATCH v1 1/2] nvme: indicate CMB support through controller capabilities register
This patch sets CMBS bit in controller capabilities register when user
configures NVMe driver with CMB support, so capabilites are correctly
reported to guest OS.
Signed-off-by: Andrzej Jakowski
Reviewed-by: Klaus Jensen
---
hw/block/nvme.c | 2 ++
include/block/nvme.h | 6 +-
2 files changed, 7 insertions(+), 1 deletion(-)
diff --git a/hw/block/nvme.c b/hw/block/nvme.c
index a21eeca2fb..f0b45704be 100644
--- a/hw/block/nvme.c
+++ b/hw/block/nvme.c
@@ -1449,6 +1449,8 @@ static void nvme_realize(PCIDevice *pci_dev, Error **errp)
n->bar.intmc = n->bar.intms = 0;
if (n->cmb_size_mb) {
+/* Contoller capabilities */
+NVME_CAP_SET_CMBS(n->bar.cap, 1);
NVME_CMBLOC_SET_BIR(n->bar.cmbloc, 2);
NVME_CMBLOC_SET_OFST(n->bar.cmbloc, 0);
diff --git a/include/block/nvme.h b/include/block/nvme.h
index 5525c8e343..b48349dbd0 100644
--- a/include/block/nvme.h
+++ b/include/block/nvme.h
@@ -35,6 +35,7 @@ enum NvmeCapShift {
CAP_MPSMIN_SHIFT = 48,
CAP_MPSMAX_SHIFT = 52,
CAP_PMR_SHIFT = 56,
+CAP_CMB_SHIFT = 57,
};
enum NvmeCapMask {
@@ -48,6 +49,7 @@ enum NvmeCapMask {
CAP_MPSMIN_MASK= 0xf,
CAP_MPSMAX_MASK= 0xf,
CAP_PMR_MASK = 0x1,
+CAP_CMB_MASK = 0x1,
};
#define NVME_CAP_MQES(cap) (((cap) >> CAP_MQES_SHIFT) & CAP_MQES_MASK)
@@ -78,8 +80,10 @@ enum NvmeCapMask {
<< CAP_MPSMIN_SHIFT)
#define NVME_CAP_SET_MPSMAX(cap, val) (cap |= (uint64_t)(val &
CAP_MPSMAX_MASK)\
<<
CAP_MPSMAX_SHIFT)
-#define NVME_CAP_SET_PMRS(cap, val) (cap |= (uint64_t)(val & CAP_PMR_MASK)\
+#define NVME_CAP_SET_PMRS(cap, val) (cap |= (uint64_t)(val & CAP_PMR_MASK)
\
<< CAP_PMR_SHIFT)
+#define NVME_CAP_SET_CMBS(cap, val) (cap |= (uint64_t)(val & CAP_CMB_MASK)
\
+ << CAP_CMB_SHIFT)
enum NvmeCcShift {
CC_EN_SHIFT = 0,
--
2.21.1
Re: [PATCH] configure: Disable -Wtautological-type-limit-compare
On Fri, 5 Jun 2020 at 18:47, Richard Henderson wrote: > > On 6/5/20 8:53 AM, Alex Bennée wrote: > >> --- a/configure > >> +++ b/configure > >> @@ -2009,6 +2009,8 @@ gcc_flags="-Wno-missing-include-dirs -Wempty-body > >> -Wnested-externs $gcc_flags" > >> gcc_flags="-Wendif-labels -Wno-shift-negative-value $gcc_flags" > >> gcc_flags="-Wno-initializer-overrides -Wexpansion-to-defined $gcc_flags" > >> gcc_flags="-Wno-string-plus-int -Wno-typedef-redefinition $gcc_flags" > >> +gcc_flags="$gcc_flags -Wno-tautological-type-limit-compare" > >> + > > > > nit: the pattern is reversed compared to the previous lines (foo $gcc_flags) > > Not a nit. The -Wno- must follow -Wtype-limit, or -Wtype-limit will turn it > back on. If there's an ordering requirement here we should make it clearer, or somebody is going to do the obvious "tidying up" at some point in the future. Perhaps this whole set of lines should be rearranged, something like: # Enable these warnings if the compiler supports them: warn_flags="-Wold-style-declaration -Wold-style-definition -Wtype-limits" warn_flags="-Wformat-security -Wformat-y2k -Winit-self -Wignored-qualifiers $warn_flags" warn_flags="-Wno-missing-include-dirs -Wempty-body -Wnested-externs $warn_flags" warn_flags="-Wendif-labels -Wexpansion-to-defined $warn_flags" # Now disable sub-types of warning we don't want but which are # enabled by some of the warning flags we do want; these must come # later in the compiler command line than the enabling warning options. nowarn_flags="-Wno-missing-include-dirs -Wno-shift-negative-value" nowarn_flags="-Wno-initializer-overrides $nowarn_flags" nowarn_flags="-Wno-string-plus-int -Wno-typedef-redefinition $nowarn_flags" warn_flags="$warn_flags $nowarn_flags" (is there a nicer shell idiom for creating a variable that's a long string of stuff without having over-long lines?) It's also tempting to pull the handful of warning related options currently set directly in QEMU_CFLAGS (-Wall, etc) into this same set of variables. thanks -- PMM
[PATCH v7 09/11] accel/Kconfig: Add the TCG selector
Expose the CONFIG_TCG selector to let minikconf.py uses it. When building with --disable-tcg build, this helps to deselect devices that are TCG-dependent. Reviewed-by: Richard Henderson Signed-off-by: Philippe Mathieu-Daudé --- Makefile | 1 + accel/Kconfig | 3 +++ 2 files changed, 4 insertions(+) diff --git a/Makefile b/Makefile index 648757f79a..f8a45e1379 100644 --- a/Makefile +++ b/Makefile @@ -405,6 +405,7 @@ endif MINIKCONF_ARGS = \ $(CONFIG_MINIKCONF_MODE) \ $@ $*/config-devices.mak.d $< $(MINIKCONF_INPUTS) \ +CONFIG_TCG=$(CONFIG_TCG) \ CONFIG_KVM=$(CONFIG_KVM) \ CONFIG_SPICE=$(CONFIG_SPICE) \ CONFIG_IVSHMEM=$(CONFIG_IVSHMEM) \ diff --git a/accel/Kconfig b/accel/Kconfig index c21802bb49..2ad94a3839 100644 --- a/accel/Kconfig +++ b/accel/Kconfig @@ -1,3 +1,6 @@ +config TCG +bool + config KVM bool -- 2.21.3
Re: [PATCH v2] fuzz: add oss-fuzz build.sh script
Hi Alex,
>From looking at another OSS Fuzz project recently (a coincidence) I
wonder if we could make this script work so that it can be run outside
of the OSS-Fuzz environment?
Specifically, for example, if $OUT is not set, then creating a subdir in
the build directory, and setting it to be that.
Similarly for some other things like $LIB_FUZZING_ENGINE?
I'm just thinking that it might help someone that is not familiar with
OSS-Fuzz to validate that the script still works without having to go
through setting up the containers, etc that would be required to
validate it.
Also, I would definitely recommend running ShellCheck against any script
to ensure that you're catching any mistakes that can so easily be put in
to shell scripts - speaking from experience here ;)
Thanks,
Darren.
On Friday, 2020-06-05 at 13:50:28 -04, Alexander Bulekov wrote:
> It is neater to keep this in the QEMU repo, since any change that
> requires an update to the oss-fuzz build configuration, can make the
> necessary changes in the same series.
>
> Suggested-by: Philippe Mathieu-Daude
> Signed-off-by: Alexander Bulekov
> ---
>
> v2 updates the script header comment.
>
> scripts/oss-fuzz/build.sh | 50 +++
> 1 file changed, 50 insertions(+)
> create mode 100755 scripts/oss-fuzz/build.sh
>
> diff --git a/scripts/oss-fuzz/build.sh b/scripts/oss-fuzz/build.sh
> new file mode 100755
> index 00..e93d6f2e03
> --- /dev/null
> +++ b/scripts/oss-fuzz/build.sh
> @@ -0,0 +1,50 @@
> +#!/bin/sh
> +#
> +# OSS-Fuzz build script. See:
> +#
> https://google.github.io/oss-fuzz/getting-started/new-project-guide/#buildsh
> +#
> +# This code is licensed under the GPL version 2 or later. See
> +# the COPYING file in the top-level directory.
> +#
> +
> +# build project
> +# e.g.
> +# ./autogen.sh
> +# ./configure
> +# make -j$(nproc) all
> +
> +# build fuzzers
> +# e.g.
> +# $CXX $CXXFLAGS -std=c++11 -Iinclude \
> +# /path/to/name_of_fuzzer.cc -o $OUT/name_of_fuzzer \
> +# $LIB_FUZZING_ENGINE /path/to/library.a
> +
> +mkdir -p $OUT/lib/ # Shared libraries
> +
> +# Build once to get the list of dynamic lib paths, and copy them over
> +./configure --datadir="./data/" --disable-werror --cc="$CC" --cxx="$CXX" \
> +--extra-cflags="$CFLAGS -U __OPTIMIZE__ "
> +make CONFIG_FUZZ=y CFLAGS="$LIB_FUZZING_ENGINE" -j$(nproc) i386-softmmu/fuzz
> +
> +for i in $(ldd ./i386-softmmu/qemu-fuzz-i386 | cut -f3 -d' '); do
> +cp $i $OUT/lib/
> +done
> +rm ./i386-softmmu/qemu-fuzz-i386
> +
> +# Build a second time to build the final binary with correct rpath
> +./configure --datadir="./data/" --disable-werror --cc="$CC" --cxx="$CXX" \
> +--extra-cflags="$CFLAGS -U __OPTIMIZE__" \
> +--extra-ldflags="-Wl,-rpath,'\$\$ORIGIN/lib'"
> +make CONFIG_FUZZ=y CFLAGS="$LIB_FUZZING_ENGINE" -j$(nproc) i386-softmmu/fuzz
> +
> +# Copy over the datadir
> +cp -r ./pc-bios/ $OUT/pc-bios
> +
> +# Run the fuzzer with no arguments, to print the help-string and get the list
> +# of available fuzz-targets. Copy over the qemu-fuzz-i386, naming it
> according
> +# to each available fuzz target (See 05509c8e6d fuzz: select fuzz target
> using
> +# executable name)
> +for target in $(./i386-softmmu/qemu-fuzz-i386 | awk '$1 ~ /\*/ {print $2}');
> +do
> +cp ./i386-softmmu/qemu-fuzz-i386 $OUT/qemu-fuzz-i386-target-$target
> +done
> --
> 2.26.2
[PATCH v7 07/11] Makefile: Write MINIKCONF variables as one entry per line
Having one entry per line helps reviews/refactors. As we are going to modify the MINIKCONF variables, split them now to ease further review. Reviewed-by: Richard Henderson Signed-off-by: Philippe Mathieu-Daudé --- Makefile | 9 ++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/Makefile b/Makefile index 6c9d718b2c..7666f81e8a 100644 --- a/Makefile +++ b/Makefile @@ -418,12 +418,15 @@ MINIKCONF_ARGS = \ CONFIG_LINUX=$(CONFIG_LINUX) \ CONFIG_PVRDMA=$(CONFIG_PVRDMA) -MINIKCONF_INPUTS = $(SRC_PATH)/Kconfig.host $(SRC_PATH)/hw/Kconfig -MINIKCONF_DEPS = $(MINIKCONF_INPUTS) $(wildcard $(SRC_PATH)/hw/*/Kconfig) +MINIKCONF_INPUTS = $(SRC_PATH)/Kconfig.host \ + $(SRC_PATH)/hw/Kconfig +MINIKCONF_DEPS = $(MINIKCONF_INPUTS) \ + $(wildcard $(SRC_PATH)/hw/*/Kconfig) MINIKCONF = $(PYTHON) $(SRC_PATH)/scripts/minikconf.py $(SUBDIR_DEVICES_MAK): %/config-devices.mak: default-configs/%.mak $(MINIKCONF_DEPS) $(BUILD_DIR)/config-host.mak - $(call quiet-command, $(MINIKCONF) $(MINIKCONF_ARGS) > $@.tmp, "GEN", "$@.tmp") + $(call quiet-command, $(MINIKCONF) $(MINIKCONF_ARGS) \ + > $@.tmp, "GEN", "$@.tmp") $(call quiet-command, if test -f $@; then \ if cmp -s $@.old $@; then \ mv $@.tmp $@; \ -- 2.21.3
[PATCH v7 08/11] accel/Kconfig: Extract accel selectors into their own config
Move the accel selectors from the global Kconfig.host to their own Kconfig file. Reviewed-by: Richard Henderson Signed-off-by: Philippe Mathieu-Daudé --- Makefile | 1 + Kconfig.host | 7 --- accel/Kconfig | 6 ++ 3 files changed, 7 insertions(+), 7 deletions(-) create mode 100644 accel/Kconfig diff --git a/Makefile b/Makefile index 7666f81e8a..648757f79a 100644 --- a/Makefile +++ b/Makefile @@ -419,6 +419,7 @@ MINIKCONF_ARGS = \ CONFIG_PVRDMA=$(CONFIG_PVRDMA) MINIKCONF_INPUTS = $(SRC_PATH)/Kconfig.host \ + $(SRC_PATH)/accel/Kconfig \ $(SRC_PATH)/hw/Kconfig MINIKCONF_DEPS = $(MINIKCONF_INPUTS) \ $(wildcard $(SRC_PATH)/hw/*/Kconfig) diff --git a/Kconfig.host b/Kconfig.host index 55136e037d..a6d871c399 100644 --- a/Kconfig.host +++ b/Kconfig.host @@ -2,9 +2,6 @@ # down to Kconfig. See also MINIKCONF_ARGS in the Makefile: # these two need to be kept in sync. -config KVM -bool - config LINUX bool @@ -31,10 +28,6 @@ config VHOST_KERNEL bool select VHOST -config XEN -bool -select FSDEV_9P if VIRTFS - config VIRTFS bool diff --git a/accel/Kconfig b/accel/Kconfig new file mode 100644 index 00..c21802bb49 --- /dev/null +++ b/accel/Kconfig @@ -0,0 +1,6 @@ +config KVM +bool + +config XEN +bool +select FSDEV_9P if VIRTFS -- 2.21.3
[PATCH v7 11/11] accel/tcg: Add stub for probe_access()
From: Philippe Mathieu-Daudé
The TCG helpers where added in b92e5a22ec3 in softmmu_template.h.
probe_write() was added in there in 3b4afc9e75a to be moved out
to accel/tcg/cputlb.c in 3b08f0a9254, and was later refactored
as probe_access() in c25c283df0f.
Since it is a TCG specific helper, add a stub to avoid failures
when building without TCG, such:
target/arm/helper.o: In function `probe_read':
include/exec/exec-all.h:362: undefined reference to `probe_access'
Reviewed-by: Richard Henderson
Reviewed-by: David Hildenbrand
Signed-off-by: Philippe Mathieu-Daudé
---
accel/stubs/tcg-stub.c | 7 +++
1 file changed, 7 insertions(+)
diff --git a/accel/stubs/tcg-stub.c b/accel/stubs/tcg-stub.c
index 677191a69c..e4bbf997aa 100644
--- a/accel/stubs/tcg-stub.c
+++ b/accel/stubs/tcg-stub.c
@@ -22,3 +22,10 @@ void tb_flush(CPUState *cpu)
void tlb_set_dirty(CPUState *cpu, target_ulong vaddr)
{
}
+
+void *probe_access(CPUArchState *env, target_ulong addr, int size,
+ MMUAccessType access_type, int mmu_idx, uintptr_t retaddr)
+{
+ /* Handled by hardware accelerator. */
+ g_assert_not_reached();
+}
--
2.21.3
[PATCH v7 03/11] MAINTAINERS: Add an entry for the HAX accelerator
Nobody replied since the first time [*] that patch was posted, so mark HAX as orphan. [*] https://mid.mail-archive.com/20200316120049.11225-4-philmd@redhat.com Cc: haxm-t...@intel.com Cc: Tao Wu Cc: Colin Xu Cc: Wenchao Wang Cc: Vincent Palatin Cc: Sergio Andres Gomez Del Real Signed-off-by: Philippe Mathieu-Daudé --- MAINTAINERS | 6 ++ 1 file changed, 6 insertions(+) diff --git a/MAINTAINERS b/MAINTAINERS index f725c12161..05d7210204 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -426,6 +426,12 @@ F: accel/accel.c F: accel/Makefile.objs F: accel/stubs/Makefile.objs +HAX Accelerator +S: Orphan +F: accel/stubs/hax-stub.c +F: target/i386/hax-all.c +F: include/sysemu/hax.h + X86 HVF CPUs M: Roman Bolshakov S: Maintained -- 2.21.3
[PATCH v7 06/11] Makefile: Remove dangerous EOL trailing backslash
One might get caught trying to understand unexpected Makefile behavior. Trailing backslash can help to split very long lines, but are rather dangerous when nothing follow. Preserve other developers debugging time by removing this one. Reviewed-by: Thomas Huth Reviewed-by: Alistair Francis Signed-off-by: Philippe Mathieu-Daudé --- Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Makefile b/Makefile index 40e4f7677b..6c9d718b2c 100644 --- a/Makefile +++ b/Makefile @@ -420,7 +420,7 @@ MINIKCONF_ARGS = \ MINIKCONF_INPUTS = $(SRC_PATH)/Kconfig.host $(SRC_PATH)/hw/Kconfig MINIKCONF_DEPS = $(MINIKCONF_INPUTS) $(wildcard $(SRC_PATH)/hw/*/Kconfig) -MINIKCONF = $(PYTHON) $(SRC_PATH)/scripts/minikconf.py \ +MINIKCONF = $(PYTHON) $(SRC_PATH)/scripts/minikconf.py $(SUBDIR_DEVICES_MAK): %/config-devices.mak: default-configs/%.mak $(MINIKCONF_DEPS) $(BUILD_DIR)/config-host.mak $(call quiet-command, $(MINIKCONF) $(MINIKCONF_ARGS) > $@.tmp, "GEN", "$@.tmp") -- 2.21.3
[PATCH v7 10/11] Makefile: Allow target-specific optional Kconfig
Allow use of target-specific Kconfig file. Reviewed-by: Richard Henderson Signed-off-by: Philippe Mathieu-Daudé --- Makefile | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/Makefile b/Makefile index f8a45e1379..d5009cd304 100644 --- a/Makefile +++ b/Makefile @@ -423,11 +423,13 @@ MINIKCONF_INPUTS = $(SRC_PATH)/Kconfig.host \ $(SRC_PATH)/accel/Kconfig \ $(SRC_PATH)/hw/Kconfig MINIKCONF_DEPS = $(MINIKCONF_INPUTS) \ - $(wildcard $(SRC_PATH)/hw/*/Kconfig) + $(wildcard $(SRC_PATH)/hw/*/Kconfig) \ + $(wildcard $(SRC_PATH)/target/*/Kconfig) MINIKCONF = $(PYTHON) $(SRC_PATH)/scripts/minikconf.py $(SUBDIR_DEVICES_MAK): %/config-devices.mak: default-configs/%.mak $(MINIKCONF_DEPS) $(BUILD_DIR)/config-host.mak $(call quiet-command, $(MINIKCONF) $(MINIKCONF_ARGS) \ + $(wildcard $(SRC_PATH)/target/$(call base-arch, $(firstword $(subst -, ,$@)))/Kconfig) \ > $@.tmp, "GEN", "$@.tmp") $(call quiet-command, if test -f $@; then \ if cmp -s $@.old $@; then \ -- 2.21.3
[PATCH v7 00/11] accel: Allow targets to use Kconfig
Missing review: - 4/11 rules.mak: Add strequal() and startswith() rules - 5/11 rules.mak: Add base-arch() rule This series include generic patches I took of the KVM/ARM specific series which will follow. - List orphan accelerators in MAINTAINERS - Add accel/Kconfig - Allow targets to use their how Kconfig Since v6: - Fixed typo 'startwith' -> 'startswith' (armbru) Since v5: - Fixed typo in patch #4 subject - Added David R-b tag - Stripped --- comments Since v4: - Addressed rth review comments in rules.mak Since v3: - Fixed base-arch() rule (rth) - Dropped 'semihosting: Make the feature depend of TCG' Since v2: - Addressed Thomas review comments - Fixed problem when including TARGET instead of BASE_TARGET Since v1: https://www.mail-archive.com/qemu-devel@nongnu.org/msg689024.html - Drop HVF MAINTAINERS patch (merged elsewhere) - Kconfig-select SEMIHOSTING (bonzini) - Drop user-mode selection patches - consider m68k/nios2/xtensa/riscv (pm215) - reword Kconfig SEMIHOSTING description (pm215) - reset some of rth R-b tags Previous RFC for semihosting posted earlier: https://www.mail-archive.com/qemu-devel@nongnu.org/msg631218.html Philippe Mathieu-Daudé (11): MAINTAINERS: Fix KVM path expansion glob MAINTAINERS: Add an 'overall' entry for accelerators MAINTAINERS: Add an entry for the HAX accelerator rules.mak: Add strequal() and startswith() rules rules.mak: Add base-arch() rule Makefile: Remove dangerous EOL trailing backslash Makefile: Write MINIKCONF variables as one entry per line accel/Kconfig: Extract accel selectors into their own config accel/Kconfig: Add the TCG selector Makefile: Allow target-specific optional Kconfig accel/tcg: Add stub for probe_access() Makefile | 15 + rules.mak | 49 ++ accel/stubs/tcg-stub.c | 7 ++ Kconfig.host | 7 -- MAINTAINERS| 19 +++- accel/Kconfig | 9 6 files changed, 94 insertions(+), 12 deletions(-) create mode 100644 accel/Kconfig -- 2.21.3
[PATCH v7 04/11] rules.mak: Add strequal() and startswith() rules
Add a rule to test if two strings are equal, and another to test if a string starts with a substring. Signed-off-by: Philippe Mathieu-Daudé --- rules.mak | 14 ++ 1 file changed, 14 insertions(+) diff --git a/rules.mak b/rules.mak index 694865b63e..7b58a6b8c5 100644 --- a/rules.mak +++ b/rules.mak @@ -191,6 +191,20 @@ ne = $(if $(subst $2,,$1)$(subst $1,,$2),y,n) isempty = $(if $1,n,y) notempty = $(if $1,y,n) +# strequal +# Usage: $(call strequal, str1, str2) +# +# This macro returns a string (TRUE) when @str1 and @str2 +# are equal, else returns the empty string (FALSE) +strequal = $(if $(subst $2,,$1)$(subst $1,,$2),,$1) + +# startswith +# Usage: $(call startswith, startstr, fullstr) +# +# This macro returns a string (TRUE) when @fullstr starts with +# @startstr, else returns the empty string (FALSE) +startswith = $(findstring :$1,:$2) + # Generate files with tracetool TRACETOOL=$(PYTHON) $(SRC_PATH)/scripts/tracetool.py -- 2.21.3
[PATCH v7 01/11] MAINTAINERS: Fix KVM path expansion glob
The KVM files has been moved from target-ARCH to the target/ARCH/
folder in commit fcf5ef2a. Fix the pathname expansion.
Fixes: fcf5ef2a ("Move target-* CPU file into a target/ folder")
Reviewed-by: Richard Henderson
Reviewed-by: Thomas Huth
Signed-off-by: Philippe Mathieu-Daudé
---
MAINTAINERS | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/MAINTAINERS b/MAINTAINERS
index 3e7d9cb0a5..948790b433 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -361,7 +361,7 @@ Overall KVM CPUs
M: Paolo Bonzini
L: k...@vger.kernel.org
S: Supported
-F: */kvm.*
+F: */*/kvm*
F: accel/kvm/
F: accel/stubs/kvm-stub.c
F: include/hw/kvm/
--
2.21.3
[PATCH v7 05/11] rules.mak: Add base-arch() rule
Add a rule to return the base architecture for a QEMU target. The current list of TARGET_BASE_ARCH is: $ git grep TARGET_BASE_ARCH configure configure:7785:TARGET_BASE_ARCH="" configure:7795:TARGET_BASE_ARCH=i386 configure:7813:TARGET_BASE_ARCH=arm configure:7846:TARGET_BASE_ARCH=mips configure:7854:TARGET_BASE_ARCH=mips configure:7864:TARGET_BASE_ARCH=openrisc configure:7871:TARGET_BASE_ARCH=ppc configure:7879:TARGET_BASE_ARCH=ppc configure:7887:TARGET_BASE_ARCH=ppc configure:7894:TARGET_BASE_ARCH=riscv configure:7900:TARGET_BASE_ARCH=riscv configure:7920:TARGET_BASE_ARCH=sparc configure:7925:TARGET_BASE_ARCH=sparc The rule can be tested calling 'print-base-arch-$TARGET': $ make \ print-base-arch-openrisc \ print-base-arch-aarch64_be \ print-base-arch-x86_64 \ print-base-arch-mips64el \ print-base-arch-ppc64 \ print-base-arch-riscv64 openrisc=openrisc aarch64_be=arm x86_64=i386 mips64el=mips ppc64=ppc riscv64=riscv Signed-off-by: Philippe Mathieu-Daudé --- rules.mak | 35 +++ 1 file changed, 35 insertions(+) diff --git a/rules.mak b/rules.mak index 7b58a6b8c5..c4f2f35754 100644 --- a/rules.mak +++ b/rules.mak @@ -452,3 +452,38 @@ atomic = $(eval $1: $(call sentinel,$1) ; @:) \ print-%: @echo '$*=$($*)' + +# base-arch +# Usage: $(call base-arch, target) +# +# @target: the target architecture. +# +# This macro will return the base architecture for a target. +# +# As example, $(call base-arch, aarch64) returns 'arm'. +base-arch = $(strip \ + $(if $(call startswith,aarch64,$1),arm,\ + $(if $(call startswith,arm,$1),arm,\ + $(if $(call startswith,microblaze,$1),microblaze,\ + $(if $(call startswith,mips,$1),mips,\ + $(if $(call startswith,ppc,$1),ppc,\ + $(if $(call startswith,riscv,$1),riscv,\ + $(if $(call startswith,sh4,$1),sh4,\ + $(if $(call startswith,sparc,$1),sparc,\ + $(if $(call startswith,xtensa,$1),xtensa,\ + $(if $(call strequal,x86_64,$1),i386,\ + $1\ + )\ +)\ + )\ +)\ + )\ +)\ + )\ +)\ + )\ +)\ + ) + +print-base-arch-%: + @echo '$*=$(call base-arch,$*)' -- 2.21.3
[PATCH v7 02/11] MAINTAINERS: Add an 'overall' entry for accelerators
Reviewed-by: Richard Henderson Signed-off-by: Philippe Mathieu-Daudé --- MAINTAINERS | 11 +++ 1 file changed, 11 insertions(+) diff --git a/MAINTAINERS b/MAINTAINERS index 948790b433..f725c12161 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -415,6 +415,17 @@ S: Supported F: target/i386/kvm.c F: scripts/kvm/vmxcap +Guest CPU Cores (other accelerators) + +Overall +M: Richard Henderson +R: Paolo Bonzini +S: Maintained +F: include/sysemu/accel.h +F: accel/accel.c +F: accel/Makefile.objs +F: accel/stubs/Makefile.objs + X86 HVF CPUs M: Roman Bolshakov S: Maintained -- 2.21.3
Re: [PATCH] fuzz: add oss-fuzz build.sh script
On 6/5/20 7:40 PM, Alexander Bulekov wrote:
> It is neater to keep this in the QEMU repo, since any change that
> requires an update to the oss-fuzz build configuration, can make the
> necessary changes in the same series.
>
> Suggested-by: Philippe Mathieu-Daude
'Philippe Mathieu-Daudé' ;)
> Signed-off-by: Alexander Bulekov
> ---
> scripts/oss-fuzz/build.sh | 47 +++
> 1 file changed, 47 insertions(+)
> create mode 100755 scripts/oss-fuzz/build.sh
>
> diff --git a/scripts/oss-fuzz/build.sh b/scripts/oss-fuzz/build.sh
> new file mode 100755
> index 00..7be6dcce4c
> --- /dev/null
> +++ b/scripts/oss-fuzz/build.sh
> @@ -0,0 +1,47 @@
> +#!/bin/sh
> +#
> +# Update syscall_nr.h files from linux headers asm-generic/unistd.h
Hmmm?
> +#
> +# This code is licensed under the GPL version 2 or later. See
> +# the COPYING file in the top-level directory.
> +#
> +
> +# build project
Please mention this file use consumed by
https://github.com/google/oss-fuzz/.../projects/qemu/Dockerfile
> +# e.g.
> +# ./autogen.sh
> +# ./configure
> +# make -j$(nproc) all
> +
> +# build fuzzers
> +# e.g.
> +# $CXX $CXXFLAGS -std=c++11 -Iinclude \
> +# /path/to/name_of_fuzzer.cc -o $OUT/name_of_fuzzer \
> +# $LIB_FUZZING_ENGINE /path/to/library.a
> +
> +mkdir -p $OUT/lib/ # Shared libraries
Maybe rename OUT -> DEST_DIR?
> +
> +# Build once to get the list of dynamic lib paths, and copy them over
> +./configure --datadir="./data/" --disable-werror --cc="$CC" --cxx="$CXX" \
> +--extra-cflags="$CFLAGS -U __OPTIMIZE__ "
So we use an in-tree build.
Still we could set some SRCDIR=./
> +make CONFIG_FUZZ=y CFLAGS="$LIB_FUZZING_ENGINE" -j$(nproc) i386-softmmu/fuzz
> +
> +for i in $(ldd ./i386-softmmu/qemu-fuzz-i386 | cut -f3 -d' '); do
> +cp $i $OUT/lib/
> +done
> +rm ./i386-softmmu/qemu-fuzz-i386
> +
> +# Build a second time to build the final binary with correct rpath
> +./configure --datadir="./data/" --disable-werror --cc="$CC" --cxx="$CXX" \
> +--extra-cflags="$CFLAGS -U __OPTIMIZE__" \
> +--extra-ldflags="-Wl,-rpath,'\$\$ORIGIN/lib'"
> +make CONFIG_FUZZ=y CFLAGS="$LIB_FUZZING_ENGINE" -j$(nproc) i386-softmmu/fuzz
> +
> +# Copy over the datadir
> +cp -r ./pc-bios/ $OUT/pc-bios
"make install-datadir"?
> +
> +# Copy over the qemu-fuzz-i386, naming it according to each available fuzz
> +# target (See 05509c8e6d fuzz: select fuzz target using executable name)
> +for target in $(./i386-softmmu/qemu-fuzz-i386 | awk '$1 ~ /\*/ {print $2}');
> +do
> +cp ./i386-softmmu/qemu-fuzz-i386 $OUT/qemu-fuzz-i386-target-$target
There seems to be an extra 'target'.
> +done
>
Or "make install", not sure.
Re: [PATCH v2 04/17] target/riscv: Implement checks for hfence
On 6/4/20 6:20 PM, Alistair Francis wrote: > Call the helper_hyp_tlb_flush() function on hfence instructions which > will generate an illegal insruction execption if we don't have > permission to flush the Hypervisor level TLBs. > > Signed-off-by: Alistair Francis > --- > target/riscv/helper.h | 5 > target/riscv/insn_trans/trans_rvh.inc.c | 32 + > target/riscv/op_helper.c| 13 ++ > 3 files changed, 24 insertions(+), 26 deletions(-) Reviewed-by: Richard Henderson r~
Re: [PATCH v2 03/17] target/riscv: Move the hfence instructions to the rvh decode
On 6/4/20 6:20 PM, Alistair Francis wrote: > Also correct the name of the VVMA instruction. > > Signed-off-by: Alistair Francis > --- > target/riscv/insn32.decode| 8 ++- > .../riscv/insn_trans/trans_privileged.inc.c | 38 - > target/riscv/insn_trans/trans_rvh.inc.c | 57 +++ > target/riscv/translate.c | 1 + > 4 files changed, 63 insertions(+), 41 deletions(-) > create mode 100644 target/riscv/insn_trans/trans_rvh.inc.c Reviewed-by: Richard Henderson r~
Re: [PATCH v2 02/17] target/riscv: Report errors validating 2nd-stage PTEs
On 6/4/20 6:20 PM, Alistair Francis wrote: > Signed-off-by: Alistair Francis > --- > target/riscv/cpu_helper.c | 9 +++-- > 1 file changed, 7 insertions(+), 2 deletions(-) Reviewed-by: Richard Henderson r~
Re: [PATCH v2 01/17] target/riscv: Set access as data_load when validating stage-2 PTEs
On 6/4/20 6:20 PM, Alistair Francis wrote: > Signed-off-by: Alistair Francis > --- > target/riscv/cpu_helper.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) Reviewed-by: Richard Henderson r~
[PATCH v2] fuzz: add oss-fuzz build.sh script
It is neater to keep this in the QEMU repo, since any change that
requires an update to the oss-fuzz build configuration, can make the
necessary changes in the same series.
Suggested-by: Philippe Mathieu-Daude
Signed-off-by: Alexander Bulekov
---
v2 updates the script header comment.
scripts/oss-fuzz/build.sh | 50 +++
1 file changed, 50 insertions(+)
create mode 100755 scripts/oss-fuzz/build.sh
diff --git a/scripts/oss-fuzz/build.sh b/scripts/oss-fuzz/build.sh
new file mode 100755
index 00..e93d6f2e03
--- /dev/null
+++ b/scripts/oss-fuzz/build.sh
@@ -0,0 +1,50 @@
+#!/bin/sh
+#
+# OSS-Fuzz build script. See:
+# https://google.github.io/oss-fuzz/getting-started/new-project-guide/#buildsh
+#
+# This code is licensed under the GPL version 2 or later. See
+# the COPYING file in the top-level directory.
+#
+
+# build project
+# e.g.
+# ./autogen.sh
+# ./configure
+# make -j$(nproc) all
+
+# build fuzzers
+# e.g.
+# $CXX $CXXFLAGS -std=c++11 -Iinclude \
+# /path/to/name_of_fuzzer.cc -o $OUT/name_of_fuzzer \
+# $LIB_FUZZING_ENGINE /path/to/library.a
+
+mkdir -p $OUT/lib/ # Shared libraries
+
+# Build once to get the list of dynamic lib paths, and copy them over
+./configure --datadir="./data/" --disable-werror --cc="$CC" --cxx="$CXX" \
+--extra-cflags="$CFLAGS -U __OPTIMIZE__ "
+make CONFIG_FUZZ=y CFLAGS="$LIB_FUZZING_ENGINE" -j$(nproc) i386-softmmu/fuzz
+
+for i in $(ldd ./i386-softmmu/qemu-fuzz-i386 | cut -f3 -d' '); do
+cp $i $OUT/lib/
+done
+rm ./i386-softmmu/qemu-fuzz-i386
+
+# Build a second time to build the final binary with correct rpath
+./configure --datadir="./data/" --disable-werror --cc="$CC" --cxx="$CXX" \
+--extra-cflags="$CFLAGS -U __OPTIMIZE__" \
+--extra-ldflags="-Wl,-rpath,'\$\$ORIGIN/lib'"
+make CONFIG_FUZZ=y CFLAGS="$LIB_FUZZING_ENGINE" -j$(nproc) i386-softmmu/fuzz
+
+# Copy over the datadir
+cp -r ./pc-bios/ $OUT/pc-bios
+
+# Run the fuzzer with no arguments, to print the help-string and get the list
+# of available fuzz-targets. Copy over the qemu-fuzz-i386, naming it according
+# to each available fuzz target (See 05509c8e6d fuzz: select fuzz target using
+# executable name)
+for target in $(./i386-softmmu/qemu-fuzz-i386 | awk '$1 ~ /\*/ {print $2}');
+do
+cp ./i386-softmmu/qemu-fuzz-i386 $OUT/qemu-fuzz-i386-target-$target
+done
--
2.26.2
Re: [PATCH] fuzz: add oss-fuzz build.sh script
On 200605 1340, Alexander Bulekov wrote:
> It is neater to keep this in the QEMU repo, since any change that
> requires an update to the oss-fuzz build configuration, can make the
> necessary changes in the same series.
>
> Suggested-by: Philippe Mathieu-Daude
> Signed-off-by: Alexander Bulekov
> ---
> scripts/oss-fuzz/build.sh | 47 +++
> 1 file changed, 47 insertions(+)
> create mode 100755 scripts/oss-fuzz/build.sh
>
> diff --git a/scripts/oss-fuzz/build.sh b/scripts/oss-fuzz/build.sh
> new file mode 100755
> index 00..7be6dcce4c
> --- /dev/null
> +++ b/scripts/oss-fuzz/build.sh
> @@ -0,0 +1,47 @@
> +#!/bin/sh
> +#
> +# Update syscall_nr.h files from linux headers asm-generic/unistd.h
This is obviously wrong... Sending v2.
> +#
> +# This code is licensed under the GPL version 2 or later. See
> +# the COPYING file in the top-level directory.
> +#
> +
> +# build project
> +# e.g.
> +# ./autogen.sh
> +# ./configure
> +# make -j$(nproc) all
> +
> +# build fuzzers
> +# e.g.
> +# $CXX $CXXFLAGS -std=c++11 -Iinclude \
> +# /path/to/name_of_fuzzer.cc -o $OUT/name_of_fuzzer \
> +# $LIB_FUZZING_ENGINE /path/to/library.a
> +
> +mkdir -p $OUT/lib/ # Shared libraries
> +
> +# Build once to get the list of dynamic lib paths, and copy them over
> +./configure --datadir="./data/" --disable-werror --cc="$CC" --cxx="$CXX" \
> +--extra-cflags="$CFLAGS -U __OPTIMIZE__ "
> +make CONFIG_FUZZ=y CFLAGS="$LIB_FUZZING_ENGINE" -j$(nproc) i386-softmmu/fuzz
> +
> +for i in $(ldd ./i386-softmmu/qemu-fuzz-i386 | cut -f3 -d' '); do
> +cp $i $OUT/lib/
> +done
> +rm ./i386-softmmu/qemu-fuzz-i386
> +
> +# Build a second time to build the final binary with correct rpath
> +./configure --datadir="./data/" --disable-werror --cc="$CC" --cxx="$CXX" \
> +--extra-cflags="$CFLAGS -U __OPTIMIZE__" \
> +--extra-ldflags="-Wl,-rpath,'\$\$ORIGIN/lib'"
> +make CONFIG_FUZZ=y CFLAGS="$LIB_FUZZING_ENGINE" -j$(nproc) i386-softmmu/fuzz
> +
> +# Copy over the datadir
> +cp -r ./pc-bios/ $OUT/pc-bios
> +
> +# Copy over the qemu-fuzz-i386, naming it according to each available fuzz
> +# target (See 05509c8e6d fuzz: select fuzz target using executable name)
> +for target in $(./i386-softmmu/qemu-fuzz-i386 | awk '$1 ~ /\*/ {print $2}');
> +do
> +cp ./i386-softmmu/qemu-fuzz-i386 $OUT/qemu-fuzz-i386-target-$target
> +done
> --
> 2.26.2
>
Re: [PULL v2 05/13] accel/tcg: Relax va restrictions on 64-bit guests
Richard Henderson writes:
> On 6/5/20 7:11 AM, Alex Bennée wrote:
>> @@ -467,7 +467,7 @@ abi_long target_mmap(abi_ulong start, abi_ulong len, int
>> prot,
>> * It can fail only on 64-bit host with 32-bit target.
>> * On any other target/host host mmap() handles this error
>> correctly.
>> */
>> -if (!guest_range_valid(start, len)) {
>> +if (end < start || !guest_range_valid(start, len)) {
>> errno = ENOMEM;
>> goto fail;
>> }
>
> Interesting. I was adjusting guest_range_valid tagged pointers yesterday, and
> thought that it looked buggy.
Should be picking this up in guest_range_valid?
--
Alex Bennée
Re: [PATCH v1 00/14] various fixes for next PR (testing, vhost, guest_base fixes)
Patchew URL: https://patchew.org/QEMU/20200605154929.26910-1-alex.ben...@linaro.org/ Hi, This series seems to have some coding style problems. See output below for more information: Message-id: 20200605154929.26910-1-alex.ben...@linaro.org Subject: [PATCH v1 00/14] various fixes for next PR (testing, vhost, guest_base fixes) Type: series === TEST SCRIPT BEGIN === #!/bin/bash git rev-parse base > /dev/null || exit 0 git config --local diff.renamelimit 0 git config --local diff.renames True git config --local diff.algorithm histogram ./scripts/checkpatch.pl --mailback base.. === TEST SCRIPT END === Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384 From https://github.com/patchew-project/qemu * [new tag] patchew/20200605165007.12095-1-peter.mayd...@linaro.org -> patchew/20200605165007.12095-1-peter.mayd...@linaro.org * [new tag] patchew/20200605165656.17578-1-phi...@redhat.com -> patchew/20200605165656.17578-1-phi...@redhat.com Auto packing the repository for optimum performance. You may also run "git gc" manually. See "git help gc" for more information. Switched to a new branch 'test' 8c192c8 linux-user: detect overflow of MAP_FIXED mmap 343b114 tests/tcg: add simple commpage test case 3ea9b6b linux-user: deal with address wrap for ARM_COMMPAGE on 32 bit bd10df6 linux-user: provide fallback pgd_find_hole for bare chroots 140a4b0 hw/virtio/vhost: re-factor vhost-section and allow DIRTY_MEMORY_CODE c3fdeb9 docker: update Ubuntu to 20.04 3827a17 tests/docker: fix pre-requisite for debian-tricore-cross 00ba0df iotests: 194: wait migration completion on target too 8252e14 .shippable: temporaily disable some cross builds 1027a95 .travis.yml: allow failure for unreliable hosts d545513 exec: flush the whole TLB if a watchpoint crosses a page boundary c286730 tests/plugin: correctly honour io_count 573745f scripts/clean-includes: Mark 'qemu/qemu-plugin.h' as special header 592de76 qemu-plugin.h: add missing include to define size_t === OUTPUT BEGIN === 1/14 Checking commit 592de76606a7 (qemu-plugin.h: add missing include to define size_t) 2/14 Checking commit 573745fa77cd (scripts/clean-includes: Mark 'qemu/qemu-plugin.h' as special header) 3/14 Checking commit c286730e2e6a (tests/plugin: correctly honour io_count) 4/14 Checking commit d545513f07b5 (exec: flush the whole TLB if a watchpoint crosses a page boundary) 5/14 Checking commit 1027a95a2300 (.travis.yml: allow failure for unreliable hosts) 6/14 Checking commit 8252e1457d91 (.shippable: temporaily disable some cross builds) 7/14 Checking commit 00ba0dfa3911 (iotests: 194: wait migration completion on target too) 8/14 Checking commit 3827a17199bf (tests/docker: fix pre-requisite for debian-tricore-cross) 9/14 Checking commit c3fdeb9b0a44 (docker: update Ubuntu to 20.04) 10/14 Checking commit 140a4b0c679f (hw/virtio/vhost: re-factor vhost-section and allow DIRTY_MEMORY_CODE) 11/14 Checking commit bd10df6e7aff (linux-user: provide fallback pgd_find_hole for bare chroots) WARNING: line over 80 characters #46: FILE: linux-user/elfload.c:2116: +static uintptr_t pgd_find_hole_fallback(uintptr_t guest_size, uintptr_t brk, long align) total: 0 errors, 1 warnings, 60 lines checked Patch 11/14 has style problems, please review. If any of these errors are false positives report them to the maintainer, see CHECKPATCH in MAINTAINERS. 12/14 Checking commit 3ea9b6bac8e8 (linux-user: deal with address wrap for ARM_COMMPAGE on 32 bit) 13/14 Checking commit 343b11418044 (tests/tcg: add simple commpage test case) WARNING: added, moved or deleted file(s), does MAINTAINERS need updating? #30: new file mode 100644 ERROR: Use of volatile is usually wrong, please add a comment #59: FILE: tests/tcg/arm/commpage.c:25: +typedef int (cmpxchg_fn)(int oldval, int newval, volatile int *ptr); ERROR: Use of volatile is usually wrong, please add a comment #65: FILE: tests/tcg/arm/commpage.c:31: + volatile int64_t *ptr); total: 2 errors, 1 warnings, 69 lines checked Patch 13/14 has style problems, please review. If any of these errors are false positives report them to the maintainer, see CHECKPATCH in MAINTAINERS. 14/14 Checking commit 8c192c8751c3 (linux-user: detect overflow of MAP_FIXED mmap) === OUTPUT END === Test command exited with code: 1 The full log is available at http://patchew.org/logs/20200605154929.26910-1-alex.ben...@linaro.org/testing.checkpatch/?type=message. --- Email generated automatically by Patchew [https://patchew.org/]. Please send your feedback to patchew-de...@redhat.com
Re: [PATCH] configure: Disable -Wtautological-type-limit-compare
On 6/5/20 8:53 AM, Alex Bennée wrote: > > Richard Henderson writes: > >> Clang 10 enables this by default with -Wtype-limit. >> >> All of the instances flagged by this Werror so far have been >> cases in which we really do want the compiler to optimize away >> the test completely. Disabling the warning will avoid having >> to add ifdefs to work around this. >> >> Signed-off-by: Richard Henderson >> --- >> configure | 2 ++ >> 1 file changed, 2 insertions(+) >> >> diff --git a/configure b/configure >> index f087d2bcd1..693f01327f 100755 >> --- a/configure >> +++ b/configure >> @@ -2009,6 +2009,8 @@ gcc_flags="-Wno-missing-include-dirs -Wempty-body >> -Wnested-externs $gcc_flags" >> gcc_flags="-Wendif-labels -Wno-shift-negative-value $gcc_flags" >> gcc_flags="-Wno-initializer-overrides -Wexpansion-to-defined $gcc_flags" >> gcc_flags="-Wno-string-plus-int -Wno-typedef-redefinition $gcc_flags" >> +gcc_flags="$gcc_flags -Wno-tautological-type-limit-compare" >> + > > nit: the pattern is reversed compared to the previous lines (foo $gcc_flags) Not a nit. The -Wno- must follow -Wtype-limit, or -Wtype-limit will turn it back on. r~ > > I had exactly the same patch in my local tree but it wasn't enough for > clang-9 which I was using for a sanitiser build. I ended up > having to apply --disable-werrror to the configuration. > > Anyway: > > Reviewed-by: Alex Bennée >
[PATCH v2 09/13] tests/docker: Added docker build support for TSan.
Added a new docker for ubuntu 20.04.
This docker has support for Thread Sanitizer
including one patch we need in one of the header files.
https://github.com/llvm/llvm-project/commit/a72dc86cd
This command will build with tsan enabled:
make docker-test-tsan-ubuntu2004
Also added the TSAN suppresion file to disable certain
cases of TSAN warnings.
Cc: Fam Zheng
Cc: Philippe Mathieu-Daudé
Signed-off-by: Robert Foley
---
tests/docker/dockerfiles/ubuntu2004.docker | 65 ++
tests/docker/test-tsan | 44 +++
tests/tsan/blacklist.tsan | 10
tests/tsan/suppressions.tsan | 14 +
4 files changed, 133 insertions(+)
create mode 100644 tests/docker/dockerfiles/ubuntu2004.docker
create mode 100755 tests/docker/test-tsan
create mode 100644 tests/tsan/blacklist.tsan
create mode 100644 tests/tsan/suppressions.tsan
diff --git a/tests/docker/dockerfiles/ubuntu2004.docker
b/tests/docker/dockerfiles/ubuntu2004.docker
new file mode 100644
index 00..6050ce7e8a
--- /dev/null
+++ b/tests/docker/dockerfiles/ubuntu2004.docker
@@ -0,0 +1,65 @@
+FROM ubuntu:20.04
+ENV PACKAGES flex bison \
+ccache \
+clang-10\
+gcc \
+gettext \
+git \
+glusterfs-common \
+libaio-dev \
+libattr1-dev \
+libbrlapi-dev \
+libbz2-dev \
+libcacard-dev \
+libcap-ng-dev \
+libcurl4-gnutls-dev \
+libdrm-dev \
+libepoxy-dev \
+libfdt-dev \
+libgbm-dev \
+libgtk-3-dev \
+libibverbs-dev \
+libiscsi-dev \
+libjemalloc-dev \
+libjpeg-turbo8-dev \
+liblzo2-dev \
+libncurses5-dev \
+libncursesw5-dev \
+libnfs-dev \
+libnss3-dev \
+libnuma-dev \
+libpixman-1-dev \
+librados-dev \
+librbd-dev \
+librdmacm-dev \
+libsasl2-dev \
+libsdl2-dev \
+libseccomp-dev \
+libsnappy-dev \
+libspice-protocol-dev \
+libspice-server-dev \
+libssh-dev \
+libusb-1.0-0-dev \
+libusbredirhost-dev \
+libvdeplug-dev \
+libvte-2.91-dev \
+libxen-dev \
+libzstd-dev \
+make \
+python3-yaml \
+python3-sphinx \
+sparse \
+texinfo \
+xfslibs-dev\
+vim
+RUN apt-get update && \
+DEBIAN_FRONTEND=noninteractive apt-get -y install $PACKAGES
+RUN dpkg -l $PACKAGES | sort > /packages.txt
+ENV FEATURES clang tsan pyyaml sdl2
+
+# https://bugs.launchpad.net/qemu/+bug/1838763
+ENV QEMU_CONFIGURE_OPTS --disable-libssh
+
+# Apply patch https://reviews.llvm.org/D75820
+# This is required for TSan in clang-10 to compile with QEMU.
+RUN sed -i 's/^const/static const/g'
/usr/lib/llvm-10/lib/clang/10.0.0/include/sanitizer/tsan_interface.h
diff --git a/tests/docker/test-tsan b/tests/docker/test-tsan
new file mode 100755
index 00..eb40ac45b7
--- /dev/null
+++ b/tests/docker/test-tsan
@@ -0,0 +1,44 @@
+#!/bin/bash -e
+#
+# This test will use TSan as part of a build and a make check.
+#
+# Copyright (c) 2020 Linaro
+# Copyright (c) 2016 Red Hat Inc.
+#
+# Authors:
+# Robert Foley
+# Originally based on test-quick from Fam Zheng
+#
+# This work is licensed under the terms of the GNU GPL, version 2
+# or (at your option) any later version. See the COPYING file in
+# the top-level directory.
+
+. common.rc
+
+setup_tsan()
+{
+requires clang tsan
+tsan_log_dir="/tmp/qemu-test/build/tsan"
+mkdir -p $tsan_log_dir > /dev/null || true
+EXTRA_CONFIGURE_OPTS="${EXTRA_CONFIGURE_OPTS} --enable-tsan \
+ --cc=clang-10 --cxx=clang++-10 \
+ --disable-werror --extra-cflags=-O0"
+# detect deadlocks is false currently simply because
+# TSan crashes immediately with deadlock detector enabled.
+# We have maxed out the history size to get the best chance of finding
+# warnings during testing.
+# Note, to get TSan to fail on warning, use exitcode=66 below.
+tsan_opts="suppressions=/tmp/qemu-test/src/tests/tsan/suppressions.tsan\
+ detect_deadlocks=false history_size=7\
+ halt_on_error=0 exitcode=0 verbose=5\
+ log_path=$tsan_log_dir/tsan_warning"
+export TSAN_OPTIONS="$tsan_opts"
+}
+
+cd "$BUILD_DIR"
+
+TARGET_LIST=${TARGET_LIST:-$DEF_TARGET_LIST} \
+setup_tsan
+build_qemu
+check_qemu
+install_qemu
diff --git a/tests/tsan/blacklist.tsan b/tests/tsan/blacklist.tsan
new file mode 100644
index 00..75e444f5dc
--- /dev/null
+++ b/tests/tsan/blacklist.tsan
@@ -0,0 +1,10 @@
+# This is an example blacklist.
+# To enable use of the blacklist add this to configure:
+# "--extra-cflags=-fsanitize-blacklist=/tests/tsan/blacklist.tsan"
+# The eventual goal would be to fix these warnings.
+
+# TSan is not happy about setting/getting of dirty bits,
+# for example, cpu_physical_memory_set_dirty_range,
+# and cpu_physical_memory_get_dirty.
+src:bitops.c
+src:bitmap.c
diff --git a/tests/tsan/suppressions.tsan b/tests/tsan/suppressions.tsan
new file mode 100644
index
[PATCH] fuzz: add oss-fuzz build.sh script
It is neater to keep this in the QEMU repo, since any change that
requires an update to the oss-fuzz build configuration, can make the
necessary changes in the same series.
Suggested-by: Philippe Mathieu-Daude
Signed-off-by: Alexander Bulekov
---
scripts/oss-fuzz/build.sh | 47 +++
1 file changed, 47 insertions(+)
create mode 100755 scripts/oss-fuzz/build.sh
diff --git a/scripts/oss-fuzz/build.sh b/scripts/oss-fuzz/build.sh
new file mode 100755
index 00..7be6dcce4c
--- /dev/null
+++ b/scripts/oss-fuzz/build.sh
@@ -0,0 +1,47 @@
+#!/bin/sh
+#
+# Update syscall_nr.h files from linux headers asm-generic/unistd.h
+#
+# This code is licensed under the GPL version 2 or later. See
+# the COPYING file in the top-level directory.
+#
+
+# build project
+# e.g.
+# ./autogen.sh
+# ./configure
+# make -j$(nproc) all
+
+# build fuzzers
+# e.g.
+# $CXX $CXXFLAGS -std=c++11 -Iinclude \
+# /path/to/name_of_fuzzer.cc -o $OUT/name_of_fuzzer \
+# $LIB_FUZZING_ENGINE /path/to/library.a
+
+mkdir -p $OUT/lib/ # Shared libraries
+
+# Build once to get the list of dynamic lib paths, and copy them over
+./configure --datadir="./data/" --disable-werror --cc="$CC" --cxx="$CXX" \
+--extra-cflags="$CFLAGS -U __OPTIMIZE__ "
+make CONFIG_FUZZ=y CFLAGS="$LIB_FUZZING_ENGINE" -j$(nproc) i386-softmmu/fuzz
+
+for i in $(ldd ./i386-softmmu/qemu-fuzz-i386 | cut -f3 -d' '); do
+cp $i $OUT/lib/
+done
+rm ./i386-softmmu/qemu-fuzz-i386
+
+# Build a second time to build the final binary with correct rpath
+./configure --datadir="./data/" --disable-werror --cc="$CC" --cxx="$CXX" \
+--extra-cflags="$CFLAGS -U __OPTIMIZE__" \
+--extra-ldflags="-Wl,-rpath,'\$\$ORIGIN/lib'"
+make CONFIG_FUZZ=y CFLAGS="$LIB_FUZZING_ENGINE" -j$(nproc) i386-softmmu/fuzz
+
+# Copy over the datadir
+cp -r ./pc-bios/ $OUT/pc-bios
+
+# Copy over the qemu-fuzz-i386, naming it according to each available fuzz
+# target (See 05509c8e6d fuzz: select fuzz target using executable name)
+for target in $(./i386-softmmu/qemu-fuzz-i386 | awk '$1 ~ /\*/ {print $2}');
+do
+cp ./i386-softmmu/qemu-fuzz-i386 $OUT/qemu-fuzz-i386-target-$target
+done
--
2.26.2
[PATCH v2 08/13] thread: add tsan annotations to QemuSpin
From: "Emilio G. Cota"
Signed-off-by: Emilio G. Cota
Signed-off-by: Robert Foley
Reviewed-by: Alex Bennée
---
include/qemu/thread.h | 39 ---
1 file changed, 36 insertions(+), 3 deletions(-)
diff --git a/include/qemu/thread.h b/include/qemu/thread.h
index e50a073889..43fc094b96 100644
--- a/include/qemu/thread.h
+++ b/include/qemu/thread.h
@@ -206,6 +206,10 @@ void qemu_thread_atexit_add(struct Notifier *notifier);
*/
void qemu_thread_atexit_remove(struct Notifier *notifier);
+#ifdef CONFIG_TSAN
+#include
+#endif
+
struct QemuSpin {
int value;
};
@@ -213,23 +217,46 @@ struct QemuSpin {
static inline void qemu_spin_init(QemuSpin *spin)
{
__sync_lock_release(>value);
+#ifdef CONFIG_TSAN
+__tsan_mutex_create(spin, __tsan_mutex_not_static);
+#endif
}
-static inline void qemu_spin_destroy(QemuSpin *spin)
-{ }
+/* const parameter because the only purpose here is the TSAN annotation */
+static inline void qemu_spin_destroy(const QemuSpin *spin)
+{
+#ifdef CONFIG_TSAN
+__tsan_mutex_destroy((void *)spin, __tsan_mutex_not_static);
+#endif
+}
static inline void qemu_spin_lock(QemuSpin *spin)
{
+#ifdef CONFIG_TSAN
+__tsan_mutex_pre_lock(spin, 0);
+#endif
while (unlikely(__sync_lock_test_and_set(>value, true))) {
while (atomic_read(>value)) {
cpu_relax();
}
}
+#ifdef CONFIG_TSAN
+__tsan_mutex_post_lock(spin, 0, 0);
+#endif
}
static inline bool qemu_spin_trylock(QemuSpin *spin)
{
-return __sync_lock_test_and_set(>value, true);
+#ifdef CONFIG_TSAN
+__tsan_mutex_pre_lock(spin, __tsan_mutex_try_lock);
+#endif
+bool busy = __sync_lock_test_and_set(>value, true);
+#ifdef CONFIG_TSAN
+unsigned flags = __tsan_mutex_try_lock;
+flags |= busy ? __tsan_mutex_try_lock_failed : 0;
+__tsan_mutex_post_lock(spin, flags, 0);
+#endif
+return busy;
}
static inline bool qemu_spin_locked(QemuSpin *spin)
@@ -239,7 +266,13 @@ static inline bool qemu_spin_locked(QemuSpin *spin)
static inline void qemu_spin_unlock(QemuSpin *spin)
{
+#ifdef CONFIG_TSAN
+__tsan_mutex_pre_unlock(spin, 0);
+#endif
__sync_lock_release(>value);
+#ifdef CONFIG_TSAN
+__tsan_mutex_post_unlock(spin, 0);
+#endif
}
struct QemuLockCnt {
--
2.17.1
Re: [PULL v2 05/13] accel/tcg: Relax va restrictions on 64-bit guests
On 6/5/20 7:11 AM, Alex Bennée wrote:
> @@ -467,7 +467,7 @@ abi_long target_mmap(abi_ulong start, abi_ulong len, int
> prot,
> * It can fail only on 64-bit host with 32-bit target.
> * On any other target/host host mmap() handles this error correctly.
> */
> -if (!guest_range_valid(start, len)) {
> +if (end < start || !guest_range_valid(start, len)) {
> errno = ENOMEM;
> goto fail;
> }
Interesting. I was adjusting guest_range_valid tagged pointers yesterday, and
thought that it looked buggy.
r~
[PATCH v2 06/13] tcg: call qemu_spin_destroy for tb->jmp_lock
From: "Emilio G. Cota"
Signed-off-by: Emilio G. Cota
Signed-off-by: Robert Foley
[RF: Minor changes to fix some checkpatch errors]
---
accel/tcg/translate-all.c | 10 +-
include/tcg/tcg.h | 3 ++-
tcg/tcg.c | 19 ---
3 files changed, 27 insertions(+), 5 deletions(-)
diff --git a/accel/tcg/translate-all.c b/accel/tcg/translate-all.c
index 42ce1dfcff..3708aab36b 100644
--- a/accel/tcg/translate-all.c
+++ b/accel/tcg/translate-all.c
@@ -384,6 +384,11 @@ static int cpu_restore_state_from_tb(CPUState *cpu,
TranslationBlock *tb,
return 0;
}
+static void tb_destroy(TranslationBlock *tb)
+{
+qemu_spin_destroy(>jmp_lock);
+}
+
bool cpu_restore_state(CPUState *cpu, uintptr_t host_pc, bool will_exit)
{
TranslationBlock *tb;
@@ -413,6 +418,7 @@ bool cpu_restore_state(CPUState *cpu, uintptr_t host_pc,
bool will_exit)
/* one-shot translation, invalidate it immediately */
tb_phys_invalidate(tb, -1);
tcg_tb_remove(tb);
+tb_destroy(tb);
}
r = true;
}
@@ -1230,7 +1236,7 @@ static void do_tb_flush(CPUState *cpu, run_on_cpu_data
tb_flush_count)
qht_reset_size(_ctx.htable, CODE_GEN_HTABLE_SIZE);
page_flush_tb();
-tcg_region_reset_all();
+tcg_region_reset_all(tb_destroy);
/* XXX: flush processor icache at this point if cache flush is
expensive */
atomic_mb_set(_ctx.tb_flush_count, tb_ctx.tb_flush_count + 1);
@@ -1886,6 +1892,7 @@ TranslationBlock *tb_gen_code(CPUState *cpu,
orig_aligned -= ROUND_UP(sizeof(*tb), qemu_icache_linesize);
atomic_set(_ctx->code_gen_ptr, (void *)orig_aligned);
+tb_destroy(tb);
return existing_tb;
}
tcg_tb_insert(tb);
@@ -2235,6 +2242,7 @@ void cpu_io_recompile(CPUState *cpu, uintptr_t retaddr)
tb_phys_invalidate(tb->orig_tb, -1);
}
tcg_tb_remove(tb);
+tb_destroy(tb);
}
/* TODO: If env->pc != tb->pc (i.e. the faulting instruction was not
diff --git a/include/tcg/tcg.h b/include/tcg/tcg.h
index 380014ed80..c8313fdcf0 100644
--- a/include/tcg/tcg.h
+++ b/include/tcg/tcg.h
@@ -818,8 +818,9 @@ void *tcg_malloc_internal(TCGContext *s, int size);
void tcg_pool_reset(TCGContext *s);
TranslationBlock *tcg_tb_alloc(TCGContext *s);
+typedef void (*tb_destroy_func)(TranslationBlock *tb);
void tcg_region_init(void);
-void tcg_region_reset_all(void);
+void tcg_region_reset_all(tb_destroy_func tb_destroy);
size_t tcg_code_size(void);
size_t tcg_code_capacity(void);
diff --git a/tcg/tcg.c b/tcg/tcg.c
index 1aa6cb47f2..7ae9dd7cf8 100644
--- a/tcg/tcg.c
+++ b/tcg/tcg.c
@@ -502,7 +502,16 @@ size_t tcg_nb_tbs(void)
return nb_tbs;
}
-static void tcg_region_tree_reset_all(void)
+static gboolean tcg_region_tree_traverse(gpointer k, gpointer v, gpointer data)
+{
+TranslationBlock *tb = v;
+tb_destroy_func tb_destroy = data;
+
+tb_destroy(tb);
+return FALSE;
+}
+
+static void tcg_region_tree_reset_all(tb_destroy_func tb_destroy)
{
size_t i;
@@ -510,6 +519,10 @@ static void tcg_region_tree_reset_all(void)
for (i = 0; i < region.n; i++) {
struct tcg_region_tree *rt = region_trees + i * tree_size;
+if (tb_destroy != NULL) {
+g_tree_foreach(rt->tree, tcg_region_tree_traverse, tb_destroy);
+}
+
/* Increment the refcount first so that destroy acts as a reset */
g_tree_ref(rt->tree);
g_tree_destroy(rt->tree);
@@ -586,7 +599,7 @@ static inline bool
tcg_region_initial_alloc__locked(TCGContext *s)
}
/* Call from a safe-work context */
-void tcg_region_reset_all(void)
+void tcg_region_reset_all(tb_destroy_func tb_destroy)
{
unsigned int n_ctxs = atomic_read(_tcg_ctxs);
unsigned int i;
@@ -603,7 +616,7 @@ void tcg_region_reset_all(void)
}
qemu_mutex_unlock();
-tcg_region_tree_reset_all();
+tcg_region_tree_reset_all(tb_destroy);
}
#ifdef CONFIG_USER_ONLY
--
2.17.1
[PATCH v2 07/13] translate-all: call qemu_spin_destroy for PageDesc
From: "Emilio G. Cota"
The radix tree is append-only, but we can fail to insert
a PageDesc if the insertion races with another thread.
Signed-off-by: Emilio G. Cota
Signed-off-by: Robert Foley
---
accel/tcg/translate-all.c | 9 +
1 file changed, 9 insertions(+)
diff --git a/accel/tcg/translate-all.c b/accel/tcg/translate-all.c
index 3708aab36b..3fb71a1503 100644
--- a/accel/tcg/translate-all.c
+++ b/accel/tcg/translate-all.c
@@ -547,6 +547,15 @@ static PageDesc *page_find_alloc(tb_page_addr_t index, int
alloc)
#endif
existing = atomic_cmpxchg(lp, NULL, pd);
if (unlikely(existing)) {
+#ifndef CONFIG_USER_ONLY
+{
+int i;
+
+for (i = 0; i < V_L2_SIZE; i++) {
+qemu_spin_destroy([i].lock);
+}
+}
+#endif
g_free(pd);
pd = existing;
}
--
2.17.1
[PATCH v2 11/13] util: Added tsan annotate for thread name.
This allows us to see the name of the thread in tsan warning reports such as this: Thread T7 'CPU 1/TCG' (tid=24317, running) created by main thread at: Signed-off-by: Robert Foley Reviewed-by: Emilio G. Cota --- util/qemu-thread-posix.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/util/qemu-thread-posix.c b/util/qemu-thread-posix.c index 838980aaa5..b4c2359272 100644 --- a/util/qemu-thread-posix.c +++ b/util/qemu-thread-posix.c @@ -15,6 +15,7 @@ #include "qemu/atomic.h" #include "qemu/notify.h" #include "qemu-thread-common.h" +#include "qemu/tsan.h" static bool name_threads; @@ -513,6 +514,7 @@ static void *qemu_thread_start(void *args) # endif } #endif +QEMU_TSAN_ANNOTATE_THREAD_NAME(qemu_thread_args->name); g_free(qemu_thread_args->name); g_free(qemu_thread_args); pthread_cleanup_push(qemu_thread_atexit_notify, NULL); -- 2.17.1
[PATCH v2 13/13] tests: Disable select tests under TSan, which hit TSan issue.
Disable a few tests under CONFIG_TSAN, which run into a known TSan issue that results in a hang. https://github.com/google/sanitizers/issues/1116 The disabled tests under TSan include all the qtests as well as the test-char, test-qga, and test-qdev-global-props. Signed-off-by: Robert Foley --- tests/Makefile.include | 9 +++-- tests/qtest/Makefile.include | 7 +-- 2 files changed, 12 insertions(+), 4 deletions(-) diff --git a/tests/Makefile.include b/tests/Makefile.include index 6e3d6370df..874a2990b3 100644 --- a/tests/Makefile.include +++ b/tests/Makefile.include @@ -53,7 +53,6 @@ SYSEMU_TARGET_LIST := $(subst -softmmu.mak,,$(notdir \ check-unit-y += tests/check-qdict$(EXESUF) check-unit-y += tests/check-block-qdict$(EXESUF) -check-unit-$(CONFIG_SOFTMMU) += tests/test-char$(EXESUF) check-unit-y += tests/check-qnum$(EXESUF) check-unit-y += tests/check-qstring$(EXESUF) check-unit-y += tests/check-qlist$(EXESUF) @@ -106,7 +105,6 @@ check-unit-y += tests/test-qht$(EXESUF) check-unit-y += tests/test-qht-par$(EXESUF) check-unit-y += tests/test-bitops$(EXESUF) check-unit-y += tests/test-bitcnt$(EXESUF) -check-unit-y += tests/test-qdev-global-props$(EXESUF) check-unit-y += tests/check-qom-interface$(EXESUF) check-unit-y += tests/check-qom-proplist$(EXESUF) check-unit-y += tests/test-qemu-opts$(EXESUF) @@ -121,9 +119,16 @@ check-speed-$(CONFIG_BLOCK) += tests/benchmark-crypto-cipher$(EXESUF) check-unit-$(CONFIG_BLOCK) += tests/test-crypto-secret$(EXESUF) check-unit-$(call land,$(CONFIG_BLOCK),$(CONFIG_GNUTLS)) += tests/test-crypto-tlscredsx509$(EXESUF) check-unit-$(call land,$(CONFIG_BLOCK),$(CONFIG_GNUTLS)) += tests/test-crypto-tlssession$(EXESUF) +ifndef CONFIG_TSAN +# Some tests: test-char, test-qdev-global-props, and test-qga, +# are not runnable under TSan due to a known issue. +# https://github.com/google/sanitizers/issues/1116 +check-unit-$(CONFIG_SOFTMMU) += tests/test-char$(EXESUF) +check-unit-y += tests/test-qdev-global-props$(EXESUF) ifneq (,$(findstring qemu-ga,$(TOOLS))) check-unit-$(call land,$(CONFIG_LINUX),$(CONFIG_VIRTIO_SERIAL)) += tests/test-qga$(EXESUF) endif +endif check-unit-y += tests/test-timed-average$(EXESUF) check-unit-$(CONFIG_INOTIFY1) += tests/test-util-filemonitor$(EXESUF) check-unit-y += tests/test-util-sockets$(EXESUF) diff --git a/tests/qtest/Makefile.include b/tests/qtest/Makefile.include index 9e5a51d033..71fd714a2a 100644 --- a/tests/qtest/Makefile.include +++ b/tests/qtest/Makefile.include @@ -313,12 +313,15 @@ tests/qtest/tpm-tis-device-test$(EXESUF): tests/qtest/tpm-tis-device-test.o test # QTest rules TARGETS=$(patsubst %-softmmu,%, $(filter %-softmmu,$(TARGET_DIRS))) +QTEST_TARGETS = +# The qtests are not runnable (yet) under TSan due to a known issue. +# https://github.com/google/sanitizers/issues/1116 +ifndef CONFIG_TSAN ifeq ($(CONFIG_POSIX),y) QTEST_TARGETS = $(TARGETS) check-qtest-y=$(foreach TARGET,$(TARGETS), $(check-qtest-$(TARGET)-y:%=tests/qtest/%$(EXESUF))) check-qtest-y += $(check-qtest-generic-y:%=tests/qtest/%$(EXESUF)) -else -QTEST_TARGETS = +endif endif qtest-obj-y = tests/qtest/libqtest.o $(test-util-obj-y) -- 2.17.1
[PATCH v2 05/13] qht: call qemu_spin_destroy for head buckets
From: "Emilio G. Cota"
Signed-off-by: Robert Foley
Reviewed-by: Alex Bennée
---
util/qht.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/util/qht.c b/util/qht.c
index aa51be3c52..67e5d5b916 100644
--- a/util/qht.c
+++ b/util/qht.c
@@ -348,6 +348,7 @@ static inline void qht_chain_destroy(const struct
qht_bucket *head)
struct qht_bucket *curr = head->next;
struct qht_bucket *prev;
+qemu_spin_destroy(>lock);
while (curr) {
prev = curr;
curr = curr->next;
--
2.17.1
[PATCH v2 03/13] thread: add qemu_spin_destroy
From: "Emilio G. Cota"
It will be used for TSAN annotations.
Signed-off-by: Emilio G. Cota
Signed-off-by: Robert Foley
Reviewed-by: Alex Bennée
---
include/qemu/thread.h | 3 +++
1 file changed, 3 insertions(+)
diff --git a/include/qemu/thread.h b/include/qemu/thread.h
index d22848138e..e50a073889 100644
--- a/include/qemu/thread.h
+++ b/include/qemu/thread.h
@@ -215,6 +215,9 @@ static inline void qemu_spin_init(QemuSpin *spin)
__sync_lock_release(>value);
}
+static inline void qemu_spin_destroy(QemuSpin *spin)
+{ }
+
static inline void qemu_spin_lock(QemuSpin *spin)
{
while (unlikely(__sync_lock_test_and_set(>value, true))) {
--
2.17.1
[PATCH v2 12/13] docs: Added details on TSan to testing.rst
Adds TSan details to testing.rst. This includes background and reference details on TSan, and details on how to build and test with TSan both with and without docker. Signed-off-by: Robert Foley Reviewed-by: Emilio G. Cota --- docs/devel/testing.rst | 107 + 1 file changed, 107 insertions(+) diff --git a/docs/devel/testing.rst b/docs/devel/testing.rst index 770a987ea4..c1ff24370b 100644 --- a/docs/devel/testing.rst +++ b/docs/devel/testing.rst @@ -397,6 +397,113 @@ list is in the ``make docker`` help text. The frequently used ones are: * ``DEBUG=1``: enables debug. See the previous "Debugging a Docker test failure" section. +Thread Sanitizer + + +Thread Sanitizer (TSan) is a tool which can detect data races. QEMU supports +building and testing with this tool. + +For more information on TSan: + +https://github.com/google/sanitizers/wiki/ThreadSanitizerCppManual + +Thread Sanitizer in Docker +--- +TSan is currently supported in the ubuntu2004 docker. + +The test-tsan test will build using TSan and then run make check. + +.. code:: + + make docker-test-tsan@ubuntu2004 + +TSan warnings under docker are placed in files located at build/tsan/. + +We recommend using DEBUG=1 to allow launching the test from inside the docker, +and to allow review of the warnings generated by TSan. + +Building and Testing with TSan +-- + +It is possible to build and test with TSan, with a few additional steps. +These steps are normally done automatically in the docker. + +There is a one time patch needed in clang-9 or clang-10 at this time: + +.. code:: + + sed -i 's/^const/static const/g' \ + /usr/lib/llvm-10/lib/clang/10.0.0/include/sanitizer/tsan_interface.h + +To configure the build for TSan: + +.. code:: + + ../configure --enable-tsan --cc=clang-10 --cxx=clang++-10 \ + --disable-werror --extra-cflags="-O0" + +The runtime behavior of TSAN is controlled by the TSAN_OPTIONS environment +variable. + +More information on the TSAN_OPTIONS can be found here: + +https://github.com/google/sanitizers/wiki/ThreadSanitizerFlags + +For example: + +.. code:: + + export TSAN_OPTIONS=suppressions=/tests/tsan/suppressions.tsan \ + detect_deadlocks=false history_size=7 exitcode=0 \ + log_path=/tsan/tsan_warning + +The above exitcode=0 has TSan continue without error if any warnings are found. +This allows for running the test and then checking the warnings afterwards. +If you want TSan to stop and exit with error on warnings, use exitcode=66. + +TSan Suppressions +- +Keep in mind that for any data race warning, although there might be a data race +detected by TSan, there might be no actual bug here. TSan provides several +different mechanisms for suppressing warnings. In general it is recommended +to fix the code if possible to eliminate the data race rather than suppress +the warning. + +A few important files for suppressing warnings are: + +tests/tsan/suppressions.tsan - Has TSan warnings we wish to suppress at runtime. +The comment on each supression will typically indicate why we are +suppressing it. More information on the file format can be found here: + +https://github.com/google/sanitizers/wiki/ThreadSanitizerSuppressions + +tests/tsan/blacklist.tsan - Has TSan warnings we wish to disable +at compile time for test or debug. +Add flags to configure to enable: + +"--extra-cflags=-fsanitize-blacklist=/tests/tsan/blacklist.tsan" + +More information on the file format can be found here under "Blacklist Format": + +https://github.com/google/sanitizers/wiki/ThreadSanitizerFlags + +TSan Annotations + +include/qemu/tsan.h defines annotations. See this file for more descriptions +of the annotations themselves. Annotations can be used to suppress +TSan warnings or give TSan more information so that it can detect proper +relationships between accesses of data. + +Annotation examples can be found here: + +https://github.com/llvm/llvm-project/tree/master/compiler-rt/test/tsan/ + +Good files to start with are: annotate_happens_before.cpp and ignore_race.cpp + +The full set of annotations can be found here: + +https://github.com/llvm/llvm-project/blob/master/compiler-rt/lib/tsan/rtl/tsan_interface_ann.cpp + VM testing == -- 2.17.1
[PATCH v2 04/13] cputlb: destroy CPUTLB with tlb_destroy
From: "Emilio G. Cota"
I was after adding qemu_spin_destroy calls, but while at
it I noticed that we are leaking some memory.
Signed-off-by: Emilio G. Cota
Signed-off-by: Robert Foley
Reviewed-by: Alex Bennée
---
accel/tcg/cputlb.c | 15 +++
exec.c | 1 +
include/exec/exec-all.h | 8
3 files changed, 24 insertions(+)
diff --git a/accel/tcg/cputlb.c b/accel/tcg/cputlb.c
index eb2cf9de5e..1e815357c7 100644
--- a/accel/tcg/cputlb.c
+++ b/accel/tcg/cputlb.c
@@ -270,6 +270,21 @@ void tlb_init(CPUState *cpu)
}
}
+void tlb_destroy(CPUState *cpu)
+{
+CPUArchState *env = cpu->env_ptr;
+int i;
+
+qemu_spin_destroy(_tlb(env)->c.lock);
+for (i = 0; i < NB_MMU_MODES; i++) {
+CPUTLBDesc *desc = _tlb(env)->d[i];
+CPUTLBDescFast *fast = _tlb(env)->f[i];
+
+g_free(fast->table);
+g_free(desc->iotlb);
+}
+}
+
/* flush_all_helper: run fn across all cpus
*
* If the wait flag is set then the src cpu's helper will be queued as
diff --git a/exec.c b/exec.c
index 5162f0d12f..da3d60b034 100644
--- a/exec.c
+++ b/exec.c
@@ -892,6 +892,7 @@ void cpu_exec_unrealizefn(CPUState *cpu)
{
CPUClass *cc = CPU_GET_CLASS(cpu);
+tlb_destroy(cpu);
cpu_list_remove(cpu);
if (cc->vmsd != NULL) {
diff --git a/include/exec/exec-all.h b/include/exec/exec-all.h
index 8792bea07a..3cf88272df 100644
--- a/include/exec/exec-all.h
+++ b/include/exec/exec-all.h
@@ -124,6 +124,11 @@ void cpu_address_space_init(CPUState *cpu, int asidx,
* @cpu: CPU whose TLB should be initialized
*/
void tlb_init(CPUState *cpu);
+/**
+ * tlb_destroy - destroy a CPU's TLB
+ * @cpu: CPU whose TLB should be destroyed
+ */
+void tlb_destroy(CPUState *cpu);
/**
* tlb_flush_page:
* @cpu: CPU whose TLB should be flushed
@@ -284,6 +289,9 @@ void tlb_set_page(CPUState *cpu, target_ulong vaddr,
static inline void tlb_init(CPUState *cpu)
{
}
+static inline void tlb_destroy(CPUState *cpu)
+{
+}
static inline void tlb_flush_page(CPUState *cpu, target_ulong addr)
{
}
--
2.17.1
[PATCH v2 10/13] include/qemu: Added tsan.h for annotations.
These annotations will allow us to give tsan additional hints. For example, we can inform tsan about reads/writes to ignore to silence certain classes of warnings. We can also annotate threads so that the proper thread naming shows up in tsan warning results. Signed-off-by: Robert Foley Reviewed-by: Emilio G. Cota --- include/qemu/tsan.h | 71 + 1 file changed, 71 insertions(+) create mode 100644 include/qemu/tsan.h diff --git a/include/qemu/tsan.h b/include/qemu/tsan.h new file mode 100644 index 00..09cc665f91 --- /dev/null +++ b/include/qemu/tsan.h @@ -0,0 +1,71 @@ +#ifndef QEMU_TSAN_H +#define QEMU_TSAN_H +/* + * tsan.h + * + * This file defines macros used to give ThreadSanitizer + * additional information to help suppress warnings. + * This is necessary since TSan does not provide a header file + * for these annotations. The standard way to include these + * is via the below macros. + * + * Annotation examples can be found here: + * https://github.com/llvm/llvm-project/tree/master/compiler-rt/test/tsan + * annotate_happens_before.cpp or ignore_race.cpp are good places to start. + * + * The full set of annotations can be found here in tsan_interface_ann.cpp. + * https://github.com/llvm/llvm-project/blob/master/compiler-rt/lib/tsan/rtl/ + * + * This work is licensed under the terms of the GNU GPL, version 2 or later. + * See the COPYING file in the top-level directory. + */ + +#ifdef CONFIG_TSAN +/* + * Informs TSan of a happens before/after relationship. + */ +#define QEMU_TSAN_ANNOTATE_HAPPENS_BEFORE(addr) \ +AnnotateHappensBefore(__FILE__, __LINE__, (void *)(addr)) +#define QEMU_TSAN_ANNOTATE_HAPPENS_AFTER(addr) \ +AnnotateHappensAfter(__FILE__, __LINE__, (void *)(addr)) +/* + * Gives TSan more information about thread names it can report the + * name of the thread in the warning report. + */ +#define QEMU_TSAN_ANNOTATE_THREAD_NAME(name) \ +AnnotateThreadName(__FILE__, __LINE__, (void *)(name)) +/* + * Allows defining a region of code on which TSan will not record memory READS. + * This has the effect of disabling race detection for this section of code. + */ +#define QEMU_TSAN_ANNOTATE_IGNORE_READS_BEGIN() \ +AnnotateIgnoreReadsBegin(__FILE__, __LINE__) +#define QEMU_TSAN_ANNOTATE_IGNORE_READS_END() \ +AnnotateIgnoreReadsEnd(__FILE__, __LINE__) +/* + * Allows defining a region of code on which TSan will not record memory + * WRITES. This has the effect of disabling race detection for this + * section of code. + */ +#define QEMU_TSAN_ANNOTATE_IGNORE_WRITES_BEGIN() \ +AnnotateIgnoreWritesBegin(__FILE__, __LINE__) +#define QEMU_TSAN_ANNOTATE_IGNORE_WRITES_END() \ +AnnotateIgnoreWritesEnd(__FILE__, __LINE__) +#else +#define QEMU_TSAN_ANNOTATE_HAPPENS_BEFORE(addr) +#define QEMU_TSAN_ANNOTATE_HAPPENS_AFTER(addr) +#define QEMU_TSAN_ANNOTATE_THREAD_NAME(name) +#define QEMU_TSAN_ANNOTATE_IGNORE_READS_BEGIN() +#define QEMU_TSAN_ANNOTATE_IGNORE_READS_END() +#define QEMU_TSAN_ANNOTATE_IGNORE_WRITES_BEGIN() +#define QEMU_TSAN_ANNOTATE_IGNORE_WRITES_END() +#endif + +void AnnotateHappensBefore(const char *f, int l, void *addr); +void AnnotateHappensAfter(const char *f, int l, void *addr); +void AnnotateThreadName(const char *f, int l, char *name); +void AnnotateIgnoreReadsBegin(const char *f, int l); +void AnnotateIgnoreReadsEnd(const char *f, int l); +void AnnotateIgnoreWritesBegin(const char *f, int l); +void AnnotateIgnoreWritesEnd(const char *f, int l); +#endif -- 2.17.1
[PATCH v2 02/13] cpu: convert queued work to a QSIMPLEQ
From: "Emilio G. Cota"
We convert queued work to a QSIMPLEQ, instead of
open-coding it.
While at it, make sure that all accesses to the list are
performed while holding the list's lock.
Reviewed-by: Richard Henderson
Reviewed-by: Alex Bennée
Reviewed-by: Philippe Mathieu-Daudé
Signed-off-by: Emilio G. Cota
Signed-off-by: Robert Foley
---
cpus-common.c | 25 -
cpus.c| 14 --
hw/core/cpu.c | 1 +
include/hw/core/cpu.h | 6 +++---
4 files changed, 24 insertions(+), 22 deletions(-)
diff --git a/cpus-common.c b/cpus-common.c
index 70a9d12981..8f5512b3d7 100644
--- a/cpus-common.c
+++ b/cpus-common.c
@@ -97,7 +97,7 @@ void cpu_list_remove(CPUState *cpu)
}
struct qemu_work_item {
-struct qemu_work_item *next;
+QSIMPLEQ_ENTRY(qemu_work_item) node;
run_on_cpu_func func;
run_on_cpu_data data;
bool free, exclusive, done;
@@ -106,13 +106,7 @@ struct qemu_work_item {
static void queue_work_on_cpu(CPUState *cpu, struct qemu_work_item *wi)
{
qemu_mutex_lock(>work_mutex);
-if (cpu->queued_work_first == NULL) {
-cpu->queued_work_first = wi;
-} else {
-cpu->queued_work_last->next = wi;
-}
-cpu->queued_work_last = wi;
-wi->next = NULL;
+QSIMPLEQ_INSERT_TAIL(>work_list, wi, node);
wi->done = false;
qemu_mutex_unlock(>work_mutex);
@@ -306,17 +300,14 @@ void process_queued_cpu_work(CPUState *cpu)
{
struct qemu_work_item *wi;
-if (cpu->queued_work_first == NULL) {
+qemu_mutex_lock(>work_mutex);
+if (QSIMPLEQ_EMPTY(>work_list)) {
+qemu_mutex_unlock(>work_mutex);
return;
}
-
-qemu_mutex_lock(>work_mutex);
-while (cpu->queued_work_first != NULL) {
-wi = cpu->queued_work_first;
-cpu->queued_work_first = wi->next;
-if (!cpu->queued_work_first) {
-cpu->queued_work_last = NULL;
-}
+while (!QSIMPLEQ_EMPTY(>work_list)) {
+wi = QSIMPLEQ_FIRST(>work_list);
+QSIMPLEQ_REMOVE_HEAD(>work_list, node);
qemu_mutex_unlock(>work_mutex);
if (wi->exclusive) {
/* Running work items outside the BQL avoids the following
deadlock:
diff --git a/cpus.c b/cpus.c
index 5670c96bcf..af44027549 100644
--- a/cpus.c
+++ b/cpus.c
@@ -97,9 +97,19 @@ bool cpu_is_stopped(CPUState *cpu)
return cpu->stopped || !runstate_is_running();
}
+static inline bool cpu_work_list_empty(CPUState *cpu)
+{
+bool ret;
+
+qemu_mutex_lock(>work_mutex);
+ret = QSIMPLEQ_EMPTY(>work_list);
+qemu_mutex_unlock(>work_mutex);
+return ret;
+}
+
static bool cpu_thread_is_idle(CPUState *cpu)
{
-if (cpu->stop || cpu->queued_work_first) {
+if (cpu->stop || !cpu_work_list_empty(cpu)) {
return false;
}
if (cpu_is_stopped(cpu)) {
@@ -1498,7 +1508,7 @@ static void *qemu_tcg_rr_cpu_thread_fn(void *arg)
cpu = first_cpu;
}
-while (cpu && !cpu->queued_work_first && !cpu->exit_request) {
+while (cpu && cpu_work_list_empty(cpu) && !cpu->exit_request) {
atomic_mb_set(_current_rr_cpu, cpu);
current_cpu = cpu;
diff --git a/hw/core/cpu.c b/hw/core/cpu.c
index 5284d384fb..77703d62b7 100644
--- a/hw/core/cpu.c
+++ b/hw/core/cpu.c
@@ -368,6 +368,7 @@ static void cpu_common_initfn(Object *obj)
cpu->nr_threads = 1;
qemu_mutex_init(>work_mutex);
+QSIMPLEQ_INIT(>work_list);
QTAILQ_INIT(>breakpoints);
QTAILQ_INIT(>watchpoints);
diff --git a/include/hw/core/cpu.h b/include/hw/core/cpu.h
index 07f7698155..d78ff1d165 100644
--- a/include/hw/core/cpu.h
+++ b/include/hw/core/cpu.h
@@ -331,8 +331,8 @@ struct qemu_work_item;
* @opaque: User data.
* @mem_io_pc: Host Program Counter at which the memory was accessed.
* @kvm_fd: vCPU file descriptor for KVM.
- * @work_mutex: Lock to prevent multiple access to queued_work_*.
- * @queued_work_first: First asynchronous work pending.
+ * @work_mutex: Lock to prevent multiple access to @work_list.
+ * @work_list: List of pending asynchronous work.
* @trace_dstate_delayed: Delayed changes to trace_dstate (includes all changes
*to @trace_dstate).
* @trace_dstate: Dynamic tracing state of events for this vCPU (bitmask).
@@ -376,7 +376,7 @@ struct CPUState {
sigjmp_buf jmp_env;
QemuMutex work_mutex;
-struct qemu_work_item *queued_work_first, *queued_work_last;
+QSIMPLEQ_HEAD(, qemu_work_item) work_list;
CPUAddressSpace *cpu_ases;
int num_ases;
--
2.17.1
[PATCH v2 01/13] configure: add --enable-tsan flag + fiber annotations for coroutine-ucontext
From: Lingfeng Yang
We tried running QEMU under tsan in 2016, but tsan's lack of support for
longjmp-based fibers was a blocker:
https://groups.google.com/forum/#!topic/thread-sanitizer/se0YuzfWazw
Fortunately, thread sanitizer gained fiber support in early 2019:
https://reviews.llvm.org/D54889
This patch brings tsan support upstream by importing the patch that annotated
QEMU's coroutines as tsan fibers in Android's QEMU fork:
https://android-review.googlesource.com/c/platform/external/qemu/+/844675
Tested with '--enable-tsan --cc=clang-9 --cxx=clang++-9 --disable-werror'
configure flags.
Signed-off-by: Lingfeng Yang
Signed-off-by: Emilio G. Cota
[cota: minor modifications + configure changes]
Signed-off-by: Robert Foley
[RF: configure changes for warnings, erorr handling + minor modifications]
---
configure | 47 ++-
util/coroutine-ucontext.c | 97 +++
2 files changed, 134 insertions(+), 10 deletions(-)
diff --git a/configure b/configure
index f087d2bcd1..9b50820366 100755
--- a/configure
+++ b/configure
@@ -395,6 +395,7 @@ gprof="no"
debug_tcg="no"
debug="no"
sanitizers="no"
+tsan="no"
fortify_source=""
strip_opt="yes"
tcg_interpreter="no"
@@ -1150,6 +1151,10 @@ for opt do
;;
--disable-sanitizers) sanitizers="no"
;;
+ --enable-tsan) tsan="yes"
+ ;;
+ --disable-tsan) tsan="no"
+ ;;
--enable-sparse) sparse="yes"
;;
--disable-sparse) sparse="no"
@@ -1750,6 +1755,7 @@ Advanced options (experts only):
--with-pkgversion=VERS use specified string as sub-version of the package
--enable-debug enable common debug build options
--enable-sanitizers enable default sanitizers
+ --enable-tsanenable thread sanitizer
--disable-strip disable stripping binaries
--disable-werror disable compilation abort on warning
--disable-stack-protector disable compiler-provided stack protection
@@ -6192,6 +6198,30 @@ if test "$fuzzing" = "yes" ; then
fi
fi
+# Thread sanitizer is, for now, much noisier than the other sanitizers;
+# keep it separate until that is not the case.
+if test "$tsan" = "yes" && test "$sanitizers" = "yes"; then
+ error_exit "TSAN is not supported with other sanitiziers."
+fi
+have_tsan=no
+have_tsan_iface_fiber=no
+if test "$tsan" = "yes" ; then
+ write_c_skeleton
+ if compile_prog "$CPU_CFLAGS -Werror -fsanitize=thread" "" ; then
+ have_tsan=yes
+ fi
+ cat > $TMPC << EOF
+#include
+int main(void) {
+ __tsan_create_fiber(0);
+ return 0;
+}
+EOF
+ if compile_prog "$CPU_CFLAGS -Werror -fsanitize=thread" "" ; then
+ have_tsan_iface_fiber=yes
+ fi
+fi
+
##
# check for libpmem
@@ -6293,6 +6323,16 @@ if test "$have_asan" = "yes"; then
"Without code annotation, the report may be inferior."
fi
fi
+if test "$have_tsan" = "yes" ; then
+ if test "$have_tsan_iface_fiber" = "yes" ; then
+QEMU_CFLAGS="-fsanitize=thread $QEMU_CFLAGS"
+QEMU_LDFLAGS="-fsanitize=thread $QEMU_LDFLAGS"
+ else
+error_exit "Cannot enable TSAN due to missing fiber annotation interface."
+ fi
+elif test "$tsan" = "yes" ; then
+ error_exit "Cannot enable TSAN due to missing sanitize thread interface."
+fi
if test "$have_ubsan" = "yes"; then
QEMU_CFLAGS="-fsanitize=undefined $QEMU_CFLAGS"
QEMU_LDFLAGS="-fsanitize=undefined $QEMU_LDFLAGS"
@@ -6328,7 +6368,8 @@ if test "$werror" = "yes"; then
QEMU_CFLAGS="-Werror $QEMU_CFLAGS"
fi
-if test "$solaris" = "no" ; then
+# Exclude --warn-common with TSan to suppress warnings from the TSan libraries.
+if test "$solaris" = "no" && test "$tsan" = "no"; then
if $ld --version 2>/dev/null | grep "GNU ld" >/dev/null 2>/dev/null ; then
QEMU_LDFLAGS="-Wl,--warn-common $QEMU_LDFLAGS"
fi
@@ -7382,6 +7423,10 @@ if test "$have_asan_iface_fiber" = "yes" ; then
echo "CONFIG_ASAN_IFACE_FIBER=y" >> $config_host_mak
fi
+if test "$have_tsan" = "yes" && test "$have_tsan_iface_fiber" = "yes" ; then
+echo "CONFIG_TSAN=y" >> $config_host_mak
+fi
+
if test "$has_environ" = "yes" ; then
echo "CONFIG_HAS_ENVIRON=y" >> $config_host_mak
fi
diff --git a/util/coroutine-ucontext.c b/util/coroutine-ucontext.c
index bd593e61bc..a3dc78e67a 100644
--- a/util/coroutine-ucontext.c
+++ b/util/coroutine-ucontext.c
@@ -37,18 +37,33 @@
#endif
#endif
+#ifdef CONFIG_TSAN
+#include
+#endif
+
typedef struct {
Coroutine base;
void *stack;
size_t stack_size;
sigjmp_buf env;
+void *tsan_co_fiber;
+void *tsan_caller_fiber;
+
#ifdef CONFIG_VALGRIND_H
unsigned int valgrind_stack_id;
#endif
} CoroutineUContext;
+#define UC_DEBUG 0
+#if UC_DEBUG && defined(CONFIG_TSAN)
+#define UC_TRACE(fmt, ...) fprintf(stderr, "%s:%d:%p " fmt "\n", \
+__func__, __LINE__, __tsan_get_current_fiber(), ##__VA_ARGS__);
+#else
+#define UC_TRACE(fmt, ...)
+#endif
+
/**
* Per-thread coroutine bookkeeping
[PATCH v2 00/13] Add Thread Sanitizer support to QEMU
v1: https://lists.gnu.org/archive/html/qemu-devel/2020-05/msg08302.html Changes in v2: - Fixed make check under TSan. With the below fixes, make check under TSan completes successfully, albeit with TSan warnings. - We found that several unit tests and the qtests hit an issue in TSan, which results in a hung test. This is a known issue: https://github.com/google/sanitizers/issues/1116 - Under TSan, disable the 3 unit tests that hit this above issue. - Under TSan, disable the qtests since they hit this issue too. - Split out the docker testing for tsan into its own test (test-tsan). - configure: Error out if tsan and other sanitizers are used together. - configure: Cleaned up warnings during tsan build caused by tsan libraries. This patch series continues the work done by Emilio Cota and others to add Thread Sanitizer (TSan) support to QEMU. The starting point for this work was Emilio's branch here: https://github.com/cota/qemu/commits/tsan specifically this commit: 0be125fc0afd47218b34d2019abdd19b644f3199 The main purpose of this patch is to enable TSan support so that QEMU developers can start using the tool. We found this tool useful and even ran it on our recent changes in the cpu-locks series, which fixes many warnings. Clearly there is work to do here to clean up all the warnings. :) We have also made an effort to introduce enough of the TSan suppression mechanisms, so that others can continue this work. This series adds support for: - configure option for --enable-tsan. - testing.rst has the full details on how to use TSan with or without docker, including all the suppression mechanisms. - We added an Ubuntu 20.04 docker that supports TSan builds. - test-tsan is a new docker test that builds and runs make check under TSan. - We added an example blacklist file for files or functions TSan should ignore at compile time. This can now be specified manually. - Added a suppression file for TSan to suppress certain warnings at run time. - Added tsan.h with annotations which also can be used to suppress warnings. Emilio G. Cota (7): cpu: convert queued work to a QSIMPLEQ thread: add qemu_spin_destroy cputlb: destroy CPUTLB with tlb_destroy qht: call qemu_spin_destroy for head buckets tcg: call qemu_spin_destroy for tb->jmp_lock translate-all: call qemu_spin_destroy for PageDesc thread: add tsan annotations to QemuSpin Lingfeng Yang (1): configure: add --enable-tsan flag + fiber annotations for coroutine-ucontext Robert Foley (5): tests/docker: Added docker build support for TSan. include/qemu: Added tsan.h for annotations. util: Added tsan annotate for thread name. docs: Added details on TSan to testing.rst tests: Disable select tests under TSan, which hit TSan issue. accel/tcg/cputlb.c | 15 +++ accel/tcg/translate-all.c | 19 +++- configure | 47 - cpus-common.c | 25 ++--- cpus.c | 14 ++- docs/devel/testing.rst | 107 + exec.c | 1 + hw/core/cpu.c | 1 + include/exec/exec-all.h| 8 ++ include/hw/core/cpu.h | 6 +- include/qemu/thread.h | 38 +++- include/qemu/tsan.h| 71 ++ include/tcg/tcg.h | 3 +- tcg/tcg.c | 19 +++- tests/Makefile.include | 9 +- tests/docker/dockerfiles/ubuntu2004.docker | 65 + tests/docker/test-tsan | 44 + tests/qtest/Makefile.include | 7 +- tests/tsan/blacklist.tsan | 10 ++ tests/tsan/suppressions.tsan | 14 +++ util/coroutine-ucontext.c | 97 +-- util/qemu-thread-posix.c | 2 + util/qht.c | 1 + 23 files changed, 581 insertions(+), 42 deletions(-) create mode 100644 include/qemu/tsan.h create mode 100644 tests/docker/dockerfiles/ubuntu2004.docker create mode 100755 tests/docker/test-tsan create mode 100644 tests/tsan/blacklist.tsan create mode 100644 tests/tsan/suppressions.tsan -- 2.17.1
Re: [PATCH 12/13] i386: hvf: Move mmio_buf into CPUX86State
On Thu, Jun 04, 2020 at 08:27:37PM +0200, Paolo Bonzini wrote: > On 28/05/20 21:37, Roman Bolshakov wrote: > > There's no similar field in CPUX86State, but it's needed for MMIO traps. > > > > It should be possible to get rid of the buffer altogether, but it's ok > to do it separately. > Hi Paolo, I will look into that. Thanks, Roman
Re: [PATCH 4/7] python/qemu: Add pipenv support
On 6/5/20 12:21 PM, Vladimir Sementsov-Ogievskiy wrote: > 03.06.2020 03:15, John Snow wrote: >> pipenv is a tool used for managing virtual environments with precisely >> specified dependencies. It is separate from the dependencies listed in >> setup.py, which are (by 'best practices') not supposed to be pinned. >> >> Note that pipenv is not required to install or use this module; this is >> just a convenience for in-tree developing. >> >> Here, a "blank" pipfile is added with no dependencies, but specifies >> Python 3.6 for the virtual environment. >> >> Pipfile will specify our version minimums, while Pipfile.lock specifies >> an exact loudout of packages that were known to operate correctly. This >> latter file provides the real value for easy setup of container images >> and CI environments. >> >> Signed-off-by: John Snow >> --- >> python/Pipfile | 11 +++ >> 1 file changed, 11 insertions(+) >> create mode 100644 python/Pipfile >> >> diff --git a/python/Pipfile b/python/Pipfile >> new file mode 100644 >> index 000..9534830b5eb >> --- /dev/null >> +++ b/python/Pipfile >> @@ -0,0 +1,11 @@ >> +[[source]] >> +name = "pypi" >> +url = "https://pypi.org/simple; >> +verify_ssl = true >> + >> +[dev-packages] >> + >> +[packages] >> + >> +[requires] >> +python_version = "3.6" >> > > Should it be >= or something like this? > I think logistically that makes sense, but I'm not sure if the tool supports it. I decided to target the oldest version of Python we support (for non-build infrastructure) to ensure a minimum viability. > And, how should I use this all? > > My failed attempt: > [root@kvm python]# pipenv install --python /usr/bin/python3 > Virtualenv already exists! > Removing existing virtualenv… > Creating a virtualenv for this project… > Pipfile: /work/src/qemu/john-python-installable/python/Pipfile > Using /usr/bin/python3 (3.7.5) to create virtualenv… > ⠹ Creating virtual environment...created virtual environment > CPython3.7.5.final.0-64 in 112ms > creator > CPython3Posix(dest=/root/.local/share/virtualenvs/python-4FwBBPCc, > clear=False, global=False) > seeder FromAppData(download=False, pip=latest, setuptools=latest, > wheel=latest, via=copy, > app_data_dir=/root/.local/share/virtualenv/seed-app-data/v1.0.1) > activators > BashActivator,CShellActivator,FishActivator,PowerShellActivator,PythonActivator,XonshActivator > > > ✔ Successfully created virtual environment! > Virtualenv location: /root/.local/share/virtualenvs/python-4FwBBPCc > Warning: Your Pipfile requires python_version 3.6, but you are using > 3.7.5 (/root/.local/share/v/p/bin/python). > $ pipenv --rm and rebuilding the virtual environment may resolve the > issue. > $ pipenv check will surely fail. > Installing dependencies from Pipfile.lock (44d7bd)… > 0/0 — 00:00:00 > To activate this project's virtualenv, run pipenv shell. > Alternatively, run a command inside the virtualenv with pipenv run. > [root@kvm python]# pipenv shell > Warning: Your Pipfile requires python_version 3.6, but you are using > 3.7.5 (/root/.local/share/v/p/bin/python). > $ pipenv --rm and rebuilding the virtual environment may resolve the > issue. > $ pipenv check will surely fail. > Launching subshell in virtual environment… > . /root/.local/share/virtualenvs/python-4FwBBPCc/bin/activate > [root@kvm work]# . > /root/.local/share/virtualenvs/python-4FwBBPCc/bin/activate > (python) [root@kvm work]# python > Python 3.7.5 (default, Oct 17 2019, 12:09:47) > [GCC 9.2.1 20190827 (Red Hat 9.2.1-1)] on linux > Type "help", "copyright", "credits" or "license" for more information. import pylint > Traceback (most recent call last): > File "", line 1, in > ModuleNotFoundError: No module named 'pylint' > > and iotest 297 says: "pylint-3 not found" > Ah! that's a bug in iotest 297. It's expecting the fedora package there. I'll have to fix that. > (honestly, I'm new to using python virtual environment) > Good questions. I'll document this in the README.rst for this folder, actually... 1. Create a virtual environment > pipenv sync --dev jsnow@probe ~/s/q/python (python-package-refactor)> pipenv sync --dev Creating a virtualenv for this project… Pipfile: /home/jsnow/src/qemu/python/Pipfile Using /usr/bin/python3.6 (3.6.10) to create virtualenv… ⠏ Creating virtual environment...Using base prefix '/usr' New python executable in /home/jsnow/.local/share/virtualenvs/python-QepCANQl/bin/python3.6 Also creating executable in /home/jsnow/.local/share/virtualenvs/python-QepCANQl/bin/python Installing setuptools, pip, wheel...done. Running virtualenv with interpreter /usr/bin/python3.6 ✔ Successfully created virtual environment! Virtualenv location: /home/jsnow/.local/share/virtualenvs/python-QepCANQl Installing dependencies from Pipfile.lock (44d7bd)… 17/17 — 00:00:07 To activate this project's virtualenv, run pipenv shell. Alternatively, run
[PULL 27/29] target/arm: Convert Neon VSHLL, VMOVL to decodetree
Convert the VSHLL and VMOVL insns from the 2-reg-shift group
to decodetree. Since the loop always has two passes, we unroll
it to avoid the awkward reassignment of one TCGv to another.
Signed-off-by: Peter Maydell
Reviewed-by: Richard Henderson
Message-id: 20200522145520.6778-8-peter.mayd...@linaro.org
---
target/arm/neon-dp.decode | 16 +++
target/arm/translate-neon.inc.c | 81 +
target/arm/translate.c | 46 +--
3 files changed, 99 insertions(+), 44 deletions(-)
diff --git a/target/arm/neon-dp.decode b/target/arm/neon-dp.decode
index 79d0bfdd70e..3dde699e97e 100644
--- a/target/arm/neon-dp.decode
+++ b/target/arm/neon-dp.decode
@@ -243,6 +243,14 @@ VMINNM_fp_3s 001 1 0 . 1 . ... 1
@3same_fp
&2reg_shift vm=%vm_dp vd=%vd_dp size=1 q=0 \
shift=%neon_rshift_i3
+# Long left shifts: again Q is part of opcode decode
+@2reg_shll_s ... . . . 1 shift:5 0 . . . \
+ &2reg_shift vm=%vm_dp vd=%vd_dp size=2 q=0
+@2reg_shll_h ... . . . 01 shift:4 0 . . . \
+ &2reg_shift vm=%vm_dp vd=%vd_dp size=1 q=0
+@2reg_shll_b ... . . . 001 shift:3 0 . . . \
+ &2reg_shift vm=%vm_dp vd=%vd_dp size=0 q=0
+
VSHR_S_2sh 001 0 1 . .. . . . 1 @2reg_shr_d
VSHR_S_2sh 001 0 1 . .. . . . 1 @2reg_shr_s
VSHR_S_2sh 001 0 1 . .. . . . 1 @2reg_shr_h
@@ -348,3 +356,11 @@ VQSHRN_U16_2sh 001 1 1 . .. 1001 . 0 . 1
@2reg_shrn_h
VQRSHRN_U64_2sh 001 1 1 . .. 1001 . 1 . 1 @2reg_shrn_d
VQRSHRN_U32_2sh 001 1 1 . .. 1001 . 1 . 1 @2reg_shrn_s
VQRSHRN_U16_2sh 001 1 1 . .. 1001 . 1 . 1 @2reg_shrn_h
+
+VSHLL_S_2sh 001 0 1 . .. 1010 . 0 . 1 @2reg_shll_s
+VSHLL_S_2sh 001 0 1 . .. 1010 . 0 . 1 @2reg_shll_h
+VSHLL_S_2sh 001 0 1 . .. 1010 . 0 . 1 @2reg_shll_b
+
+VSHLL_U_2sh 001 1 1 . .. 1010 . 0 . 1 @2reg_shll_s
+VSHLL_U_2sh 001 1 1 . .. 1010 . 0 . 1 @2reg_shll_h
+VSHLL_U_2sh 001 1 1 . .. 1010 . 0 . 1 @2reg_shll_b
diff --git a/target/arm/translate-neon.inc.c b/target/arm/translate-neon.inc.c
index 562470ca08c..3d566044f3d 100644
--- a/target/arm/translate-neon.inc.c
+++ b/target/arm/translate-neon.inc.c
@@ -1578,3 +1578,84 @@ DO_2SN_32(VQSHRN_U16, gen_helper_neon_shl_u16,
gen_helper_neon_narrow_sat_u8)
DO_2SN_64(VQRSHRN_U64, gen_helper_neon_rshl_u64,
gen_helper_neon_narrow_sat_u32)
DO_2SN_32(VQRSHRN_U32, gen_helper_neon_rshl_u32,
gen_helper_neon_narrow_sat_u16)
DO_2SN_32(VQRSHRN_U16, gen_helper_neon_rshl_u16, gen_helper_neon_narrow_sat_u8)
+
+static bool do_vshll_2sh(DisasContext *s, arg_2reg_shift *a,
+ NeonGenWidenFn *widenfn, bool u)
+{
+TCGv_i64 tmp;
+TCGv_i32 rm0, rm1;
+uint64_t widen_mask = 0;
+
+if (!arm_dc_feature(s, ARM_FEATURE_NEON)) {
+return false;
+}
+
+/* UNDEF accesses to D16-D31 if they don't exist. */
+if (!dc_isar_feature(aa32_simd_r32, s) &&
+((a->vd | a->vm) & 0x10)) {
+return false;
+}
+
+if (a->vd & 1) {
+return false;
+}
+
+if (!vfp_access_check(s)) {
+return true;
+}
+
+/*
+ * This is a widen-and-shift operation. The shift is always less
+ * than the width of the source type, so after widening the input
+ * vector we can simply shift the whole 64-bit widened register,
+ * and then clear the potential overflow bits resulting from left
+ * bits of the narrow input appearing as right bits of the left
+ * neighbour narrow input. Calculate a mask of bits to clear.
+ */
+if ((a->shift != 0) && (a->size < 2 || u)) {
+int esize = 8 << a->size;
+widen_mask = MAKE_64BIT_MASK(0, esize);
+widen_mask >>= esize - a->shift;
+widen_mask = dup_const(a->size + 1, widen_mask);
+}
+
+rm0 = neon_load_reg(a->vm, 0);
+rm1 = neon_load_reg(a->vm, 1);
+tmp = tcg_temp_new_i64();
+
+widenfn(tmp, rm0);
+if (a->shift != 0) {
+tcg_gen_shli_i64(tmp, tmp, a->shift);
+tcg_gen_andi_i64(tmp, tmp, ~widen_mask);
+}
+neon_store_reg64(tmp, a->vd);
+
+widenfn(tmp, rm1);
+if (a->shift != 0) {
+tcg_gen_shli_i64(tmp, tmp, a->shift);
+tcg_gen_andi_i64(tmp, tmp, ~widen_mask);
+}
+neon_store_reg64(tmp, a->vd + 1);
+tcg_temp_free_i64(tmp);
+return true;
+}
+
+static bool trans_VSHLL_S_2sh(DisasContext *s, arg_2reg_shift *a)
+{
+NeonGenWidenFn *widenfn[] = {
+gen_helper_neon_widen_s8,
+gen_helper_neon_widen_s16,
+tcg_gen_ext_i32_i64,
+};
+return do_vshll_2sh(s, a, widenfn[a->size], false);
+}
+
+static bool
[PULL 29/29] target/arm: Convert Neon one-register-and-immediate insns to decodetree
Convert the insns in the one-register-and-immediate group to decodetree.
In the new decode, our asimd_imm_const() function returns a 64-bit value
rather than a 32-bit one, which means we don't need to treat cmode=14 op=1
as a special case in the decoder (it is the only encoding where the two
halves of the 64-bit value are different).
Signed-off-by: Peter Maydell
Reviewed-by: Richard Henderson
Message-id: 20200522145520.6778-10-peter.mayd...@linaro.org
---
target/arm/neon-dp.decode | 22 ++
target/arm/translate-neon.inc.c | 118
target/arm/translate.c | 101 +--
3 files changed, 142 insertions(+), 99 deletions(-)
diff --git a/target/arm/neon-dp.decode b/target/arm/neon-dp.decode
index 47a5c90b5d8..bd1b0e13f7d 100644
--- a/target/arm/neon-dp.decode
+++ b/target/arm/neon-dp.decode
@@ -375,3 +375,25 @@ VCVT_SF_2sh 001 0 1 . .. 1110 0 . . 1
@2reg_vcvt
VCVT_UF_2sh 001 1 1 . .. 1110 0 . . 1 @2reg_vcvt
VCVT_FS_2sh 001 0 1 . .. 0 . . 1 @2reg_vcvt
VCVT_FU_2sh 001 1 1 . .. 0 . . 1 @2reg_vcvt
+
+##
+# 1-reg-and-modified-immediate grouping:
+# 001 i 1 D 000 imm:3 Vd:4 cmode:4 0 Q op 1 Vm:4
+##
+
+&1reg_immvd q imm cmode op
+
+%asimd_imm_value 24:1 16:3 0:4
+
+@1reg_imm ... . . . ... ... . q:1 . . \
+ &1reg_imm imm=%asimd_imm_value vd=%vd_dp
+
+# The cmode/op bits here decode VORR/VBIC/VMOV/VMNV, but
+# not in a way we can conveniently represent in decodetree without
+# a lot of repetition:
+# VORR: op=0, (cmode & 1) && cmode < 12
+# VBIC: op=1, (cmode & 1) && cmode < 12
+# VMOV: everything else
+# So we have a single decode line and check the cmode/op in the
+# trans function.
+Vimm_1r 001 . 1 . 000 ... cmode:4 0 . op:1 1 @1reg_imm
diff --git a/target/arm/translate-neon.inc.c b/target/arm/translate-neon.inc.c
index 2a445c7589c..664d3612607 100644
--- a/target/arm/translate-neon.inc.c
+++ b/target/arm/translate-neon.inc.c
@@ -1708,3 +1708,121 @@ DO_FP_2SH(VCVT_SF, gen_helper_vfp_sltos)
DO_FP_2SH(VCVT_UF, gen_helper_vfp_ultos)
DO_FP_2SH(VCVT_FS, gen_helper_vfp_tosls_round_to_zero)
DO_FP_2SH(VCVT_FU, gen_helper_vfp_touls_round_to_zero)
+
+static uint64_t asimd_imm_const(uint32_t imm, int cmode, int op)
+{
+/*
+ * Expand the encoded constant.
+ * Note that cmode = 2,3,4,5,6,7,10,11,12,13 imm=0 is UNPREDICTABLE.
+ * We choose to not special-case this and will behave as if a
+ * valid constant encoding of 0 had been given.
+ * cmode = 15 op = 1 must UNDEF; we assume decode has handled that.
+ */
+switch (cmode) {
+case 0: case 1:
+/* no-op */
+break;
+case 2: case 3:
+imm <<= 8;
+break;
+case 4: case 5:
+imm <<= 16;
+break;
+case 6: case 7:
+imm <<= 24;
+break;
+case 8: case 9:
+imm |= imm << 16;
+break;
+case 10: case 11:
+imm = (imm << 8) | (imm << 24);
+break;
+case 12:
+imm = (imm << 8) | 0xff;
+break;
+case 13:
+imm = (imm << 16) | 0x;
+break;
+case 14:
+if (op) {
+/*
+ * This is the only case where the top and bottom 32 bits
+ * of the encoded constant differ.
+ */
+uint64_t imm64 = 0;
+int n;
+
+for (n = 0; n < 8; n++) {
+if (imm & (1 << n)) {
+imm64 |= (0xffULL << (n * 8));
+}
+}
+return imm64;
+}
+imm |= (imm << 8) | (imm << 16) | (imm << 24);
+break;
+case 15:
+imm = ((imm & 0x80) << 24) | ((imm & 0x3f) << 19)
+| ((imm & 0x40) ? (0x1f << 25) : (1 << 30));
+break;
+}
+if (op) {
+imm = ~imm;
+}
+return dup_const(MO_32, imm);
+}
+
+static bool do_1reg_imm(DisasContext *s, arg_1reg_imm *a,
+GVecGen2iFn *fn)
+{
+uint64_t imm;
+int reg_ofs, vec_size;
+
+if (!arm_dc_feature(s, ARM_FEATURE_NEON)) {
+return false;
+}
+
+/* UNDEF accesses to D16-D31 if they don't exist. */
+if (!dc_isar_feature(aa32_simd_r32, s) && (a->vd & 0x10)) {
+return false;
+}
+
+if (a->vd & a->q) {
+return false;
+}
+
+if (!vfp_access_check(s)) {
+return true;
+}
+
+reg_ofs = neon_reg_offset(a->vd, 0);
+vec_size = a->q ? 16 : 8;
+imm = asimd_imm_const(a->imm, a->cmode, a->op);
+
+fn(MO_64, reg_ofs, reg_ofs, imm, vec_size, vec_size);
+return true;
+}
+
+static void gen_VMOV_1r(unsigned vece, uint32_t dofs, uint32_t aofs,
+int64_t c, uint32_t oprsz, uint32_t maxsz)
[PATCH] MAINTAINERS: Add an entry to review Avocado based acceptance tests
Add an entry for to allow reviewers to be notified when acceptance / integration tests are added or modified. The designated reviewers are not maintainers, subsystem maintainers are expected to merge their tests. Signed-off-by: Philippe Mathieu-Daudé --- MAINTAINERS | 8 1 file changed, 8 insertions(+) diff --git a/MAINTAINERS b/MAINTAINERS index 3e7d9cb0a5..c2ae2bf390 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -2923,6 +2923,14 @@ S: Maintained F: tests/tcg/Makefile F: tests/tcg/Makefile.include +Acceptance (Integration) Testing with the Avocado framework +W: https://trello.com/b/6Qi1pxVn/avocado-qemu +R: Cleber Rosa +R: Philippe Mathieu-Daudé +R: Wainer dos Santos Moschetta +S: Odd Fixes +F: tests/acceptance/ + Documentation - Build system architecture -- 2.21.3
[PULL 26/29] target/arm: Convert Neon narrowing shifts with op==9 to decodetree
Convert the remaining Neon narrowing shifts to decodetree:
* VQSHRN
* VQRSHRN
Signed-off-by: Peter Maydell
Reviewed-by: Richard Henderson
Message-id: 20200522145520.6778-7-peter.mayd...@linaro.org
---
target/arm/neon-dp.decode | 20 ++
target/arm/translate-neon.inc.c | 15 +
target/arm/translate.c | 110 +---
3 files changed, 37 insertions(+), 108 deletions(-)
diff --git a/target/arm/neon-dp.decode b/target/arm/neon-dp.decode
index 8161995aee8..79d0bfdd70e 100644
--- a/target/arm/neon-dp.decode
+++ b/target/arm/neon-dp.decode
@@ -328,3 +328,23 @@ VQSHRUN_16_2sh 001 1 1 . .. 1000 . 0 . 1
@2reg_shrn_h
VQRSHRUN_64_2sh 001 1 1 . .. 1000 . 1 . 1 @2reg_shrn_d
VQRSHRUN_32_2sh 001 1 1 . .. 1000 . 1 . 1 @2reg_shrn_s
VQRSHRUN_16_2sh 001 1 1 . .. 1000 . 1 . 1 @2reg_shrn_h
+
+# VQSHRN with signed input
+VQSHRN_S64_2sh 001 0 1 . .. 1001 . 0 . 1 @2reg_shrn_d
+VQSHRN_S32_2sh 001 0 1 . .. 1001 . 0 . 1 @2reg_shrn_s
+VQSHRN_S16_2sh 001 0 1 . .. 1001 . 0 . 1 @2reg_shrn_h
+
+# VQRSHRN with signed input
+VQRSHRN_S64_2sh 001 0 1 . .. 1001 . 1 . 1 @2reg_shrn_d
+VQRSHRN_S32_2sh 001 0 1 . .. 1001 . 1 . 1 @2reg_shrn_s
+VQRSHRN_S16_2sh 001 0 1 . .. 1001 . 1 . 1 @2reg_shrn_h
+
+# VQSHRN with unsigned input
+VQSHRN_U64_2sh 001 1 1 . .. 1001 . 0 . 1 @2reg_shrn_d
+VQSHRN_U32_2sh 001 1 1 . .. 1001 . 0 . 1 @2reg_shrn_s
+VQSHRN_U16_2sh 001 1 1 . .. 1001 . 0 . 1 @2reg_shrn_h
+
+# VQRSHRN with unsigned input
+VQRSHRN_U64_2sh 001 1 1 . .. 1001 . 1 . 1 @2reg_shrn_d
+VQRSHRN_U32_2sh 001 1 1 . .. 1001 . 1 . 1 @2reg_shrn_s
+VQRSHRN_U16_2sh 001 1 1 . .. 1001 . 1 . 1 @2reg_shrn_h
diff --git a/target/arm/translate-neon.inc.c b/target/arm/translate-neon.inc.c
index fe3fb7f62f3..562470ca08c 100644
--- a/target/arm/translate-neon.inc.c
+++ b/target/arm/translate-neon.inc.c
@@ -1563,3 +1563,18 @@ DO_2SN_32(VQSHRUN_16, gen_helper_neon_shl_s16,
gen_helper_neon_unarrow_sat8)
DO_2SN_64(VQRSHRUN_64, gen_helper_neon_rshl_s64, gen_helper_neon_unarrow_sat32)
DO_2SN_32(VQRSHRUN_32, gen_helper_neon_rshl_s32, gen_helper_neon_unarrow_sat16)
DO_2SN_32(VQRSHRUN_16, gen_helper_neon_rshl_s16, gen_helper_neon_unarrow_sat8)
+DO_2SN_64(VQSHRN_S64, gen_sshl_i64, gen_helper_neon_narrow_sat_s32)
+DO_2SN_32(VQSHRN_S32, gen_sshl_i32, gen_helper_neon_narrow_sat_s16)
+DO_2SN_32(VQSHRN_S16, gen_helper_neon_shl_s16, gen_helper_neon_narrow_sat_s8)
+
+DO_2SN_64(VQRSHRN_S64, gen_helper_neon_rshl_s64,
gen_helper_neon_narrow_sat_s32)
+DO_2SN_32(VQRSHRN_S32, gen_helper_neon_rshl_s32,
gen_helper_neon_narrow_sat_s16)
+DO_2SN_32(VQRSHRN_S16, gen_helper_neon_rshl_s16, gen_helper_neon_narrow_sat_s8)
+
+DO_2SN_64(VQSHRN_U64, gen_ushl_i64, gen_helper_neon_narrow_sat_u32)
+DO_2SN_32(VQSHRN_U32, gen_ushl_i32, gen_helper_neon_narrow_sat_u16)
+DO_2SN_32(VQSHRN_U16, gen_helper_neon_shl_u16, gen_helper_neon_narrow_sat_u8)
+
+DO_2SN_64(VQRSHRN_U64, gen_helper_neon_rshl_u64,
gen_helper_neon_narrow_sat_u32)
+DO_2SN_32(VQRSHRN_U32, gen_helper_neon_rshl_u32,
gen_helper_neon_narrow_sat_u16)
+DO_2SN_32(VQRSHRN_U16, gen_helper_neon_rshl_u16, gen_helper_neon_narrow_sat_u8)
diff --git a/target/arm/translate.c b/target/arm/translate.c
index 11330b92966..883c1a29c7b 100644
--- a/target/arm/translate.c
+++ b/target/arm/translate.c
@@ -3201,40 +3201,6 @@ static inline void gen_neon_unarrow_sats(int size,
TCGv_i32 dest, TCGv_i64 src)
}
}
-static inline void gen_neon_shift_narrow(int size, TCGv_i32 var, TCGv_i32
shift,
- int q, int u)
-{
-if (q) {
-if (u) {
-switch (size) {
-case 1: gen_helper_neon_rshl_u16(var, var, shift); break;
-case 2: gen_helper_neon_rshl_u32(var, var, shift); break;
-default: abort();
-}
-} else {
-switch (size) {
-case 1: gen_helper_neon_rshl_s16(var, var, shift); break;
-case 2: gen_helper_neon_rshl_s32(var, var, shift); break;
-default: abort();
-}
-}
-} else {
-if (u) {
-switch (size) {
-case 1: gen_helper_neon_shl_u16(var, var, shift); break;
-case 2: gen_ushl_i32(var, var, shift); break;
-default: abort();
-}
-} else {
-switch (size) {
-case 1: gen_helper_neon_shl_s16(var, var, shift); break;
-case 2: gen_sshl_i32(var, var, shift); break;
-default: abort();
-}
-}
-}
-}
-
static inline void gen_neon_widen(TCGv_i64 dest, TCGv_i32 src, int size, int u)
{
if (u) {
@@ -5281,6 +5247,7 @@ static int disas_neon_data_insn(DisasContext *s, uint32_t
[PULL 28/29] target/arm: Convert VCVT fixed-point ops to decodetree
Convert the VCVT fixed-point conversion operations in the
Neon 2-regs-and-shift group to decodetree.
Signed-off-by: Peter Maydell
Reviewed-by: Richard Henderson
Message-id: 20200522145520.6778-9-peter.mayd...@linaro.org
---
target/arm/neon-dp.decode | 11 +
target/arm/translate-neon.inc.c | 49 +
target/arm/translate.c | 75 +
3 files changed, 62 insertions(+), 73 deletions(-)
diff --git a/target/arm/neon-dp.decode b/target/arm/neon-dp.decode
index 3dde699e97e..47a5c90b5d8 100644
--- a/target/arm/neon-dp.decode
+++ b/target/arm/neon-dp.decode
@@ -251,6 +251,10 @@ VMINNM_fp_3s 001 1 0 . 1 . ... 1
@3same_fp
@2reg_shll_b ... . . . 001 shift:3 0 . . . \
&2reg_shift vm=%vm_dp vd=%vd_dp size=0 q=0
+# We use size=0 for fp32 and size=1 for fp16 to match the 3-same encodings.
+@2reg_vcvt ... . . . 1 . . q:1 . . \
+ &2reg_shift vm=%vm_dp vd=%vd_dp size=0 shift=%neon_rshift_i5
+
VSHR_S_2sh 001 0 1 . .. . . . 1 @2reg_shr_d
VSHR_S_2sh 001 0 1 . .. . . . 1 @2reg_shr_s
VSHR_S_2sh 001 0 1 . .. . . . 1 @2reg_shr_h
@@ -364,3 +368,10 @@ VSHLL_S_2sh 001 0 1 . .. 1010 . 0 . 1
@2reg_shll_b
VSHLL_U_2sh 001 1 1 . .. 1010 . 0 . 1 @2reg_shll_s
VSHLL_U_2sh 001 1 1 . .. 1010 . 0 . 1 @2reg_shll_h
VSHLL_U_2sh 001 1 1 . .. 1010 . 0 . 1 @2reg_shll_b
+
+# VCVT fixed<->float conversions
+# TODO: FP16 fixed<->float conversions are opc==0b1100 and 0b1101
+VCVT_SF_2sh 001 0 1 . .. 1110 0 . . 1 @2reg_vcvt
+VCVT_UF_2sh 001 1 1 . .. 1110 0 . . 1 @2reg_vcvt
+VCVT_FS_2sh 001 0 1 . .. 0 . . 1 @2reg_vcvt
+VCVT_FU_2sh 001 1 1 . .. 0 . . 1 @2reg_vcvt
diff --git a/target/arm/translate-neon.inc.c b/target/arm/translate-neon.inc.c
index 3d566044f3d..2a445c7589c 100644
--- a/target/arm/translate-neon.inc.c
+++ b/target/arm/translate-neon.inc.c
@@ -1659,3 +1659,52 @@ static bool trans_VSHLL_U_2sh(DisasContext *s,
arg_2reg_shift *a)
};
return do_vshll_2sh(s, a, widenfn[a->size], true);
}
+
+static bool do_fp_2sh(DisasContext *s, arg_2reg_shift *a,
+ NeonGenTwoSingleOPFn *fn)
+{
+/* FP operations in 2-reg-and-shift group */
+TCGv_i32 tmp, shiftv;
+TCGv_ptr fpstatus;
+int pass;
+
+if (!arm_dc_feature(s, ARM_FEATURE_NEON)) {
+return false;
+}
+
+/* UNDEF accesses to D16-D31 if they don't exist. */
+if (!dc_isar_feature(aa32_simd_r32, s) &&
+((a->vd | a->vm) & 0x10)) {
+return false;
+}
+
+if ((a->vm | a->vd) & a->q) {
+return false;
+}
+
+if (!vfp_access_check(s)) {
+return true;
+}
+
+fpstatus = get_fpstatus_ptr(1);
+shiftv = tcg_const_i32(a->shift);
+for (pass = 0; pass < (a->q ? 4 : 2); pass++) {
+tmp = neon_load_reg(a->vm, pass);
+fn(tmp, tmp, shiftv, fpstatus);
+neon_store_reg(a->vd, pass, tmp);
+}
+tcg_temp_free_ptr(fpstatus);
+tcg_temp_free_i32(shiftv);
+return true;
+}
+
+#define DO_FP_2SH(INSN, FUNC) \
+static bool trans_##INSN##_2sh(DisasContext *s, arg_2reg_shift *a) \
+{ \
+return do_fp_2sh(s, a, FUNC); \
+}
+
+DO_FP_2SH(VCVT_SF, gen_helper_vfp_sltos)
+DO_FP_2SH(VCVT_UF, gen_helper_vfp_ultos)
+DO_FP_2SH(VCVT_FS, gen_helper_vfp_tosls_round_to_zero)
+DO_FP_2SH(VCVT_FU, gen_helper_vfp_touls_round_to_zero)
diff --git a/target/arm/translate.c b/target/arm/translate.c
index a9f52049e7c..166349ee203 100644
--- a/target/arm/translate.c
+++ b/target/arm/translate.c
@@ -5193,7 +5193,6 @@ static int disas_neon_data_insn(DisasContext *s, uint32_t
insn)
int q;
int rd, rn, rm, rd_ofs, rn_ofs, rm_ofs;
int size;
-int shift;
int pass;
int u;
int vec_size;
@@ -5234,78 +5233,8 @@ static int disas_neon_data_insn(DisasContext *s,
uint32_t insn)
return 1;
} else if (insn & (1 << 4)) {
if ((insn & 0x00380080) != 0) {
-/* Two registers and shift. */
-op = (insn >> 8) & 0xf;
-
-switch (op) {
-case 0: /* VSHR */
-case 1: /* VSRA */
-case 2: /* VRSHR */
-case 3: /* VRSRA */
-case 4: /* VSRI */
-case 5: /* VSHL, VSLI */
-case 6: /* VQSHLU */
-case 7: /* VQSHL */
-case 8: /* VSHRN, VRSHRN, VQSHRUN, VQRSHRUN */
-case 9: /* VQSHRN, VQRSHRN */
-case 10: /* VSHLL, including VMOVL */
-return 1; /* handled by decodetree
[PULL 22/29] target/arm: Convert Neon VSHR 2-reg-shift insns to decodetree
Convert the VSHR 2-reg-shift insns to decodetree.
Note that unlike the legacy decoder, we present the right shift
amount to the trans_ function as a positive integer.
Signed-off-by: Peter Maydell
Reviewed-by: Richard Henderson
Message-id: 20200522145520.6778-3-peter.mayd...@linaro.org
---
target/arm/neon-dp.decode | 25
target/arm/translate-neon.inc.c | 41 +
target/arm/translate.c | 21 +
3 files changed, 67 insertions(+), 20 deletions(-)
diff --git a/target/arm/neon-dp.decode b/target/arm/neon-dp.decode
index fcce2edacd4..1b877cc68f6 100644
--- a/target/arm/neon-dp.decode
+++ b/target/arm/neon-dp.decode
@@ -208,6 +208,21 @@ VMINNM_fp_3s 001 1 0 . 1 . ... 1
@3same_fp
##
&2reg_shift vm vd q shift size
+# Right shifts are encoded as N - shift, where N is the element size in bits.
+%neon_rshift_i6 16:6 !function=rsub_64
+%neon_rshift_i5 16:5 !function=rsub_32
+%neon_rshift_i4 16:4 !function=rsub_16
+%neon_rshift_i3 16:3 !function=rsub_8
+
+@2reg_shr_d ... . . . .. 1 q:1 . . \
+ &2reg_shift vm=%vm_dp vd=%vd_dp size=3 shift=%neon_rshift_i6
+@2reg_shr_s ... . . . 1 . 0 q:1 . . \
+ &2reg_shift vm=%vm_dp vd=%vd_dp size=2 shift=%neon_rshift_i5
+@2reg_shr_h ... . . . 01 0 q:1 . . \
+ &2reg_shift vm=%vm_dp vd=%vd_dp size=1 shift=%neon_rshift_i4
+@2reg_shr_b ... . . . 001 ... 0 q:1 . . \
+ &2reg_shift vm=%vm_dp vd=%vd_dp size=0 shift=%neon_rshift_i3
+
@2reg_shl_d ... . . . shift:6 1 q:1 . . \
&2reg_shift vm=%vm_dp vd=%vd_dp size=3
@2reg_shl_s ... . . . 1 shift:5 0 q:1 . . \
@@ -217,6 +232,16 @@ VMINNM_fp_3s 001 1 0 . 1 . ... 1
@3same_fp
@2reg_shl_b ... . . . 001 shift:3 0 q:1 . . \
&2reg_shift vm=%vm_dp vd=%vd_dp size=0
+VSHR_S_2sh 001 0 1 . .. . . . 1 @2reg_shr_d
+VSHR_S_2sh 001 0 1 . .. . . . 1 @2reg_shr_s
+VSHR_S_2sh 001 0 1 . .. . . . 1 @2reg_shr_h
+VSHR_S_2sh 001 0 1 . .. . . . 1 @2reg_shr_b
+
+VSHR_U_2sh 001 1 1 . .. . . . 1 @2reg_shr_d
+VSHR_U_2sh 001 1 1 . .. . . . 1 @2reg_shr_s
+VSHR_U_2sh 001 1 1 . .. . . . 1 @2reg_shr_h
+VSHR_U_2sh 001 1 1 . .. . . . 1 @2reg_shr_b
+
VSHL_2sh 001 0 1 . .. 0101 . . . 1 @2reg_shl_d
VSHL_2sh 001 0 1 . .. 0101 . . . 1 @2reg_shl_s
VSHL_2sh 001 0 1 . .. 0101 . . . 1 @2reg_shl_h
diff --git a/target/arm/translate-neon.inc.c b/target/arm/translate-neon.inc.c
index 7f05323fdff..8693b9aa99d 100644
--- a/target/arm/translate-neon.inc.c
+++ b/target/arm/translate-neon.inc.c
@@ -31,6 +31,24 @@ static inline int plus1(DisasContext *s, int x)
return x + 1;
}
+static inline int rsub_64(DisasContext *s, int x)
+{
+return 64 - x;
+}
+
+static inline int rsub_32(DisasContext *s, int x)
+{
+return 32 - x;
+}
+static inline int rsub_16(DisasContext *s, int x)
+{
+return 16 - x;
+}
+static inline int rsub_8(DisasContext *s, int x)
+{
+return 8 - x;
+}
+
/* Include the generated Neon decoder */
#include "decode-neon-dp.inc.c"
#include "decode-neon-ls.inc.c"
@@ -1240,3 +1258,26 @@ static bool do_vector_2sh(DisasContext *s,
arg_2reg_shift *a, GVecGen2iFn *fn)
DO_2SH(VSHL, tcg_gen_gvec_shli)
DO_2SH(VSLI, gen_gvec_sli)
+
+static bool trans_VSHR_S_2sh(DisasContext *s, arg_2reg_shift *a)
+{
+/* Signed shift out of range results in all-sign-bits */
+a->shift = MIN(a->shift, (8 << a->size) - 1);
+return do_vector_2sh(s, a, tcg_gen_gvec_sari);
+}
+
+static void gen_zero_rd_2sh(unsigned vece, uint32_t rd_ofs, uint32_t rm_ofs,
+int64_t shift, uint32_t oprsz, uint32_t maxsz)
+{
+tcg_gen_gvec_dup_imm(vece, rd_ofs, oprsz, maxsz, 0);
+}
+
+static bool trans_VSHR_U_2sh(DisasContext *s, arg_2reg_shift *a)
+{
+/* Shift out of range is architecturally valid and results in zero. */
+if (a->shift >= (8 << a->size)) {
+return do_vector_2sh(s, a, gen_zero_rd_2sh);
+} else {
+return do_vector_2sh(s, a, tcg_gen_gvec_shri);
+}
+}
diff --git a/target/arm/translate.c b/target/arm/translate.c
index 41fef49dbe7..4acc94e3cbc 100644
--- a/target/arm/translate.c
+++ b/target/arm/translate.c
@@ -5296,6 +5296,7 @@ static int disas_neon_data_insn(DisasContext *s, uint32_t
insn)
op = (insn >> 8) & 0xf;
switch (op) {
+case 0: /* VSHR */
