[PULL 04/10] gitlab: fix inconsistent indentation
From: Daniel P. Berrangé The standard is to use 2 space indent, not 3. Signed-off-by: Daniel P. Berrangé Reviewed-by: Philippe Mathieu-Daudé Message-Id: <20210216132954.295906-4-berra...@redhat.com> Signed-off-by: Thomas Huth --- .gitlab-ci.yml | 16 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index 32cc6bd4a2..870d5f83f5 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -282,14 +282,14 @@ check-system-opensuse: MAKE_CHECK_ARGS: check acceptance-system-opensuse: - <<: *native_test_job_definition - needs: - - job: build-system-opensuse - artifacts: true - variables: - IMAGE: opensuse-leap - MAKE_CHECK_ARGS: check-acceptance - <<: *acceptance_definition + <<: *native_test_job_definition + needs: +- job: build-system-opensuse + artifacts: true + variables: +IMAGE: opensuse-leap +MAKE_CHECK_ARGS: check-acceptance + <<: *acceptance_definition build-disabled: -- 2.27.0
[PULL 05/10] gitlab-ci: Display Avocado log content when tests timeout
From: Philippe Mathieu-Daudé
Since commit ba2232bae6d ("gitlab-ci: Refactor code that show logs
of failed acceptances") we display the log content of failing tests
(Avocado "FAIL" event).
Since we are also interested in tests timeouting, update our global
Avocado config to display log content for the "INTERRUPT" event,
"possible when the timeout is reached" (See [*]).
[*]
https://avocado-framework.readthedocs.io/en/latest/guides/writer/chapters/writing.html#test-statuses
Suggested-by: Willian Rampazzo
Signed-off-by: Philippe Mathieu-Daudé
Reviewed-by: Willian Rampazzo
Reviewed-by: Wainer dos Santos Moschetta
Message-Id: <20210215171438.935665-1-phi...@redhat.com>
Signed-off-by: Thomas Huth
---
.gitlab-ci.yml | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml
index 870d5f83f5..fcbec77a91 100644
--- a/.gitlab-ci.yml
+++ b/.gitlab-ci.yml
@@ -64,7 +64,7 @@ include:
- echo "[datadir.paths]" > ~/.config/avocado/avocado.conf
- echo "cache_dirs = ['${CI_PROJECT_DIR}/avocado-cache']"
>> ~/.config/avocado/avocado.conf
-- echo -e '[job.output.testlogs]\nstatuses = ["FAIL"]'
+- echo -e '[job.output.testlogs]\nstatuses = ["FAIL", "INTERRUPT"]'
>> ~/.config/avocado/avocado.conf
- if [ -d ${CI_PROJECT_DIR}/avocado-cache ]; then
du -chs ${CI_PROJECT_DIR}/avocado-cache ;
--
2.27.0
[PULL 03/10] gitlab: add fine grained job deps for all build jobs
From: Daniel P. Berrangé This allows the build jobs to start running as soon as their respective container image is ready, instead of waiting for all container builds to finish. Signed-off-by: Daniel P. Berrangé Reviewed-by: Philippe Mathieu-Daudé Message-Id: <20210216132954.295906-3-berra...@redhat.com> Signed-off-by: Thomas Huth --- .gitlab-ci.d/crossbuilds.yml | 46 .gitlab-ci.yml | 58 2 files changed, 104 insertions(+) diff --git a/.gitlab-ci.d/crossbuilds.yml b/.gitlab-ci.d/crossbuilds.yml index 66547b6683..d5098c986b 100644 --- a/.gitlab-ci.d/crossbuilds.yml +++ b/.gitlab-ci.d/crossbuilds.yml @@ -41,114 +41,158 @@ cross-armel-system: extends: .cross_system_build_job + needs: +job: armel-debian-cross-container variables: IMAGE: debian-armel-cross cross-armel-user: extends: .cross_user_build_job + needs: +job: armel-debian-cross-container variables: IMAGE: debian-armel-cross cross-armhf-system: extends: .cross_system_build_job + needs: +job: armhf-debian-cross-container variables: IMAGE: debian-armhf-cross cross-armhf-user: extends: .cross_user_build_job + needs: +job: armhf-debian-cross-container variables: IMAGE: debian-armhf-cross cross-arm64-system: extends: .cross_system_build_job + needs: +job: arm64-debian-cross-container variables: IMAGE: debian-arm64-cross cross-arm64-user: extends: .cross_user_build_job + needs: +job: arm64-debian-cross-container variables: IMAGE: debian-arm64-cross cross-i386-system: extends: .cross_system_build_job + needs: +job: i386-fedora-cross-container variables: IMAGE: fedora-i386-cross MAKE_CHECK_ARGS: check-qtest cross-i386-user: extends: .cross_user_build_job + needs: +job: i386-fedora-cross-container variables: IMAGE: fedora-i386-cross MAKE_CHECK_ARGS: check cross-mips-system: extends: .cross_system_build_job + needs: +job: mips-debian-cross-container variables: IMAGE: debian-mips-cross cross-mips-user: extends: .cross_user_build_job + needs: +job: mips-debian-cross-container variables: IMAGE: debian-mips-cross cross-mipsel-system: extends: .cross_system_build_job + needs: +job: mipsel-debian-cross-container variables: IMAGE: debian-mipsel-cross cross-mipsel-user: extends: .cross_user_build_job + needs: +job: mipsel-debian-cross-container variables: IMAGE: debian-mipsel-cross cross-mips64el-system: extends: .cross_system_build_job + needs: +job: mips64el-debian-cross-container variables: IMAGE: debian-mips64el-cross cross-mips64el-user: extends: .cross_user_build_job + needs: +job: mips64el-debian-cross-container variables: IMAGE: debian-mips64el-cross cross-ppc64el-system: extends: .cross_system_build_job + needs: +job: ppc64el-debian-cross-container variables: IMAGE: debian-ppc64el-cross cross-ppc64el-user: extends: .cross_user_build_job + needs: +job: ppc64el-debian-cross-container variables: IMAGE: debian-ppc64el-cross cross-s390x-system: extends: .cross_system_build_job + needs: +job: s390x-debian-cross-container variables: IMAGE: debian-s390x-cross cross-s390x-user: extends: .cross_user_build_job + needs: +job: s390x-debian-cross-container variables: IMAGE: debian-s390x-cross cross-s390x-kvm-only: extends: .cross_accel_build_job + needs: +job: s390x-debian-cross-container variables: IMAGE: debian-s390x-cross ACCEL_CONFIGURE_OPTS: --disable-tcg cross-win32-system: extends: .cross_system_build_job + needs: +job: win32-fedora-cross-container variables: IMAGE: fedora-win32-cross cross-win64-system: extends: .cross_system_build_job + needs: +job: win64-fedora-cross-container variables: IMAGE: fedora-win64-cross cross-amd64-xen-only: extends: .cross_accel_build_job + needs: +job: amd64-debian-cross-container variables: IMAGE: debian-amd64-cross ACCEL: xen @@ -156,6 +200,8 @@ cross-amd64-xen-only: cross-arm64-xen-only: extends: .cross_accel_build_job + needs: +job: arm64-debian-cross-container variables: IMAGE: debian-arm64-cross ACCEL: xen diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index 7adb9a4cef..32cc6bd4a2 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -109,6 +109,8 @@ acceptance-system-alpine: build-system-ubuntu: <<: *native_build_job_definition + needs: +job: amd64-ubuntu2004-container variables: IMAGE: ubuntu2004 CONFIGURE_ARGS: --enable-fdt=system --enable-slirp=system @@ -141,6 +143,8 @@ acceptance-system-ubuntu: build-system-debian: <<: *native_build_job_definition + needs: +job: amd64-debian-container variables: IMAGE: debian-amd64 CONFIGURE_ARGS: --enable-f
[PULL 08/10] tests/qtest/boot-sector: Check that the guest did not panic
The s390-ccw bios code panics if it can not boot successfully. In
this case, it does not make sense that we wait the full 600 seconds
for the boot sector test to finish and can signal the failure
immediately, thus let's check the status of the guest with the
"query-status" QMP command here, too.
Reported-by: Peter Maydell
Message-Id: <20210212113141.854871-1-th...@redhat.com>
Signed-off-by: Thomas Huth
---
tests/qtest/boot-sector.c | 9 +
1 file changed, 9 insertions(+)
diff --git a/tests/qtest/boot-sector.c b/tests/qtest/boot-sector.c
index 24df5c4734..ea8f264661 100644
--- a/tests/qtest/boot-sector.c
+++ b/tests/qtest/boot-sector.c
@@ -138,6 +138,7 @@ void boot_sector_test(QTestState *qts)
uint8_t signature_low;
uint8_t signature_high;
uint16_t signature;
+QDict *qrsp, *qret;
int i;
/* Wait at most 600 seconds (test is slow with TCI and --enable-debug) */
@@ -155,6 +156,14 @@ void boot_sector_test(QTestState *qts)
if (signature == SIGNATURE) {
break;
}
+
+/* check that guest is still in "running" state and did not panic */
+qrsp = qtest_qmp(qts, "{ 'execute': 'query-status' }");
+qret = qdict_get_qdict(qrsp, "return");
+g_assert_nonnull(qret);
+g_assert_cmpstr(qdict_get_try_str(qret, "status"), ==, "running");
+qobject_unref(qrsp);
+
g_usleep(TEST_DELAY);
}
--
2.27.0
[PULL 06/10] scripts/checkpatch: Improve the check for authors mangled by the mailing list
There were recently some patches on the list which had their "From:"
line mangled like this:
From: qemu_oss--- via
Since our test in the checkpatch.pl script did not trigger here, the
patches finally also ended up in a pull request, with the wrong author
set. So let's improve the regular expression to also complain on
these new patterns, too.
Message-Id: <20210216071512.1199827-1-th...@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé
Reviewed-by: Christian Schoenebeck
Signed-off-by: Thomas Huth
---
scripts/checkpatch.pl | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index e47ad878d8..7f194c842b 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -1499,7 +1499,7 @@ sub process {
$is_patch = 1;
}
- if ($line =~ /^Author: .*via
Qemu-devel.*/) {
+ if ($line =~ /^(Author|From): .* via
.*/) {
ERROR("Author email address is mangled by the mailing
list\n" . $herecurr);
}
--
2.27.0
[PULL 10/10] travis.yml: Limit simultaneous jobs to 3
Even though the host machines that run the Travis CI jobs have
quite a lot of CPUs (e.g. nproc in an aarch64 job reports 32), the
containers on Travis are still limited to 2 vCPUs according to:
https://docs.travis-ci.com/user/reference/overview/#approx-boot-time
So we do not gain much when compiling with a job number based on
the output of "getconf _NPROCESSORS_ONLN" - quite the contrary, the
aarch64 containers are currently aborting quite often since they
are running out of memory. Thus let's rather use a fixed number
like 3 in the jobs here, so that e.g. two threads can actively run
while a third one might be waiting for I/O operations to complete.
This should hopefully fix the out-of-memory failures in the aarch64
CI jobs.
Signed-off-by: Thomas Huth
Reviewed-by: Philippe Mathieu-Daudé
Message-Id: <20210217102531.1441557-1-th...@redhat.com>
[AJB: add comment]
Signed-off-by: Alex Bennée
Message-Id: <20210217121932.19986-6-alex.ben...@linaro.org>
Signed-off-by: Thomas Huth
---
.travis.yml | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/.travis.yml b/.travis.yml
index fc27fd6330..4609240b5a 100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -86,9 +86,11 @@ git:
submodules: false
# Common first phase for all steps
+# We no longer use nproc to calculate jobs:
+# https://travis-ci.community/t/nproc-reports-32-cores-on-arm64/5851
before_install:
- if command -v ccache ; then ccache --zero-stats ; fi
- - export JOBS=$(($(getconf _NPROCESSORS_ONLN) + 1))
+ - export JOBS=3
- echo "=== Using ${JOBS} simultaneous jobs ==="
# Configure step - may be overridden
--
2.27.0
[PULL 09/10] gitlab-ci.yml: Run check-tcg with TCI
It's now possible to also run the non-x86 TCG tests with TCI.
Message-Id: <20210127055903.40148-1-th...@redhat.com>
Signed-off-by: Thomas Huth
---
.gitlab-ci.yml | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml
index c06c20be6c..8b6d495288 100644
--- a/.gitlab-ci.yml
+++ b/.gitlab-ci.yml
@@ -551,9 +551,9 @@ build-oss-fuzz:
build-tci:
<<: *native_build_job_definition
needs:
-job: amd64-fedora-container
+job: amd64-debian-user-cross-container
variables:
-IMAGE: fedora
+IMAGE: debian-all-test-cross
script:
- TARGETS="aarch64 alpha arm hppa m68k microblaze moxie ppc64 s390x x86_64"
- mkdir build
@@ -561,7 +561,6 @@ build-tci:
- ../configure --enable-tcg-interpreter
--target-list="$(for tg in $TARGETS; do echo -n ${tg}'-softmmu ';
done)" || { cat config.log meson-logs/meson-log.txt && exit 1; }
- make -j"$JOBS"
-- make run-tcg-tests-x86_64-softmmu
- make tests/qtest/boot-serial-test tests/qtest/cdrom-test
tests/qtest/pxe-test
- for tg in $TARGETS ; do
export QTEST_QEMU_BINARY="./qemu-system-${tg}" ;
@@ -570,6 +569,7 @@ build-tci:
done
- QTEST_QEMU_BINARY="./qemu-system-x86_64" ./tests/qtest/pxe-test
- QTEST_QEMU_BINARY="./qemu-system-s390x" ./tests/qtest/pxe-test -m slow
+- make check-tcg
# Alternate coroutines implementations are only really of interest to KVM users
# However we can't test against KVM on Gitlab-CI so we can only run unit tests
--
2.27.0
[PULL 07/10] gitlab-ci: Disable vhost-kernel in build-disable job
From: Philippe Mathieu-Daudé
Commit 299e6f19b3e ("vhost-net: revamp configure logic") added
the --enable-vhost-kernel option.
Disable it in the build-disable job.
Signed-off-by: Philippe Mathieu-Daudé
Reviewed-by: Wainer dos Santos Moschetta
Message-Id: <20210131104621.221602-1-f4...@amsat.org>
Signed-off-by: Thomas Huth
---
.gitlab-ci.yml | 1 +
1 file changed, 1 insertion(+)
diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml
index fcbec77a91..c06c20be6c 100644
--- a/.gitlab-ci.yml
+++ b/.gitlab-ci.yml
@@ -358,6 +358,7 @@ build-disabled:
--disable-vhost-crypto
--disable-vhost-net
--disable-vhost-scsi
+ --disable-vhost-kernel
--disable-vhost-user
--disable-vhost-vdpa
--disable-vhost-vsock
--
2.27.0
Re: [PATCH] net: eepro100: validate various address values
Am 19.02.21 um 07:11 schrieb P J P: Hello Alex, Stefan, all +-- On Thu, 18 Feb 2021, Alexander Bulekov wrote --+ | Maybe the infinite loop mentioned in the commit message is actually a DMA | recursion issue? I'm providing a reproducer for a DMA re-entracy issue | below. With this patch applied, the reproducer triggers the assert(), rather | than overflowing the stack, so maybe it is the same issue? -Alex | | cat << EOF | ./qemu-system-i386 -display none -machine accel=qtest, -m \ | 512M -device i82559er,netdev=net0 -netdev user,id=net0 -nodefaults \ | -qtest stdio | outl 0xcf8 0x80001014 | outl 0xcfc 0xc000 | outl 0xcf8 0x80001010 | outl 0xcfc 0xe002 | outl 0xcf8 0x80001004 | outw 0xcfc 0x7 | write 0x1c0b 0x1 0x55 | write 0x1c0c 0x1 0xfc | write 0x1c0d 0x1 0x46 | write 0x1c0e 0x1 0x07 | write 0x746fc59 0x1 0x02 | write 0x746fc5b 0x1 0x02 | write 0x746fc5c 0x1 0xe0 | write 0x4 0x1 0x07 | write 0x5 0x1 0xfc | write 0x6 0x1 0xff | write 0x7 0x1 0x1f | outw 0xc002 0x20 | EOF | * Yes, it is an infinite recursion induced stack overflow. I should've said recursion instead of loop. Thank you for sharing a reproducer and the stack trace. Okay, I can confirm the infinite recursion now. The test case triggers memory writes by the hardware which cause new actions of the same hardware and so on. I don't know how the real hardware would handle that case. For QEMU we can extend the current code which tries to prevent endless loops: the device status EEPRO100State can be extended by a recursion counter to limit the number of recursions, or maybe a boolean flag could be used to stop any recursion of action_command(). I prefer the second variant (no recursion at all) and suggest to add a diagnostic message as well like it is done for the endless loop case. Stefan
Re: [PATCH] net: eepro100: validate various address values
Am 19.02.21 um 09:08 schrieb Stefan Weil:
Okay, I can confirm the infinite recursion now.
The test case triggers memory writes by the hardware which cause new
actions of the same hardware and so on.
I don't know how the real hardware would handle that case.
For QEMU we can extend the current code which tries to prevent endless
loops: the device status EEPRO100State can be extended by a recursion
counter to limit the number of recursions, or maybe a boolean flag
could be used to stop any recursion of action_command(). I prefer the
second variant (no recursion at all) and suggest to add a diagnostic
message as well like it is done for the endless loop case.
If there are no recursions in normal use, the following patch should work:
diff --git a/hw/net/eepro100.c b/hw/net/eepro100.c
index 16e95ef9cc..2474cf3dc2 100644
--- a/hw/net/eepro100.c
+++ b/hw/net/eepro100.c
@@ -279,6 +279,9 @@ typedef struct {
/* Quasi static device properties (no need to save them). */
uint16_t stats_size;
bool has_extended_tcb_support;
+
+ /* Flag to avoid recursions. */
+ bool busy;
} EEPRO100State;
/* Word indices in EEPROM. */
@@ -837,6 +840,14 @@ static void action_command(EEPRO100State *s)
Therefore we limit the number of iterations. */
unsigned max_loop_count = 16;
+ if (s->busy) {
+ /* Prevent recursions. */
+ logout("recursion in %s:%u\n", __FILE__, __LINE__);
+ return;
+ }
+
+ s->busy = true;
+
for (;;) {
bool bit_el;
bool bit_s;
@@ -933,6 +944,7 @@ static void action_command(EEPRO100State *s)
}
TRACE(OTHER, logout("CU list empty\n"));
/* List is empty. Now CU is idle or suspended. */
+ s->busy = false;
}
static void eepro100_cu_command(EEPRO100State * s, uint8_t val)
[PATCH v3] qga: return a more explicit error on why a command is disabled
From: Marc-André Lureau
qmp_disable_command() now takes an optional error string to return a
more explicit error message.
Fixes:
https://bugzilla.redhat.com/show_bug.cgi?id=1928806
Signed-off-by: Marc-André Lureau
---
v3:
- back to passing an error string, but appended to the error message.
include/qapi/qmp/dispatch.h | 3 ++-
qapi/qmp-dispatch.c | 6 --
qapi/qmp-registry.c | 10 ++
qga/main.c | 4 ++--
4 files changed, 14 insertions(+), 9 deletions(-)
diff --git a/include/qapi/qmp/dispatch.h b/include/qapi/qmp/dispatch.h
index 1486cac3ef..bd15d9c35a 100644
--- a/include/qapi/qmp/dispatch.h
+++ b/include/qapi/qmp/dispatch.h
@@ -36,6 +36,7 @@ typedef struct QmpCommand
QmpCommandOptions options;
QTAILQ_ENTRY(QmpCommand) node;
bool enabled;
+const char *disable_reason;
} QmpCommand;
typedef QTAILQ_HEAD(QmpCommandList, QmpCommand) QmpCommandList;
@@ -44,7 +45,7 @@ void qmp_register_command(QmpCommandList *cmds, const char
*name,
QmpCommandFunc *fn, QmpCommandOptions options);
const QmpCommand *qmp_find_command(const QmpCommandList *cmds,
const char *name);
-void qmp_disable_command(QmpCommandList *cmds, const char *name);
+void qmp_disable_command(QmpCommandList *cmds, const char *name, const char
*err_msg);
void qmp_enable_command(QmpCommandList *cmds, const char *name);
bool qmp_command_is_enabled(const QmpCommand *cmd);
diff --git a/qapi/qmp-dispatch.c b/qapi/qmp-dispatch.c
index 0a2b20a4e4..5e597c76f7 100644
--- a/qapi/qmp-dispatch.c
+++ b/qapi/qmp-dispatch.c
@@ -157,8 +157,10 @@ QDict *qmp_dispatch(const QmpCommandList *cmds, QObject
*request,
}
if (!cmd->enabled) {
error_set(&err, ERROR_CLASS_COMMAND_NOT_FOUND,
- "The command %s has been disabled for this instance",
- command);
+ "Command %s has been disabled%s%s",
+ command,
+ cmd->disable_reason ? ": " : "",
+ cmd->disable_reason ?: "");
goto out;
}
if (oob && !(cmd->options & QCO_ALLOW_OOB)) {
diff --git a/qapi/qmp-registry.c b/qapi/qmp-registry.c
index 58c65b5052..f78c064aae 100644
--- a/qapi/qmp-registry.c
+++ b/qapi/qmp-registry.c
@@ -43,26 +43,28 @@ const QmpCommand *qmp_find_command(const QmpCommandList
*cmds, const char *name)
}
static void qmp_toggle_command(QmpCommandList *cmds, const char *name,
- bool enabled)
+ bool enabled, const char *disable_reason)
{
QmpCommand *cmd;
QTAILQ_FOREACH(cmd, cmds, node) {
if (strcmp(cmd->name, name) == 0) {
cmd->enabled = enabled;
+cmd->disable_reason = disable_reason;
return;
}
}
}
-void qmp_disable_command(QmpCommandList *cmds, const char *name)
+void qmp_disable_command(QmpCommandList *cmds, const char *name,
+ const char *disable_reason)
{
-qmp_toggle_command(cmds, name, false);
+qmp_toggle_command(cmds, name, false, disable_reason);
}
void qmp_enable_command(QmpCommandList *cmds, const char *name)
{
-qmp_toggle_command(cmds, name, true);
+qmp_toggle_command(cmds, name, true, NULL);
}
bool qmp_command_is_enabled(const QmpCommand *cmd)
diff --git a/qga/main.c b/qga/main.c
index e7f8f3b161..f2cc8e89a3 100644
--- a/qga/main.c
+++ b/qga/main.c
@@ -375,7 +375,7 @@ static void ga_disable_non_whitelisted(const QmpCommand
*cmd, void *opaque)
}
if (!whitelisted) {
g_debug("disabling command: %s", name);
-qmp_disable_command(&ga_commands, name);
+qmp_disable_command(&ga_commands, name, "the agent is in frozen
state");
}
}
@@ -1329,7 +1329,7 @@ static GAState *initialize_agent(GAConfig *config, int
socket_activation)
s->blacklist = config->blacklist;
do {
g_debug("disabling command: %s", (char *)l->data);
-qmp_disable_command(&ga_commands, l->data);
+qmp_disable_command(&ga_commands, l->data, NULL);
l = g_list_next(l);
} while (l);
}
--
2.29.0
Re: [PATCH 1/1] hw/s390x: modularize virtio-gpu-ccw
Hi, > I'm not against cleaning up the includes for virtio-ccw devices, but I > tend to see that as a separate, less pressing issue. Well, it would allow to build virtio-ccw as common code (i.e. move from specific_ss to softmmu_ss). > > Alternatively add support for > > target-specific modules (which we don't really have right now). > > I think a target-specific module is what I did in this patch. Nope. Specifically target-specific modules must be built once for each target and have a target-specific name (prefix or subdir). So when modularizing -- for example -- vga you'll get modules such as x86_64/hw-display-vga.so, ppc64/hw-display-vga.so, ... Qemu needs support for loading the correct version. What you did is a rather incomplete version of that which happens to work for ccw because ccw is used by s390x only so you can shortcut the "build once for each target" part of the problem. > Furthermore, I think any virtio-ccw device that is about to be built as > a module, must be built as a target-specific module. If the realized > (guest) architecture is not s390x, then there are no s390 IO instructions > and ccw won't fly. Well, it can happen that generic modules don't load into some qemu versions. Devices which need PCI can't be loaded by qemu-system-avr for example because the avr target lacks PCI bus support. ccw modules only loading into qemu-system-s390x because that is the only target providing the bus needed is pretty much the same. take care, Gerd
[PATCH] ui/cocoa: Statically allocate dcl
There is no need of dynamic allocation as dcl is a small singleton.
Static allocation reduces code size and makes hacking with ui/cocoa a
bit easier.
Signed-off-by: Akihiko Odaki
---
ui/cocoa.m | 65 ++
1 file changed, 31 insertions(+), 34 deletions(-)
diff --git a/ui/cocoa.m b/ui/cocoa.m
index 13fba8103e1..efa3bea8f5f 100644
--- a/ui/cocoa.m
+++ b/ui/cocoa.m
@@ -74,8 +74,24 @@
int bitsPerPixel;
} QEMUScreen;
+static void cocoa_update(DisplayChangeListener *dcl,
+ int x, int y, int w, int h);
+
+static void cocoa_switch(DisplayChangeListener *dcl,
+ DisplaySurface *surface);
+
+static void cocoa_refresh(DisplayChangeListener *dcl);
+
NSWindow *normalWindow, *about_window;
-static DisplayChangeListener *dcl;
+static const DisplayChangeListenerOps dcl_ops = {
+.dpy_name = "cocoa",
+.dpy_gfx_update = cocoa_update,
+.dpy_gfx_switch = cocoa_switch,
+.dpy_refresh = cocoa_refresh,
+};
+static DisplayChangeListener dcl = {
+.ops = &dcl_ops,
+};
static int last_buttons;
static int cursor_hide = 1;
@@ -602,15 +618,15 @@ - (void) toggleModifier: (int)keycode {
// Toggle the stored state.
modifiers_state[keycode] = !modifiers_state[keycode];
// Send a keyup or keydown depending on the state.
-qemu_input_event_send_key_qcode(dcl->con, keycode,
modifiers_state[keycode]);
+qemu_input_event_send_key_qcode(dcl.con, keycode,
modifiers_state[keycode]);
}
- (void) toggleStatefulModifier: (int)keycode {
// Toggle the stored state.
modifiers_state[keycode] = !modifiers_state[keycode];
// Generate keydown and keyup.
-qemu_input_event_send_key_qcode(dcl->con, keycode, true);
-qemu_input_event_send_key_qcode(dcl->con, keycode, false);
+qemu_input_event_send_key_qcode(dcl.con, keycode, true);
+qemu_input_event_send_key_qcode(dcl.con, keycode, false);
}
// Does the work of sending input to the monitor
@@ -794,7 +810,7 @@ - (bool) handleEventLocked:(NSEvent *)event
}
if (qemu_console_is_graphic(NULL)) {
-qemu_input_event_send_key_qcode(dcl->con, keycode, true);
+qemu_input_event_send_key_qcode(dcl.con, keycode, true);
} else {
[self handleMonitorInput: event];
}
@@ -809,7 +825,7 @@ - (bool) handleEventLocked:(NSEvent *)event
}
if (qemu_console_is_graphic(NULL)) {
-qemu_input_event_send_key_qcode(dcl->con, keycode, false);
+qemu_input_event_send_key_qcode(dcl.con, keycode, false);
}
break;
case NSEventTypeMouseMoved:
@@ -895,9 +911,9 @@ - (bool) handleEventLocked:(NSEvent *)event
/* Determine if this is a scroll up or scroll down event */
buttons = ([event deltaY] > 0) ?
INPUT_BUTTON_WHEEL_UP : INPUT_BUTTON_WHEEL_DOWN;
-qemu_input_queue_btn(dcl->con, buttons, true);
+qemu_input_queue_btn(dcl.con, buttons, true);
qemu_input_event_sync();
-qemu_input_queue_btn(dcl->con, buttons, false);
+qemu_input_queue_btn(dcl.con, buttons, false);
qemu_input_event_sync();
}
/*
@@ -925,7 +941,7 @@ - (bool) handleEventLocked:(NSEvent *)event
[INPUT_BUTTON_MIDDLE] = MOUSE_EVENT_MBUTTON,
[INPUT_BUTTON_RIGHT] = MOUSE_EVENT_RBUTTON
};
-qemu_input_update_buttons(dcl->con, bmap, last_buttons, buttons);
+qemu_input_update_buttons(dcl.con, bmap, last_buttons, buttons);
last_buttons = buttons;
}
if (isMouseGrabbed) {
@@ -935,12 +951,12 @@ - (bool) handleEventLocked:(NSEvent *)event
* clicks in the titlebar.
*/
if ([self screenContainsPoint:p]) {
-qemu_input_queue_abs(dcl->con, INPUT_AXIS_X, p.x, 0,
screen.width);
-qemu_input_queue_abs(dcl->con, INPUT_AXIS_Y, screen.height
- p.y, 0, screen.height);
+qemu_input_queue_abs(dcl.con, INPUT_AXIS_X, p.x, 0,
screen.width);
+qemu_input_queue_abs(dcl.con, INPUT_AXIS_Y, screen.height
- p.y, 0, screen.height);
}
} else {
-qemu_input_queue_rel(dcl->con, INPUT_AXIS_X, (int)[event
deltaX]);
-qemu_input_queue_rel(dcl->con, INPUT_AXIS_Y, (int)[event
deltaY]);
+qemu_input_queue_rel(dcl.con, INPUT_AXIS_X, (int)[event
deltaX]);
+qemu_input_queue_rel(dcl.con, INPUT_AXIS_Y, (int)[event
deltaY]);
}
} else {
return false;
@@ -1009,7 +1025,7 @@ - (void) raiseAllKeys
for (index = 0; index < max_index; index++) {
if (modifiers_state[index]) {
modifiers_st
[PATCH] ui/cocoa: Use kCGColorSpaceSRGB
kCGColorSpaceGenericRGB | Apple Developer Documentation https://developer.apple.com/documentation/coregraphics/kcgcolorspacegenericrgb > Deprecated > Use kCGColorSpaceSRGB instead. Signed-off-by: Akihiko Odaki --- ui/cocoa.m | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ui/cocoa.m b/ui/cocoa.m index 13fba8103e1..686fbb1b457 100644 --- a/ui/cocoa.m +++ b/ui/cocoa.m @@ -437,7 +437,7 @@ - (void) drawRect:(NSRect) rect screen.bitsPerPixel, //bitsPerPixel (screen.width * (screen.bitsPerComponent/2)), //bytesPerRow #ifdef __LITTLE_ENDIAN__ -CGColorSpaceCreateWithName(kCGColorSpaceGenericRGB), //colorspace for OS X >= 10.4 +CGColorSpaceCreateWithName(kCGColorSpaceSRGB), //colorspace for OS X >= 10.5 kCGBitmapByteOrder32Little | kCGImageAlphaNoneSkipFirst, #else CGColorSpaceCreateDeviceRGB(), //colorspace for OS X < 10.4 (actually ppc) -- 2.24.3 (Apple Git-128)
Re: [PATCH 1/1] hw/s390x: modularize virtio-gpu-ccw
Hi, > I don't see way around target-specific modules. With the modifications > suggested by Thomas and Connie, I was able to get the new module to > compile regardless of the target, Cool (should have checked all mails before sending replies ...). > but that "fixes" s390x at the expense > of breaking all the other targets. For example: > ./qemu-system-x86_64 -device help > Type 'virtio-gpu-ccw' is missing its parent 'virtio-ccw-device' > Aborted Hmm, this is a new failure variant. In the PCI case (see other mail) the module doesn't load in the first place due to missing symbols. Maybe we need a type_register_mayfail() variant which doesn't abort in case the parent isn't found (see also commit 501093207eb1ed4845e0a65ee1ce7db7b9676e0b). HTH, Gerd
[PATCH] ui/cocoa: Do not exit immediately after shutdown
ui/cocoa used to call exit immediately after calling
qemu_system_shutdown_request, which prevents QEMU from actually
perfoming system shutdown. Just sleep forever, and wait QEMU to call
exit and kill the Cocoa thread.
Signed-off-by: Akihiko Odaki
---
ui/cocoa.m | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/ui/cocoa.m b/ui/cocoa.m
index 13fba8103e1..65bb74134ca 100644
--- a/ui/cocoa.m
+++ b/ui/cocoa.m
@@ -1121,7 +1121,7 @@ - (void)applicationWillTerminate:(NSNotification
*)aNotification
COCOA_DEBUG("QemuCocoaAppController: applicationWillTerminate\n");
qemu_system_shutdown_request(SHUTDOWN_CAUSE_HOST_UI);
-exit(0);
+[NSThread sleepForTimeInterval:INFINITY];
}
- (BOOL)applicationShouldTerminateAfterLastWindowClosed:(NSApplication
*)theApplication
--
2.24.3 (Apple Git-128)
[PATCH] block/file-posix: Optimize for macOS
This commit introduces "punch hole" operation and optimizes transfer
block size for macOS.
This commit introduces two additional members,
discard_granularity and opt_io to BlockSizes type in
include/block/block.h. Also, the members of the type are now
optional. Set -1 to discard_granularity and 0 to other members
for the default values.
Signed-off-by: Akihiko Odaki
---
block/file-posix.c| 40 ++--
block/nvme.c | 2 ++
hw/block/block.c | 12 ++--
include/block/block.h | 2 ++
4 files changed, 52 insertions(+), 4 deletions(-)
diff --git a/block/file-posix.c b/block/file-posix.c
index 00cdaaa2d41..defbc04c8e7 100644
--- a/block/file-posix.c
+++ b/block/file-posix.c
@@ -44,6 +44,7 @@
#if defined(__APPLE__) && (__MACH__)
#include
#include
+#include
#include
#include
#include
@@ -1274,6 +1275,8 @@ static int hdev_probe_blocksizes(BlockDriverState *bs,
BlockSizes *bsz)
if (check_for_dasd(s->fd) < 0) {
return -ENOTSUP;
}
+bsz->opt_io = 0;
+bsz->discard_granularity = -1;
ret = probe_logical_blocksize(s->fd, &bsz->log);
if (ret < 0) {
return ret;
@@ -1568,6 +1571,7 @@ out:
}
}
+G_GNUC_UNUSED
static int translate_err(int err)
{
if (err == -ENODEV || err == -ENOSYS || err == -EOPNOTSUPP ||
@@ -1777,16 +1781,27 @@ static int handle_aiocb_discard(void *opaque)
}
} while (errno == EINTR);
-ret = -errno;
+ret = translate_err(-errno);
#endif
} else {
#ifdef CONFIG_FALLOCATE_PUNCH_HOLE
ret = do_fallocate(s->fd, FALLOC_FL_PUNCH_HOLE | FALLOC_FL_KEEP_SIZE,
aiocb->aio_offset, aiocb->aio_nbytes);
+ret = translate_err(-errno);
+#elif defined(__APPLE__) && (__MACH__)
+fpunchhole_t fpunchhole;
+fpunchhole.fp_flags = 0;
+fpunchhole.reserved = 0;
+fpunchhole.fp_offset = aiocb->aio_offset;
+fpunchhole.fp_length = aiocb->aio_nbytes;
+if (fcntl(s->fd, F_PUNCHHOLE, &fpunchhole) == -1) {
+ret = errno == ENODEV ? -ENOTSUP : -errno;
+} else {
+ret = 0;
+}
#endif
}
-ret = translate_err(ret);
if (ret == -ENOTSUP) {
s->has_discard = false;
}
@@ -2095,6 +2110,26 @@ static int raw_co_flush_to_disk(BlockDriverState *bs)
return raw_thread_pool_submit(bs, handle_aiocb_flush, &acb);
}
+static int raw_probe_blocksizes(BlockDriverState *bs, BlockSizes *bsz)
+{
+#if defined(__APPLE__) && (__MACH__)
+BDRVRawState *s = bs->opaque;
+struct statfs buf;
+
+if (!fstatfs(s->fd, &buf)) {
+bsz->phys = 0;
+bsz->log = 0;
+bsz->opt_io = buf.f_iosize;
+bsz->discard_granularity = buf.f_bsize;
+return 0;
+}
+
+return -errno;
+#else
+return -ENOTSUP;
+#endif
+}
+
static void raw_aio_attach_aio_context(BlockDriverState *bs,
AioContext *new_context)
{
@@ -3229,6 +3264,7 @@ BlockDriver bdrv_file = {
.bdrv_refresh_limits = raw_refresh_limits,
.bdrv_io_plug = raw_aio_plug,
.bdrv_io_unplug = raw_aio_unplug,
+.bdrv_probe_blocksizes = raw_probe_blocksizes,
.bdrv_attach_aio_context = raw_aio_attach_aio_context,
.bdrv_co_truncate = raw_co_truncate,
diff --git a/block/nvme.c b/block/nvme.c
index 5a6fbacf4a5..6d84bbdb632 100644
--- a/block/nvme.c
+++ b/block/nvme.c
@@ -983,6 +983,8 @@ static int nvme_probe_blocksizes(BlockDriverState *bs,
BlockSizes *bsz)
uint32_t blocksize = nvme_get_blocksize(bs);
bsz->phys = blocksize;
bsz->log = blocksize;
+bsz->opt_io = 0;
+bsz->discard_granularity = -1;
return 0;
}
diff --git a/hw/block/block.c b/hw/block/block.c
index 1e34573da71..c907e5a7722 100644
--- a/hw/block/block.c
+++ b/hw/block/block.c
@@ -70,19 +70,27 @@ bool blkconf_blocksizes(BlockConf *conf, Error **errp)
backend_ret = blk_probe_blocksizes(blk, &blocksizes);
/* fill in detected values if they are not defined via qemu command line */
if (!conf->physical_block_size) {
-if (!backend_ret) {
+if (!backend_ret && blocksizes.phys) {
conf->physical_block_size = blocksizes.phys;
} else {
conf->physical_block_size = BDRV_SECTOR_SIZE;
}
}
if (!conf->logical_block_size) {
-if (!backend_ret) {
+if (!backend_ret && blocksizes.log) {
conf->logical_block_size = blocksizes.log;
} else {
conf->logical_block_size = BDRV_SECTOR_SIZE;
}
}
+if (!backend_ret) {
+if (!conf->opt_io_size) {
+conf->opt_io_size = blocksizes.opt_io;
+}
+if (conf->discard_granularity == -1) {
+conf->discard_granularity = blocksizes.discard_granularity;
+}
+}
if (conf->logical_block_size > conf->physical_block_size) {
error_setg(errp,
diff --git a/include/block/block.h b/includ
Re: [PATCH] block/file-posix: Optimize for macOS
Patchew URL:
https://patchew.org/QEMU/20210219085148.90545-1-akihiko.od...@gmail.com/
Hi,
This series seems to have some coding style problems. See output below for
more information:
Type: series
Message-id: 20210219085148.90545-1-akihiko.od...@gmail.com
Subject: [PATCH] block/file-posix: Optimize for macOS
=== TEST SCRIPT BEGIN ===
#!/bin/bash
git rev-parse base > /dev/null || exit 0
git config --local diff.renamelimit 0
git config --local diff.renames True
git config --local diff.algorithm histogram
./scripts/checkpatch.pl --mailback base..
=== TEST SCRIPT END ===
Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
From https://github.com/patchew-project/qemu
* [new tag] patchew/20210219085148.90545-1-akihiko.od...@gmail.com ->
patchew/20210219085148.90545-1-akihiko.od...@gmail.com
Switched to a new branch 'test'
455b4a3 block/file-posix: Optimize for macOS
=== OUTPUT BEGIN ===
WARNING: architecture specific defines should be avoided
#90: FILE: block/file-posix.c:2133:
+#if defined(__APPLE__) && (__MACH__)
ERROR: suspect code indent for conditional statements (8, 11)
#141: FILE: hw/block/block.c:73:
+if (!backend_ret && blocksizes.phys) {
conf->physical_block_size = blocksizes.phys;
total: 1 errors, 1 warnings, 129 lines checked
Commit 455b4a328821 (block/file-posix: Optimize for macOS) has style problems,
please review. If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
=== OUTPUT END ===
Test command exited with code: 1
The full log is available at
http://patchew.org/logs/20210219085148.90545-1-akihiko.od...@gmail.com/testing.checkpatch/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-de...@redhat.com
Re: [PATCH] net: e1000: check transmit descriptor field values
+-- On Thu, 18 Feb 2021, Alexander Bulekov wrote --+ | Is this a DMA re-entracy/stack-overflow issue? IIRC the plan was to have | some sort of wider fix for these issues. There are bunch of these reports | floating around at this point, I believe. * It is similar to the eepro100 one. Thank you. -- Prasad J Pandit / Red Hat Product Security Team 8685 545E B54C 486B C6EB 271E E285 8B5A F050 DE8D
Re: [PATCH v2 4/6] qapi: Apply aliases in qobject-input-visitor
Kevin Wolf writes:
> Am 18.02.2021 um 14:39 hat Markus Armbruster geschrieben:
>> Kevin Wolf writes:
>>
>> > Am 17.02.2021 um 16:32 hat Markus Armbruster geschrieben:
>> >> Kevin Wolf writes:
>> >>
>> >> > When looking for an object in a struct in the external representation,
>> >> > check not only the currently visited struct, but also whether an alias
>> >> > in the current StackObject matches and try to fetch the value from the
>> >> > alias then. Providing two values for the same object through different
>> >> > aliases is an error.
>> >> >
>> >> > Signed-off-by: Kevin Wolf
>> >>
>> >> Looking just at qobject_input_try_get_object() for now.
>> >
>> > :-(
>> >
>> > This patch doesn't even feel that complicated to me.
>>
>> I suspect it's just me having an unusually obtuse week.
>>
>> The code is straightforward enough. What I'm missing is a bit of "how
>> does this accomplish the goal" and "why is this safe" here and there.
>>
>> > Old: Get the value from the QDict of the current StackObject with the
>> > given name.
>> >
>> > New: First do alias resolution (with find_object_member), which results
>> > in a StackObject and a name, and that's the QDict and key where you get
>> > the value from.
>>
>> This part I understand.
>>
>> We simultaneously walk the QAPI type and the input QObject, consuming
>> input as we go.
>>
>> Whenever the walk leaves a QAPI object type, we check the corresponding
>> QObject has been consumed completely.
>>
>> With aliases, we additionally look for input in a certain enclosing
>> input object (i.e. up the recursion stack). If found, consume.
>>
>> > Minor complication: Aliases can refer to members of nested objects that
>> > may not be provided in the input. But we want these to work.
>> >
>> > For example, my chardev series defines aliases to flatten
>> > SocketAddressLegacy (old syntax, I haven't rebased it yet):
>> >
>> > { 'union': 'SocketAddressLegacy',
>> > 'data': {
>> > 'inet': 'InetSocketAddress',
>> > 'unix': 'UnixSocketAddress',
>> > 'vsock': 'VsockSocketAddress',
>> > 'fd': 'String' },
>> > 'aliases': [
>> > {'source': ['data']},
>> > {'alias': 'fd', 'source': ['data', 'str']}
>> > ]}
>> >
>> > Of course, the idea is that this input should work:
>> >
>> > { 'type': 'inet', 'hostname': 'localhost', 'port': '1234' }
>> >
>> > However, without implicit objects, parsing 'data' fails because it
>> > expects an object, but none is given (we specified all of its members on
>> > the top level through aliases). What we would have to give is:
>> >
>> > { 'type': 'inet', 'hostname': 'localhost', 'port': '1234', 'data': {} }
>> >
>> > And _that_ would work. Visiting 'data' succeeds because we now have the
>> > object that the visitor expects, and when visiting its members, the
>> > aliases fill in all of the mandatory values.
>> >
>> > So what this patch does is to implicitly assume the 'data': {} instead
>> > of erroring out when we know that aliases exist that can still provide
>> > values for the content of 'data'.
>>
>> Aliases exist than can still provide, but will they? What if input is
>>
>> {"type": "inet"}
>>
>> ?
>>
>> Your explanation makes me guess this is equivalent to
>>
>> {"type": "inet", "data": {}}
>>
>> which fails the visit, because mandatory members of "data" are missing.
>> Fine.
>
> Okay, if you want the gory details, then the answer is yes for this
> case, but it depends.
I'm afraid I need the gory details to get over the review hump.
> If we're aliasing a single member, then we can easily check whether the
> alias is actually specified. If it's not in the input, no implicit
> object.
I figure this check is in the code parts I haven't gotten to, yet. Not
your fault.
> But in our example, it is a wildcard alias and we don't know yet which
> aliases it will use. This depends on what the visitor for the implicit
> object will do (future tense).
>
>> If we make the members optional, it succeeds: qobject_input_optional()
>> checks both the regular and the aliased input, finds neither, and
>> returns false. Still fine.
>>
>> What if "data" is optional, too? Hmmm...
>
> Yes, don't use optional objects in the middle of the path of a wildcard
> alias unless there is no semantic difference between empty object and
> absent object.
Aha! So my spidey-sense wasn't entirely wrong.
>This is documented in the code, but it might actually
> still be missing from qapi-code-gen.txt.
I can't find it there. Needs fixing, obviously.
I guess checking "path of a wildcard alias crosses optional objects" is
hard (impractical?) for the same reasons checking "alias can't resolve"
is.
I'd expect "alias can't resolve" to be caused by typos, incomplete
renames, and such. Basic testing should catch at least the typos. Not
ideal, but I guess it'll do, at least for now.
Relying on testing to catch "crosses optional objects" mistakes feels
iffier to me, because it takes more careful tests.
Re: [PATCH 0/2] g_return_if_fail(), g_return_val_if_fail() misuse
I think this could go via qemu-trivial. Markus Armbruster writes: > Markus Armbruster (2): > backends/dbus-vmstate: Fix short read error handling > vhost_user_gpu: Drop dead check for g_malloc() failure > > backends/dbus-vmstate.c | 5 - > hw/display/vhost-user-gpu.c | 1 - > 2 files changed, 4 insertions(+), 2 deletions(-)
Re: [PULL 18/40] linux-user: Fix types in uaccess.c
Hi Richard,
I think this commit is the cause of CID 1446711.
There is no concistancy between the various declarations of unlock_user():
bsd-user/qemu.h
static inline void unlock_user(void *host_ptr, abi_ulong guest_addr,
long len)
include/exec/softmmu-semi.h
static void softmmu_unlock_user(CPUArchState *env, void *p, target_ulong addr,
target_ulong len)
...
#define unlock_user(s, args, len) softmmu_unlock_user(env, s, args, len)
linux-user/qemu.h
#ifndef DEBUG_REMAP
static inline void unlock_user(void *host_ptr, abi_ulong guest_addr, size_t len)
{ }
#else
void unlock_user(void *host_ptr, abi_ulong guest_addr, long len);
#endif
To take a signed long here allows to unconditionnaly call the unlock_user()
function after the
syscall and not to copy the buffer if the value is negative.
Thanks,
Laurent
Le 16/02/2021 à 17:16, Peter Maydell a écrit :
> From: Richard Henderson
>
> For copy_*_user, only 0 and -TARGET_EFAULT are returned; no need
> to involve abi_long. Use size_t for lengths. Use bool for the
> lock_user copy argument. Use ssize_t for target_strlen, because
> we can't overflow the host memory space.
>
> Reviewed-by: Peter Maydell
> Signed-off-by: Richard Henderson
> Reviewed-by: Philippe Mathieu-Daudé
> Message-id: 20210212184902.1251044-19-richard.hender...@linaro.org
> [PMM: moved fix for ifdef error to previous commit]
> Signed-off-by: Peter Maydell
> ---
> linux-user/qemu.h| 12 +---
> linux-user/uaccess.c | 45 ++--
> 2 files changed, 28 insertions(+), 29 deletions(-)
>
> diff --git a/linux-user/qemu.h b/linux-user/qemu.h
> index 971af97a2fb..d25a5dafc0f 100644
> --- a/linux-user/qemu.h
> +++ b/linux-user/qemu.h
> @@ -7,8 +7,6 @@
> #include "exec/cpu_ldst.h"
>
> #undef DEBUG_REMAP
> -#ifdef DEBUG_REMAP
> -#endif /* DEBUG_REMAP */
>
> #include "exec/user/abitypes.h"
>
> @@ -629,8 +627,8 @@ static inline bool access_ok(CPUState *cpu, int type,
> * buffers between the target and host. These internally perform
> * locking/unlocking of the memory.
> */
> -abi_long copy_from_user(void *hptr, abi_ulong gaddr, size_t len);
> -abi_long copy_to_user(abi_ulong gaddr, void *hptr, size_t len);
> +int copy_from_user(void *hptr, abi_ulong gaddr, size_t len);
> +int copy_to_user(abi_ulong gaddr, void *hptr, size_t len);
>
> /* Functions for accessing guest memory. The tget and tput functions
> read/write single values, byteswapping as necessary. The lock_user
> function
> @@ -640,13 +638,13 @@ abi_long copy_to_user(abi_ulong gaddr, void *hptr,
> size_t len);
>
> /* Lock an area of guest memory into the host. If copy is true then the
> host area will have the same contents as the guest. */
> -void *lock_user(int type, abi_ulong guest_addr, long len, int copy);
> +void *lock_user(int type, abi_ulong guest_addr, size_t len, bool copy);
>
> /* Unlock an area of guest memory. The first LEN bytes must be
> flushed back to guest memory. host_ptr = NULL is explicitly
> allowed and does nothing. */
> #ifndef DEBUG_REMAP
> -static inline void unlock_user(void *host_ptr, abi_ulong guest_addr, long
> len)
> +static inline void unlock_user(void *host_ptr, abi_ulong guest_addr, size_t
> len)
> { }
> #else
> void unlock_user(void *host_ptr, abi_ulong guest_addr, long len);
> @@ -654,7 +652,7 @@ void unlock_user(void *host_ptr, abi_ulong guest_addr,
> long len);
>
> /* Return the length of a string in target memory or -TARGET_EFAULT if
> access error. */
> -abi_long target_strlen(abi_ulong gaddr);
> +ssize_t target_strlen(abi_ulong gaddr);
>
> /* Like lock_user but for null terminated strings. */
> void *lock_user_string(abi_ulong guest_addr);
> diff --git a/linux-user/uaccess.c b/linux-user/uaccess.c
> index bba012ed159..76af6a92b11 100644
> --- a/linux-user/uaccess.c
> +++ b/linux-user/uaccess.c
> @@ -4,7 +4,7 @@
>
> #include "qemu.h"
>
> -void *lock_user(int type, abi_ulong guest_addr, long len, int copy)
> +void *lock_user(int type, abi_ulong guest_addr, size_t len, bool copy)
> {
> if (!access_ok_untagged(type, guest_addr, len)) {
> return NULL;
> @@ -26,7 +26,7 @@ void *lock_user(int type, abi_ulong guest_addr, long len,
> int copy)
> }
>
> #ifdef DEBUG_REMAP
> -void unlock_user(void *host_ptr, abi_ulong guest_addr, long len);
> +void unlock_user(void *host_ptr, abi_ulong guest_addr, size_t len);
> {
> if (!host_ptr) {
> return;
> @@ -34,7 +34,7 @@ void unlock_user(void *host_ptr, abi_ulong guest_addr, long
> len);
> if (host_ptr == g2h_untagged(guest_addr)) {
> return;
> }
> -if (len > 0) {
> +if (len != 0) {
> memcpy(g2h_untagged(guest_addr), host_ptr, len);
> }
> g_free(host_ptr);
> @@ -43,53 +43,53 @@ void unlock_user(void *host_ptr, abi_ulong guest_addr,
> long len);
>
> void *lock_user_string(abi_ulong guest_addr)
> {
> -
Re: [PATCH] net: eepro100: validate various address values
Hello Stefan,
+-- On Fri, 19 Feb 2021, Stefan Weil wrote --+
| If there are no recursions in normal use, the following patch should work:
|
| diff --git a/hw/net/eepro100.c b/hw/net/eepro100.c
| index 16e95ef9cc..2474cf3dc2 100644
| --- a/hw/net/eepro100.c
| +++ b/hw/net/eepro100.c
| @@ -279,6 +279,9 @@ typedef struct {
| /* Quasi static device properties (no need to save them). */
| uint16_t stats_size;
| bool has_extended_tcb_support;
| +
| + /* Flag to avoid recursions. */
| + bool busy;
| } EEPRO100State;
|
| /* Word indices in EEPROM. */
| @@ -837,6 +840,14 @@ static void action_command(EEPRO100State *s)
| Therefore we limit the number of iterations. */
| unsigned max_loop_count = 16;
|
| + if (s->busy) {
| + /* Prevent recursions. */
| + logout("recursion in %s:%u\n", __FILE__, __LINE__);
| + return;
| + }
| +
| + s->busy = true;
| +
| for (;;) {
| bool bit_el;
| bool bit_s;
| @@ -933,6 +944,7 @@ static void action_command(EEPRO100State *s)
| }
| TRACE(OTHER, logout("CU list empty\n"));
| /* List is empty. Now CU is idle or suspended. */
| + s->busy = false;
| }
|
| static void eepro100_cu_command(EEPRO100State * s, uint8_t val)
Please see:
->
https://ruhr-uni-bochum.sciebo.de/s/NNWP2GfwzYKeKwE?path=%2Feepro100_stackoverflow1
* It does not seem to address above case.
Thank you.
--
Prasad J Pandit / Red Hat Product Security Team
8685 545E B54C 486B C6EB 271E E285 8B5A F050 DE8D
Re: [PATCH 0/3] virtio-net: graceful drop of vhost for TAP
On Thu, Feb 18, 2021 at 09:55:25PM +0200, Yuri Benditovich wrote: > On Thu, Feb 18, 2021 at 11:35 AM Daniel P. Berrangé > wrote: > > > On Wed, Feb 10, 2021 at 02:19:59PM +0800, Jason Wang wrote: > > > > > > On 2021/2/9 下午11:04, Michael S. Tsirkin wrote: > > > > On Tue, Feb 09, 2021 at 02:51:05PM +, Daniel P. Berrangé wrote: > > > > > On Tue, Feb 09, 2021 at 09:34:20AM -0500, Michael S. Tsirkin wrote: > > > > > > On Thu, Feb 04, 2021 at 10:29:12PM +0200, Yuri Benditovich wrote: > > > > > > > This set of patches introduces graceful switch from tap-vhost to > > > > > > > tap-no-vhost depending on guest features. Before that the > > features > > > > > > > that vhost does not support were silently cleared in > > get_features. > > > > > > > This creates potential problem of migration from the machine > > where > > > > > > > some of virtio-net features are supported by the vhost kernel to > > the > > > > > > > machine where they are not supported (packed ring as an example). > > > > > > I still worry that adding new features will silently disable vhost > > for people. > > > > > > Can we limit the change to when a VM is migrated in? > > > > > Some management applications expect bi-directional live migration to > > > > > work, so taking specific actions on incoming migration only feels > > > > > dangerous. > > > > Could you be more specific? > > > > > > > > Bi-directional migration is currently broken > > > > when migrating new kernel->old kernel. > > > > > > > > This seems to be the motivation for this patch, though I wish > > > > it was spelled out more explicitly. > > > > > > > > People don't complain much, but I'm fine with fixing that > > > > with a userspace fallback. > > > > > > > > > > > > I'd rather not force the fallback on others though: vhost is generally > > > > specified explicitly by user while features are generally set > > > > automatically, so this patch will make us override what user specified, > > > > not nice. > > > > > > > > > > > > > IMHO if the features we're adding cannot be expected to exist in > > > > > host kernels in general, then the feature should defualt to off > > > > > and require explicit user config to enable. > > > > > Downstream distros which can guarantee newer kernels can flip the > > > > > default in their custom machine types if they desire. > > > > > > > > > > Regards, > > > > > Daniel > > > > Unfortunately that will basically mean we are stuck with no new > > features > > > > for years. We did what this patch is trying to change for years now, in > > > > particular KVM also seems to happily disable CPU features not supported > > > > by kernel so I wonder why we can't keep doing it, with tweaks for some > > > > corner cases. > > > > > > > > > It's probably not the corner case. > > > > > > So my understanding is when a feature is turned on via command line, it > > > should not be cleared silently otherwise we may break migration for sure. > > > > > > E.g when packed=on is specified, we should disable vhost instead of > > clear it > > > from the device. > > > > If something is explicitly turned on by the user, they expect that feature > > to be honoured, or an error to be raised. > > > > If something is not explicitly turned on by the user, the behaviour wrt the > > default should be stable for any given machine type version. > > > > IOW, if you disable vhost by default when packed=on is set, then you can't > > later switch to letting vhost be enabled with packed=on, unless you tie > > that change to a new machine type. > > > > If the user has explicitly said packed=on *and* vhost=on, then should > > must honour that, or raise an error if the combination is unsupportable. > > Silently disabling vhost, then vhost=on is not ok. > > > > If I'm not mistaken: > Inside qemu there is no possibility to determine whether the user > explicitly turned vhost on. > For qemu the vhost is off by default but libvirt creates a new profile with > vhost on. Yes, libvirt will always attempt to enable vhost if it is present on te host with /dev/vhost-net, except where the user explicitly told us not to. Regards, Daniel -- |: https://berrange.com -o-https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o-https://fstop138.berrange.com :| |: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|
Re: [PATCH v4 1/4] slirp: Advance libslirp submodule to add ipv6 host-forward support
On Thu, Feb 18, 2021 at 12:15:35PM -0800, Doug Evans wrote: FWIW, normally when QEMU updates libslirp, the commit message is set to contain the "git shortlog old..new" output > Signed-off-by: Doug Evans > --- > > Changes from v3: > - pick up latest libslirp patch to reject ipv6 addr-any for guest address > - libslirp currently only provides a stateless DHCPv6 server, which means > it can't know in advance what the guest's IP address is, and thus > cannot do the "addr-any -> guest ip address" translation that is done > for ipv4 > > Changes from v2: > - this patch is new in v3, split out from v2 > > slirp | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/slirp b/slirp > index 8f43a99191..26ae658a83 16 > --- a/slirp > +++ b/slirp > @@ -1 +1 @@ > -Subproject commit 8f43a99191afb47ca3f3c6972f6306209f367ece > +Subproject commit 26ae658a83eeca16780cf5615c8247cbb151c3fa > -- > 2.30.0.617.g56c4b15f3c-goog > Regards, Daniel -- |: https://berrange.com -o-https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o-https://fstop138.berrange.com :| |: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|
Re: [PATCH] ui/cocoa: Remove the uses of full screen APIs
2021年2月17日(水) 22:09 Gerd Hoffmann : > > On Fri, Feb 12, 2021 at 09:05:40AM +0900, Akihiko Odaki wrote: > > The detections of full screen APIs were wrong. A detection is coded as: > > [NSView respondsToSelector:@selector(exitFullScreenModeWithOptions:)] > > but it should be: > > [NSView > > instancesRespondToSelector:@selector(exitFullScreenModeWithOptions:)] > > > > The uses of full screen APIs were also incorrect, and if you fix the > > detections, the full screen view stretches the video, changing the > > aspect ratio, even if zooming is disabled. > > > > Remove the code as it does nothing good. > > So, it's broken right now (and probably for quite a while without anyone > complaining). And the attempt to fix it didn't work out very well. > Correct? Because the detections of APIs are wrong, the code using those APIs were never executed and nobody realized it was broken. I did not seriously attempt to fix it because the APIs are no longer the best ways to implement fullscreen. ([NSWindow -toggleFullScreen:] is more favorable today.) There is not much to reuse even if implementing fullscreen with [NSView -enterFullScreenModeWithOptions:] since the code is so small. > > Just dropping the code makes sense to me then. > > Any objections or better suggestions from the macos camp? > If not I'll go queue it for the next UI pull request in a day or two. > > thanks, > Gerd > Thank you for responding to my patches. Akihiko Odaki
Re: [PATCH 1/1] hw/s390x: modularize virtio-gpu-ccw
On Fri, 19 Feb 2021 09:45:45 +0100
Gerd Hoffmann wrote:
> > but that "fixes" s390x at the expense
> > of breaking all the other targets. For example:
> > ./qemu-system-x86_64 -device help
> > Type 'virtio-gpu-ccw' is missing its parent 'virtio-ccw-device'
> > Aborted
>
> Hmm, this is a new failure variant. In the PCI case (see other mail)
> the module doesn't load in the first place due to missing symbols.
>
> Maybe we need a type_register_mayfail() variant which doesn't abort in
> case the parent isn't found (see also commit
> 501093207eb1ed4845e0a65ee1ce7db7b9676e0b).
I was also thinking along the same lines last night, and came up with
this workaround:
diff --git a/hw/s390x/virtio-ccw-gpu.c b/hw/s390x/virtio-ccw-gpu.c
index c301e2586b..bbe591cd62 100644
--- a/hw/s390x/virtio-ccw-gpu.c
+++ b/hw/s390x/virtio-ccw-gpu.c
@@ -62,7 +62,16 @@ static const TypeInfo virtio_ccw_gpu = {
static void virtio_ccw_gpu_register(void)
{
+#ifdef CONFIG_MODULES
+/*
+ * Ugly hack: Avoid targets that don't have it aborting when this module
+ * is loaded.
+if (object_class_by_name(TYPE_VIRTIO_CCW_DEVICE)) {
+type_register_static(&virtio_ccw_gpu);
+}
+#else
type_register_static(&virtio_ccw_gpu);
+#endif
}
type_init(virtio_ccw_gpu_register)
but then I decided it is too ugly to post. Something like
type_register_mayfail() would be certainly nicer, although I would still
prefer the failing type register if the device ain't built as a module.
Today I'm on a vacation so I will pick this up again next week. I will
also think some more of how the situation with virtio-ccw compares to pci
with respect to modularization.
Regards,
Halil
[RFC PATCH 3/3] hw/arm/smmuv3: Post-load stage 1 configurations to the host
In nested mode, we call the set_pasid_table() callback on each STE
update to pass the guest stage 1 configuration to the host and
apply it at physical level.
In the case of live migration, we need to manual call the
set_pasid_table() to load the guest stage 1 configurations to the
host. If this operation is fail, the migration is fail.
Signed-off-by: Kunkun Jiang
---
hw/arm/smmuv3.c | 60 +
hw/arm/trace-events | 1 +
2 files changed, 61 insertions(+)
diff --git a/hw/arm/smmuv3.c b/hw/arm/smmuv3.c
index 6c6ed84e78..94ca15375c 100644
--- a/hw/arm/smmuv3.c
+++ b/hw/arm/smmuv3.c
@@ -1473,6 +1473,65 @@ static void smmu_realize(DeviceState *d, Error **errp)
smmu_init_irq(s, dev);
}
+static int smmuv3_manual_set_pci_device_pasid_table(SMMUDevice *sdev)
+{
+#ifdef __linux__
+IOMMUMemoryRegion *mr = &(sdev->iommu);
+int sid = smmu_get_sid(sdev);
+SMMUEventInfo event = {.type = SMMU_EVT_NONE, .sid = sid,
+ .inval_ste_allowed = true};
+IOMMUConfig iommu_config = {};
+SMMUTransCfg *cfg;
+int ret = -1;
+
+cfg = smmuv3_get_config(sdev, &event);
+if (!cfg) {
+return ret;
+}
+
+iommu_config.pasid_cfg.argsz = sizeof(struct iommu_pasid_table_config);
+iommu_config.pasid_cfg.version = PASID_TABLE_CFG_VERSION_1;
+iommu_config.pasid_cfg.format = IOMMU_PASID_FORMAT_SMMUV3;
+iommu_config.pasid_cfg.base_ptr = cfg->s1ctxptr;
+iommu_config.pasid_cfg.pasid_bits = 0;
+iommu_config.pasid_cfg.vendor_data.smmuv3.version =
PASID_TABLE_SMMUV3_CFG_VERSION_1;
+
+if (cfg->disabled || cfg->bypassed) {
+iommu_config.pasid_cfg.config = IOMMU_PASID_CONFIG_BYPASS;
+} else if (cfg->aborted) {
+iommu_config.pasid_cfg.config = IOMMU_PASID_CONFIG_ABORT;
+} else {
+iommu_config.pasid_cfg.config = IOMMU_PASID_CONFIG_TRANSLATE;
+}
+
+ret = pci_device_set_pasid_table(sdev->bus, sdev->devfn, &iommu_config);
+if (ret) {
+error_report("Failed to pass PASID table to host for iommu mr %s (%m)",
+ mr->parent_obj.name);
+}
+
+return ret;
+#endif
+}
+
+static int smmuv3_post_load(void *opaque, int version_id)
+{
+SMMUv3State *s3 = opaque;
+SMMUState *s = &(s3->smmu_state);
+SMMUDevice *sdev;
+int ret = 0;
+
+QLIST_FOREACH(sdev, &s->devices_with_notifiers, next) {
+trace_smmuv3_post_load_sdev(sdev->devfn, sdev->iommu.parent_obj.name);
+ret = smmuv3_manual_set_pci_device_pasid_table(sdev);
+if (ret) {
+break;
+}
+}
+
+return ret;
+}
+
static const VMStateDescription vmstate_smmuv3_queue = {
.name = "smmuv3_queue",
.version_id = 1,
@@ -1491,6 +1550,7 @@ static const VMStateDescription vmstate_smmuv3 = {
.version_id = 1,
.minimum_version_id = 1,
.priority = MIG_PRI_IOMMU,
+.post_load = smmuv3_post_load,
.fields = (VMStateField[]) {
VMSTATE_UINT32(features, SMMUv3State),
VMSTATE_UINT8(sid_size, SMMUv3State),
diff --git a/hw/arm/trace-events b/hw/arm/trace-events
index 35e562ab74..caa864dd72 100644
--- a/hw/arm/trace-events
+++ b/hw/arm/trace-events
@@ -53,4 +53,5 @@ smmuv3_notify_flag_add(const char *iommu) "ADD SMMUNotifier
node for iommu mr=%s
smmuv3_notify_flag_del(const char *iommu) "DEL SMMUNotifier node for iommu
mr=%s"
smmuv3_inv_notifiers_iova(const char *name, uint16_t asid, uint64_t iova,
uint8_t tg, uint64_t num_pages) "iommu mr=%s asid=%d iova=0x%"PRIx64" tg=%d
num_pages=0x%"PRIx64
smmuv3_notify_config_change(const char *name, uint8_t config, uint64_t
s1ctxptr) "iommu mr=%s config=%d s1ctxptr=0x%"PRIx64
+smmuv3_post_load_sdev(int devfn, const char *name) "sdev devfn=%d iommu
mr=%s"PRIx64
--
2.23.0
RE: supported machines for aarch64
Hello folks, These are the machine lists that included cortex-a72 when I gave qemu-system-aarch64 --machine xxx --cpu help. (I replaced xxx with all the machine I got from qemu-system-aarch64 --machine help. Of course I used simple script for that. And I remove 'virt' from the list.) akita highbankmidway netduino2 realview-pb-a8 sx1-v1 xilinx-zynq-a9 ast2500-evb imx25-pdk mps2-an385 netduinoplus2 realview-pbx-a9 tacoma-bmc xlnx-zcu102 ast2600-evb integratorcpmps2-an505 none romulus-bmc terrier z2 borzoi kzm mps2-an511 nuri sabrelite tosa canon-a1100 lm3s6965evb mps2-an521 orangepi-pc sbsa-refverdex cheetah lm3s811evb musca-a palmetto-bmc smdkc210versatileab collie mainstone musca-b1raspi2 sonorapass-bmc versatilepb connex mcimx6ul-evkmusicpalraspi3 spitz vexpress-a15 cubieboard mcimx7d-sabre n800realview-eb swift-bmc vexpress-a9 emcraft-sf2 microbitn810 realview-eb-mpcore sx1 witherspoon-bmc Do they all really support cortex-a72? I ask this because for example, when I search for the first machine akita on internet, it says it's using PXA255 which ARMv5 chip. But cortex-a72 is ARMv8 chip. Can akita really emulate cortex-a72? Thanks! Chan Kim > -Original Message- > From: Philippe Mathieu-Daudé > Sent: Monday, February 15, 2021 4:38 PM > To: c...@etri.re.kr; qemu-disc...@nongnu.org > Cc: qemu-devel ; Markus Armbruster > ; Paolo Bonzini > Subject: Re: supported machines for aarch64 > > Hi, > > On 2/15/21 6:26 AM, c...@etri.re.kr wrote: > > Hello, > > > > I tried qemu-system-arm machine help and it gave me 75 machines. > > > > Then I tried qemu-system-aarch64 machine help, and it gave me > > almost the same result except it had raspi3, sbsa-ref, > > xlnx-versal-virt and > > xlnx-zcu102 in addition. > > > > I asked myself, Does this mean most machines work both in 32bit mode > > and 64bit mode and those added 4 machines work only in 64bit mode? > > Yes. > > > So I tried for integrator machine which is supported both by > > qemu-system-arm and qemu-system-aarch64, > > > > qemu-system-arm machine integrator cpu help and > > qemu-system-aarch64 machine integrator cpu help, and this time, > > > > The cpu list was almost the same(mostly seemd older 32 bit versions) > > but the aarch64 cpu list gave me 3 more cpus (cortex-a53, a57 and a72). > > Indeed. > > > > > So I realized the qemu-system-aarch64 can emulate both 32bit and 64bit > > machine and cpu cores as the document says. > > Correct. > > > .(its still strange qemu-system-arm machine help doesnt give me > > xlnx-versal-virt in the list, but qemu-system-arm machine > > xlnx-versal-virt cpu help still gives some machines in the list..) > > This is because '-cpu' is processed *before* '-machine', so this works: > > $ qemu-system-arm -M adsfafdadsfasdfdafadfasdfa -cpu help Available CPUs: > arm1026 > arm1136 > arm1136-r2 > ... > > > I started this as a question but found out the answer while writing.. > > :) > > > Thank you and correct me if Im wrong > > Regards, > > Phil. >
[RFC PATCH 0/3] Add migration support for VFIO PCI devices in SMMUv3 nested stage mode
Hi all, Since the SMMUv3's nested translation stages[1] has been introduced by Eric, we need to pay attention to the migration of VFIO PCI devices in SMMUv3 nested stage mode. At present, it is not yet supported in QEMU. There are two problems in the existing framework. First, the current way to get dirty pages is not applicable to nested stage mode. Because of the "Caching Mode", VTD can map the RAM through the host single stage (giova->hpa). "vfio_listener_log_sync" gets dirty pages by transferring "giova" to the kernel for the RAM block section of mapped MMIO region. In nested stage mode, we setup the stage 2 (gpa->hpa) and the stage 1(giova->gpa) separately. So it is inapplicable to get dirty pages by the current way in nested stage mode. Second, it also need to pass stage 1 configurations to the destination host after the migration. In Eric's patch, it passes the stage 1 configuration to the host on each STE update for the devices set the PASID PciOps. The configuration will be applied at physical level. But the data of physical level will not be sent to the destination host. So we have to pass stage 1 configurations to the destination host after the migration. This Patch set includes patches as below: Patch 1-2: - Refactor the vfio_listener_log_sync and added a new function to get dirty pages in nested stage mode. Patch 3: - Added the post_load function to vmstate_smmuv3 for passing stage 1 configuration to the destination host after the migration. @Eric, Could you please add this Patch set to your future version of "vSMMUv3/pSMMUv3 2 stage VFIO integration", if you think this Patch set makes sense? :) Best Regards Kunkun Jiang [1] [RFC,v7,00/26] vSMMUv3/pSMMUv3 2 stage VFIO integration http://patchwork.ozlabs.org/project/qemu-devel/cover/20201116181349.11908-1-eric.au...@redhat.com/ Kunkun Jiang (3): vfio: Introduce helpers to mark dirty pages of a RAM section vfio: Add vfio_prereg_listener_log_sync in nested stage hw/arm/smmuv3: Post-load stage 1 configurations to the host hw/arm/smmuv3.c | 60 + hw/arm/trace-events | 1 + hw/vfio/common.c| 47 +-- 3 files changed, 100 insertions(+), 8 deletions(-) -- 2.23.0
[RFC PATCH 1/3] vfio: Introduce helpers to mark dirty pages of a RAM section
Extract part of the code from vfio_sync_dirty_bitmap to form a
new helper, which allows to mark dirty pages of a RAM section.
This helper will be called for nested stage.
Signed-off-by: Kunkun Jiang
---
hw/vfio/common.c | 22 ++
1 file changed, 14 insertions(+), 8 deletions(-)
diff --git a/hw/vfio/common.c b/hw/vfio/common.c
index 9225f10722..7c50905856 100644
--- a/hw/vfio/common.c
+++ b/hw/vfio/common.c
@@ -1203,6 +1203,19 @@ err_out:
return ret;
}
+static int vfio_dma_sync_ram_section_dirty_bitmap(VFIOContainer *container,
+ MemoryRegionSection *section)
+{
+ram_addr_t ram_addr;
+
+ram_addr = memory_region_get_ram_addr(section->mr) +
+ section->offset_within_region;
+
+return vfio_get_dirty_bitmap(container,
+ TARGET_PAGE_ALIGN(section->offset_within_address_space),
+ int128_get64(section->size), ram_addr);
+}
+
typedef struct {
IOMMUNotifier n;
VFIOGuestIOMMU *giommu;
@@ -1244,8 +1257,6 @@ static void vfio_iommu_map_dirty_notify(IOMMUNotifier *n,
IOMMUTLBEntry *iotlb)
static int vfio_sync_dirty_bitmap(VFIOContainer *container,
MemoryRegionSection *section)
{
-ram_addr_t ram_addr;
-
if (memory_region_is_iommu(section->mr)) {
VFIOGuestIOMMU *giommu;
@@ -1274,12 +1285,7 @@ static int vfio_sync_dirty_bitmap(VFIOContainer
*container,
return 0;
}
-ram_addr = memory_region_get_ram_addr(section->mr) +
- section->offset_within_region;
-
-return vfio_get_dirty_bitmap(container,
- TARGET_PAGE_ALIGN(section->offset_within_address_space),
- int128_get64(section->size), ram_addr);
+return vfio_dma_sync_ram_section_dirty_bitmap(container, section);
}
static void vfio_listerner_log_sync(MemoryListener *listener,
--
2.23.0
[RFC PATCH 2/3] vfio: Add vfio_prereg_listener_log_sync in nested stage
On Intel, the DMA mapped through the host single stage. Instead
we set up the stage 2 and stage 1 separately in nested mode as there
is no "Caching Mode".
Legacy vfio_listener_log_sync cannot be used in nested stage as we
don't need to pay close attention to stage 1 mapping. This patch adds
vfio_prereg_listener_log_sync to mark dirty pages in nested mode.
Signed-off-by: Kunkun Jiang
---
hw/vfio/common.c | 25 +
1 file changed, 25 insertions(+)
diff --git a/hw/vfio/common.c b/hw/vfio/common.c
index 7c50905856..af333e0dee 100644
--- a/hw/vfio/common.c
+++ b/hw/vfio/common.c
@@ -1216,6 +1216,22 @@ static int
vfio_dma_sync_ram_section_dirty_bitmap(VFIOContainer *container,
int128_get64(section->size), ram_addr);
}
+static void vfio_prereg_listener_log_sync(MemoryListener *listener,
+ MemoryRegionSection *section)
+{
+VFIOContainer *container =
+container_of(listener, VFIOContainer, prereg_listener);
+
+if (!memory_region_is_ram(section->mr) ||
+!container->dirty_pages_supported) {
+return;
+}
+
+if (vfio_devices_all_saving(container)) {
+vfio_dma_sync_ram_section_dirty_bitmap(container, section);
+}
+}
+
typedef struct {
IOMMUNotifier n;
VFIOGuestIOMMU *giommu;
@@ -1260,6 +1276,14 @@ static int vfio_sync_dirty_bitmap(VFIOContainer
*container,
if (memory_region_is_iommu(section->mr)) {
VFIOGuestIOMMU *giommu;
+/*
+ * In nested mode, stage 2 and stage 1 are set up separately. We
+ * only need to focus on stage 2 mapping when marking dirty pages.
+ */
+if (container->iommu_type == VFIO_TYPE1_NESTING_IOMMU) {
+return 0;
+}
+
QLIST_FOREACH(giommu, &container->giommu_list, giommu_next) {
if (MEMORY_REGION(giommu->iommu) == section->mr &&
giommu->n.start == section->offset_within_region) {
@@ -1312,6 +1336,7 @@ static const MemoryListener vfio_memory_listener = {
static MemoryListener vfio_memory_prereg_listener = {
.region_add = vfio_prereg_listener_region_add,
.region_del = vfio_prereg_listener_region_del,
+.log_sync = vfio_prereg_listener_log_sync,
};
static void vfio_listener_release(VFIOContainer *container)
--
2.23.0
[PATCH] ui/console: Remove dpy_gl_ctx_get_current
It is not used, and it is unlikely that a new use case will emerge
anytime soon because the scope of OpenGL contexts are limited due to
the nature of the frontend, VirGL, processing simple commands from the
guest.
Remove the function and ease implementing a new OpenGL backend a little.
Signed-off-by: Akihiko Odaki
---
include/ui/console.h | 2 --
include/ui/egl-context.h | 1 -
include/ui/gtk.h | 1 -
include/ui/sdl2.h| 1 -
ui/console.c | 6 --
ui/egl-context.c | 5 -
ui/egl-headless.c| 1 -
ui/gtk-gl-area.c | 5 -
ui/gtk.c | 2 --
ui/sdl2-gl.c | 8
ui/sdl2.c| 1 -
ui/spice-display.c | 1 -
12 files changed, 34 deletions(-)
diff --git a/include/ui/console.h b/include/ui/console.h
index 5dd21976a37..f81ad1c5693 100644
--- a/include/ui/console.h
+++ b/include/ui/console.h
@@ -201,7 +201,6 @@ typedef struct DisplayChangeListenerOps {
QEMUGLContext ctx);
int (*dpy_gl_ctx_make_current)(DisplayChangeListener *dcl,
QEMUGLContext ctx);
-QEMUGLContext (*dpy_gl_ctx_get_current)(DisplayChangeListener *dcl);
void (*dpy_gl_scanout_disable)(DisplayChangeListener *dcl);
void (*dpy_gl_scanout_texture)(DisplayChangeListener *dcl,
@@ -303,7 +302,6 @@ QEMUGLContext dpy_gl_ctx_create(QemuConsole *con,
QEMUGLParams *params);
void dpy_gl_ctx_destroy(QemuConsole *con, QEMUGLContext ctx);
int dpy_gl_ctx_make_current(QemuConsole *con, QEMUGLContext ctx);
-QEMUGLContext dpy_gl_ctx_get_current(QemuConsole *con);
bool console_has_gl(QemuConsole *con);
bool console_has_gl_dmabuf(QemuConsole *con);
diff --git a/include/ui/egl-context.h b/include/ui/egl-context.h
index f004ce11a7b..9374fe41e32 100644
--- a/include/ui/egl-context.h
+++ b/include/ui/egl-context.h
@@ -9,6 +9,5 @@ QEMUGLContext qemu_egl_create_context(DisplayChangeListener
*dcl,
void qemu_egl_destroy_context(DisplayChangeListener *dcl, QEMUGLContext ctx);
int qemu_egl_make_context_current(DisplayChangeListener *dcl,
QEMUGLContext ctx);
-QEMUGLContext qemu_egl_get_current_context(DisplayChangeListener *dcl);
#endif /* EGL_CONTEXT_H */
diff --git a/include/ui/gtk.h b/include/ui/gtk.h
index 3f395d7f943..c82c4a569a6 100644
--- a/include/ui/gtk.h
+++ b/include/ui/gtk.h
@@ -143,7 +143,6 @@ void gd_gl_area_scanout_texture(DisplayChangeListener *dcl,
void gd_gl_area_scanout_flush(DisplayChangeListener *dcl,
uint32_t x, uint32_t y, uint32_t w, uint32_t h);
void gtk_gl_area_init(void);
-QEMUGLContext gd_gl_area_get_current_context(DisplayChangeListener *dcl);
int gd_gl_area_make_current(DisplayChangeListener *dcl,
QEMUGLContext ctx);
diff --git a/include/ui/sdl2.h b/include/ui/sdl2.h
index 0875b8d56b7..f85c117a78f 100644
--- a/include/ui/sdl2.h
+++ b/include/ui/sdl2.h
@@ -70,7 +70,6 @@ QEMUGLContext sdl2_gl_create_context(DisplayChangeListener
*dcl,
void sdl2_gl_destroy_context(DisplayChangeListener *dcl, QEMUGLContext ctx);
int sdl2_gl_make_context_current(DisplayChangeListener *dcl,
QEMUGLContext ctx);
-QEMUGLContext sdl2_gl_get_current_context(DisplayChangeListener *dcl);
void sdl2_gl_scanout_disable(DisplayChangeListener *dcl);
void sdl2_gl_scanout_texture(DisplayChangeListener *dcl,
diff --git a/ui/console.c b/ui/console.c
index d80ce7037c3..c0b1a3689c3 100644
--- a/ui/console.c
+++ b/ui/console.c
@@ -1803,12 +1803,6 @@ int dpy_gl_ctx_make_current(QemuConsole *con,
QEMUGLContext ctx)
return con->gl->ops->dpy_gl_ctx_make_current(con->gl, ctx);
}
-QEMUGLContext dpy_gl_ctx_get_current(QemuConsole *con)
-{
-assert(con->gl);
-return con->gl->ops->dpy_gl_ctx_get_current(con->gl);
-}
-
void dpy_gl_scanout_disable(QemuConsole *con)
{
assert(con->gl);
diff --git a/ui/egl-context.c b/ui/egl-context.c
index 4aa1cbb50c2..368ffa49d82 100644
--- a/ui/egl-context.c
+++ b/ui/egl-context.c
@@ -35,8 +35,3 @@ int qemu_egl_make_context_current(DisplayChangeListener *dcl,
return eglMakeCurrent(qemu_egl_display,
EGL_NO_SURFACE, EGL_NO_SURFACE, ctx);
}
-
-QEMUGLContext qemu_egl_get_current_context(DisplayChangeListener *dcl)
-{
-return eglGetCurrentContext();
-}
diff --git a/ui/egl-headless.c b/ui/egl-headless.c
index fe2a0d1eab9..da377a74af6 100644
--- a/ui/egl-headless.c
+++ b/ui/egl-headless.c
@@ -160,7 +160,6 @@ static const DisplayChangeListenerOps egl_ops = {
.dpy_gl_ctx_create = egl_create_context,
.dpy_gl_ctx_destroy = qemu_egl_destroy_context,
.dpy_gl_ctx_make_current = qemu_egl_make_context_current,
-.dpy_gl_ctx_get_current = qemu_egl_get_current_context,
.dpy_gl_scanout_disable = egl_scanout_disable,
.dpy_gl_scanout_texture = egl_scanout_texture,
diff --git a/ui/gtk-gl-area.c b/ui/gtk-gl-
[PATCH] opengl: Do not convert format with glTexImage2D on OpenGL ES
OpenGL ES does not support conversion from the given data format
to the internal format with glTexImage2D.
Use the given data format as the internal format, and ignore
the given alpha channels with GL_TEXTURE_SWIZZLE_A in case the
format contains alpha channels.
Signed-off-by: Akihiko Odaki
---
ui/console-gl.c | 19 ++-
1 file changed, 14 insertions(+), 5 deletions(-)
diff --git a/ui/console-gl.c b/ui/console-gl.c
index 0a6478161fe..7c9894a51d9 100644
--- a/ui/console-gl.c
+++ b/ui/console-gl.c
@@ -73,11 +73,20 @@ void surface_gl_create_texture(QemuGLShader *gls,
glBindTexture(GL_TEXTURE_2D, surface->texture);
glPixelStorei(GL_UNPACK_ROW_LENGTH_EXT,
surface_stride(surface) / surface_bytes_per_pixel(surface));
-glTexImage2D(GL_TEXTURE_2D, 0, GL_RGB,
- surface_width(surface),
- surface_height(surface),
- 0, surface->glformat, surface->gltype,
- surface_data(surface));
+if (epoxy_is_desktop_gl()) {
+glTexImage2D(GL_TEXTURE_2D, 0, GL_RGB,
+ surface_width(surface),
+ surface_height(surface),
+ 0, surface->glformat, surface->gltype,
+ surface_data(surface));
+} else {
+glTexImage2D(GL_TEXTURE_2D, 0, surface->glformat,
+ surface_width(surface),
+ surface_height(surface),
+ 0, surface->glformat, surface->gltype,
+ surface_data(surface));
+glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_SWIZZLE_A, GL_ONE);
+}
glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MAG_FILTER, GL_LINEAR);
glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MIN_FILTER, GL_LINEAR);
--
2.24.3 (Apple Git-128)
Re: [PATCH] net: eepro100: validate various address values
Am 19.02.21 um 10:26 schrieb P J P:
Hello Stefan,
+-- On Fri, 19 Feb 2021, Stefan Weil wrote --+
| If there are no recursions in normal use, the following patch should work:
|
| diff --git a/hw/net/eepro100.c b/hw/net/eepro100.c
| index 16e95ef9cc..2474cf3dc2 100644
| --- a/hw/net/eepro100.c
| +++ b/hw/net/eepro100.c
| @@ -279,6 +279,9 @@ typedef struct {
| /* Quasi static device properties (no need to save them). */
| uint16_t stats_size;
| bool has_extended_tcb_support;
| +
| + /* Flag to avoid recursions. */
| + bool busy;
| } EEPRO100State;
|
| /* Word indices in EEPROM. */
| @@ -837,6 +840,14 @@ static void action_command(EEPRO100State *s)
| Therefore we limit the number of iterations. */
| unsigned max_loop_count = 16;
|
| + if (s->busy) {
| + /* Prevent recursions. */
| + logout("recursion in %s:%u\n", __FILE__, __LINE__);
| + return;
| + }
| +
| + s->busy = true;
| +
| for (;;) {
| bool bit_el;
| bool bit_s;
| @@ -933,6 +944,7 @@ static void action_command(EEPRO100State *s)
| }
| TRACE(OTHER, logout("CU list empty\n"));
| /* List is empty. Now CU is idle or suspended. */
| + s->busy = false;
| }
|
| static void eepro100_cu_command(EEPRO100State * s, uint8_t val)
Please see:
->
https://ruhr-uni-bochum.sciebo.de/s/NNWP2GfwzYKeKwE?path=%2Feepro100_stackoverflow1
* It does not seem to address above case.
My suggested patch fixes that test case: it no longer crashes because of
an endless recursion.
Stefan
Re: [PATCH] s390x/pci: restore missing Query PCI Function CLP data
On Thu, 18 Feb 2021 15:53:29 -0500
Matthew Rosato wrote:
> Some CLP response data was accidentally dropped when fixing endianness
> issues with the Query PCI Function CLP response. All of these values are
> sent as 0s to the guest for emulated devices, so the impact is only
> observed on passthrough devices.
>
> Fixes: a4e2fff1b104 ("s390x/pci: fix endianness issues")
> Signed-off-by: Matthew Rosato
> ---
> hw/s390x/s390-pci-inst.c | 5 +
> 1 file changed, 5 insertions(+)
Whoops.
Thanks, applied.
[PATCH] target/riscv: fix TB_FLAGS bits overlapping bug for rvv/rvh
From: Frank Chang TB_FLAGS mem_idx bits was extended from 2 bits to 3 bits in commit: c445593, but other TB_FLAGS bits for rvv and rvh were not shift as well so these bits may overlap with each other when rvv is enabled. Signed-off-by: Frank Chang --- target/riscv/cpu.h | 11 ++- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h index 02758ae0eb4..1b49eb9950b 100644 --- a/target/riscv/cpu.h +++ b/target/riscv/cpu.h @@ -379,12 +379,13 @@ typedef CPURISCVState CPUArchState; typedef RISCVCPU ArchCPU; #include "exec/cpu-all.h" -FIELD(TB_FLAGS, VL_EQ_VLMAX, 2, 1) -FIELD(TB_FLAGS, LMUL, 3, 2) -FIELD(TB_FLAGS, SEW, 5, 3) -FIELD(TB_FLAGS, VILL, 8, 1) +/* Skip mem_idx bits */ +FIELD(TB_FLAGS, VL_EQ_VLMAX, 3, 1) +FIELD(TB_FLAGS, LMUL, 4, 2) +FIELD(TB_FLAGS, SEW, 6, 3) +FIELD(TB_FLAGS, VILL, 9, 1) /* Is a Hypervisor instruction load/store allowed? */ -FIELD(TB_FLAGS, HLSX, 9, 1) +FIELD(TB_FLAGS, HLSX, 10, 1) bool riscv_cpu_is_32bit(CPURISCVState *env); -- 2.17.1
Re: [PATCH v4 2/4] util/qemu-sockets.c: Split host:port parsing out of inet_parse
On Thu, Feb 18, 2021 at 12:15:36PM -0800, Doug Evans wrote:
> The parsing is moved into new function inet_parse_host_and_port.
> This is done in preparation for using the function in net/slirp.c.
>
> Signed-off-by: Doug Evans
> ---
>
> Changes from v3:
> - this patch is new in v4
> - provides new utility: inet_parse_host_and_port, updates inet_parse
> to use it
>
> include/qemu/sockets.h | 3 ++
> util/qemu-sockets.c| 94 +++---
> 2 files changed, 72 insertions(+), 25 deletions(-)
>
> diff --git a/include/qemu/sockets.h b/include/qemu/sockets.h
> index 7d1f813576..f720378a6b 100644
> --- a/include/qemu/sockets.h
> +++ b/include/qemu/sockets.h
> @@ -31,6 +31,9 @@ int socket_set_fast_reuse(int fd);
>
> int inet_ai_family_from_address(InetSocketAddress *addr,
> Error **errp);
> +const char* inet_parse_host_and_port(const char* str, int terminator,
> + char **addr, char **port, bool *is_v6,
> + Error **errp);
> int inet_parse(InetSocketAddress *addr, const char *str, Error **errp);
> int inet_connect(const char *str, Error **errp);
> int inet_connect_saddr(InetSocketAddress *saddr, Error **errp);
> diff --git a/util/qemu-sockets.c b/util/qemu-sockets.c
> index 8af0278f15..9fca7d9212 100644
> --- a/util/qemu-sockets.c
> +++ b/util/qemu-sockets.c
> @@ -615,44 +615,88 @@ static int inet_parse_flag(const char *flagname, const
> char *optstr, bool *val,
> return 0;
> }
>
> -int inet_parse(InetSocketAddress *addr, const char *str, Error **errp)
> +/*
> + * Parse an inet host and port as "host:port".
> + * Terminator may be '\0'.
> + * The syntax for ipv4 addresses is: address:port.
> + * The syntax for ipv6 addresses is: [address]:port.
It also supports
"The syntax for hostnames is hostname:port
> + * On success, returns a pointer to the terminator. Space for the address and
> + * port is malloced and stored in *host, *port, the caller must free.
> + * *is_v6 indicates whether the address is ipv4 or ipv6. If ipv6 then the
> + * surrounding [] brackets are removed.
When is_v6 is true, it indicates that a numeric ipv6 address was given.
When false either a numberic ipv4 address or hostname was given.
> + * On failure NULL is returned with the error stored in *errp.
> + */
> +const char* inet_parse_host_and_port(const char* str, int terminator,
> + char **hostp, char **portp, bool *is_v6,
> + Error **errp)
> {
> -const char *optstr, *h;
> +const char *terminator_ptr = strchr(str, terminator);
> +g_autofree char *buf = NULL;
> char host[65];
> char port[33];
> -int to;
> -int pos;
> -char *begin;
>
> -memset(addr, 0, sizeof(*addr));
> +if (terminator_ptr == NULL) {
> +/* If the terminator isn't found then use the entire string. */
> +terminator_ptr = str + strlen(str);
> +}
> +buf = g_strndup(str, terminator_ptr - str);
>
> -/* parse address */
> -if (str[0] == ':') {
> -/* no host given */
> -host[0] = '\0';
> -if (sscanf(str, ":%32[^,]%n", port, &pos) != 1) {
> -error_setg(errp, "error parsing port in address '%s'", str);
> -return -1;
> -}
> -} else if (str[0] == '[') {
> +if (buf[0] == '[') {
> /* IPv6 addr */
> -if (sscanf(str, "[%64[^]]]:%32[^,]%n", host, port, &pos) != 2) {
> -error_setg(errp, "error parsing IPv6 address '%s'", str);
> -return -1;
> +if (buf[1] == ']') {
> +/* sscanf %[ doesn't recognize empty contents. */
> +host[0] = '\0';
> +if (sscanf(buf, "[]:%32s", port) != 1) {
> +error_setg(errp, "error parsing IPv6 host:port '%s'", buf);
> +return NULL;
> +}
This is introducing new functionality to the parser. Current callers
let empty string ":port" be used for both ipv4 and ipv6, based
on whether the flags ",ipv4[=on|off],ipv6[=on|off]" later follow.
I presume you want an explicit way to represent an empty ipv6 hostname
to avoid changing semantics for existing slirp CLI args, where the
existing ":port" exclusively means ipv4. IIC, this is also why you
needed to introduce the "is_v6" flag, because any non-empty address
can be reliably parsed without needing this flag.
This is reasonable, but any such functional change should be in a
separate commit from refactoring.
IOW, remove this and the is_v6 flag, and add them in a separate
patch to explain to the need for new functionality in the parsing.
Given that existing callers don't need to support "[]", we should
not let that be parsed, unless the caller passing a "is_v6" pointer
which is not NULL.
> +} else {
> +if (sscanf(buf, "[%64[^]]]:%32s", host, port) != 2) {
> +error_setg(errp, "error parsi
Re: supported machines for aarch64
On Fri, 19 Feb 2021 at 08:05, wrote: > These are the machine lists that included cortex-a72 when I gave > qemu-system-aarch64 --machine xxx --cpu help. Adding '--machine whatever' to your command line does not change the output of '--cpu help'. As Philippe says, it happens that QEMU processes --cpu before --machine, so it handles '--cpu help', prints the fixed list of supported CPUs, and ignores whether you passed a valid --machine option or not. There is no automated way to get QEMU to tell you which CPUs a particular board model supports. I recommend that you follow the advice given here https://qemu.readthedocs.io/en/latest/system/target-arm.html#choosing-a-board-model for how to choose a board model. (Short answer: if you know you want to run guest code for a specific board type, use that board type. Otherwise, use 'virt'.) Then, you should stick with the default CPU type (ie, do not pass --cpu) for that board, for all board types *except* 'virt'. For 'virt' you can pass in the CPU type you want (and the documentation lists which types it supports). -- PMM
Re: [PATCH v3 4/4] pcie: don't set link state active if the slot is empty
Hi,
Any comment?
This fix is really needed to fix a bug that prevents to use virtio-net failover
on recent
kernel.
Thanks,
Laurent
On 12/02/2021 14:52, Laurent Vivier wrote:
> When the pcie slot is initialized, by default PCI_EXP_LNKSTA_DLLLA
> (Data Link Layer Link Active) is set in PCI_EXP_LNKSTA
> (Link Status) without checking if the slot is empty or not.
>
> This is confusing for the kernel because as it sees the link is up
> it tries to read the vendor ID and fails:
>
> (From https://bugzilla.kernel.org/show_bug.cgi?id=211691)
>
> [1.661105] pcieport :00:02.2: pciehp: Slot Capabilities :
> 0x0002007b
> [1.661115] pcieport :00:02.2: pciehp: Slot Status: 0x0010
> [1.661123] pcieport :00:02.2: pciehp: Slot Control : 0x07c0
> [1.661138] pcieport :00:02.2: pciehp: Slot #0 AttnBtn+ PwrCtrl+ MRL-
> AttnInd+ PwrInd+ HotPlug+ Surprise+ Interlock+ NoCompl- IbPresDis- LLActRep+
> [1.662581] pcieport :00:02.2: pciehp: pciehp_get_power_status:
> SLOTCTRL 6c value read 7c0
> [1.662597] pcieport :00:02.2: pciehp: pciehp_check_link_active:
> lnk_status = 2204
> [1.662703] pcieport :00:02.2: pciehp: pending interrupts 0x0010 from
> Slot Status
> [1.662706] pcieport :00:02.2: pciehp: pcie_enable_notification:
> SLOTCTRL 6c write cmd 1031
> [1.662730] pcieport :00:02.2: pciehp: pciehp_check_link_active:
> lnk_status = 2204
> [1.662748] pcieport :00:02.2: pciehp: pciehp_check_link_active:
> lnk_status = 2204
> [1.662750] pcieport :00:02.2: pciehp: Slot(0-2): Link Up
> [2.896132] pcieport :00:02.2: pciehp: pciehp_check_link_status:
> lnk_status = 2204
> [2.896135] pcieport :00:02.2: pciehp: Slot(0-2): No device found
> [2.896900] pcieport :00:02.2: pciehp: pending interrupts 0x0010 from
> Slot Status
> [2.896903] pcieport :00:02.2: pciehp: pciehp_power_off_slot: SLOTCTRL
> 6c write cmd 400
> [3.656901] pcieport :00:02.2: pciehp: pending interrupts 0x0009 from
> Slot Status
>
> This is really a problem with virtio-net failover that hotplugs a VFIO
> card during the boot process. The kernel can shutdown the slot while
> QEMU is hotplugging it, and this likely ends by an automatic unplug of
> the card. At the end of the boot sequence the card has disappeared.
>
> To fix that, don't set the "Link Active" state in the init function, but
> rely on the plug function to do it, as the mechanism has already been
> introduced by 2f2b18f60bf1.
>
> Fixes: 2f2b18f60bf1 ("pcie: set link state inactive/active after hot
> unplug/plug")
> Cc: zhengxia...@huawei.com
> Fixes: 3d67447fe7c2 ("pcie: Fill PCIESlot link fields to support higher
> speeds and widths")
> Cc: alex.william...@redhat.com
> Fixes: b2101eae63ea ("pcie: Set the "link active" in the link status
> register")
> Cc: b...@kernel.crashing.org
> Signed-off-by: Laurent Vivier
> ---
> hw/pci/pcie.c | 19 +--
> 1 file changed, 9 insertions(+), 10 deletions(-)
>
> diff --git a/hw/pci/pcie.c b/hw/pci/pcie.c
> index d4010cf8f361..a733e2fb879a 100644
> --- a/hw/pci/pcie.c
> +++ b/hw/pci/pcie.c
> @@ -75,11 +75,6 @@ pcie_cap_v1_fill(PCIDevice *dev, uint8_t port, uint8_t
> type, uint8_t version)
> QEMU_PCI_EXP_LNKSTA_NLW(QEMU_PCI_EXP_LNK_X1) |
> QEMU_PCI_EXP_LNKSTA_CLS(QEMU_PCI_EXP_LNK_2_5GT));
>
> -if (dev->cap_present & QEMU_PCIE_LNKSTA_DLLLA) {
> -pci_word_test_and_set_mask(exp_cap + PCI_EXP_LNKSTA,
> - PCI_EXP_LNKSTA_DLLLA);
> -}
> -
> /* We changed link status bits over time, and changing them across
> * migrations is generally fine as hardware changes them too.
> * Let's not bother checking.
> @@ -125,8 +120,7 @@ static void pcie_cap_fill_slot_lnk(PCIDevice *dev)
> */
> pci_long_test_and_set_mask(exp_cap + PCI_EXP_LNKCAP,
> PCI_EXP_LNKCAP_DLLLARC);
> -pci_word_test_and_set_mask(exp_cap + PCI_EXP_LNKSTA,
> - PCI_EXP_LNKSTA_DLLLA);
> +/* the PCI_EXP_LNKSTA_DLLLA will be set in the hotplug function */
>
> /*
> * Target Link Speed defaults to the highest link speed supported by
> @@ -427,6 +421,7 @@ void pcie_cap_slot_plug_cb(HotplugHandler *hotplug_dev,
> DeviceState *dev,
> PCIDevice *hotplug_pdev = PCI_DEVICE(hotplug_dev);
> uint8_t *exp_cap = hotplug_pdev->config + hotplug_pdev->exp.exp_cap;
> PCIDevice *pci_dev = PCI_DEVICE(dev);
> +uint32_t lnkcap = pci_get_long(exp_cap + PCI_EXP_LNKCAP);
>
> /* Don't send event when device is enabled during qemu machine creation:
> * it is present on boot, no hotplug event is necessary. We do send an
> @@ -434,7 +429,8 @@ void pcie_cap_slot_plug_cb(HotplugHandler *hotplug_dev,
> DeviceState *dev,
> if (!dev->hotplugged) {
> pci_word_test_and_set_mask(exp_cap + PCI_EXP_SLTSTA
[PATCH] ui/console: Assert graphic console surface is not NULL
ui/console used to accept NULL as graphic console surface, but its
semantics was inconsistent among displays:
- cocoa and gtk-egl perform NULL dereference.
- egl-headless, spice and spice-egl do nothing.
- gtk releases underlying resources.
- sdl2-2d and sdl2-gl destroys the window.
- vnc shows a message, "Display output is not active."
Fortunately, there are only three cases where NULL is assigned as
graphic console surface, and we can study them to figure out the
desired behavior:
- virtio-gpu-base assigns NULL when the device is realized.
We have nothing to do in the case because graphic consoles
already have a surface with a message saying the content is
not initializd yet.
- virtio-gpu-3d assigns NULL when the device is reset. The initial
graphic console surfaces shows a message, so it would be
appropriate to do similar.
- virtio-gpu-3d assigns NULL when scanout is disabled. That
affects its operations later but itself do not mean any effects
on displays. Removing the operation should be fine.
This change eliminates NULL as graphic console surface by
implementing those behaviors.
An assertion is added and NULL checks are removed in ui/console
to prevent NULL from being set again. However, this do not change
any references except those in ui/console because there are too
many of them to fix for a mere motal.
Signed-off-by: Akihiko Odaki
---
hw/display/virtio-gpu-3d.c | 11 ++-
hw/display/virtio-gpu-base.c | 3 ---
ui/console.c | 31 +--
3 files changed, 15 insertions(+), 30 deletions(-)
diff --git a/hw/display/virtio-gpu-3d.c b/hw/display/virtio-gpu-3d.c
index 0b0c11474dd..4cf1901b47f 100644
--- a/hw/display/virtio-gpu-3d.c
+++ b/hw/display/virtio-gpu-3d.c
@@ -179,10 +179,6 @@ static void virgl_cmd_set_scanout(VirtIOGPU *g,
info.width, info.height,
ss.r.x, ss.r.y, ss.r.width, ss.r.height);
} else {
-if (ss.scanout_id != 0) {
-dpy_gfx_replace_surface(
-g->parent_obj.scanout[ss.scanout_id].con, NULL);
-}
dpy_gl_scanout_disable(g->parent_obj.scanout[ss.scanout_id].con);
}
g->parent_obj.scanout[ss.scanout_id].resource_id = ss.resource_id;
@@ -596,7 +592,12 @@ void virtio_gpu_virgl_reset(VirtIOGPU *g)
virgl_renderer_reset();
for (i = 0; i < g->parent_obj.conf.max_outputs; i++) {
if (i != 0) {
-dpy_gfx_replace_surface(g->parent_obj.scanout[i].con, NULL);
+DisplaySurface *surface =
+qemu_create_message_surface(g->parent_obj.conf.xres,
+g->parent_obj.conf.yres,
+"Guest reset display.");
+
+dpy_gfx_replace_surface(g->parent_obj.scanout[i].con, surface);
}
dpy_gl_scanout_disable(g->parent_obj.scanout[i].con);
}
diff --git a/hw/display/virtio-gpu-base.c b/hw/display/virtio-gpu-base.c
index 40ccd00f942..abc5fc89b9b 100644
--- a/hw/display/virtio-gpu-base.c
+++ b/hw/display/virtio-gpu-base.c
@@ -168,9 +168,6 @@ virtio_gpu_base_device_realize(DeviceState *qdev,
for (i = 0; i < g->conf.max_outputs; i++) {
g->scanout[i].con =
graphic_console_init(DEVICE(g), i, &virtio_gpu_ops, g);
-if (i > 0) {
-dpy_gfx_replace_surface(g->scanout[i].con, NULL);
-}
}
return true;
diff --git a/ui/console.c b/ui/console.c
index d80ce7037c3..bc300182c84 100644
--- a/ui/console.c
+++ b/ui/console.c
@@ -1099,10 +1099,8 @@ void console_select(unsigned int index)
dcl->ops->dpy_gfx_switch(dcl, s->surface);
}
}
-if (s->surface) {
-dpy_gfx_update(s, 0, 0, surface_width(s->surface),
- surface_height(s->surface));
-}
+dpy_gfx_update(s, 0, 0, surface_width(s->surface),
+ surface_height(s->surface));
}
if (ds->have_text) {
dpy_text_resize(s, s->width, s->height);
@@ -1587,13 +1585,9 @@ void dpy_gfx_update(QemuConsole *con, int x, int y, int
w, int h)
{
DisplayState *s = con->ds;
DisplayChangeListener *dcl;
-int width = w;
-int height = h;
+int width = surface_width(con->surface);
+int height = surface_height(con->surface);
-if (con->surface) {
-width = surface_width(con->surface);
-height = surface_height(con->surface);
-}
x = MAX(x, 0);
y = MAX(y, 0);
x = MIN(x, width);
@@ -1616,9 +1610,6 @@ void dpy_gfx_update(QemuConsole *con, int x, int y, int
w, int h)
void dpy_gfx_update_full(QemuConsole *con)
{
-if (!con->surface) {
-return;
-}
dpy_gfx_update(con, 0, 0,
surface_width(con->surface),
surface_height(con->surface));
@@ -1631,7 +1622,8 @@ void dpy_gfx_replace_surface(QemuConsole *con,
DisplaySurface *old_surface
[PATCH] virtio-blk: Respect discard granularity
Signed-off-by: Akihiko Odaki --- hw/block/virtio-blk.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c index bac2d6fa2b2..692fd17b0e0 100644 --- a/hw/block/virtio-blk.c +++ b/hw/block/virtio-blk.c @@ -965,7 +965,7 @@ static void virtio_blk_update_config(VirtIODevice *vdev, uint8_t *config) virtio_stl_p(vdev, &blkcfg.max_discard_sectors, s->conf.max_discard_sectors); virtio_stl_p(vdev, &blkcfg.discard_sector_alignment, - blk_size >> BDRV_SECTOR_BITS); + conf->discard_granularity >> BDRV_SECTOR_BITS); /* * We support only one segment per request since multiple segments * are not widely used and there are no userspace APIs that allow -- 2.24.3 (Apple Git-128)
RE: supported machines for aarch64
Hi Peter Maydell, Thanks, that made it clearer to me.(actually I was reading the page you mentioned) Chan Kim > -Original Message- > From: Peter Maydell > Sent: Friday, February 19, 2021 7:04 PM > To: Chan Kim > Cc: Philippe Mathieu-Daudé ; qemu-discuss disc...@nongnu.org>; Paolo Bonzini ; qemu-devel > ; Markus Armbruster > Subject: Re: supported machines for aarch64 > > On Fri, 19 Feb 2021 at 08:05, wrote: > > These are the machine lists that included cortex-a72 when I gave > > qemu-system-aarch64 --machine xxx --cpu help. > > Adding '--machine whatever' to your command line does not change the > output of '--cpu help'. As Philippe says, it happens that QEMU processes - > -cpu before --machine, so it handles '--cpu help', prints the fixed list > of supported CPUs, and ignores whether you passed a valid --machine option > or not. > > There is no automated way to get QEMU to tell you which CPUs a particular > board model supports. > > I recommend that you follow the advice given here > https://qemu.readthedocs.io/en/latest/system/target-arm.html#choosing-a- > board-model > for how to choose a board model. (Short answer: if you know you want to > run guest code for a specific board type, use that board type. Otherwise, > use 'virt'.) Then, you should stick with the default CPU type (ie, do not > pass --cpu) for that board, for all board types *except* 'virt'. For > 'virt' you can pass in the CPU type you want (and the documentation lists > which types it supports). > > -- PMM
Re: [PATCH] ui/cocoa: Do not exit immediately after shutdown
On Fri, 19 Feb 2021 at 08:46, Akihiko Odaki wrote:
>
> ui/cocoa used to call exit immediately after calling
> qemu_system_shutdown_request, which prevents QEMU from actually
> perfoming system shutdown. Just sleep forever, and wait QEMU to call
> exit and kill the Cocoa thread.
>
> Signed-off-by: Akihiko Odaki
> ---
> ui/cocoa.m | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/ui/cocoa.m b/ui/cocoa.m
> index 13fba8103e1..65bb74134ca 100644
> --- a/ui/cocoa.m
> +++ b/ui/cocoa.m
> @@ -1121,7 +1121,7 @@ - (void)applicationWillTerminate:(NSNotification
> *)aNotification
> COCOA_DEBUG("QemuCocoaAppController: applicationWillTerminate\n");
>
> qemu_system_shutdown_request(SHUTDOWN_CAUSE_HOST_UI);
> -exit(0);
> +[NSThread sleepForTimeInterval:INFINITY];
I think that as it stands this change is probably a bit risky,
because the QEMU action for "what do I do on a shutdown request"
is not necessarily "exit" -- the QMP 'set-action' command allows
a user to configure QEMU to just pause on a shutdown, which will
result in this loop going forever (or until OSX gets bored and
forcibly terminates the process).
It would also be useful to have a comment, something like:
/*
* Sleep here, because returning will cause OSX to kill us
* immediately; the QEMU main loop will handle the shutdown
* request and terminate the process.
*/
?
> }
Gerd: I notice that the only ui frontends that try to do this
SHUTDOWN_CAUSE_HOST_UI thing are the Cocoa UI and the SDL2 UI.
The GTK UI does
qmp_quit()
instead; the SDL2 UI does
shutdown_action = SHUTDOWN_ACTION_POWEROFF;
qemu_system_shutdown_request(SHUTDOWN_CAUSE_HOST_UI);
(presumably to avoid the "maybe the user told us to 'pause'"
issue I mention above). None of the other UI frontends have
any shutdown related handling. Shouldn't we be consistent
about how we do this ?
thanks
-- PMM
Re: [PATCH] ui/cocoa: Remove the uses of full screen APIs
On Fri, 19 Feb 2021, Akihiko Odaki wrote: 2021年2月17日(水) 22:09 Gerd Hoffmann : On Fri, Feb 12, 2021 at 09:05:40AM +0900, Akihiko Odaki wrote: The detections of full screen APIs were wrong. A detection is coded as: [NSView respondsToSelector:@selector(exitFullScreenModeWithOptions:)] but it should be: [NSView instancesRespondToSelector:@selector(exitFullScreenModeWithOptions:)] The uses of full screen APIs were also incorrect, and if you fix the detections, the full screen view stretches the video, changing the aspect ratio, even if zooming is disabled. Remove the code as it does nothing good. So, it's broken right now (and probably for quite a while without anyone complaining). And the attempt to fix it didn't work out very well. Correct? Because the detections of APIs are wrong, the code using those APIs were never executed and nobody realized it was broken. Full screen on MacOS X worked when I've last tried but that was 2 years ago. I did not seriously attempt to fix it because the APIs are no longer the best ways to implement fullscreen. ([NSWindow -toggleFullScreen:] is more favorable today.) There is not much to reuse even if implementing fullscreen with [NSView -enterFullScreenModeWithOptions:] since the code is so small. I think there are people using QEMU to run old MacOS versions on MacOS X/macOS and may not follow this mailing list but I'm sure they'll complain once you break it. Regards, BALATON Zoltan Just dropping the code makes sense to me then. Any objections or better suggestions from the macos camp? If not I'll go queue it for the next UI pull request in a day or two. thanks, Gerd Thank you for responding to my patches. Akihiko Odaki
Re: [PATCH] ui/cocoa: Use kCGColorSpaceSRGB
On Fri, 19 Feb 2021 at 08:45, Akihiko Odaki wrote: > > kCGColorSpaceGenericRGB | Apple Developer Documentation > https://developer.apple.com/documentation/coregraphics/kcgcolorspacegenericrgb > > Deprecated > > Use kCGColorSpaceSRGB instead. > > Signed-off-by: Akihiko Odaki > --- > ui/cocoa.m | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/ui/cocoa.m b/ui/cocoa.m > index 13fba8103e1..686fbb1b457 100644 > --- a/ui/cocoa.m > +++ b/ui/cocoa.m > @@ -437,7 +437,7 @@ - (void) drawRect:(NSRect) rect > screen.bitsPerPixel, //bitsPerPixel > (screen.width * (screen.bitsPerComponent/2)), //bytesPerRow > #ifdef __LITTLE_ENDIAN__ > -CGColorSpaceCreateWithName(kCGColorSpaceGenericRGB), > //colorspace for OS X >= 10.4 > +CGColorSpaceCreateWithName(kCGColorSpaceSRGB), //colorspace for > OS X >= 10.5 > kCGBitmapByteOrder32Little | kCGImageAlphaNoneSkipFirst, > #else > CGColorSpaceCreateDeviceRGB(), //colorspace for OS X < 10.4 > (actually ppc) > -- The comment change here looks a little odd, because previously it was a matched pair with the one in the other half of the #ifdef: one side is "for OS X >= 10.4" and the other "for < 10.4". After this change we have a mismatch. In fact it doesn't matter because we don't support any OSX version that old any more anyway. I think we should delete the whole #ifdef...#else...#endif block here, and replace it with just the CGColorSpaceCreateWithName(kCGColorSpaceSRGB), kCGBitmapByteOrder32Little | kCGImageAlphaNoneSkipFirst, lines, because we dropped PPC support a long long time ago. (And we don't need any comment about OSX version if we do that.) thanks -- PMM
Re: [PATCH] net: e1000: check transmit descriptor field values
On Fri, 19 Feb 2021 at 03:06, Alexander Bulekov wrote: > > On 210210 2022, P J P wrote: > > From: Prasad J Pandit > > > > While processing transmit (tx) descriptors in process_tx_desc() > > various descriptor fields are not checked properly. This may lead > > to infinite loop like issue. Add checks to avoid them. > > > > +CC Peter Maydell > > Is this a DMA re-entracy/stack-overflow issue? IIRC the plan was to have > some sort of wider fix for these issues. There are bunch of these > reports floating around at this point, I believe. I have no idea, because the commit message for this patch does not describe the failure in any detail at all and has no links to any bug report or test case for the failures it claims to be fixing... -- PMM
Re: [PATCH v2] Correct CRIS TCG register lifetime management
> On 18 Feb 2021, at 21:26, Philippe Mathieu-Daudé wrote: > > On 2/18/21 9:10 PM, Stefan Sandström wrote: >>> On 18 Feb 2021, at 20:59, Philippe Mathieu-Daudé wrote: >>> >>> Hi Stefan, >>> On 2/18/21 7:43 PM, Stefan Sandström wrote: From: Stefan Sandstrom Add and fix deallocation of temporary TCG registers in CRIS code generation. >>> >>> What did you run to figure this out? >> >> Hi Philippe, >> >> We were looking at the code in search for an issue that showed up when >> switching to a new version of GCC. >> It looked like QEMU executed the CRIS code wrong, so we took a look at the >> code. The problem turned out to be outside QEMU. These potential issues were >> spotted in the process. > > Good to know. Do you mind sharing your GCC tests, so we can have more > CRIS testing? Unfortunately, we do not have such tests. But if we end up writing such tests, we'll be happy to share them. In this case, tt was the GCC used for building QEMU that was upgraded, and some production firmware that was tested in QEMU that stopped working. Best regards, -stefan > > Regards, > > Phil.
Re: [RFC PATCH v2 1/3] vfio: Move the saving of the config space to the right place in VFIO migration
On 2021/2/18 22:42, Kirti Wankhede wrote:
>
>
> On 12/9/2020 1:39 PM, Shenming Lu wrote:
>> On ARM64 the VFIO SET_IRQS ioctl is dependent on the VM interrupt
>> setup, if the restoring of the VFIO PCI device config space is
>> before the VGIC, an error might occur in the kernel.
>>
>> So we move the saving of the config space to the non-iterable
>> process, so that it will be called after the VGIC according to
>> their priorities.
>>
>> As for the possible dependence of the device specific migration
>> data on it's config space, we can let the vendor driver to
>> include any config info it needs in its own data stream.
>> (Should we note this in the header file linux-headers/linux/vfio.h?)
>>
>> Signed-off-by: Shenming Lu
>> ---
>> hw/vfio/migration.c | 25 +++--
>> 1 file changed, 15 insertions(+), 10 deletions(-)
>>
>> diff --git a/hw/vfio/migration.c b/hw/vfio/migration.c
>> index 00daa50ed8..3b9de1353a 100644
>> --- a/hw/vfio/migration.c
>> +++ b/hw/vfio/migration.c
>> @@ -575,11 +575,6 @@ static int vfio_save_complete_precopy(QEMUFile *f, void
>> *opaque)
>> return ret;
>> }
>> - ret = vfio_save_device_config_state(f, opaque);
>> - if (ret) {
>> - return ret;
>> - }
>> -
>> ret = vfio_update_pending(vbasedev);
>> if (ret) {
>> return ret;
>> @@ -620,6 +615,19 @@ static int vfio_save_complete_precopy(QEMUFile *f, void
>> *opaque)
>> return ret;
>> }
>> +static void vfio_save_state(QEMUFile *f, void *opaque)
>> +{
>> + VFIODevice *vbasedev = opaque;
>> + int ret;
>> +
>> + /* The device specific data is migrated in the iterable process. */
>> + ret = vfio_save_device_config_state(f, opaque);
>> + if (ret) {
>> + error_report("%s: Failed to save device config space",
>> + vbasedev->name);
>> + }
>> +}
>> +
>
> Since error is not propagated, set error in migration stream for migration to
> fail, use qemu_file_set_error() on error.
Makes sense. I will send a v3 soon. Thanks,
Shenming
>
> Thanks,
> Kirti
>
>> static int vfio_load_setup(QEMUFile *f, void *opaque)
>> {
>> VFIODevice *vbasedev = opaque;
>> @@ -670,11 +678,7 @@ static int vfio_load_state(QEMUFile *f, void *opaque,
>> int version_id)
>> switch (data) {
>> case VFIO_MIG_FLAG_DEV_CONFIG_STATE:
>> {
>> - ret = vfio_load_device_config_state(f, opaque);
>> - if (ret) {
>> - return ret;
>> - }
>> - break;
>> + return vfio_load_device_config_state(f, opaque);
>> }
>> case VFIO_MIG_FLAG_DEV_SETUP_STATE:
>> {
>> @@ -720,6 +724,7 @@ static SaveVMHandlers savevm_vfio_handlers = {
>> .save_live_pending = vfio_save_pending,
>> .save_live_iterate = vfio_save_iterate,
>> .save_live_complete_precopy = vfio_save_complete_precopy,
>> + .save_state = vfio_save_state,
>> .load_setup = vfio_load_setup,
>> .load_cleanup = vfio_load_cleanup,
>> .load_state = vfio_load_state,
>>
> .
FreeBSD build regressions
Hi, It looks like the build has been broken on Cirrus since at least 7b2c4c: https://cirrus-ci.com/github/qemu/qemu I did attempt to have a look but "vm-build-freebsd" seems to be failing with a different error: 10:31:47 [alex.bennee@hackbox2:~/l/q/b/all] master|✔ + make vm-build-freebsd GIT ui/keycodemapdb tests/fp/berkeley-testfloat-3 tests/fp/berkeley-softfloat-3 meson dtc capstone slirp VM-BUILD freebsd cross containers no NOTE: guest cross-compilers enabled: cc cc ld-elf.so.1: /usr/local/lib/libpython3.7m.so.1.0: Undefined symbol "close_range@FBSD_1.6" ld-elf.so.1: /usr/local/lib/libpython3.7m.so.1.0: Undefined symbol "close_range@FBSD_1.6" The Meson build system Version: 0.55.3 Source dir: /usr/home/qemu/qemu-test.egp8wG/src Build dir: /usr/home/qemu/qemu-test.egp8wG/build Build type: native build Project name: qemu Project version: 5.2.50 ld-elf.so.1: /usr/local/lib/libpython3.7m.so.1.0: Undefined symbol "close_range@FBSD_1.6" ../src/meson.build:1:0: ERROR: Executables created by c compiler cc are not runnable. A full log can be found at /usr/home/qemu/qemu-test.egp8wG/build/meson-logs/meson-log.txt ERROR: meson setup failed /home/alex.bennee/lsrc/qemu.git/tests/vm/Makefile.include:95: recipe for target 'vm-build-freebsd' failed make: *** [vm-build-freebsd] Error 3 Tracking back to before the previously mentioned commit it was still failing which makes me think something has happened to the BSD image (or something is missing since the build changes). I'd appreciate it if someone with more FreeBSD knowledge can look into both regressions because frankly I find it exhausting enough tracking down Linux regressions when they occur. Thanks, -- Alex Bennée
Re: FreeBSD build regressions
On Fri, 19 Feb 2021 at 10:39, Alex Bennée wrote: > > > Hi, > > It looks like the build has been broken on Cirrus since at least 7b2c4c: > > https://cirrus-ci.com/github/qemu/qemu > > I did attempt to have a look but "vm-build-freebsd" seems to be failing > with a different error FWIW the vm-build-freebsd build-and-test works for me, as I continue to run it as part of the merge tests. Is this something to do with whether you already have a freebsd image cached as opposed to it getting re-built from scratch (perhaps with a newer FreeBSD)? -- PMM
Re: FreeBSD build regressions
On Fri, Feb 19, 2021 at 10:29:50AM +, Alex Bennée wrote: > > Hi, > > It looks like the build has been broken on Cirrus since at least 7b2c4c: > > https://cirrus-ci.com/github/qemu/qemu > > I did attempt to have a look but "vm-build-freebsd" seems to be failing > with a different error: > > 10:31:47 [alex.bennee@hackbox2:~/l/q/b/all] master|✔ + make > vm-build-freebsd > GIT ui/keycodemapdb tests/fp/berkeley-testfloat-3 > tests/fp/berkeley-softfloat-3 meson dtc capstone slirp > VM-BUILD freebsd > cross containers no > > NOTE: guest cross-compilers enabled: cc cc > ld-elf.so.1: /usr/local/lib/libpython3.7m.so.1.0: Undefined symbol > "close_range@FBSD_1.6" > ld-elf.so.1: /usr/local/lib/libpython3.7m.so.1.0: Undefined symbol > "close_range@FBSD_1.6" > The Meson build system > Version: 0.55.3 > Source dir: /usr/home/qemu/qemu-test.egp8wG/src > Build dir: /usr/home/qemu/qemu-test.egp8wG/build > Build type: native build > Project name: qemu > Project version: 5.2.50 > ld-elf.so.1: /usr/local/lib/libpython3.7m.so.1.0: Undefined symbol > "close_range@FBSD_1.6" > > ../src/meson.build:1:0: ERROR: Executables created by c compiler cc are not > runnable. > > A full log can be found at > /usr/home/qemu/qemu-test.egp8wG/build/meson-logs/meson-log.txt > > ERROR: meson setup failed > > /home/alex.bennee/lsrc/qemu.git/tests/vm/Makefile.include:95: recipe for > target 'vm-build-freebsd' failed > make: *** [vm-build-freebsd] Error 3 > > Tracking back to before the previously mentioned commit it was still > failing which makes me think something has happened to the BSD image (or > something is missing since the build changes). I'd appreciate it if > someone with more FreeBSD knowledge can look into both regressions > because frankly I find it exhausting enough tracking down Linux > regressions when they occur. Yes, this is a problem with the image in Cirrus, which hit libvirt CI too. We "fixed" it by changing to the freebsd-12-2 instead of freebsd-12-1 image. Regards, Daniel -- |: https://berrange.com -o-https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o-https://fstop138.berrange.com :| |: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|
[PATCH v3] Correct CRIS TCG register lifetime management
From: Stefan Sandstrom
Add and fix deallocation of temporary TCG registers in CRIS code
generation.
Change-Id: I17fce5d95bdc4418337ba885d53ba97afb1bafcc
---
target/cris/translate.c | 127 +++-
target/cris/translate_v10.c.inc | 70 --
2 files changed, 138 insertions(+), 59 deletions(-)
diff --git a/target/cris/translate.c b/target/cris/translate.c
index c893f877ab..2b35d818dd 100644
--- a/target/cris/translate.c
+++ b/target/cris/translate.c
@@ -172,14 +172,21 @@ static int preg_sizes[] = {
tcg_gen_ld_tl(tn, cpu_env, offsetof(CPUCRISState, member))
#define t_gen_mov_env_TN(member, tn) \
tcg_gen_st_tl(tn, cpu_env, offsetof(CPUCRISState, member))
+#define t_gen_movi_env_TN(member, c) \
+do { \
+TCGv tc = tcg_const_tl(c); \
+t_gen_mov_env_TN(member, tc); \
+tcg_temp_free(tc); \
+} while (0)
+
static inline void t_gen_mov_TN_preg(TCGv tn, int r)
{
assert(r >= 0 && r <= 15);
if (r == PR_BZ || r == PR_WZ || r == PR_DZ) {
-tcg_gen_mov_tl(tn, tcg_const_tl(0));
+tcg_gen_movi_tl(tn, 0);
} else if (r == PR_VR) {
-tcg_gen_mov_tl(tn, tcg_const_tl(32));
+tcg_gen_movi_tl(tn, 32);
} else {
tcg_gen_mov_tl(tn, cpu_PR[r]);
}
@@ -204,6 +211,8 @@ static inline void t_gen_mov_preg_TN(DisasContext *dc, int
r, TCGv tn)
}
}
+
+
/* Sign extend at translation time. */
static int sign_extend(unsigned int val, unsigned int width)
{
@@ -256,7 +265,7 @@ static int cris_fetch(CPUCRISState *env, DisasContext *dc,
uint32_t addr,
static void cris_lock_irq(DisasContext *dc)
{
dc->clear_locked_irq = 0;
-t_gen_mov_env_TN(locked_irq, tcg_const_tl(1));
+t_gen_movi_env_TN(locked_irq, 1);
}
static inline void t_gen_raise_exception(uint32_t index)
@@ -885,8 +894,7 @@ static void gen_tst_cc (DisasContext *dc, TCGv cc, int cond)
case CC_EQ:
if ((arith_opt || move_opt)
&& dc->cc_x_uptodate != (2 | X_FLAG)) {
-tcg_gen_setcond_tl(TCG_COND_EQ, cc,
-cc_result, tcg_const_tl(0));
+tcg_gen_setcondi_tl(TCG_COND_EQ, cc, cc_result, 0);
} else {
cris_evaluate_flags(dc);
tcg_gen_andi_tl(cc,
@@ -1330,14 +1338,17 @@ static int dec_addoq(CPUCRISState *env, DisasContext
*dc)
}
static int dec_addq(CPUCRISState *env, DisasContext *dc)
{
+TCGv c;
LOG_DIS("addq %u, $r%u\n", dc->op1, dc->op2);
dc->op1 = EXTRACT_FIELD(dc->ir, 0, 5);
cris_cc_mask(dc, CC_MASK_NZVC);
+c = tcg_const_tl(dc->op1);
cris_alu(dc, CC_OP_ADD,
-cpu_R[dc->op2], cpu_R[dc->op2], tcg_const_tl(dc->op1), 4);
+cpu_R[dc->op2], cpu_R[dc->op2], c, 4);
+tcg_temp_free(c);
return 2;
}
static int dec_moveq(CPUCRISState *env, DisasContext *dc)
@@ -1353,62 +1364,77 @@ static int dec_moveq(CPUCRISState *env, DisasContext
*dc)
}
static int dec_subq(CPUCRISState *env, DisasContext *dc)
{
+TCGv c;
dc->op1 = EXTRACT_FIELD(dc->ir, 0, 5);
LOG_DIS("subq %u, $r%u\n", dc->op1, dc->op2);
cris_cc_mask(dc, CC_MASK_NZVC);
+c = tcg_const_tl(dc->op1);
cris_alu(dc, CC_OP_SUB,
-cpu_R[dc->op2], cpu_R[dc->op2], tcg_const_tl(dc->op1), 4);
+cpu_R[dc->op2], cpu_R[dc->op2], c, 4);
+tcg_temp_free(c);
return 2;
}
static int dec_cmpq(CPUCRISState *env, DisasContext *dc)
{
uint32_t imm;
+TCGv c;
dc->op1 = EXTRACT_FIELD(dc->ir, 0, 5);
imm = sign_extend(dc->op1, 5);
LOG_DIS("cmpq %d, $r%d\n", imm, dc->op2);
cris_cc_mask(dc, CC_MASK_NZVC);
+c = tcg_const_tl(imm);
cris_alu(dc, CC_OP_CMP,
-cpu_R[dc->op2], cpu_R[dc->op2], tcg_const_tl(imm), 4);
+cpu_R[dc->op2], cpu_R[dc->op2], c, 4);
+tcg_temp_free(c);
return 2;
}
static int dec_andq(CPUCRISState *env, DisasContext *dc)
{
uint32_t imm;
+TCGv c;
dc->op1 = EXTRACT_FIELD(dc->ir, 0, 5);
imm = sign_extend(dc->op1, 5);
LOG_DIS("andq %d, $r%d\n", imm, dc->op2);
cris_cc_mask(dc, CC_MASK_NZ);
+c = tcg_const_tl(imm);
cris_alu(dc, CC_OP_AND,
-cpu_R[dc->op2], cpu_R[dc->op2], tcg_const_tl(imm), 4);
+cpu_R[dc->op2], cpu_R[dc->op2], c, 4);
+tcg_temp_free(c);
return 2;
}
static int dec_orq(CPUCRISState *env, DisasContext *dc)
{
uint32_t imm;
+TCGv c;
dc->op1 = EXTRACT_FIELD(dc->ir, 0, 5);
imm = sign_extend(dc->op1, 5);
LOG_DIS("orq %d, $r%d\n", imm, dc->op2);
cris_cc_mask(dc, CC_MASK_NZ);
+c = tcg_const_tl(imm);
cris_alu(dc, CC_OP_OR,
-cpu_R[dc->op2], cpu_R[dc->op2], tcg_const_tl(imm), 4);
+cpu_R[dc->op2], cpu_R[dc->op2], c, 4);
+tcg_temp_free(c);
return 2;
}
static int dec_btstq(CPUCRISState *env, DisasContext *dc)
{
+TCGv c;
dc->op1 = EXTRACT_FIELD(dc->ir, 0, 4);
LOG_DIS("btstq %u, $r%d\n", dc->op1, dc->op2);
cris_cc
Re: [PATCH v3] Correct CRIS TCG register lifetime management
Patchew URL: https://patchew.org/QEMU/20210219104548.4675-1-stef...@axis.com/ Hi, This series seems to have some coding style problems. See output below for more information: Type: series Message-id: 20210219104548.4675-1-stef...@axis.com Subject: [PATCH v3] Correct CRIS TCG register lifetime management === TEST SCRIPT BEGIN === #!/bin/bash git rev-parse base > /dev/null || exit 0 git config --local diff.renamelimit 0 git config --local diff.renames True git config --local diff.algorithm histogram ./scripts/checkpatch.pl --mailback base.. === TEST SCRIPT END === Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384 From https://github.com/patchew-project/qemu * [new tag] patchew/20210219104548.4675-1-stef...@axis.com -> patchew/20210219104548.4675-1-stef...@axis.com Switched to a new branch 'test' de034b5 Correct CRIS TCG register lifetime management === OUTPUT BEGIN === ERROR: Missing Signed-off-by: line(s) total: 1 errors, 0 warnings, 596 lines checked Commit de034b5ced42 (Correct CRIS TCG register lifetime management) has style problems, please review. If any of these errors are false positives report them to the maintainer, see CHECKPATCH in MAINTAINERS. === OUTPUT END === Test command exited with code: 1 The full log is available at http://patchew.org/logs/20210219104548.4675-1-stef...@axis.com/testing.checkpatch/?type=message. --- Email generated automatically by Patchew [https://patchew.org/]. Please send your feedback to patchew-de...@redhat.com
Re: [PULL v2 00/35] hexagon initial commit
On Thu, 18 Feb 2021 at 16:29, Richard Henderson wrote: > > Initial commit for the Qualcomm Hexagon processor. > > Hi; Coverity Scan reports a pile of new issues in the Hexagon code; could one of you go through them and confirm whether they are either false positives or else provide fixes for them, please? thanks -- PMM
[PATCH v3] Correct CRIS TCG register lifetime management
From: Stefan Sandstrom
Add and fix deallocation of temporary TCG registers in CRIS code
generation.
Change-Id: I17fce5d95bdc4418337ba885d53ba97afb1bafcc
Signed-off-by: Stefan Sandström
---
target/cris/translate.c | 127 +++-
target/cris/translate_v10.c.inc | 70 --
2 files changed, 138 insertions(+), 59 deletions(-)
diff --git a/target/cris/translate.c b/target/cris/translate.c
index c893f877ab..2b35d818dd 100644
--- a/target/cris/translate.c
+++ b/target/cris/translate.c
@@ -172,14 +172,21 @@ static int preg_sizes[] = {
tcg_gen_ld_tl(tn, cpu_env, offsetof(CPUCRISState, member))
#define t_gen_mov_env_TN(member, tn) \
tcg_gen_st_tl(tn, cpu_env, offsetof(CPUCRISState, member))
+#define t_gen_movi_env_TN(member, c) \
+do { \
+TCGv tc = tcg_const_tl(c); \
+t_gen_mov_env_TN(member, tc); \
+tcg_temp_free(tc); \
+} while (0)
+
static inline void t_gen_mov_TN_preg(TCGv tn, int r)
{
assert(r >= 0 && r <= 15);
if (r == PR_BZ || r == PR_WZ || r == PR_DZ) {
-tcg_gen_mov_tl(tn, tcg_const_tl(0));
+tcg_gen_movi_tl(tn, 0);
} else if (r == PR_VR) {
-tcg_gen_mov_tl(tn, tcg_const_tl(32));
+tcg_gen_movi_tl(tn, 32);
} else {
tcg_gen_mov_tl(tn, cpu_PR[r]);
}
@@ -204,6 +211,8 @@ static inline void t_gen_mov_preg_TN(DisasContext *dc, int
r, TCGv tn)
}
}
+
+
/* Sign extend at translation time. */
static int sign_extend(unsigned int val, unsigned int width)
{
@@ -256,7 +265,7 @@ static int cris_fetch(CPUCRISState *env, DisasContext *dc,
uint32_t addr,
static void cris_lock_irq(DisasContext *dc)
{
dc->clear_locked_irq = 0;
-t_gen_mov_env_TN(locked_irq, tcg_const_tl(1));
+t_gen_movi_env_TN(locked_irq, 1);
}
static inline void t_gen_raise_exception(uint32_t index)
@@ -885,8 +894,7 @@ static void gen_tst_cc (DisasContext *dc, TCGv cc, int cond)
case CC_EQ:
if ((arith_opt || move_opt)
&& dc->cc_x_uptodate != (2 | X_FLAG)) {
-tcg_gen_setcond_tl(TCG_COND_EQ, cc,
-cc_result, tcg_const_tl(0));
+tcg_gen_setcondi_tl(TCG_COND_EQ, cc, cc_result, 0);
} else {
cris_evaluate_flags(dc);
tcg_gen_andi_tl(cc,
@@ -1330,14 +1338,17 @@ static int dec_addoq(CPUCRISState *env, DisasContext
*dc)
}
static int dec_addq(CPUCRISState *env, DisasContext *dc)
{
+TCGv c;
LOG_DIS("addq %u, $r%u\n", dc->op1, dc->op2);
dc->op1 = EXTRACT_FIELD(dc->ir, 0, 5);
cris_cc_mask(dc, CC_MASK_NZVC);
+c = tcg_const_tl(dc->op1);
cris_alu(dc, CC_OP_ADD,
-cpu_R[dc->op2], cpu_R[dc->op2], tcg_const_tl(dc->op1), 4);
+cpu_R[dc->op2], cpu_R[dc->op2], c, 4);
+tcg_temp_free(c);
return 2;
}
static int dec_moveq(CPUCRISState *env, DisasContext *dc)
@@ -1353,62 +1364,77 @@ static int dec_moveq(CPUCRISState *env, DisasContext
*dc)
}
static int dec_subq(CPUCRISState *env, DisasContext *dc)
{
+TCGv c;
dc->op1 = EXTRACT_FIELD(dc->ir, 0, 5);
LOG_DIS("subq %u, $r%u\n", dc->op1, dc->op2);
cris_cc_mask(dc, CC_MASK_NZVC);
+c = tcg_const_tl(dc->op1);
cris_alu(dc, CC_OP_SUB,
-cpu_R[dc->op2], cpu_R[dc->op2], tcg_const_tl(dc->op1), 4);
+cpu_R[dc->op2], cpu_R[dc->op2], c, 4);
+tcg_temp_free(c);
return 2;
}
static int dec_cmpq(CPUCRISState *env, DisasContext *dc)
{
uint32_t imm;
+TCGv c;
dc->op1 = EXTRACT_FIELD(dc->ir, 0, 5);
imm = sign_extend(dc->op1, 5);
LOG_DIS("cmpq %d, $r%d\n", imm, dc->op2);
cris_cc_mask(dc, CC_MASK_NZVC);
+c = tcg_const_tl(imm);
cris_alu(dc, CC_OP_CMP,
-cpu_R[dc->op2], cpu_R[dc->op2], tcg_const_tl(imm), 4);
+cpu_R[dc->op2], cpu_R[dc->op2], c, 4);
+tcg_temp_free(c);
return 2;
}
static int dec_andq(CPUCRISState *env, DisasContext *dc)
{
uint32_t imm;
+TCGv c;
dc->op1 = EXTRACT_FIELD(dc->ir, 0, 5);
imm = sign_extend(dc->op1, 5);
LOG_DIS("andq %d, $r%d\n", imm, dc->op2);
cris_cc_mask(dc, CC_MASK_NZ);
+c = tcg_const_tl(imm);
cris_alu(dc, CC_OP_AND,
-cpu_R[dc->op2], cpu_R[dc->op2], tcg_const_tl(imm), 4);
+cpu_R[dc->op2], cpu_R[dc->op2], c, 4);
+tcg_temp_free(c);
return 2;
}
static int dec_orq(CPUCRISState *env, DisasContext *dc)
{
uint32_t imm;
+TCGv c;
dc->op1 = EXTRACT_FIELD(dc->ir, 0, 5);
imm = sign_extend(dc->op1, 5);
LOG_DIS("orq %d, $r%d\n", imm, dc->op2);
cris_cc_mask(dc, CC_MASK_NZ);
+c = tcg_const_tl(imm);
cris_alu(dc, CC_OP_OR,
-cpu_R[dc->op2], cpu_R[dc->op2], tcg_const_tl(imm), 4);
+cpu_R[dc->op2], cpu_R[dc->op2], c, 4);
+tcg_temp_free(c);
return 2;
}
static int dec_btstq(CPUCRISState *env, DisasContext *dc)
{
+TCGv c;
dc->op1 = EXTRACT_FIELD(dc->ir, 0, 4);
LOG_DIS("btstq %u, $r%d\n",
Re: FreeBSD build regressions
Peter Maydell writes: > On Fri, 19 Feb 2021 at 10:39, Alex Bennée wrote: >> >> >> Hi, >> >> It looks like the build has been broken on Cirrus since at least 7b2c4c: >> >> https://cirrus-ci.com/github/qemu/qemu >> >> I did attempt to have a look but "vm-build-freebsd" seems to be failing >> with a different error > > FWIW the vm-build-freebsd build-and-test works for me, as I > continue to run it as part of the merge tests. Is this something > to do with whether you already have a freebsd image cached > as opposed to it getting re-built from scratch (perhaps with > a newer FreeBSD)? It did re-run the installation when I first called the target so I guess it was that. -- Alex Bennée
Re: [PATCH] ui/cocoa: Remove the uses of full screen APIs
2021年2月19日(金) 19:24 BALATON Zoltan : > > On Fri, 19 Feb 2021, Akihiko Odaki wrote: > > 2021年2月17日(水) 22:09 Gerd Hoffmann : > >> > >> On Fri, Feb 12, 2021 at 09:05:40AM +0900, Akihiko Odaki wrote: > >>> The detections of full screen APIs were wrong. A detection is coded as: > >>> [NSView respondsToSelector:@selector(exitFullScreenModeWithOptions:)] > >>> but it should be: > >>> [NSView > >>> instancesRespondToSelector:@selector(exitFullScreenModeWithOptions:)] > >>> > >>> The uses of full screen APIs were also incorrect, and if you fix the > >>> detections, the full screen view stretches the video, changing the > >>> aspect ratio, even if zooming is disabled. > >>> > >>> Remove the code as it does nothing good. > >> > >> So, it's broken right now (and probably for quite a while without anyone > >> complaining). And the attempt to fix it didn't work out very well. > >> Correct? > > > > Because the detections of APIs are wrong, the code using those APIs > > were never executed and nobody realized it was broken. > > Full screen on MacOS X worked when I've last tried but that was 2 years > ago. > > > I did not seriously attempt to fix it because the APIs are no longer > > the best ways to implement fullscreen. ([NSWindow -toggleFullScreen:] > > is more favorable today.) There is not much to reuse even if > > implementing fullscreen with [NSView -enterFullScreenModeWithOptions:] > > since the code is so small. > > I think there are people using QEMU to run old MacOS versions on MacOS > X/macOS and may not follow this mailing list but I'm sure they'll complain > once you break it. It was not clear what "full screen APIs" refer to in my patch. Today macOS have three different methods to enter fullscreen: - [NSWindow -toggleFullscreen:] (the latest one) - [NSView -enterFullScreenModeWithOptions:] - Make a borderless window whose frame matches with the screen ui/cocoa checks if [NSView -enterFullScreenModeWithOptions:] exists and uses it in this case. Otherwise, it chooses the last method. However, the detection of [NSView -enterFullScreenModeWithOptions:] was broken and I fixed it to find the use of [NSView -enterFullScreenModeWithOptions:] was wrong. This patch deletes references to [NSView -enterFullScreenModeWithOptions:] but the code implementing the last method is still intact and properly functions. Akihiko Odaki > > Regards, > BALATON Zoltan > > >> Just dropping the code makes sense to me then. > >> > >> Any objections or better suggestions from the macos camp? > >> If not I'll go queue it for the next UI pull request in a day or two. > >> > >> thanks, > >> Gerd > >> > > > > Thank you for responding to my patches. > > > > Akihiko Odaki > > > >
[PATCH] tests/acceptance: linux-related tests fix
This patch allows cloudinit images download when ssh key is not specified. Signed-off-by: Pavel Dovgalyuk --- tests/acceptance/avocado_qemu/__init__.py |6 -- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/tests/acceptance/avocado_qemu/__init__.py b/tests/acceptance/avocado_qemu/__init__.py index df167b142c..4ac9f72876 100644 --- a/tests/acceptance/avocado_qemu/__init__.py +++ b/tests/acceptance/avocado_qemu/__init__.py @@ -304,8 +304,10 @@ def prepare_cloudinit(self, ssh_pubkey=None): try: cloudinit_iso = os.path.join(self.workdir, 'cloudinit.iso') self.phone_home_port = network.find_free_port() -with open(ssh_pubkey) as pubkey: -pubkey_content = pubkey.read() +pubkey_content = None +if ssh_pubkey: +with open(ssh_pubkey) as pubkey: +pubkey_content = pubkey.read() cloudinit.iso(cloudinit_iso, self.name, username='root', password='password',
Re: [PATCH v2 0/2] block: Use 'read-zeroes=true' mode by default with 'null-co' driver
On 13.02.21 22:54, Fam Zheng wrote: On 2021-02-11 15:26, Philippe Mathieu-Daudé wrote: The null-co driver doesn't zeroize buffer in its default config, because it is designed for testing and tests want to run fast. However this confuses security researchers (access to uninit buffers). I'm a little surprised. Is changing default the only way to fix this? I'm not opposed to changing the default but I'm not convinced this is the easiest way. block/nvme.c also doesn't touch the memory, but defers to the device DMA, why doesn't that confuse the security checker? Cannot we just somehow annotate it in a way that the checker can understand (akin to how we provide coverity models) and be done? The question is, why wouldn’t we change the default? read-zeroes=true seems the better default to me. I consider silencing valgrind warnings and the like a nice side effect. Max
[PATCH] gitlab-ci: Remove unused container images
We're building a lot of containers in the gitlab-CI that we never use. This takes away network bandwidth and CPU time from other jobs for no use, so let's remove them for now. The individual containers could be re-added later when we really need them. Signed-off-by: Thomas Huth --- .gitlab-ci.d/containers.yml | 92 - 1 file changed, 92 deletions(-) diff --git a/.gitlab-ci.d/containers.yml b/.gitlab-ci.d/containers.yml index 90fac85ce4..233d34c59b 100644 --- a/.gitlab-ci.d/containers.yml +++ b/.gitlab-ci.d/containers.yml @@ -53,13 +53,6 @@ amd64-debian11-container: variables: NAME: debian11 -alpha-debian-cross-container: - <<: *container_job_definition - stage: containers-layer2 - needs: ['amd64-debian10-container'] - variables: -NAME: debian-alpha-cross - amd64-debian-cross-container: <<: *container_job_definition stage: containers-layer2 @@ -88,13 +81,6 @@ arm64-debian-cross-container: variables: NAME: debian-arm64-cross -arm64-test-debian-cross-container: - <<: *container_job_definition - stage: containers-layer2 - needs: ['amd64-debian11-container'] - variables: -NAME: debian-arm64-test-cross - armel-debian-cross-container: <<: *container_job_definition stage: containers-layer2 @@ -109,27 +95,6 @@ armhf-debian-cross-container: variables: NAME: debian-armhf-cross -hppa-debian-cross-container: - <<: *container_job_definition - stage: containers-layer2 - needs: ['amd64-debian10-container'] - variables: -NAME: debian-hppa-cross - -m68k-debian-cross-container: - <<: *container_job_definition - stage: containers-layer2 - needs: ['amd64-debian10-container'] - variables: -NAME: debian-m68k-cross - -mips64-debian-cross-container: - <<: *container_job_definition - stage: containers-layer2 - needs: ['amd64-debian10-container'] - variables: -NAME: debian-mips64-cross - mips64el-debian-cross-container: <<: *container_job_definition stage: containers-layer2 @@ -151,20 +116,6 @@ mipsel-debian-cross-container: variables: NAME: debian-mipsel-cross -powerpc-debian-cross-container: - <<: *container_job_definition - stage: containers-layer2 - needs: ['amd64-debian10-container'] - variables: -NAME: debian-powerpc-cross - -ppc64-debian-cross-container: - <<: *container_job_definition - stage: containers-layer2 - needs: ['amd64-debian10-container'] - variables: -NAME: debian-ppc64-cross - ppc64el-debian-cross-container: <<: *container_job_definition stage: containers-layer2 @@ -172,13 +123,6 @@ ppc64el-debian-cross-container: variables: NAME: debian-ppc64el-cross -riscv64-debian-cross-container: - <<: *container_job_definition - stage: containers-layer2 - needs: ['amd64-debian10-container'] - variables: -NAME: debian-riscv64-cross - s390x-debian-cross-container: <<: *container_job_definition stage: containers-layer2 @@ -186,37 +130,6 @@ s390x-debian-cross-container: variables: NAME: debian-s390x-cross -sh4-debian-cross-container: - <<: *container_job_definition - stage: containers-layer2 - needs: ['amd64-debian10-container'] - variables: -NAME: debian-sh4-cross - -sparc64-debian-cross-container: - <<: *container_job_definition - stage: containers-layer2 - needs: ['amd64-debian10-container'] - variables: -NAME: debian-sparc64-cross - -tricore-debian-cross-container: - <<: *container_job_definition - stage: containers-layer2 - needs: ['amd64-debian10-container'] - variables: -NAME: debian-tricore-cross - -xtensa-debian-cross-container: - <<: *container_job_definition - variables: -NAME: debian-xtensa-cross - -cris-fedora-cross-container: - <<: *container_job_definition - variables: -NAME: fedora-cris-cross - amd64-fedora-container: <<: *container_job_definition variables: @@ -237,11 +150,6 @@ win64-fedora-cross-container: variables: NAME: fedora-win64-cross -amd64-ubuntu1804-container: - <<: *container_job_definition - variables: -NAME: ubuntu1804 - amd64-ubuntu2004-container: <<: *container_job_definition variables: -- 2.27.0
[PATCH v2] ui/cocoa: Do not exit immediately after shutdown
ui/cocoa used to call exit immediately after calling
qemu_system_shutdown_request, which prevents QEMU from actually
perfoming system shutdown. Just sleep forever, and wait QEMU to call
exit and kill the Cocoa thread.
Signed-off-by: Akihiko Odaki
---
ui/cocoa.m | 8 +++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/ui/cocoa.m b/ui/cocoa.m
index 13fba8103e1..cbd03667bd8 100644
--- a/ui/cocoa.m
+++ b/ui/cocoa.m
@@ -1121,7 +1121,13 @@ - (void)applicationWillTerminate:(NSNotification
*)aNotification
COCOA_DEBUG("QemuCocoaAppController: applicationWillTerminate\n");
qemu_system_shutdown_request(SHUTDOWN_CAUSE_HOST_UI);
-exit(0);
+
+/*
+ * Sleep here, because returning will cause OSX to kill us
+ * immediately; the QEMU main loop will handle the shutdown
+ * request and terminate the process.
+ */
+[NSThread sleepForTimeInterval:INFINITY];
}
- (BOOL)applicationShouldTerminateAfterLastWindowClosed:(NSApplication
*)theApplication
--
2.24.3 (Apple Git-128)
Re: [PATCH v3] Correct CRIS TCG register lifetime management
On Fri, Feb 19, 2021 at 11:53:48AM +0100, Stefan Sandström wrote:
> From: Stefan Sandstrom
>
> Add and fix deallocation of temporary TCG registers in CRIS code
> generation.
Thanks Stefan,
There's still a couple of minor stylistic issues.
The Subject/Summary should be prefixed with the code area you're
changing. I'd suggest changing it
from:
Correct CRIS TCG register lifetime management
to:
target/cris: Plug leakage of TCG temporaries
We also try to avoid unrelated whitespace changes.
I've commented on the 2 I found inline.
Would be good if you could remove those in the next version.
Other than that, the patch looks good to me.
So, with those issues fixed, feel free to add the following tags:
Tested-by: Edgar E. Iglesias
Reviewed-by: Edgar E. Iglesias
Best regards,
Edgar
>
> Change-Id: I17fce5d95bdc4418337ba885d53ba97afb1bafcc
> Signed-off-by: Stefan Sandström
> ---
> target/cris/translate.c | 127 +++-
> target/cris/translate_v10.c.inc | 70 --
> 2 files changed, 138 insertions(+), 59 deletions(-)
>
> diff --git a/target/cris/translate.c b/target/cris/translate.c
> index c893f877ab..2b35d818dd 100644
> --- a/target/cris/translate.c
> +++ b/target/cris/translate.c
> @@ -172,14 +172,21 @@ static int preg_sizes[] = {
> tcg_gen_ld_tl(tn, cpu_env, offsetof(CPUCRISState, member))
> #define t_gen_mov_env_TN(member, tn) \
> tcg_gen_st_tl(tn, cpu_env, offsetof(CPUCRISState, member))
> +#define t_gen_movi_env_TN(member, c) \
> +do { \
> +TCGv tc = tcg_const_tl(c); \
> +t_gen_mov_env_TN(member, tc); \
> +tcg_temp_free(tc); \
> +} while (0)
> +
Remove this extra blank line.
>
> static inline void t_gen_mov_TN_preg(TCGv tn, int r)
> {
> assert(r >= 0 && r <= 15);
> if (r == PR_BZ || r == PR_WZ || r == PR_DZ) {
> -tcg_gen_mov_tl(tn, tcg_const_tl(0));
> +tcg_gen_movi_tl(tn, 0);
> } else if (r == PR_VR) {
> -tcg_gen_mov_tl(tn, tcg_const_tl(32));
> +tcg_gen_movi_tl(tn, 32);
> } else {
> tcg_gen_mov_tl(tn, cpu_PR[r]);
> }
> @@ -204,6 +211,8 @@ static inline void t_gen_mov_preg_TN(DisasContext *dc,
> int r, TCGv tn)
> }
> }
>
> +
> +
Remove this unrelated blank lines.
Cheers,
Edgar
Re: [PATCH] virtio-ccw: commands on revision-less devices
On Tue, 16 Feb 2021 12:18:30 +0100 Cornelia Huck wrote: > The virtio standard specifies that any non-transitional device must > reject commands prior to revision setting (which we do) and else > assume revision 0 (legacy) if the driver sends a non-revision-setting > command first. We neglected to do the latter. > > Fortunately, nearly everything worked as intended anyway; the only > problem was not properly rejecting revision setting after some other > command had been issued. Easy to fix by setting revision to 0 if > we see a non-revision command on a legacy-capable revision-less > device. > > Signed-off-by: Cornelia Huck > --- > hw/s390x/virtio-ccw.c | 21 ++--- > 1 file changed, 14 insertions(+), 7 deletions(-) I now have: Author: Cornelia Huck Date: Tue Feb 16 12:18:30 2021 +0100 virtio-ccw: commands on revision-less devices The virtio standard specifies that any non-transitional device must reject commands prior to revision setting (which we do). Devices that are transitional need to assume revision 0 (legacy) if the driver sends a non-revision-setting command first in order to support legacy drivers. We neglected to do the latter. Fortunately, nearly everything worked as intended anyway; the only problem was not properly rejecting revision setting after some other command had been issued. Easy to fix by setting revision to 0 if we see a non-revision command on a legacy-capable revision-less device. Found by code inspection, not observed in the wild. Signed-off-by: Cornelia Huck Reviewed-by: Thomas Huth Reviewed-by: Michael S. Tsirkin Acked-by: Halil Pasic Message-Id: <20210216111830.1087847-1-coh...@redhat.com> Any objections?
[PATCH v2] ui/cocoa: Use kCGColorSpaceSRGB
kCGColorSpaceGenericRGB | Apple Developer Documentation https://developer.apple.com/documentation/coregraphics/kcgcolorspacegenericrgb > Deprecated > Use kCGColorSpaceSRGB instead. This change also removes the legacy color space specification for PowerPC. Signed-off-by: Akihiko Odaki --- ui/cocoa.m | 9 ++--- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/ui/cocoa.m b/ui/cocoa.m index 13fba8103e1..7710835c4c1 100644 --- a/ui/cocoa.m +++ b/ui/cocoa.m @@ -436,13 +436,8 @@ - (void) drawRect:(NSRect) rect screen.bitsPerComponent, //bitsPerComponent screen.bitsPerPixel, //bitsPerPixel (screen.width * (screen.bitsPerComponent/2)), //bytesPerRow -#ifdef __LITTLE_ENDIAN__ -CGColorSpaceCreateWithName(kCGColorSpaceGenericRGB), //colorspace for OS X >= 10.4 -kCGBitmapByteOrder32Little | kCGImageAlphaNoneSkipFirst, -#else -CGColorSpaceCreateDeviceRGB(), //colorspace for OS X < 10.4 (actually ppc) -kCGImageAlphaNoneSkipFirst, //bitmapInfo -#endif +CGColorSpaceCreateWithName(kCGColorSpaceSRGB), //colorspace +kCGBitmapByteOrder32Little | kCGImageAlphaNoneSkipFirst, //bitmapInfo dataProviderRef, //provider NULL, //decode 0, //interpolate -- 2.24.3 (Apple Git-128)
[PATCH] configure: Do not require GBM for OpenGL
Signed-off-by: Akihiko Odaki
---
configure| 11 +--
include/ui/egl-helpers.h | 2 ++
ui/meson.build | 1 +
3 files changed, 8 insertions(+), 6 deletions(-)
diff --git a/configure b/configure
index 9f016b06b54..bd96929d7b4 100755
--- a/configure
+++ b/configure
@@ -3576,14 +3576,13 @@ if $pkg_config gbm; then
fi
if test "$opengl" != "no" ; then
- opengl_pkgs="epoxy gbm"
- if $pkg_config $opengl_pkgs; then
-opengl_cflags="$($pkg_config --cflags $opengl_pkgs)"
-opengl_libs="$($pkg_config --libs $opengl_pkgs)"
+ if $pkg_config epoxy; then
+opengl_cflags="$($pkg_config --cflags epoxy)"
+opengl_libs="$($pkg_config --libs epoxy)"
opengl=yes
else
if test "$opengl" = "yes" ; then
- feature_not_found "opengl" "Please install opengl (mesa) devel pkgs:
$opengl_pkgs"
+ feature_not_found "opengl" "Please install epoxy"
fi
opengl_cflags=""
opengl_libs=""
@@ -3591,7 +3590,7 @@ if test "$opengl" != "no" ; then
fi
fi
-if test "$opengl" = "yes"; then
+if test "$gbm" = "yes" && test "$opengl" = "yes"; then
cat > $TMPC << EOF
#include
#ifndef EGL_MESA_image_dma_buf_export
diff --git a/include/ui/egl-helpers.h b/include/ui/egl-helpers.h
index 94a4b3e6f3b..cc12761c53b 100644
--- a/include/ui/egl-helpers.h
+++ b/include/ui/egl-helpers.h
@@ -3,7 +3,9 @@
#include
#include
+#ifdef CONFIG_OPENGL_DMABUF
#include
+#endif
#include "ui/console.h"
#include "ui/shader.h"
diff --git a/ui/meson.build b/ui/meson.build
index 634fabab0d5..33aa514cad0 100644
--- a/ui/meson.build
+++ b/ui/meson.build
@@ -45,6 +45,7 @@ endif
if config_host.has_key('CONFIG_OPENGL')
opengl_ss = ss.source_set()
+ opengl_ss.add(gbm)
opengl_ss.add(when: [opengl, pixman, 'CONFIG_OPENGL'],
if_true: files('shader.c', 'console-gl.c', 'egl-helpers.c',
'egl-context.c'))
ui_modules += {'opengl' : opengl_ss}
--
2.24.3 (Apple Git-128)
Re: [PATCH] gitlab-ci: Remove unused container images
On Fri, Feb 19, 2021 at 12:09:50PM +0100, Thomas Huth wrote: > We're building a lot of containers in the gitlab-CI that we never use. > This takes away network bandwidth and CPU time from other jobs for no > use, so let's remove them for now. The individual containers could be > re-added later when we really need them. Rather than removing the jobs, how about just setting "when: manual" that way users can trigger the creation of the images to populate their gitlab registry for convenient usage Regards, Daniel -- |: https://berrange.com -o-https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o-https://fstop138.berrange.com :| |: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|
Re: [PATCH v2] ui/cocoa: Use kCGColorSpaceSRGB
On Fri, 19 Feb 2021 at 11:28, Akihiko Odaki wrote: > > kCGColorSpaceGenericRGB | Apple Developer Documentation > https://developer.apple.com/documentation/coregraphics/kcgcolorspacegenericrgb > > Deprecated > > Use kCGColorSpaceSRGB instead. > > This change also removes the legacy color space specification for > PowerPC. > > Signed-off-by: Akihiko Odaki Reviewed-by: Peter Maydell thanks -- PMM
[PATCH 0/7] hw/kvm: Exit gracefully when KVM is not supported
Hi, This series aims to improve user experience by providing a better error message when the user tries to enable KVM on machines not supporting it. Regards, Phil. Philippe Mathieu-Daudé (7): accel/kvm: Check MachineClass kvm_type() return value hw/boards: Introduce 'kvm_supported' field to MachineClass hw/arm: Set kvm_supported for KVM-compatible machines hw/mips: Set kvm_supported for KVM-compatible machines hw/ppc: Set kvm_supported for KVM-compatible machines hw/s390x: Set kvm_supported to s390-ccw-virtio machines accel/kvm: Exit gracefully when KVM is not supported include/hw/boards.h| 6 +- accel/kvm/kvm-all.c| 12 hw/arm/sbsa-ref.c | 1 + hw/arm/virt.c | 1 + hw/arm/xlnx-versal-virt.c | 1 + hw/mips/loongson3_virt.c | 1 + hw/mips/malta.c| 1 + hw/ppc/e500plat.c | 1 + hw/ppc/mac_newworld.c | 1 + hw/ppc/mac_oldworld.c | 1 + hw/ppc/mpc8544ds.c | 1 + hw/ppc/ppc440_bamboo.c | 1 + hw/ppc/prep.c | 1 + hw/ppc/sam460ex.c | 1 + hw/ppc/spapr.c | 1 + hw/s390x/s390-virtio-ccw.c | 1 + 16 files changed, 31 insertions(+), 1 deletion(-) -- 2.26.2
[PATCH 1/7] accel/kvm: Check MachineClass kvm_type() return value
MachineClass::kvm_type() can return -1 on failure.
Document it, and add a check in kvm_init().
Signed-off-by: Philippe Mathieu-Daudé
---
include/hw/boards.h | 3 ++-
accel/kvm/kvm-all.c | 6 ++
2 files changed, 8 insertions(+), 1 deletion(-)
diff --git a/include/hw/boards.h b/include/hw/boards.h
index a46dfe5d1a6..68d3d10f6b0 100644
--- a/include/hw/boards.h
+++ b/include/hw/boards.h
@@ -127,7 +127,8 @@ typedef struct {
*implement and a stub device is required.
* @kvm_type:
*Return the type of KVM corresponding to the kvm-type string option or
- *computed based on other criteria such as the host kernel capabilities.
+ *computed based on other criteria such as the host kernel capabilities
+ *(which can't be negative), or -1 on error.
* @numa_mem_supported:
*true if '--numa node.mem' option is supported and false otherwise
* @smp_parse:
diff --git a/accel/kvm/kvm-all.c b/accel/kvm/kvm-all.c
index 84c943fcdb2..b069938d881 100644
--- a/accel/kvm/kvm-all.c
+++ b/accel/kvm/kvm-all.c
@@ -2057,6 +2057,12 @@ static int kvm_init(MachineState *ms)
"kvm-type",
&error_abort);
type = mc->kvm_type(ms, kvm_type);
+if (type < 0) {
+ret = -EINVAL;
+fprintf(stderr, "Failed to detect kvm-type for machine '%s'\n",
+mc->name);
+goto err;
+}
}
do {
--
2.26.2
[PATCH 2/7] hw/boards: Introduce 'kvm_supported' field to MachineClass
Introduce the 'kvm_supported' field to express whether
a machine supports KVM acceleration or not.
Signed-off-by: Philippe Mathieu-Daudé
---
include/hw/boards.h | 3 +++
1 file changed, 3 insertions(+)
diff --git a/include/hw/boards.h b/include/hw/boards.h
index 68d3d10f6b0..0959aa743ee 100644
--- a/include/hw/boards.h
+++ b/include/hw/boards.h
@@ -129,6 +129,8 @@ typedef struct {
*Return the type of KVM corresponding to the kvm-type string option or
*computed based on other criteria such as the host kernel capabilities
*(which can't be negative), or -1 on error.
+ * @kvm_supported:
+ *true if '-enable-kvm' option is supported and false otherwise.
* @numa_mem_supported:
*true if '--numa node.mem' option is supported and false otherwise
* @smp_parse:
@@ -209,6 +211,7 @@ struct MachineClass {
bool nvdimm_supported;
bool numa_mem_supported;
bool auto_enable_numa;
+bool kvm_supported;
const char *default_ram_id;
HotplugHandler *(*get_hotplug_handler)(MachineState *machine,
--
2.26.2
[RFC PATCH 5/7] hw/ppc: Set kvm_supported for KVM-compatible machines
The following PowerPC machines support KVM:
- 40p
- bamboo
- g3beige
- mac99
- mpc8544ds
- ppce500
- pseries
- sam460ex
- virtex-ml507
Signed-off-by: Philippe Mathieu-Daudé
---
RFC: I'm surprise by this list, but this is the result of
auditing calls to kvm_enabled() checks.
---
hw/ppc/e500plat.c | 1 +
hw/ppc/mac_newworld.c | 1 +
hw/ppc/mac_oldworld.c | 1 +
hw/ppc/mpc8544ds.c | 1 +
hw/ppc/ppc440_bamboo.c | 1 +
hw/ppc/prep.c | 1 +
hw/ppc/sam460ex.c | 1 +
hw/ppc/spapr.c | 1 +
8 files changed, 8 insertions(+)
diff --git a/hw/ppc/e500plat.c b/hw/ppc/e500plat.c
index bddd5e7c48f..bf95b68bc03 100644
--- a/hw/ppc/e500plat.c
+++ b/hw/ppc/e500plat.c
@@ -98,6 +98,7 @@ static void e500plat_machine_class_init(ObjectClass *oc, void
*data)
mc->max_cpus = 32;
mc->default_cpu_type = POWERPC_CPU_TYPE_NAME("e500v2_v30");
mc->default_ram_id = "mpc8544ds.ram";
+mc->kvm_supported = true;
machine_class_allow_dynamic_sysbus_dev(mc, TYPE_ETSEC_COMMON);
}
diff --git a/hw/ppc/mac_newworld.c b/hw/ppc/mac_newworld.c
index e991db4addb..573502f7b5b 100644
--- a/hw/ppc/mac_newworld.c
+++ b/hw/ppc/mac_newworld.c
@@ -595,6 +595,7 @@ static void core99_machine_class_init(ObjectClass *oc, void
*data)
mc->max_cpus = MAX_CPUS;
mc->default_boot_order = "cd";
mc->default_display = "std";
+mc->kvm_supported = true;
mc->kvm_type = core99_kvm_type;
#ifdef TARGET_PPC64
mc->default_cpu_type = POWERPC_CPU_TYPE_NAME("970fx_v3.1");
diff --git a/hw/ppc/mac_oldworld.c b/hw/ppc/mac_oldworld.c
index 44ee99be886..4b34106919d 100644
--- a/hw/ppc/mac_oldworld.c
+++ b/hw/ppc/mac_oldworld.c
@@ -444,6 +444,7 @@ static void heathrow_class_init(ObjectClass *oc, void *data)
#endif
/* TOFIX "cad" when Mac floppy is implemented */
mc->default_boot_order = "cd";
+mc->kvm_supported = true;
mc->kvm_type = heathrow_kvm_type;
mc->default_cpu_type = POWERPC_CPU_TYPE_NAME("750_v3.1");
mc->default_display = "std";
diff --git a/hw/ppc/mpc8544ds.c b/hw/ppc/mpc8544ds.c
index 81177505f02..4b750ca0555 100644
--- a/hw/ppc/mpc8544ds.c
+++ b/hw/ppc/mpc8544ds.c
@@ -56,6 +56,7 @@ static void e500plat_machine_class_init(ObjectClass *oc, void
*data)
mc->max_cpus = 15;
mc->default_cpu_type = POWERPC_CPU_TYPE_NAME("e500v2_v30");
mc->default_ram_id = "mpc8544ds.ram";
+mc->kvm_supported = true;
}
#define TYPE_MPC8544DS_MACHINE MACHINE_TYPE_NAME("mpc8544ds")
diff --git a/hw/ppc/ppc440_bamboo.c b/hw/ppc/ppc440_bamboo.c
index b156bcb9990..f3258a1f229 100644
--- a/hw/ppc/ppc440_bamboo.c
+++ b/hw/ppc/ppc440_bamboo.c
@@ -304,6 +304,7 @@ static void bamboo_machine_init(MachineClass *mc)
mc->init = bamboo_init;
mc->default_cpu_type = POWERPC_CPU_TYPE_NAME("440epb");
mc->default_ram_id = "ppc4xx.sdram";
+mc->kvm_supported = true;
}
DEFINE_MACHINE("bamboo", bamboo_machine_init)
diff --git a/hw/ppc/prep.c b/hw/ppc/prep.c
index 7e72f6e4a9b..96b6f68d663 100644
--- a/hw/ppc/prep.c
+++ b/hw/ppc/prep.c
@@ -441,6 +441,7 @@ static void ibm_40p_machine_init(MachineClass *mc)
mc->default_boot_order = "c";
mc->default_cpu_type = POWERPC_CPU_TYPE_NAME("604");
mc->default_display = "std";
+mc->kvm_supported = true;
}
DEFINE_MACHINE("40p", ibm_40p_machine_init)
diff --git a/hw/ppc/sam460ex.c b/hw/ppc/sam460ex.c
index e459b43065b..43cccad1591 100644
--- a/hw/ppc/sam460ex.c
+++ b/hw/ppc/sam460ex.c
@@ -513,6 +513,7 @@ static void sam460ex_machine_init(MachineClass *mc)
mc->default_cpu_type = POWERPC_CPU_TYPE_NAME("460exb");
mc->default_ram_size = 512 * MiB;
mc->default_ram_id = "ppc4xx.sdram";
+mc->kvm_supported = true;
}
DEFINE_MACHINE("sam460ex", sam460ex_machine_init)
diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index 85fe65f8947..3f83c2ce2ca 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -4426,6 +4426,7 @@ static void spapr_machine_class_init(ObjectClass *oc,
void *data)
mc->default_ram_size = 512 * MiB;
mc->default_ram_id = "ppc_spapr.ram";
mc->default_display = "std";
+mc->kvm_supported = true;
mc->kvm_type = spapr_kvm_type;
machine_class_allow_dynamic_sysbus_dev(mc, TYPE_SPAPR_PCI_HOST_BRIDGE);
mc->pci_allow_0_address = true;
--
2.26.2
[PATCH 3/7] hw/arm: Set kvm_supported for KVM-compatible machines
The following ARM machines support KVM:
- virt
- sbsa-ref
- xlnx-versal-virt
Signed-off-by: Philippe Mathieu-Daudé
---
hw/arm/sbsa-ref.c | 1 +
hw/arm/virt.c | 1 +
hw/arm/xlnx-versal-virt.c | 1 +
3 files changed, 3 insertions(+)
diff --git a/hw/arm/sbsa-ref.c b/hw/arm/sbsa-ref.c
index 9f707351531..6923b6e77ff 100644
--- a/hw/arm/sbsa-ref.c
+++ b/hw/arm/sbsa-ref.c
@@ -858,6 +858,7 @@ static void sbsa_ref_class_init(ObjectClass *oc, void *data)
mc->possible_cpu_arch_ids = sbsa_ref_possible_cpu_arch_ids;
mc->cpu_index_to_instance_props = sbsa_ref_cpu_index_to_props;
mc->get_default_cpu_node_id = sbsa_ref_get_default_cpu_node_id;
+mc->kvm_supported = true;
}
static const TypeInfo sbsa_ref_info = {
diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index 371147f3ae9..7e3748b6cd6 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -2582,6 +2582,7 @@ static void virt_machine_class_init(ObjectClass *oc, void
*data)
mc->cpu_index_to_instance_props = virt_cpu_index_to_props;
mc->default_cpu_type = ARM_CPU_TYPE_NAME("cortex-a15");
mc->get_default_cpu_node_id = virt_get_default_cpu_node_id;
+mc->kvm_supported = true;
mc->kvm_type = virt_kvm_type;
assert(!mc->get_hotplug_handler);
mc->get_hotplug_handler = virt_machine_get_hotplug_handler;
diff --git a/hw/arm/xlnx-versal-virt.c b/hw/arm/xlnx-versal-virt.c
index 8482cd61960..fb623c0cd54 100644
--- a/hw/arm/xlnx-versal-virt.c
+++ b/hw/arm/xlnx-versal-virt.c
@@ -621,6 +621,7 @@ static void versal_virt_machine_class_init(ObjectClass *oc,
void *data)
mc->default_cpus = XLNX_VERSAL_NR_ACPUS;
mc->no_cdrom = true;
mc->default_ram_id = "ddr";
+mc->kvm_supported = true;
}
static const TypeInfo versal_virt_machine_init_typeinfo = {
--
2.26.2
[PATCH 7/7] accel/kvm: Exit gracefully when KVM is not supported
Now that we added the 'kvm_supported' field to MachineClass
and all our machines able to use KVM have this field set,
we can check it in kvm_init() and exit gracefully with
a friendly error message.
Before:
$ qemu-system-aarch64 -M raspi3b -enable-kvm
qemu-system-aarch64: /build/qemu-ETIdrs/qemu-4.2/exec.c:865:
cpu_address_space_init: Assertion `asidx == 0 || !kvm_enabled()' failed.
Aborted
$ qemu-system-aarch64 -M xlnx-zcu102 -enable-kvm -smp 6
qemu-system-aarch64: kvm_init_vcpu: kvm_arch_init_vcpu failed (0): Invalid
argument
After:
$ qemu-system-aarch64 -M raspi3b -enable-kvm
Machine 'raspi3b' does not support KVM
$ qemu-system-aarch64 -M xlnx-zcu102 -enable-kvm -smp 6
Machine 'xlnx-zcu102' does not support KVM
Signed-off-by: Philippe Mathieu-Daudé
---
accel/kvm/kvm-all.c | 6 ++
1 file changed, 6 insertions(+)
diff --git a/accel/kvm/kvm-all.c b/accel/kvm/kvm-all.c
index b069938d881..8a8d3f64248 100644
--- a/accel/kvm/kvm-all.c
+++ b/accel/kvm/kvm-all.c
@@ -2001,6 +2001,12 @@ static int kvm_init(MachineState *ms)
s = KVM_STATE(ms->accelerator);
+if (!mc->kvm_supported) {
+ret = -EINVAL;
+fprintf(stderr, "Machine '%s' does not support KVM\n", mc->name);
+exit(1);
+}
+
/*
* On systems where the kernel can support different base page
* sizes, host page size may be different from TARGET_PAGE_SIZE,
--
2.26.2
Re: [PATCH 3/5] parallels: support bitmap extension for read-only mode
On 2/16/21 7:45 PM, Vladimir Sementsov-Ogievskiy wrote:
> Signed-off-by: Vladimir Sementsov-Ogievskiy
> ---
> block/parallels.h | 6 +-
> block/parallels-ext.c | 286 ++
> block/parallels.c | 18 +++
> block/meson.build | 3 +-
> 4 files changed, 311 insertions(+), 2 deletions(-)
> create mode 100644 block/parallels-ext.c
>
> diff --git a/block/parallels.h b/block/parallels.h
> index 5aa101cfc8..2dbb7668a3 100644
> --- a/block/parallels.h
> +++ b/block/parallels.h
> @@ -48,7 +48,8 @@ typedef struct ParallelsHeader {
> uint64_t nb_sectors;
> uint32_t inuse;
> uint32_t data_off;
> -char padding[12];
> +uint32_t flags;
> +uint64_t ext_off;
> } QEMU_PACKED ParallelsHeader;
>
> typedef enum ParallelsPreallocMode {
> @@ -84,4 +85,7 @@ typedef struct BDRVParallelsState {
> Error *migration_blocker;
> } BDRVParallelsState;
>
> +int parallels_read_format_extension(BlockDriverState *bs,
> +int64_t ext_off, Error **errp);
> +
> #endif
> diff --git a/block/parallels-ext.c b/block/parallels-ext.c
> new file mode 100644
> index 00..b825b55124
> --- /dev/null
> +++ b/block/parallels-ext.c
> @@ -0,0 +1,286 @@
> +/*
> + * Support for Parallels Format Extansion. It's a part of parallels format
> + * driver.
s/Extansion/Extension/
s/Support for Parallels/Support of Parallels/
s/It's a part of parallels format/It's a part of Parallels format/
> + *
> + * Copyright (c) 2021 Virtuozzo International GmbH
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a
> copy
> + * of this software and associated documentation files (the "Software"), to
> deal
> + * in the Software without restriction, including without limitation the
> rights
> + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
> + * copies of the Software, and to permit persons to whom the Software is
> + * furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice shall be included in
> + * all copies or substantial portions of the Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
> FROM,
> + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
> + * THE SOFTWARE.
> + */
> +
> +#include "qemu/osdep.h"
> +#include "qapi/error.h"
> +#include "block/block_int.h"
> +#include "parallels.h"
> +#include "crypto/hash.h"
> +#include "qemu/uuid.h"
> +
> +#define PARALLELS_FORMAT_EXTENSION_MAGIC 0xAB234CEF23DCEA87ULL
> +
> +#define PARALLELS_END_OF_FEATURES_MAGIC 0x0ULL
> +#define PARALLELS_DIRTY_BITMAP_FEATURE_MAGIC 0x20385FAE252CB34AULL
> +
> +typedef struct ParallelsFormatExtensionHeader {
> +uint64_t magic; /* PARALLELS_FORMAT_EXTENSION_MAGIC */
> +uint8_t check_sum[16];
> +} QEMU_PACKED ParallelsFormatExtensionHeader;
> +
> +typedef struct ParallelsFeatureHeader {
> +uint64_t magic;
> +uint64_t flags;
> +uint32_t data_size;
> +uint32_t _unused;
> +} QEMU_PACKED ParallelsFeatureHeader;
> +
> +typedef struct ParallelsDirtyBitmapFeature {
> +uint64_t size;
> +uint8_t id[16];
> +uint32_t granularity;
> +uint32_t l1_size;
> +/* L1 table follows */
> +} QEMU_PACKED ParallelsDirtyBitmapFeature;
> +
> +/* Given L1 table read bitmap data from the image and populate @bitmap */
> +static int parallels_load_bitmap_data(BlockDriverState *bs,
> + const uint64_t *l1_table,
> + uint32_t l1_size,
> + BdrvDirtyBitmap *bitmap,
> + Error **errp)
> +{
> +BDRVParallelsState *s = bs->opaque;
> +int ret = 0;
> +uint64_t offset, limit;
> +int cluster_size = s->tracks << BDRV_SECTOR_BITS;
should we save cluster size to BDS or create helper for the purpose?
Such operation through the code is looking terrible. Originally it was
available in one place only and that was acceptable. Now it spreads
over and over.
> +uint64_t bm_size = bdrv_dirty_bitmap_size(bitmap);
> +uint8_t *buf = NULL;
> +uint64_t i, tab_size =
> +DIV_ROUND_UP(bdrv_dirty_bitmap_serialization_size(bitmap, 0,
> bm_size),
> + cluster_size);
> +
> +if (tab_size != l1_size) {
> +error_setg(errp, "Bitmap table size %" PRIu32 " does not correspond "
> + "to bitmap size and cluster size. Expected %" PRIu64,
> + l1_size, tab_size);
> +return -EINVAL;
> +}
> +
> +buf = qemu_blockalign(bs, cluster
Re: [PATCH v2 0/3] virtiofsd: Add options to enable/disable posix acl
Vivek Goyal writes: > Hi, > > This is V2 of the patches. Changes since v1 are. > > - Rebased on top of latest master. > - Took care of Miklos's comments to block acl xattrs if user > explicitly disabled posix acl. > > Luis Henriques reported that fstest generic/099 fails with virtiofs. > Little debugging showed that we don't enable acl support. So this > patch series provides option to enable/disable posix acl support. By > default it is disabled. > > I have run blogbench and pjdfstests with posix acl enabled and > things work fine. > > Luis, can you please apply these patches, and run virtiofsd with > "-o posix_acl" and see if it fixes the failure you are seeing. I > ran the steps you provided manually and it fixes the issue for > me. Sorry for the delay. I've finally tested these patches and they indeed fix the problem I reported. My only question about this fix is why is this option not enabled by default, since this is the documented behavior in acl(5) and umask(2)? In fact, why is this an option at all? Cheers, -- Luis
[PATCH 4/7] hw/mips: Set kvm_supported for KVM-compatible machines
The following MIPS machines support KVM:
- malta
- loongson3-virt
Signed-off-by: Philippe Mathieu-Daudé
---
hw/mips/loongson3_virt.c | 1 +
hw/mips/malta.c | 1 +
2 files changed, 2 insertions(+)
diff --git a/hw/mips/loongson3_virt.c b/hw/mips/loongson3_virt.c
index d4a82fa5367..c5ef44819a7 100644
--- a/hw/mips/loongson3_virt.c
+++ b/hw/mips/loongson3_virt.c
@@ -622,6 +622,7 @@ static void loongson3v_machine_class_init(ObjectClass *oc,
void *data)
mc->max_cpus = LOONGSON_MAX_VCPUS;
mc->default_ram_id = "loongson3.highram";
mc->default_ram_size = 1600 * MiB;
+mc->kvm_supported = true;
mc->kvm_type = mips_kvm_type;
mc->minimum_page_bits = 14;
}
diff --git a/hw/mips/malta.c b/hw/mips/malta.c
index 9afc0b427bf..f06bd3eff25 100644
--- a/hw/mips/malta.c
+++ b/hw/mips/malta.c
@@ -1456,6 +1456,7 @@ static void mips_malta_machine_init(MachineClass *mc)
mc->default_cpu_type = MIPS_CPU_TYPE_NAME("24Kf");
#endif
mc->default_ram_id = "mips_malta.ram";
+mc->kvm_supported = true;
}
DEFINE_MACHINE("malta", mips_malta_machine_init)
--
2.26.2
[PATCH] hw/arm/xlnx-zynqmp: Remove obsolete 'has_rpu' property
We hint the 'has_rpu' property is no longer required since commit
6908ec448b4 ("xlnx-zynqmp: Properly support the smp command line
option") which was released in QEMU v2.11.0.
3 years later we feel safe enough to remove it without using the
usual deprecation policy.
Signed-off-by: Philippe Mathieu-Daudé
---
include/hw/arm/xlnx-zynqmp.h | 2 --
hw/arm/xlnx-zynqmp.c | 6 --
2 files changed, 8 deletions(-)
diff --git a/include/hw/arm/xlnx-zynqmp.h b/include/hw/arm/xlnx-zynqmp.h
index 6f45387a173..0678b419a23 100644
--- a/include/hw/arm/xlnx-zynqmp.h
+++ b/include/hw/arm/xlnx-zynqmp.h
@@ -115,8 +115,6 @@ struct XlnxZynqMPState {
bool secure;
/* Has the ARM Virtualization extensions? */
bool virt;
-/* Has the RPU subsystem? */
-bool has_rpu;
/* CAN bus. */
CanBusState *canbus[XLNX_ZYNQMP_NUM_CAN];
diff --git a/hw/arm/xlnx-zynqmp.c b/hw/arm/xlnx-zynqmp.c
index 881847255b4..46030c1ef81 100644
--- a/hw/arm/xlnx-zynqmp.c
+++ b/hw/arm/xlnx-zynqmp.c
@@ -443,11 +443,6 @@ static void xlnx_zynqmp_realize(DeviceState *dev, Error
**errp)
}
}
-if (s->has_rpu) {
-info_report("The 'has_rpu' property is no longer required, to use the "
-"RPUs just use -smp 6.");
-}
-
xlnx_zynqmp_create_rpu(ms, s, boot_cpu, &err);
if (err) {
error_propagate(errp, err);
@@ -646,7 +641,6 @@ static Property xlnx_zynqmp_props[] = {
DEFINE_PROP_STRING("boot-cpu", XlnxZynqMPState, boot_cpu),
DEFINE_PROP_BOOL("secure", XlnxZynqMPState, secure, false),
DEFINE_PROP_BOOL("virtualization", XlnxZynqMPState, virt, false),
-DEFINE_PROP_BOOL("has_rpu", XlnxZynqMPState, has_rpu, false),
DEFINE_PROP_LINK("ddr-ram", XlnxZynqMPState, ddr_ram, TYPE_MEMORY_REGION,
MemoryRegion *),
DEFINE_PROP_LINK("canbus0", XlnxZynqMPState, canbus[0], TYPE_CAN_BUS,
--
2.26.2
[PATCH 6/7] hw/s390x: Set kvm_supported to s390-ccw-virtio machines
All s390-ccw-virtio machines support KVM.
Signed-off-by: Philippe Mathieu-Daudé
---
hw/s390x/s390-virtio-ccw.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c
index 2972b607f36..259b4b4397e 100644
--- a/hw/s390x/s390-virtio-ccw.c
+++ b/hw/s390x/s390-virtio-ccw.c
@@ -612,6 +612,7 @@ static void ccw_machine_class_init(ObjectClass *oc, void
*data)
mc->possible_cpu_arch_ids = s390_possible_cpu_arch_ids;
/* it is overridden with 'host' cpu *in kvm_arch_init* */
mc->default_cpu_type = S390_CPU_TYPE_NAME("qemu");
+mc->kvm_supported = true;
hc->plug = s390_machine_device_plug;
hc->unplug_request = s390_machine_device_unplug_request;
nc->nmi_monitor_handler = s390_nmi;
--
2.26.2
Re: [PATCH 0/7] hw/kvm: Exit gracefully when KVM is not supported
On Fri, 19 Feb 2021 at 11:44, Philippe Mathieu-Daudé wrote: > This series aims to improve user experience by providing > a better error message when the user tries to enable KVM > on machines not supporting it. Thanks for having a look at this; fixing the ugly assertion failure if you try to enable KVM for the raspi boards has been vaguely on my todo list but never made it up to the top... > Philippe Mathieu-Daudé (7): > accel/kvm: Check MachineClass kvm_type() return value > hw/boards: Introduce 'kvm_supported' field to MachineClass > hw/arm: Set kvm_supported for KVM-compatible machines > hw/mips: Set kvm_supported for KVM-compatible machines > hw/ppc: Set kvm_supported for KVM-compatible machines > hw/s390x: Set kvm_supported to s390-ccw-virtio machines > accel/kvm: Exit gracefully when KVM is not supported Don't we also need to set kvm_supported for the relevant machine types in hw/i386 ? thanks -- PMM
Re: [PATCH 5/5] iotests: add parallels-read-bitmap test
On 2/16/21 7:45 PM, Vladimir Sementsov-Ogievskiy wrote:
> Test support for reading bitmap from parallels image format.
> parallels-with-bitmap.bz2 is generated on Virtuozzo by
> parallels-with-bitmap.sh
>
> Signed-off-by: Vladimir Sementsov-Ogievskiy
> ---
> .../sample_images/parallels-with-bitmap.bz2 | Bin 0 -> 203 bytes
> .../sample_images/parallels-with-bitmap.sh| 33 ++
> .../qemu-iotests/tests/parallels-read-bitmap | 57 ++
> .../tests/parallels-read-bitmap.out | 6 ++
> 4 files changed, 96 insertions(+)
> create mode 100644 tests/qemu-iotests/sample_images/parallels-with-bitmap.bz2
> create mode 100755 tests/qemu-iotests/sample_images/parallels-with-bitmap.sh
> create mode 100755 tests/qemu-iotests/tests/parallels-read-bitmap
> create mode 100644 tests/qemu-iotests/tests/parallels-read-bitmap.out
>
> diff --git a/tests/qemu-iotests/sample_images/parallels-with-bitmap.bz2
> b/tests/qemu-iotests/sample_images/parallels-with-bitmap.bz2
> new file mode 100644
> index
> ..54892fd4d01bf743d395bd4f3d896494146ab5a9
> GIT binary patch
> literal 203
> zcmV;+05tzXT4*^jL0KkKS@=;0bpT+Hf7|^?Km zk&7Szk`SoS002EkfMftPG z5P$(X{&Tq5C` zv(i3x^K~wt!aLPcRBP+PckUsIh6*LgjYSh0`}#7hMC9NR5D)+W0d&8Mxgwk>NPH-R
> Fx`3oHQ9u9y
>
> literal 0
> HcmV?d1
>
> diff --git a/tests/qemu-iotests/sample_images/parallels-with-bitmap.sh
> b/tests/qemu-iotests/sample_images/parallels-with-bitmap.sh
> new file mode 100755
> index 00..e4a1d71277
> --- /dev/null
> +++ b/tests/qemu-iotests/sample_images/parallels-with-bitmap.sh
> @@ -0,0 +1,33 @@
do we need Copyright notice here? I am unsure that this script is to be
acceptable in QEMU repo. Anyway, it looks fine :)
> +#!/bin/bash
> +
> +CT=parallels-with-bitmap-ct
> +DIR=$PWD/parallels-with-bitmap-dir
> +IMG=$DIR/root.hds
> +XML=$DIR/DiskDescriptor.xml
> +TARGET=parallels-with-bitmap.bz2
> +
> +rm -rf $DIR
> +
> +prlctl create $CT --vmtype ct
> +prlctl set $CT --device-add hdd --image $DIR --recreate --size 2G
> +
> +# cleanup the image
> +qemu-img create -f parallels $IMG 64G
> +
> +# create bitmap
> +prlctl backup $CT
> +
> +prlctl set $CT --device-del hdd1
> +prlctl destroy $CT
> +
> +dev=$(ploop mount $XML | sed -n 's/^Adding delta
> dev=\(\/dev\/ploop[0-9]\+\).*/\1/p')
> +dd if=/dev/zero of=$dev bs=64K seek=5 count=2 oflag=direct
> +dd if=/dev/zero of=$dev bs=64K seek=30 count=1 oflag=direct
> +dd if=/dev/zero of=$dev bs=64K seek=10 count=3 oflag=direct
> +ploop umount $XML # bitmap name will be in the output
> +
> +bzip2 -z $IMG
> +
> +mv $IMG.bz2 $TARGET
> +
> +rm -rf $DIR
> diff --git a/tests/qemu-iotests/tests/parallels-read-bitmap
> b/tests/qemu-iotests/tests/parallels-read-bitmap
> new file mode 100755
> index 00..b50b79f509
> --- /dev/null
> +++ b/tests/qemu-iotests/tests/parallels-read-bitmap
> @@ -0,0 +1,57 @@
> +#!/usr/bin/env python3
> +#
> +# Test parallels load bitmap
> +#
> +# Copyright (c) 2021 Virtuozzo International GmbH.
> +#
> +# 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 2/7] hw/boards: Introduce 'kvm_supported' field to MachineClass
On Fri, Feb 19, 2021 at 12:44:23PM +0100, Philippe Mathieu-Daudé wrote:
> Introduce the 'kvm_supported' field to express whether
> a machine supports KVM acceleration or not.
>
> Signed-off-by: Philippe Mathieu-Daudé
> ---
> include/hw/boards.h | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/include/hw/boards.h b/include/hw/boards.h
> index 68d3d10f6b0..0959aa743ee 100644
> --- a/include/hw/boards.h
> +++ b/include/hw/boards.h
> @@ -129,6 +129,8 @@ typedef struct {
> *Return the type of KVM corresponding to the kvm-type string option or
> *computed based on other criteria such as the host kernel capabilities
> *(which can't be negative), or -1 on error.
> + * @kvm_supported:
> + *true if '-enable-kvm' option is supported and false otherwise.
Is the behaviour reported really related to KVM specifically, as opposed
to all hardware based virt backends ?
eg is it actually a case of some machine types being "tcg_only" ?
Regards,
Daniel
--
|: https://berrange.com -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|
Re: [PATCH] hw/arm/xlnx-zynqmp: Remove obsolete 'has_rpu' property
On Fri, 19 Feb 2021 at 11:51, Philippe Mathieu-Daudé wrote:
>
> We hint the 'has_rpu' property is no longer required since commit
> 6908ec448b4 ("xlnx-zynqmp: Properly support the smp command line
> option") which was released in QEMU v2.11.0.
>
> 3 years later we feel safe enough to remove it without using the
> usual deprecation policy.
This device is marked user_creatable = false, so the only thing
that could be setting the property is the board code that creates
the device. So the property is not user-facing and we can remove it
without going through the deprecation process.
Reviewed-by: Peter Maydell
(maybe noting the above in the commit message).
thanks
-- PMM
Re: [PATCH] iotests: Drop deprecated 'props' from object-add
On 16.02.21 18:16, Alberto Garcia wrote:
Signed-off-by: Alberto Garcia
---
tests/qemu-iotests/087 | 8 ++--
tests/qemu-iotests/184 | 18 ++
tests/qemu-iotests/218 | 2 +-
tests/qemu-iotests/235 | 2 +-
tests/qemu-iotests/245 | 4 ++--
tests/qemu-iotests/258 | 7 +++
tests/qemu-iotests/258.out | 4 ++--
tests/qemu-iotests/295 | 2 +-
tests/qemu-iotests/296 | 2 +-
9 files changed, 19 insertions(+), 30 deletions(-)
Reviewed-by: Max Reitz
Two Python syntax nit picks below.
[...]
diff --git a/tests/qemu-iotests/218 b/tests/qemu-iotests/218
index ae7c4fb187..cbb38923cf 100755
--- a/tests/qemu-iotests/218
+++ b/tests/qemu-iotests/218
@@ -152,7 +152,7 @@ with iotests.VM() as vm, \
vm.launch()
ret = vm.qmp('object-add', qom_type='throttle-group', id='tg',
- props={'x-bps-read': 4096})
+ x_bps_read = 4096)
To stay consistent, I think there shouldn’t be spaces around '=' here.
(flake8 thinks so, too)
assert ret['return'] == {}
ret = vm.qmp('blockdev-add',
[..]
diff --git a/tests/qemu-iotests/258 b/tests/qemu-iotests/258
index 9a2d33ae5e..65ce02501a 100755
--- a/tests/qemu-iotests/258
+++ b/tests/qemu-iotests/258
@@ -103,10 +103,9 @@ def test_concurrent_finish(write_to_stream_node):
vm.qmp_log('object-add',
qom_type='throttle-group',
id='tg',
- props={
- 'x-iops-write': 1,
- 'x-iops-write-max': 1
- })
+ x_iops_write=1,
+ x_iops_write_max=1
+ )
This indentation looks weird to me now. Unfortunately, flake8 finds
this is the only correct indentation, so I have no reason to complain.
Perhaps putting it on the preceding line would be better?
Re: [PATCH 0/7] hw/kvm: Exit gracefully when KVM is not supported
On Fri, Feb 19, 2021 at 12:44:21PM +0100, Philippe Mathieu-Daudé wrote: > Hi, > > This series aims to improve user experience by providing > a better error message when the user tries to enable KVM > on machines not supporting it. Improved error message is good, but it is better if the mgmt apps knows not to try this in the first place. IOW, I think we want "query-machines" to filter out machines which are not available with the currently configured accelerator. libvirt will probe separately with both TCG and KVM enabled, so if query-machines can give the right answer in these cases, libvirt will probably "just work" and not offer to even start such a VM. Regards, Daniel -- |: https://berrange.com -o-https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o-https://fstop138.berrange.com :| |: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|
Re: [PATCH] gitlab-ci: Remove unused container images
On 2/19/21 12:09 PM, Thomas Huth wrote: > We're building a lot of containers in the gitlab-CI that we never use. > This takes away network bandwidth and CPU time from other jobs for no > use, so let's remove them for now. The individual containers could be > re-added later when we really need them. > > Signed-off-by: Thomas Huth > --- > .gitlab-ci.d/containers.yml | 92 - > 1 file changed, 92 deletions(-) I'm not enthusiast with this patch because I use various in this list from time to time for testing or cross build/disas binaries. Not having these containers used mainstream probably show the failure of the project to add good testing coverage on these targets. Most of them are for hobbyist with little time. Removing them will make it even harder to add tests. Can't we keep them disabled? Or put them in manual mode? Why is the CI rebuilding them, shouldn't them be cached or pulled from the registry? Maybe this show having all them in the same containers.yml file is not good enough? Any suggestion for splitting it, so lowly used containers don't get rebuild every time another often used one change the YAML file? Thanks, Phil.
[PATCH 11/18] qapi/introspect.py: improve _tree_to_qlit error message
From: John Snow
Trivial; make the error message just a pinch more explicit in case we
trip this by accident in the future.
Signed-off-by: John Snow
Message-Id: <20210216021809.134886-12-js...@redhat.com>
Reviewed-by: Markus Armbruster
Signed-off-by: Markus Armbruster
---
scripts/qapi/introspect.py | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/scripts/qapi/introspect.py b/scripts/qapi/introspect.py
index 715220afe7..96dfbb4cef 100644
--- a/scripts/qapi/introspect.py
+++ b/scripts/qapi/introspect.py
@@ -126,7 +126,9 @@ def indent(level):
elif isinstance(obj, bool):
ret += 'QLIT_QBOOL(%s)' % ('true' if obj else 'false')
else:
-assert False# not implemented
+raise NotImplementedError(
+f"type '{type(obj).__name__}' not implemented"
+)
if level > 0:
ret += ','
return ret
--
2.26.2
[PATCH 00/18] QAPI patches patches for 2021-02-18
The following changes since commit 91416a4254015e1e3f602f2b241b9ddb7879c10b: Merge remote-tracking branch 'remotes/stsquad/tags/pull-plugin-updates-180221-1' into staging (2021-02-18 13:27:03 +) are available in the Git repository at: git://repo.or.cz/qemu/armbru.git tags/pull-qapi-2021-02-18 for you to fetch changes up to 9b77d946990e7497469bb98171b90b4f3ab186a9: qapi/introspect.py: set _gen_tree's default ifcond argument to () (2021-02-18 19:51:14 +0100) QAPI patches patches for 2021-02-18 John Snow (18): qapi: Replace List[str] with Sequence[str] for ifcond qapi/introspect.py: assert schema is not None qapi/introspect.py: use _make_tree for features nodes qapi/introspect.py: add _gen_features helper qapi/introspect.py: guard against ifcond/comment misuse qapi/introspect.py: Unify return type of _make_tree() qapi/introspect.py: replace 'extra' dict with 'comment' argument qapi/introspect.py: Always define all 'extra' dict keys qapi/introspect.py: Introduce preliminary tree typing qapi/introspect.py: create a typed 'Annotated' data strutcure qapi/introspect.py: improve _tree_to_qlit error message qapi/introspect.py: improve readability of _tree_to_qlit qapi/introspect.py: remove _gen_variants helper qapi/introspect.py: add type hint annotations qapi/introspect.py: Add docstrings to _gen_tree and _tree_to_qlit qapi/introspect.py: Update copyright and authors list qapi/introspect.py: Type _gen_tree variants as Sequence[str] qapi/introspect.py: set _gen_tree's default ifcond argument to () scripts/qapi/commands.py | 3 +- scripts/qapi/events.py | 4 +- scripts/qapi/gen.py| 12 +- scripts/qapi/introspect.py | 327 - scripts/qapi/mypy.ini | 5 - scripts/qapi/schema.py | 2 +- scripts/qapi/types.py | 12 +- scripts/qapi/visit.py | 10 +- 8 files changed, 255 insertions(+), 120 deletions(-) -- 2.26.2
[PATCH 01/18] qapi: Replace List[str] with Sequence[str] for ifcond
From: John Snow It does happen to be a list (as of now), but we can describe it in more general terms with no loss in accuracy to allow tuples and other constructs. In the future, we can write "ifcond: Sequence[str] = ()" as a default parameter, which we could not do safely with a Mutable type like a List. Signed-off-by: John Snow Message-Id: <20210216021809.134886-2-js...@redhat.com> Reviewed-by: Markus Armbruster [Commit message tweaked] Signed-off-by: Markus Armbruster --- scripts/qapi/commands.py | 3 ++- scripts/qapi/events.py | 4 ++-- scripts/qapi/gen.py | 12 ++-- scripts/qapi/types.py| 12 ++-- scripts/qapi/visit.py| 10 +- 5 files changed, 21 insertions(+), 20 deletions(-) diff --git a/scripts/qapi/commands.py b/scripts/qapi/commands.py index 54af519f44..0a75a9371b 100644 --- a/scripts/qapi/commands.py +++ b/scripts/qapi/commands.py @@ -17,6 +17,7 @@ Dict, List, Optional, +Sequence, Set, ) @@ -297,7 +298,7 @@ def visit_end(self) -> None: def visit_command(self, name: str, info: Optional[QAPISourceInfo], - ifcond: List[str], + ifcond: Sequence[str], features: List[QAPISchemaFeature], arg_type: Optional[QAPISchemaObjectType], ret_type: Optional[QAPISchemaType], diff --git a/scripts/qapi/events.py b/scripts/qapi/events.py index 8c57deb2b8..90d2f6156d 100644 --- a/scripts/qapi/events.py +++ b/scripts/qapi/events.py @@ -12,7 +12,7 @@ See the COPYING file in the top-level directory. """ -from typing import List, Optional +from typing import List, Optional, Sequence from .common import c_enum_const, c_name, mcgen from .gen import QAPISchemaModularCVisitor, build_params, ifcontext @@ -214,7 +214,7 @@ def visit_end(self) -> None: def visit_event(self, name: str, info: Optional[QAPISourceInfo], -ifcond: List[str], +ifcond: Sequence[str], features: List[QAPISchemaFeature], arg_type: Optional[QAPISchemaObjectType], boxed: bool) -> None: diff --git a/scripts/qapi/gen.py b/scripts/qapi/gen.py index 63549cc8d4..1fa503bdbd 100644 --- a/scripts/qapi/gen.py +++ b/scripts/qapi/gen.py @@ -17,8 +17,8 @@ from typing import ( Dict, Iterator, -List, Optional, +Sequence, Tuple, ) @@ -85,7 +85,7 @@ def write(self, output_dir: str) -> None: fp.write(text) -def _wrap_ifcond(ifcond: List[str], before: str, after: str) -> str: +def _wrap_ifcond(ifcond: Sequence[str], before: str, after: str) -> str: if before == after: return after # suppress empty #if ... #endif @@ -127,9 +127,9 @@ def build_params(arg_type: Optional[QAPISchemaObjectType], class QAPIGenCCode(QAPIGen): def __init__(self, fname: str): super().__init__(fname) -self._start_if: Optional[Tuple[List[str], str, str]] = None +self._start_if: Optional[Tuple[Sequence[str], str, str]] = None -def start_if(self, ifcond: List[str]) -> None: +def start_if(self, ifcond: Sequence[str]) -> None: assert self._start_if is None self._start_if = (ifcond, self._body, self._preamble) @@ -187,11 +187,11 @@ def _bottom(self) -> str: @contextmanager -def ifcontext(ifcond: List[str], *args: QAPIGenCCode) -> Iterator[None]: +def ifcontext(ifcond: Sequence[str], *args: QAPIGenCCode) -> Iterator[None]: """ A with-statement context manager that wraps with `start_if()` / `end_if()`. -:param ifcond: A list of conditionals, passed to `start_if()`. +:param ifcond: A sequence of conditionals, passed to `start_if()`. :param args: any number of `QAPIGenCCode`. Example:: diff --git a/scripts/qapi/types.py b/scripts/qapi/types.py index 2bdd626847..20d572a23a 100644 --- a/scripts/qapi/types.py +++ b/scripts/qapi/types.py @@ -13,7 +13,7 @@ # See the COPYING file in the top-level directory. """ -from typing import List, Optional +from typing import List, Optional, Sequence from .common import ( c_enum_const, @@ -139,7 +139,7 @@ def gen_struct_members(members: List[QAPISchemaObjectTypeMember]) -> str: return ret -def gen_object(name: str, ifcond: List[str], +def gen_object(name: str, ifcond: Sequence[str], base: Optional[QAPISchemaObjectType], members: List[QAPISchemaObjectTypeMember], variants: Optional[QAPISchemaVariants]) -> str: @@ -307,7 +307,7 @@ def _gen_type_cleanup(self, name: str) -> None: def visit_enum_type(self, name: str, info: Optional[QAPISourceInfo], -ifcond: List[str], +ifcond: Sequence[str], features: List[QAPISchemaFeature],
[PATCH 03/18] qapi/introspect.py: use _make_tree for features nodes
From: John Snow
At present, we open-code this in _make_tree itself; but if the structure
of the tree changes, this is brittle. Use an explicit recursive call to
_make_tree when appropriate to help keep the interior node typing
consistent.
A consequence of doing this is that the 'ifcond' key of the features
dict will be omitted when ifcond is false-ish, just like it is omitted
in top-level calls to _make_tree. This also increases consistency in our
handling of this property.
Signed-off-by: John Snow
Message-Id: <20210216021809.134886-4-js...@redhat.com>
Reviewed-by: Markus Armbruster
Signed-off-by: Markus Armbruster
---
scripts/qapi/introspect.py | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/scripts/qapi/introspect.py b/scripts/qapi/introspect.py
index 43ab4be1f7..3295a15c98 100644
--- a/scripts/qapi/introspect.py
+++ b/scripts/qapi/introspect.py
@@ -30,7 +30,9 @@ def _make_tree(obj, ifcond, features, extra=None):
if ifcond:
extra['if'] = ifcond
if features:
-obj['features'] = [(f.name, {'if': f.ifcond}) for f in features]
+obj['features'] = [
+_make_tree(f.name, f.ifcond, None) for f in features
+]
if extra:
return (obj, extra)
return obj
--
2.26.2
[PATCH 18/18] qapi/introspect.py: set _gen_tree's default ifcond argument to ()
From: John Snow
We don't need to create an empty, mutable list to pass to _gen_tree;
since it is now typed as a Sequence, we can use the empty tuple as a
default and omit the argument.
Signed-off-by: John Snow
Message-Id: <20210216021809.134886-19-js...@redhat.com>
Reviewed-by: Markus Armbruster
Signed-off-by: Markus Armbruster
---
scripts/qapi/introspect.py | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/scripts/qapi/introspect.py b/scripts/qapi/introspect.py
index d0b0fd19ed..9a348ca2e5 100644
--- a/scripts/qapi/introspect.py
+++ b/scripts/qapi/introspect.py
@@ -254,7 +254,7 @@ def _gen_features(features: Sequence[QAPISchemaFeature]
return [Annotated(f.name, f.ifcond) for f in features]
def _gen_tree(self, name: str, mtype: str, obj: Dict[str, object],
- ifcond: Sequence[str],
+ ifcond: Sequence[str] = (),
features: Sequence[QAPISchemaFeature] = ()) -> None:
"""
Build and append a SchemaInfo object to self._trees.
@@ -302,7 +302,7 @@ def _gen_variant(self, variant: QAPISchemaVariant
def visit_builtin_type(self, name: str, info: Optional[QAPISourceInfo],
json_type: str) -> None:
-self._gen_tree(name, 'builtin', {'json-type': json_type}, [])
+self._gen_tree(name, 'builtin', {'json-type': json_type})
def visit_enum_type(self, name: str, info: Optional[QAPISourceInfo],
ifcond: Sequence[str],
--
2.26.2
[PATCH 08/18] qapi/introspect.py: Always define all 'extra' dict keys
From: John Snow
This mimics how a typed object works, where 'if' and 'comment' are
always set, regardless of if they have a value set or not.
It is safe to do this because of the way that _tree_to_qlit processes
these values (using dict.get with a default of None), resulting in no
change of output from _tree_to_qlit. There are no other users of this
data.
Signed-off-by: John Snow
Message-Id: <20210216021809.134886-9-js...@redhat.com>
Reviewed-by: Markus Armbruster
Signed-off-by: Markus Armbruster
---
scripts/qapi/introspect.py | 9 -
1 file changed, 4 insertions(+), 5 deletions(-)
diff --git a/scripts/qapi/introspect.py b/scripts/qapi/introspect.py
index c4326d42cb..88af5383d5 100644
--- a/scripts/qapi/introspect.py
+++ b/scripts/qapi/introspect.py
@@ -27,11 +27,10 @@
def _make_tree(obj, ifcond, comment=None):
-extra = {}
-if ifcond:
-extra['if'] = ifcond
-if comment:
-extra['comment'] = comment
+extra = {
+'if': ifcond,
+'comment': comment
+}
return (obj, extra)
--
2.26.2
[PATCH 02/18] qapi/introspect.py: assert schema is not None
From: John Snow
The introspect visitor is stateful, but expects that it will have a
schema to refer to. Add assertions that state this.
Signed-off-by: John Snow
Message-Id: <20210216021809.134886-3-js...@redhat.com>
Reviewed-by: Markus Armbruster
Signed-off-by: Markus Armbruster
---
scripts/qapi/introspect.py | 5 +
1 file changed, 5 insertions(+)
diff --git a/scripts/qapi/introspect.py b/scripts/qapi/introspect.py
index fafec94e02..43ab4be1f7 100644
--- a/scripts/qapi/introspect.py
+++ b/scripts/qapi/introspect.py
@@ -147,6 +147,8 @@ def _name(self, name):
return self._name_map[name]
def _use_type(self, typ):
+assert self._schema is not None
+
# Map the various integer types to plain int
if typ.json_type() == 'int':
typ = self._schema.lookup_type('int')
@@ -225,6 +227,8 @@ def visit_alternate_type(self, name, info, ifcond,
features, variants):
def visit_command(self, name, info, ifcond, features,
arg_type, ret_type, gen, success_response, boxed,
allow_oob, allow_preconfig, coroutine):
+assert self._schema is not None
+
arg_type = arg_type or self._schema.the_empty_object_type
ret_type = ret_type or self._schema.the_empty_object_type
obj = {'arg-type': self._use_type(arg_type),
@@ -234,6 +238,7 @@ def visit_command(self, name, info, ifcond, features,
self._gen_tree(name, 'command', obj, ifcond, features)
def visit_event(self, name, info, ifcond, features, arg_type, boxed):
+assert self._schema is not None
arg_type = arg_type or self._schema.the_empty_object_type
self._gen_tree(name, 'event', {'arg-type': self._use_type(arg_type)},
ifcond, features)
--
2.26.2
[PATCH 07/18] qapi/introspect.py: replace 'extra' dict with 'comment' argument
From: John Snow
This is only used to pass in a dictionary with a comment already set, so
skip the runaround and just accept the (optional) comment.
Signed-off-by: John Snow
Message-Id: <20210216021809.134886-8-js...@redhat.com>
Reviewed-by: Markus Armbruster
Signed-off-by: Markus Armbruster
---
scripts/qapi/introspect.py | 15 +--
1 file changed, 9 insertions(+), 6 deletions(-)
diff --git a/scripts/qapi/introspect.py b/scripts/qapi/introspect.py
index 7cce0de975..c4326d42cb 100644
--- a/scripts/qapi/introspect.py
+++ b/scripts/qapi/introspect.py
@@ -10,6 +10,8 @@
See the COPYING file in the top-level directory.
"""
+from typing import Optional
+
from .common import (
c_name,
gen_endif,
@@ -24,11 +26,12 @@
)
-def _make_tree(obj, ifcond, extra=None):
-if extra is None:
-extra = {}
+def _make_tree(obj, ifcond, comment=None):
+extra = {}
if ifcond:
extra['if'] = ifcond
+if comment:
+extra['comment'] = comment
return (obj, extra)
@@ -175,18 +178,18 @@ def _gen_features(features):
return [_make_tree(f.name, f.ifcond) for f in features]
def _gen_tree(self, name, mtype, obj, ifcond, features):
-extra = None
+comment: Optional[str] = None
if mtype not in ('command', 'event', 'builtin', 'array'):
if not self._unmask:
# Output a comment to make it easy to map masked names
# back to the source when reading the generated output.
-extra = {'comment': '"%s" = %s' % (self._name(name), name)}
+comment = f'"{self._name(name)}" = {name}'
name = self._name(name)
obj['name'] = name
obj['meta-type'] = mtype
if features:
obj['features'] = self._gen_features(features)
-self._trees.append(_make_tree(obj, ifcond, extra))
+self._trees.append(_make_tree(obj, ifcond, comment))
def _gen_member(self, member):
obj = {'name': member.name, 'type': self._use_type(member.type)}
--
2.26.2
[PATCH 14/18] qapi/introspect.py: add type hint annotations
From: John Snow
NB: The type aliases (SchemaInfo et al) declare intent for some of the
"dictly-typed" objects we pass around in introspect.py. They do not
enforce the shape of those objects, and cannot, until Python 3.7 or
later. (And even then, it may not be "worth it".)
Annotations are also added to the QAPISchemaEntity __init__ method in
schema.py to allow mypy to statically prove the type of typ.name,
needed to prove the return type of
QAPISchemaGenIntrospectVisitor._use_type().
Signed-off-by: John Snow
Message-Id: <20210216021809.134886-15-js...@redhat.com>
Reviewed-by: Markus Armbruster
[Note on QAPISchemaEntity.__init__() squashed into commit message,
Comment wrapped to conform to PEP 8]
Signed-off-by: Markus Armbruster
---
scripts/qapi/introspect.py | 125 +++--
scripts/qapi/mypy.ini | 5 --
scripts/qapi/schema.py | 2 +-
3 files changed, 93 insertions(+), 39 deletions(-)
diff --git a/scripts/qapi/introspect.py b/scripts/qapi/introspect.py
index da7bc8883c..05c1a196e9 100644
--- a/scripts/qapi/introspect.py
+++ b/scripts/qapi/introspect.py
@@ -17,6 +17,7 @@
Iterable,
List,
Optional,
+Sequence,
Tuple,
TypeVar,
Union,
@@ -30,10 +31,19 @@
)
from .gen import QAPISchemaMonolithicCVisitor
from .schema import (
+QAPISchema,
QAPISchemaArrayType,
QAPISchemaBuiltinType,
+QAPISchemaEntity,
+QAPISchemaEnumMember,
+QAPISchemaFeature,
+QAPISchemaObjectType,
+QAPISchemaObjectTypeMember,
QAPISchemaType,
+QAPISchemaVariant,
+QAPISchemaVariants,
)
+from .source import QAPISourceInfo
# This module constructs a tree data structure that is used to
@@ -58,6 +68,16 @@
_Value = Union[_Scalar, _NonScalar]
JSONValue = Union[_Value, 'Annotated[_Value]']
+# These types are based on structures defined in QEMU's schema, so we
+# lack precise types for them here. Python 3.6 does not offer
+# TypedDict constructs, so they are broadly typed here as simple
+# Python Dicts.
+SchemaInfo = Dict[str, object]
+SchemaInfoObject = Dict[str, object]
+SchemaInfoObjectVariant = Dict[str, object]
+SchemaInfoObjectMember = Dict[str, object]
+SchemaInfoCommand = Dict[str, object]
+
_ValueT = TypeVar('_ValueT', bound=_Value)
@@ -77,9 +97,11 @@ def __init__(self, value: _ValueT, ifcond: Iterable[str],
self.ifcond: Tuple[str, ...] = tuple(ifcond)
-def _tree_to_qlit(obj, level=0, dict_value=False):
+def _tree_to_qlit(obj: JSONValue,
+ level: int = 0,
+ dict_value: bool = False) -> str:
-def indent(level):
+def indent(level: int) -> str:
return level * 4 * ' '
if isinstance(obj, Annotated):
@@ -137,21 +159,21 @@ def indent(level):
return ret
-def to_c_string(string):
+def to_c_string(string: str) -> str:
return '"' + string.replace('\\', r'\\').replace('"', r'\"') + '"'
class QAPISchemaGenIntrospectVisitor(QAPISchemaMonolithicCVisitor):
-def __init__(self, prefix, unmask):
+def __init__(self, prefix: str, unmask: bool):
super().__init__(
prefix, 'qapi-introspect',
' * QAPI/QMP schema introspection', __doc__)
self._unmask = unmask
-self._schema = None
-self._trees = []
-self._used_types = []
-self._name_map = {}
+self._schema: Optional[QAPISchema] = None
+self._trees: List[Annotated[SchemaInfo]] = []
+self._used_types: List[QAPISchemaType] = []
+self._name_map: Dict[str, str] = {}
self._genc.add(mcgen('''
#include "qemu/osdep.h"
#include "%(prefix)sqapi-introspect.h"
@@ -159,10 +181,10 @@ def __init__(self, prefix, unmask):
''',
prefix=prefix))
-def visit_begin(self, schema):
+def visit_begin(self, schema: QAPISchema) -> None:
self._schema = schema
-def visit_end(self):
+def visit_end(self) -> None:
# visit the types that are actually used
for typ in self._used_types:
typ.visit(self)
@@ -184,18 +206,18 @@ def visit_end(self):
self._used_types = []
self._name_map = {}
-def visit_needed(self, entity):
+def visit_needed(self, entity: QAPISchemaEntity) -> bool:
# Ignore types on first pass; visit_end() will pick up used types
return not isinstance(entity, QAPISchemaType)
-def _name(self, name):
+def _name(self, name: str) -> str:
if self._unmask:
return name
if name not in self._name_map:
self._name_map[name] = '%d' % len(self._name_map)
return self._name_map[name]
-def _use_type(self, typ):
+def _use_type(self, typ: QAPISchemaType) -> str:
assert self._schema is not None
# Map the various integer types to plain int
@@ -217,10 +239,13 @@ def _use_type(self, typ):
return self._name(typ.name)
@staticmethod
-def _gen_features(features):
+def
[PATCH 12/18] qapi/introspect.py: improve readability of _tree_to_qlit
From: John Snow
Subjective, but I find getting rid of the comprehensions helps. Also,
divide the sections into scalar and non-scalar sections, and remove
old-style string formatting.
Signed-off-by: John Snow
Message-Id: <20210216021809.134886-13-js...@redhat.com>
Reviewed-by: Markus Armbruster
Signed-off-by: Markus Armbruster
---
scripts/qapi/introspect.py | 33 ++---
1 file changed, 18 insertions(+), 15 deletions(-)
diff --git a/scripts/qapi/introspect.py b/scripts/qapi/introspect.py
index 96dfbb4cef..26e6f73e5d 100644
--- a/scripts/qapi/introspect.py
+++ b/scripts/qapi/introspect.py
@@ -91,7 +91,7 @@ def indent(level):
ret = ''
if obj.comment:
-ret += indent(level) + '/* %s */\n' % obj.comment
+ret += indent(level) + f"/* {obj.comment} */\n"
if obj.ifcond:
ret += gen_if(obj.ifcond)
ret += _tree_to_qlit(obj.value, level)
@@ -102,33 +102,36 @@ def indent(level):
ret = ''
if not dict_value:
ret += indent(level)
+
+# Scalars:
if obj is None:
ret += 'QLIT_QNULL'
elif isinstance(obj, str):
-ret += 'QLIT_QSTR(' + to_c_string(obj) + ')'
+ret += f"QLIT_QSTR({to_c_string(obj)})"
+elif isinstance(obj, bool):
+ret += f"QLIT_QBOOL({str(obj).lower()})"
+
+# Non-scalars:
elif isinstance(obj, list):
-elts = [_tree_to_qlit(elt, level + 1).strip('\n')
-for elt in obj]
-elts.append(indent(level + 1) + "{}")
ret += 'QLIT_QLIST(((QLitObject[]) {\n'
-ret += '\n'.join(elts) + '\n'
+for value in obj:
+ret += _tree_to_qlit(value, level + 1).strip('\n') + '\n'
+ret += indent(level + 1) + '{}\n'
ret += indent(level) + '}))'
elif isinstance(obj, dict):
-elts = []
-for key, value in sorted(obj.items()):
-elts.append(indent(level + 1) + '{ %s, %s }' %
-(to_c_string(key),
- _tree_to_qlit(value, level + 1, True)))
-elts.append(indent(level + 1) + '{}')
ret += 'QLIT_QDICT(((QLitDictEntry[]) {\n'
-ret += ',\n'.join(elts) + '\n'
+for key, value in sorted(obj.items()):
+ret += indent(level + 1) + "{{ {:s}, {:s} }},\n".format(
+to_c_string(key),
+_tree_to_qlit(value, level + 1, dict_value=True)
+)
+ret += indent(level + 1) + '{}\n'
ret += indent(level) + '}))'
-elif isinstance(obj, bool):
-ret += 'QLIT_QBOOL(%s)' % ('true' if obj else 'false')
else:
raise NotImplementedError(
f"type '{type(obj).__name__}' not implemented"
)
+
if level > 0:
ret += ','
return ret
--
2.26.2
[PATCH 05/18] qapi/introspect.py: guard against ifcond/comment misuse
From: John Snow
_tree_to_qlit is called recursively on dict values (isolated from their
keys); at such a point in generating output it is too late to apply an
ifcond. Similarly, comments do not necessarily have a "tidy" place they
can be printed in such a circumstance.
Forbid this usage by renaming "suppress_first_indent" to "dict_value" to
emphasize that indents are suppressed only for the benefit of dict
values; then add an assertion assuring we do not pass ifcond/comments
in this case.
Signed-off-by: John Snow
Message-Id: <20210216021809.134886-6-js...@redhat.com>
Reviewed-by: Markus Armbruster
[Comment wrapped to conform to PEP 8]
Signed-off-by: Markus Armbruster
---
scripts/qapi/introspect.py | 11 +--
1 file changed, 9 insertions(+), 2 deletions(-)
diff --git a/scripts/qapi/introspect.py b/scripts/qapi/introspect.py
index 4749f65ea3..a111cec725 100644
--- a/scripts/qapi/introspect.py
+++ b/scripts/qapi/introspect.py
@@ -34,7 +34,7 @@ def _make_tree(obj, ifcond, extra=None):
return obj
-def _tree_to_qlit(obj, level=0, suppress_first_indent=False):
+def _tree_to_qlit(obj, level=0, dict_value=False):
def indent(level):
return level * 4 * ' '
@@ -43,6 +43,13 @@ def indent(level):
ifobj, extra = obj
ifcond = extra.get('if')
comment = extra.get('comment')
+
+# NB: _tree_to_qlit is called recursively on the values of a
+# key:value pair; those values can't be decorated with
+# comments or conditionals.
+msg = "dict values cannot have attached comments or if-conditionals."
+assert not dict_value, msg
+
ret = ''
if comment:
ret += indent(level) + '/* %s */\n' % comment
@@ -54,7 +61,7 @@ def indent(level):
return ret
ret = ''
-if not suppress_first_indent:
+if not dict_value:
ret += indent(level)
if obj is None:
ret += 'QLIT_QNULL'
--
2.26.2
Re: [PATCH 0/7] hw/kvm: Exit gracefully when KVM is not supported
On 2/19/21 12:55 PM, Peter Maydell wrote: > On Fri, 19 Feb 2021 at 11:44, Philippe Mathieu-Daudé > wrote: >> This series aims to improve user experience by providing >> a better error message when the user tries to enable KVM >> on machines not supporting it. > > Thanks for having a look at this; fixing the ugly assertion > failure if you try to enable KVM for the raspi boards has > been vaguely on my todo list but never made it up to the top... The other one annoying was the xlnx-zcu102 when creating the Cortex-R cores. >> Philippe Mathieu-Daudé (7): >> accel/kvm: Check MachineClass kvm_type() return value >> hw/boards: Introduce 'kvm_supported' field to MachineClass >> hw/arm: Set kvm_supported for KVM-compatible machines >> hw/mips: Set kvm_supported for KVM-compatible machines >> hw/ppc: Set kvm_supported for KVM-compatible machines >> hw/s390x: Set kvm_supported to s390-ccw-virtio machines >> accel/kvm: Exit gracefully when KVM is not supported > > Don't we also need to set kvm_supported for the relevant > machine types in hw/i386 ? Lol, clearly a parapraxis =) I'll send it as 8/7 until I get more review comments for a v2 (in particular on the PPC patch): -- >8 -- diff --git a/hw/i386/x86.c b/hw/i386/x86.c index 6329f90ef90..da895aa051d 100644 --- a/hw/i386/x86.c +++ b/hw/i386/x86.c @@ -1218,6 +1218,7 @@ static void x86_machine_class_init(ObjectClass *oc, void *data) mc->cpu_index_to_instance_props = x86_cpu_index_to_props; mc->get_default_cpu_node_id = x86_get_default_cpu_node_id; mc->possible_cpu_arch_ids = x86_possible_cpu_arch_ids; +mc->kvm_supported = true; x86mc->compat_apic_id_mode = false; x86mc->save_tsc_khz = true; nc->nmi_monitor_handler = x86_nmi; --- Regards, Phil.
Re: [PATCH 0/7] hw/kvm: Exit gracefully when KVM is not supported
On 2/19/21 1:00 PM, Daniel P. Berrangé wrote: > On Fri, Feb 19, 2021 at 12:44:21PM +0100, Philippe Mathieu-Daudé wrote: >> Hi, >> >> This series aims to improve user experience by providing >> a better error message when the user tries to enable KVM >> on machines not supporting it. > > Improved error message is good, but it is better if the mgmt apps knows > not to try this in the first place. I am not sure this is the same problem. This series addresses users from the command line (without mgmt app). > IOW, I think we want "query-machines" to filter out machines > which are not available with the currently configured accelerator. > > libvirt will probe separately with both TCG and KVM enabled, so if > query-machines can give the right answer in these cases, libvirt > will probably "just work" and not offer to even start such a VM. Yes, agreed. There are other discussions about 'query-machines' and an eventual 'query-accels'. This series doesn't aim to fix the mgmt app problems.
[PATCH 16/18] qapi/introspect.py: Update copyright and authors list
From: John Snow To reflect the work that went into strictly typing introspect.py, punish myself by claiming credit. Signed-off-by: John Snow Message-Id: <20210216021809.134886-17-js...@redhat.com> Reviewed-by: Markus Armbruster Signed-off-by: Markus Armbruster --- scripts/qapi/introspect.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/scripts/qapi/introspect.py b/scripts/qapi/introspect.py index 15cce6854d..e770c9432b 100644 --- a/scripts/qapi/introspect.py +++ b/scripts/qapi/introspect.py @@ -1,10 +1,11 @@ """ QAPI introspection generator -Copyright (C) 2015-2018 Red Hat, Inc. +Copyright (C) 2015-2021 Red Hat, Inc. Authors: Markus Armbruster + John Snow This work is licensed under the terms of the GNU GPL, version 2. See the COPYING file in the top-level directory. -- 2.26.2
