Discussion:
[Qemu-devel] [PATCH v1 0/3] MTTCG start-up tweaks
Alex Bennée
2017-02-28 15:07:06 UTC
Permalink
Hi,

These are a couple of quick tweaks to command line handling for MTTCG.
The first patch moves the paring of --accel tcg to after icount and
simplifies the logic to disable mttcg when icount is enabled. There is
still a regression to icount to be fixed (caused by 8d04fb5).

Finally there is a patch to declare the TCG_GUEST_DEFAULT_MO for x86
guests to avoid the bogus warning about memory order when forcing
mttcg on. A new warning has been added specifically for guests that
haven't declared TARGET_SUPPORT_MTTCG which means the front-end has
had atomics/barrier/helpers converted for a MTTCG world.

Alex Bennée (3):
vl/cpus: be smarter with icount and MTTCG
target/i386/cpu.h: declare TCG_GUEST_DEFAULT_MO
cpus.c: add additional error_report when !TARGET_SUPPORT_MTTCG

cpus.c | 11 +++++++----
target/i386/cpu.h | 3 +++
vl.c | 7 ++-----
3 files changed, 12 insertions(+), 9 deletions(-)
--
2.11.0
Alex Bennée
2017-02-28 15:07:09 UTC
Permalink
While we may fail the memory ordering check later that can be
confusing. So in cases where TARGET_SUPPORT_MTTCG has yet to be
defined we should say so specifically.

Signed-off-by: Alex Bennée <***@linaro.org>
---
cpus.c | 4 ++++
1 file changed, 4 insertions(+)

diff --git a/cpus.c b/cpus.c
index e9aab9f70f..2459f564ea 100644
--- a/cpus.c
+++ b/cpus.c
@@ -206,6 +206,10 @@ void qemu_tcg_configure(QemuOpts *opts, Error **errp)
} else if (use_icount) {
error_setg(errp, "No MTTCG when icount is enabled");
} else {
+#ifndef TARGET_SUPPORT_MTTCG
+ error_report("Guest not yet converted to MTTCG - "
+ "you may get unexpected results");
+#endif
if (!check_tcg_memory_orders_compatible()) {
error_report("Guest expects a stronger memory ordering "
"than the host provides");
--
2.11.0
Alex Bennée
2017-02-28 15:07:07 UTC
Permalink
The sense of the test was inverted. Make it simple, if icount is
enabled then we disabled MTTCG by default. If the user tries to force
MTTCG upon us then we tell them "no".

Signed-off-by: Alex Bennée <***@linaro.org>
---
cpus.c | 7 +++----
vl.c | 7 ++-----
2 files changed, 5 insertions(+), 9 deletions(-)

diff --git a/cpus.c b/cpus.c
index 8200ac6b75..e9aab9f70f 100644
--- a/cpus.c
+++ b/cpus.c
@@ -185,10 +185,7 @@ static bool check_tcg_memory_orders_compatible(void)

static bool default_mttcg_enabled(void)
{
- QemuOpts *icount_opts = qemu_find_opts_singleton("icount");
- const char *rr = qemu_opt_get(icount_opts, "rr");
-
- if (rr || TCG_OVERSIZED_GUEST) {
+ if (use_icount || TCG_OVERSIZED_GUEST) {
return false;
} else {
#ifdef TARGET_SUPPORTS_MTTCG
@@ -206,6 +203,8 @@ void qemu_tcg_configure(QemuOpts *opts, Error **errp)
if (strcmp(t, "multi") == 0) {
if (TCG_OVERSIZED_GUEST) {
error_setg(errp, "No MTTCG when guest word size > hosts");
+ } else if (use_icount) {
+ error_setg(errp, "No MTTCG when icount is enabled");
} else {
if (!check_tcg_memory_orders_compatible()) {
error_report("Guest expects a stronger memory ordering "
diff --git a/vl.c b/vl.c
index e10a27bdd6..bbbf1baadf 100644
--- a/vl.c
+++ b/vl.c
@@ -4025,8 +4025,6 @@ int main(int argc, char **argv, char **envp)

replay_configure(icount_opts);

- qemu_tcg_configure(accel_opts, &error_fatal);
-
machine_class = select_machine();

set_memory_options(&ram_slots, &maxram_size, machine_class);
@@ -4393,14 +4391,13 @@ int main(int argc, char **argv, char **envp)
if (!tcg_enabled()) {
error_report("-icount is not allowed with hardware virtualization");
exit(1);
- } else if (qemu_tcg_mttcg_enabled()) {
- error_report("-icount does not currently work with MTTCG");
- exit(1);
}
configure_icount(icount_opts, &error_abort);
qemu_opts_del(icount_opts);
}

+ qemu_tcg_configure(accel_opts, &error_fatal);
+
if (default_net) {
QemuOptsList *net = qemu_find_opts("net");
qemu_opts_set(net, NULL, "type", "nic", &error_abort);
--
2.11.0
Alex Bennée
2017-02-28 15:07:08 UTC
Permalink
This suppresses the incorrect warning when forcing MTTCG for x86
guests on x86 hosts. A future patch will still warn when
TARGET_SUPPORT_MTTCG hasn't been defined for the guest (which is still
pending for x86).

Reported-by: Paolo Bonzini <***@redhat.com>
Signed-off-by: Alex Bennée <***@linaro.org>
---
target/i386/cpu.h | 3 +++
1 file changed, 3 insertions(+)

diff --git a/target/i386/cpu.h b/target/i386/cpu.h
index 573f2aa988..6be19d7e74 100644
--- a/target/i386/cpu.h
+++ b/target/i386/cpu.h
@@ -30,6 +30,9 @@
#define TARGET_LONG_BITS 32
#endif

+/* The x86 has a strong memory model with some store-after-load re-ordering */
+#define TCG_GUEST_DEFAULT_MO (TCG_MO_ALL & ~TCG_MO_ST_LD)
+
/* Maximum instruction code size */
#define TARGET_MAX_INSN_SIZE 16
--
2.11.0
Richard Henderson
2017-02-28 18:11:04 UTC
Permalink
Post by Alex Bennée
Hi,
These are a couple of quick tweaks to command line handling for MTTCG.
The first patch moves the paring of --accel tcg to after icount and
simplifies the logic to disable mttcg when icount is enabled. There is
still a regression to icount to be fixed (caused by 8d04fb5).
Finally there is a patch to declare the TCG_GUEST_DEFAULT_MO for x86
guests to avoid the bogus warning about memory order when forcing
mttcg on. A new warning has been added specifically for guests that
haven't declared TARGET_SUPPORT_MTTCG which means the front-end has
had atomics/barrier/helpers converted for a MTTCG world.
vl/cpus: be smarter with icount and MTTCG
target/i386/cpu.h: declare TCG_GUEST_DEFAULT_MO
cpus.c: add additional error_report when !TARGET_SUPPORT_MTTCG
cpus.c | 11 +++++++----
target/i386/cpu.h | 3 +++
vl.c | 7 ++-----
3 files changed, 12 insertions(+), 9 deletions(-)
Reviewed-by: Richard Henderson <***@twiddle.net>


r~

Loading...