Re: [PATCH] scripts/get_maintainer: Use .ignoredmailmap to ignore invalid emails
On 01.07.2020 19:54, Philippe Mathieu-Daudé wrote: +Pavel/Paul/Alexander On 7/1/20 5:12 PM, Paolo Bonzini wrote: On 01/07/20 17:07, Philippe Mathieu-Daudé wrote: $ cat .ignoredmailmap # # From man git-shortlog the forms are: # # Proper Name # # Jean-Christophe PLAGNIOL-VILLARD Caio Carrara Yongbok Kim James Hogan Paul Burton Alexander Graf Roy Franz Dmitry Solodkiy Evgeny Voevodin Serge Hallyn Pavel Dovgalyuk For at least Paul Burton, Pavel Dovgalyuk and Alex Graf we should just use .mailmap, anyway I think the concept of the patch is okay. Pavel has been using a GMail account, but seems to be back to ispras.ru, so it might have be a temporary failure (over few days although). Right, my primary e-mail for QEMU-related work was pavel.dovga...@ispras.ru. However, I recently added alias with better transliteration (pavel.dovgal...@ispras.ru), and now I will use it for the patches. I can send a pair of patches for Paul and Alexander if they are OK. The others seem MIA. Paolo
[PATCH 2/2] configure: add support for Control-Flow Integrity
This patch adds a flag to enable/disable control flow integrity checks
on indirect function calls. This feature is only provided by LLVM/Clang
v3.9 or higher, and only allows indirect function calls to functions
with compatible signatures.
We also add an option to enable a debugging version of cfi, with verbose
output in case of a CFI violation.
CFI on indirect function calls does not support calls to functions in
shared libraries (since they were not known at compile time), and such
calls are forbidden. QEMU relies on dlopen/dlsym when using modules,
so we make modules incompatible with CFI.
We introduce a blacklist file, to disable CFI checks in a limited number
of TCG functions.
The feature relies on link-time optimization (lto), which requires the
use of the gold linker, and the LLVM versions of ar, ranlib and nm.
This patch take care of checking that all the compiler toolchain
dependencies are met.
Signed-off-by: Daniele Buono
---
cfi-blacklist.txt | 27 +++
configure | 177 ++
2 files changed, 204 insertions(+)
create mode 100644 cfi-blacklist.txt
diff --git a/cfi-blacklist.txt b/cfi-blacklist.txt
new file mode 100644
index 00..bf804431a5
--- /dev/null
+++ b/cfi-blacklist.txt
@@ -0,0 +1,27 @@
+# List of functions that should not be compiled with Control-Flow Integrity
+
+[cfi-icall]
+# TCG creates binary blobs at runtime, with the transformed code.
+# When it's time to execute it, the code is called with an indirect function
+# call. Since such function did not exist at compile time, the runtime has no
+# way to verify its signature. Disable CFI checks in the function that calls
+# the binary blob
+fun:cpu_tb_exec
+
+# TCI (Tiny Compiler Interpreter) is an interpreter for TCG pseudo code.
+# One possible operation in the pseudo code is a call to binary code.
+# Therefore, disable CFI checks in the interpreter function
+fun:tcg_qemu_tb_exec
+
+# TCG Plugins Callback Functions. The mechanism rely on opening external
+# shared libraries at runtime and get pointers to functions in such libraries
+# Since these pointers are external to the QEMU binary, the runtime cannot
+# verify their signature. Disable CFI Checks in all the functions that use
+# such pointers.
+fun:plugin_vcpu_cb__simple
+fun:plugin_cb__simple
+fun:plugin_cb__udata
+fun:qemu_plugin_tb_trans_cb
+fun:qemu_plugin_vcpu_syscall
+fun:qemu_plugin_vcpu_syscall_ret
+fun:plugin_load
diff --git a/configure b/configure
index 4a22dcd563..86fb0f390c 100755
--- a/configure
+++ b/configure
@@ -27,6 +27,7 @@ fi
TMPB="qemu-conf"
TMPC="${TMPDIR1}/${TMPB}.c"
TMPO="${TMPDIR1}/${TMPB}.o"
+TMPA="${TMPDIR1}/lib${TMPB}.a"
TMPCXX="${TMPDIR1}/${TMPB}.cxx"
TMPE="${TMPDIR1}/${TMPB}.exe"
TMPMO="${TMPDIR1}/${TMPB}.mo"
@@ -134,6 +135,43 @@ compile_prog() {
do_cc $QEMU_CFLAGS $local_cflags -o $TMPE $TMPC $QEMU_LDFLAGS $local_ldflags
}
+do_run() {
+# Run a generic program, capturing its output to the log.
+# First argument is binary to execute.
+local program="$1"
+shift
+echo $program $@ >> config.log
+$program $@ >> config.log 2>&1 || return $?
+}
+
+do_run_filter() {
+# Run a generic program, capturing its output to the log,
+# but also filtering the output with grep.
+# Returns the return value of grep.
+# First argument is the filter string.
+# Second argument is binary to execute.
+local filter="$1"
+shift
+local program="$1"
+shift
+echo $program $@ >> config.log
+$program $@ >> config.log 2>&1
+$program $@ 2>&1 | grep ${filter} >> /dev/null || return $?
+
+}
+
+create_library() {
+ do_run "$ar" -rc${1} $TMPA $TMPO
+}
+
+create_index() {
+ do_run "$ranlib" $TMPA
+}
+
+find_library_symbol() {
+ do_run_filter ${1} "$nm" $TMPA
+}
+
# symbolically link $1 to $2. Portable version of "ln -sf".
symlink() {
rm -rf "$2"
@@ -306,6 +344,8 @@ libs_tools=""
audio_win_int=""
libs_qga=""
debug_info="yes"
+cfi="no"
+cfi_debug="no"
stack_protector=""
safe_stack=""
use_containers="yes"
@@ -1285,6 +1325,14 @@ for opt do
;;
--disable-werror) werror="no"
;;
+ --enable-cfi) cfi="yes"
+ ;;
+ --disable-cfi) cfi="no"
+ ;;
+ --enable-cfi-debug) cfi_debug="yes"
+ ;;
+ --disable-cfi-debug) cfi_debug="no"
+ ;;
--enable-stack-protector) stack_protector="yes"
;;
--disable-stack-protector) stack_protector="no"
@@ -1838,6 +1886,10 @@ disabled with --disable-FEATURE, default is enabled if
available:
module-upgrades try to load modules from alternate paths for upgrades
debug-tcg TCG debugging (default is disabled)
debug-info debugging information
+ cfi Enable Control-Flow Integrity for indirect function calls.
+ Depends on clang/llvm >= 3.9 and is incompatible with modules
+ cfi-debug In case of a cfi violation, a message containing the line
that
+ triggered the error is written to stderr
sparse sparse checker
[PATCH 1/2] check-block: enable iotests with cfi-icall
cfi-icall is a form of Control-Flow Integrity for indirect function
calls implemented by llvm. It is enabled with a -fsanitize flag.
iotests are currently disabled when -fsanitize options is used, with the
exception of SafeStack.
This patch implements a generic filtering mechanism to allow iotests
with a set of known-to-be-safe -fsanitize option. Then mark SafeStack
and the new options used for cfi-icall safe for iotests
Signed-off-by: Daniele Buono
---
tests/check-block.sh | 18 +++---
1 file changed, 11 insertions(+), 7 deletions(-)
diff --git a/tests/check-block.sh b/tests/check-block.sh
index 8e29c868e5..7691213bd9 100755
--- a/tests/check-block.sh
+++ b/tests/check-block.sh
@@ -21,14 +21,18 @@ if grep -q "CONFIG_GPROF=y" config-host.mak 2>/dev/null ;
then
exit 0
fi
-# Disable tests with any sanitizer except for SafeStack
-CFLAGS=$( grep "CFLAGS.*-fsanitize" config-host.mak 2>/dev/null )
-SANITIZE_FLAGS=""
-#Remove all occurrencies of -fsanitize=safe-stack
-for i in ${CFLAGS}; do
-if [ "${i}" != "-fsanitize=safe-stack" ]; then
-SANITIZE_FLAGS="${SANITIZE_FLAGS} ${i}"
+# Disable tests with any sanitizer except for specific ones
+SANITIZE_FLAGS=$( grep "CFLAGS.*-fsanitize" config-host.mak 2>/dev/null )
+ALLOWED_SANITIZE_FLAGS="safe-stack cfi-icall sanitize-blacklist"
+#Remove all occurrencies of allowed Sanitize flags
+for j in ${ALLOWED_SANITIZE_FLAGS}; do
+TMP_FLAGS=${SANITIZE_FLAGS}
+SANITIZE_FLAGS=""
+for i in ${TMP_FLAGS}; do
+if ! echo ${i} | grep -q "${j}" 2>/dev/null; then
+SANITIZE_FLAGS="${SANITIZE_FLAGS} ${i}"
fi
+done
done
if echo ${SANITIZE_FLAGS} | grep -q "\-fsanitize" 2>/dev/null; then
# Have a sanitize flag that is not allowed, stop
--
2.26.2
[PATCH 0/2] Add support for Control-Flow Integrity
LLVM/Clang, starting from v3.9, supports runtime checks for forward-edge Control-Flow Integrity (CFI). CFI on indirect function calls can have a huge impact in enhancing QEMU security, by significantly limiting one of the most used attack vectors for VM Escape. Attacks demonstrated in [1],[2] and [3] will, at some point, change a function pointer in a QEMU data structure. At high level, LLVM's implementation relies on compile-time information to create a range of consecutive trampolines for "compatible functions". At runtime, if the pointer is not in the valid range, it is assumed that the control flow was hijacked, and the process is terminated with an "Illegal Instruction" exception. CAVEATS: 1) For this CFI check to work, the code must always respect the function signature when using function pointer. While this is generally true in QEMU, there are a few instances where pointers are handled as generic void* from the caller. Since this is a common approach, Clang offer a flag to relax pointer checks and consider all pointer types to be compatible. 2) Since CFI relies on compile-time information, it requires using link-time optimization (LTO) to support CFI across compilation units. This adds a requirement for the gold linker, and LLVM's versions of static libraries tools (ar, ranlib, nm). 3) CFI checks cannot be performed on shared libraries (given that functions are not known at compile time). This means that indirect function calls will fail if the function pointer belong to a shared library. This does not seem to be a big issue for a standard QEMU deployment today, but QEMU modules won't be able to work when CFI is enabled. There is a way to allow shared library pointers, but it is experimental in LLVM, requires some work and reduces performance and security. For these reasons, I think it's best to start with this version, and discuss an extension for modules later. 4) CFI cannot be fully applied to TCG. The purpose of TCG is to transform code on-the-fly from one ISA to another. In doing so, binary blobs of executable code are created and called with function pointers. Since the code did not exist at compile time, runtime CFI checks find such functions illegal. To allow the code to keep running, CFI checks are not performed in the core function of TCG/TCI, and in the code that implements TCG plugins. This does not affect QEMU when executed with KVM, and all the device emulation code is always protected, even when using TCG. 5) Most of the logic to enable CFI goes in the configure, since it's just a matter of checking for dependencies and incompatible options. However, I had to disable CFI checks for a few TCG functions. This can only be done through a blacklist file. I added a file in the root of QEMU, called cfi-blacklist.txt for such purpose. I am open to suggestions on where the file should go, and I am willing to become the maintainer of it, if deemed necessary. PERFORMANCE: Enabling CFI creates a larger binary, which may be an issue in some use cases. However, the increase is not exceptionally large. On my Ubuntu system, with default options, I see an increase of stripped size from 14MiB to 15.3MiB when enabling CFI with Clang v9. There is also a possible performance issue, since for every indirect function call, and additional address check is performed, followed by an additional indirect call to the trampoline function. However, especially in the case of KVM-based virtualization, the impact should be minimal, since indirect function pointers should be used mostly for device emulation. I used Kata Container's metrics tests since that is a simple, reproducible set of tests to stress storage and network between VMs, and run a Lifecycle test to measure VM startup times under a specific workload. A full report is available here [4]. The difference between LLVM with and without CFI is generally low. Sometimes CFI is actually offering better performance, which may be explained by having a different binary layout because of LTO. Lifecycle and network do not seem to be affected much. With storage, the situation is a bit more variable, but the oscillations seem to be more related to the benchmark variability than the CFI overhead. I also run a quick check-acceptance on full system VMs with and without CFI, the results are at [4] and show comparable results, with CFI slightly outperforming the default binary produced by LLVM. [1] Mehdi Talbi and Paul Fariello. VM escape - QEMU Case Study [2] Nelson Elhage. Virtunoid: Breaking out of KVM [3] Marco Grassi and Kira. Vulnerability Discovery and Exploitation of Virtualization Solutions for Cloud Computing and Desktops [4] https://github.com/dbuono/QEMU-CFI-Performance Daniele Buono (2): check-block: enable iotests with cfi-icall configure: add support for Control-Flow Integrity cfi-blacklist.txt| 27 +++ configure| 177 +++ tests/check-block.sh | 18 +++-- 3 files
Re: [PATCH 2/3] util: support detailed error reporting for qemu_open
Daniel P. Berrangé writes:
> Create a "qemu_open_err" method which does the same as "qemu_open",
> but with a "Error **errp" for error reporting. There should be no
> behavioural difference for existing callers at this stage.
>
> Signed-off-by: Daniel P. Berrangé
> ---
> include/qemu/osdep.h | 1 +
> util/osdep.c | 71 +++-
> 2 files changed, 58 insertions(+), 14 deletions(-)
>
> diff --git a/include/qemu/osdep.h b/include/qemu/osdep.h
> index 0d26a1b9bd..e41701a308 100644
> --- a/include/qemu/osdep.h
> +++ b/include/qemu/osdep.h
> @@ -483,6 +483,7 @@ int qemu_madvise(void *addr, size_t len, int advice);
> int qemu_mprotect_rwx(void *addr, size_t size);
> int qemu_mprotect_none(void *addr, size_t size);
>
> +int qemu_open_err(const char *name, int flags, Error **errp, ...);
> int qemu_open(const char *name, int flags, ...);
> int qemu_close(int fd);
> int qemu_unlink(const char *name);
> diff --git a/util/osdep.c b/util/osdep.c
> index 4bdbe81cec..450b3a5da3 100644
> --- a/util/osdep.c
> +++ b/util/osdep.c
> @@ -22,6 +22,7 @@
> * THE SOFTWARE.
> */
> #include "qemu/osdep.h"
> +#include "qapi/error.h"
>
> /* Needed early for CONFIG_BSD etc. */
>
> @@ -282,7 +283,7 @@ int qemu_lock_fd_test(int fd, int64_t start, int64_t len,
> bool exclusive)
> /*
> * Opens a file with FD_CLOEXEC set
> */
> -int qemu_open(const char *name, int flags, ...)
> +static int qemu_openv(const char *name, int flags, Error **errp, va_list ap)
> {
> int ret;
> int mode = 0;
> @@ -297,24 +298,31 @@ int qemu_open(const char *name, int flags, ...)
>
> fdset_id = qemu_parse_fdset(fdset_id_str);
> if (fdset_id == -1) {
> +error_setg(errp, "Unable to parse fdset %s", name);
> errno = EINVAL;
> return -1;
> }
>
> fd = monitor_fdset_get_fd(fdset_id, flags);
> if (fd < 0) {
> +error_setg_errno(errp, -fd, "Unable acquire FD for %s flags %x",
> + name, flags);
> errno = -fd;
> return -1;
> }
>
> dupfd = qemu_dup_flags(fd, flags);
> if (dupfd == -1) {
> +error_setg_errno(errp, errno, "Unable dup FD for %s flags %x",
> + name, flags);
> return -1;
> }
>
> ret = monitor_fdset_dup_fd_add(fdset_id, dupfd);
> if (ret == -1) {
> close(dupfd);
> +error_setg(errp, "Unable save FD for %s flags %x",
> + name, flags);
> errno = EINVAL;
> return -1;
> }
> @@ -324,11 +332,7 @@ int qemu_open(const char *name, int flags, ...)
> #endif
>
> if (flags & O_CREAT) {
> -va_list ap;
> -
> -va_start(ap, flags);
> mode = va_arg(ap, int);
> -va_end(ap);
> }
>
> #ifdef O_CLOEXEC
> @@ -340,25 +344,64 @@ int qemu_open(const char *name, int flags, ...)
> }
> #endif
>
> +if (ret == -1) {
> +const char *action = "open";
> +if (flags & O_CREAT) {
> +action = "create";
> +}
> #ifdef O_DIRECT
> -if (ret == -1 && errno == EINVAL && (flags & O_DIRECT)) {
> -int newflags = flags & ~O_DIRECT;
> +if (errno == EINVAL && (flags & O_DIRECT)) {
> +int newflags = flags & ~O_DIRECT;
> # ifdef O_CLOEXEC
> -ret = open(name, newflags | O_CLOEXEC, mode);
> +ret = open(name, newflags | O_CLOEXEC, mode);
> # else
> -ret = open(name, newflags, mode);
> +ret = open(name, newflags, mode);
> # endif
> -if (ret != -1) {
> -close(ret);
> -error_report("file system does not support O_DIRECT");
> -errno = EINVAL;
> +if (ret != -1) {
> +close(ret);
> +error_setg(errp, "Unable to %s '%s' flags 0x%x: "
> + "filesystem does not support O_DIRECT",
> + action, name, flags);
> +if (!errp) {
If the caller ignores errors, ...
> +error_report("file system does not support O_DIRECT");
... we report this error to stderr (but not any of the other ones).
This is weird. I figure you do it here to reproduce the weirdness of
qemu_open() before the patch. Goes back to
commit a5813077aac7865f69b7ee46a594f3705429f7cd
Author: Stefan Hajnoczi
Date: Thu Aug 22 11:29:03 2013 +0200
osdep: warn if open(O_DIRECT) on fails with EINVAL
Print a warning when opening a file O_DIRECT fails with EINVAL. This
saves users a lot of time trying to figure out the EINVAL error, which
is typical when attempting to open a file O_DIRECT on Linux tmpfs.
Reported-by: Deepak C Shetty
Signed-off-by: Stefan Hajnoczi
Reviewed-by: Eric Blake
The message isn't phrased as a warning, though. Should it
[PATCH] MAINTAINERS: Remove myself from FPU emulation maintenance
Signed-off-by: Aurelien Jarno --- MAINTAINERS | 1 - 1 file changed, 1 deletion(-) diff --git a/MAINTAINERS b/MAINTAINERS index dec252f38b..0535e043f0 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -130,7 +130,6 @@ F: include/sysemu/cpus.h F: include/sysemu/tcg.h FPU emulation -M: Aurelien Jarno M: Peter Maydell M: Alex Bennée S: Maintained -- 2.27.0
Re: [PATCH v3 2/2] MAINTAINERS: Adjust MIPS maintainership
On 2020-07-01 20:55, Aurelien Jarno wrote: > NACK This NACK was because I find inacceptable to claim that you got not answer from Paul or from myself after very few time. Now about the content of the patch, QEMU used to be a fun ride, but it happens that interactions are now hurtful, especially on the MIPS side. It's time for me to leave this community and say goodbye. So as far as removing me goes: Acked-by: Aurelien Jarno Regards, Aurelien > On 2020-07-01 20:25, Aleksandar Markovic wrote: > > Paul Burton and Aurelien Jarno removed for not being present. > > A polite email was sent to them with question whether they > > intend to actively participate, but there was no response. > > I indeed received a polite email, but it was sent less than 12 hours ago > (Peter Maydell was Cc:ed and can confirm). I didn't even got time to > answer it, I am still processing my emails after coming back from paid > work. > > I don't understand this sudden need to rush things in adjusting the > MIPS maintainership. > > Regards, > Aurelien > > -- > Aurelien Jarno GPG: 4096R/1DDD8C9B > aurel...@aurel32.net http://www.aurel32.net -- Aurelien Jarno GPG: 4096R/1DDD8C9B aurel...@aurel32.net http://www.aurel32.net
Re: [PATCH v3 20/26] x86: Fix x86_cpu_new() error handling
Igor, Paolo, you showed me the error in v2. Could you have a look at
this revision?
Markus Armbruster writes:
> The Error ** argument must be NULL, _abort, _fatal, or a
> pointer to a variable containing NULL. Passing an argument of the
> latter kind twice without clearing it in between is wrong: if the
> first call sets an error, it no longer points to NULL for the second
> call.
>
> x86_cpu_new() is wrong that way: it passes _err to
> object_property_set_uint() without checking it, and then to
> qdev_realize(). If both fail, we'll trip error_setv()'s assertion.
> To assess the bug's impact, we'd need to figure out how to make both
> calls fail. Too much work for ignorant me, sorry.
>
> Fix by checking for failure right away.
>
> Cc: Igor Mammedov
> Cc: Paolo Bonzini
> Cc: Richard Henderson
> Cc: Eduardo Habkost
> Signed-off-by: Markus Armbruster
> ---
> hw/i386/x86.c | 8 +---
> 1 file changed, 5 insertions(+), 3 deletions(-)
>
> diff --git a/hw/i386/x86.c b/hw/i386/x86.c
> index 34229b45c7..93f7371a56 100644
> --- a/hw/i386/x86.c
> +++ b/hw/i386/x86.c
> @@ -118,14 +118,16 @@ uint32_t x86_cpu_apic_id_from_index(X86MachineState
> *x86ms,
>
> void x86_cpu_new(X86MachineState *x86ms, int64_t apic_id, Error **errp)
> {
> -Object *cpu = NULL;
> Error *local_err = NULL;
> -
> -cpu = object_new(MACHINE(x86ms)->cpu_type);
> +Object *cpu = object_new(MACHINE(x86ms)->cpu_type);
>
> object_property_set_uint(cpu, apic_id, "apic-id", _err);
> +if (local_err) {
> +goto out;
> +}
> qdev_realize(DEVICE(cpu), NULL, _err);
>
> +out:
> object_unref(cpu);
> error_propagate(errp, local_err);
> }
Re: [PATCH v2] chardev/tcp: fix error message double free error
Paolo Bonzini writes:
> On 01/07/20 09:06, Markus Armbruster wrote:
>> lichun writes:
>>
>>> Signed-off-by: lichun
>>> ---
>>> chardev/char-socket.c | 3 ++-
>>> 1 file changed, 2 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/chardev/char-socket.c b/chardev/char-socket.c
>>> index afebeec5c3..569d54c144 100644
>>> --- a/chardev/char-socket.c
>>> +++ b/chardev/char-socket.c
>>> @@ -142,6 +142,8 @@ static void check_report_connect_error(Chardev *chr,
>>>"Unable to connect character device %s: ",
>>>chr->label);
>>> s->connect_err_reported = true;
>>> +} else {
>>> +error_free(err);
>>> }
>>> qemu_chr_socket_restart_timer(chr);
>>> }
>>> @@ -1086,7 +1088,6 @@ static void qemu_chr_socket_connected(QIOTask *task,
>>> void *opaque)
>>> if (qio_task_propagate_error(task, )) {
>>> tcp_chr_change_state(s, TCP_CHARDEV_STATE_DISCONNECTED);
>>> check_report_connect_error(chr, err);
>>> -error_free(err);
>>> goto cleanup;
>>> }
>>
>> Reviewed-by: Markus Armbruster
>>
>> and queued, thanks!
>>
>
> Can you please add a note to the commit message?
>
> Errors are already freed by error_report_err, so we only need to call
> error_free when that function is not called.
>
> and Cc qemu-stable? Or I can queue it too since it's chardev stuff.
Done in my tree, expect PR later today.
Re: [PATCH] cpus: Move CPU code from exec.c to cpus.c
On 7/1/20 10:54 AM, Philippe Mathieu-Daudé wrote: > This code was introduced with SMP support in commit 6a00d60127, > later commit 296af7c952 moved CPU parts to cpus.c but forgot this > code. Move now and simplify ifdef'ry. > > Signed-off-by: Philippe Mathieu-Daudé > --- > cpus.c | 18 ++ > exec.c | 22 -- > 2 files changed, 18 insertions(+), 22 deletions(-) Reviewed-by: Richard Henderson r~
Re: [PATCH 2/2] xen: cleanup unrealized flash devices
Jason Andryuk writes:
> On Wed, Jul 1, 2020 at 3:03 AM Paul Durrant wrote:
>>
>> > -Original Message-
>> > From: Philippe Mathieu-Daudé
>> > Sent: 30 June 2020 18:27
>> > To: p...@xen.org; xen-de...@lists.xenproject.org; qemu-devel@nongnu.org
>> > Cc: 'Eduardo Habkost' ; 'Michael S. Tsirkin'
>> > ; 'Paul Durrant'
>> > ; 'Jason Andryuk' ; 'Paolo
>> > Bonzini' ;
>> > 'Richard Henderson'
>> > Subject: Re: [PATCH 2/2] xen: cleanup unrealized flash devices
>> >
>> > On 6/30/20 5:44 PM, Paul Durrant wrote:
>> > >> -Original Message-
>> > >> From: Philippe Mathieu-Daudé
>> > >> Sent: 30 June 2020 16:26
>> > >> To: Paul Durrant ; xen-de...@lists.xenproject.org;
>> > >> qemu-devel@nongnu.org
>> > >> Cc: Eduardo Habkost ; Michael S. Tsirkin
>> > >> ; Paul Durrant
>> > >> ; Jason Andryuk ; Paolo
>> > >> Bonzini ;
>> > >> Richard Henderson
>> > >> Subject: Re: [PATCH 2/2] xen: cleanup unrealized flash devices
>> > >>
>> > >> On 6/24/20 2:18 PM, Paul Durrant wrote:
>> > >>> From: Paul Durrant
>> > >>>
>> > >>> The generic pc_machine_initfn() calls pc_system_flash_create() which
>> > >>> creates
>> > >>> 'system.flash0' and 'system.flash1' devices. These devices are then
>> > >>> realized
>> > >>> by pc_system_flash_map() which is called from
>> > >>> pc_system_firmware_init() which
>> > >>> itself is called via pc_memory_init(). The latter however is not
>> > >>> called when
>> > >>> xen_enable() is true and hence the following assertion fails:
>> > >>>
>> > >>> qemu-system-i386: hw/core/qdev.c:439: qdev_assert_realized_properly:
>> > >>> Assertion `dev->realized' failed
>> > >>>
>> > >>> These flash devices are unneeded when using Xen so this patch avoids
>> > >>> the
>> > >>> assertion by simply removing them using
>> > >>> pc_system_flash_cleanup_unused().
>> > >>>
>> > >>> Reported-by: Jason Andryuk
>> > >>> Fixes: ebc29e1beab0 ("pc: Support firmware configuration with
>> > >>> -blockdev")
>> > >>> Signed-off-by: Paul Durrant
>> > >>> Tested-by: Jason Andryuk
>> > >>> ---
>> > >>> Cc: Paolo Bonzini
>> > >>> Cc: Richard Henderson
>> > >>> Cc: Eduardo Habkost
>> > >>> Cc: "Michael S. Tsirkin"
>> > >>> Cc: Marcel Apfelbaum
>> > >>> ---
>> > >>> hw/i386/pc_piix.c| 9 ++---
>> > >>> hw/i386/pc_sysfw.c | 2 +-
>> > >>> include/hw/i386/pc.h | 1 +
>> > >>> 3 files changed, 8 insertions(+), 4 deletions(-)
>> > >>>
>> > >>> diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
>> > >>> index 1497d0e4ae..977d40afb8 100644
>> > >>> --- a/hw/i386/pc_piix.c
>> > >>> +++ b/hw/i386/pc_piix.c
>> > >>> @@ -186,9 +186,12 @@ static void pc_init1(MachineState *machine,
>> > >>> if (!xen_enabled()) {
>> > >>> pc_memory_init(pcms, system_memory,
>> > >>> rom_memory, _memory);
>> > >>> -} else if (machine->kernel_filename != NULL) {
>> > >>> -/* For xen HVM direct kernel boot, load linux here */
>> > >>> -xen_load_linux(pcms);
>> > >>> +} else {
>> > >>> +pc_system_flash_cleanup_unused(pcms);
>> > >>
>> > >> TIL pc_system_flash_cleanup_unused().
>> > >>
>> > >> What about restricting at the source?
>> > >>
>> > >
>> > > And leave the devices in place? They are not relevant for Xen, so why
>> > > not clean up?
>> >
>> > No, I meant to not create them in the first place, instead of
>> > create+destroy.
>> >
>> > Anyway what you did works, so I don't have any problem.
>>
>> IIUC Jason originally tried restricting creation but encountered a problem
>> because xen_enabled() would always return false at that point, because
>> machine creation occurs before accelerators are initialized.
>
> Correct. Quoting my previous email:
> """
> Removing the call to pc_system_flash_create() from pc_machine_initfn()
> lets QEMU startup and run a Xen HVM again. xen_enabled() doesn't work
> there since accelerators have not been initialized yes, I guess?
> """
>
> If you want to remove the creation in the first place, then I have two
> questions. Why does pc_system_flash_create()/pc_pflash_create() get
> called so early creating the pflash devices? Why aren't they just
> created as needed in pc_system_flash_map()?
commit ebc29e1beab02646702c8cb9a1d29b68f72ad503
pc: Support firmware configuration with -blockdev
[...]
Properties need to be created in .instance_init() methods. For PC
machines, that's pc_machine_initfn(). To make alias properties work,
we need to create the onboard flash devices there, too. [...]
For context, read the entire commit message. If you have questions
then, don't hesitate to ask them here.
Re: [PATCH v1 2/3] hw/riscv: Allow 64 bit access to SiFive CLINT
On Tue, Jun 30, 2020 at 5:19 PM LIU Zhiwei wrote:
>
>
>
> On 2020/7/1 4:12, Alistair Francis wrote:
> > Commit 5d971f9e672507210e77d020d89e0e89165c8fc9
> > "memory: Revert "memory: accept mismatching sizes in
> > memory_region_access_valid"" broke most RISC-V boards as they do 64 bit
> > accesses to the CLINT and QEMU would trigger a fault. Fix this failure
> > by allowing 8 byte accesses.
> >
> > Signed-off-by: Alistair Francis
> > ---
> > hw/riscv/sifive_clint.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/hw/riscv/sifive_clint.c b/hw/riscv/sifive_clint.c
> > index b11ffa0edc..669c21adc2 100644
> > --- a/hw/riscv/sifive_clint.c
> > +++ b/hw/riscv/sifive_clint.c
> > @@ -181,7 +181,7 @@ static const MemoryRegionOps sifive_clint_ops = {
> > .endianness = DEVICE_LITTLE_ENDIAN,
> > .valid = {
> > .min_access_size = 4,
> > -.max_access_size = 4
> > +.max_access_size = 8
> > }
> > };
> >
>
> Reviewed-by: LIU Zhiwei
Thanks for the review. As most RISC-V machines are broken without this
patch I have applied it to my next PR.
Alistair
>
>
Re: [PATCH v12 00/61] target/riscv: support vector extension v0.7.1
On Wed, Jul 1, 2020 at 8:26 AM LIU Zhiwei wrote:
>
> This patchset implements the vector extension for RISC-V on QEMU.
>
> You can also find the patchset and all *test cases* in
> my repo(https://github.com/romanheros/qemu.git branch:vector-upstream-v12).
> All the test cases are in the directory qemu/tests/riscv/vector/. They are
> riscv64 linux user mode programs.
>
> You can test the patchset by the script qemu/tests/riscv/vector/runcase.sh.
>
> Features:
> * support specification
> riscv-v-spec-0.7.1.(https://github.com/riscv/riscv-v-spec/releases/tag/0.7.1/)
> * support basic vector extension.
> * support Zvlsseg.
> * support Zvamo.
> * not support Zvediv as it is changing.
> * SLEN always equals VLEN.
> * element width support 8bit, 16bit, 32bit, 64bit.
>
> Changelog:
>
> v12
> * fix compile error on big endian machines.
Thanks!
Applied to riscv-to-apply.next
Alistair
>
> v11
> * fix all non-ASCII characters.
>
> v10
> * rebase to https://github.com/alistair23/qemu/tree/riscv-to-apply.next.
> * fix compile error in patch 57/61.
> * fix review tag typo.
>
> v9
> * always set dynamic rounding mode for vector float insns.
> * bug fix atomic implementation.
> * bug fix first-only-fault.
> * some small tidy up.
>
> v8
> * support different float rounding modes for vector instructions.
> * use lastest released TCG GVEC DUP IR.
> * set RV_VLEN_MAX to 256 bits, as GVEC IR uses simd_desc.
>
> v7
> * move vl == 0 check to translation time by add a global cpu_vl.
> * implement vector element inline load and store function by TCG IR.
> * based on vec_element_load(store), implement some permutation instructions.
> * implement rsubs GVEC IR.
> * fixup vsmul, vmfne, vfmerge, vslidedown.
> * some other small bugs and indentation errors.
>
> v6
> * use gvec_dup Gvec IR to accellerate move and merge.
> * a better way to implement fixed point instructions.
> * a global check when vl == 0.
> * limit some macros to only one inline function call.
> * fixup sew error when use Gvec IR.
> * fixup bugs for corner cases.
>
> v5
> * fixup a bug in tb flags.
>
> v4
> * no change
>
> v3
> * move check code from execution-time to translation-time
> * use a continous memory block for vector register description.
> * vector registers as direct fields in RISCVCPUState.
> * support VLEN configure from qemu command line.
> * support ELEN configure from qemu command line.
> * support vector specification version configure from qemu command line.
> * probe pages before real load or store access.
> * use probe_page_check for no-fault operations in linux user mode.
> * generation atomic exit exception when in parallel environment.
> * fixup a lot of concrete bugs.
>
> V2
> * use float16_compare{_quiet}
> * only use GETPC() in outer most helper
> * add ctx.ext_v Property
>
>
> LIU Zhiwei (61):
> target/riscv: add vector extension field in CPURISCVState
> target/riscv: implementation-defined constant parameters
> target/riscv: support vector extension csr
> target/riscv: add vector configure instruction
> target/riscv: add an internals.h header
> target/riscv: add vector stride load and store instructions
> target/riscv: add vector index load and store instructions
> target/riscv: add fault-only-first unit stride load
> target/riscv: add vector amo operations
> target/riscv: vector single-width integer add and subtract
> target/riscv: vector widening integer add and subtract
> target/riscv: vector integer add-with-carry / subtract-with-borrow
> instructions
> target/riscv: vector bitwise logical instructions
> target/riscv: vector single-width bit shift instructions
> target/riscv: vector narrowing integer right shift instructions
> target/riscv: vector integer comparison instructions
> target/riscv: vector integer min/max instructions
> target/riscv: vector single-width integer multiply instructions
> target/riscv: vector integer divide instructions
> target/riscv: vector widening integer multiply instructions
> target/riscv: vector single-width integer multiply-add instructions
> target/riscv: vector widening integer multiply-add instructions
> target/riscv: vector integer merge and move instructions
> target/riscv: vector single-width saturating add and subtract
> target/riscv: vector single-width averaging add and subtract
> target/riscv: vector single-width fractional multiply with rounding
> and saturation
> target/riscv: vector widening saturating scaled multiply-add
> target/riscv: vector single-width scaling shift instructions
> target/riscv: vector narrowing fixed-point clip instructions
> target/riscv: vector single-width floating-point add/subtract
> instructions
> target/riscv: vector widening floating-point add/subtract instructions
> target/riscv: vector single-width floating-point multiply/divide
> instructions
> target/riscv: vector widening
Re: [PATCH v4 28/40] tests/acceptance: skip multicore mips_malta tests on GitLab
在 2020/7/2 1:01, Philippe Mathieu-Daudé 写道: On 7/1/20 6:43 PM, Alex Bennée wrote: Philippe Mathieu-Daudé writes: On 7/1/20 3:56 PM, Alex Bennée wrote: For some reason these tests fail all the time on GitLab. I can re-create the hang around 3% of the time locally but it doesn't seem to be MTTCG related. For now skipIf on GITLAB_CI. Signed-off-by: Alex Bennée Cc: Aleksandar Markovic --- tests/acceptance/machine_mips_malta.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/tests/acceptance/machine_mips_malta.py b/tests/acceptance/machine_mips_malta.py index 92b4f28a112..7c9a4ee4d2d 100644 --- a/tests/acceptance/machine_mips_malta.py +++ b/tests/acceptance/machine_mips_malta.py @@ -15,6 +15,7 @@ from avocado import skipUnless from avocado_qemu import Test from avocado_qemu import wait_for_console_pattern from avocado.utils import archive +from avocado import skipIf NUMPY_AVAILABLE = True @@ -99,6 +100,7 @@ class MaltaMachineFramebuffer(Test): """ self.do_test_i6400_framebuffer_logo(1) So the test works using a single core... Good we have a test to figure the bug! It's about a 1-3% failure rate on my big test box but hits every time on CI. However I did disable MTTCG and still saw failures so I think it's a more subtle breakage than just a straight race. I first thought it was a MTTCG problem, but then I realized you didn't disable the single core test. When using >1 core, the malta uses a different device, the CPS for Coherent Processing System. It contains a Inter-Thread Communication Unit and a Global Interrupt Controller. There might be a I/O locking problem. In particular, some of these devices access the >env (the ITU is more of micro-architecture). This is why I was excited by your finding :) We might have a way to figure it out. FYI: This issue seems relative with the performence of host machine. I can reproduce the issue in high frequency if I unplug my laptop from AC adapter (it will switch to powersave governor). So my first thought was it just because TCG runs too slow so cores failed to respond IPI timely. Thanks. - Jiaxun
Re: [RFC v2 1/1] memory: Delete assertion in memory_region_unregister_iommu_notifier
On 2020/7/1 下午4:09, Jason Wang wrote: On 2020/6/30 下午11:39, Peter Xu wrote: On Tue, Jun 30, 2020 at 10:41:10AM +0800, Jason Wang wrote: /* According to ATS spec table 2.4: * S = 0, bits 15:12 = range size: 4K * S = 1, bits 15:12 = xxx0 range size: 8K * S = 1, bits 15:12 = xx01 range size: 16K * S = 1, bits 15:12 = x011 range size: 32K * S = 1, bits 15:12 = 0111 range size: 64K * ... */ Right, but the comment is probably misleading here, since it's for the PCI-E transaction between IOMMU and device not for the device IOTLB invalidation descriptor. For device IOTLB invalidation descriptor, spec allows a [0, ~0ULL] invalidation: " 6.5.2.5 Device-TLB Invalidate Descriptor ... Size (S): The size field indicates the number of consecutive pages targeted by this invalidation request. If S field is zero, a single page at page address specified by Address [63:12] is requested to be invalidated. If S field is Set, the least significant bit in the Address field with value 0b indicates the invalidation address range. For example, if S field is Set and Address[12] is Clear, it indicates an 8KB invalidation address range with base address in Address [63:13]. If S field and Address[12] is Set and bit 13 is Clear, it indicates a 16KB invalidation address range with base address in Address [63:14], etc. " So if we receive an address whose [63] is 0 and the rest is all 1, it's then a [0, ~0ULL] invalidation. Yes. I think invalidating the whole range is always fine. It's still not arbitrary, right? E.g., we can't even invalidate (0x1000, 0x3000) with device-iotlb because of the address mask, not to say sub-pages. Yes. How about just convert to use a range [start, end] for any notifier and move the checks (e.g the assert) into the actual notifier implemented (vhost or vfio)? IOMMUTLBEntry itself is the abstraction layer of TLB entry. Hardware TLB entry is definitely not arbitrary range either (because AFAICT the hardware should only cache PFN rather than address, so at least PAGE_SIZE aligned). Introducing this flag will already make this trickier just to avoid introducing another similar struct to IOMMUTLBEntry, but I really don't want to make it a default option... Not to mention I probably have no reason to urge the rest iommu notifier users (tcg, vfio) to change their existing good code to suite any of the backend who can cooperate with arbitrary address ranges... Ok, so it looks like we need a dedicated notifiers to device IOTLB. Or we can also make a new flag for device iotlb just like current UNMAP? Then we replace the vhost type from UNMAP to DEVICE_IOTLB. But IMHO using the ARBITRARY_LENGTH flag would work in a similar way. DEVICE_IOTLB flag could also allow virtio/vhost to only receive one invalidation (now IIUC it'll receive both iotlb and device-iotlb for unmapping a page when ats=on), but then ats=on will be a must and it could break some old (misconfiged) qemu because afaict previously virtio/vhost could even work with vIOMMU (accidentally) even without ats=on. That's a bug and I don't think we need to workaround mis-configurated qemu :) IMHO it depends on the strictness we want on the qemu cmdline API. :) We should at least check libvirt to make sure it's using ats=on always, then I agree maybe we can avoid considering the rest... Thanks, Cc libvirt list, but I think we should fix libvirt if they don't provide "ats=on". Thanks Libvirt looks fine, according to the domain XML documentation[1]: QEMU's virtio devices have some attributes related to the virtio transport under the driver element: The iommu attribute enables the use of emulated IOMMU by the device. The attribute ats controls the Address Translation Service support for PCIe devices. This is needed to make use of IOTLB support (see IOMMU device). Possible values are on or off. Since 3.5.0 So I think we agree that a new notifier is needed? Thanks [1] https://libvirt.org/formatdomain.html
Re: [RFC v2 1/1] memory: Delete assertion in memory_region_unregister_iommu_notifier
On 2020/7/1 下午8:41, Peter Xu wrote: On Wed, Jul 01, 2020 at 08:30:07PM +0800, Jason Wang wrote: I overlooked myself that the IR region will be there even if ir=off. Yes, but the point stands still but the issue is still if ir=off. So I think the assert should stand. Do you mean vhost can't trigger the assert()? If yes, I don't get how it can't. Yes. vhost can only trigger the translate() via memory API. No matter whether ir is on/off, the memory region is always enabled, so any access to 0xfeex from memory API should still land at s->mr_ir, afaict, rather than the dmar region. Right. Thanks
Re: [PATCH] MAINTAINERS: add VT-d entry
On 2020/7/1 下午8:44, Peter Xu wrote: Add this entry as suggested by Jason and Michael. CC: Jason Wang CC: Michael S. Tsirkin CC: Paolo Bonzini Signed-off-by: Peter Xu --- MAINTAINERS | 9 + 1 file changed, 9 insertions(+) diff --git a/MAINTAINERS b/MAINTAINERS index dec252f38b..569cfc1fcd 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -2615,6 +2615,15 @@ F: tests/uefi-test-tools/ F: .gitlab-ci.d/edk2.yml F: .gitlab-ci.d/edk2/ +VT-d Emulation +M: Michael S. Tsirkin +M: Peter Xu +R: Jason Wang +S: Supported +F: hw/i386/intel_iommu.c +F: hw/i386/intel_iommu_internal.h +F: include/hw/i386/intel_iommu.h + Usermode Emulation -- Overall usermode emulation Acked-by: Jason Wang Thanks
Re: [PATCH v3 13/13] vhost-vdpa: introduce vhost-vdpa net client
Hi Michael, Eric,
This was fix in the latest version, v4
https://patchew.org/QEMU/20200701145538.22333-1-l...@redhat.com/20200701145538.22333-15-l...@redhat.com/
On Wed, Jul 1, 2020 at 11:21 PM Michael S. Tsirkin wrote:
>
> On Wed, Jul 01, 2020 at 09:28:27AM -0500, Eric Blake wrote:
> > On 7/1/20 4:24 AM, Cindy Lu wrote:
> > > This patch set introduces a new net client type: vhost-vdpa.
> > > vhost-vdpa net client will set up a vDPA device which is specified
> > > by a "vhostdev" parameter.
> > >
> > > Signed-off-by: Lingshan Zhu
> > > Signed-off-by: Tiwei Bie
> > > Signed-off-by: Cindy Lu
> > > Signed-off-by: Jason Wang
> > > ---
> >
> > > +++ b/qapi/net.json
> >
> > > ##
> > > # @NetClientDriver:
> > > #
> > > # Available netdev drivers.
> > > #
> > > -# Since: 2.7
> > > +# Since: 5.1
> >
> > This should be:
> >
> > # Since 2.7
> > # @vhost-vdpa since 5.1
> >
> > since the enum itself is still available in older releases, it is only the
> > new member that was introduced in this series.
> >
> > > ##
> > > { 'enum': 'NetClientDriver',
> > > 'data': [ 'none', 'nic', 'user', 'tap', 'l2tpv3', 'socket', 'vde',
> > > -'bridge', 'hubport', 'netmap', 'vhost-user' ] }
> > > +'bridge', 'hubport', 'netmap', 'vhost-user', 'vhost-vdpa' ] }
> >
>
>
> Thanks! I will fix it up when applying.
> Ack with that fixed?
>
>
> > --
> > Eric Blake, Principal Software Engineer
> > Red Hat, Inc. +1-919-301-3226
> > Virtualization: qemu.org | libvirt.org
>
RE: [PATCH] MAINTAINERS: add VT-d entry
> From: Peter Xu > Sent: Wednesday, July 1, 2020 8:44 PM> > Add this entry as suggested by Jason and Michael. > > CC: Jason Wang > CC: Michael S. Tsirkin > CC: Paolo Bonzini > Signed-off-by: Peter Xu > --- > MAINTAINERS | 9 + > 1 file changed, 9 insertions(+) > > diff --git a/MAINTAINERS b/MAINTAINERS > index dec252f38b..569cfc1fcd 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -2615,6 +2615,15 @@ F: tests/uefi-test-tools/ > F: .gitlab-ci.d/edk2.yml > F: .gitlab-ci.d/edk2/ > > +VT-d Emulation > +M: Michael S. Tsirkin > +M: Peter Xu > +R: Jason Wang great! Regards, Yi Liu > +S: Supported > +F: hw/i386/intel_iommu.c > +F: hw/i386/intel_iommu_internal.h > +F: include/hw/i386/intel_iommu.h > + > Usermode Emulation > -- > Overall usermode emulation > -- > 2.26.2 >
Re: [PATCH v4 28/40] tests/acceptance: skip multicore mips_malta tests on GitLab
On Wed, Jul 1, 2020 at 4:03 PM Alex Bennée wrote:
>
> For some reason these tests fail all the time on GitLab. I can
> re-create the hang around 3% of the time locally but it doesn't seem
> to be MTTCG related. For now skipIf on GITLAB_CI.
>
> Signed-off-by: Alex Bennée
> Cc: Aleksandar Markovic
> ---
Alex,
Thanks for having this test at all. I will review its content, but
here is my stupid question:
How can I, as a regular developer, repro the test in question? I am
not familiar with GitLab at all.
Thanks,
Aleksandar
> tests/acceptance/machine_mips_malta.py | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/tests/acceptance/machine_mips_malta.py
> b/tests/acceptance/machine_mips_malta.py
> index 92b4f28a112..7c9a4ee4d2d 100644
> --- a/tests/acceptance/machine_mips_malta.py
> +++ b/tests/acceptance/machine_mips_malta.py
> @@ -15,6 +15,7 @@ from avocado import skipUnless
> from avocado_qemu import Test
> from avocado_qemu import wait_for_console_pattern
> from avocado.utils import archive
> +from avocado import skipIf
>
>
> NUMPY_AVAILABLE = True
> @@ -99,6 +100,7 @@ class MaltaMachineFramebuffer(Test):
> """
> self.do_test_i6400_framebuffer_logo(1)
>
> +@skipIf(os.getenv('GITLAB_CI'), 'Running on GitLab')
> def test_mips_malta_i6400_framebuffer_logo_7cores(self):
> """
> :avocado: tags=arch:mips64el
> @@ -108,6 +110,7 @@ class MaltaMachineFramebuffer(Test):
> """
> self.do_test_i6400_framebuffer_logo(7)
>
> +@skipIf(os.getenv('GITLAB_CI'), 'Running on GitLab')
> def test_mips_malta_i6400_framebuffer_logo_8cores(self):
> """
> :avocado: tags=arch:mips64el
> --
> 2.20.1
>
Re: [PATCH] tcg: Fix do_nonatomic_op_* vs signed operations
On 2020/7/2 0:56, Richard Henderson wrote: The smin/smax/umin/umax operations require the operands to be properly sign extended. Do not drop the MO_SIGN bit from the load, and additionally extend the val input. Reported-by: LIU Zhiwei Signed-off-by: Richard Henderson --- tcg/tcg-op.c | 10 ++ 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/tcg/tcg-op.c b/tcg/tcg-op.c index e60b74fb82..4b8a473fad 100644 --- a/tcg/tcg-op.c +++ b/tcg/tcg-op.c @@ -3189,8 +3189,9 @@ static void do_nonatomic_op_i32(TCGv_i32 ret, TCGv addr, TCGv_i32 val, memop = tcg_canonicalize_memop(memop, 0, 0); -tcg_gen_qemu_ld_i32(t1, addr, idx, memop & ~MO_SIGN); -gen(t2, t1, val); +tcg_gen_qemu_ld_i32(t1, addr, idx, memop); +tcg_gen_ext_i32(t2, val, memop); +gen(t2, t1, t2); tcg_gen_qemu_st_i32(t2, addr, idx, memop); tcg_gen_ext_i32(ret, (new_val ? t2 : t1), memop); @@ -3232,8 +3233,9 @@ static void do_nonatomic_op_i64(TCGv_i64 ret, TCGv addr, TCGv_i64 val, memop = tcg_canonicalize_memop(memop, 1, 0); -tcg_gen_qemu_ld_i64(t1, addr, idx, memop & ~MO_SIGN); -gen(t2, t1, val); +tcg_gen_qemu_ld_i64(t1, addr, idx, memop); +tcg_gen_ext_i64(t2, val, memop); +gen(t2, t1, t2); tcg_gen_qemu_st_i64(t2, addr, idx, memop); tcg_gen_ext_i64(ret, (new_val ? t2 : t1), memop); Reviewed-by: LIU Zhiwei Zhiwei
[PATCH v4 10/11] target/ppc: add vmulh{su}d instructions
vmulhsd: Vector Multiply High Signed Doubleword
vmulhud: Vector Multiply High Unsigned Doubleword
Signed-off-by: Lijun Pan
---
Reviewed-by: Richard Henderson
v3: simplify helper_vmulh{su}d
v2: fix coding style
use Power ISA 3.1 flag
target/ppc/helper.h | 2 ++
target/ppc/int_helper.c | 16
target/ppc/translate/vmx-impl.inc.c | 2 ++
target/ppc/translate/vmx-ops.inc.c | 2 ++
4 files changed, 22 insertions(+)
diff --git a/target/ppc/helper.h b/target/ppc/helper.h
index 3b3013866a..0036788919 100644
--- a/target/ppc/helper.h
+++ b/target/ppc/helper.h
@@ -186,6 +186,8 @@ DEF_HELPER_3(vmulouh, void, avr, avr, avr)
DEF_HELPER_3(vmulouw, void, avr, avr, avr)
DEF_HELPER_3(vmulhsw, void, avr, avr, avr)
DEF_HELPER_3(vmulhuw, void, avr, avr, avr)
+DEF_HELPER_3(vmulhsd, void, avr, avr, avr)
+DEF_HELPER_3(vmulhud, void, avr, avr, avr)
DEF_HELPER_3(vslo, void, avr, avr, avr)
DEF_HELPER_3(vsro, void, avr, avr, avr)
DEF_HELPER_3(vsrv, void, avr, avr, avr)
diff --git a/target/ppc/int_helper.c b/target/ppc/int_helper.c
index a3a20821fc..57d6767f60 100644
--- a/target/ppc/int_helper.c
+++ b/target/ppc/int_helper.c
@@ -1105,6 +1105,22 @@ void helper_vmulhuw(ppc_avr_t *r, ppc_avr_t *a,
ppc_avr_t *b)
}
}
+void helper_vmulhsd(ppc_avr_t *r, ppc_avr_t *a, ppc_avr_t *b)
+{
+uint64_t discard;
+
+muls64(, >u64[0], a->s64[0], b->s64[0]);
+muls64(, >u64[1], a->s64[1], b->s64[1]);
+}
+
+void helper_vmulhud(ppc_avr_t *r, ppc_avr_t *a, ppc_avr_t *b)
+{
+uint64_t discard;
+
+mulu64(, >u64[0], a->u64[0], b->u64[0]);
+mulu64(, >u64[1], a->u64[1], b->u64[1]);
+}
+
void helper_vperm(CPUPPCState *env, ppc_avr_t *r, ppc_avr_t *a, ppc_avr_t *b,
ppc_avr_t *c)
{
diff --git a/target/ppc/translate/vmx-impl.inc.c
b/target/ppc/translate/vmx-impl.inc.c
index 50bac375fc..0910807232 100644
--- a/target/ppc/translate/vmx-impl.inc.c
+++ b/target/ppc/translate/vmx-impl.inc.c
@@ -812,6 +812,7 @@ GEN_VXFORM(vmuleub, 4, 8);
GEN_VXFORM(vmuleuh, 4, 9);
GEN_VXFORM(vmuleuw, 4, 10);
GEN_VXFORM(vmulhuw, 4, 10);
+GEN_VXFORM(vmulhud, 4, 11);
GEN_VXFORM_DUAL(vmuleuw, PPC_ALTIVEC, PPC_NONE,
vmulhuw, PPC_NONE, PPC2_ISA310);
GEN_VXFORM(vmulesb, 4, 12);
@@ -820,6 +821,7 @@ GEN_VXFORM(vmulesw, 4, 14);
GEN_VXFORM(vmulhsw, 4, 14);
GEN_VXFORM_DUAL(vmulesw, PPC_ALTIVEC, PPC_NONE,
vmulhsw, PPC_NONE, PPC2_ISA310);
+GEN_VXFORM(vmulhsd, 4, 15);
GEN_VXFORM_V(vslb, MO_8, tcg_gen_gvec_shlv, 2, 4);
GEN_VXFORM_V(vslh, MO_16, tcg_gen_gvec_shlv, 2, 5);
GEN_VXFORM_V(vslw, MO_32, tcg_gen_gvec_shlv, 2, 6);
diff --git a/target/ppc/translate/vmx-ops.inc.c
b/target/ppc/translate/vmx-ops.inc.c
index 29701ad778..f3f4855111 100644
--- a/target/ppc/translate/vmx-ops.inc.c
+++ b/target/ppc/translate/vmx-ops.inc.c
@@ -111,9 +111,11 @@ GEN_VXFORM_310(vmulld, 4, 7),
GEN_VXFORM(vmuleub, 4, 8),
GEN_VXFORM(vmuleuh, 4, 9),
GEN_VXFORM_DUAL(vmuleuw, vmulhuw, 4, 10, PPC_ALTIVEC, PPC_NONE),
+GEN_VXFORM_310(vmulhud, 4, 11),
GEN_VXFORM(vmulesb, 4, 12),
GEN_VXFORM(vmulesh, 4, 13),
GEN_VXFORM_DUAL(vmulesw, vmulhsw, 4, 14, PPC_ALTIVEC, PPC_NONE),
+GEN_VXFORM_310(vmulhsd, 4, 15),
GEN_VXFORM(vslb, 2, 4),
GEN_VXFORM(vslh, 2, 5),
GEN_VXFORM_DUAL(vslw, vrlwnm, 2, 6, PPC_ALTIVEC, PPC_NONE),
--
2.23.0
[PATCH v4 11/11] target/ppc: add vdiv{su}{wd} vmod{su}{wd} instructions
vdivsw: Vector Divide Signed Word
vdivuw: Vector Divide Unsigned Word
vdivsd: Vector Divide Signed Doubleword
vdivud: Vector Divide Unsigned Doubleword
vmodsw: Vector Modulo Signed Word
vmoduw: Vector Modulo Unsigned Word
vmodsd: Vector Modulo Signed Doubleword
vmodud: Vector Modulo Unsigned Doubleword
Signed-off-by: Lijun Pan
---
v4: add a comment on undefined result of divide operation.
fix if(){} coding style issue, remove blank line.
v3: add missing divided-by-zero, divided-by-(-1) handling
v2: fix coding style
use Power ISA 3.1 flag
target/ppc/helper.h | 8
target/ppc/int_helper.c | 27 +++
target/ppc/translate.c | 3 +++
target/ppc/translate/vmx-impl.inc.c | 15 +++
target/ppc/translate/vmx-ops.inc.c | 17 +++--
5 files changed, 68 insertions(+), 2 deletions(-)
diff --git a/target/ppc/helper.h b/target/ppc/helper.h
index 0036788919..70a14029ca 100644
--- a/target/ppc/helper.h
+++ b/target/ppc/helper.h
@@ -188,6 +188,14 @@ DEF_HELPER_3(vmulhsw, void, avr, avr, avr)
DEF_HELPER_3(vmulhuw, void, avr, avr, avr)
DEF_HELPER_3(vmulhsd, void, avr, avr, avr)
DEF_HELPER_3(vmulhud, void, avr, avr, avr)
+DEF_HELPER_3(vdivsw, void, avr, avr, avr)
+DEF_HELPER_3(vdivuw, void, avr, avr, avr)
+DEF_HELPER_3(vdivsd, void, avr, avr, avr)
+DEF_HELPER_3(vdivud, void, avr, avr, avr)
+DEF_HELPER_3(vmodsw, void, avr, avr, avr)
+DEF_HELPER_3(vmoduw, void, avr, avr, avr)
+DEF_HELPER_3(vmodsd, void, avr, avr, avr)
+DEF_HELPER_3(vmodud, void, avr, avr, avr)
DEF_HELPER_3(vslo, void, avr, avr, avr)
DEF_HELPER_3(vsro, void, avr, avr, avr)
DEF_HELPER_3(vsrv, void, avr, avr, avr)
diff --git a/target/ppc/int_helper.c b/target/ppc/int_helper.c
index 57d6767f60..62b93b4568 100644
--- a/target/ppc/int_helper.c
+++ b/target/ppc/int_helper.c
@@ -1121,6 +1121,33 @@ void helper_vmulhud(ppc_avr_t *r, ppc_avr_t *a,
ppc_avr_t *b)
mulu64(, >u64[1], a->u64[1], b->u64[1]);
}
+#define VDIV_MOD_DO(name, op, element, sign, bit) \
+void helper_v##name(ppc_avr_t *r, ppc_avr_t *a, ppc_avr_t *b) \
+{ \
+int i; \
+\
+for (i = 0; i < ARRAY_SIZE(r->element); i++) { \
+if (unlikely((b->element[i] == 0) ||\
+(sign &&\
+(b->element[i] == UINT##bit##_MAX) && \
+(a->element[i] == INT##bit##_MIN { \
+/* Undefined, No Special Registers Altered */ \
+continue; \
+} \
+r->element[i] = a->element[i] op b->element[i]; \
+} \
+}
+VDIV_MOD_DO(divsw, /, s32, 1, 32)
+VDIV_MOD_DO(divuw, /, u32, 0, 32)
+VDIV_MOD_DO(divsd, /, s64, 1, 64)
+VDIV_MOD_DO(divud, /, u64, 0, 64)
+VDIV_MOD_DO(modsw, %, s32, 1, 32)
+VDIV_MOD_DO(moduw, %, u32, 0, 32)
+VDIV_MOD_DO(modsd, %, s64, 1, 64)
+VDIV_MOD_DO(modud, %, u64, 0, 64)
+#undef VDIV_MOD_DO
+
+
void helper_vperm(CPUPPCState *env, ppc_avr_t *r, ppc_avr_t *a, ppc_avr_t *b,
ppc_avr_t *c)
{
diff --git a/target/ppc/translate.c b/target/ppc/translate.c
index 590c3e3bc7..55f0e1a01d 100644
--- a/target/ppc/translate.c
+++ b/target/ppc/translate.c
@@ -388,6 +388,9 @@ GEN_OPCODE3(name, opc1, opc2, opc3, opc4, inval, type,
type2)
#define GEN_HANDLER2_E_2(name, onam, opc1, opc2, opc3, opc4, inval, typ, typ2)
\
GEN_OPCODE4(name, onam, opc1, opc2, opc3, opc4, inval, typ, typ2)
+#define GEN_HANDLER_BOTH(name, opc1, opc2, opc3, inval0, inval1, type0, type1)
\
+GEN_OPCODE_DUAL(name, opc1, opc2, opc3, inval0, inval1, type0, type1)
+
typedef struct opcode_t {
unsigned char opc1, opc2, opc3, opc4;
#if HOST_LONG_BITS == 64 /* Explicitly align to 64 bits */
diff --git a/target/ppc/translate/vmx-impl.inc.c
b/target/ppc/translate/vmx-impl.inc.c
index 0910807232..ac5e820541 100644
--- a/target/ppc/translate/vmx-impl.inc.c
+++ b/target/ppc/translate/vmx-impl.inc.c
@@ -798,6 +798,9 @@ static void trans_vclzd(DisasContext *ctx)
tcg_temp_free_i64(avr);
}
+static void gen_vexptefp(DisasContext *ctx);
+static void gen_vlogefp(DisasContext *ctx);
+
GEN_VXFORM(vmuloub, 4, 0);
GEN_VXFORM(vmulouh, 4, 1);
GEN_VXFORM(vmulouw, 4, 2);
@@ -822,6 +825,18 @@ GEN_VXFORM(vmulhsw, 4, 14);
GEN_VXFORM_DUAL(vmulesw, PPC_ALTIVEC, PPC_NONE,
vmulhsw, PPC_NONE, PPC2_ISA310);
GEN_VXFORM(vmulhsd, 4, 15);
+GEN_VXFORM(vdivuw, 5, 2);
+GEN_VXFORM(vdivud, 5, 3);
+GEN_VXFORM(vdivsw, 5, 6);
+GEN_VXFORM_DUAL_EXT(vexptefp,
Re: [PATCH 1/1] tcg/tcg-op: nonatomic_op should work with smaller memop
On 2020/7/2 0:25, Richard Henderson wrote: On 7/1/20 8:21 AM, LIU Zhiwei wrote: -tcg_gen_qemu_ld_i32(t1, addr, idx, memop & ~MO_SIGN); +tcg_gen_qemu_ld_i32(t1, addr, idx, memop); +tcg_gen_ext_i32(val, val, memop); gen(t2, t1, val); I was just about to post a simiar patch. The difference with mine is that I do not modify val: -gen(t2, t1, val); +tcg_gen_ext_i32(t2, val, memop); +gen(t2, t1, t2); I see. So just ignore this patch.:-) Zhiwei r~
[PATCH v4 05/11] target/ppc: add vmulld instruction
vmulld: Vector Multiply Low Doubleword. Signed-off-by: Lijun Pan --- v4: add missing changes, and split to 5/11, 6/11, 7/11 v3: use tcg_gen_gvec_mul() v2: fix coding style use Power ISA 3.1 flag target/ppc/translate/vmx-impl.inc.c | 1 + target/ppc/translate/vmx-ops.inc.c | 4 2 files changed, 5 insertions(+) diff --git a/target/ppc/translate/vmx-impl.inc.c b/target/ppc/translate/vmx-impl.inc.c index 6e79ffa650..8c89738552 100644 --- a/target/ppc/translate/vmx-impl.inc.c +++ b/target/ppc/translate/vmx-impl.inc.c @@ -807,6 +807,7 @@ GEN_VXFORM_DUAL(vmulouw, PPC_ALTIVEC, PPC_NONE, GEN_VXFORM(vmulosb, 4, 4); GEN_VXFORM(vmulosh, 4, 5); GEN_VXFORM(vmulosw, 4, 6); +GEN_VXFORM_V(vmulld, MO_64, tcg_gen_gvec_mul, 4, 7); GEN_VXFORM(vmuleub, 4, 8); GEN_VXFORM(vmuleuh, 4, 9); GEN_VXFORM(vmuleuw, 4, 10); diff --git a/target/ppc/translate/vmx-ops.inc.c b/target/ppc/translate/vmx-ops.inc.c index 84e05fb827..b49787ac97 100644 --- a/target/ppc/translate/vmx-ops.inc.c +++ b/target/ppc/translate/vmx-ops.inc.c @@ -48,6 +48,9 @@ GEN_HANDLER_E(name, 0x04, opc2, opc3, inval, PPC_NONE, PPC2_ISA300) GEN_HANDLER_E_2(name, 0x04, opc2, opc3, opc4, 0x, PPC_NONE, \ PPC2_ISA300) +#define GEN_VXFORM_310(name, opc2, opc3)\ +GEN_HANDLER_E(name, 0x04, opc2, opc3, 0x, PPC_NONE, PPC2_ISA310) + #define GEN_VXFORM_DUAL(name0, name1, opc2, opc3, type0, type1) \ GEN_HANDLER_E(name0##_##name1, 0x4, opc2, opc3, 0x, type0, type1) @@ -104,6 +107,7 @@ GEN_VXFORM_DUAL(vmulouw, vmuluwm, 4, 2, PPC_ALTIVEC, PPC_NONE), GEN_VXFORM(vmulosb, 4, 4), GEN_VXFORM(vmulosh, 4, 5), GEN_VXFORM_207(vmulosw, 4, 6), +GEN_VXFORM_310(vmulld, 4, 7), GEN_VXFORM(vmuleub, 4, 8), GEN_VXFORM(vmuleuh, 4, 9), GEN_VXFORM_207(vmuleuw, 4, 10), -- 2.23.0
[PATCH v4 03/11] target/ppc: add byte-reverse br[dwh] instructions
POWER ISA 3.1 introduces following byte-reverse instructions:
brd: Byte-Reverse Doubleword X-form
brw: Byte-Reverse Word X-form
brh: Byte-Reverse Halfword X-form
Signed-off-by: Lijun Pan
---
v4: make it compile on all targets
v3: fix the store issue in br[dwh]
simplify brw implementation
add "if defined(TARGET_PPC64)"
v2: fix coding style
use Power ISA 3.1 flag
target/ppc/translate.c | 40
1 file changed, 40 insertions(+)
diff --git a/target/ppc/translate.c b/target/ppc/translate.c
index 4ce3d664b5..590c3e3bc7 100644
--- a/target/ppc/translate.c
+++ b/target/ppc/translate.c
@@ -6971,7 +6971,47 @@ static void gen_dform3D(DisasContext *ctx)
return gen_invalid(ctx);
}
+#if defined(TARGET_PPC64)
+/* brd */
+static void gen_brd(DisasContext *ctx)
+{
+tcg_gen_bswap64_i64(cpu_gpr[rA(ctx->opcode)], cpu_gpr[rS(ctx->opcode)]);
+}
+
+/* brw */
+static void gen_brw(DisasContext *ctx)
+{
+tcg_gen_bswap64_i64(cpu_gpr[rA(ctx->opcode)], cpu_gpr[rS(ctx->opcode)]);
+tcg_gen_rotli_i64(cpu_gpr[rA(ctx->opcode)], cpu_gpr[rA(ctx->opcode)], 32);
+
+}
+
+/* brh */
+static void gen_brh(DisasContext *ctx)
+{
+TCGv_i64 t0 = tcg_temp_new_i64();
+TCGv_i64 t1 = tcg_temp_new_i64();
+TCGv_i64 t2 = tcg_temp_new_i64();
+
+tcg_gen_movi_i64(t0, 0x00ff00ff00ff00ffull);
+tcg_gen_shri_i64(t1, cpu_gpr[rS(ctx->opcode)], 8);
+tcg_gen_and_i64(t2, t1, t0);
+tcg_gen_and_i64(t1, cpu_gpr[rS(ctx->opcode)], t0);
+tcg_gen_shli_i64(t1, t1, 8);
+tcg_gen_or_i64(cpu_gpr[rA(ctx->opcode)], t1, t2);
+
+tcg_temp_free_i64(t0);
+tcg_temp_free_i64(t1);
+tcg_temp_free_i64(t2);
+}
+#endif
+
static opcode_t opcodes[] = {
+#if defined(TARGET_PPC64)
+GEN_HANDLER_E(brd, 0x1F, 0x1B, 0x05, 0xF801, PPC_NONE, PPC2_ISA310),
+GEN_HANDLER_E(brw, 0x1F, 0x1B, 0x04, 0xF801, PPC_NONE, PPC2_ISA310),
+GEN_HANDLER_E(brh, 0x1F, 0x1B, 0x06, 0xF801, PPC_NONE, PPC2_ISA310),
+#endif
GEN_HANDLER(invalid, 0x00, 0x00, 0x00, 0x, PPC_NONE),
GEN_HANDLER(cmp, 0x1F, 0x00, 0x00, 0x0040, PPC_INTEGER),
GEN_HANDLER(cmpi, 0x0B, 0xFF, 0xFF, 0x0040, PPC_INTEGER),
--
2.23.0
[PATCH v4 09/11] fix the prototype of muls64/mulu64
The prototypes of muls64/mulu64 in host-utils.h should match the definitions in host-utils.c Signed-off-by: Lijun Pan --- Reviewed-by: Richard Henderson no change since v1 include/qemu/host-utils.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/include/qemu/host-utils.h b/include/qemu/host-utils.h index 4cd170e6cd..cdca2991d8 100644 --- a/include/qemu/host-utils.h +++ b/include/qemu/host-utils.h @@ -77,8 +77,8 @@ static inline int divs128(int64_t *plow, int64_t *phigh, int64_t divisor) } } #else -void muls64(uint64_t *phigh, uint64_t *plow, int64_t a, int64_t b); -void mulu64(uint64_t *phigh, uint64_t *plow, uint64_t a, uint64_t b); +void muls64(uint64_t *plow, uint64_t *phigh, int64_t a, int64_t b); +void mulu64(uint64_t *plow, uint64_t *phigh, uint64_t a, uint64_t b); int divu128(uint64_t *plow, uint64_t *phigh, uint64_t divisor); int divs128(int64_t *plow, int64_t *phigh, int64_t divisor); -- 2.23.0
[PATCH v4 04/11] target/ppc: convert vmuluwm to tcg_gen_gvec_mul
Convert the original implementation of vmuluwm to the more generic
tcg_gen_gvec_mul.
Signed-off-by: Lijun Pan
---
Reviewed-by: Richard Henderson
v3: newly introduced
target/ppc/helper.h | 1 -
target/ppc/int_helper.c | 13 -
target/ppc/translate/vmx-impl.inc.c | 2 +-
3 files changed, 1 insertion(+), 15 deletions(-)
diff --git a/target/ppc/helper.h b/target/ppc/helper.h
index 2dfa1c6942..69416b6d7c 100644
--- a/target/ppc/helper.h
+++ b/target/ppc/helper.h
@@ -184,7 +184,6 @@ DEF_HELPER_3(vmulosw, void, avr, avr, avr)
DEF_HELPER_3(vmuloub, void, avr, avr, avr)
DEF_HELPER_3(vmulouh, void, avr, avr, avr)
DEF_HELPER_3(vmulouw, void, avr, avr, avr)
-DEF_HELPER_3(vmuluwm, void, avr, avr, avr)
DEF_HELPER_3(vslo, void, avr, avr, avr)
DEF_HELPER_3(vsro, void, avr, avr, avr)
DEF_HELPER_3(vsrv, void, avr, avr, avr)
diff --git a/target/ppc/int_helper.c b/target/ppc/int_helper.c
index be53cd6f68..bd3e6d7cc7 100644
--- a/target/ppc/int_helper.c
+++ b/target/ppc/int_helper.c
@@ -523,19 +523,6 @@ void helper_vprtybq(ppc_avr_t *r, ppc_avr_t *b)
r->VsrD(0) = 0;
}
-#define VARITH_DO(name, op, element)\
-void helper_v##name(ppc_avr_t *r, ppc_avr_t *a, ppc_avr_t *b) \
-{ \
-int i; \
-\
-for (i = 0; i < ARRAY_SIZE(r->element); i++) { \
-r->element[i] = a->element[i] op b->element[i]; \
-} \
-}
-VARITH_DO(muluwm, *, u32)
-#undef VARITH_DO
-#undef VARITH
-
#define VARITHFP(suffix, func) \
void helper_v##suffix(CPUPPCState *env, ppc_avr_t *r, ppc_avr_t *a, \
ppc_avr_t *b) \
diff --git a/target/ppc/translate/vmx-impl.inc.c
b/target/ppc/translate/vmx-impl.inc.c
index 403ed3a01c..6e79ffa650 100644
--- a/target/ppc/translate/vmx-impl.inc.c
+++ b/target/ppc/translate/vmx-impl.inc.c
@@ -801,7 +801,7 @@ static void trans_vclzd(DisasContext *ctx)
GEN_VXFORM(vmuloub, 4, 0);
GEN_VXFORM(vmulouh, 4, 1);
GEN_VXFORM(vmulouw, 4, 2);
-GEN_VXFORM(vmuluwm, 4, 2);
+GEN_VXFORM_V(vmuluwm, MO_32, tcg_gen_gvec_mul, 4, 2);
GEN_VXFORM_DUAL(vmulouw, PPC_ALTIVEC, PPC_NONE,
vmuluwm, PPC_NONE, PPC2_ALTIVEC_207)
GEN_VXFORM(vmulosb, 4, 4);
--
2.23.0
[PATCH v4 07/11] target/ppc: add vmulld to INDEX_op_mul_vec case
Group vmuluwm and vmulld. Make vmulld-specific
changes since it belongs to new ISA 3.1.
Signed-off-by: Lijun Pan
---
v4: add missing changes, and split to 5/11, 6/11, 7/11
v3: use tcg_gen_gvec_mul()
v2: fix coding style
use Power ISA 3.1 flag
tcg/ppc/tcg-target.h | 2 ++
tcg/ppc/tcg-target.inc.c | 12 ++--
2 files changed, 12 insertions(+), 2 deletions(-)
diff --git a/tcg/ppc/tcg-target.h b/tcg/ppc/tcg-target.h
index 4fa21f0e71..ff1249ef8e 100644
--- a/tcg/ppc/tcg-target.h
+++ b/tcg/ppc/tcg-target.h
@@ -63,6 +63,7 @@ typedef enum {
tcg_isa_2_06,
tcg_isa_2_07,
tcg_isa_3_00,
+tcg_isa_3_10,
} TCGPowerISA;
extern TCGPowerISA have_isa;
@@ -72,6 +73,7 @@ extern bool have_vsx;
#define have_isa_2_06 (have_isa >= tcg_isa_2_06)
#define have_isa_2_07 (have_isa >= tcg_isa_2_07)
#define have_isa_3_00 (have_isa >= tcg_isa_3_00)
+#define have_isa_3_10 (have_isa >= tcg_isa_3_10)
/* optional instructions automatically implemented */
#define TCG_TARGET_HAS_ext8u_i320 /* andi */
diff --git a/tcg/ppc/tcg-target.inc.c b/tcg/ppc/tcg-target.inc.c
index ee1f9227c1..caa8985b46 100644
--- a/tcg/ppc/tcg-target.inc.c
+++ b/tcg/ppc/tcg-target.inc.c
@@ -564,6 +564,7 @@ static int tcg_target_const_match(tcg_target_long val,
TCGType type,
#define VMULOUHVX4(72)
#define VMULOUWVX4(136) /* v2.07 */
#define VMULUWMVX4(137) /* v2.07 */
+#define VMULLD VX4(457) /* v3.10 */
#define VMSUMUHM VX4(38)
#define VMRGHB VX4(12)
@@ -3015,6 +3016,8 @@ int tcg_can_emit_vec_op(TCGOpcode opc, TCGType type,
unsigned vece)
return -1;
case MO_32:
return have_isa_2_07 ? 1 : -1;
+case MO_64:
+return have_isa_3_10;
}
return 0;
case INDEX_op_bitsel_vec:
@@ -3149,6 +3152,7 @@ static void tcg_out_vec_op(TCGContext *s, TCGOpcode opc,
static const uint32_t
add_op[4] = { VADDUBM, VADDUHM, VADDUWM, VADDUDM },
sub_op[4] = { VSUBUBM, VSUBUHM, VSUBUWM, VSUBUDM },
+mul_op[4] = { 0, 0, VMULUWM, VMULLD },
neg_op[4] = { 0, 0, VNEGW, VNEGD },
eq_op[4] = { VCMPEQUB, VCMPEQUH, VCMPEQUW, VCMPEQUD },
ne_op[4] = { VCMPNEB, VCMPNEH, VCMPNEW, 0 },
@@ -3199,8 +3203,7 @@ static void tcg_out_vec_op(TCGContext *s, TCGOpcode opc,
a1 = 0;
break;
case INDEX_op_mul_vec:
-tcg_debug_assert(vece == MO_32 && have_isa_2_07);
-insn = VMULUWM;
+insn = mul_op[vece];
break;
case INDEX_op_ssadd_vec:
insn = ssadd_op[vece];
@@ -3709,6 +3712,11 @@ static void tcg_target_init(TCGContext *s)
have_isa = tcg_isa_3_00;
}
#endif
+#ifdef PPC_FEATURE2_ARCH_3_10
+if (hwcap2 & PPC_FEATURE2_ARCH_3_10) {
+have_isa = tcg_isa_3_10;
+}
+#endif
#ifdef PPC_FEATURE2_HAS_ISEL
/* Prefer explicit instruction from the kernel. */
--
2.23.0
[PATCH v4 00/11] Add several Power ISA 3.1 32/64-bit vector instructions
This patch series add several newly introduced 32/64-bit vector
instructions in Power ISA 3.1. Power ISA 3.1 flag is introduced in
this version. In v4 version, coding style issues are fixed, community
reviews/suggestions are taken into consideration.
Lijun Pan (11):
target/ppc: Introduce Power ISA 3.1 flag
target/ppc: Enable Power ISA 3.1
target/ppc: add byte-reverse br[dwh] instructions
target/ppc: convert vmuluwm to tcg_gen_gvec_mul
target/ppc: add vmulld instruction
Update PowerPC AT_HWCAP2 definition
target/ppc: add vmulld to INDEX_op_mul_vec case
target/ppc: add vmulh{su}w instructions
fix the prototype of muls64/mulu64
target/ppc: add vmulh{su}d instructions
target/ppc: add vdiv{su}{wd} vmod{su}{wd} instructions
include/elf.h | 1 +
include/qemu/host-utils.h | 4 +-
target/ppc/cpu.h| 4 +-
target/ppc/helper.h | 13 -
target/ppc/int_helper.c | 75 -
target/ppc/translate.c | 43 +
target/ppc/translate/vmx-impl.inc.c | 26 +-
target/ppc/translate/vmx-ops.inc.c | 27 +--
target/ppc/translate_init.inc.c | 2 +-
tcg/ppc/tcg-target.h| 2 +
tcg/ppc/tcg-target.inc.c| 12 -
11 files changed, 184 insertions(+), 25 deletions(-)
--
2.23.0
[PATCH v4 08/11] target/ppc: add vmulh{su}w instructions
vmulhsw: Vector Multiply High Signed Word
vmulhuw: Vector Multiply High Unsigned Word
Signed-off-by: Lijun Pan
---
Reviewed-by: Richard Henderson
v3: inline the helper_vmulh{su}w multiply directly instead of using macro
v2: fix coding style
use Power ISA 3.1 flag
target/ppc/helper.h | 2 ++
target/ppc/int_helper.c | 19 +++
target/ppc/translate/vmx-impl.inc.c | 6 ++
target/ppc/translate/vmx-ops.inc.c | 4 ++--
4 files changed, 29 insertions(+), 2 deletions(-)
diff --git a/target/ppc/helper.h b/target/ppc/helper.h
index 69416b6d7c..3b3013866a 100644
--- a/target/ppc/helper.h
+++ b/target/ppc/helper.h
@@ -184,6 +184,8 @@ DEF_HELPER_3(vmulosw, void, avr, avr, avr)
DEF_HELPER_3(vmuloub, void, avr, avr, avr)
DEF_HELPER_3(vmulouh, void, avr, avr, avr)
DEF_HELPER_3(vmulouw, void, avr, avr, avr)
+DEF_HELPER_3(vmulhsw, void, avr, avr, avr)
+DEF_HELPER_3(vmulhuw, void, avr, avr, avr)
DEF_HELPER_3(vslo, void, avr, avr, avr)
DEF_HELPER_3(vsro, void, avr, avr, avr)
DEF_HELPER_3(vsrv, void, avr, avr, avr)
diff --git a/target/ppc/int_helper.c b/target/ppc/int_helper.c
index bd3e6d7cc7..a3a20821fc 100644
--- a/target/ppc/int_helper.c
+++ b/target/ppc/int_helper.c
@@ -1086,6 +1086,25 @@ VMUL(uw, u32, VsrW, VsrD, uint64_t)
#undef VMUL_DO_ODD
#undef VMUL
+void helper_vmulhsw(ppc_avr_t *r, ppc_avr_t *a, ppc_avr_t *b)
+{
+int i;
+
+for (i = 0; i < 4; i++) {
+r->s32[i] = (int32_t)(((int64_t)a->s32[i] * (int64_t)b->s32[i]) >> 32);
+}
+}
+
+void helper_vmulhuw(ppc_avr_t *r, ppc_avr_t *a, ppc_avr_t *b)
+{
+int i;
+
+for (i = 0; i < 4; i++) {
+r->u32[i] = (uint32_t)(((uint64_t)a->u32[i] *
+ (uint64_t)b->u32[i]) >> 32);
+}
+}
+
void helper_vperm(CPUPPCState *env, ppc_avr_t *r, ppc_avr_t *a, ppc_avr_t *b,
ppc_avr_t *c)
{
diff --git a/target/ppc/translate/vmx-impl.inc.c
b/target/ppc/translate/vmx-impl.inc.c
index 8c89738552..50bac375fc 100644
--- a/target/ppc/translate/vmx-impl.inc.c
+++ b/target/ppc/translate/vmx-impl.inc.c
@@ -811,9 +811,15 @@ GEN_VXFORM_V(vmulld, MO_64, tcg_gen_gvec_mul, 4, 7);
GEN_VXFORM(vmuleub, 4, 8);
GEN_VXFORM(vmuleuh, 4, 9);
GEN_VXFORM(vmuleuw, 4, 10);
+GEN_VXFORM(vmulhuw, 4, 10);
+GEN_VXFORM_DUAL(vmuleuw, PPC_ALTIVEC, PPC_NONE,
+vmulhuw, PPC_NONE, PPC2_ISA310);
GEN_VXFORM(vmulesb, 4, 12);
GEN_VXFORM(vmulesh, 4, 13);
GEN_VXFORM(vmulesw, 4, 14);
+GEN_VXFORM(vmulhsw, 4, 14);
+GEN_VXFORM_DUAL(vmulesw, PPC_ALTIVEC, PPC_NONE,
+vmulhsw, PPC_NONE, PPC2_ISA310);
GEN_VXFORM_V(vslb, MO_8, tcg_gen_gvec_shlv, 2, 4);
GEN_VXFORM_V(vslh, MO_16, tcg_gen_gvec_shlv, 2, 5);
GEN_VXFORM_V(vslw, MO_32, tcg_gen_gvec_shlv, 2, 6);
diff --git a/target/ppc/translate/vmx-ops.inc.c
b/target/ppc/translate/vmx-ops.inc.c
index b49787ac97..29701ad778 100644
--- a/target/ppc/translate/vmx-ops.inc.c
+++ b/target/ppc/translate/vmx-ops.inc.c
@@ -110,10 +110,10 @@ GEN_VXFORM_207(vmulosw, 4, 6),
GEN_VXFORM_310(vmulld, 4, 7),
GEN_VXFORM(vmuleub, 4, 8),
GEN_VXFORM(vmuleuh, 4, 9),
-GEN_VXFORM_207(vmuleuw, 4, 10),
+GEN_VXFORM_DUAL(vmuleuw, vmulhuw, 4, 10, PPC_ALTIVEC, PPC_NONE),
GEN_VXFORM(vmulesb, 4, 12),
GEN_VXFORM(vmulesh, 4, 13),
-GEN_VXFORM_207(vmulesw, 4, 14),
+GEN_VXFORM_DUAL(vmulesw, vmulhsw, 4, 14, PPC_ALTIVEC, PPC_NONE),
GEN_VXFORM(vslb, 2, 4),
GEN_VXFORM(vslh, 2, 5),
GEN_VXFORM_DUAL(vslw, vrlwnm, 2, 6, PPC_ALTIVEC, PPC_NONE),
--
2.23.0
[PATCH v4 02/11] target/ppc: Enable Power ISA 3.1
This patch enables the Power ISA 3.1 in QEMU.
Signed-off-by: Lijun Pan
---
v4: split to 01/11 and 02/11
v2: add Power ISA 3.1 flag
target/ppc/cpu.h| 2 +-
target/ppc/translate_init.inc.c | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/target/ppc/cpu.h b/target/ppc/cpu.h
index a5e9c08dcc..ebb5a0811a 100644
--- a/target/ppc/cpu.h
+++ b/target/ppc/cpu.h
@@ -2201,7 +2201,7 @@ enum {
PPC2_BCTAR_ISA207 | PPC2_LSQ_ISA207 | \
PPC2_ALTIVEC_207 | PPC2_ISA207S | PPC2_DFP | \
PPC2_FP_CVT_S64 | PPC2_TM | PPC2_PM_ISA206 | \
-PPC2_ISA300)
+PPC2_ISA300 | PPC2_ISA310)
};
/*/
diff --git a/target/ppc/translate_init.inc.c b/target/ppc/translate_init.inc.c
index 38cb773ab4..3f72310e60 100644
--- a/target/ppc/translate_init.inc.c
+++ b/target/ppc/translate_init.inc.c
@@ -9206,7 +9206,7 @@ POWERPC_FAMILY(POWER10)(ObjectClass *oc, void *data)
PPC2_FP_TST_ISA206 | PPC2_BCTAR_ISA207 |
PPC2_LSQ_ISA207 | PPC2_ALTIVEC_207 |
PPC2_ISA205 | PPC2_ISA207S | PPC2_FP_CVT_S64 |
-PPC2_TM | PPC2_ISA300 | PPC2_PRCNTL;
+PPC2_TM | PPC2_ISA300 | PPC2_PRCNTL | PPC2_ISA310;
pcc->msr_mask = (1ull << MSR_SF) |
(1ull << MSR_HV) |
(1ull << MSR_TM) |
--
2.23.0
[PATCH v4 06/11] Update PowerPC AT_HWCAP2 definition
Add PPC2_FEATURE2_ARCH_3_10 to the PowerPC AT_HWCAP2 definitions.
Signed-off-by: Lijun Pan
---
v4: add missing changes, and split to 5/11, 6/11, 7/11
v3: use tcg_gen_gvec_mul()
v2: fix coding style
use Power ISA 3.1 flag
include/elf.h | 1 +
1 file changed, 1 insertion(+)
diff --git a/include/elf.h b/include/elf.h
index 8fbfe60e09..1858b95acf 100644
--- a/include/elf.h
+++ b/include/elf.h
@@ -554,6 +554,7 @@ typedef struct {
#define PPC_FEATURE2_HTM_NOSC 0x0100
#define PPC_FEATURE2_ARCH_3_00 0x0080
#define PPC_FEATURE2_HAS_IEEE1280x0040
+#define PPC_FEATURE2_ARCH_3_10 0x0020
/* Bits present in AT_HWCAP for Sparc. */
--
2.23.0
[PATCH v4 01/11] target/ppc: Introduce Power ISA 3.1 flag
This flag will be used for Power10 instructions.
Signed-off-by: Lijun Pan
---
v4: split to 01/11 and 02/11
v2: add Power ISA 3.1 flag
target/ppc/cpu.h | 2 ++
1 file changed, 2 insertions(+)
diff --git a/target/ppc/cpu.h b/target/ppc/cpu.h
index 1988b436cb..a5e9c08dcc 100644
--- a/target/ppc/cpu.h
+++ b/target/ppc/cpu.h
@@ -2191,6 +2191,8 @@ enum {
PPC2_PM_ISA206 = 0x0004ULL,
/* POWER ISA 3.0 */
PPC2_ISA300= 0x0008ULL,
+/* POWER ISA 3.1 */
+PPC2_ISA310= 0x0010ULL,
#define PPC_TCG_INSNS2 (PPC2_BOOKE206 | PPC2_VSX | PPC2_PRCNTL | PPC2_DBRX | \
PPC2_ISA205 | PPC2_VSX207 | PPC2_PERM_ISA206 | \
--
2.23.0
Re: [PATCH v3 3/4] hw/block/nvme: Fix pmrmsc register size
On 6/30/20 9:45 AM, Klaus Jensen wrote:
> On Jun 30 17:16, Philippe Mathieu-Daudé wrote:
>> On 6/30/20 5:10 PM, Andrzej Jakowski wrote:
>>> On 6/30/20 4:04 AM, Philippe Mathieu-Daudé wrote:
The Persistent Memory Region Controller Memory Space Control
register is 64-bit wide. See 'Figure 68: Register Definition'
of the 'NVM Express Base Specification Revision 1.4'.
Fixes: 6cf9413229 ("introduce PMR support from NVMe 1.4 spec")
Reported-by: Klaus Jensen
Reviewed-by: Klaus Jensen
Signed-off-by: Philippe Mathieu-Daudé
---
Cc: Andrzej Jakowski
---
include/block/nvme.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/include/block/nvme.h b/include/block/nvme.h
index 71c5681912..82c384614a 100644
--- a/include/block/nvme.h
+++ b/include/block/nvme.h
@@ -21,7 +21,7 @@ typedef struct QEMU_PACKED NvmeBar {
uint32_tpmrsts;
uint32_tpmrebs;
uint32_tpmrswtp;
-uint32_tpmrmsc;
+uint64_tpmrmsc;
} NvmeBar;
enum NvmeCapShift {
-- 2.21.3
>>>
>>> This is good catch, though I wanted to highlight that this will still
>>> need to change as this register is not aligned properly and thus not in
>>> compliance with spec.
>>
>> I was wondering the odd alignment too. So you are saying at some time
>> in the future the spec will be updated to correct the alignment?
Yep I think so.
So PMRMSC currently is 64-bit register that is defined at E14h offset.
It is in conflict with spec because spec requires 64-bit registers to
be 64-bit aligned.
I anticipate that changes will be needed.
>>
>> Should we use this instead?
>>
>> uint32_tpmrmsc;
>> +uint32_tpmrmsc_upper32; /* the spec define this, but *
>> + * only low 32-bit are used */
>>
>> Or eventually an unnamed struct:
>>
>> -uint32_tpmrmsc;
>> +struct {
>> +uint32_t pmrmsc;
>> +uint32_t pmrmsc_upper32; /* the spec define this, but *
>> + * only low 32-bit are used */
>> +};
>>
>>>
>>> Reviewed-by Andrzej Jakowski
>>>
>>
>
> I'm also not sure what you mean Andrzej. The odd alignment is exactly
> what the spec (v1.4) says?
>
Re: [PATCH] block: Raise an error when backing file parameter is an empty string
Hi Kevin & Max,
Just pinging this patch for your consideration.
Thank you,
Connor
On 6/17/20 11:27 AM, Connor Kuehl wrote:
Providing an empty string for the backing file parameter like so:
qemu-img create -f qcow2 -b '' /tmp/foo
allows the flow of control to reach and subsequently fail an assert
statement because passing an empty string to
bdrv_get_full_backing_filename_from_filename()
simply results in NULL being returned without an error being raised.
To fix this, let's check for an empty string when getting the value from
the opts list.
Reported-by: Attila Fazekas
Fixes: https://bugzilla.redhat.com/1809553
Signed-off-by: Connor Kuehl
---
block.c| 4
tests/qemu-iotests/298 | 47 ++
tests/qemu-iotests/298.out | 5
tests/qemu-iotests/group | 1 +
4 files changed, 57 insertions(+)
create mode 100755 tests/qemu-iotests/298
create mode 100644 tests/qemu-iotests/298.out
diff --git a/block.c b/block.c
index 6dbcb7e083..b335d6bcb2 100644
--- a/block.c
+++ b/block.c
@@ -6116,6 +6116,10 @@ void bdrv_img_create(const char *filename, const char
*fmt,
"same filename as the backing file");
goto out;
}
+if (backing_file[0] == '\0') {
+error_setg(errp, "Expected backing file name, got empty string");
+goto out;
+}
}
backing_fmt = qemu_opt_get(opts, BLOCK_OPT_BACKING_FMT);
diff --git a/tests/qemu-iotests/298 b/tests/qemu-iotests/298
new file mode 100755
index 00..1e30caebec
--- /dev/null
+++ b/tests/qemu-iotests/298
@@ -0,0 +1,47 @@
+#!/usr/bin/env python3
+#
+# Copyright (C) 2020 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] target/arm: Treat unknown SMC calls as NOP
On 01.07.20 22:47, Peter Maydell wrote:
> On Wed, 1 Jul 2020 at 21:08, Alexander Graf wrote:
>> We currently treat unknown SMC calls as UNDEF. This behavior is different
>> from KVM, which treats them as NOP.
>>
>> Unfortunately, the UNDEF exception breaks running Windows for ARM in QEMU,
>> as that probes an OEM SMCCC call on boot, but does not expect to receive
>> an UNDEF exception as response.
>>
>> So instead, let's follow the KVM path and ignore SMC calls that we don't
>> handle. This fixes booting the Windows 10 for ARM preview in TCG for me.
>>
>> Signed-off-by: Alexander Graf
>> +if (cs->exception_index == EXCP_SMC &&
>> +!arm_feature(env, ARM_FEATURE_EL3) &&
>> +cpu->psci_conduit != QEMU_PSCI_CONDUIT_SMC) {
> This condition says: "we got an SMC, and this CPU doesn't
> have EL3, and we're not imitating real EL3 firmware".
I like to think of it as "This CPU exposes an environment that looks
like KVM, so it implements HVC calls (EL2) and is responsible for
handling SMC calls as well.
The main difference between the two semantics is that in yours, you
don't have EL3. In mine, there is an EL3, but it's virtualized by the
same layer that implements EL2.
> The architecturally correct behaviour here (since we don't
> implement nested-virt yet, which might allow it to trap
> to guest EL2) is to UNDEF, as far as I can see from a quick
> look at the AArch64.CheckForSMCUndefOrTrap().
>
> I'm not sure why KVM makes these NOP; if I'm right about the
> architectural behaviour then making them NOP would be a KVM bug.
>
> If Windows makes an SMC call on boot that seems like a guest
> bug: it would crash on a real CPU without EL2/EL3 as well.
I don't think there can be a real SBBR compatible CPU without EL2/EL3,
because PSCI is a base requirement. That means either SMC calls succeed
(Windows running in EL2) or SMC calls are trapped into EL2 and it's up
to the hypervisor to decide what to do with them.
>
> * Conduit SMC, valid call Trap to EL2 PSCI Call
> * Conduit SMC, inval call Trap to EL2 Undef insn
> - * Conduit not SMC Undef insn Undef insn
> + * Conduit not SMC nop nop
>
> The line in this table that your commit message says you're
> fixing is "Conduit SMC, inval call"; the line your code
> change affects is "Conduit not SMC", which is not the same
> thing. (I'd have to look at the PSCI spec to see what it
> requires for SMCs that aren't valid PSCI calls.)
The patch fixes the default environment, which is "Conduit HVC, PSCI
over HVC implemented by QEMU". If the patch description wasn't clear,
I'm happy to reword it :).
Alex
[PATCH v4 1/2] nvme: indicate CMB support through controller capabilities register
This patch sets CMBS bit in controller capabilities register when user
configures NVMe driver with CMB support, so capabilites are correctly
reported to guest OS.
Signed-off-by: Andrzej Jakowski
Reviewed-by: Klaus Jensen
---
hw/block/nvme.c | 2 +-
include/block/nvme.h | 6 +-
2 files changed, 6 insertions(+), 2 deletions(-)
diff --git a/hw/block/nvme.c b/hw/block/nvme.c
index 1aee042d4c..9f11f3e9da 100644
--- a/hw/block/nvme.c
+++ b/hw/block/nvme.c
@@ -1582,6 +1582,7 @@ static void nvme_init_ctrl(NvmeCtrl *n, PCIDevice
*pci_dev)
NVME_CAP_SET_TO(n->bar.cap, 0xf);
NVME_CAP_SET_CSS(n->bar.cap, 1);
NVME_CAP_SET_MPSMAX(n->bar.cap, 4);
+NVME_CAP_SET_CMBS(n->bar.cap, n->params.cmb_size_mb ? 1 : 0);
n->bar.vs = 0x00010200;
n->bar.intmc = n->bar.intms = 0;
@@ -1591,7 +1592,6 @@ static void nvme_realize(PCIDevice *pci_dev, Error **errp)
{
NvmeCtrl *n = NVME(pci_dev);
Error *local_err = NULL;
-
int i;
nvme_check_constraints(n, _err);
diff --git a/include/block/nvme.h b/include/block/nvme.h
index 1720ee1d51..14cf398dfa 100644
--- a/include/block/nvme.h
+++ b/include/block/nvme.h
@@ -35,6 +35,7 @@ enum NvmeCapShift {
CAP_MPSMIN_SHIFT = 48,
CAP_MPSMAX_SHIFT = 52,
CAP_PMR_SHIFT = 56,
+CAP_CMB_SHIFT = 57,
};
enum NvmeCapMask {
@@ -48,6 +49,7 @@ enum NvmeCapMask {
CAP_MPSMIN_MASK= 0xf,
CAP_MPSMAX_MASK= 0xf,
CAP_PMR_MASK = 0x1,
+CAP_CMB_MASK = 0x1,
};
#define NVME_CAP_MQES(cap) (((cap) >> CAP_MQES_SHIFT) & CAP_MQES_MASK)
@@ -78,8 +80,10 @@ enum NvmeCapMask {
<< CAP_MPSMIN_SHIFT)
#define NVME_CAP_SET_MPSMAX(cap, val) (cap |= (uint64_t)(val &
CAP_MPSMAX_MASK)\
<<
CAP_MPSMAX_SHIFT)
-#define NVME_CAP_SET_PMRS(cap, val) (cap |= (uint64_t)(val & CAP_PMR_MASK)\
+#define NVME_CAP_SET_PMRS(cap, val) (cap |= (uint64_t)(val & CAP_PMR_MASK)
\
<< CAP_PMR_SHIFT)
+#define NVME_CAP_SET_CMBS(cap, val) (cap |= (uint64_t)(val & CAP_CMB_MASK)
\
+ << CAP_CMB_SHIFT)
enum NvmeCcShift {
CC_EN_SHIFT = 0,
--
2.21.1
[PATCH v4 2/2] nvme: allow cmb and pmr to be enabled on same device
So far it was not possible to have CMB and PMR emulated on the same
device, because BAR2 was used exclusively either of PMR or CMB. This
patch places CMB at BAR4 offset so it not conflicts with MSI-X vectors.
Signed-off-by: Andrzej Jakowski
---
hw/block/nvme.c | 101 +--
hw/block/nvme.h | 1 +
include/block/nvme.h | 4 +-
3 files changed, 72 insertions(+), 34 deletions(-)
diff --git a/hw/block/nvme.c b/hw/block/nvme.c
index 9f11f3e9da..f5156bcfe5 100644
--- a/hw/block/nvme.c
+++ b/hw/block/nvme.c
@@ -22,12 +22,12 @@
* [pmrdev=,] \
* max_ioqpairs=
*
- * Note cmb_size_mb denotes size of CMB in MB. CMB is assumed to be at
- * offset 0 in BAR2 and supports only WDS, RDS and SQS for now.
+ * Note cmb_size_mb denotes size of CMB in MB. CMB when configured is assumed
+ * to be resident in BAR4 at certain offset - this is because BAR4 is also
+ * used for storing MSI-X table that is available at offset 0 in BAR4.
*
- * cmb_size_mb= and pmrdev= options are mutually exclusive due to limitation
- * in available BAR's. cmb_size_mb= will take precedence over pmrdev= when
- * both provided.
+ * pmrdev is assumed to be resident in BAR2/BAR3. When configured it consumes
+ * whole BAR2/BAR3 exclusively.
* Enabling pmr emulation can be achieved by pointing to memory-backend-file.
* For example:
* -object memory-backend-file,id=,share=on,mem-path=, \
@@ -57,8 +57,8 @@
#define NVME_MAX_IOQPAIRS 0x
#define NVME_REG_SIZE 0x1000
#define NVME_DB_SIZE 4
-#define NVME_CMB_BIR 2
#define NVME_PMR_BIR 2
+#define NVME_MSIX_BIR 4
#define NVME_GUEST_ERR(trace, fmt, ...) \
do { \
@@ -1395,7 +1395,7 @@ static void nvme_check_constraints(NvmeCtrl *n, Error
**errp)
return;
}
-if (!n->params.cmb_size_mb && n->pmrdev) {
+if (n->pmrdev) {
if (host_memory_backend_is_mapped(n->pmrdev)) {
char *path =
object_get_canonical_path_component(OBJECT(n->pmrdev));
error_setg(errp, "can't use already busy memdev: %s", path);
@@ -1453,33 +1453,70 @@ static void nvme_init_namespace(NvmeCtrl *n,
NvmeNamespace *ns, Error **errp)
id_ns->nuse = id_ns->ncap;
}
-static void nvme_init_cmb(NvmeCtrl *n, PCIDevice *pci_dev)
+static void nvme_bar4_init(PCIDevice *pci_dev, Error **errp)
{
-NVME_CMBLOC_SET_BIR(n->bar.cmbloc, NVME_CMB_BIR);
-NVME_CMBLOC_SET_OFST(n->bar.cmbloc, 0);
-
-NVME_CMBSZ_SET_SQS(n->bar.cmbsz, 1);
-NVME_CMBSZ_SET_CQS(n->bar.cmbsz, 0);
-NVME_CMBSZ_SET_LISTS(n->bar.cmbsz, 0);
-NVME_CMBSZ_SET_RDS(n->bar.cmbsz, 1);
-NVME_CMBSZ_SET_WDS(n->bar.cmbsz, 1);
-NVME_CMBSZ_SET_SZU(n->bar.cmbsz, 2); /* MBs */
-NVME_CMBSZ_SET_SZ(n->bar.cmbsz, n->params.cmb_size_mb);
-
-n->cmbuf = g_malloc0(NVME_CMBSZ_GETSIZE(n->bar.cmbsz));
-memory_region_init_io(>ctrl_mem, OBJECT(n), _cmb_ops, n,
- "nvme-cmb", NVME_CMBSZ_GETSIZE(n->bar.cmbsz));
-pci_register_bar(pci_dev, NVME_CMBLOC_BIR(n->bar.cmbloc),
+NvmeCtrl *n = NVME(pci_dev);
+int status;
+uint64_t bar_size, cmb_offset = 0;
+uint32_t msix_vectors;
+uint32_t nvme_pba_offset;
+uint32_t cmb_size_units;
+
+msix_vectors = n->params.max_ioqpairs + 1;
+nvme_pba_offset = PCI_MSIX_ENTRY_SIZE * msix_vectors;
+bar_size = nvme_pba_offset + QEMU_ALIGN_UP(msix_vectors, 64) / 8;
+
+if (n->params.cmb_size_mb) {
+NVME_CMBSZ_SET_SQS(n->bar.cmbsz, 1);
+NVME_CMBSZ_SET_CQS(n->bar.cmbsz, 0);
+NVME_CMBSZ_SET_LISTS(n->bar.cmbsz, 0);
+NVME_CMBSZ_SET_RDS(n->bar.cmbsz, 1);
+NVME_CMBSZ_SET_WDS(n->bar.cmbsz, 1);
+NVME_CMBSZ_SET_SZU(n->bar.cmbsz, 2); /* MBs */
+NVME_CMBSZ_SET_SZ(n->bar.cmbsz, n->params.cmb_size_mb);
+
+cmb_size_units = NVME_CMBSZ_GETSIZEUNITS(n->bar.cmbsz);
+cmb_offset = QEMU_ALIGN_UP(bar_size, cmb_size_units);
+
+NVME_CMBLOC_SET_BIR(n->bar.cmbloc, NVME_MSIX_BIR);
+NVME_CMBLOC_SET_OFST(n->bar.cmbloc, cmb_offset / cmb_size_units);
+
+n->cmbuf = g_malloc0(NVME_CMBSZ_GETSIZE(n->bar.cmbsz));
+
+bar_size += cmb_offset;
+bar_size += NVME_CMBSZ_GETSIZE(n->bar.cmbsz);
+}
+
+bar_size = pow2ceil(bar_size);
+
+/* Create memory region for BAR4, then overlap cmb, msix and pba
+ * tables on top of it.*/
+memory_region_init(>bar4, OBJECT(n), "nvme-bar4", bar_size);
+
+if (n->params.cmb_size_mb) {
+memory_region_init_io(>ctrl_mem, OBJECT(n), _cmb_ops, n,
+ "nvme-cmb", NVME_CMBSZ_GETSIZE(n->bar.cmbsz));
+
+memory_region_add_subregion(>bar4, cmb_offset, >ctrl_mem);
+}
+
+status = msix_init(pci_dev, n->params.msix_qsize,
+ >bar4, NVME_MSIX_BIR, 0,
+ >bar4, NVME_MSIX_BIR, nvme_pba_offset,
+ 0, errp);
+
+if (status) {
+return;
+}
+
+pci_register_bar(pci_dev, NVME_MSIX_BIR,
[PATCH v4] nvme: allow cmb and pmr emulation on same device
Hi All, Resending series recently posted on mailing list related to nvme device extension with couple of fixes after review. This patch series does following: - Fixes problem where CMBS bit was not set in controller capabilities register, so support for CMB was not correctly advertised to guest. This is resend of patch that has been submitted and reviewed by Klaus [1] - Introduces BAR4 sharing between MSI-X vectors and CMB. This allows to have CMB and PMR emulated on the same device. This extension was indicated by Keith [2] v4: - modified BAR4 initialization, so now it consists of CMB, MSIX and PBA memory regions overlapping on top of it. This reduces patch complexity significantly (Klaus [5]) v3: - code style fixes including: removal of spurious line break, moving define into define section and code alignment (Klaus [4]) - removed unintended code reintroduction (Klaus [4]) v2: - rebase on Kevin's latest block branch (Klaus [3]) - improved comments section (Klaus [3]) - simplified calculation of BAR4 size (Klaus [3]) v1: - initial push of the patch [1]: https://lore.kernel.org/qemu-devel/20200408055607.g2ii7gwqbnv6cd3w@apples.localdomain/ [2]: https://lore.kernel.org/qemu-devel/20200330165518.ga8...@redsun51.ssa.fujisawa.hgst.com/ [3]: https://lore.kernel.org/qemu-devel/20200605181043.28782-1-andrzej.jakow...@linux.intel.com/ [4]: https://lore.kernel.org/qemu-devel/20200618092524.posxi5mysb3jjtpn@apples.localdomain/ [5]: https://lore.kernel.org/qemu-devel/20200626055033.6vxqvi4s5pll7som@apples.localdomain/
Re: [PATCH v2 2/2] MAINTAINERS: Adjust MIPS maintainership
On Wednesday, July 1, 2020, Aurelien Jarno wrote: > Hi Aleksandar, > > I know you have sent a v3 of this patch in the meantime, but I would > still like to comment on it. > > First of all I confirm, that I do not have time to contribute to QEMU > anymore and that I said I would like to resign from QEMU maintainership. > We discussed that in person in Lyon back in October. I am happy to see > that people are interested to take the maintainership. > > It happens that I known Huacai Chen from the time he was upstreaming the > Loongson 3 support to the kernel, I have been testing and reviewing his > patches. I also know Jiaxun Yang from the #debian-mips IRC channel. I > know that they are both very competent and have a good knowledge of the > open source world. I therefore agree that they are good additions to > maintain and/or review the MIPS part of QEMU. > > All that said, they have been contributing only relatively recently to > QEMU, and only to some areas. While I do support adding them to those > areas, I wonder for example why adding them to the MIPS TCG target or > to the Jazz board. That's probably something that should be done in a > second step if they have an interest in doing so. > > I am fully aware that you posted a v3, that fixes exactly that after you > got pointed out. However, I am still concerned by the way things > happened. > > Sure, I understand your point. I am glad you respond in a reasonable, civilized, way, about a reasonable topic, and I find talking to you a very pleasant experience - even if we disagree, it doesn't make a difference at all. The point of the segments you described is: You have to throw somebody in unkown water, on purpose, in order to teach him how to swim in the best way, and get out of him the best, his best. That is all what I meant. No ulterior motive whatsoever. Somebody like it, somebody does not. Huacai doesn't, and I respect it, as shown in v3. If you were here, all will be easier - we could talk, consult each other, advice each other. I would enjoy working with you. I have also strong feeling we would find common interest outside technical world, probably in various intelectual realms. Perhaps this is not known to you, but french thinkers are much appreciated and understood here in Serbia. We are frankofilles. But you choose not to be here. And what can I do? I can't force you to be somewhere you don't want to be. I felt the loss of you not being here throughout all my online presence. Mixed feelings, ha? Truly yours, Aleksandar > Regards, > Aurelien > > > On 2020-06-30 18:46, Aleksandar Markovic wrote: > > Paul Burton and Aurelien Jarno removed for not being present. > > > > Huacai Chen and Jiaxun Yang step in as new energy. > > > > CC: Paul Burton > > CC: Aurelien Jarno > > Signed-off-by: Aleksandar Markovic > > --- > > MAINTAINERS | 15 ++- > > 1 file changed, 10 insertions(+), 5 deletions(-) > > > > diff --git a/MAINTAINERS b/MAINTAINERS > > index 5d8acf8d31..7fc16e21c9 100644 > > --- a/MAINTAINERS > > +++ b/MAINTAINERS > > @@ -213,7 +213,8 @@ F: disas/microblaze.c > > > > MIPS TCG CPUs > > M: Aleksandar Markovic > > -R: Aurelien Jarno > > +M: Huacai Chen > > +R: Jiaxun Yang > > R: Aleksandar Rikalo > > S: Maintained > > F: target/mips/ > > @@ -377,6 +378,7 @@ F: target/arm/kvm.c > > > > MIPS KVM CPUs > > M: Aleksandar Markovic > > +M: Huacai Chen > > S: Odd Fixes > > F: target/mips/kvm.c > > > > @@ -1052,6 +1054,7 @@ MIPS Machines > > - > > Jazz > > M: Hervé Poussineau > > +M: Huacai Chen > > R: Aleksandar Rikalo > > S: Maintained > > F: hw/mips/jazz.c > > @@ -1060,8 +1063,8 @@ F: hw/dma/rc4030.c > > > > Malta > > M: Aleksandar Markovic > > +M: Huacai Chen > > M: Philippe Mathieu-Daudé > > -R: Aurelien Jarno > > S: Maintained > > F: hw/isa/piix4.c > > F: hw/acpi/piix4.c > > @@ -1073,6 +1076,7 @@ F: tests/acceptance/machine_mips_malta.py > > > > Mipssim > > M: Aleksandar Markovic > > +M: Huacai Chen > > R: Aleksandar Rikalo > > S: Odd Fixes > > F: hw/mips/mipssim.c > > @@ -1080,7 +1084,6 @@ F: hw/net/mipsnet.c > > > > R4000 > > M: Aleksandar Markovic > > -R: Aurelien Jarno > > R: Aleksandar Rikalo > > S: Obsolete > > F: hw/mips/r4k.c > > @@ -1103,7 +1106,8 @@ S: Maintained > > F: hw/intc/loongson_liointc.c > > > > Boston > > -M: Paul Burton > > +M: Aleksandar Markovic > > +M: Huacai Chen > > R: Aleksandar Rikalo > > S: Maintained > > F: hw/core/loader-fit.c > > @@ -2677,7 +2681,8 @@ F: disas/i386.c > > > > MIPS TCG target > > M: Aleksandar Markovic > > -R: Aurelien Jarno > > +M: Huacai Chen > > +R: Jiaxun Yang > > R: Aleksandar Rikalo > > S: Maintained > > F: tcg/mips/ > > -- > > 2.20.1 > > > > > > > > -- > Aurelien Jarno GPG: 4096R/1DDD8C9B > aurel...@aurel32.net http://www.aurel32.net >
Re: [PATCH v3 0/5] hw/mips/malta: Add the 'malta-strict' machine, matching Malta hardware
Aleksandar, On 2020-07-01 20:51, Aleksandar Markovic wrote: > On Wed, Jul 1, 2020 at 7:39 PM Aurelien Jarno wrote: > > > > Aleksandar, > > > > On 2020-06-30 23:54, Aleksandar Markovic wrote: > > > As, in a very clear way, evidenced from the previous versions of this > > > series, this series real goal was not not to create some new > > > "malta-strict" machine, but to prepare path to creation of some > > > imagined "malta-unleashed" machine which will, to the contrary of > > > proclaimed goal, create a machine that could never exist in reality. > > > That is why I can't accept this series. > > > > I do not understand your opposition to this, and why it is an issue to > > support more than 2GiB of RAM for such a board. Supporting more than 2GiB > > of memory doesn't prevent people to emulate a real Malta board with less > > memory. > > > > In addition to that, the Malta board in QEMU has been supporting for > > many years more than the maximum 256MiB that is possible on a physical > > board. The QEMU version also supports way more than CPU variants than > > the physical board. In other word the existing malta machine in QEMU is > > already a "malta-unleashed". > > > > Aurelien, > > Glad to see you again. I am really sorry you were absent for so long. I assumed that since haven't dramatically changes in QEMU since I left, however if I missed some recent discussions that goes again what I am saying below, please feel free to point me to them. > Those (what you described in the paragraphs above) were mistakes from > the past. At some point, we needed to stop doing it, and finally > returned to the root QEMU principles of emulating systems as > faithfully as possible. I think there is a big misunderstanding here. The root QEMU principle is to emulate each *device* or *feature* as faithfully as possible. The *default* system that is emulated should also match as much as possible the real hardware, but QEMU also gives users the possibility to create a system as they want. And the amount of memory is one them. That's actually all the beauty of QEMU. Remember that QEMU solely exists because it has users, and that the possibility to extend the RAM of the Malta board to 2GB and to select various CPUs is widely used by users. So overall there are plenty of counter examples to your "root QEMU principles". Daniel P. Berrangé mentioned the memory support on the i440fx x86 hardware. As other examples you can also add AMD 3D Now instructions to an Intel CPU, or add an AC97 sound device to a SH4 machine. Virtio is another example. > Knowing the needs like you described exist, my vision is that, just > for occasions you described, we create a virtual board that would have > very wide set of feature, unconstrained by real world. That way we > would avoid situations to "lie" in our emulations. Adding a "virt" machine like it has been done on some other architectures is probably a good idea to give users even more possibilities. Now I do not believe its a reason to not allow users to slightly extend an existing system. In addition to that, creating a new virt machine and getting it fully usable is a multi year project. In addition to the QEMU changes, you also need to get kernel and bootloader support. And then it has to reach the distributions. > If you needed something more that is currently provided, you should > have issued a feature request through regular channels, and that would > have the people the chance to develop a solid solution, not some quick > fixes that pushes us further and further in wring direction. QEMU doesn't have an upstream bug tracker, so I guess that regular channels basically mean the mailing list. I therefore express the need for a MIPS "virt" machine that supports more than 2GB. Please ping me when it's ready. Best regards, Aurelien > Why didn't you respond on my mail from the other day? Do you plan to respond? I just responded to it, overall in less than 12 hours. -- Aurelien Jarno GPG: 4096R/1DDD8C9B aurel...@aurel32.net http://www.aurel32.net
Re: [PATCH] target/arm: Treat unknown SMC calls as NOP
On Wed, 1 Jul 2020 at 21:08, Alexander Graf wrote:
>
> We currently treat unknown SMC calls as UNDEF. This behavior is different
> from KVM, which treats them as NOP.
>
> Unfortunately, the UNDEF exception breaks running Windows for ARM in QEMU,
> as that probes an OEM SMCCC call on boot, but does not expect to receive
> an UNDEF exception as response.
>
> So instead, let's follow the KVM path and ignore SMC calls that we don't
> handle. This fixes booting the Windows 10 for ARM preview in TCG for me.
>
> Signed-off-by: Alexander Graf
> +if (cs->exception_index == EXCP_SMC &&
> +!arm_feature(env, ARM_FEATURE_EL3) &&
> +cpu->psci_conduit != QEMU_PSCI_CONDUIT_SMC) {
This condition says: "we got an SMC, and this CPU doesn't
have EL3, and we're not imitating real EL3 firmware".
The architecturally correct behaviour here (since we don't
implement nested-virt yet, which might allow it to trap
to guest EL2) is to UNDEF, as far as I can see from a quick
look at the AArch64.CheckForSMCUndefOrTrap().
I'm not sure why KVM makes these NOP; if I'm right about the
architectural behaviour then making them NOP would be a KVM bug.
If Windows makes an SMC call on boot that seems like a guest
bug: it would crash on a real CPU without EL2/EL3 as well.
* Conduit SMC, valid call Trap to EL2 PSCI Call
* Conduit SMC, inval call Trap to EL2 Undef insn
- * Conduit not SMC Undef insn Undef insn
+ * Conduit not SMC nop nop
The line in this table that your commit message says you're
fixing is "Conduit SMC, inval call"; the line your code
change affects is "Conduit not SMC", which is not the same
thing. (I'd have to look at the PSCI spec to see what it
requires for SMCs that aren't valid PSCI calls.)
thanks
-- PMM
Re: [RFC PATCH] cpus: Initialize current_cpu with the first vCPU created
On Wed, 1 Jul 2020 at 19:21, Philippe Mathieu-Daudé wrote:
>
> We can run I/O access with the 'i' or 'o' HMP commands in the
> monitor. These commands are expected to run on a vCPU. The
> monitor is not a vCPU thread. To avoid crashing, initialize
> the 'current_cpu' variable with the first vCPU created. The
> command executed on the monitor will end using it.
>
> RFC because I believe the correct fix is to NOT use current_cpu
> out of cpus.c, at least use qemu_get_cpu(0) to get the first vCPU.
Yes, I agree -- I don't think this is the correct fix.
current_cpu is documented as "only valid inside cpu_exec()",
ie if you're actually on a vcpu thread you can use it, but if
you're not on a CPU thread, like the monitor, you need to
say which vCPU you want to affect. For the monitor, that
would be the current "default cpu" as set by the "cpu"
command (cf monitor_set_cpu()). The bug here will be that
somewhere along the line we are probably missing plumbing
sufficient to pass down "which CPU do we want".
https://bugs.launchpad.net/qemu/+bug/1602247 is a bug of
a similar nature -- if the gdbstub does a memory access
we know which CPU we're trying to do that memory access as,
but it doesn't get plumbed through and so in the Arm GIC
register read/write function that looks at current_cpu
we get a segfault.
One way to fix this would be to do something akin to how
real hardware works -- encode into the MemTxAttrs an
indication of what the transaction master was (eg the
CPU number for CPUs); then the handful of devices that
care about which CPU was doing the transaction can use
that, rather than directly looking at current_cpu, and
when gdbstub or monitor perform an access that should
act like it's from a particular CPU they can indicate
that by supplying appropriate transaction attributes.
That would potentially be quite a bit of work though
(but I think it would be a neat design if we want to
try to fix this kind of "segfault if the user prods
a device via the monitor" bug).
+if (!current_cpu) {
+current_cpu = cpu;
+}
Some more specific issues with this -- current_cpu is
a thread-local variable, so this will set that for
whatever thread happens to make this call, which
might or might not be the one that ends up handling
the monitor command. Also some code assumes that
current_cpu is non-NULL only if this is a vCPU thread,
eg sigbus_handler().
thanks
-- PMM
[PATCH v4 2/2] target/m68k: consolidate physical translation offset into get_physical_address()
Since all callers to get_physical_address() now apply the same page offset to
the translation result, move the logic into get_physical_address() itself to
avoid duplication.
Suggested-by: Philippe Mathieu-Daudé
Signed-off-by: Mark Cave-Ayland
---
target/m68k/helper.c | 17 ++---
1 file changed, 6 insertions(+), 11 deletions(-)
diff --git a/target/m68k/helper.c b/target/m68k/helper.c
index 631eab7774..3ff5765795 100644
--- a/target/m68k/helper.c
+++ b/target/m68k/helper.c
@@ -643,7 +643,7 @@ static int get_physical_address(CPUM68KState *env, hwaddr
*physical,
/* Transparent Translation Register bit */
env->mmu.mmusr = M68K_MMU_T_040 | M68K_MMU_R_040;
}
-*physical = address & TARGET_PAGE_MASK;
+*physical = address;
*page_size = TARGET_PAGE_SIZE;
return 0;
}
@@ -771,7 +771,7 @@ static int get_physical_address(CPUM68KState *env, hwaddr
*physical,
}
*page_size = 1 << page_bits;
page_mask = ~(*page_size - 1);
-*physical = next & page_mask;
+*physical = (next & page_mask) + (address & (*page_size - 1));
if (access_type & ACCESS_PTEST) {
env->mmu.mmusr |= next & M68K_MMU_SR_MASK_040;
@@ -826,8 +826,6 @@ hwaddr m68k_cpu_get_phys_page_debug(CPUState *cs, vaddr
addr)
return -1;
}
-addr &= TARGET_PAGE_MASK;
-phys_addr += addr & (page_size - 1);
return phys_addr;
}
@@ -891,10 +889,8 @@ bool m68k_cpu_tlb_fill(CPUState *cs, vaddr address, int
size,
ret = get_physical_address(>env, , ,
address, access_type, _size);
if (likely(ret == 0)) {
-address &= TARGET_PAGE_MASK;
-physical += address & (page_size - 1);
-tlb_set_page(cs, address, physical,
- prot, mmu_idx, TARGET_PAGE_SIZE);
+tlb_set_page(cs, address & TARGET_PAGE_MASK,
+ physical & TARGET_PAGE_MASK, prot, mmu_idx, page_size);
return true;
}
@@ -1383,9 +1379,8 @@ void HELPER(ptest)(CPUM68KState *env, uint32_t addr,
uint32_t is_read)
ret = get_physical_address(env, , , addr,
access_type, _size);
if (ret == 0) {
-addr &= TARGET_PAGE_MASK;
-physical += addr & (page_size - 1);
-tlb_set_page(env_cpu(env), addr, physical,
+tlb_set_page(env_cpu(env), addr & TARGET_PAGE_MASK,
+ physical & TARGET_PAGE_MASK,
prot, access_type & ACCESS_SUPER ?
MMU_KERNEL_IDX : MMU_USER_IDX, page_size);
}
--
2.20.1
[PATCH v4 1/2] target/m68k: fix physical address translation in m68k_cpu_get_phys_page_debug()
The result of the get_physical_address() function should be combined with the
offset of the original page access before being returned. Otherwise the
m68k_cpu_get_phys_page_debug() function can round to the wrong page causing
incorrect lookups in gdbstub and various "Disassembler disagrees with
translator over instruction decoding" warnings to appear at translation time.
Fixes: 88b2fef6c3 ("target/m68k: add MC68040 MMU")
Signed-off-by: Mark Cave-Ayland
Reviewed-by: Philippe Mathieu-Daudé
Reviewed-by: Laurent Vivier
---
target/m68k/helper.c | 4
1 file changed, 4 insertions(+)
diff --git a/target/m68k/helper.c b/target/m68k/helper.c
index 79b0b10ea9..631eab7774 100644
--- a/target/m68k/helper.c
+++ b/target/m68k/helper.c
@@ -820,10 +820,14 @@ hwaddr m68k_cpu_get_phys_page_debug(CPUState *cs, vaddr
addr)
if (env->sr & SR_S) {
access_type |= ACCESS_SUPER;
}
+
if (get_physical_address(env, _addr, ,
addr, access_type, _size) != 0) {
return -1;
}
+
+addr &= TARGET_PAGE_MASK;
+phys_addr += addr & (page_size - 1);
return phys_addr;
}
--
2.20.1
[PATCH v4 0/2] target/m68k: fix physical address translation in m68k_cpu_get_phys_page_debug()
The first patch in the series fixes the original bug, whilst the second patch implements the suggestion by Philippe to consolidate the translation offset logic into get_physical_address() itself now that all callers are identical. Signed-off-by: Mark Cave-Ayland v4: - Remove extra TARGET_PAGE_MASK when calculating translated address since whilst it was required when being done by the caller, it is already handled in get_physical_address() v3: - Fix Transparent Translation as indicated by Laurent - Always apply TARGET_PAGE_MASK to tlb_set_page() parameters v2: - Add R-B tags from Philippe and Laurent - Add patch 2 to consolidate the translation offset logic into get_physical_address() Mark Cave-Ayland (2): target/m68k: fix physical address translation in m68k_cpu_get_phys_page_debug() target/m68k: consolidate physical translation offset into get_physical_address() target/m68k/helper.c | 17 - 1 file changed, 8 insertions(+), 9 deletions(-) -- 2.20.1
Re: [PATCH v3 2/2] MAINTAINERS: Adjust MIPS maintainership
Hi Aleksandar, On Wed, Jul 1, 2020 at 11:26 AM Aleksandar Markovic wrote: > Paul Burton and Aurelien Jarno removed for not being present. > A polite email was sent to them with question whether they > intend to actively participate, but there was no response. It was 2 days ago, not 2 months :) I'm fine with being removed though - I no longer have access to modern MIPS CPUs or Boston hardware, and wouldn't currently have time to spend on them if I did. So as far as removing me goes: Acked-by: Paul Burton > In cases where needed, other persons step in instead. > > Huacai Chen and Jiaxun Yang step in as new energy. All the best with it moving forwards, and thanks to Huacai & Jiaxun for stepping up! Paul
Seeing a problem in multi cpu runs where memory mapped pcie device register reads are returning incorrect values
Background I have a test environment which runs QEMU 4.2 with a plugin that runs two copies of a PCIE device simulator on a CentOS 7.5 host with an Ubuntu 18.04 guest. When running with a single QEMU CPU using: -cpu kvm64,+lahf_lm -M q35,kernel-irqchip=off -device intel-iommu,intremap=on Our tests run fine. But when running with multiple cpu's: -cpu kvm64,+lahf_lm -M q35,kernel-irqchip=off -device intel-iommu,intremap=on -smp 2,sockets=1,cores=2 The values retuned are correct all the way up the call stack and in KVM_EXIT_MMIO in kvm_cpu_exec (qemu-4.2.0/accel/kvm/kvm-all.c:2365) but the value returned to the device driver which initiated the read is 0. Question Is anyone else running QEMU 4.2 in multi cpu mode? Is anyone getting incorrect reads from memory mapped device registers when running in this mode? I would appreciate any pointers on how best to debug the flow from KVM_EXIT_MMIO back to the device driver running on the guest
Re: [PATCH v2 2/2] MAINTAINERS: Adjust MIPS maintainership
Hi Aleksandar, I know you have sent a v3 of this patch in the meantime, but I would still like to comment on it. First of all I confirm, that I do not have time to contribute to QEMU anymore and that I said I would like to resign from QEMU maintainership. We discussed that in person in Lyon back in October. I am happy to see that people are interested to take the maintainership. It happens that I known Huacai Chen from the time he was upstreaming the Loongson 3 support to the kernel, I have been testing and reviewing his patches. I also know Jiaxun Yang from the #debian-mips IRC channel. I know that they are both very competent and have a good knowledge of the open source world. I therefore agree that they are good additions to maintain and/or review the MIPS part of QEMU. All that said, they have been contributing only relatively recently to QEMU, and only to some areas. While I do support adding them to those areas, I wonder for example why adding them to the MIPS TCG target or to the Jazz board. That's probably something that should be done in a second step if they have an interest in doing so. I am fully aware that you posted a v3, that fixes exactly that after you got pointed out. However, I am still concerned by the way things happened. Regards, Aurelien On 2020-06-30 18:46, Aleksandar Markovic wrote: > Paul Burton and Aurelien Jarno removed for not being present. > > Huacai Chen and Jiaxun Yang step in as new energy. > > CC: Paul Burton > CC: Aurelien Jarno > Signed-off-by: Aleksandar Markovic > --- > MAINTAINERS | 15 ++- > 1 file changed, 10 insertions(+), 5 deletions(-) > > diff --git a/MAINTAINERS b/MAINTAINERS > index 5d8acf8d31..7fc16e21c9 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -213,7 +213,8 @@ F: disas/microblaze.c > > MIPS TCG CPUs > M: Aleksandar Markovic > -R: Aurelien Jarno > +M: Huacai Chen > +R: Jiaxun Yang > R: Aleksandar Rikalo > S: Maintained > F: target/mips/ > @@ -377,6 +378,7 @@ F: target/arm/kvm.c > > MIPS KVM CPUs > M: Aleksandar Markovic > +M: Huacai Chen > S: Odd Fixes > F: target/mips/kvm.c > > @@ -1052,6 +1054,7 @@ MIPS Machines > - > Jazz > M: Hervé Poussineau > +M: Huacai Chen > R: Aleksandar Rikalo > S: Maintained > F: hw/mips/jazz.c > @@ -1060,8 +1063,8 @@ F: hw/dma/rc4030.c > > Malta > M: Aleksandar Markovic > +M: Huacai Chen > M: Philippe Mathieu-Daudé > -R: Aurelien Jarno > S: Maintained > F: hw/isa/piix4.c > F: hw/acpi/piix4.c > @@ -1073,6 +1076,7 @@ F: tests/acceptance/machine_mips_malta.py > > Mipssim > M: Aleksandar Markovic > +M: Huacai Chen > R: Aleksandar Rikalo > S: Odd Fixes > F: hw/mips/mipssim.c > @@ -1080,7 +1084,6 @@ F: hw/net/mipsnet.c > > R4000 > M: Aleksandar Markovic > -R: Aurelien Jarno > R: Aleksandar Rikalo > S: Obsolete > F: hw/mips/r4k.c > @@ -1103,7 +1106,8 @@ S: Maintained > F: hw/intc/loongson_liointc.c > > Boston > -M: Paul Burton > +M: Aleksandar Markovic > +M: Huacai Chen > R: Aleksandar Rikalo > S: Maintained > F: hw/core/loader-fit.c > @@ -2677,7 +2681,8 @@ F: disas/i386.c > > MIPS TCG target > M: Aleksandar Markovic > -R: Aurelien Jarno > +M: Huacai Chen > +R: Jiaxun Yang > R: Aleksandar Rikalo > S: Maintained > F: tcg/mips/ > -- > 2.20.1 > > > -- Aurelien Jarno GPG: 4096R/1DDD8C9B aurel...@aurel32.net http://www.aurel32.net
[PATCH] target/arm: Treat unknown SMC calls as NOP
We currently treat unknown SMC calls as UNDEF. This behavior is different
from KVM, which treats them as NOP.
Unfortunately, the UNDEF exception breaks running Windows for ARM in QEMU,
as that probes an OEM SMCCC call on boot, but does not expect to receive
an UNDEF exception as response.
So instead, let's follow the KVM path and ignore SMC calls that we don't
handle. This fixes booting the Windows 10 for ARM preview in TCG for me.
Signed-off-by: Alexander Graf
---
target/arm/helper.c| 8
target/arm/op_helper.c | 13 -
2 files changed, 12 insertions(+), 9 deletions(-)
diff --git a/target/arm/helper.c b/target/arm/helper.c
index dc9c29f998..bc1bd2e704 100644
--- a/target/arm/helper.c
+++ b/target/arm/helper.c
@@ -9778,6 +9778,14 @@ void arm_cpu_do_interrupt(CPUState *cs)
return;
}
+if (cs->exception_index == EXCP_SMC &&
+!arm_feature(env, ARM_FEATURE_EL3) &&
+cpu->psci_conduit != QEMU_PSCI_CONDUIT_SMC) {
+/* Treat unknown SMC calls as NOP, just like KVM */
+qemu_log_mask(CPU_LOG_INT, "...handled as NOP\n");
+return;
+}
+
/*
* Semihosting semantics depend on the register width of the code
* that caused the exception, not the target exception level, so
diff --git a/target/arm/op_helper.c b/target/arm/op_helper.c
index b1065216b2..42b1687860 100644
--- a/target/arm/op_helper.c
+++ b/target/arm/op_helper.c
@@ -823,7 +823,7 @@ void HELPER(pre_smc)(CPUARMState *env, uint32_t syndrome)
*
* Conduit SMC, valid call Trap to EL2 PSCI Call
* Conduit SMC, inval call Trap to EL2 Undef insn
- * Conduit not SMC Undef insn Undef insn
+ * Conduit not SMC nop nop
*/
/* On ARMv8 with EL3 AArch64, SMD applies to both S and NS state.
@@ -838,16 +838,11 @@ void HELPER(pre_smc)(CPUARMState *env, uint32_t syndrome)
if (!arm_feature(env, ARM_FEATURE_EL3) &&
cpu->psci_conduit != QEMU_PSCI_CONDUIT_SMC) {
-/* If we have no EL3 then SMC always UNDEFs and can't be
- * trapped to EL2. PSCI-via-SMC is a sort of ersatz EL3
- * firmware within QEMU, and we want an EL2 guest to be able
- * to forbid its EL1 from making PSCI calls into QEMU's
- * "firmware" via HCR.TSC, so for these purposes treat
- * PSCI-via-SMC as implying an EL3.
+/* If we have no EL3 then we simulate KVM behavior which
+ * simply treats every unknown SMC as a nop.
* This handles the very last line of the previous table.
*/
-raise_exception(env, EXCP_UDEF, syn_uncategorized(),
-exception_target_el(env));
+return;
}
if (cur_el == 1 && (arm_hcr_el2_eff(env) & HCR_TSC)) {
--
2.16.4
Re: [PATCH v3 2/2] target/m68k: consolidate physical translation offset into get_physical_address()
On 30/06/2020 22:20, Laurent Vivier wrote:
> Le 30/06/2020 à 13:27, Mark Cave-Ayland a écrit :
>> Since all callers to get_physical_address() now apply the same page offset to
>> the translation result, move the logic into get_physical_address() itself to
>> avoid duplication.
>>
>> Suggested-by: Philippe Mathieu-Daudé
>> Signed-off-by: Mark Cave-Ayland
>> ---
>> target/m68k/helper.c | 18 +++---
>> 1 file changed, 7 insertions(+), 11 deletions(-)
>>
>> diff --git a/target/m68k/helper.c b/target/m68k/helper.c
>> index 631eab7774..71c2376910 100644
>> --- a/target/m68k/helper.c
>> +++ b/target/m68k/helper.c
>> @@ -643,7 +643,7 @@ static int get_physical_address(CPUM68KState *env,
>> hwaddr *physical,
>> /* Transparent Translation Register bit */
>> env->mmu.mmusr = M68K_MMU_T_040 | M68K_MMU_R_040;
>> }
>> -*physical = address & TARGET_PAGE_MASK;
>> +*physical = address;
>> *page_size = TARGET_PAGE_SIZE;
>> return 0;
>> }
>> @@ -771,7 +771,8 @@ static int get_physical_address(CPUM68KState *env,
>> hwaddr *physical,
>> }
>> *page_size = 1 << page_bits;
>> page_mask = ~(*page_size - 1);
>> -*physical = next & page_mask;
>> +address &= TARGET_PAGE_MASK;
>
> I don't think you need TARGET_PAGE_MASK here:
> - TARGET_PAGE_MASK is 4096
> - page_mask is either 4096 or 8192
Ah yes, of course - that will get handled fine by the statement below.
>> +*physical = (next & page_mask) + (address & (*page_size - 1));
>>
>> if (access_type & ACCESS_PTEST) {
>> env->mmu.mmusr |= next & M68K_MMU_SR_MASK_040;
>> @@ -826,8 +827,6 @@ hwaddr m68k_cpu_get_phys_page_debug(CPUState *cs, vaddr
>> addr)
>> return -1;
>> }
>>
>> -addr &= TARGET_PAGE_MASK;
>> -phys_addr += addr & (page_size - 1);
>> return phys_addr;
>> }
>>
>> @@ -891,10 +890,8 @@ bool m68k_cpu_tlb_fill(CPUState *cs, vaddr address, int
>> size,
>> ret = get_physical_address(>env, , ,
>> address, access_type, _size);
>> if (likely(ret == 0)) {
>> -address &= TARGET_PAGE_MASK;
>> -physical += address & (page_size - 1);
>> -tlb_set_page(cs, address, physical,
>> - prot, mmu_idx, TARGET_PAGE_SIZE);
>> +tlb_set_page(cs, address & TARGET_PAGE_MASK,
>> + physical & TARGET_PAGE_MASK, prot, mmu_idx, page_size);
>
> I had a look to tl_set_page() to see how it manages the entry when the
> addresses are not aligned to page_size, and it calls
> tlb_set_page_with_attrs() where we have a comment:
>
> /* Add a new TLB entry. At most one entry for a given virtual address
> * is permitted. Only a single TARGET_PAGE_SIZE region is mapped, the
> * supplied size is only used by tlb_flush_page.
> ...
>
> So I think it's correct to use TARGET_PAGE_MASK and page_size.
Indeed, it certainly agrees with the documentation and what the majority of the
callers are doing - I'm just surprised that tlb_set_page_with_attrs() doesn't
assert() if any bits below TARGET_PAGE_MASK are set, rather than masking both
vaddr
and paddr itself. But I'm happy with this part anyhow.
ATB,
Mark.
Re: [PATCH] target/i386: implement undocumented "smsw r32" behavior
On 6/26/20 3:44 AM, Paolo Bonzini wrote: > In 32-bit mode, the higher 16 bits of the destination > register are undefined. In practice CR0[31:0] is stored, > just like in 64-bit mode, so just remove the "if" that > currently differentiates the behavior. > > Signed-off-by: Paolo Bonzini > --- > target/i386/translate.c | 13 +++-- > 1 file changed, 7 insertions(+), 6 deletions(-) Reviewed-by: Richard Henderson r~
Re: [PATCH] scripts/get_maintainer: Use .ignoredmailmap to ignore invalid emails
On 01.07.20 18:54, Philippe Mathieu-Daudé wrote: +Pavel/Paul/Alexander On 7/1/20 5:12 PM, Paolo Bonzini wrote: On 01/07/20 17:07, Philippe Mathieu-Daudé wrote: $ cat .ignoredmailmap # # From man git-shortlog the forms are: # # Proper Name # # Jean-Christophe PLAGNIOL-VILLARD Caio Carrara Yongbok Kim James Hogan Paul Burton Alexander Graf Roy Franz Dmitry Solodkiy Evgeny Voevodin Serge Hallyn Pavel Dovgalyuk For at least Paul Burton, Pavel Dovgalyuk and Alex Graf we should just use .mailmap, anyway I think the concept of the patch is okay. Pavel has been using a GMail account, but seems to be back to ispras.ru, so it might have be a temporary failure (over few days although). I can send a pair of patches for Paul and Alexander if they are OK. The others seem MIA. Yes, please! I would appreciate if you could use ag...@csgraf.de for QEMU related work :). Alex
Re: [PATCH v2 2/3] docs/devel: convert and update MTTCG design document
On 7/1/20 9:11 AM, Alex Bennée wrote: > -This document outlines the design for multi-threaded TCG system-mode > -emulation. The current user-mode emulation mirrors the thread > -structure of the translated executable. Some of the work will be > -applicable to both system and linux-user emulation. > +This document outlines the design for multi-threaded TCG (a.k.a MTTCG) > +system-mode emulation. user-mode emulation has always mirrored the Full stop wants capitalization for the next sentence. Though perhaps em-dash or semicolon would work as well to connect these clauses. > +guest (TCG_GUEST_DEFAULT_MO & ~TCG_TARGET_DEFAULT_MO is zero) and the Isn't there some markup for code? `...` perhaps? Otherwise, Reviewed-by: Richard Henderson r~
Re: [PATCH v2 1/3] docs/booting.rst: start documenting the boot process
On 7/1/20 9:11 AM, Alex Bennée wrote: > While working on some test cases I realised there was quite a lot of > assumed knowledge about how things boot up. I thought it would be > worth gathering this together in a user facing document where we could > pour in the details and background to the boot process. As it's quite > wordy I thought it should be a separate document to the manual (which > can obviously reference this). > > The document follows the socratic method and leaves the reader to ask > themselves some questions in an effort to elucidate them about any > problems they may be having. > > Signed-off-by: Alex Bennée > Message-Id: <20190308211557.22589-1-alex.ben...@linaro.org> Reviewed-by: Richard Henderson r~
[PATCH v3 1/2] net: tap: check if the file descriptor is valid before using it
qemu_set_nonblock() checks that the file descriptor can be used and, if
not, crashes QEMU. An assert() is used for that. The use of assert() is
used to detect programming error and the coredump will allow to debug
the problem.
But in the case of the tap device, this assert() can be triggered by
a misconfiguration by the user. At startup, it's not a real problem, but it
can also happen during the hot-plug of a new device, and here it's a
problem because we can crash a perfectly healthy system.
For instance:
# ip link add link virbr0 name macvtap0 type macvtap mode bridge
# ip link set macvtap0 up
# TAP=/dev/tap$(ip -o link show macvtap0 | cut -d: -f1)
# qemu-system-x86_64 -machine q35 -device pcie-root-port,id=pcie-root-port-0
-monitor stdio 9<> $TAP
(qemu) netdev_add type=tap,id=hostnet0,vhost=on,fd=9
(qemu) device_add
driver=virtio-net-pci,netdev=hostnet0,id=net0,bus=pcie-root-port-0
(qemu) device_del net0
(qemu) netdev_del hostnet0
(qemu) netdev_add type=tap,id=hostnet1,vhost=on,fd=9
qemu-system-x86_64: .../util/oslib-posix.c:247: qemu_set_nonblock: Assertion
`f != -1' failed.
Aborted (core dumped)
To avoid that, add a function, qemu_try_set_nonblock(), that allows to report
the
problem without crashing.
Signed-off-by: Laurent Vivier
---
include/qemu/sockets.h | 1 +
net/tap.c | 16 +---
util/oslib-posix.c | 26 +--
util/oslib-win32.c | 57 --
4 files changed, 64 insertions(+), 36 deletions(-)
diff --git a/include/qemu/sockets.h b/include/qemu/sockets.h
index 57cd049d6edd..7d1f8135767d 100644
--- a/include/qemu/sockets.h
+++ b/include/qemu/sockets.h
@@ -18,6 +18,7 @@ int qemu_accept(int s, struct sockaddr *addr, socklen_t
*addrlen);
int socket_set_cork(int fd, int v);
int socket_set_nodelay(int fd);
void qemu_set_block(int fd);
+int qemu_try_set_nonblock(int fd);
void qemu_set_nonblock(int fd);
int socket_set_fast_reuse(int fd);
diff --git a/net/tap.c b/net/tap.c
index 6207f61f84ab..fb04c9044ce2 100644
--- a/net/tap.c
+++ b/net/tap.c
@@ -766,6 +766,7 @@ int net_init_tap(const Netdev *netdev, const char *name,
Error *err = NULL;
const char *vhostfdname;
char ifname[128];
+int ret = 0;
assert(netdev->type == NET_CLIENT_DRIVER_TAP);
tap = >u.tap;
@@ -795,7 +796,12 @@ int net_init_tap(const Netdev *netdev, const char *name,
return -1;
}
-qemu_set_nonblock(fd);
+ret = qemu_try_set_nonblock(fd);
+if (ret < 0) {
+error_setg_errno(errp, -ret, "%s: Can't use file descriptor %d",
+ name, fd);
+return -1;
+}
vnet_hdr = tap_probe_vnet_hdr(fd);
@@ -810,7 +816,6 @@ int net_init_tap(const Netdev *netdev, const char *name,
char **fds;
char **vhost_fds;
int nfds = 0, nvhosts = 0;
-int ret = 0;
if (tap->has_ifname || tap->has_script || tap->has_downscript ||
tap->has_vnet_hdr || tap->has_helper || tap->has_queues ||
@@ -843,7 +848,12 @@ int net_init_tap(const Netdev *netdev, const char *name,
goto free_fail;
}
-qemu_set_nonblock(fd);
+ret = qemu_try_set_nonblock(fd);
+if (ret < 0) {
+error_setg_errno(errp, -ret, "%s: Can't use file descriptor
%d",
+ name, fd);
+goto free_fail;
+}
if (i == 0) {
vnet_hdr = tap_probe_vnet_hdr(fd);
diff --git a/util/oslib-posix.c b/util/oslib-posix.c
index 916f1be2243a..149254cd691f 100644
--- a/util/oslib-posix.c
+++ b/util/oslib-posix.c
@@ -253,25 +253,35 @@ void qemu_set_block(int fd)
assert(f != -1);
}
-void qemu_set_nonblock(int fd)
+int qemu_try_set_nonblock(int fd)
{
int f;
f = fcntl(fd, F_GETFL);
-assert(f != -1);
-f = fcntl(fd, F_SETFL, f | O_NONBLOCK);
-#ifdef __OpenBSD__
if (f == -1) {
+return -errno;
+}
+if (fcntl(fd, F_SETFL, f | O_NONBLOCK) == -1) {
+#ifdef __OpenBSD__
/*
* Previous to OpenBSD 6.3, fcntl(F_SETFL) is not permitted on
* memory devices and sets errno to ENODEV.
* It's OK if we fail to set O_NONBLOCK on devices like /dev/null,
* because they will never block anyway.
*/
-assert(errno == ENODEV);
-}
-#else
-assert(f != -1);
+if (errno == ENODEV) {
+return 0;
+}
#endif
+return -errno;
+}
+return 0;
+}
+
+void qemu_set_nonblock(int fd)
+{
+int f;
+f = qemu_try_set_nonblock(fd);
+assert(f == 0);
}
int socket_set_fast_reuse(int fd)
diff --git a/util/oslib-win32.c b/util/oslib-win32.c
index e9b14ab17847..5548ce6038f3 100644
--- a/util/oslib-win32.c
+++ b/util/oslib-win32.c
@@ -132,31 +132,6 @@ struct tm *localtime_r(const time_t *timep, struct tm
*result)
}
#endif /* CONFIG_LOCALTIME_R */
[PATCH v3 0/2] net: tap: check file descriptor can be used
v3: move qemu_fd_is_valid() checking into a new function qemu_try_set_nonblock(), and use qemu_try_set_nonblock() in qemu_set_nonblock(). v2: Add patch from Daniel to check the fd can be used I have updated Daniel's patch not to check for EINVAL on TUNGETIFF as I think we can avoid this special case because TUNGETIFF is available since kernel v2.6.27 (October 2008) Moreover I think the code was wrong as it was checking with -EINVAL and not EINVAL. Daniel P. Berrang�� (1): net: detect errors from probing vnet hdr flag for TAP devices Laurent Vivier (1): net: tap: check if the file descriptor is valid before using it include/qemu/sockets.h | 1 + net/tap-bsd.c | 2 +- net/tap-linux.c| 8 +++--- net/tap-solaris.c | 2 +- net/tap-stub.c | 2 +- net/tap.c | 41 -- net/tap_int.h | 2 +- util/oslib-posix.c | 26 +-- util/oslib-win32.c | 57 -- 9 files changed, 93 insertions(+), 48 deletions(-) -- 2.26.2
[PATCH v3 2/2] net: detect errors from probing vnet hdr flag for TAP devices
From: "Daniel P. Berrange"
When QEMU sets up a tap based network device backend, it mostly ignores errors
reported from various ioctl() calls it makes, assuming the TAP file descriptor
is valid. This assumption can easily be violated when the user is passing in a
pre-opened file descriptor. At best, the ioctls may fail with a -EBADF, but if
the user passes in a bogus FD number that happens to clash with a FD number that
QEMU has opened internally for another reason, a wide variety of errnos may
result, as the TUNGETIFF ioctl number may map to a completely different command
on a different type of file.
By ignoring all these errors, QEMU sets up a zombie network backend that will
never pass any data. Even worse, when QEMU shuts down, or that network backend
is hot-removed, it will close this bogus file descriptor, which could belong to
another QEMU device backend.
There's no obvious guaranteed reliable way to detect that a FD genuinely is a
TAP device, as opposed to a UNIX socket, or pipe, or something else. Checking
the errno from probing vnet hdr flag though, does catch the big common cases.
ie calling TUNGETIFF will return EBADF for an invalid FD, and ENOTTY when FD is
a UNIX socket, or pipe which catches accidental collisions with FDs used for
stdio, or monitor socket.
Previously the example below where bogus fd 9 collides with the FD used for the
chardev saw:
$ ./x86_64-softmmu/qemu-system-x86_64 -netdev tap,id=hostnet0,fd=9 \
-chardev socket,id=charchannel0,path=/tmp/qga,server,nowait \
-monitor stdio -vnc :0
qemu-system-x86_64: -netdev tap,id=hostnet0,fd=9: TUNGETIFF ioctl() failed:
Inappropriate ioctl for device
TUNSETOFFLOAD ioctl() failed: Bad address
QEMU 2.9.1 monitor - type 'help' for more information
(qemu) Warning: netdev hostnet0 has no peer
which gives a running QEMU with a zombie network backend.
With this change applied we get an error message and QEMU immediately exits
before carrying on and making a bigger disaster:
$ ./x86_64-softmmu/qemu-system-x86_64 -netdev tap,id=hostnet0,fd=9 \
-chardev socket,id=charchannel0,path=/tmp/qga,server,nowait \
-monitor stdio -vnc :0
qemu-system-x86_64: -netdev tap,id=hostnet0,vhost=on,fd=9: Unable to query
TUNGETIFF on FD 9: Inappropriate ioctl for device
Reported-by: Dr. David Alan Gilbert
Signed-off-by: Daniel P. Berrange
Tested-by: Dr. David Alan Gilbert
Message-id: 20171027085548.3472-1-berra...@redhat.com
[lv: to simplify, don't check on EINVAL with TUNGETIFF as it exists since
v2.6.27]
Signed-off-by: Laurent Vivier
---
net/tap-bsd.c | 2 +-
net/tap-linux.c | 8 +---
net/tap-solaris.c | 2 +-
net/tap-stub.c| 2 +-
net/tap.c | 25 -
net/tap_int.h | 2 +-
6 files changed, 29 insertions(+), 12 deletions(-)
diff --git a/net/tap-bsd.c b/net/tap-bsd.c
index a5c3707f806d..77aaf674b19d 100644
--- a/net/tap-bsd.c
+++ b/net/tap-bsd.c
@@ -211,7 +211,7 @@ void tap_set_sndbuf(int fd, const NetdevTapOptions *tap,
Error **errp)
{
}
-int tap_probe_vnet_hdr(int fd)
+int tap_probe_vnet_hdr(int fd, Error **errp)
{
return 0;
}
diff --git a/net/tap-linux.c b/net/tap-linux.c
index e0dd442ee34f..b0635e9e32ce 100644
--- a/net/tap-linux.c
+++ b/net/tap-linux.c
@@ -147,13 +147,15 @@ void tap_set_sndbuf(int fd, const NetdevTapOptions *tap,
Error **errp)
}
}
-int tap_probe_vnet_hdr(int fd)
+int tap_probe_vnet_hdr(int fd, Error **errp)
{
struct ifreq ifr;
if (ioctl(fd, TUNGETIFF, ) != 0) {
-error_report("TUNGETIFF ioctl() failed: %s", strerror(errno));
-return 0;
+/* TUNGETIFF is available since kernel v2.6.27 */
+error_setg_errno(errp, errno,
+ "Unable to query TUNGETIFF on FD %d", fd);
+return -1;
}
return ifr.ifr_flags & IFF_VNET_HDR;
diff --git a/net/tap-solaris.c b/net/tap-solaris.c
index 4725d2314eef..ae2ba6828415 100644
--- a/net/tap-solaris.c
+++ b/net/tap-solaris.c
@@ -206,7 +206,7 @@ void tap_set_sndbuf(int fd, const NetdevTapOptions *tap,
Error **errp)
{
}
-int tap_probe_vnet_hdr(int fd)
+int tap_probe_vnet_hdr(int fd, Error **errp)
{
return 0;
}
diff --git a/net/tap-stub.c b/net/tap-stub.c
index a9ab8f829362..de525a2e69d4 100644
--- a/net/tap-stub.c
+++ b/net/tap-stub.c
@@ -37,7 +37,7 @@ void tap_set_sndbuf(int fd, const NetdevTapOptions *tap,
Error **errp)
{
}
-int tap_probe_vnet_hdr(int fd)
+int tap_probe_vnet_hdr(int fd, Error **errp)
{
return 0;
}
diff --git a/net/tap.c b/net/tap.c
index fb04c9044ce2..208d9c0f8d35 100644
--- a/net/tap.c
+++ b/net/tap.c
@@ -597,7 +597,11 @@ int net_init_bridge(const Netdev *netdev, const char *name,
}
qemu_set_nonblock(fd);
-vnet_hdr = tap_probe_vnet_hdr(fd);
+vnet_hdr = tap_probe_vnet_hdr(fd, errp);
+if (vnet_hdr < 0) {
+close(fd);
+return -1;
+}
s = net_tap_fd_init(peer, "bridge", name, fd, vnet_hdr);
snprintf(s->nc.info_str, sizeof(s->nc.info_str),
Re: [PATCH v12 00/61] target/riscv: support vector extension v0.7.1
Patchew URL: https://patchew.org/QEMU/20200701152549.1218-1-zhiwei_...@c-sky.com/ Hi, This series seems to have some coding style problems. See output below for more information: Subject: [PATCH v12 00/61] target/riscv: support vector extension v0.7.1 Type: series Message-id: 20200701152549.1218-1-zhiwei_...@c-sky.com === TEST SCRIPT BEGIN === #!/bin/bash git rev-parse base > /dev/null || exit 0 git config --local diff.renamelimit 0 git config --local diff.renames True git config --local diff.algorithm histogram ./scripts/checkpatch.pl --mailback base.. === TEST SCRIPT END === Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384 From https://github.com/patchew-project/qemu - [tag update] patchew/20200626104419.15504-1-pbonz...@redhat.com -> patchew/20200626104419.15504-1-pbonz...@redhat.com * [new tag] patchew/20200701152549.1218-1-zhiwei_...@c-sky.com -> patchew/20200701152549.1218-1-zhiwei_...@c-sky.com Switched to a new branch 'test' 431919b target/riscv: configure and turn on vector extension from command line 0e35d15 target/riscv: vector compress instruction 6be622b target/riscv: vector register gather instruction b9779c8 target/riscv: vector slide instructions 6a0b312 target/riscv: floating-point scalar move instructions d91398e target/riscv: integer scalar move instruction 5fa31d1 target/riscv: integer extract instruction 7e8b41e target/riscv: vector element index instruction 97b52ff target/riscv: vector iota instruction 7aa4709 target/riscv: set-X-first mask bit e7a21a1 target/riscv: vmfirst find-first-set mask bit 4aaf979 target/riscv: vector mask population count vmpopc 9096e02 target/riscv: vector mask-register logical instructions 3c204ad target/riscv: vector widening floating-point reduction instructions 406af0e target/riscv: vector single-width floating-point reduction instructions cbf5ed6 target/riscv: vector wideing integer reduction instructions 9e3b9a7 target/riscv: vector single-width integer reduction instructions f0e515c target/riscv: narrowing floating-point/integer type-convert instructions 8d7d672 target/riscv: widening floating-point/integer type-convert instructions 60e2880 target/riscv: vector floating-point/integer type-convert instructions 58f2df3 target/riscv: vector floating-point merge instructions 5ba22de target/riscv: vector floating-point classify instructions beb2569 target/riscv: vector floating-point compare instructions 2fb6943 target/riscv: vector floating-point sign-injection instructions db252a1 target/riscv: vector floating-point min/max instructions 2c81552 target/riscv: vector floating-point square-root instruction fe99515 target/riscv: vector widening floating-point fused multiply-add instructions a0a573f target/riscv: vector single-width floating-point fused multiply-add instructions 5d77f38 target/riscv: vector widening floating-point multiply 69f13ff target/riscv: vector single-width floating-point multiply/divide instructions 58dced0 target/riscv: vector widening floating-point add/subtract instructions 9138276 target/riscv: vector single-width floating-point add/subtract instructions 7c11a84 target/riscv: vector narrowing fixed-point clip instructions 36e77fe target/riscv: vector single-width scaling shift instructions 20df1e8 target/riscv: vector widening saturating scaled multiply-add ad56fce target/riscv: vector single-width fractional multiply with rounding and saturation 5b7c118 target/riscv: vector single-width averaging add and subtract c4e27ab target/riscv: vector single-width saturating add and subtract df1e815 target/riscv: vector integer merge and move instructions d9bb4dc target/riscv: vector widening integer multiply-add instructions 27c8027 target/riscv: vector single-width integer multiply-add instructions 2390fd7 target/riscv: vector widening integer multiply instructions 340fd96 target/riscv: vector integer divide instructions a13adf3 target/riscv: vector single-width integer multiply instructions fe02626 target/riscv: vector integer min/max instructions 80b43d4 target/riscv: vector integer comparison instructions bf7783a target/riscv: vector narrowing integer right shift instructions c26f462 target/riscv: vector single-width bit shift instructions b82cf9d target/riscv: vector bitwise logical instructions a8fad7e target/riscv: vector integer add-with-carry / subtract-with-borrow instructions 176c4b1 target/riscv: vector widening integer add and subtract e9ae85d target/riscv: vector single-width integer add and subtract e44fb13 target/riscv: add vector amo operations cd3d6cf target/riscv: add fault-only-first unit stride load 2a93e9f target/riscv: add vector index load and store instructions 85edb50 target/riscv: add vector stride load and store instructions 0b3e234 target/riscv: add an internals.h header 441a043 target/riscv: add vector configure instruction 66c80a1 target/riscv: support vector extension csr ba5f712 target/riscv: implementation-defined constant parameters a4675d3 target/riscv: add vector
Re: [PATCH v6 0/7] dwc-hsotg (aka dwc2) USB host controller emulation
On Wed, Jul 1, 2020 at 11:30 AM Gerd Hoffmann wrote: > Hi, > > > This patch series adds emulation for the dwc-hsotg USB controller, > > which is used on the Raspberry Pi 3 and earlier, as well as a number > > of other development boards. The main benefit for Raspberry Pi is that > > this enables networking on these boards, since the network adapter is > > attached via USB. > > Doesn't apply cleanly to master. Either some conflict due to other rpi3 > changes merged meanwhile, or a dependency on unmerged patches. Can you > check and resend? > > thanks, > Gerd > > Hi Gerd, Peter already applied this series to master. Thanks, Paul
Re: [PATCH v3 2/2] MAINTAINERS: Adjust MIPS maintainership
On Wed, Jul 1, 2020 at 8:55 PM Aurelien Jarno wrote: > > NACK > > On 2020-07-01 20:25, Aleksandar Markovic wrote: > > Paul Burton and Aurelien Jarno removed for not being present. > > A polite email was sent to them with question whether they > > intend to actively participate, but there was no response. > > I indeed received a polite email, but it was sent less than 12 hours ago > (Peter Maydell was Cc:ed and can confirm). I didn't even got time to > answer it, I am still processing my emails after coming back from paid > work. > > I don't understand this sudden need to rush things in adjusting the > MIPS maintainership. > Well, from time to time it happens - there is nor rush. Good, I am glad. Welcome back! :) > Regards, > Aurelien > > -- > Aurelien Jarno GPG: 4096R/1DDD8C9B > aurel...@aurel32.net http://www.aurel32.net
Re: [PATCH v3 2/2] MAINTAINERS: Adjust MIPS maintainership
On Wed, Jul 1, 2020 at 8:43 PM Paul Burton wrote: > > Hi Aleksandar, > > On Wed, Jul 1, 2020 at 11:26 AM Aleksandar Markovic > wrote: > > Paul Burton and Aurelien Jarno removed for not being present. > > A polite email was sent to them with question whether they > > intend to actively participate, but there was no response. > > It was 2 days ago, not 2 months :) > :) Haha, but nobody said 2 months ago. What do you need - 2 years notice? :) Paul, you know I would like you be with me here more than anything else. I still feel the excitement of working together with you - can you believe it? Some people here even (so wrongfully) think I have some bad feelings about you, while the truth is the total opposite, and I am sure you know too. All doors are open to you, if you want to get involved, and wherever I will be. You are among the best engineers, actually, now I am thinking, probably the very best, I worked with! I long our path cross again! All the best on your pursuits! Aleksandar > I'm fine with being removed though - I no longer have access to modern > MIPS CPUs or Boston hardware, and wouldn't currently have time to > spend on them if I did. So as far as removing me goes: > > Acked-by: Paul Burton > > > In cases where needed, other persons step in instead. > > > > Huacai Chen and Jiaxun Yang step in as new energy. > > All the best with it moving forwards, and thanks to Huacai & Jiaxun > for stepping up! > > Paul
Re: [PATCH v3 2/2] MAINTAINERS: Adjust MIPS maintainership
NACK On 2020-07-01 20:25, Aleksandar Markovic wrote: > Paul Burton and Aurelien Jarno removed for not being present. > A polite email was sent to them with question whether they > intend to actively participate, but there was no response. I indeed received a polite email, but it was sent less than 12 hours ago (Peter Maydell was Cc:ed and can confirm). I didn't even got time to answer it, I am still processing my emails after coming back from paid work. I don't understand this sudden need to rush things in adjusting the MIPS maintainership. Regards, Aurelien -- Aurelien Jarno GPG: 4096R/1DDD8C9B aurel...@aurel32.net http://www.aurel32.net
Re: [RFC PATCH] cpus: Initialize current_cpu with the first vCPU created
On 200701 2021, Philippe Mathieu-Daudé wrote:
> We can run I/O access with the 'i' or 'o' HMP commands in the
> monitor. These commands are expected to run on a vCPU. The
> monitor is not a vCPU thread. To avoid crashing, initialize
> the 'current_cpu' variable with the first vCPU created. The
> command executed on the monitor will end using it.
>
> This fixes:
>
> $ cat << EOF| qemu-system-i386 -M q35 -nographic -serial none -monitor stdio
> o/4 0xcf8 0x8400f841
> o/4 0xcfc 0xaa215d6d
> o/4 0x6d30 0x2ef8ffbe
> o/1 0xb2 0x20
> EOF
> Segmentation fault (core dumped)
>
> Thread 1 "qemu-system-i38" received signal SIGSEGV, Segmentation fault.
> 0x558946c7 in tcg_handle_interrupt (cpu=0x0, mask=64) at
> accel/tcg/tcg-all.c:57
> 57 old_mask = cpu->interrupt_request;
> (gdb) bt
> #0 0x558946c7 in tcg_handle_interrupt (cpu=0x0, mask=64) at
> accel/tcg/tcg-all.c:57
> #1 0x558ed7d2 in cpu_interrupt (cpu=0x0, mask=64) at
> include/hw/core/cpu.h:877
> #2 0x558ee776 in ich9_apm_ctrl_changed (val=32,
> arg=0x56e2ff50) at hw/isa/lpc_ich9.c:442
> #3 0x55b47f96 in apm_ioport_writeb (opaque=0x56e308c0, addr=0,
> val=32, size=1) at hw/isa/apm.c:44
> #4 0x55879931 in memory_region_write_accessor (mr=0x56e308e0,
> addr=0, value=0x7fffb9f8, size=1, shift=0, mask=255, attrs=...) at
> memory.c:483
> #5 0x55879b5a in access_with_adjusted_size (addr=0,
> value=0x7fffb9f8, size=4, access_size_min=1, access_size_max=1, access_fn=
> 0x5587984e , mr=0x56e308e0,
> attrs=...) at memory.c:544
> #6 0x5587ca32 in memory_region_dispatch_write (mr=0x56e308e0,
> addr=0, data=32, op=MO_32, attrs=...) at memory.c:1465
> #7 0x5581b7e9 in flatview_write_continue (fv=0x5698a790,
> addr=178, attrs=..., ptr=0x7fffbb84, len=4, addr1=0, l=4,
> mr=0x56e308e0) at exec.c:3198
> #8 0x5581b92e in flatview_write (fv=0x5698a790, addr=178,
> attrs=..., buf=0x7fffbb84, len=4) at exec.c:3238
> #9 0x5581bc81 in address_space_write (as=0x56441220
> , addr=178, attrs=..., buf=0x7fffbb84, len=4) at
> exec.c:3329
> #10 0x55873f08 in cpu_outl (addr=178, val=32) at ioport.c:80
> #11 0x5598a26a in hmp_ioport_write (mon=0x567621b0,
> qdict=0x57702600) at monitor/misc.c:937
> #12 0x55c9c5a5 in handle_hmp_command (mon=0x567621b0,
> cmdline=0x5676aae1 "/1 0xb2 0x20") at monitor/hmp.c:1082
> #13 0x55c99e02 in monitor_command_cb (opaque=0x567621b0,
> cmdline=0x5676aae0 "o/1 0xb2 0x20", readline_opaque=0x0) at
> monitor/hmp.c:47
> ^
> HMP command from monitor
>
> Reported-by: Alexander Bulekov
> Buglink: https://bugs.launchpad.net/qemu/+bug/1878645
> Signed-off-by: Philippe Mathieu-Daudé
> ---
> Cc: Bug 1878645 <1878...@bugs.launchpad.net>
>
> RFC because I believe the correct fix is to NOT use current_cpu
> out of cpus.c, at least use qemu_get_cpu(0) to get the first vCPU.
> ---
> cpus.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/cpus.c b/cpus.c
> index 41d1c5099f..1f6f43d221 100644
> --- a/cpus.c
> +++ b/cpus.c
> @@ -2106,6 +2106,9 @@ void qemu_init_vcpu(CPUState *cpu)
> {
> MachineState *ms = MACHINE(qdev_get_machine());
>
> +if (!current_cpu) {
> +current_cpu = cpu;
> +}
Seems like a neat solution.
is it fair to assume that qemu_init_vcpu is called before any threads
that can do I/O are created? I confirmed that the qtest and hmp crashes
are avoided.
-Alex
> cpu->nr_cores = ms->smp.cores;
> cpu->nr_threads = ms->smp.threads;
> cpu->stopped = true;
> --
> 2.21.3
>
Re: [PATCH v3 0/5] hw/mips/malta: Add the 'malta-strict' machine, matching Malta hardware
On Wed, Jul 1, 2020 at 7:39 PM Aurelien Jarno wrote: > > Aleksandar, > > On 2020-06-30 23:54, Aleksandar Markovic wrote: > > As, in a very clear way, evidenced from the previous versions of this > > series, this series real goal was not not to create some new > > "malta-strict" machine, but to prepare path to creation of some > > imagined "malta-unleashed" machine which will, to the contrary of > > proclaimed goal, create a machine that could never exist in reality. > > That is why I can't accept this series. > > I do not understand your opposition to this, and why it is an issue to > support more than 2GiB of RAM for such a board. Supporting more than 2GiB > of memory doesn't prevent people to emulate a real Malta board with less > memory. > > In addition to that, the Malta board in QEMU has been supporting for > many years more than the maximum 256MiB that is possible on a physical > board. The QEMU version also supports way more than CPU variants than > the physical board. In other word the existing malta machine in QEMU is > already a "malta-unleashed". > Aurelien, Glad to see you again. I am really sorry you were absent for so long. Those (what you described in the paragraphs above) were mistakes from the past. At some point, we needed to stop doing it, and finally returned to the root QEMU principles of emulating systems as faithfully as possible. Knowing the needs like you described exist, my vision is that, just for occasions you described, we create a virtual board that would have very wide set of feature, unconstrained by real world. That way we would avoid situations to "lie" in our emulations. If you needed something more that is currently provided, you should have issued a feature request through regular channels, and that would have the people the chance to develop a solid solution, not some quick fixes that pushes us further and further in wring direction. Best wishes, Aleksandar Why didn't you respond on my mail from the other day? Do you plan to respond? > And these possibilities have been used by MIPS* employees to develop > MIPS R6 based distributions. Limiting the board in terms of RAM, CPU or > virtio support would just make our users life more difficult for no > gain. > > Regards, > Aurelien > > * By MIPS employee, I mean persons that have been employed by companies > owning MIPS over the last few years, including Imagination Technologies > and Wave Computing. > > > > > уто, 30. јун 2020. у 21:58 Philippe Mathieu-Daudé је > > написао/ла: > > > > > > Hi, > > > > > > This series add a new 'malta-strict' machine, that aims to properly > > > model the real hardware (which is not what the current 'malta' > > > machine models). > > > > > > Since v2: > > > - Initialize missing malta_machine_types::class_size > > > - Remove RFC patch that confuses Aleksandar > > > - Added examples of 'malta-strict' use > > > > > > $ git backport-diff -u v2 > > > Key: > > > [] : patches are identical > > > [] : number of functional differences between upstream/downstream > > > patch > > > [down] : patch is downstream-only > > > The flags [FC] indicate (F)unctional and (C)ontextual differences, > > > respectively > > > > > > 001/5:[] [--] 'hw/mips/malta: Trivial code movement' > > > 002/5:[] [--] 'hw/mips/malta: Register the machine as a TypeInfo' > > > 003/5:[0001] [FC] 'hw/mips/malta: Introduce > > > MaltaMachineClass::max_ramsize' > > > 004/5:[] [--] 'hw/mips/malta: Introduce the 'malta-strict' machine' > > > 005/5:[] [--] 'hw/mips/malta: Verify malta-strict machine uses > > > correct DIMM sizes' > > > > > > Philippe Mathieu-Daudé (5): > > > hw/mips/malta: Trivial code movement > > > hw/mips/malta: Register the machine as a TypeInfo > > > hw/mips/malta: Introduce MaltaMachineClass::max_ramsize > > > hw/mips/malta: Introduce the 'malta-strict' machine > > > hw/mips/malta: Verify malta-strict machine uses correct DIMM sizes > > > > > > hw/mips/malta.c | 105 +--- > > > 1 file changed, 91 insertions(+), 14 deletions(-) > > > > > > -- > > > 2.21.3 > > > > > > > > > > -- > Aurelien Jarno GPG: 4096R/1DDD8C9B > aurel...@aurel32.net http://www.aurel32.net
Re: [PATCH v2 4/9] i386: hvf: Implement CPU kick
Thanks, sounds good! Of course the best solution would be in HVF itself,
similar to KVM and WHPX, but at least it's possible to work around it.
Paolo
Il mer 1 lug 2020, 20:37 Roman Bolshakov ha scritto:
> On Tue, Jun 30, 2020 at 06:04:23PM +0200, Paolo Bonzini wrote:
> > On 30/06/20 17:50, Roman Bolshakov wrote:
> > > On Tue, Jun 30, 2020 at 02:33:42PM +0200, Paolo Bonzini wrote:
> > >> Can a signal interrupt hv_vcpu_run? If so you actually don't need
> > >> hv_vcpu_interrupt at all.
> > >
> > > Existing signal masking and SIG_IPI didn't work IIRC when I tried to
> add
> > > a primitive version of gdbstub support.
> >
> > You can try pthread_kill followed by hv_vcpu_interrupt if it doesn't.
> > The signal would be delivered after return to userspace.
> >
>
> I looked at the signal setup for HVF again. I was wrong with regards to
> SIG_IPI. It isn't delivered to vCPU because the signal is masked, this
> fixes it:
>
> diff --git a/target/i386/hvf/hvf.c b/target/i386/hvf/hvf.c
> index d81f569aed..7bf05bca21 100644
> --- a/target/i386/hvf/hvf.c
> +++ b/target/i386/hvf/hvf.c
> @@ -479,6 +479,7 @@ int hvf_init_vcpu(CPUState *cpu)
>
> pthread_sigmask(SIG_BLOCK, NULL, );
> sigdelset(, SIG_IPI);
> +pthread_sigmask(SIG_SETMASK, , NULL);
>
> init_emu();
> init_decoder();
>
> But the signal is delivered only after vmxexit, perhaps a sequence of
> pthread_kill() and hv_vcpu_interrupt() is really needed.
>
> So, there are two race windows on kernel-to-user border in v2: just
> before checking the deadline and vmenter and just after vmxexit and
> re-arm of preemption timer, that's two places where kicks could be lost.
> The approach you proposed seems to address them.
>
> Thanks,
> Roman
>
> > >> You can also require the preemption time, all
> > >> processor that support HVF have it, but never set it by default. The
> > >> deadline can be left at 0 all the time; instead, you toggle the bit in
> > >> the pin-based controls. In the signal handler you do:
> > >>
> > >>if (atomic_xchg(>hvf_in_guest, false)) {
> > >>wvmcs(cpu->hvf_fd, VMCS_PIN_BASED_CTLS,
> > >> rvmcs(cpu->hvf_fd, VMCS_PIN_BASED_CTLS)
> > >>| VMCS_PIN_BASED_CTLS_VMX_PREEMPT_TIMER);
> > >>}
> > >>
> > >> In the main loop you do:
> > >>
> > >>atomic_set(>hvf_guest_mode, true);
> > >>smp_mb();
> > >>hv_vcpu_run(...);
> > >>atomic_set(>hvf_guest_mode, false);
> > >>
> > >> and in the preemption timer vmexit handler:
> > >>
> > >>wvmcs(cpu->hvf_fd, VMCS_PIN_BASED_CTLS,
> > >> rvmcs(cpu->hvf_fd, VMCS_PIN_BASED_CTLS)
> > >>& ~VMCS_PIN_BASED_CTLS_VMX_PREEMPT_TIMER);
> > >>
> > >
>
>
[PATCH v4 2/4] RISC-V: Copy the fdt in dram instead of ROM
Currently, the fdt is copied to the ROM after the reset vector. The firmware
has to copy it to DRAM. Instead of this, directly copy the device tree to a
pre-computed dram address. The device tree load address should be as far as
possible from kernel and initrd images. That's why it is kept at the end of
the DRAM or 4GB whichever is lesser.
Signed-off-by: Atish Patra
Reviewed-by: Alistair Francis
---
hw/riscv/boot.c | 53 +
hw/riscv/sifive_u.c | 28 ++
hw/riscv/spike.c| 7 +-
hw/riscv/virt.c | 7 +-
include/hw/riscv/boot.h | 4 +++-
5 files changed, 66 insertions(+), 33 deletions(-)
diff --git a/hw/riscv/boot.c b/hw/riscv/boot.c
index 3df802380a36..c62f545f15e7 100644
--- a/hw/riscv/boot.c
+++ b/hw/riscv/boot.c
@@ -159,45 +159,68 @@ hwaddr riscv_load_initrd(const char *filename, uint64_t
mem_size,
return *start + size;
}
+uint32_t riscv_load_fdt(hwaddr dram_base, uint64_t mem_size, void *fdt)
+{
+uint32_t temp, fdt_addr;
+hwaddr dram_end = dram_base + mem_size;
+int fdtsize = fdt_totalsize(fdt);
+
+if (fdtsize <= 0) {
+error_report("invalid device-tree");
+exit(1);
+}
+
+/*
+ * We should put fdt as far as possible to avoid kernel/initrd overwriting
+ * its content. But it should be addressable by 32 bit system as well.
+ * Thus, put it at an aligned address that less than fdt size from end of
+ * dram or 4GB whichever is lesser.
+ */
+temp = MIN(dram_end, 4096 * MiB);
+fdt_addr = QEMU_ALIGN_DOWN(temp - fdtsize, 2 * MiB);
+
+fdt_pack(fdt);
+/* copy in the device tree */
+qemu_fdt_dumpdtb(fdt, fdtsize);
+
+rom_add_blob_fixed_as("fdt", fdt, fdtsize, fdt_addr,
+ _space_memory);
+
+return fdt_addr;
+}
+
void riscv_setup_rom_reset_vec(hwaddr start_addr, hwaddr rom_base,
- hwaddr rom_size, void *fdt)
+ hwaddr rom_size,
+ uint32_t fdt_load_addr, void *fdt)
{
int i;
/* reset vector */
-uint32_t reset_vec[8] = {
+uint32_t reset_vec[10] = {
0x0297, /* 1: auipc t0, %pcrel_hi(dtb) */
-0x02028593, /* addi a1, t0, %pcrel_lo(1b) */
0xf1402573, /* csrr a0, mhartid */
#if defined(TARGET_RISCV32)
+0x0202a583, /* lw a1, 32(t0) */
0x0182a283, /* lw t0, 24(t0) */
#elif defined(TARGET_RISCV64)
+0x0202b583, /* ld a1, 32(t0) */
0x0182b283, /* ld t0, 24(t0) */
#endif
0x00028067, /* jr t0 */
0x,
start_addr, /* start: .dword */
+0x,
+fdt_load_addr, /* fdt_laddr: .dword */
0x,
/* dtb: */
};
/* copy in the reset vector in little_endian byte order */
-for (i = 0; i < sizeof(reset_vec) >> 2; i++) {
+for (i = 0; i < ARRAY_SIZE(reset_vec); i++) {
reset_vec[i] = cpu_to_le32(reset_vec[i]);
}
rom_add_blob_fixed_as("mrom.reset", reset_vec, sizeof(reset_vec),
rom_base, _space_memory);
-/* copy in the device tree */
-if (fdt_pack(fdt) || fdt_totalsize(fdt) >
-rom_size - sizeof(reset_vec)) {
-error_report("not enough space to store device-tree");
-exit(1);
-}
-qemu_fdt_dumpdtb(fdt, fdt_totalsize(fdt));
-rom_add_blob_fixed_as("mrom.fdt", fdt, fdt_totalsize(fdt),
- rom_base + sizeof(reset_vec),
- _space_memory);
-
return;
}
diff --git a/hw/riscv/sifive_u.c b/hw/riscv/sifive_u.c
index 395b21703ab4..aed814da9b94 100644
--- a/hw/riscv/sifive_u.c
+++ b/hw/riscv/sifive_u.c
@@ -379,6 +379,7 @@ static void sifive_u_machine_init(MachineState *machine)
MemoryRegion *flash0 = g_new(MemoryRegion, 1);
target_ulong start_addr = memmap[SIFIVE_U_DRAM].base;
int i;
+uint32_t fdt_load_addr;
/* Initialize SoC */
object_initialize_child(OBJECT(machine), "soc", >soc, TYPE_RISCV_U_SOC);
@@ -450,40 +451,37 @@ static void sifive_u_machine_init(MachineState *machine)
}
}
+/* Compute the fdt load address in dram */
+fdt_load_addr = riscv_load_fdt(memmap[SIFIVE_U_DRAM].base,
+ machine->ram_size, s->fdt);
+
/* reset vector */
-uint32_t reset_vec[8] = {
+uint32_t reset_vec[11] = {
s->msel, /* MSEL pin state */
0x0297,/* 1: auipc t0, %pcrel_hi(dtb) */
-0x01c28593,/* addi a1, t0, %pcrel_lo(1b) */
0xf1402573,/* csrr a0, mhartid */
#if
[PATCH v4 3/4] riscv: Add opensbi firmware dynamic support
OpenSBI is the default firmware in Qemu and has various firmware loading
options. Currently, qemu loader uses fw_jump which has a compile time
pre-defined address where fdt & kernel image must reside. This puts a
constraint on image size of the Linux kernel depending on the fdt location
and available memory. However, fw_dynamic allows the loader to specify
the next stage location (i.e. Linux kernel/U-Boot) in memory and other
configurable boot options available in OpenSBI.
Add support for OpenSBI dynamic firmware loading support. This doesn't
break existing setup and fw_jump will continue to work as it is. Any
other firmware will continue to work without any issues as long as it
doesn't expect anything specific from loader in "a2" register.
Signed-off-by: Atish Patra
Reviewed-by: Alistair Francis
---
hw/riscv/boot.c | 42 +---
hw/riscv/sifive_u.c | 20 +---
hw/riscv/spike.c| 13 ++--
hw/riscv/virt.c | 12 +--
include/hw/riscv/boot.h | 5 ++-
include/hw/riscv/boot_opensbi.h | 58 +
6 files changed, 134 insertions(+), 16 deletions(-)
create mode 100644 include/hw/riscv/boot_opensbi.h
diff --git a/hw/riscv/boot.c b/hw/riscv/boot.c
index c62f545f15e7..feff6e3f4ed5 100644
--- a/hw/riscv/boot.c
+++ b/hw/riscv/boot.c
@@ -25,6 +25,7 @@
#include "hw/boards.h"
#include "hw/loader.h"
#include "hw/riscv/boot.h"
+#include "hw/riscv/boot_opensbi.h"
#include "elf.h"
#include "sysemu/device_tree.h"
#include "sysemu/qtest.h"
@@ -33,8 +34,10 @@
#if defined(TARGET_RISCV32)
# define KERNEL_BOOT_ADDRESS 0x8040
+#define fw_dynamic_info_data(__val) cpu_to_le32(__val)
#else
# define KERNEL_BOOT_ADDRESS 0x8020
+#define fw_dynamic_info_data(__val) cpu_to_le64(__val)
#endif
void riscv_find_and_load_firmware(MachineState *machine,
@@ -189,15 +192,45 @@ uint32_t riscv_load_fdt(hwaddr dram_base, uint64_t
mem_size, void *fdt)
return fdt_addr;
}
+void riscv_rom_copy_firmware_info(hwaddr rom_base, hwaddr rom_size,
+ uint32_t reset_vec_size, uint64_t kernel_entry)
+{
+struct fw_dynamic_info dinfo;
+size_t dinfo_len;
+
+dinfo.magic = fw_dynamic_info_data(FW_DYNAMIC_INFO_MAGIC_VALUE);
+dinfo.version = fw_dynamic_info_data(FW_DYNAMIC_INFO_VERSION);
+dinfo.next_mode = fw_dynamic_info_data(FW_DYNAMIC_INFO_NEXT_MODE_S);
+dinfo.next_addr = fw_dynamic_info_data(kernel_entry);
+dinfo.options = 0;
+dinfo.boot_hart = 0;
+dinfo_len = sizeof(dinfo);
+
+/**
+ * copy the dynamic firmware info. This information is specific to
+ * OpenSBI but doesn't break any other firmware as long as they don't
+ * expect any certain value in "a2" register.
+ */
+if (dinfo_len > (rom_size - reset_vec_size)) {
+error_report("not enough space to store dynamic firmware info");
+exit(1);
+}
+
+rom_add_blob_fixed_as("mrom.finfo", , dinfo_len,
+ rom_base + reset_vec_size,
+ _space_memory);
+}
+
void riscv_setup_rom_reset_vec(hwaddr start_addr, hwaddr rom_base,
- hwaddr rom_size,
+ hwaddr rom_size, uint64_t kernel_entry,
uint32_t fdt_load_addr, void *fdt)
{
int i;
/* reset vector */
uint32_t reset_vec[10] = {
-0x0297, /* 1: auipc t0, %pcrel_hi(dtb) */
+0x0297, /* 1: auipc t0, %pcrel_hi(fw_dyn) */
+0x02828613, /* addi a2, t0, %pcrel_lo(1b) */
0xf1402573, /* csrr a0, mhartid */
#if defined(TARGET_RISCV32)
0x0202a583, /* lw a1, 32(t0) */
@@ -207,12 +240,11 @@ void riscv_setup_rom_reset_vec(hwaddr start_addr, hwaddr
rom_base,
0x0182b283, /* ld t0, 24(t0) */
#endif
0x00028067, /* jr t0 */
-0x,
start_addr, /* start: .dword */
0x,
fdt_load_addr, /* fdt_laddr: .dword */
0x,
- /* dtb: */
+ /* fw_dyn: */
};
/* copy in the reset vector in little_endian byte order */
@@ -221,6 +253,8 @@ void riscv_setup_rom_reset_vec(hwaddr start_addr, hwaddr
rom_base,
}
rom_add_blob_fixed_as("mrom.reset", reset_vec, sizeof(reset_vec),
rom_base, _space_memory);
+riscv_rom_copy_firmware_info(rom_base, rom_size, sizeof(reset_vec),
+ kernel_entry);
return;
}
diff --git a/hw/riscv/sifive_u.c b/hw/riscv/sifive_u.c
index aed814da9b94..901efab9d5bd 100644
--- a/hw/riscv/sifive_u.c
+++ b/hw/riscv/sifive_u.c
@@ -380,6 +380,7 @@ static void sifive_u_machine_init(MachineState
[PATCH v4 0/4] Add OpenSBI dynamic firmware support
This series adds support OpenSBI dynamic firmware support to Qemu. Qemu loader passes the information about the DT and next stage (i.e. kernel or U-boot) via "a2" register. It allows the user to build bigger OS images without worrying about overwriting DT. It also unifies the reset vector code in rom and dt placement. Now, the DT is copied directly in DRAM instead of ROM. The changes have been verified on following qemu machines. 64bit: - spike, sifive_u, virt 32bit: - virt I have also verified fw_jump on all the above platforms to ensure that this series doesn't break the existing setup. Changes from v3->v4: 1. Addressed all review comments. 2. Added another patch that allows to boot a qemu machine from 64bit start address for RV64. Changes from v2->v3: 1. Removed redundant header includes. Changes from v1->v2: 1. Rebased on top of latest upstream Qemu (with MSEL changes for sifive_u). 2. Improved the code organization Atish Patra (4): riscv: Unify Qemu's reset vector code path RISC-V: Copy the fdt in dram instead of ROM riscv: Add opensbi firmware dynamic support RISC-V: Support 64 bit start address hw/riscv/boot.c | 107 hw/riscv/sifive_u.c | 51 +-- hw/riscv/spike.c| 57 + hw/riscv/virt.c | 55 +--- include/hw/riscv/boot.h | 7 +++ include/hw/riscv/boot_opensbi.h | 58 + 6 files changed, 236 insertions(+), 99 deletions(-) create mode 100644 include/hw/riscv/boot_opensbi.h -- 2.26.2
[PATCH v4 1/4] riscv: Unify Qemu's reset vector code path
Currently, all riscv machines except sifive_u have identical reset vector
code implementations with memory addresses being different for all machines.
They can be easily combined into a single function in common code.
Move it to common function and let all the machines use the common function.
Signed-off-by: Atish Patra
Reviewed-by: Alistair Francis
Reviewed-by: Bin Meng
Tested-by: Bin Meng
---
hw/riscv/boot.c | 46 +
hw/riscv/sifive_u.c | 1 -
hw/riscv/spike.c| 41 +++-
hw/riscv/virt.c | 40 +++
include/hw/riscv/boot.h | 2 ++
5 files changed, 54 insertions(+), 76 deletions(-)
diff --git a/hw/riscv/boot.c b/hw/riscv/boot.c
index adb421b91b68..3df802380a36 100644
--- a/hw/riscv/boot.c
+++ b/hw/riscv/boot.c
@@ -26,8 +26,11 @@
#include "hw/loader.h"
#include "hw/riscv/boot.h"
#include "elf.h"
+#include "sysemu/device_tree.h"
#include "sysemu/qtest.h"
+#include
+
#if defined(TARGET_RISCV32)
# define KERNEL_BOOT_ADDRESS 0x8040
#else
@@ -155,3 +158,46 @@ hwaddr riscv_load_initrd(const char *filename, uint64_t
mem_size,
return *start + size;
}
+
+void riscv_setup_rom_reset_vec(hwaddr start_addr, hwaddr rom_base,
+ hwaddr rom_size, void *fdt)
+{
+int i;
+
+/* reset vector */
+uint32_t reset_vec[8] = {
+0x0297, /* 1: auipc t0, %pcrel_hi(dtb) */
+0x02028593, /* addi a1, t0, %pcrel_lo(1b) */
+0xf1402573, /* csrr a0, mhartid */
+#if defined(TARGET_RISCV32)
+0x0182a283, /* lw t0, 24(t0) */
+#elif defined(TARGET_RISCV64)
+0x0182b283, /* ld t0, 24(t0) */
+#endif
+0x00028067, /* jr t0 */
+0x,
+start_addr, /* start: .dword */
+0x,
+ /* dtb: */
+};
+
+/* copy in the reset vector in little_endian byte order */
+for (i = 0; i < sizeof(reset_vec) >> 2; i++) {
+reset_vec[i] = cpu_to_le32(reset_vec[i]);
+}
+rom_add_blob_fixed_as("mrom.reset", reset_vec, sizeof(reset_vec),
+ rom_base, _space_memory);
+
+/* copy in the device tree */
+if (fdt_pack(fdt) || fdt_totalsize(fdt) >
+rom_size - sizeof(reset_vec)) {
+error_report("not enough space to store device-tree");
+exit(1);
+}
+qemu_fdt_dumpdtb(fdt, fdt_totalsize(fdt));
+rom_add_blob_fixed_as("mrom.fdt", fdt, fdt_totalsize(fdt),
+ rom_base + sizeof(reset_vec),
+ _space_memory);
+
+return;
+}
diff --git a/hw/riscv/sifive_u.c b/hw/riscv/sifive_u.c
index 7d051e7c9299..395b21703ab4 100644
--- a/hw/riscv/sifive_u.c
+++ b/hw/riscv/sifive_u.c
@@ -56,7 +56,6 @@
#include "sysemu/device_tree.h"
#include "sysemu/runstate.h"
#include "sysemu/sysemu.h"
-#include "exec/address-spaces.h"
#include
diff --git a/hw/riscv/spike.c b/hw/riscv/spike.c
index 3c87e04fdceb..c696077cbc16 100644
--- a/hw/riscv/spike.c
+++ b/hw/riscv/spike.c
@@ -41,9 +41,6 @@
#include "sysemu/device_tree.h"
#include "sysemu/qtest.h"
#include "sysemu/sysemu.h"
-#include "exec/address-spaces.h"
-
-#include
#if defined(TARGET_RISCV32)
# define BIOS_FILENAME "opensbi-riscv32-spike-fw_jump.elf"
@@ -165,7 +162,6 @@ static void spike_board_init(MachineState *machine)
MemoryRegion *system_memory = get_system_memory();
MemoryRegion *main_mem = g_new(MemoryRegion, 1);
MemoryRegion *mask_rom = g_new(MemoryRegion, 1);
-int i;
unsigned int smp_cpus = machine->smp.cpus;
/* Initialize SOC */
@@ -212,40 +208,9 @@ static void spike_board_init(MachineState *machine)
}
}
-/* reset vector */
-uint32_t reset_vec[8] = {
-0x0297, /* 1: auipc t0, %pcrel_hi(dtb) */
-0x02028593, /* addi a1, t0, %pcrel_lo(1b) */
-0xf1402573, /* csrr a0, mhartid */
-#if defined(TARGET_RISCV32)
-0x0182a283, /* lw t0, 24(t0) */
-#elif defined(TARGET_RISCV64)
-0x0182b283, /* ld t0, 24(t0) */
-#endif
-0x00028067, /* jr t0 */
-0x,
-memmap[SPIKE_DRAM].base, /* start: .dword DRAM_BASE */
-0x,
- /* dtb: */
-};
-
-/* copy in the reset vector in little_endian byte order */
-for (i = 0; i < sizeof(reset_vec) >> 2; i++) {
-reset_vec[i] = cpu_to_le32(reset_vec[i]);
-}
-rom_add_blob_fixed_as("mrom.reset", reset_vec, sizeof(reset_vec),
- memmap[SPIKE_MROM].base, _space_memory);
-
-/* copy in the device tree */
-if (fdt_pack(s->fdt) ||
[PATCH v4 4/4] RISC-V: Support 64 bit start address
Even though the start address in ROM code is declared as a 64 bit address
for RV64, it can't be used as upper bits are set to zero in ROM code.
Update the ROM code correctly to reflect the 64bit value.
Signed-off-by: Atish Patra
---
hw/riscv/boot.c | 6 +-
hw/riscv/sifive_u.c | 6 +-
2 files changed, 10 insertions(+), 2 deletions(-)
diff --git a/hw/riscv/boot.c b/hw/riscv/boot.c
index feff6e3f4ed5..4c6c101ff179 100644
--- a/hw/riscv/boot.c
+++ b/hw/riscv/boot.c
@@ -226,7 +226,11 @@ void riscv_setup_rom_reset_vec(hwaddr start_addr, hwaddr
rom_base,
uint32_t fdt_load_addr, void *fdt)
{
int i;
+uint32_t start_addr_hi32 = 0x;
+#if defined(TARGET_RISCV64)
+start_addr_hi32 = start_addr >> 32;
+#endif
/* reset vector */
uint32_t reset_vec[10] = {
0x0297, /* 1: auipc t0, %pcrel_hi(fw_dyn) */
@@ -241,7 +245,7 @@ void riscv_setup_rom_reset_vec(hwaddr start_addr, hwaddr
rom_base,
#endif
0x00028067, /* jr t0 */
start_addr, /* start: .dword */
-0x,
+start_addr_hi32,
fdt_load_addr, /* fdt_laddr: .dword */
0x,
/* fw_dyn: */
diff --git a/hw/riscv/sifive_u.c b/hw/riscv/sifive_u.c
index 901efab9d5bd..3aaee82f1f28 100644
--- a/hw/riscv/sifive_u.c
+++ b/hw/riscv/sifive_u.c
@@ -378,6 +378,7 @@ static void sifive_u_machine_init(MachineState *machine)
MemoryRegion *main_mem = g_new(MemoryRegion, 1);
MemoryRegion *flash0 = g_new(MemoryRegion, 1);
target_ulong start_addr = memmap[SIFIVE_U_DRAM].base;
+uint32_t start_addr_hi32 = 0x;
int i;
uint32_t fdt_load_addr;
uint64_t kernel_entry;
@@ -460,6 +461,9 @@ static void sifive_u_machine_init(MachineState *machine)
/* Compute the fdt load address in dram */
fdt_load_addr = riscv_load_fdt(memmap[SIFIVE_U_DRAM].base,
machine->ram_size, s->fdt);
+#if defined(TARGET_RISCV64)
+start_addr_hi32 = start_addr >> 32;
+#endif
/* reset vector */
uint32_t reset_vec[11] = {
@@ -476,7 +480,7 @@ static void sifive_u_machine_init(MachineState *machine)
#endif
0x00028067,/* jr t0 */
start_addr,/* start: .dword */
-0x,
+start_addr_hi32,
fdt_load_addr, /* fdt_laddr: .dword */
0x,
/* fw_dyn: */
--
2.26.2
Re: [PATCH v2 4/9] i386: hvf: Implement CPU kick
On Tue, Jun 30, 2020 at 06:04:23PM +0200, Paolo Bonzini wrote:
> On 30/06/20 17:50, Roman Bolshakov wrote:
> > On Tue, Jun 30, 2020 at 02:33:42PM +0200, Paolo Bonzini wrote:
> >> Can a signal interrupt hv_vcpu_run? If so you actually don't need
> >> hv_vcpu_interrupt at all.
> >
> > Existing signal masking and SIG_IPI didn't work IIRC when I tried to add
> > a primitive version of gdbstub support.
>
> You can try pthread_kill followed by hv_vcpu_interrupt if it doesn't.
> The signal would be delivered after return to userspace.
>
I looked at the signal setup for HVF again. I was wrong with regards to
SIG_IPI. It isn't delivered to vCPU because the signal is masked, this
fixes it:
diff --git a/target/i386/hvf/hvf.c b/target/i386/hvf/hvf.c
index d81f569aed..7bf05bca21 100644
--- a/target/i386/hvf/hvf.c
+++ b/target/i386/hvf/hvf.c
@@ -479,6 +479,7 @@ int hvf_init_vcpu(CPUState *cpu)
pthread_sigmask(SIG_BLOCK, NULL, );
sigdelset(, SIG_IPI);
+pthread_sigmask(SIG_SETMASK, , NULL);
init_emu();
init_decoder();
But the signal is delivered only after vmxexit, perhaps a sequence of
pthread_kill() and hv_vcpu_interrupt() is really needed.
So, there are two race windows on kernel-to-user border in v2: just
before checking the deadline and vmenter and just after vmxexit and
re-arm of preemption timer, that's two places where kicks could be lost.
The approach you proposed seems to address them.
Thanks,
Roman
> >> You can also require the preemption time, all
> >> processor that support HVF have it, but never set it by default. The
> >> deadline can be left at 0 all the time; instead, you toggle the bit in
> >> the pin-based controls. In the signal handler you do:
> >>
> >>if (atomic_xchg(>hvf_in_guest, false)) {
> >>wvmcs(cpu->hvf_fd, VMCS_PIN_BASED_CTLS,
> >> rvmcs(cpu->hvf_fd, VMCS_PIN_BASED_CTLS)
> >>| VMCS_PIN_BASED_CTLS_VMX_PREEMPT_TIMER);
> >>}
> >>
> >> In the main loop you do:
> >>
> >>atomic_set(>hvf_guest_mode, true);
> >>smp_mb();
> >>hv_vcpu_run(...);
> >>atomic_set(>hvf_guest_mode, false);
> >>
> >> and in the preemption timer vmexit handler:
> >>
> >>wvmcs(cpu->hvf_fd, VMCS_PIN_BASED_CTLS,
> >> rvmcs(cpu->hvf_fd, VMCS_PIN_BASED_CTLS)
> >>& ~VMCS_PIN_BASED_CTLS_VMX_PREEMPT_TIMER);
> >>
> >
Re: [PATCH v6 0/7] dwc-hsotg (aka dwc2) USB host controller emulation
Hi, > This patch series adds emulation for the dwc-hsotg USB controller, > which is used on the Raspberry Pi 3 and earlier, as well as a number > of other development boards. The main benefit for Raspberry Pi is that > this enables networking on these boards, since the network adapter is > attached via USB. Doesn't apply cleanly to master. Either some conflict due to other rpi3 changes merged meanwhile, or a dependency on unmerged patches. Can you check and resend? thanks, Gerd
Re: [PATCH] target-i386: remove gen_io_end
On 6/26/20 3:44 AM, Paolo Bonzini wrote: > Force the end of a translation block after an I/O instruction in > icount mode. For consistency, all CF_USE_ICOUNT code is kept in > disas_insn instead of having it in gen_ins and gen_outs. > > Signed-off-by: Paolo Bonzini > --- > target/i386/translate.c | 22 +- > 1 file changed, 9 insertions(+), 13 deletions(-) Reviewed-by: Richard Henderson r~
[PATCH v3 0/2] target mips: Misc fixes and improvements
A collection of pending fixes and improvements. v2->v3: - minor content and commit message changes v1->v2: - minor content and commit message changes Aleksandar Markovic (2): target/mips: Remove identical if/else branches MAINTAINERS: Adjust MIPS maintainership MAINTAINERS | 11 +-- target/mips/cp0_helper.c | 9 + 2 files changed, 6 insertions(+), 14 deletions(-) -- 2.20.1
[PATCH v3 2/2] MAINTAINERS: Adjust MIPS maintainership
Paul Burton and Aurelien Jarno removed for not being present. A polite email was sent to them with question whether they intend to actively participate, but there was no response. In cases where needed, other persons step in instead. Huacai Chen and Jiaxun Yang step in as new energy. CC: Paul Burton CC: Aurelien Jarno CC: Huacai Chen CC: Jiaxun Yang Signed-off-by: Aleksandar Markovic --- MAINTAINERS | 11 +-- 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/MAINTAINERS b/MAINTAINERS index 5d8acf8d31..6f96c03f3e 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -213,7 +213,8 @@ F: disas/microblaze.c MIPS TCG CPUs M: Aleksandar Markovic -R: Aurelien Jarno +R: Huacai Chen +R: Jiaxun Yang R: Aleksandar Rikalo S: Maintained F: target/mips/ @@ -377,6 +378,7 @@ F: target/arm/kvm.c MIPS KVM CPUs M: Aleksandar Markovic +M: Huacai Chen S: Odd Fixes F: target/mips/kvm.c @@ -1061,7 +1063,6 @@ F: hw/dma/rc4030.c Malta M: Aleksandar Markovic M: Philippe Mathieu-Daudé -R: Aurelien Jarno S: Maintained F: hw/isa/piix4.c F: hw/acpi/piix4.c @@ -1080,7 +1081,6 @@ F: hw/net/mipsnet.c R4000 M: Aleksandar Markovic -R: Aurelien Jarno R: Aleksandar Rikalo S: Obsolete F: hw/mips/r4k.c @@ -1103,8 +1103,7 @@ S: Maintained F: hw/intc/loongson_liointc.c Boston -M: Paul Burton -R: Aleksandar Rikalo +M: Aleksandar Rikalo S: Maintained F: hw/core/loader-fit.c F: hw/mips/boston.c @@ -2677,7 +2676,7 @@ F: disas/i386.c MIPS TCG target M: Aleksandar Markovic -R: Aurelien Jarno +R: Jiaxun Yang R: Aleksandar Rikalo S: Maintained F: tcg/mips/ -- 2.20.1
[PATCH v3 1/2] target/mips: Remove identical if/else branches
Remove the segment:
if (other_tc == other->current_tc) {
tccause = other->CP0_Cause;
} else {
tccause = other->CP0_Cause;
}
Original contributor can't remember what was his intention.
Fixes: 5a25ce9487 ("mips: Hook in more reg accesses via mttr/mftr")
Buglink: https://bugs.launchpad.net/qemu/+bug/1885718
Signed-off-by: Aleksandar Markovic
---
target/mips/cp0_helper.c | 9 +
1 file changed, 1 insertion(+), 8 deletions(-)
diff --git a/target/mips/cp0_helper.c b/target/mips/cp0_helper.c
index bbf12e4a97..de64add038 100644
--- a/target/mips/cp0_helper.c
+++ b/target/mips/cp0_helper.c
@@ -375,16 +375,9 @@ target_ulong helper_mftc0_entryhi(CPUMIPSState *env)
target_ulong helper_mftc0_cause(CPUMIPSState *env)
{
int other_tc = env->CP0_VPEControl & (0xff << CP0VPECo_TargTC);
-int32_t tccause;
CPUMIPSState *other = mips_cpu_map_tc(env, _tc);
-if (other_tc == other->current_tc) {
-tccause = other->CP0_Cause;
-} else {
-tccause = other->CP0_Cause;
-}
-
-return tccause;
+return other->CP0_Cause;
}
target_ulong helper_mftc0_status(CPUMIPSState *env)
--
2.20.1
Re: [PATCH] util/drm: make portable
On 7/1/20 8:03 PM, Gerd Hoffmann wrote:
> Given this isn't perforance critical at all lets avoid the non-portable
> d_type and use fstat instead to check whenever the file is a chardev.
>
> Signed-off-by: Gerd Hoffmann
Reported-by: David Carlier
Reviewed-by: Philippe Mathieu-Daudé
> ---
> util/drm.c | 17 -
> 1 file changed, 12 insertions(+), 5 deletions(-)
>
> diff --git a/util/drm.c b/util/drm.c
> index a23ff2453826..6ba87f34f4ee 100644
> --- a/util/drm.c
> +++ b/util/drm.c
> @@ -24,7 +24,8 @@ int qemu_drm_rendernode_open(const char *rendernode)
> {
> DIR *dir;
> struct dirent *e;
> -int r, fd;
> +struct stat st;
> +int r, fd, ret;
> char *p;
>
> if (rendernode) {
> @@ -38,10 +39,6 @@ int qemu_drm_rendernode_open(const char *rendernode)
>
> fd = -1;
> while ((e = readdir(dir))) {
> -if (e->d_type != DT_CHR) {
> -continue;
> -}
> -
> if (strncmp(e->d_name, "renderD", 7)) {
> continue;
> }
> @@ -53,6 +50,16 @@ int qemu_drm_rendernode_open(const char *rendernode)
> g_free(p);
> continue;
> }
> +
> +/* prefer fstat() over checking e->d_type == DT_CHR for
> + * portability reasons */
> +ret = fstat(r, );
> +if (ret < 0 || (st.st_mode & S_IFMT) != S_IFCHR) {
> +close(r);
> +g_free(p);
> +continue;
> +}
> +
> fd = r;
> g_free(p);
> break;
>
[RFC PATCH] cpus: Initialize current_cpu with the first vCPU created
We can run I/O access with the 'i' or 'o' HMP commands in the
monitor. These commands are expected to run on a vCPU. The
monitor is not a vCPU thread. To avoid crashing, initialize
the 'current_cpu' variable with the first vCPU created. The
command executed on the monitor will end using it.
This fixes:
$ cat << EOF| qemu-system-i386 -M q35 -nographic -serial none -monitor stdio
o/4 0xcf8 0x8400f841
o/4 0xcfc 0xaa215d6d
o/4 0x6d30 0x2ef8ffbe
o/1 0xb2 0x20
EOF
Segmentation fault (core dumped)
Thread 1 "qemu-system-i38" received signal SIGSEGV, Segmentation fault.
0x558946c7 in tcg_handle_interrupt (cpu=0x0, mask=64) at
accel/tcg/tcg-all.c:57
57 old_mask = cpu->interrupt_request;
(gdb) bt
#0 0x558946c7 in tcg_handle_interrupt (cpu=0x0, mask=64) at
accel/tcg/tcg-all.c:57
#1 0x558ed7d2 in cpu_interrupt (cpu=0x0, mask=64) at
include/hw/core/cpu.h:877
#2 0x558ee776 in ich9_apm_ctrl_changed (val=32, arg=0x56e2ff50)
at hw/isa/lpc_ich9.c:442
#3 0x55b47f96 in apm_ioport_writeb (opaque=0x56e308c0, addr=0,
val=32, size=1) at hw/isa/apm.c:44
#4 0x55879931 in memory_region_write_accessor (mr=0x56e308e0,
addr=0, value=0x7fffb9f8, size=1, shift=0, mask=255, attrs=...) at
memory.c:483
#5 0x55879b5a in access_with_adjusted_size (addr=0,
value=0x7fffb9f8, size=4, access_size_min=1, access_size_max=1, access_fn=
0x5587984e , mr=0x56e308e0,
attrs=...) at memory.c:544
#6 0x5587ca32 in memory_region_dispatch_write (mr=0x56e308e0,
addr=0, data=32, op=MO_32, attrs=...) at memory.c:1465
#7 0x5581b7e9 in flatview_write_continue (fv=0x5698a790,
addr=178, attrs=..., ptr=0x7fffbb84, len=4, addr1=0, l=4,
mr=0x56e308e0) at exec.c:3198
#8 0x5581b92e in flatview_write (fv=0x5698a790, addr=178,
attrs=..., buf=0x7fffbb84, len=4) at exec.c:3238
#9 0x5581bc81 in address_space_write (as=0x56441220
, addr=178, attrs=..., buf=0x7fffbb84, len=4) at
exec.c:3329
#10 0x55873f08 in cpu_outl (addr=178, val=32) at ioport.c:80
#11 0x5598a26a in hmp_ioport_write (mon=0x567621b0,
qdict=0x57702600) at monitor/misc.c:937
#12 0x55c9c5a5 in handle_hmp_command (mon=0x567621b0,
cmdline=0x5676aae1 "/1 0xb2 0x20") at monitor/hmp.c:1082
#13 0x55c99e02 in monitor_command_cb (opaque=0x567621b0,
cmdline=0x5676aae0 "o/1 0xb2 0x20", readline_opaque=0x0) at monitor/hmp.c:47
^
HMP command from monitor
Reported-by: Alexander Bulekov
Buglink: https://bugs.launchpad.net/qemu/+bug/1878645
Signed-off-by: Philippe Mathieu-Daudé
---
Cc: Bug 1878645 <1878...@bugs.launchpad.net>
RFC because I believe the correct fix is to NOT use current_cpu
out of cpus.c, at least use qemu_get_cpu(0) to get the first vCPU.
---
cpus.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/cpus.c b/cpus.c
index 41d1c5099f..1f6f43d221 100644
--- a/cpus.c
+++ b/cpus.c
@@ -2106,6 +2106,9 @@ void qemu_init_vcpu(CPUState *cpu)
{
MachineState *ms = MACHINE(qdev_get_machine());
+if (!current_cpu) {
+current_cpu = cpu;
+}
cpu->nr_cores = ms->smp.cores;
cpu->nr_threads = ms->smp.threads;
cpu->stopped = true;
--
2.21.3
[PATCH] ui: fix vc_chr_write call in text_console_do_init
In case the string doesn't fit into the buffer snprintf returns the size
it would need, so len can be larger than the buffer. Fix this by simply
using g_strdup_printf() instead of a static buffer.
Reported-by: Wenxiang Qian
Signed-off-by: Gerd Hoffmann
---
ui/console.c | 8
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/ui/console.c b/ui/console.c
index 865fa3263597..5ba0f21831cb 100644
--- a/ui/console.c
+++ b/ui/console.c
@@ -2184,12 +2184,12 @@ static void text_console_do_init(Chardev *chr,
DisplayState *ds)
text_console_resize(s);
if (chr->label) {
-char msg[128];
-int len;
+char *msg;
s->t_attrib.bgcol = QEMU_COLOR_BLUE;
-len = snprintf(msg, sizeof(msg), "%s console\r\n", chr->label);
-vc_chr_write(chr, (uint8_t *)msg, len);
+msg = g_strdup_printf("%s console\r\n", chr->label);
+vc_chr_write(chr, (uint8_t *)msg, strlen(msg));
+g_free(msg);
s->t_attrib = s->t_attrib_default;
}
--
2.18.4
Re: [PATCH v4 01/40] hw/isa: check for current_cpu before generating IRQ
On 7/1/20 7:48 PM, Philippe Mathieu-Daudé wrote:
> +MST/Igor for ICH9
>
> On 7/1/20 7:37 PM, Philippe Mathieu-Daudé wrote:
>> On 7/1/20 7:34 PM, Philippe Mathieu-Daudé wrote:
>>> +Paolo
>>>
>>> On 7/1/20 7:09 PM, Alex Bennée wrote:
Philippe Mathieu-Daudé writes:
> On 7/1/20 6:40 PM, Alex Bennée wrote:
>> Philippe Mathieu-Daudé writes:
>>
>>> On 7/1/20 3:56 PM, Alex Bennée wrote:
It's possible to trigger this function from qtest/monitor at which
point current_cpu won't point at the right place. Check it and
fall back to first_cpu if it's NULL.
Signed-off-by: Alex Bennée
Cc: Bug 1878645 <1878...@bugs.launchpad.net>
---
hw/isa/lpc_ich9.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/hw/isa/lpc_ich9.c b/hw/isa/lpc_ich9.c
index cd6e169d47a..791c878eb0b 100644
--- a/hw/isa/lpc_ich9.c
+++ b/hw/isa/lpc_ich9.c
@@ -439,7 +439,7 @@ static void ich9_apm_ctrl_changed(uint32_t val,
void *arg)
cpu_interrupt(cs, CPU_INTERRUPT_SMI);
}
} else {
-cpu_interrupt(current_cpu, CPU_INTERRUPT_SMI);
+cpu_interrupt(current_cpu ? current_cpu : first_cpu,
CPU_INTERRUPT_SMI);
>>>
>>> I'm not sure this change anything, as first_cpu is NULL when using
>>> qtest accelerator or none-machine, see 508b4ecc39 ("gdbstub.c: fix
>>> GDB connection segfault caused by empty machines").
>>
>> Good point - anyway feel free to ignore - it shouldn't have been in this
>> series. It was just some random experimentation I was doing when looking
>> at that bug.
>
> See commit c781a2cc42 ("hw/i386/vmport: Allow QTest use without
> crashing") for a similar approach, but here I was thinking about
> a more generic fix, not very intrusive:
>
> -- >8 --
> diff --git a/hw/isa/apm.c b/hw/isa/apm.c
> index bce266b957..809afeb3e4 100644
> --- a/hw/isa/apm.c
> +++ b/hw/isa/apm.c
> @@ -40,7 +40,7 @@ static void apm_ioport_writeb(void *opaque, hwaddr
> addr, uint64_t val,
> if (addr == 0) {
> apm->apmc = val;
>
> -if (apm->callback) {
> +if (apm->callback && !qtest_enabled()) {
> (apm->callback)(val, apm->arg);
> }
But the other failure mode reported on the bug thread was via the
monitor - so I'm not sure just checking for qtest catches that.
>>>
>>> Ah indeed.
>>>
>>> in exec.c:
>>>
>>> /* current CPU in the current thread. It is only valid inside
>>>cpu_exec() */
>>> __thread CPUState *current_cpu;
>>>
>>> Maybe we shouldn't use current_cpu out of exec.c...
>>
>> I meant, out of cpu_exec(), a cpu thread. Here we access it
>> from an I/O thread.
Ah! we are in the monitor thread... It makes sense there is not
current_cpu assigned :)
>
> ARM and S390X use:
>
> hw/arm/boot.c:460:ARMCPU *armcpu = ARM_CPU(qemu_get_cpu(0));
> hw/arm/virt.c:331:armcpu = ARM_CPU(qemu_get_cpu(0));
> hw/arm/virt.c:549:armcpu = ARM_CPU(qemu_get_cpu(0));
> hw/cpu/a15mpcore.c:69:cpuobj = OBJECT(qemu_get_cpu(0));
> hw/cpu/a9mpcore.c:76:cpuobj = OBJECT(qemu_get_cpu(0));
> target/s390x/cpu_models.c:155:cpu = S390_CPU(qemu_get_cpu(0));
> target/s390x/cpu_models.c:169:cpu = S390_CPU(qemu_get_cpu(0));
> target/s390x/cpu_models.c:184:cpu = S390_CPU(qemu_get_cpu(0));
> target/s390x/cpu_models.c:204:cpu = S390_CPU(qemu_get_cpu(0));
> target/s390x/cpu_models.c:218:cpu = S390_CPU(qemu_get_cpu(0));
>
> It seems odd that the ICH9 delivers the SMI on a random core.
> Usually the IRQ lines are wired to a particular unit.
>
[PATCH] util/drm: make portable
Given this isn't perforance critical at all lets avoid the non-portable
d_type and use fstat instead to check whenever the file is a chardev.
Signed-off-by: Gerd Hoffmann
---
util/drm.c | 17 -
1 file changed, 12 insertions(+), 5 deletions(-)
diff --git a/util/drm.c b/util/drm.c
index a23ff2453826..6ba87f34f4ee 100644
--- a/util/drm.c
+++ b/util/drm.c
@@ -24,7 +24,8 @@ int qemu_drm_rendernode_open(const char *rendernode)
{
DIR *dir;
struct dirent *e;
-int r, fd;
+struct stat st;
+int r, fd, ret;
char *p;
if (rendernode) {
@@ -38,10 +39,6 @@ int qemu_drm_rendernode_open(const char *rendernode)
fd = -1;
while ((e = readdir(dir))) {
-if (e->d_type != DT_CHR) {
-continue;
-}
-
if (strncmp(e->d_name, "renderD", 7)) {
continue;
}
@@ -53,6 +50,16 @@ int qemu_drm_rendernode_open(const char *rendernode)
g_free(p);
continue;
}
+
+/* prefer fstat() over checking e->d_type == DT_CHR for
+ * portability reasons */
+ret = fstat(r, );
+if (ret < 0 || (st.st_mode & S_IFMT) != S_IFCHR) {
+close(r);
+g_free(p);
+continue;
+}
+
fd = r;
g_free(p);
break;
--
2.18.4
[PATCH] cpus: Move CPU code from exec.c to cpus.c
This code was introduced with SMP support in commit 6a00d60127,
later commit 296af7c952 moved CPU parts to cpus.c but forgot this
code. Move now and simplify ifdef'ry.
Signed-off-by: Philippe Mathieu-Daudé
---
cpus.c | 18 ++
exec.c | 22 --
2 files changed, 18 insertions(+), 22 deletions(-)
diff --git a/cpus.c b/cpus.c
index 41d1c5099f..472686cbbc 100644
--- a/cpus.c
+++ b/cpus.c
@@ -92,6 +92,11 @@ static unsigned int throttle_percentage;
#define CPU_THROTTLE_PCT_MAX 99
#define CPU_THROTTLE_TIMESLICE_NS 1000
+CPUTailQ cpus = QTAILQ_HEAD_INITIALIZER(cpus);
+
+/* current CPU in the current thread. It is only valid inside cpu_exec() */
+__thread CPUState *current_cpu;
+
bool cpu_is_stopped(CPUState *cpu)
{
return cpu->stopped || !runstate_is_running();
@@ -134,6 +139,19 @@ static bool all_cpu_threads_idle(void)
return true;
}
+CPUState *qemu_get_cpu(int index)
+{
+CPUState *cpu;
+
+CPU_FOREACH(cpu) {
+if (cpu->cpu_index == index) {
+return cpu;
+}
+}
+
+return NULL;
+}
+
/***/
/* guest cycle counter */
diff --git a/exec.c b/exec.c
index 21926dc9c7..997b7db15f 100644
--- a/exec.c
+++ b/exec.c
@@ -98,12 +98,6 @@ AddressSpace address_space_memory;
static MemoryRegion io_mem_unassigned;
#endif
-CPUTailQ cpus = QTAILQ_HEAD_INITIALIZER(cpus);
-
-/* current CPU in the current thread. It is only valid inside
- cpu_exec() */
-__thread CPUState *current_cpu;
-
uintptr_t qemu_host_page_size;
intptr_t qemu_host_page_mask;
@@ -832,22 +826,6 @@ const VMStateDescription vmstate_cpu_common = {
}
};
-#endif
-
-CPUState *qemu_get_cpu(int index)
-{
-CPUState *cpu;
-
-CPU_FOREACH(cpu) {
-if (cpu->cpu_index == index) {
-return cpu;
-}
-}
-
-return NULL;
-}
-
-#if !defined(CONFIG_USER_ONLY)
void cpu_address_space_init(CPUState *cpu, int asidx,
const char *prefix, MemoryRegion *mr)
{
--
2.21.3
Re: [PATCH v4 01/40] hw/isa: check for current_cpu before generating IRQ
+MST/Igor for ICH9
On 7/1/20 7:37 PM, Philippe Mathieu-Daudé wrote:
> On 7/1/20 7:34 PM, Philippe Mathieu-Daudé wrote:
>> +Paolo
>>
>> On 7/1/20 7:09 PM, Alex Bennée wrote:
>>> Philippe Mathieu-Daudé writes:
On 7/1/20 6:40 PM, Alex Bennée wrote:
> Philippe Mathieu-Daudé writes:
>
>> On 7/1/20 3:56 PM, Alex Bennée wrote:
>>> It's possible to trigger this function from qtest/monitor at which
>>> point current_cpu won't point at the right place. Check it and
>>> fall back to first_cpu if it's NULL.
>>>
>>> Signed-off-by: Alex Bennée
>>> Cc: Bug 1878645 <1878...@bugs.launchpad.net>
>>> ---
>>> hw/isa/lpc_ich9.c | 2 +-
>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/hw/isa/lpc_ich9.c b/hw/isa/lpc_ich9.c
>>> index cd6e169d47a..791c878eb0b 100644
>>> --- a/hw/isa/lpc_ich9.c
>>> +++ b/hw/isa/lpc_ich9.c
>>> @@ -439,7 +439,7 @@ static void ich9_apm_ctrl_changed(uint32_t val,
>>> void *arg)
>>> cpu_interrupt(cs, CPU_INTERRUPT_SMI);
>>> }
>>> } else {
>>> -cpu_interrupt(current_cpu, CPU_INTERRUPT_SMI);
>>> +cpu_interrupt(current_cpu ? current_cpu : first_cpu,
>>> CPU_INTERRUPT_SMI);
>>
>> I'm not sure this change anything, as first_cpu is NULL when using
>> qtest accelerator or none-machine, see 508b4ecc39 ("gdbstub.c: fix
>> GDB connection segfault caused by empty machines").
>
> Good point - anyway feel free to ignore - it shouldn't have been in this
> series. It was just some random experimentation I was doing when looking
> at that bug.
See commit c781a2cc42 ("hw/i386/vmport: Allow QTest use without
crashing") for a similar approach, but here I was thinking about
a more generic fix, not very intrusive:
-- >8 --
diff --git a/hw/isa/apm.c b/hw/isa/apm.c
index bce266b957..809afeb3e4 100644
--- a/hw/isa/apm.c
+++ b/hw/isa/apm.c
@@ -40,7 +40,7 @@ static void apm_ioport_writeb(void *opaque, hwaddr
addr, uint64_t val,
if (addr == 0) {
apm->apmc = val;
-if (apm->callback) {
+if (apm->callback && !qtest_enabled()) {
(apm->callback)(val, apm->arg);
}
>>>
>>> But the other failure mode reported on the bug thread was via the
>>> monitor - so I'm not sure just checking for qtest catches that.
>>
>> Ah indeed.
>>
>> in exec.c:
>>
>> /* current CPU in the current thread. It is only valid inside
>>cpu_exec() */
>> __thread CPUState *current_cpu;
>>
>> Maybe we shouldn't use current_cpu out of exec.c...
>
> I meant, out of cpu_exec(), a cpu thread. Here we access it
> from an I/O thread.
ARM and S390X use:
hw/arm/boot.c:460:ARMCPU *armcpu = ARM_CPU(qemu_get_cpu(0));
hw/arm/virt.c:331:armcpu = ARM_CPU(qemu_get_cpu(0));
hw/arm/virt.c:549:armcpu = ARM_CPU(qemu_get_cpu(0));
hw/cpu/a15mpcore.c:69:cpuobj = OBJECT(qemu_get_cpu(0));
hw/cpu/a9mpcore.c:76:cpuobj = OBJECT(qemu_get_cpu(0));
target/s390x/cpu_models.c:155:cpu = S390_CPU(qemu_get_cpu(0));
target/s390x/cpu_models.c:169:cpu = S390_CPU(qemu_get_cpu(0));
target/s390x/cpu_models.c:184:cpu = S390_CPU(qemu_get_cpu(0));
target/s390x/cpu_models.c:204:cpu = S390_CPU(qemu_get_cpu(0));
target/s390x/cpu_models.c:218:cpu = S390_CPU(qemu_get_cpu(0));
It seems odd that the ICH9 delivers the SMI on a random core.
Usually the IRQ lines are wired to a particular unit.
Re: [REPORT] [GSoC - TCG Continuous Benchmarking] [#2] Dissecting QEMU Into Three Main Parts
On Wed, Jul 1, 2020 at 5:42 PM Alex Bennée wrote: > > > Ahmed Karaman writes: > > > On Mon, Jun 29, 2020 at 6:03 PM Alex Bennée wrote: > >> > >> Assuming your test case is constant execution (i.e. runs the same each > >> time) you could run in through a plugins build to extract the number of > >> guest instructions, e.g.: > >> > >> ./aarch64-linux-user/qemu-aarch64 -plugin tests/plugin/libinsn.so -d > >> plugin ./tests/tcg/aarch64-linux-user/sha1 > >> SHA1=15dd99a1991e0b3826fede3deffc1feba42278e6 > >> insns: 158603512 > >> > >> -- > >> Alex Bennée > > > > Hi Mr. Alex, > > I've created a plugins build as you've said using "--enable-plugins" option. > > I've searched for "libinsn.so" plugin that you've mentioned in your > > command but it isn't in that path. > > make plugins > > and you should find them in tests/plugins/ > > > > > Are there any other options that I should configure my build with? > > Thanks in advance. > > > > Regards, > > Ahmed Karaman > > > -- > Alex Bennée Thanks a lot.
[PATCH v2 0/3] Fix couple of issues with AMD topology
This series fixes couple of issues with recent topology related code. 1. Maintain consistency while building the topology. Use the numa information passed from user to build the apic_id. 2. Fix uninitialized memory with -device and CPU hotplug Here are the discussion thread. https://lore.kernel.org/qemu-devel/159164739269.20543.3074052993891532749.st...@naples-babu.amd.com/ https://lore.kernel.org/qemu-devel/20200602175212.gh577...@habkost.net/ Fixes: Link: https://bugzilla.redhat.com/show_bug.cgi?id=1828750 --- v2: - Used the numa information from CpuInstanceProperties for building the apic_id suggested by Igor. - Also did some minor code re-aarangement to take care of changes. - Dropped the patch "Simplify CPUID_8000_001E" from v1. Will send it later. v1: https://lore.kernel.org/qemu-devel/159164739269.20543.3074052993891532749.st...@naples-babu.amd.com Babu Moger (3): hw/i386: Initialize topo_ids from CpuInstanceProperties hw/i386: Build apic_id from CpuInstanceProperties hw/386: Fix uninitialized memory with -device and CPU hotplug hw/i386/pc.c | 16 +++- hw/i386/x86.c | 19 +-- include/hw/i386/topology.h | 33 ++--- include/hw/i386/x86.h |6 -- tests/test-x86-cpuid.c | 39 --- 5 files changed, 82 insertions(+), 31 deletions(-) -- Signature
Re: [PATCH v3 0/5] hw/mips/malta: Add the 'malta-strict' machine, matching Malta hardware
Aleksandar, On 2020-06-30 23:54, Aleksandar Markovic wrote: > As, in a very clear way, evidenced from the previous versions of this > series, this series real goal was not not to create some new > "malta-strict" machine, but to prepare path to creation of some > imagined "malta-unleashed" machine which will, to the contrary of > proclaimed goal, create a machine that could never exist in reality. > That is why I can't accept this series. I do not understand your opposition to this, and why it is an issue to support more than 2GiB of RAM for such a board. Supporting more than 2GiB of memory doesn't prevent people to emulate a real Malta board with less memory. In addition to that, the Malta board in QEMU has been supporting for many years more than the maximum 256MiB that is possible on a physical board. The QEMU version also supports way more than CPU variants than the physical board. In other word the existing malta machine in QEMU is already a "malta-unleashed". And these possibilities have been used by MIPS* employees to develop MIPS R6 based distributions. Limiting the board in terms of RAM, CPU or virtio support would just make our users life more difficult for no gain. Regards, Aurelien * By MIPS employee, I mean persons that have been employed by companies owning MIPS over the last few years, including Imagination Technologies and Wave Computing. > уто, 30. јун 2020. у 21:58 Philippe Mathieu-Daudé је > написао/ла: > > > > Hi, > > > > This series add a new 'malta-strict' machine, that aims to properly > > model the real hardware (which is not what the current 'malta' > > machine models). > > > > Since v2: > > - Initialize missing malta_machine_types::class_size > > - Remove RFC patch that confuses Aleksandar > > - Added examples of 'malta-strict' use > > > > $ git backport-diff -u v2 > > Key: > > [] : patches are identical > > [] : number of functional differences between upstream/downstream patch > > [down] : patch is downstream-only > > The flags [FC] indicate (F)unctional and (C)ontextual differences, > > respectively > > > > 001/5:[] [--] 'hw/mips/malta: Trivial code movement' > > 002/5:[] [--] 'hw/mips/malta: Register the machine as a TypeInfo' > > 003/5:[0001] [FC] 'hw/mips/malta: Introduce MaltaMachineClass::max_ramsize' > > 004/5:[] [--] 'hw/mips/malta: Introduce the 'malta-strict' machine' > > 005/5:[] [--] 'hw/mips/malta: Verify malta-strict machine uses correct > > DIMM sizes' > > > > Philippe Mathieu-Daudé (5): > > hw/mips/malta: Trivial code movement > > hw/mips/malta: Register the machine as a TypeInfo > > hw/mips/malta: Introduce MaltaMachineClass::max_ramsize > > hw/mips/malta: Introduce the 'malta-strict' machine > > hw/mips/malta: Verify malta-strict machine uses correct DIMM sizes > > > > hw/mips/malta.c | 105 +--- > > 1 file changed, 91 insertions(+), 14 deletions(-) > > > > -- > > 2.21.3 > > > > > -- Aurelien Jarno GPG: 4096R/1DDD8C9B aurel...@aurel32.net http://www.aurel32.net signature.asc Description: PGP signature
Re: [PATCH v4 01/40] hw/isa: check for current_cpu before generating IRQ
On 7/1/20 7:34 PM, Philippe Mathieu-Daudé wrote:
> +Paolo
>
> On 7/1/20 7:09 PM, Alex Bennée wrote:
>> Philippe Mathieu-Daudé writes:
>>> On 7/1/20 6:40 PM, Alex Bennée wrote:
Philippe Mathieu-Daudé writes:
> On 7/1/20 3:56 PM, Alex Bennée wrote:
>> It's possible to trigger this function from qtest/monitor at which
>> point current_cpu won't point at the right place. Check it and
>> fall back to first_cpu if it's NULL.
>>
>> Signed-off-by: Alex Bennée
>> Cc: Bug 1878645 <1878...@bugs.launchpad.net>
>> ---
>> hw/isa/lpc_ich9.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/hw/isa/lpc_ich9.c b/hw/isa/lpc_ich9.c
>> index cd6e169d47a..791c878eb0b 100644
>> --- a/hw/isa/lpc_ich9.c
>> +++ b/hw/isa/lpc_ich9.c
>> @@ -439,7 +439,7 @@ static void ich9_apm_ctrl_changed(uint32_t val, void
>> *arg)
>> cpu_interrupt(cs, CPU_INTERRUPT_SMI);
>> }
>> } else {
>> -cpu_interrupt(current_cpu, CPU_INTERRUPT_SMI);
>> +cpu_interrupt(current_cpu ? current_cpu : first_cpu,
>> CPU_INTERRUPT_SMI);
>
> I'm not sure this change anything, as first_cpu is NULL when using
> qtest accelerator or none-machine, see 508b4ecc39 ("gdbstub.c: fix
> GDB connection segfault caused by empty machines").
Good point - anyway feel free to ignore - it shouldn't have been in this
series. It was just some random experimentation I was doing when looking
at that bug.
>>>
>>> See commit c781a2cc42 ("hw/i386/vmport: Allow QTest use without
>>> crashing") for a similar approach, but here I was thinking about
>>> a more generic fix, not very intrusive:
>>>
>>> -- >8 --
>>> diff --git a/hw/isa/apm.c b/hw/isa/apm.c
>>> index bce266b957..809afeb3e4 100644
>>> --- a/hw/isa/apm.c
>>> +++ b/hw/isa/apm.c
>>> @@ -40,7 +40,7 @@ static void apm_ioport_writeb(void *opaque, hwaddr
>>> addr, uint64_t val,
>>> if (addr == 0) {
>>> apm->apmc = val;
>>>
>>> -if (apm->callback) {
>>> +if (apm->callback && !qtest_enabled()) {
>>> (apm->callback)(val, apm->arg);
>>> }
>>
>> But the other failure mode reported on the bug thread was via the
>> monitor - so I'm not sure just checking for qtest catches that.
>
> Ah indeed.
>
> in exec.c:
>
> /* current CPU in the current thread. It is only valid inside
>cpu_exec() */
> __thread CPUState *current_cpu;
>
> Maybe we shouldn't use current_cpu out of exec.c...
I meant, out of cpu_exec(), a cpu thread. Here we access it
from an I/O thread.
Re: [PATCH v4 01/40] hw/isa: check for current_cpu before generating IRQ
+Paolo
On 7/1/20 7:09 PM, Alex Bennée wrote:
> Philippe Mathieu-Daudé writes:
>> On 7/1/20 6:40 PM, Alex Bennée wrote:
>>> Philippe Mathieu-Daudé writes:
>>>
On 7/1/20 3:56 PM, Alex Bennée wrote:
> It's possible to trigger this function from qtest/monitor at which
> point current_cpu won't point at the right place. Check it and
> fall back to first_cpu if it's NULL.
>
> Signed-off-by: Alex Bennée
> Cc: Bug 1878645 <1878...@bugs.launchpad.net>
> ---
> hw/isa/lpc_ich9.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/hw/isa/lpc_ich9.c b/hw/isa/lpc_ich9.c
> index cd6e169d47a..791c878eb0b 100644
> --- a/hw/isa/lpc_ich9.c
> +++ b/hw/isa/lpc_ich9.c
> @@ -439,7 +439,7 @@ static void ich9_apm_ctrl_changed(uint32_t val, void
> *arg)
> cpu_interrupt(cs, CPU_INTERRUPT_SMI);
> }
> } else {
> -cpu_interrupt(current_cpu, CPU_INTERRUPT_SMI);
> +cpu_interrupt(current_cpu ? current_cpu : first_cpu,
> CPU_INTERRUPT_SMI);
I'm not sure this change anything, as first_cpu is NULL when using
qtest accelerator or none-machine, see 508b4ecc39 ("gdbstub.c: fix
GDB connection segfault caused by empty machines").
>>>
>>> Good point - anyway feel free to ignore - it shouldn't have been in this
>>> series. It was just some random experimentation I was doing when looking
>>> at that bug.
>>
>> See commit c781a2cc42 ("hw/i386/vmport: Allow QTest use without
>> crashing") for a similar approach, but here I was thinking about
>> a more generic fix, not very intrusive:
>>
>> -- >8 --
>> diff --git a/hw/isa/apm.c b/hw/isa/apm.c
>> index bce266b957..809afeb3e4 100644
>> --- a/hw/isa/apm.c
>> +++ b/hw/isa/apm.c
>> @@ -40,7 +40,7 @@ static void apm_ioport_writeb(void *opaque, hwaddr
>> addr, uint64_t val,
>> if (addr == 0) {
>> apm->apmc = val;
>>
>> -if (apm->callback) {
>> +if (apm->callback && !qtest_enabled()) {
>> (apm->callback)(val, apm->arg);
>> }
>
> But the other failure mode reported on the bug thread was via the
> monitor - so I'm not sure just checking for qtest catches that.
Ah indeed.
in exec.c:
/* current CPU in the current thread. It is only valid inside
cpu_exec() */
__thread CPUState *current_cpu;
Maybe we shouldn't use current_cpu out of exec.c...
[PATCH v2 3/3] hw/386: Fix uninitialized memory with -device and CPU hotplug
Noticed the following command failure while testing CPU hotplug.
$ qemu-system-x86_64 -machine q35,accel=kvm -smp 1,maxcpus=2,
cores=1, threads=1,sockets=2 -cpu EPYC -device EPYC-x86_64-
cpu,core-id=0,socket-id=1,thread-id=0
qemu-system-x86_64: -device EPYC-x86_64-cpu,core-id=0,socket-id=1,
thread-id=0: Invalid CPU [socket: 21855, die: 0, core: 0, thread: 0]
with APIC ID 21855, valid index range 0:1
This happens because APIC ID is calculated using uninitialized memory.
This is happening after the addition of new field node_id in X86CPUTopoIDs
structure. The node_id field is uninitialized while calling
apicid_from_topo_ids. The problem is discussed in the thread below.
https://lore.kernel.org/qemu-devel/20200602171838.gg577...@habkost.net/
Fix the problem by initializing the node_id from the device being added.
Fixes:
Link: https://bugzilla.redhat.com/show_bug.cgi?id=1828750
Signed-off-by: Babu Moger
---
hw/i386/pc.c | 10 ++
1 file changed, 10 insertions(+)
diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index e613b2299f..aa9fb48834 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -1553,6 +1553,15 @@ static void pc_cpu_pre_plug(HotplugHandler *hotplug_dev,
cpu->die_id = 0;
}
+/*
+ * If node_id is not set, initialize it to zero for now. If the user
+ * does not pass the correct node in case of numa configuration, it
+ * will be rejected eventually.
+ */
+if (cpu->node_id < 0) {
+cpu->node_id = 0;
+}
+
if (cpu->socket_id < 0) {
error_setg(errp, "CPU socket-id is not set");
return;
@@ -1587,6 +1596,7 @@ static void pc_cpu_pre_plug(HotplugHandler *hotplug_dev,
}
topo_ids.pkg_id = cpu->socket_id;
+topo_ids.node_id = cpu->node_id;
topo_ids.die_id = cpu->die_id;
topo_ids.core_id = cpu->core_id;
topo_ids.smt_id = cpu->thread_id;
[PATCH v2 2/3] hw/i386: Build apic_id from CpuInstanceProperties
Build apic_id from CpuInstanceProperties if numa configured.
Use the node_id from user provided numa information. This
will avoid conflicts between numa information and apic_id
generated.
Re-arranged the code little bit to make sure CpuInstanceProperties
is initialized before calling.
Signed-off-by: Babu Moger
---
hw/i386/pc.c |6 +-
hw/i386/x86.c | 19 +--
include/hw/i386/topology.h | 14 +++---
include/hw/i386/x86.h |6 --
tests/test-x86-cpuid.c | 39 ---
5 files changed, 53 insertions(+), 31 deletions(-)
diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index d103b8c0ab..e613b2299f 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -800,13 +800,17 @@ void pc_smp_parse(MachineState *ms, QemuOpts *opts)
void pc_hot_add_cpu(MachineState *ms, const int64_t id, Error **errp)
{
X86MachineState *x86ms = X86_MACHINE(ms);
-int64_t apic_id = x86_cpu_apic_id_from_index(x86ms, id);
+CpuInstanceProperties props;
+int64_t apic_id;
Error *local_err = NULL;
if (id < 0) {
error_setg(errp, "Invalid CPU id: %" PRIi64, id);
return;
}
+props = ms->possible_cpus->cpus[id].props;
+
+apic_id = x86_cpu_apic_id_from_index(x86ms, id, props);
if (apic_id >= ACPI_CPU_HOTPLUG_ID_LIMIT) {
error_setg(errp, "Unable to add CPU: %" PRIi64
diff --git a/hw/i386/x86.c b/hw/i386/x86.c
index 34229b45c7..7554416ae0 100644
--- a/hw/i386/x86.c
+++ b/hw/i386/x86.c
@@ -93,7 +93,8 @@ static void x86_set_epyc_topo_handlers(MachineState *machine)
* all CPUs up to max_cpus.
*/
uint32_t x86_cpu_apic_id_from_index(X86MachineState *x86ms,
-unsigned int cpu_index)
+unsigned int cpu_index,
+CpuInstanceProperties props)
{
X86MachineClass *x86mc = X86_MACHINE_GET_CLASS(x86ms);
X86CPUTopoInfo topo_info;
@@ -102,7 +103,7 @@ uint32_t x86_cpu_apic_id_from_index(X86MachineState *x86ms,
init_topo_info(_info, x86ms);
-correct_id = x86ms->apicid_from_cpu_idx(_info, cpu_index);
+correct_id = x86ms->apicid_from_cpu_idx(_info, cpu_index, props);
if (x86mc->compat_apic_id_mode) {
if (cpu_index != correct_id && !warned && !qtest_enabled()) {
error_report("APIC IDs set in compatibility mode, "
@@ -136,6 +137,8 @@ void x86_cpus_init(X86MachineState *x86ms, int
default_cpu_version)
const CPUArchIdList *possible_cpus;
MachineState *ms = MACHINE(x86ms);
MachineClass *mc = MACHINE_GET_CLASS(x86ms);
+CpuInstanceProperties props;
+
/* Check for apicid encoding */
if (cpu_x86_use_epyc_apic_id_encoding(ms->cpu_type)) {
@@ -144,6 +147,8 @@ void x86_cpus_init(X86MachineState *x86ms, int
default_cpu_version)
x86_cpu_set_default_version(default_cpu_version);
+possible_cpus = mc->possible_cpu_arch_ids(ms);
+
/*
* Calculates the limit to CPU APIC ID values
*
@@ -152,13 +157,15 @@ void x86_cpus_init(X86MachineState *x86ms, int
default_cpu_version)
*
* This is used for FW_CFG_MAX_CPUS. See comments on fw_cfg_arch_create().
*/
-x86ms->apic_id_limit = x86_cpu_apic_id_from_index(x86ms,
- ms->smp.max_cpus - 1) +
1;
-possible_cpus = mc->possible_cpu_arch_ids(ms);
+props = ms->possible_cpus->cpus[ms->smp.max_cpus - 1].props;
+x86ms->apic_id_limit = x86_cpu_apic_id_from_index(x86ms,
+ ms->smp.max_cpus - 1,
+ props) + 1;
for (i = 0; i < ms->possible_cpus->len; i++) {
+props = ms->possible_cpus->cpus[i].props;
ms->possible_cpus->cpus[i].arch_id =
-x86_cpu_apic_id_from_index(x86ms, i);
+x86_cpu_apic_id_from_index(x86ms, i, props);
}
for (i = 0; i < ms->smp.cpus; i++) {
diff --git a/include/hw/i386/topology.h b/include/hw/i386/topology.h
index 7cb21e9c82..a800fc905f 100644
--- a/include/hw/i386/topology.h
+++ b/include/hw/i386/topology.h
@@ -221,10 +221,17 @@ static inline void x86_init_topo_ids(X86CPUTopoInfo
*topo_info,
* 'cpu_index' is a sequential, contiguous ID for the CPU.
*/
static inline apic_id_t x86_apicid_from_cpu_idx_epyc(X86CPUTopoInfo *topo_info,
- unsigned cpu_index)
+ unsigned cpu_index,
+ CpuInstanceProperties
props)
{
X86CPUTopoIDs topo_ids;
-x86_topo_ids_from_idx_epyc(topo_info, cpu_index, _ids);
+
+if (props.has_node_id) {
+x86_init_topo_ids(topo_info, props, _ids);
+} else {
+x86_topo_ids_from_idx_epyc(topo_info, cpu_index, _ids);
+}
+
return x86_apicid_from_topo_ids_epyc(topo_info, _ids);
}
/* Make APIC ID for
[PATCH v2 1/3] hw/i386: Initialize topo_ids from CpuInstanceProperties
This is in preparation to build the apic_id from user
provided topology information.
Signed-off-by: Babu Moger
---
include/hw/i386/topology.h | 19 +++
1 file changed, 19 insertions(+)
diff --git a/include/hw/i386/topology.h b/include/hw/i386/topology.h
index 07239f95f4..7cb21e9c82 100644
--- a/include/hw/i386/topology.h
+++ b/include/hw/i386/topology.h
@@ -40,6 +40,7 @@
#include "qemu/bitops.h"
+#include "qapi/qapi-types-machine.h"
/* APIC IDs can be 32-bit, but beware: APIC IDs > 255 require x2APIC support
*/
@@ -196,6 +197,24 @@ static inline void x86_topo_ids_from_apicid_epyc(apic_id_t
apicid,
topo_ids->pkg_id = apicid >> apicid_pkg_offset_epyc(topo_info);
}
+
+/*
+ * Initialize topo_ids from CpuInstanceProperties
+ * node_id in CpuInstanceProperties(or in CPU device) is a sequential
+ * number, but while building the topology we need to separate it for
+ * each socket(mod nodes_per_pkg).
+ */
+static inline void x86_init_topo_ids(X86CPUTopoInfo *topo_info,
+ CpuInstanceProperties props,
+ X86CPUTopoIDs *topo_ids)
+{
+topo_ids->smt_id = props.has_thread_id ? props.thread_id : 0;
+topo_ids->core_id = props.has_core_id ? props.core_id : 0;
+topo_ids->die_id = props.has_die_id ? props.die_id : 0;
+topo_ids->node_id = props.has_node_id ?
+props.node_id % MAX(topo_info->nodes_per_pkg, 1) : 0;
+topo_ids->pkg_id = props.has_socket_id ? props.socket_id : 0;
+}
/*
* Make APIC ID for the CPU 'cpu_index'
*
[PATCH v12 61/61] target/riscv: configure and turn on vector extension from command line
Vector extension is default off. The only way to use vector extension is
1. use cpu rv32 or rv64
2. turn on it by command line
"-cpu rv64,x-v=true,vlen=128,elen=64,vext_spec=v0.7.1".
vlen is the vector register length, default value is 128 bit.
elen is the max operator size in bits, default value is 64 bit.
vext_spec is the vector specification version, default value is v0.7.1.
These properties can be specified with other values.
Signed-off-by: LIU Zhiwei
Reviewed-by: Alistair Francis
Reviewed-by: Richard Henderson
---
target/riscv/cpu.c | 43 +++
target/riscv/cpu.h | 4 +++-
2 files changed, 46 insertions(+), 1 deletion(-)
diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index d525cfb687..228b9bdb5d 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -430,6 +430,45 @@ static void riscv_cpu_realize(DeviceState *dev, Error
**errp)
if (cpu->cfg.ext_h) {
target_misa |= RVH;
}
+if (cpu->cfg.ext_v) {
+target_misa |= RVV;
+if (!is_power_of_2(cpu->cfg.vlen)) {
+error_setg(errp,
+"Vector extension VLEN must be power of 2");
+return;
+}
+if (cpu->cfg.vlen > RV_VLEN_MAX || cpu->cfg.vlen < 128) {
+error_setg(errp,
+"Vector extension implementation only supports VLEN "
+"in the range [128, %d]", RV_VLEN_MAX);
+return;
+}
+if (!is_power_of_2(cpu->cfg.elen)) {
+error_setg(errp,
+"Vector extension ELEN must be power of 2");
+return;
+}
+if (cpu->cfg.elen > 64 || cpu->cfg.vlen < 8) {
+error_setg(errp,
+"Vector extension implementation only supports ELEN "
+"in the range [8, 64]");
+return;
+}
+if (cpu->cfg.vext_spec) {
+if (!g_strcmp0(cpu->cfg.vext_spec, "v0.7.1")) {
+vext_version = VEXT_VERSION_0_07_1;
+} else {
+error_setg(errp,
+ "Unsupported vector spec version '%s'",
+ cpu->cfg.vext_spec);
+return;
+}
+} else {
+qemu_log("vector verison is not specified, "
+"use the default value v0.7.1\n");
+}
+set_vext_version(env, vext_version);
+}
set_misa(env, RVXLEN | target_misa);
}
@@ -469,10 +508,14 @@ static Property riscv_cpu_properties[] = {
DEFINE_PROP_BOOL("u", RISCVCPU, cfg.ext_u, true),
/* This is experimental so mark with 'x-' */
DEFINE_PROP_BOOL("x-h", RISCVCPU, cfg.ext_h, false),
+DEFINE_PROP_BOOL("x-v", RISCVCPU, cfg.ext_v, false),
DEFINE_PROP_BOOL("Counters", RISCVCPU, cfg.ext_counters, true),
DEFINE_PROP_BOOL("Zifencei", RISCVCPU, cfg.ext_ifencei, true),
DEFINE_PROP_BOOL("Zicsr", RISCVCPU, cfg.ext_icsr, true),
DEFINE_PROP_STRING("priv_spec", RISCVCPU, cfg.priv_spec),
+DEFINE_PROP_STRING("vext_spec", RISCVCPU, cfg.vext_spec),
+DEFINE_PROP_UINT16("vlen", RISCVCPU, cfg.vlen, 128),
+DEFINE_PROP_UINT16("elen", RISCVCPU, cfg.elen, 64),
DEFINE_PROP_BOOL("mmu", RISCVCPU, cfg.mmu, true),
DEFINE_PROP_BOOL("pmp", RISCVCPU, cfg.pmp, true),
DEFINE_PROP_END_OF_LIST(),
diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
index 0ad51c6580..eef20ca6e5 100644
--- a/target/riscv/cpu.h
+++ b/target/riscv/cpu.h
@@ -92,7 +92,7 @@ typedef struct CPURISCVState CPURISCVState;
#include "pmp.h"
-#define RV_VLEN_MAX 512
+#define RV_VLEN_MAX 256
FIELD(VTYPE, VLMUL, 0, 2)
FIELD(VTYPE, VSEW, 2, 3)
@@ -279,12 +279,14 @@ typedef struct RISCVCPU {
bool ext_s;
bool ext_u;
bool ext_h;
+bool ext_v;
bool ext_counters;
bool ext_ifencei;
bool ext_icsr;
char *priv_spec;
char *user_spec;
+char *vext_spec;
uint16_t vlen;
uint16_t elen;
bool mmu;
--
2.23.0
[PATCH v12 60/61] target/riscv: vector compress instruction
Signed-off-by: LIU Zhiwei
Reviewed-by: Richard Henderson
---
target/riscv/helper.h | 5
target/riscv/insn32.decode | 1 +
target/riscv/insn_trans/trans_rvv.inc.c | 32 +
target/riscv/vector_helper.c| 26
4 files changed, 64 insertions(+)
diff --git a/target/riscv/helper.h b/target/riscv/helper.h
index eca1ab541b..acc298219d 100644
--- a/target/riscv/helper.h
+++ b/target/riscv/helper.h
@@ -1145,3 +1145,8 @@ DEF_HELPER_6(vrgather_vx_b, void, ptr, ptr, tl, ptr, env,
i32)
DEF_HELPER_6(vrgather_vx_h, void, ptr, ptr, tl, ptr, env, i32)
DEF_HELPER_6(vrgather_vx_w, void, ptr, ptr, tl, ptr, env, i32)
DEF_HELPER_6(vrgather_vx_d, void, ptr, ptr, tl, ptr, env, i32)
+
+DEF_HELPER_6(vcompress_vm_b, void, ptr, ptr, ptr, ptr, env, i32)
+DEF_HELPER_6(vcompress_vm_h, void, ptr, ptr, ptr, ptr, env, i32)
+DEF_HELPER_6(vcompress_vm_w, void, ptr, ptr, ptr, ptr, env, i32)
+DEF_HELPER_6(vcompress_vm_d, void, ptr, ptr, ptr, ptr, env, i32)
diff --git a/target/riscv/insn32.decode b/target/riscv/insn32.decode
index 80d5ff74a9..bdd8563067 100644
--- a/target/riscv/insn32.decode
+++ b/target/riscv/insn32.decode
@@ -577,6 +577,7 @@ vslide1down_vx 00 . . . 110 . 1010111 @r_vm
vrgather_vv 001100 . . . 000 . 1010111 @r_vm
vrgather_vx 001100 . . . 100 . 1010111 @r_vm
vrgather_vi 001100 . . . 011 . 1010111 @r_vm
+vcompress_vm010111 - . . 010 . 1010111 @r
vsetvli 0 ... . 111 . 1010111 @r2_zimm
vsetvl 100 . . 111 . 1010111 @r
diff --git a/target/riscv/insn_trans/trans_rvv.inc.c
b/target/riscv/insn_trans/trans_rvv.inc.c
index c0b7745a63..dc333e6a91 100644
--- a/target/riscv/insn_trans/trans_rvv.inc.c
+++ b/target/riscv/insn_trans/trans_rvv.inc.c
@@ -2854,3 +2854,35 @@ static bool trans_vrgather_vi(DisasContext *s, arg_rmrr
*a)
}
return true;
}
+
+/* Vector Compress Instruction */
+static bool vcompress_vm_check(DisasContext *s, arg_r *a)
+{
+return (vext_check_isa_ill(s) &&
+vext_check_reg(s, a->rd, false) &&
+vext_check_reg(s, a->rs2, false) &&
+vext_check_overlap_group(a->rd, 1 << s->lmul, a->rs1, 1) &&
+(a->rd != a->rs2));
+}
+
+static bool trans_vcompress_vm(DisasContext *s, arg_r *a)
+{
+if (vcompress_vm_check(s, a)) {
+uint32_t data = 0;
+static gen_helper_gvec_4_ptr * const fns[4] = {
+gen_helper_vcompress_vm_b, gen_helper_vcompress_vm_h,
+gen_helper_vcompress_vm_w, gen_helper_vcompress_vm_d,
+};
+TCGLabel *over = gen_new_label();
+tcg_gen_brcondi_tl(TCG_COND_EQ, cpu_vl, 0, over);
+
+data = FIELD_DP32(data, VDATA, MLEN, s->mlen);
+data = FIELD_DP32(data, VDATA, LMUL, s->lmul);
+tcg_gen_gvec_4_ptr(vreg_ofs(s, a->rd), vreg_ofs(s, 0),
+ vreg_ofs(s, a->rs1), vreg_ofs(s, a->rs2),
+ cpu_env, 0, s->vlen / 8, data, fns[s->sew]);
+gen_set_label(over);
+return true;
+}
+return false;
+}
diff --git a/target/riscv/vector_helper.c b/target/riscv/vector_helper.c
index 3179b1faef..39f44d1029 100644
--- a/target/riscv/vector_helper.c
+++ b/target/riscv/vector_helper.c
@@ -4871,3 +4871,29 @@ GEN_VEXT_VRGATHER_VX(vrgather_vx_b, uint8_t, H1, clearb)
GEN_VEXT_VRGATHER_VX(vrgather_vx_h, uint16_t, H2, clearh)
GEN_VEXT_VRGATHER_VX(vrgather_vx_w, uint32_t, H4, clearl)
GEN_VEXT_VRGATHER_VX(vrgather_vx_d, uint64_t, H8, clearq)
+
+/* Vector Compress Instruction */
+#define GEN_VEXT_VCOMPRESS_VM(NAME, ETYPE, H, CLEAR_FN) \
+void HELPER(NAME)(void *vd, void *v0, void *vs1, void *vs2, \
+ CPURISCVState *env, uint32_t desc) \
+{ \
+uint32_t mlen = vext_mlen(desc); \
+uint32_t vlmax = env_archcpu(env)->cfg.vlen / mlen; \
+uint32_t vl = env->vl;\
+uint32_t num = 0, i; \
+ \
+for (i = 0; i < vl; i++) {\
+if (!vext_elem_mask(vs1, mlen, i)) { \
+continue; \
+} \
+*((ETYPE *)vd + H(num)) = *((ETYPE *)vs2 + H(i)); \
+num++;\
+} \
+CLEAR_FN(vd, num, num * sizeof(ETYPE), vlmax * sizeof(ETYPE));\
+}
+
+/* Compress into vd elements of vs2
Re: [PATCH v2 17/18] hw/block/nvme: Use zone metadata file for persistence
On Jun 18 06:34, Dmitry Fomichev wrote:
> A ZNS drive that is emulated by this driver is currently initialized
> with all zones Empty upon startup. However, actual ZNS SSDs save the
> state and condition of all zones in their internal NVRAM in the event
> of power loss. When such a drive is powered up again, it closes or
> finishes all zones that were open at the moment of shutdown. Besides
> that, the write pointer position as well as the state and condition
> of all zones is preserved across power-downs.
>
> This commit adds the capability to have a persistent zone metadata
> to the driver. The new optional driver property, "zone_file",
> is introduced. If added to the command line, this property specifies
> the name of the file that stores the zone metadata. If "zone_file" is
> omitted, the driver will initialize with all zones empty, the same as
> before.
>
> If zone metadata is configured to be persistent, then zone descriptor
> extensions also persist across controller shutdowns.
>
> Signed-off-by: Dmitry Fomichev
Stefan, before I review this in depth, can you comment on if mmap'ing a
file from a device model and issuing regular msync's is an acceptable
approach to storing state persistently across QEMU invocations?
I could not find any examples of this in hw/, so I am unsure. I
implemented something like this using an additional blockdev on the
device and doing blk_aio's, but just mmaping a file seems much simpler,
but at the cost of portability? On the other hand, I can't find any
examples of using an additional blockdev either.
Can you shed any light on the preferred approach?
> ---
> hw/block/nvme.c | 371 +---
> hw/block/nvme.h | 38 +
> 2 files changed, 388 insertions(+), 21 deletions(-)
>
> diff --git a/hw/block/nvme.c b/hw/block/nvme.c
> index 14d5f1d155..63e7a6352e 100644
> --- a/hw/block/nvme.c
> +++ b/hw/block/nvme.c
> @@ -69,6 +69,8 @@
> } while (0)
>
> static void nvme_process_sq(void *opaque);
> +static void nvme_sync_zone_file(NvmeCtrl *n, NvmeNamespace *ns,
> +NvmeZone *zone, int len);
>
> /*
> * Add a zone to the tail of a zone list.
> @@ -90,6 +92,7 @@ static void nvme_add_zone_tail(NvmeCtrl *n, NvmeNamespace
> *ns, NvmeZoneList *zl,
> zl->tail = idx;
> }
> zl->size++;
> +nvme_set_zone_meta_dirty(n, ns, true);
> }
>
> /*
> @@ -106,12 +109,15 @@ static void nvme_remove_zone(NvmeCtrl *n, NvmeNamespace
> *ns, NvmeZoneList *zl,
> if (zl->size == 0) {
> zl->head = NVME_ZONE_LIST_NIL;
> zl->tail = NVME_ZONE_LIST_NIL;
> +nvme_set_zone_meta_dirty(n, ns, true);
> } else if (idx == zl->head) {
> zl->head = zone->next;
> ns->zone_array[zl->head].prev = NVME_ZONE_LIST_NIL;
> +nvme_set_zone_meta_dirty(n, ns, true);
> } else if (idx == zl->tail) {
> zl->tail = zone->prev;
> ns->zone_array[zl->tail].next = NVME_ZONE_LIST_NIL;
> +nvme_set_zone_meta_dirty(n, ns, true);
> } else {
> ns->zone_array[zone->next].prev = zone->prev;
> ns->zone_array[zone->prev].next = zone->next;
> @@ -138,6 +144,7 @@ static NvmeZone *nvme_remove_zone_head(NvmeCtrl *n,
> NvmeNamespace *ns,
> ns->zone_array[zl->head].prev = NVME_ZONE_LIST_NIL;
> }
> zone->prev = zone->next = 0;
> +nvme_set_zone_meta_dirty(n, ns, true);
> }
>
> return zone;
> @@ -476,6 +483,7 @@ static void nvme_assign_zone_state(NvmeCtrl *n,
> NvmeNamespace *ns,
> case NVME_ZONE_STATE_READ_ONLY:
> zone->tstamp = 0;
> }
> +nvme_sync_zone_file(n, ns, zone, sizeof(NvmeZone));
> }
>
> static bool nvme_addr_is_cmb(NvmeCtrl *n, hwaddr addr)
> @@ -2976,9 +2984,114 @@ static const MemoryRegionOps nvme_cmb_ops = {
> },
> };
>
> -static int nvme_init_zone_meta(NvmeCtrl *n, NvmeNamespace *ns,
> +static int nvme_validate_zone_file(NvmeCtrl *n, NvmeNamespace *ns,
> uint64_t capacity)
> {
> +NvmeZoneMeta *meta = ns->zone_meta;
> +NvmeZone *zone = ns->zone_array;
> +uint64_t start = 0, zone_size = n->params.zone_size;
> +int i, n_imp_open = 0, n_exp_open = 0, n_closed = 0, n_full = 0;
> +
> +if (meta->magic != NVME_ZONE_META_MAGIC) {
> +return 1;
> +}
> +if (meta->version != NVME_ZONE_META_VER) {
> +return 2;
> +}
> +if (meta->zone_size != zone_size) {
> +return 3;
> +}
> +if (meta->zone_capacity != n->params.zone_capacity) {
> +return 4;
> +}
> +if (meta->nr_offline_zones != n->params.nr_offline_zones) {
> +return 5;
> +}
> +if (meta->nr_rdonly_zones != n->params.nr_rdonly_zones) {
> +return 6;
> +}
> +if (meta->lba_size != n->conf.logical_block_size) {
> +return 7;
> +}
> +if (meta->zd_extension_size != n->params.zd_extension_size) {
> +return 8;
> +}
> +
> +for (i = 0; i < n->num_zones; i++, zone++) {
> +
[PATCH v12 59/61] target/riscv: vector register gather instruction
Signed-off-by: LIU Zhiwei
Reviewed-by: Richard Henderson
---
target/riscv/helper.h | 9 +++
target/riscv/insn32.decode | 3 +
target/riscv/insn_trans/trans_rvv.inc.c | 78 +
target/riscv/vector_helper.c| 60 +++
4 files changed, 150 insertions(+)
diff --git a/target/riscv/helper.h b/target/riscv/helper.h
index 29a5eb7049..eca1ab541b 100644
--- a/target/riscv/helper.h
+++ b/target/riscv/helper.h
@@ -1136,3 +1136,12 @@ DEF_HELPER_6(vslide1down_vx_b, void, ptr, ptr, tl, ptr,
env, i32)
DEF_HELPER_6(vslide1down_vx_h, void, ptr, ptr, tl, ptr, env, i32)
DEF_HELPER_6(vslide1down_vx_w, void, ptr, ptr, tl, ptr, env, i32)
DEF_HELPER_6(vslide1down_vx_d, void, ptr, ptr, tl, ptr, env, i32)
+
+DEF_HELPER_6(vrgather_vv_b, void, ptr, ptr, ptr, ptr, env, i32)
+DEF_HELPER_6(vrgather_vv_h, void, ptr, ptr, ptr, ptr, env, i32)
+DEF_HELPER_6(vrgather_vv_w, void, ptr, ptr, ptr, ptr, env, i32)
+DEF_HELPER_6(vrgather_vv_d, void, ptr, ptr, ptr, ptr, env, i32)
+DEF_HELPER_6(vrgather_vx_b, void, ptr, ptr, tl, ptr, env, i32)
+DEF_HELPER_6(vrgather_vx_h, void, ptr, ptr, tl, ptr, env, i32)
+DEF_HELPER_6(vrgather_vx_w, void, ptr, ptr, tl, ptr, env, i32)
+DEF_HELPER_6(vrgather_vx_d, void, ptr, ptr, tl, ptr, env, i32)
diff --git a/target/riscv/insn32.decode b/target/riscv/insn32.decode
index 36123f71b9..80d5ff74a9 100644
--- a/target/riscv/insn32.decode
+++ b/target/riscv/insn32.decode
@@ -574,6 +574,9 @@ vslide1up_vx001110 . . . 110 . 1010111 @r_vm
vslidedown_vx 00 . . . 100 . 1010111 @r_vm
vslidedown_vi 00 . . . 011 . 1010111 @r_vm
vslide1down_vx 00 . . . 110 . 1010111 @r_vm
+vrgather_vv 001100 . . . 000 . 1010111 @r_vm
+vrgather_vx 001100 . . . 100 . 1010111 @r_vm
+vrgather_vi 001100 . . . 011 . 1010111 @r_vm
vsetvli 0 ... . 111 . 1010111 @r2_zimm
vsetvl 100 . . 111 . 1010111 @r
diff --git a/target/riscv/insn_trans/trans_rvv.inc.c
b/target/riscv/insn_trans/trans_rvv.inc.c
index 4ed6d1ee2e..c0b7745a63 100644
--- a/target/riscv/insn_trans/trans_rvv.inc.c
+++ b/target/riscv/insn_trans/trans_rvv.inc.c
@@ -2776,3 +2776,81 @@ GEN_OPIVI_TRANS(vslideup_vi, 1, vslideup_vx,
slideup_check)
GEN_OPIVX_TRANS(vslidedown_vx, opivx_check)
GEN_OPIVX_TRANS(vslide1down_vx, opivx_check)
GEN_OPIVI_TRANS(vslidedown_vi, 1, vslidedown_vx, opivx_check)
+
+/* Vector Register Gather Instruction */
+static bool vrgather_vv_check(DisasContext *s, arg_rmrr *a)
+{
+return (vext_check_isa_ill(s) &&
+vext_check_overlap_mask(s, a->rd, a->vm, true) &&
+vext_check_reg(s, a->rd, false) &&
+vext_check_reg(s, a->rs1, false) &&
+vext_check_reg(s, a->rs2, false) &&
+(a->rd != a->rs2) && (a->rd != a->rs1));
+}
+
+GEN_OPIVV_TRANS(vrgather_vv, vrgather_vv_check)
+
+static bool vrgather_vx_check(DisasContext *s, arg_rmrr *a)
+{
+return (vext_check_isa_ill(s) &&
+vext_check_overlap_mask(s, a->rd, a->vm, true) &&
+vext_check_reg(s, a->rd, false) &&
+vext_check_reg(s, a->rs2, false) &&
+(a->rd != a->rs2));
+}
+
+/* vrgather.vx vd, vs2, rs1, vm # vd[i] = (x[rs1] >= VLMAX) ? 0 : vs2[rs1] */
+static bool trans_vrgather_vx(DisasContext *s, arg_rmrr *a)
+{
+if (!vrgather_vx_check(s, a)) {
+return false;
+}
+
+if (a->vm && s->vl_eq_vlmax) {
+int vlmax = s->vlen / s->mlen;
+TCGv_i64 dest = tcg_temp_new_i64();
+
+if (a->rs1 == 0) {
+vec_element_loadi(s, dest, a->rs2, 0);
+} else {
+vec_element_loadx(s, dest, a->rs2, cpu_gpr[a->rs1], vlmax);
+}
+
+tcg_gen_gvec_dup_i64(s->sew, vreg_ofs(s, a->rd),
+ MAXSZ(s), MAXSZ(s), dest);
+tcg_temp_free_i64(dest);
+} else {
+static gen_helper_opivx * const fns[4] = {
+gen_helper_vrgather_vx_b, gen_helper_vrgather_vx_h,
+gen_helper_vrgather_vx_w, gen_helper_vrgather_vx_d
+};
+return opivx_trans(a->rd, a->rs1, a->rs2, a->vm, fns[s->sew], s);
+}
+return true;
+}
+
+/* vrgather.vi vd, vs2, imm, vm # vd[i] = (imm >= VLMAX) ? 0 : vs2[imm] */
+static bool trans_vrgather_vi(DisasContext *s, arg_rmrr *a)
+{
+if (!vrgather_vx_check(s, a)) {
+return false;
+}
+
+if (a->vm && s->vl_eq_vlmax) {
+if (a->rs1 >= s->vlen / s->mlen) {
+tcg_gen_gvec_dup_imm(SEW64, vreg_ofs(s, a->rd),
+ MAXSZ(s), MAXSZ(s), 0);
+} else {
+tcg_gen_gvec_dup_mem(s->sew, vreg_ofs(s, a->rd),
+ endian_ofs(s, a->rs2, a->rs1),
+ MAXSZ(s), MAXSZ(s));
+}
+} else {
+static gen_helper_opivx * const fns[4] = {
+gen_helper_vrgather_vx_b,
[PATCH v12 58/61] target/riscv: vector slide instructions
Signed-off-by: LIU Zhiwei
Reviewed-by: Richard Henderson
---
target/riscv/helper.h | 17
target/riscv/insn32.decode | 6 ++
target/riscv/insn_trans/trans_rvv.inc.c | 18
target/riscv/vector_helper.c| 114
4 files changed, 155 insertions(+)
diff --git a/target/riscv/helper.h b/target/riscv/helper.h
index c6695ea7a8..29a5eb7049 100644
--- a/target/riscv/helper.h
+++ b/target/riscv/helper.h
@@ -1119,3 +1119,20 @@ DEF_HELPER_4(vid_v_b, void, ptr, ptr, env, i32)
DEF_HELPER_4(vid_v_h, void, ptr, ptr, env, i32)
DEF_HELPER_4(vid_v_w, void, ptr, ptr, env, i32)
DEF_HELPER_4(vid_v_d, void, ptr, ptr, env, i32)
+
+DEF_HELPER_6(vslideup_vx_b, void, ptr, ptr, tl, ptr, env, i32)
+DEF_HELPER_6(vslideup_vx_h, void, ptr, ptr, tl, ptr, env, i32)
+DEF_HELPER_6(vslideup_vx_w, void, ptr, ptr, tl, ptr, env, i32)
+DEF_HELPER_6(vslideup_vx_d, void, ptr, ptr, tl, ptr, env, i32)
+DEF_HELPER_6(vslidedown_vx_b, void, ptr, ptr, tl, ptr, env, i32)
+DEF_HELPER_6(vslidedown_vx_h, void, ptr, ptr, tl, ptr, env, i32)
+DEF_HELPER_6(vslidedown_vx_w, void, ptr, ptr, tl, ptr, env, i32)
+DEF_HELPER_6(vslidedown_vx_d, void, ptr, ptr, tl, ptr, env, i32)
+DEF_HELPER_6(vslide1up_vx_b, void, ptr, ptr, tl, ptr, env, i32)
+DEF_HELPER_6(vslide1up_vx_h, void, ptr, ptr, tl, ptr, env, i32)
+DEF_HELPER_6(vslide1up_vx_w, void, ptr, ptr, tl, ptr, env, i32)
+DEF_HELPER_6(vslide1up_vx_d, void, ptr, ptr, tl, ptr, env, i32)
+DEF_HELPER_6(vslide1down_vx_b, void, ptr, ptr, tl, ptr, env, i32)
+DEF_HELPER_6(vslide1down_vx_h, void, ptr, ptr, tl, ptr, env, i32)
+DEF_HELPER_6(vslide1down_vx_w, void, ptr, ptr, tl, ptr, env, i32)
+DEF_HELPER_6(vslide1down_vx_d, void, ptr, ptr, tl, ptr, env, i32)
diff --git a/target/riscv/insn32.decode b/target/riscv/insn32.decode
index 17288a3c95..36123f71b9 100644
--- a/target/riscv/insn32.decode
+++ b/target/riscv/insn32.decode
@@ -568,6 +568,12 @@ vext_x_v001100 1 . . 010 . 1010111 @r
vmv_s_x 001101 1 0 . 110 . 1010111 @r2
vfmv_f_s001100 1 . 0 001 . 1010111 @r2rd
vfmv_s_f001101 1 0 . 101 . 1010111 @r2
+vslideup_vx 001110 . . . 100 . 1010111 @r_vm
+vslideup_vi 001110 . . . 011 . 1010111 @r_vm
+vslide1up_vx001110 . . . 110 . 1010111 @r_vm
+vslidedown_vx 00 . . . 100 . 1010111 @r_vm
+vslidedown_vi 00 . . . 011 . 1010111 @r_vm
+vslide1down_vx 00 . . . 110 . 1010111 @r_vm
vsetvli 0 ... . 111 . 1010111 @r2_zimm
vsetvl 100 . . 111 . 1010111 @r
diff --git a/target/riscv/insn_trans/trans_rvv.inc.c
b/target/riscv/insn_trans/trans_rvv.inc.c
index 7af16ce0a8..4ed6d1ee2e 100644
--- a/target/riscv/insn_trans/trans_rvv.inc.c
+++ b/target/riscv/insn_trans/trans_rvv.inc.c
@@ -2758,3 +2758,21 @@ static bool trans_vfmv_s_f(DisasContext *s, arg_vfmv_s_f
*a)
}
return false;
}
+
+/* Vector Slide Instructions */
+static bool slideup_check(DisasContext *s, arg_rmrr *a)
+{
+return (vext_check_isa_ill(s) &&
+vext_check_overlap_mask(s, a->rd, a->vm, true) &&
+vext_check_reg(s, a->rd, false) &&
+vext_check_reg(s, a->rs2, false) &&
+(a->rd != a->rs2));
+}
+
+GEN_OPIVX_TRANS(vslideup_vx, slideup_check)
+GEN_OPIVX_TRANS(vslide1up_vx, slideup_check)
+GEN_OPIVI_TRANS(vslideup_vi, 1, vslideup_vx, slideup_check)
+
+GEN_OPIVX_TRANS(vslidedown_vx, opivx_check)
+GEN_OPIVX_TRANS(vslide1down_vx, opivx_check)
+GEN_OPIVI_TRANS(vslidedown_vi, 1, vslidedown_vx, opivx_check)
diff --git a/target/riscv/vector_helper.c b/target/riscv/vector_helper.c
index 0fa899b6ff..51c4bc5756 100644
--- a/target/riscv/vector_helper.c
+++ b/target/riscv/vector_helper.c
@@ -4697,3 +4697,117 @@ GEN_VEXT_VID_V(vid_v_b, uint8_t, H1, clearb)
GEN_VEXT_VID_V(vid_v_h, uint16_t, H2, clearh)
GEN_VEXT_VID_V(vid_v_w, uint32_t, H4, clearl)
GEN_VEXT_VID_V(vid_v_d, uint64_t, H8, clearq)
+
+/*
+ *** Vector Permutation Instructions
+ */
+
+/* Vector Slide Instructions */
+#define GEN_VEXT_VSLIDEUP_VX(NAME, ETYPE, H, CLEAR_FN)\
+void HELPER(NAME)(void *vd, void *v0, target_ulong s1, void *vs2, \
+ CPURISCVState *env, uint32_t desc) \
+{ \
+uint32_t mlen = vext_mlen(desc); \
+uint32_t vlmax = env_archcpu(env)->cfg.vlen / mlen; \
+uint32_t vm = vext_vm(desc); \
+uint32_t vl = env->vl;\
+target_ulong offset = s1, i; \
+ \
+for (i = offset; i < vl; i++) { \
+if (!vm &&
[PATCH v12 57/61] target/riscv: floating-point scalar move instructions
Signed-off-by: LIU Zhiwei
Reviewed-by: Richard Henderson
---
target/riscv/insn32.decode | 3 ++
target/riscv/insn_trans/trans_rvv.inc.c | 49 +
2 files changed, 52 insertions(+)
diff --git a/target/riscv/insn32.decode b/target/riscv/insn32.decode
index e06c0ffc22..17288a3c95 100644
--- a/target/riscv/insn32.decode
+++ b/target/riscv/insn32.decode
@@ -72,6 +72,7 @@
@r2_vm .. vm:1 . . ... . ... %rs2 %rd
@r1_vm .. vm:1 . . ... . ... %rd
@r_nfvm ... ... vm:1 . . ... . ... %nf %rs2 %rs1 %rd
+@r2rd... . . ... . ... %rs2 %rd
@r_vm.. vm:1 . . ... . ... %rs2 %rs1 %rd
@r_vm_1 .. . . . ... . ... vm=1 %rs2 %rs1 %rd
@r_vm_0 .. . . . ... . ... vm=0 %rs2 %rs1 %rd
@@ -565,6 +566,8 @@ viota_m 010110 . . 1 010 . 1010111
@r2_vm
vid_v 010110 . 0 10001 010 . 1010111 @r1_vm
vext_x_v001100 1 . . 010 . 1010111 @r
vmv_s_x 001101 1 0 . 110 . 1010111 @r2
+vfmv_f_s001100 1 . 0 001 . 1010111 @r2rd
+vfmv_s_f001101 1 0 . 101 . 1010111 @r2
vsetvli 0 ... . 111 . 1010111 @r2_zimm
vsetvl 100 . . 111 . 1010111 @r
diff --git a/target/riscv/insn_trans/trans_rvv.inc.c
b/target/riscv/insn_trans/trans_rvv.inc.c
index b10b89daa9..7af16ce0a8 100644
--- a/target/riscv/insn_trans/trans_rvv.inc.c
+++ b/target/riscv/insn_trans/trans_rvv.inc.c
@@ -2709,3 +2709,52 @@ static bool trans_vmv_s_x(DisasContext *s, arg_vmv_s_x
*a)
}
return false;
}
+
+/* Floating-Point Scalar Move Instructions */
+static bool trans_vfmv_f_s(DisasContext *s, arg_vfmv_f_s *a)
+{
+if (!s->vill && has_ext(s, RVF) &&
+(s->mstatus_fs != 0) && (s->sew != 0)) {
+unsigned int len = 8 << s->sew;
+
+vec_element_loadi(s, cpu_fpr[a->rd], a->rs2, 0);
+if (len < 64) {
+tcg_gen_ori_i64(cpu_fpr[a->rd], cpu_fpr[a->rd],
+MAKE_64BIT_MASK(len, 64 - len));
+}
+
+mark_fs_dirty(s);
+return true;
+}
+return false;
+}
+
+/* vfmv.s.f vd, rs1 # vd[0] = rs1 (vs2=0) */
+static bool trans_vfmv_s_f(DisasContext *s, arg_vfmv_s_f *a)
+{
+if (!s->vill && has_ext(s, RVF) && (s->sew != 0)) {
+TCGv_i64 t1;
+/* The instructions ignore LMUL and vector register group. */
+uint32_t vlmax = s->vlen >> 3;
+
+/* if vl == 0, skip vector register write back */
+TCGLabel *over = gen_new_label();
+tcg_gen_brcondi_tl(TCG_COND_EQ, cpu_vl, 0, over);
+
+/* zeroed all elements */
+tcg_gen_gvec_dup_imm(SEW64, vreg_ofs(s, a->rd), vlmax, vlmax, 0);
+
+/* NaN-box f[rs1] as necessary for SEW */
+t1 = tcg_temp_new_i64();
+if (s->sew == MO_64 && !has_ext(s, RVD)) {
+tcg_gen_ori_i64(t1, cpu_fpr[a->rs1], MAKE_64BIT_MASK(32, 32));
+} else {
+tcg_gen_mov_i64(t1, cpu_fpr[a->rs1]);
+}
+vec_element_storei(s, a->rd, 0, t1);
+tcg_temp_free_i64(t1);
+gen_set_label(over);
+return true;
+}
+return false;
+}
--
2.23.0
Re: [PATCH v4 00/16] python: add mypy support to python/qemu
On 6/29/20 10:30 AM, John Snow wrote:
>
>
> On 6/29/20 4:26 AM, Philippe Mathieu-Daudé wrote:
>> +Cleber/Wainer.
>>
>> On 6/26/20 10:41 PM, John Snow wrote:
>>> Based-on: 20200626202350.11060-1-js...@redhat.com
>>>
>>> This series modifies the python/qemu library to comply with mypy --strict.
>>> This requires my "refactor shutdown" patch as a pre-requisite.
>>>
>>> v4:
>>> 001/16:[] [--] 'python/qmp.py: Define common types'
>>> 002/16:[] [--] 'iotests.py: use qemu.qmp type aliases'
>>> 003/16:[] [--] 'python/qmp.py: re-absorb MonitorResponseError'
>>> 004/16:[] [--] 'python/qmp.py: Do not return None from cmd_obj'
>>> 005/16:[] [--] 'python/qmp.py: add casts to JSON deserialization'
>>> 006/16:[] [--] 'python/qmp.py: add QMPProtocolError'
>>> 007/16:[] [--] 'python/machine.py: Fix monitor address typing'
>>> 008/16:[] [--] 'python/machine.py: reorder __init__'
>>> 009/16:[] [--] 'python/machine.py: Don't modify state in _base_args()'
>>> 010/16:[] [--] 'python/machine.py: Handle None events in events_wait'
>>> 011/16:[] [--] 'python/machine.py: use qmp.command'
>>> 012/16:[0010] [FC] 'python/machine.py: Add _qmp access shim'
>>> 013/16:[] [-C] 'python/machine.py: fix _popen access'
>>> 014/16:[] [--] 'python/qemu: make 'args' style arguments immutable'
>>> 015/16:[] [--] 'iotests.py: Adjust HMP kwargs typing'
>>> 016/16:[] [-C] 'python/qemu: Add mypy type annotations'
>>>
>>> - Rebased on "refactor shutdown" v4
>>> - Fixed _qmp access for scripts that disable QMP
>>
>> See:
>>
>> https://travis-ci.org/github/philmd/qemu/jobs/702507625#L5439
>>
>> / # uname -a
>> Linux buildroot 4.5.0-2-4kc-malta #1 Debian 4.5.5-1 (2016-05-29) mips
>> GNU/Linux
>> / # reboot
>> / # reboot: Restarting system
> {'execute': 'quit'}
>> qemu received signal 9; command: "mips-softmmu/qemu-system-mips -display
>> none -vga none -chardev
>> socket,id=mon,path=/tmp/tmpcojsc5u3/qemu-14540-monitor.sock -mon
>> chardev=mon,mode=control -machine malta -chardev
>> socket,id=console,path=/tmp/tmpcojsc5u3/qemu-14540-console.sock,server,nowait
>> -serial chardev:console -kernel
>> /tmp/avocado_xgmck2k5/avocado_job_83wkzs2f/12-tests_acceptance_boot_linux_console.py_BootLinuxConsole.test_mips_malta_cpio/boot/vmlinux-4.5.0-2-4kc-malta
>> -initrd
>> /tmp/avocado_xgmck2k5/avocado_job_83wkzs2f/12-tests_acceptance_boot_linux_console.py_BootLinuxConsole.test_mips_malta_cpiorootfs.cpio
>> -append printk.time=0 console=ttyS0 console=tty rdinit=/sbin/init
>> noreboot -no-reboot"
>>
>> Reproduced traceback from:
>> /home/travis/build/philmd/qemu/build/tests/venv/lib/python3.6/site-packages/avocado/core/test.py:886
>> Traceback (most recent call last):
>> File
>> "/home/travis/build/philmd/qemu/build/tests/acceptance/avocado_qemu/__init__.py",
>> line 195, in tearDown
>> vm.shutdown()
>> File "/home/travis/build/philmd/qemu/python/qemu/machine.py", line
>> 457, in shutdown
>> self._do_shutdown(has_quit)
>> File "/home/travis/build/philmd/qemu/python/qemu/machine.py", line
>> 434, in _do_shutdown
>> self._soft_shutdown(has_quit, timeout)
>> File "/home/travis/build/philmd/qemu/python/qemu/machine.py", line
>> 414, in _soft_shutdown
>> self._qmp.cmd('quit')
>> File "/home/travis/build/philmd/qemu/python/qemu/qmp.py", line 271, in cmd
>> return self.cmd_obj(qmp_cmd)
>> File "/home/travis/build/philmd/qemu/python/qemu/qmp.py", line 249, in
>> cmd_obj
>> self.__sock.sendall(json.dumps(qmp_cmd).encode('utf-8'))
>> BrokenPipeError: [Errno 32] Broken pipe
>>
>
> Might be racing between QMP going away and Python not being aware that
> the process has closed yet. It's the only explanation I have left.
>
> I wish I could reproduce this, though. When I submit jobs to Travis I am
> not seeing these failures.
>
> I'll see what I can do, but at this point I'll not re-send the patch
> series until I can conclusively fix your build testing.
>
> --js
>
OK, this is a very big mea culpa. There are two problems.
1. I should catch ConnectionReset and not ConnectionResetError; one is a
catch-all for socket problems and the other is a very specific errno
that does not include BrokenPipeError.
2. Even if I do, it can still race, actually. QEMU might be in the
middle of shutting down, but has already lost the control channel.
Solving the second problem as the interface is currently designed is
hard. You can wait, but then if the wait failed, you need to re-raise
the control channel error instead of the wait failure. IOW, you need to
wait in order to learn if the control channel error is "important" or not.
So, the architecture of this is starting to look wrong; _soft_shutdown
should keep clarity of purpose. Either it was able to to a nice, soft
shutdown -- or it wasn't.
I'm starting to think that the real problem is that machine.py shouldn't
try to hide connection errors on shutdown at all if we expected to be
able to issue a quit command.
Changing my mind
