Re: [PATCH v2 06/15] ppc/vof: Fix unaligned FDT property access
On Fri, Jun 28, 2024 at 04:20:02PM +0100, Peter Maydell wrote:
> On Thu, 27 Jun 2024 at 14:39, Akihiko Odaki wrote:
> >
> > FDT properties are aligned by 4 bytes, not 8 bytes.
> >
> > Signed-off-by: Akihiko Odaki
> > ---
> > hw/ppc/vof.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/hw/ppc/vof.c b/hw/ppc/vof.c
> > index e3b430a81f4f..b5b6514d79fc 100644
> > --- a/hw/ppc/vof.c
> > +++ b/hw/ppc/vof.c
> > @@ -646,7 +646,7 @@ static void vof_dt_memory_available(void *fdt, GArray
> > *claimed, uint64_t base)
> > mem0_reg = fdt_getprop(fdt, offset, "reg", );
> > g_assert(mem0_reg && proplen == sizeof(uint32_t) * (ac + sc));
> > if (sc == 2) {
> > -mem0_end = be64_to_cpu(*(uint64_t *)(mem0_reg + sizeof(uint32_t) *
> > ac));
> > +mem0_end = ldq_be_p(mem0_reg + sizeof(uint32_t) * ac);
> > } else {
> > mem0_end = be32_to_cpu(*(uint32_t *)(mem0_reg + sizeof(uint32_t) *
> > ac));
> > }
>
> I did wonder if there was a better way to do what this is doing,
> but neither we (in system/device_tree.c) nor libfdt seem to
> provide one.
libfdt does provide unaligned access helpers (fdt32_ld() etc.), but
not an automatic aligned-or-unaligned helper. Maybe we should add that?
--
David Gibson (he or they) | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you, not the other way
| around.
http://www.ozlabs.org/~dgibson
signature.asc
Description: PGP signature
[PATCH v2] util/cpuinfo-ppc: Add FreeBSD support
util/cpuinfo-ppc: Add FreeBSD support Signed-off-by: Brad Smith --- v2: Use ifndef with PPC_FEATURE2_ARCH_3_1 util/cpuinfo-ppc.c | 9 - 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/util/cpuinfo-ppc.c b/util/cpuinfo-ppc.c index 47af55aa0c..f0b9b895f1 100644 --- a/util/cpuinfo-ppc.c +++ b/util/cpuinfo-ppc.c @@ -14,6 +14,13 @@ # include "elf.h" # endif #endif +#ifdef __FreeBSD__ +# include +# ifndef PPC_FEATURE2_ARCH_3_1 +# define PPC_FEATURE2_ARCH_3_10 +# endif +# define PPC_FEATURE2_VEC_CRYPTO PPC_FEATURE2_HAS_VEC_CRYPTO +#endif unsigned cpuinfo; @@ -28,7 +35,7 @@ unsigned __attribute__((constructor)) cpuinfo_init(void) info = CPUINFO_ALWAYS; -#ifdef CONFIG_LINUX +#if defined(CONFIG_LINUX) || defined(__FreeBSD__) unsigned long hwcap = qemu_getauxval(AT_HWCAP); unsigned long hwcap2 = qemu_getauxval(AT_HWCAP2); -- 2.45.2
Re: [PATCH] util/cpuinfo-ppc: Add FreeBSD support
On 2024-06-28 12:19 p.m., Richard Henderson wrote: On 6/27/24 19:00, Brad Smith wrote: util/cpuinfo-ppc: Add FreeBSD support Signed-off-by: Brad Smith --- With corrected sign-off. Also this was based on the tcg-next branch. util/cpuinfo-ppc.c | 7 ++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/util/cpuinfo-ppc.c b/util/cpuinfo-ppc.c index 47af55aa0c..0ad634b46f 100644 --- a/util/cpuinfo-ppc.c +++ b/util/cpuinfo-ppc.c @@ -14,6 +14,11 @@ # include "elf.h" # endif #endif +#ifdef __FreeBSD__ +# include +# define PPC_FEATURE2_ARCH_3_1 0 I assume freebsd will eventually add this bit. Possibly. The other flags are mostly in sync with the Linux flags. There is no Power 10 support so far. Perhaps better with ifndef? I'll do so just in case. r~ +# define PPC_FEATURE2_VEC_CRYPTO PPC_FEATURE2_HAS_VEC_CRYPTO +#endif unsigned cpuinfo; @@ -28,7 +33,7 @@ unsigned __attribute__((constructor)) cpuinfo_init(void) info = CPUINFO_ALWAYS; -#ifdef CONFIG_LINUX +#if defined(CONFIG_LINUX) || defined(__FreeBSD__) unsigned long hwcap = qemu_getauxval(AT_HWCAP); unsigned long hwcap2 = qemu_getauxval(AT_HWCAP2);
Re: [PATCH 02/23] target/i386: fix gen_prepare_size_nz condition
On 6/28/24 10:54, Richard Henderson wrote:
On 6/28/24 05:42, Alex Bennée wrote:
Incorrect brace positions causes an unintended overflow on 32 bit
builds and shenanigans result.
Resolves: https://gitlab.com/qemu-project/qemu/-/issues/2413
Suggested-by: Mark Cave-Ayland
Signed-off-by: Alex Bennée
---
target/i386/tcg/translate.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/target/i386/tcg/translate.c b/target/i386/tcg/translate.c
index ad1819815a..94f13541c3 100644
--- a/target/i386/tcg/translate.c
+++ b/target/i386/tcg/translate.c
@@ -877,7 +877,7 @@ static CCPrepare gen_prepare_sign_nz(TCGv src, MemOp size)
return (CCPrepare) { .cond = TCG_COND_LT, .reg = src };
} else {
return (CCPrepare) { .cond = TCG_COND_TSTNE, .reg = src,
- .imm = 1ull << ((8 << size) - 1) };
+ .imm = (1ull << (8 << size)) - 1 };
This is incorrect -- we want only to test the sign bit.
Perhaps MAKE_64BIT_MASK((8 << size) - 1, 1) would make this more explicit?
I'll have a quick look at the issue and see if I can reproduce.
I can't get the cdrom test to run at all; I have no idea why.
1/1 qemu:qtest+qtest-x86_64 / qtest-x86_64/cdrom-testSKIP
0.00s
But
QTEST_QEMU_BINARY='./qemu-system-x86_64' ./tests/qtest/bios-tables-test -v -p
/x86_64/acpi/q35/mmio64
fails for me, and is resolved at 15957eb9e by reverting
diff --git a/target/i386/tcg/translate.c b/target/i386/tcg/translate.c
index 4735f084d4..022469845e 100644
--- a/target/i386/tcg/translate.c
+++ b/target/i386/tcg/translate.c
@@ -1084,13 +1084,8 @@ static CCPrepare gen_prepare_eflags_z(DisasContext *s,
TCGv reg)
default:
{
MemOp size = (s->cc_op - CC_OP_ADDB) & 3;
-if (size == MO_TL) {
-return (CCPrepare) { .cond = TCG_COND_EQ, .reg = cpu_cc_dst,
- .mask = -1 };
-} else {
-return (CCPrepare) { .cond = TCG_COND_TSTEQ, .reg = cpu_cc_dst,
- .mask = -1, .imm = (1ull << (8 << size))
- 1 };
-}
+TCGv t0 = gen_ext_tl(reg, cpu_cc_dst, size, false);
+return (CCPrepare) { .cond = TCG_COND_EQ, .reg = t0, .mask = -1 };
}
}
}
I fought all afternoon to try and debug this, but was defeated by qtest.
I really wish we could change our tooling to simplify debugging.
r~
Re: [PATCH 10/23] plugins/lockstep: make mixed-mode safe
On 6/28/24 05:42, Alex Bennée wrote:
The ExecState is shared across the socket and if we want to compare
say 64 bit and 32 bit binaries we need the two to use the same sizes
for things.
Signed-off-by: Alex Bennée
---
contrib/plugins/lockstep.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/contrib/plugins/lockstep.c b/contrib/plugins/lockstep.c
index 111ec3fa27..761bcdf363 100644
--- a/contrib/plugins/lockstep.c
+++ b/contrib/plugins/lockstep.c
@@ -57,7 +57,7 @@ typedef struct {
/* The execution state we compare */
typedef struct {
uint64_t pc;
-unsigned long insn_count;
+uint64_t insn_count;
} ExecState;
Or long long, but I suppose this is more explicit about the width.
Reviewed-by: Richard Henderson
r~
typedef struct {
@@ -148,7 +148,7 @@ static void report_divergance(ExecState *us, ExecState
*them)
g_string_printf(out,
"Δ insn_count @ 0x%016" PRIx64
-" (%ld) vs 0x%016" PRIx64 " (%ld)\n",
+" (%"PRId64") vs 0x%016" PRIx64 " (%"PRId64")\n",
us->pc, us->insn_count, them->pc, them->insn_count);
for (entry = log, i = 0;
Re: [RFC PATCH v3 2/5] rust: add bindgen step as a meson dependency
On Fri, Jun 28, 2024 at 9:12 PM Pierrick Bouvier wrote: > However, even tough I can build the executable, I get this error: > $ ./build/qemu-system-aarch64 -M virt > C:\w\qemu\build\qemu-system-aarch64.exe: unknown type 'x-pl011-rust' > > Any idea of what could be missing here? Maybe the underlying mechanism to invoke constructors is different? Perhaps we could use https://crates.io/crates/ctor instead? Paolo
Re: [PATCH v4] hw/core/loader: allow loading larger ROMs
On Fri, Jun 28, 2024 at 11:27:06AM -0700, Gregor Haas wrote: > The read() syscall is not guaranteed to return all data from a file. The > default ROM loader implementation currently does not take this into account, > instead failing if all bytes are not read at once. This change loads the ROM > using g_file_get_contents() instead, which correctly reads all data using > multiple calls to read() while also returning the loaded ROM size. > > Signed-off-by: Gregor Haas > --- > hw/core/loader.c | 30 +- > 1 file changed, 5 insertions(+), 25 deletions(-) Reviewed-by: Daniel P. Berrangé With regards, Daniel -- |: https://berrange.com -o-https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o-https://fstop138.berrange.com :| |: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|
[PATCH v3 0/2] Consider discard option when writing zeros
Punch holes only when the image is opened with discard=on or discard=unmap. Tested by: - new write-zeroes-unmap iotest on xfs, ext4, and tmpfs - tests/qemu-iotests/check -raw - tests/qemu-iotests/check -qcow2 Changes since v2 - Add write-zeroes-unmap iotest - Fix iotest missing discard=unmap v2 was here: https://lists.nongnu.org/archive/html/qemu-block/2024-06/msg00231.html Nir Soffer (2): qemu-iotest/245: Add missing discard=unmap Consider discard option when writing zeros block/io.c| 9 +- tests/qemu-iotests/245| 2 +- tests/qemu-iotests/tests/write-zeroes-unmap | 127 ++ .../qemu-iotests/tests/write-zeroes-unmap.out | 81 +++ 4 files changed, 214 insertions(+), 5 deletions(-) create mode 100755 tests/qemu-iotests/tests/write-zeroes-unmap create mode 100644 tests/qemu-iotests/tests/write-zeroes-unmap.out -- 2.45.2
[PATCH v3 1/2] qemu-iotest/245: Add missing discard=unmap
The test works since we punch holes by default even when opening the
image without discard=on or discard=unmap. Fix the test to enable
discard.
---
tests/qemu-iotests/245 | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tests/qemu-iotests/245 b/tests/qemu-iotests/245
index a934c9d1e6..f96610f510 100755
--- a/tests/qemu-iotests/245
+++ b/tests/qemu-iotests/245
@@ -590,11 +590,11 @@ class TestBlockdevReopen(iotests.QMPTestCase):
# Insert (and remove) a compress filter
@iotests.skip_if_unsupported(['compress'])
def test_insert_compress_filter(self):
# Add an image to the VM: hd (raw) -> hd0 (qcow2) -> hd0-file (file)
-opts = {'driver': 'raw', 'node-name': 'hd', 'file': hd_opts(0)}
+opts = {'driver': 'raw', 'node-name': 'hd', 'file': hd_opts(0),
'discard': 'unmap'}
self.vm.cmd('blockdev-add', conv_keys = False, **opts)
# Add a 'compress' filter
filter_opts = {'driver': 'compress',
'node-name': 'compress0',
--
2.45.2
[PATCH v3 2/2] Consider discard option when writing zeros
When opening an image with discard=off, we punch hole in the image when
writing zeroes, making the image sparse. This breaks users that want to
ensure that writes cannot fail with ENOSPACE by using fully allocated
images[1].
bdrv_co_pwrite_zeroes() correctly disables BDRV_REQ_MAY_UNMAP if we
opened the child without discard=unmap or discard=on. But we don't go
through this function when accessing the top node. Move the check down
to bdrv_co_do_pwrite_zeroes() which seems to be used in all code paths.
This change implements the documented behavior, punching holes only when
opening the image with discard=on or discard=unmap. This may not be the
best default but can improve it later.
The test depends on a file system supporting discard, deallocating the
entire file when punching hole with the length of the entire file.
Tested with xfs, ext4, and tmpfs.
[1] https://lists.nongnu.org/archive/html/qemu-discuss/2024-06/msg3.html
Signed-off-by: Nir Soffer
---
block/io.c| 9 +-
tests/qemu-iotests/tests/write-zeroes-unmap | 127 ++
.../qemu-iotests/tests/write-zeroes-unmap.out | 81 +++
3 files changed, 213 insertions(+), 4 deletions(-)
create mode 100755 tests/qemu-iotests/tests/write-zeroes-unmap
create mode 100644 tests/qemu-iotests/tests/write-zeroes-unmap.out
diff --git a/block/io.c b/block/io.c
index 7217cf811b..301514c880 100644
--- a/block/io.c
+++ b/block/io.c
@@ -1860,10 +1860,15 @@ bdrv_co_do_pwrite_zeroes(BlockDriverState *bs, int64_t
offset, int64_t bytes,
/* By definition there is no user buffer so this flag doesn't make sense */
if (flags & BDRV_REQ_REGISTERED_BUF) {
return -EINVAL;
}
+/* If opened with discard=off we should never unmap. */
+if (!(bs->open_flags & BDRV_O_UNMAP)) {
+flags &= ~BDRV_REQ_MAY_UNMAP;
+}
+
/* Invalidate the cached block-status data range if this write overlaps */
bdrv_bsc_invalidate_range(bs, offset, bytes);
assert(alignment % bs->bl.request_alignment == 0);
head = offset % alignment;
@@ -2313,14 +2318,10 @@ int coroutine_fn bdrv_co_pwrite_zeroes(BdrvChild
*child, int64_t offset,
{
IO_CODE();
trace_bdrv_co_pwrite_zeroes(child->bs, offset, bytes, flags);
assert_bdrv_graph_readable();
-if (!(child->bs->open_flags & BDRV_O_UNMAP)) {
-flags &= ~BDRV_REQ_MAY_UNMAP;
-}
-
return bdrv_co_pwritev(child, offset, bytes, NULL,
BDRV_REQ_ZERO_WRITE | flags);
}
/*
diff --git a/tests/qemu-iotests/tests/write-zeroes-unmap
b/tests/qemu-iotests/tests/write-zeroes-unmap
new file mode 100755
index 00..7cfeeaf839
--- /dev/null
+++ b/tests/qemu-iotests/tests/write-zeroes-unmap
@@ -0,0 +1,127 @@
+#!/usr/bin/env bash
+# group: quick
+#
+# Test write zeros unmap.
+#
+# Copyright (C) Red Hat, Inc.
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 2 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program. If not, see
Re: [PATCH v2] Consider discard option when writing zeros
On Thu, Jun 27, 2024 at 2:42 PM Kevin Wolf wrote:
> Am 26.06.2024 um 18:27 hat Nir Soffer geschrieben:
> > On Wed, Jun 26, 2024 at 12:17 PM Daniel P. Berrangé >
> > wrote:
> >
> > > On Mon, Jun 24, 2024 at 06:08:26PM +0200, Kevin Wolf wrote:
> > > > Am 24.06.2024 um 17:23 hat Stefan Hajnoczi geschrieben:
> > > > > On Wed, Jun 19, 2024 at 08:43:25PM +0300, Nir Soffer wrote:
> > > > > > Tested using:
> > > > >
> > > > > Hi Nir,
> > > > > This looks like a good candidate for the qemu-iotests test suite.
> > > Adding
> > > > > it to the automated tests will protect against future regressions.
> > > > >
> > > > > Please add the script and the expected output to
> > > > > tests/qemu-iotests/test/write-zeroes-unmap and run it using
> > > > > `(cd build && tests/qemu-iotests/check write-zeroes-unmap)`.
> > > > >
> > > > > See the existing test cases in tests/qemu-iotests/ and
> > > > > tests/qemu-iotests/tests/ for examples. Some are shell scripts and
> > > > > others are Python. I think shell makes sense for this test case.
> You
> > > > > can copy the test framework boilerplate from an existing test case.
> > > >
> > > > 'du' can't be used like this in qemu-iotests because it makes
> > > > assumptions that depend on the filesystem. A test case replicating
> what
> > > > Nir did manually would likely fail on XFS with its preallocation.
> > > >
> > > > Maybe we could operate on a file exposed by the FUSE export that is
> > > > backed by qcow2, and then you can use 'qemu-img map' on that qcow2
> image
> > > > to verify the allocation status. Somewhat complicated, but I think it
> > > > could work.
> > >
> > > A simpler option would be to use 'du' but with a fuzzy range test,
> > > rather than an exact equality test.
> > >
> > > For the tests which write 1 MB, check the 'du' usage is "at least 1MB",
> > > for the tests which expect to unmap blocks, check that the 'du' usage
> > > is "less than 256kb". This should be within bounds of xfs speculative
> > > allocation.
> >
> > This should work, I'll start with this approach.
>
> If we're okay with accepting tests that depend on filesystem behaviour,
> then 'qemu-img map -f raw --output=json' should be the less risky
> approach than checking 'du'.
>
Unfortunately it does not work since qemu-img map and qemu-nbd reports the
allocated
area as zero area with no data.
I tried this:
$ cat test-print-allocation.sh
#!/bin/sh
qemu=${1:?Usage: $0 qemu-executable}
img=/tmp/qemu-test-unmap.img
echo
echo "discard=unmap - write zeroes"
fallocate -l 1m $img
echo -e 'qemu-io none0 "write -z 0 1m"\nquit' | $qemu -monitor stdio \
-drive if=none,file=$img,format=raw,discard=unmap >/dev/null
echo "du:"
du -sh $img
echo "qemu-img map:"
qemu-img map -f raw --output json $img
echo "nbdinfo --map:"
nbdinfo --map -- [ qemu-nbd -r -f raw $img ]
echo
echo "discard=unmap - write zeroes unmap"
fallocate -l 1m $img
echo -e 'qemu-io none0 "write -zu 0 1m"\nquit' | $qemu -monitor stdio \
-drive if=none,file=$img,format=raw,discard=unmap >/dev/null
echo "du:"
du -sh $img
echo "qemu-img map:"
qemu-img map -f raw --output json $img
echo "nbdinfo --map:"
nbdinfo --map -- [ qemu-nbd -r -f raw $img ]
rm $img
$ ./test-print-allocation.sh ./qemu-system-x86_64
discard=unmap - write zeroes
du:
1.0M /tmp/qemu-test-unmap.img
qemu-img map:
[{ "start": 0, "length": 1048576, "depth": 0, "present": true, "zero":
true, "data": false, "offset": 0}]
nbdinfo --map:
0 10485763 hole,zero
discard=unmap - write zeroes unmap
du:
0 /tmp/qemu-test-unmap.img
qemu-img map:
[{ "start": 0, "length": 1048576, "depth": 0, "present": true, "zero":
true, "data": false, "offset": 0}]
nbdinfo --map:
0 10485763 hole,zero
Re: [RFC PATCH v3 2/5] rust: add bindgen step as a meson dependency
On 6/27/24 16:47, Pierrick Bouvier wrote:
On 6/25/24 11:08, Manos Pitsidianakis wrote:
On Tue, 25 Jun 2024 19:00, Zhao Liu wrote:
[snip]
This is for future-proofing the Rust integration in general. I
haven't been
able to compile under macos yet because bindgen cannot find the system clang
header. I also don't have a windows pc to test it on. But it should work
theoretically under all three.
Yes, they should work. EMM, but there is no particular need for them at
the moment, so just to be safe, we can put these two platforms on hold
for now, and they can be easily added when the tests are covered.
A TODO can remind support for them.
I'm still trying to figure out why bindgen doesn't find the /Library/***
include paths.. it's frustrating! I will remove them if I don't succeed
and also no one volunteers to attempt a windows build. :)
I'm currently doing it, and managed to run until bindgen step. Same
problem that you found on MacOS, it can't locate some headers
(strings.h, included from osdep.h). I'll try to dig into this, but if
you found a solution already, you're welcome to share it.
'gcc | grep' command you used should work, but should be adapted because
windows paths start with C:/ instead of /.
I've been able to build rust device on windows, with a few tweaks needed.
- specificy the target for libclang (used by bindgen), which targets
MSVC by default (so different set of headers)
- additional headers (libclang searches its own header with a relative
path instead of absolute)
- additional windows libs that must be linked in final executable
However, even tough I can build the executable, I get this error:
$ ./build/qemu-system-aarch64 -M virt
C:\w\qemu\build\qemu-system-aarch64.exe: unknown type 'x-pl011-rust'
Any idea of what could be missing here?
By the way, I noticed configure --enable-with-rust does not trigger
error when not finding cargo, it just deactivates rust support, which is
probably not what is expected.
---
QEMU Build instructions for windows are here:
https://wiki.qemu.org/Hosts/W32#Native_builds_with_MSYS2
Additional steps needed:
$ cargo install bindgen-cli
$ export PATH=/c/Users/user/.cargo/bin/:$PATH
$ wget
https://github.com/llvm/llvm-project/releases/download/llvmorg-18.1.6/LLVM-18.1.6-win64.exe
# for libclang.dll
$ pacman -S p7zip
$ mkdir llvm && cd llvm && 7z x ../LLVM-18.1.6-win64.exe && cd ..
$ export LIBCLANG_PATH=$(cygpath -m $(pwd)/llvm/bin/libclang.dll)
Additional libs to link can be found with:
$ touch empty.rs
$ rustc empty.rs --print=native-static-libs --crate-type=staticlib
note: Link against the following native artifacts when linking against
this static library. The order and any duplication can be significant on so
me platforms.
note: native-static-libs: -lkernel32 -ladvapi32 -lkernel32 -lntdll
-luserenv -lws2_32 -lkernel32 -lws2_32 -lkernel32
---
diff --git a/meson.build b/meson.build
index ca40a39ad7e..98faa4777b7 100644
--- a/meson.build
+++ b/meson.build
@@ -3896,7 +3896,8 @@ foreach target : target_dirs
input: copy,
dependencies: arch_deps + lib_deps,
output: target + '-generated.rs',
-include_directories: include_directories('.', 'include'),
+include_directories: include_directories('.', 'include',
+'llvm/lib/clang/18/include/'),
args: [
'--ctypes-prefix', 'core::ffi',
'--formatter', 'rustfmt',
@@ -3910,7 +3911,10 @@ foreach target : target_dirs
'--with-derive-default',
'--allowlist-file', meson.project_source_root() + '/include/.*',
'--allowlist-file', meson.project_source_root() + '/.*',
- '--allowlist-file', meson.project_build_root() + '/.*'
+ '--allowlist-file', meson.project_build_root() + '/.*',
+],
+c_args: [
+ '--target=x86_64-pc-windows-gnu'
],
)
@@ -3925,7 +3929,12 @@ foreach target : target_dirs
rust_dep = declare_dependency(link_args: [
'-Wl,--whole-archive',
t['output-path'],
- '-Wl,--no-whole-archive'
+ '-Wl,--no-whole-archive',
+ '-lkernel32',
+ '-ladvapi32',
+ '-lntdll',
+ '-luserenv',
+ '-lws2_32',
],
sources: [rust_device_cargo])
rust_hw.add(rust_dep)
[PATCH v4] hw/core/loader: allow loading larger ROMs
The read() syscall is not guaranteed to return all data from a file. The
default ROM loader implementation currently does not take this into account,
instead failing if all bytes are not read at once. This change loads the ROM
using g_file_get_contents() instead, which correctly reads all data using
multiple calls to read() while also returning the loaded ROM size.
Signed-off-by: Gregor Haas
---
hw/core/loader.c | 30 +-
1 file changed, 5 insertions(+), 25 deletions(-)
diff --git a/hw/core/loader.c b/hw/core/loader.c
index 2f8105d7de..4a5714 100644
--- a/hw/core/loader.c
+++ b/hw/core/loader.c
@@ -1075,8 +1075,7 @@ ssize_t rom_add_file(const char *file, const char *fw_dir,
{
MachineClass *mc = MACHINE_GET_CLASS(qdev_get_machine());
Rom *rom;
-ssize_t rc;
-int fd = -1;
+g_autoptr(GError) gerr = NULL;
char devpath[100];
if (as && mr) {
@@ -1094,35 +1093,19 @@ ssize_t rom_add_file(const char *file, const char
*fw_dir,
rom->path = g_strdup(file);
}
-fd = open(rom->path, O_RDONLY | O_BINARY);
-if (fd == -1) {
-fprintf(stderr, "Could not open option rom '%s': %s\n",
-rom->path, strerror(errno));
-goto err;
-}
-
if (fw_dir) {
rom->fw_dir = g_strdup(fw_dir);
rom->fw_file = g_strdup(file);
}
rom->addr = addr;
-rom->romsize = lseek(fd, 0, SEEK_END);
-if (rom->romsize == -1) {
-fprintf(stderr, "rom: file %-20s: get size error: %s\n",
-rom->name, strerror(errno));
+if (!g_file_get_contents(rom->path, (gchar **) >data,
+ >romsize, )) {
+fprintf(stderr, "rom: file %-20s: error %s\n",
+rom->name, gerr->message);
goto err;
}
rom->datasize = rom->romsize;
-rom->data = g_malloc0(rom->datasize);
-lseek(fd, 0, SEEK_SET);
-rc = read(fd, rom->data, rom->datasize);
-if (rc != rom->datasize) {
-fprintf(stderr, "rom: file %-20s: read error: rc=%zd (expected %zd)\n",
-rom->name, rc, rom->datasize);
-goto err;
-}
-close(fd);
rom_insert(rom);
if (rom->fw_file && fw_cfg) {
const char *basename;
@@ -1159,9 +1142,6 @@ ssize_t rom_add_file(const char *file, const char *fw_dir,
return 0;
err:
-if (fd != -1)
-close(fd);
-
rom_free(rom);
return -1;
}
--
2.45.2
Re: [PATCH v6 06/11] target/arm: Factor out code for setting MTE TCF0 field
Hi Richard, On 6/28/24 2:00 PM, Richard Henderson wrote: On 6/28/24 08:49, Gustavo Romero wrote: I thought you meant osdep.h should not be included _at all_ in my case, either in mte_user_helper.h or in mte_user_helper.c. Maybe the wording in the docs should be "Do not include "qemu/osdep.h" from header files. Include it from .c files, when necessary.". Not "when necessary", always, and always first. Got it! See the "Include directives" section of docs/devel/style.rst, which does explicitly say 'Do not include "qemu/osdep.h" from header files'. Yep, Phil pointed out this doc when we were discussing it in v5. I was actually referring to it about the wording. Maybe then it should be more explicitly that osdep.h _always_ has to be present. Re-reading it after your clarifications makes it clear, but the first time Phil pointed it out the phrases: "[...] since the .c file will have already included it." and "Headers should normally include everything they need beyond osdep.h." weren't enough to me to make it clear that osdep.h must always be included (present) in the .c files. "will have already included" sounded ambiguous to me, more like, if necessary it would have already be included in .c (but not always). But, well, that can be a falt in my interpretation.. Thanks a lot for the clarification. I think we agree osdep.h is necessary and must be put in mte_user_helper.c. But that left me wondering how it would work for sources including mte_user_helper.h, because it can be the case they don't have the declarations for the types used in the function prototypes, in this case, for CPUArchState and abi_long types in arm_set_mte_tcf0. CPUArchState will come from qemu/typedefs.h via osdep.h. For this particular function, 'int' would have been enough, since we only care about the low two bits. hmm, right. I'll send a follow up patch to improve it since Alex already picked up the series to gdbstub/next. Thanks! Cheers, Gustavo
Re: [PATCH 04/23] tracepoints: move physmem trace points
On 6/28/24 05:42, Alex Bennée wrote:
@@ -1885,7 +1885,7 @@ static void ram_block_add(RAMBlock *new_block, Error
**errp)
} else { /* list is empty */
QLIST_INSERT_HEAD_RCU(_list.blocks, new_block, next);
}
-ram_list.mru_block = NULL;
+qatomic_rcu_set(_list.mru_block, NULL);
/* Write list before version */
smp_wmb();
This is unrelated to tracepoints.
Otherwise,
Reviewed-by: Richard Henderson
r~
Re: [PATCH 01/23] tests/lcitool: fix debian-i686-cross toolchain prefix
On 6/28/24 05:42, Alex Bennée wrote: I guess we never noticed and tried to build with this cross image. Fix the toolchain prefix so we actually build 32 bit images. Signed-off-by: Alex Bennée --- tests/docker/dockerfiles/debian-i686-cross.docker | 2 +- tests/lcitool/refresh | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) OMG. Reviewed-by: Richard Henderson r~
Re: [PATCH 1/3] target/riscv: zimop and zcmop extension for riscv
Please ignore this one.
Wrong command and all patches came out as one.
Sorry about that.
On Fri, Jun 28, 2024 at 10:50 AM Deepak Gupta wrote:
>
> `zimop` stands for `may be operations`. `zcmop` stands for compressed
> `may be operations`. For some RISC-V CPU extension, once compiled into
> the binary are part of generated code which can't be gated behind a probe
> of whether an instruction set is supported or not. One such example is
> `zicfiss` [1] extension where `shadow stack push` and `shadow stack pop
> and check` will be part of every function body. Thus binaries compiled
> with such extensions need to run in following scenarios
>
> - On machines where extension is present and enabled
> - On machines where extension is present and disabled
> - On machines where extension is not present/implemented.
>
> `zimop` (for 32bit instructions) and `zcmop` (for compressed) were devised
> and defined [2] to support such future (like zicfiss) CPU extensions
> where zimops and zcmops provide a base non-faulting behavior for
> codepoints that may claimed by future ISA extensions. Minimally, any
> CPU implementation wanting to have binary compatibility with such
> binaries only has to implement `zimop and zcmop`. Furthermore, this
> allows per-task optin for software where user has the option to enable
> the feature on per-task basis.
>
> `zimop` are defined to write zero to `rd`. `zcmop` are defined to *not* write
> to any register.
>
> [1] - https://github.com/riscv/riscv-cfi/blob/main/src/cfi_backward.adoc
> [2] - https://github.com/riscv/riscv-isa-manual/blob/main/src/zimop.adoc
>
> Signed-off-by: Deepak Gupta
> ---
> target/riscv/cpu.c | 2 ++
> target/riscv/cpu_cfg.h | 1 +
> 2 files changed, 3 insertions(+)
>
> diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
> index eb1a2e7d6d..3caf8553d1 100644
> --- a/target/riscv/cpu.c
> +++ b/target/riscv/cpu.c
> @@ -113,6 +113,7 @@ const RISCVIsaExtData isa_edata_arr[] = {
> ISA_EXT_DATA_ENTRY(zihintntl, PRIV_VERSION_1_10_0, ext_zihintntl),
> ISA_EXT_DATA_ENTRY(zihintpause, PRIV_VERSION_1_10_0, ext_zihintpause),
> ISA_EXT_DATA_ENTRY(zihpm, PRIV_VERSION_1_12_0, ext_zihpm),
> +ISA_EXT_DATA_ENTRY(zimops, PRIV_VERSION_1_12_0, ext_zimops),
> ISA_EXT_DATA_ENTRY(zmmul, PRIV_VERSION_1_12_0, ext_zmmul),
> ISA_EXT_DATA_ENTRY(za64rs, PRIV_VERSION_1_12_0, has_priv_1_11),
> ISA_EXT_DATA_ENTRY(zaamo, PRIV_VERSION_1_12_0, ext_zaamo),
> @@ -2273,6 +2274,7 @@ static Property riscv_cpu_properties[] = {
> * it with -x and default to 'false'.
> */
> DEFINE_PROP_BOOL("x-misa-w", RISCVCPU, cfg.misa_w, false),
> +DEFINE_PROP_BOOL("zimops", RISCVCPU, cfg.ext_zimops, true),
> DEFINE_PROP_END_OF_LIST(),
> };
>
> diff --git a/target/riscv/cpu_cfg.h b/target/riscv/cpu_cfg.h
> index cb750154bd..5c42ff8cdf 100644
> --- a/target/riscv/cpu_cfg.h
> +++ b/target/riscv/cpu_cfg.h
> @@ -124,6 +124,7 @@ struct RISCVCPUConfig {
> uint32_t mvendorid;
> uint64_t marchid;
> uint64_t mimpid;
> +bool ext_zimops;
>
> /* Named features */
> bool ext_svade;
> --
> 2.34.1
>
>
> From 4d15b0e0037f444eb75e60b398e19dcf476f07d4 Mon Sep 17 00:00:00 2001
> From: Deepak Gupta
> Date: Fri, 28 Jun 2024 00:13:58 -0700
> Subject: [PATCH 2/3] target/riscv: zimop instruction encoding and its
> implementation
>
> This patch adds assigned codepoints for decoder for 32bit instructions
> and provide implementation for instruction. If extension is present,
> then moves 0 to `rd`.
>
> Signed-off-by: Deepak Gupta
> ---
> target/riscv/insn32.decode | 15 +++
> target/riscv/insn_trans/trans_zimops.c.inc | 50 ++
> target/riscv/translate.c | 3 ++
> 3 files changed, 68 insertions(+)
> create mode 100644 target/riscv/insn_trans/trans_zimops.c.inc
>
> diff --git a/target/riscv/insn32.decode b/target/riscv/insn32.decode
> index f22df04cfd..fca3838a9f 100644
> --- a/target/riscv/insn32.decode
> +++ b/target/riscv/insn32.decode
> @@ -167,6 +167,21 @@ csrrwi . 101 . 1110011 @csr
> csrrsi . 110 . 1110011 @csr
> csrrci . 111 . 1110011 @csr
>
> +# zimops (unpriv integer may be operations) instructions with system opcode
> +# zimops_r and zimops_rr are two code points assigned to zimops
> +# Any new extension that claims zimops encoding should be placed above mop.r
> +# and mop.rr
> +
> +# mop.r
> +{
> + zimops_r 1-00--0 111-- - 100 . 1110011 %rd
> +}
> +
> +# mop.rr
> +{
> + zimops_rr 1-00--1 - - 100 . 1110011 %rd
> +}
> +
> # *** RV64I Base Instruction Set (in addition to RV32I) ***
> lwu . 110 . 011 @i
> ld . 011 . 011 @i
> diff --git a/target/riscv/insn_trans/trans_zimops.c.inc
> b/target/riscv/insn_trans/trans_zimops.c.inc
> new file mode 100644
> index 00..b5ad7bded8
> ---
[PATCH 3/3] target/riscv: Introduce `compressed zimop` aka `zcmop`
Analogous to zimop, there are 8 encodings carved out of illegal space
encodings (c.lui xn, 0) in compressed instructions which are defined
to be zcmops short for compressed may be operations.
Unlike zimops (which write 0 to rd), zcmops don't actually write anything
to any register. Their encodings allow future extensions to define them to
read register x[n].
Signed-off-by: Deepak Gupta
---
target/riscv/insn16.decode | 6 ++
target/riscv/insn_trans/trans_zimops.c.inc | 11 +++
2 files changed, 17 insertions(+)
diff --git a/target/riscv/insn16.decode b/target/riscv/insn16.decode
index b96c534e73..d24b54d319 100644
--- a/target/riscv/insn16.decode
+++ b/target/riscv/insn16.decode
@@ -32,6 +32,7 @@
%uimm_cl_w 5:1 10:3 6:1 !function=ex_shift_2
%imm_cb12:s1 5:2 2:1 10:2 3:2 !function=ex_shift_1
%imm_cj12:s1 8:1 9:2 6:1 7:1 2:1 11:1 3:3 !function=ex_shift_1
+%zcmop_n 8:3
%shlimm_6bit 12:1 2:5 !function=ex_rvc_shiftli
%shrimm_6bit 12:1 2:5 !function=ex_rvc_shiftri
@@ -66,6 +67,8 @@
urlist spimm
index
+ zcmop_n
+
# Formats 16:
@cr . . .. rs2=%rs2_5 rs1=%rd %rd
@ci... . . . .. imm=%imm_ci rs1=%rd %rd
@@ -109,6 +112,8 @@
@cm_mv... ... ... .. ... .. _s rs2=%r2s rs1=%r1s
@cm_jt... ... ..%index
+@c_mop... . . . .. %zcmop_n
+
# *** RV32/64C Standard Extension (Quadrant 0) ***
{
# Opcode of all zeros is illegal; rd != 0, nzuimm == 0 is reserved.
@@ -140,6 +145,7 @@ sw110 ... ... .. ... 00 @cs_w
addi 000 . . . 01 @ci
addi 010 . . . 01 @c_li
{
+ zcmops 011 0 0...1 0 01 @c_mop # zcmop carving out of illegal
c.lui xn,0 space
illegal 011 0 - 0 01 # c.addi16sp and c.lui, RES nzimm=0
addi011 . 00010 . 01 @c_addi16sp
lui 011 . . . 01 @c_lui
diff --git a/target/riscv/insn_trans/trans_zimops.c.inc
b/target/riscv/insn_trans/trans_zimops.c.inc
index b5ad7bded8..99f25bd9ea 100644
--- a/target/riscv/insn_trans/trans_zimops.c.inc
+++ b/target/riscv/insn_trans/trans_zimops.c.inc
@@ -48,3 +48,14 @@ static bool trans_zimops_rr(DisasContext *ctx, arg_zimops_r
* a)
gen_set_gpr(ctx, a->rd, dest);
return true;
}
+
+static bool trans_zcmops(DisasContext *ctx, arg_zcmops * a)
+{
+/* zimops not implemented, return false */
+if (!ctx->cfg_ptr->ext_zimops) {
+gen_exception_illegal(ctx);
+return false;
+}
+
+return true;
+}
--
2.34.1
[PATCH 2/3] target/riscv: zimop instruction encoding and its implementation
This patch adds assigned codepoints for decoder for 32bit instructions
and provide implementation for instruction. If extension is present,
then moves 0 to `rd`.
Signed-off-by: Deepak Gupta
---
target/riscv/insn32.decode | 15 +++
target/riscv/insn_trans/trans_zimops.c.inc | 50 ++
target/riscv/translate.c | 3 ++
3 files changed, 68 insertions(+)
create mode 100644 target/riscv/insn_trans/trans_zimops.c.inc
diff --git a/target/riscv/insn32.decode b/target/riscv/insn32.decode
index f22df04cfd..fca3838a9f 100644
--- a/target/riscv/insn32.decode
+++ b/target/riscv/insn32.decode
@@ -167,6 +167,21 @@ csrrwi . 101 . 1110011 @csr
csrrsi . 110 . 1110011 @csr
csrrci . 111 . 1110011 @csr
+# zimops (unpriv integer may be operations) instructions with system opcode
+# zimops_r and zimops_rr are two code points assigned to zimops
+# Any new extension that claims zimops encoding should be placed above mop.r
+# and mop.rr
+
+# mop.r
+{
+ zimops_r 1-00--0 111-- - 100 . 1110011 %rd
+}
+
+# mop.rr
+{
+ zimops_rr 1-00--1 - - 100 . 1110011 %rd
+}
+
# *** RV64I Base Instruction Set (in addition to RV32I) ***
lwu . 110 . 011 @i
ld . 011 . 011 @i
diff --git a/target/riscv/insn_trans/trans_zimops.c.inc
b/target/riscv/insn_trans/trans_zimops.c.inc
new file mode 100644
index 00..b5ad7bded8
--- /dev/null
+++ b/target/riscv/insn_trans/trans_zimops.c.inc
@@ -0,0 +1,50 @@
+/*
+ * RISC-V translation routines for the Control-Flow Integrity Extension
+ *
+ * Copyright (c) 2024 Rivos Inc.
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2 or later, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for
+ * more details.
+ *
+ * You should have received a copy of the GNU General Public License along with
+ * this program. If not, see
[PATCH 1/3] target/riscv: zimop and zcmop extension for riscv
`zimop` stands for `may be operations`. `zcmop` stands for compressed
`may be operations`. For some RISC-V CPU extension, once compiled into
the binary are part of generated code which can't be gated behind a probe
of whether an instruction set is supported or not. One such example is
`zicfiss` [1] extension where `shadow stack push` and `shadow stack pop
and check` will be part of every function body. Thus binaries compiled
with such extensions need to run in following scenarios
- On machines where extension is present and enabled
- On machines where extension is present and disabled
- On machines where extension is not present/implemented.
`zimop` (for 32bit instructions) and `zcmop` (for compressed) were devised
and defined [2] to support such future (like zicfiss) CPU extensions
where zimops and zcmops provide a base non-faulting behavior for
codepoints that may claimed by future ISA extensions. Minimally, any
CPU implementation wanting to have binary compatibility with such
binaries only has to implement `zimop and zcmop`. Furthermore, this
allows per-task optin for software where user has the option to enable
the feature on per-task basis.
`zimop` are defined to write zero to `rd`. `zcmop` are defined to *not* write
to any register.
[1] - https://github.com/riscv/riscv-cfi/blob/main/src/cfi_backward.adoc
[2] - https://github.com/riscv/riscv-isa-manual/blob/main/src/zimop.adoc
Signed-off-by: Deepak Gupta
---
target/riscv/cpu.c | 2 ++
target/riscv/cpu_cfg.h | 1 +
2 files changed, 3 insertions(+)
diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index eb1a2e7d6d..3caf8553d1 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -113,6 +113,7 @@ const RISCVIsaExtData isa_edata_arr[] = {
ISA_EXT_DATA_ENTRY(zihintntl, PRIV_VERSION_1_10_0, ext_zihintntl),
ISA_EXT_DATA_ENTRY(zihintpause, PRIV_VERSION_1_10_0, ext_zihintpause),
ISA_EXT_DATA_ENTRY(zihpm, PRIV_VERSION_1_12_0, ext_zihpm),
+ISA_EXT_DATA_ENTRY(zimops, PRIV_VERSION_1_12_0, ext_zimops),
ISA_EXT_DATA_ENTRY(zmmul, PRIV_VERSION_1_12_0, ext_zmmul),
ISA_EXT_DATA_ENTRY(za64rs, PRIV_VERSION_1_12_0, has_priv_1_11),
ISA_EXT_DATA_ENTRY(zaamo, PRIV_VERSION_1_12_0, ext_zaamo),
@@ -2273,6 +2274,7 @@ static Property riscv_cpu_properties[] = {
* it with -x and default to 'false'.
*/
DEFINE_PROP_BOOL("x-misa-w", RISCVCPU, cfg.misa_w, false),
+DEFINE_PROP_BOOL("zimops", RISCVCPU, cfg.ext_zimops, true),
DEFINE_PROP_END_OF_LIST(),
};
diff --git a/target/riscv/cpu_cfg.h b/target/riscv/cpu_cfg.h
index cb750154bd..5c42ff8cdf 100644
--- a/target/riscv/cpu_cfg.h
+++ b/target/riscv/cpu_cfg.h
@@ -124,6 +124,7 @@ struct RISCVCPUConfig {
uint32_t mvendorid;
uint64_t marchid;
uint64_t mimpid;
+bool ext_zimops;
/* Named features */
bool ext_svade;
--
2.34.1
Re: [PATCH 0/7] pc-bios/s390-ccw: Merge the netboot loader into s390-ccw.img
On 6/24/24 1:55 AM, Thomas Huth wrote: [...] I think it should be fine, both functions are basically just a wrapper around the write() function in sclp.c, with sclp_print() being rather dumb while printf() is doing the usual string formatting before writing it out. I think in the long run, it would be nice to get rid of sclp_print() and replace it by puts() or printf() in the whole code, but doing that right now would likely cause quite some conflicts for Jared with his patch series, so I'd rather postpone that to a later point in time. Hi Thomas, Converting the panics to returns will require me to modify/move some of the sclp_print() calls. Shall I go ahead and change them to printf() and puts() while I'm at it, or would you rather preserve the sclp_print() for now and then have a dedicated patch for the all replacements later? I'm not sure if we want to try to maintain some amount of consistency until we do a total conversion, or if you are OK with a mix of sclp_print() and printf() throughout in the meantime. Regards, Jared Rossi
Re: [PATCH 02/23] target/i386: fix gen_prepare_size_nz condition
On 6/28/24 05:42, Alex Bennée wrote:
Incorrect brace positions causes an unintended overflow on 32 bit
builds and shenanigans result.
Resolves: https://gitlab.com/qemu-project/qemu/-/issues/2413
Suggested-by: Mark Cave-Ayland
Signed-off-by: Alex Bennée
---
target/i386/tcg/translate.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/target/i386/tcg/translate.c b/target/i386/tcg/translate.c
index ad1819815a..94f13541c3 100644
--- a/target/i386/tcg/translate.c
+++ b/target/i386/tcg/translate.c
@@ -877,7 +877,7 @@ static CCPrepare gen_prepare_sign_nz(TCGv src, MemOp size)
return (CCPrepare) { .cond = TCG_COND_LT, .reg = src };
} else {
return (CCPrepare) { .cond = TCG_COND_TSTNE, .reg = src,
- .imm = 1ull << ((8 << size) - 1) };
+ .imm = (1ull << (8 << size)) - 1 };
This is incorrect -- we want only to test the sign bit.
Perhaps MAKE_64BIT_MASK((8 << size) - 1, 1) would make this more explicit?
I'll have a quick look at the issue and see if I can reproduce.
r~
[PATCH 1/3] target/riscv: zimop and zcmop extension for riscv
`zimop` stands for `may be operations`. `zcmop` stands for compressed
`may be operations`. For some RISC-V CPU extension, once compiled into
the binary are part of generated code which can't be gated behind a probe
of whether an instruction set is supported or not. One such example is
`zicfiss` [1] extension where `shadow stack push` and `shadow stack pop
and check` will be part of every function body. Thus binaries compiled
with such extensions need to run in following scenarios
- On machines where extension is present and enabled
- On machines where extension is present and disabled
- On machines where extension is not present/implemented.
`zimop` (for 32bit instructions) and `zcmop` (for compressed) were devised
and defined [2] to support such future (like zicfiss) CPU extensions
where zimops and zcmops provide a base non-faulting behavior for
codepoints that may claimed by future ISA extensions. Minimally, any
CPU implementation wanting to have binary compatibility with such
binaries only has to implement `zimop and zcmop`. Furthermore, this
allows per-task optin for software where user has the option to enable
the feature on per-task basis.
`zimop` are defined to write zero to `rd`. `zcmop` are defined to *not* write
to any register.
[1] - https://github.com/riscv/riscv-cfi/blob/main/src/cfi_backward.adoc
[2] - https://github.com/riscv/riscv-isa-manual/blob/main/src/zimop.adoc
Signed-off-by: Deepak Gupta
---
target/riscv/cpu.c | 2 ++
target/riscv/cpu_cfg.h | 1 +
2 files changed, 3 insertions(+)
diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index eb1a2e7d6d..3caf8553d1 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -113,6 +113,7 @@ const RISCVIsaExtData isa_edata_arr[] = {
ISA_EXT_DATA_ENTRY(zihintntl, PRIV_VERSION_1_10_0, ext_zihintntl),
ISA_EXT_DATA_ENTRY(zihintpause, PRIV_VERSION_1_10_0, ext_zihintpause),
ISA_EXT_DATA_ENTRY(zihpm, PRIV_VERSION_1_12_0, ext_zihpm),
+ISA_EXT_DATA_ENTRY(zimops, PRIV_VERSION_1_12_0, ext_zimops),
ISA_EXT_DATA_ENTRY(zmmul, PRIV_VERSION_1_12_0, ext_zmmul),
ISA_EXT_DATA_ENTRY(za64rs, PRIV_VERSION_1_12_0, has_priv_1_11),
ISA_EXT_DATA_ENTRY(zaamo, PRIV_VERSION_1_12_0, ext_zaamo),
@@ -2273,6 +2274,7 @@ static Property riscv_cpu_properties[] = {
* it with -x and default to 'false'.
*/
DEFINE_PROP_BOOL("x-misa-w", RISCVCPU, cfg.misa_w, false),
+DEFINE_PROP_BOOL("zimops", RISCVCPU, cfg.ext_zimops, true),
DEFINE_PROP_END_OF_LIST(),
};
diff --git a/target/riscv/cpu_cfg.h b/target/riscv/cpu_cfg.h
index cb750154bd..5c42ff8cdf 100644
--- a/target/riscv/cpu_cfg.h
+++ b/target/riscv/cpu_cfg.h
@@ -124,6 +124,7 @@ struct RISCVCPUConfig {
uint32_t mvendorid;
uint64_t marchid;
uint64_t mimpid;
+bool ext_zimops;
/* Named features */
bool ext_svade;
--
2.34.1
>From 4d15b0e0037f444eb75e60b398e19dcf476f07d4 Mon Sep 17 00:00:00 2001
From: Deepak Gupta
Date: Fri, 28 Jun 2024 00:13:58 -0700
Subject: [PATCH 2/3] target/riscv: zimop instruction encoding and its
implementation
This patch adds assigned codepoints for decoder for 32bit instructions
and provide implementation for instruction. If extension is present,
then moves 0 to `rd`.
Signed-off-by: Deepak Gupta
---
target/riscv/insn32.decode | 15 +++
target/riscv/insn_trans/trans_zimops.c.inc | 50 ++
target/riscv/translate.c | 3 ++
3 files changed, 68 insertions(+)
create mode 100644 target/riscv/insn_trans/trans_zimops.c.inc
diff --git a/target/riscv/insn32.decode b/target/riscv/insn32.decode
index f22df04cfd..fca3838a9f 100644
--- a/target/riscv/insn32.decode
+++ b/target/riscv/insn32.decode
@@ -167,6 +167,21 @@ csrrwi . 101 . 1110011 @csr
csrrsi . 110 . 1110011 @csr
csrrci . 111 . 1110011 @csr
+# zimops (unpriv integer may be operations) instructions with system opcode
+# zimops_r and zimops_rr are two code points assigned to zimops
+# Any new extension that claims zimops encoding should be placed above mop.r
+# and mop.rr
+
+# mop.r
+{
+ zimops_r 1-00--0 111-- - 100 . 1110011 %rd
+}
+
+# mop.rr
+{
+ zimops_rr 1-00--1 - - 100 . 1110011 %rd
+}
+
# *** RV64I Base Instruction Set (in addition to RV32I) ***
lwu . 110 . 011 @i
ld . 011 . 011 @i
diff --git a/target/riscv/insn_trans/trans_zimops.c.inc
b/target/riscv/insn_trans/trans_zimops.c.inc
new file mode 100644
index 00..b5ad7bded8
--- /dev/null
+++ b/target/riscv/insn_trans/trans_zimops.c.inc
@@ -0,0 +1,50 @@
+/*
+ * RISC-V translation routines for the Control-Flow Integrity Extension
+ *
+ * Copyright (c) 2024 Rivos Inc.
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2 or later, as
[PULL 06/23] meson: remove dead optimization option
Reviewed-by: Richard Henderson
Signed-off-by: Paolo Bonzini
---
meson.build | 13 -
meson_options.txt | 2 --
scripts/meson-buildoptions.sh | 3 ---
3 files changed, 18 deletions(-)
diff --git a/meson.build b/meson.build
index 6e694ecd9fe..54e6b09f4fb 100644
--- a/meson.build
+++ b/meson.build
@@ -2874,18 +2874,6 @@ config_host_data.set('CONFIG_AVX2_OPT',
get_option('avx2') \
int main(int argc, char *argv[]) { return bar(argv[argc - 1]); }
'''), error_message: 'AVX2 not available').allowed())
-config_host_data.set('CONFIG_AVX512F_OPT', get_option('avx512f') \
- .require(have_cpuid_h, error_message: 'cpuid.h not available, cannot enable
AVX512F') \
- .require(cc.links('''
-#include
-#include
-static int __attribute__((target("avx512f"))) bar(void *a) {
- __m512i x = *(__m512i *)a;
- return _mm512_test_epi64_mask(x, x);
-}
-int main(int argc, char *argv[]) { return bar(argv[argc - 1]); }
- '''), error_message: 'AVX512F not available').allowed())
-
config_host_data.set('CONFIG_AVX512BW_OPT', get_option('avx512bw') \
.require(have_cpuid_h, error_message: 'cpuid.h not available, cannot enable
AVX512BW') \
.require(cc.links('''
@@ -4283,7 +4271,6 @@ summary_info += {'mutex debugging':
get_option('debug_mutex')}
summary_info += {'memory allocator': get_option('malloc')}
summary_info += {'avx2 optimization': config_host_data.get('CONFIG_AVX2_OPT')}
summary_info += {'avx512bw optimization':
config_host_data.get('CONFIG_AVX512BW_OPT')}
-summary_info += {'avx512f optimization':
config_host_data.get('CONFIG_AVX512F_OPT')}
summary_info += {'gcov': get_option('b_coverage')}
summary_info += {'thread sanitizer': get_option('tsan')}
summary_info += {'CFI support': get_option('cfi')}
diff --git a/meson_options.txt b/meson_options.txt
index 6065ed2d352..0269fa0f16e 100644
--- a/meson_options.txt
+++ b/meson_options.txt
@@ -119,8 +119,6 @@ option('membarrier', type: 'feature', value: 'disabled',
option('avx2', type: 'feature', value: 'auto',
description: 'AVX2 optimizations')
-option('avx512f', type: 'feature', value: 'disabled',
- description: 'AVX512F optimizations')
option('avx512bw', type: 'feature', value: 'auto',
description: 'AVX512BW optimizations')
option('keyring', type: 'feature', value: 'auto',
diff --git a/scripts/meson-buildoptions.sh b/scripts/meson-buildoptions.sh
index 62842d47e88..cfadb5ea86a 100644
--- a/scripts/meson-buildoptions.sh
+++ b/scripts/meson-buildoptions.sh
@@ -95,7 +95,6 @@ meson_options_help() {
printf "%s\n" ' auth-pamPAM access control'
printf "%s\n" ' avx2AVX2 optimizations'
printf "%s\n" ' avx512bwAVX512BW optimizations'
- printf "%s\n" ' avx512f AVX512F optimizations'
printf "%s\n" ' blkio libblkio block device driver'
printf "%s\n" ' bochs bochs image format support'
printf "%s\n" ' bpf eBPF support'
@@ -240,8 +239,6 @@ _meson_option_parse() {
--disable-avx2) printf "%s" -Davx2=disabled ;;
--enable-avx512bw) printf "%s" -Davx512bw=enabled ;;
--disable-avx512bw) printf "%s" -Davx512bw=disabled ;;
---enable-avx512f) printf "%s" -Davx512f=enabled ;;
---disable-avx512f) printf "%s" -Davx512f=disabled ;;
--enable-gcov) printf "%s" -Db_coverage=true ;;
--disable-gcov) printf "%s" -Db_coverage=false ;;
--enable-lto) printf "%s" -Db_lto=true ;;
--
2.45.2
[PULL 03/23] Revert "host/i386: assume presence of SSSE3"
This reverts commit 433cd6d94a8256af70a5200f236dc8047c3c1468.
The x86-64 instruction set can now be tuned down to x86-64 v1
or i386 Pentium Pro.
Signed-off-by: Paolo Bonzini
---
util/cpuinfo-i386.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/util/cpuinfo-i386.c b/util/cpuinfo-i386.c
index 6d474a6259a..ca74ef04f54 100644
--- a/util/cpuinfo-i386.c
+++ b/util/cpuinfo-i386.c
@@ -38,8 +38,8 @@ unsigned __attribute__((constructor)) cpuinfo_init(void)
info |= (c & bit_POPCNT ? CPUINFO_POPCNT : 0);
info |= (c & bit_PCLMUL ? CPUINFO_PCLMUL : 0);
-/* NOTE: our AES support requires SSSE3 (PSHUFB) as well. */
-info |= (c & bit_AES) ? CPUINFO_AES : 0;
+/* Our AES support requires PSHUFB as well. */
+info |= ((c & bit_AES) && (c & bit_SSSE3) ? CPUINFO_AES : 0);
/* For AVX features, we must check available and usable. */
if ((c & bit_AVX) && (c & bit_OSXSAVE)) {
--
2.45.2
[PULL 07/23] block: make assertion more generic
.bdrv_needs_filename is only set for drivers that also set bdrv_file_open,
i.e. protocol drivers.
So we can make the assertion always, it will always pass for those drivers
that use bdrv_open.
Signed-off-by: Paolo Bonzini
---
block.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/block.c b/block.c
index 468cf5e67d7..69a2905178a 100644
--- a/block.c
+++ b/block.c
@@ -1655,8 +1655,8 @@ bdrv_open_driver(BlockDriverState *bs, BlockDriver *drv,
const char *node_name,
bs->drv = drv;
bs->opaque = g_malloc0(drv->instance_size);
+assert(!drv->bdrv_needs_filename || bs->filename[0]);
if (drv->bdrv_file_open) {
-assert(!drv->bdrv_needs_filename || bs->filename[0]);
ret = drv->bdrv_file_open(bs, options, open_flags, _err);
} else if (drv->bdrv_open) {
ret = drv->bdrv_open(bs, options, open_flags, _err);
--
2.45.2
[PULL 15/23] target/i386: use cpu_cc_dst for CC_OP_POPCNT
It is the only CCOp, among those that compute ZF from one of the cc_op_*
registers, that uses cpu_cc_src. Do not make it the odd one off,
instead use cpu_cc_dst like the others.
Reviewed-by: Richard Henderson
Signed-off-by: Paolo Bonzini
---
target/i386/cpu.h | 2 +-
target/i386/tcg/cc_helper.c | 2 +-
target/i386/tcg/translate.c | 4 ++--
target/i386/tcg/emit.c.inc | 4 ++--
4 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/target/i386/cpu.h b/target/i386/cpu.h
index 52571ababe2..1b4edbe0580 100644
--- a/target/i386/cpu.h
+++ b/target/i386/cpu.h
@@ -1332,7 +1332,7 @@ typedef enum {
CC_OP_BMILGQ,
CC_OP_CLR, /* Z set, all other flags clear. */
-CC_OP_POPCNT, /* Z via CC_SRC, all other flags clear. */
+CC_OP_POPCNT, /* Z via CC_DST, all other flags clear. */
CC_OP_NB,
} CCOp;
diff --git a/target/i386/tcg/cc_helper.c b/target/i386/tcg/cc_helper.c
index f76e9cb8cfb..301ed954064 100644
--- a/target/i386/tcg/cc_helper.c
+++ b/target/i386/tcg/cc_helper.c
@@ -107,7 +107,7 @@ target_ulong helper_cc_compute_all(target_ulong dst,
target_ulong src1,
case CC_OP_CLR:
return CC_Z | CC_P;
case CC_OP_POPCNT:
-return src1 ? 0 : CC_Z;
+return dst ? 0 : CC_Z;
case CC_OP_MULB:
return compute_all_mulb(dst, src1);
diff --git a/target/i386/tcg/translate.c b/target/i386/tcg/translate.c
index ad1819815ab..eb353dc3c9f 100644
--- a/target/i386/tcg/translate.c
+++ b/target/i386/tcg/translate.c
@@ -324,7 +324,7 @@ static const uint8_t cc_op_live[CC_OP_NB] = {
[CC_OP_ADOX] = USES_CC_SRC | USES_CC_SRC2,
[CC_OP_ADCOX] = USES_CC_DST | USES_CC_SRC | USES_CC_SRC2,
[CC_OP_CLR] = 0,
-[CC_OP_POPCNT] = USES_CC_SRC,
+[CC_OP_POPCNT] = USES_CC_DST,
};
static void set_cc_op_1(DisasContext *s, CCOp op, bool dirty)
@@ -1020,7 +1020,7 @@ static CCPrepare gen_prepare_eflags_z(DisasContext *s,
TCGv reg)
case CC_OP_CLR:
return (CCPrepare) { .cond = TCG_COND_ALWAYS };
case CC_OP_POPCNT:
-return (CCPrepare) { .cond = TCG_COND_EQ, .reg = cpu_cc_src };
+return (CCPrepare) { .cond = TCG_COND_EQ, .reg = cpu_cc_dst };
default:
{
MemOp size = (s->cc_op - CC_OP_ADDB) & 3;
diff --git a/target/i386/tcg/emit.c.inc b/target/i386/tcg/emit.c.inc
index 11faa70b5e2..fc7477833bc 100644
--- a/target/i386/tcg/emit.c.inc
+++ b/target/i386/tcg/emit.c.inc
@@ -2804,10 +2804,10 @@ static void gen_POPA(DisasContext *s, X86DecodedInsn
*decode)
static void gen_POPCNT(DisasContext *s, X86DecodedInsn *decode)
{
-decode->cc_src = tcg_temp_new();
+decode->cc_dst = tcg_temp_new();
decode->cc_op = CC_OP_POPCNT;
-tcg_gen_mov_tl(decode->cc_src, s->T0);
+tcg_gen_mov_tl(decode->cc_dst, s->T0);
tcg_gen_ctpop_tl(s->T0, s->T0);
}
--
2.45.2
[PULL 19/23] target/i386: SEV: store pointer to decoded id_block in SevSnpGuest
Do not rely on finish->id_block_uaddr, so that there are no casts from
pointer to uint64_t. They break on 32-bit hosts.
Reviewed-by: Richard Henderson
Signed-off-by: Paolo Bonzini
---
target/i386/sev.c | 11 ++-
1 file changed, 6 insertions(+), 5 deletions(-)
diff --git a/target/i386/sev.c b/target/i386/sev.c
index 6daa8c264cd..2d4cfd41e83 100644
--- a/target/i386/sev.c
+++ b/target/i386/sev.c
@@ -153,6 +153,7 @@ struct SevSnpGuestState {
/* configuration parameters */
char *guest_visible_workarounds;
char *id_block_base64;
+uint8_t *id_block;
char *id_auth;
char *host_data;
@@ -2170,16 +2171,15 @@ sev_snp_guest_set_id_block(Object *obj, const char
*value, Error **errp)
gsize len;
finish->id_block_en = 0;
+g_free(sev_snp_guest->id_block);
g_free(sev_snp_guest->id_block_base64);
-g_free((guchar *)finish->id_block_uaddr);
/* store the base64 str so we don't need to re-encode in getter */
sev_snp_guest->id_block_base64 = g_strdup(value);
+sev_snp_guest->id_block =
+qbase64_decode(sev_snp_guest->id_block_base64, -1, , errp);
-finish->id_block_uaddr =
-(uint64_t)qbase64_decode(sev_snp_guest->id_block_base64, -1, ,
errp);
-
-if (!finish->id_block_uaddr) {
+if (!sev_snp_guest->id_block) {
return;
}
@@ -2190,6 +2190,7 @@ sev_snp_guest_set_id_block(Object *obj, const char
*value, Error **errp)
}
finish->id_block_en = 1;
+finish->id_block_uaddr = (uintptr_t)sev_snp_guest->id_block;
}
static char *
--
2.45.2
[PULL 18/23] target/i386: SEV: rename sev_snp_guest->id_block
Free the "id_block" name for the binary version of the data.
Reviewed-by: Richard Henderson
Signed-off-by: Paolo Bonzini
---
target/i386/sev.c | 12 ++--
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/target/i386/sev.c b/target/i386/sev.c
index 30b83f1d77d..6daa8c264cd 100644
--- a/target/i386/sev.c
+++ b/target/i386/sev.c
@@ -152,7 +152,7 @@ struct SevSnpGuestState {
/* configuration parameters */
char *guest_visible_workarounds;
-char *id_block;
+char *id_block_base64;
char *id_auth;
char *host_data;
@@ -1296,7 +1296,7 @@ sev_snp_launch_finish(SevCommonState *sev_common)
}
}
-trace_kvm_sev_snp_launch_finish(sev_snp->id_block, sev_snp->id_auth,
+trace_kvm_sev_snp_launch_finish(sev_snp->id_block_base64, sev_snp->id_auth,
sev_snp->host_data);
ret = sev_ioctl(sev_common->sev_fd, KVM_SEV_SNP_LAUNCH_FINISH,
finish, );
@@ -2159,7 +2159,7 @@ sev_snp_guest_get_id_block(Object *obj, Error **errp)
{
SevSnpGuestState *sev_snp_guest = SEV_SNP_GUEST(obj);
-return g_strdup(sev_snp_guest->id_block);
+return g_strdup(sev_snp_guest->id_block_base64);
}
static void
@@ -2170,14 +2170,14 @@ sev_snp_guest_set_id_block(Object *obj, const char
*value, Error **errp)
gsize len;
finish->id_block_en = 0;
-g_free(sev_snp_guest->id_block);
+g_free(sev_snp_guest->id_block_base64);
g_free((guchar *)finish->id_block_uaddr);
/* store the base64 str so we don't need to re-encode in getter */
-sev_snp_guest->id_block = g_strdup(value);
+sev_snp_guest->id_block_base64 = g_strdup(value);
finish->id_block_uaddr =
-(uint64_t)qbase64_decode(sev_snp_guest->id_block, -1, , errp);
+(uint64_t)qbase64_decode(sev_snp_guest->id_block_base64, -1, ,
errp);
if (!finish->id_block_uaddr) {
return;
--
2.45.2
[PULL 20/23] target/i386: SEV: rename sev_snp_guest->id_auth
Free the "id_auth" name for the binary version of the data.
Reviewed-by: Richard Henderson
Signed-off-by: Paolo Bonzini
---
target/i386/sev.c | 12 ++--
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/target/i386/sev.c b/target/i386/sev.c
index 2d4cfd41e83..a6b063b762c 100644
--- a/target/i386/sev.c
+++ b/target/i386/sev.c
@@ -154,7 +154,7 @@ struct SevSnpGuestState {
char *guest_visible_workarounds;
char *id_block_base64;
uint8_t *id_block;
-char *id_auth;
+char *id_auth_base64;
char *host_data;
struct kvm_sev_snp_launch_start kvm_start_conf;
@@ -1297,7 +1297,7 @@ sev_snp_launch_finish(SevCommonState *sev_common)
}
}
-trace_kvm_sev_snp_launch_finish(sev_snp->id_block_base64, sev_snp->id_auth,
+trace_kvm_sev_snp_launch_finish(sev_snp->id_block_base64,
sev_snp->id_auth_base64,
sev_snp->host_data);
ret = sev_ioctl(sev_common->sev_fd, KVM_SEV_SNP_LAUNCH_FINISH,
finish, );
@@ -2198,7 +2198,7 @@ sev_snp_guest_get_id_auth(Object *obj, Error **errp)
{
SevSnpGuestState *sev_snp_guest = SEV_SNP_GUEST(obj);
-return g_strdup(sev_snp_guest->id_auth);
+return g_strdup(sev_snp_guest->id_auth_base64);
}
static void
@@ -2208,14 +2208,14 @@ sev_snp_guest_set_id_auth(Object *obj, const char
*value, Error **errp)
struct kvm_sev_snp_launch_finish *finish = _snp_guest->kvm_finish_conf;
gsize len;
-g_free(sev_snp_guest->id_auth);
+g_free(sev_snp_guest->id_auth_base64);
g_free((guchar *)finish->id_auth_uaddr);
/* store the base64 str so we don't need to re-encode in getter */
-sev_snp_guest->id_auth = g_strdup(value);
+sev_snp_guest->id_auth_base64 = g_strdup(value);
finish->id_auth_uaddr =
-(uint64_t)qbase64_decode(sev_snp_guest->id_auth, -1, , errp);
+(uint64_t)qbase64_decode(sev_snp_guest->id_auth_base64, -1, ,
errp);
if (!finish->id_auth_uaddr) {
return;
--
2.45.2
[PULL 17/23] target/i386: remove unused enum
Reviewed-by: Richard Henderson
Signed-off-by: Paolo Bonzini
---
target/i386/tcg/translate.c | 16
1 file changed, 16 deletions(-)
diff --git a/target/i386/tcg/translate.c b/target/i386/tcg/translate.c
index 934c514e64f..95bad55bf46 100644
--- a/target/i386/tcg/translate.c
+++ b/target/i386/tcg/translate.c
@@ -282,22 +282,6 @@ enum {
JCC_LE,
};
-enum {
-/* I386 int registers */
-OR_EAX, /* MUST be even numbered */
-OR_ECX,
-OR_EDX,
-OR_EBX,
-OR_ESP,
-OR_EBP,
-OR_ESI,
-OR_EDI,
-
-OR_TMP0 = 16,/* temporary operand register */
-OR_TMP1,
-OR_A0, /* temporary register used when doing address evaluation */
-};
-
enum {
USES_CC_DST = 1,
USES_CC_SRC = 2,
--
2.45.2
[PULL 05/23] meson: allow configuring the x86-64 baseline
Add a Meson option to configure which x86-64 instruction
set to use. QEMU will now default to x86-64-v1 + cmpxchg16b for
64-bit builds (that corresponds to a Pentium 4 for 32-bit builds).
The baseline can be tuned down to Pentium Pro for 32-bit builds (with
-Dx86_version=0), or up as desired.
Acked-by: Richard Henderson
Signed-off-by: Paolo Bonzini
---
meson.build | 41 ---
meson_options.txt | 3 +++
scripts/meson-buildoptions.sh | 3 +++
3 files changed, 39 insertions(+), 8 deletions(-)
diff --git a/meson.build b/meson.build
index 97e00d6f59b..6e694ecd9fe 100644
--- a/meson.build
+++ b/meson.build
@@ -336,15 +336,40 @@ if host_arch == 'i386' and not cc.links('''
qemu_common_flags = ['-march=i486'] + qemu_common_flags
endif
-# Assume x86-64-v2 (minus CMPXCHG16B for 32-bit code)
-if host_arch == 'i386'
- qemu_common_flags = ['-mfpmath=sse'] + qemu_common_flags
-endif
+# Pick x86-64 baseline version
if host_arch in ['i386', 'x86_64']
- qemu_common_flags = ['-mpopcnt', '-msse4.2'] + qemu_common_flags
-endif
-if host_arch == 'x86_64'
- qemu_common_flags = ['-mcx16'] + qemu_common_flags
+ if get_option('x86_version') == '0' and host_arch == 'x86_64'
+error('x86_64-v1 required for x86-64 hosts')
+ endif
+
+ # add flags for individual instruction set extensions
+ if get_option('x86_version') >= '1'
+if host_arch == 'i386'
+ qemu_common_flags = ['-mfpmath=sse'] + qemu_common_flags
+else
+ # present on basically all processors but technically not part of
+ # x86-64-v1, so only include -mneeded for x86-64 version 2 and above
+ qemu_common_flags = ['-mcx16'] + qemu_common_flags
+endif
+ endif
+ if get_option('x86_version') >= '2'
+qemu_common_flags = ['-mpopcnt'] + qemu_common_flags
+qemu_common_flags = cc.get_supported_arguments('-mneeded') +
qemu_common_flags
+ endif
+ if get_option('x86_version') >= '3'
+qemu_common_flags = ['-mmovbe', '-mabm', '-mbmi1', '-mbmi2', '-mfma',
'-mf16c'] + qemu_common_flags
+ endif
+
+ # add required vector instruction set (each level implies those below)
+ if get_option('x86_version') == '1'
+qemu_common_flags = ['-msse2'] + qemu_common_flags
+ elif get_option('x86_version') == '2'
+qemu_common_flags = ['-msse4.2'] + qemu_common_flags
+ elif get_option('x86_version') == '3'
+qemu_common_flags = ['-mavx2'] + qemu_common_flags
+ elif get_option('x86_version') == '4'
+qemu_common_flags = ['-mavx512f', '-mavx512bw', '-mavx512cd',
'-mavx512dq', '-mavx512vl'] + qemu_common_flags
+ endif
endif
if get_option('prefer_static')
diff --git a/meson_options.txt b/meson_options.txt
index 7a79dd89706..6065ed2d352 100644
--- a/meson_options.txt
+++ b/meson_options.txt
@@ -370,3 +370,6 @@ option('qemu_ga_version', type: 'string', value: '',
option('hexagon_idef_parser', type : 'boolean', value : true,
description: 'use idef-parser to automatically generate TCG code for
the Hexagon frontend')
+
+option('x86_version', type : 'combo', choices : ['0', '1', '2', '3', '4'],
value: '1',
+ description: 'tweak required x86_64 architecture version beyond
compiler default')
diff --git a/scripts/meson-buildoptions.sh b/scripts/meson-buildoptions.sh
index 58d49a447d5..62842d47e88 100644
--- a/scripts/meson-buildoptions.sh
+++ b/scripts/meson-buildoptions.sh
@@ -82,6 +82,8 @@ meson_options_help() {
printf "%s\n" ' --with-suffix=VALUE Suffix for QEMU
data/modules/config directories'
printf "%s\n" ' (can be empty) [qemu]'
printf "%s\n" ' --with-trace-file=VALUE Trace file prefix for simple
backend [trace]'
+ printf "%s\n" ' --x86-version=CHOICE tweak required x86_64 architecture
version beyond'
+ printf "%s\n" ' compiler default [1] (choices:
0/1/2/3)'
printf "%s\n" ''
printf "%s\n" 'Optional features, enabled with --enable-FEATURE and'
printf "%s\n" 'disabled with --disable-FEATURE, default is enabled if
available'
@@ -552,6 +554,7 @@ _meson_option_parse() {
--disable-werror) printf "%s" -Dwerror=false ;;
--enable-whpx) printf "%s" -Dwhpx=enabled ;;
--disable-whpx) printf "%s" -Dwhpx=disabled ;;
+--x86-version=*) quote_sh "-Dx86_version=$2" ;;
--enable-xen) printf "%s" -Dxen=enabled ;;
--disable-xen) printf "%s" -Dxen=disabled ;;
--enable-xen-pci-passthrough) printf "%s" -Dxen_pci_passthrough=enabled ;;
--
2.45.2
[PULL 14/23] target/i386: fix CC_OP dump
POPCNT was missing, and the entries were all out of order after
ADCX/ADOX/ADCOX were moved close to EFLAGS. Just use designated
initializers.
Fixes: 4885c3c4953 ("target-i386: Use ctpop helper", 2017-01-10)
Fixes: cc155f19717 ("target/i386: rewrite flags writeback for ADCX/ADOX",
2024-06-11)
Signed-off-by: Paolo Bonzini
---
target/i386/cpu-dump.c | 101 +
1 file changed, 51 insertions(+), 50 deletions(-)
diff --git a/target/i386/cpu-dump.c b/target/i386/cpu-dump.c
index 40697064d92..3bb8e440916 100644
--- a/target/i386/cpu-dump.c
+++ b/target/i386/cpu-dump.c
@@ -28,69 +28,70 @@
/* x86 debug */
static const char *cc_op_str[CC_OP_NB] = {
-"DYNAMIC",
-"EFLAGS",
+[CC_OP_DYNAMIC] = "DYNAMIC",
-"MULB",
-"MULW",
-"MULL",
-"MULQ",
+[CC_OP_EFLAGS] = "EFLAGS",
+[CC_OP_ADCX] = "ADCX",
+[CC_OP_ADOX] = "ADOX",
+[CC_OP_ADCOX] = "ADCOX",
-"ADDB",
-"ADDW",
-"ADDL",
-"ADDQ",
+[CC_OP_MULB] = "MULB",
+[CC_OP_MULW] = "MULW",
+[CC_OP_MULL] = "MULL",
+[CC_OP_MULQ] = "MULQ",
-"ADCB",
-"ADCW",
-"ADCL",
-"ADCQ",
+[CC_OP_ADDB] = "ADDB",
+[CC_OP_ADDW] = "ADDW",
+[CC_OP_ADDL] = "ADDL",
+[CC_OP_ADDQ] = "ADDQ",
-"SUBB",
-"SUBW",
-"SUBL",
-"SUBQ",
+[CC_OP_ADCB] = "ADCB",
+[CC_OP_ADCW] = "ADCW",
+[CC_OP_ADCL] = "ADCL",
+[CC_OP_ADCQ] = "ADCQ",
-"SBBB",
-"SBBW",
-"SBBL",
-"SBBQ",
+[CC_OP_SUBB] = "SUBB",
+[CC_OP_SUBW] = "SUBW",
+[CC_OP_SUBL] = "SUBL",
+[CC_OP_SUBQ] = "SUBQ",
-"LOGICB",
-"LOGICW",
-"LOGICL",
-"LOGICQ",
+[CC_OP_SBBB] = "SBBB",
+[CC_OP_SBBW] = "SBBW",
+[CC_OP_SBBL] = "SBBL",
+[CC_OP_SBBQ] = "SBBQ",
-"INCB",
-"INCW",
-"INCL",
-"INCQ",
+[CC_OP_LOGICB] = "LOGICB",
+[CC_OP_LOGICW] = "LOGICW",
+[CC_OP_LOGICL] = "LOGICL",
+[CC_OP_LOGICQ] = "LOGICQ",
-"DECB",
-"DECW",
-"DECL",
-"DECQ",
+[CC_OP_INCB] = "INCB",
+[CC_OP_INCW] = "INCW",
+[CC_OP_INCL] = "INCL",
+[CC_OP_INCQ] = "INCQ",
-"SHLB",
-"SHLW",
-"SHLL",
-"SHLQ",
+[CC_OP_DECB] = "DECB",
+[CC_OP_DECW] = "DECW",
+[CC_OP_DECL] = "DECL",
+[CC_OP_DECQ] = "DECQ",
-"SARB",
-"SARW",
-"SARL",
-"SARQ",
+[CC_OP_SHLB] = "SHLB",
+[CC_OP_SHLW] = "SHLW",
+[CC_OP_SHLL] = "SHLL",
+[CC_OP_SHLQ] = "SHLQ",
-"BMILGB",
-"BMILGW",
-"BMILGL",
-"BMILGQ",
+[CC_OP_SARB] = "SARB",
+[CC_OP_SARW] = "SARW",
+[CC_OP_SARL] = "SARL",
+[CC_OP_SARQ] = "SARQ",
-"ADCX",
-"ADOX",
-"ADCOX",
+[CC_OP_BMILGB] = "BMILGB",
+[CC_OP_BMILGW] = "BMILGW",
+[CC_OP_BMILGL] = "BMILGL",
+[CC_OP_BMILGQ] = "BMILGQ",
-"CLR",
+[CC_OP_POPCNT] = "POPCNT",
+[CC_OP_CLR] = "CLR",
};
static void
--
2.45.2
[PULL 11/23] exec: avoid using C++ keywords in function parameters
From: Roman Kiryanov
to use the QEMU headers with a C++ compiler.
Signed-off-by: Roman Kiryanov
Link: https://lore.kernel.org/r/20240618224553.878869-1-r...@google.com
Signed-off-by: Paolo Bonzini
---
include/exec/memory.h | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/include/exec/memory.h b/include/exec/memory.h
index 0903513d132..154626f9ad2 100644
--- a/include/exec/memory.h
+++ b/include/exec/memory.h
@@ -925,7 +925,7 @@ struct MemoryListener {
* the current transaction.
*/
void (*log_start)(MemoryListener *listener, MemoryRegionSection *section,
- int old, int new);
+ int old_val, int new_val);
/**
* @log_stop:
@@ -944,7 +944,7 @@ struct MemoryListener {
* the current transaction.
*/
void (*log_stop)(MemoryListener *listener, MemoryRegionSection *section,
- int old, int new);
+ int old_val, int new_val);
/**
* @log_sync:
--
2.45.2
[PULL 04/23] Revert "host/i386: assume presence of SSE2"
This reverts commit b18236897ca15c3db1506d8edb9a191dfe51429c.
The x86-64 instruction set can now be tuned down to x86-64 v1
or i386 Pentium Pro.
Signed-off-by: Paolo Bonzini
---
host/include/i386/host/cpuinfo.h | 1 +
util/cpuinfo-i386.c | 1 +
host/include/i386/host/bufferiszero.c.inc | 5 +++--
3 files changed, 5 insertions(+), 2 deletions(-)
diff --git a/host/include/i386/host/cpuinfo.h b/host/include/i386/host/cpuinfo.h
index 72f6fad61e5..81771733eaa 100644
--- a/host/include/i386/host/cpuinfo.h
+++ b/host/include/i386/host/cpuinfo.h
@@ -14,6 +14,7 @@
#define CPUINFO_POPCNT (1u << 4)
#define CPUINFO_BMI1(1u << 5)
#define CPUINFO_BMI2(1u << 6)
+#define CPUINFO_SSE2(1u << 7)
#define CPUINFO_AVX1(1u << 9)
#define CPUINFO_AVX2(1u << 10)
#define CPUINFO_AVX512F (1u << 11)
diff --git a/util/cpuinfo-i386.c b/util/cpuinfo-i386.c
index ca74ef04f54..90f92a42dc8 100644
--- a/util/cpuinfo-i386.c
+++ b/util/cpuinfo-i386.c
@@ -34,6 +34,7 @@ unsigned __attribute__((constructor)) cpuinfo_init(void)
if (max >= 1) {
__cpuid(1, a, b, c, d);
+info |= (d & bit_SSE2 ? CPUINFO_SSE2 : 0);
info |= (c & bit_MOVBE ? CPUINFO_MOVBE : 0);
info |= (c & bit_POPCNT ? CPUINFO_POPCNT : 0);
info |= (c & bit_PCLMUL ? CPUINFO_PCLMUL : 0);
diff --git a/host/include/i386/host/bufferiszero.c.inc
b/host/include/i386/host/bufferiszero.c.inc
index 3b9605d806f..74ae98580f6 100644
--- a/host/include/i386/host/bufferiszero.c.inc
+++ b/host/include/i386/host/bufferiszero.c.inc
@@ -110,13 +110,14 @@ static biz_accel_fn const accel_table[] = {
static unsigned best_accel(void)
{
-#ifdef CONFIG_AVX2_OPT
unsigned info = cpuinfo_init();
+
+#ifdef CONFIG_AVX2_OPT
if (info & CPUINFO_AVX2) {
return 2;
}
#endif
-return 1;
+return info & CPUINFO_SSE2 ? 1 : 0;
}
#else
--
2.45.2
[PULL 16/23] target/i386: give CC_OP_POPCNT low bits corresponding to MO_TL
Handle it like the other arithmetic cc_ops. This simplifies a
bit the implementation of bit test instructions.
Reviewed-by: Richard Henderson
Signed-off-by: Paolo Bonzini
---
target/i386/cpu.h | 13 +++--
target/i386/tcg/translate.c | 3 +--
2 files changed, 12 insertions(+), 4 deletions(-)
diff --git a/target/i386/cpu.h b/target/i386/cpu.h
index 1b4edbe0580..29daf370485 100644
--- a/target/i386/cpu.h
+++ b/target/i386/cpu.h
@@ -1275,6 +1275,7 @@ typedef enum {
CC_OP_ADCX, /* CC_DST = C, CC_SRC = rest. */
CC_OP_ADOX, /* CC_SRC2 = O, CC_SRC = rest. */
CC_OP_ADCOX, /* CC_DST = C, CC_SRC2 = O, CC_SRC = rest. */
+CC_OP_CLR, /* Z and P set, all other flags clear. */
CC_OP_MULB, /* modify all flags, C, O = (CC_SRC != 0) */
CC_OP_MULW,
@@ -1331,8 +1332,16 @@ typedef enum {
CC_OP_BMILGL,
CC_OP_BMILGQ,
-CC_OP_CLR, /* Z set, all other flags clear. */
-CC_OP_POPCNT, /* Z via CC_DST, all other flags clear. */
+/*
+ * Note that only CC_OP_POPCNT (i.e. the one with MO_TL size)
+ * is used or implemented, because the translation needs
+ * to zero-extend CC_DST anyway.
+ */
+CC_OP_POPCNTB__, /* Z via CC_DST, all other flags clear. */
+CC_OP_POPCNTW__,
+CC_OP_POPCNTL__,
+CC_OP_POPCNTQ__,
+CC_OP_POPCNT = sizeof(target_ulong) == 8 ? CC_OP_POPCNTQ__ :
CC_OP_POPCNTL__,
CC_OP_NB,
} CCOp;
diff --git a/target/i386/tcg/translate.c b/target/i386/tcg/translate.c
index eb353dc3c9f..934c514e64f 100644
--- a/target/i386/tcg/translate.c
+++ b/target/i386/tcg/translate.c
@@ -1019,8 +1019,6 @@ static CCPrepare gen_prepare_eflags_z(DisasContext *s,
TCGv reg)
.imm = CC_Z };
case CC_OP_CLR:
return (CCPrepare) { .cond = TCG_COND_ALWAYS };
-case CC_OP_POPCNT:
-return (CCPrepare) { .cond = TCG_COND_EQ, .reg = cpu_cc_dst };
default:
{
MemOp size = (s->cc_op - CC_OP_ADDB) & 3;
@@ -3177,6 +3175,7 @@ static void disas_insn_old(DisasContext *s, CPUState
*cpu, int b)
case CC_OP_SHLB ... CC_OP_SHLQ:
case CC_OP_SARB ... CC_OP_SARQ:
case CC_OP_BMILGB ... CC_OP_BMILGQ:
+case CC_OP_POPCNT:
/* Z was going to be computed from the non-zero status of CC_DST.
We can get that same Z value (and the new C value) by leaving
CC_DST alone, setting CC_SRC, and using a CC_OP_SAR of the
--
2.45.2
[PULL 02/23] Revert "host/i386: assume presence of POPCNT"
This reverts commit 45ccdbcb24baf99667997fac5cf60318e5e7db51.
The x86-64 instruction set can now be tuned down to x86-64 v1
or i386 Pentium Pro.
Signed-off-by: Paolo Bonzini
---
host/include/i386/host/cpuinfo.h | 1 +
tcg/i386/tcg-target.h| 5 +++--
util/cpuinfo-i386.c | 1 +
3 files changed, 5 insertions(+), 2 deletions(-)
diff --git a/host/include/i386/host/cpuinfo.h b/host/include/i386/host/cpuinfo.h
index c1e94d75ce1..72f6fad61e5 100644
--- a/host/include/i386/host/cpuinfo.h
+++ b/host/include/i386/host/cpuinfo.h
@@ -11,6 +11,7 @@
#define CPUINFO_ALWAYS (1u << 0) /* so cpuinfo is nonzero */
#define CPUINFO_MOVBE (1u << 2)
#define CPUINFO_LZCNT (1u << 3)
+#define CPUINFO_POPCNT (1u << 4)
#define CPUINFO_BMI1(1u << 5)
#define CPUINFO_BMI2(1u << 6)
#define CPUINFO_AVX1(1u << 9)
diff --git a/tcg/i386/tcg-target.h b/tcg/i386/tcg-target.h
index ecc69827287..2f67a97e059 100644
--- a/tcg/i386/tcg-target.h
+++ b/tcg/i386/tcg-target.h
@@ -111,6 +111,7 @@ typedef enum {
#endif
#define have_bmi1 (cpuinfo & CPUINFO_BMI1)
+#define have_popcnt (cpuinfo & CPUINFO_POPCNT)
#define have_avx1 (cpuinfo & CPUINFO_AVX1)
#define have_avx2 (cpuinfo & CPUINFO_AVX2)
#define have_movbe(cpuinfo & CPUINFO_MOVBE)
@@ -142,7 +143,7 @@ typedef enum {
#define TCG_TARGET_HAS_nor_i32 0
#define TCG_TARGET_HAS_clz_i32 1
#define TCG_TARGET_HAS_ctz_i32 1
-#define TCG_TARGET_HAS_ctpop_i321
+#define TCG_TARGET_HAS_ctpop_i32have_popcnt
#define TCG_TARGET_HAS_deposit_i32 1
#define TCG_TARGET_HAS_extract_i32 1
#define TCG_TARGET_HAS_sextract_i32 1
@@ -177,7 +178,7 @@ typedef enum {
#define TCG_TARGET_HAS_nor_i64 0
#define TCG_TARGET_HAS_clz_i64 1
#define TCG_TARGET_HAS_ctz_i64 1
-#define TCG_TARGET_HAS_ctpop_i641
+#define TCG_TARGET_HAS_ctpop_i64have_popcnt
#define TCG_TARGET_HAS_deposit_i64 1
#define TCG_TARGET_HAS_extract_i64 1
#define TCG_TARGET_HAS_sextract_i64 0
diff --git a/util/cpuinfo-i386.c b/util/cpuinfo-i386.c
index 8f2694d88f2..6d474a6259a 100644
--- a/util/cpuinfo-i386.c
+++ b/util/cpuinfo-i386.c
@@ -35,6 +35,7 @@ unsigned __attribute__((constructor)) cpuinfo_init(void)
__cpuid(1, a, b, c, d);
info |= (c & bit_MOVBE ? CPUINFO_MOVBE : 0);
+info |= (c & bit_POPCNT ? CPUINFO_POPCNT : 0);
info |= (c & bit_PCLMUL ? CPUINFO_PCLMUL : 0);
/* NOTE: our AES support requires SSSE3 (PSHUFB) as well. */
--
2.45.2
[PULL 23/23] target/i386/sev: Fix printf formats
From: Richard Henderson
hwaddr uses HWADDR_PRIx, sizeof yields size_t so uses %zu,
and gsize uses G_GSIZE_FORMAT.
Signed-off-by: Richard Henderson
Reviewed-by: Philippe Mathieu-Daudé
Link:
https://lore.kernel.org/r/20240626194950.1725800-4-richard.hender...@linaro.org
Signed-off-by: Paolo Bonzini
---
target/i386/sev.c | 17 +++--
1 file changed, 11 insertions(+), 6 deletions(-)
diff --git a/target/i386/sev.c b/target/i386/sev.c
index 0ffdf8952c3..3ab8b3c28b7 100644
--- a/target/i386/sev.c
+++ b/target/i386/sev.c
@@ -934,8 +934,9 @@ sev_snp_launch_update(SevSnpGuestState *sev_snp_guest,
out:
if (!ret && update.gfn_start << TARGET_PAGE_BITS != data->gpa + data->len)
{
-error_report("SEV-SNP: expected update of GPA range %lx-%lx,"
- "got GPA range %lx-%llx",
+error_report("SEV-SNP: expected update of GPA range %"
+ HWADDR_PRIx "-%" HWADDR_PRIx ","
+ "got GPA range %" HWADDR_PRIx "-%llx",
data->gpa, data->gpa + data->len, data->gpa,
update.gfn_start << TARGET_PAGE_BITS);
ret = -EIO;
@@ -2148,7 +2149,8 @@ sev_snp_guest_set_guest_visible_workarounds(Object *obj,
const char *value,
}
if (len != sizeof(start->gosvw)) {
-error_setg(errp, "parameter length of %lu exceeds max of %lu",
+error_setg(errp, "parameter length of %" G_GSIZE_FORMAT
+ " exceeds max of %zu",
len, sizeof(start->gosvw));
return;
}
@@ -2185,7 +2187,8 @@ sev_snp_guest_set_id_block(Object *obj, const char
*value, Error **errp)
}
if (len != KVM_SEV_SNP_ID_BLOCK_SIZE) {
-error_setg(errp, "parameter length of %lu not equal to %u",
+error_setg(errp, "parameter length of %" G_GSIZE_FORMAT
+ " not equal to %u",
len, KVM_SEV_SNP_ID_BLOCK_SIZE);
return;
}
@@ -2223,7 +2226,8 @@ sev_snp_guest_set_id_auth(Object *obj, const char *value,
Error **errp)
}
if (len > KVM_SEV_SNP_ID_AUTH_SIZE) {
-error_setg(errp, "parameter length:ID_AUTH %lu exceeds max of %u",
+error_setg(errp, "parameter length:ID_AUTH %" G_GSIZE_FORMAT
+ " exceeds max of %u",
len, KVM_SEV_SNP_ID_AUTH_SIZE);
return;
}
@@ -2291,7 +2295,8 @@ sev_snp_guest_set_host_data(Object *obj, const char
*value, Error **errp)
}
if (len != sizeof(finish->host_data)) {
-error_setg(errp, "parameter length of %lu not equal to %lu",
+error_setg(errp, "parameter length of %" G_GSIZE_FORMAT
+ " not equal to %zu",
len, sizeof(finish->host_data));
return;
}
--
2.45.2
[PULL 22/23] target/i386/sev: Use size_t for object sizes
From: Richard Henderson
This code was using both uint32_t and uint64_t for len.
Consistently use size_t instead.
Signed-off-by: Richard Henderson
Reviewed-by: Philippe Mathieu-Daudé
Link:
https://lore.kernel.org/r/20240626194950.1725800-3-richard.hender...@linaro.org
Signed-off-by: Paolo Bonzini
---
target/i386/sev.c| 16
target/i386/trace-events | 2 +-
2 files changed, 9 insertions(+), 9 deletions(-)
diff --git a/target/i386/sev.c b/target/i386/sev.c
index 28d6bd3adfa..0ffdf8952c3 100644
--- a/target/i386/sev.c
+++ b/target/i386/sev.c
@@ -121,7 +121,7 @@ struct SevCommonStateClass {
Error **errp);
int (*launch_start)(SevCommonState *sev_common);
void (*launch_finish)(SevCommonState *sev_common);
-int (*launch_update_data)(SevCommonState *sev_common, hwaddr gpa, uint8_t
*ptr, uint64_t len);
+int (*launch_update_data)(SevCommonState *sev_common, hwaddr gpa, uint8_t
*ptr, size_t len);
int (*kvm_init)(ConfidentialGuestSupport *cgs, Error **errp);
};
@@ -173,7 +173,7 @@ typedef struct SevLaunchUpdateData {
QTAILQ_ENTRY(SevLaunchUpdateData) next;
hwaddr gpa;
void *hva;
-uint64_t len;
+size_t len;
int type;
} SevLaunchUpdateData;
@@ -886,7 +886,7 @@ sev_snp_launch_update(SevSnpGuestState *sev_snp_guest,
if (!data->hva || !data->len) {
error_report("SNP_LAUNCH_UPDATE called with invalid address"
- "/ length: %p / %lx",
+ "/ length: %p / %zx",
data->hva, data->len);
return 1;
}
@@ -945,7 +945,8 @@ out:
}
static int
-sev_launch_update_data(SevCommonState *sev_common, hwaddr gpa, uint8_t *addr,
uint64_t len)
+sev_launch_update_data(SevCommonState *sev_common, hwaddr gpa,
+ uint8_t *addr, size_t len)
{
int ret, fw_error;
struct kvm_sev_launch_update_data update;
@@ -1090,8 +1091,7 @@ sev_launch_finish(SevCommonState *sev_common)
}
static int
-snp_launch_update_data(uint64_t gpa, void *hva,
- uint32_t len, int type)
+snp_launch_update_data(uint64_t gpa, void *hva, size_t len, int type)
{
SevLaunchUpdateData *data;
@@ -1108,7 +1108,7 @@ snp_launch_update_data(uint64_t gpa, void *hva,
static int
sev_snp_launch_update_data(SevCommonState *sev_common, hwaddr gpa,
- uint8_t *ptr, uint64_t len)
+ uint8_t *ptr, size_t len)
{
int ret = snp_launch_update_data(gpa, ptr, len,
KVM_SEV_SNP_PAGE_TYPE_NORMAL);
@@ -1165,7 +1165,7 @@ sev_snp_cpuid_info_fill(SnpCpuidInfo *snp_cpuid_info,
}
static int
-snp_launch_update_cpuid(uint32_t cpuid_addr, void *hva, uint32_t cpuid_len)
+snp_launch_update_cpuid(uint32_t cpuid_addr, void *hva, size_t cpuid_len)
{
KvmCpuidInfo kvm_cpuid_info = {0};
SnpCpuidInfo snp_cpuid_info;
diff --git a/target/i386/trace-events b/target/i386/trace-events
index 06b44ead2e2..51301673f0c 100644
--- a/target/i386/trace-events
+++ b/target/i386/trace-events
@@ -6,7 +6,7 @@ kvm_memcrypt_register_region(void *addr, size_t len) "addr %p
len 0x%zx"
kvm_memcrypt_unregister_region(void *addr, size_t len) "addr %p len 0x%zx"
kvm_sev_change_state(const char *old, const char *new) "%s -> %s"
kvm_sev_launch_start(int policy, void *session, void *pdh) "policy 0x%x
session %p pdh %p"
-kvm_sev_launch_update_data(void *addr, uint64_t len) "addr %p len 0x%" PRIx64
+kvm_sev_launch_update_data(void *addr, size_t len) "addr %p len 0x%zx"
kvm_sev_launch_measurement(const char *value) "data %s"
kvm_sev_launch_finish(void) ""
kvm_sev_launch_secret(uint64_t hpa, uint64_t hva, uint64_t secret, int len)
"hpa 0x%" PRIx64 " hva 0x%" PRIx64 " data 0x%" PRIx64 " len %d"
--
2.45.2
[PULL 21/23] target/i386: SEV: store pointer to decoded id_auth in SevSnpGuest
Do not rely on finish->id_auth_uaddr, so that there are no casts from
pointer to uint64_t. They break on 32-bit hosts.
Reviewed-by: Richard Henderson
Signed-off-by: Paolo Bonzini
---
target/i386/sev.c | 13 -
1 file changed, 8 insertions(+), 5 deletions(-)
diff --git a/target/i386/sev.c b/target/i386/sev.c
index a6b063b762c..28d6bd3adfa 100644
--- a/target/i386/sev.c
+++ b/target/i386/sev.c
@@ -155,6 +155,7 @@ struct SevSnpGuestState {
char *id_block_base64;
uint8_t *id_block;
char *id_auth_base64;
+uint8_t *id_auth;
char *host_data;
struct kvm_sev_snp_launch_start kvm_start_conf;
@@ -2208,16 +2209,16 @@ sev_snp_guest_set_id_auth(Object *obj, const char
*value, Error **errp)
struct kvm_sev_snp_launch_finish *finish = _snp_guest->kvm_finish_conf;
gsize len;
+finish->id_auth_uaddr = 0;
+g_free(sev_snp_guest->id_auth);
g_free(sev_snp_guest->id_auth_base64);
-g_free((guchar *)finish->id_auth_uaddr);
/* store the base64 str so we don't need to re-encode in getter */
sev_snp_guest->id_auth_base64 = g_strdup(value);
+sev_snp_guest->id_auth =
+qbase64_decode(sev_snp_guest->id_auth_base64, -1, , errp);
-finish->id_auth_uaddr =
-(uint64_t)qbase64_decode(sev_snp_guest->id_auth_base64, -1, ,
errp);
-
-if (!finish->id_auth_uaddr) {
+if (!sev_snp_guest->id_auth) {
return;
}
@@ -2226,6 +2227,8 @@ sev_snp_guest_set_id_auth(Object *obj, const char *value,
Error **errp)
len, KVM_SEV_SNP_ID_AUTH_SIZE);
return;
}
+
+finish->id_auth_uaddr = (uintptr_t)sev_snp_guest->id_auth;
}
static bool
--
2.45.2
[PULL 13/23] include: move typeof_strip_qual to compiler.h, use it in QAPI_LIST_LENGTH()
The typeof_strip_qual() is most useful for the atomic fetch-and-modify
operations in atomic.h, but it can be used elsewhere as well. For example,
QAPI_LIST_LENGTH() assumes that the argument is not const, which is not a
requirement.
Move the macro to compiler.h and, while at it, move it under #ifndef
__cplusplus to emphasize that it uses C-only constructs. A C++ version
of typeof_strip_qual() using type traits is possible[1], but beyond the
scope of this patch because the little C++ code that is in QEMU does not
use QAPI.
The patch was tested by changing the declaration of strv_from_str_list()
in qapi/qapi-type-helpers.c to:
char **strv_from_str_list(const strList *const list)
This is valid C code, and it fails to compile without this change.
[1] https://lore.kernel.org/qemu-devel/20240624205647.112034-1-f...@google.com/
Reviewed-by: Richard Henderson
Reviewed-by: Manos Pitsidianakis
Tested-by: Manos Pitsidianakis
Signed-off-by: Paolo Bonzini
---
include/qapi/util.h | 2 +-
include/qemu/atomic.h | 42 -
include/qemu/compiler.h | 46 +
3 files changed, 47 insertions(+), 43 deletions(-)
diff --git a/include/qapi/util.h b/include/qapi/util.h
index 20dfea8a545..b8254247b8d 100644
--- a/include/qapi/util.h
+++ b/include/qapi/util.h
@@ -62,7 +62,7 @@ int parse_qapi_name(const char *name, bool complete);
#define QAPI_LIST_LENGTH(list) \
({ \
size_t _len = 0;\
-typeof(list) _tail; \
+typeof_strip_qual(list) _tail; \
for (_tail = list; _tail != NULL; _tail = _tail->next) {\
_len++; \
} \
diff --git a/include/qemu/atomic.h b/include/qemu/atomic.h
index 99110abefb3..dc4118ddd9e 100644
--- a/include/qemu/atomic.h
+++ b/include/qemu/atomic.h
@@ -20,48 +20,6 @@
/* Compiler barrier */
#define barrier() ({ asm volatile("" ::: "memory"); (void)0; })
-/* The variable that receives the old value of an atomically-accessed
- * variable must be non-qualified, because atomic builtins return values
- * through a pointer-type argument as in __atomic_load(, , MODEL).
- *
- * This macro has to handle types smaller than int manually, because of
- * implicit promotion. int and larger types, as well as pointers, can be
- * converted to a non-qualified type just by applying a binary operator.
- */
-#define typeof_strip_qual(expr)
\
- typeof(
\
-__builtin_choose_expr(
\
- __builtin_types_compatible_p(typeof(expr), bool) ||
\
-__builtin_types_compatible_p(typeof(expr), const bool) ||
\
-__builtin_types_compatible_p(typeof(expr), volatile bool) ||
\
-__builtin_types_compatible_p(typeof(expr), const volatile bool),
\
-(bool)1,
\
-__builtin_choose_expr(
\
- __builtin_types_compatible_p(typeof(expr), signed char) ||
\
-__builtin_types_compatible_p(typeof(expr), const signed char) ||
\
-__builtin_types_compatible_p(typeof(expr), volatile signed char) ||
\
-__builtin_types_compatible_p(typeof(expr), const volatile signed
char),\
-(signed char)1,
\
-__builtin_choose_expr(
\
- __builtin_types_compatible_p(typeof(expr), unsigned char) ||
\
-__builtin_types_compatible_p(typeof(expr), const unsigned char) ||
\
-__builtin_types_compatible_p(typeof(expr), volatile unsigned char) ||
\
-__builtin_types_compatible_p(typeof(expr), const volatile unsigned
char), \
-(unsigned char)1,
\
-__builtin_choose_expr(
\
- __builtin_types_compatible_p(typeof(expr), signed short) ||
\
-__builtin_types_compatible_p(typeof(expr), const signed short) ||
\
-__builtin_types_compatible_p(typeof(expr), volatile signed short) ||
\
-__builtin_types_compatible_p(typeof(expr), const volatile signed
short), \
-(signed short)1,
\
-__builtin_choose_expr(
[PULL 08/23] block: do not check bdrv_file_open
The set of BlockDrivers that have .bdrv_file_open coincides with those
that have .protocol_name and guess what---checking drv->bdrv_file_open
is done to see if the driver is a protocol. So check drv->protocol_name
instead.
Signed-off-by: Paolo Bonzini
---
block.c | 11 +--
1 file changed, 5 insertions(+), 6 deletions(-)
diff --git a/block.c b/block.c
index 69a2905178a..dd14ba85fc3 100644
--- a/block.c
+++ b/block.c
@@ -926,7 +926,6 @@ BlockDriver *bdrv_find_protocol(const char *filename,
int i;
GLOBAL_STATE_CODE();
-/* TODO Drivers without bdrv_file_open must be specified explicitly */
/*
* XXX(hch): we really should not let host device detection
@@ -1983,7 +1982,7 @@ static int bdrv_open_common(BlockDriverState *bs,
BlockBackend *file,
open_flags = bdrv_open_flags(bs, bs->open_flags);
node_name = qemu_opt_get(opts, "node-name");
-assert(!drv->bdrv_file_open || file == NULL);
+assert(!drv->protocol_name || file == NULL);
ret = bdrv_open_driver(bs, drv, node_name, options, open_flags, errp);
if (ret < 0) {
goto fail_opts;
@@ -2084,7 +2083,7 @@ static int bdrv_fill_options(QDict **options, const char
*filename,
}
/* If the user has explicitly specified the driver, this choice should
* override the BDRV_O_PROTOCOL flag */
-protocol = drv->bdrv_file_open;
+protocol = drv->protocol_name;
}
if (protocol) {
@@ -4123,7 +4122,7 @@ bdrv_open_inherit(const char *filename, const char
*reference, QDict *options,
}
/* BDRV_O_PROTOCOL must be set iff a protocol BDS is about to be created */
-assert(!!(flags & BDRV_O_PROTOCOL) == !!drv->bdrv_file_open);
+assert(!!(flags & BDRV_O_PROTOCOL) == !!drv->protocol_name);
/* file must be NULL if a protocol BDS is about to be created
* (the inverse results in an error message from bdrv_open_common()) */
assert(!(flags & BDRV_O_PROTOCOL) || !file);
@@ -5971,7 +5970,7 @@ int64_t coroutine_fn
bdrv_co_get_allocated_file_size(BlockDriverState *bs)
return drv->bdrv_co_get_allocated_file_size(bs);
}
-if (drv->bdrv_file_open) {
+if (drv->protocol_name) {
/*
* Protocol drivers default to -ENOTSUP (most of their data is
* not stored in any of their children (if they even have any),
@@ -8030,7 +8029,7 @@ void bdrv_refresh_filename(BlockDriverState *bs)
* Both of these conditions are represented by
generate_json_filename.
*/
if (primary_child_bs->exact_filename[0] &&
-primary_child_bs->drv->bdrv_file_open &&
+primary_child_bs->drv->protocol_name &&
!drv->is_filter && !generate_json_filename)
{
strcpy(bs->exact_filename, primary_child_bs->exact_filename);
--
2.45.2
[PULL 09/23] block: remove separate bdrv_file_open callback
bdrv_file_open and bdrv_open are completely equivalent, they are
never checked except to see which one to invoke. So merge them
into a single one.
Signed-off-by: Paolo Bonzini
---
include/block/block_int-common.h | 3 ---
block.c | 4 +---
block/blkdebug.c | 2 +-
block/blkio.c| 2 +-
block/blkverify.c| 2 +-
block/curl.c | 8
block/file-posix.c | 8
block/file-win32.c | 4 ++--
block/gluster.c | 6 +++---
block/iscsi.c| 4 ++--
block/nbd.c | 6 +++---
block/nfs.c | 2 +-
block/null.c | 4 ++--
block/nvme.c | 2 +-
block/rbd.c | 3 ++-
block/ssh.c | 2 +-
block/vvfat.c| 2 +-
17 files changed, 30 insertions(+), 34 deletions(-)
diff --git a/include/block/block_int-common.h b/include/block/block_int-common.h
index 761276127ed..ebb4e56a503 100644
--- a/include/block/block_int-common.h
+++ b/include/block/block_int-common.h
@@ -248,9 +248,6 @@ struct BlockDriver {
int GRAPH_UNLOCKED_PTR (*bdrv_open)(
BlockDriverState *bs, QDict *options, int flags, Error **errp);
-/* Protocol drivers should implement this instead of bdrv_open */
-int GRAPH_UNLOCKED_PTR (*bdrv_file_open)(
-BlockDriverState *bs, QDict *options, int flags, Error **errp);
void (*bdrv_close)(BlockDriverState *bs);
int coroutine_fn GRAPH_UNLOCKED_PTR (*bdrv_co_create)(
diff --git a/block.c b/block.c
index dd14ba85fc3..c1cc313d216 100644
--- a/block.c
+++ b/block.c
@@ -1655,9 +1655,7 @@ bdrv_open_driver(BlockDriverState *bs, BlockDriver *drv,
const char *node_name,
bs->opaque = g_malloc0(drv->instance_size);
assert(!drv->bdrv_needs_filename || bs->filename[0]);
-if (drv->bdrv_file_open) {
-ret = drv->bdrv_file_open(bs, options, open_flags, _err);
-} else if (drv->bdrv_open) {
+if (drv->bdrv_open) {
ret = drv->bdrv_open(bs, options, open_flags, _err);
} else {
ret = 0;
diff --git a/block/blkdebug.c b/block/blkdebug.c
index 9da8c9eddc2..c95c818c388 100644
--- a/block/blkdebug.c
+++ b/block/blkdebug.c
@@ -1073,7 +1073,7 @@ static BlockDriver bdrv_blkdebug = {
.is_filter = true,
.bdrv_parse_filename= blkdebug_parse_filename,
-.bdrv_file_open = blkdebug_open,
+.bdrv_open = blkdebug_open,
.bdrv_close = blkdebug_close,
.bdrv_reopen_prepare= blkdebug_reopen_prepare,
.bdrv_child_perm= blkdebug_child_perm,
diff --git a/block/blkio.c b/block/blkio.c
index 882e1c297b4..1a38064ce76 100644
--- a/block/blkio.c
+++ b/block/blkio.c
@@ -1088,7 +1088,7 @@ static void blkio_refresh_limits(BlockDriverState *bs,
Error **errp)
*/
#define BLKIO_DRIVER_COMMON \
.instance_size = sizeof(BDRVBlkioState), \
-.bdrv_file_open = blkio_file_open, \
+.bdrv_open = blkio_file_open, \
.bdrv_close = blkio_close, \
.bdrv_co_getlength = blkio_co_getlength, \
.bdrv_co_truncate= blkio_truncate, \
diff --git a/block/blkverify.c b/block/blkverify.c
index ec45d8335ed..5a9bf674d9c 100644
--- a/block/blkverify.c
+++ b/block/blkverify.c
@@ -321,7 +321,7 @@ static BlockDriver bdrv_blkverify = {
.instance_size= sizeof(BDRVBlkverifyState),
.bdrv_parse_filename = blkverify_parse_filename,
-.bdrv_file_open = blkverify_open,
+.bdrv_open= blkverify_open,
.bdrv_close = blkverify_close,
.bdrv_child_perm = bdrv_default_perms,
.bdrv_co_getlength= blkverify_co_getlength,
diff --git a/block/curl.c b/block/curl.c
index 419f7c89ef2..ef5252d00b5 100644
--- a/block/curl.c
+++ b/block/curl.c
@@ -1034,7 +1034,7 @@ static BlockDriver bdrv_http = {
.instance_size = sizeof(BDRVCURLState),
.bdrv_parse_filename= curl_parse_filename,
-.bdrv_file_open = curl_open,
+.bdrv_open = curl_open,
.bdrv_close = curl_close,
.bdrv_co_getlength = curl_co_getlength,
@@ -1053,7 +1053,7 @@ static BlockDriver bdrv_https = {
.instance_size = sizeof(BDRVCURLState),
.bdrv_parse_filename= curl_parse_filename,
-.bdrv_file_open = curl_open,
+.bdrv_open = curl_open,
.bdrv_close = curl_close,
.bdrv_co_getlength = curl_co_getlength,
@@ -1072,7 +1072,7 @@ static BlockDriver bdrv_ftp = {
.instance_size = sizeof(BDRVCURLState),
.bdrv_parse_filename= curl_parse_filename,
-.bdrv_file_open = curl_open,
+.bdrv_open
[PULL 10/23] block: rename former bdrv_file_open callbacks
Since there is no bdrv_file_open callback anymore, rename the implementations
so that they end with "_open" instead of "_file_open". NFS is the exception
because all the functions are named nfs_file_*.
Suggested-by: Kevin Wolf
Signed-off-by: Paolo Bonzini
---
block/blkio.c | 8
block/null.c | 8
block/nvme.c | 8
block/ssh.c | 6 +++---
4 files changed, 15 insertions(+), 15 deletions(-)
diff --git a/block/blkio.c b/block/blkio.c
index 1a38064ce76..3d9a2e764c3 100644
--- a/block/blkio.c
+++ b/block/blkio.c
@@ -713,7 +713,7 @@ static int blkio_virtio_blk_connect(BlockDriverState *bs,
QDict *options,
* for example will fail.
*
* In order to open the device read-only, we are using the `read-only`
- * property of the libblkio driver in blkio_file_open().
+ * property of the libblkio driver in blkio_open().
*/
fd = qemu_open(path, O_RDWR, NULL);
if (fd < 0) {
@@ -791,8 +791,8 @@ static int blkio_virtio_blk_connect(BlockDriverState *bs,
QDict *options,
return 0;
}
-static int blkio_file_open(BlockDriverState *bs, QDict *options, int flags,
- Error **errp)
+static int blkio_open(BlockDriverState *bs, QDict *options, int flags,
+ Error **errp)
{
const char *blkio_driver = bs->drv->protocol_name;
BDRVBlkioState *s = bs->opaque;
@@ -1088,7 +1088,7 @@ static void blkio_refresh_limits(BlockDriverState *bs,
Error **errp)
*/
#define BLKIO_DRIVER_COMMON \
.instance_size = sizeof(BDRVBlkioState), \
-.bdrv_open = blkio_file_open, \
+.bdrv_open = blkio_open, \
.bdrv_close = blkio_close, \
.bdrv_co_getlength = blkio_co_getlength, \
.bdrv_co_truncate= blkio_truncate, \
diff --git a/block/null.c b/block/null.c
index 6fa64d20d86..4730acc1eb2 100644
--- a/block/null.c
+++ b/block/null.c
@@ -77,8 +77,8 @@ static void null_aio_parse_filename(const char *filename,
QDict *options,
}
}
-static int null_file_open(BlockDriverState *bs, QDict *options, int flags,
- Error **errp)
+static int null_open(BlockDriverState *bs, QDict *options, int flags,
+ Error **errp)
{
QemuOpts *opts;
BDRVNullState *s = bs->opaque;
@@ -283,7 +283,7 @@ static BlockDriver bdrv_null_co = {
.protocol_name = "null-co",
.instance_size = sizeof(BDRVNullState),
-.bdrv_open = null_file_open,
+.bdrv_open = null_open,
.bdrv_parse_filename= null_co_parse_filename,
.bdrv_co_getlength = null_co_getlength,
.bdrv_co_get_allocated_file_size = null_co_get_allocated_file_size,
@@ -304,7 +304,7 @@ static BlockDriver bdrv_null_aio = {
.protocol_name = "null-aio",
.instance_size = sizeof(BDRVNullState),
-.bdrv_open = null_file_open,
+.bdrv_open = null_open,
.bdrv_parse_filename= null_aio_parse_filename,
.bdrv_co_getlength = null_co_getlength,
.bdrv_co_get_allocated_file_size = null_co_get_allocated_file_size,
diff --git a/block/nvme.c b/block/nvme.c
index c84914af6dd..3b588b139f6 100644
--- a/block/nvme.c
+++ b/block/nvme.c
@@ -889,7 +889,7 @@ out:
qemu_vfio_pci_unmap_bar(s->vfio, 0, (void *)regs, 0, sizeof(NvmeBar));
}
-/* Cleaning up is done in nvme_file_open() upon error. */
+/* Cleaning up is done in nvme_open() upon error. */
return ret;
}
@@ -967,8 +967,8 @@ static void nvme_close(BlockDriverState *bs)
g_free(s->device);
}
-static int nvme_file_open(BlockDriverState *bs, QDict *options, int flags,
- Error **errp)
+static int nvme_open(BlockDriverState *bs, QDict *options, int flags,
+ Error **errp)
{
const char *device;
QemuOpts *opts;
@@ -1630,7 +1630,7 @@ static BlockDriver bdrv_nvme = {
.create_opts = _create_opts_simple,
.bdrv_parse_filename = nvme_parse_filename,
-.bdrv_open= nvme_file_open,
+.bdrv_open= nvme_open,
.bdrv_close = nvme_close,
.bdrv_co_getlength= nvme_co_getlength,
.bdrv_probe_blocksizes= nvme_probe_blocksizes,
diff --git a/block/ssh.c b/block/ssh.c
index 1344822ed85..27d582e0e3d 100644
--- a/block/ssh.c
+++ b/block/ssh.c
@@ -837,8 +837,8 @@ static int connect_to_ssh(BDRVSSHState *s,
BlockdevOptionsSsh *opts,
return ret;
}
-static int ssh_file_open(BlockDriverState *bs, QDict *options, int bdrv_flags,
- Error **errp)
+static int ssh_open(BlockDriverState *bs, QDict *options, int bdrv_flags,
+Error **errp)
{
BDRVSSHState *s = bs->opaque;
BlockdevOptionsSsh *opts;
@@ -1362,7 +1362,7 @@ static BlockDriver bdrv_ssh = {
.protocol_name= "ssh",
.instance_size
[PULL 12/23] exec: don't use void* in pointer arithmetic in headers
From: Roman Kiryanov
void* pointer arithmetic is a GCC extentension which could not be
available in other build tools (e.g. C++). This changes removes this
assumption.
Signed-off-by: Roman Kiryanov
Suggested-by: Paolo Bonzini
Link: https://lore.kernel.org/r/20240620201654.598024-1-r...@google.com
Signed-off-by: Paolo Bonzini
---
include/exec/memory.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/include/exec/memory.h b/include/exec/memory.h
index 154626f9ad2..c26ede33d21 100644
--- a/include/exec/memory.h
+++ b/include/exec/memory.h
@@ -2764,7 +2764,7 @@ MemTxResult address_space_write_rom(AddressSpace *as,
hwaddr addr,
#include "exec/memory_ldst_phys.h.inc"
struct MemoryRegionCache {
-void *ptr;
+uint8_t *ptr;
hwaddr xlat;
hwaddr len;
FlatView *fv;
--
2.45.2
[PULL v3 00/23] Misc changes for 2024-06-28
The following changes since commit 28b8a57ad63670aa0ce90334523dc552b13b4336: Merge tag 'pull-riscv-to-apply-20240627-1' of https://github.com/alistair23/qemu into staging (2024-06-27 07:36:16 -0700) are available in the Git repository at: https://gitlab.com/bonzini/qemu.git tags/for-upstream for you to fetch changes up to b31d386781cf85c193f3b1355dd0604cd6a59943: target/i386/sev: Fix printf formats (2024-06-28 19:26:54 +0200) I dropped the bit test instructions and the rest of the decoder updates, because they were buggy and I didn't like any of the fixes I could come up with. Supersedes: <20240624135939.632257-1-pbonz...@redhat.com> * configure: detect --cpu=mipsisa64r6 * target/i386: decode address before going back to translate.c * meson: allow configuring the x86-64 baseline * meson: remove dead optimization option * exec: small changes to allow compilation with C++ in Android emulator * fix SEV compilation on 32-bit systems Paolo Bonzini (19): configure: detect --cpu=mipsisa64r6 Revert "host/i386: assume presence of POPCNT" Revert "host/i386: assume presence of SSSE3" Revert "host/i386: assume presence of SSE2" meson: allow configuring the x86-64 baseline meson: remove dead optimization option block: make assertion more generic block: do not check bdrv_file_open block: remove separate bdrv_file_open callback block: rename former bdrv_file_open callbacks include: move typeof_strip_qual to compiler.h, use it in QAPI_LIST_LENGTH() target/i386: fix CC_OP dump target/i386: use cpu_cc_dst for CC_OP_POPCNT target/i386: give CC_OP_POPCNT low bits corresponding to MO_TL target/i386: remove unused enum target/i386: SEV: rename sev_snp_guest->id_block target/i386: SEV: store pointer to decoded id_block in SevSnpGuest target/i386: SEV: rename sev_snp_guest->id_auth target/i386: SEV: store pointer to decoded id_auth in SevSnpGuest Richard Henderson (2): target/i386/sev: Use size_t for object sizes target/i386/sev: Fix printf formats Roman Kiryanov (2): exec: avoid using C++ keywords in function parameters exec: don't use void* in pointer arithmetic in headers configure | 2 +- meson.build | 54 +--- host/include/i386/host/cpuinfo.h | 2 + include/block/block_int-common.h | 3 - include/exec/memory.h | 6 +- include/qapi/util.h | 2 +- include/qemu/atomic.h | 42 - include/qemu/compiler.h | 46 ++ target/i386/cpu.h | 13 +++- tcg/i386/tcg-target.h | 5 +- block.c | 17 +++-- block/blkdebug.c | 2 +- block/blkio.c | 8 +-- block/blkverify.c | 2 +- block/curl.c | 8 +-- block/file-posix.c| 8 +-- block/file-win32.c| 4 +- block/gluster.c | 6 +- block/iscsi.c | 4 +- block/nbd.c | 6 +- block/nfs.c | 2 +- block/null.c | 8 +-- block/nvme.c | 8 +-- block/rbd.c | 3 +- block/ssh.c | 6 +- block/vvfat.c | 2 +- target/i386/cpu-dump.c| 101 +++--- target/i386/sev.c | 71 - target/i386/tcg/cc_helper.c | 2 +- target/i386/tcg/translate.c | 21 +-- util/cpuinfo-i386.c | 6 +- host/include/i386/host/bufferiszero.c.inc | 5 +- target/i386/tcg/emit.c.inc| 4 +- meson_options.txt | 5 +- scripts/meson-buildoptions.sh | 6 +- target/i386/trace-events | 2 +- 36 files changed, 256 insertions(+), 236 deletions(-) -- 2.45.2
[PULL 01/23] configure: detect --cpu=mipsisa64r6
Treat it as a MIPS64 machine. Reviewed-by: Philippe Mathieu-Daudé Reviewed-by: Thomas Huth Signed-off-by: Paolo Bonzini --- configure | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/configure b/configure index 5ad1674ca5f..8b6a2f16ceb 100755 --- a/configure +++ b/configure @@ -450,7 +450,7 @@ case "$cpu" in linux_arch=loongarch ;; - mips64*) + mips64*|mipsisa64*) cpu=mips64 host_arch=mips linux_arch=mips -- 2.45.2
Re: [PATCH v2 5/6] tests/tcg/aarch64: Do not use x constraint
On 6/27/24 06:58, Akihiko Odaki wrote: clang version 18.1.6 does not support x constraint for AArch64. Use w instead. Signed-off-by: Akihiko Odaki --- tests/tcg/arm/fcvt.c | 14 +++--- 1 file changed, 7 insertions(+), 7 deletions(-) Oops, this was an error from the beginning. Reviewed-by: Richard Henderson r~
Re: [PATCH v2 4/6] tests/tcg/aarch64: Fix irg operand type
On 6/27/24 06:58, Akihiko Odaki wrote: irg expects 64-bit integers. Passing a 32-bit integer results in compilation failure with clang version 18.1.6. Signed-off-by: Akihiko Odaki --- tests/tcg/aarch64/mte-1.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) Reviewed-by: Richard Henderson r~
Re: [PATCH v2 3/6] tests/tcg/aarch64: Explicitly specify register width
On 6/27/24 06:58, Akihiko Odaki wrote: clang version 18.1.6 assumes a register is 64-bit by default and complains if a 32-bit value is given. Explicitly specify register width when passing a 32-bit value. Signed-off-by: Akihiko Odaki Reviewed-by: Philippe Mathieu-Daudé --- tests/tcg/aarch64/bti-1.c | 6 +++--- tests/tcg/aarch64/bti-3.c | 6 +++--- 2 files changed, 6 insertions(+), 6 deletions(-) This is true of clang 14 as well, so perhaps remove the version statement entirely. Reviewed-by: Richard Henderson r~
Re: [PATCH v2 2/6] tests/tcg/aarch64: Fix test architecture specification
On 6/27/24 06:58, Akihiko Odaki wrote:
sme-smopa-2.c requires sme-i16i64 but the compiler option used not to
specify it. Instead, the extension was specified with the inline
assembly, resulting in mixing assembly code targeting sme-i1664 and C
code that does not target sme-i1664.
clang version 18.1.6 does not support such mixing so properly specify
the extension with the compiler option instead.
Signed-off-by: Akihiko Odaki
---
tests/tcg/aarch64/sme-smopa-2.c | 2 +-
tests/tcg/aarch64/Makefile.target | 11 +--
2 files changed, 10 insertions(+), 3 deletions(-)
diff --git a/tests/tcg/aarch64/sme-smopa-2.c b/tests/tcg/aarch64/sme-smopa-2.c
index c9f48c3bfca2..2c9707065992 100644
--- a/tests/tcg/aarch64/sme-smopa-2.c
+++ b/tests/tcg/aarch64/sme-smopa-2.c
@@ -14,7 +14,7 @@ int main()
long svl;
/* Validate that we have a wide enough vector for 4 elements. */
-asm(".arch armv8-r+sme-i64\n\trdsvl %0, #1" : "=r"(svl));
+asm("rdsvl %0, #1" : "=r"(svl));
if (svl < 32) {
return 0;
}
diff --git a/tests/tcg/aarch64/Makefile.target
b/tests/tcg/aarch64/Makefile.target
index 70d728ae9af7..ad99e0e3b198 100644
--- a/tests/tcg/aarch64/Makefile.target
+++ b/tests/tcg/aarch64/Makefile.target
@@ -27,7 +27,8 @@ config-cc.mak: Makefile
$(call cc-option,-march=armv8.5-a,
CROSS_CC_HAS_ARMV8_5); \
$(call cc-option,-mbranch-protection=standard,
CROSS_CC_HAS_ARMV8_BTI); \
$(call cc-option,-march=armv8.5-a+memtag,
CROSS_CC_HAS_ARMV8_MTE); \
- $(call cc-option,-Wa$(COMMA)-march=armv9-a+sme,
CROSS_AS_HAS_ARMV9_SME)) 3> config-cc.mak
+ $(call cc-option,-Wa$(COMMA)-march=armv9-a+sme,
CROSS_AS_HAS_ARMV9_SME); \
+ $(call cc-option,-march=armv9-a+sme-i16i64,
CROSS_AS_HAS_ARMV9_SME_I1664)) 3> config-cc.mak
-include config-cc.mak
ifneq ($(CROSS_CC_HAS_ARMV8_2),)
@@ -68,7 +69,13 @@ endif
# SME Tests
ifneq ($(CROSS_AS_HAS_ARMV9_SME),)
-AARCH64_TESTS += sme-outprod1 sme-smopa-1 sme-smopa-2
+AARCH64_TESTS += sme-outprod1 sme-smopa-1
+endif
+
+# SME I16I64 Tests
+ifneq ($(CROSS_AS_HAS_ARMV9_SME_I1664),)
+AARCH64_TESTS += sme-smopa-2
+sme-smopa-2: CFLAGS += -march=armv9-a+sme-i16i64
endif
How interesting. We were not actually passing -march=armv9-a+sme to the assembler
previously. Lack of this is what is causing sme-outprod1 to fail to build, as reported by
Alex.
That said, if we use compiler directives we must have gcc-14 or newer to test this,
whereas binutils supported sme (and extensions) much earlier. Given that this is all
inline assembly, we do not really need compiler support.
I think we should continue to pass assembler options (-Wa,...) and detect and use clang's
-no-integrated-as option as well, at least for the SME tests.
r~
Re: [PATCH v2 1/6] tests/tcg/arm: Fix fcvt result messages
On 6/27/24 06:58, Akihiko Odaki wrote:
The test cases for "converting double-precision to single-precision"
emits float but the result variable was typed as uint32_t and corrupted
the printed values. Propertly type it as float.
Signed-off-by: Akihiko Odaki
Fixes: 8ec8a55e3fc9 ("tests/tcg/arm: add fcvt test cases for AArch32/64")
---
tests/tcg/arm/fcvt.c | 2 +-
tests/tcg/aarch64/fcvt.ref | 604 ++---
2 files changed, 303 insertions(+), 303 deletions(-)
Reviewed-by: Richard Henderson
r~
Re: [PATCH v3 1/3] tests/acpi: pc: allow DSDT acpi table changes
Hi Igor On Fri, 28 Jun 2024 at 13:25, Igor Mammedov wrote: > > On Fri, 7 Jun 2024 14:17:24 + > Ricardo Ribalda wrote: > > > Signed-off-by: Ricardo Ribalda > > --- > > tests/qtest/bios-tables-test-allowed-diff.h | 1 + > > 1 file changed, 1 insertion(+) > > > > diff --git a/tests/qtest/bios-tables-test-allowed-diff.h > > b/tests/qtest/bios-tables-test-allowed-diff.h > > index dfb8523c8b..b2c2c10cbc 100644 > > --- a/tests/qtest/bios-tables-test-allowed-diff.h > > +++ b/tests/qtest/bios-tables-test-allowed-diff.h > > @@ -1 +1,2 @@ > > /* List of comma-separated changed AML files to ignore */ > > +"tests/data/acpi/pc/DSDT", > > that's no enough, a lot more tables expected blobs are affected by > the next patch. > Sorry about that, I did not realise that the check was quitting after the first different file was found. will post a new version soon Thanks! > > before posting, make sure that 'make check-qtest' passes fine > -- Ricardo Ribalda
Re: [PATCH v6 06/11] target/arm: Factor out code for setting MTE TCF0 field
On 6/28/24 08:49, Gustavo Romero wrote: I thought you meant osdep.h should not be included _at all_ in my case, either in mte_user_helper.h or in mte_user_helper.c. Maybe the wording in the docs should be "Do not include "qemu/osdep.h" from header files. Include it from .c files, when necessary.". Not "when necessary", always, and always first. See the "Include directives" section of docs/devel/style.rst, which does explicitly say 'Do not include "qemu/osdep.h" from header files'. I think we agree osdep.h is necessary and must be put in mte_user_helper.c. But that left me wondering how it would work for sources including mte_user_helper.h, because it can be the case they don't have the declarations for the types used in the function prototypes, in this case, for CPUArchState and abi_long types in arm_set_mte_tcf0. CPUArchState will come from qemu/typedefs.h via osdep.h. For this particular function, 'int' would have been enough, since we only care about the low two bits. r~
Re: [PATCH 0/2] target/arm: Always build Aarch64 gdbstub helpers
On 6/28/24 09:37, Philippe Mathieu-Daudé wrote: On 28/6/24 16:31, Richard Henderson wrote: On 6/19/24 05:49, Philippe Mathieu-Daudé wrote: Merge gdbstub64.c in gdbstub.c and remove uses of target specific TARGET_AARCH64 definition. Small step toward single ARM/Aarch64 binary. Philippe Mathieu-Daudé (2): target/arm: Merge gdbstub64.c within gdbstub.c target/arm: Always build Aarch64 gdbstub helpers target/arm/cpu.h | 8 +- target/arm/internals.h | 2 - target/arm/gdbstub.c | 363 +- target/arm/gdbstub64.c | 383 - target/arm/meson.build | 1 - 5 files changed, 364 insertions(+), 393 deletions(-) delete mode 100644 target/arm/gdbstub64.c Are we attempting a single binary for user-only as well? No, due to ABI constraints, right? I did a user-emulation smoke build, no failure, did I miss something? Well, no. But qemu-arm does not need gdbstub64.c. Given TARGET_AARCH64 will be set on a combined build, I'm not sure what is the fix? r~
Re: [PATCH 0/2] target/arm: Always build Aarch64 gdbstub helpers
On 28/6/24 16:31, Richard Henderson wrote: On 6/19/24 05:49, Philippe Mathieu-Daudé wrote: Merge gdbstub64.c in gdbstub.c and remove uses of target specific TARGET_AARCH64 definition. Small step toward single ARM/Aarch64 binary. Philippe Mathieu-Daudé (2): target/arm: Merge gdbstub64.c within gdbstub.c target/arm: Always build Aarch64 gdbstub helpers target/arm/cpu.h | 8 +- target/arm/internals.h | 2 - target/arm/gdbstub.c | 363 +- target/arm/gdbstub64.c | 383 - target/arm/meson.build | 1 - 5 files changed, 364 insertions(+), 393 deletions(-) delete mode 100644 target/arm/gdbstub64.c Are we attempting a single binary for user-only as well? No, due to ABI constraints, right? I did a user-emulation smoke build, no failure, did I miss something?
Re: [PATCH 4/9] target/arm: Support migration when FPSR/FPCR won't fit in the FPSCR
On Fri, 28 Jun 2024 at 17:01, Richard Henderson
wrote:
>
> On 6/28/24 07:23, Peter Maydell wrote:
> > To support FPSR and FPCR bits that don't exist in the AArch32 FPSCR
> > view of floating point control and status (such as the FEAT_AFP ones),
> > we need to make sure those bits can be migrated. This commit allows
> > that, whilst maintaining backwards and forwards migration compatibility
> > for CPUs where there are no such bits:
> >
> > On sending:
> > * If either the FPCR or the FPSR include set bits that are not
> > visible in the AArch32 FPSCR view of floating point control/status
> > then we send the FPCR and FPSR as two separate fields in a new
> > cpu/vfp/fpcr_fpsr subsection, and we send a 0 for the old
> > FPSCR field in cpu/vfp
> > * Otherwise, we don't send the fpcr_fpsr subsection, and we send
> > an FPSCR-format value in cpu/vfp as we did previously
> >
> > On receiving:
> > * if we see a non-zero FPSCR field, that is the right information
> > * if we see a fpcr_fpsr subsection then that has the information
> > * if we see neither, then FPSCR/FPCR/FPSR are all zero on the source;
> > cpu_pre_load() ensures the CPU state defaults to that
> > * if we see both, then the migration source is buggy or malicious;
> > either the fpcr_fpsr or the FPSCR will "win" depending which
> > is first in the migration stream; we don't care which that is
> >
> > We make the new FPCR and FPSR on-the-wire data be 64 bits, because
> > architecturally these registers are that wide, and this avoids the
> > need to engage in further migration-compatibility contortions in
> > future if some new architecture revision defines bits in the high
> > half of either register.
> >
> > (We won't ever send the new migration subsection until we add support
> > for a CPU feature which enables setting overlapping FPCR bits, like
> > FEAT_AFP.)
> >
> > Signed-off-by: Peter Maydell
> > ---
> > target/arm/machine.c | 134 ++-
> > 1 file changed, 132 insertions(+), 2 deletions(-)
>
> Reviewed-by: Richard Henderson
>
> Not ideal, as vfp_get_{fpcr,fpsr} are called 3 or 4 times during migration.
> But unless we
> have separate 'fp*r_migrate' fields in cpu state, initialized in pre_save,
> there's no
> getting around it. And I suppose migration isn't exactly performance
> critical.
Yeah, we could have done it that way, but I am assuming that
the time taken for this is pretty miniscule in the general
scheme of how long migration takes, so I preferred the
way that doesn't clutter up the CPU state struct with
migration-only fields.
If somebody cares about migration downtime performance (which
does actually matter for some workload/use cases AIUI) they
can do some benchmarking and tell us what the actually
slow parts are :-)
thanks
-- PMM
Re: [PATCH v2] hw/ide/macio.c: switch from using qemu_allocate_irq() to qdev input GPIOs
On 28/6/24 18:03, Mark Cave-Ayland wrote: This prevents the IRQs from being leaked when the macio IDE device is used. Signed-off-by: Mark Cave-Ayland Reviewed-by: Peter Maydell --- hw/ide/macio.c| 10 ++ include/hw/misc/macio/macio.h | 7 +-- 2 files changed, 11 insertions(+), 6 deletions(-) Reviewed-by: Philippe Mathieu-Daudé
Re: [PATCH] util/cpuinfo-ppc: Add FreeBSD support
On 6/27/24 19:00, Brad Smith wrote: util/cpuinfo-ppc: Add FreeBSD support Signed-off-by: Brad Smith --- With corrected sign-off. Also this was based on the tcg-next branch. util/cpuinfo-ppc.c | 7 ++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/util/cpuinfo-ppc.c b/util/cpuinfo-ppc.c index 47af55aa0c..0ad634b46f 100644 --- a/util/cpuinfo-ppc.c +++ b/util/cpuinfo-ppc.c @@ -14,6 +14,11 @@ # include "elf.h" # endif #endif +#ifdef __FreeBSD__ +# include +# define PPC_FEATURE2_ARCH_3_1 0 I assume freebsd will eventually add this bit. Perhaps better with ifndef? r~ +# define PPC_FEATURE2_VEC_CRYPTO PPC_FEATURE2_HAS_VEC_CRYPTO +#endif unsigned cpuinfo; @@ -28,7 +33,7 @@ unsigned __attribute__((constructor)) cpuinfo_init(void) info = CPUINFO_ALWAYS; -#ifdef CONFIG_LINUX +#if defined(CONFIG_LINUX) || defined(__FreeBSD__) unsigned long hwcap = qemu_getauxval(AT_HWCAP); unsigned long hwcap2 = qemu_getauxval(AT_HWCAP2);
Re: [PATCH v6 06/11] target/arm: Factor out code for setting MTE TCF0 field
On 28/6/24 17:49, Gustavo Romero wrote: Hi Phil, On 6/28/24 4:08 AM, Philippe Mathieu-Daudé wrote: On 28/6/24 07:08, Gustavo Romero wrote: Factor out the code used for setting the MTE TCF0 field from the prctl code into a convenient function. Other subsystems, like gdbstub, need to set this field as well, so keep it as a separate function to avoid duplication and ensure consistency in how this field is set across the board. Signed-off-by: Gustavo Romero --- linux-user/aarch64/meson.build | 2 ++ linux-user/aarch64/mte_user_helper.c | 34 linux-user/aarch64/mte_user_helper.h | 25 linux-user/aarch64/target_prctl.h | 22 ++ 4 files changed, 63 insertions(+), 20 deletions(-) create mode 100644 linux-user/aarch64/mte_user_helper.c create mode 100644 linux-user/aarch64/mte_user_helper.h So, how about: diff --git a/linux-user/aarch64/mte_user_helper.c b/linux-user/aarch64/mte_user_helper.c index 8be6deaf03..a0e8abd551 100644 --- a/linux-user/aarch64/mte_user_helper.c +++ b/linux-user/aarch64/mte_user_helper.c @@ -6,7 +6,9 @@ * SPDX-License-Identifier: LGPL-2.1-or-later */ +#include "qemu/osdep.h" #include +#include "cpu.h" #include "mte_user_helper.h" void arm_set_mte_tcf0(CPUArchState *env, abi_long value) diff --git a/linux-user/aarch64/mte_user_helper.h b/linux-user/aarch64/mte_user_helper.h index ee3f6b190a..07fc0bcebf 100644 --- a/linux-user/aarch64/mte_user_helper.h +++ b/linux-user/aarch64/mte_user_helper.h @@ -9,9 +9,6 @@ #ifndef AARCH64_MTE_USER_HELPER_H #define AARCH64_MTE USER_HELPER_H -#include "qemu/osdep.h" -#include "qemu.h" - /** * arm_set_mte_tcf0 - Set TCF0 field in SCTLR_EL1 register * @env: The CPU environment Reviewed-by: Philippe Mathieu-Daudé
Re: [PATCH 9/9] target/arm: Allow FPCR bits that aren't in FPSCR
On 6/28/24 07:23, Peter Maydell wrote: In order to allow FPCR bits that aren't in the FPSCR (like the new bits that are defined for FEAT_AFP), we need to make sure that writes to the FPSCR only write to the bits of FPCR that are architecturally mapped, and not the others. Implement this with a new function vfp_set_fpcr_masked() which takes a mask of which bits to update. (We could do the same for FPSR, but we leave that until we actually are likely to need it.) Signed-off-by: Peter Maydell --- target/arm/vfp_helper.c | 54 ++--- 1 file changed, 34 insertions(+), 20 deletions(-) Reviewed-by: Richard Henderson r~
Re: [PATCH 8/9] target/arm: Rename FPSR_MASK and FPCR_MASK and define them symbolically
On 6/28/24 07:23, Peter Maydell wrote: Now that we store FPSR and FPCR separately, the FPSR_MASK and FPCR_MASK macros are slightly confusingly named and the comment describing them is out of date. Rename them to FPSCR_FPSR_MASK and FPSCR_FPCR_MASK, document that they are the mask of which FPSCR bits are architecturally mapped to which AArch64 register, and define them symbolically rather than as hex values. (This latter requires defining some extra macros for bits which we haven't previously defined.) Signed-off-by: Peter Maydell --- target/arm/cpu.h| 41 ++--- target/arm/machine.c| 3 ++- target/arm/vfp_helper.c | 7 --- 3 files changed, 40 insertions(+), 11 deletions(-) Reviewed-by: Richard Henderson r~
Re: [PATCH 7/9] target/arm: Rename FPCR_ QC, NZCV macros to FPSR_
On 6/28/24 07:23, Peter Maydell wrote: The QC, N, Z, C, V bits live in the FPSR, not the FPCR. Rename the macros that define these bits accordingly. Signed-off-by: Peter Maydell --- target/arm/cpu.h | 17 ++--- target/arm/tcg/mve_helper.c | 8 target/arm/tcg/translate-m-nocp.c | 16 target/arm/tcg/translate-vfp.c| 2 +- target/arm/vfp_helper.c | 8 5 files changed, 27 insertions(+), 24 deletions(-) Reviewed-by: Richard Henderson r~
Re: [PATCH 6/9] target/arm: Store FPSR and FPCR in separate CPU state fields
On 6/28/24 07:23, Peter Maydell wrote: Now that we have refactored the set/get functions so that the FPSCR format is no longer the authoritative one, we can keep FPSR and FPCR in separate CPU state fields. As well as the get and set functions, we also have a scattering of places in the code which directly access vfp.xregs[ARM_VFP_FPSCR] to extract single fields which are stored there. These all change to directly access either vfp.fpsr or vfp.fpcr, depending on the location of the field. (Most commonly, this is the NZCV flags.) We make the field in the CPU state struct 64 bits, because architecturally FPSR and FPCR are 64 bits. However we leave the types of the arguments and return values of the get/set functions as 32 bits, since we don't need to make that change with the current architecture and various callsites would be unable to handle set bits in the high half (for instance the gdbstub protocol assumes they're only 32 bit registers). Signed-off-by: Peter Maydell --- target/arm/cpu.h | 7 +++ target/arm/tcg/translate.h| 3 +-- target/arm/tcg/mve_helper.c | 12 ++-- target/arm/tcg/translate-m-nocp.c | 6 +++--- target/arm/tcg/translate-vfp.c| 2 +- target/arm/vfp_helper.c | 25 ++--- 6 files changed, 28 insertions(+), 27 deletions(-) Reviewed-by: Richard Henderson r~
[PATCH v2] hw/ide/macio.c: switch from using qemu_allocate_irq() to qdev input GPIOs
This prevents the IRQs from being leaked when the macio IDE device is used.
Signed-off-by: Mark Cave-Ayland
Reviewed-by: Peter Maydell
---
hw/ide/macio.c| 10 ++
include/hw/misc/macio/macio.h | 7 +--
2 files changed, 11 insertions(+), 6 deletions(-)
v2:
- Delete dma_irq and ide_irq from MACIOIDEState
- Add R-B tag from Peter
diff --git a/hw/ide/macio.c b/hw/ide/macio.c
index aca90d04f0..e84bf2c9f6 100644
--- a/hw/ide/macio.c
+++ b/hw/ide/macio.c
@@ -420,7 +420,8 @@ static void macio_ide_realizefn(DeviceState *dev, Error
**errp)
{
MACIOIDEState *s = MACIO_IDE(dev);
-ide_bus_init_output_irq(>bus, s->ide_irq);
+ide_bus_init_output_irq(>bus,
+qdev_get_gpio_in(dev, MACIO_IDE_PMAC_IDE_IRQ));
/* Register DMA callbacks */
s->dma.ops = _ops;
@@ -456,8 +457,8 @@ static void macio_ide_initfn(Object *obj)
sysbus_init_mmio(d, >mem);
sysbus_init_irq(d, >real_ide_irq);
sysbus_init_irq(d, >real_dma_irq);
-s->dma_irq = qemu_allocate_irq(pmac_ide_irq, s, 0);
-s->ide_irq = qemu_allocate_irq(pmac_ide_irq, s, 1);
+
+qdev_init_gpio_in(DEVICE(obj), pmac_ide_irq, MACIO_IDE_PMAC_NIRQS);
object_property_add_link(obj, "dbdma", TYPE_MAC_DBDMA,
(Object **) >dbdma,
@@ -508,7 +509,8 @@ void macio_ide_init_drives(MACIOIDEState *s, DriveInfo
**hd_table)
void macio_ide_register_dma(MACIOIDEState *s)
{
-DBDMA_register_channel(s->dbdma, s->channel, s->dma_irq,
+DBDMA_register_channel(s->dbdma, s->channel,
+ qdev_get_gpio_in(DEVICE(s), MACIO_IDE_PMAC_DMA_IRQ),
pmac_ide_transfer, pmac_ide_flush, s);
}
diff --git a/include/hw/misc/macio/macio.h b/include/hw/misc/macio/macio.h
index 2b54da6b31..16aa95b876 100644
--- a/include/hw/misc/macio/macio.h
+++ b/include/hw/misc/macio/macio.h
@@ -80,8 +80,6 @@ struct MACIOIDEState {
uint32_t channel;
qemu_irq real_ide_irq;
qemu_irq real_dma_irq;
-qemu_irq ide_irq;
-qemu_irq dma_irq;
MemoryRegion mem;
IDEBus bus;
@@ -92,6 +90,11 @@ struct MACIOIDEState {
uint32_t irq_reg;
};
+#define MACIO_IDE_PMAC_NIRQS 2
+
+#define MACIO_IDE_PMAC_DMA_IRQ 0
+#define MACIO_IDE_PMAC_IDE_IRQ 1
+
void macio_ide_init_drives(MACIOIDEState *ide, DriveInfo **hd_table);
void macio_ide_register_dma(MACIOIDEState *ide);
--
2.39.2
Re: [PATCH 5/9] target/arm: Implement store_cpu_field_low32() macro
On 6/28/24 07:23, Peter Maydell wrote: We already have a load_cpu_field_low32() to load the low half of a 64-bit CPU struct field to a TCGv_i32; however we haven't yet needed the store equivalent. We'll want that in the next patch, so implement it. Signed-off-by: Peter Maydell --- target/arm/tcg/translate-a32.h | 7 +++ 1 file changed, 7 insertions(+) Reviewed-by: Richard Henderson r~
Re: [PATCH 4/9] target/arm: Support migration when FPSR/FPCR won't fit in the FPSCR
On 6/28/24 07:23, Peter Maydell wrote:
To support FPSR and FPCR bits that don't exist in the AArch32 FPSCR
view of floating point control and status (such as the FEAT_AFP ones),
we need to make sure those bits can be migrated. This commit allows
that, whilst maintaining backwards and forwards migration compatibility
for CPUs where there are no such bits:
On sending:
* If either the FPCR or the FPSR include set bits that are not
visible in the AArch32 FPSCR view of floating point control/status
then we send the FPCR and FPSR as two separate fields in a new
cpu/vfp/fpcr_fpsr subsection, and we send a 0 for the old
FPSCR field in cpu/vfp
* Otherwise, we don't send the fpcr_fpsr subsection, and we send
an FPSCR-format value in cpu/vfp as we did previously
On receiving:
* if we see a non-zero FPSCR field, that is the right information
* if we see a fpcr_fpsr subsection then that has the information
* if we see neither, then FPSCR/FPCR/FPSR are all zero on the source;
cpu_pre_load() ensures the CPU state defaults to that
* if we see both, then the migration source is buggy or malicious;
either the fpcr_fpsr or the FPSCR will "win" depending which
is first in the migration stream; we don't care which that is
We make the new FPCR and FPSR on-the-wire data be 64 bits, because
architecturally these registers are that wide, and this avoids the
need to engage in further migration-compatibility contortions in
future if some new architecture revision defines bits in the high
half of either register.
(We won't ever send the new migration subsection until we add support
for a CPU feature which enables setting overlapping FPCR bits, like
FEAT_AFP.)
Signed-off-by: Peter Maydell
---
target/arm/machine.c | 134 ++-
1 file changed, 132 insertions(+), 2 deletions(-)
Reviewed-by: Richard Henderson
Not ideal, as vfp_get_{fpcr,fpsr} are called 3 or 4 times during migration. But unless we
have separate 'fp*r_migrate' fields in cpu state, initialized in pre_save, there's no
getting around it. And I suppose migration isn't exactly performance critical.
r~
Re: [PATCH] hw/ide/macio.c: switch from using qemu_allocate_irq() to qdev input GPIOs
On 28/06/2024 16:28, Peter Maydell wrote:
On Fri, 28 Jun 2024 at 11:55, Mark Cave-Ayland
wrote:
This prevents the IRQs from being leaked when the macio IDE device is used.
Signed-off-by: Mark Cave-Ayland
---
hw/ide/macio.c| 10 ++
include/hw/misc/macio/macio.h | 5 +
2 files changed, 11 insertions(+), 4 deletions(-)
diff --git a/hw/ide/macio.c b/hw/ide/macio.c
index aca90d04f0..e84bf2c9f6 100644
--- a/hw/ide/macio.c
+++ b/hw/ide/macio.c
@@ -420,7 +420,8 @@ static void macio_ide_realizefn(DeviceState *dev, Error
**errp)
{
MACIOIDEState *s = MACIO_IDE(dev);
-ide_bus_init_output_irq(>bus, s->ide_irq);
+ide_bus_init_output_irq(>bus,
+qdev_get_gpio_in(dev, MACIO_IDE_PMAC_IDE_IRQ));
/* Register DMA callbacks */
s->dma.ops = _ops;
@@ -456,8 +457,8 @@ static void macio_ide_initfn(Object *obj)
sysbus_init_mmio(d, >mem);
sysbus_init_irq(d, >real_ide_irq);
sysbus_init_irq(d, >real_dma_irq);
-s->dma_irq = qemu_allocate_irq(pmac_ide_irq, s, 0);
-s->ide_irq = qemu_allocate_irq(pmac_ide_irq, s, 1);
+
+qdev_init_gpio_in(DEVICE(obj), pmac_ide_irq, MACIO_IDE_PMAC_NIRQS);
object_property_add_link(obj, "dbdma", TYPE_MAC_DBDMA,
(Object **) >dbdma,
@@ -508,7 +509,8 @@ void macio_ide_init_drives(MACIOIDEState *s, DriveInfo
**hd_table)
void macio_ide_register_dma(MACIOIDEState *s)
{
-DBDMA_register_channel(s->dbdma, s->channel, s->dma_irq,
+DBDMA_register_channel(s->dbdma, s->channel,
+ qdev_get_gpio_in(DEVICE(s), MACIO_IDE_PMAC_DMA_IRQ),
pmac_ide_transfer, pmac_ide_flush, s);
}
diff --git a/include/hw/misc/macio/macio.h b/include/hw/misc/macio/macio.h
index 2b54da6b31..869b66055b 100644
--- a/include/hw/misc/macio/macio.h
+++ b/include/hw/misc/macio/macio.h
@@ -92,6 +92,11 @@ struct MACIOIDEState {
uint32_t irq_reg;
};
+#define MACIO_IDE_PMAC_NIRQS 2
+
+#define MACIO_IDE_PMAC_DMA_IRQ 0
+#define MACIO_IDE_PMAC_IDE_IRQ 1
+
void macio_ide_init_drives(MACIOIDEState *ide, DriveInfo **hd_table);
void macio_ide_register_dma(MACIOIDEState *ide);
Can we also now delete the dma_irq and ide_irq fields from the
MACIOIDEState struct?
Otherwise
Reviewed-by: Peter Maydell
Ooops, yes. I'll update and send a v2 including your Reviewed-by tag.
ATB,
Mark.
Re: [PATCH 3/9] target/arm: Make vfp_set_fpscr() call vfp_set_{fpcr, fpsr}
On 6/28/24 07:23, Peter Maydell wrote:
+void vfp_set_fpsr(CPUARMState *env, uint32_t val)
+{
+ARMCPU *cpu = env_archcpu(env);
+
+vfp_set_fpsr_to_host(env, val);
+
+if (arm_feature(env, ARM_FEATURE_NEON) ||
+cpu_isar_feature(aa32_mve, cpu)) {
+/*
+ * The bit we set within fpscr_q is arbitrary; the register as a
+ * whole being zero/non-zero is what counts.
+ */
+env->vfp.qc[0] = val & FPCR_QC;
While it's code movement, the comment is out of date.
Update s/fpscr_q/vfp.qc[]/, possibly as a follow-up.
Otherwise,
Reviewed-by: Richard Henderson
r~
Re: [PATCH v6 06/11] target/arm: Factor out code for setting MTE TCF0 field
Hi Phil,
On 6/28/24 4:08 AM, Philippe Mathieu-Daudé wrote:
On 28/6/24 07:08, Gustavo Romero wrote:
Factor out the code used for setting the MTE TCF0 field from the prctl
code into a convenient function. Other subsystems, like gdbstub, need to
set this field as well, so keep it as a separate function to avoid
duplication and ensure consistency in how this field is set across the
board.
Signed-off-by: Gustavo Romero
---
linux-user/aarch64/meson.build | 2 ++
linux-user/aarch64/mte_user_helper.c | 34
linux-user/aarch64/mte_user_helper.h | 25
linux-user/aarch64/target_prctl.h | 22 ++
4 files changed, 63 insertions(+), 20 deletions(-)
create mode 100644 linux-user/aarch64/mte_user_helper.c
create mode 100644 linux-user/aarch64/mte_user_helper.h
diff --git a/linux-user/aarch64/meson.build b/linux-user/aarch64/meson.build
index 248c578d15..f75bb3cd75 100644
--- a/linux-user/aarch64/meson.build
+++ b/linux-user/aarch64/meson.build
@@ -9,3 +9,5 @@ vdso_le_inc = gen_vdso.process('vdso-le.so',
extra_args: ['-r', '__kernel_rt_sigreturn'])
linux_user_ss.add(when: 'TARGET_AARCH64', if_true: [vdso_be_inc, vdso_le_inc])
+
+linux_user_ss.add(when: 'TARGET_AARCH64', if_true:
[files('mte_user_helper.c')])
diff --git a/linux-user/aarch64/mte_user_helper.c
b/linux-user/aarch64/mte_user_helper.c
new file mode 100644
index 00..8be6deaf03
--- /dev/null
+++ b/linux-user/aarch64/mte_user_helper.c
@@ -0,0 +1,34 @@
+/*
+ * ARM MemTag convenience functions.
+ *
+ * This code is licensed under the GNU GPL v2 or later.
+ *
+ * SPDX-License-Identifier: LGPL-2.1-or-later
+ */
+
#include "qemu/osdep.h"
#include "qemu.h"
+#include
+#include "mte_user_helper.h"
+
+void arm_set_mte_tcf0(CPUArchState *env, abi_long value)
+{
+ /*
+ * Write PR_MTE_TCF to SCTLR_EL1[TCF0].
+ *
+ * The kernel has a per-cpu configuration for the sysadmin,
+ * /sys/devices/system/cpu/cpu/mte_tcf_preferred,
+ * which qemu does not implement.
+ *
+ * Because there is no performance difference between the modes, and
+ * because SYNC is most useful for debugging MTE errors, choose SYNC
+ * as the preferred mode. With this preference, and the way the API
+ * uses only two bits, there is no way for the program to select
+ * ASYMM mode.
+ */
+ unsigned tcf = 0;
+ if (value & PR_MTE_TCF_SYNC) {
+ tcf = 1;
+ } else if (value & PR_MTE_TCF_ASYNC) {
+ tcf = 2;
+ }
+ env->cp15.sctlr_el[1] = deposit64(env->cp15.sctlr_el[1], 38, 2, tcf);
+}
diff --git a/linux-user/aarch64/mte_user_helper.h
b/linux-user/aarch64/mte_user_helper.h
new file mode 100644
index 00..ee3f6b190a
--- /dev/null
+++ b/linux-user/aarch64/mte_user_helper.h
@@ -0,0 +1,25 @@
+/*
+ * ARM MemTag convenience functions.
+ *
+ * This code is licensed under the GNU GPL v2 or later.
+ *
+ * SPDX-License-Identifier: LGPL-2.1-or-later
+ */
+
+#ifndef AARCH64_MTE_USER_HELPER_H
+#define AARCH64_MTE USER_HELPER_H
+
+#include "qemu/osdep.h"
+#include "qemu.h"
NACK. See my comment on v5.
Yes, I saw your comment in v5 about it, I haven't ignored it, I just wanted to
publish v6 updating the parts we reached out a consensus.
So,
diff --git a/linux-user/aarch64/mte_user_helper.h
b/linux-user/aarch64/mte_user_helper.h
new file mode 100644
index 00..ee3f6b190a
--- /dev/null
+++ b/linux-user/aarch64/mte_user_helper.h
@@ -0,0 +1,25 @@
+/*
+ * ARM MemTag convenience functions.
+ *
+ * This code is licensed under the GNU GPL v2 or later.
+ *
+ * SPDX-License-Identifier: LGPL-2.1-or-later
+ */
+
+#ifndef AARCH64_MTE_USER_HELPER_H
+#define AARCH64_MTE USER_HELPER_H
+
+#include "qemu/osdep.h"
https://www.qemu.org/docs/master/devel/style.html#include-directives
Do not include “qemu/osdep.h” from header files since the .c file
will have already included it.
I thought you meant osdep.h should not be included _at all_ in my case, either
in mte_user_helper.h or in mte_user_helper.c. Maybe the wording in the docs
should be "Do not include "qemu/osdep.h" from header files. Include it from .c
files, when necessary.".
I think we agree osdep.h is necessary and must be put in mte_user_helper.c. But
that left me wondering how it would work for sources including
mte_user_helper.h,
because it can be the case they don't have the declarations for the types used
in
the function prototypes, in this case, for CPUArchState and abi_long types in
arm_set_mte_tcf0. It just happens that gdbstub64.c, that includes this header
file,
actually includes osdep.h at the top of includes, so all good, but how about
other
types not provided by the osdep.h, they would have to be included in the .c that
defines the function (in this case mte_user_helper.c) and also in the .h that
includes the function prototype (in this case gdbstub64.c) anyways, which is the
case of abi_long type, which is not provided
Re: [PATCH v2 10/21] qapi: convert "Note" sections to plain rST
On Fri, Jun 28, 2024, 5:52 AM Markus Armbruster wrote: > John Snow writes: > > > We do not need a dedicated section for notes. By eliminating a specially > > parsed section, these notes can be treated as normal rST paragraphs in > > the new QMP reference manual, and can be placed and styled much more > > flexibly. > > > > Convert all existing "Note" and "Notes" sections to pure rST. As part of > > the conversion, capitalize the first letter of each sentence and add > > trailing punctuation where appropriate to ensure notes look sensible and > > consistent in rendered HTML documentation. Markup is also re-aligned to > > the de-facto standard of 3 spaces for directives. > > > > Update docs/devel/qapi-code-gen.rst to reflect the new paradigm, and > > update the QAPI parser to prohibit "Note" sections while suggesting a > > new syntax. The exact formatting to use is a matter of taste, but a good > > candidate is simply: > > > > .. note:: lorem ipsum ... > >... dolor sit amet ... > >... consectetur adipiscing elit ... > > > > ... but there are other choices, too. The Sphinx readthedocs theme > > offers theming for the following forms (capitalization unimportant); all > > are adorned with a (!) symbol () in the title bar for rendered HTML > > docs. > > > > See > > > https://sphinx-rtd-theme.readthedocs.io/en/stable/demo/demo.html#admonitions > > for examples of each directive/admonition in use. > > > > These are rendered in orange: > > > > .. Attention:: ... > > .. Caution:: ... > > .. WARNING:: ... > > > > These are rendered in red: > > > > .. DANGER:: ... > > .. Error:: ... > > > > These are rendered in green: > > > > .. Hint:: ... > > .. Important:: ... > > .. Tip:: ... > > > > These are rendered in blue: > > > > .. Note:: ... > > .. admonition:: custom title > > > >admonition body text > > > > This patch uses ".. note::" almost everywhere, with just two "caution" > > directives. Several instances of "Notes:" have been converted to merely > > ".. note::" where appropriate, but ".. admonition:: notes" is used in a > > few places where we had an ordered list of multiple notes that would not > > make sense as standalone/separate admonitions. > > I looked for hunks that don't 1:1 replace "Note:" or "Notes:" by > ".. note::." Findings: > > * Two hunks replace by ".. caution::" instead. Commit message got it. > Good. > > * Four hunks replace by ".. admonition:: notes", one of them as a test. > Commit message got it. Good. > > * Three hunks split "Notes:" into multiple ".. note::". Good, but could > be mentioned in commit message. > I meant to imply it when discussing when admonition was used, but yeah. > * Two hunks drop "Note:", changing it into paragraph. The paragraph > merges into the preceding "Example" section. Good, but should be > mentioned in the commit message, or turned into a separate patch. > Eh. we got enough commits. I think it's helpful to keep the whole conversion in one giant bang so that the diff is helpful in illustrating all of the different types of conversions. (In fact, even though I split out Example conversion for your sake in review, I think it'd be helpful to squash them together on merge for the same exact reason.) Let's just amend the commit message. > * One hunk adjusts a test case for the removal of the "Note:" tag. > Good, but could be mentioned in the commit message. > > Perhaps tweak the paragraph above: > > This patch uses ".. note::" almost everywhere, with just two "caution" > directives. Several instances of "Notes:" have been converted to > merely ".. note::", or multiple ".. note::" where appropriate. > ".. admonition:: notes" is used in a few places where we had an > ordered list of multiple notes that would not make sense as > standalone/separate admonitions. Two "Note:" following "Example:" > have been turned into ordinary paragraphs within the example. > > Okay? > Yep, suits me fine. > > NOTE: Because qapidoc.py does not attempt to preserve source ordering of > > sections, the conversion of Notes from a "tagged section" to an > > "untagged section" means that rendering order for some notes *may > > change* as a result of this patch. The forthcoming qapidoc.py rewrite > > strictly preserves source ordering in the rendered documentation, so > > this issue will be rectified in the new generator. > > > > Signed-off-by: John Snow > > Acked-by: Stefan Hajnoczi [for block*.json] > > I dislike the indentation changes, and may revert them in my tree. > Would you take a patch adjusting the indent later, or will you then tell me it's not worth the git blame fuzz? :) > Reviewed-by: Markus Armbruster > >
Re: [PATCH 2/9] target/arm: Make vfp_get_fpscr() call vfp_get_{fpcr, fpsr}
On 6/28/24 07:23, Peter Maydell wrote: In AArch32, the floating point control and status bits are all in a single register, FPSCR. In AArch64, these were split into separate FPCR and FPSR registers, but the bit layouts remained the same, with no overlaps, so that you could construct an FPSCR value by ORing FPCR and FPSR, or equivalently could produce FPSR and FPCR by masking an FPSCR value. For QEMU's implementation, we opted to use masking to produce FPSR and FPCR, because we started with an AArch32 implementation of FPSCR. The addition of the (AArch64-only) FEAT_AFP adds new bits to the FPCR which overlap with some bits in the FPSR. This means we'll no longer be able to consider the FPSCR-encoded value as the primary one, but instead need to treat FPSR/FPCR as the primary encoding and construct the FPSCR from those. (This remains possible because the FEAT_AFP bits in FPCR don't appear in the FPSCR.) As the first step in this refactoring, make vfp_get_fpscr() call vfp_get_fpcr() and vfp_get_fpsr(), instead of the other way around. Note that vfp_get_fpcsr_from_host() returns only bits in the FPSR (for the cumulative fp exception bits), so we can simply rename it without needing to add a new function for getting FPCR bits. Signed-off-by: Peter Maydell --- target/arm/cpu.h| 24 +++- target/arm/vfp_helper.c | 34 ++ 2 files changed, 37 insertions(+), 21 deletions(-) Reviewed-by: Richard Henderson r~
Re: [PATCH] hw/ide/macio.c: switch from using qemu_allocate_irq() to qdev input GPIOs
On Fri, 28 Jun 2024 at 11:55, Mark Cave-Ayland
wrote:
>
> This prevents the IRQs from being leaked when the macio IDE device is used.
>
> Signed-off-by: Mark Cave-Ayland
> ---
> hw/ide/macio.c| 10 ++
> include/hw/misc/macio/macio.h | 5 +
> 2 files changed, 11 insertions(+), 4 deletions(-)
>
> diff --git a/hw/ide/macio.c b/hw/ide/macio.c
> index aca90d04f0..e84bf2c9f6 100644
> --- a/hw/ide/macio.c
> +++ b/hw/ide/macio.c
> @@ -420,7 +420,8 @@ static void macio_ide_realizefn(DeviceState *dev, Error
> **errp)
> {
> MACIOIDEState *s = MACIO_IDE(dev);
>
> -ide_bus_init_output_irq(>bus, s->ide_irq);
> +ide_bus_init_output_irq(>bus,
> +qdev_get_gpio_in(dev, MACIO_IDE_PMAC_IDE_IRQ));
>
> /* Register DMA callbacks */
> s->dma.ops = _ops;
> @@ -456,8 +457,8 @@ static void macio_ide_initfn(Object *obj)
> sysbus_init_mmio(d, >mem);
> sysbus_init_irq(d, >real_ide_irq);
> sysbus_init_irq(d, >real_dma_irq);
> -s->dma_irq = qemu_allocate_irq(pmac_ide_irq, s, 0);
> -s->ide_irq = qemu_allocate_irq(pmac_ide_irq, s, 1);
> +
> +qdev_init_gpio_in(DEVICE(obj), pmac_ide_irq, MACIO_IDE_PMAC_NIRQS);
>
> object_property_add_link(obj, "dbdma", TYPE_MAC_DBDMA,
> (Object **) >dbdma,
> @@ -508,7 +509,8 @@ void macio_ide_init_drives(MACIOIDEState *s, DriveInfo
> **hd_table)
>
> void macio_ide_register_dma(MACIOIDEState *s)
> {
> -DBDMA_register_channel(s->dbdma, s->channel, s->dma_irq,
> +DBDMA_register_channel(s->dbdma, s->channel,
> + qdev_get_gpio_in(DEVICE(s),
> MACIO_IDE_PMAC_DMA_IRQ),
> pmac_ide_transfer, pmac_ide_flush, s);
> }
>
> diff --git a/include/hw/misc/macio/macio.h b/include/hw/misc/macio/macio.h
> index 2b54da6b31..869b66055b 100644
> --- a/include/hw/misc/macio/macio.h
> +++ b/include/hw/misc/macio/macio.h
> @@ -92,6 +92,11 @@ struct MACIOIDEState {
> uint32_t irq_reg;
> };
>
> +#define MACIO_IDE_PMAC_NIRQS 2
> +
> +#define MACIO_IDE_PMAC_DMA_IRQ 0
> +#define MACIO_IDE_PMAC_IDE_IRQ 1
> +
> void macio_ide_init_drives(MACIOIDEState *ide, DriveInfo **hd_table);
> void macio_ide_register_dma(MACIOIDEState *ide);
Can we also now delete the dma_irq and ide_irq fields from the
MACIOIDEState struct?
Otherwise
Reviewed-by: Peter Maydell
thanks
-- PMM
Re: [PATCH v2 07/21] docs/qapidoc: fix nested parsing under untagged sections
On Fri, Jun 28, 2024, 11:10 AM John Snow wrote: > > > On Fri, Jun 28, 2024, 3:55 AM Markus Armbruster wrote: > >> John Snow writes: >> >> > Sphinx does not like sections without titles, because it wants to >> > convert every section into a reference. When there is no title, it >> > struggles to do this and transforms the tree inproperly. >> > >> > Depending on the rST used, this may result in an assertion error deep in >> > the docutils HTMLWriter. >> > >> > (Observed when using ".. admonition:: Notes" under such a section - When >> > this is transformed with its own element, Sphinx is fooled into >> > believing this title belongs to the section and incorrect mutates the >> > docutils tree, leading to errors during rendering time.) >> > >> > When parsing an untagged section (free paragraphs), skip making a hollow >> > section and instead append the parse results to the prior section. >> > >> > Many Bothans died to bring us this information. >> > >> > Signed-off-by: John Snow >> > Acked-by: Markus Armbruster >> >> Generated HTML changes, but the diff is hard to review due to id >> attribute changes all over the place. >> >> Generated qemu-ga-ref.7 also changes: >> >> diff -rup old/qemu-ga-ref.7 new/qemu-ga-ref.7 >> --- old/qemu-ga-ref.7 2024-06-27 10:42:21.466096276 +0200 >> +++ new/qemu-ga-ref.7 2024-06-27 10:45:36.502414099 +0200 >> @@ -397,6 +397,7 @@ shutdown request, with no guarantee of s >> .B \fBmode\fP: \fBstring\fP (optional) >> \(dqhalt\(dq, \(dqpowerdown\(dq (default), or \(dqreboot\(dq >> .UNINDENT >> +.sp >> This command does NOT return a response on success. Success >> condition is indicated by the VM exiting with a zero exit status or, >> when running with \-\-no\-shutdown, by issuing the query\-status QMP >> @@ -1348,6 +1349,7 @@ the new password entry string, base64 en >> .B \fBcrypted\fP: \fBboolean\fP >> true if password is already crypt()d, false if raw >> .UNINDENT >> +.sp >> If the \fBcrypted\fP flag is true, it is the caller\(aqs >> responsibility to >> ensure the correct crypt() encryption scheme is used. This command >> does not attempt to interpret or report on the encryption scheme. >> >> We add vertical space. Visible when viewed with man. Looks like an >> improvement to me. >> >> Here's the first of these two spots in HTML: >> >> - >> -> class="docutils literal notranslate">> class="pre">guest-shutdown (Command)> href="#qapidoc-31" title="Permalink to this heading"> >> + >> +> class="docutils literal notranslate">> class="pre">guest-shutdown (Command)> href="#qapidoc-30" title="Permalink to this heading"> >> Initiate guest-activated shutdown. Note: this is an asynchronous >> shutdown request, with no guarantee of successful shutdown. >> >> @@ -502,22 +502,20 @@ shutdown request, with no guarantee of s >> >> >> >> - >> This command does NOT return a response on success. Success >> condition is indicated by the VM exiting with a zero exit status or, >> when running with –no-shutdown, by issuing the query-status QMP >> command to confirm the VM status is “shutdown”. >> - >> - >> -Since >> + >> +Since >> 0.15.0 >> >> >> >> The id changes muddy the waters. With them manually removed: >> >> >> > class="docutils literal notranslate">> class="pre">guest-shutdown (Command)> href="#qapidoc-31" title="Permalink to this heading"> >> Initiate guest-activated shutdown. Note: this is an asynchronous >> shutdown request, with no guarantee of successful shutdown. >> >> @@ -502,22 +502,20 @@ shutdown request, with no guarantee of s >> >> >> >> - >> This command does NOT return a response on success. Success >> condition is indicated by the VM exiting with a zero exit status or, >> when running with –no-shutdown, by issuing the query-status QMP >> command to confirm the VM status is “shutdown”. >> - >> >> Since >> 0.15.0 >> >> >> >> Makes no visual difference in my browser. >> >> Do these differences match your expectations? >> > > Yep! > > It does change the output just a little, but Sphinx really doesn't like > title-less sections. > > I thought the change looked fine, and I'm still planning on removing this > old generator anyway, so... > Oh, pro tip: try using the xml builder before and after for a cleaner comparison. --js >
Re: [PATCH v2 15/15] tests/qtest: Free GThread
On Thu, 27 Jun 2024 at 14:41, Akihiko Odaki wrote: > > These GThreads are never referenced. > > Signed-off-by: Akihiko Odaki > --- Reviewed-by: Peter Maydell thanks -- PMM
Re: [PATCH v2 15/21] docs/qapidoc: create qmp-example directive
On Fri, Jun 28, 2024, 9:24 AM Markus Armbruster wrote:
> John Snow writes:
>
> > This is a directive that creates a syntactic sugar for creating
> > "Example" boxes very similar to the ones already used in the bitmaps.rst
> > document, please see e.g.
> >
> https://www.qemu.org/docs/master/interop/bitmaps.html#creation-block-dirty-bitmap-add
> >
> > In its simplest form, when a custom title is not needed or wanted, and
> > the example body is *solely* a QMP example:
> >
> > ```
> > .. qmp-example::
> >
> >{body}
> > ```
> >
> > is syntactic sugar for:
> >
> > ```
> > .. admonition:: Example:
> >
> >.. code-block:: QMP
> >
> > {body}
> > ```
> >
> > When a custom, plaintext title that describes the example is desired,
> > this form:
> >
> > ```
> > .. qmp-example::
> >:title: Defrobnification
> >
> >{body}
> > ```
> >
> > Is syntactic sugar for:
> >
> > ```
> > .. admonition:: Example: Defrobnification
> >
> >.. code-block:: QMP
> >
> > {body}
> > ```
> >
> > Lastly, when Examples are multi-step processes that require non-QMP
> > exposition, have lengthy titles, or otherwise involve prose with rST
> > markup (lists, cross-references, etc), the most complex form:
> >
> > ```
> > .. qmp-example::
> >:annotated:
> >
> >This example shows how to use `foo-command`::
> >
> > {body}
> > ```
> >
> > Is desugared to:
> >
> > ```
> > .. admonition:: Example:
> >
> >This example shows how to use `foo-command`::
> >
> > {body}
> >
> >For more information, please see `frobnozz`.
> > ```
>
^ Whoops, added prose in the desugar block without modifying the original.
> Can we combine the latter two? Like this:
>
> .. qmp-example::
> :title: Defrobnification
> :annotated:
>
> This example shows how to use `foo-command`::
>
>{body}
>
Yes! I only didn't use that form in the series because splitting longer
Examples into title and prose felt like an editorial decision, but
absolutely you can use both.
> > The primary benefit here being documentation source consistently using
> > the same directive for all forms of examples to ensure consistent visual
> > styling, and ensuring all relevant prose is visually grouped alongside
> > the code literal block.
> >
> > Note that as of this commit, the code-block rST syntax "::" does not
> > apply QMP highlighting; you would need to use ".. code-block:: QMP". The
> > very next commit changes this behavior to assume all "::" code blocks
> > within this directive are QMP blocks.
> >
> > Signed-off-by: John Snow
> > ---
> > docs/sphinx/qapidoc.py | 60 --
> > 1 file changed, 58 insertions(+), 2 deletions(-)
>
> No tests? Hmm, I see you convert existing tests in PATCH 19-21. While
> that works, test coverage now would make it easier to see how each patch
> affects doc generator output.
>
Mmm. Do you want me to move the test changes up to this patch ... ?
> > diff --git a/docs/sphinx/qapidoc.py b/docs/sphinx/qapidoc.py
> > index 43dd99e21e6..a2fa05fc491 100644
> > --- a/docs/sphinx/qapidoc.py
> > +++ b/docs/sphinx/qapidoc.py
> > @@ -27,16 +27,19 @@
> > import os
> > import re
> > import textwrap
> > +from typing import List
> >
> > from docutils import nodes
> > -from docutils.parsers.rst import Directive, directives
> > +from docutils.parsers.rst import directives
> > from docutils.statemachine import ViewList
> > from qapi.error import QAPIError, QAPISemError
> > from qapi.gen import QAPISchemaVisitor
> > from qapi.schema import QAPISchema
> >
> > import sphinx
> > +from sphinx.directives.code import CodeBlock
> > from sphinx.errors import ExtensionError
> > +from sphinx.util.docutils import SphinxDirective
> > from sphinx.util.nodes import nested_parse_with_titles
> >
> >
> > @@ -494,7 +497,7 @@ def visit_module(self, name):
> > super().visit_module(name)
> >
> >
> > -class NestedDirective(Directive):
> > +class NestedDirective(SphinxDirective):
>
> What is this about?
>
Hmm. Strictly it's for access to sphinx configuration which I use only in
the next patch, but practically I suspect if I don't change it *here* that
the multiple inheritance from CodeBlock (which is a SphinxDirective) would
possibly be stranger.
I can try delaying that change by a patch and see if it hurts anything ...
> > def run(self):
> > raise NotImplementedError
> >
> > @@ -567,10 +570,63 @@ def run(self):
> > raise ExtensionError(str(err)) from err
> >
> >
> > +class QMPExample(CodeBlock, NestedDirective):
> > +"""
> > +Custom admonition for QMP code examples.
> > +
> > +When the :annotated: option is present, the body of this directive
> > +is parsed as normal rST instead. Code blocks must be explicitly
> > +written by the user, but this allows for intermingling explanatory
> > +paragraphs with arbitrary rST syntax and code blocks for more
> > +involved examples.
> > +
> > +When :annotated: is
Re: [PATCH v2 12/15] tests/qtest: Free old machine variable name
On Thu, 27 Jun 2024 at 14:40, Akihiko Odaki wrote:
>
> This fixes LeakSanitizer warnings.
>
> Signed-off-by: Akihiko Odaki
> ---
> tests/qtest/libqtest.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/tests/qtest/libqtest.c b/tests/qtest/libqtest.c
> index f89da7b80797..1605c0c9f615 100644
> --- a/tests/qtest/libqtest.c
> +++ b/tests/qtest/libqtest.c
> @@ -1509,6 +1509,7 @@ static struct MachInfo *qtest_get_machines(const char
> *var)
> int idx;
>
> if (g_strcmp0(qemu_var, var)) {
> +g_free(qemu_var);
> qemu_var = g_strdup(var);
>
> /* new qemu, clear the cache */
>
> --
> 2.45.2
Reviewed-by: Peter Maydell
thanks
-- PMM
Re: [PATCH v2 06/15] ppc/vof: Fix unaligned FDT property access
On Thu, 27 Jun 2024 at 14:39, Akihiko Odaki wrote:
>
> FDT properties are aligned by 4 bytes, not 8 bytes.
>
> Signed-off-by: Akihiko Odaki
> ---
> hw/ppc/vof.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/hw/ppc/vof.c b/hw/ppc/vof.c
> index e3b430a81f4f..b5b6514d79fc 100644
> --- a/hw/ppc/vof.c
> +++ b/hw/ppc/vof.c
> @@ -646,7 +646,7 @@ static void vof_dt_memory_available(void *fdt, GArray
> *claimed, uint64_t base)
> mem0_reg = fdt_getprop(fdt, offset, "reg", );
> g_assert(mem0_reg && proplen == sizeof(uint32_t) * (ac + sc));
> if (sc == 2) {
> -mem0_end = be64_to_cpu(*(uint64_t *)(mem0_reg + sizeof(uint32_t) *
> ac));
> +mem0_end = ldq_be_p(mem0_reg + sizeof(uint32_t) * ac);
> } else {
> mem0_end = be32_to_cpu(*(uint32_t *)(mem0_reg + sizeof(uint32_t) *
> ac));
> }
I did wonder if there was a better way to do what this is doing,
but neither we (in system/device_tree.c) nor libfdt seem to
provide one.
Reviewed-by: Peter Maydell
thanks
-- PMM
Re: [PATCH v2 14/21] docs/qapidoc: factor out do_parse()
On Fri, Jun 28, 2024, 9:09 AM Markus Armbruster wrote: > John Snow writes: > > > Factor out the compatibility parser helper so it can be shared by other > > directives. > > Suggest "Factor out the compatibility parser helper into a base class, > so it can be shared by other directives." Sure. Haven't read the other mails yet. I'll make the change if you want a v3, otherwise feel free to edit. > > > > Signed-off-by: John Snow > > --- > > docs/sphinx/qapidoc.py | 64 +++--- > > 1 file changed, 35 insertions(+), 29 deletions(-) > > > > diff --git a/docs/sphinx/qapidoc.py b/docs/sphinx/qapidoc.py > > index efcd84656fa..43dd99e21e6 100644 > > --- a/docs/sphinx/qapidoc.py > > +++ b/docs/sphinx/qapidoc.py > > @@ -494,7 +494,41 @@ def visit_module(self, name): > > super().visit_module(name) > > > > > > -class QAPIDocDirective(Directive): > > +class NestedDirective(Directive): > > +def run(self): > > +raise NotImplementedError > > Should this class be abstract? > It could be ... *sneezes* I plan to delete it by the end of the qapi-domain series anyway, or perhaps I could even delete it *before* with a dedicated "require sphinx >= 3.x" miniseries. Actually, that's probably a really good idea... > > + > > +def do_parse(self, rstlist, node): > > +""" > > +Parse rST source lines and add them to the specified node > > + > > +Take the list of rST source lines rstlist, parse them as > > +rST, and add the resulting docutils nodes as children of node. > > +The nodes are parsed in a way that allows them to include > > +subheadings (titles) without confusing the rendering of > > +anything else. > > +""" > > +# This is from kerneldoc.py -- it works around an API change in > > +# Sphinx between 1.6 and 1.7. Unlike kerneldoc.py, we use > > +# sphinx.util.nodes.nested_parse_with_titles() rather than the > > +# plain self.state.nested_parse(), and so we can drop the saving > > +# of title_styles and section_level that kerneldoc.py does, > > +# because nested_parse_with_titles() does that for us. > > +if USE_SSI: > > +with switch_source_input(self.state, rstlist): > > +nested_parse_with_titles(self.state, rstlist, node) > > +else: > > +save = self.state.memo.reporter > > +self.state.memo.reporter = AutodocReporter( > > +rstlist, self.state.memo.reporter > > +) > > +try: > > +nested_parse_with_titles(self.state, rstlist, node) > > +finally: > > +self.state.memo.reporter = save > > + > > + > > +class QAPIDocDirective(NestedDirective): > > """Extract documentation from the specified QAPI .json file""" > > > > required_argument = 1 > > @@ -532,34 +566,6 @@ def run(self): > > # so they are displayed nicely to the user > > raise ExtensionError(str(err)) from err > > > > -def do_parse(self, rstlist, node): > > -"""Parse rST source lines and add them to the specified node > > - > > -Take the list of rST source lines rstlist, parse them as > > -rST, and add the resulting docutils nodes as children of node. > > -The nodes are parsed in a way that allows them to include > > -subheadings (titles) without confusing the rendering of > > -anything else. > > -""" > > -# This is from kerneldoc.py -- it works around an API change in > > -# Sphinx between 1.6 and 1.7. Unlike kerneldoc.py, we use > > -# sphinx.util.nodes.nested_parse_with_titles() rather than the > > -# plain self.state.nested_parse(), and so we can drop the saving > > -# of title_styles and section_level that kerneldoc.py does, > > -# because nested_parse_with_titles() does that for us. > > -if USE_SSI: > > -with switch_source_input(self.state, rstlist): > > -nested_parse_with_titles(self.state, rstlist, node) > > -else: > > -save = self.state.memo.reporter > > -self.state.memo.reporter = AutodocReporter( > > -rstlist, self.state.memo.reporter > > -) > > -try: > > -nested_parse_with_titles(self.state, rstlist, node) > > -finally: > > -self.state.memo.reporter = save > > - > > > > def setup(app): > > """Register qapi-doc directive with Sphinx""" > > Reviewed-by: Markus Armbruster > >
Re: [PATCH v6 06/11] target/arm: Factor out code for setting MTE TCF0 field
Gustavo Romero writes:
> Hi Alex,
>
> On 6/28/24 9:14 AM, Alex Bennée wrote:
>> Gustavo Romero writes:
>>
>>> Factor out the code used for setting the MTE TCF0 field from the prctl
>>> code into a convenient function. Other subsystems, like gdbstub, need to
>>> set this field as well, so keep it as a separate function to avoid
>>> duplication and ensure consistency in how this field is set across the
>>> board.
>>>
>>> Signed-off-by: Gustavo Romero
>>> ---
>>> linux-user/aarch64/meson.build | 2 ++
>>> linux-user/aarch64/mte_user_helper.c | 34
>>> linux-user/aarch64/mte_user_helper.h | 25
>>> linux-user/aarch64/target_prctl.h| 22 ++
>>> 4 files changed, 63 insertions(+), 20 deletions(-)
>>> create mode 100644 linux-user/aarch64/mte_user_helper.c
>>> create mode 100644 linux-user/aarch64/mte_user_helper.h
>>>
>>> diff --git a/linux-user/aarch64/meson.build b/linux-user/aarch64/meson.build
>>> index 248c578d15..f75bb3cd75 100644
>>> --- a/linux-user/aarch64/meson.build
>>> +++ b/linux-user/aarch64/meson.build
>>> @@ -9,3 +9,5 @@ vdso_le_inc = gen_vdso.process('vdso-le.so',
>>> extra_args: ['-r',
>>> '__kernel_rt_sigreturn'])
>>> linux_user_ss.add(when: 'TARGET_AARCH64', if_true:
>>> [vdso_be_inc, vdso_le_inc])
>>> +
>>> +linux_user_ss.add(when: 'TARGET_AARCH64', if_true:
>>> [files('mte_user_helper.c')])
>>> diff --git a/linux-user/aarch64/mte_user_helper.c
>>> b/linux-user/aarch64/mte_user_helper.c
>>> new file mode 100644
>>> index 00..8be6deaf03
>>> --- /dev/null
>>> +++ b/linux-user/aarch64/mte_user_helper.c
>>> @@ -0,0 +1,34 @@
>>> +/*
>>> + * ARM MemTag convenience functions.
>>> + *
>>> + * This code is licensed under the GNU GPL v2 or later.
>>> + *
>>> + * SPDX-License-Identifier: LGPL-2.1-or-later
>>> + */
>>> +
>>> +#include
>> Aside from missing the osdep Phillipe pointed out including prctl.h
>> here
>> is very suspect as its a system header. I assume if we need
>> PR_MTE_TCF_SYNC we should hoist the definition that linux-user uses into
>> a common header.
> Other .c files include for other PR_ definitions. For example,
> syscall.c and elfload.c. Is this really a problem?
If your building on an arch that doesn't natively have MTE but you'll
run an aarch64 linux-user guest.
> I see that would be a
> problem when trying to build, for instance, aarch64-linux-user target on a
> BSD host, but we don't support it. Building *-linux-user target is only
> supported on Linux host, no?
True, but multiple libcs. Anyway I've fixed up and posted the maintainer
tree.
>
>
> Cheers,
> Gustavo
--
Alex Bennée
Virtualisation Tech Lead @ Linaro
Re: [PATCH v2 01/15] cpu: Free cpu_ases
On Thu, 27 Jun 2024 at 14:39, Akihiko Odaki wrote: > > This fixes LeakSanitizer warnings. > > Signed-off-by: Akihiko Odaki > --- > hw/core/cpu-common.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/hw/core/cpu-common.c b/hw/core/cpu-common.c > index f131cde2c038..a3073c17d098 100644 > --- a/hw/core/cpu-common.c > +++ b/hw/core/cpu-common.c > @@ -289,6 +289,7 @@ static void cpu_common_finalize(Object *obj) > qemu_cond_destroy(cpu->halt_cond); > g_free(cpu->halt_cond); > g_free(cpu->thread); > +g_free(cpu->cpu_ases); I think this is likely not sufficient. There's a patch lurking in the vcpu-hotplug series: https://lore.kernel.org/qemu-devel/20240607115649.214622-7-salil.me...@huawei.com/ which adds a cpu_address_space_destroy() function, which is probably what we need to have happen on CPU unrealize. NB that that patch isn't actually sufficient, though: see discussion here on previous version of patchset https://lore.kernel.org/qemu-devel/cafeaca92ncppk0qa6xjrqrgtq_xdyrsvvaz67wgjbezcxoe...@mail.gmail.com/ and the link from there to a different earlier patch from Philippe. thanks -- PMM
Re: [PATCH v2 07/21] docs/qapidoc: fix nested parsing under untagged sections
On Fri, Jun 28, 2024, 3:55 AM Markus Armbruster wrote: > John Snow writes: > > > Sphinx does not like sections without titles, because it wants to > > convert every section into a reference. When there is no title, it > > struggles to do this and transforms the tree inproperly. > > > > Depending on the rST used, this may result in an assertion error deep in > > the docutils HTMLWriter. > > > > (Observed when using ".. admonition:: Notes" under such a section - When > > this is transformed with its own element, Sphinx is fooled into > > believing this title belongs to the section and incorrect mutates the > > docutils tree, leading to errors during rendering time.) > > > > When parsing an untagged section (free paragraphs), skip making a hollow > > section and instead append the parse results to the prior section. > > > > Many Bothans died to bring us this information. > > > > Signed-off-by: John Snow > > Acked-by: Markus Armbruster > > Generated HTML changes, but the diff is hard to review due to id > attribute changes all over the place. > > Generated qemu-ga-ref.7 also changes: > > diff -rup old/qemu-ga-ref.7 new/qemu-ga-ref.7 > --- old/qemu-ga-ref.7 2024-06-27 10:42:21.466096276 +0200 > +++ new/qemu-ga-ref.7 2024-06-27 10:45:36.502414099 +0200 > @@ -397,6 +397,7 @@ shutdown request, with no guarantee of s > .B \fBmode\fP: \fBstring\fP (optional) > \(dqhalt\(dq, \(dqpowerdown\(dq (default), or \(dqreboot\(dq > .UNINDENT > +.sp > This command does NOT return a response on success. Success > condition is indicated by the VM exiting with a zero exit status or, > when running with \-\-no\-shutdown, by issuing the query\-status QMP > @@ -1348,6 +1349,7 @@ the new password entry string, base64 en > .B \fBcrypted\fP: \fBboolean\fP > true if password is already crypt()d, false if raw > .UNINDENT > +.sp > If the \fBcrypted\fP flag is true, it is the caller\(aqs > responsibility to > ensure the correct crypt() encryption scheme is used. This command > does not attempt to interpret or report on the encryption scheme. > > We add vertical space. Visible when viewed with man. Looks like an > improvement to me. > > Here's the first of these two spots in HTML: > > - > - class="docutils literal notranslate"> class="pre">guest-shutdown (Command) href="#qapidoc-31" title="Permalink to this heading"> > + > + class="docutils literal notranslate"> class="pre">guest-shutdown (Command) href="#qapidoc-30" title="Permalink to this heading"> > Initiate guest-activated shutdown. Note: this is an asynchronous > shutdown request, with no guarantee of successful shutdown. > > @@ -502,22 +502,20 @@ shutdown request, with no guarantee of s > > > > - > This command does NOT return a response on success. Success > condition is indicated by the VM exiting with a zero exit status or, > when running with –no-shutdown, by issuing the query-status QMP > command to confirm the VM status is “shutdown”. > - > - > -Since > + > +Since > 0.15.0 > > > > The id changes muddy the waters. With them manually removed: > > > class="docutils literal notranslate"> class="pre">guest-shutdown (Command) href="#qapidoc-31" title="Permalink to this heading"> > Initiate guest-activated shutdown. Note: this is an asynchronous > shutdown request, with no guarantee of successful shutdown. > > @@ -502,22 +502,20 @@ shutdown request, with no guarantee of s > > > > - > This command does NOT return a response on success. Success > condition is indicated by the VM exiting with a zero exit status or, > when running with –no-shutdown, by issuing the query-status QMP > command to confirm the VM status is “shutdown”. > - > > Since > 0.15.0 > > > > Makes no visual difference in my browser. > > Do these differences match your expectations? > Yep! It does change the output just a little, but Sphinx really doesn't like title-less sections. I thought the change looked fine, and I'm still planning on removing this old generator anyway, so... >
Re: [PATCH] hw/usb/hcd-ohci: Set transfer error code with no dev
On Sat, 22 Jun 2024 at 13:57, Ryan Wendland wrote:
>
> When a usb device is disconnected the transfer service functions bails
> before appropraite transfer error flags are set.
(typo: "appropriate")
> This patch sets the appropriate condition code OHCI_CC_DEVICENOTRESPONDING
> when a device is disconnected and consequently has no response on the USB bus.
>
> Fixes: https://gitlab.com/qemu-project/qemu/-/issues/2081
>
> Signed-off-by: Ryan Wendland
> ---
> hw/usb/hcd-ohci.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/hw/usb/hcd-ohci.c b/hw/usb/hcd-ohci.c
> index acd6016980..8cd25d74af 100644
> --- a/hw/usb/hcd-ohci.c
> +++ b/hw/usb/hcd-ohci.c
> @@ -980,7 +980,8 @@ static int ohci_service_td(OHCIState *ohci, struct
> ohci_ed *ed)
> dev = ohci_find_device(ohci, OHCI_BM(ed->flags, ED_FA));
> if (dev == NULL) {
> trace_usb_ohci_td_dev_error();
> -return 1;
> +OHCI_SET_BM(td.flags, TD_CC, OHCI_CC_DEVICENOTRESPONDING);
> +goto exit_and_retire;
> }
> ep = usb_ep_get(dev, pid, OHCI_BM(ed->flags, ED_EN));
> if (ohci->async_td) {
> @@ -1087,6 +1088,7 @@ static int ohci_service_td(OHCIState *ohci, struct
> ohci_ed *ed)
> ed->head |= OHCI_ED_H;
> }
>
> +exit_and_retire:
> /* Retire this TD */
> ed->head &= ~OHCI_DPTR_MASK;
> ed->head |= td.next & OHCI_DPTR_MASK;
Thanks for this patch; I have a couple of questions:
(1) Do we also need to do something similar for the call in
ohci_service_iso_td() ?
(2) The error handling path for the other way we can
set the DEVICENOTRESPONDING flag also does:
* set done_count to 0
* OR in OCHI_ED_H into ed->head
Do we need to do those things here ? (My guess is "yes".)
thanks
-- PMM
Re: [PATCH 1/9] target/arm: Correct comments about M-profile FPSCR
On 6/28/24 07:23, Peter Maydell wrote: The M-profile FPSCR LTPSIZE is bits [18:16]; this is the same field as A-profile FPSCR Len, not Stride. Correct the comment in vfp_get_fpscr(). We also implemented M-profile FPSCR.QC, but forgot to delete a TODO comment from vfp_set_fpscr(); remove it now. Signed-off-by: Peter Maydell --- target/arm/vfp_helper.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) Reviewed-by: Richard Henderson r~
[RFC PATCH v2 2/5] vhost_user: Add frontend command for shmem config
The frontend can use this command to retrieve
VIRTIO Shared Memory Regions configuration from
the backend. The response contains the number of
shared memory regions, their size, and shmid.
This is useful when the frontend is unaware of
specific backend type and configuration,
for example, in the `vhost-user-device` case.
Signed-off-by: Albert Esteve
---
docs/interop/vhost-user.rst | 31 +++
hw/virtio/vhost-user.c| 42 +++
include/hw/virtio/vhost-backend.h | 6 +
include/hw/virtio/vhost-user.h| 1 +
4 files changed, 80 insertions(+)
diff --git a/docs/interop/vhost-user.rst b/docs/interop/vhost-user.rst
index d52ba719d5..51f01d1d84 100644
--- a/docs/interop/vhost-user.rst
+++ b/docs/interop/vhost-user.rst
@@ -348,6 +348,19 @@ Device state transfer parameters
In the future, additional phases might be added e.g. to allow
iterative migration while the device is running.
+VIRTIO Shared Memory Region configuration
+^
+
++-+-++++
+| num regions | padding | mem size 0 | .. | mem size 7 |
++-+-++++
+
+:num regions: a 32-bit number of regions
+
+:padding: 32-bit
+
+:mem size: 64-bit size of VIRTIO Shared Memory Region
+
C structure
---
@@ -369,6 +382,10 @@ In QEMU the vhost-user message is implemented with the
following struct:
VhostUserConfig config;
VhostUserVringArea area;
VhostUserInflight inflight;
+ VhostUserShared object;
+ VhostUserTransferDeviceState transfer_state;
+ VhostUserMMap mmap;
+ VhostUserShMemConfig shmem;
};
} QEMU_PACKED VhostUserMsg;
@@ -1051,6 +1068,7 @@ Protocol features
#define VHOST_USER_PROTOCOL_F_XEN_MMAP 17
#define VHOST_USER_PROTOCOL_F_SHARED_OBJECT18
#define VHOST_USER_PROTOCOL_F_DEVICE_STATE 19
+ #define VHOST_USER_PROTOCOL_F_SHMEM20
Front-end message types
---
@@ -1725,6 +1743,19 @@ Front-end message types
Using this function requires prior negotiation of the
``VHOST_USER_PROTOCOL_F_DEVICE_STATE`` feature.
+``VHOST_USER_GET_SHMEM_CONFIG``
+ :id: 44
+ :equivalent ioctl: N/A
+ :request payload: N/A
+ :reply payload: ``struct VhostUserShMemConfig``
+
+ When the ``VHOST_USER_PROTOCOL_F_SHMEM`` protocol feature has been
+ successfully negotiated, this message can be submitted by the front-end
+ to gather the VIRTIO Shared Memory Region configuration. Back-end will
respond
+ with the number of VIRTIO Shared Memory Regions it requires, and each shared
memory
+ region size in an array. The shared memory IDs are represented by the index
+ of the array.
+
Back-end message types
--
diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c
index 7ee8a472c6..57406dc8b4 100644
--- a/hw/virtio/vhost-user.c
+++ b/hw/virtio/vhost-user.c
@@ -104,6 +104,7 @@ typedef enum VhostUserRequest {
VHOST_USER_GET_SHARED_OBJECT = 41,
VHOST_USER_SET_DEVICE_STATE_FD = 42,
VHOST_USER_CHECK_DEVICE_STATE = 43,
+VHOST_USER_GET_SHMEM_CONFIG = 44,
VHOST_USER_MAX
} VhostUserRequest;
@@ -138,6 +139,12 @@ typedef struct VhostUserMemRegMsg {
VhostUserMemoryRegion region;
} VhostUserMemRegMsg;
+typedef struct VhostUserShMemConfig {
+uint32_t nregions;
+uint32_t padding;
+uint64_t memory_sizes[VHOST_MEMORY_BASELINE_NREGIONS];
+} VhostUserShMemConfig;
+
typedef struct VhostUserLog {
uint64_t mmap_size;
uint64_t mmap_offset;
@@ -245,6 +252,7 @@ typedef union {
VhostUserShared object;
VhostUserTransferDeviceState transfer_state;
VhostUserMMap mmap;
+VhostUserShMemConfig shmem;
} VhostUserPayload;
typedef struct VhostUserMsg {
@@ -3136,6 +3144,39 @@ static int vhost_user_check_device_state(struct
vhost_dev *dev, Error **errp)
return 0;
}
+static int vhost_user_get_shmem_config(struct vhost_dev *dev,
+ int *nregions,
+ uint64_t *memory_sizes,
+ Error **errp)
+{
+int ret;
+VhostUserMsg msg = {
+.hdr.request = VHOST_USER_GET_SHMEM_CONFIG,
+.hdr.flags = VHOST_USER_VERSION,
+};
+
+if (!virtio_has_feature(dev->protocol_features,
+VHOST_USER_PROTOCOL_F_SHMEM)) {
+return 0;
+}
+
+ret = vhost_user_write(dev, , NULL, 0);
+if (ret < 0) {
+return ret;
+}
+
+ret = vhost_user_read(dev, );
+if (ret < 0) {
+return ret;
+}
+
+*nregions = msg.payload.shmem.nregions;
+memcpy(memory_sizes,
+ _sizes,
+ sizeof(uint64_t) * VHOST_MEMORY_BASELINE_NREGIONS);
+return 0;
+}
+
const VhostOps user_ops = {
.backend_type = VHOST_BACKEND_TYPE_USER,
[RFC PATCH v2 4/5] vhost_user: Add MEM_READ/WRITE backend requests
With SHMEM_MAP messages, sharing descriptors between
devices will cause that these devices do not see the
mappings, and fail to access these memory regions.
To solve this, introduce MEM_READ/WRITE requests
that will get triggered as a fallback when
vhost-user memory translation fails.
Signed-off-by: Albert Esteve
---
hw/virtio/vhost-user.c| 31 +
subprojects/libvhost-user/libvhost-user.c | 84 +++
subprojects/libvhost-user/libvhost-user.h | 38 ++
3 files changed, 153 insertions(+)
diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c
index 57406dc8b4..18cacb2d68 100644
--- a/hw/virtio/vhost-user.c
+++ b/hw/virtio/vhost-user.c
@@ -118,6 +118,8 @@ typedef enum VhostUserBackendRequest {
VHOST_USER_BACKEND_SHARED_OBJECT_LOOKUP = 8,
VHOST_USER_BACKEND_SHMEM_MAP = 9,
VHOST_USER_BACKEND_SHMEM_UNMAP = 10,
+VHOST_USER_BACKEND_MEM_READ = 11,
+VHOST_USER_BACKEND_MEM_WRITE = 12,
VHOST_USER_BACKEND_MAX
} VhostUserBackendRequest;
@@ -145,6 +147,12 @@ typedef struct VhostUserShMemConfig {
uint64_t memory_sizes[VHOST_MEMORY_BASELINE_NREGIONS];
} VhostUserShMemConfig;
+typedef struct VhostUserMemRWMsg {
+uint64_t guest_address;
+uint32_t size;
+uint8_t data[];
+} VhostUserMemRWMsg;
+
typedef struct VhostUserLog {
uint64_t mmap_size;
uint64_t mmap_offset;
@@ -253,6 +261,7 @@ typedef union {
VhostUserTransferDeviceState transfer_state;
VhostUserMMap mmap;
VhostUserShMemConfig shmem;
+VhostUserMemRWMsg mem_rw;
} VhostUserPayload;
typedef struct VhostUserMsg {
@@ -1871,6 +1880,22 @@ vhost_user_backend_handle_shmem_unmap(struct vhost_dev
*dev,
return 0;
}
+static int
+vhost_user_backend_handle_mem_read(struct vhost_dev *dev,
+ VhostUserMemRWMsg *mem_rw)
+{
+/* TODO */
+return -EPERM;
+}
+
+static int
+vhost_user_backend_handle_mem_write(struct vhost_dev *dev,
+ VhostUserMemRWMsg *mem_rw)
+{
+/* TODO */
+return -EPERM;
+}
+
static void close_backend_channel(struct vhost_user *u)
{
g_source_destroy(u->backend_src);
@@ -1946,6 +1971,12 @@ static gboolean backend_read(QIOChannel *ioc,
GIOCondition condition,
case VHOST_USER_BACKEND_SHMEM_UNMAP:
ret = vhost_user_backend_handle_shmem_unmap(dev, );
break;
+case VHOST_USER_BACKEND_MEM_READ:
+ret = vhost_user_backend_handle_mem_read(dev, _rw);
+break;
+case VHOST_USER_BACKEND_MEM_WRITE:
+ret = vhost_user_backend_handle_mem_write(dev, _rw);
+break;
default:
error_report("Received unexpected msg type: %d.", hdr.request);
ret = -EINVAL;
diff --git a/subprojects/libvhost-user/libvhost-user.c
b/subprojects/libvhost-user/libvhost-user.c
index 28556d183a..b5184064b5 100644
--- a/subprojects/libvhost-user/libvhost-user.c
+++ b/subprojects/libvhost-user/libvhost-user.c
@@ -1651,6 +1651,90 @@ vu_shmem_unmap(VuDev *dev, uint8_t shmid, uint64_t
fd_offset,
return vu_process_message_reply(dev, );
}
+bool
+vu_send_mem_read(VuDev *dev, uint64_t guest_addr, uint32_t size,
+ uint8_t *data)
+{
+VhostUserMsg msg_reply;
+VhostUserMsg msg = {
+.request = VHOST_USER_BACKEND_MEM_READ,
+.size = sizeof(msg.payload.mem_rw),
+.flags = VHOST_USER_VERSION | VHOST_USER_NEED_REPLY_MASK,
+.payload = {
+.mem_rw = {
+.guest_address = guest_addr,
+.size = size,
+}
+}
+};
+
+pthread_mutex_lock(>backend_mutex);
+if (!vu_message_write(dev, dev->backend_fd, )) {
+goto out_err;
+}
+
+if (!vu_message_read_default(dev, dev->backend_fd, _reply)) {
+goto out_err;
+}
+
+if (msg_reply.request != msg.request) {
+DPRINT("Received unexpected msg type. Expected %d, received %d",
+ msg.request, msg_reply.request);
+goto out_err;
+}
+
+if (msg_reply.payload.mem_rw.size != size) {
+DPRINT("Received unexpected number of bytes in the response. "
+ "Expected %d, received %d",
+ size, msg_reply.payload.mem_rw.size);
+goto out_err;
+}
+
+data = malloc(msg_reply.payload.mem_rw.size);
+if (!data) {
+DPRINT("Failed to malloc read memory data");
+goto out_err;
+}
+
+memcpy(data, msg_reply.payload.mem_rw.data, size);
+pthread_mutex_unlock(>backend_mutex);
+return true;
+
+out_err:
+pthread_mutex_unlock(>backend_mutex);
+return false;
+}
+
+bool
+vu_send_mem_write(VuDev *dev, uint64_t guest_addr, uint32_t size,
+ uint8_t *data)
+{
+VhostUserMsg msg = {
+.request = VHOST_USER_BACKEND_MEM_WRITE,
+.size = sizeof(msg.payload.mem_rw),
+.flags = VHOST_USER_VERSION,
+.payload = {
+.mem_rw = {
+.guest_address = guest_addr,
+
[RFC PATCH v2 5/5] vhost_user: Implement mem_read/mem_write handlers
Implement function handlers for memory read and write
operations.
Signed-off-by: Albert Esteve
---
hw/virtio/vhost-user.c | 34 ++
1 file changed, 30 insertions(+), 4 deletions(-)
diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c
index 18cacb2d68..79becbc87b 100644
--- a/hw/virtio/vhost-user.c
+++ b/hw/virtio/vhost-user.c
@@ -1884,16 +1884,42 @@ static int
vhost_user_backend_handle_mem_read(struct vhost_dev *dev,
VhostUserMemRWMsg *mem_rw)
{
-/* TODO */
-return -EPERM;
+ram_addr_t offset;
+int fd;
+MemoryRegion *mr;
+
+mr = vhost_user_get_mr_data(mem_rw->guest_address, , );
+
+if (!mr) {
+error_report("Failed to get memory region with address %" PRIx64,
+ mem_rw->guest_address);
+return -EFAULT;
+}
+
+memcpy(mem_rw->data, memory_region_get_ram_ptr(mr) + offset, mem_rw->size);
+
+return 0;
}
static int
vhost_user_backend_handle_mem_write(struct vhost_dev *dev,
VhostUserMemRWMsg *mem_rw)
{
-/* TODO */
-return -EPERM;
+ram_addr_t offset;
+int fd;
+MemoryRegion *mr;
+
+mr = vhost_user_get_mr_data(mem_rw->guest_address, , );
+
+if (!mr) {
+error_report("Failed to get memory region with address %" PRIx64,
+ mem_rw->guest_address);
+return -EFAULT;
+}
+
+memcpy(memory_region_get_ram_ptr(mr) + offset, mem_rw->data, mem_rw->size);
+
+return 0;
}
static void close_backend_channel(struct vhost_user *u)
--
2.45.2
[RFC PATCH v2 0/5] vhost-user: Add SHMEM_MAP/UNMAP requests
Hi all, v1->v2: - Corrected typos and clarifications from first review - Added SHMEM_CONFIG frontend request to query VIRTIO shared memory regions from backends - vhost-user-device to use SHMEM_CONFIG to request and initialise regions - Added MEM_READ/WRITE backend requests in case address translation fails accessing VIRTIO Shared Memory Regions with MMAPs This is an update of my attempt to have backends support dynamic fd mapping into VIRTIO Shared Memory Regions. After the first review I have added more commits and new messages to the vhost-user protocol. However, I still have some doubts as to how will this work, specially regarding the MEM_READ and MEM_WRITE commands. Thus, I am still looking for feedback, to ensure that I am going in the right direction with the implementation. The usecase for this patch is, e.g., to support vhost-user-gpu RESOURCE_BLOB operations, or DAX Window request for virtio-fs. In general, any operation where a backend need to request the frontend to mmap an fd into a VIRTIO Shared Memory Region, so that the guest can then access it. After receiving the SHMEM_MAP/UNMAP request, the frontend will perform the mmap with the instructed parameters (i.e., shmid, shm_offset, fd_offset, fd, lenght). As there are already a couple devices that could benefit of such a feature, and more could require it in the future, the goal is to make the implementation generic. To that end, the VIRTIO Shared Memory Region list is declared in the `VirtIODevice` struct. This patch also includes: SHMEM_CONFIG frontend request that is specifically meant to allow generic vhost-user-device frontend to be able to query VIRTIO Shared Memory settings from the backend (as this device is generic and agnostic of the actual backend configuration). Finally, MEM_READ/WRITE backend requests are added to deal with a potential issue when having any backend sharing a descriptor that references a mapping to another backend. The first backend will not be able to see these mappings. So these requests are a fallback for vhost-user memory translation fails. Albert Esteve (5): vhost-user: Add VIRTIO Shared Memory map request vhost_user: Add frontend command for shmem config vhost-user-dev: Add cache BAR vhost_user: Add MEM_READ/WRITE backend requests vhost_user: Implement mem_read/mem_write handlers docs/interop/vhost-user.rst | 58 ++ hw/virtio/vhost-user-base.c | 39 +++- hw/virtio/vhost-user-device-pci.c | 37 +++- hw/virtio/vhost-user.c| 221 ++ hw/virtio/virtio.c| 12 ++ include/hw/virtio/vhost-backend.h | 6 + include/hw/virtio/vhost-user.h| 1 + include/hw/virtio/virtio.h| 5 + subprojects/libvhost-user/libvhost-user.c | 149 +++ subprojects/libvhost-user/libvhost-user.h | 91 + 10 files changed, 614 insertions(+), 5 deletions(-) -- 2.45.2
[RFC PATCH v2 3/5] vhost-user-dev: Add cache BAR
Add a cache BAR in the vhost-user-device
into which files can be directly mapped.
The number, shmid, and size of the VIRTIO Shared
Memory subregions is retrieved through a get_shmem_config
message sent by the vhost-user-base module
on the realize step, after virtio_init().
By default, if VHOST_USER_PROTOCOL_F_SHMEM
feature is not supported by the backend,
there is no cache.
Signed-off-by: Albert Esteve
---
hw/virtio/vhost-user-base.c | 39 +--
hw/virtio/vhost-user-device-pci.c | 37 ++---
2 files changed, 71 insertions(+), 5 deletions(-)
diff --git a/hw/virtio/vhost-user-base.c b/hw/virtio/vhost-user-base.c
index a83167191e..e47c568a55 100644
--- a/hw/virtio/vhost-user-base.c
+++ b/hw/virtio/vhost-user-base.c
@@ -268,7 +268,9 @@ static void vub_device_realize(DeviceState *dev, Error
**errp)
{
VirtIODevice *vdev = VIRTIO_DEVICE(dev);
VHostUserBase *vub = VHOST_USER_BASE(dev);
-int ret;
+uint64_t memory_sizes[8];
+void *cache_ptr;
+int i, ret, nregions;
if (!vub->chardev.chr) {
error_setg(errp, "vhost-user-base: missing chardev");
@@ -311,7 +313,7 @@ static void vub_device_realize(DeviceState *dev, Error
**errp)
/* Allocate queues */
vub->vqs = g_ptr_array_sized_new(vub->num_vqs);
-for (int i = 0; i < vub->num_vqs; i++) {
+for (i = 0; i < vub->num_vqs; i++) {
g_ptr_array_add(vub->vqs,
virtio_add_queue(vdev, vub->vq_size,
vub_handle_output));
@@ -328,6 +330,39 @@ static void vub_device_realize(DeviceState *dev, Error
**errp)
do_vhost_user_cleanup(vdev, vub);
}
+ret = vub->vhost_dev.vhost_ops->vhost_get_shmem_config(>vhost_dev,
+ ,
+ memory_sizes,
+ errp);
+
+if (ret < 0) {
+do_vhost_user_cleanup(vdev, vub);
+}
+
+for (i = 0; i < nregions; i++) {
+if (memory_sizes[i]) {
+if (!is_power_of_2(memory_sizes[i]) ||
+memory_sizes[i] < qemu_real_host_page_size()) {
+error_setg(errp, "Shared memory %d size must be a power of 2 "
+ "no smaller than the page size", i);
+return;
+}
+
+cache_ptr = mmap(NULL, memory_sizes[i], PROT_READ,
+MAP_ANONYMOUS | MAP_PRIVATE, -1, 0);
+if (cache_ptr == MAP_FAILED) {
+error_setg(errp, "Unable to mmap blank cache: %s",
+ strerror(errno));
+return;
+}
+
+virtio_new_shmem_region(vdev);
+memory_region_init_ram_ptr(>shmem_list[i],
+OBJECT(vdev), "vub-shm-" + i,
+memory_sizes[i], cache_ptr);
+}
+}
+
qemu_chr_fe_set_handlers(>chardev, NULL, NULL, vub_event, NULL,
dev, NULL, true);
}
diff --git a/hw/virtio/vhost-user-device-pci.c
b/hw/virtio/vhost-user-device-pci.c
index efaf55d3dd..314bacfb7a 100644
--- a/hw/virtio/vhost-user-device-pci.c
+++ b/hw/virtio/vhost-user-device-pci.c
@@ -8,14 +8,18 @@
*/
#include "qemu/osdep.h"
+#include "qapi/error.h"
#include "hw/qdev-properties.h"
#include "hw/virtio/vhost-user-base.h"
#include "hw/virtio/virtio-pci.h"
+#define VIRTIO_DEVICE_PCI_CACHE_BAR 2
+
struct VHostUserDevicePCI {
VirtIOPCIProxy parent_obj;
VHostUserBase vub;
+MemoryRegion cachebar;
};
#define TYPE_VHOST_USER_DEVICE_PCI "vhost-user-device-pci-base"
@@ -25,10 +29,37 @@ OBJECT_DECLARE_SIMPLE_TYPE(VHostUserDevicePCI,
VHOST_USER_DEVICE_PCI)
static void vhost_user_device_pci_realize(VirtIOPCIProxy *vpci_dev, Error
**errp)
{
VHostUserDevicePCI *dev = VHOST_USER_DEVICE_PCI(vpci_dev);
-DeviceState *vdev = DEVICE(>vub);
-
+DeviceState *dev_state = DEVICE(>vub);
+VirtIODevice *vdev = VIRTIO_DEVICE(dev_state);
+uint64_t offset = 0, cache_size = 0;
+int i;
+
vpci_dev->nvectors = 1;
-qdev_realize(vdev, BUS(_dev->bus), errp);
+qdev_realize(dev_state, BUS(_dev->bus), errp);
+
+for (i = 0; i < vdev->n_shmem_regions; i++) {
+if (vdev->shmem_list[i].size > UINT64_MAX - cache_size) {
+error_setg(errp, "Total shared memory required overflow");
+return;
+}
+cache_size = cache_size + vdev->shmem_list[i].size;
+}
+if (cache_size) {
+memory_region_init(>cachebar, OBJECT(vpci_dev),
+ "vhost-device-pci-cachebar", cache_size);
+for (i = 0; i < vdev->n_shmem_regions; i++) {
+memory_region_add_subregion(>cachebar, offset,
+>shmem_list[i]);
+virtio_pci_add_shm_cap(vpci_dev,
[RFC PATCH v2 1/5] vhost-user: Add VIRTIO Shared Memory map request
Add SHMEM_MAP/UNMAP requests to vhost-user to
handle VIRTIO Shared Memory mappings.
This request allows backends to dynamically map
fds into a VIRTIO Shared Memory Region indentified
by its `shmid`. Then, the fd memory is advertised
to the driver as a base addres + offset, so it
can be read/written (depending on the mmap flags
requested) while its valid.
The backend can munmap the memory range
in a given VIRTIO Shared Memory Region (again,
identified by its `shmid`), to free it. Upon
receiving this message, the front-end must
mmap the regions with PROT_NONE to reserve
the virtual memory space.
The device model needs to create MemoryRegion
instances for the VIRTIO Shared Memory Regions
and add them to the `VirtIODevice` instance.
Signed-off-by: Albert Esteve
---
docs/interop/vhost-user.rst | 27 +
hw/virtio/vhost-user.c| 122 ++
hw/virtio/virtio.c| 12 +++
include/hw/virtio/virtio.h| 5 +
subprojects/libvhost-user/libvhost-user.c | 65
subprojects/libvhost-user/libvhost-user.h | 53 ++
6 files changed, 284 insertions(+)
diff --git a/docs/interop/vhost-user.rst b/docs/interop/vhost-user.rst
index d8419fd2f1..d52ba719d5 100644
--- a/docs/interop/vhost-user.rst
+++ b/docs/interop/vhost-user.rst
@@ -1859,6 +1859,33 @@ is sent by the front-end.
when the operation is successful, or non-zero otherwise. Note that if the
operation fails, no fd is sent to the backend.
+``VHOST_USER_BACKEND_SHMEM_MAP``
+ :id: 9
+ :equivalent ioctl: N/A
+ :request payload: fd and ``struct VhostUserMMap``
+ :reply payload: N/A
+
+ This message can be submitted by the backends to advertise a new mapping
+ to be made in a given VIRTIO Shared Memory Region. Upon receiving the
message,
+ The front-end will mmap the given fd into the VIRTIO Shared Memory Region
+ with the requested ``shmid``. A reply is generated indicating whether mapping
+ succeeded.
+
+ Mapping over an already existing map is not allowed and request shall fail.
+ Therefore, the memory range in the request must correspond with a valid,
+ free region of the VIRTIO Shared Memory Region.
+
+``VHOST_USER_BACKEND_SHMEM_UNMAP``
+ :id: 10
+ :equivalent ioctl: N/A
+ :request payload: ``struct VhostUserMMap``
+ :reply payload: N/A
+
+ This message can be submitted by the backends so that the front-end un-mmap
+ a given range (``offset``, ``len``) in the VIRTIO Shared Memory Region with
+ the requested ``shmid``.
+ A reply is generated indicating whether unmapping succeeded.
+
.. _reply_ack:
VHOST_USER_PROTOCOL_F_REPLY_ACK
diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c
index cdf9af4a4b..7ee8a472c6 100644
--- a/hw/virtio/vhost-user.c
+++ b/hw/virtio/vhost-user.c
@@ -115,6 +115,8 @@ typedef enum VhostUserBackendRequest {
VHOST_USER_BACKEND_SHARED_OBJECT_ADD = 6,
VHOST_USER_BACKEND_SHARED_OBJECT_REMOVE = 7,
VHOST_USER_BACKEND_SHARED_OBJECT_LOOKUP = 8,
+VHOST_USER_BACKEND_SHMEM_MAP = 9,
+VHOST_USER_BACKEND_SHMEM_UNMAP = 10,
VHOST_USER_BACKEND_MAX
} VhostUserBackendRequest;
@@ -192,6 +194,24 @@ typedef struct VhostUserShared {
unsigned char uuid[16];
} VhostUserShared;
+/* For the flags field of VhostUserMMap */
+#define VHOST_USER_FLAG_MAP_R (1u << 0)
+#define VHOST_USER_FLAG_MAP_W (1u << 1)
+
+typedef struct {
+/* VIRTIO Shared Memory Region ID */
+uint8_t shmid;
+uint8_t padding[7];
+/* File offset */
+uint64_t fd_offset;
+/* Offset within the VIRTIO Shared Memory Region */
+uint64_t shm_offset;
+/* Size of the mapping */
+uint64_t len;
+/* Flags for the mmap operation, from VHOST_USER_FLAG_* */
+uint64_t flags;
+} VhostUserMMap;
+
typedef struct {
VhostUserRequest request;
@@ -224,6 +244,7 @@ typedef union {
VhostUserInflight inflight;
VhostUserShared object;
VhostUserTransferDeviceState transfer_state;
+VhostUserMMap mmap;
} VhostUserPayload;
typedef struct VhostUserMsg {
@@ -1748,6 +1769,100 @@ vhost_user_backend_handle_shared_object_lookup(struct
vhost_user *u,
return 0;
}
+static int
+vhost_user_backend_handle_shmem_map(struct vhost_dev *dev,
+VhostUserMMap *vu_mmap,
+int fd)
+{
+void *addr = 0;
+MemoryRegion *mr = NULL;
+
+if (fd < 0) {
+error_report("Bad fd for map");
+return -EBADF;
+}
+
+if (!dev->vdev->shmem_list ||
+dev->vdev->n_shmem_regions <= vu_mmap->shmid) {
+error_report("Device only has %d VIRTIO Shared Memory Regions. "
+ "Requested ID: %d",
+ dev->vdev->n_shmem_regions, vu_mmap->shmid);
+return -EFAULT;
+}
+
+mr = >vdev->shmem_list[vu_mmap->shmid];
+
+if (!mr) {
+error_report("VIRTIO Shared Memory Region at "
+ "ID %d unitialized",
Re: [PATCH v6 06/11] target/arm: Factor out code for setting MTE TCF0 field
Hi Alex,
On 6/28/24 9:14 AM, Alex Bennée wrote:
Gustavo Romero writes:
Factor out the code used for setting the MTE TCF0 field from the prctl
code into a convenient function. Other subsystems, like gdbstub, need to
set this field as well, so keep it as a separate function to avoid
duplication and ensure consistency in how this field is set across the
board.
Signed-off-by: Gustavo Romero
---
linux-user/aarch64/meson.build | 2 ++
linux-user/aarch64/mte_user_helper.c | 34
linux-user/aarch64/mte_user_helper.h | 25
linux-user/aarch64/target_prctl.h| 22 ++
4 files changed, 63 insertions(+), 20 deletions(-)
create mode 100644 linux-user/aarch64/mte_user_helper.c
create mode 100644 linux-user/aarch64/mte_user_helper.h
diff --git a/linux-user/aarch64/meson.build b/linux-user/aarch64/meson.build
index 248c578d15..f75bb3cd75 100644
--- a/linux-user/aarch64/meson.build
+++ b/linux-user/aarch64/meson.build
@@ -9,3 +9,5 @@ vdso_le_inc = gen_vdso.process('vdso-le.so',
extra_args: ['-r', '__kernel_rt_sigreturn'])
linux_user_ss.add(when: 'TARGET_AARCH64', if_true: [vdso_be_inc, vdso_le_inc])
+
+linux_user_ss.add(when: 'TARGET_AARCH64', if_true:
[files('mte_user_helper.c')])
diff --git a/linux-user/aarch64/mte_user_helper.c
b/linux-user/aarch64/mte_user_helper.c
new file mode 100644
index 00..8be6deaf03
--- /dev/null
+++ b/linux-user/aarch64/mte_user_helper.c
@@ -0,0 +1,34 @@
+/*
+ * ARM MemTag convenience functions.
+ *
+ * This code is licensed under the GNU GPL v2 or later.
+ *
+ * SPDX-License-Identifier: LGPL-2.1-or-later
+ */
+
+#include
Aside from missing the osdep Phillipe pointed out including prctl.h here
is very suspect as its a system header. I assume if we need
PR_MTE_TCF_SYNC we should hoist the definition that linux-user uses into
a common header.
Other .c files include for other PR_ definitions. For example,
syscall.c and elfload.c. Is this really a problem? I see that would be a
problem when trying to build, for instance, aarch64-linux-user target on a
BSD host, but we don't support it. Building *-linux-user target is only
supported on Linux host, no?
Cheers,
Gustavo
Re: [Bug Report] Possible Missing Endianness Conversion
On Tue, 25 Jun 2024 at 08:18, Stefano Garzarella wrote:
>
> On Mon, Jun 24, 2024 at 04:19:52PM GMT, Peter Maydell wrote:
> >On Mon, 24 Jun 2024 at 16:11, Stefano Garzarella wrote:
> >>
> >> CCing Jason.
> >>
> >> On Mon, Jun 24, 2024 at 4:30 PM Xoykie wrote:
> >> >
> >> > The virtio packed virtqueue support patch[1] suggests converting
> >> > endianness by lines:
> >> >
> >> > virtio_tswap16s(vdev, >off_wrap);
> >> > virtio_tswap16s(vdev, >flags);
> >> >
> >> > Though both of these conversion statements aren't present in the
> >> > latest qemu code here[2]
> >> >
> >> > Is this intentional?
> >>
> >> Good catch!
> >>
> >> It looks like it was removed (maybe by mistake) by commit
> >> d152cdd6f6 ("virtio: use virtio accessor to access packed event")
> >
> >That commit changes from:
> >
> >-address_space_read_cached(cache, off_off, >off_wrap,
> >- sizeof(e->off_wrap));
> >-virtio_tswap16s(vdev, >off_wrap);
> >
> >which does a byte read of 2 bytes and then swaps the bytes
> >depending on the host endianness and the value of
> >virtio_access_is_big_endian()
> >
> >to this:
> >
> >+e->off_wrap = virtio_lduw_phys_cached(vdev, cache, off_off);
> >
> >virtio_lduw_phys_cached() is a small function which calls
> >either lduw_be_phys_cached() or lduw_le_phys_cached()
> >depending on the value of virtio_access_is_big_endian().
> >(And lduw_be_phys_cached() and lduw_le_phys_cached() do
> >the right thing for the host-endianness to do a "load
> >a specifically big or little endian 16-bit value".)
> >
> >Which is to say that because we use a load/store function that's
> >explicit about the size of the data type it is accessing, the
> >function itself can handle doing the load as big or little
> >endian, rather than the calling code having to do a manual swap after
> >it has done a load-as-bag-of-bytes. This is generally preferable
> >as it's less error-prone.
>
> Thanks for the details!
>
> So, should we also remove `virtio_tswap16s(vdev, >flags);` ?
>
> I mean:
> diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
> index 893a072c9d..2e5e67bdb9 100644
> --- a/hw/virtio/virtio.c
> +++ b/hw/virtio/virtio.c
> @@ -323,7 +323,6 @@ static void vring_packed_event_read(VirtIODevice *vdev,
> /* Make sure flags is seen before off_wrap */
> smp_rmb();
> e->off_wrap = virtio_lduw_phys_cached(vdev, cache, off_off);
> -virtio_tswap16s(vdev, >flags);
> }
That definitely looks like it's probably not correct...
-- PMM
Re: [PATCH v4 0/3] Add boot-mode property for zynq
On Fri, 21 Jun 2024 at 13:59, Sai Pavan Boddu wrote: > > Add a way to update the boot-mode via machine properties. > > Changes for V2: > Make boot-mode property work with string, > Fixed few code style issues, > Added zynq board doc. > Changes for V3: > Mentioned about zynq doc in MAINTAINERS file, > Stick to small case for mentioning boot modes in doc, > fixed commit message to mention right property name. > Changes for V4: > Use strncasecmp, > Fix boot mode names to use small case in few other places, > Fix code indentation. > > Sai Pavan Boddu (3): > hw/misc/zynq_slcr: Add boot-mode property > hw/arm/xilinx_zynq: Add boot-mode property > docs/system/arm: Add a doc for zynq board Applied to target-arm.next, thanks. -- PMM
Re: [PATCH] MAINTAINERS: Update my family name
On Wed, 26 Jun 2024 at 22:16, Patrick Leis wrote: > > Signed-off-by: Patrick Leis > --- > MAINTAINERS | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/MAINTAINERS b/MAINTAINERS > index 19f67dc5d2..13255d4a3b 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -2496,7 +2496,7 @@ F: hw/net/tulip.c > F: hw/net/tulip.h > > pca954x > -M: Patrick Venture > +M: Patrick Leis > S: Maintained > F: hw/i2c/i2c_mux_pca954x.c > F: include/hw/i2c/i2c_mux_pca954x.h > -- > 2.45.2.741.gdbec12cfda-goog Applied to target-arm.next (since I'm doing a pullreq anyway), thanks. -- PMM
Re: [PATCH v3 0/3] target/arm: Enable FEAT_Debugv8p8 for -cpu max
On Mon, 24 Jun 2024 at 19:09, Gustavo Romero wrote: > > Enable FEAT_Debugv8p8 on Arm max CPU. > > v2: > - Revert to the original comment above call to aa32_max_features() > > v3: > - Added feature entry to docs/system/arm/emulation.rst > - Explicitly set t=0 before using it to set DBGDEVID reg. > - Put indent fix in a separate patch Applied to target-arm.next, thanks. -- PMM
Re: [PATCH v2 00/13] target/arm: AdvSIMD conversion, part 2
On Tue, 25 Jun 2024 at 19:41, Richard Henderson wrote: > > Convert another hand-full of instructions, plus fixes > for two issues that are related. > > Applied to target-arm.next, thanks. -- PMM
Re: [PATCH v2 04/21] docs/qapidoc: delint a tiny portion of the module
On Fri, Jun 28, 2024, 3:29 AM Markus Armbruster wrote: > John Snow writes: > > > In a forthcoming series that adds a new QMP documentation generator, it > > will be helpful to have a linting baseline. However, there's no need to > > shuffle around the deck chairs too much, because most of this code will > > be removed once that new qapidoc generator (the "transmogrifier") is in > > place. > > > > To ease my pain: just turn off the black auto-formatter for most, but > > not all, of qapidoc.py. This will help ensure that *new* code follows a > > coding standard without bothering too much with cleaning up the existing > > code. > > > > Code that I intend to keep is still subject to the delinting beam. > > > > Signed-off-by: John Snow > > Reviewed-by: Markus Armbruster > > Not an objection, just so you know: I still see a few C0411 like 'third > party import "import sphinx" should be placed before ...' > > R-by stands. > Yeah, I think it depends on precisely where you run the script. I think because the folder is named "sphinx" that it confuses the tools in certain contexts. I'm not worried about it because we don't have an enforcement paradigm yet - I stick to my little self-test script just to make sure I'm being self-consistent, but I figured I'd worry about broader compatibility later when I reshuffle the deck chairs for qapi.
Re: [PATCH 02/23] target/i386: fix gen_prepare_size_nz condition
Alex Bennée writes:
> Incorrect brace positions causes an unintended overflow on 32 bit
> builds and shenanigans result.
>
> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/2413
> Suggested-by: Mark Cave-Ayland
> Signed-off-by: Alex Bennée
This seems to trigger regressions in:
qtest-x86_64/bios-tables-test
qtest-x86_64/pxe-test
qtest-x86_64/vmgenid-test
Could that be down to generated test data?
> ---
> target/i386/tcg/translate.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/target/i386/tcg/translate.c b/target/i386/tcg/translate.c
> index ad1819815a..94f13541c3 100644
> --- a/target/i386/tcg/translate.c
> +++ b/target/i386/tcg/translate.c
> @@ -877,7 +877,7 @@ static CCPrepare gen_prepare_sign_nz(TCGv src, MemOp size)
> return (CCPrepare) { .cond = TCG_COND_LT, .reg = src };
> } else {
> return (CCPrepare) { .cond = TCG_COND_TSTNE, .reg = src,
> - .imm = 1ull << ((8 << size) - 1) };
> + .imm = (1ull << (8 << size)) - 1 };
> }
> }
--
Alex Bennée
Virtualisation Tech Lead @ Linaro
Re: [PATCH 0/2] target/arm: Always build Aarch64 gdbstub helpers
On 6/19/24 05:49, Philippe Mathieu-Daudé wrote: Merge gdbstub64.c in gdbstub.c and remove uses of target specific TARGET_AARCH64 definition. Small step toward single ARM/Aarch64 binary. Philippe Mathieu-Daudé (2): target/arm: Merge gdbstub64.c within gdbstub.c target/arm: Always build Aarch64 gdbstub helpers target/arm/cpu.h | 8 +- target/arm/internals.h | 2 - target/arm/gdbstub.c | 363 +- target/arm/gdbstub64.c | 383 - target/arm/meson.build | 1 - 5 files changed, 364 insertions(+), 393 deletions(-) delete mode 100644 target/arm/gdbstub64.c Are we attempting a single binary for user-only as well? r~
[PATCH 3/9] target/arm: Make vfp_set_fpscr() call vfp_set_{fpcr, fpsr}
Make vfp_set_fpscr() call vfp_set_fpsr() and vfp_set_fpcr()
instead of the other way around.
The masking we do when getting and setting vfp.xregs[ARM_VFP_FPSCR]
is a little awkward, but we are going to change where we store the
underlying FPSR and FPCR information in a later commit, so it will
go away then.
Signed-off-by: Peter Maydell
---
target/arm/cpu.h| 22 +
target/arm/vfp_helper.c | 100 ++--
2 files changed, 78 insertions(+), 44 deletions(-)
diff --git a/target/arm/cpu.h b/target/arm/cpu.h
index 68a9922f88e..0a570afcab4 100644
--- a/target/arm/cpu.h
+++ b/target/arm/cpu.h
@@ -1730,17 +1730,19 @@ uint32_t vfp_get_fpsr(CPUARMState *env);
*/
uint32_t vfp_get_fpcr(CPUARMState *env);
-static inline void vfp_set_fpsr(CPUARMState *env, uint32_t val)
-{
-uint32_t new_fpscr = (vfp_get_fpscr(env) & ~FPSR_MASK) | (val & FPSR_MASK);
-vfp_set_fpscr(env, new_fpscr);
-}
+/**
+ * vfp_set_fpsr: write the AArch64 FPSR
+ * @env: CPU context
+ * @value: new value
+ */
+void vfp_set_fpsr(CPUARMState *env, uint32_t value);
-static inline void vfp_set_fpcr(CPUARMState *env, uint32_t val)
-{
-uint32_t new_fpscr = (vfp_get_fpscr(env) & ~FPCR_MASK) | (val & FPCR_MASK);
-vfp_set_fpscr(env, new_fpscr);
-}
+/**
+ * vfp_set_fpcr: write the AArch64 FPCR
+ * @env: CPU context
+ * @value: new value
+ */
+void vfp_set_fpcr(CPUARMState *env, uint32_t value);
enum arm_cpu_mode {
ARM_CPU_MODE_USR = 0x10,
diff --git a/target/arm/vfp_helper.c b/target/arm/vfp_helper.c
index a87d39e4d9b..38c8aadf9b4 100644
--- a/target/arm/vfp_helper.c
+++ b/target/arm/vfp_helper.c
@@ -99,14 +99,27 @@ static uint32_t vfp_get_fpsr_from_host(CPUARMState *env)
return vfp_exceptbits_from_host(i);
}
-static void vfp_set_fpscr_to_host(CPUARMState *env, uint32_t val)
+static void vfp_set_fpsr_to_host(CPUARMState *env, uint32_t val)
+{
+/*
+ * The exception flags are ORed together when we read fpscr so we
+ * only need to preserve the current state in one of our
+ * float_status values.
+ */
+int i = vfp_exceptbits_to_host(val);
+set_float_exception_flags(i, >vfp.fp_status);
+set_float_exception_flags(0, >vfp.fp_status_f16);
+set_float_exception_flags(0, >vfp.standard_fp_status);
+set_float_exception_flags(0, >vfp.standard_fp_status_f16);
+}
+
+static void vfp_set_fpcr_to_host(CPUARMState *env, uint32_t val)
{
-int i;
uint32_t changed = env->vfp.xregs[ARM_VFP_FPSCR];
changed ^= val;
if (changed & (3 << 22)) {
-i = (val >> 22) & 3;
+int i = (val >> 22) & 3;
switch (i) {
case FPROUNDING_TIEEVEN:
i = float_round_nearest_even;
@@ -141,17 +154,6 @@ static void vfp_set_fpscr_to_host(CPUARMState *env,
uint32_t val)
set_default_nan_mode(dnan_enabled, >vfp.fp_status);
set_default_nan_mode(dnan_enabled, >vfp.fp_status_f16);
}
-
-/*
- * The exception flags are ORed together when we read fpscr so we
- * only need to preserve the current state in one of our
- * float_status values.
- */
-i = vfp_exceptbits_to_host(val);
-set_float_exception_flags(i, >vfp.fp_status);
-set_float_exception_flags(0, >vfp.fp_status_f16);
-set_float_exception_flags(0, >vfp.standard_fp_status);
-set_float_exception_flags(0, >vfp.standard_fp_status_f16);
}
#else
@@ -161,7 +163,11 @@ static uint32_t vfp_get_fpsr_from_host(CPUARMState *env)
return 0;
}
-static void vfp_set_fpscr_to_host(CPUARMState *env, uint32_t val)
+static void vfp_set_fpsr_to_host(CPUARMState *env, uint32_t val)
+{
+}
+
+static void vfp_set_fpcr_to_host(CPUARMState *env, uint32_t val)
{
}
@@ -204,7 +210,37 @@ uint32_t vfp_get_fpscr(CPUARMState *env)
return HELPER(vfp_get_fpscr)(env);
}
-void HELPER(vfp_set_fpscr)(CPUARMState *env, uint32_t val)
+void vfp_set_fpsr(CPUARMState *env, uint32_t val)
+{
+ARMCPU *cpu = env_archcpu(env);
+
+vfp_set_fpsr_to_host(env, val);
+
+if (arm_feature(env, ARM_FEATURE_NEON) ||
+cpu_isar_feature(aa32_mve, cpu)) {
+/*
+ * The bit we set within fpscr_q is arbitrary; the register as a
+ * whole being zero/non-zero is what counts.
+ */
+env->vfp.qc[0] = val & FPCR_QC;
+env->vfp.qc[1] = 0;
+env->vfp.qc[2] = 0;
+env->vfp.qc[3] = 0;
+}
+
+/*
+ * The only FPSR bits we keep in vfp.xregs[FPSCR] are NZCV:
+ * the exception flags IOC|DZC|OFC|UFC|IXC|IDC are stored in
+ * fp_status, and QC is in vfp.qc[]. Store the NZCV bits there,
+ * and zero any of the other FPSR bits (but preserve the FPCR
+ * bits).
+ */
+val &= FPCR_NZCV_MASK;
+env->vfp.xregs[ARM_VFP_FPSCR] &= ~FPSR_MASK;
+env->vfp.xregs[ARM_VFP_FPSCR] |= val;
+}
+
+void vfp_set_fpcr(CPUARMState *env, uint32_t val)
{
ARMCPU *cpu = env_archcpu(env);
@@ -213,7 +249,7 @@ void HELPER(vfp_set_fpscr)(CPUARMState *env, uint32_t val)
