Re: [Qemu-devel] [PATCH v2 10/45] translate-all: guarantee that tb_hash only holds valid TBs

[Richard Henderson]

On 07/17/2017 06:54 PM, Emilio G. Cota wrote:

What threw me off was that in lookup_tb_ptr we're not checking tb->invalid,
and that biased me into thinking that it's not needed. But I should have
tried harder. Also, that's a bug, and yet another reason to have tb_lookup,
so that we fix these things at once in one place.


Yes, me as well.  Quite right we need only one copy of this code.


 (tb->flags & (CF_HASH_MASK | CF_INVALID)) == cf_mask

So that we continue to check CF_INVALID each time we lookup a TB, but now we
get it for free as a part of the other flags check.


With the annoying atomic_read thrown in there :-) but yes, will do.


Yes of course.  ;-)


r~

Re: [Qemu-devel] [PATCH v3] spapr: disable decrementer during reset

[Nikunj A Dadhania]

David Gibson  writes:

> On Mon, Jul 17, 2017 at 09:46:39AM +0530, Nikunj A Dadhania wrote:
>> Rebooting a SMP TCG guest is broken for both single/multi threaded TCG.
>> 
>> When reset happens, all the CPUs are in halted state. First CPU is brought 
>> out
>> of reset and secondary CPUs would be initialized by the guest kernel using a
>> rtas call start-cpu.
>> 
>> However, in case of TCG, decrementer interrupts keep on coming and waking the
>> secondary CPUs up.
>> 
>> These secondary CPUs would see the decrementer interrupt pending, which makes
>> cpu::has_work() to bring them out of wait loop and start executing
>> tcg_exec_cpu().
>> 
>> The problem with this is all the CPUs wake up and start booting SLOF image,
>> causing the following exception(4 CPUs TCG VM):
>
> Ok, I'm still trying to understand why the behaviour on reboot is
> different from the first boot.

During first boot, the cpu is in the stopped state, so
cpus.c:cpu_thread_is_idle returns true and CPU remains in halted state
until rtas start-cpu. Therefore, we never check the cpu_has_work()

In case of reboot, all CPUs are resumed after reboot. So we check the
next condition cpu_has_work() in cpu_thread_is_idle(), where we see a
DECR interrupt and remove the CPU from halted state as the CPU has work.

> AFAICT on initial boot, the LPCR will
> have DEE / PECE3 enabled.  So why aren't we getting the same problem
> then?

Regards
Nikunj

Re: [Qemu-devel] [PATCH v3] spapr: disable decrementer during reset

[Nikunj A Dadhania]

David Gibson  writes:

> On Mon, Jul 17, 2017 at 09:46:39AM +0530, Nikunj A Dadhania wrote:
>> Rebooting a SMP TCG guest is broken for both single/multi threaded TCG.
>> 
>> When reset happens, all the CPUs are in halted state. First CPU is brought 
>> out
>> of reset and secondary CPUs would be initialized by the guest kernel using a
>> rtas call start-cpu.
>> 
>> However, in case of TCG, decrementer interrupts keep on coming and waking the
>> secondary CPUs up.
>> 
>> These secondary CPUs would see the decrementer interrupt pending, which makes
>> cpu::has_work() to bring them out of wait loop and start executing
>> tcg_exec_cpu().
>> 
>> The problem with this is all the CPUs wake up and start booting SLOF image,
>> causing the following exception(4 CPUs TCG VM):
>
> Ok, I'm still trying to understand why the behaviour on reboot is
> different from the first boot.

During first boot, the cpu is in the stopped state, so
cpus.c:cpu_thread_is_idle returns true and CPU remains in halted state
until rtas start-cpu. Therefore, we never check the cpu_has_work()

In case of reboot, all CPUs are resumed after reboot. So we check the
next condition cpu_has_work() in cpu_thread_is_idle(), where we see a
DECR interrupt and remove the CPU from halted state as the CPU has work.

> AFAICT on initial boot, the LPCR will
> have DEE / PECE3 enabled.  So why aren't we getting the same problem
> then?

Regards
Nikunj

Re: [Qemu-devel] [PATCH v3] spapr: disable decrementer during reset

[Nikunj A Dadhania]

David Gibson  writes:

> On Mon, Jul 17, 2017 at 09:46:39AM +0530, Nikunj A Dadhania wrote:
>> Rebooting a SMP TCG guest is broken for both single/multi threaded TCG.
>> 
>> When reset happens, all the CPUs are in halted state. First CPU is brought 
>> out
>> of reset and secondary CPUs would be initialized by the guest kernel using a
>> rtas call start-cpu.
>> 
>> However, in case of TCG, decrementer interrupts keep on coming and waking the
>> secondary CPUs up.
>> 
>> These secondary CPUs would see the decrementer interrupt pending, which makes
>> cpu::has_work() to bring them out of wait loop and start executing
>> tcg_exec_cpu().
>> 
>> The problem with this is all the CPUs wake up and start booting SLOF image,
>> causing the following exception(4 CPUs TCG VM):
>
> Ok, I'm still trying to understand why the behaviour on reboot is
> different from the first boot.

During first boot, the cpu is in the stopped state, so
cpus.c:cpu_thread_is_idle returns true and CPU remains in halted state
until rtas start-cpu. Therefore, we never check the cpu_has_work()

In case of reboot, all CPUs are resumed after reboot. So we check the
next condition cpu_has_work() in cpu_thread_is_idle(), where we see a
DECR interrupt and remove the CPU from halted state as the CPU has work.

> AFAICT on initial boot, the LPCR will
> have DEE / PECE3 enabled.  So why aren't we getting the same problem
> then?

Regards
Nikunj

[Qemu-devel] [PULL 6/8] qemu-ga: add guest-get-osinfo command

[Michael Roth]

From: Tomáš Golembiovský 

Add a new 'guest-get-osinfo' command for reporting basic information of
the guest operating system. This includes machine architecture,
version and release of the kernel and several fields from os-release
file if it is present (as defined in [1]).

[1] https://www.freedesktop.org/software/systemd/man/os-release.html

Signed-off-by: Vinzenz Feenstra 
Signed-off-by: Tomáš Golembiovský 
Signed-off-by: Michael Roth 
---
 qga/commands-posix.c | 134 
 qga/commands-win32.c | 188 +++
 qga/qapi-schema.json |  65 ++
 3 files changed, 387 insertions(+)

diff --git a/qga/commands-posix.c b/qga/commands-posix.c
index e7a047e..672bf29 100644
--- a/qga/commands-posix.c
+++ b/qga/commands-posix.c
@@ -13,6 +13,7 @@
 
 #include "qemu/osdep.h"
 #include 
+#include 
 #include 
 #include 
 #include "qga/guest-agent-core.h"
@@ -2592,3 +2593,136 @@ GuestUserList *qmp_guest_get_users(Error **errp)
 }
 
 #endif
+
+/* Replace escaped special characters with theire real values. The replacement
+ * is done in place -- returned value is in the original string.
+ */
+static void ga_osrelease_replace_special(gchar *value)
+{
+gchar *p, *p2, quote;
+
+/* Trim the string at first space or semicolon if it is not enclosed in
+ * single or double quotes. */
+if ((value[0] != '"') || (value[0] == '\'')) {
+p = strchr(value, ' ');
+if (p != NULL) {
+*p = 0;
+}
+p = strchr(value, ';');
+if (p != NULL) {
+*p = 0;
+}
+return;
+}
+
+quote = value[0];
+p2 = value;
+p = value + 1;
+while (*p != 0) {
+if (*p == '\\') {
+p++;
+switch (*p) {
+case '$':
+case '\'':
+case '"':
+case '\\':
+case '`':
+break;
+default:
+/* Keep literal backslash followed by whatever is there */
+p--;
+break;
+}
+} else if (*p == quote) {
+*p2 = 0;
+break;
+}
+*(p2++) = *(p++);
+}
+}
+
+static GKeyFile *ga_parse_osrelease(const char *fname)
+{
+gchar *content = NULL;
+gchar *content2 = NULL;
+GError *err = NULL;
+GKeyFile *keys = g_key_file_new();
+const char *group = "[os-release]\n";
+
+if (!g_file_get_contents(fname, , NULL, )) {
+slog("failed to read '%s', error: %s", fname, err->message);
+goto fail;
+}
+
+if (!g_utf8_validate(content, -1, NULL)) {
+slog("file is not utf-8 encoded: %s", fname);
+goto fail;
+}
+content2 = g_strdup_printf("%s%s", group, content);
+
+if (!g_key_file_load_from_data(keys, content2, -1, G_KEY_FILE_NONE,
+   )) {
+slog("failed to parse file '%s', error: %s", fname, err->message);
+goto fail;
+}
+
+g_free(content);
+g_free(content2);
+return keys;
+
+fail:
+g_error_free(err);
+g_free(content);
+g_free(content2);
+g_key_file_free(keys);
+return NULL;
+}
+
+GuestOSInfo *qmp_guest_get_osinfo(Error **errp)
+{
+GuestOSInfo *info = NULL;
+struct utsname kinfo = {0};
+
+info = g_new0(GuestOSInfo, 1);
+
+if (uname() != 0) {
+error_setg_errno(errp, errno, "uname failed");
+} else {
+info->has_kernel_version = true;
+info->kernel_version = g_strdup(kinfo.version);
+info->has_kernel_release = true;
+info->kernel_release = g_strdup(kinfo.release);
+info->has_machine = true;
+info->machine = g_strdup(kinfo.machine);
+}
+
+GKeyFile *osrelease = ga_parse_osrelease("/etc/os-release");
+if (osrelease == NULL) {
+osrelease = ga_parse_osrelease("/usr/lib/os-release");
+}
+
+if (osrelease != NULL) {
+char *value;
+
+#define GET_FIELD(field, osfield) do { \
+value = g_key_file_get_value(osrelease, "os-release", osfield, NULL); \
+if (value != NULL) { \
+ga_osrelease_replace_special(value); \
+info->has_ ## field = true; \
+info->field = value; \
+} \
+} while (0)
+GET_FIELD(id, "ID");
+GET_FIELD(name, "NAME");
+GET_FIELD(pretty_name, "PRETTY_NAME");
+GET_FIELD(version, "VERSION");
+GET_FIELD(version_id, "VERSION_ID");
+GET_FIELD(variant, "VARIANT");
+GET_FIELD(variant_id, "VARIANT_ID");
+#undef GET_FIELD
+
+g_key_file_free(osrelease);
+}
+
+return info;
+}
diff --git a/qga/commands-win32.c b/qga/commands-win32.c
index 6f16457..524c71b 100644
--- a/qga/commands-win32.c
+++ b/qga/commands-win32.c
@@ -1642,3 +1642,191 @@ GuestUserList *qmp_guest_get_users(Error **err)
 return NULL;
 #endif
 }
+
+typedef struct _ga_matrix_lookup_t {
+   

[Qemu-devel] [PULL 3/8] qemu-ga: check if utmpx.h is available on the system

[Michael Roth]

From: Tomáš Golembiovský 

Commit 161a56a9065 added command guest-get-users and requires the
utmpx.h (defined by POSIX) to work. It is however not always available
(e.g. on OpenBSD) therefor a check for its existence is necessary.

Signed-off-by: Tomáš Golembiovský 
Signed-off-by: Michael Roth 
---
 configure| 19 +++
 qga/commands-posix.c | 17 -
 2 files changed, 35 insertions(+), 1 deletion(-)

diff --git a/configure b/configure
index a3f0522..e8798ce 100755
--- a/configure
+++ b/configure
@@ -4915,6 +4915,21 @@ if compile_prog "" "" ; then
 fi
 
 ##
+# check for utmpx.h, it is missing e.g. on OpenBSD
+
+have_utmpx=no
+cat > $TMPC << EOF
+#include 
+struct utmpx user_info;
+int main(void) {
+return 0;
+}
+EOF
+if compile_prog "" "" ; then
+have_utmpx=yes
+fi
+
+##
 # End of CC checks
 # After here, no more $cc or $ld runs
 
@@ -5959,6 +5974,10 @@ if test "$have_static_assert" = "yes" ; then
   echo "CONFIG_STATIC_ASSERT=y" >> $config_host_mak
 fi
 
+if test "$have_utmpx" = "yes" ; then
+  echo "HAVE_UTMPX=y" >> $config_host_mak
+fi
+
 # Hold two types of flag:
 #   CONFIG_THREAD_SETNAME_BYTHREAD  - we've got a way of setting the name on
 # a thread we have a handle to
diff --git a/qga/commands-posix.c b/qga/commands-posix.c
index d8e4122..e7a047e 100644
--- a/qga/commands-posix.c
+++ b/qga/commands-posix.c
@@ -15,7 +15,6 @@
 #include 
 #include 
 #include 
-#include 
 #include "qga/guest-agent-core.h"
 #include "qga-qmp-commands.h"
 #include "qapi/qmp/qerror.h"
@@ -25,6 +24,10 @@
 #include "qemu/base64.h"
 #include "qemu/cutils.h"
 
+#ifdef HAVE_UTMPX
+#include 
+#endif
+
 #ifndef CONFIG_HAS_ENVIRON
 #ifdef __APPLE__
 #include 
@@ -2519,6 +2522,8 @@ void ga_command_state_init(GAState *s, GACommandState *cs)
 #endif
 }
 
+#ifdef HAVE_UTMPX
+
 #define QGA_MICRO_SECOND_TO_SECOND 100
 
 static double ga_get_login_time(struct utmpx *user_info)
@@ -2577,3 +2582,13 @@ GuestUserList *qmp_guest_get_users(Error **err)
 g_hash_table_destroy(cache);
 return head;
 }
+
+#else
+
+GuestUserList *qmp_guest_get_users(Error **errp)
+{
+error_setg(errp, QERR_UNSUPPORTED);
+return NULL;
+}
+
+#endif
-- 
2.7.4

Re: [Qemu-devel] [PATCH v2 45/45] tcg: enable multiple TCG contexts in softmmu

[Richard Henderson]

On 07/16/2017 10:04 AM, Emilio G. Cota wrote:

+
+/* claim the first free pointer in tcg_ctxs and increment n_tcg_ctxs */
+for (i = 0; i < smp_cpus; i++) {
+if (atomic_cmpxchg(_ctxs[i], NULL, s) == NULL) {
+unsigned int n;
+
+n = atomic_fetch_inc(_tcg_ctxs);


Surely this is too much effort.  The increment on n_tcg_ctxs is sufficient to 
produce an index for assignment.  We never free the contexts...


Which also suggests that it might be better to avoid an indirection in tcg_ctxs 
and allocate all of the structures in one big block?  I.e.


TCGContext *tcg_ctxs;

// At the end of tcg_context_init.
#ifdef CONFIG_USER_ONLY
tcg_ctxs = s;
#else
// No need to zero; we'll completely overwrite each structure
// during tcg_register_thread.
tcg_ctxs = g_new(TCGContext, smp_cpus);
#endif


r~

[Qemu-devel] [PULL 1/8] qga-win: fix installation on localized windows

[Michael Roth]

From: Daniel Rempel 

Bug: https://bugzilla.redhat.com/show_bug.cgi?id=1357789
Replace hardcoded user and group names ("Administrators", "SYSTEM") with the 
ones acquired from system. Windows uses localized strings for these names and 
it may cause the installation to fail.
Windows has Well-known SIDs for "Administrators" group and "SYSTEM" user so 
they were used to identify required users and groups.
Well-known SIDs: 
https://support.microsoft.com/en-us/help/243330/well-known-security-identifiers-in-windows-operating-systems

Signed-off-by: Daniel Rempel 
Signed-off-by: Sameeh Jubran 
Reviewed-by: Sameeh Jubran 
Signed-off-by: Michael Roth 
---
 qga/vss-win32/install.cpp | 35 +--
 1 file changed, 33 insertions(+), 2 deletions(-)

diff --git a/qga/vss-win32/install.cpp b/qga/vss-win32/install.cpp
index f41fcdf..ba7c94e 100644
--- a/qga/vss-win32/install.cpp
+++ b/qga/vss-win32/install.cpp
@@ -18,6 +18,9 @@
 #include 
 #include 
 #include 
+#include 
+
+#define BUFFER_SIZE 1024
 
 extern HINSTANCE g_hinstDll;
 
@@ -135,6 +138,27 @@ out:
 return hr;
 }
 
+/* Acquire group or user name by SID */
+static HRESULT getNameByStringSID(
+const wchar_t *sid, LPWSTR buffer, LPDWORD bufferLen)
+{
+HRESULT hr = S_OK;
+PSID psid = NULL;
+SID_NAME_USE groupType;
+DWORD domainNameLen = BUFFER_SIZE;
+wchar_t domainName[BUFFER_SIZE];
+
+chk(ConvertStringSidToSidW(sid, ));
+LookupAccountSidW(NULL, psid, buffer, bufferLen,
+domainName, , );
+hr = HRESULT_FROM_WIN32(GetLastError());
+
+LocalFree(psid);
+
+out:
+return hr;
+}
+
 /* Find and iterate QGA VSS provider in COM+ Application Catalog */
 static HRESULT QGAProviderFind(
 HRESULT (*found)(ICatalogCollection *, int, void *), void *arg)
@@ -216,6 +240,10 @@ STDAPI COMRegister(void)
 CHAR dllPath[MAX_PATH], tlbPath[MAX_PATH];
 bool unregisterOnFailure = false;
 int count = 0;
+DWORD bufferLen = BUFFER_SIZE;
+wchar_t buffer[BUFFER_SIZE];
+const wchar_t *administratorsGroupSID = L"S-1-5-32-544";
+const wchar_t *systemUserSID = L"S-1-5-18";
 
 if (!g_hinstDll) {
 errmsg(E_FAIL, "Failed to initialize DLL");
@@ -284,11 +312,12 @@ STDAPI COMRegister(void)
 
 /* Setup roles of the applicaion */
 
+chk(getNameByStringSID(administratorsGroupSID, buffer, ));
 chk(pApps->GetCollection(_bstr_t(L"Roles"), key,
  (IDispatch **)pRoles.replace()));
 chk(pRoles->Populate());
 chk(pRoles->Add((IDispatch **)pObj.replace()));
-chk(put_Value(pObj, L"Name",L"Administrators"));
+chk(put_Value(pObj, L"Name", buffer));
 chk(put_Value(pObj, L"Description", L"Administrators group"));
 chk(pRoles->SaveChanges());
 chk(pObj->get_Key());
@@ -303,8 +332,10 @@ STDAPI COMRegister(void)
 chk(GetAdminName());
 chk(put_Value(pObj, L"User", _bstr_t(".\\") + name));
 
+bufferLen = BUFFER_SIZE;
+chk(getNameByStringSID(systemUserSID, buffer, ));
 chk(pUsersInRole->Add((IDispatch **)pObj.replace()));
-chk(put_Value(pObj, L"User", L"SYSTEM"));
+chk(put_Value(pObj, L"User", buffer));
 chk(pUsersInRole->SaveChanges());
 
 out:
-- 
2.7.4

[Qemu-devel] [PULL 7/8] test-qga: pass environemnt to qemu-ga

[Michael Roth]

From: Tomáš Golembiovský 

Modify fixture_setup() to pass environemnt variables to spawned qemu-ga
instance.

Signed-off-by: Tomáš Golembiovský 
Reviewed-by: Marc-André Lureau 
Signed-off-by: Michael Roth 
---
 tests/test-qga.c | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/tests/test-qga.c b/tests/test-qga.c
index c77f241..631b986 100644
--- a/tests/test-qga.c
+++ b/tests/test-qga.c
@@ -46,7 +46,7 @@ static void qga_watch(GPid pid, gint status, gpointer 
user_data)
 }
 
 static void
-fixture_setup(TestFixture *fixture, gconstpointer data)
+fixture_setup(TestFixture *fixture, gconstpointer data, gchar **envp)
 {
 const gchar *extra_arg = data;
 GError *error = NULL;
@@ -67,7 +67,7 @@ fixture_setup(TestFixture *fixture, gconstpointer data)
 g_shell_parse_argv(cmd, NULL, , );
 g_assert_no_error(error);
 
-g_spawn_async(fixture->test_dir, argv, NULL,
+g_spawn_async(fixture->test_dir, argv, envp,
   G_SPAWN_SEARCH_PATH|G_SPAWN_DO_NOT_REAP_CHILD,
   NULL, NULL, >pid, );
 g_assert_no_error(error);
@@ -707,7 +707,7 @@ static void test_qga_blacklist(gconstpointer data)
 QDict *ret, *error;
 const gchar *class, *desc;
 
-fixture_setup(, "-b guest-ping,guest-get-time");
+fixture_setup(, "-b guest-ping,guest-get-time", NULL);
 
 /* check blacklist */
 ret = qmp_fd(fix.fd, "{'execute': 'guest-ping'}");
@@ -943,7 +943,7 @@ int main(int argc, char **argv)
 
 setlocale (LC_ALL, "");
 g_test_init(, , NULL);
-fixture_setup(, NULL);
+fixture_setup(, NULL, NULL);
 
 g_test_add_data_func("/qga/sync-delimited", , test_qga_sync_delimited);
 g_test_add_data_func("/qga/sync", , test_qga_sync);
-- 
2.7.4

[Qemu-devel] [PULL 4/8] qga-win32: remove a redundancy code

[Michael Roth]

From: Peng Hao 

In the first line of run_agent,it has set ga_state = s,don't need
set ga_state = s again behind.

Signed-off-by: Peng Hao 
Signed-off-by: Michael Roth 
---
 qga/main.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/qga/main.c b/qga/main.c
index 405c129..dcd6104 100644
--- a/qga/main.c
+++ b/qga/main.c
@@ -1314,7 +1314,7 @@ static int run_agent(GAState *s, GAConfig *config, int 
socket_activation)
 ga_command_state_init(s, s->command_state);
 ga_command_state_init_all(s->command_state);
 json_message_parser_init(>parser, process_event);
-ga_state = s;
+
 #ifndef _WIN32
 if (!register_signal_handlers()) {
 g_critical("failed to register signal handlers");
-- 
2.7.4

[Qemu-devel] [PULL 0/8] qemu-ga patch queue

[Michael Roth]

The following changes since commit ca4e667dbf431d4a2a5a619cde79d30dd2ac3eb2:

  Merge remote-tracking branch 'remotes/kraxel/tags/usb-20170717-pull-request' 
into staging (2017-07-17 17:54:17 +0100)

are available in the git repository at:

  git://github.com/mdroth/qemu.git tags/qga-pull-2017-07-17-tag

for you to fetch changes up to 48f66332e435ed35c23f3c6ec40b2f1af521758f:

  test-qga: add test for guest-get-osinfo (2017-07-17 19:27:45 -0500)


qemu-ga patch queue
* new command: qemu-get-osinfo
* build fix for OpenBSD
* better error-reporting for failure on keyfile dump
* remove redundant initialization of qa_state global
* include libpcre in w32 package
* w32 localization fixes for service installation/registration


Daniel Rempel (1):
  qga-win: fix installation on localized windows

Marc-André Lureau (1):
  qga: report error on keyfile dump error

Peng Hao (1):
  qga-win32: remove a redundancy code

Thomas Lamprecht (1):
  qemu-ga: add missing libpcre to MSI build

Tomáš Golembiovský (4):
  qemu-ga: check if utmpx.h is available on the system
  qemu-ga: add guest-get-osinfo command
  test-qga: pass environemnt to qemu-ga
  test-qga: add test for guest-get-osinfo

 configure  |  19 +
 qga/commands-posix.c   | 158 +-
 qga/commands-win32.c   | 188 +
 qga/installer/qemu-ga.wxs  |   4 +
 qga/main.c |   9 +-
 qga/qapi-schema.json   |  65 ++
 qga/vss-win32/install.cpp  |  35 +++-
 tests/data/test-qga-os-release |   7 ++
 tests/test-qga.c   |  64 +-
 9 files changed, 540 insertions(+), 9 deletions(-)
 create mode 100644 tests/data/test-qga-os-release

[Qemu-devel] [PULL 5/8] qga: report error on keyfile dump error

[Michael Roth]

From: Marc-André Lureau 

Signed-off-by: Marc-André Lureau 
Cc:qemu-triv...@nongnu.org
Signed-off-by: Michael Roth 
---
 qga/main.c | 7 ++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/qga/main.c b/qga/main.c
index dcd6104..1b381d0 100644
--- a/qga/main.c
+++ b/qga/main.c
@@ -1074,7 +1074,12 @@ static void config_dump(GAConfig *config)
 g_free(tmp);
 
 tmp = g_key_file_to_data(keyfile, NULL, );
-printf("%s", tmp);
+if (error) {
+g_critical("Failed to dump keyfile: %s", error->message);
+g_clear_error();
+} else {
+printf("%s", tmp);
+}
 
 g_free(tmp);
 g_key_file_free(keyfile);
-- 
2.7.4

[Qemu-devel] [PULL 2/8] qemu-ga: add missing libpcre to MSI build

[Michael Roth]

From: Thomas Lamprecht 

glib depends on libpcre which was not shipped with the MSI, thus
starting of the qemu-ga.exe failed with the respective error message.

Tell WIXL to ship this library with the MSI to avoid this problem.

Signed-off-by: Thomas Lamprecht 
CC: Stefan Weil 
CC: Michael Roth 
Reviewed-by: Marc-André Lureau 
Signed-off-by: Michael Roth 
---
 qga/installer/qemu-ga.wxs | 4 
 1 file changed, 4 insertions(+)

diff --git a/qga/installer/qemu-ga.wxs b/qga/installer/qemu-ga.wxs
index fa2260c..5af1162 100644
--- a/qga/installer/qemu-ga.wxs
+++ b/qga/installer/qemu-ga.wxs
@@ -125,6 +125,9 @@
   
 
   
+  
+
+  
   
 
@@ -173,6 +176,7 @@
   
   
   
+  
 
 
 
-- 
2.7.4

[Qemu-devel] [PULL 8/8] test-qga: add test for guest-get-osinfo

[Michael Roth]

From: Tomáš Golembiovský 

Add test for guest-get-osinfo command.

Qemu-ga was modified to accept QGA_OS_RELEASE environment variable. If
the variable is defined it is interpreted as path to the os-release file
and it is parsed instead of the default paths.

Signed-off-by: Tomáš Golembiovský 
Reviewed-by: Marc-André Lureau 
Signed-off-by: Michael Roth 
---
 qga/commands-posix.c   | 13 +++---
 tests/data/test-qga-os-release |  7 ++
 tests/test-qga.c   | 56 ++
 3 files changed, 73 insertions(+), 3 deletions(-)
 create mode 100644 tests/data/test-qga-os-release

diff --git a/qga/commands-posix.c b/qga/commands-posix.c
index 672bf29..86c20e9 100644
--- a/qga/commands-posix.c
+++ b/qga/commands-posix.c
@@ -2682,6 +2682,8 @@ GuestOSInfo *qmp_guest_get_osinfo(Error **errp)
 {
 GuestOSInfo *info = NULL;
 struct utsname kinfo = {0};
+GKeyFile *osrelease = NULL;
+
 
 info = g_new0(GuestOSInfo, 1);
 
@@ -2696,9 +2698,14 @@ GuestOSInfo *qmp_guest_get_osinfo(Error **errp)
 info->machine = g_strdup(kinfo.machine);
 }
 
-GKeyFile *osrelease = ga_parse_osrelease("/etc/os-release");
-if (osrelease == NULL) {
-osrelease = ga_parse_osrelease("/usr/lib/os-release");
+const char *qga_os_release = g_getenv("QGA_OS_RELEASE");
+if (qga_os_release != NULL) {
+osrelease = ga_parse_osrelease(qga_os_release);
+} else {
+osrelease = ga_parse_osrelease("/etc/os-release");
+if (osrelease == NULL) {
+osrelease = ga_parse_osrelease("/usr/lib/os-release");
+}
 }
 
 if (osrelease != NULL) {
diff --git a/tests/data/test-qga-os-release b/tests/data/test-qga-os-release
new file mode 100644
index 000..70664eb
--- /dev/null
+++ b/tests/data/test-qga-os-release
@@ -0,0 +1,7 @@
+ID=qemu-ga-test
+NAME=QEMU-GA
+PRETTY_NAME="QEMU Guest Agent test"
+VERSION="Test 1"
+VERSION_ID=1
+VARIANT="Unit test \"\'\$\`\\ and  etc."
+VARIANT_ID=unit-test
diff --git a/tests/test-qga.c b/tests/test-qga.c
index 631b986..06783e7 100644
--- a/tests/test-qga.c
+++ b/tests/test-qga.c
@@ -936,6 +936,60 @@ static void test_qga_guest_exec_invalid(gconstpointer fix)
 QDECREF(ret);
 }
 
+static void test_qga_guest_get_osinfo(gconstpointer data)
+{
+TestFixture fixture;
+const gchar *str;
+gchar *cwd, *env[2];
+QDict *ret, *val;
+
+cwd = g_get_current_dir();
+env[0] = g_strdup_printf(
+"QGA_OS_RELEASE=%s%ctests%cdata%ctest-qga-os-release",
+cwd, G_DIR_SEPARATOR, G_DIR_SEPARATOR, G_DIR_SEPARATOR);
+env[1] = NULL;
+g_free(cwd);
+fixture_setup(, NULL, env);
+
+ret = qmp_fd(fixture.fd, "{'execute': 'guest-get-osinfo'}");
+g_assert_nonnull(ret);
+qmp_assert_no_error(ret);
+
+val = qdict_get_qdict(ret, "return");
+
+str = qdict_get_try_str(val, "id");
+g_assert_nonnull(str);
+g_assert_cmpstr(str, ==, "qemu-ga-test");
+
+str = qdict_get_try_str(val, "name");
+g_assert_nonnull(str);
+g_assert_cmpstr(str, ==, "QEMU-GA");
+
+str = qdict_get_try_str(val, "pretty-name");
+g_assert_nonnull(str);
+g_assert_cmpstr(str, ==, "QEMU Guest Agent test");
+
+str = qdict_get_try_str(val, "version");
+g_assert_nonnull(str);
+g_assert_cmpstr(str, ==, "Test 1");
+
+str = qdict_get_try_str(val, "version-id");
+g_assert_nonnull(str);
+g_assert_cmpstr(str, ==, "1");
+
+str = qdict_get_try_str(val, "variant");
+g_assert_nonnull(str);
+g_assert_cmpstr(str, ==, "Unit test \"'$`\\ and  etc.");
+
+str = qdict_get_try_str(val, "variant-id");
+g_assert_nonnull(str);
+g_assert_cmpstr(str, ==, "unit-test");
+
+QDECREF(ret);
+g_free(env[0]);
+fixture_tear_down(, NULL);
+}
+
 int main(int argc, char **argv)
 {
 TestFixture fix;
@@ -972,6 +1026,8 @@ int main(int argc, char **argv)
 g_test_add_data_func("/qga/guest-exec", , test_qga_guest_exec);
 g_test_add_data_func("/qga/guest-exec-invalid", ,
  test_qga_guest_exec_invalid);
+g_test_add_data_func("/qga/guest-get-osinfo", ,
+ test_qga_guest_get_osinfo);
 
 if (g_getenv("QGA_TEST_SIDE_EFFECTING")) {
 g_test_add_data_func("/qga/fsfreeze-and-thaw", ,
-- 
2.7.4

Re: [Qemu-devel] [PULL 6/9] Convert error_report() to warn_report()

[Markus Armbruster]

Kevin Wolf  writes:

> Am 13.07.2017 um 15:27 hat Markus Armbruster geschrieben:
>> From: Alistair Francis 
>> 
>> Convert all uses of error_report("warning:"... to use warn_report()
>> instead. This helps standardise on a single method of printing warnings
>> to the user.
>> 
>> All of the warnings were changed using these two commands:
>> find ./* -type f -exec sed -i \
>>   's|error_report(".*warning[,:] |warn_report("|Ig' {} +
>> 
>> Indentation fixed up manually afterwards.
>> 
>> The test-qdev-global-props test case was manually updated to ensure that
>> this patch passes make check (as the test cases are case sensitive).
>
> This patch broke qemu-iotests 051 because it neglected to update the
> reference output. Not sure if a change of the message was even intended,
> but with a error location prefix, the order changes:
>
> -(qemu) QEMU_PROG: -drive if=scsi,media=cdrom: warning: bus=0,unit=0 is 
> deprecated with this machine type
> +(qemu) warning: qemu-system-x86_64: -drive if=scsi,media=cdrom: bus=0,unit=0 
> is deprecated with this machine type
>
> Personally, I would expect the error location or at least the program
> name to come first even for warnings.

I'll fix it.

While focusing on something other than block, I forget qemu-iotests
exist.  My fault, but it's a pretty common fault.  I reiterate my plea
to include (a sensible subset of) it in "make check".

Re: [Qemu-devel] [PATCH v2 43/45] tcg: introduce regions to split code_gen_buffer

[Richard Henderson]

On 07/16/2017 10:04 AM, Emilio G. Cota wrote:

+#ifdef CONFIG_SOFTMMU
+/*
+ * It is likely that some vCPUs will translate more code than others, so we
+ * first try to set more regions than smp_cpus, with those regions being
+ * larger than the minimum code_gen_buffer size. If that's not possible we
+ * make do by evenly dividing the code_gen_buffer among the vCPUs.
+ */
+void softmmu_tcg_region_init(void)
+{
+size_t i;
+
+/* Use a single region if all we have is one vCPU thread */
+if (smp_cpus == 1 || !qemu_tcg_mttcg_enabled()) {
+tcg_region_init(0);
+return;
+}
+
+for (i = 8; i > 0; i--) {
+size_t regions_per_thread = i;
+size_t region_size;
+
+region_size = tcg_init_ctx.code_gen_buffer_size;
+region_size /= smp_cpus * regions_per_thread;
+
+if (region_size >= 2 * MIN_CODE_GEN_BUFFER_SIZE) {
+tcg_region_init(smp_cpus * regions_per_thread);
+return;
+}
+}
+tcg_region_init(smp_cpus);
+}
+#endif


Any reason this code wouldn't just live in tcg_region_init?
It would certainly simplify the interface.

In particular it appears to be a mistake to ever call with n_regions == 0, 
since it's just as easy to call with n_regions == 1.



diff --git a/cpus.c b/cpus.c
index 14bb8d5..5455819 100644
--- a/cpus.c
+++ b/cpus.c
@@ -1664,6 +1664,18 @@ static void qemu_tcg_init_vcpu(CPUState *cpu)
  char thread_name[VCPU_THREAD_NAME_SIZE];
  static QemuCond *single_tcg_halt_cond;
  static QemuThread *single_tcg_cpu_thread;
+static int tcg_region_inited;
+
+/*
+ * Initialize TCG regions--once, of course. Now is a good time, because:
+ * (1) TCG's init context, prologue and target globals have been set up.
+ * (2) qemu_tcg_mttcg_enabled() works now (TCG init code runs before the
+ * -accel flag is processed, so the check doesn't work then).
+ */
+if (!tcg_region_inited) {
+softmmu_tcg_region_init();
+tcg_region_inited = 1;
+}


Nit: Do not require the compiler to hold the address of the global across 
another call, or recompute it.  Generically this pattern is better as


  if (!flag) {
flag = true;
do_init();
  }

unless there's some compelling threaded reason to delay the set of the flag. 
Which is not the case here.



+/* Call from a safe-work context */
+void tcg_region_reset_all(void)
+{
+unsigned int i;
+
+qemu_mutex_lock();
+region.current = 0;
+region.n_full = 0;
+
+for (i = 0; i < n_tcg_ctxs; i++) {
+if (unlikely(tcg_region_initial_alloc__locked(tcg_ctxs[i]))) {
+tcg_abort();
+}


Nit: I prefer

  bool ok = foo();
  assert(ok);

over

  if (!foo())
abort();


+static void tcg_region_set_guard_pages(void)
+{
+size_t guard_size = qemu_real_host_page_size;
+size_t i;
+
+for (i = 0; i < region.n; i++) {
+void *guard = region.buf + region.size + i * (region.size + 
guard_size);
+
+if (qemu_mprotect_none(guard, qemu_real_host_page_size)) {


If you're going to have the local variable at all, guard_size here too.


+tcg_abort();
+}
+}
+}
+
+/*
+ * Initializes region partitioning, setting the number of regions via
+ * @n_regions.
+ * Set @n_regions to 0 or 1 to use a single region that uses all of
+ * code_gen_buffer.
+ *
+ * Called at init time from the parent thread (i.e. the one calling
+ * tcg_context_init), after the target's TCG globals have been set.
+ *
+ * Region partitioning works by splitting code_gen_buffer into separate 
regions,
+ * and then assigning regions to TCG threads so that the threads can translate
+ * code in parallel without synchronization.
+ */
+void tcg_region_init(size_t n_regions)
+{
+void *buf = tcg_init_ctx.code_gen_buffer;
+size_t size = tcg_init_ctx.code_gen_buffer_size;
+
+if (!n_regions) {
+n_regions = 1;
+}
+
+/* start on a page-aligned address */
+buf = QEMU_ALIGN_PTR_UP(buf, qemu_real_host_page_size);
+if (unlikely(buf > tcg_init_ctx.code_gen_buffer + size)) {
+tcg_abort();
+}


assert.


+/* discard that initial portion */
+size -= buf - tcg_init_ctx.code_gen_buffer;
+
+/* make region.size a multiple of page_size */
+region.size = size / n_regions;
+region.size &= qemu_real_host_page_mask;


QEMU_ALIGN_DOWN.


+
+/* A region must have at least 2 pages; one code, one guard */
+if (unlikely(region.size < 2 * qemu_real_host_page_size)) {
+tcg_abort();
+}


assert.


+
+/* do not count the guard page in region.size */
+region.size -= qemu_real_host_page_size;
+region.n = n_regions;
+region.buf = buf;
+tcg_region_set_guard_pages();


I think it would be clearer to inline the subroutine.  I was asking myself why 
we weren't subtracting the guard_size from region->size.



r~

[Qemu-devel] [PATCH v5 10/10] tcg/tci: enable bswap16_i64

[Philippe Mathieu-Daudé]

Altough correctly implemented, bswap16_i64() never got tested/executed so the
safety TODO() statement was never removed.

Since it got now tested the TODO() can be removed.

while running Alex Bennée's image aarch64-linux-3.15rc2-buildroot.img:

Trace 0x7fa1904b0890 [0: ffc00036cd04]

IN:
0xffc00036cd24:  5ac00694  rev16 w20, w20

OP:
  ffc00036cd24  
 ext32u_i64 tmp3,x20
 ext16u_i64 tmp2,tmp3
 bswap16_i64 x20,tmp2
 movi_i64 tmp4,$0x10
 shr_i64 tmp2,tmp3,tmp4
 ext16u_i64 tmp2,tmp2
 bswap16_i64 tmp2,tmp2
 deposit_i64 x20,x20,tmp2,$0x10,$0x10

Linking TBs 0x7fa1904b0890 [ffc00036cd04] index 0 -> 0x7fa1904b0aa0 
[ffc00036cd24]
Trace 0x7fa1904b0aa0 [0: ffc00036cd24]
TODO qemu/tci.c:1049: tcg_qemu_tb_exec()
qemu/tci.c:1049: tcg fatal error
Aborted

Signed-off-by: Philippe Mathieu-Daudé 
Signed-off-by: Jaroslaw Pelczar 
Reviewed-by: Alex Bennée 
Reviewed-by: Eric Blake 
Reviewed-by: Stefan Weil 
---
 tcg/tci.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/tcg/tci.c b/tcg/tci.c
index 4bdc645f2a..f39bfb95c0 100644
--- a/tcg/tci.c
+++ b/tcg/tci.c
@@ -1046,7 +1046,6 @@ uintptr_t tcg_qemu_tb_exec(CPUArchState *env, uint8_t 
*tb_ptr)
 break;
 #if TCG_TARGET_HAS_bswap16_i64
 case INDEX_op_bswap16_i64:
-TODO();
 t0 = *tb_ptr++;
 t1 = tci_read_r16(_ptr);
 tci_write_reg64(t0, bswap16(t1));
-- 
2.13.2

Re: [Qemu-devel] [PATCH v2 40/45] osdep: introduce qemu_mprotect_rwx/none

[Emilio G. Cota]

On Mon, Jul 17, 2017 at 18:26:09 -1000, Richard Henderson wrote:
> On 07/16/2017 10:04 AM, Emilio G. Cota wrote:
> >+static int qemu_mprotect__osdep(void *addr, size_t size, int prot)
> >+{
> >+void *start = QEMU_ALIGN_PTR_DOWN(addr, qemu_real_host_page_size);
> >+void *end = QEMU_ALIGN_PTR_UP(addr + size, qemu_real_host_page_size);
> 
> I'm not keen on this.  Any good reason for it as opposed to asserting that
> the inputs are already page aligned?

No particular reason other than "kept the same behaviour we had".

Let's go with asserts, I like that approach much better actually.

E.

[Qemu-devel] [PATCH v5 06/10] target/sparc: optimize various functions using extract op

[Philippe Mathieu-Daudé]

Done with the Coccinelle semantic patch from commit 58daf05d07dd
(see scripts/coccinelle/tcg_gen_extract.cocci)

Signed-off-by: Philippe Mathieu-Daudé 
Reviewed-by: Richard Henderson 
---
Richard: maybe you need to update 58daf05d07dd to your commit...

 target/sparc/translate.c | 12 
 1 file changed, 4 insertions(+), 8 deletions(-)

diff --git a/target/sparc/translate.c b/target/sparc/translate.c
index aa6734d54e..962ce08f80 100644
--- a/target/sparc/translate.c
+++ b/target/sparc/translate.c
@@ -380,29 +380,25 @@ static inline void gen_goto_tb(DisasContext *s, int 
tb_num,
 static inline void gen_mov_reg_N(TCGv reg, TCGv_i32 src)
 {
 tcg_gen_extu_i32_tl(reg, src);
-tcg_gen_shri_tl(reg, reg, PSR_NEG_SHIFT);
-tcg_gen_andi_tl(reg, reg, 0x1);
+tcg_gen_extract_tl(reg, reg, PSR_NEG_SHIFT, 1);
 }
 
 static inline void gen_mov_reg_Z(TCGv reg, TCGv_i32 src)
 {
 tcg_gen_extu_i32_tl(reg, src);
-tcg_gen_shri_tl(reg, reg, PSR_ZERO_SHIFT);
-tcg_gen_andi_tl(reg, reg, 0x1);
+tcg_gen_extract_tl(reg, reg, PSR_ZERO_SHIFT, 1);
 }
 
 static inline void gen_mov_reg_V(TCGv reg, TCGv_i32 src)
 {
 tcg_gen_extu_i32_tl(reg, src);
-tcg_gen_shri_tl(reg, reg, PSR_OVF_SHIFT);
-tcg_gen_andi_tl(reg, reg, 0x1);
+tcg_gen_extract_tl(reg, reg, PSR_OVF_SHIFT, 1);
 }
 
 static inline void gen_mov_reg_C(TCGv reg, TCGv_i32 src)
 {
 tcg_gen_extu_i32_tl(reg, src);
-tcg_gen_shri_tl(reg, reg, PSR_CARRY_SHIFT);
-tcg_gen_andi_tl(reg, reg, 0x1);
+tcg_gen_extract_tl(reg, reg, PSR_CARRY_SHIFT, 1);
 }
 
 static inline void gen_op_add_cc(TCGv dst, TCGv src1, TCGv src2)
-- 
2.13.2

[Qemu-devel] [PATCH v5 08/10] target/sparc: optimize gen_op_mulscc() using deposit op

[Philippe Mathieu-Daudé]

Suggested-by: Richard Henderson 
Signed-off-by: Philippe Mathieu-Daudé 
---
 target/sparc/translate.c | 5 +
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/target/sparc/translate.c b/target/sparc/translate.c
index 67a83b77cc..a425efb1f1 100644
--- a/target/sparc/translate.c
+++ b/target/sparc/translate.c
@@ -632,11 +632,8 @@ static inline void gen_op_mulscc(TCGv dst, TCGv src1, TCGv 
src2)
 
 // b2 = T0 & 1;
 // env->y = (b2 << 31) | (env->y >> 1);
-tcg_gen_andi_tl(r_temp, cpu_cc_src, 0x1);
-tcg_gen_shli_tl(r_temp, r_temp, 31);
 tcg_gen_extract_tl(t0, cpu_y, 1, 31);
-tcg_gen_or_tl(t0, t0, r_temp);
-tcg_gen_andi_tl(cpu_y, t0, 0x);
+tcg_gen_deposit_tl(cpu_y, cpu_y, cpu_cc_src, 31, 1);
 
 // b1 = N ^ V;
 gen_mov_reg_N(t0, cpu_psr);
-- 
2.13.2

[Qemu-devel] [PATCH v5 09/10] target/alpha: optimize gen_cvtlq() using deposit op

[Philippe Mathieu-Daudé]

Suggested-by: Richard Henderson 
Signed-off-by: Philippe Mathieu-Daudé 
---
 target/alpha/translate.c | 8 +++-
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/target/alpha/translate.c b/target/alpha/translate.c
index 232af9e177..2bffbae92f 100644
--- a/target/alpha/translate.c
+++ b/target/alpha/translate.c
@@ -756,11 +756,9 @@ static void gen_cvtlq(TCGv vc, TCGv vb)
 
 /* The arithmetic right shift here, plus the sign-extended mask below
yields a sign-extended result without an explicit ext32s_i64.  */
-tcg_gen_sari_i64(tmp, vb, 32);
-tcg_gen_shri_i64(vc, vb, 29);
-tcg_gen_andi_i64(tmp, tmp, (int32_t)0xc000);
-tcg_gen_andi_i64(vc, vc, 0x3fff);
-tcg_gen_or_i64(vc, vc, tmp);
+tcg_gen_shri_i64(tmp, vb, 29);
+tcg_gen_sari_i64(vc, vb, 32);
+tcg_gen_deposit_i64(vc, vc, tmp, 0, 30);
 
 tcg_temp_free(tmp);
 }
-- 
2.13.2

[Qemu-devel] [PATCH v5 05/10] target/ppc: optimize various functions using extract op

[Philippe Mathieu-Daudé]

Done with the Coccinelle semantic patch from commit 58daf05d07dd
(see scripts/coccinelle/tcg_gen_extract.cocci)

Signed-off-by: Philippe Mathieu-Daudé 
Reviewed-by: Richard Henderson 
Acked-by: David Gibson 
---
Richard: maybe you need to update 58daf05d07dd to your commit...

 target/ppc/translate.c  | 21 +++--
 target/ppc/translate/vsx-impl.inc.c | 24 
 2 files changed, 15 insertions(+), 30 deletions(-)

diff --git a/target/ppc/translate.c b/target/ppc/translate.c
index c0cd64d927..de271af52b 100644
--- a/target/ppc/translate.c
+++ b/target/ppc/translate.c
@@ -873,8 +873,7 @@ static inline void gen_op_arith_add(DisasContext *ctx, TCGv 
ret, TCGv arg1,
 }
 tcg_gen_xor_tl(cpu_ca, t0, t1);/* bits changed w/ carry */
 tcg_temp_free(t1);
-tcg_gen_shri_tl(cpu_ca, cpu_ca, 32);   /* extract bit 32 */
-tcg_gen_andi_tl(cpu_ca, cpu_ca, 1);
+tcg_gen_extract_tl(cpu_ca, cpu_ca, 32, 1);
 if (is_isa300(ctx)) {
 tcg_gen_mov_tl(cpu_ca32, cpu_ca);
 }
@@ -1404,8 +1403,7 @@ static inline void gen_op_arith_subf(DisasContext *ctx, 
TCGv ret, TCGv arg1,
 tcg_temp_free(inv1);
 tcg_gen_xor_tl(cpu_ca, t0, t1); /* bits changes w/ carry */
 tcg_temp_free(t1);
-tcg_gen_shri_tl(cpu_ca, cpu_ca, 32);/* extract bit 32 */
-tcg_gen_andi_tl(cpu_ca, cpu_ca, 1);
+tcg_gen_extract_tl(cpu_ca, cpu_ca, 32, 1);
 if (is_isa300(ctx)) {
 tcg_gen_mov_tl(cpu_ca32, cpu_ca);
 }
@@ -4336,8 +4334,7 @@ static void gen_mfsrin(DisasContext *ctx)
 
 CHK_SV;
 t0 = tcg_temp_new();
-tcg_gen_shri_tl(t0, cpu_gpr[rB(ctx->opcode)], 28);
-tcg_gen_andi_tl(t0, t0, 0xF);
+tcg_gen_extract_tl(t0, cpu_gpr[rB(ctx->opcode)], 28, 4);
 gen_helper_load_sr(cpu_gpr[rD(ctx->opcode)], cpu_env, t0);
 tcg_temp_free(t0);
 #endif /* defined(CONFIG_USER_ONLY) */
@@ -4368,8 +4365,7 @@ static void gen_mtsrin(DisasContext *ctx)
 CHK_SV;
 
 t0 = tcg_temp_new();
-tcg_gen_shri_tl(t0, cpu_gpr[rB(ctx->opcode)], 28);
-tcg_gen_andi_tl(t0, t0, 0xF);
+tcg_gen_extract_tl(t0, cpu_gpr[rB(ctx->opcode)], 28, 4);
 gen_helper_store_sr(cpu_env, t0, cpu_gpr[rD(ctx->opcode)]);
 tcg_temp_free(t0);
 #endif /* defined(CONFIG_USER_ONLY) */
@@ -4403,8 +4399,7 @@ static void gen_mfsrin_64b(DisasContext *ctx)
 
 CHK_SV;
 t0 = tcg_temp_new();
-tcg_gen_shri_tl(t0, cpu_gpr[rB(ctx->opcode)], 28);
-tcg_gen_andi_tl(t0, t0, 0xF);
+tcg_gen_extract_tl(t0, cpu_gpr[rB(ctx->opcode)], 28, 4);
 gen_helper_load_sr(cpu_gpr[rD(ctx->opcode)], cpu_env, t0);
 tcg_temp_free(t0);
 #endif /* defined(CONFIG_USER_ONLY) */
@@ -4435,8 +4430,7 @@ static void gen_mtsrin_64b(DisasContext *ctx)
 
 CHK_SV;
 t0 = tcg_temp_new();
-tcg_gen_shri_tl(t0, cpu_gpr[rB(ctx->opcode)], 28);
-tcg_gen_andi_tl(t0, t0, 0xF);
+tcg_gen_extract_tl(t0, cpu_gpr[rB(ctx->opcode)], 28, 4);
 gen_helper_store_sr(cpu_env, t0, cpu_gpr[rS(ctx->opcode)]);
 tcg_temp_free(t0);
 #endif /* defined(CONFIG_USER_ONLY) */
@@ -5414,8 +5408,7 @@ static void gen_mfsri(DisasContext *ctx)
 CHK_SV;
 t0 = tcg_temp_new();
 gen_addr_reg_index(ctx, t0);
-tcg_gen_shri_tl(t0, t0, 28);
-tcg_gen_andi_tl(t0, t0, 0xF);
+tcg_gen_extract_tl(t0, t0, 28, 4);
 gen_helper_load_sr(cpu_gpr[rd], cpu_env, t0);
 tcg_temp_free(t0);
 if (ra != 0 && ra != rd)
diff --git a/target/ppc/translate/vsx-impl.inc.c 
b/target/ppc/translate/vsx-impl.inc.c
index 7f12908029..85ed135d44 100644
--- a/target/ppc/translate/vsx-impl.inc.c
+++ b/target/ppc/translate/vsx-impl.inc.c
@@ -1248,8 +1248,7 @@ static void gen_xsxexpdp(DisasContext *ctx)
 gen_exception(ctx, POWERPC_EXCP_VSXU);
 return;
 }
-tcg_gen_shri_i64(rt, cpu_vsrh(xB(ctx->opcode)), 52);
-tcg_gen_andi_i64(rt, rt, 0x7FF);
+tcg_gen_extract_i64(rt, cpu_vsrh(xB(ctx->opcode)), 52, 11);
 }
 
 static void gen_xsxexpqp(DisasContext *ctx)
@@ -1262,8 +1261,7 @@ static void gen_xsxexpqp(DisasContext *ctx)
 gen_exception(ctx, POWERPC_EXCP_VSXU);
 return;
 }
-tcg_gen_shri_i64(xth, xbh, 48);
-tcg_gen_andi_i64(xth, xth, 0x7FFF);
+tcg_gen_extract_i64(xth, xbh, 48, 15);
 tcg_gen_movi_i64(xtl, 0);
 }
 
@@ -1323,8 +1321,7 @@ static void gen_xsxsigdp(DisasContext *ctx)
 zr = tcg_const_i64(0);
 nan = tcg_const_i64(2047);
 
-tcg_gen_shri_i64(exp, cpu_vsrh(xB(ctx->opcode)), 52);
-tcg_gen_andi_i64(exp, exp, 0x7FF);
+tcg_gen_extract_i64(exp, cpu_vsrh(xB(ctx->opcode)), 52, 11);
 tcg_gen_movi_i64(t0, 0x0010);
 tcg_gen_movcond_i64(TCG_COND_EQ, t0, exp, zr, zr, t0);
 tcg_gen_movcond_i64(TCG_COND_EQ, t0, exp, nan, zr, t0);
@@ -1352,8 +1349,7 @@ static void gen_xsxsigqp(DisasContext *ctx)

[Qemu-devel] [PATCH v5 07/10] target/sparc: optimize gen_op_mulscc() using extract op

[Philippe Mathieu-Daudé]

Done with the Coccinelle semantic patch from commit 58daf05d07dd
(see scripts/coccinelle/tcg_gen_extract.cocci)

Signed-off-by: Philippe Mathieu-Daudé 
---

Richard: are you ok squashing it with previous commit?
 maybe you need to update 58daf05d07dd to your commit...

 target/sparc/translate.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/target/sparc/translate.c b/target/sparc/translate.c
index 962ce08f80..67a83b77cc 100644
--- a/target/sparc/translate.c
+++ b/target/sparc/translate.c
@@ -634,8 +634,7 @@ static inline void gen_op_mulscc(TCGv dst, TCGv src1, TCGv 
src2)
 // env->y = (b2 << 31) | (env->y >> 1);
 tcg_gen_andi_tl(r_temp, cpu_cc_src, 0x1);
 tcg_gen_shli_tl(r_temp, r_temp, 31);
-tcg_gen_shri_tl(t0, cpu_y, 1);
-tcg_gen_andi_tl(t0, t0, 0x7fff);
+tcg_gen_extract_tl(t0, cpu_y, 1, 31);
 tcg_gen_or_tl(t0, t0, r_temp);
 tcg_gen_andi_tl(cpu_y, t0, 0x);
 
-- 
2.13.2

[Qemu-devel] [PATCH v5 02/10] coccinelle: add a script to optimize tcg op using tcg_gen_extract()

[Philippe Mathieu-Daudé]

The following thread was helpful while writing this script:

https://github.com/coccinelle/coccinelle/issues/86

Signed-off-by: Philippe Mathieu-Daudé 
---
 scripts/coccinelle/tcg_gen_extract.cocci | 107 +++
 1 file changed, 107 insertions(+)
 create mode 100644 scripts/coccinelle/tcg_gen_extract.cocci

diff --git a/scripts/coccinelle/tcg_gen_extract.cocci 
b/scripts/coccinelle/tcg_gen_extract.cocci
new file mode 100644
index 00..81e66a35ae
--- /dev/null
+++ b/scripts/coccinelle/tcg_gen_extract.cocci
@@ -0,0 +1,107 @@
+// optimize TCG using extract op
+//
+// Copyright: (C) 2017 Philippe Mathieu-Daudé. GPLv2+.
+// Confidence: High
+// Options: --macro-file scripts/cocci-macro-file.h
+//
+// Nikunj A Dadhania optimization:
+// http://lists.nongnu.org/archive/html/qemu-devel/2017-02/msg05211.html
+// Aurelien Jarno optimization:
+// http://lists.nongnu.org/archive/html/qemu-devel/2017-05/msg01466.html
+//
+// This script can be run either using spatch locally or via a docker image:
+//
+// $ spatch \
+// --macro-file scripts/cocci-macro-file.h \
+// --sp-file scripts/coccinelle/tcg_gen_extract.cocci \
+// --keep-comments --in-place \
+// --use-gitgrep --dir target
+//
+// $ docker run --rm -v `pwd`:`pwd` -w `pwd` philmd/coccinelle \
+// --macro-file scripts/cocci-macro-file.h \
+// --sp-file scripts/coccinelle/tcg_gen_extract.cocci \
+// --keep-comments --in-place \
+// --use-gitgrep --dir target
+
+@initialize:python@
+@@
+import sys
+fd = sys.stderr
+def debug(msg="", trailer="\n"):
+fd.write("[DBG] " + msg + trailer)
+def low_bits_count(value):
+bits_count = 0
+while (value & (1 << bits_count)):
+bits_count += 1
+return bits_count
+def Mn(order): # Mersenne number
+return (1 << order) - 1
+
+@match@
+identifier ret;
+metavariable arg;
+constant ofs, msk;
+position shr_p, and_p;
+@@
+(
+tcg_gen_shri_i32@shr_p
+|
+tcg_gen_shri_i64@shr_p
+|
+tcg_gen_shri_tl@shr_p
+)(ret, arg, ofs);
+...  WHEN != ret
+(
+tcg_gen_andi_i32@and_p
+|
+tcg_gen_andi_i64@and_p
+|
+tcg_gen_andi_tl@and_p
+)(ret, ret, msk);
+
+@script:python verify_len depends on match@
+ret_s << match.ret;
+msk_s << match.msk;
+shr_p << match.shr_p;
+extract_len;
+@@
+is_optimizable = False
+debug("candidate at %s:%s" % (shr_p[0].file, shr_p[0].line))
+try: # only eval integer, no #define like 'SR_M' (cpp did this, else some 
headers are missing).
+msk_v = long(msk_s.strip("UL"), 0)
+msk_b = low_bits_count(msk_v)
+if msk_b == 0:
+debug("  value: 0x%x low_bits: %d" % (msk_v, msk_b))
+else:
+debug("  value: 0x%x low_bits: %d [Mersenne number: 0x%x]" % (msk_v, 
msk_b, Mn(msk_b)))
+is_optimizable = Mn(msk_b) == msk_v # check low_bits
+coccinelle.extract_len = "%d" % msk_b
+debug("  candidate %s optimizable" % ("IS" if is_optimizable else "is 
NOT"))
+except:
+debug("  ERROR (check included headers?)")
+cocci.include_match(is_optimizable)
+debug()
+
+@replacement depends on verify_len@
+identifier match.ret;
+metavariable match.arg;
+constant match.ofs, match.msk;
+position match.shr_p, match.and_p;
+identifier verify_len.extract_len;
+@@
+(
+-tcg_gen_shri_i32@shr_p(ret, arg, ofs);
++tcg_gen_extract_i32(ret, arg, ofs, extract_len);
+...  WHEN != ret
+-tcg_gen_andi_i32@and_p(ret, ret, msk);
+|
+-tcg_gen_shri_i64@shr_p(ret, arg, ofs);
++tcg_gen_extract_i64(ret, arg, ofs, extract_len);
+...  WHEN != ret
+-tcg_gen_andi_i64@and_p(ret, ret, msk);
+|
+-tcg_gen_shri_tl@shr_p(ret, arg, ofs);
++tcg_gen_extract_tl(ret, arg, ofs, extract_len);
+...  WHEN != ret
+-tcg_gen_andi_tl@and_p(ret, ret, msk);
+)
-- 
2.13.2

[Qemu-devel] [PATCH v5 00/10] optimize various tcg_gen() functions using extract/deposit op

[Philippe Mathieu-Daudé]

Hi Richard,

Please find here the update series. Maybe you'll need to update the commit sha-1
58daf05d07dd in commits 3-8.

Regards,

Phil.

[v5]

- gitignore entries for cocci generated files
- cleaned/improved cocci script, updated usage
- fix output using Mersenne "number" instead of "prime" (Eric Blake)
- use deposit() on alpha and sparc (Richard Henderson)
- enable tci bswap16_i64()

[v4]

Tried to fix wrong previous attempt...
After getting some nice/fast pieces of advice from Coccinelle folks, I tried to
improved the script (not much inline documentation yet although).
- correctly check if this optimizable?
- document as Mersenne number instead of prime (Eric Blake)
- try to write Python code instead of BASIC (Markus Elfring advices)
- try to reduce regex usage
- try to match shri(); unrelated(); andi(); pattern to optimize, I was surprised
  to see the alpha diff Coccinelle found.

This is surely not the last version of this patchset, but I think now the
generated patches are correct and I prefer reviewers to look at them fixed
instead of wrong one in the ML.
Still lot of work to do in the cocci script, now it seems to hang trying to
parse "target/arm/translate.c".

[v3]

In my first attempt I misunderstood tcg_gen_extract() intrinsics, and Richard
Henderson pointed that out.
In this patchset the cocci script is corrected and clarified, it also print how
arguments are checked while running.
Also:
- incorrect patches have been removed. (Richard Henderson, Nikunj A Dadhania)
- Coccinelle script licensed GPLv2+ (Eric Blake)
- comment in each commit about how to apply the patch (Eric Blake)
- added Acked-by for m68k (Laurent Vivier)
- Cc: Coccinelle developers.

[v2]

Resent the cocci script.
 
[v1]

While reviewing a commit from Aurelien Jarno where he optimized a TCG generator
for SH-4 [1] I found the same optimization done on PPC by Nikunj A Dadhania few
months ago [2].
After asking on the ML about a cocci script [3] I thought it would be easier to
learn about Coccinelle.

citing Aurelien Jarno:
This doesn't change the generated code on x86, but optimizes it on most
RISC architectures and makes the code simpler to read.

I actually applied the script using the following command:

$ docker run -v `pwd`:`pwd` -w `pwd` petersenna/coccinelle \
--sp-file scripts/coccinelle/tcg_gen_extract.cocci \
--macro-file scripts/cocci-macro-file.h \
--dir target \
--in-place

Please review again! thanks.

[1] http://lists.nongnu.org/archive/html/qemu-devel/2017-05/msg01466.html
[2] http://lists.nongnu.org/archive/html/qemu-devel/2017-02/msg05211.html
[3] http://lists.nongnu.org/archive/html/qemu-devel/2017-05/msg01499.html

Philippe Mathieu-Daudé (10):
  coccinelle: ignore ASTs pre-parsed cached C files
  coccinelle: add a script to optimize tcg op using tcg_gen_extract()
  target/arm: optimize aarch64 rev16() using extract op
  target/m68k: optimize bcd_flags() using extract op
  target/ppc: optimize various functions using extract op
  target/sparc: optimize various functions using extract op
  target/sparc: optimize gen_op_mulscc() using extract op
  target/sparc: optimize gen_op_mulscc() using deposit op
  target/alpha: optimize gen_cvtlq() using extract op
  tcg/tci: enable bswap16_i64

 .gitignore   |   2 +
 scripts/coccinelle/tcg_gen_extract.cocci | 107 +++
 target/alpha/translate.c |   8 +--
 target/arm/translate-a64.c   |   6 +-
 target/m68k/translate.c  |   3 +-
 target/ppc/translate.c   |  21 ++
 target/ppc/translate/vsx-impl.inc.c  |  24 +++
 target/sparc/translate.c |  20 ++
 tcg/tci.c|   1 -
 9 files changed, 136 insertions(+), 56 deletions(-)
 create mode 100644 scripts/coccinelle/tcg_gen_extract.cocci

-- 
2.13.2

[Qemu-devel] [PATCH v5 03/10] target/arm: optimize aarch64 rev16() using extract op

[Philippe Mathieu-Daudé]

Aurelien Jarno denoted this function could be implemented more effectively using
the aarch32 rev16() pattern.
  [http://lists.nongnu.org/archive/html/qemu-devel/2017-05/msg03180.html]

Done with the Coccinelle semantic patch from commit 58daf05d07dd
(see scripts/coccinelle/tcg_gen_extract.cocci)

Signed-off-by: Philippe Mathieu-Daudé 
---
Richard: maybe you need to update 58daf05d07dd to your commit...

 target/arm/translate-a64.c | 6 ++
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/target/arm/translate-a64.c b/target/arm/translate-a64.c
index e55547d95d..8ade865481 100644
--- a/target/arm/translate-a64.c
+++ b/target/arm/translate-a64.c
@@ -4046,14 +4046,12 @@ static void handle_rev16(DisasContext *s, unsigned int 
sf,
 tcg_gen_andi_i64(tcg_tmp, tcg_rn, 0x);
 tcg_gen_bswap16_i64(tcg_rd, tcg_tmp);
 
-tcg_gen_shri_i64(tcg_tmp, tcg_rn, 16);
-tcg_gen_andi_i64(tcg_tmp, tcg_tmp, 0x);
+tcg_gen_extract_i64(tcg_tmp, tcg_rn, 16, 16);
 tcg_gen_bswap16_i64(tcg_tmp, tcg_tmp);
 tcg_gen_deposit_i64(tcg_rd, tcg_rd, tcg_tmp, 16, 16);
 
 if (sf) {
-tcg_gen_shri_i64(tcg_tmp, tcg_rn, 32);
-tcg_gen_andi_i64(tcg_tmp, tcg_tmp, 0x);
+tcg_gen_extract_i64(tcg_tmp, tcg_rn, 32, 16);
 tcg_gen_bswap16_i64(tcg_tmp, tcg_tmp);
 tcg_gen_deposit_i64(tcg_rd, tcg_rd, tcg_tmp, 32, 16);
 
-- 
2.13.2

[Qemu-devel] [PATCH v5 01/10] coccinelle: ignore ASTs pre-parsed cached C files

[Philippe Mathieu-Daudé]

files generated using coccinelle tool: 'spatch --use-cache'

Signed-off-by: Philippe Mathieu-Daudé 
---
 .gitignore | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/.gitignore b/.gitignore
index 09c2363acf..cf65316863 100644
--- a/.gitignore
+++ b/.gitignore
@@ -116,6 +116,8 @@ tags
 TAGS
 docker-src.*
 *~
+*.ast_raw
+*.depend_raw
 trace.h
 trace.c
 trace-ust.h
-- 
2.13.2

[Qemu-devel] [PATCH v5 04/10] target/m68k: optimize bcd_flags() using extract op

[Philippe Mathieu-Daudé]

Done with the Coccinelle semantic patch from commit 58daf05d07dd
(see scripts/coccinelle/tcg_gen_extract.cocci)

Signed-off-by: Philippe Mathieu-Daudé 
Acked-by: Laurent Vivier 
Reviewed-by: Richard Henderson 
---
Richard: maybe you need to update 58daf05d07dd to your commit...

 target/m68k/translate.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/target/m68k/translate.c b/target/m68k/translate.c
index 3a519b790d..e709e6cde2 100644
--- a/target/m68k/translate.c
+++ b/target/m68k/translate.c
@@ -1749,8 +1749,7 @@ static void bcd_flags(TCGv val)
 tcg_gen_andi_i32(QREG_CC_C, val, 0x0ff);
 tcg_gen_or_i32(QREG_CC_Z, QREG_CC_Z, QREG_CC_C);
 
-tcg_gen_shri_i32(QREG_CC_C, val, 8);
-tcg_gen_andi_i32(QREG_CC_C, QREG_CC_C, 1);
+tcg_gen_extract_i32(QREG_CC_C, val, 8, 1);
 
 tcg_gen_mov_i32(QREG_CC_X, QREG_CC_C);
 }
-- 
2.13.2

Re: [Qemu-devel] [PATCH v2 10/45] translate-all: guarantee that tb_hash only holds valid TBs

[Emilio G. Cota]

On Mon, Jul 17, 2017 at 17:40:29 -1000, Richard Henderson wrote:
> On 07/17/2017 02:27 PM, Emilio G. Cota wrote:
> >On Mon, Jul 17, 2017 at 12:55:03 -1000, Richard Henderson wrote:
> >>On 07/16/2017 10:03 AM, Emilio G. Cota wrote:
> >>>@@ -1073,13 +1073,17 @@ void tb_phys_invalidate(TranslationBlock *tb, 
> >>>tb_page_addr_t page_addr)
> >>>  assert_tb_locked();
> >>>-atomic_set(>invalid, true);
> >>>-
> >>>  /* remove the TB from the hash list */
> >>>  phys_pc = tb->page_addr[0] + (tb->pc & ~TARGET_PAGE_MASK);
> >>>  h = tb_hash_func(phys_pc, tb->pc, tb->flags, tb->trace_vcpu_dstate);
> >>>  qht_remove(_ctx.tb_ctx.htable, tb, h);
> >>>+/*
> >>>+ * Mark the TB as invalid *after* it's been removed from tb_hash, 
> >>>which
> >>>+ * eliminates the need to check this bit on lookups.
> >>>+ */
> >>>+tb->invalid = true;
> >>
> >>I believe you need atomic_store_release here.  Previously we were relying on
> >>the lock acquisition in qht_remove to provide the required memory barrier.
> >>
> >>We definitely need to make sure this reaches memory before we zap the TB in
> >>the CPU_FOREACH loop.
> >
> >After this patch tb->invalid is only read/set with tb_lock held, so no need 
> >for
> >atomics while accessing it.
> 
> I think there's a path by which we do get stale data.
> For threads A and B,
> 
>   (A) Lookup succeeds for TB in hash without tb_lock
>(B) Removes TB from hash
>(B) Sets tb->invalid
>(B) Clears FORALL_CPU jmp_cache
>   (A) Store TB into local jmp_cache
> 
> ... and since we never check for invalid again, there's nothing to evict TB
> from the jmp_cache again.

Ouch. Yes I see it now.

What threw me off was that in lookup_tb_ptr we're not checking tb->invalid,
and that biased me into thinking that it's not needed. But I should have
tried harder. Also, that's a bug, and yet another reason to have tb_lookup,
so that we fix these things at once in one place.

> Here's a plan that will make me happy:
> 
> (1) Drop this patch, leaving the set of tb->invalid (or CF_INVALID) in place.
> (2) Include CF_INVALID in the mask of bits compared in tb_lookup__cpu_state.
> (a) At this point in the patch set that's just
> 
>   (tb->flags & CF_INVALID) == 0
> 
> (b) Later in the patch series when CF_PARALLEL is introduced
> (and CF_HASH_MASK, lets call it, instead of the cf_mask
> function you have now), this becomes
> 
> (tb->flags & (CF_HASH_MASK | CF_INVALID)) == cf_mask
> 
> So that we continue to check CF_INVALID each time we lookup a TB, but now we
> get it for free as a part of the other flags check.

With the annoying atomic_read thrown in there :-) but yes, will do.

E.

Re: [Qemu-devel] [PATCH v3] spapr: disable decrementer during reset

[David Gibson]

On Mon, Jul 17, 2017 at 09:46:39AM +0530, Nikunj A Dadhania wrote:
> Rebooting a SMP TCG guest is broken for both single/multi threaded TCG.
> 
> When reset happens, all the CPUs are in halted state. First CPU is brought out
> of reset and secondary CPUs would be initialized by the guest kernel using a
> rtas call start-cpu.
> 
> However, in case of TCG, decrementer interrupts keep on coming and waking the
> secondary CPUs up.
> 
> These secondary CPUs would see the decrementer interrupt pending, which makes
> cpu::has_work() to bring them out of wait loop and start executing
> tcg_exec_cpu().
> 
> The problem with this is all the CPUs wake up and start booting SLOF image,
> causing the following exception(4 CPUs TCG VM):

Ok, I'm still trying to understand why the behaviour on reboot is
different from the first boot.  AFAICT on initial boot, the LPCR will
have DEE / PECE3 enabled.  So why aren't we getting the same problem
then?


> 
> [   81.440850] reboot: Restarting system
> 
> SLOF
> S
> SLOF
> SLOFLOF[0[0m 
> **
> QEMU Starting
>  Build Date = Mar  3 2017 13:29:19
>  FW Version = git-66d250ef0fd06bb8
> [0m **
> QEMU Starting
>  Build Date = Mar  3 2017 13:29:19
>  FW Version = git-66d250ef0fd06bb8
> [0m *m**[?25l 
> **
> QEMU Starting
>  Build Date = Mar  3 2017 13:29:19
>  FW Version = git-66d250ef0fd06bb8
> ***
> QEMU Starting
>  Build Date = Mar  3 2017 13:29:19
>  FW Version = git-66d250ef0fd06bb8
> ERROR: Flatten device tree not available!
> 
>  exception 300
> SRR0 = 60e4  SRR1 = 80008000
> SPRG2 = 0040  SPRG3 = 4bd8
> ERROR: Flatten device tree not available!
> 
>  exception 300
> SRR0 = 60e4  SRR1 = 80008000
> SPRG2 = 0040  SPRG3 = 4bd8
> 
> During reset, disable decrementer interrupt for secondary CPUs and enable them
> when the secondary CPUs are brought online by rtas start-cpu call.
> 
> Reported-by: Bharata B Rao 
> Signed-off-by: Nikunj A Dadhania 
> ---
>  hw/ppc/spapr_cpu_core.c | 9 +
>  hw/ppc/spapr_rtas.c | 8 
>  2 files changed, 17 insertions(+)
> 
> diff --git a/hw/ppc/spapr_cpu_core.c b/hw/ppc/spapr_cpu_core.c
> index ea278ce..bbfe8c2 100644
> --- a/hw/ppc/spapr_cpu_core.c
> +++ b/hw/ppc/spapr_cpu_core.c
> @@ -87,6 +87,15 @@ static void spapr_cpu_reset(void *opaque)
>  
>  env->spr[SPR_HIOR] = 0;
>  
> +/* Disable DECR for secondary cpus */
> +if (cs != first_cpu) {
> +if (env->mmu_model == POWERPC_MMU_3_00) {
> +env->spr[SPR_LPCR] &= ~LPCR_DEE;
> +} else {
> +/* P7 and P8 both have same bit for DECR */
> +env->spr[SPR_LPCR] &= ~LPCR_P8_PECE3;
> +}
> +}
>  /*
>   * This is a hack for the benefit of KVM PR - it abuses the SDR1
>   * slot in kvm_sregs to communicate the userspace address of the
> diff --git a/hw/ppc/spapr_rtas.c b/hw/ppc/spapr_rtas.c
> index 94a2799..4623d1d 100644
> --- a/hw/ppc/spapr_rtas.c
> +++ b/hw/ppc/spapr_rtas.c
> @@ -174,6 +174,14 @@ static void rtas_start_cpu(PowerPCCPU *cpu_, 
> sPAPRMachineState *spapr,
>  kvm_cpu_synchronize_state(cs);
>  
>  env->msr = (1ULL << MSR_SF) | (1ULL << MSR_ME);
> +
> +/* Enable DECR interrupt */
> +if (env->mmu_model == POWERPC_MMU_3_00) {
> +env->spr[SPR_LPCR] |= LPCR_DEE;
> +} else {
> +/* P7 and P8 both have same bit for DECR */
> +env->spr[SPR_LPCR] |= LPCR_P8_PECE3;
> +}
>  env->nip = start;
>  env->gpr[3] = r3;
>  cs->halted = 0;

-- 
David Gibson| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au  | minimalist, thank you.  NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson


signature.asc
Description: PGP signature

Re: [Qemu-devel] [PATCH v2 36/45] tcg: dynamically allocate optimizer globals + fold into TCGContext

[Richard Henderson]

On 07/17/2017 06:33 PM, Emilio G. Cota wrote:

I would prefer either

   (1) Dynamic allocation.  I know we eschew that most places during,
   but surely this is the exact situation for which it's handy.

...

But I guess that's not what you mean with (1)? You mean to allocate
every single time we call tcg_optimize, allocating only the space
we need on each call?


Yes.

r~

Re: [Qemu-devel] [PATCH v2 44/45] translate-all: do not allocate a guard page for code_gen_buffer

[Richard Henderson]

On 07/16/2017 10:04 AM, Emilio G. Cota wrote:

TCG regions already have a guard page.

Signed-off-by: Emilio G. Cota
---
  accel/tcg/translate-all.c | 47 ---
  1 file changed, 12 insertions(+), 35 deletions(-)


This should just be folded into the previous patch that creates TCG Regions.


r~

Re: [Qemu-devel] [PATCH v2 36/45] tcg: dynamically allocate optimizer globals + fold into TCGContext

[Emilio G. Cota]

On Mon, Jul 17, 2017 at 17:53:33 -1000, Richard Henderson wrote:
> On 07/16/2017 10:04 AM, Emilio G. Cota wrote:
> >Groundwork for supporting multiple TCG contexts.
(snip)
> >  struct TCGContext {
> >  uint8_t *pool_cur, *pool_end;
> >  TCGPool *pool_first, *pool_current, *pool_first_large;
> >@@ -717,6 +725,10 @@ struct TCGContext {
> >  TCGTempSet free_temps[TCG_TYPE_COUNT * 2];
> >  TCGTemp temps[TCG_MAX_TEMPS]; /* globals first, temps after */
> >+/* optimizer */
> >+struct tcg_temp_info *opt_temps;
> >+TCGTempSet opt_temps_used;
> 
> I would prefer either
> 
>   (1) Dynamic allocation.  I know we eschew that most places during,
>   but surely this is the exact situation for which it's handy.
> 
>   (2) Make opt_temps an array of TCG_MAX_TEMPS and drop the pointer.

Originally I implemented (2). But the array is pretty large and
realised that the init ctx doesn't use it at all. So I made
the allocation dynamic, i.e. tcg_optimize will allocate the
array if the ctx doesn't have it yet.

But I guess that's not what you mean with (1)? You mean to allocate
every single time we call tcg_optimize, allocating only the space
we need on each call?

> I think the TCGTempSet should be a local within tcg_optimize.

Will do.

E.

Re: [Qemu-devel] [PATCH v2 40/45] osdep: introduce qemu_mprotect_rwx/none

[Richard Henderson]

On 07/16/2017 10:04 AM, Emilio G. Cota wrote:

+static int qemu_mprotect__osdep(void *addr, size_t size, int prot)
+{
+void *start = QEMU_ALIGN_PTR_DOWN(addr, qemu_real_host_page_size);
+void *end = QEMU_ALIGN_PTR_UP(addr + size, qemu_real_host_page_size);


I'm not keen on this.  Any good reason for it as opposed to asserting that the 
inputs are already page aligned?



r~

Re: [Qemu-devel] [PATCH v2 39/45] osdep: move qemu_real_host_page_size/mask to osdep

[Richard Henderson]

On 07/16/2017 10:04 AM, Emilio G. Cota wrote:

These only depend on the host and therefore belong in the common
osdep, not in a target-dependent object.

Signed-off-by: Emilio G. Cota
---
  include/exec/cpu-all.h | 2 --
  include/qemu/osdep.h   | 8 
  exec.c | 5 +
  util/osdep.c   | 9 +
  4 files changed, 18 insertions(+), 6 deletions(-)


I do wonder if a new file in util/ with a constructor to init would be cleaner.

But, ok I guess,

Reviewed-by: Richard Henderson 


r~

Re: [Qemu-devel] [PATCH v2 37/45] tcg: introduce **tcg_ctxs to keep track of all TCGContext's

[Richard Henderson]

On 07/16/2017 10:04 AM, Emilio G. Cota wrote:

Groundwork for supporting multiple TCG contexts.

Note that having n_tcg_ctxs is unnecessary. However, it is
convenient to have it, since it will simplify iterating over the
array: we'll have just a for loop instead of having to iterate
over a NULL-terminated array (which would require n+1 elems)
or having to check with ifdef's for usermode/softmmu.

Signed-off-by: Emilio G. Cota 
---
  tcg/tcg.c | 10 ++
  1 file changed, 10 insertions(+)

diff --git a/tcg/tcg.c b/tcg/tcg.c
index f907c47..8094278 100644
--- a/tcg/tcg.c
+++ b/tcg/tcg.c
@@ -115,6 +115,8 @@ static int tcg_target_const_match(tcg_target_long val, 
TCGType type,
  static void tcg_out_tb_init(TCGContext *s);
  static bool tcg_out_tb_finalize(TCGContext *s);
  
+static TCGContext **tcg_ctxs;

+static unsigned int n_tcg_ctxs;


I'm perfectly happy introducing these now, and converting stuff to use them.


+static void tcg_ctxs_init(TCGContext *s)
+{
+tcg_ctxs = g_new(TCGContext *, 1);
+tcg_ctxs[0] = s;
+n_tcg_ctxs = 1;
+}


This was confusing to me, trying to figure out how this function would be 
extended for multi-threading.  But it turns out it isn't -- it just goes away.



@@ -381,6 +390,7 @@ void tcg_context_init(TCGContext *s)
  indirect_reg_alloc_order[i] = tcg_target_reg_alloc_order[i];
  }
  
+tcg_ctxs_init(s);

  tcg_ctx = s;
  }


Thus I think it would be simpler for the interim to do

tcg_ctx = s;
tcg_ctxs = _ctx;
n_tcg_ctxs = 1;


r~

[Qemu-devel] [PULL 2/2] live-block-ops.txt: Rename, rewrite, and improve it

[Jeff Cody]

From: Kashyap Chamarthy 

This patch documents (including their QMP invocations) all the four
major kinds of live block operations:

  - `block-stream`
  - `block-commit`
  - `drive-mirror` (& `blockdev-mirror`)
  - `drive-backup` (& `blockdev-backup`)

Things considered while writing this document:

  - Use reStructuredText as markup language (with the goal of generating
the HTML output using the Sphinx Documentation Generator).  It is
gentler on the eye, and can be trivially converted to different
formats.  (Another reason: upstream QEMU is considering to switch to
Sphinx, which uses reStructuredText as its markup language.)

  - Raw QMP JSON output vs. 'qmp-shell'.  I debated with myself whether
to only show raw QMP JSON output (as that is the canonical
representation), or use 'qmp-shell', which takes key-value pairs.  I
settled on the approach of: for the first occurrence of a command,
use raw JSON; for subsequent occurrences, use 'qmp-shell', with an
occasional exception.

  - Usage of `-blockdev` command-line.

  - Usage of 'node-name' vs. file path to refer to disks.  While we have
`blockdev-{mirror, backup}` as 'node-name'-alternatives for
`drive-{mirror, backup}`, the `block-commit` command still operates
on file names for parameters 'base' and 'top'.  So I added a caveat
at the beginning to that effect.

Refer this related thread that I started (where I learnt
`block-stream` was recently reworked to accept 'node-name' for 'top'
and 'base' parameters):
https://lists.nongnu.org/archive/html/qemu-devel/2017-05/msg06466.html
"[RFC] Making 'block-stream', and 'block-commit' accept node-name"

All commands showed in this document were tested while documenting.

Thanks: Eric Blake for the section: "A note on points-in-time vs file
names".  This useful bit was originally articulated by Eric in his
KVMForum 2015 presentation, so I included that specific bit in this
document.

Signed-off-by: Kashyap Chamarthy 
Reviewed-by: Eric Blake 
Message-id: 20170717105205.32639-3-kcham...@redhat.com
Signed-off-by: Jeff Cody 
---
 docs/interop/live-block-operations.rst | 1088 
 docs/live-block-ops.txt|   72 ---
 2 files changed, 1088 insertions(+), 72 deletions(-)
 create mode 100644 docs/interop/live-block-operations.rst
 delete mode 100644 docs/live-block-ops.txt

diff --git a/docs/interop/live-block-operations.rst 
b/docs/interop/live-block-operations.rst
new file mode 100644
index 000..5f01797
--- /dev/null
+++ b/docs/interop/live-block-operations.rst
@@ -0,0 +1,1088 @@
+..
+Copyright (C) 2017 Red Hat Inc.
+
+This work is licensed under the terms of the GNU GPL, version 2 or
+later.  See the COPYING file in the top-level directory.
+
+
+Live Block Device Operations
+
+
+QEMU Block Layer currently (as of QEMU 2.9) supports four major kinds of
+live block device jobs -- stream, commit, mirror, and backup.  These can
+be used to manipulate disk image chains to accomplish certain tasks,
+namely: live copy data from backing files into overlays; shorten long
+disk image chains by merging data from overlays into backing files; live
+synchronize data from a disk image chain (including current active disk)
+to another target image; and point-in-time (and incremental) backups of
+a block device.  Below is a description of the said block (QMP)
+primitives, and some (non-exhaustive list of) examples to illustrate
+their use.
+
+.. note::
+The file ``qapi/block-core.json`` in the QEMU source tree has the
+canonical QEMU API (QAPI) schema documentation for the QMP
+primitives discussed here.
+
+.. todo (kashyapc):: Remove the ".. contents::" directive when Sphinx is
+ integrated.
+
+.. contents::
+
+Disk image backing chain notation
+-
+
+A simple disk image chain.  (This can be created live using QMP
+``blockdev-snapshot-sync``, or offline via ``qemu-img``)::
+
+   (Live QEMU)
+|
+.
+V
+
+[A] <- [B]
+
+(backing file)(overlay)
+
+The arrow can be read as: Image [A] is the backing file of disk image
+[B].  And live QEMU is currently writing to image [B], consequently, it
+is also referred to as the "active layer".
+
+There are two kinds of terminology that are common when referring to
+files in a disk image backing chain:
+
+(1) Directional: 'base' and 'top'.  Given the simple disk image chain
+above, image [A] can be referred to as 'base', and image [B] as
+'top'.  (This terminology can be seen in in QAPI schema file,
+block-core.json.)
+
+(2) Relational: 'backing file' and 'overlay'.  Again, taking the same
+simple disk image chain from the above, disk image [A] is 

Re: [Qemu-devel] [PATCH v2 38/45] tcg: distribute profiling counters across TCGContext's

[Richard Henderson]

On 07/16/2017 10:04 AM, Emilio G. Cota wrote:

+#define PROF_ADD_MAX(to, from, field)   \
+do {\
+typeof((from)->field) val__ = atomic_read(&((from)->field));\
+if (val__ > (to)->field) {  \
+(to)->field = val__;\
+}   \
+} while (0)


PROF_MAX?  There's no addition involved.

Otherwise,

Reviewed-by: Richard Henderson 


r~

[Qemu-devel] [PULL 1/2] bitmaps.md: Convert to rST; move it into 'interop' dir

[Jeff Cody]

From: Kashyap Chamarthy 

This is part of the on-going effort to convert QEMU upstream
documentation syntax to reStructuredText (rST).

The conversion to rST was done using:

$ pandoc -f markdown -t rst bitmaps.md -o bitmaps.rst

Then, make a couple of small syntactical adjustments.  While at it,
reword a statement to avoid ambiguity.  Addressing the feedback from
this thread:

https://lists.nongnu.org/archive/html/qemu-devel/2017-06/msg05428.html

Signed-off-by: Kashyap Chamarthy 
Reviewed-by: John Snow 
Reviewed-by: Eric Blake 
Message-id: 20170717105205.32639-2-kcham...@redhat.com
Signed-off-by: Jeff Cody 
---
 docs/devel/bitmaps.md| 505 --
 docs/interop/bitmaps.rst | 555 +++
 2 files changed, 555 insertions(+), 505 deletions(-)
 delete mode 100644 docs/devel/bitmaps.md
 create mode 100644 docs/interop/bitmaps.rst

diff --git a/docs/devel/bitmaps.md b/docs/devel/bitmaps.md
deleted file mode 100644
index a2e8d51..000
--- a/docs/devel/bitmaps.md
+++ /dev/null
@@ -1,505 +0,0 @@
-
-
-# Dirty Bitmaps and Incremental Backup
-
-* Dirty Bitmaps are objects that track which data needs to be backed up for the
-  next incremental backup.
-
-* Dirty bitmaps can be created at any time and attached to any node
-  (not just complete drives.)
-
-## Dirty Bitmap Names
-
-* A dirty bitmap's name is unique to the node, but bitmaps attached to 
different
-  nodes can share the same name.
-
-* Dirty bitmaps created for internal use by QEMU may be anonymous and have no
-  name, but any user-created bitmaps may not be. There can be any number of
-  anonymous bitmaps per node.
-
-* The name of a user-created bitmap must not be empty ("").
-
-## Bitmap Modes
-
-* A Bitmap can be "frozen," which means that it is currently in-use by a backup
-  operation and cannot be deleted, renamed, written to, reset,
-  etc.
-
-* The normal operating mode for a bitmap is "active."
-
-## Basic QMP Usage
-
-### Supported Commands ###
-
-* block-dirty-bitmap-add
-* block-dirty-bitmap-remove
-* block-dirty-bitmap-clear
-
-### Creation
-
-* To create a new bitmap, enabled, on the drive with id=drive0:
-
-```json
-{ "execute": "block-dirty-bitmap-add",
-  "arguments": {
-"node": "drive0",
-"name": "bitmap0"
-  }
-}
-```
-
-* This bitmap will have a default granularity that matches the cluster size of
-  its associated drive, if available, clamped to between [4KiB, 64KiB].
-  The current default for qcow2 is 64KiB.
-
-* To create a new bitmap that tracks changes in 32KiB segments:
-
-```json
-{ "execute": "block-dirty-bitmap-add",
-  "arguments": {
-"node": "drive0",
-"name": "bitmap0",
-"granularity": 32768
-  }
-}
-```
-
-### Deletion
-
-* Bitmaps that are frozen cannot be deleted.
-
-* Deleting the bitmap does not impact any other bitmaps attached to the same
-  node, nor does it affect any backups already created from this node.
-
-* Because bitmaps are only unique to the node to which they are attached,
-  you must specify the node/drive name here, too.
-
-```json
-{ "execute": "block-dirty-bitmap-remove",
-  "arguments": {
-"node": "drive0",
-"name": "bitmap0"
-  }
-}
-```
-
-### Resetting
-
-* Resetting a bitmap will clear all information it holds.
-
-* An incremental backup created from an empty bitmap will copy no data,
-  as if nothing has changed.
-
-```json
-{ "execute": "block-dirty-bitmap-clear",
-  "arguments": {
-"node": "drive0",
-"name": "bitmap0"
-  }
-}
-```
-
-## Transactions
-
-### Justification
-
-Bitmaps can be safely modified when the VM is paused or halted by using
-the basic QMP commands. For instance, you might perform the following actions:
-
-1. Boot the VM in a paused state.
-2. Create a full drive backup of drive0.
-3. Create a new bitmap attached to drive0.
-4. Resume execution of the VM.
-5. Incremental backups are ready to be created.
-
-At this point, the bitmap and drive backup would be correctly in sync,
-and incremental backups made from this point forward would be correctly aligned
-to the full drive backup.
-
-This is not particularly useful if we decide we want to start incremental
-backups after the VM has been running for a while, for which we will need to
-perform actions such as the following:
-
-1. Boot the VM and begin execution.
-2. Using a single transaction, perform the following operations:
-* Create bitmap0.
-* Create a full drive backup of drive0.
-3. Incremental backups are now ready to be created.
-
-### Supported Bitmap Transactions
-
-* block-dirty-bitmap-add
-* block-dirty-bitmap-clear
-
-The usages are identical to their respective QMP commands, but see below
-for examples.
-
-### Example: New Incremental Backup
-
-As outlined in the justification, perhaps we want to create a new incremental
-backup chain attached to a drive.
-
-```json
-{ "execute": 

[Qemu-devel] [PULL 0/2] Block patches

[Jeff Cody]

The following changes since commit ca4e667dbf431d4a2a5a619cde79d30dd2ac3eb2:

  Merge remote-tracking branch 'remotes/kraxel/tags/usb-20170717-pull-request' 
into staging (2017-07-17 17:54:17 +0100)

are available in the git repository at:

  git://github.com/codyprime/qemu-kvm-jtc.git tags/block-pull-request

for you to fetch changes up to 8508eee740c78d1465e25dad7c3e06137485dfbc:

  live-block-ops.txt: Rename, rewrite, and improve it (2017-07-18 00:11:01 
-0400)


Block patches (documentation)


Kashyap Chamarthy (2):
  bitmaps.md: Convert to rST; move it into 'interop' dir
  live-block-ops.txt: Rename, rewrite, and improve it

 docs/devel/bitmaps.md  |  505 ---
 docs/interop/bitmaps.rst   |  555 
 docs/interop/live-block-operations.rst | 1088 
 docs/live-block-ops.txt|   72 ---
 4 files changed, 1643 insertions(+), 577 deletions(-)
 delete mode 100644 docs/devel/bitmaps.md
 create mode 100644 docs/interop/bitmaps.rst
 create mode 100644 docs/interop/live-block-operations.rst
 delete mode 100644 docs/live-block-ops.txt

-- 
2.9.4

Re: [Qemu-devel] [Qemu-block] [PATCH v7 0/2] Rewrite 'live-block-ops.txt'; convert 'bitmaps.md' to rST

[Jeff Cody]

On Mon, Jul 17, 2017 at 12:52:03PM +0200, Kashyap Chamarthy wrote:
> v7: Address feedback from Eric; add his 'Reviewed-by' on both patches; 
> also retain John Snow's 'Reviewed-by' on bitmaps.rst
> v6: https://lists.nongnu.org/archive/html/qemu-devel/2017-07/msg02188.html
> v5: https://lists.nongnu.org/archive/html/qemu-devel/2017-07/msg01368.html
> v4: https://lists.nongnu.org/archive/html/qemu-devel/2017-06/msg06395.html
> 
> Rewrite the 'live-block-ops.txt' document (after renaming it to
> 'live-block-operations.rst') in reStructuredText (rST) format.  Given
> upstream QEMU's desire[*] to take advantage of the Sphinx + rST
> framework to gerate its documentation:
> 
> "Based on experience from the Linux kernel, QEMU's docs pipeline is
> going to be based on Sphinx [...] Sphinx is extensible and it is
> easy to add new input formats and input directives."
> 
> And as part of review, John Snow suggested[+] to link to the
> 'bitmaps.md' document.  So while at it, convert the 'bitmaps.md'
> document also into rST syntax.
> 
> Then, moved both the documents ('live-block-operations.rst', and
> 'bitmaps.rst') to 'qemu/docs/interop' directory.  (Paolo explained the
> term "interop" as: "management & interoperability with QEMU".)
> 
> That's the result of this series:
> 
> (1) Rewrite the 'live-block-ops.txt' document (for details, refer the
> commit message of the patch) in rST.
> 
> Sphinx-generted HTML rendering:
> 
> 
> https://kashyapc.fedorapeople.org/v7-QEMU-Docs/_build/html/docs/live-block-operations.html
> 
> (2) Convert the 'bitmaps.md' document to rST, as discussed on this[+]
> thread.
> 
> 
> [*] http://wiki.qemu.org/Features/Documentation
> [+] https://lists.nongnu.org/archive/html/qemu-devel/2017-06/msg05428.html
> 
> 
> Kashyap Chamarthy (2):
>   bitmaps.md: Convert to rST; move it into 'interop' dir
>   live-block-ops.txt: Rename, rewrite, and improve it
> 
>  docs/devel/bitmaps.md  |  505 ---
>  docs/interop/bitmaps.rst   |  555 
>  docs/interop/live-block-operations.rst | 1088 
> 
>  docs/live-block-ops.txt|   72 ---
>  4 files changed, 1643 insertions(+), 577 deletions(-)
>  delete mode 100644 docs/devel/bitmaps.md
>  create mode 100644 docs/interop/bitmaps.rst
>  create mode 100644 docs/interop/live-block-operations.rst
>  delete mode 100644 docs/live-block-ops.txt
> 
> -- 
> 2.9.4
> 
> 

Thanks,

Applied to my block branch:

git://github.com/codyprime/qemu-kvm-jtc block

-Jeff

Re: [Qemu-devel] [PATCH v2 36/45] tcg: dynamically allocate optimizer globals + fold into TCGContext

[Richard Henderson]

On 07/16/2017 10:04 AM, Emilio G. Cota wrote:

Groundwork for supporting multiple TCG contexts.

Signed-off-by: Emilio G. Cota 
---
  tcg/tcg.h  | 12 
  tcg/optimize.c | 40 +++-
  2 files changed, 35 insertions(+), 17 deletions(-)

diff --git a/tcg/tcg.h b/tcg/tcg.h
index 569f823..175d4de 100644
--- a/tcg/tcg.h
+++ b/tcg/tcg.h
@@ -641,6 +641,14 @@ QEMU_BUILD_BUG_ON(OPPARAM_BUF_SIZE > (1 << 14));
  /* Make sure that we don't overflow 64 bits without noticing.  */
  QEMU_BUILD_BUG_ON(sizeof(TCGOp) > 8);
  
+struct tcg_temp_info {

+bool is_const;
+uint16_t prev_copy;
+uint16_t next_copy;
+tcg_target_ulong val;
+tcg_target_ulong mask;
+};
+
  struct TCGContext {
  uint8_t *pool_cur, *pool_end;
  TCGPool *pool_first, *pool_current, *pool_first_large;
@@ -717,6 +725,10 @@ struct TCGContext {
  TCGTempSet free_temps[TCG_TYPE_COUNT * 2];
  TCGTemp temps[TCG_MAX_TEMPS]; /* globals first, temps after */
  
+/* optimizer */

+struct tcg_temp_info *opt_temps;
+TCGTempSet opt_temps_used;


I would prefer either

  (1) Dynamic allocation.  I know we eschew that most places during,
  but surely this is the exact situation for which it's handy.

  (2) Make opt_temps an array of TCG_MAX_TEMPS and drop the pointer.

I think the TCGTempSet should be a local within tcg_optimize.


r~

[Qemu-devel] [PATCH v3 09/10] migration: provide migrate_cap_add()

[Peter Xu]

Abstracted from migrate_set_block_enabled() to allocate
MigrationCapabilityStatusList properly.

Reviewed-by: Juan Quintela 
Signed-off-by: Peter Xu 
---
 migration/migration.c | 19 ---
 1 file changed, 16 insertions(+), 3 deletions(-)

diff --git a/migration/migration.c b/migration/migration.c
index 43a2471..db869c4 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -841,14 +841,27 @@ void migrate_set_state(int *state, int old_state, int 
new_state)
 }
 }
 
-void migrate_set_block_enabled(bool value, Error **errp)
+static MigrationCapabilityStatusList *migrate_cap_add(
+MigrationCapabilityStatusList *list,
+MigrationCapability index,
+bool state)
 {
 MigrationCapabilityStatusList *cap;
 
 cap = g_new0(MigrationCapabilityStatusList, 1);
 cap->value = g_new0(MigrationCapabilityStatus, 1);
-cap->value->capability = MIGRATION_CAPABILITY_BLOCK;
-cap->value->state = value;
+cap->value->capability = index;
+cap->value->state = state;
+cap->next = list;
+
+return cap;
+}
+
+void migrate_set_block_enabled(bool value, Error **errp)
+{
+MigrationCapabilityStatusList *cap;
+
+cap = migrate_cap_add(NULL, MIGRATION_CAPABILITY_BLOCK, value);
 qmp_migrate_set_capabilities(cap, errp);
 qapi_free_MigrationCapabilityStatusList(cap);
 }
-- 
2.7.4

[Qemu-devel] [PATCH v3 08/10] migration: provide migrate_caps_check()

[Peter Xu]

Abstract helper function to check migration capabilities (from the old
qmp_migrate_set_capabilities).  Prepare to be used somewhere else.

There is side effect on the change: when applying the capabilities, we
were skipping the invalid ones, but still applying the valid ones (if
they are provided in the same QMP request). After this refactoring,
we'll ignore all the capabilities if we detected invalid setup along the
way. However, I don't think it is a problem since general users should
not provide anything invalid after all.

Reviewed-by: Juan Quintela 
Signed-off-by: Peter Xu 
---
 migration/migration.c | 79 ++-
 1 file changed, 53 insertions(+), 26 deletions(-)

diff --git a/migration/migration.c b/migration/migration.c
index 0f0de04..43a2471 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -587,43 +587,49 @@ MigrationInfo *qmp_query_migrate(Error **errp)
 return info;
 }
 
-void qmp_migrate_set_capabilities(MigrationCapabilityStatusList *params,
-  Error **errp)
+/**
+ * @migration_caps_check - check capability validity
+ *
+ * @cap_list: old capability list, array of bool
+ * @params: new capabilities to be applied soon
+ * @errp: set *errp if the check failed, with reason
+ *
+ * Returns true if check passed, otherwise false.
+ */
+static bool migrate_caps_check(bool *cap_list,
+   MigrationCapabilityStatusList *params,
+   Error **errp)
 {
-MigrationState *s = migrate_get_current();
 MigrationCapabilityStatusList *cap;
-bool old_postcopy_cap = migrate_postcopy_ram();
+bool old_postcopy_cap;
 
-if (migration_is_setup_or_active(s->state)) {
-error_setg(errp, QERR_MIGRATION_ACTIVE);
-return;
-}
+old_postcopy_cap = cap_list[MIGRATION_CAPABILITY_POSTCOPY_RAM];
 
 for (cap = params; cap; cap = cap->next) {
+cap_list[cap->value->capability] = cap->value->state;
+}
+
 #ifndef CONFIG_LIVE_BLOCK_MIGRATION
-if (cap->value->capability == MIGRATION_CAPABILITY_BLOCK
-&& cap->value->state) {
-error_setg(errp, "QEMU compiled without old-style (blk/-b, inc/-i) 
"
-   "block migration");
-error_append_hint(errp, "Use drive_mirror+NBD instead.\n");
-continue;
-}
-#endif
-s->enabled_capabilities[cap->value->capability] = cap->value->state;
+if (cap_list[MIGRATION_CAPABILITY_BLOCK]) {
+error_setg(errp, "QEMU compiled without old-style (blk/-b, inc/-i) "
+   "block migration");
+error_append_hint(errp, "Use drive_mirror+NBD instead.\n");
+return false;
 }
+#endif
 
-if (migrate_postcopy_ram()) {
-if (migrate_use_compression()) {
+if (cap_list[MIGRATION_CAPABILITY_POSTCOPY_RAM]) {
+if (cap_list[MIGRATION_CAPABILITY_COMPRESS]) {
 /* The decompression threads asynchronously write into RAM
  * rather than use the atomic copies needed to avoid
  * userfaulting.  It should be possible to fix the decompression
  * threads for compatibility in future.
  */
-error_report("Postcopy is not currently compatible with "
- "compression");
-s->enabled_capabilities[MIGRATION_CAPABILITY_POSTCOPY_RAM] =
-false;
+error_setg(errp, "Postcopy is not currently compatible "
+   "with compression");
+return false;
 }
+
 /* This check is reasonably expensive, so only when it's being
  * set the first time, also it's only the destination that needs
  * special support.
@@ -633,11 +639,32 @@ void 
qmp_migrate_set_capabilities(MigrationCapabilityStatusList *params,
 /* postcopy_ram_supported_by_host will have emitted a more
  * detailed message
  */
-error_report("Postcopy is not supported");
-s->enabled_capabilities[MIGRATION_CAPABILITY_POSTCOPY_RAM] =
-false;
+error_setg(errp, "Postcopy is not supported");
+return false;
 }
 }
+
+return true;
+}
+
+void qmp_migrate_set_capabilities(MigrationCapabilityStatusList *params,
+  Error **errp)
+{
+MigrationState *s = migrate_get_current();
+MigrationCapabilityStatusList *cap;
+
+if (migration_is_setup_or_active(s->state)) {
+error_setg(errp, QERR_MIGRATION_ACTIVE);
+return;
+}
+
+if (!migrate_caps_check(s->enabled_capabilities, params, errp)) {
+return;
+}
+
+for (cap = params; cap; cap = cap->next) {
+s->enabled_capabilities[cap->value->capability] = cap->value->state;
+}
 }
 
 /*
-- 
2.7.4

[Qemu-devel] [PATCH v3 07/10] migration: remove check against colo support

[Peter Xu]

Since commit a15215f3 ("build: remove --enable-colo/--disable-colo"),
colo is always supported. We don't need any colo_supported() now since
it is always true. Removing any extra code that depends on it.

CC: Paolo Bonzini 
CC: Hailiang Zhang 
Reviewed-by: Juan Quintela 
Signed-off-by: Peter Xu 
---
 include/migration/colo.h |  1 -
 migration/colo.c |  5 -
 migration/migration.c| 11 ---
 3 files changed, 17 deletions(-)

diff --git a/include/migration/colo.h b/include/migration/colo.h
index be6beba..ff9874e 100644
--- a/include/migration/colo.h
+++ b/include/migration/colo.h
@@ -15,7 +15,6 @@
 
 #include "qemu-common.h"
 
-bool colo_supported(void);
 void colo_info_init(void);
 
 void migrate_start_colo_process(MigrationState *s);
diff --git a/migration/colo.c b/migration/colo.c
index ef35f00..a425543 100644
--- a/migration/colo.c
+++ b/migration/colo.c
@@ -29,11 +29,6 @@ static bool vmstate_loading;
 
 #define COLO_BUFFER_BASE_SIZE (4 * 1024 * 1024)
 
-bool colo_supported(void)
-{
-return true;
-}
-
 bool migration_in_colo_state(void)
 {
 MigrationState *s = migrate_get_current();
diff --git a/migration/migration.c b/migration/migration.c
index a4a8ed8..0f0de04 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -411,9 +411,6 @@ MigrationCapabilityStatusList 
*qmp_query_migrate_capabilities(Error **errp)
 continue;
 }
 #endif
-if (i == MIGRATION_CAPABILITY_X_COLO && !colo_supported()) {
-continue;
-}
 if (head == NULL) {
 head = g_malloc0(sizeof(*caps));
 caps = head;
@@ -612,14 +609,6 @@ void 
qmp_migrate_set_capabilities(MigrationCapabilityStatusList *params,
 continue;
 }
 #endif
-if (cap->value->capability == MIGRATION_CAPABILITY_X_COLO) {
-if (!colo_supported()) {
-error_setg(errp, "COLO is not currently supported, please"
- " configure with --enable-colo option in order to"
- " support COLO feature");
-continue;
-}
-}
 s->enabled_capabilities[cap->value->capability] = cap->value->state;
 }
 
-- 
2.7.4

[Qemu-devel] [PATCH v3 06/10] migration: check global params for validity

[Peter Xu]

Adding validity check for the migration parameters passed in via global
properties.

Signed-off-by: Peter Xu 
---
 migration/migration.c | 38 --
 1 file changed, 36 insertions(+), 2 deletions(-)

diff --git a/migration/migration.c b/migration/migration.c
index 8c65054..a4a8ed8 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -102,14 +102,22 @@ enum mig_rp_message_type {
 
 static MigrationState *current_migration;
 
+static bool migration_object_check(MigrationState *ms, Error **errp);
+
 void migration_object_init(void)
 {
 MachineState *ms = MACHINE(qdev_get_machine());
+Error *err = NULL;
 
 /* This can only be called once. */
 assert(!current_migration);
 current_migration = MIGRATION_OBJ(object_new(TYPE_MIGRATION));
 
+if (!migration_object_check(current_migration, )) {
+error_report_err(err);
+exit(1);
+}
+
 /*
  * We cannot really do this in migration_instance_init() since at
  * that time global properties are not yet applied, then this
@@ -2101,12 +2109,38 @@ static void migration_class_init(ObjectClass *klass, 
void *data)
 static void migration_instance_init(Object *obj)
 {
 MigrationState *ms = MIGRATION_OBJ(obj);
+MigrationParameters *params = >parameters;
 
 ms->state = MIGRATION_STATUS_NONE;
 ms->xbzrle_cache_size = DEFAULT_MIGRATE_CACHE_SIZE;
 ms->mbps = -1;
-ms->parameters.tls_creds = g_strdup("");
-ms->parameters.tls_hostname = g_strdup("");
+
+params->tls_hostname = g_strdup("");
+params->tls_creds = g_strdup("");
+
+/* Set has_* up only for parameter checks */
+params->has_compress_level = true;
+params->has_compress_threads = true;
+params->has_decompress_threads = true;
+params->has_cpu_throttle_initial = true;
+params->has_cpu_throttle_increment = true;
+params->has_max_bandwidth = true;
+params->has_downtime_limit = true;
+params->has_x_checkpoint_delay = true;
+params->has_block_incremental = true;
+}
+
+/*
+ * Return true if check pass, false otherwise. Error will be put
+ * inside errp if provided.
+ */
+static bool migration_object_check(MigrationState *ms, Error **errp)
+{
+if (!migrate_params_check(>parameters, errp)) {
+return false;
+}
+
+return true;
 }
 
 static const TypeInfo migration_type = {
-- 
2.7.4

Re: [Qemu-devel] [PATCH v4 6/6] target/sparc: optimize various functions using extract op

[Richard Henderson]

On 07/17/2017 05:18 PM, Philippe Mathieu-Daudé wrote:

On 05/12/2017 09:08 PM, Richard Henderson wrote:

On 05/12/2017 04:38 PM, Philippe Mathieu-Daudé wrote:

[...]

  static inline void gen_op_add_cc(TCGv dst, TCGv src1, TCGv src2)
@@ -638,8 +634,7 @@ static inline void gen_op_mulscc(TCGv dst, TCGv src1, 
TCGv src2)

  // env->y = (b2 << 31) | (env->y >> 1);
  tcg_gen_andi_tl(r_temp, cpu_cc_src, 0x1);
  tcg_gen_shli_tl(r_temp, r_temp, 31);
-tcg_gen_shri_tl(t0, cpu_y, 1);
-tcg_gen_andi_tl(t0, t0, 0x7fff);
+tcg_gen_extract_tl(t0, cpu_y, 1, 31);
  tcg_gen_or_tl(t0, t0, r_temp);
  tcg_gen_andi_tl(cpu_y, t0, 0x);


So this 0x mask is incorrect and should be 0x7fff?


No, this has nothing to do with the second andi.


r~

[Qemu-devel] [PATCH v3 04/10] migration: introduce migrate_params_check()

[Peter Xu]

Helper to check the parameters. Abstracted from
qmp_migrate_set_parameters().

Reviewed-by: Juan Quintela 
Signed-off-by: Peter Xu 
---
 migration/migration.c | 42 --
 1 file changed, 32 insertions(+), 10 deletions(-)

diff --git a/migration/migration.c b/migration/migration.c
index 3208162..2821f8a 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -643,64 +643,86 @@ void 
qmp_migrate_set_capabilities(MigrationCapabilityStatusList *params,
 }
 }
 
-void qmp_migrate_set_parameters(MigrationParameters *params, Error **errp)
+/*
+ * Check whether the parameters are valid. Error will be put into errp
+ * (if provided). Return true if valid, otherwise false.
+ */
+static bool migrate_params_check(MigrationParameters *params, Error **errp)
 {
-MigrationState *s = migrate_get_current();
-
 if (params->has_compress_level &&
 (params->compress_level < 0 || params->compress_level > 9)) {
 error_setg(errp, QERR_INVALID_PARAMETER_VALUE, "compress_level",
"is invalid, it should be in the range of 0 to 9");
-return;
+return false;
 }
+
 if (params->has_compress_threads &&
 (params->compress_threads < 1 || params->compress_threads > 255)) {
 error_setg(errp, QERR_INVALID_PARAMETER_VALUE,
"compress_threads",
"is invalid, it should be in the range of 1 to 255");
-return;
+return false;
 }
+
 if (params->has_decompress_threads &&
 (params->decompress_threads < 1 || params->decompress_threads > 255)) {
 error_setg(errp, QERR_INVALID_PARAMETER_VALUE,
"decompress_threads",
"is invalid, it should be in the range of 1 to 255");
-return;
+return false;
 }
+
 if (params->has_cpu_throttle_initial &&
 (params->cpu_throttle_initial < 1 ||
  params->cpu_throttle_initial > 99)) {
 error_setg(errp, QERR_INVALID_PARAMETER_VALUE,
"cpu_throttle_initial",
"an integer in the range of 1 to 99");
-return;
+return false;
 }
+
 if (params->has_cpu_throttle_increment &&
 (params->cpu_throttle_increment < 1 ||
  params->cpu_throttle_increment > 99)) {
 error_setg(errp, QERR_INVALID_PARAMETER_VALUE,
"cpu_throttle_increment",
"an integer in the range of 1 to 99");
-return;
+return false;
 }
+
 if (params->has_max_bandwidth &&
 (params->max_bandwidth < 0 || params->max_bandwidth > SIZE_MAX)) {
 error_setg(errp, "Parameter 'max_bandwidth' expects an integer in the"
  " range of 0 to %zu bytes/second", SIZE_MAX);
-return;
+return false;
 }
+
 if (params->has_downtime_limit &&
 (params->downtime_limit < 0 ||
  params->downtime_limit > MAX_MIGRATE_DOWNTIME)) {
 error_setg(errp, "Parameter 'downtime_limit' expects an integer in "
  "the range of 0 to %d milliseconds",
  MAX_MIGRATE_DOWNTIME);
-return;
+return false;
 }
+
 if (params->has_x_checkpoint_delay && (params->x_checkpoint_delay < 0)) {
 error_setg(errp, QERR_INVALID_PARAMETER_VALUE,
 "x_checkpoint_delay",
 "is invalid, it should be positive");
+return false;
+}
+
+return true;
+}
+
+void qmp_migrate_set_parameters(MigrationParameters *params, Error **errp)
+{
+MigrationState *s = migrate_get_current();
+
+if (!migrate_params_check(params, errp)) {
+/* Invalid parameter */
+return;
 }
 
 if (params->has_compress_level) {
-- 
2.7.4

Re: [Qemu-devel] [PATCH v2 10/45] translate-all: guarantee that tb_hash only holds valid TBs

[Richard Henderson]

On 07/17/2017 02:27 PM, Emilio G. Cota wrote:

On Mon, Jul 17, 2017 at 12:55:03 -1000, Richard Henderson wrote:

On 07/16/2017 10:03 AM, Emilio G. Cota wrote:

@@ -1073,13 +1073,17 @@ void tb_phys_invalidate(TranslationBlock *tb, 
tb_page_addr_t page_addr)
  assert_tb_locked();
-atomic_set(>invalid, true);
-
  /* remove the TB from the hash list */
  phys_pc = tb->page_addr[0] + (tb->pc & ~TARGET_PAGE_MASK);
  h = tb_hash_func(phys_pc, tb->pc, tb->flags, tb->trace_vcpu_dstate);
  qht_remove(_ctx.tb_ctx.htable, tb, h);
+/*
+ * Mark the TB as invalid *after* it's been removed from tb_hash, which
+ * eliminates the need to check this bit on lookups.
+ */
+tb->invalid = true;


I believe you need atomic_store_release here.  Previously we were relying on
the lock acquisition in qht_remove to provide the required memory barrier.

We definitely need to make sure this reaches memory before we zap the TB in
the CPU_FOREACH loop.


After this patch tb->invalid is only read/set with tb_lock held, so no need for
atomics while accessing it.


I think there's a path by which we do get stale data.
For threads A and B,

  (A) Lookup succeeds for TB in hash without tb_lock
   (B) Removes TB from hash
   (B) Sets tb->invalid
   (B) Clears FORALL_CPU jmp_cache
  (A) Store TB into local jmp_cache

... and since we never check for invalid again, there's nothing to evict TB 
from the jmp_cache again.


Here's a plan that will make me happy:

(1) Drop this patch, leaving the set of tb->invalid (or CF_INVALID) in place.
(2) Include CF_INVALID in the mask of bits compared in tb_lookup__cpu_state.
(a) At this point in the patch set that's just

(tb->flags & CF_INVALID) == 0

(b) Later in the patch series when CF_PARALLEL is introduced
(and CF_HASH_MASK, lets call it, instead of the cf_mask
function you have now), this becomes

(tb->flags & (CF_HASH_MASK | CF_INVALID)) == cf_mask

So that we continue to check CF_INVALID each time we lookup a TB, but now we 
get it for free as a part of the other flags check.



r~

[Qemu-devel] [PATCH v3 03/10] migration: export capabilities to props

[Peter Xu]

Do the same thing to migration capabilities, just like what we did in
previous patch for migration parameters.

Reviewed-by: Juan Quintela 
Reviewed-by: Eduardo Habkost 
Signed-off-by: Peter Xu 
---
 migration/migration.c | 17 +
 1 file changed, 17 insertions(+)

diff --git a/migration/migration.c b/migration/migration.c
index ad2505c..3208162 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -2001,6 +2001,9 @@ void migration_global_dump(Monitor *mon)
ms->send_configuration, ms->send_section_footer);
 }
 
+#define DEFINE_PROP_MIG_CAP(name, x) \
+DEFINE_PROP_BOOL(name, MigrationState, enabled_capabilities[x], false)
+
 static Property migration_properties[] = {
 DEFINE_PROP_BOOL("store-global-state", MigrationState,
  store_global_state, true),
@@ -2034,6 +2037,20 @@ static Property migration_properties[] = {
 DEFINE_PROP_INT64("x-checkpoint-delay", MigrationState,
   parameters.x_checkpoint_delay,
   DEFAULT_MIGRATE_X_CHECKPOINT_DELAY),
+
+/* Migration capabilities */
+DEFINE_PROP_MIG_CAP("x-xbzrle", MIGRATION_CAPABILITY_XBZRLE),
+DEFINE_PROP_MIG_CAP("x-rdma-pin-all", MIGRATION_CAPABILITY_RDMA_PIN_ALL),
+DEFINE_PROP_MIG_CAP("x-auto-converge", MIGRATION_CAPABILITY_AUTO_CONVERGE),
+DEFINE_PROP_MIG_CAP("x-zero-blocks", MIGRATION_CAPABILITY_ZERO_BLOCKS),
+DEFINE_PROP_MIG_CAP("x-compress", MIGRATION_CAPABILITY_COMPRESS),
+DEFINE_PROP_MIG_CAP("x-events", MIGRATION_CAPABILITY_EVENTS),
+DEFINE_PROP_MIG_CAP("x-postcopy-ram", MIGRATION_CAPABILITY_POSTCOPY_RAM),
+DEFINE_PROP_MIG_CAP("x-colo", MIGRATION_CAPABILITY_X_COLO),
+DEFINE_PROP_MIG_CAP("x-release-ram", MIGRATION_CAPABILITY_RELEASE_RAM),
+DEFINE_PROP_MIG_CAP("x-block", MIGRATION_CAPABILITY_BLOCK),
+DEFINE_PROP_MIG_CAP("x-return-path", MIGRATION_CAPABILITY_RETURN_PATH),
+
 DEFINE_PROP_END_OF_LIST(),
 };
 
-- 
2.7.4

[Qemu-devel] [PATCH v3 02/10] migration: export parameters to props

[Peter Xu]

Export migration parameters to qdev properties. Then we can use, for
example:

  -global migration.x-cpu-throttle-initial=xxx

To specify migration parameters during init.

Prefix "x-" is appended for each parameter exported to show that this is
not a stable interface, and only for debugging/testing purpose.

Reviewed-by: Juan Quintela 
Signed-off-by: Peter Xu 
---
 migration/migration.c | 35 +--
 1 file changed, 25 insertions(+), 10 deletions(-)

diff --git a/migration/migration.c b/migration/migration.c
index a0db40d..ad2505c 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -2009,6 +2009,31 @@ static Property migration_properties[] = {
  send_configuration, true),
 DEFINE_PROP_BOOL("send-section-footer", MigrationState,
  send_section_footer, true),
+
+/* Migration parameters */
+DEFINE_PROP_INT64("x-compress-level", MigrationState,
+  parameters.compress_level,
+  DEFAULT_MIGRATE_COMPRESS_LEVEL),
+DEFINE_PROP_INT64("x-compress-threads", MigrationState,
+  parameters.compress_threads,
+  DEFAULT_MIGRATE_COMPRESS_THREAD_COUNT),
+DEFINE_PROP_INT64("x-decompress-threads", MigrationState,
+  parameters.decompress_threads,
+  DEFAULT_MIGRATE_DECOMPRESS_THREAD_COUNT),
+DEFINE_PROP_INT64("x-cpu-throttle-initial", MigrationState,
+  parameters.cpu_throttle_initial,
+  DEFAULT_MIGRATE_CPU_THROTTLE_INITIAL),
+DEFINE_PROP_INT64("x-cpu-throttle-increment", MigrationState,
+  parameters.cpu_throttle_increment,
+  DEFAULT_MIGRATE_CPU_THROTTLE_INCREMENT),
+DEFINE_PROP_INT64("x-max-bandwidth", MigrationState,
+  parameters.max_bandwidth, MAX_THROTTLE),
+DEFINE_PROP_INT64("x-downtime-limit", MigrationState,
+  parameters.downtime_limit,
+  DEFAULT_MIGRATE_SET_DOWNTIME),
+DEFINE_PROP_INT64("x-checkpoint-delay", MigrationState,
+  parameters.x_checkpoint_delay,
+  DEFAULT_MIGRATE_X_CHECKPOINT_DELAY),
 DEFINE_PROP_END_OF_LIST(),
 };
 
@@ -2027,16 +2052,6 @@ static void migration_instance_init(Object *obj)
 ms->state = MIGRATION_STATUS_NONE;
 ms->xbzrle_cache_size = DEFAULT_MIGRATE_CACHE_SIZE;
 ms->mbps = -1;
-ms->parameters = (MigrationParameters) {
-.compress_level = DEFAULT_MIGRATE_COMPRESS_LEVEL,
-.compress_threads = DEFAULT_MIGRATE_COMPRESS_THREAD_COUNT,
-.decompress_threads = DEFAULT_MIGRATE_DECOMPRESS_THREAD_COUNT,
-.cpu_throttle_initial = DEFAULT_MIGRATE_CPU_THROTTLE_INITIAL,
-.cpu_throttle_increment = DEFAULT_MIGRATE_CPU_THROTTLE_INCREMENT,
-.max_bandwidth = MAX_THROTTLE,
-.downtime_limit = DEFAULT_MIGRATE_SET_DOWNTIME,
-.x_checkpoint_delay = DEFAULT_MIGRATE_X_CHECKPOINT_DELAY,
-};
 ms->parameters.tls_creds = g_strdup("");
 ms->parameters.tls_hostname = g_strdup("");
 }
-- 
2.7.4

Re: [Qemu-devel] [PATCH v2 21/45] tcg: check CF_PARALLEL instead of parallel_cpus

[Richard Henderson]

On 07/17/2017 02:34 PM, Emilio G. Cota wrote:

On Mon, Jul 17, 2017 at 13:55:42 -1000, Richard Henderson wrote:

On 07/16/2017 10:04 AM, Emilio G. Cota wrote:

Thereby decoupling the resulting translated code from the current state
of the system.

The tb->cflags field is not passed to tcg generation functions. So
we add a bit to TCGContext, storing there whether CF_PARALLEL is set
before translating every TB.

Most architectures have <= 32 registers, which results in a 4-byte hole
in TCGContext. Use this hole for the bit we need; use a uint8_t instead
of a bool, since a bool might take more than one byte in some systems.


I would much rather use bool.

(1) I don't care about OSX and its broken ABI,
(2) Even then OSX still *works*.


Will do.


Otherwise,


Missing R-b tag?


Oops, yes.  Must have fat-fingered the ctrl-paste.

Reviewed-by: Richard Henderson 


r~

[Qemu-devel] Is compressed qcow2 better in read/write performance?

[陳培泓]

qcow2 support compress option to compress image file.

I try to read both files through API by libguestfs(such as pread_device)

I'm confused with the performance during read or write

I tried to compress A.qcow2 to A_compress.qcow2 and it changes from 16GB to
23MB

I tried read on A_compress.qcow2, it seems faster than A.qcow2

maybe something wrong happened?

[Qemu-devel] [PATCH v3 10/10] migration: check global caps for validity

[Peter Xu]

Checks validity for all the capabilities that we enabled with command
line.

Signed-off-by: Peter Xu 
---
 migration/migration.c | 18 +-
 1 file changed, 17 insertions(+), 1 deletion(-)

diff --git a/migration/migration.c b/migration/migration.c
index db869c4..cdcf989 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -2165,11 +2165,27 @@ static void migration_instance_init(Object *obj)
  */
 static bool migration_object_check(MigrationState *ms, Error **errp)
 {
+MigrationCapabilityStatusList *head = NULL;
+/* Assuming all off */
+bool cap_list[MIGRATION_CAPABILITY__MAX] = { 0 }, ret;
+int i;
+
 if (!migrate_params_check(>parameters, errp)) {
 return false;
 }
 
-return true;
+for (i = 0; i < MIGRATION_CAPABILITY__MAX; i++) {
+if (ms->enabled_capabilities[i]) {
+head = migrate_cap_add(head, i, true);
+}
+}
+
+ret = migrate_caps_check(cap_list, head, errp);
+
+/* It works with head == NULL */
+qapi_free_MigrationCapabilityStatusList(head);
+
+return ret;
 }
 
 static const TypeInfo migration_type = {
-- 
2.7.4

[Qemu-devel] [PATCH v3 05/10] migration: provide migrate_params_apply()

[Peter Xu]

Abstracted from qmp_migrate_set_parameters().

Reviewed-by: Juan Quintela 
Signed-off-by: Peter Xu 
---
 migration/migration.c | 26 --
 1 file changed, 20 insertions(+), 6 deletions(-)

diff --git a/migration/migration.c b/migration/migration.c
index 2821f8a..8c65054 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -716,38 +716,40 @@ static bool migrate_params_check(MigrationParameters 
*params, Error **errp)
 return true;
 }
 
-void qmp_migrate_set_parameters(MigrationParameters *params, Error **errp)
+static void migrate_params_apply(MigrationParameters *params)
 {
 MigrationState *s = migrate_get_current();
 
-if (!migrate_params_check(params, errp)) {
-/* Invalid parameter */
-return;
-}
-
 if (params->has_compress_level) {
 s->parameters.compress_level = params->compress_level;
 }
+
 if (params->has_compress_threads) {
 s->parameters.compress_threads = params->compress_threads;
 }
+
 if (params->has_decompress_threads) {
 s->parameters.decompress_threads = params->decompress_threads;
 }
+
 if (params->has_cpu_throttle_initial) {
 s->parameters.cpu_throttle_initial = params->cpu_throttle_initial;
 }
+
 if (params->has_cpu_throttle_increment) {
 s->parameters.cpu_throttle_increment = params->cpu_throttle_increment;
 }
+
 if (params->has_tls_creds) {
 g_free(s->parameters.tls_creds);
 s->parameters.tls_creds = g_strdup(params->tls_creds);
 }
+
 if (params->has_tls_hostname) {
 g_free(s->parameters.tls_hostname);
 s->parameters.tls_hostname = g_strdup(params->tls_hostname);
 }
+
 if (params->has_max_bandwidth) {
 s->parameters.max_bandwidth = params->max_bandwidth;
 if (s->to_dst_file) {
@@ -755,6 +757,7 @@ void qmp_migrate_set_parameters(MigrationParameters 
*params, Error **errp)
 s->parameters.max_bandwidth / 
XFER_LIMIT_RATIO);
 }
 }
+
 if (params->has_downtime_limit) {
 s->parameters.downtime_limit = params->downtime_limit;
 }
@@ -765,11 +768,22 @@ void qmp_migrate_set_parameters(MigrationParameters 
*params, Error **errp)
 colo_checkpoint_notify(s);
 }
 }
+
 if (params->has_block_incremental) {
 s->parameters.block_incremental = params->block_incremental;
 }
 }
 
+void qmp_migrate_set_parameters(MigrationParameters *params, Error **errp)
+{
+if (!migrate_params_check(params, errp)) {
+/* Invalid parameter */
+return;
+}
+
+migrate_params_apply(params);
+}
+
 
 void qmp_migrate_start_postcopy(Error **errp)
 {
-- 
2.7.4

Re: [Qemu-devel] [Qemu-ppc] [PULL 06/21] spapr: Add a "no HPT" encoding to HTAB migration stream

[David Gibson]

On Mon, Jul 17, 2017 at 09:54:45PM +0200, Laurent Vivier wrote:
> On 30/06/2017 12:46, David Gibson wrote:
> > From: Bharata B Rao 
> > 
> > Add a "no HPT" encoding (using value -1) to the HTAB migration
> > stream (in the place of HPT size) when the guest doesn't allocate HPT.
> > This will help the target side to match target HPT with the source HPT
> > and thus enable successful migration.
> > 
> > Suggested-by: David Gibson 
> > Signed-off-by: Bharata B Rao 
> > Signed-off-by: David Gibson 
> > ---
> >  hw/ppc/spapr.c | 29 +
> >  1 file changed, 25 insertions(+), 4 deletions(-)
> > 
> > diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> > index 52f4e72..f3e0b9b 100644
> > --- a/hw/ppc/spapr.c
> > +++ b/hw/ppc/spapr.c
> > @@ -1560,13 +1560,19 @@ static int htab_save_setup(QEMUFile *f, void 
> > *opaque)
> >  sPAPRMachineState *spapr = opaque;
> >  
> >  /* "Iteration" header */
> > -qemu_put_be32(f, spapr->htab_shift);
> > +if (!spapr->htab_shift) {
> > +qemu_put_be32(f, -1);
> > +} else {
> > +qemu_put_be32(f, spapr->htab_shift);
> > +}
> >  
> >  if (spapr->htab) {
> >  spapr->htab_save_index = 0;
> >  spapr->htab_first_pass = true;
> >  } else {
> > -assert(kvm_enabled());
> > +if (spapr->htab_shift) {
> > +assert(kvm_enabled());
> > +}
> >  }
> >  
> >  
> > @@ -1712,7 +1718,12 @@ static int htab_save_iterate(QEMUFile *f, void 
> > *opaque)
> >  int rc = 0;
> >  
> >  /* Iteration header */
> > -qemu_put_be32(f, 0);
> > +if (!spapr->htab_shift) {
> > +qemu_put_be32(f, -1);
> > +return 0;
> > +} else {
> > +qemu_put_be32(f, 0);
> > +}
> >  
> >  if (!spapr->htab) {
> >  assert(kvm_enabled());
> > @@ -1746,7 +1757,12 @@ static int htab_save_complete(QEMUFile *f, void 
> > *opaque)
> >  int fd;
> >  
> >  /* Iteration header */
> > -qemu_put_be32(f, 0);
> > +if (!spapr->htab_shift) {
> > +qemu_put_be32(f, -1);
> > +return 0;
> > +} else {
> > +qemu_put_be32(f, 0);
> > +}
> >  
> >  if (!spapr->htab) {
> >  int rc;
> > @@ -1790,6 +1806,11 @@ static int htab_load(QEMUFile *f, void *opaque, int 
> > version_id)
> >  
> >  section_hdr = qemu_get_be32(f);
> >  
> > +if (section_hdr == -1) {
> > +spapr_free_hpt(spapr);
> > +return 0;
> > +}
> > +
> >  if (section_hdr) {
> >  Error *local_err = NULL;
> >  
> > 
> 
> It seems there is a bug in this patch: when using from QEMU console the
> command "savevm", it never stops and the qcow2 image grows without limit.
> 
> I think this is because htab_save_iterate() and htab_save_complete()
> should mark the end of the sequence (the empty one, -1) by returning 1
> and not 0 (see kvmppc_save_htab()).

Ah, yes, I think you're right.  The end condition is one of the subtle
differences between the savevm and migration paths.

> This fixes the problem for me:
> 
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index 970093e..fa01511 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -1827,7 +1827,7 @@ static int htab_save_iterate(QEMUFile *f, void
> *opaque)
>  /* Iteration header */
>  if (!spapr->htab_shift) {
>  qemu_put_be32(f, -1);
> -return 0;
> +return 1;
>  } else {
>  qemu_put_be32(f, 0);
>  }
> @@ -1866,7 +1866,7 @@ static int htab_save_complete(QEMUFile *f, void
> *opaque)
>  /* Iteration header */
>  if (!spapr->htab_shift) {
>  qemu_put_be32(f, -1);
> -return 0;
> +return 1;
>  } else {
>  qemu_put_be32(f, 0);
>  }

Can you polish this up and submit for merge, please?

-- 
David Gibson| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au  | minimalist, thank you.  NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson


signature.asc
Description: PGP signature

[Qemu-devel] [PATCH v3 00/10] migration: export cap/params to qdev props

[Peter Xu]

v3:
- add r-b properly on each patch
- patch 1: fix commit message [Eric]
- patch 4: dropped since not used any more [Eduardo]
- patch 7 (new patch 6): move the check from post_init() into
  migration_object_init() [Eduardo]
- patch 10 (new patch 9): rename var "head" -> "list" [Juan]
- patch 11: rebased a lot due to patch 7's change, so removed Juan's r-b

v2:
- extended the series from 3 -> 11 patches
- renamed all the properties with "x-" prefix
- handled the cap/param check for these new properties (mostly patch
  4-11, but it contains lots of refactorings in general)

We have the MigrationState as QDev now (which seems crazy). Let's
continue to benefit.

This series is exporting all migration capabilities/params as global
parameters. Then we can do something like this:

  qemu -global migration.postcopy-ram=true \
   -global migration.max-bandwidth=4096

The values will be inited just like we typed these values into HMP
monitor. It'll simplify lots of migration scripts.

The changes are fairly straightforward. One tiny loss is that we still
don't support:

  -global migration.max-bandwidth=1g

...just like what we did in HMP:

  migrate_set_speed 1g

...while we need to use:

  -global migration.max-bandwidth=1073741824

However that should only be used in scripts, and that's good enough
imho.

These properties should only be used for debugging/testing purpose,
and we should not guarantee any interface compatibility for them (just
like HMP).

Please review. Thanks.

Peter Xu (10):
  qdev: provide DEFINE_PROP_INT64()
  migration: export parameters to props
  migration: export capabilities to props
  migration: introduce migrate_params_check()
  migration: provide migrate_params_apply()
  migration: check global params for validity
  migration: remove check against colo support
  migration: provide migrate_caps_check()
  migration: provide migrate_cap_add()
  migration: check global caps for validity

 hw/core/qdev-properties.c|  32 ++
 include/hw/qdev-properties.h |   3 +
 include/migration/colo.h |   1 -
 migration/colo.c |   5 -
 migration/migration.c| 267 +--
 5 files changed, 242 insertions(+), 66 deletions(-)

-- 
2.7.4

[Qemu-devel] [PATCH v3 01/10] qdev: provide DEFINE_PROP_INT64()

[Peter Xu]

We have nearly all the stuff, but this one is missing. Add it in.

Am going to use this new helper for MigrationParameters fields, since
most of them are int64_t.

CC: Markus Armbruster 
CC: Eduardo Habkost 
CC: Marc-André Lureau 
CC: Peter Xu 
CC: Juan Quintela 
CC: Marcel Apfelbaum 
CC: Eric Blake 
Reviewed-by: Marc-André Lureau 
Reviewed-by: Marcel Apfelbaum 
Reviewed-by: Juan Quintela 
Reviewed-by: Eduardo Habkost 
Signed-off-by: Peter Xu 
---
 hw/core/qdev-properties.c| 32 
 include/hw/qdev-properties.h |  3 +++
 2 files changed, 35 insertions(+)

diff --git a/hw/core/qdev-properties.c b/hw/core/qdev-properties.c
index dcecdf0..c1d4e54 100644
--- a/hw/core/qdev-properties.c
+++ b/hw/core/qdev-properties.c
@@ -404,6 +404,31 @@ static void set_uint64(Object *obj, Visitor *v, const char 
*name,
 visit_type_uint64(v, name, ptr, errp);
 }
 
+static void get_int64(Object *obj, Visitor *v, const char *name,
+  void *opaque, Error **errp)
+{
+DeviceState *dev = DEVICE(obj);
+Property *prop = opaque;
+int64_t *ptr = qdev_get_prop_ptr(dev, prop);
+
+visit_type_int64(v, name, ptr, errp);
+}
+
+static void set_int64(Object *obj, Visitor *v, const char *name,
+  void *opaque, Error **errp)
+{
+DeviceState *dev = DEVICE(obj);
+Property *prop = opaque;
+int64_t *ptr = qdev_get_prop_ptr(dev, prop);
+
+if (dev->realized) {
+qdev_prop_set_after_realize(dev, name, errp);
+return;
+}
+
+visit_type_int64(v, name, ptr, errp);
+}
+
 const PropertyInfo qdev_prop_uint64 = {
 .name  = "uint64",
 .get   = get_uint64,
@@ -411,6 +436,13 @@ const PropertyInfo qdev_prop_uint64 = {
 .set_default_value = set_default_value_uint,
 };
 
+const PropertyInfo qdev_prop_int64 = {
+.name  = "int64",
+.get   = get_int64,
+.set   = set_int64,
+.set_default_value = set_default_value_int,
+};
+
 /* --- string --- */
 
 static void release_string(Object *obj, const char *name, void *opaque)
diff --git a/include/hw/qdev-properties.h b/include/hw/qdev-properties.h
index f6692d5..30af33b 100644
--- a/include/hw/qdev-properties.h
+++ b/include/hw/qdev-properties.h
@@ -13,6 +13,7 @@ extern const PropertyInfo qdev_prop_uint16;
 extern const PropertyInfo qdev_prop_uint32;
 extern const PropertyInfo qdev_prop_int32;
 extern const PropertyInfo qdev_prop_uint64;
+extern const PropertyInfo qdev_prop_int64;
 extern const PropertyInfo qdev_prop_size;
 extern const PropertyInfo qdev_prop_string;
 extern const PropertyInfo qdev_prop_chr;
@@ -136,6 +137,8 @@ extern const PropertyInfo qdev_prop_link;
 DEFINE_PROP_SIGNED(_n, _s, _f, _d, qdev_prop_int32, int32_t)
 #define DEFINE_PROP_UINT64(_n, _s, _f, _d)  \
 DEFINE_PROP_UNSIGNED(_n, _s, _f, _d, qdev_prop_uint64, uint64_t)
+#define DEFINE_PROP_INT64(_n, _s, _f, _d)  \
+DEFINE_PROP_SIGNED(_n, _s, _f, _d, qdev_prop_int64, int64_t)
 #define DEFINE_PROP_SIZE(_n, _s, _f, _d)   \
 DEFINE_PROP_UNSIGNED(_n, _s, _f, _d, qdev_prop_size, uint64_t)
 #define DEFINE_PROP_PCI_DEVFN(_n, _s, _f, _d)   \
-- 
2.7.4

Re: [Qemu-devel] [PATCH qemu] spapr_pci: Fix obsolete comment about MSIX encoding in addr/data

[David Gibson]

On Tue, Jul 18, 2017 at 12:00:33PM +1000, Alexey Kardashevskiy wrote:
> f1c2dc7c866a "spapr-pci: rework MSI/MSIX" (07/2013) changed MSIX encoding
> but forgot to change the comment so this changes it.
> 
> Signed-off-by: Alexey Kardashevskiy 

Applied to ppc-for-2.10, thanks.

> ---
>  hw/ppc/spapr_pci.c | 4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)
> 
> diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c
> index a52dcf8ec0..4b895c5a2a 100644
> --- a/hw/ppc/spapr_pci.c
> +++ b/hw/ppc/spapr_pci.c
> @@ -723,9 +723,7 @@ static PCIINTxRoute spapr_route_intx_pin_to_irq(void 
> *opaque, int pin)
>  /*
>   * MSI/MSIX memory region implementation.
>   * The handler handles both MSI and MSIX.
> - * For MSI-X, the vector number is encoded as a part of the address,
> - * data is set to 0.
> - * For MSI, the vector number is encoded in least bits in data.
> + * The vector number is encoded in least bits in data.
>   */
>  static void spapr_msi_write(void *opaque, hwaddr addr,
>  uint64_t data, unsigned size)

-- 
David Gibson| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au  | minimalist, thank you.  NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson


signature.asc
Description: PGP signature

Re: [Qemu-devel] [PATCH v4 6/6] target/sparc: optimize various functions using extract op

[Philippe Mathieu-Daudé]

On 05/12/2017 09:08 PM, Richard Henderson wrote:

On 05/12/2017 04:38 PM, Philippe Mathieu-Daudé wrote:

[...]

  static inline void gen_op_add_cc(TCGv dst, TCGv src1, TCGv src2)
@@ -638,8 +634,7 @@ static inline void gen_op_mulscc(TCGv dst, TCGv 
src1, TCGv src2)

  // env->y = (b2 << 31) | (env->y >> 1);
  tcg_gen_andi_tl(r_temp, cpu_cc_src, 0x1);
  tcg_gen_shli_tl(r_temp, r_temp, 31);
-tcg_gen_shri_tl(t0, cpu_y, 1);
-tcg_gen_andi_tl(t0, t0, 0x7fff);
+tcg_gen_extract_tl(t0, cpu_y, 1, 31);
  tcg_gen_or_tl(t0, t0, r_temp);
  tcg_gen_andi_tl(cpu_y, t0, 0x);


So this 0x mask is incorrect and should be 0x7fff?



But this should use

   tcg_gen_extract_tl(cpu_y, cpu_y, 1, 31);
   tcg_gen_deposit_tl(cpu_y, cpu_y, cpu_cc_src, 31, 1);


r~

Re: [Qemu-devel] [PATCH v2 10/11] migration: provide migrate_cap_add()

[Peter Xu]

On Mon, Jul 17, 2017 at 07:14:44PM +0200, Juan Quintela wrote:
> Peter Xu  wrote:
> > Abstracted from migrate_set_block_enabled() to allocate
> > MigrationCapabilityStatusList properly.
> >
> > Signed-off-by: Peter Xu 
> 
> Reviewed-by: Juan Quintela 
> 
> 
> Nitpick
> 
> > -void migrate_set_block_enabled(bool value, Error **errp)
> > +static MigrationCapabilityStatusList *migrate_cap_add(
> > +MigrationCapabilityStatusList *head,
> 
> We have a parameter called head
> 
> > +MigrationCapability index,
> > +bool state)
> >  {
> >  MigrationCapabilityStatusList *cap;
> >  
> >  cap = g_new0(MigrationCapabilityStatusList, 1);
> >  cap->value = g_new0(MigrationCapabilityStatus, 1);
> > -cap->value->capability = MIGRATION_CAPABILITY_BLOCK;
> > -cap->value->state = value;
> > +cap->value->capability = index;
> > +cap->value->state = state;
> > +cap->next = head;
> > +
> > +return cap;
> 
> 
> But we don't do the *head = cap?
> 
> Pelhaps is better to call it "next" or "list"?

It's the (old) head, but sure. :)

Thanks,

-- 
Peter Xu

Re: [Qemu-devel] [PATCH 3/8] target/alpha: Merge several flag bytes into ENV->FLAGS

[Richard Henderson]

On 07/17/2017 03:53 PM, Emilio G. Cota wrote:

On Thu, Jul 13, 2017 at 14:18:14 -1000, Richard Henderson wrote:

The flags are arranged such that we can manipulate them either
a whole, or as individual bytes.  The computation within
cpu_get_tb_cpu_state is now reduced to a single load and mask.

Signed-off-by: Richard Henderson 
---
  target/alpha/cpu.h   | 70 +
  hw/alpha/dp264.c |  1 -
  linux-user/main.c| 25 +++--
  target/alpha/cpu.c   |  7 ++--
  target/alpha/helper.c| 12 +++
  target/alpha/machine.c   | 10 ++
  target/alpha/translate.c | 91 +++-
  7 files changed, 117 insertions(+), 99 deletions(-)

diff --git a/target/alpha/cpu.h b/target/alpha/cpu.h
index aa83417..e95be2b 100644
--- a/target/alpha/cpu.h
+++ b/target/alpha/cpu.h
@@ -242,13 +242,11 @@ struct CPUAlphaState {
  uint8_t fpcr_dyn_round;
  uint8_t fpcr_flush_to_zero;
  
-/* The Internal Processor Registers.  Some of these we assume always

-   exist for use in user-mode.  */
-uint8_t ps;
-uint8_t intr_flag;
-uint8_t pal_mode;
-uint8_t fen;
+/* Mask of PALmode, Processor State et al.  Most of this gets copied
+   into the TranslatorBlock flags and controls code generation.  */
+uint32_t flags;


Did you consider doing something like the appended? I don't like it
because it messes with endianness, which is always a pain. But it
lets you preserve the code that only touches the u8's as-is; this
comes at the price of requiring a fast-path swap in big-endian hosts.

An alternative would be to store the u8's in an order dependent on
the endianness of the host -- but that would break vmstate saving,
I guess.

Emilio

---8<---

diff --git a/target/alpha/cpu.h b/target/alpha/cpu.h
index aa83417..22c52ac 100644
--- a/target/alpha/cpu.h
+++ b/target/alpha/cpu.h
@@ -244,10 +244,15 @@ struct CPUAlphaState {
  
  /* The Internal Processor Registers.  Some of these we assume always

 exist for use in user-mode.  */
-uint8_t ps;
-uint8_t intr_flag;
-uint8_t pal_mode;
-uint8_t fen;
+union {
+struct {
+uint8_t pal_mode;
+uint8_t ps;
+uint8_t intr_flag;
+uint8_t fen;
+};
+uint32_t flags;
+};


I did consider this.

Doing things this way requires that I consider host endianness when forming the 
bit masks.  As opposed to only needing to consider host endianness while 
computing byte offsets within the translator.



r~

[Qemu-devel] Question for iotests 188, 189 and 087

[Jing Liu]

Hi all,

Do you anybody have iotests failure: 188, 189 and 087 of the latest qemu 
upsteam?


I just wondered if it has something wrong with my test machines because 
I have different results with two machines.



Thanks.

Jing

[Qemu-devel] [PATCH v2.5 3/5] docker: add debian Ports base image

[Philippe Mathieu-Daudé]

Signed-off-by: Philippe Mathieu-Daudé 
---
 tests/docker/dockerfiles/debian-ports.docker | 35 
 1 file changed, 35 insertions(+)
 create mode 100644 tests/docker/dockerfiles/debian-ports.docker

diff --git a/tests/docker/dockerfiles/debian-ports.docker 
b/tests/docker/dockerfiles/debian-ports.docker
new file mode 100644
index 00..fba224f760
--- /dev/null
+++ b/tests/docker/dockerfiles/debian-ports.docker
@@ -0,0 +1,35 @@
+#
+# Docker multiarch cross-compiler target
+#
+# This docker target is builds on Debian Ports cross compiler targets
+# to build distro with a selection of cross compilers for building test 
binaries.
+#
+# On its own you can't build much but the docker-foo-cross targets
+# build on top of the base debian image.
+#
+FROM debian:unstable
+
+MAINTAINER Philippe Mathieu-Daudé 
+
+RUN echo "deb [arch=amd64] http://deb.debian.org/debian unstable main" > 
/etc/apt/sources.list
+
+# Duplicate deb line as deb-src
+RUN cat /etc/apt/sources.list | sed -ne "s/^deb\ \(\[.*\]\ \)\?\(.*\)/deb-src 
\2/p" >> /etc/apt/sources.list
+
+# Setup some basic tools we need
+RUN apt-get update && \
+DEBIAN_FRONTEND=noninteractive apt-get install -yy eatmydata
+RUN DEBIAN_FRONTEND=noninteractive eatmydata \
+apt-get install -y --no-install-recommends \
+bison \
+build-essential \
+ca-certificates \
+clang \
+debian-ports-archive-keyring \
+flex \
+git \
+pkg-config \
+psmisc \
+python \
+texinfo \
+$(apt-get -s build-dep qemu | egrep ^Inst | fgrep '[all]' | cut -d\  
-f2)
-- 
2.13.2

[Qemu-devel] [PATCH v2.5 4/5] docker: warn users to use newer debian8/debian9 base image

[Philippe Mathieu-Daudé]

to stay backward incompatible.

Signed-off-by: Philippe Mathieu-Daudé 
---
 tests/docker/Makefile.include  |  1 +
 tests/docker/dockerfiles/debian.docker | 13 +
 2 files changed, 14 insertions(+)
 create mode 100644 tests/docker/dockerfiles/debian.docker

diff --git a/tests/docker/Makefile.include b/tests/docker/Makefile.include
index e993e149e7..aaab1a4208 100644
--- a/tests/docker/Makefile.include
+++ b/tests/docker/Makefile.include
@@ -58,6 +58,7 @@ docker-image-%: $(DOCKER_FILES_DIR)/%.docker
 docker-image-debian-powerpc-cross: 
EXTRA_FILES:=tests/docker/dockerfiles/debian-apt-fake.sh
 
 # Enforce dependancies for composite images
+docker-image-debian: docker-image-debian9
 docker-image-debian8-mxe: docker-image-debian8
 docker-image-debian-amd64: docker-image-debian9
 docker-image-debian-armel-cross: docker-image-debian9
diff --git a/tests/docker/dockerfiles/debian.docker 
b/tests/docker/dockerfiles/debian.docker
new file mode 100644
index 00..fd32e71b79
--- /dev/null
+++ b/tests/docker/dockerfiles/debian.docker
@@ -0,0 +1,13 @@
+# This template is deprecated and was previously based on Jessie on QEMU 2.9.
+# Now than Stretch is out, please use qemu:debian8 as base for Jessie,
+# and qemu:debian9 for Stretch.
+#
+FROM qemu:debian9
+
+MAINTAINER Philippe Mathieu-Daudé 
+
+RUN for n in $(seq 8); do echo; done && \
+echo "\n\t\tThis image is deprecated." && echo && \
+echo "\tUse 'FROM qemu:debian9' to use the stable Debian Stretch image" && 
\
+echo "\tor 'FROM qemu:debian8' to use old Debian Jessie." && \
+for n in $(seq 8); do echo; done
-- 
2.13.2

[Qemu-devel] [PATCH v2.5 1/5] docker: add MXE (M cross environment) base image for MinGW-w64

[Philippe Mathieu-Daudé]

see http://mxe.cc/

Signed-off-by: Philippe Mathieu-Daudé 
---
 tests/docker/Makefile.include  |  3 ++
 tests/docker/dockerfiles/debian-win32-cross.docker | 32 ++
 tests/docker/dockerfiles/debian-win64-cross.docker | 32 ++
 tests/docker/dockerfiles/debian8-mxe.docker| 18 
 4 files changed, 85 insertions(+)
 create mode 100644 tests/docker/dockerfiles/debian-win32-cross.docker
 create mode 100644 tests/docker/dockerfiles/debian-win64-cross.docker
 create mode 100644 tests/docker/dockerfiles/debian8-mxe.docker

diff --git a/tests/docker/Makefile.include b/tests/docker/Makefile.include
index 5a8283674a..e993e149e7 100644
--- a/tests/docker/Makefile.include
+++ b/tests/docker/Makefile.include
@@ -58,6 +58,7 @@ docker-image-%: $(DOCKER_FILES_DIR)/%.docker
 docker-image-debian-powerpc-cross: 
EXTRA_FILES:=tests/docker/dockerfiles/debian-apt-fake.sh
 
 # Enforce dependancies for composite images
+docker-image-debian8-mxe: docker-image-debian8
 docker-image-debian-amd64: docker-image-debian9
 docker-image-debian-armel-cross: docker-image-debian9
 docker-image-debian-armhf-cross: docker-image-debian9
@@ -67,6 +68,8 @@ docker-image-debian-mips64el-cross: docker-image-debian9
 docker-image-debian-powerpc-cross: docker-image-debian8
 docker-image-debian-ppc64el-cross: docker-image-debian9
 docker-image-debian-s390x-cross: docker-image-debian9
+docker-image-debian-win32-cross: docker-image-debian8-mxe
+docker-image-debian-win64-cross: docker-image-debian8-mxe
 
 # Expand all the pre-requistes for each docker image and test combination
 $(foreach i,$(DOCKER_IMAGES), \
diff --git a/tests/docker/dockerfiles/debian-win32-cross.docker 
b/tests/docker/dockerfiles/debian-win32-cross.docker
new file mode 100644
index 00..dd021f2df0
--- /dev/null
+++ b/tests/docker/dockerfiles/debian-win32-cross.docker
@@ -0,0 +1,32 @@
+#
+# Docker mingw32 cross-compiler target
+#
+# This docker target builds on the debian Jessie MXE base image.
+#
+FROM qemu:debian8-mxe
+
+MAINTAINER Philippe Mathieu-Daudé 
+
+ENV TARGET i686
+
+RUN DEBIAN_FRONTEND=noninteractive eatmydata \
+apt-get install -y --no-install-recommends \
+mxe-$TARGET-w64-mingw32.shared-bzip2 \
+mxe-$TARGET-w64-mingw32.shared-curl \
+mxe-$TARGET-w64-mingw32.shared-glib \
+mxe-$TARGET-w64-mingw32.shared-libgcrypt \
+mxe-$TARGET-w64-mingw32.shared-libssh2 \
+mxe-$TARGET-w64-mingw32.shared-libusb1 \
+mxe-$TARGET-w64-mingw32.shared-lzo \
+mxe-$TARGET-w64-mingw32.shared-nettle \
+mxe-$TARGET-w64-mingw32.shared-ncurses \
+mxe-$TARGET-w64-mingw32.shared-pixman \
+mxe-$TARGET-w64-mingw32.shared-pkgconf \
+mxe-$TARGET-w64-mingw32.shared-pthreads \
+mxe-$TARGET-w64-mingw32.shared-sdl2 \
+mxe-$TARGET-w64-mingw32.shared-sdl2-mixer \
+mxe-$TARGET-w64-mingw32.shared-sdl2-gfx \
+mxe-$TARGET-w64-mingw32.shared-zlib
+
+# Specify the cross prefix for this image (see tests/docker/common.rc)
+ENV QEMU_CONFIGURE_OPTS --cross-prefix=$TARGET-w64-mingw32.shared-
diff --git a/tests/docker/dockerfiles/debian-win64-cross.docker 
b/tests/docker/dockerfiles/debian-win64-cross.docker
new file mode 100644
index 00..4542bcc821
--- /dev/null
+++ b/tests/docker/dockerfiles/debian-win64-cross.docker
@@ -0,0 +1,32 @@
+#
+# Docker mingw64 cross-compiler target
+#
+# This docker target builds on the debian Jessie MXE base image.
+#
+FROM qemu:debian8-mxe
+
+MAINTAINER Philippe Mathieu-Daudé 
+
+ENV TARGET x86-64
+
+RUN DEBIAN_FRONTEND=noninteractive eatmydata \
+apt-get install -y --no-install-recommends \
+mxe-$TARGET-w64-mingw32.shared-bzip2 \
+mxe-$TARGET-w64-mingw32.shared-curl \
+mxe-$TARGET-w64-mingw32.shared-glib \
+mxe-$TARGET-w64-mingw32.shared-libgcrypt \
+mxe-$TARGET-w64-mingw32.shared-libssh2 \
+mxe-$TARGET-w64-mingw32.shared-libusb1 \
+mxe-$TARGET-w64-mingw32.shared-lzo \
+mxe-$TARGET-w64-mingw32.shared-nettle \
+mxe-$TARGET-w64-mingw32.shared-ncurses \
+mxe-$TARGET-w64-mingw32.shared-pixman \
+mxe-$TARGET-w64-mingw32.shared-pkgconf \
+mxe-$TARGET-w64-mingw32.shared-pthreads \
+mxe-$TARGET-w64-mingw32.shared-sdl2 \
+mxe-$TARGET-w64-mingw32.shared-sdl2-mixer \
+mxe-$TARGET-w64-mingw32.shared-sdl2-gfx \
+mxe-$TARGET-w64-mingw32.shared-zlib
+
+# Specify the cross prefix for this image (see tests/docker/common.rc)
+ENV QEMU_CONFIGURE_OPTS --cross-prefix=x86_64-w64-mingw32.shared-
diff --git a/tests/docker/dockerfiles/debian8-mxe.docker 
b/tests/docker/dockerfiles/debian8-mxe.docker
new file mode 100644
index 00..7bf1b59e54
--- /dev/null
+++ b/tests/docker/dockerfiles/debian8-mxe.docker
@@ -0,0 +1,18 @@
+#
+# Docker mingw cross-compiler target
+#
+# This docker target builds on the debian Jessie 

[Qemu-devel] [PATCH v2.5 5/5] docker: install clang since Shippable setup_ve() verify it is available

[Philippe Mathieu-Daudé]

Signed-off-by: Philippe Mathieu-Daudé 
---
 tests/docker/dockerfiles/debian9.docker | 1 +
 1 file changed, 1 insertion(+)

diff --git a/tests/docker/dockerfiles/debian9.docker 
b/tests/docker/dockerfiles/debian9.docker
index 056e5389cc..a4509950e6 100644
--- a/tests/docker/dockerfiles/debian9.docker
+++ b/tests/docker/dockerfiles/debian9.docker
@@ -20,6 +20,7 @@ RUN DEBIAN_FRONTEND=noninteractive eatmydata \
 bison \
 build-essential \
 ca-certificates \
+clang \
 flex \
 git \
 pkg-config \
-- 
2.13.2

[Qemu-devel] [PATCH v2.5 2/5] shippable: add win32/64 targets

[Philippe Mathieu-Daudé]

Signed-off-by: Philippe Mathieu-Daudé 
---
 .shippable.yml | 4 
 1 file changed, 4 insertions(+)

diff --git a/.shippable.yml b/.shippable.yml
index 53b43b349f..dd4bbc84b1 100644
--- a/.shippable.yml
+++ b/.shippable.yml
@@ -7,6 +7,10 @@ env:
   matrix:
 - IMAGE=debian-amd64
   TARGET_LIST=x86_64-softmmu,x86_64-linux-user
+- IMAGE=debian-win32-cross
+  TARGET_LIST=arm-softmmu,i386-softmmu,lm32-softmmu
+- IMAGE=debian-win64-cross
+  TARGET_LIST=aarch64-softmmu,sparc64-softmmu,x86_64-softmmu
 - IMAGE=debian-armel-cross
   TARGET_LIST=arm-softmmu,arm-linux-user,armeb-linux-user
 - IMAGE=debian-armhf-cross
-- 
2.13.2

[Qemu-devel] [PATCH v2.5 0/5] (more) Updated Travis Queue

[Philippe Mathieu-Daudé]

Sorry I messed previous series... just updated last 5 patches of v2.

v2.5:
- MXE replaced  by s
- docker9/ports: lost package 'clang' re-added
- include patch keep 'debian.docker' base image backward incompatible 

On 07/17/2017 09:31 PM, Philippe Mathieu-Daudé wrote:
> Hi,
> 
> a bit late, but in case you can take some for 2.10:
> 
> v2:
> - cleanup few packages (to improve image caching)
> - dropped mipsEL and keep mipsEB image (32-bit big endian)
> - add mips64EL (64-bit little endian)
> - add mingw32 and mingw64 targets (shared libs, could be static)
> - add debian Ports image [can be dropped, use as base for SH4 for 2.11]
> 
> On 07/17/2017 11:48 AM, Alex Bennée wrote:
>> Hi,
>>
>> This is the current status of the travis/next patch queue. The
>> includes updates from Paolo to allow parallelism while testing in the
>> docker environment. I've extended the travis image so we can actually
>> run our travis.py script in the Travis image.
>>
>> There are also a number of updates from Phillipe which add a bunch of
>> additional cross compile targets to our shippable setup. The cachinfo
>> patch is temporary and won't make the pull as it is already queued in
>> Richard's tcg-next.
>>
>> I'm currently trying to catch one of our Travis hangs in the act
>> (postcopy-test) but it seems to be very much a heavy load race
>> condition which annoyingly stops happening once you try and get
>> debugging tools on it. This is the reason I've updated the travis
>> docker image to include the debug tools;-)
>>
>> As long as there are no screams of outrage I'll roll a pullreq for
>> softfreeze tomorrow.

Philippe Mathieu-Daudé (5):
  docker: add MXE (M cross environment) base image for MinGW-w64
  shippable: add win32/64 targets
  docker: add debian Ports base image
  docker: warn users to use newer debian8/debian9 base image
  docker: install clang since Shippable setup_ve() verify it is
available

 .shippable.yml |  4 +++
 tests/docker/Makefile.include  |  4 +++
 tests/docker/dockerfiles/debian-ports.docker   | 35 ++
 tests/docker/dockerfiles/debian-win32-cross.docker | 32 
 tests/docker/dockerfiles/debian-win64-cross.docker | 32 
 tests/docker/dockerfiles/debian.docker | 13 
 tests/docker/dockerfiles/debian8-mxe.docker| 18 +++
 tests/docker/dockerfiles/debian9.docker|  1 +
 8 files changed, 139 insertions(+)
 create mode 100644 tests/docker/dockerfiles/debian-ports.docker
 create mode 100644 tests/docker/dockerfiles/debian-win32-cross.docker
 create mode 100644 tests/docker/dockerfiles/debian-win64-cross.docker
 create mode 100644 tests/docker/dockerfiles/debian.docker
 create mode 100644 tests/docker/dockerfiles/debian8-mxe.docker

-- 
2.13.2

Re: [Qemu-devel] [PATCH 2/2] vfio/ccw: fix initialization of the Object DeviceState pointer in the common base-device

[Dong Jia Shi]

* Alex Williamson  [2017-07-17 20:26:53 -0600]:

> On Tue, 18 Jul 2017 03:49:26 +0200
> Dong Jia Shi  wrote:
> 
> > Commit 7da624e2 ("vfio: Test realized when using VFIOGroup.device_list
> > iterator") introduced a pointer to the Object DeviceState in the VFIO
> > common base-device and skiped non-realized devices as we iterate
> > VFIOGroup.device_list. While it missed to initialize the pointer for
> > the vfio-ccw case. Let's fix it.
> > 
> > Fixes: 7da624e2 ("vfio: Test realized when using VFIOGroup.device_list
> >   iterator")
> 
> Sorry for that.
No problem.

> 
> Reviewed-by: Alex Williamson 
Thanks! :D

> 
> > Cc: Alex Williamson 
> > Reviewed-by: Halil Pasic 
> > Signed-off-by: Dong Jia Shi 
> > ---
> >  hw/vfio/ccw.c | 1 +
> >  1 file changed, 1 insertion(+)
> > 
> > diff --git a/hw/vfio/ccw.c b/hw/vfio/ccw.c
> > index 8d97b53e77..a8baadf57a 100644
> > --- a/hw/vfio/ccw.c
> > +++ b/hw/vfio/ccw.c
> > @@ -338,6 +338,7 @@ static void vfio_ccw_realize(DeviceState *dev, Error 
> > **errp)
> >  vcdev->vdev.type = VFIO_DEVICE_TYPE_CCW;
> >  vcdev->vdev.name = g_strdup_printf("%x.%x.%04x", cdev->hostid.cssid,
> > cdev->hostid.ssid, 
> > cdev->hostid.devid);
> > +vcdev->vdev.dev = dev;
> >  QLIST_FOREACH(vbasedev, >device_list, next) {
> >  if (strcmp(vbasedev->name, vcdev->vdev.name) == 0) {
> >  error_setg(, "vfio: subchannel %s has already been 
> > attached",
> 

-- 
Dong Jia Shi

Re: [Qemu-devel] [PATCH v2 07/11] migration: check global params for validity

[Peter Xu]

On Mon, Jul 17, 2017 at 11:33:55PM -0300, Eduardo Habkost wrote:
> On Tue, Jul 18, 2017 at 09:56:06AM +0800, Peter Xu wrote:
> > On Mon, Jul 17, 2017 at 03:25:05PM -0300, Eduardo Habkost wrote:
> > > On Mon, Jul 17, 2017 at 04:26:07PM +0800, Peter Xu wrote:
> > > > Adding validity check for the migration parameters passed in via global
> > > > properties.
> > > > 
> > > > Signed-off-by: Peter Xu 
> > > > ---
> > > >  migration/migration.c | 34 ++
> > > >  1 file changed, 34 insertions(+)
> > > > 
> > > > diff --git a/migration/migration.c b/migration/migration.c
> > > > index 8c65054..5a7f22c 100644
> > > > --- a/migration/migration.c
> > > > +++ b/migration/migration.c
> > > > @@ -2109,6 +2109,39 @@ static void migration_instance_init(Object *obj)
> > > >  ms->parameters.tls_hostname = g_strdup("");
> > > >  }
> > > >  
> > > > +static void migration_instance_post_init(Object *obj)
> > > > +{
> > > > +MigrationState *ms = (MigrationState *)obj;
> > > > +Error *err = NULL;
> > > > +MigrationParameters params = {
> > > > +.has_compress_level = true,
> > > > +.compress_level = ms->parameters.compress_level,
> > > > +.has_compress_threads = true,
> > > > +.compress_threads = ms->parameters.compress_threads,
> > > > +.has_decompress_threads = true,
> > > > +.decompress_threads = ms->parameters.decompress_threads,
> > > > +.has_cpu_throttle_initial = true,
> > > > +.cpu_throttle_initial = ms->parameters.cpu_throttle_initial,
> > > > +.has_cpu_throttle_increment = true,
> > > > +.cpu_throttle_increment = 
> > > > ms->parameters.cpu_throttle_increment,
> > > > +.has_max_bandwidth = true,
> > > > +.max_bandwidth = ms->parameters.max_bandwidth,
> > > > +.has_downtime_limit = true,
> > > > +.downtime_limit = ms->parameters.downtime_limit,
> > > > +.has_x_checkpoint_delay = true,
> > > > +.x_checkpoint_delay = ms->parameters.x_checkpoint_delay,
> > > > +.has_block_incremental = true,
> > > > +.block_incremental = ms->parameters.block_incremental,
> > > > +};
> > > > +
> > > > +/* We have applied all the migration properties... */
> > > > +
> > > > +if (!migrate_params_check(, )) {
> > > 
> > > Why not just:
> > >   if (!migrate_params_check(>parameters, ))
> > > ?
> > > 
> > > If the ms->parameters.has_* fields are not set to true anywhere,
> > > we can set them to true in instance_init.
> > 
> > Sure.
> > 
> > > 
> > > (This would also also us to use QAPI_CLONE in
> > > qmp_query_migrate_parameters() instead of manually copying each
> > > field.
> > > 
> > > 
> > > > +error_report_err(err);
> > > > +exit(1);
> > > > +}
> > > > +}
> > > 
> > > On real devices, this is normally done on realize (which has
> > > proper error reporting).  We never realize the TYPE_MIGRATION
> > > object because it's not a real device, though.
> > 
> > Hmm...
> > 
> > > 
> > > Doing error checks on post_init feels fragile, as the only way
> > > errors can be handled is triggering exit(1) in the middle of an
> > > object_new() call.
> > > 
> > > As we have only one place where the TYPE_MIGRATION object is
> > > created, I suggest calling migrate_params_check() inside
> > > migration_object_init().
> > 
> > ... How about I just provide a realize() for it? Then I can drop the
> > QOM patch 4, also, I can keep the checks along with the object itself.
> 
> qdev does lots of other stuff at realize time (e.g. it adds the
> device to the device tree), and I don't think we want to trigger
> that.

Ok. Then let me take your suggestion to move this to
migration_object_init().  Thanks,

-- 
Peter Xu

Re: [Qemu-devel] [PATCH v2 07/11] migration: check global params for validity

[Eduardo Habkost]

On Tue, Jul 18, 2017 at 09:56:06AM +0800, Peter Xu wrote:
> On Mon, Jul 17, 2017 at 03:25:05PM -0300, Eduardo Habkost wrote:
> > On Mon, Jul 17, 2017 at 04:26:07PM +0800, Peter Xu wrote:
> > > Adding validity check for the migration parameters passed in via global
> > > properties.
> > > 
> > > Signed-off-by: Peter Xu 
> > > ---
> > >  migration/migration.c | 34 ++
> > >  1 file changed, 34 insertions(+)
> > > 
> > > diff --git a/migration/migration.c b/migration/migration.c
> > > index 8c65054..5a7f22c 100644
> > > --- a/migration/migration.c
> > > +++ b/migration/migration.c
> > > @@ -2109,6 +2109,39 @@ static void migration_instance_init(Object *obj)
> > >  ms->parameters.tls_hostname = g_strdup("");
> > >  }
> > >  
> > > +static void migration_instance_post_init(Object *obj)
> > > +{
> > > +MigrationState *ms = (MigrationState *)obj;
> > > +Error *err = NULL;
> > > +MigrationParameters params = {
> > > +.has_compress_level = true,
> > > +.compress_level = ms->parameters.compress_level,
> > > +.has_compress_threads = true,
> > > +.compress_threads = ms->parameters.compress_threads,
> > > +.has_decompress_threads = true,
> > > +.decompress_threads = ms->parameters.decompress_threads,
> > > +.has_cpu_throttle_initial = true,
> > > +.cpu_throttle_initial = ms->parameters.cpu_throttle_initial,
> > > +.has_cpu_throttle_increment = true,
> > > +.cpu_throttle_increment = ms->parameters.cpu_throttle_increment,
> > > +.has_max_bandwidth = true,
> > > +.max_bandwidth = ms->parameters.max_bandwidth,
> > > +.has_downtime_limit = true,
> > > +.downtime_limit = ms->parameters.downtime_limit,
> > > +.has_x_checkpoint_delay = true,
> > > +.x_checkpoint_delay = ms->parameters.x_checkpoint_delay,
> > > +.has_block_incremental = true,
> > > +.block_incremental = ms->parameters.block_incremental,
> > > +};
> > > +
> > > +/* We have applied all the migration properties... */
> > > +
> > > +if (!migrate_params_check(, )) {
> > 
> > Why not just:
> >   if (!migrate_params_check(>parameters, ))
> > ?
> > 
> > If the ms->parameters.has_* fields are not set to true anywhere,
> > we can set them to true in instance_init.
> 
> Sure.
> 
> > 
> > (This would also also us to use QAPI_CLONE in
> > qmp_query_migrate_parameters() instead of manually copying each
> > field.
> > 
> > 
> > > +error_report_err(err);
> > > +exit(1);
> > > +}
> > > +}
> > 
> > On real devices, this is normally done on realize (which has
> > proper error reporting).  We never realize the TYPE_MIGRATION
> > object because it's not a real device, though.
> 
> Hmm...
> 
> > 
> > Doing error checks on post_init feels fragile, as the only way
> > errors can be handled is triggering exit(1) in the middle of an
> > object_new() call.
> > 
> > As we have only one place where the TYPE_MIGRATION object is
> > created, I suggest calling migrate_params_check() inside
> > migration_object_init().
> 
> ... How about I just provide a realize() for it? Then I can drop the
> QOM patch 4, also, I can keep the checks along with the object itself.

qdev does lots of other stuff at realize time (e.g. it adds the
device to the device tree), and I don't think we want to trigger
that.

-- 
Eduardo

Re: [Qemu-devel] [PATCH 2/2] vfio/ccw: fix initialization of the Object DeviceState pointer in the common base-device

[Alex Williamson]

On Tue, 18 Jul 2017 03:49:26 +0200
Dong Jia Shi  wrote:

> Commit 7da624e2 ("vfio: Test realized when using VFIOGroup.device_list
> iterator") introduced a pointer to the Object DeviceState in the VFIO
> common base-device and skiped non-realized devices as we iterate
> VFIOGroup.device_list. While it missed to initialize the pointer for
> the vfio-ccw case. Let's fix it.
> 
> Fixes: 7da624e2 ("vfio: Test realized when using VFIOGroup.device_list
>   iterator")

Sorry for that.

Reviewed-by: Alex Williamson 

> Cc: Alex Williamson 
> Reviewed-by: Halil Pasic 
> Signed-off-by: Dong Jia Shi 
> ---
>  hw/vfio/ccw.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/hw/vfio/ccw.c b/hw/vfio/ccw.c
> index 8d97b53e77..a8baadf57a 100644
> --- a/hw/vfio/ccw.c
> +++ b/hw/vfio/ccw.c
> @@ -338,6 +338,7 @@ static void vfio_ccw_realize(DeviceState *dev, Error 
> **errp)
>  vcdev->vdev.type = VFIO_DEVICE_TYPE_CCW;
>  vcdev->vdev.name = g_strdup_printf("%x.%x.%04x", cdev->hostid.cssid,
> cdev->hostid.ssid, 
> cdev->hostid.devid);
> +vcdev->vdev.dev = dev;
>  QLIST_FOREACH(vbasedev, >device_list, next) {
>  if (strcmp(vbasedev->name, vcdev->vdev.name) == 0) {
>  error_setg(, "vfio: subchannel %s has already been attached",

Re: [Qemu-devel] Disable image locking for snapshot drive?

[Andrew Baumann via Qemu-devel]

> From: John Snow [mailto:js...@redhat.com]
> Sent: Monday, 17 July 2017 17:34
> On 07/17/2017 07:30 PM, Andrew Baumann via Qemu-devel wrote:
> > Hi all,
> >
> > I'm running a recent Linux build of qemu on Windows Subsystem for Linux
> (WSL) which doesn't appear to implement file locking:
> >
> > $ qemu-system-aarch64 ... -drive file=test.vhdx,if=none,id=hd0 -device
> virtio-blk-pci,drive=hd0
> > qemu-system-aarch64: -drive file=test.vhdx,if=none,id=hd0: Failed to
> unlock byte 100
> > qemu-system-aarch64: -drive file=test.vhdx,if=none,id=hd0: Failed to
> unlock byte 100
> > qemu-system-aarch64: -drive file=test.vhdx,if=none,id=hd0: Failed to lock
> byte 100
> >
> > That's no big deal; I can switch it off:
> >
> > $ qemu-system-aarch64 ... -drive
> file=test.vhdx,if=none,file.locking=off,id=hd0 ...
> > (all good)
> >
> > But how can I do the same for a snapshot drive?
> >
> > $ qemu-system-aarch64 ... -drive
> file=test.vhdx,if=none,file.locking=off,id=hd0 -snapshot ...
> > qemu-system-aarch64: -drive
> file=test.vhdx,if=none,file.locking=off,id=hd0: Failed to unlock byte 100
> > qemu-system-aarch64: -drive
> file=test.vhdx,if=none,file.locking=off,id=hd0: Failed to unlock byte 100
> > qemu-system-aarch64: -drive
> file=test.vhdx,if=none,file.locking=off,id=hd0: Could not create temporary
> overlay '/var/tmp/vl.o83dxn': Failed to lock byte 100
> >
> > (I also tried the snapshot=on drive option with similar results.)
> >
> > Thanks,
> > Andrew
> >
> 
> Looks like the shorthand "-snapshot" doesn't let you specify any further
> options, which is a bummer.
> 
> You may need to do something a little more manual, and create your own
> temporary overlay, and launch QEMU pointing to that overlay instead.

Can you give me some more clues what this might look like?

> That sounds like a bit of a hassle.
> 
> Can we compile locking support out of QEMU instead for this platform? Or
> is there a runtime option for disabling it globally?

The compile target is just Linux, so at best it would need to be a runtime 
choice based on platform detection, and I doubt that is a good idea (WSL may 
well get around to implementing this syscall in the future). I agree a runtime 
command-line flag to disable it globally would be ideal, but from a quick look 
at the code it doesn't seem to exist at present.

Thanks,
Andrew

Re: [Qemu-devel] [PATCH V2 1/4] net/colo-compare.c: Add checkpoint min period to optimize performance

[Zhang Chen]

On 07/17/2017 08:24 PM, Dr. David Alan Gilbert wrote:

* Zhang Chen (zhangchen.f...@cn.fujitsu.com) wrote:


On 07/14/2017 08:10 PM, Dr. David Alan Gilbert wrote:

* Zhang Chen (zhangchen.f...@cn.fujitsu.com) wrote:

If colo-compare find out the first different packet that means
the following packet almost is different. we needn't do a lot
of checkpoint in this time, so we set the no-need-checkpoint
peroid, default just set 3 second.

Signed-off-by: Zhang Chen 
---
   net/colo-compare.c | 13 -
   1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/net/colo-compare.c b/net/colo-compare.c
index 6d500e1..0f8e198 100644
--- a/net/colo-compare.c
+++ b/net/colo-compare.c
@@ -40,6 +40,9 @@
   /* TODO: Should be configurable */
   #define REGULAR_PACKET_CHECK_MS 3000
+/* TODO: Should be configurable */

Yes it should!


+#define CHECKPOINT_MIN_TIME 3000
+
   /*
 + CompareState ++
 |   |
@@ -455,6 +458,7 @@ static void colo_compare_connection(void *opaque, void 
*user_data)
   Packet *pkt = NULL;
   GList *result = NULL;
   int ret;
+static int64_t checkpoint_time_ms;
   while (!g_queue_is_empty(>primary_list) &&
  !g_queue_is_empty(>secondary_list)) {
@@ -494,7 +498,14 @@ static void colo_compare_connection(void *opaque, void 
*user_data)
*/
   trace_colo_compare_main("packet different");
   g_queue_push_tail(>primary_list, pkt);
-/* TODO: colo_notify_checkpoint();*/
+
+if (pkt->creation_ms - checkpoint_time_ms > CHECKPOINT_MIN_TIME) {
+/*
+ * TODO: Notify colo frame to do checkpoint.
+ * colo_compare_inconsistent_notify();
+ */
+checkpoint_time_ms = pkt->creation_ms;
+}

You need to be careful how this interacts with the actual start of the
checkpoint.   Lets say you have two miscompared packets close to each
other:


  miscompare!
   checkpoint
  miscompare!
   ignore it because it was close to the 1st one

 That means we never trigger the 2nd checkpoint and it'll carry on
until the maximum checkpoint length.

 But also, I think you need to consider what happens to future packets
being compared; you can't release any packets now until the checkpoint
as soon as you know there's a miscompare.

We need some time to do the checkpoint, and in this period we can ignore
the miscompare to get better performance. Like that:

currently:

 miscompare!
  notify checkpoint
 miscompare!
  notify checkpoint
 miscompare!
  notify checkpoint
 miscompare!
  notify checkpoint
 vm_stop and do checkpoint

 vm_start and finish checkpoint

 vm_stop and do checkpoint

 vm_start and finish checkpoint

 vm_stop and do checkpoint

 vm_start and finish checkpoint

 vm_stop and do checkpoint

 vm_start and finish checkpoint


running normally.


after:

 miscompare!
  notify checkpoint
 miscompare!
  ignore
 miscompare!
  ignore
 miscompare!
  ignore
 vm_stop and do checkpoint

 vm_start and finish checkpoint

running normally.

Yes, but you must make sure that you don't
ignore any miscompares after the start of the next checkpoint - I don't
see how you avoid that.


Good catch, I will fix it in next version.



Also we must be careful about packets released after the 1st miscompare.


Yes, after the 1st miscompare, all ignored packet will be enqueued.
Then, we will flush all packet in the queue during do checkpoint.

Thanks
Zhang Chen



Dave



Thanks
Zhang Chen



Dave


   break;
   }
   }
--
2.7.4





--
Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK


.


--
Thanks
Zhang Chen




--
Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK


.



--
Thanks
Zhang Chen

Re: [Qemu-devel] [PATCH v12 6/8] mm: support reporting free page blocks

[Wei Wang]

On 07/17/2017 11:24 PM, Michal Hocko wrote:

On Fri 14-07-17 22:17:13, Michael S. Tsirkin wrote:

On Fri, Jul 14, 2017 at 02:30:23PM +0200, Michal Hocko wrote:

On Wed 12-07-17 20:40:19, Wei Wang wrote:

This patch adds support for reporting blocks of pages on the free list
specified by the caller.

As pages can leave the free list during this call or immediately
afterwards, they are not guaranteed to be free after the function
returns. The only guarantee this makes is that the page was on the free
list at some point in time after the function has been invoked.

Therefore, it is not safe for caller to use any pages on the returned
block or to discard data that is put there after the function returns.
However, it is safe for caller to discard data that was in one of these
pages before the function was invoked.

I do not understand what is the point of such a function and how it is
used because the patch doesn't give us any user (I haven't checked other
patches yet).

But just from the semantic point of view this sounds like a horrible
idea. The only way to get a free block of pages is to call the page
allocator. I am tempted to give it Nack right on those grounds but I
would like to hear more about what you actually want to achieve.

Basically it's a performance hint to the hypervisor.
For example, these pages would be good candidates to
move around as they are not mapped into any running
applications.

As such, it's important not to slow down other parts of the system too
much - otherwise we are speeding up one part of the system while we slow
down other parts of it, which is why it's trying to drop the lock as
soon a possible.



Probably I should have included the introduction of the usages in
the log. Hope it is not too later to explain here:

Live migration needs to transfer the VM's memory from the source
machine to the destination round by round. For the 1st round, all the VM's
memory is transferred. From the 2nd round, only the pieces of memory
that were written by the guest (after the 1st round) are transferred. One
method that is popularly used by the hypervisor to track which part of
memory is written is to write-protect all the guest memory.

This patch enables the optimization of the 1st round memory transfer -
the hypervisor can skip the transfer of guest unused pages in the 1st round.
It is not concerned that the memory pages are used after they are given to
the hypervisor as a hint of the unused pages, because they will be tracked
by the hypervisor and transferred in the next round if they are used and
written.



So why cannot you simply allocate those page and then do whatever you
need. You can tell the page allocator to do only a lightweight
allocation by the gfp_mask - e.g. GFP_NOWAIT or if you even do not want
to risk kswapd intervening then 0 mask.



Here are the 2 reasons that we can't get the hint of unused pages by 
allocating

them:

1) It's expected that live migration shouldn't affect the things running 
inside
the VM - take away all the free pages from the guest would greatly slow 
down the
activities inside guest (e.g. the network transmission may be stuck due 
to the lack of

sk_buf).

2) The hint of free pages are used to optimize the 1st round memory 
transfer, so the hint
is expect to be gotten by the hypervisor as quick as possible. Depending 
on the memory
size of the guest, allocation of all the free memory would be too long 
for the case.


Hope it clarifies the use case.



As long as hypervisor does not assume it can drop these pages, and as
long it's correct in most cases.  we are OK even if the hint is slightly
wrong because hypervisor notifications are racing with allocations.

But the page could have been reused anytime after the lock is dropped
and you cannot check for that except for elevating the reference count.


As also introduced above, the hypervisor uses a dirty page logging mechanism
to track which memory page is written by the guest when live migration 
begins.



Best,
Wei

[Qemu-devel] [PATCH qemu] spapr_pci: Fix obsolete comment about MSIX encoding in addr/data

[Alexey Kardashevskiy]

f1c2dc7c866a "spapr-pci: rework MSI/MSIX" (07/2013) changed MSIX encoding
but forgot to change the comment so this changes it.

Signed-off-by: Alexey Kardashevskiy 
---
 hw/ppc/spapr_pci.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c
index a52dcf8ec0..4b895c5a2a 100644
--- a/hw/ppc/spapr_pci.c
+++ b/hw/ppc/spapr_pci.c
@@ -723,9 +723,7 @@ static PCIINTxRoute spapr_route_intx_pin_to_irq(void 
*opaque, int pin)
 /*
  * MSI/MSIX memory region implementation.
  * The handler handles both MSI and MSIX.
- * For MSI-X, the vector number is encoded as a part of the address,
- * data is set to 0.
- * For MSI, the vector number is encoded in least bits in data.
+ * The vector number is encoded in least bits in data.
  */
 static void spapr_msi_write(void *opaque, hwaddr addr,
 uint64_t data, unsigned size)
-- 
2.11.0

Re: [Qemu-devel] [PATCH v2 07/11] migration: check global params for validity

[Peter Xu]

On Mon, Jul 17, 2017 at 03:25:05PM -0300, Eduardo Habkost wrote:
> On Mon, Jul 17, 2017 at 04:26:07PM +0800, Peter Xu wrote:
> > Adding validity check for the migration parameters passed in via global
> > properties.
> > 
> > Signed-off-by: Peter Xu 
> > ---
> >  migration/migration.c | 34 ++
> >  1 file changed, 34 insertions(+)
> > 
> > diff --git a/migration/migration.c b/migration/migration.c
> > index 8c65054..5a7f22c 100644
> > --- a/migration/migration.c
> > +++ b/migration/migration.c
> > @@ -2109,6 +2109,39 @@ static void migration_instance_init(Object *obj)
> >  ms->parameters.tls_hostname = g_strdup("");
> >  }
> >  
> > +static void migration_instance_post_init(Object *obj)
> > +{
> > +MigrationState *ms = (MigrationState *)obj;
> > +Error *err = NULL;
> > +MigrationParameters params = {
> > +.has_compress_level = true,
> > +.compress_level = ms->parameters.compress_level,
> > +.has_compress_threads = true,
> > +.compress_threads = ms->parameters.compress_threads,
> > +.has_decompress_threads = true,
> > +.decompress_threads = ms->parameters.decompress_threads,
> > +.has_cpu_throttle_initial = true,
> > +.cpu_throttle_initial = ms->parameters.cpu_throttle_initial,
> > +.has_cpu_throttle_increment = true,
> > +.cpu_throttle_increment = ms->parameters.cpu_throttle_increment,
> > +.has_max_bandwidth = true,
> > +.max_bandwidth = ms->parameters.max_bandwidth,
> > +.has_downtime_limit = true,
> > +.downtime_limit = ms->parameters.downtime_limit,
> > +.has_x_checkpoint_delay = true,
> > +.x_checkpoint_delay = ms->parameters.x_checkpoint_delay,
> > +.has_block_incremental = true,
> > +.block_incremental = ms->parameters.block_incremental,
> > +};
> > +
> > +/* We have applied all the migration properties... */
> > +
> > +if (!migrate_params_check(, )) {
> 
> Why not just:
>   if (!migrate_params_check(>parameters, ))
> ?
> 
> If the ms->parameters.has_* fields are not set to true anywhere,
> we can set them to true in instance_init.

Sure.

> 
> (This would also also us to use QAPI_CLONE in
> qmp_query_migrate_parameters() instead of manually copying each
> field.
> 
> 
> > +error_report_err(err);
> > +exit(1);
> > +}
> > +}
> 
> On real devices, this is normally done on realize (which has
> proper error reporting).  We never realize the TYPE_MIGRATION
> object because it's not a real device, though.

Hmm...

> 
> Doing error checks on post_init feels fragile, as the only way
> errors can be handled is triggering exit(1) in the middle of an
> object_new() call.
> 
> As we have only one place where the TYPE_MIGRATION object is
> created, I suggest calling migrate_params_check() inside
> migration_object_init().

... How about I just provide a realize() for it? Then I can drop the
QOM patch 4, also, I can keep the checks along with the object itself.

Thanks,

-- 
Peter Xu

Re: [Qemu-devel] [PATCH 3/8] target/alpha: Merge several flag bytes into ENV->FLAGS

[Emilio G. Cota]

On Thu, Jul 13, 2017 at 14:18:14 -1000, Richard Henderson wrote:
> The flags are arranged such that we can manipulate them either
> a whole, or as individual bytes.  The computation within
> cpu_get_tb_cpu_state is now reduced to a single load and mask.
> 
> Signed-off-by: Richard Henderson 
> ---
>  target/alpha/cpu.h   | 70 +
>  hw/alpha/dp264.c |  1 -
>  linux-user/main.c| 25 +++--
>  target/alpha/cpu.c   |  7 ++--
>  target/alpha/helper.c| 12 +++
>  target/alpha/machine.c   | 10 ++
>  target/alpha/translate.c | 91 
> +++-
>  7 files changed, 117 insertions(+), 99 deletions(-)
> 
> diff --git a/target/alpha/cpu.h b/target/alpha/cpu.h
> index aa83417..e95be2b 100644
> --- a/target/alpha/cpu.h
> +++ b/target/alpha/cpu.h
> @@ -242,13 +242,11 @@ struct CPUAlphaState {
>  uint8_t fpcr_dyn_round;
>  uint8_t fpcr_flush_to_zero;
>  
> -/* The Internal Processor Registers.  Some of these we assume always
> -   exist for use in user-mode.  */
> -uint8_t ps;
> -uint8_t intr_flag;
> -uint8_t pal_mode;
> -uint8_t fen;
> +/* Mask of PALmode, Processor State et al.  Most of this gets copied
> +   into the TranslatorBlock flags and controls code generation.  */
> +uint32_t flags;

Did you consider doing something like the appended? I don't like it
because it messes with endianness, which is always a pain. But it
lets you preserve the code that only touches the u8's as-is; this
comes at the price of requiring a fast-path swap in big-endian hosts.

An alternative would be to store the u8's in an order dependent on
the endianness of the host -- but that would break vmstate saving,
I guess.

Emilio

---8<---

diff --git a/target/alpha/cpu.h b/target/alpha/cpu.h
index aa83417..22c52ac 100644
--- a/target/alpha/cpu.h
+++ b/target/alpha/cpu.h
@@ -244,10 +244,15 @@ struct CPUAlphaState {
 
 /* The Internal Processor Registers.  Some of these we assume always
exist for use in user-mode.  */
-uint8_t ps;
-uint8_t intr_flag;
-uint8_t pal_mode;
-uint8_t fen;
+union {
+struct {
+uint8_t pal_mode;
+uint8_t ps;
+uint8_t intr_flag;
+uint8_t fen;
+};
+uint32_t flags;
+};
 
 uint32_t pcc_ofs;
 
@@ -397,15 +402,35 @@ enum {
 EXC_M_IOV = 64  /* Integer Overflow */
 };
 
-/* Processor status constants.  */
-enum {
-/* Low 3 bits are interrupt mask level.  */
-PS_INT_MASK = 7,
 
-/* Bits 4 and 5 are the mmu mode.  The VMS PALcode uses all 4 modes;
-   The Unix PALcode only uses bit 4.  */
-PS_USER_MODE = 8
-};
+/* Low 3 bits are interrupt mask level.  */
+#define PS_INT_MASK   7u
+/* Bits 4 and 5 are the mmu mode.  The VMS PALcode uses all 4 modes;
+   The Unix PALcode only uses bit 4.  */
+#define PS_USER_MODE  8u
+
+/* CPUAlphaState->flags constants.  These are layed out so that we
+   can set or reset the pieces individually by assigning to the byte,
+   or manipulated as a whole.  */
+
+#define ENV_FLAG_PAL_SHIFT0
+#define ENV_FLAG_PS_SHIFT 8
+#define ENV_FLAG_RX_SHIFT 16
+#define ENV_FLAG_FEN_SHIFT24
+
+#define ENV_FLAG_PAL_MODE (1u << ENV_FLAG_PAL_SHIFT)
+#define ENV_FLAG_PS_USER  (PS_USER_MODE << ENV_FLAG_PS_SHIFT)
+#define ENV_FLAG_RX_FLAG  (1u << ENV_FLAG_RX_SHIFT)
+#define ENV_FLAG_FEN  (1u << ENV_FLAG_FEN_SHIFT)
+
+#define ENV_FLAG_TB_MASK \
+(ENV_FLAG_PAL_MODE | ENV_FLAG_PS_USER | ENV_FLAG_FEN)
+
+#ifdef HOST_WORDS_BIGENDIAN
+#define ENV_FLAG_TB(flags) (bswap32(flags) & ENV_FLAG_TB_MASK)
+#else
+#define ENV_FLAG_TB(flags) ((flags) & ENV_FLAG_TB_MASK)
+#endif
 
 static inline int cpu_mmu_index(CPUAlphaState *env, bool ifetch)
 {
@@ -492,21 +517,10 @@ enum {
 static inline void cpu_get_tb_cpu_state(CPUAlphaState *env, target_ulong *pc,
 target_ulong *cs_base, uint32_t 
*pflags)
 {
-int flags = 0;
-
 *pc = env->pc;
 *cs_base = 0;
 
-if (env->pal_mode) {
-flags = TB_FLAGS_PAL_MODE;
-} else {
-flags = env->ps & PS_USER_MODE;
-}
-if (env->fen) {
-flags |= TB_FLAGS_FEN;
-}
-
-*pflags = flags;
+*pflags = ENV_FLAG_TB(env->flags);
 }
 
 #endif /* ALPHA_CPU_H */

[Qemu-devel] [PATCH 2/2] vfio/ccw: fix initialization of the Object DeviceState pointer in the common base-device

[Dong Jia Shi]

Commit 7da624e2 ("vfio: Test realized when using VFIOGroup.device_list
iterator") introduced a pointer to the Object DeviceState in the VFIO
common base-device and skiped non-realized devices as we iterate
VFIOGroup.device_list. While it missed to initialize the pointer for
the vfio-ccw case. Let's fix it.

Fixes: 7da624e2 ("vfio: Test realized when using VFIOGroup.device_list
  iterator")

Cc: Alex Williamson 
Reviewed-by: Halil Pasic 
Signed-off-by: Dong Jia Shi 
---
 hw/vfio/ccw.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/hw/vfio/ccw.c b/hw/vfio/ccw.c
index 8d97b53e77..a8baadf57a 100644
--- a/hw/vfio/ccw.c
+++ b/hw/vfio/ccw.c
@@ -338,6 +338,7 @@ static void vfio_ccw_realize(DeviceState *dev, Error **errp)
 vcdev->vdev.type = VFIO_DEVICE_TYPE_CCW;
 vcdev->vdev.name = g_strdup_printf("%x.%x.%04x", cdev->hostid.cssid,
cdev->hostid.ssid, cdev->hostid.devid);
+vcdev->vdev.dev = dev;
 QLIST_FOREACH(vbasedev, >device_list, next) {
 if (strcmp(vbasedev->name, vcdev->vdev.name) == 0) {
 error_setg(, "vfio: subchannel %s has already been attached",
-- 
2.11.2

[Qemu-devel] [PATCH 1/2] vfio/ccw: allocate irq info with the right size

[Dong Jia Shi]

From: Jing Zhang 

When allocating memory for the vfio_irq_info parameter of the
VFIO_DEVICE_GET_IRQ_INFO ioctl, we used the wrong size. Let's
fix it by using the right size.

Reviewed-by: Dong Jia Shi 
Signed-off-by: Jing Zhang 
Signed-off-by: Dong Jia Shi 
---
 hw/vfio/ccw.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/vfio/ccw.c b/hw/vfio/ccw.c
index 12d0262336..8d97b53e77 100644
--- a/hw/vfio/ccw.c
+++ b/hw/vfio/ccw.c
@@ -168,7 +168,7 @@ static void vfio_ccw_register_io_notifier(VFIOCCWDevice 
*vcdev, Error **errp)
 return;
 }
 
-argsz = sizeof(*irq_set);
+argsz = sizeof(*irq_info);
 irq_info = g_malloc0(argsz);
 irq_info->index = VFIO_CCW_IO_IRQ_INDEX;
 irq_info->argsz = argsz;
-- 
2.11.2

[Qemu-devel] [PATCH 0/2] vfio-ccw bugfixs

[Dong Jia Shi]

Dear Conny,

Here we got two bugfix patches for vfio-ccw:
- fix commit 7da624e2 which missed to initialize a new introduced
  pointer for the vfio-ccw case
- fix a memory allocation that used a wrong size for an irq info
  variable

Dong Jia Shi (1):
  vfio/ccw: fix initialization of the Object DeviceState pointer in the
common base-device

Jing Zhang (1):
  vfio/ccw: allocate irq info with the right size

 hw/vfio/ccw.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

-- 
2.11.2

Re: [Qemu-devel] [PULL 00/18] target-arm queue

[no-reply]

Hi,

This series seems to have some coding style problems. See output below for
more information:

Subject: [Qemu-devel] [PULL 00/18] target-arm queue
Message-id: 1500295494-8991-1-git-send-email-peter.mayd...@linaro.org
Type: series

=== TEST SCRIPT BEGIN ===
#!/bin/bash

BASE=base
n=1
total=$(git log --oneline $BASE.. | wc -l)
failed=0

git config --local diff.renamelimit 0
git config --local diff.renames True

commits="$(git log --format=%H --reverse $BASE..)"
for c in $commits; do
echo "Checking PATCH $n/$total: $(git log -n 1 --format=%s $c)..."
if ! git show $c --format=email | ./scripts/checkpatch.pl --mailback -; then
failed=1
echo
fi
n=$((n+1))
done

exit $failed
=== TEST SCRIPT END ===

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
From https://github.com/patchew-project/qemu
 - [tag update]  patchew/20170717110936.23314-1-dgilb...@redhat.com -> 
patchew/20170717110936.23314-1-dgilb...@redhat.com
Switched to a new branch 'test'
f4eface MAINTAINERS: Add entries for MPS2 board
f1fecfb hw/arm/mps2: Add ethernet
5cc8e71 hw/arm/mps2: Add SCC
5404d5a hw/misc/mps2_scc: Implement MPS2 Serial Communication Controller
3b85601 hw/arm/mps2: Add timers
8784008 hw/char/cmsdk-apb-timer: Implement CMSDK APB timer device
d7ae3d4 hw/arm/mps2: Add UARTs
41c0f2d hw/char/cmsdk-apb-uart.c: Implement CMSDK APB UART
c7eb8eb hw/arm/mps2: Implement skeleton mps2-an385 and mps2-an511 board models
bdd7af5 target/arm: use DISAS_EXIT for eret handling
ec55bc9 target/arm: use gen_goto_tb for ISB handling
dabdd10 target/arm/translate: ensure gen_goto_tb sets exit flags
759a7d2 target/arm/translate.h: expand comment on DISAS_EXIT
538a900 target/arm/translate: make DISAS_UPDATE match declared semantics
c4d68d9 include/exec/exec-all: document common exit conditions
2d96cb8 target/arm: Make Cortex-M3 and M4 default to 8 PMSA regions
0793ffd qdev: support properties which don't set a default value
90ea21c qdev-properties.h: Explicitly set the default value for arraylen 
properties

=== OUTPUT BEGIN ===
Checking PATCH 1/18: qdev-properties.h: Explicitly set the default value for 
arraylen properties...
Checking PATCH 2/18: qdev: support properties which don't set a default value...
Checking PATCH 3/18: target/arm: Make Cortex-M3 and M4 default to 8 PMSA 
regions...
Checking PATCH 4/18: include/exec/exec-all: document common exit conditions...
Checking PATCH 5/18: target/arm/translate: make DISAS_UPDATE match declared 
semantics...
Checking PATCH 6/18: target/arm/translate.h: expand comment on DISAS_EXIT...
Checking PATCH 7/18: target/arm/translate: ensure gen_goto_tb sets exit flags...
Checking PATCH 8/18: target/arm: use gen_goto_tb for ISB handling...
Checking PATCH 9/18: target/arm: use DISAS_EXIT for eret handling...
Checking PATCH 10/18: hw/arm/mps2: Implement skeleton mps2-an385 and mps2-an511 
board models...
ERROR: line over 90 characters
#77: FILE: hw/arm/mps2.c:22:
+ *   
https://developer.arm.com/products/system-design/development-boards/cortex-m-prototyping-system

total: 1 errors, 0 warnings, 281 lines checked

Your patch has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

Checking PATCH 11/18: hw/char/cmsdk-apb-uart.c: Implement CMSDK APB UART...
ERROR: line over 90 characters
#62: FILE: hw/char/cmsdk-apb-uart.c:15:
+ * 
https://developer.arm.com/products/system-design/system-design-kits/cortex-m-system-design-kit

total: 1 errors, 0 warnings, 508 lines checked

Your patch has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

Checking PATCH 12/18: hw/arm/mps2: Add UARTs...
Checking PATCH 13/18: hw/char/cmsdk-apb-timer: Implement CMSDK APB timer 
device...
ERROR: line over 90 characters
#57: FILE: hw/timer/cmsdk-apb-timer.c:15:
+ * 
https://developer.arm.com/products/system-design/system-design-kits/cortex-m-system-design-kit

total: 1 errors, 0 warnings, 331 lines checked

Your patch has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

Checking PATCH 14/18: hw/arm/mps2: Add timers...
Checking PATCH 15/18: hw/misc/mps2_scc: Implement MPS2 Serial Communication 
Controller...
ERROR: line over 90 characters
#70: FILE: hw/misc/mps2-scc.c:16:
+ * 
http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.100112_0100_03_en/index.html

ERROR: spaces required around that '*' (ctx:WxV)
#105: FILE: hw/misc/mps2-scc.c:51:
+static bool scc_cfg_write(MPS2SCC *s, unsigned function,
   ^

total: 2 errors, 0 warnings, 379 lines checked

Your patch has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

Checking PATCH 16/18: hw/arm/mps2: Add SCC...
Checking PATCH 17/18: hw/arm/mps2: Add ethernet...
Checking PATCH 

Re: [Qemu-devel] [PATCH v2 03/11] migration: export capabilities to props

[Peter Xu]

On Mon, Jul 17, 2017 at 06:58:44PM +0200, Juan Quintela wrote:
> Peter Xu  wrote:
> > Do the same thing to migration capabilities, just like what we did in
> > previous patch for migration parameters.
> >
> > Signed-off-by: Peter Xu 
> 
> Reviewed-by: Juan Quintela 

Thanks!

> 
> 
> A pitty that the preprocessor is not able to pass to uppercase ...
> 
> > +#define DEFINE_PROP_MIG_CAP(name, x) \
> > +DEFINE_PROP_BOOL(name, MigrationState, enabled_capabilities[x], false)
> 
> #define DEFINE_PROP_MIG_CAP(name) \
> DEFINE_PROP_BOOL(#name, MigrationState,
> enabled_capabilities[MIGRATION_CAPABILITY_##name], false)

Yes a pity, this is more beautiful (though this may let the grepping
of specific MIGRATION_CAPABILITY_* slightly harder since it breaks the
macro).

-- 
Peter Xu

Re: [Qemu-devel] [PATCH v2 01/11] qdev: provide DEFINE_PROP_INT64()

[Peter Xu]

On Mon, Jul 17, 2017 at 11:30:47AM -0500, Eric Blake wrote:
> On 07/17/2017 03:26 AM, Peter Xu wrote:
> > We have merely all the stuff, but this one is missing. Add it in.
> 
> s/merely/nearly/

Will fix this without removing r-bs.  Thanks,

> 
> > 
> > Am going to use this new helper for MigrationParameters fields, since
> > most of them are int64_t.
> > 
> > CC: Markus Armbruster 
> > CC: Eduardo Habkost 
> > CC: Marc-André Lureau 
> > CC: Peter Xu 
> > CC: Juan Quintela 
> > CC: Marcel Apfelbaum 
> > Reviewed-by: Marc-André Lureau 
> > Signed-off-by: Peter Xu 
> > ---
> >  hw/core/qdev-properties.c| 32 
> >  include/hw/qdev-properties.h |  3 +++
> >  2 files changed, 35 insertions(+)
> > 
> 
> -- 
> Eric Blake, Principal Software Engineer
> Red Hat, Inc.   +1-919-301-3266
> Virtualization:  qemu.org | libvirt.org
> 




-- 
Peter Xu

Re: [Qemu-devel] [PATCH v4 4/6] migration/rdma: Allow cancelling while waiting for wrid

[Peter Xu]

On Mon, Jul 17, 2017 at 12:09:34PM +0100, Dr. David Alan Gilbert (git) wrote:
> From: "Dr. David Alan Gilbert" 
> 
> When waiting for a WRID, if the other side dies we end up waiting
> for ever with no way to cancel the migration.
> Cure this by poll()ing the fd first with a timeout and checking
> error flags and migration state.
> 
> Signed-off-by: Dr. David Alan Gilbert 

Reviewed-by: Peter Xu 

-- 
Peter Xu

Re: [Qemu-devel] [PATCH v4 3/6] migration/rdma: fix qemu_rdma_block_for_wrid error paths

[Peter Xu]

On Mon, Jul 17, 2017 at 12:09:33PM +0100, Dr. David Alan Gilbert (git) wrote:
> From: "Dr. David Alan Gilbert" 
> 
> The two places that 'goto err_block_for_wrid' weren't setting ret
> and so would end up returning 0 even though we've failed.
> 
> Signed-off-by: Dr. David Alan Gilbert 
> ---
>  migration/rdma.c | 8 ++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/migration/rdma.c b/migration/rdma.c
> index 6111e10c70..59810aec2e 100644
> --- a/migration/rdma.c
> +++ b/migration/rdma.c
> @@ -1521,14 +1521,16 @@ static int qemu_rdma_block_for_wrid(RDMAContext 
> *rdma, int wrid_requested,
>  yield_until_fd_readable(rdma->comp_channel->fd);
>  }
>  
> -if (ibv_get_cq_event(rdma->comp_channel, , _ctx)) {
> +ret = ibv_get_cq_event(rdma->comp_channel, , _ctx);
> +if (ret) {
>  perror("ibv_get_cq_event");
>  goto err_block_for_wrid;
>  }
>  
>  num_cq_events++;
>  
> -if (ibv_req_notify_cq(cq, 0)) {
> +ret = -ibv_req_notify_cq(cq, 0);

(I didn't really notice that it is returning a positive value for
 error...)

Reviewed-by: Peter Xu 

> +if (ret) {
>  goto err_block_for_wrid;
>  }
>  
> @@ -1564,6 +1566,8 @@ err_block_for_wrid:
>  if (num_cq_events) {
>  ibv_ack_cq_events(cq, num_cq_events);
>  }
> +
> +rdma->error_state = ret;
>  return ret;
>  }
>  
> -- 
> 2.13.0
> 

-- 
Peter Xu

Re: [Qemu-devel] [PATCH v2 03/11] migration: export capabilities to props

[Peter Xu]

On Mon, Jul 17, 2017 at 02:52:31PM -0300, Eduardo Habkost wrote:
> On Mon, Jul 17, 2017 at 04:26:03PM +0800, Peter Xu wrote:
> > Do the same thing to migration capabilities, just like what we did in
> > previous patch for migration parameters.
> > 
> > Signed-off-by: Peter Xu 
> > ---
> >  migration/migration.c | 17 +
> >  1 file changed, 17 insertions(+)
> > 
> > diff --git a/migration/migration.c b/migration/migration.c
> > index ad2505c..3208162 100644
> > --- a/migration/migration.c
> > +++ b/migration/migration.c
> > @@ -2001,6 +2001,9 @@ void migration_global_dump(Monitor *mon)
> > ms->send_configuration, ms->send_section_footer);
> >  }
> >  
> > +#define DEFINE_PROP_MIG_CAP(name, x) \
> > +DEFINE_PROP_BOOL(name, MigrationState, enabled_capabilities[x], false)
> > +
> 
> Maybe for the future: have you considered replacing the
> enabled_capabilities array with a uint32_t and using
> DEFINE_PROP_BIT?

Yes, this sounds reasonable.  Noted.

> 
> Reviewed-by: Eduardo Habkost 

Thanks!

-- 
Peter Xu

Re: [Qemu-devel] [PATCH v5 0/2] qemu-img: Check for backing image if specified during create

[John Snow]

Kevin: I took a stab at this 'feature', but if there are any fixups or
changes that need to occur and it's important that it happens before I'm
awake, please feel free to steal these patches and do whatever you need
to them, including setting them on fire.

Thanks,
--John

post-script: I think the only thing that I don't really do here is
attempt to force-open an image to see if it exists if a size is already
provided in order to quiet errors related to locking.

That change would just eliminate a little bit of "this image is locked!"
whining in the case that -u was omitted but a size was provided, which
is mostly QOL.

On 07/17/2017 08:34 PM, John Snow wrote:
> We do not currently guarantee that QEMU will or will not open a
> backing file when creating a new overlay file. Presently, QEMU will
> not open that file if you provide a filesize, because it has no reason
> to want to open it in that case.
> 
> This series makes the contract more explicit: if '-u' is provided to
> create, we will not open the backing image regardless, erroring out if
> a size was not provided.
> 
> In the other case, if '-u' is not provided, we now endeavor to open the
> backing image if possible to check that it exists. For now, if a size
> is provided and the image does not exist, QEMU will only warn to maintain
> compatibility with legacy behavior.
> 
> In the future, QEMU may treat the operation as a failure if '-u' was not
> provided.
> 
> Tests are amended primarily to pass the '-u' flag where it makes sense;
> which is when creating overlays for objects already open by QEMU. These
> will now generally fail to succeed because of image locking. In this
> case, they only warn instead of fail, but this keeps the output cleaner.
> 
> Test 111 is updated to accommodate a new error message.
> 082, 085, 139, 156 and 158 add '-u' just to suppress warnings.
> 
> John Snow (2):
>   blockdev: move BDRV_O_NO_BACKING option forward
>   qemu-img: Check for backing image if specified during create
> 
>  block.c| 96 
> +-
>  blockdev.c | 11 +++---
>  qemu-img-cmds.hx   |  4 +-
>  qemu-img.c | 16 +---
>  tests/qemu-iotests/082 |  4 +-
>  tests/qemu-iotests/082.out |  4 +-
>  tests/qemu-iotests/085 |  2 +-
>  tests/qemu-iotests/111.out |  1 +
>  tests/qemu-iotests/139 |  2 +-
>  tests/qemu-iotests/156 |  2 +-
>  tests/qemu-iotests/158 |  2 +-
>  11 files changed, 81 insertions(+), 63 deletions(-)
> 

Re: [Qemu-devel] [PATCH v2 21/45] tcg: check CF_PARALLEL instead of parallel_cpus

[Emilio G. Cota]

On Mon, Jul 17, 2017 at 13:55:42 -1000, Richard Henderson wrote:
> On 07/16/2017 10:04 AM, Emilio G. Cota wrote:
> >Thereby decoupling the resulting translated code from the current state
> >of the system.
> >
> >The tb->cflags field is not passed to tcg generation functions. So
> >we add a bit to TCGContext, storing there whether CF_PARALLEL is set
> >before translating every TB.
> >
> >Most architectures have <= 32 registers, which results in a 4-byte hole
> >in TCGContext. Use this hole for the bit we need; use a uint8_t instead
> >of a bool, since a bool might take more than one byte in some systems.
> 
> I would much rather use bool.
> 
> (1) I don't care about OSX and its broken ABI,
> (2) Even then OSX still *works*.

Will do.

> Otherwise,

Missing R-b tag?

E.

[Qemu-devel] [PATCH] docker: warn users to use newer debian8/debian9 base image

[Philippe Mathieu-Daudé]

to stay backward incompatible.

Signed-off-by: Philippe Mathieu-Daudé 
---
 tests/docker/Makefile.include  |  1 +
 tests/docker/dockerfiles/debian.docker | 13 +
 2 files changed, 14 insertions(+)
 create mode 100644 tests/docker/dockerfiles/debian.docker

diff --git a/tests/docker/Makefile.include b/tests/docker/Makefile.include
index e993e149e7..aaab1a4208 100644
--- a/tests/docker/Makefile.include
+++ b/tests/docker/Makefile.include
@@ -58,6 +58,7 @@ docker-image-%: $(DOCKER_FILES_DIR)/%.docker
 docker-image-debian-powerpc-cross: 
EXTRA_FILES:=tests/docker/dockerfiles/debian-apt-fake.sh
 
 # Enforce dependancies for composite images
+docker-image-debian: docker-image-debian9
 docker-image-debian8-mxe: docker-image-debian8
 docker-image-debian-amd64: docker-image-debian9
 docker-image-debian-armel-cross: docker-image-debian9
diff --git a/tests/docker/dockerfiles/debian.docker 
b/tests/docker/dockerfiles/debian.docker
new file mode 100644
index 00..fd32e71b79
--- /dev/null
+++ b/tests/docker/dockerfiles/debian.docker
@@ -0,0 +1,13 @@
+# This template is deprecated and was previously based on Jessie on QEMU 2.9.
+# Now than Stretch is out, please use qemu:debian8 as base for Jessie,
+# and qemu:debian9 for Stretch.
+#
+FROM qemu:debian9
+
+MAINTAINER Philippe Mathieu-Daudé 
+
+RUN for n in $(seq 8); do echo; done && \
+echo "\n\t\tThis image is deprecated." && echo && \
+echo "\tUse 'FROM qemu:debian9' to use the stable Debian Stretch image" && 
\
+echo "\tor 'FROM qemu:debian8' to use old Debian Jessie." && \
+for n in $(seq 8); do echo; done
-- 
2.13.2

[Qemu-devel] [PATCH v5 2/2] qemu-img: Check for backing image if specified during create

[John Snow]

Or, rather, force the open of a backing image if one was specified
for creation. Using a similar -unsafe option as rebase, allow qemu-img
to ignore the backing file validation if possible.

It may not always be possible, as in the existing case when a filesize
for the new image was not specified.

This is accomplished by shifting around the conditionals in
bdrv_img_create, such that a backing file is always opened unless we
provide BDRV_O_NO_BACKING. qemu-img is adjusted to pass this new flag
when -u is provided to create.

Sorry for the heinous looking diffstat, but it's mostly whitespace.

Inspired by: https://bugzilla.redhat.com/show_bug.cgi?id=1213786

Reviewed-by: Eric Blake 
Signed-off-by: John Snow 
---
 block.c| 96 +-
 qemu-img-cmds.hx   |  4 +-
 qemu-img.c | 16 +---
 tests/qemu-iotests/082 |  4 +-
 tests/qemu-iotests/082.out |  4 +-
 tests/qemu-iotests/085 |  2 +-
 tests/qemu-iotests/111.out |  1 +
 tests/qemu-iotests/139 |  2 +-
 tests/qemu-iotests/156 |  2 +-
 tests/qemu-iotests/158 |  2 +-
 10 files changed, 75 insertions(+), 58 deletions(-)

diff --git a/block.c b/block.c
index 98a9209..17d0d4a 100644
--- a/block.c
+++ b/block.c
@@ -4396,55 +4396,65 @@ void bdrv_img_create(const char *filename, const char 
*fmt,
 
 backing_fmt = qemu_opt_get(opts, BLOCK_OPT_BACKING_FMT);
 
-// The size for the image must always be specified, with one exception:
-// If we are using a backing file, we can obtain the size from there
+/* The size for the image must always be specified, unless we have a 
backing
+ * file and we have not been forbidden from opening it */
 size = qemu_opt_get_size(opts, BLOCK_OPT_SIZE, 0);
-if (size == -1) {
-if (backing_file) {
-BlockDriverState *bs;
-char *full_backing = g_new0(char, PATH_MAX);
-int64_t size;
-int back_flags;
-QDict *backing_options = NULL;
-
-bdrv_get_full_backing_filename_from_filename(filename, 
backing_file,
- full_backing, 
PATH_MAX,
- _err);
-if (local_err) {
-g_free(full_backing);
-goto out;
-}
-
-/* backing files always opened read-only */
-back_flags = flags;
-back_flags &= ~(BDRV_O_RDWR | BDRV_O_SNAPSHOT | BDRV_O_NO_BACKING);
-
-if (backing_fmt) {
-backing_options = qdict_new();
-qdict_put_str(backing_options, "driver", backing_fmt);
-}
-
-bs = bdrv_open(full_backing, NULL, backing_options, back_flags,
-   _err);
+if (backing_file && !(flags & BDRV_O_NO_BACKING)) {
+BlockDriverState *bs;
+char *full_backing = g_new0(char, PATH_MAX);
+int back_flags;
+QDict *backing_options = NULL;
+
+bdrv_get_full_backing_filename_from_filename(filename, backing_file,
+ full_backing, PATH_MAX,
+ _err);
+if (local_err) {
 g_free(full_backing);
-if (!bs) {
-goto out;
-}
-size = bdrv_getlength(bs);
-if (size < 0) {
-error_setg_errno(errp, -size, "Could not get size of '%s'",
- backing_file);
-bdrv_unref(bs);
-goto out;
-}
+goto out;
+}
 
-qemu_opt_set_number(opts, BLOCK_OPT_SIZE, size, _abort);
+/* backing files always opened read-only */
+back_flags = flags;
+back_flags &= ~(BDRV_O_RDWR | BDRV_O_SNAPSHOT | BDRV_O_NO_BACKING);
 
+if (backing_fmt) {
+backing_options = qdict_new();
+qdict_put_str(backing_options, "driver", backing_fmt);
+}
+
+bs = bdrv_open(full_backing, NULL, backing_options, back_flags,
+   _err);
+g_free(full_backing);
+if (!bs && size != -1) {
+/* Couldn't open BS, but we have a size, so it's nonfatal */
+error_reportf_err(local_err,
+  "Warning: could not verify backing image. "
+  "This may become an error in future 
versions.\n");
+local_err = NULL;
+} else if (!bs) {
+/* Couldn't open bs, do not have size */
+error_append_hint(_err,
+  "Could not open backing image to determine 
size.\n");
+goto out;
+} else {
+if (size == -1) {
+/* Opened BS, have no size */
+size = bdrv_getlength(bs);
+if (size < 0) {
+

[Qemu-devel] [PATCH v5 0/2] qemu-img: Check for backing image if specified during create

[John Snow]

We do not currently guarantee that QEMU will or will not open a
backing file when creating a new overlay file. Presently, QEMU will
not open that file if you provide a filesize, because it has no reason
to want to open it in that case.

This series makes the contract more explicit: if '-u' is provided to
create, we will not open the backing image regardless, erroring out if
a size was not provided.

In the other case, if '-u' is not provided, we now endeavor to open the
backing image if possible to check that it exists. For now, if a size
is provided and the image does not exist, QEMU will only warn to maintain
compatibility with legacy behavior.

In the future, QEMU may treat the operation as a failure if '-u' was not
provided.

Tests are amended primarily to pass the '-u' flag where it makes sense;
which is when creating overlays for objects already open by QEMU. These
will now generally fail to succeed because of image locking. In this
case, they only warn instead of fail, but this keeps the output cleaner.

Test 111 is updated to accommodate a new error message.
082, 085, 139, 156 and 158 add '-u' just to suppress warnings.

John Snow (2):
  blockdev: move BDRV_O_NO_BACKING option forward
  qemu-img: Check for backing image if specified during create

 block.c| 96 +-
 blockdev.c | 11 +++---
 qemu-img-cmds.hx   |  4 +-
 qemu-img.c | 16 +---
 tests/qemu-iotests/082 |  4 +-
 tests/qemu-iotests/082.out |  4 +-
 tests/qemu-iotests/085 |  2 +-
 tests/qemu-iotests/111.out |  1 +
 tests/qemu-iotests/139 |  2 +-
 tests/qemu-iotests/156 |  2 +-
 tests/qemu-iotests/158 |  2 +-
 11 files changed, 81 insertions(+), 63 deletions(-)

-- 
2.9.4

[Qemu-devel] [PATCH v5 1/2] blockdev: move BDRV_O_NO_BACKING option forward

[John Snow]

For both external_snapshot_prepare and qmp_drive_mirror, we eventually
append the option BDRV_O_NO_BACKING. However, we generally do so after
we create the image.

To accommodate image creation wanting to verify that a backing file
exists or not, add this option prior to create to override checking
the existence of the backing file. This prevents QEMU from trying to
re-open a backing file that's already in use (thanks to qcow2 locking).

Signed-off-by: John Snow 
---
 blockdev.c | 11 ++-
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/blockdev.c b/blockdev.c
index 7f53cc8..6469f16 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -1710,7 +1710,8 @@ static void external_snapshot_prepare(BlkActionState 
*common,
 }
 
 flags = state->old_bs->open_flags;
-flags &= ~(BDRV_O_SNAPSHOT | BDRV_O_NO_BACKING | BDRV_O_COPY_ON_READ);
+flags &= ~(BDRV_O_SNAPSHOT | BDRV_O_COPY_ON_READ);
+flags |= BDRV_O_NO_BACKING;
 
 /* create new image w/backing file */
 mode = s->has_mode ? s->mode : NEW_IMAGE_MODE_ABSOLUTE_PATHS;
@@ -1735,8 +1736,6 @@ static void external_snapshot_prepare(BlkActionState 
*common,
 qdict_put_str(options, "node-name", snapshot_node_name);
 }
 qdict_put_str(options, "driver", format);
-
-flags |= BDRV_O_NO_BACKING;
 }
 
 state->new_bs = bdrv_open(new_image_file, snapshot_ref, options, flags,
@@ -3548,6 +3547,9 @@ void qmp_drive_mirror(DriveMirror *arg, Error **errp)
 backing_mode = MIRROR_OPEN_BACKING_CHAIN;
 }
 
+/* Don't open backing image in create() */
+flags |= BDRV_O_NO_BACKING;
+
 if ((arg->sync == MIRROR_SYNC_MODE_FULL || !source)
 && arg->mode != NEW_IMAGE_MODE_EXISTING)
 {
@@ -3587,8 +3589,7 @@ void qmp_drive_mirror(DriveMirror *arg, Error **errp)
 /* Mirroring takes care of copy-on-write using the source's backing
  * file.
  */
-target_bs = bdrv_open(arg->target, NULL, options,
-  flags | BDRV_O_NO_BACKING, errp);
+target_bs = bdrv_open(arg->target, NULL, options, flags, errp);
 if (!target_bs) {
 goto out;
 }
-- 
2.9.4

Re: [Qemu-devel] Disable image locking for snapshot drive?

[John Snow]

On 07/17/2017 07:30 PM, Andrew Baumann via Qemu-devel wrote:
> Hi all,
> 
> I'm running a recent Linux build of qemu on Windows Subsystem for Linux (WSL) 
> which doesn't appear to implement file locking:
> 
> $ qemu-system-aarch64 ... -drive file=test.vhdx,if=none,id=hd0 -device 
> virtio-blk-pci,drive=hd0
> qemu-system-aarch64: -drive file=test.vhdx,if=none,id=hd0: Failed to unlock 
> byte 100
> qemu-system-aarch64: -drive file=test.vhdx,if=none,id=hd0: Failed to unlock 
> byte 100
> qemu-system-aarch64: -drive file=test.vhdx,if=none,id=hd0: Failed to lock 
> byte 100
> 
> That's no big deal; I can switch it off:
> 
> $ qemu-system-aarch64 ... -drive 
> file=test.vhdx,if=none,file.locking=off,id=hd0 ...
> (all good)
> 
> But how can I do the same for a snapshot drive?
> 
> $ qemu-system-aarch64 ... -drive 
> file=test.vhdx,if=none,file.locking=off,id=hd0 -snapshot ...
> qemu-system-aarch64: -drive file=test.vhdx,if=none,file.locking=off,id=hd0: 
> Failed to unlock byte 100
> qemu-system-aarch64: -drive file=test.vhdx,if=none,file.locking=off,id=hd0: 
> Failed to unlock byte 100
> qemu-system-aarch64: -drive file=test.vhdx,if=none,file.locking=off,id=hd0: 
> Could not create temporary overlay '/var/tmp/vl.o83dxn': Failed to lock byte 
> 100
> 
> (I also tried the snapshot=on drive option with similar results.)
> 
> Thanks,
> Andrew
> 

Looks like the shorthand "-snapshot" doesn't let you specify any further
options, which is a bummer.

You may need to do something a little more manual, and create your own
temporary overlay, and launch QEMU pointing to that overlay instead.

That sounds like a bit of a hassle.

Can we compile locking support out of QEMU instead for this platform? Or
is there a runtime option for disabling it globally?

[Qemu-devel] [PATCH v2 27/31] docker: add debian/mips64el image

[Philippe Mathieu-Daudé]

Signed-off-by: Philippe Mathieu-Daudé 
---
 tests/docker/Makefile.include  |  1 +
 .../dockerfiles/debian-mips64el-cross.docker   | 30 ++
 2 files changed, 31 insertions(+)
 create mode 100644 tests/docker/dockerfiles/debian-mips64el-cross.docker

diff --git a/tests/docker/Makefile.include b/tests/docker/Makefile.include
index e0807c0917..5a8283674a 100644
--- a/tests/docker/Makefile.include
+++ b/tests/docker/Makefile.include
@@ -63,6 +63,7 @@ docker-image-debian-armel-cross: docker-image-debian9
 docker-image-debian-armhf-cross: docker-image-debian9
 docker-image-debian-arm64-cross: docker-image-debian9
 docker-image-debian-mips-cross: docker-image-debian9
+docker-image-debian-mips64el-cross: docker-image-debian9
 docker-image-debian-powerpc-cross: docker-image-debian8
 docker-image-debian-ppc64el-cross: docker-image-debian9
 docker-image-debian-s390x-cross: docker-image-debian9
diff --git a/tests/docker/dockerfiles/debian-mips64el-cross.docker 
b/tests/docker/dockerfiles/debian-mips64el-cross.docker
new file mode 100644
index 00..fd2aafeb01
--- /dev/null
+++ b/tests/docker/dockerfiles/debian-mips64el-cross.docker
@@ -0,0 +1,30 @@
+#
+# Docker mips64el cross-compiler target
+#
+# This docker target builds on the debian Stretch base image.
+#
+
+FROM qemu:debian9
+
+MAINTAINER Philippe Mathieu-Daudé 
+
+# Add the foreign architecture we want and install dependencies
+RUN dpkg --add-architecture mips64el && \
+apt-get update
+RUN DEBIAN_FRONTEND=noninteractive eatmydata \
+apt-get install -y --no-install-recommends \
+gcc-mips64el-linux-gnuabi64
+
+RUN DEBIAN_FRONTEND=noninteractive eatmydata \
+apt-get build-dep -yy -a mips64el qemu
+
+# Specify the cross prefix for this image (see tests/docker/common.rc)
+ENV QEMU_CONFIGURE_OPTS --cross-prefix=mips64el-linux-gnuabi64-
+
+# Install extra libraries to increase code coverage
+RUN DEBIAN_FRONTEND=noninteractive eatmydata \
+apt-get install -y --no-install-recommends \
+libbz2-dev:mips64el \
+liblzo2-dev:mips64el \
+librdmacm-dev:mips64el \
+libsnappy-dev:mips64el
-- 
2.13.2

[Qemu-devel] [PATCH v2 31/31] docker: add debian Ports base image

[Philippe Mathieu-Daudé]

Signed-off-by: Philippe Mathieu-Daudé 
---
 tests/docker/dockerfiles/debian-ports.docker | 34 
 1 file changed, 34 insertions(+)
 create mode 100644 tests/docker/dockerfiles/debian-ports.docker

diff --git a/tests/docker/dockerfiles/debian-ports.docker 
b/tests/docker/dockerfiles/debian-ports.docker
new file mode 100644
index 00..907ebdef62
--- /dev/null
+++ b/tests/docker/dockerfiles/debian-ports.docker
@@ -0,0 +1,34 @@
+#
+# Docker multiarch cross-compiler target
+#
+# This docker target is builds on Debian Ports cross compiler targets
+# to build distro with a selection of cross compilers for building test 
binaries.
+#
+# On its own you can't build much but the docker-foo-cross targets
+# build on top of the base debian image.
+#
+FROM debian:unstable
+
+MAINTAINER Philippe Mathieu-Daudé 
+
+RUN echo "deb [arch=amd64] http://deb.debian.org/debian unstable main" > 
/etc/apt/sources.list
+
+# Duplicate deb line as deb-src
+RUN cat /etc/apt/sources.list | sed -ne "s/^deb\ \(\[.*\]\ \)\?\(.*\)/deb-src 
\2/p" >> /etc/apt/sources.list
+
+# Setup some basic tools we need
+RUN apt-get update && \
+DEBIAN_FRONTEND=noninteractive apt-get install -yy eatmydata
+RUN DEBIAN_FRONTEND=noninteractive eatmydata \
+apt-get install -y --no-install-recommends \
+bison \
+build-essential \
+ca-certificates \
+debian-ports-archive-keyring \
+flex \
+git \
+pkg-config \
+psmisc \
+python \
+texinfo \
+$(apt-get -s build-dep qemu | egrep ^Inst | fgrep '[all]' | cut -d\  
-f2)
-- 
2.13.2

[Qemu-devel] [PATCH v2 26/31] shippable: use debian/mips[eb] targets

[Philippe Mathieu-Daudé]

previous commit change image mips little -> big endian

Signed-off-by: Philippe Mathieu-Daudé 
---
 .shippable.yml | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/.shippable.yml b/.shippable.yml
index fa54df6cff..f2a904014a 100644
--- a/.shippable.yml
+++ b/.shippable.yml
@@ -16,8 +16,8 @@ env:
 - IMAGE=debian-s390x-cross
   TARGET_LIST=s390x-softmmu,s390x-linux-user
 # mips64el-softmmu disabled due to libfdt problem
-- IMAGE=debian-mipsel-cross
-  TARGET_LIST=mipsel-softmmu,mipsel-linux-user,mips64el-linux-user
+- IMAGE=debian-mips-cross
+  TARGET_LIST=mips-softmmu,mipsel-linux-user
 - IMAGE=debian-powerpc-cross
   TARGET_LIST=ppc-softmmu,ppcemb-softmmu,ppc-linux-user
 - IMAGE=debian-ppc64el-cross
-- 
2.13.2

[Qemu-devel] [PATCH v2 30/31] shippable: add win32/64 targets

[Philippe Mathieu-Daudé]

Signed-off-by: Philippe Mathieu-Daudé 
---
 .shippable.yml | 4 
 1 file changed, 4 insertions(+)

diff --git a/.shippable.yml b/.shippable.yml
index 53b43b349f..dd4bbc84b1 100644
--- a/.shippable.yml
+++ b/.shippable.yml
@@ -7,6 +7,10 @@ env:
   matrix:
 - IMAGE=debian-amd64
   TARGET_LIST=x86_64-softmmu,x86_64-linux-user
+- IMAGE=debian-win32-cross
+  TARGET_LIST=arm-softmmu,i386-softmmu,lm32-softmmu
+- IMAGE=debian-win64-cross
+  TARGET_LIST=aarch64-softmmu,sparc64-softmmu,x86_64-softmmu
 - IMAGE=debian-armel-cross
   TARGET_LIST=arm-softmmu,arm-linux-user,armeb-linux-user
 - IMAGE=debian-armhf-cross
-- 
2.13.2

[Qemu-devel] [PATCH v2 20/31] util/cacheinfo: add missing include

[Philippe Mathieu-Daudé]

This include was forgotten when splitting cacheinfo.c out of
tcg/ppc/tcg-target.inc.c (see commit b255b2c8).

While compiling on powerpc:

  CC  util/cacheinfo.o
qemu/util/cacheinfo.c: In function 'arch_cache_info':
qemu/util/cacheinfo.c:137:33: error: 'AT_ICACHEBSIZE' undeclared (first use 
in this function)
 *isize = qemu_getauxval(AT_ICACHEBSIZE);
 ^
qemu/util/cacheinfo.c:137:33: note: each undeclared identifier is reported 
only once for each function it appears in
qemu/util/cacheinfo.c:140:33: error: 'AT_DCACHEBSIZE' undeclared (first use 
in this function)
 *dsize = qemu_getauxval(AT_DCACHEBSIZE);
 ^
qemu/rules.mak:66: recipe for target 'util/cacheinfo.o' failed
make: *** [util/cacheinfo.o] Error 1

Signed-off-by: Philippe Mathieu-Daudé 
---
 util/cacheinfo.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/util/cacheinfo.c b/util/cacheinfo.c
index 6253049533..593940f27b 100644
--- a/util/cacheinfo.c
+++ b/util/cacheinfo.c
@@ -129,6 +129,7 @@ static void arch_cache_info(int *isize, int *dsize)
 }
 
 #elif defined(_ARCH_PPC) && defined(__linux__)
+# include "elf.h"
 
 static void arch_cache_info(int *isize, int *dsize)
 {
-- 
2.13.2